[PATCH v3 11/13] drm/fb-helper: Fix single-probe color-format selection

[Thomas Zimmermann]

Hi

Am 12.05.23 um 15:20 schrieb Linus Walleij:
> Sorry for late regression detection but this patch regresses
> the Integrator AB IMPD-1 graphics, I bisected down to this
> patch.
[...]
> This is the driver:
> drivers/gpu/drm/pl111/pl111_versatile.c
> with the pl110_impd1 variant, so these are the supported modes:
> 
> /* PL110 pixel formats for Integrator, vanilla PL110 */
> static const u32 pl110_integrator_pixel_formats[] = {
>          DRM_FORMAT_ABGR8888,
>          DRM_FORMAT_XBGR8888,
>          DRM_FORMAT_ARGB8888,
>          DRM_FORMAT_XRGB8888,
>          DRM_FORMAT_ABGR1555,
>          DRM_FORMAT_XBGR1555,
>          DRM_FORMAT_ARGB1555,
>          DRM_FORMAT_XRGB1555,
> };
> (...)
> /*
>   * The IM-PD1 variant is a PL110 with a bunch of broken, or not
>   * yet implemented features
>   */
> static const struct pl111_variant_data pl110_impd1 = {
>          .name = "PL110 IM-PD1",
>          .is_pl110 = true,
>          .broken_clockdivider = true,
>          .broken_vblank = true,
>          .formats = pl110_integrator_pixel_formats,
>          .nformats = ARRAY_SIZE(pl110_integrator_pixel_formats),
>          .fb_bpp = 16,
> };
> 
> Notice the absence of RGB565!
> Then we initialized the frambuffer like this:
> 
>          drm_fbdev_dma_setup(drm, priv->variant->fb_bpp);
> 
> And as you see priv->variant->fb_bpp will be 16, so we want some
> 16bpp mode however the only supported depth is 15 (the 1555 modes)
> so it would use that by scaling back depth to 15.
> 
> However after this patch that doesn't work anymore.
> 
> Any hints on how we can fix this?

According to a quick grep for fb_bpp, it's only used for the call to 
drm_fbdev_dma_setup(), right? In this case, you should set it to 15 for 
the models without rgb565. The switch at [1] should then pick the 
correct values.

The preferred_bpp parameter had a change in semantics. It used to be the 
number of bits per pixel. But that makes it hard to select between 
RGB1555 and RGB565.  So it's now a special color-mode value that works 
like the kernel's video= parameter. Values of 15 and 32 are different 
from the rest. That switch at [1] explains it. Maybe you should rename 
fb_bpp to color_mode for clarity.

Let me know if this helps.

Best regards
Thomas

[1] 
https://elixir.bootlin.com/linux/v6.4-rc1/source/drivers/gpu/drm/drm_fb_helper.c#L1827

> 
> Yours,
> Linus Walleij

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20230512/0de9ed6a/attachment-0001.sig>