Re: [PATCHv7 2/6] drm/core: Add drm_afbc_framebuffer and a corresponding helper

[Andrzej Pietrasiewicz <andrzej.p@collabora.com>]

Hi Daniel,

W dniu 30.03.2020 o 19:01, Daniel Vetter pisze:
> On Wed, Mar 11, 2020 at 3:55 PM Andrzej Pietrasiewicz
> <andrzej.p@collabora.com> wrote:
>>
>> The new struct contains afbc-specific data.

<snip>

>> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
>> index 439656f55c5d..37a3a023c114 100644
>> --- a/Documentation/gpu/todo.rst
>> +++ b/Documentation/gpu/todo.rst
>> @@ -404,6 +404,21 @@ Contact: Laurent Pinchart, respective driver maintainers
>>
>>   Level: Intermediate
>>
>> +Encode cpp properly in malidp
>> +-----------------------------
>> +
>> +cpp (chars per pixel) is not encoded properly in malidp, zero is
>> +used instead. afbc implementation needs bpp or cpp, but if it is
>> +zero it needs to be provided elsewhere, and so the bpp field exists
>> +in struct drm_afbc_framebuffer.
>> +
>> +Properly encode cpp in malidp and remove the bpp field in struct
>> +drm_afbc_framebuffer.
>> +
>> +Contact: malidp maintainers
>> +
>> +Level: Intermediate
> 
> Just stumbled over this todo, which is really surprising. Also
> definitely not something I wanted to ack, earlier versions at least
> didn't have this.
> 
> Why is this needed? drm_afbc_framebuffer contains a drm_framebuffer,
> which has a pointer to drm_format_info, which we're always setting
> (the core does that for all drivers, both for addfb and addfb2). Why
> is that not good enough to get at cpp for everyone?
> 
> Cheers, Daniel
> 

Let me quote James https://patchwork.freedesktop.org/patch/345603/#comment_653081:

"Seems we can remove this bpp or no need to define it as a pass in argument
for size check, maybe the komeda/malidp get_afbc_bpp() function mislead
you that afbc formats may have vendor specific bpp.

But the story is:

for afbc only formats like DRM_FORMAT_YUV420_8BIT/10BIT, we have set
nothing in drm_format_info, neither cpp nor block_size, so both malidp
or komeda introduce a get_bpp(), but actually the two funcs basically
are same.

So my suggestion is we can temporary use the get_afbc_bpp() in malidp
or komeda. and eventually I think we'd better set the block size
for these formats, then we can defines a common get_bpp() like pitch".

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