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

[Benjamin Gaignard]

2016-12-07 15:35 GMT+01:00 Laurent Pinchart <laurent.pinchart at ideasonboard.com>:
> 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 at 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
>



-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog