<div dir="ltr"><div>Bas, the code was literally rejecting swizzle modes that were not in the modifier list, which was incorrect. That's because the modifier list is a subset of all supported swizzle modes.<br></div><div><br></div><div>Marek<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Jun 12, 2022 at 7:54 PM Bas Nieuwenhuizen <<a href="mailto:bas@basnieuwenhuizen.nl">bas@basnieuwenhuizen.nl</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Thu, Jun 9, 2022 at 4:27 PM Aurabindo Pillai<br>
<<a href="mailto:aurabindo.pillai@amd.com" target="_blank">aurabindo.pillai@amd.com</a>> wrote:<br>
><br>
> [Why&How]<br>
> There are cases where swizzle modes are set but modifiers arent. For<br>
> such a userspace, we need not check modifiers while checking<br>
> compatibilty in the drm hook for checking plane format.<br>
><br>
> Ignore checking modifiers but check the DCN generation for the<br>
> supported swizzle mode.<br>
><br>
> Signed-off-by: Aurabindo Pillai <<a href="mailto:aurabindo.pillai@amd.com" target="_blank">aurabindo.pillai@amd.com</a>><br>
> ---<br>
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 51 +++++++++++++++++--<br>
>  1 file changed, 46 insertions(+), 5 deletions(-)<br>
><br>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c<br>
> index 2023baf41b7e..1322df491736 100644<br>
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c<br>
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c<br>
> @@ -4938,6 +4938,7 @@ static bool dm_plane_format_mod_supported(struct drm_plane *plane,<br>
>  {<br>
>         struct amdgpu_device *adev = drm_to_adev(plane->dev);<br>
>         const struct drm_format_info *info = drm_format_info(format);<br>
> +       struct hw_asic_id asic_id = adev->dm.dc->ctx->asic_id;<br>
>         int i;<br>
><br>
>         enum dm_micro_swizzle microtile = modifier_gfx9_swizzle_mode(modifier) & 3;<br>
> @@ -4955,13 +4956,53 @@ static bool dm_plane_format_mod_supported(struct drm_plane *plane,<br>
>                 return true;<br>
>         }<br>
><br>
> -       /* Check that the modifier is on the list of the plane's supported modifiers. */<br>
> -       for (i = 0; i < plane->modifier_count; i++) {<br>
> -               if (modifier == plane->modifiers[i])<br>
> +       /* check if swizzle mode is supported by this version of DCN */<br>
> +       switch (asic_id.chip_family) {<br>
> +               case FAMILY_SI:<br>
> +               case FAMILY_CI:<br>
> +               case FAMILY_KV:<br>
> +               case FAMILY_CZ:<br>
> +               case FAMILY_VI:<br>
> +                       /* AI and earlier asics does not have modifier support */<br>
> +                       return false;<br>
> +                       break;<br>
> +               case FAMILY_AI:<br>
> +               case FAMILY_RV:<br>
> +               case FAMILY_NV:<br>
> +               case FAMILY_VGH:<br>
> +               case FAMILY_YELLOW_CARP:<br>
> +               case AMDGPU_FAMILY_GC_10_3_6:<br>
> +               case AMDGPU_FAMILY_GC_10_3_7:<br>
> +                       switch (AMD_FMT_MOD_GET(TILE, modifier)) {<br>
> +                               case AMD_FMT_MOD_TILE_GFX9_64K_R_X:<br>
> +                               case AMD_FMT_MOD_TILE_GFX9_64K_D_X:<br>
> +                               case AMD_FMT_MOD_TILE_GFX9_64K_S_X:<br>
> +                               case AMD_FMT_MOD_TILE_GFX9_64K_D:<br>
> +                                       return true;<br>
> +                                       break;<br>
> +                               default:<br>
> +                                       return false;<br>
> +                                       break;<br>
> +                       }<br>
> +                       break;<br>
> +               case AMDGPU_FAMILY_GC_11_0_0:<br>
> +                       switch (AMD_FMT_MOD_GET(TILE, modifier)) {<br>
> +                               case AMD_FMT_MOD_TILE_GFX11_256K_R_X:<br>
> +                               case AMD_FMT_MOD_TILE_GFX9_64K_R_X:<br>
> +                               case AMD_FMT_MOD_TILE_GFX9_64K_D_X:<br>
> +                               case AMD_FMT_MOD_TILE_GFX9_64K_S_X:<br>
> +                               case AMD_FMT_MOD_TILE_GFX9_64K_D:<br>
> +                                       return true;<br>
> +                                       break;<br>
> +                               default:<br>
> +                                       return false;<br>
> +                                       break;<br>
> +                       }<br>
> +                       break;<br>
> +               default:<br>
> +                       ASSERT(0); /* Unknown asic */<br>
>                         break;<br>
>         }<br>
<br>
This seems broken to me. AFAICT we always return in the switch so the<br>
code after this that checks for valid DCC usage isn't executed.<br>
Checking the list of modifiers is also essential to make sure other<br>
stuff in the modifier is set properly.<br>
<br>
If you have userspace that is not using modifiers that is giving you<br>
issues, a better place to look might be<br>
convert_tiling_flags_to_modifier in amdgpu_display.c<br>
<br>
> -       if (i == plane->modifier_count)<br>
> -               return false;<br>
><br>
>         /*<br>
>          * For D swizzle the canonical modifier depends on the bpp, so check<br>
> --<br>
> 2.36.1<br>
><br>
</blockquote></div>