Re: [PATCH v2] amdgpu: check tiling flags when creating FB on GFX8-

[Alex Deucher]

On Mon, Sep 27, 2021 at 11:09 AM Simon Ser  wrote:
>
> On GFX9+, format modifiers are always enabled and ensure the
> frame-buffers can be scanned out at ADDFB2 time.
>
> On GFX8-, format modifiers are not supported and no other check
> is performed. This means ADDFB2 IOCTLs will succeed even if the
> tiling isn't supported for scan-out, and will result in garbage
> displayed on screen [1].
>
> Fix this by adding a check for tiling flags for GFX8 and older.
> The check is taken from radeonsi in Mesa (see how is_displayable
> is populated in gfx6_compute_surface).
>
> Changes in v2: use drm_WARN_ONCE instead of drm_WARN (Michel)
>
> [1]: https://github.com/swaywm/wlroots/issues/3185
>
> Signed-off-by: Simon Ser 
> Cc: sta...@vger.kernel.org
> Acked-by: Michel Dänzer 
> Cc: Alex Deucher 
> Cc: Harry Wentland 
> Cc: Nicholas Kazlauskas 
> Cc: Bas Nieuwenhuizen 

Applied.  Thanks.

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 31 +
>  1 file changed, 31 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index 58bfc7f00d76..5faf3ef28080 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -837,6 +837,28 @@ static int convert_tiling_flags_to_modifier(struct 
> amdgpu_framebuffer *afb)
> return 0;
>  }
>
> +/* Mirrors the is_displayable check in radeonsi's gfx6_compute_surface */
> +static int check_tiling_flags_gfx6(struct amdgpu_framebuffer *afb)
> +{
> +   u64 micro_tile_mode;
> +
> +   /* Zero swizzle mode means linear */
> +   if (AMDGPU_TILING_GET(afb->tiling_flags, SWIZZLE_MODE) == 0)
> +   return 0;
> +
> +   micro_tile_mode = AMDGPU_TILING_GET(afb->tiling_flags, 
> MICRO_TILE_MODE);
> +   switch (micro_tile_mode) {
> +   case 0: /* DISPLAY */
> +   case 3: /* RENDER */
> +   return 0;
> +   default:
> +   drm_dbg_kms(afb->base.dev,
> +   "Micro tile mode %llu not supported for 
> scanout\n",
> +   micro_tile_mode);
> +   return -EINVAL;
> +   }
> +}
> +
>  static void get_block_dimensions(unsigned int block_log2, unsigned int cpp,
>  unsigned int *width, unsigned int *height)
>  {
> @@ -1103,6 +1125,7 @@ int amdgpu_display_framebuffer_init(struct drm_device 
> *dev,
> const struct drm_mode_fb_cmd2 *mode_cmd,
> struct drm_gem_object *obj)
>  {
> +   struct amdgpu_device *adev = drm_to_adev(dev);
> int ret, i;
>
> /*
> @@ -1122,6 +1145,14 @@ int amdgpu_display_framebuffer_init(struct drm_device 
> *dev,
> if (ret)
> return ret;
>
> +   if (!dev->mode_config.allow_fb_modifiers) {
> +   drm_WARN_ONCE(dev, adev->family >= AMDGPU_FAMILY_AI,
> + "GFX9+ requires FB check based on format 
> modifier\n");
> +   ret = check_tiling_flags_gfx6(rfb);
> +   if (ret)
> +   return ret;
> +   }
> +
> if (dev->mode_config.allow_fb_modifiers &&
> !(rfb->base.flags & DRM_MODE_FB_MODIFIERS)) {
> ret = convert_tiling_flags_to_modifier(rfb);
> --
> 2.33.0
>
>

[PATCH v2] amdgpu: check tiling flags when creating FB on GFX8-

[Simon Ser]

On GFX9+, format modifiers are always enabled and ensure the
frame-buffers can be scanned out at ADDFB2 time.

On GFX8-, format modifiers are not supported and no other check
is performed. This means ADDFB2 IOCTLs will succeed even if the
tiling isn't supported for scan-out, and will result in garbage
displayed on screen [1].

Fix this by adding a check for tiling flags for GFX8 and older.
The check is taken from radeonsi in Mesa (see how is_displayable
is populated in gfx6_compute_surface).

Changes in v2: use drm_WARN_ONCE instead of drm_WARN (Michel)

[1]: https://github.com/swaywm/wlroots/issues/3185

Signed-off-by: Simon Ser 
Cc: sta...@vger.kernel.org
Acked-by: Michel Dänzer 
Cc: Alex Deucher 
Cc: Harry Wentland 
Cc: Nicholas Kazlauskas 
Cc: Bas Nieuwenhuizen 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 31 +
 1 file changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 58bfc7f00d76..5faf3ef28080 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -837,6 +837,28 @@ static int convert_tiling_flags_to_modifier(struct 
amdgpu_framebuffer *afb)
return 0;
 }
 
+/* Mirrors the is_displayable check in radeonsi's gfx6_compute_surface */
+static int check_tiling_flags_gfx6(struct amdgpu_framebuffer *afb)
+{
+   u64 micro_tile_mode;
+
+   /* Zero swizzle mode means linear */
+   if (AMDGPU_TILING_GET(afb->tiling_flags, SWIZZLE_MODE) == 0)
+   return 0;
+
+   micro_tile_mode = AMDGPU_TILING_GET(afb->tiling_flags, MICRO_TILE_MODE);
+   switch (micro_tile_mode) {
+   case 0: /* DISPLAY */
+   case 3: /* RENDER */
+   return 0;
+   default:
+   drm_dbg_kms(afb->base.dev,
+   "Micro tile mode %llu not supported for scanout\n",
+   micro_tile_mode);
+   return -EINVAL;
+   }
+}
+
 static void get_block_dimensions(unsigned int block_log2, unsigned int cpp,
 unsigned int *width, unsigned int *height)
 {
@@ -1103,6 +1125,7 @@ int amdgpu_display_framebuffer_init(struct drm_device 
*dev,
const struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_gem_object *obj)
 {
+   struct amdgpu_device *adev = drm_to_adev(dev);
int ret, i;
 
/*
@@ -1122,6 +1145,14 @@ int amdgpu_display_framebuffer_init(struct drm_device 
*dev,
if (ret)
return ret;
 
+   if (!dev->mode_config.allow_fb_modifiers) {
+   drm_WARN_ONCE(dev, adev->family >= AMDGPU_FAMILY_AI,
+ "GFX9+ requires FB check based on format 
modifier\n");
+   ret = check_tiling_flags_gfx6(rfb);
+   if (ret)
+   return ret;
+   }
+
if (dev->mode_config.allow_fb_modifiers &&
!(rfb->base.flags & DRM_MODE_FB_MODIFIERS)) {
ret = convert_tiling_flags_to_modifier(rfb);
-- 
2.33.0