Re: [PATCH v2 1/3] fbdev: Put mmap for deferred I/O into drivers

[Sam Ravnborg <sam@ravnborg.org>]

Hi Thomas,

> diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
> index 6aaf6d0abf39..6924d489a289 100644
> --- a/drivers/video/fbdev/core/fb_defio.c
> +++ b/drivers/video/fbdev/core/fb_defio.c
> @@ -181,6 +181,7 @@ int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma)
>  	vma->vm_private_data = info;
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(fb_deferred_io_mmap);
>  
>  /* workqueue callback */
>  static void fb_deferred_io_work(struct work_struct *work)
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index 84427470367b..52440e3f8f69 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -1334,7 +1334,6 @@ static int
>  fb_mmap(struct file *file, struct vm_area_struct * vma)
>  {
>  	struct fb_info *info = file_fb_info(file);
> -	int (*fb_mmap_fn)(struct fb_info *info, struct vm_area_struct *vma);
>  	unsigned long mmio_pgoff;
>  	unsigned long start;
>  	u32 len;
> @@ -1343,14 +1342,7 @@ fb_mmap(struct file *file, struct vm_area_struct * vma)
>  		return -ENODEV;
>  	mutex_lock(&info->mm_lock);
>  
> -	fb_mmap_fn = info->fbops->fb_mmap;
> -
> -#if IS_ENABLED(CONFIG_FB_DEFERRED_IO)
> -	if (info->fbdefio)
> -		fb_mmap_fn = fb_deferred_io_mmap;
> -#endif
> -
> -	if (fb_mmap_fn) {
> +	if (info->fbops->fb_mmap) {
>  		int res;
>  
>  		/*
> @@ -1358,11 +1350,18 @@ fb_mmap(struct file *file, struct vm_area_struct * vma)
>  		 * SME protection is removed ahead of the call
>  		 */
>  		vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
> -		res = fb_mmap_fn(info, vma);
> +		res = info->fbops->fb_mmap(info, vma);
>  		mutex_unlock(&info->mm_lock);
>  		return res;
>  	}
>  
> +	/*
> +	 * FB deferred I/O wants you to handle mmap in your drivers. At a
> +	 * minimum, point struct fb_ops.fb_mmap to fb_deferred_io_mmap().
> +	 */
> +	if (dev_WARN_ONCE(info->dev, info->fbdefio, "fbdev mmap not set up for defered I/O.\n"))
> +		return -ENODEV;
> +

If not configured - then why not just call fb_deferred_io_mmap(), as
this seems to be the default implementation anyway.
Then drivers that needs it can override - and the rest fallback to the
default.

	Sam