Re: [PATCH v4 2/4] fbmem: add a default get_fb_unmapped_area function

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Benjamin,

On Wednesday 07 Dec 2016 15:57:49 Benjamin Gaignard wrote:
> 2016-12-07 15:35 GMT+01:00 Laurent Pinchart:
> > On Wednesday 07 Dec 2016 11:06:49 Benjamin Gaignard wrote:
> >> Allow generic frame-buffer to provide a default
> >> get_fb_unmapped_area function if specific devices need it.
> >> 
> >> Usually this function is defined in architecture directories but
> >> define it here may limit code duplication especially for all ARM
> >> platforms without MMU.
> > 
> > I still would like to see an explanation why an architecture-specific
> > version is sometimes needed, and why this implementation is a reasonable
> > default. Furthermore, it looks very similar to the blackfin
> > implementation, so if you can't unify the two I'd like to know why.
> 
> Obviously I don't know all the legacy behind this function but it is
> definitively link to architectures memory mapping strategies in no MMU
> cases.
> 
> I think this function is a reasonable default when the architecture is
> doing a direct mapping (no translation) of the memory like it is done in
> ARM.
> 
> Unlike what I propose blackfin implementation doesn't do any check on pgoff
> and length and always return fb base address even is an offset has been
> requested. I don't know if it is on purpose or just because nobody has
> never try to mmap blackfin framebuffer with an offset...

And that's exactly what I'd like you to try and find out :-) git blame and 
contacting the original authors of that code could be a first step.

> >> version 4:
> >> - introdude a configuration flag to be independent of architecture
> >> 
> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> >> ---
> >> 
> >>  drivers/video/fbdev/Kconfig      |  8 ++++++++
> >>  drivers/video/fbdev/core/fbmem.c | 15 +++++++++++++++
> >>  2 files changed, 23 insertions(+)
> >> 
> >> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> >> index 5d3b0db..922e4ea 100644
> >> --- a/drivers/video/fbdev/Kconfig
> >> +++ b/drivers/video/fbdev/Kconfig
> >> @@ -138,6 +138,14 @@ config FB_SYS_IMAGEBLIT
> >> 
> >>         blitting. This is used by drivers that don't provide their own
> >>         (accelerated) version and the framebuffer is in system RAM.
> >> 
> >> +config FB_PROVIDE_GET_FB_UNMAPPED_AREA
> >> +     bool
> >> +     depends on FB
> >> +     default n
> >> +     ---help---
> >> +       Allow generic frame-buffer to provide get_fb_unmapped_area
> >> +       function.
> >> +
> >> 
> >>  menuconfig FB_FOREIGN_ENDIAN
> >>  
> >>       bool "Framebuffer foreign endianness support"
> >>       depends on FB
> >> 
> >> diff --git a/drivers/video/fbdev/core/fbmem.c
> >> b/drivers/video/fbdev/core/fbmem.c index 76c1ad9..22321a2 100644
> >> --- a/drivers/video/fbdev/core/fbmem.c
> >> +++ b/drivers/video/fbdev/core/fbmem.c
> >> @@ -1492,6 +1492,21 @@ static long fb_compat_ioctl(struct file *file,
> >> unsigned int cmd, return 0;
> >> 
> >>  }
> >> 
> >> +#ifdef CONFIG_FB_PROVIDE_GET_FB_UNMAPPED_AREA
> >> +unsigned long get_fb_unmapped_area(struct file *filp,
> >> +                                unsigned long addr, unsigned long len,
> >> +                                unsigned long pgoff, unsigned long
> >> flags)
> >> +{
> >> +     struct fb_info * const info = filp->private_data;
> >> +     unsigned long fb_size = PAGE_ALIGN(info->fix.smem_len);
> >> +
> >> +     if (pgoff > fb_size || len > fb_size - pgoff)
> >> +             return -EINVAL;
> >> +
> >> +     return (unsigned long)info->screen_base + pgoff;
> >> +}
> >> +#endif
> >> +
> >> 
> >>  static const struct file_operations fb_fops = {
> >>  
> >>       .owner =        THIS_MODULE,
> >>       .read =         fb_read,
> > 
> > --
> > Regards,
> > 
> > Laurent Pinchart

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel