video: deferred io fixes and sh_mobile_lcdcfb support

PATCH In review Magnus Damm 3 participants 14 messages Dec 19, 2008 – Dec 25, 2008

Subsystems (2)

Framebuffer layer SuperH

lore

Share & tools

lore

Magnus Damm

author

2008-12-19 06:33 UTC #0 lore raw

cover video: deferred io fixes and sh_mobile_lcdcfb support

video: deferred io fixes and sh_mobile_lcdcfb support

[PATCH 01/05] video: fix deferred io fsync()
[PATCH 02/05] video: deferred io cleanup
[PATCH 03/05] video: deferred io with physically contiguous memory [PATCH 04/05] video: sh_mobile_lcdcfb deferred io support [PATCH 05/05] sh: enable deferred io LCDC on Migo-R

This patchset adds deferred io support to sh_mobile_lcdcfb.

The LCDC hardware block managed by the sh_mobile_lcdcfb driver supports RGB or SYS panel configurations. SYS panels come with an external display controller that is resposible for refreshing the actual LCD panel. RGB panels are controlled directly by the LCDC and they need to be refreshed by the LCDC hardware.

In the case of SYS panels we can save some power by configuring the LCDC hardware block in one-shot mode. In this one-shot mode panel refresh is managed by software. This works well together with deferred io since it allows us to stop clocks for most of the time and only enable clocks when we actually want to trigger an update. When there is no fbdev activity the clocks are kept stopped which allows us to deep sleep.

May I suggest merging the first 3 patches with other framebuffer changes, but handling patch 4 and 5 with other SuperH changes? That strategy should be fine since there are no compile time dependencies. To keep things running please apply patch number 5 when all other changes are in.

Signed-off-by Magnus Damm <[email protected]>

---

 arch/sh/boards/mach-migor/setup.c |    2 
 drivers/video/Kconfig             |    1 
 drivers/video/fb_defio.c          |   27 ++++-
 drivers/video/sh_mobile_lcdcfb.c  |  170 ++++++++++++++++++++++++++++++++-----
 include/video/sh_mobile_lcdc.h    |    1 
 5 files changed, 174 insertions(+), 27 deletions(-)

Magnus Damm

author

2008-12-19 06:34 UTC #1 lore raw

PATCH 1/5 video: fix deferred io fsync()

From: Magnus Damm <[email protected]>

If CONFIG_FB_DEFERRED_IO is set, but there are framebuffers registered that does not make use of deferred io, then fsync() on those framebuffers will result in a crash. Fix that.

This is needed for sh_mobile_lcdcfb since we always enable deferred io at compile time but we may disable deferred io for some types of hardware configurations.

Signed-off-by Magnus Damm <[email protected]>

---

 drivers/video/fb_defio.c |    4 ++++
 1 file changed, 4 insertions(+)

drivers/video/fb_defio.c +4 -0

@@ -60,6 +60,10 @@ int fb_deferred_io_fsync(struct file *fi
 {
 	struct fb_info *info = file->private_data;
 
+	/* Skip if deferred io is complied-in but disabled on this fbdev */
+	if (!info->fbdefio)
+		return 0;
+
 	/* Kill off the delayed work */
 	cancel_rearming_delayed_work(&info->deferred_work);

Jaya Kumar

Acked-by on v1 1/5

2008-12-24 05:05 UTC #2 lore raw

Re: [Linux-fbdev-devel] [PATCH 01/05] video: fix deferred io fsync()

Quoted 11 lines · Magnus Damm

From: Magnus Damm <[email protected]>
If CONFIG_FB_DEFERRED_IO is set, but there are framebuffers registered that does not make use of deferred io, then fsync() on those framebuffers will result in a crash. Fix that.
This is needed for sh_mobile_lcdcfb since we always enable deferred io at compile time but we may disable deferred io for some types of hardware configurations.
Signed-off-by Magnus Damm <[email protected]>

Acked-by Jaya Kumar <[email protected]>

Magnus Damm

author

2008-12-19 06:34 UTC #3 lore raw

PATCH 2/5 video: deferred io cleanup

In reply to Magnus Damm

From: Magnus Damm <[email protected]>

Make sure the mmap callback is set to NULL in the deferred io cleanup function. This way we can enable and disable deferred io on the fly.

Signed-off-by Magnus Damm <[email protected]>

---

 drivers/video/fb_defio.c |    3 +++
 1 file changed, 3 insertions(+)

drivers/video/fb_defio.c +3 -0

@@ -202,6 +202,9 @@ void fb_deferred_io_cleanup(struct fb_in
 		page = vmalloc_to_page(screen_base + i);
 		page->mapping = NULL;
 	}
+
+	info->fbops->fb_mmap = NULL;
+	mutex_destroy(&fbdefio->lock);
 }
 EXPORT_SYMBOL_GPL(fb_deferred_io_cleanup);

Jaya Kumar

Acked-by on v1 2/5

2008-12-24 05:05 UTC #4 lore raw

Re: [Linux-fbdev-devel] [PATCH 02/05] video: deferred io cleanup

Quoted 7 lines · Magnus Damm

From: Magnus Damm <[email protected]>
Make sure the mmap callback is set to NULL in the deferred io cleanup function. This way we can enable and disable deferred io on the fly.
Signed-off-by Magnus Damm <[email protected]>

Acked-by Jaya Kumar <[email protected]>

Magnus Damm

author

2008-12-19 06:34 UTC #5 lore raw

PATCH 3/5 video: deferred io with physically contiguous memory

In reply to Magnus Damm

From: Magnus Damm <[email protected]>

Extend the deferred io code from only supporting vmalloc()ed frame buffer memory to support both vmalloc()ed and physically contiguous frame buffer memory.

The sh_mobile_lcdcfb hardware does not support scatter gather so we need physically contiguous memory to back our frame buffer.

Signed-off-by Magnus Damm <[email protected]>

---

 drivers/video/fb_defio.c |   20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

drivers/video/fb_defio.c +15 -5

@@ -24,6 +24,19 @@
 #include <linux/rmap.h>
 #include <linux/pagemap.h>
 
+struct page *fb_deferred_io_page(struct fb_info *info, unsigned long offs)
+{
+	void *screen_base = (void __force *) info->screen_base;
+	struct page *page;
+
+	if (is_vmalloc_addr(screen_base + offs))
+		page = vmalloc_to_page(screen_base + offs);
+	else
+		page = pfn_to_page((info->fix.smem_start + offs) >> PAGE_SHIFT);
+
+	return page;
+}
+
 /* this is to find and return the vmalloc-ed fb pages */
 static int fb_deferred_io_fault(struct vm_area_struct *vma,
 				struct vm_fault *vmf)

@@ -31,14 +44,12 @@ static int fb_deferred_io_fault(struct v
 	unsigned long offset;
 	struct page *page;
 	struct fb_info *info = vma->vm_private_data;
-	/* info->screen_base is virtual memory */
-	void *screen_base = (void __force *) info->screen_base;
 
 	offset = vmf->pgoff << PAGE_SHIFT;
 	if (offset >= info->fix.smem_len)
 		return VM_FAULT_SIGBUS;
 
-	page = vmalloc_to_page(screen_base + offset);
+	page = fb_deferred_io_page(info, offset);
 	if (!page)
 		return VM_FAULT_SIGBUS;

@@ -188,7 +199,6 @@ EXPORT_SYMBOL_GPL(fb_deferred_io_open);
 
 void fb_deferred_io_cleanup(struct fb_info *info)
 {
-	void *screen_base = (void __force *) info->screen_base;
 	struct fb_deferred_io *fbdefio = info->fbdefio;
 	struct page *page;
 	int i;

@@ -199,7 +209,7 @@ void fb_deferred_io_cleanup(struct fb_in
 
 	/* clear out the mapping that we setup */
 	for (i = 0 ; i < info->fix.smem_len; i += PAGE_SIZE) {
-		page = vmalloc_to_page(screen_base + i);
+		page = fb_deferred_io_page(info, i);
 		page->mapping = NULL;
 	}

Jaya Kumar

Acked-by on v1 3/5

2008-12-24 05:13 UTC #6 lore raw

Re: [Linux-fbdev-devel] [PATCH 03/05] video: deferred io with physically contiguous memory

Quoted 24 lines · Magnus Damm

From: Magnus Damm <[email protected]>
Extend the deferred io code from only supporting vmalloc()ed frame buffer memory to support both vmalloc()ed and physically contiguous frame buffer memory.
The sh_mobile_lcdcfb hardware does not support scatter gather so we need physically contiguous memory to back our frame buffer.
Signed-off-by Magnus Damm <[email protected]>
---
 drivers/video/fb_defio.c |   20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)
drivers/video/fb_defio.c 2008-12-19 14:29:45.000000000 +0900 renamed from drivers/video/fb_defio.c +3 -0
@@ -24,6 +24,19 @@
 #include <linux/rmap.h>
 #include <linux/pagemap.h>
 
+struct page *fb_deferred_io_page(struct fb_info *info, unsigned long offs)
+{
+       void *screen_base = (void __force *) info->screen_base;

drivers/video/fb_defio.c 2008-12-19 14:29:45.000000000 +0900 line 39

Quoted 10 lines

drivers/video/fb_defio.c 2008-12-19 14:29:45.000000000 +0900 renamed from drivers/video/fb_defio.c +10 -0

+       struct page *page;
+
+       if (is_vmalloc_addr(screen_base + offs))
+               page = vmalloc_to_page(screen_base + offs);
+       else
+               page = pfn_to_page((info->fix.smem_start + offs) >> PAGE_SHIFT);
+
+       return page;
+}
+

Just nitpicking about naming. I think a better name is possible for above, no? Like fb_to_page or fbdefio_to_page or similar to remain consistent with vmalloc/pfn_to_page. Also the comment below about vmalloc-ed pages would no longer be accurate. Otherwise, looks fine.

Acked-by Jaya Kumar <[email protected]>

Quoted 43 lines

drivers/video/fb_defio.c 2008-12-19 14:29:45.000000000 +0900 renamed from drivers/video/fb_defio.c +2 -6

 /* this is to find and return the vmalloc-ed fb pages */
 static int fb_deferred_io_fault(struct vm_area_struct *vma,
                               struct vm_fault *vmf)

@@ -31,14 +44,12 @@ static int fb_deferred_io_fault(struct v
       unsigned long offset;
       struct page *page;
       struct fb_info *info = vma->vm_private_data;
-       /* info->screen_base is virtual memory */
-       void *screen_base = (void __force *) info->screen_base;
 
       offset = vmf->pgoff << PAGE_SHIFT;
       if (offset >= info->fix.smem_len)
               return VM_FAULT_SIGBUS;
 
-       page = vmalloc_to_page(screen_base + offset);
+       page = fb_deferred_io_page(info, offset);
       if (!page)
               return VM_FAULT_SIGBUS;

@@ -188,7 +199,6 @@ EXPORT_SYMBOL_GPL(fb_deferred_io_open);
 
 void fb_deferred_io_cleanup(struct fb_info *info)
 {
-       void *screen_base = (void __force *) info->screen_base;
       struct fb_deferred_io *fbdefio = info->fbdefio;
       struct page *page;
       int i;

@@ -199,7 +209,7 @@ void fb_deferred_io_cleanup(struct fb_in
 
       /* clear out the mapping that we setup */
       for (i = 0 ; i < info->fix.smem_len; i += PAGE_SIZE) {
-               page = vmalloc_to_page(screen_base + i);
+               page = fb_deferred_io_page(info, i);
               page->mapping = NULL;
       }
 
 
------------------------------------------------------------------------------

_______________________________________________
Linux-fbdev-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

Jaya Kumar

2008-12-25 19:04 UTC #7 lore raw

Re: [Linux-fbdev-devel] [PATCH 03/05] video: deferred io with physically contiguous memory

Quoted 16 lines · Jaya Kumar

Quoted 15 lines · Magnus Damm
From: Magnus Damm <[email protected]>
Extend the deferred io code from only supporting vmalloc()ed frame buffer memory to support both vmalloc()ed and physically contiguous frame buffer memory.
The sh_mobile_lcdcfb hardware does not support scatter gather so we need physically contiguous memory to back our frame buffer.
Signed-off-by Magnus Damm <[email protected]>
---
 drivers/video/fb_defio.c |   20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

Quoted 7 lines

drivers/video/fb_defio.c 2008-12-19 14:29:45.000000000 +0900 line 27
Quoted 7 lines
drivers/video/fb_defio.c 2008-12-19 14:29:45.000000000 +0900 renamed from drivers/video/fb_defio.c +1 -0
@@ -24,6 +24,19 @@
 #include <linux/rmap.h>
 #include <linux/pagemap.h>
 
+struct page *fb_deferred_io_page(struct fb_info *info, unsigned long offs)

I think this should be a static function.

Magnus Damm

author

2008-12-19 06:34 UTC #8 lore raw

PATCH 4/5 video: sh_mobile_lcdcfb deferred io support

In reply to Magnus Damm

From: Magnus Damm <[email protected]>

This patch adds sh_mobile_lcdcfb deferred io support for SYS panels.

The refresh rate in deferred io mode is set using platform data. The same platform data can also be used to disable deferred io mode.

As with other deferred io frame buffers user space code should use fsync() on the frame buffer device to trigger an update.

Signed-off-by Magnus Damm <[email protected]>

---

 drivers/video/Kconfig            |    1 
 drivers/video/sh_mobile_lcdcfb.c |  170 +++++++++++++++++++++++++++++++++-----
 include/video/sh_mobile_lcdc.h   |    1 
 3 files changed, 150 insertions(+), 22 deletions(-)

drivers/video/Kconfig +1 -0

@@ -1893,6 +1893,7 @@ config FB_SH_MOBILE_LCDC
 	select FB_SYS_COPYAREA
 	select FB_SYS_IMAGEBLIT
 	select FB_SYS_FOPS
+	select FB_DEFERRED_IO
 	---help---
 	  Frame buffer driver for the on-chip SH-Mobile LCD controller.

drivers/video/sh_mobile_lcdcfb.c +148 ~~-22~~

305 lines not on this page · raw message

include/video/sh_mobile_lcdc.h +1 -0

@@ -37,6 +37,7 @@ enum { LCDC_CLK_BUS, LCDC_CLK_PERIPHERAL
 struct sh_mobile_lcdc_sys_bus_cfg {
 	unsigned long ldmt2r;
 	unsigned long ldmt3r;
+	unsigned long deferred_io_msec;
 };
 
 struct sh_mobile_lcdc_sys_bus_ops {

Magnus Damm

author

2008-12-19 06:34 UTC #9 lore raw

PATCH 5/5 sh: enable deferred io LCDC on Migo-R

In reply to Magnus Damm

From: Magnus Damm <[email protected]>

Enable LCDC deferred io on Migo-R using 1s delay.

As with other deferred io frame buffers user space code should use fsync() on the frame buffer device to trigger an update.

Signed-off-by Magnus Damm <[email protected]>

---

 arch/sh/boards/mach-migor/setup.c |    2 ++
 1 file changed, 2 insertions(+)

arch/sh/boards/mach-migor/setup.c +2 -0

@@ -262,6 +262,8 @@ static struct sh_mobile_lcdc_info sh_mob
 		.sys_bus_cfg = {
 			.ldmt2r = 0x06000a09,
 			.ldmt3r = 0x180e3418,
+			/* set 1s delay to encourage fsync() */
+			.deferred_io_msec = 1000,
 		},
 	}
 #endif

Paul Mundt

2008-12-21 06:53 UTC #10 lore raw

In reply to Magnus Damm

Quoted 23 lines · Magnus Damm

video: deferred io fixes and sh_mobile_lcdcfb support
[PATCH 01/05] video: fix deferred io fsync()
[PATCH 02/05] video: deferred io cleanup
[PATCH 03/05] video: deferred io with physically contiguous memory [PATCH 04/05] video: sh_mobile_lcdcfb deferred io support [PATCH 05/05] sh: enable deferred io LCDC on Migo-R
This patchset adds deferred io support to sh_mobile_lcdcfb.
The LCDC hardware block managed by the sh_mobile_lcdcfb driver supports RGB or SYS panel configurations. SYS panels come with an external display controller that is resposible for refreshing the actual LCD panel. RGB panels are controlled directly by the LCDC and they need to be refreshed by the LCDC hardware.
In the case of SYS panels we can save some power by configuring the LCDC hardware block in one-shot mode. In this one-shot mode panel refresh is managed by software. This works well together with deferred io since it allows us to stop clocks for most of the time and only enable clocks when we actually want to trigger an update. When there is no fbdev activity the clocks are kept stopped which allows us to deep sleep.

Quoted 6 lines

May I suggest merging the first 3 patches with other framebuffer changes, but handling patch 4 and 5 with other SuperH changes? That strategy should be fine since there are no compile time dependencies. To keep things running please apply patch number 5 when all other changes are in.
Signed-off-by Magnus Damm <[email protected]>

I have no particular issues with this series anyways. If Jaya is fine with the first three, I will take the last two. Or I can just take the entire series, as it's fairly insular and no one else is likely to care about it presently. Jaya?

Jaya Kumar

2008-12-21 07:11 UTC #11 lore raw

Quoted 6 lines · Paul Mundt

I have no particular issues with this series anyways. If Jaya is fine with the first three, I will take the last two. Or I can just take the entire series, as it's fairly insular and no one else is likely to care about it presently. Jaya?

Hi Paul,

Looks fine to me too. Nice work. Thanks for the effort Magnus.

I agree that it'd be best if you could take the entire series as it is self-contained, plus there's no fbdev specific tree currently and I'm not sure if Tony is back yet.

Thanks,
jaya

Paul Mundt

2008-12-21 16:14 UTC #12 lore raw

Quoted 8 lines · Jaya Kumar

Quoted 6 lines · Paul Mundt
I have no particular issues with this series anyways. If Jaya is fine with the first three, I will take the last two. Or I can just take the entire series, as it's fairly insular and no one else is likely to care about it presently. Jaya?

Quoted 8 lines

Hi Paul,
Looks fine to me too. Nice work. Thanks for the effort Magnus.
I agree that it'd be best if you could take the entire series as it is self-contained, plus there's no fbdev specific tree currently and I'm not sure if Tony is back yet.

Ok, I will carry it then. Can I add your Acked-by?

Jaya Kumar

2008-12-24 05:01 UTC #13 lore raw

Quoted 11 lines · Paul Mundt

Quoted 10 lines · Jaya Kumar
Quoted 6 lines · Paul Mundt
I have no particular issues with this series anyways. If Jaya is fine with the first three, I will take the last two. Or I can just take the entire series, as it's fairly insular and no one else is likely to care about it presently. Jaya?
Hi Paul,

Quoted 8 lines

Quoted 6 lines
Looks fine to me too. Nice work. Thanks for the effort Magnus.
I agree that it'd be best if you could take the entire series as it is self-contained, plus there's no fbdev specific tree currently and I'm not sure if Tony is back yet.
Ok, I will carry it then. Can I add your Acked-by?

Ok, I'll find Magnus' patches and add my Acked-by to be thorough for each one.

Thanks,
jaya