[PATCH 01/13] drm/amdgpu: check for LINEAR_ALIGNED correctly in check_tiling_flags_gfx6

[PATCH 01/13] drm/amdgpu: check for LINEAR_ALIGNED correctly in check_tiling_flags_gfx6

[Marek Olšák]

Signed-off-by: Marek Olšák <marek.olsak@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index cfec85563bc6..3c5fb907bdd9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -954,8 +954,7 @@ 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)
+	if (AMDGPU_TILING_GET(afb->tiling_flags, ARRAY_MODE) == 1) /* LINEAR_ALIGNED */
 		return 0;
 
 	micro_tile_mode = AMDGPU_TILING_GET(afb->tiling_flags, MICRO_TILE_MODE);
-- 
2.34.1

[PATCH 02/13] drm/amdgpu/gfx11: remove superfluous cache flags

[Marek Olšák]

If any INV flags are needed, they should be executed via ACQUIRE_MEM
before INDIRECT_BUFFER.

Signed-off-by: Marek Olšák <marek.olsak@amd.com>
Acked-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
index 5c17409439f8..0d078d0db162 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
@@ -5604,11 +5604,7 @@ static void gfx_v11_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
 	amdgpu_ring_write(ring, PACKET3(PACKET3_RELEASE_MEM, 6));
 	amdgpu_ring_write(ring, (PACKET3_RELEASE_MEM_GCR_SEQ |
 				 PACKET3_RELEASE_MEM_GCR_GL2_WB |
-				 PACKET3_RELEASE_MEM_GCR_GL2_INV |
-				 PACKET3_RELEASE_MEM_GCR_GL2_US |
-				 PACKET3_RELEASE_MEM_GCR_GL1_INV |
-				 PACKET3_RELEASE_MEM_GCR_GLV_INV |
-				 PACKET3_RELEASE_MEM_GCR_GLM_INV |
+				 PACKET3_RELEASE_MEM_GCR_GLM_INV | /* must be set with GLM_WB */
 				 PACKET3_RELEASE_MEM_GCR_GLM_WB |
 				 PACKET3_RELEASE_MEM_CACHE_POLICY(3) |
 				 PACKET3_RELEASE_MEM_EVENT_TYPE(CACHE_FLUSH_AND_INV_TS_EVENT) |
-- 
2.34.1

[PATCH 03/13] drm/amdgpu/gfx12: remove superfluous cache flags

[Marek Olšák]

If any INV flags are needed, they should be executed via ACQUIRE_MEM
before INDIRECT_BUFFER.

GLM flags are also removed because the hw ignores them.

Signed-off-by: Marek Olšák <marek.olsak@amd.com>
Acked-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
index e9559bdd8264..a638696b2142 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
@@ -4144,12 +4144,6 @@ static void gfx_v12_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
 	amdgpu_ring_write(ring, PACKET3(PACKET3_RELEASE_MEM, 6));
 	amdgpu_ring_write(ring, (PACKET3_RELEASE_MEM_GCR_SEQ |
 				 PACKET3_RELEASE_MEM_GCR_GL2_WB |
-				 PACKET3_RELEASE_MEM_GCR_GL2_INV |
-				 PACKET3_RELEASE_MEM_GCR_GL2_US |
-				 PACKET3_RELEASE_MEM_GCR_GL1_INV |
-				 PACKET3_RELEASE_MEM_GCR_GLV_INV |
-				 PACKET3_RELEASE_MEM_GCR_GLM_INV |
-				 PACKET3_RELEASE_MEM_GCR_GLM_WB |
 				 PACKET3_RELEASE_MEM_CACHE_POLICY(3) |
 				 PACKET3_RELEASE_MEM_EVENT_TYPE(CACHE_FLUSH_AND_INV_TS_EVENT) |
 				 PACKET3_RELEASE_MEM_EVENT_INDEX(5)));
-- 
2.34.1

[PATCH 04/13] drm/amdgpu/gfx12: remove GDS leftovers

[Marek Olšák]

GDS doesn't exist in gfx12. The incomplete packet allows userspace to hang
the hw from the kernel.

Signed-off-by: Marek Olšák <marek.olsak@amd.com>
Acked-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
index a638696b2142..ccb26f78252a 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
@@ -4108,21 +4108,6 @@ static void gfx_v12_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
 		/* inherit vmid from mqd */
 		control |= 0x40000000;
 
-	/* Currently, there is a high possibility to get wave ID mismatch
-	 * between ME and GDS, leading to a hw deadlock, because ME generates
-	 * different wave IDs than the GDS expects. This situation happens
-	 * randomly when at least 5 compute pipes use GDS ordered append.
-	 * The wave IDs generated by ME are also wrong after suspend/resume.
-	 * Those are probably bugs somewhere else in the kernel driver.
-	 *
-	 * Writing GDS_COMPUTE_MAX_WAVE_ID resets wave ID counters in ME and
-	 * GDS to 0 for this ring (me/pipe).
-	 */
-	if (ib->flags & AMDGPU_IB_FLAG_RESET_GDS_MAX_WAVE_ID) {
-		amdgpu_ring_write(ring, PACKET3(PACKET3_SET_CONFIG_REG, 1));
-		amdgpu_ring_write(ring, regGDS_COMPUTE_MAX_WAVE_ID);
-	}
-
 	amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
 	BUG_ON(ib->gpu_addr & 0x3); /* Dword align */
 	amdgpu_ring_write(ring,
@@ -4721,7 +4706,6 @@ static const struct amdgpu_ring_funcs gfx_v12_0_ring_funcs_gfx = {
 		SOC15_FLUSH_GPU_TLB_NUM_REG_WAIT * 7 +
 		2 + /* VM_FLUSH */
 		8 + /* FENCE for VM_FLUSH */
-		20 + /* GDS switch */
 		5 + /* COND_EXEC */
 		7 + /* HDP_flush */
 		4 + /* VGT_flush */
-- 
2.34.1

[PATCH 05/13] drm/amdgpu: remove AMD_FMT_MOD_GFX12_DCC_MAX_COMPRESSED_BLOCK_* definitions

[Marek Olšák]

They were added accidentally.

Signed-off-by: Marek Olšák <marek.olsak@amd.com>
---
 include/uapi/drm/drm_fourcc.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index d0063ac6e09f..4168445fbb8b 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -1540,9 +1540,6 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)
 #define AMD_FMT_MOD_DCC_MAX_COMPRESSED_BLOCK_SHIFT 18
 #define AMD_FMT_MOD_DCC_MAX_COMPRESSED_BLOCK_MASK 0x3
 
-#define AMD_FMT_MOD_GFX12_DCC_MAX_COMPRESSED_BLOCK_SHIFT     3
-#define AMD_FMT_MOD_GFX12_DCC_MAX_COMPRESSED_BLOCK_MASK      0x3 /* 0:64B, 1:128B, 2:256B */
-
 /*
  * DCC supports embedding some clear colors directly in the DCC surface.
  * However, on older GPUs the rendering HW ignores the embedded clear color
-- 
2.34.1

Re: [PATCH 05/13] drm/amdgpu: remove AMD_FMT_MOD_GFX12_DCC_MAX_COMPRESSED_BLOCK_* definitions

[Aurabindo Pillai]

Reviewed-by: Aurabindo Pillai <aurabindo.pillai@amd.com>

On 6/26/24 2:31 PM, Marek Olšák wrote:
> They were added accidentally.
> 
> Signed-off-by: Marek Olšák <marek.olsak@amd.com>
> ---
>   include/uapi/drm/drm_fourcc.h | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index d0063ac6e09f..4168445fbb8b 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -1540,9 +1540,6 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)
>   #define AMD_FMT_MOD_DCC_MAX_COMPRESSED_BLOCK_SHIFT 18
>   #define AMD_FMT_MOD_DCC_MAX_COMPRESSED_BLOCK_MASK 0x3
>   
> -#define AMD_FMT_MOD_GFX12_DCC_MAX_COMPRESSED_BLOCK_SHIFT     3
> -#define AMD_FMT_MOD_GFX12_DCC_MAX_COMPRESSED_BLOCK_MASK      0x3 /* 0:64B, 1:128B, 2:256B */
> -
>   /*
>    * DCC supports embedding some clear colors directly in the DCC surface.
>    * However, on older GPUs the rendering HW ignores the embedded clear color

-- 
--

Thanks & Regards,
Aurabindo Pillai

[PATCH 06/13] drm/amdgpu/display: handle gfx12 in dm_check_cursor_fb

[Marek Olšák]

Checking SWIZZLE_MODE has undefined behavior on gfx12.

Signed-off-by: Marek Olšák <marek.olsak@amd.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 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 8f99ba6aa6a7..c3347e8bee54 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -10666,12 +10666,14 @@ static int dm_check_cursor_fb(struct amdgpu_crtc *new_acrtc,
 	 * check tiling flags when the FB doesn't have a modifier.
 	 */
 	if (!(fb->flags & DRM_MODE_FB_MODIFIERS)) {
-		if (adev->family < AMDGPU_FAMILY_AI) {
+		if (adev->family >= AMDGPU_FAMILY_GC_12_0_0) {
+			linear = AMDGPU_TILING_GET(afb->tiling_flags, GFX12_SWIZZLE_MODE) == 0;
+		} else if (adev->family >= AMDGPU_FAMILY_AI) {
+			linear = AMDGPU_TILING_GET(afb->tiling_flags, SWIZZLE_MODE) == 0;
+		} else {
 			linear = AMDGPU_TILING_GET(afb->tiling_flags, ARRAY_MODE) != DC_ARRAY_2D_TILED_THIN1 &&
 				 AMDGPU_TILING_GET(afb->tiling_flags, ARRAY_MODE) != DC_ARRAY_1D_TILED_THIN1 &&
 				 AMDGPU_TILING_GET(afb->tiling_flags, MICRO_TILE_MODE) == 0;
-		} else {
-			linear = AMDGPU_TILING_GET(afb->tiling_flags, SWIZZLE_MODE) == 0;
 		}
 		if (!linear) {
 			DRM_DEBUG_ATOMIC("Cursor FB not linear");
-- 
2.34.1

Re: [PATCH 06/13] drm/amdgpu/display: handle gfx12 in dm_check_cursor_fb

[Aurabindo Pillai]

Reviewed-by: Aurabindo Pillai <aurabindo.pillai@amd.com>

On 6/26/24 2:31 PM, Marek Olšák wrote:
> Checking SWIZZLE_MODE has undefined behavior on gfx12.
> 
> Signed-off-by: Marek Olšák <marek.olsak@amd.com>
> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 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 8f99ba6aa6a7..c3347e8bee54 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -10666,12 +10666,14 @@ static int dm_check_cursor_fb(struct amdgpu_crtc *new_acrtc,
>   	 * check tiling flags when the FB doesn't have a modifier.
>   	 */
>   	if (!(fb->flags & DRM_MODE_FB_MODIFIERS)) {
> -		if (adev->family < AMDGPU_FAMILY_AI) {
> +		if (adev->family >= AMDGPU_FAMILY_GC_12_0_0) {
> +			linear = AMDGPU_TILING_GET(afb->tiling_flags, GFX12_SWIZZLE_MODE) == 0;
> +		} else if (adev->family >= AMDGPU_FAMILY_AI) {
> +			linear = AMDGPU_TILING_GET(afb->tiling_flags, SWIZZLE_MODE) == 0;
> +		} else {
>   			linear = AMDGPU_TILING_GET(afb->tiling_flags, ARRAY_MODE) != DC_ARRAY_2D_TILED_THIN1 &&
>   				 AMDGPU_TILING_GET(afb->tiling_flags, ARRAY_MODE) != DC_ARRAY_1D_TILED_THIN1 &&
>   				 AMDGPU_TILING_GET(afb->tiling_flags, MICRO_TILE_MODE) == 0;
> -		} else {
> -			linear = AMDGPU_TILING_GET(afb->tiling_flags, SWIZZLE_MODE) == 0;
>   		}
>   		if (!linear) {
>   			DRM_DEBUG_ATOMIC("Cursor FB not linear");

-- 
--

Thanks & Regards,
Aurabindo Pillai

[PATCH 07/13] drm/amdgpu: add amdgpu_framebuffer::gfx12_dcc

[Marek Olšák]

amdgpu_framebuffer doesn't have tiling_flags, so we need this.

amdgpu_display_get_fb_info never gets NULL parameters, so checking for NULL
was useless.

Signed-off-by: Marek Olšák <marek.olsak@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 15 ++++++++-------
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h    |  1 +
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 3c5fb907bdd9..3f431e6b155a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -1143,7 +1143,8 @@ static int amdgpu_display_verify_sizes(struct amdgpu_framebuffer *rfb)
 }
 
 static int amdgpu_display_get_fb_info(const struct amdgpu_framebuffer *amdgpu_fb,
-				      uint64_t *tiling_flags, bool *tmz_surface)
+				      uint64_t *tiling_flags, bool *tmz_surface,
+				      bool *gfx12_dcc)
 {
 	struct amdgpu_bo *rbo;
 	int r;
@@ -1151,6 +1152,7 @@ static int amdgpu_display_get_fb_info(const struct amdgpu_framebuffer *amdgpu_fb
 	if (!amdgpu_fb) {
 		*tiling_flags = 0;
 		*tmz_surface = false;
+		*gfx12_dcc = false;
 		return 0;
 	}
 
@@ -1164,11 +1166,9 @@ static int amdgpu_display_get_fb_info(const struct amdgpu_framebuffer *amdgpu_fb
 		return r;
 	}
 
-	if (tiling_flags)
-		amdgpu_bo_get_tiling_flags(rbo, tiling_flags);
-
-	if (tmz_surface)
-		*tmz_surface = amdgpu_bo_encrypted(rbo);
+	amdgpu_bo_get_tiling_flags(rbo, tiling_flags);
+	*tmz_surface = amdgpu_bo_encrypted(rbo);
+	*gfx12_dcc = rbo->flags & AMDGPU_GEM_CREATE_GFX12_DCC;
 
 	amdgpu_bo_unreserve(rbo);
 
@@ -1237,7 +1237,8 @@ static int amdgpu_display_framebuffer_init(struct drm_device *dev,
 		}
 	}
 
-	ret = amdgpu_display_get_fb_info(rfb, &rfb->tiling_flags, &rfb->tmz_surface);
+	ret = amdgpu_display_get_fb_info(rfb, &rfb->tiling_flags, &rfb->tmz_surface,
+					 &rfb->gfx12_dcc);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index 1fe21a70ddd0..d002b845d8ac 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -300,6 +300,7 @@ struct amdgpu_framebuffer {
 
 	uint64_t tiling_flags;
 	bool tmz_surface;
+	bool gfx12_dcc;
 
 	/* caching for later use */
 	uint64_t address;
-- 
2.34.1

Re: [PATCH 07/13] drm/amdgpu: add amdgpu_framebuffer::gfx12_dcc

[Alex Deucher]

On Wed, Jun 26, 2024 at 2:42 PM Marek Olšák <maraeo@gmail.com> wrote:
>
> amdgpu_framebuffer doesn't have tiling_flags, so we need this.

Maybe move this patch before patch 13?  It took me a while to see
where this was used.  Either that or maybe mention where it will get
used in the later patch.
With that fixed, the series is:
Acked-by: Alex Deucher <alexander.deucher@amd.com>



Alex


>
> amdgpu_display_get_fb_info never gets NULL parameters, so checking for NULL
> was useless.
>
> Signed-off-by: Marek Olšák <marek.olsak@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 15 ++++++++-------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h    |  1 +
>  2 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index 3c5fb907bdd9..3f431e6b155a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -1143,7 +1143,8 @@ static int amdgpu_display_verify_sizes(struct amdgpu_framebuffer *rfb)
>  }
>
>  static int amdgpu_display_get_fb_info(const struct amdgpu_framebuffer *amdgpu_fb,
> -                                     uint64_t *tiling_flags, bool *tmz_surface)
> +                                     uint64_t *tiling_flags, bool *tmz_surface,
> +                                     bool *gfx12_dcc)
>  {
>         struct amdgpu_bo *rbo;
>         int r;
> @@ -1151,6 +1152,7 @@ static int amdgpu_display_get_fb_info(const struct amdgpu_framebuffer *amdgpu_fb
>         if (!amdgpu_fb) {
>                 *tiling_flags = 0;
>                 *tmz_surface = false;
> +               *gfx12_dcc = false;
>                 return 0;
>         }
>
> @@ -1164,11 +1166,9 @@ static int amdgpu_display_get_fb_info(const struct amdgpu_framebuffer *amdgpu_fb
>                 return r;
>         }
>
> -       if (tiling_flags)
> -               amdgpu_bo_get_tiling_flags(rbo, tiling_flags);
> -
> -       if (tmz_surface)
> -               *tmz_surface = amdgpu_bo_encrypted(rbo);
> +       amdgpu_bo_get_tiling_flags(rbo, tiling_flags);
> +       *tmz_surface = amdgpu_bo_encrypted(rbo);
> +       *gfx12_dcc = rbo->flags & AMDGPU_GEM_CREATE_GFX12_DCC;
>
>         amdgpu_bo_unreserve(rbo);
>
> @@ -1237,7 +1237,8 @@ static int amdgpu_display_framebuffer_init(struct drm_device *dev,
>                 }
>         }
>
> -       ret = amdgpu_display_get_fb_info(rfb, &rfb->tiling_flags, &rfb->tmz_surface);
> +       ret = amdgpu_display_get_fb_info(rfb, &rfb->tiling_flags, &rfb->tmz_surface,
> +                                        &rfb->gfx12_dcc);
>         if (ret)
>                 return ret;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> index 1fe21a70ddd0..d002b845d8ac 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> @@ -300,6 +300,7 @@ struct amdgpu_framebuffer {
>
>         uint64_t tiling_flags;
>         bool tmz_surface;
> +       bool gfx12_dcc;
>
>         /* caching for later use */
>         uint64_t address;
> --
> 2.34.1
>

[PATCH 08/13] drm/amdgpu: don't use amdgpu_lookup_format_info on gfx12

[Marek Olšák]

It only uses fields for GFX9-11 related to the separate DCC buffer,
which doesn't exist in GFX12.

Signed-off-by: Marek Olšák <marek.olsak@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 3f431e6b155a..3a7f6e77806a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -654,6 +654,10 @@ amdgpu_lookup_format_info(u32 format, uint64_t modifier)
 	if (!IS_AMD_FMT_MOD(modifier))
 		return NULL;
 
+	if (AMD_FMT_MOD_GET(TILE_VERSION, modifier) < AMD_FMT_MOD_TILE_VER_GFX9 ||
+	    AMD_FMT_MOD_GET(TILE_VERSION, modifier) >= AMD_FMT_MOD_TILE_VER_GFX12)
+		return NULL;
+
 	if (AMD_FMT_MOD_GET(DCC_RETILE, modifier))
 		return lookup_format_info(dcc_retile_formats,
 					  ARRAY_SIZE(dcc_retile_formats),
-- 
2.34.1

[PATCH 09/13] drm/amdgpu: handle gfx12 in amdgpu_display_verify_sizes

[Marek Olšák]

It verified GFX9-11 swizzle modes on GFX12, which has undefined behavior.

Signed-off-by: Marek Olšák <marek.olsak@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 27 ++++++++++++++++++++-
 include/uapi/drm/drm_fourcc.h               |  2 ++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 3a7f6e77806a..ec0299c5918e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -1082,6 +1082,30 @@ static int amdgpu_display_verify_sizes(struct amdgpu_framebuffer *rfb)
 			block_width = 256 / format_info->cpp[i];
 			block_height = 1;
 			block_size_log2 = 8;
+		} else if (AMD_FMT_MOD_GET(TILE_VERSION, modifier) >= AMD_FMT_MOD_TILE_VER_GFX12) {
+			int swizzle = AMD_FMT_MOD_GET(TILE, modifier);
+
+			switch (swizzle) {
+			case AMD_FMT_MOD_TILE_GFX12_256B_2D:
+				block_size_log2 = 8;
+				break;
+			case AMD_FMT_MOD_TILE_GFX12_4K_2D:
+				block_size_log2 = 12;
+				break;
+			case AMD_FMT_MOD_TILE_GFX12_64K_2D:
+				block_size_log2 = 16;
+				break;
+			case AMD_FMT_MOD_TILE_GFX12_256K_2D:
+				block_size_log2 = 18;
+				break;
+			default:
+				drm_dbg_kms(rfb->base.dev,
+					    "Gfx12 swizzle mode with unknown block size: %d\n", swizzle);
+				return -EINVAL;
+			}
+
+			get_block_dimensions(block_size_log2, format_info->cpp[i],
+					     &block_width, &block_height);
 		} else {
 			int swizzle = AMD_FMT_MOD_GET(TILE, modifier);
 
@@ -1117,7 +1141,8 @@ static int amdgpu_display_verify_sizes(struct amdgpu_framebuffer *rfb)
 			return ret;
 	}
 
-	if (AMD_FMT_MOD_GET(DCC, modifier)) {
+	if (AMD_FMT_MOD_GET(TILE_VERSION, modifier) <= AMD_FMT_MOD_TILE_VER_GFX11 &&
+	    AMD_FMT_MOD_GET(DCC, modifier)) {
 		if (AMD_FMT_MOD_GET(DCC_RETILE, modifier)) {
 			block_size_log2 = get_dcc_block_size(modifier, false, false);
 			get_block_dimensions(block_size_log2 + 8, format_info->cpp[0],
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 4168445fbb8b..2d84a8052b15 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -1506,6 +1506,8 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)
  *    6 - 64KB_3D
  *    7 - 256KB_3D
  */
+#define AMD_FMT_MOD_TILE_GFX12_256B_2D 1
+#define AMD_FMT_MOD_TILE_GFX12_4K_2D 2
 #define AMD_FMT_MOD_TILE_GFX12_64K_2D 3
 #define AMD_FMT_MOD_TILE_GFX12_256K_2D 4
 
-- 
2.34.1

[PATCH 10/13] drm/amdgpu/display: handle gfx12 in amdgpu_dm_plane_format_mod_supported

[Marek Olšák]

All this code has undefined behavior on GFX12 and shouldn't be executed.

Signed-off-by: Marek Olšák <marek.olsak@amd.com>
---
 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 47 ++++++++++---------
 1 file changed, 25 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index bb534b2b0b71..5a6a21e28548 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -1419,8 +1419,6 @@ static bool amdgpu_dm_plane_format_mod_supported(struct drm_plane *plane,
 	const struct drm_format_info *info = drm_format_info(format);
 	int i;
 
-	enum dm_micro_swizzle microtile = amdgpu_dm_plane_modifier_gfx9_swizzle_mode(modifier) & 3;
-
 	if (!info)
 		return false;
 
@@ -1442,29 +1440,34 @@ static bool amdgpu_dm_plane_format_mod_supported(struct drm_plane *plane,
 	if (i == plane->modifier_count)
 		return false;
 
-	/*
-	 * For D swizzle the canonical modifier depends on the bpp, so check
-	 * it here.
-	 */
-	if (AMD_FMT_MOD_GET(TILE_VERSION, modifier) == AMD_FMT_MOD_TILE_VER_GFX9 &&
-	    adev->family >= AMDGPU_FAMILY_NV) {
-		if (microtile == MICRO_SWIZZLE_D && info->cpp[0] == 4)
-			return false;
-	}
-
-	if (adev->family >= AMDGPU_FAMILY_RV && microtile == MICRO_SWIZZLE_D &&
-	    info->cpp[0] < 8)
-		return false;
+	/* GFX12 doesn't have these limitations. */
+	if (AMD_FMT_MOD_GET(TILE_VERSION, modifier) <= AMD_FMT_MOD_TILE_VER_GFX11) {
+		enum dm_micro_swizzle microtile = amdgpu_dm_plane_modifier_gfx9_swizzle_mode(modifier) & 3;
 
-	if (amdgpu_dm_plane_modifier_has_dcc(modifier)) {
-		/* Per radeonsi comments 16/64 bpp are more complicated. */
-		if (info->cpp[0] != 4)
-			return false;
-		/* We support multi-planar formats, but not when combined with
-		 * additional DCC metadata planes.
+		/*
+		 * For D swizzle the canonical modifier depends on the bpp, so check
+		 * it here.
 		 */
-		if (info->num_planes > 1)
+		if (AMD_FMT_MOD_GET(TILE_VERSION, modifier) == AMD_FMT_MOD_TILE_VER_GFX9 &&
+		    adev->family >= AMDGPU_FAMILY_NV) {
+			if (microtile == MICRO_SWIZZLE_D && info->cpp[0] == 4)
+				return false;
+		}
+
+		if (adev->family >= AMDGPU_FAMILY_RV && microtile == MICRO_SWIZZLE_D &&
+		    info->cpp[0] < 8)
 			return false;
+
+		if (amdgpu_dm_plane_modifier_has_dcc(modifier)) {
+			/* Per radeonsi comments 16/64 bpp are more complicated. */
+			if (info->cpp[0] != 4)
+				return false;
+			/* We support multi-planar formats, but not when combined with
+			 * additional DCC metadata planes.
+			 */
+			if (info->num_planes > 1)
+				return false;
+		}
 	}
 
 	return true;
-- 
2.34.1

Re: [PATCH 10/13] drm/amdgpu/display: handle gfx12 in amdgpu_dm_plane_format_mod_supported

[Aurabindo Pillai]

Reviewed-by: Aurabindo Pillai <aurabindo.pillai@amd.com>

On 6/26/24 2:31 PM, Marek Olšák wrote:
> All this code has undefined behavior on GFX12 and shouldn't be executed.
> 
> Signed-off-by: Marek Olšák <marek.olsak@amd.com>
> ---
>   .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 47 ++++++++++---------
>   1 file changed, 25 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> index bb534b2b0b71..5a6a21e28548 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> @@ -1419,8 +1419,6 @@ static bool amdgpu_dm_plane_format_mod_supported(struct drm_plane *plane,
>   	const struct drm_format_info *info = drm_format_info(format);
>   	int i;
>   
> -	enum dm_micro_swizzle microtile = amdgpu_dm_plane_modifier_gfx9_swizzle_mode(modifier) & 3;
> -
>   	if (!info)
>   		return false;
>   
> @@ -1442,29 +1440,34 @@ static bool amdgpu_dm_plane_format_mod_supported(struct drm_plane *plane,
>   	if (i == plane->modifier_count)
>   		return false;
>   
> -	/*
> -	 * For D swizzle the canonical modifier depends on the bpp, so check
> -	 * it here.
> -	 */
> -	if (AMD_FMT_MOD_GET(TILE_VERSION, modifier) == AMD_FMT_MOD_TILE_VER_GFX9 &&
> -	    adev->family >= AMDGPU_FAMILY_NV) {
> -		if (microtile == MICRO_SWIZZLE_D && info->cpp[0] == 4)
> -			return false;
> -	}
> -
> -	if (adev->family >= AMDGPU_FAMILY_RV && microtile == MICRO_SWIZZLE_D &&
> -	    info->cpp[0] < 8)
> -		return false;
> +	/* GFX12 doesn't have these limitations. */
> +	if (AMD_FMT_MOD_GET(TILE_VERSION, modifier) <= AMD_FMT_MOD_TILE_VER_GFX11) {
> +		enum dm_micro_swizzle microtile = amdgpu_dm_plane_modifier_gfx9_swizzle_mode(modifier) & 3;
>   
> -	if (amdgpu_dm_plane_modifier_has_dcc(modifier)) {
> -		/* Per radeonsi comments 16/64 bpp are more complicated. */
> -		if (info->cpp[0] != 4)
> -			return false;
> -		/* We support multi-planar formats, but not when combined with
> -		 * additional DCC metadata planes.
> +		/*
> +		 * For D swizzle the canonical modifier depends on the bpp, so check
> +		 * it here.
>   		 */
> -		if (info->num_planes > 1)
> +		if (AMD_FMT_MOD_GET(TILE_VERSION, modifier) == AMD_FMT_MOD_TILE_VER_GFX9 &&
> +		    adev->family >= AMDGPU_FAMILY_NV) {
> +			if (microtile == MICRO_SWIZZLE_D && info->cpp[0] == 4)
> +				return false;
> +		}
> +
> +		if (adev->family >= AMDGPU_FAMILY_RV && microtile == MICRO_SWIZZLE_D &&
> +		    info->cpp[0] < 8)
>   			return false;
> +
> +		if (amdgpu_dm_plane_modifier_has_dcc(modifier)) {
> +			/* Per radeonsi comments 16/64 bpp are more complicated. */
> +			if (info->cpp[0] != 4)
> +				return false;
> +			/* We support multi-planar formats, but not when combined with
> +			 * additional DCC metadata planes.
> +			 */
> +			if (info->num_planes > 1)
> +				return false;
> +		}
>   	}
>   
>   	return true;

-- 
--

Thanks & Regards,
Aurabindo Pillai

[PATCH 11/13] drm/amdgpu/display: set plane attributes for gfx12 correctly

[Marek Olšák]

It used gfx9 flags, which has undefined behavior on gfx12.

Signed-off-by: Marek Olšák <marek.olsak@amd.com>
---
 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 50 ++++++++++++++++++-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 5a6a21e28548..e13938e01b70 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -352,6 +352,46 @@ static int amdgpu_dm_plane_fill_gfx9_plane_attributes_from_modifiers(struct amdg
 	return ret;
 }
 
+static int amdgpu_dm_plane_fill_gfx12_plane_attributes_from_modifiers(struct amdgpu_device *adev,
+								      const struct amdgpu_framebuffer *afb,
+								      const enum surface_pixel_format format,
+								      const enum dc_rotation_angle rotation,
+								      const struct plane_size *plane_size,
+								      union dc_tiling_info *tiling_info,
+								      struct dc_plane_dcc_param *dcc,
+								      struct dc_plane_address *address,
+								      const bool force_disable_dcc)
+{
+	const uint64_t modifier = afb->base.modifier;
+	int ret = 0;
+
+	/* TODO: Most of this function shouldn't be needed on GFX12. */
+	amdgpu_dm_plane_fill_gfx9_tiling_info_from_device(adev, tiling_info);
+
+	tiling_info->gfx9.swizzle = amdgpu_dm_plane_modifier_gfx9_swizzle_mode(modifier);
+
+	if (amdgpu_dm_plane_modifier_has_dcc(modifier) && !force_disable_dcc) {
+		int max_compressed_block = AMD_FMT_MOD_GET(DCC_MAX_COMPRESSED_BLOCK, modifier);
+
+		dcc->enable = 1;
+		dcc->independent_64b_blks = max_compressed_block == 0;
+
+		if (max_compressed_block == 0)
+			dcc->dcc_ind_blk = hubp_ind_block_64b;
+		else if (max_compressed_block == 1)
+			dcc->dcc_ind_blk = hubp_ind_block_128b;
+		else
+			dcc->dcc_ind_blk = hubp_ind_block_unconstrained;
+	}
+
+	/* TODO: This seems wrong because there is no DCC plane on GFX12. */
+	ret = amdgpu_dm_plane_validate_dcc(adev, format, rotation, tiling_info, dcc, address, plane_size);
+	if (ret)
+		drm_dbg_kms(adev_to_drm(adev), "amdgpu_dm_plane_validate_dcc: returned error: %d\n", ret);
+
+	return ret;
+}
+
 static void amdgpu_dm_plane_add_gfx10_1_modifiers(const struct amdgpu_device *adev,
 						  uint64_t **mods,
 						  uint64_t *size,
@@ -835,7 +875,15 @@ int amdgpu_dm_plane_fill_plane_buffer_attributes(struct amdgpu_device *adev,
 			upper_32_bits(chroma_addr);
 	}
 
-	if (adev->family >= AMDGPU_FAMILY_AI) {
+	if (adev->family >= AMDGPU_FAMILY_GC_12_0_0) {
+		ret = amdgpu_dm_plane_fill_gfx12_plane_attributes_from_modifiers(adev, afb, format,
+										 rotation, plane_size,
+										 tiling_info, dcc,
+										 address,
+										 force_disable_dcc);
+		if (ret)
+			return ret;
+	} else if (adev->family >= AMDGPU_FAMILY_AI) {
 		ret = amdgpu_dm_plane_fill_gfx9_plane_attributes_from_modifiers(adev, afb, format,
 										rotation, plane_size,
 										tiling_info, dcc,
-- 
2.34.1

Re: [PATCH 11/13] drm/amdgpu/display: set plane attributes for gfx12 correctly

[Aurabindo Pillai]

On 6/26/24 2:31 PM, Marek Olšák wrote:
> It used gfx9 flags, which has undefined behavior on gfx12.
> 
> Signed-off-by: Marek Olšák <marek.olsak@amd.com>
> ---
>   .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 50 ++++++++++++++++++-
>   1 file changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> index 5a6a21e28548..e13938e01b70 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> @@ -352,6 +352,46 @@ static int amdgpu_dm_plane_fill_gfx9_plane_attributes_from_modifiers(struct amdg
>   	return ret;
>   }
>   
> +static int amdgpu_dm_plane_fill_gfx12_plane_attributes_from_modifiers(struct amdgpu_device *adev,
> +								      const struct amdgpu_framebuffer *afb,
> +								      const enum surface_pixel_format format,
> +								      const enum dc_rotation_angle rotation,
> +								      const struct plane_size *plane_size,
> +								      union dc_tiling_info *tiling_info,
> +								      struct dc_plane_dcc_param *dcc,
> +								      struct dc_plane_address *address,
> +								      const bool force_disable_dcc)
> +{
> +	const uint64_t modifier = afb->base.modifier;
> +	int ret = 0;
> +
> +	/* TODO: Most of this function shouldn't be needed on GFX12. */
> +	amdgpu_dm_plane_fill_gfx9_tiling_info_from_device(adev, tiling_info);
> +
> +	tiling_info->gfx9.swizzle = amdgpu_dm_plane_modifier_gfx9_swizzle_mode(modifier);
> +
> +	if (amdgpu_dm_plane_modifier_has_dcc(modifier) && !force_disable_dcc) {
> +		int max_compressed_block = AMD_FMT_MOD_GET(DCC_MAX_COMPRESSED_BLOCK, modifier);
> +
> +		dcc->enable = 1;
> +		dcc->independent_64b_blks = max_compressed_block == 0;
> +
> +		if (max_compressed_block == 0)
> +			dcc->dcc_ind_blk = hubp_ind_block_64b;
> +		else if (max_compressed_block == 1)
> +			dcc->dcc_ind_blk = hubp_ind_block_128b;
> +		else
> +			dcc->dcc_ind_blk = hubp_ind_block_unconstrained;
> +	}
> +
> +	/* TODO: This seems wrong because there is no DCC plane on GFX12. */
> +	ret = amdgpu_dm_plane_validate_dcc(adev, format, rotation, tiling_info, dcc, address, plane_size);
> +	if (ret)
> +		drm_dbg_kms(adev_to_drm(adev), "amdgpu_dm_plane_validate_dcc: returned error: %d\n", ret);

This can probably be removed, but lets do so after some more testing.

> +
> +	return ret;
> +}
> +
>   static void amdgpu_dm_plane_add_gfx10_1_modifiers(const struct amdgpu_device *adev,
>   						  uint64_t **mods,
>   						  uint64_t *size,
> @@ -835,7 +875,15 @@ int amdgpu_dm_plane_fill_plane_buffer_attributes(struct amdgpu_device *adev,
>   			upper_32_bits(chroma_addr);
>   	}
>   
> -	if (adev->family >= AMDGPU_FAMILY_AI) {
> +	if (adev->family >= AMDGPU_FAMILY_GC_12_0_0) {
> +		ret = amdgpu_dm_plane_fill_gfx12_plane_attributes_from_modifiers(adev, afb, format,
> +										 rotation, plane_size,
> +										 tiling_info, dcc,
> +										 address,
> +										 force_disable_dcc);
> +		if (ret)
> +			return ret;
> +	} else if (adev->family >= AMDGPU_FAMILY_AI) {
>   		ret = amdgpu_dm_plane_fill_gfx9_plane_attributes_from_modifiers(adev, afb, format,
>   										rotation, plane_size,
>   										tiling_info, dcc,

Reviewed-by: Aurabindo Pillai <aurabindo.pillai@amd.com>

--

Thanks & Regards,
Aurabindo Pillai

[PATCH 12/13] drm/amdgpu/display: add all gfx12 modifiers

[Marek Olšák]

Signed-off-by: Marek Olšák <marek.olsak@amd.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index e13938e01b70..0320200dae94 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -688,12 +688,13 @@ static void amdgpu_dm_plane_add_gfx11_modifiers(struct amdgpu_device *adev,
 static void amdgpu_dm_plane_add_gfx12_modifiers(struct amdgpu_device *adev,
 		      uint64_t **mods, uint64_t *size, uint64_t *capacity)
 {
-	uint64_t mod_64K_2D = AMD_FMT_MOD |
-		AMD_FMT_MOD_SET(TILE_VERSION, AMD_FMT_MOD_TILE_VER_GFX12) |
-		AMD_FMT_MOD_SET(TILE, AMD_FMT_MOD_TILE_GFX12_64K_2D);
+	uint64_t ver = AMD_FMT_MOD | AMD_FMT_MOD_SET(TILE_VERSION, AMD_FMT_MOD_TILE_VER_GFX12);
 
-	/* 64K without DCC */
-	amdgpu_dm_plane_add_modifier(mods, size, capacity, mod_64K_2D);
+	/* Without DCC: */
+	amdgpu_dm_plane_add_modifier(mods, size, capacity, ver | AMD_FMT_MOD_SET(TILE, AMD_FMT_MOD_TILE_GFX12_256K_2D));
+	amdgpu_dm_plane_add_modifier(mods, size, capacity, ver | AMD_FMT_MOD_SET(TILE, AMD_FMT_MOD_TILE_GFX12_64K_2D));
+	amdgpu_dm_plane_add_modifier(mods, size, capacity, ver | AMD_FMT_MOD_SET(TILE, AMD_FMT_MOD_TILE_GFX12_4K_2D));
+	amdgpu_dm_plane_add_modifier(mods, size, capacity, ver | AMD_FMT_MOD_SET(TILE, AMD_FMT_MOD_TILE_GFX12_256B_2D));
 	amdgpu_dm_plane_add_modifier(mods, size, capacity, DRM_FORMAT_MOD_LINEAR);
 }
 
-- 
2.34.1

Re: [PATCH 12/13] drm/amdgpu/display: add all gfx12 modifiers

[Aurabindo Pillai]

Reviewed-by: Aurabindo Pillai <aurabindo.pillai@amd.com>

On 6/26/24 2:31 PM, Marek Olšák wrote:
> Signed-off-by: Marek Olšák <marek.olsak@amd.com>
> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> index e13938e01b70..0320200dae94 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> @@ -688,12 +688,13 @@ static void amdgpu_dm_plane_add_gfx11_modifiers(struct amdgpu_device *adev,
>   static void amdgpu_dm_plane_add_gfx12_modifiers(struct amdgpu_device *adev,
>   		      uint64_t **mods, uint64_t *size, uint64_t *capacity)
>   {
> -	uint64_t mod_64K_2D = AMD_FMT_MOD |
> -		AMD_FMT_MOD_SET(TILE_VERSION, AMD_FMT_MOD_TILE_VER_GFX12) |
> -		AMD_FMT_MOD_SET(TILE, AMD_FMT_MOD_TILE_GFX12_64K_2D);
> +	uint64_t ver = AMD_FMT_MOD | AMD_FMT_MOD_SET(TILE_VERSION, AMD_FMT_MOD_TILE_VER_GFX12);
>   
> -	/* 64K without DCC */
> -	amdgpu_dm_plane_add_modifier(mods, size, capacity, mod_64K_2D);
> +	/* Without DCC: */
> +	amdgpu_dm_plane_add_modifier(mods, size, capacity, ver | AMD_FMT_MOD_SET(TILE, AMD_FMT_MOD_TILE_GFX12_256K_2D));
> +	amdgpu_dm_plane_add_modifier(mods, size, capacity, ver | AMD_FMT_MOD_SET(TILE, AMD_FMT_MOD_TILE_GFX12_64K_2D));
> +	amdgpu_dm_plane_add_modifier(mods, size, capacity, ver | AMD_FMT_MOD_SET(TILE, AMD_FMT_MOD_TILE_GFX12_4K_2D));
> +	amdgpu_dm_plane_add_modifier(mods, size, capacity, ver | AMD_FMT_MOD_SET(TILE, AMD_FMT_MOD_TILE_GFX12_256B_2D));
>   	amdgpu_dm_plane_add_modifier(mods, size, capacity, DRM_FORMAT_MOD_LINEAR);
>   }
>   

-- 
--

Thanks & Regards,
Aurabindo Pillai

[PATCH 13/13] drm/amdgpu: rewrite convert_tiling_flags_to_modifier_gfx12

[Marek Olšák]

There were multiple bugs, like checking SWIZZLE_MODE before checking
GFX12_SWIZZLE_MODE, which has undefined behavior.

The function had no effect before (it always returned -EINVAL).

Signed-off-by: Marek Olšák <marek.olsak@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 45 +++++++++------------
 1 file changed, 18 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index ec0299c5918e..092ec11258cd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -724,32 +724,25 @@ extract_render_dcc_offset(struct amdgpu_device *adev,
 
 static int convert_tiling_flags_to_modifier_gfx12(struct amdgpu_framebuffer *afb)
 {
-	struct amdgpu_device *adev = drm_to_adev(afb->base.dev);
-	const struct drm_format_info *format_info;
 	u64 modifier = 0;
-	int tile = 0;
-	int swizzle = 0;
+	int swizzle_mode = AMDGPU_TILING_GET(afb->tiling_flags, GFX12_SWIZZLE_MODE);
 
-	if (amdgpu_ip_version(adev, GC_HWIP, 0) >= IP_VERSION(12, 0, 0)) {
-		tile = AMD_FMT_MOD_TILE_VER_GFX12;
-		swizzle =  AMDGPU_TILING_GET(afb->tiling_flags, GFX12_SWIZZLE_MODE);
+	if (!swizzle_mode) {
+		modifier = DRM_FORMAT_MOD_LINEAR;
+	} else {
+		int max_comp_block =
+			AMDGPU_TILING_GET(afb->tiling_flags, GFX12_DCC_MAX_COMPRESSED_BLOCK);
+
+		modifier =
+			AMD_FMT_MOD |
+			AMD_FMT_MOD_SET(TILE_VERSION, AMD_FMT_MOD_TILE_VER_GFX12) |
+			AMD_FMT_MOD_SET(TILE, swizzle_mode) |
+			AMD_FMT_MOD_SET(DCC, afb->gfx12_dcc) |
+			AMD_FMT_MOD_SET(DCC_MAX_COMPRESSED_BLOCK, max_comp_block);
 	}
 
-	modifier =
-		AMD_FMT_MOD |
-		AMD_FMT_MOD_SET(TILE, swizzle) |
-		AMD_FMT_MOD_SET(TILE_VERSION,  tile) |
-		AMD_FMT_MOD_SET(DCC, 0) |
-		AMD_FMT_MOD_SET(DCC_MAX_COMPRESSED_BLOCK, 0);
-
-	format_info = amdgpu_lookup_format_info(afb->base.format->format,
-						modifier);
-	if (!format_info)
-		return -EINVAL;
-
 	afb->base.modifier = modifier;
 	afb->base.flags |= DRM_MODE_FB_MODIFIERS;
-
 	return 0;
 }
 
@@ -777,12 +770,6 @@ static int convert_tiling_flags_to_modifier(struct amdgpu_framebuffer *afb)
 		int pipes = ilog2(num_pipes);
 		uint32_t dcc_offset = AMDGPU_TILING_GET(afb->tiling_flags, DCC_OFFSET_256B);
 
-
-		if (amdgpu_ip_version(adev, GC_HWIP, 0) >= IP_VERSION(12, 0, 0)) {
-			convert_tiling_flags_to_modifier_gfx12(afb);
-			return 0;
-		}
-
 		switch (swizzle >> 2) {
 		case 0: /* 256B */
 			block_size_bits = 8;
@@ -1281,7 +1268,11 @@ static int amdgpu_display_framebuffer_init(struct drm_device *dev,
 
 	if (!dev->mode_config.fb_modifiers_not_supported &&
 	    !(rfb->base.flags & DRM_MODE_FB_MODIFIERS)) {
-		ret = convert_tiling_flags_to_modifier(rfb);
+		if (amdgpu_ip_version(adev, GC_HWIP, 0) >= IP_VERSION(12, 0, 0))
+			ret = convert_tiling_flags_to_modifier_gfx12(rfb);
+		else
+			ret = convert_tiling_flags_to_modifier(rfb);
+
 		if (ret) {
 			drm_dbg_kms(dev, "Failed to convert tiling flags 0x%llX to a modifier",
 				    rfb->tiling_flags);
-- 
2.34.1

Re: [PATCH 13/13] drm/amdgpu: rewrite convert_tiling_flags_to_modifier_gfx12

[Aurabindo Pillai]

Reviewed-by: Aurabindo Pillai <aurabindo.pillai@amd.com>

On 6/26/24 2:31 PM, Marek Olšák wrote:
> There were multiple bugs, like checking SWIZZLE_MODE before checking
> GFX12_SWIZZLE_MODE, which has undefined behavior.
> 
> The function had no effect before (it always returned -EINVAL).
> 
> Signed-off-by: Marek Olšák <marek.olsak@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 45 +++++++++------------
>   1 file changed, 18 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index ec0299c5918e..092ec11258cd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -724,32 +724,25 @@ extract_render_dcc_offset(struct amdgpu_device *adev,
>   
>   static int convert_tiling_flags_to_modifier_gfx12(struct amdgpu_framebuffer *afb)
>   {
> -	struct amdgpu_device *adev = drm_to_adev(afb->base.dev);
> -	const struct drm_format_info *format_info;
>   	u64 modifier = 0;
> -	int tile = 0;
> -	int swizzle = 0;
> +	int swizzle_mode = AMDGPU_TILING_GET(afb->tiling_flags, GFX12_SWIZZLE_MODE);
>   
> -	if (amdgpu_ip_version(adev, GC_HWIP, 0) >= IP_VERSION(12, 0, 0)) {
> -		tile = AMD_FMT_MOD_TILE_VER_GFX12;
> -		swizzle =  AMDGPU_TILING_GET(afb->tiling_flags, GFX12_SWIZZLE_MODE);
> +	if (!swizzle_mode) {
> +		modifier = DRM_FORMAT_MOD_LINEAR;
> +	} else {
> +		int max_comp_block =
> +			AMDGPU_TILING_GET(afb->tiling_flags, GFX12_DCC_MAX_COMPRESSED_BLOCK);
> +
> +		modifier =
> +			AMD_FMT_MOD |
> +			AMD_FMT_MOD_SET(TILE_VERSION, AMD_FMT_MOD_TILE_VER_GFX12) |
> +			AMD_FMT_MOD_SET(TILE, swizzle_mode) |
> +			AMD_FMT_MOD_SET(DCC, afb->gfx12_dcc) |
> +			AMD_FMT_MOD_SET(DCC_MAX_COMPRESSED_BLOCK, max_comp_block);
>   	}
>   
> -	modifier =
> -		AMD_FMT_MOD |
> -		AMD_FMT_MOD_SET(TILE, swizzle) |
> -		AMD_FMT_MOD_SET(TILE_VERSION,  tile) |
> -		AMD_FMT_MOD_SET(DCC, 0) |
> -		AMD_FMT_MOD_SET(DCC_MAX_COMPRESSED_BLOCK, 0);
> -
> -	format_info = amdgpu_lookup_format_info(afb->base.format->format,
> -						modifier);
> -	if (!format_info)
> -		return -EINVAL;
> -
>   	afb->base.modifier = modifier;
>   	afb->base.flags |= DRM_MODE_FB_MODIFIERS;
> -
>   	return 0;
>   }
>   
> @@ -777,12 +770,6 @@ static int convert_tiling_flags_to_modifier(struct amdgpu_framebuffer *afb)
>   		int pipes = ilog2(num_pipes);
>   		uint32_t dcc_offset = AMDGPU_TILING_GET(afb->tiling_flags, DCC_OFFSET_256B);
>   
> -
> -		if (amdgpu_ip_version(adev, GC_HWIP, 0) >= IP_VERSION(12, 0, 0)) {
> -			convert_tiling_flags_to_modifier_gfx12(afb);
> -			return 0;
> -		}
> -
>   		switch (swizzle >> 2) {
>   		case 0: /* 256B */
>   			block_size_bits = 8;
> @@ -1281,7 +1268,11 @@ static int amdgpu_display_framebuffer_init(struct drm_device *dev,
>   
>   	if (!dev->mode_config.fb_modifiers_not_supported &&
>   	    !(rfb->base.flags & DRM_MODE_FB_MODIFIERS)) {
> -		ret = convert_tiling_flags_to_modifier(rfb);
> +		if (amdgpu_ip_version(adev, GC_HWIP, 0) >= IP_VERSION(12, 0, 0))
> +			ret = convert_tiling_flags_to_modifier_gfx12(rfb);
> +		else
> +			ret = convert_tiling_flags_to_modifier(rfb);
> +
>   		if (ret) {
>   			drm_dbg_kms(dev, "Failed to convert tiling flags 0x%llX to a modifier",
>   				    rfb->tiling_flags);

-- 
--

Thanks & Regards,
Aurabindo Pillai