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@xxxxxxxxx> > Cc: Eric Anholt <eric@xxxxxxxxxx> > Cc: Noralf Trønnes <noralf@xxxxxxxxxxx> > Cc: Maxime Ripard <mripard@xxxxxxxxxx> > --- > 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. -- Ville Syrjälä Intel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
