Re: [PATCH 2/5] fbdev/core: Export framebuffer read and write code as cfb_ function

[daniel@ffwll.ch]

On Wed, Jul 29, 2020 at 03:41:45PM +0200, Thomas Zimmermann wrote:
> DRM fb helpers require read and write functions for framebuffer
> memory. Export the existing code from fbdev.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Hm I'm not super sure whether we want to actually reuse this stuff ... We
kinda don't care about the sparc special case, and just having an fbdev
implementation witch has the switch between memcpy and memcpy_to/from_io
in one single place sounds a lot simpler ...

This way we can have a clean split between the old horrors of real fbdev
drivers, and a much cleaner world in drm. It would mean a bit of
copypasting, but I think that's actually a good thing.

In general my idea for drm fbdev emulation is that for any area we have a
problem we just ignore the entire fbmem.c code and write our own: mmap,
backlight handling (still unsolved, and horrible), cfb vs sys here. This
entire fbmem.c stuff is pretty bad midlayer, trying to avoid code
duplication here doesn't seem worth it imo.

Thoughts?
-Daniel

> ---
>  drivers/video/fbdev/core/fbmem.c | 53 ++++++++++++++++++++++----------
>  include/linux/fb.h               |  5 +++
>  2 files changed, 41 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index dd0ccf35f7b7..b496ff90db3e 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -759,25 +759,18 @@ static struct fb_info *file_fb_info(struct file *file)
>  	return info;
>  }
>  
> -static ssize_t
> -fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
> +ssize_t fb_cfb_read(struct fb_info *info, char __user *buf, size_t count,
> +		    loff_t *ppos)
>  {
>  	unsigned long p = *ppos;
> -	struct fb_info *info = file_fb_info(file);
>  	u8 *buffer, *dst;
>  	u8 __iomem *src;
>  	int c, cnt = 0, err = 0;
>  	unsigned long total_size;
>  
> -	if (!info || ! info->screen_base)
> -		return -ENODEV;
> -
>  	if (info->state != FBINFO_STATE_RUNNING)
>  		return -EPERM;
>  
> -	if (info->fbops->fb_read)
> -		return info->fbops->fb_read(info, buf, count, ppos);
> -
>  	total_size = info->screen_size;
>  
>  	if (total_size == 0)
> @@ -823,16 +816,12 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
>  
>  	return (err) ? err : cnt;
>  }
> +EXPORT_SYMBOL(fb_cfb_read);
>  
>  static ssize_t
> -fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
> +fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
>  {
> -	unsigned long p = *ppos;
>  	struct fb_info *info = file_fb_info(file);
> -	u8 *buffer, *src;
> -	u8 __iomem *dst;
> -	int c, cnt = 0, err = 0;
> -	unsigned long total_size;
>  
>  	if (!info || !info->screen_base)
>  		return -ENODEV;
> @@ -840,8 +829,20 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
>  	if (info->state != FBINFO_STATE_RUNNING)
>  		return -EPERM;
>  
> -	if (info->fbops->fb_write)
> -		return info->fbops->fb_write(info, buf, count, ppos);
> +	if (info->fbops->fb_read)
> +		return info->fbops->fb_read(info, buf, count, ppos);
> +	else
> +		return fb_cfb_read(info, buf, count, ppos);
> +}
> +
> +ssize_t fb_cfb_write(struct fb_info *info, const char __user *buf,
> +		     size_t count, loff_t *ppos)
> +{
> +	unsigned long p = *ppos;
> +	u8 *buffer, *src;
> +	u8 __iomem *dst;
> +	int c, cnt = 0, err = 0;
> +	unsigned long total_size;
>  
>  	total_size = info->screen_size;
>  
> @@ -895,6 +896,24 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
>  
>  	return (cnt) ? cnt : err;
>  }
> +EXPORT_SYMBOL(fb_cfb_write);
> +
> +static ssize_t
> +fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
> +{
> +	struct fb_info *info = file_fb_info(file);
> +
> +	if (!info || !info->screen_base)
> +		return -ENODEV;
> +
> +	if (info->state != FBINFO_STATE_RUNNING)
> +		return -EPERM;
> +
> +	if (info->fbops->fb_write)
> +		return info->fbops->fb_write(info, buf, count, ppos);
> +	else
> +		return fb_cfb_write(info, buf, count, ppos);
> +}
>  
>  int
>  fb_pan_display(struct fb_info *info, struct fb_var_screeninfo *var)
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 714187bc13ac..12ad83963db5 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -593,6 +593,11 @@ extern int fb_blank(struct fb_info *info, int blank);
>  extern void cfb_fillrect(struct fb_info *info, const struct fb_fillrect *rect);
>  extern void cfb_copyarea(struct fb_info *info, const struct fb_copyarea *area);
>  extern void cfb_imageblit(struct fb_info *info, const struct fb_image *image);
> +extern ssize_t fb_cfb_read(struct fb_info *info, char __user *buf,
> +			   size_t count, loff_t *ppos);
> +extern ssize_t fb_cfb_write(struct fb_info *info, const char __user *buf,
> +			    size_t count, loff_t *ppos);
> +
>  /*
>   * Drawing operations where framebuffer is in system RAM
>   */
> -- 
> 2.27.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel