Hi Daniel, On Wed, Aug 10, 2022 at 5:59 PM Daniel Vetter <daniel@xxxxxxxx> wrote: > On Fri, Jul 08, 2022 at 08:20:46PM +0200, Geert Uytterhoeven wrote: > > Add a helper to retrieve the actual number of bits per pixel for a > > plane, taking into account the number of characters and pixels per > > block for tiled formats. > > > > Signed-off-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> > > Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx> > > --- a/drivers/gpu/drm/drm_fourcc.c > > +++ b/drivers/gpu/drm/drm_fourcc.c > > @@ -370,6 +370,25 @@ unsigned int drm_format_info_block_height(const struct drm_format_info *info, > > } > > EXPORT_SYMBOL(drm_format_info_block_height); > > > > +/** > > + * drm_format_info_bpp - number of bits per pixel > > + * @info: pixel format info > > + * @plane: plane index > > + * > > + * Returns: > > + * The actual number of bits per pixel, depending on the plane index. > > + */ > > +unsigned int drm_format_info_bpp(const struct drm_format_info *info, int plane) > > +{ > > + if (!info || plane < 0 || plane >= info->num_planes) > > + return 0; > > + > > + return info->char_per_block[plane] * 8 / > > + (drm_format_info_block_width(info, plane) * > > + drm_format_info_block_height(info, plane)); > > Do we really needs this for blocky formats where this is potentially > ill-defined? I think if there's no need then this should also return 0 > when block_width/height != 1, it doesn't make much sense to compute bpp > when it's not really bits per _pixel_. Yes, we do need this. For low-color formats, the number of bits per pixel is less than eight, and block_width is larger than one. That is actually the point of this patch. > Minimally this needs to check whether the division actually makes sense or > whether there's a reminder, and if there's reminder, then fail. But that > feels like a bad hack and I think we should avoid it if it's not > absolutely necessary. Looking at drivers/gpu/drm/drm_fourcc.c, the only supported format where there can be a remainder is P030, which has 2 spare bits per 32-bit word, and thus is special anyway. Still, 4 * 8 / 3 = 10, so you get the correct numbers of bits for the first plane. For the second plane, you get 8 * 8 / 3 = 21, but as .is_yuv = true, you have to divide that result by two again, so you get 10 again. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds