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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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: signature.asc
Description: OpenPGP digital signature

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

[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux