[PATCH 00/10] fbdev: Generate deferred-I/O helpers

[PATCH 00/10] fbdev: Generate deferred-I/O helpers

[Thomas Zimmermann]

Generate the I/O callbacks for drivers with deferred I/O. As in
the old, opencoded functions, the generated functions operate on
system memory and trigger damage handling if necessary. Also bring
the drivers' Kconfig options up to date.

Generating and initializing via helpers macros will later allow for
a fine-grained setup, depending on Kconfig options. For example, it
will be possible to leave out file I/O if FB_DEVICE has not been set.

Thomas Zimmermann (10):
  fbdev/broadsheetfb: Select FB_SYS_HELPERS_DEFERRED
  fbdev/broadsheetfb: Generate deferred I/O ops
  fbdev/hecubafb: Select FB_SYS_HELPERS_DEFERRED
  fbdev/hecubafb: Generate deferred I/O ops
  fbdev/metronomefb: Select FB_SYS_HELPERS_DEFERRED
  fbdev/metronomefb: Generate deferred I/O ops
  fbdev/ssd1307fb: Select FB_SYS_HELPERS_DEFERRED
  fbdev/ssd1307fb: Generate deferred I/O ops
  fbdev/xen-fbfront: Select FB_SYS_HELPERS_DEFERRED
  fbdev/xen-fbfront: Generate deferred I/O ops

 drivers/video/fbdev/Kconfig        | 31 ++----------
 drivers/video/fbdev/broadsheetfb.c | 78 +++---------------------------
 drivers/video/fbdev/hecubafb.c     | 78 +++---------------------------
 drivers/video/fbdev/metronomefb.c  | 74 +++-------------------------
 drivers/video/fbdev/ssd1307fb.c    | 69 +++++---------------------
 drivers/video/fbdev/xen-fbfront.c  | 61 ++++++++---------------
 6 files changed, 60 insertions(+), 331 deletions(-)

-- 
2.41.0

[PATCH 01/10] fbdev/broadsheetfb: Select FB_SYS_HELPERS_DEFERRED

[Thomas Zimmermann]

The Kconfig token FB_SYS_HELPERS_DEFERRED selects everything that
is required for deferred I/O on system-memory framebuffers. Select
it from FB_BROADSHEET in favor of the existing identical selection.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/Kconfig | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index f14229757311..fd862faafe66 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -2179,11 +2179,7 @@ config FB_MX3
 config FB_BROADSHEET
 	tristate "E-Ink Broadsheet/Epson S1D13521 controller support"
 	depends on FB && (ARCH_PXA || COMPILE_TEST)
-	select FB_SYS_FILLRECT
-	select FB_SYS_COPYAREA
-	select FB_SYS_IMAGEBLIT
-	select FB_SYS_FOPS
-	select FB_DEFERRED_IO
+	select FB_SYS_HELPERS_DEFERRED
 	help
 	  This driver implements support for the E-Ink Broadsheet
 	  controller. The release name for this device was Epson S1D13521
-- 
2.41.0

Re: [PATCH 01/10] fbdev/broadsheetfb: Select FB_SYS_HELPERS_DEFERRED

[Helge Deller]

On 7/6/23 17:08, Thomas Zimmermann wrote:
> The Kconfig token FB_SYS_HELPERS_DEFERRED selects everything that
> is required for deferred I/O on system-memory framebuffers. Select
> it from FB_BROADSHEET in favor of the existing identical selection.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

I'm actually not a fan of deferred-IO for older drivers, but
in this case where the drivers operate on system memory anyway,
this is the right approach.

The series looks good:

Acked-by: Helge Deller <deller@gmx.de>

Helge

> ---
>   drivers/video/fbdev/Kconfig | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index f14229757311..fd862faafe66 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -2179,11 +2179,7 @@ config FB_MX3
>   config FB_BROADSHEET
>   	tristate "E-Ink Broadsheet/Epson S1D13521 controller support"
>   	depends on FB && (ARCH_PXA || COMPILE_TEST)
> -	select FB_SYS_FILLRECT
> -	select FB_SYS_COPYAREA
> -	select FB_SYS_IMAGEBLIT
> -	select FB_SYS_FOPS
> -	select FB_DEFERRED_IO
> +	select FB_SYS_HELPERS_DEFERRED
>   	help
>   	  This driver implements support for the E-Ink Broadsheet
>   	  controller. The release name for this device was Epson S1D13521

Re: [PATCH 01/10] fbdev/broadsheetfb: Select FB_SYS_HELPERS_DEFERRED

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 2075 bytes --]

Hi

Am 06.07.23 um 21:13 schrieb Helge Deller:
> On 7/6/23 17:08, Thomas Zimmermann wrote:
>> The Kconfig token FB_SYS_HELPERS_DEFERRED selects everything that
>> is required for deferred I/O on system-memory framebuffers. Select
>> it from FB_BROADSHEET in favor of the existing identical selection.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> I'm actually not a fan of deferred-IO for older drivers, but
> in this case where the drivers operate on system memory anyway,
> this is the right approach.

Yeah, deferred I/O has a certain overhead. But for this kind of hardware 
there's no good way to make mmap work reliably without. (Well, there's 
msync(), but who uses that?) These drivers already implement defio, so 
it's not really a change, just a cleanup.

> 
> The series looks good:
> 
> Acked-by: Helge Deller <deller@gmx.de>

Thanks a lot.

Best regards
Thomas

> 
> Helge
> 
>> ---
>>   drivers/video/fbdev/Kconfig | 6 +-----
>>   1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
>> index f14229757311..fd862faafe66 100644
>> --- a/drivers/video/fbdev/Kconfig
>> +++ b/drivers/video/fbdev/Kconfig
>> @@ -2179,11 +2179,7 @@ config FB_MX3
>>   config FB_BROADSHEET
>>       tristate "E-Ink Broadsheet/Epson S1D13521 controller support"
>>       depends on FB && (ARCH_PXA || COMPILE_TEST)
>> -    select FB_SYS_FILLRECT
>> -    select FB_SYS_COPYAREA
>> -    select FB_SYS_IMAGEBLIT
>> -    select FB_SYS_FOPS
>> -    select FB_DEFERRED_IO
>> +    select FB_SYS_HELPERS_DEFERRED
>>       help
>>         This driver implements support for the E-Ink Broadsheet
>>         controller. The release name for this device was Epson S1D13521
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[PATCH 02/10] fbdev/broadsheetfb: Generate deferred I/O ops

[Thomas Zimmermann]

Use the existing generator macros to create deferred-I/O helpers
for broadsheetfb and set them in the fb_ops structure. Functions
for damage handling on memory ranges and areas are provided by
the driver.

Broadsheedfb's implementation of fb_write writes to system memory,
so the generated code can use the respective helper internally.
This also fixes a long-standing bug where fb_write returned an
errno code instead of the number of written bytes. See the commit
message of commit 921b7383f348 ("fbdev: Return number of bytes
read or written") for more details.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/broadsheetfb.c | 78 +++---------------------------
 1 file changed, 8 insertions(+), 70 deletions(-)

diff --git a/drivers/video/fbdev/broadsheetfb.c b/drivers/video/fbdev/broadsheetfb.c
index 5a5fe4bbc10b..cb725a91b6bb 100644
--- a/drivers/video/fbdev/broadsheetfb.c
+++ b/drivers/video/fbdev/broadsheetfb.c
@@ -970,90 +970,28 @@ static void broadsheetfb_dpy_deferred_io(struct fb_info *info, struct list_head
 	}
 }
 
-static void broadsheetfb_fillrect(struct fb_info *info,
-				   const struct fb_fillrect *rect)
+static void broadsheetfb_defio_damage_range(struct fb_info *info, off_t off, size_t len)
 {
 	struct broadsheetfb_par *par = info->par;
 
-	sys_fillrect(info, rect);
-
 	broadsheetfb_dpy_update(par);
 }
 
-static void broadsheetfb_copyarea(struct fb_info *info,
-				   const struct fb_copyarea *area)
+static void broadsheetfb_defio_damage_area(struct fb_info *info, u32 x, u32 y,
+					   u32 width, u32 height)
 {
 	struct broadsheetfb_par *par = info->par;
 
-	sys_copyarea(info, area);
-
 	broadsheetfb_dpy_update(par);
 }
 
-static void broadsheetfb_imageblit(struct fb_info *info,
-				const struct fb_image *image)
-{
-	struct broadsheetfb_par *par = info->par;
-
-	sys_imageblit(info, image);
-
-	broadsheetfb_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 broadsheetfb_write(struct fb_info *info, const char __user *buf,
-				size_t count, loff_t *ppos)
-{
-	struct broadsheetfb_par *par = info->par;
-	unsigned long p = *ppos;
-	void *dst;
-	int err = 0;
-	unsigned long total_size;
-
-	if (!info->screen_buffer)
-		return -ENODEV;
-
-	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 = info->screen_buffer + p;
-
-	if (copy_from_user(dst, buf, count))
-		err = -EFAULT;
-
-	if  (!err)
-		*ppos += count;
-
-	broadsheetfb_dpy_update(par);
-
-	return (err) ? err : count;
-}
+FB_GEN_DEFAULT_DEFERRED_SYS_OPS(broadsheetfb,
+				broadsheetfb_defio_damage_range,
+				broadsheetfb_defio_damage_area)
 
 static const struct fb_ops broadsheetfb_ops = {
-	.owner		= THIS_MODULE,
-	.fb_read        = fb_sys_read,
-	.fb_write	= broadsheetfb_write,
-	.fb_fillrect	= broadsheetfb_fillrect,
-	.fb_copyarea	= broadsheetfb_copyarea,
-	.fb_imageblit	= broadsheetfb_imageblit,
-	.fb_mmap	= fb_deferred_io_mmap,
+	.owner	= THIS_MODULE,
+	FB_DEFAULT_DEFERRED_OPS(broadsheetfb),
 };
 
 static struct fb_deferred_io broadsheetfb_defio = {
-- 
2.41.0

[PATCH 03/10] fbdev/hecubafb: Select FB_SYS_HELPERS_DEFERRED

[Thomas Zimmermann]

The Kconfig token FB_SYS_HELPERS_DEFERRED selects everything that
is required for deferred I/O on system-memory framebuffers. Select
it from FB_HECUBA.

Deferred I/O helpers were previously selected by n411, which builds
upon hecubafb. Remove these select statements in favor of the new one.
N411 does not implement any framebuffer I/O by itself.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/Kconfig | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index fd862faafe66..7587f06cb793 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -200,7 +200,7 @@ config FB_SYS_HELPERS_DEFERRED
 config FB_HECUBA
 	tristate
 	depends on FB
-	depends on FB_DEFERRED_IO
+	select FB_SYS_HELPERS_DEFERRED
 
 config FB_SVGALIB
 	tristate
@@ -691,11 +691,6 @@ config FB_EFI
 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
-- 
2.41.0

[PATCH 04/10] fbdev/hecubafb: Generate deferred I/O ops

[Thomas Zimmermann]

Use the existing generator macros to create deferred-I/O helpers
for hecubafb and set them in the fb_ops structure. Functions
for damage handling on memory ranges and areas are provided by
the driver.

Hecubafb's implementation of fb_write writes to system memory,
so the generated code can use the respective helper internally.
This also fixes a long-standing bug where fb_write returned an
errno code instead of the number of written bytes. See the commit
message of commit 921b7383f348 ("fbdev: Return number of bytes
read or written") for more details.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/hecubafb.c | 78 ++++------------------------------
 1 file changed, 8 insertions(+), 70 deletions(-)

diff --git a/drivers/video/fbdev/hecubafb.c b/drivers/video/fbdev/hecubafb.c
index 7ce0a16ce8b9..5043d08ade54 100644
--- a/drivers/video/fbdev/hecubafb.c
+++ b/drivers/video/fbdev/hecubafb.c
@@ -120,90 +120,28 @@ static void hecubafb_dpy_deferred_io(struct fb_info *info, struct list_head *pag
 	hecubafb_dpy_update(info->par);
 }
 
-static void hecubafb_fillrect(struct fb_info *info,
-				   const struct fb_fillrect *rect)
+static void hecubafb_defio_damage_range(struct fb_info *info, off_t off, size_t len)
 {
 	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)
+static void hecubafb_defio_damage_area(struct fb_info *info, u32 x, u32 y,
+				       u32 width, u32 height)
 {
 	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->screen_buffer)
-		return -ENODEV;
-
-	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 = info->screen_buffer + p;
-
-	if (copy_from_user(dst, buf, count))
-		err = -EFAULT;
-
-	if  (!err)
-		*ppos += count;
-
-	hecubafb_dpy_update(par);
-
-	return (err) ? err : count;
-}
+FB_GEN_DEFAULT_DEFERRED_SYS_OPS(hecubafb,
+				hecubafb_defio_damage_range,
+				hecubafb_defio_damage_area)
 
 static const 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_mmap	= fb_deferred_io_mmap,
+	.owner	= THIS_MODULE,
+	FB_DEFAULT_DEFERRED_OPS(hecubafb),
 };
 
 static struct fb_deferred_io hecubafb_defio = {
-- 
2.41.0

[PATCH 05/10] fbdev/metronomefb: Select FB_SYS_HELPERS_DEFERRED

[Thomas Zimmermann]

The Kconfig token FB_SYS_HELPERS_DEFERRED selects everything that
is required for deferred I/O on system-memory framebuffers. Select
it from FB_METRONOME in favor of the existing identical selection.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/Kconfig | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 7587f06cb793..1155d7aa0917 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -2088,11 +2088,7 @@ 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
+	select FB_SYS_HELPERS_DEFERRED
 	help
 	  This driver implements support for the E-Ink Metronome
 	  controller. The pre-release name for this device was 8track
-- 
2.41.0

[PATCH 06/10] fbdev/metronomefb: Generate deferred I/O ops

[Thomas Zimmermann]

Use the existing generator macros to create deferred-I/O helpers
for metronomefb and set them in the fb_ops structure. Functions
for damage handling on memory ranges and areas are provided by
the driver.

Metronomefb's implementation of fb_write writes to system memory,
so the generated code can use the respective helper internally.
This also fixes a long-standing bug where fb_write returned an
errno code instead of the number of written bytes. See the commit
message of commit 921b7383f348 ("fbdev: Return number of bytes
read or written") for more details.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/metronomefb.c | 74 ++++---------------------------
 1 file changed, 8 insertions(+), 66 deletions(-)

diff --git a/drivers/video/fbdev/metronomefb.c b/drivers/video/fbdev/metronomefb.c
index 3e1daca76e11..667bef10738c 100644
--- a/drivers/video/fbdev/metronomefb.c
+++ b/drivers/video/fbdev/metronomefb.c
@@ -483,86 +483,28 @@ static void metronomefb_dpy_deferred_io(struct fb_info *info, struct list_head *
 	metronome_display_cmd(par);
 }
 
-static void metronomefb_fillrect(struct fb_info *info,
-				   const struct fb_fillrect *rect)
+static void metronomefb_defio_damage_range(struct fb_info *info, off_t off, size_t len)
 {
 	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)
+static void metronomefb_defio_damage_area(struct fb_info *info, u32 x, u32 y,
+					  u32 width, u32 height)
 {
 	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->screen_buffer)
-		return -ENODEV;
-
-	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 = info->screen_buffer + p;
-
-	if (copy_from_user(dst, buf, count))
-		err = -EFAULT;
-
-	if  (!err)
-		*ppos += count;
-
-	metronomefb_dpy_update(par);
-
-	return (err) ? err : count;
-}
+FB_GEN_DEFAULT_DEFERRED_SYS_OPS(metronomefb,
+				metronomefb_defio_damage_range,
+				metronomefb_defio_damage_area)
 
 static const 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_mmap	= fb_deferred_io_mmap,
+	.owner	= THIS_MODULE,
+	FB_DEFAULT_DEFERRED_OPS(metronomefb),
 };
 
 static struct fb_deferred_io metronomefb_defio = {
-- 
2.41.0

[PATCH 07/10] fbdev/ssd1307fb: Select FB_SYS_HELPERS_DEFERRED

[Thomas Zimmermann]

The Kconfig token FB_SYS_HELPERS_DEFERRED selects everything that
is required for deferred I/O on system-memory framebuffers. Select
it from FB_SSD1307 in favor of the existing identical selection.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/Kconfig | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 1155d7aa0917..763316acfc02 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -2211,12 +2211,8 @@ config FB_SSD1307
 	tristate "Solomon SSD1307 framebuffer support"
 	depends on FB && I2C
 	depends on GPIOLIB || COMPILE_TEST
-	select FB_SYS_FOPS
-	select FB_SYS_FILLRECT
-	select FB_SYS_COPYAREA
-	select FB_SYS_IMAGEBLIT
-	select FB_DEFERRED_IO
 	select FB_BACKLIGHT
+	select FB_SYS_HELPERS_DEFERRED
 	help
 	  This driver implements support for the Solomon SSD1307
 	  OLED controller over I2C.
-- 
2.41.0

[PATCH 08/10] fbdev/ssd1307fb: Generate deferred I/O ops

[Thomas Zimmermann]

Use the existing generator macros to create deferred-I/O helpers
for ssd1307fb and set them in the fb_ops structure. Functions
for damage handling on memory ranges and areas are provided by
the driver.

Ssd1307fb's implementation of fb_write writes to system memory,
so the generated code can use the respective helper internally.
This also fixes a long-standing bug where fb_write returned an
errno code instead of the number of written bytes. See the commit
message of commit 921b7383f348 ("fbdev: Return number of bytes
read or written") for more details.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/ssd1307fb.c | 69 ++++++---------------------------
 1 file changed, 11 insertions(+), 58 deletions(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 11c373798279..a2b939342a4f 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -292,43 +292,6 @@ static int ssd1307fb_update_display(struct ssd1307fb_par *par)
 	return ssd1307fb_update_rect(par, 0, 0, par->width, par->height);
 }
 
-static ssize_t ssd1307fb_write(struct fb_info *info, const char __user *buf,
-		size_t count, loff_t *ppos)
-{
-	struct ssd1307fb_par *par = info->par;
-	unsigned long total_size;
-	unsigned long p = *ppos;
-	void *dst;
-	int ret;
-
-	if (!info->screen_buffer)
-		return -ENODEV;
-
-	total_size = info->fix.smem_len;
-
-	if (p > total_size)
-		return -EINVAL;
-
-	if (count + p > total_size)
-		count = total_size - p;
-
-	if (!count)
-		return -EINVAL;
-
-	dst = info->screen_buffer + p;
-
-	if (copy_from_user(dst, buf, count))
-		return -EFAULT;
-
-	ret = ssd1307fb_update_display(par);
-	if (ret < 0)
-		return ret;
-
-	*ppos += count;
-
-	return count;
-}
-
 static int ssd1307fb_blank(int blank_mode, struct fb_info *info)
 {
 	struct ssd1307fb_par *par = info->par;
@@ -339,39 +302,29 @@ static int ssd1307fb_blank(int blank_mode, struct fb_info *info)
 		return ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_ON);
 }
 
-static void ssd1307fb_fillrect(struct fb_info *info, const struct fb_fillrect *rect)
+static void ssd1307fb_defio_damage_range(struct fb_info *info, off_t off, size_t len)
 {
 	struct ssd1307fb_par *par = info->par;
-	sys_fillrect(info, rect);
-	ssd1307fb_update_rect(par, rect->dx, rect->dy, rect->width,
-			      rect->height);
-}
 
-static void ssd1307fb_copyarea(struct fb_info *info, const struct fb_copyarea *area)
-{
-	struct ssd1307fb_par *par = info->par;
-	sys_copyarea(info, area);
-	ssd1307fb_update_rect(par, area->dx, area->dy, area->width,
-			      area->height);
+	ssd1307fb_update_display(par);
 }
 
-static void ssd1307fb_imageblit(struct fb_info *info, const struct fb_image *image)
+static void ssd1307fb_defio_damage_area(struct fb_info *info, u32 x, u32 y,
+					u32 width, u32 height)
 {
 	struct ssd1307fb_par *par = info->par;
-	sys_imageblit(info, image);
-	ssd1307fb_update_rect(par, image->dx, image->dy, image->width,
-			      image->height);
+
+	ssd1307fb_update_rect(par, x, y, width, height);
 }
 
+FB_GEN_DEFAULT_DEFERRED_SYS_OPS(ssd1307fb,
+				ssd1307fb_defio_damage_range,
+				ssd1307fb_defio_damage_area)
+
 static const struct fb_ops ssd1307fb_ops = {
 	.owner		= THIS_MODULE,
-	.fb_read	= fb_sys_read,
-	.fb_write	= ssd1307fb_write,
+	FB_DEFAULT_DEFERRED_OPS(ssd1307fb),
 	.fb_blank	= ssd1307fb_blank,
-	.fb_fillrect	= ssd1307fb_fillrect,
-	.fb_copyarea	= ssd1307fb_copyarea,
-	.fb_imageblit	= ssd1307fb_imageblit,
-	.fb_mmap	= fb_deferred_io_mmap,
 };
 
 static void ssd1307fb_deferred_io(struct fb_info *info, struct list_head *pagereflist)
-- 
2.41.0

Re: [PATCH 08/10] fbdev/ssd1307fb: Generate deferred I/O ops

[Sam Ravnborg]

On Thu, Jul 06, 2023 at 05:08:51PM +0200, Thomas Zimmermann wrote:
> Use the existing generator macros to create deferred-I/O helpers
> for ssd1307fb and set them in the fb_ops structure. Functions
> for damage handling on memory ranges and areas are provided by
> the driver.
> 
> Ssd1307fb's implementation of fb_write writes to system memory,
> so the generated code can use the respective helper internally.
> This also fixes a long-standing bug where fb_write returned an
> errno code instead of the number of written bytes. See the commit
> message of commit 921b7383f348 ("fbdev: Return number of bytes
> read or written") for more details.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/video/fbdev/ssd1307fb.c | 69 ++++++---------------------------
>  1 file changed, 11 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
> index 11c373798279..a2b939342a4f 100644
> --- a/drivers/video/fbdev/ssd1307fb.c
> +++ b/drivers/video/fbdev/ssd1307fb.c
> @@ -292,43 +292,6 @@ static int ssd1307fb_update_display(struct ssd1307fb_par *par)
>  	return ssd1307fb_update_rect(par, 0, 0, par->width, par->height);
>  }
>  
> -static ssize_t ssd1307fb_write(struct fb_info *info, const char __user *buf,
> -		size_t count, loff_t *ppos)
> -{
> -	struct ssd1307fb_par *par = info->par;
> -	unsigned long total_size;
> -	unsigned long p = *ppos;
> -	void *dst;
> -	int ret;
> -
> -	if (!info->screen_buffer)
> -		return -ENODEV;
> -
> -	total_size = info->fix.smem_len;
> -
> -	if (p > total_size)
> -		return -EINVAL;
> -
> -	if (count + p > total_size)
> -		count = total_size - p;
> -
> -	if (!count)
> -		return -EINVAL;
> -
> -	dst = info->screen_buffer + p;
> -
> -	if (copy_from_user(dst, buf, count))
> -		return -EFAULT;
> -
> -	ret = ssd1307fb_update_display(par);
> -	if (ret < 0)
> -		return ret;
> -
> -	*ppos += count;
> -
> -	return count;
> -}
> -
>  static int ssd1307fb_blank(int blank_mode, struct fb_info *info)
>  {
>  	struct ssd1307fb_par *par = info->par;
> @@ -339,39 +302,29 @@ static int ssd1307fb_blank(int blank_mode, struct fb_info *info)
>  		return ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_ON);
>  }
>  
> -static void ssd1307fb_fillrect(struct fb_info *info, const struct fb_fillrect *rect)
> +static void ssd1307fb_defio_damage_range(struct fb_info *info, off_t off, size_t len)
>  {
>  	struct ssd1307fb_par *par = info->par;
> -	sys_fillrect(info, rect);
> -	ssd1307fb_update_rect(par, rect->dx, rect->dy, rect->width,
> -			      rect->height);
> -}
>  
> -static void ssd1307fb_copyarea(struct fb_info *info, const struct fb_copyarea *area)
> -{
> -	struct ssd1307fb_par *par = info->par;
> -	sys_copyarea(info, area);
> -	ssd1307fb_update_rect(par, area->dx, area->dy, area->width,
> -			      area->height);
> +	ssd1307fb_update_display(par);
>  }
>  
> -static void ssd1307fb_imageblit(struct fb_info *info, const struct fb_image *image)
> +static void ssd1307fb_defio_damage_area(struct fb_info *info, u32 x, u32 y,
> +					u32 width, u32 height)
>  {
>  	struct ssd1307fb_par *par = info->par;
> -	sys_imageblit(info, image);
> -	ssd1307fb_update_rect(par, image->dx, image->dy, image->width,
> -			      image->height);
> +
> +	ssd1307fb_update_rect(par, x, y, width, height);
>  }
>  
> +FB_GEN_DEFAULT_DEFERRED_SYS_OPS(ssd1307fb,
> +				ssd1307fb_defio_damage_range,
> +				ssd1307fb_defio_damage_area)
> +
>  static const struct fb_ops ssd1307fb_ops = {
>  	.owner		= THIS_MODULE,

Nitpick..
In other patches the tab after the equal sign is replaced by a single
space.

	Sam

> -	.fb_read	= fb_sys_read,
> -	.fb_write	= ssd1307fb_write,
> +	FB_DEFAULT_DEFERRED_OPS(ssd1307fb),
>  	.fb_blank	= ssd1307fb_blank,
> -	.fb_fillrect	= ssd1307fb_fillrect,
> -	.fb_copyarea	= ssd1307fb_copyarea,
> -	.fb_imageblit	= ssd1307fb_imageblit,
> -	.fb_mmap	= fb_deferred_io_mmap,
>  };
>  
>  static void ssd1307fb_deferred_io(struct fb_info *info, struct list_head *pagereflist)
> -- 
> 2.41.0

[PATCH 09/10] fbdev/xen-fbfront: Select FB_SYS_HELPERS_DEFERRED

[Thomas Zimmermann]

The Kconfig token FB_SYS_HELPERS_DEFERRED selects everything that
is required for deferred I/O on system-memory framebuffers. Select
it from XEN_FBDEV_FRONTEND in favor of the existing identical selection.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/Kconfig | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 763316acfc02..ec2abe882ed9 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -2073,11 +2073,7 @@ 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
+	select FB_SYS_HELPERS_DEFERRED
 	select XEN_XENBUS_FRONTEND
 	default y
 	help
-- 
2.41.0

[PATCH 10/10] fbdev/xen-fbfront: Generate deferred I/O ops

[Thomas Zimmermann]

Use the existing generator macros to create deferred-I/O helpers
for xen-fbfront and set them in the fb_ops structure. Functions
for damage handling on memory ranges and areas are provided by
the driver.

Xen-fbfront's implementation of fb_write writes to system memory,
so the generated code can use the respective helper internally.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/xen-fbfront.c | 61 ++++++++++---------------------
 1 file changed, 20 insertions(+), 41 deletions(-)

diff --git a/drivers/video/fbdev/xen-fbfront.c b/drivers/video/fbdev/xen-fbfront.c
index 9b2a786621a6..6664dc7a5a41 100644
--- a/drivers/video/fbdev/xen-fbfront.c
+++ b/drivers/video/fbdev/xen-fbfront.c
@@ -240,41 +240,6 @@ static int xenfb_setcolreg(unsigned regno, unsigned red, unsigned green,
 	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)
 {
@@ -326,17 +291,31 @@ static int xenfb_set_par(struct fb_info *info)
 	return 0;
 }
 
+static void xenfb_defio_damage_range(struct fb_info *info, off_t off, size_t len)
+{
+	struct xenfb_info *xenfb_info = info->par;
+
+	xenfb_refresh(xenfb_info, 0, 0, xenfb_info->page->width, xenfb_info->page->height);
+}
+
+static void xenfb_defio_damage_area(struct fb_info *info, u32 x, u32 y,
+				    u32 width, u32 height)
+{
+	struct xenfb_info *xenfb_info = info->par;
+
+	xenfb_refresh(xenfb_info, x, y, width, height);
+}
+
+FB_GEN_DEFAULT_DEFERRED_SYS_OPS(xenfb,
+				xenfb_defio_damage_range,
+				xenfb_defio_damage_area)
+
 static const struct fb_ops xenfb_fb_ops = {
 	.owner		= THIS_MODULE,
-	.fb_read	= fb_sys_read,
-	.fb_write	= xenfb_write,
+	FB_DEFAULT_DEFERRED_OPS(xenfb),
 	.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,
-	.fb_mmap	= fb_deferred_io_mmap,
 };
 
 static irqreturn_t xenfb_event_handler(int rq, void *dev_id)
-- 
2.41.0

Re: [PATCH 00/10] fbdev: Generate deferred-I/O helpers

[Sam Ravnborg]

Hi Thomas,

On Thu, Jul 06, 2023 at 05:08:43PM +0200, Thomas Zimmermann wrote:
> Generate the I/O callbacks for drivers with deferred I/O. As in
> the old, opencoded functions, the generated functions operate on
> system memory and trigger damage handling if necessary. Also bring
> the drivers' Kconfig options up to date.
> 
> Generating and initializing via helpers macros will later allow for
> a fine-grained setup, depending on Kconfig options. For example, it
> will be possible to leave out file I/O if FB_DEVICE has not been set.
> 
> Thomas Zimmermann (10):
>   fbdev/broadsheetfb: Select FB_SYS_HELPERS_DEFERRED
>   fbdev/broadsheetfb: Generate deferred I/O ops
>   fbdev/hecubafb: Select FB_SYS_HELPERS_DEFERRED
>   fbdev/hecubafb: Generate deferred I/O ops
>   fbdev/metronomefb: Select FB_SYS_HELPERS_DEFERRED
>   fbdev/metronomefb: Generate deferred I/O ops
>   fbdev/ssd1307fb: Select FB_SYS_HELPERS_DEFERRED
>   fbdev/ssd1307fb: Generate deferred I/O ops
>   fbdev/xen-fbfront: Select FB_SYS_HELPERS_DEFERRED
>   fbdev/xen-fbfront: Generate deferred I/O ops

Browsed the full series - nice reduction in complexity all over.
Everything looked good so:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>