[PATCH v4 01/14] drm: Centralize format information

[Tomi Valkeinen]

On 16/09/16 02:30, Laurent Pinchart wrote:

> I've checked the existing code that this patch series is replacing, and the 
> [ARGB]{4}8888 formats are currently reported as having a depth of 32. I'm not 
> sure why that's the case, but I'd rather not touch it in this patch. If this 
> is a bug we should fix it in a separate patch.

I agree, I don't want this series to be held up. But this depth is
clearly broken, in some way or another. Even more reason to move it to
fb code =).

>>> I'm not sure if it's worth the hassle, but if the depth is only for
>>> fbdev compat code, maybe a separate format->depth table in fbdev code
>>> (for the formats fbdev supports) would make this cleaner and avoid any
>>> future mistakes with new drm drivers.
>>
>> I agree actually, having it here will encourage anyone to use it. If you
>> don't want to split it out, at least add a comment along the lines of
>> your reply:
>>
>>>> This is used to implement the fbdev compatibility code, as fbdev (unlike
>>>> kms) makes use of that information.
> 
> I've double-checked the existing usage of the depth value, and it turns out 
> that quite a few drivers still use it for various purpose, through struct 
> drm_framebuffer.depth. I thus wonder whether splitting the depth value from 
> the format information table would really help, as drivers would have a way to 
> access it anyway.

Ok.

Btw, are you sure alpha is not counted into depth? With a quick glance,
also drm_mode_legacy_fb_format() seems to expect depth to include alpha.

I'm just thinking here that if "depth" is not clearly defined anywhere,
and the most common formats, 24bit RGB formats, are defined with depth
including alpha, well, maybe then that's how depth should be defined.

Then again, I had a quick glance at the fbdev code, and
fb_get_color_depth() suggests that alpha is not counted in...

 Tomi

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160916/30b0cc4a/attachment.sig>