Hi
Am 11.03.24 um 20:34 schrieb Frej Drejhammar:
Hi Thomas,
Thanks for the review and suggestions. My experience with the drm parts
of the kernel is limited to some weekends trying to fix the regression,
so I'm afraid I have some questions to check my understanding before
making a v2 of the patch.
Thomas Zimmermann <tzimmermann@xxxxxxx> writes:
I suggest to switch all fbdev code over to drm_driver_legacy_fb_format
<https://elixir.bootlin.com/linux/latest/C/ident/drm_driver_legacy_fb_format>()
first and then modify the format indrm_driver_legacy_fb_format
<https://elixir.bootlin.com/linux/latest/C/ident/drm_driver_legacy_fb_format>()
after reading it from drm_fb_legacy_fb_format().
I see how doing the format massaging in drm_driver_legacy_fb_format()
would fix the original regression (starting with the format returned by
drm_mode_legacy_fb_format(), drm_fb_legacy_fb_format() is a typo,
right?).
Yeah, a typo.
As drm_driver_legacy_fb_format() has only two callers,
drm_mode_addfb() and __drm_fb_helper_find_sizes() that change is
probably less likely to do something unintended. As far as I can tell,
drm_driver_legacy_fb_format() is only used when userland hasn't
specified a format or the kernel is initializing and have no format
information. For these code paths it's clear that only formats which are
actually supported by the hardware are meaningful.
Right.
What I can't really see is what "switch all fbdev code over to
drm_driver_legacy_fb_format" would entail and what the benefit would
be. How do I determine when drm_mode_legacy_fb_format() should be
replaced with drm_driver_legacy_fb_format()? I have already mistakenly
considered the change to drm_mode_legacy_fb_format() as harmless and
broken ofdrm... Shouldn't it be enough to make
drm_driver_legacy_fb_format() select a format which is supported by the
driver?
Your patch modifies drm_mode_legacy_fb_format() in a number of
*_fbdev_*.c files. In those instances, the code could certainly use
drm_driver_legacy_fb_format() instead.
The fbdev files provide the base for the kernel framebuffer console and
should behave like DRM clients in userspace; just that they are
implemented in the kernel. As userspace ioctls use
drm_driver_legacy_fb_format(), converting the in-kernel clients makes
sense. And that's it. We keep drm_mode_legacy_fb_format(), but call
drm_driver_legacy_fb_format() where necessary.
About tilcdc: it uses fbdev-dma, which sets up an XRGB format [1].
Shouldn't this already fail? Do you see a framebuffer console?
[1]
https://elixir.bootlin.com/linux/v6.8/source/drivers/gpu/drm/drm_fbdev_dma.c#L93
Best regards,
Frej
--
--
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)
