[PATCH v3 11/13] drm/fb-helper: Fix single-probe color-format selection
Thomas Zimmermann
tzimmermann at suse.de
Mon May 15 08:42:20 UTC 2023
Hi
Am 15.05.23 um 10:16 schrieb Daniel Vetter:
> On Fri, May 12, 2023 at 04:11:48PM +0200, Thomas Zimmermann wrote:
>> 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.
>
> Shouldn't the helpers try to do this automatically? I think they kinda did
> that in the past in some limited circumstances like this ...
That was the intention, but it never really worked at all. IIRC the
color-format selection mixed up depth and bpp values freely. Factor in
the command-line override (video=@bpp) and some odd case has always been
broken.
So fbdev emulation now mostly uses the color-mode value that works as on
the command line. The current semantics is:
* select the user-given color mode, if non-zero
* select the driver-given color mode, if non-zero
* otherwise select a color mode of 32 by default (that's XRGB8888)
And later during fb_probe:
* try the selected color mode
* otherwise try to auto-detect if the selected color mode doesn't work,
* otherwise use XRGB8888 as a last resort
That nicely splits the code into color-mode selection and color-format
selection. And all the public interfaces (command line,
drm_fbdev_{}_setup(), etc) use the same semantics, which is the color mode.
Best regards
Thomas
> -Daniel
>
>>
>> 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)
>
>
>
>
--
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/20230515/5a220dcb/attachment-0001.sig>
More information about the dri-devel
mailing list