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.
Marek On Sun, Jun 12, 2022 at 7:54 PM Bas Nieuwenhuizen <b...@basnieuwenhuizen.nl> wrote: > On Thu, Jun 9, 2022 at 4:27 PM Aurabindo Pillai > <aurabindo.pil...@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.pil...@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 > > >