Re: [PATCH] drm: Don't return unsupported formats in drm_mode_legacy_fb_format

[Frej Drejhammar <frej.drejhammar@gmail.com>]

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@suse.de> 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?). 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.

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?

Best regards,

Frej