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

[Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>]

On Thu, Jun 9, 2022 at 4:27 PM Aurabindo Pillai
<aurabindo.pillai@amd.com> wrote:
>
> [Why&How]
> There are cases where swizzle modes are set but modifiers arent. For
> such a userspace, we need not check modifiers while checking
> compatibilty in the drm hook for checking plane format.
>
> Ignore checking modifiers but check the DCN generation for the
> supported swizzle mode.
>
> Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com>
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 51 +++++++++++++++++--
>  1 file changed, 46 insertions(+), 5 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 2023baf41b7e..1322df491736 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4938,6 +4938,7 @@ 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;
> @@ -4955,13 +4956,53 @@ static bool dm_plane_format_mod_supported(struct drm_plane *plane,
>                 return true;
>         }
>
> -       /* 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])
> +       /* 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:
> +                       /* AI and earlier asics 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 */
>                         break;
>         }

This seems broken to me. AFAICT we always return in the switch so the
code after this that checks for valid DCC usage isn't executed.
Checking the list of modifiers is also essential to make sure other
stuff in the modifier is set properly.

If you have userspace that is not using modifiers that is giving you
issues, a better place to look might be
convert_tiling_flags_to_modifier in amdgpu_display.c

> -       if (i == plane->modifier_count)
> -               return false;
>
>         /*
>          * For D swizzle the canonical modifier depends on the bpp, so check
> --
> 2.36.1
>