[PATCH 00/05] video: deferred io sys helpers

[PATCH 00/05] video: deferred io sys helpers

[Magnus Damm]

video: deferred io sys helpers

[PATCH 01/05] video: deferred io sys helpers - core
[PATCH 02/05] video: deferred io sys helpers - sh_mobile_lcdcfb
[PATCH 03/05] video: deferred io sys helpers - hecuba / n411
[PATCH 04/05] video: deferred io sys helpers - metronome
[PATCH 05/05] video: deferred io sys helpers - xen

This patchset extends the deferred io core code with commonly used functions.

The following functions are added to and exported from fb_defio.c:
 - fb_deferred_io_read()
 - fb_deferred_io_write()
 - fb_deferred_io_fillrect()
 - fb_deferred_io_copyarea()
 - fb_deferred_io_imageblit()

To be backwards compatible with the existing behavior the delay variable
"sysdelay" is introduced. It is a separate timeout value for the functions
above. In the sh_mobile_lcdcfb driver we set it to the same as "delay",
but the other drivers keep it as zero which means flush directly.

The deferred io core code now keeps track of a dirty area between the
calls to the functions above. Only xen is currently making use of this.

In the future we may want to extend the dirty area tracking code to
instead modify the dirty bits for the pages backing the frame buffer.

Status:
- sh_mobile_lcdcfb: compiles and works well
- hecuba / n411: compiles on superh, but untested
- metronome: compiles on superh, but untested
- xen: unknown

Signed-off-by: Magnus Damm <damm@igel.co.jp>
---

 drivers/video/Kconfig            |   33 +++---------
 drivers/video/fb_defio.c         |   71 ++++++++++++++++++++++++++
 drivers/video/hecubafb.c         |   86 +-------------------------------
 drivers/video/metronomefb.c      |  102 ++++++--------------------------------
 drivers/video/sh_mobile_lcdcfb.c |   40 ++------------
 drivers/video/xen-fbfront.c      |   52 ++++---------------
 include/linux/fb.h               |   14 +++++
 7 files changed, 135 insertions(+), 263 deletions(-)

[PATCH 01/05] video: deferred io sys helpers - core

[Magnus Damm]

From: Magnus Damm <damm@igel.co.jp>

Add shared sys helpers to the deferred io code. Instead of duplicating
the code in each driver we can keep it in one place. This saves a
few lines. This patch also adds "sysdelay" which allows the driver to
select timeout for sys helper functions. While at it, keep track of
the dirty area to allow partial screen update.

Signed-off-by: Magnus Damm <damm@igel.co.jp>
---

 drivers/video/Kconfig    |    4 ++
 drivers/video/fb_defio.c |   71 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fb.h       |   14 +++++++++
 3 files changed, 89 insertions(+)

--- 0006/drivers/video/Kconfig
+++ work/drivers/video/Kconfig	2008-12-22 13:50:17.000000000 +0900
@@ -179,6 +179,10 @@ config FB_SYS_FOPS
 config FB_DEFERRED_IO
 	bool
 	depends on FB
+	select FB_SYS_FILLRECT
+	select FB_SYS_COPYAREA
+	select FB_SYS_IMAGEBLIT
+	select FB_SYS_FOPS
 
 config FB_HECUBA
 	tristate
--- 0005/drivers/video/fb_defio.c
+++ work/drivers/video/fb_defio.c	2008-12-22 14:03:35.000000000 +0900
@@ -83,6 +83,32 @@ int fb_deferred_io_fsync(struct file *fi
 }
 EXPORT_SYMBOL_GPL(fb_deferred_io_fsync);
 
+static void fb_deferred_io_touch(struct fb_info *info,
+				 int dx, int dy, int width, int height)
+{
+	struct fb_deferred_io *fbdefio = info->fbdefio;
+
+	/* Skip if deferred io is complied-in but disabled on this fbdev */
+	if (!fbdefio)
+		return;
+
+	/* Remember area to redraw and expand if necessary */
+	if (fbdefio->dx == -1) {
+		fbdefio->dx = dx;
+		fbdefio->dy = dy;
+		fbdefio->width = width;
+		fbdefio->height = height;
+	} else {
+		fbdefio->dx = min_t(int, dx, fbdefio->dx);
+		fbdefio->dy = min_t(int, dx, fbdefio->dy);
+		fbdefio->width = max_t(int, width, fbdefio->width);
+		fbdefio->height = max_t(int, height, fbdefio->height);
+	}
+
+	/* come back after sysdelay to process the deferred IO */
+	schedule_delayed_work(&info->deferred_work, fbdefio->sysdelay);
+}
+
 /* vm_ops->page_mkwrite handler */
 static int fb_deferred_io_mkwrite(struct vm_area_struct *vma,
 				  struct page *page)
@@ -172,9 +198,53 @@ static void fb_deferred_io_work(struct w
 	list_for_each_safe(node, next, &fbdefio->pagelist) {
 		list_del(node);
 	}
+
+	/* reset sys operations state */
+	fbdefio->dx = -1;
+
 	mutex_unlock(&fbdefio->lock);
 }
 
+ssize_t fb_deferred_io_read(struct fb_info *info, char __user *buf,
+			    size_t count, loff_t *ppos)
+{
+	return fb_sys_read(info, buf, count, ppos);
+}
+EXPORT_SYMBOL_GPL(fb_deferred_io_read);
+
+ssize_t fb_deferred_io_write(struct fb_info *info, const char __user *buf,
+			     size_t count, loff_t *ppos)
+{
+	int ret = fb_sys_write(info, buf, count, ppos);
+
+	if (ret > 0)
+		fb_deferred_io_touch(info, 0, 0, INT_MAX, INT_MAX);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(fb_deferred_io_write);
+
+void fb_deferred_io_fillrect(struct fb_info *info, const struct fb_fillrect *r)
+{
+	sys_fillrect(info, r);
+	fb_deferred_io_touch(info, r->dx, r->dy, r->width, r->height);
+}
+EXPORT_SYMBOL_GPL(fb_deferred_io_fillrect);
+
+void fb_deferred_io_copyarea(struct fb_info *info, const struct fb_copyarea *a)
+{
+	sys_copyarea(info, a);
+	fb_deferred_io_touch(info, a->dx, a->dy, a->width, a->height);
+}
+EXPORT_SYMBOL_GPL(fb_deferred_io_copyarea);
+
+void fb_deferred_io_imageblit(struct fb_info *info, const struct fb_image *i)
+{
+	sys_imageblit(info, i);
+	fb_deferred_io_touch(info, i->dx, i->dy, i->width, i->height);
+}
+EXPORT_SYMBOL_GPL(fb_deferred_io_imageblit);
+
 void fb_deferred_io_init(struct fb_info *info)
 {
 	struct fb_deferred_io *fbdefio = info->fbdefio;
@@ -184,6 +254,7 @@ void fb_deferred_io_init(struct fb_info 
 	info->fbops->fb_mmap = fb_deferred_io_mmap;
 	INIT_DELAYED_WORK(&info->deferred_work, fb_deferred_io_work);
 	INIT_LIST_HEAD(&fbdefio->pagelist);
+	fbdefio->dx = -1;
 	if (fbdefio->delay == 0) /* set a default of 1 s */
 		fbdefio->delay = HZ;
 }
--- 0001/include/linux/fb.h
+++ work/include/linux/fb.h	2008-12-22 13:50:17.000000000 +0900
@@ -587,8 +587,11 @@ struct fb_pixmap {
 struct fb_deferred_io {
 	/* delay between mkwrite and deferred handler */
 	unsigned long delay;
+	/* delay between write/fillrect/copyarea/imageblit and handler */
+	unsigned long sysdelay;
 	struct mutex lock; /* mutex that protects the page list */
 	struct list_head pagelist; /* list of touched pages */
+	int dx, dy, width, height; /* modified area */
 	/* callback */
 	void (*deferred_io)(struct fb_info *info, struct list_head *pagelist);
 };
@@ -983,6 +986,17 @@ extern void fb_deferred_io_open(struct f
 extern void fb_deferred_io_cleanup(struct fb_info *info);
 extern int fb_deferred_io_fsync(struct file *file, struct dentry *dentry,
 				int datasync);
+extern ssize_t fb_deferred_io_read(struct fb_info *info, char __user *buf,
+				   size_t count, loff_t *ppos);
+extern ssize_t fb_deferred_io_write(struct fb_info *info,
+				    const char __user *buf,
+				    size_t count, loff_t *ppos);
+extern void fb_deferred_io_fillrect(struct fb_info *info,
+				    const struct fb_fillrect *rect);
+extern void fb_deferred_io_copyarea(struct fb_info *info,
+				    const struct fb_copyarea *area);
+extern void fb_deferred_io_imageblit(struct fb_info *info,
+				     const struct fb_image *image);
 
 static inline bool fb_be_math(struct fb_info *info)
 {

[PATCH 02/05] video: deferred io sys helpers - sh_mobile_lcdcfb

[Magnus Damm]

From: Magnus Damm <damm@igel.co.jp>

Change the sh_mobile_lcdcfb driver to use the new shared sys helpers.
This allows us to remove some code. Also, "sysdelay" is set to the same
value as "delay" to keep same behavior as before.

Signed-off-by: Magnus Damm <damm@igel.co.jp>
---

 drivers/video/Kconfig            |    4 ---
 drivers/video/sh_mobile_lcdcfb.c |   40 +++++---------------------------------
 2 files changed, 6 insertions(+), 38 deletions(-)

--- 0049/drivers/video/Kconfig
+++ work/drivers/video/Kconfig	2008-12-22 14:05:10.000000000 +0900
@@ -1893,10 +1893,6 @@ config FB_W100
 config FB_SH_MOBILE_LCDC
 	tristate "SuperH Mobile LCDC framebuffer support"
 	depends on FB && SUPERH
-	select FB_SYS_FILLRECT
-	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.
--- 0006/drivers/video/sh_mobile_lcdcfb.c
+++ work/drivers/video/sh_mobile_lcdcfb.c	2008-12-22 14:05:10.000000000 +0900
@@ -215,14 +215,6 @@ static void sh_mobile_lcdc_deferred_io(s
 	lcdc_write_chan(ch, LDSM2R, 1);
 }
 
-static void sh_mobile_lcdc_deferred_io_touch(struct fb_info *info)
-{
-	struct fb_deferred_io *fbdefio = info->fbdefio;
-
-	if (fbdefio)
-		schedule_delayed_work(&info->deferred_work, fbdefio->delay);
-}
-
 static irqreturn_t sh_mobile_lcdc_irq(int irq, void *data)
 {
 	struct sh_mobile_lcdc_priv *priv = data;
@@ -410,6 +402,7 @@ static int sh_mobile_lcdc_start(struct s
 		if (ch->ldmt1r_value & (1 << 12) && tmp) {
 			ch->defio.deferred_io = sh_mobile_lcdc_deferred_io;
 			ch->defio.delay = msecs_to_jiffies(tmp);
+			ch->defio.sysdelay = msecs_to_jiffies(tmp);
 			ch->info.fbdefio = &ch->defio;
 			fb_deferred_io_init(&ch->info);
 
@@ -586,34 +579,13 @@ static struct fb_fix_screeninfo sh_mobil
 	.accel =	FB_ACCEL_NONE,
 };
 
-static void sh_mobile_lcdc_fillrect(struct fb_info *info,
-				    const struct fb_fillrect *rect)
-{
-	sys_fillrect(info, rect);
-	sh_mobile_lcdc_deferred_io_touch(info);
-}
-
-static void sh_mobile_lcdc_copyarea(struct fb_info *info,
-				    const struct fb_copyarea *area)
-{
-	sys_copyarea(info, area);
-	sh_mobile_lcdc_deferred_io_touch(info);
-}
-
-static void sh_mobile_lcdc_imageblit(struct fb_info *info,
-				     const struct fb_image *image)
-{
-	sys_imageblit(info, image);
-	sh_mobile_lcdc_deferred_io_touch(info);
-}
-
 static struct fb_ops sh_mobile_lcdc_ops = {
 	.fb_setcolreg	= sh_mobile_lcdc_setcolreg,
-	.fb_read        = fb_sys_read,
-	.fb_write       = fb_sys_write,
-	.fb_fillrect	= sh_mobile_lcdc_fillrect,
-	.fb_copyarea	= sh_mobile_lcdc_copyarea,
-	.fb_imageblit	= sh_mobile_lcdc_imageblit,
+	.fb_read        = fb_deferred_io_read,
+	.fb_write       = fb_deferred_io_write,
+	.fb_fillrect	= fb_deferred_io_fillrect,
+	.fb_copyarea	= fb_deferred_io_copyarea,
+	.fb_imageblit	= fb_deferred_io_imageblit,
 };
 
 static int sh_mobile_lcdc_set_bpp(struct fb_var_screeninfo *var, int bpp)

[PATCH 03/05] video: deferred io sys helpers - hecuba / n411

[Magnus Damm]

From: Magnus Damm <damm@igel.co.jp>

Change the hecubafb driver and the n411 code to use the new shared sys
helpers. This allows us to remove some duplicated code. In the future
this driver can setup the "sysdelay" value to delay flushing.

While at it, convert space to tabs for the n411 kconfig entry.

Signed-off-by: Magnus Damm <damm@igel.co.jp>
---

 drivers/video/Kconfig    |   17 +++------
 drivers/video/hecubafb.c |   86 ++--------------------------------------------
 2 files changed, 11 insertions(+), 92 deletions(-)

--- 0050/drivers/video/Kconfig
+++ work/drivers/video/Kconfig	2008-12-22 14:06:31.000000000 +0900
@@ -712,17 +712,12 @@ config FB_EFI
 	  using the EFI framebuffer as your console.
 
 config FB_N411
-       tristate "N411 Apollo/Hecuba devkit support"
-       depends on FB && X86 && MMU
-       select FB_SYS_FILLRECT
-       select FB_SYS_COPYAREA
-       select FB_SYS_IMAGEBLIT
-       select FB_SYS_FOPS
-       select FB_DEFERRED_IO
-       select FB_HECUBA
-       help
-         This enables support for the Apollo display controller in its
-         Hecuba form using the n411 devkit.
+	tristate "N411 Apollo/Hecuba devkit support"
+	depends on FB && X86 && MMU
+	select FB_HECUBA
+	help
+	  This enables support for the Apollo display controller in its
+	  Hecuba form using the n411 devkit.
 
 config FB_HGA
 	tristate "Hercules mono graphics support"
--- 0001/drivers/video/hecubafb.c
+++ work/drivers/video/hecubafb.c	2008-12-22 14:06:31.000000000 +0900
@@ -122,89 +122,13 @@ static void hecubafb_dpy_deferred_io(str
 	hecubafb_dpy_update(info->par);
 }
 
-static void hecubafb_fillrect(struct fb_info *info,
-				   const struct fb_fillrect *rect)
-{
-	struct hecubafb_par *par = info->par;
-
-	sys_fillrect(info, rect);
-
-	hecubafb_dpy_update(par);
-}
-
-static void hecubafb_copyarea(struct fb_info *info,
-				   const struct fb_copyarea *area)
-{
-	struct hecubafb_par *par = info->par;
-
-	sys_copyarea(info, area);
-
-	hecubafb_dpy_update(par);
-}
-
-static void hecubafb_imageblit(struct fb_info *info,
-				const struct fb_image *image)
-{
-	struct hecubafb_par *par = info->par;
-
-	sys_imageblit(info, image);
-
-	hecubafb_dpy_update(par);
-}
-
-/*
- * this is the slow path from userspace. they can seek and write to
- * the fb. it's inefficient to do anything less than a full screen draw
- */
-static ssize_t hecubafb_write(struct fb_info *info, const char __user *buf,
-				size_t count, loff_t *ppos)
-{
-	struct hecubafb_par *par = info->par;
-	unsigned long p = *ppos;
-	void *dst;
-	int err = 0;
-	unsigned long total_size;
-
-	if (info->state != FBINFO_STATE_RUNNING)
-		return -EPERM;
-
-	total_size = info->fix.smem_len;
-
-	if (p > total_size)
-		return -EFBIG;
-
-	if (count > total_size) {
-		err = -EFBIG;
-		count = total_size;
-	}
-
-	if (count + p > total_size) {
-		if (!err)
-			err = -ENOSPC;
-
-		count = total_size - p;
-	}
-
-	dst = (void __force *) (info->screen_base + p);
-
-	if (copy_from_user(dst, buf, count))
-		err = -EFAULT;
-
-	if  (!err)
-		*ppos += count;
-
-	hecubafb_dpy_update(par);
-
-	return (err) ? err : count;
-}
-
 static struct fb_ops hecubafb_ops = {
 	.owner		= THIS_MODULE,
-	.fb_read        = fb_sys_read,
-	.fb_write	= hecubafb_write,
-	.fb_fillrect	= hecubafb_fillrect,
-	.fb_copyarea	= hecubafb_copyarea,
-	.fb_imageblit	= hecubafb_imageblit,
+	.fb_read        = fb_deferred_io_read,
+	.fb_write	= fb_deferred_io_write,
+	.fb_fillrect	= fb_deferred_io_fillrect,
+	.fb_copyarea	= fb_deferred_io_copyarea,
+	.fb_imageblit	= fb_deferred_io_imageblit,
 };
 
 static struct fb_deferred_io hecubafb_defio = {

Re: [PATCH 03/05] video: deferred io sys helpers - hecuba / n411

[Paul Mundt]

On Mon, Dec 22, 2008 at 02:53:01PM +0900, Magnus Damm wrote:
> From: Magnus Damm <damm@igel.co.jp>
> 
> Change the hecubafb driver and the n411 code to use the new shared sys
> helpers. This allows us to remove some duplicated code. In the future
> this driver can setup the "sysdelay" value to delay flushing.
> 
> While at it, convert space to tabs for the n411 kconfig entry.
> 
> Signed-off-by: Magnus Damm <damm@igel.co.jp>
> ---
> 
>  drivers/video/Kconfig    |   17 +++------
>  drivers/video/hecubafb.c |   86 ++--------------------------------------------
>  2 files changed, 11 insertions(+), 92 deletions(-)
> 
> --- 0050/drivers/video/Kconfig
> +++ work/drivers/video/Kconfig	2008-12-22 14:06:31.000000000 +0900
> @@ -712,17 +712,12 @@ config FB_EFI
>  	  using the EFI framebuffer as your console.
>  
>  config FB_N411
> -       tristate "N411 Apollo/Hecuba devkit support"
> -       depends on FB && X86 && MMU
> -       select FB_SYS_FILLRECT
> -       select FB_SYS_COPYAREA
> -       select FB_SYS_IMAGEBLIT
> -       select FB_SYS_FOPS
> -       select FB_DEFERRED_IO
> -       select FB_HECUBA
> -       help
> -         This enables support for the Apollo display controller in its
> -         Hecuba form using the n411 devkit.
> +	tristate "N411 Apollo/Hecuba devkit support"
> +	depends on FB && X86 && MMU
> +	select FB_HECUBA
> +	help
> +	  This enables support for the Apollo display controller in its
> +	  Hecuba form using the n411 devkit.

This drops the FB_DEFERRED_IO select.. FB_HECUBA has a depends on, but
nothing else will select it.

Re: [PATCH 03/05] video: deferred io sys helpers - hecuba / n411

[Magnus Damm]

On Mon, Dec 22, 2008 at 2:58 PM, Paul Mundt <lethal@linux-sh.org> wrote:
> On Mon, Dec 22, 2008 at 02:53:01PM +0900, Magnus Damm wrote:
>> From: Magnus Damm <damm@igel.co.jp>
>>
>> Change the hecubafb driver and the n411 code to use the new shared sys
>> helpers. This allows us to remove some duplicated code. In the future
>> this driver can setup the "sysdelay" value to delay flushing.
>>
>> While at it, convert space to tabs for the n411 kconfig entry.
>>
>> Signed-off-by: Magnus Damm <damm@igel.co.jp>
>> ---
>>
>>  drivers/video/Kconfig    |   17 +++------
>>  drivers/video/hecubafb.c |   86 ++--------------------------------------------
>>  2 files changed, 11 insertions(+), 92 deletions(-)
>>
>> --- 0050/drivers/video/Kconfig
>> +++ work/drivers/video/Kconfig        2008-12-22 14:06:31.000000000 +0900
>> @@ -712,17 +712,12 @@ config FB_EFI
>>         using the EFI framebuffer as your console.
>>
>>  config FB_N411
>> -       tristate "N411 Apollo/Hecuba devkit support"
>> -       depends on FB && X86 && MMU
>> -       select FB_SYS_FILLRECT
>> -       select FB_SYS_COPYAREA
>> -       select FB_SYS_IMAGEBLIT
>> -       select FB_SYS_FOPS
>> -       select FB_DEFERRED_IO
>> -       select FB_HECUBA
>> -       help
>> -         This enables support for the Apollo display controller in its
>> -         Hecuba form using the n411 devkit.
>> +     tristate "N411 Apollo/Hecuba devkit support"
>> +     depends on FB && X86 && MMU
>> +     select FB_HECUBA
>> +     help
>> +       This enables support for the Apollo display controller in its
>> +       Hecuba form using the n411 devkit.
>
> This drops the FB_DEFERRED_IO select.. FB_HECUBA has a depends on, but
> nothing else will select it.

Uhm, right. FB_HECUBA should select instead of depend. Thanks for checking!

/ magnus

[PATCH 04/05] video: deferred io sys helpers - metronome

[Magnus Damm]

From: Magnus Damm <damm@igel.co.jp>

Change the metronome driver to use the new shared sys helpers.
In the future this driver can setup the "sysdelay" value to
delay flushing.

Signed-off-by: Magnus Damm <damm@igel.co.jp>
---

 drivers/video/Kconfig       |    4 -
 drivers/video/metronomefb.c |  102 +++++++------------------------------------
 2 files changed, 17 insertions(+), 89 deletions(-)

--- 0051/drivers/video/Kconfig
+++ work/drivers/video/Kconfig	2008-12-22 14:07:15.000000000 +0900
@@ -2071,10 +2071,6 @@ config XEN_FBDEV_FRONTEND
 config FB_METRONOME
 	tristate "E-Ink Metronome/8track controller support"
 	depends on FB
-	select FB_SYS_FILLRECT
-	select FB_SYS_COPYAREA
-	select FB_SYS_IMAGEBLIT
-	select FB_SYS_FOPS
 	select FB_DEFERRED_IO
 	help
 	  This driver implements support for the E-Ink Metronome
--- 0001/drivers/video/metronomefb.c
+++ work/drivers/video/metronomefb.c	2008-12-22 14:07:15.000000000 +0900
@@ -445,7 +445,6 @@ static void metronomefb_dpy_update(struc
 
 	cksum = calc_img_cksum((u16 *) par->metromem_img, fbsize/2);
 	*((u16 *)(par->metromem_img) + fbsize/2) = cksum;
-	metronome_display_cmd(par);
 }
 
 static u16 metronomefb_dpy_update_page(struct metronomefb_par *par, int index)
@@ -472,97 +471,30 @@ static void metronomefb_dpy_deferred_io(
 	struct fb_deferred_io *fbdefio = info->fbdefio;
 	struct metronomefb_par *par = info->par;
 
-	/* walk the written page list and swizzle the data */
-	list_for_each_entry(cur, &fbdefio->pagelist, lru) {
-		cksum = metronomefb_dpy_update_page(par,
-					(cur->index << PAGE_SHIFT));
-		par->metromem_img_csum -= par->csum_table[cur->index];
-		par->csum_table[cur->index] = cksum;
-		par->metromem_img_csum += cksum;
+	if (fbdefio->dx != -1) {
+		/* update the entire screen */
+		metronomefb_dpy_update(par);
+	} else {
+		/* walk the written page list and swizzle the data */
+		list_for_each_entry(cur, &fbdefio->pagelist, lru) {
+			cksum = metronomefb_dpy_update_page(par,
+						(cur->index << PAGE_SHIFT));
+			par->metromem_img_csum -= par->csum_table[cur->index];
+			par->csum_table[cur->index] = cksum;
+			par->metromem_img_csum += cksum;
+		}
 	}
 
 	metronome_display_cmd(par);
 }
 
-static void metronomefb_fillrect(struct fb_info *info,
-				   const struct fb_fillrect *rect)
-{
-	struct metronomefb_par *par = info->par;
-
-	sys_fillrect(info, rect);
-	metronomefb_dpy_update(par);
-}
-
-static void metronomefb_copyarea(struct fb_info *info,
-				   const struct fb_copyarea *area)
-{
-	struct metronomefb_par *par = info->par;
-
-	sys_copyarea(info, area);
-	metronomefb_dpy_update(par);
-}
-
-static void metronomefb_imageblit(struct fb_info *info,
-				const struct fb_image *image)
-{
-	struct metronomefb_par *par = info->par;
-
-	sys_imageblit(info, image);
-	metronomefb_dpy_update(par);
-}
-
-/*
- * this is the slow path from userspace. they can seek and write to
- * the fb. it is based on fb_sys_write
- */
-static ssize_t metronomefb_write(struct fb_info *info, const char __user *buf,
-				size_t count, loff_t *ppos)
-{
-	struct metronomefb_par *par = info->par;
-	unsigned long p = *ppos;
-	void *dst;
-	int err = 0;
-	unsigned long total_size;
-
-	if (info->state != FBINFO_STATE_RUNNING)
-		return -EPERM;
-
-	total_size = info->fix.smem_len;
-
-	if (p > total_size)
-		return -EFBIG;
-
-	if (count > total_size) {
-		err = -EFBIG;
-		count = total_size;
-	}
-
-	if (count + p > total_size) {
-		if (!err)
-			err = -ENOSPC;
-
-		count = total_size - p;
-	}
-
-	dst = (void __force *)(info->screen_base + p);
-
-	if (copy_from_user(dst, buf, count))
-		err = -EFAULT;
-
-	if  (!err)
-		*ppos += count;
-
-	metronomefb_dpy_update(par);
-
-	return (err) ? err : count;
-}
-
 static struct fb_ops metronomefb_ops = {
 	.owner		= THIS_MODULE,
-	.fb_write	= metronomefb_write,
-	.fb_fillrect	= metronomefb_fillrect,
-	.fb_copyarea	= metronomefb_copyarea,
-	.fb_imageblit	= metronomefb_imageblit,
+	.fb_read        = fb_deferred_io_read,
+	.fb_write	= fb_deferred_io_write,
+	.fb_fillrect	= fb_deferred_io_fillrect,
+	.fb_copyarea	= fb_deferred_io_copyarea,
+	.fb_imageblit	= fb_deferred_io_imageblit,
 };
 
 static struct fb_deferred_io metronomefb_defio = {

Re: [PATCH 04/05] video: deferred io sys helpers - metronome

[Jaya Kumar]

On Mon, Dec 22, 2008 at 12:53 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> From: Magnus Damm <damm@igel.co.jp>
>
> Change the metronome driver to use the new shared sys helpers.
> In the future this driver can setup the "sysdelay" value to
> delay flushing.
>
> Signed-off-by: Magnus Damm <damm@igel.co.jp>
> ---
>
>  drivers/video/Kconfig       |    4 -
>  drivers/video/metronomefb.c |  102 +++++++------------------------------------
>  2 files changed, 17 insertions(+), 89 deletions(-)
>
> --- 0051/drivers/video/Kconfig
> +++ work/drivers/video/Kconfig  2008-12-22 14:07:15.000000000 +0900
> @@ -2071,10 +2071,6 @@ config XEN_FBDEV_FRONTEND
>  config FB_METRONOME
>        tristate "E-Ink Metronome/8track controller support"
>        depends on FB
> -       select FB_SYS_FILLRECT
> -       select FB_SYS_COPYAREA
> -       select FB_SYS_IMAGEBLIT
> -       select FB_SYS_FOPS
>        select FB_DEFERRED_IO
>        help
>          This driver implements support for the E-Ink Metronome
> --- 0001/drivers/video/metronomefb.c
> +++ work/drivers/video/metronomefb.c    2008-12-22 14:07:15.000000000 +0900
> @@ -445,7 +445,6 @@ static void metronomefb_dpy_update(struc
>
>        cksum = calc_img_cksum((u16 *) par->metromem_img, fbsize/2);
>        *((u16 *)(par->metromem_img) + fbsize/2) = cksum;
> -       metronome_display_cmd(par);
>  }
>
>  static u16 metronomefb_dpy_update_page(struct metronomefb_par *par, int index)
> @@ -472,97 +471,30 @@ static void metronomefb_dpy_deferred_io(
>        struct fb_deferred_io *fbdefio = info->fbdefio;
>        struct metronomefb_par *par = info->par;
>
> -       /* walk the written page list and swizzle the data */
> -       list_for_each_entry(cur, &fbdefio->pagelist, lru) {
> -               cksum = metronomefb_dpy_update_page(par,
> -                                       (cur->index << PAGE_SHIFT));
> -               par->metromem_img_csum -= par->csum_table[cur->index];
> -               par->csum_table[cur->index] = cksum;
> -               par->metromem_img_csum += cksum;
> +       if (fbdefio->dx != -1) {
> +               /* update the entire screen */
> +               metronomefb_dpy_update(par);

Okay, overall seems okay. But I'm not confident about this specific
change above. The meaning of dx, etc is not clear to me. Okay, I think
I found a problem with above. Here's my explanation, please correct me
if I'm wrong in my reading of the code you've posted. The typical use
case is just a single mmap client (eg: Xfbdev) and nothing else. No
fbcon. The only code path I see that sets defio->dx is in the
write/blit/* paths via the defio_touch in patch 1/5 and none in the
mmap path so with this implementation, all mmap clients would
repeatedly only hit the above full update scenario and never use the
page based update mechanism.

> +       } else {
> +               /* walk the written page list and swizzle the data */
> +               list_for_each_entry(cur, &fbdefio->pagelist, lru) {
> +                       cksum = metronomefb_dpy_update_page(par,
> +                                               (cur->index << PAGE_SHIFT));
> +                       par->metromem_img_csum -= par->csum_table[cur->index];
> +                       par->csum_table[cur->index] = cksum;
> +                       par->metromem_img_csum += cksum;
> +               }
>        }
>
>        metronome_display_cmd(par);
>  }
>
> -static void metronomefb_fillrect(struct fb_info *info,
> -                                  const struct fb_fillrect *rect)
> -{
> -       struct metronomefb_par *par = info->par;
> -
> -       sys_fillrect(info, rect);
> -       metronomefb_dpy_update(par);
> -}
> -
> -static void metronomefb_copyarea(struct fb_info *info,
> -                                  const struct fb_copyarea *area)
> -{
> -       struct metronomefb_par *par = info->par;
> -
> -       sys_copyarea(info, area);
> -       metronomefb_dpy_update(par);
> -}
> -
> -static void metronomefb_imageblit(struct fb_info *info,
> -                               const struct fb_image *image)
> -{
> -       struct metronomefb_par *par = info->par;
> -
> -       sys_imageblit(info, image);
> -       metronomefb_dpy_update(par);
> -}
> -
> -/*
> - * this is the slow path from userspace. they can seek and write to
> - * the fb. it is based on fb_sys_write
> - */
> -static ssize_t metronomefb_write(struct fb_info *info, const char __user *buf,
> -                               size_t count, loff_t *ppos)
> -{
> -       struct metronomefb_par *par = info->par;
> -       unsigned long p = *ppos;
> -       void *dst;
> -       int err = 0;
> -       unsigned long total_size;
> -
> -       if (info->state != FBINFO_STATE_RUNNING)
> -               return -EPERM;
> -
> -       total_size = info->fix.smem_len;
> -
> -       if (p > total_size)
> -               return -EFBIG;
> -
> -       if (count > total_size) {
> -               err = -EFBIG;
> -               count = total_size;
> -       }
> -
> -       if (count + p > total_size) {
> -               if (!err)
> -                       err = -ENOSPC;
> -
> -               count = total_size - p;
> -       }
> -
> -       dst = (void __force *)(info->screen_base + p);
> -
> -       if (copy_from_user(dst, buf, count))
> -               err = -EFAULT;
> -
> -       if  (!err)
> -               *ppos += count;
> -
> -       metronomefb_dpy_update(par);
> -
> -       return (err) ? err : count;
> -}
> -
>  static struct fb_ops metronomefb_ops = {
>        .owner          = THIS_MODULE,
> -       .fb_write       = metronomefb_write,
> -       .fb_fillrect    = metronomefb_fillrect,
> -       .fb_copyarea    = metronomefb_copyarea,
> -       .fb_imageblit   = metronomefb_imageblit,
> +       .fb_read        = fb_deferred_io_read,
> +       .fb_write       = fb_deferred_io_write,
> +       .fb_fillrect    = fb_deferred_io_fillrect,
> +       .fb_copyarea    = fb_deferred_io_copyarea,
> +       .fb_imageblit   = fb_deferred_io_imageblit,
>  };

Other than the above problem, looks okay and its nice to get rid of
all that stuff above. :-)

Thanks,
jaya

Re: [PATCH 04/05] video: deferred io sys helpers - metronome

[Magnus Damm]

On Wed, Dec 24, 2008 at 1:58 PM, Jaya Kumar <jayakumar.lkml@gmail.com> wrote:
> On Mon, Dec 22, 2008 at 12:53 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> +       if (fbdefio->dx != -1) {
>> +               /* update the entire screen */
>> +               metronomefb_dpy_update(par);
>
> Okay, overall seems okay. But I'm not confident about this specific
> change above. The meaning of dx, etc is not clear to me. Okay, I think
> I found a problem with above. Here's my explanation, please correct me
> if I'm wrong in my reading of the code you've posted. The typical use
> case is just a single mmap client (eg: Xfbdev) and nothing else. No
> fbcon. The only code path I see that sets defio->dx is in the
> write/blit/* paths via the defio_touch in patch 1/5 and none in the
> mmap path so with this implementation, all mmap clients would
> repeatedly only hit the above full update scenario and never use the
> page based update mechanism.

Hm, I wonder if I reversed the logic by mistake, but I don't think so.
The idea is that dx should be set to -1 by default and stay that way
unless write/blit/* has been used. After each deferred io work dx is
reset to -1. If dx is not -1 in the deferred io callback then we need
to refresh the area specified by dx, dy, width, height _and_ the
pages. The xen driver does this.

In the metronome driver case we update the entire screen if dx has
been set which means we can skip the pages since they are considered
part of the entire screen. This should be the same behavior as before,
the local write/bit/* functions all call metronome_dpy_update()
without this patch.

Or maybe my logic is broken?

Btw, patch [1/5] is somewhat broken today with the handling of
sysdelay vs delay. Ideally I'd like to get rid of sysdelay and let
write/blit/* use the same delay as mmap(). Not sure if you are keen on
that though since it changes the behavior of your drivers. Otoh that
may make fbcon usable on e-paper since it decouples the soft cursor
refresh rate from the screen refresh rate.

> Other than the above problem, looks okay and its nice to get rid of
> all that stuff above. :-)

Excellent, thanks!

Cheers,

/ magnus

Re: [PATCH 04/05] video: deferred io sys helpers - metronome

[Jaya Kumar]

On Wed, Dec 24, 2008 at 12:46 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>
> Btw, patch [1/5] is somewhat broken today with the handling of
> sysdelay vs delay. Ideally I'd like to get rid of sysdelay and let
> write/blit/* use the same delay as mmap(). Not sure if you are keen on
> that though since it changes the behavior of your drivers. Otoh that
> may make fbcon usable on e-paper since it decouples the soft cursor
> refresh rate from the screen refresh rate.

You're right that that would make fbcon be better. I don't think there
are too many users currently so now would be a good time to change. So
yes, I agree with changing the current behavior and droping the
sysdelay stuff.

Thanks,
jaya

[PATCH 05/05] video: deferred io sys helpers - xen

[Magnus Damm]

From: Magnus Damm <damm@igel.co.jp>

Change the xen driver to use the new shared sys helpers. This allows
us to remove some duplicated code. In the future this driver can
setup the "sysdelay" value to delay flushing.

Signed-off-by: Magnus Damm <damm@igel.co.jp>
---

 drivers/video/Kconfig       |    4 ---
 drivers/video/xen-fbfront.c |   52 +++++++++----------------------------------
 2 files changed, 12 insertions(+), 44 deletions(-)

--- 0052/drivers/video/Kconfig
+++ work/drivers/video/Kconfig	2008-12-22 14:08:03.000000000 +0900
@@ -2057,10 +2057,6 @@ config FB_VIRTUAL
 config XEN_FBDEV_FRONTEND
 	tristate "Xen virtual frame buffer support"
 	depends on FB && XEN
-	select FB_SYS_FILLRECT
-	select FB_SYS_COPYAREA
-	select FB_SYS_IMAGEBLIT
-	select FB_SYS_FOPS
 	select FB_DEFERRED_IO
 	default y
 	help
--- 0001/drivers/video/xen-fbfront.c
+++ work/drivers/video/xen-fbfront.c	2008-12-22 14:08:03.000000000 +0900
@@ -177,11 +177,18 @@ static void xenfb_refresh(struct xenfb_i
 static void xenfb_deferred_io(struct fb_info *fb_info,
 			      struct list_head *pagelist)
 {
+	struct fb_deferred_io *fbdefio = fb_info->fbdefio;
 	struct xenfb_info *info = fb_info->par;
 	struct page *page;
 	unsigned long beg, end;
 	int y1, y2, miny, maxy;
 
+	if (fbdefio->dx != -1) {
+		xenfb_refresh(info, fbdefio->dx, fbdefio->dy,
+			      min_t(int, info->page->width, fbdefio->width),
+			      min_t(int, info->page->height, fbdefio->height));
+	}
+
 	miny = INT_MAX;
 	maxy = 0;
 	list_for_each_entry(page, pagelist, lru) {
@@ -235,41 +242,6 @@ static int xenfb_setcolreg(unsigned regn
 	return 0;
 }
 
-static void xenfb_fillrect(struct fb_info *p, const struct fb_fillrect *rect)
-{
-	struct xenfb_info *info = p->par;
-
-	sys_fillrect(p, rect);
-	xenfb_refresh(info, rect->dx, rect->dy, rect->width, rect->height);
-}
-
-static void xenfb_imageblit(struct fb_info *p, const struct fb_image *image)
-{
-	struct xenfb_info *info = p->par;
-
-	sys_imageblit(p, image);
-	xenfb_refresh(info, image->dx, image->dy, image->width, image->height);
-}
-
-static void xenfb_copyarea(struct fb_info *p, const struct fb_copyarea *area)
-{
-	struct xenfb_info *info = p->par;
-
-	sys_copyarea(p, area);
-	xenfb_refresh(info, area->dx, area->dy, area->width, area->height);
-}
-
-static ssize_t xenfb_write(struct fb_info *p, const char __user *buf,
-			size_t count, loff_t *ppos)
-{
-	struct xenfb_info *info = p->par;
-	ssize_t res;
-
-	res = fb_sys_write(p, buf, count, ppos);
-	xenfb_refresh(info, 0, 0, info->page->width, info->page->height);
-	return res;
-}
-
 static int
 xenfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
 {
@@ -323,12 +295,12 @@ static int xenfb_set_par(struct fb_info 
 
 static struct fb_ops xenfb_fb_ops = {
 	.owner		= THIS_MODULE,
-	.fb_read	= fb_sys_read,
-	.fb_write	= xenfb_write,
+	.fb_read        = fb_deferred_io_read,
+	.fb_write	= fb_deferred_io_write,
+	.fb_fillrect	= fb_deferred_io_fillrect,
+	.fb_copyarea	= fb_deferred_io_copyarea,
+	.fb_imageblit	= fb_deferred_io_imageblit,
 	.fb_setcolreg	= xenfb_setcolreg,
-	.fb_fillrect	= xenfb_fillrect,
-	.fb_copyarea	= xenfb_copyarea,
-	.fb_imageblit	= xenfb_imageblit,
 	.fb_check_var	= xenfb_check_var,
 	.fb_set_par     = xenfb_set_par,
 };