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

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 1474 bytes --]

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


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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