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

[Aurabindo Pillai]

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>