[PATCH 02/23] drm: omapdrm: fb: Don't store format BPP for each plane

[Tomi Valkeinen]

On 09/05/16 23:55, Laurent Pinchart wrote:

>> To be honest, I'd rather go into more complex struct than simpler one.
>> The current one is already confusing, I think, and your version is too.
>> The main issue is that the sub_x is encoded into the stride_bpp. In
>> kmsxx I used this format:
>>
>> { PixelFormat::NV12, { 2, { { 8, 1, 1, }, { 8, 2, 2 } }, } },
>>
>> The first number is the number of planes, and for each plane, bitspp,
>> xsub and ysub. It's more verbose, but (I think) easier to understand.
> 
> I'm fine with that assuming we'd have a use for it :-) Shouldn't we aim for a 
> simpler structure to save memory by only storing the information we need, 
> compared to a more verbose structure that duplicates information or stores 
> data that we don't need ?

Well, my take is: if we save a few bytes by obfuscating the code, it's
not worth it. "Obfuscating" is, of course, relative, but a "stride_bpp"
is confusing to me. If it's a common term, I'm fine with using it and I
just need to learn it =).

Also, I like tables like these to be somewhat generic, so that if and
when we get new color formats, the whole table doesn't need to be rewritten.

That said, the kmsxx format above is not super clear either, as the
bitspp for the second plane is 8, i.e. it also has the sub_x encoded...
I don't remember why I made it like that.

So, in the end, all I know is that describing pixel formats well is not
easy =).

 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/20160510/e800ce7a/attachment-0001.sig>