On Thu, Sep 15, 2016 at 09:22:54AM +0300, Tomi Valkeinen wrote:
> On 15/09/16 01:22, Laurent Pinchart wrote:
> > No, the depth value is the number of colour bits, excluding the alpha bits.
> > This is used to implement the fbdev compatibility code, as fbdev (unlike
> > kms)
> > makes use of that information.
> >
> > The total number of bits per pixel is not stored in the table as it can be
> > computed by cpp[0]*8 + (cpp[1]+cpp[2])*8/hsub/vsub. For RGB formats, which
> > are
> > the only formats supported by the existing drm_fb_get_bpp_depth() function,
> > this simplifies to just cpp[0] * 8.
>
> Ok. Then the ARGB8888 & co. formats have depth wrong. I presumed those
> were right as they're the "normal" ones =).
Good catch, these should be 24 not 32.
I must admit I kinda skipped over that table the first time, and only
checked a few random values.
I just checked the whole table, and all the C and RGB formats are all
good (with the 4 /[ARGB]{4}8888/ formats set to .depth=24), and all the
YUV formats I know (~3/4) are good, but I don't know them all :)
>
> 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.
Cheers,
Eric
