Hi Thomas,
On Mon, Apr 25, 2022 at 07:26:32PM +0200, Sam Ravnborg wrote:
> 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.
Just to be clear - I already read:
"
Leave the mmap handling to drivers and expect them to call the
helper for deferred I/O by thmeselves.
"
But this does not help me understand why we need to explicit do what
could be a simple default implementation.
Chances are that I am stupid and it is obvious..
Sam
