2016-12-07 15:35 GMT+01:00 Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>: > Hi Benjamin, > > Thank you for the patch. > > 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... > >> version 4: >> - introdude a configuration flag to be independent of architecture >> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxx> >> --- >> 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 > -- Benjamin Gaignard Graphic Study Group Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel