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

[Sam Ravnborg <sam@ravnborg.org>]

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