[Intel-gfx] [PATCH libdrm] drm: Remove create_handle() drm_framebuffer "virtual".

[Emil Velikov]

On 10 August 2017 at 20:29, Joe Kniss <djmk at google.com> wrote:
> On Wed, Aug 9, 2017 at 4:13 PM, Joe Kniss <djmk at google.com> wrote:
>> On Wed, Aug 9, 2017 at 12:14 PM, Noralf Trønnes <noralf at tronnes.org> wrote:
>>>
>>> Den 09.08.2017 01.42, skrev Joe Kniss:
>>>>
>>>> Because all drivers currently use gem objects for framebuffer planes,
>>>> the virtual create_handle() is not required.  This change adds a
>>>> struct drm_gem_object *gems[4] field to drm_framebuffer and removes
>>>> create_handle() function pointer from drm_framebuffer_funcs.  The
>>>> corresponding *_create_handle() function is removed from each driver.
>>>>
>>>> In many cases this change eliminates a struct *_framebuffer object,
>>>> as the only need for the derived struct is the addition of the gem
>>>> object pointer.
>>>>
>>>> TESTED: compiled: allyesconfig ARCH=x86,arm platforms:i915, rockchip
>>>>
>>>> Signed-off-by: Joe Kniss <djmk at google.com>
>>>> ---
>>>
>>>
>>> Hi Joe,
>>>
>>> I'm also looking into adding gem objs to drm_framebuffer in this patch:
>>> [PATCH v2 01/22] drm: Add GEM backed framebuffer library
>>> https://lists.freedesktop.org/archives/dri-devel/2017-August/149782.html
>>>
>>
>> Great.  There's only minimal overlap here.  I'll rebase this change on yours
>> once it's in.
>>
>>> [...]
>>>
>>>> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
>>>> b/drivers/gpu/drm/drm_fb_cma_helper.c
>>>> index ade319d10e70..f5f011b910b1 100644
>>>> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
>>>> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
>>>> @@ -31,14 +31,9 @@
>>>>     #define DEFAULT_FBDEFIO_DELAY_MS 50
>>>>   -struct drm_fb_cma {
>>>> -       struct drm_framebuffer          fb;
>>>> -       struct drm_gem_cma_object       *obj[4];
>>>> -};
>>>> -
>>>>   struct drm_fbdev_cma {
>>>>         struct drm_fb_helper    fb_helper;
>>>> -       struct drm_fb_cma       *fb;
>>>> +       struct drm_framebuffer  *fb;
>>>
>>>
>>> This fb pointer isn't necessary, since fb_helper already has one.
>>>
>
> So, looking deeper into this, it seems that the struct
> drm_framebuffer_funcs *fb_funcs is also redundant here?  In which case
> this whole struct can go...
>
I think you're spot on.

Perhaps the goal was to allow drivers to use separate funcs for fb vs fbdev?
Not sure how well that will fair, yet again, all current users use the
same funcs for both.

-Emil