[PATCH] Revert "drm/amd/display: ignore modifiers when checking for format support"

[Alex Deucher]

Applied.  Thanks!

Alex

On Thu, Jun 23, 2022 at 9:34 AM Aurabindo Pillai
<aurabindo.pillai at amd.com> wrote:
>
>
>
> On 2022-06-19 20:37, Bas Nieuwenhuizen wrote:
> > This reverts commit 5089c4a8ebea3c3ad9eedf038dad7098ebc06131.
> >
> > This breaks validation and enumeration of display capable modifiers.
> >
> > The early return true means the rest of the validation code never gets
> > executed, and we need that to enumerate the right modifiers to userspace
> > for the format.
> >
> > The modifiers that are in the initial list generated for a plane are the
> > superset for all formats and we need the proper checks in this function
> > to filter some of them out for formats with which they're invalid to be
> > used.
> >
> > Furthermore, the safety contract here is that we validate the incoming
> > modifiers to ensure the kernel can handle them and the display hardware
> > can handle them. This includes e.g. rejecting multi-plane images with DCC.
> >
> > Note that the legacy swizzle mechanism allows encoding more swizzles, and
> > at fb creation time we convert them to modifiers and reject those with
> > no corresponding modifiers. If we are seeing rejections I'm happy to
> > help define modifiers that correspond to those, or if absolutely needed
> > implement a fallback path to allow for less strict validation of the
> > legacy path.
> >
> > However, I'd like to revert this patch, since any of these is going to
> > be a significant rework of the patch, and I'd rather not the regression
> > gets into a release or forgotten in the meantime.
> >
> > Signed-off-by: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
> > ---
> >   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 53 +++----------------
> >   1 file changed, 7 insertions(+), 46 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index 60fb99b74713..698741fd39f4 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -4936,7 +4936,8 @@ static bool dm_plane_format_mod_supported(struct drm_plane *plane,
> >   {
> >       struct amdgpu_device *adev = drm_to_adev(plane->dev);
> >       const struct drm_format_info *info = drm_format_info(format);
> > -     struct hw_asic_id asic_id = adev->dm.dc->ctx->asic_id;
> > +     int i;
> > +
> >       enum dm_micro_swizzle microtile = modifier_gfx9_swizzle_mode(modifier) & 3;
> >
> >       if (!info)
> > @@ -4952,53 +4953,13 @@ static bool dm_plane_format_mod_supported(struct drm_plane *plane,
> >               return true;
> >       }
> >
> > -     /* check if swizzle mode is supported by this version of DCN */
> > -     switch (asic_id.chip_family) {
> > -             case FAMILY_SI:
> > -             case FAMILY_CI:
> > -             case FAMILY_KV:
> > -             case FAMILY_CZ:
> > -             case FAMILY_VI:
> > -                     /* asics before AI does not have modifier support */
> > -                     return false;
> > -                     break;
> > -             case FAMILY_AI:
> > -             case FAMILY_RV:
> > -             case FAMILY_NV:
> > -             case FAMILY_VGH:
> > -             case FAMILY_YELLOW_CARP:
> > -             case AMDGPU_FAMILY_GC_10_3_6:
> > -             case AMDGPU_FAMILY_GC_10_3_7:
> > -                     switch (AMD_FMT_MOD_GET(TILE, modifier)) {
> > -                             case AMD_FMT_MOD_TILE_GFX9_64K_R_X:
> > -                             case AMD_FMT_MOD_TILE_GFX9_64K_D_X:
> > -                             case AMD_FMT_MOD_TILE_GFX9_64K_S_X:
> > -                             case AMD_FMT_MOD_TILE_GFX9_64K_D:
> > -                                     return true;
> > -                                     break;
> > -                             default:
> > -                                     return false;
> > -                                     break;
> > -                     }
> > -                     break;
> > -             case AMDGPU_FAMILY_GC_11_0_0:
> > -                     switch (AMD_FMT_MOD_GET(TILE, modifier)) {
> > -                             case AMD_FMT_MOD_TILE_GFX11_256K_R_X:
> > -                             case AMD_FMT_MOD_TILE_GFX9_64K_R_X:
> > -                             case AMD_FMT_MOD_TILE_GFX9_64K_D_X:
> > -                             case AMD_FMT_MOD_TILE_GFX9_64K_S_X:
> > -                             case AMD_FMT_MOD_TILE_GFX9_64K_D:
> > -                                     return true;
> > -                                     break;
> > -                             default:
> > -                                     return false;
> > -                                     break;
> > -                     }
> > -                     break;
> > -             default:
> > -                     ASSERT(0); /* Unknown asic */
> > +     /* Check that the modifier is on the list of the plane's supported modifiers. */
> > +     for (i = 0; i < plane->modifier_count; i++) {
> > +             if (modifier == plane->modifiers[i])
> >                       break;
> >       }
> > +     if (i == plane->modifier_count)
> > +             return false;
> >
> >       /*
> >        * For D swizzle the canonical modifier depends on the bpp, so check
>
> We can expose an additional swizzle mode to work around the original
> problem with usermode which doesn't support modifiers but supports
> swizzle modes.
>
> Reviewed-by: Aurabindo Pillai <aurabindo.pillai at amd.com>