On Sun, Jan 24, 2021 at 09:47:48PM +0100, Mario Kleiner wrote:
The check was introduced to make sure that only the DRM_FORMAT_MOD_LINEAR modifier is accepted by tinydrm.
However, if .format_mod_supported is not hooked up to drm_simple_kms_format_mod_supported then the core will simply validate modifiers against the format_modifiers list passed into drm_simple_display_pipe_init() or drm_universal_plane_init() and perform the same validation as drm_simple_kms_format_mod_supported() would have done.
Additionally, if a kms driver / plane does not support modifiers, it will not reject fb updates with no modifiers/ DRM_FORMAT_MOD_INVALID. This is important, because some simple drm drivers, e.g., pl111, pass NULL as format_modifiers list, so modifier support is disabled for these drivers, userspace would fall back to drmAddFB() without modifiers, and ergo the current drm_simple_kms_format_mod_supported() function would reject valid modifier-less fb's.
So this should fix display on non-tinydrm drivers like pl111, and probably also for non-atomic clients?
The Mesa vc4 gallium driver mentions pl111 as one possible display driver in render_only mode, so i assume this matters for some SoC's?
The background for the patch that introduced this was to fix atomic modesetting in the X-Servers modesetting-ddx, but with atomic modesetting and format modifiers disabled in modesetting-ddx (and also current kernels when interacting with modesetting-ddx), i assume this should fix some panels.
Note that i don't have any of the hw required for testing this, this is purely based on looking at the code, so this patch is only compile-tested.
For more reference, this fix was motivated by some discussions around broken page-flipping on VideoCore6 / RaspberryPi 4 with current Raspbian OS, so the experts may want to weigh in on that Mesa bug report as well, under the following link:
https://gitlab.freedesktop.org/mesa/mesa/-/issues/3601
Fixes: dff906c3f91c ("drm/tinydrm: Advertise that we can do only DRM_FORMAT_MOD_LINEAR.") Signed-off-by: Mario Kleiner mario.kleiner.de@gmail.com Cc: Eric Anholt eric@anholt.net Cc: Noralf Trønnes noralf@tronnes.org Cc: Maxime Ripard mripard@kernel.org
drivers/gpu/drm/drm_simple_kms_helper.c | 8 -------- 1 file changed, 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c index 743e57c1b44f..5f3e30553172 100644 --- a/drivers/gpu/drm/drm_simple_kms_helper.c +++ b/drivers/gpu/drm/drm_simple_kms_helper.c @@ -229,13 +229,6 @@ static void drm_simple_kms_plane_cleanup_fb(struct drm_plane *plane, pipe->funcs->cleanup_fb(pipe, state); }
-static bool drm_simple_kms_format_mod_supported(struct drm_plane *plane,
uint32_t format,uint64_t modifier)-{
- return modifier == DRM_FORMAT_MOD_LINEAR;
-}
static const struct drm_plane_helper_funcs drm_simple_kms_plane_helper_funcs = { .prepare_fb = drm_simple_kms_plane_prepare_fb, .cleanup_fb = drm_simple_kms_plane_cleanup_fb, @@ -250,7 +243,6 @@ static const struct drm_plane_funcs drm_simple_kms_plane_funcs = { .reset = drm_atomic_helper_plane_reset, .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state, .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
- .format_mod_supported = drm_simple_kms_format_mod_supported,
This will now cause also this driver to report a totally borked IN_FORMATS blob.
Would need this https://patchwork.freedesktop.org/series/83018/
but IIRC the bikeshed around that kinda suggested we should just implement .format_mod_support() always. Can't quite recall the details anymore.