[PATCH V2 00/11] Enable FW assisted shadowing for GFX11

[PATCH V2 00/11] Enable FW assisted shadowing for GFX11

[Alex Deucher]

This patch set allows for FW assisted shadowing on supported
platforms.  A new enough CP FW is required.  This feature is
required for mid command buffer preemption and proper SR-IOV
support.  This also simplifies the UMDs by allowing persistent
hardware state when the command submission executes.  UMDs
that use this will have their state retained across command
submissions.

The mesa MR to implement the user mode side of this is:
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/21986

v2: Integrate feedback from V1.  UMDs now need to specify init_shadow
    on first use.

Alex Deucher (9):
  drm/amdgpu/gfx11: add FW version check for new CP GFX shadow feature
  drm/amdgpu/gfx11: check the CP FW version CP GFX shadow support
  drm/amdgpu/UAPI: add new CS chunk for GFX shadow buffers
  drm/amdgpu: don't require a job for cond_exec and shadow
  drm/amdgpu: add UAPI to query GFX shadow sizes
  drm/amdgpu: add gfx shadow callback
  drm/amdgpu: add get_gfx_shadow_info callback for gfx11
  drm/amdgpu: add support for new GFX shadow size query
  drm/amdgpu: bump driver version number for CP GFX shadow

Christian König (2):
  drm/amdgpu: add gfx shadow CS IOCTL support
  drm/amdgpu: add gfx11 emit shadow callback

 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c   | 26 +++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c  |  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h  | 14 +++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c   | 19 +++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.h  |  6 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c  | 26 +++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  3 +
 drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c   | 74 ++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/nvd.h         |  5 +-
 include/uapi/drm/amdgpu_drm.h            | 20 +++++++
 10 files changed, 192 insertions(+), 4 deletions(-)

-- 
2.39.2

[PATCH 01/11] drm/amdgpu/gfx11: add FW version check for new CP GFX shadow feature

[Alex Deucher]

Use this to determine if we support the new SET_Q_PREEMPTION_MODE
packet.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h |  2 ++
 drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c  | 13 +++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
index de9e7a00bb15..4ad9e225d6e6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
@@ -364,6 +364,8 @@ struct amdgpu_gfx {
 
 	struct amdgpu_ring		sw_gfx_ring[AMDGPU_MAX_SW_GFX_RINGS];
 	struct amdgpu_ring_mux          muxer;
+
+	bool				cp_gfx_shadow; /* for gfx11 */
 };
 
 #define amdgpu_gfx_get_gpu_clock_counter(adev) (adev)->gfx.funcs->get_gpu_clock_counter((adev))
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
index ecf8ceb53311..a0d830dc0d01 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
@@ -463,6 +463,18 @@ static int gfx_v11_0_init_toc_microcode(struct amdgpu_device *adev, const char *
 	return err;
 }
 
+static void gfx_v11_0_check_fw_cp_gfx_shadow(struct amdgpu_device *adev)
+{
+	switch (adev->ip_versions[GC_HWIP][0]) {
+	case IP_VERSION(11, 0, 0):
+	case IP_VERSION(11, 0, 2):
+	case IP_VERSION(11, 0, 3):
+	default:
+		adev->gfx.cp_gfx_shadow = false;
+		break;
+	}
+}
+
 static int gfx_v11_0_init_microcode(struct amdgpu_device *adev)
 {
 	char fw_name[40];
@@ -539,6 +551,7 @@ static int gfx_v11_0_init_microcode(struct amdgpu_device *adev)
 	/* only one MEC for gfx 11.0.0. */
 	adev->gfx.mec2_fw = NULL;
 
+	gfx_v11_0_check_fw_cp_gfx_shadow(adev);
 out:
 	if (err) {
 		amdgpu_ucode_release(&adev->gfx.pfp_fw);
-- 
2.39.2

[PATCH 02/11] drm/amdgpu/gfx11: check the CP FW version CP GFX shadow support

[Alex Deucher]

Only set the supported flag if we have new enough CP FW.

XXX: don't commit this until the CP FW versions are finalized!

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
index a0d830dc0d01..4a50d0fbcdcf 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
@@ -469,6 +469,15 @@ static void gfx_v11_0_check_fw_cp_gfx_shadow(struct amdgpu_device *adev)
 	case IP_VERSION(11, 0, 0):
 	case IP_VERSION(11, 0, 2):
 	case IP_VERSION(11, 0, 3):
+		/* XXX fix me! */
+		if ((adev->gfx.me_fw_version >= 1498) &&
+		    (adev->gfx.me_feature_version >= 29) &&
+		    (adev->gfx.pfp_fw_version >= 1541) &&
+		    (adev->gfx.pfp_feature_version >= 29) &&
+		    (adev->gfx.mec_fw_version >= 507) &&
+		    (adev->gfx.mec_feature_version >= 29))
+			adev->gfx.cp_gfx_shadow = true;
+		break;
 	default:
 		adev->gfx.cp_gfx_shadow = false;
 		break;
-- 
2.39.2

[PATCH 03/11] drm/amdgpu/UAPI: add new CS chunk for GFX shadow buffers

[Alex Deucher]

For GFX11, the UMD needs to allocate some shadow buffers
to be used for preemption.  The UMD allocates the buffers
and passes the GPU virtual address to the kernel since the
kernel will program the packet that specified these
addresses as part of its IB submission frame.

v2: UMD passes shadow init to tell kernel when to initialize
    the shadow

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 include/uapi/drm/amdgpu_drm.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index b6eb90df5d05..3d9474af6566 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -592,6 +592,7 @@ struct drm_amdgpu_gem_va {
 #define AMDGPU_CHUNK_ID_SCHEDULED_DEPENDENCIES	0x07
 #define AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_WAIT    0x08
 #define AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_SIGNAL  0x09
+#define AMDGPU_CHUNK_ID_CP_GFX_SHADOW   0x0a
 
 struct drm_amdgpu_cs_chunk {
 	__u32		chunk_id;
@@ -708,6 +709,15 @@ struct drm_amdgpu_cs_chunk_data {
 	};
 };
 
+#define AMDGPU_CS_CHUNK_CP_GFX_SHADOW_FLAGS_INIT_SHADOW         0x1
+
+struct drm_amdgpu_cs_chunk_cp_gfx_shadow {
+	__u64 shadow_va;
+	__u64 csa_va;
+	__u64 gds_va;
+	__u64 flags;
+};
+
 /*
  *  Query h/w info: Flag that this is integrated (a.h.a. fusion) GPU
  *
-- 
2.39.2

[PATCH 04/11] drm/amdgpu: add gfx shadow CS IOCTL support

[Alex Deucher]

From: Christian König <christian.koenig@amd.com>

Add support for submitting the shadow update packet
when submitting an IB.  Needed for MCBP on GFX11.

v2: update API for CSA (Alex)
v3: fix ordering; SET_Q_PREEMPTION_MODE most come before COND_EXEC
    Add missing check for AMDGPU_CHUNK_ID_CP_GFX_SHADOW in
    amdgpu_cs_pass1()
    Only initialize shadow on first use
    (Alex)
v4: Pass parameters rather than job to new ring callback (Alex)

Signed-off-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c   | 26 ++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c   | 17 +++++++++++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.h  |  6 ++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  3 +++
 4 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index f6144b378617..50cc117bc628 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -280,6 +280,7 @@ static int amdgpu_cs_pass1(struct amdgpu_cs_parser *p,
 		case AMDGPU_CHUNK_ID_SCHEDULED_DEPENDENCIES:
 		case AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_WAIT:
 		case AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_SIGNAL:
+		case AMDGPU_CHUNK_ID_CP_GFX_SHADOW:
 			break;
 
 		default:
@@ -587,6 +588,26 @@ static int amdgpu_cs_p2_syncobj_timeline_signal(struct amdgpu_cs_parser *p,
 	return 0;
 }
 
+static int amdgpu_cs_p2_shadow(struct amdgpu_cs_parser *p,
+			       struct amdgpu_cs_chunk *chunk)
+{
+	struct drm_amdgpu_cs_chunk_cp_gfx_shadow *shadow = chunk->kdata;
+	int i;
+
+	if (shadow->flags & ~AMDGPU_CS_CHUNK_CP_GFX_SHADOW_FLAGS_INIT_SHADOW)
+		return -EINVAL;
+
+	for (i = 0; i < p->gang_size; ++i) {
+		p->jobs[i]->shadow_va = shadow->shadow_va;
+		p->jobs[i]->csa_va = shadow->csa_va;
+		p->jobs[i]->gds_va = shadow->gds_va;
+		p->jobs[i]->init_shadow =
+			shadow->flags & AMDGPU_CS_CHUNK_CP_GFX_SHADOW_FLAGS_INIT_SHADOW;
+	}
+
+	return 0;
+}
+
 static int amdgpu_cs_pass2(struct amdgpu_cs_parser *p)
 {
 	unsigned int ce_preempt = 0, de_preempt = 0;
@@ -629,6 +650,11 @@ static int amdgpu_cs_pass2(struct amdgpu_cs_parser *p)
 			if (r)
 				return r;
 			break;
+		case AMDGPU_CHUNK_ID_CP_GFX_SHADOW:
+			r = amdgpu_cs_p2_shadow(p, chunk);
+			if (r)
+				return r;
+			break;
 		}
 	}
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index bcccc348dbe2..7fb8e6691d13 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -136,7 +136,9 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 	uint64_t fence_ctx;
 	uint32_t status = 0, alloc_size;
 	unsigned fence_flags = 0;
-	bool secure;
+	bool secure, init_shadow;
+	u64 shadow_va, csa_va, gds_va;
+	int vmid = AMDGPU_JOB_GET_VMID(job);
 
 	unsigned i;
 	int r = 0;
@@ -150,9 +152,17 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 		vm = job->vm;
 		fence_ctx = job->base.s_fence ?
 			job->base.s_fence->scheduled.context : 0;
+		shadow_va = job->shadow_va;
+		csa_va = job->csa_va;
+		gds_va = job->gds_va;
+		init_shadow = job->init_shadow;
 	} else {
 		vm = NULL;
 		fence_ctx = 0;
+		shadow_va = 0;
+		csa_va = 0;
+		gds_va = 0;
+		init_shadow = false;
 	}
 
 	if (!ring->sched.ready && !ring->is_mes_queue) {
@@ -212,6 +222,11 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 	}
 
 	amdgpu_ring_ib_begin(ring);
+
+	if (job && ring->funcs->emit_gfx_shadow)
+		amdgpu_ring_emit_gfx_shadow(ring, shadow_va, csa_va, gds_va,
+					    init_shadow, vmid);
+
 	if (job && ring->funcs->init_cond_exec)
 		patch_offset = amdgpu_ring_init_cond_exec(ring);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
index 9790def34815..b470808fa40e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
@@ -68,6 +68,12 @@ struct amdgpu_job {
 	uint64_t		uf_addr;
 	uint64_t		uf_sequence;
 
+	/* virtual addresses for shadow/GDS/CSA */
+	uint64_t		shadow_va;
+	uint64_t		csa_va;
+	uint64_t		gds_va;
+	bool			init_shadow;
+
 	/* job_run_counter >= 1 means a resubmit job */
 	uint32_t		job_run_counter;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index 3989e755a5b4..7942cb62e52c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -212,6 +212,8 @@ struct amdgpu_ring_funcs {
 	void (*end_use)(struct amdgpu_ring *ring);
 	void (*emit_switch_buffer) (struct amdgpu_ring *ring);
 	void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags);
+	void (*emit_gfx_shadow)(struct amdgpu_ring *ring, u64 shadow_va, u64 csa_va,
+				u64 gds_va, bool init_shadow, int vmid);
 	void (*emit_rreg)(struct amdgpu_ring *ring, uint32_t reg,
 			  uint32_t reg_val_offs);
 	void (*emit_wreg)(struct amdgpu_ring *ring, uint32_t reg, uint32_t val);
@@ -307,6 +309,7 @@ struct amdgpu_ring {
 #define amdgpu_ring_emit_hdp_flush(r) (r)->funcs->emit_hdp_flush((r))
 #define amdgpu_ring_emit_switch_buffer(r) (r)->funcs->emit_switch_buffer((r))
 #define amdgpu_ring_emit_cntxcntl(r, d) (r)->funcs->emit_cntxcntl((r), (d))
+#define amdgpu_ring_emit_gfx_shadow(r, s, c, g, i, v) (r)->funcs->emit_gfx_shadow((r), (s), (c), (g), (i), (v))
 #define amdgpu_ring_emit_rreg(r, d, o) (r)->funcs->emit_rreg((r), (d), (o))
 #define amdgpu_ring_emit_wreg(r, d, v) (r)->funcs->emit_wreg((r), (d), (v))
 #define amdgpu_ring_emit_reg_wait(r, d, v, m) (r)->funcs->emit_reg_wait((r), (d), (v), (m))
-- 
2.39.2

[PATCH 05/11] drm/amdgpu: add gfx11 emit shadow callback

[Alex Deucher]

From: Christian König <christian.koenig@amd.com>

Add ring callback for gfx to update the CP firmware
with the new shadow information before we process the
IB.

v2: add implementation for new packet (Alex)
v3: add current FW version checks (Alex)
v4: only initialize shadow on first use
    Only set IB_VMID when a valid shadow buffer is present
    (Alex)
v5: Pass parameters rather than job to new ring callback (Alex)

Signed-off-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 25 +++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/nvd.h       |  5 ++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
index 4a50d0fbcdcf..1fc1e941f7df 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
@@ -5598,6 +5598,29 @@ static void gfx_v11_0_ring_emit_cntxcntl(struct amdgpu_ring *ring,
 	amdgpu_ring_write(ring, 0);
 }
 
+static void gfx_v11_0_ring_emit_gfx_shadow(struct amdgpu_ring *ring,
+					   u64 shadow_va, u64 csa_va,
+					   u64 gds_va, bool init_shadow,
+					   int vmid)
+{
+	struct amdgpu_device *adev = ring->adev;
+
+	if (!adev->gfx.cp_gfx_shadow)
+		return;
+
+	amdgpu_ring_write(ring, PACKET3(PACKET3_SET_Q_PREEMPTION_MODE, 7));
+	amdgpu_ring_write(ring, lower_32_bits(shadow_va));
+	amdgpu_ring_write(ring, upper_32_bits(shadow_va));
+	amdgpu_ring_write(ring, lower_32_bits(gds_va));
+	amdgpu_ring_write(ring, upper_32_bits(gds_va));
+	amdgpu_ring_write(ring, lower_32_bits(csa_va));
+	amdgpu_ring_write(ring, upper_32_bits(csa_va));
+	amdgpu_ring_write(ring, shadow_va ?
+			  PACKET3_SET_Q_PREEMPTION_MODE_IB_VMID(vmid): 0);
+	amdgpu_ring_write(ring, init_shadow ?
+			  PACKET3_SET_Q_PREEMPTION_MODE_INIT_SHADOW_MEM : 0);
+}
+
 static unsigned gfx_v11_0_ring_emit_init_cond_exec(struct amdgpu_ring *ring)
 {
 	unsigned ret;
@@ -6219,6 +6242,7 @@ static const struct amdgpu_ring_funcs gfx_v11_0_ring_funcs_gfx = {
 	.set_wptr = gfx_v11_0_ring_set_wptr_gfx,
 	.emit_frame_size = /* totally 242 maximum if 16 IBs */
 		5 + /* COND_EXEC */
+		9 + /* SET_Q_PREEMPTION_MODE */
 		7 + /* PIPELINE_SYNC */
 		SOC15_FLUSH_GPU_TLB_NUM_WREG * 5 +
 		SOC15_FLUSH_GPU_TLB_NUM_REG_WAIT * 7 +
@@ -6245,6 +6269,7 @@ static const struct amdgpu_ring_funcs gfx_v11_0_ring_funcs_gfx = {
 	.insert_nop = amdgpu_ring_insert_nop,
 	.pad_ib = amdgpu_ring_generic_pad_ib,
 	.emit_cntxcntl = gfx_v11_0_ring_emit_cntxcntl,
+	.emit_gfx_shadow = gfx_v11_0_ring_emit_gfx_shadow,
 	.init_cond_exec = gfx_v11_0_ring_emit_init_cond_exec,
 	.patch_cond_exec = gfx_v11_0_ring_emit_patch_cond_exec,
 	.preempt_ib = gfx_v11_0_ring_preempt_ib,
diff --git a/drivers/gpu/drm/amd/amdgpu/nvd.h b/drivers/gpu/drm/amd/amdgpu/nvd.h
index fd6b58243b03..631dafb92299 100644
--- a/drivers/gpu/drm/amd/amdgpu/nvd.h
+++ b/drivers/gpu/drm/amd/amdgpu/nvd.h
@@ -462,6 +462,9 @@
 #              define PACKET3_QUERY_STATUS_ENG_SEL(x)          ((x) << 25)
 #define	PACKET3_RUN_LIST				0xA5
 #define	PACKET3_MAP_PROCESS_VM				0xA6
-
+/* GFX11 */
+#define	PACKET3_SET_Q_PREEMPTION_MODE			0xF0
+#              define PACKET3_SET_Q_PREEMPTION_MODE_IB_VMID(x)  ((x) << 0)
+#              define PACKET3_SET_Q_PREEMPTION_MODE_INIT_SHADOW_MEM    (1 << 0)
 
 #endif
-- 
2.39.2

[PATCH 06/11] drm/amdgpu: don't require a job for cond_exec and shadow

[Alex Deucher]

We need to reset the shadow state every time we submit an
IB and there needs to be a COND_EXEC packet after the
SET_Q_PREEMPTION_MODE packet for it to work properly, so
we should emit both of these packets regardless of whether
there is a job present or not.

Reviewed-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 7fb8e6691d13..3b35e21eb934 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -223,11 +223,11 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 
 	amdgpu_ring_ib_begin(ring);
 
-	if (job && ring->funcs->emit_gfx_shadow)
+	if (ring->funcs->emit_gfx_shadow)
 		amdgpu_ring_emit_gfx_shadow(ring, shadow_va, csa_va, gds_va,
 					    init_shadow, vmid);
 
-	if (job && ring->funcs->init_cond_exec)
+	if (ring->funcs->init_cond_exec)
 		patch_offset = amdgpu_ring_init_cond_exec(ring);
 
 	amdgpu_device_flush_hdp(adev, ring);
-- 
2.39.2

[PATCH 07/11] drm/amdgpu: add UAPI to query GFX shadow sizes

[Alex Deucher]

Add UAPI to query the GFX shadow buffer requirements
for preemption on GFX11.  UMDs need to specify the shadow
areas for preemption.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 include/uapi/drm/amdgpu_drm.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index 3d9474af6566..19a806145371 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -886,6 +886,7 @@ struct drm_amdgpu_cs_chunk_cp_gfx_shadow {
 	#define AMDGPU_INFO_VIDEO_CAPS_DECODE		0
 	/* Subquery id: Encode */
 	#define AMDGPU_INFO_VIDEO_CAPS_ENCODE		1
+#define AMDGPU_INFO_CP_GFX_SHADOW_SIZE		0x22
 
 #define AMDGPU_INFO_MMR_SE_INDEX_SHIFT	0
 #define AMDGPU_INFO_MMR_SE_INDEX_MASK	0xff
@@ -1203,6 +1204,15 @@ struct drm_amdgpu_info_video_caps {
 	struct drm_amdgpu_info_video_codec_info codec_info[AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_COUNT];
 };
 
+struct drm_amdgpu_info_cp_gfx_shadow_size {
+	__u32 shadow_size;
+	__u32 shadow_alignment;
+	__u32 csa_size;
+	__u32 csa_alignment;
+	__u32 gds_size;
+	__u32 gds_alignment;
+};
+
 /*
  * Supported GPU families
  */
-- 
2.39.2

[PATCH 08/11] drm/amdgpu: add gfx shadow callback

[Alex Deucher]

To provide IP specific shadow sizes.  UMDs will use
this to query the kernel driver for the size of the
shadow buffers.

v2: make callback return an int (Alex)

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
index 4ad9e225d6e6..8826f1efc75f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
@@ -219,6 +219,15 @@ struct amdgpu_gfx_ras {
 						struct amdgpu_iv_entry *entry);
 };
 
+struct amdgpu_gfx_shadow_info {
+	u32 shadow_size;
+	u32 shadow_alignment;
+	u32 csa_size;
+	u32 csa_alignment;
+	u32 gds_size;
+	u32 gds_alignment;
+};
+
 struct amdgpu_gfx_funcs {
 	/* get the gpu clock counter */
 	uint64_t (*get_gpu_clock_counter)(struct amdgpu_device *adev);
@@ -236,6 +245,8 @@ struct amdgpu_gfx_funcs {
 				 u32 queue, u32 vmid);
 	void (*init_spm_golden)(struct amdgpu_device *adev);
 	void (*update_perfmon_mgcg)(struct amdgpu_device *adev, bool enable);
+	int (*get_gfx_shadow_info)(struct amdgpu_device *adev,
+				   struct amdgpu_gfx_shadow_info *shadow_info);
 };
 
 struct sq_work {
@@ -372,6 +383,7 @@ struct amdgpu_gfx {
 #define amdgpu_gfx_select_se_sh(adev, se, sh, instance) (adev)->gfx.funcs->select_se_sh((adev), (se), (sh), (instance))
 #define amdgpu_gfx_select_me_pipe_q(adev, me, pipe, q, vmid) (adev)->gfx.funcs->select_me_pipe_q((adev), (me), (pipe), (q), (vmid))
 #define amdgpu_gfx_init_spm_golden(adev) (adev)->gfx.funcs->init_spm_golden((adev))
+#define amdgpu_gfx_get_gfx_shadow_info(adev, si) (adev)->gfx.funcs->get_gfx_shadow_info((adev), (si))
 
 /**
  * amdgpu_gfx_create_bitmask - create a bitmask
-- 
2.39.2

[PATCH 09/11] drm/amdgpu: add get_gfx_shadow_info callback for gfx11

[Alex Deucher]

Used to get the size and alignment requirements for
the gfx shadow buffer for preemption.

v2: use FW version check to determine whether to
    return a valid size here
    return an error if not supported (Alex)

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 27 ++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
index 1fc1e941f7df..d6aeaf530fe2 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
@@ -822,6 +822,32 @@ static void gfx_v11_0_select_me_pipe_q(struct amdgpu_device *adev,
 	soc21_grbm_select(adev, me, pipe, q, vm);
 }
 
+/* all sizes are in bytes */
+#define MQD_SHADOW_BASE_SIZE      73728
+#define MQD_SHADOW_BASE_ALIGNMENT 256
+#define MQD_FWWORKAREA_SIZE       484
+#define MQD_FWWORKAREA_ALIGNMENT  256
+
+static int gfx_v11_0_get_gfx_shadow_info(struct amdgpu_device *adev,
+					 struct amdgpu_gfx_shadow_info *shadow_info)
+{
+	if (shadow_info) {
+		if (adev->gfx.cp_gfx_shadow) {
+			shadow_info->shadow_size = MQD_SHADOW_BASE_SIZE;
+			shadow_info->shadow_alignment = MQD_SHADOW_BASE_ALIGNMENT;
+			shadow_info->csa_size = MQD_FWWORKAREA_SIZE;
+			shadow_info->csa_alignment = MQD_FWWORKAREA_ALIGNMENT;
+			shadow_info->gds_size = 0x1000;
+			shadow_info->gds_alignment = 256;
+			return 0;
+		} else {
+			memset(shadow_info, 0, sizeof(struct amdgpu_gfx_shadow_info));
+			return -ENOTSUPP;
+		}
+	}
+	return -EINVAL;
+}
+
 static const struct amdgpu_gfx_funcs gfx_v11_0_gfx_funcs = {
 	.get_gpu_clock_counter = &gfx_v11_0_get_gpu_clock_counter,
 	.select_se_sh = &gfx_v11_0_select_se_sh,
@@ -830,6 +856,7 @@ static const struct amdgpu_gfx_funcs gfx_v11_0_gfx_funcs = {
 	.read_wave_vgprs = &gfx_v11_0_read_wave_vgprs,
 	.select_me_pipe_q = &gfx_v11_0_select_me_pipe_q,
 	.update_perfmon_mgcg = &gfx_v11_0_update_perf_clk,
+	.get_gfx_shadow_info = &gfx_v11_0_get_gfx_shadow_info,
 };
 
 static int gfx_v11_0_gpu_early_init(struct amdgpu_device *adev)
-- 
2.39.2

[PATCH 10/11] drm/amdgpu: add support for new GFX shadow size query

[Alex Deucher]

Use the new callback to fetch the data.  Return an error if
not supported.  UMDs should use this query to check whether
shadow buffers are supported and if so what size they
should be.

v2: return an error rather than a zerod structure.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 26 +++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 9e85eedb57d8..8a6764756dcf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -1139,6 +1139,32 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 		kfree(caps);
 		return r;
 	}
+	case AMDGPU_INFO_CP_GFX_SHADOW_SIZE: {
+		struct amdgpu_gfx_shadow_info shadow_info;
+		struct drm_amdgpu_info_cp_gfx_shadow_size drm_shadow_size;
+		int r;
+
+		memset(&shadow_info, 0, sizeof(struct amdgpu_gfx_shadow_info));
+		if (adev->gfx.funcs->get_gfx_shadow_info) {
+			r = amdgpu_gfx_get_gfx_shadow_info(adev, &shadow_info);
+			if (r)
+				return r;
+			drm_shadow_size.shadow_size = shadow_info.shadow_size;
+			drm_shadow_size.shadow_alignment = shadow_info.shadow_alignment;
+			drm_shadow_size.csa_size = shadow_info.csa_size;
+			drm_shadow_size.csa_alignment = shadow_info.csa_alignment;
+			drm_shadow_size.gds_size = shadow_info.gds_size;
+			drm_shadow_size.gds_alignment = shadow_info.gds_alignment;
+		} else {
+			return -ENOTSUPP;
+		}
+		r = copy_to_user(out, &drm_shadow_size,
+				 min((size_t)size,
+				     sizeof(struct drm_amdgpu_info_cp_gfx_shadow_size))) ?
+			-EFAULT : 0;
+		return r;
+
+	}
 	default:
 		DRM_DEBUG_KMS("Invalid request %d\n", info->query);
 		return -EINVAL;
-- 
2.39.2

[PATCH 11/11] drm/amdgpu: bump driver version number for CP GFX shadow

[Alex Deucher]

So UMDs can determine whether the kernel supports this.

Mesa MR: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/21986

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 64214996278b..81e409501f58 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -110,9 +110,10 @@
  *   3.52.0 - Add AMDGPU_IDS_FLAGS_CONFORMANT_TRUNC_COORD, add device_info fields:
  *            tcp_cache_size, num_sqc_per_wgp, sqc_data_cache_size, sqc_inst_cache_size,
  *            gl1c_cache_size, gl2c_cache_size, mall_size, enabled_rb_pipes_mask_hi
+ *   3.53.0 - Support for GFX11 CP GFX shadowing
  */
 #define KMS_DRIVER_MAJOR	3
-#define KMS_DRIVER_MINOR	52
+#define KMS_DRIVER_MINOR	53
 #define KMS_DRIVER_PATCHLEVEL	0
 
 unsigned int amdgpu_vram_limit = UINT_MAX;
-- 
2.39.2

Re: [PATCH 07/11] drm/amdgpu: add UAPI to query GFX shadow sizes

[Marek Olšák]

[-- Attachment #1: Type: text/plain, Size: 1524 bytes --]

On Mon, Mar 20, 2023 at 1:38 PM Alex Deucher <alexander.deucher@amd.com>
wrote:

> Add UAPI to query the GFX shadow buffer requirements
> for preemption on GFX11.  UMDs need to specify the shadow
> areas for preemption.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  include/uapi/drm/amdgpu_drm.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index 3d9474af6566..19a806145371 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -886,6 +886,7 @@ struct drm_amdgpu_cs_chunk_cp_gfx_shadow {
>         #define AMDGPU_INFO_VIDEO_CAPS_DECODE           0
>         /* Subquery id: Encode */
>         #define AMDGPU_INFO_VIDEO_CAPS_ENCODE           1
> +#define AMDGPU_INFO_CP_GFX_SHADOW_SIZE         0x22
>
>  #define AMDGPU_INFO_MMR_SE_INDEX_SHIFT 0
>  #define AMDGPU_INFO_MMR_SE_INDEX_MASK  0xff
> @@ -1203,6 +1204,15 @@ struct drm_amdgpu_info_video_caps {
>         struct drm_amdgpu_info_video_codec_info
> codec_info[AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_COUNT];
>  };
>
> +struct drm_amdgpu_info_cp_gfx_shadow_size {
> +       __u32 shadow_size;
> +       __u32 shadow_alignment;
> +       __u32 csa_size;
> +       __u32 csa_alignment;
> +       __u32 gds_size;
> +       __u32 gds_alignment;
>

Can you document the fields? What is CSA? Also, why is GDS there when the
hw deprecated it and replaced it with GDS registers?

Thanks,
Marek

[-- Attachment #2: Type: text/html, Size: 2053 bytes --]

Re: [PATCH 07/11] drm/amdgpu: add UAPI to query GFX shadow sizes

[Marek Olšák]

[-- Attachment #1: Type: text/plain, Size: 1009 bytes --]

On Mon, Mar 20, 2023 at 1:38 PM Alex Deucher <alexander.deucher@amd.com>
wrote:

> Add UAPI to query the GFX shadow buffer requirements
> for preemption on GFX11.  UMDs need to specify the shadow
> areas for preemption.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  include/uapi/drm/amdgpu_drm.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index 3d9474af6566..19a806145371 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -886,6 +886,7 @@ struct drm_amdgpu_cs_chunk_cp_gfx_shadow {
>         #define AMDGPU_INFO_VIDEO_CAPS_DECODE           0
>         /* Subquery id: Encode */
>         #define AMDGPU_INFO_VIDEO_CAPS_ENCODE           1
> +#define AMDGPU_INFO_CP_GFX_SHADOW_SIZE         0x22
>

Can you put this into the device structure instead? Let's minimize the
number of kernel queries as much as possible.

Thanks,
Marek

[-- Attachment #2: Type: text/html, Size: 1503 bytes --]

Re: [PATCH 02/11] drm/amdgpu/gfx11: check the CP FW version CP GFX shadow support

[Christian König]

Am 20.03.23 um 18:38 schrieb Alex Deucher:
> Only set the supported flag if we have new enough CP FW.
>
> XXX: don't commit this until the CP FW versions are finalized!
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

Acked-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> index a0d830dc0d01..4a50d0fbcdcf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> @@ -469,6 +469,15 @@ static void gfx_v11_0_check_fw_cp_gfx_shadow(struct amdgpu_device *adev)
>   	case IP_VERSION(11, 0, 0):
>   	case IP_VERSION(11, 0, 2):
>   	case IP_VERSION(11, 0, 3):
> +		/* XXX fix me! */
> +		if ((adev->gfx.me_fw_version >= 1498) &&
> +		    (adev->gfx.me_feature_version >= 29) &&
> +		    (adev->gfx.pfp_fw_version >= 1541) &&
> +		    (adev->gfx.pfp_feature_version >= 29) &&
> +		    (adev->gfx.mec_fw_version >= 507) &&
> +		    (adev->gfx.mec_feature_version >= 29))
> +			adev->gfx.cp_gfx_shadow = true;
> +		break;
>   	default:
>   		adev->gfx.cp_gfx_shadow = false;
>   		break;

Re: [PATCH 07/11] drm/amdgpu: add UAPI to query GFX shadow sizes

[Alex Deucher]

On Mon, Mar 20, 2023 at 8:30 PM Marek Olšák <maraeo@gmail.com> wrote:
>
>
> On Mon, Mar 20, 2023 at 1:38 PM Alex Deucher <alexander.deucher@amd.com> wrote:
>>
>> Add UAPI to query the GFX shadow buffer requirements
>> for preemption on GFX11.  UMDs need to specify the shadow
>> areas for preemption.
>>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> ---
>>  include/uapi/drm/amdgpu_drm.h | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
>> index 3d9474af6566..19a806145371 100644
>> --- a/include/uapi/drm/amdgpu_drm.h
>> +++ b/include/uapi/drm/amdgpu_drm.h
>> @@ -886,6 +886,7 @@ struct drm_amdgpu_cs_chunk_cp_gfx_shadow {
>>         #define AMDGPU_INFO_VIDEO_CAPS_DECODE           0
>>         /* Subquery id: Encode */
>>         #define AMDGPU_INFO_VIDEO_CAPS_ENCODE           1
>> +#define AMDGPU_INFO_CP_GFX_SHADOW_SIZE         0x22
>>
>>  #define AMDGPU_INFO_MMR_SE_INDEX_SHIFT 0
>>  #define AMDGPU_INFO_MMR_SE_INDEX_MASK  0xff
>> @@ -1203,6 +1204,15 @@ struct drm_amdgpu_info_video_caps {
>>         struct drm_amdgpu_info_video_codec_info codec_info[AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_COUNT];
>>  };
>>
>> +struct drm_amdgpu_info_cp_gfx_shadow_size {
>> +       __u32 shadow_size;
>> +       __u32 shadow_alignment;
>> +       __u32 csa_size;
>> +       __u32 csa_alignment;
>> +       __u32 gds_size;
>> +       __u32 gds_alignment;
>
>
> Can you document the fields? What is CSA? Also, why is GDS there when the hw deprecated it and replaced it with GDS registers?

Will add documentation.  For reference:
CSA (Context Save Area) - used as a scratch area for FW for saving
various things
Shadow - stores the pipeline state
GDS backup - stores the GDS state used by the pipeline.  I'm not sure
if this is registers or the old GDS memory.  Presumably the former.

Alex

>
> Thanks,
> Marek

Re: [PATCH 07/11] drm/amdgpu: add UAPI to query GFX shadow sizes

[Alex Deucher]

On Mon, Mar 20, 2023 at 8:31 PM Marek Olšák <maraeo@gmail.com> wrote:
>
> On Mon, Mar 20, 2023 at 1:38 PM Alex Deucher <alexander.deucher@amd.com> wrote:
>>
>> Add UAPI to query the GFX shadow buffer requirements
>> for preemption on GFX11.  UMDs need to specify the shadow
>> areas for preemption.
>>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> ---
>>  include/uapi/drm/amdgpu_drm.h | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
>> index 3d9474af6566..19a806145371 100644
>> --- a/include/uapi/drm/amdgpu_drm.h
>> +++ b/include/uapi/drm/amdgpu_drm.h
>> @@ -886,6 +886,7 @@ struct drm_amdgpu_cs_chunk_cp_gfx_shadow {
>>         #define AMDGPU_INFO_VIDEO_CAPS_DECODE           0
>>         /* Subquery id: Encode */
>>         #define AMDGPU_INFO_VIDEO_CAPS_ENCODE           1
>> +#define AMDGPU_INFO_CP_GFX_SHADOW_SIZE         0x22
>
>
> Can you put this into the device structure instead? Let's minimize the number of kernel queries as much as possible.

I guess, but one nice thing about this is that we can use the query as
a way to determine if the kernel supports this functionality or not.
If not, the query returns -ENOTSUP.

Alex


>
> Thanks,
> Marek
>

Re: [PATCH 07/11] drm/amdgpu: add UAPI to query GFX shadow sizes

[Christian König]

Am 21.03.23 um 20:53 schrieb Alex Deucher:
> On Mon, Mar 20, 2023 at 8:31 PM Marek Olšák <maraeo@gmail.com> wrote:
>> On Mon, Mar 20, 2023 at 1:38 PM Alex Deucher <alexander.deucher@amd.com> wrote:
>>> Add UAPI to query the GFX shadow buffer requirements
>>> for preemption on GFX11.  UMDs need to specify the shadow
>>> areas for preemption.
>>>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> ---
>>>   include/uapi/drm/amdgpu_drm.h | 10 ++++++++++
>>>   1 file changed, 10 insertions(+)
>>>
>>> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
>>> index 3d9474af6566..19a806145371 100644
>>> --- a/include/uapi/drm/amdgpu_drm.h
>>> +++ b/include/uapi/drm/amdgpu_drm.h
>>> @@ -886,6 +886,7 @@ struct drm_amdgpu_cs_chunk_cp_gfx_shadow {
>>>          #define AMDGPU_INFO_VIDEO_CAPS_DECODE           0
>>>          /* Subquery id: Encode */
>>>          #define AMDGPU_INFO_VIDEO_CAPS_ENCODE           1
>>> +#define AMDGPU_INFO_CP_GFX_SHADOW_SIZE         0x22
>>
>> Can you put this into the device structure instead? Let's minimize the number of kernel queries as much as possible.
> I guess, but one nice thing about this is that we can use the query as
> a way to determine if the kernel supports this functionality or not.
> If not, the query returns -ENOTSUP.

Well if we put it at the end of the device info structure the sizes 
should be zero on older kernels/fw.

That will also work nicely.

Christian.

>
> Alex
>
>
>> Thanks,
>> Marek
>>

Re: [PATCH 07/11] drm/amdgpu: add UAPI to query GFX shadow sizes

[Marek Olšák]

[-- Attachment #1: Type: text/plain, Size: 1581 bytes --]

On Tue, Mar 21, 2023 at 3:54 PM Alex Deucher <alexdeucher@gmail.com> wrote:

> On Mon, Mar 20, 2023 at 8:31 PM Marek Olšák <maraeo@gmail.com> wrote:
> >
> > On Mon, Mar 20, 2023 at 1:38 PM Alex Deucher <alexander.deucher@amd.com>
> wrote:
> >>
> >> Add UAPI to query the GFX shadow buffer requirements
> >> for preemption on GFX11.  UMDs need to specify the shadow
> >> areas for preemption.
> >>
> >> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> >> ---
> >>  include/uapi/drm/amdgpu_drm.h | 10 ++++++++++
> >>  1 file changed, 10 insertions(+)
> >>
> >> diff --git a/include/uapi/drm/amdgpu_drm.h
> b/include/uapi/drm/amdgpu_drm.h
> >> index 3d9474af6566..19a806145371 100644
> >> --- a/include/uapi/drm/amdgpu_drm.h
> >> +++ b/include/uapi/drm/amdgpu_drm.h
> >> @@ -886,6 +886,7 @@ struct drm_amdgpu_cs_chunk_cp_gfx_shadow {
> >>         #define AMDGPU_INFO_VIDEO_CAPS_DECODE           0
> >>         /* Subquery id: Encode */
> >>         #define AMDGPU_INFO_VIDEO_CAPS_ENCODE           1
> >> +#define AMDGPU_INFO_CP_GFX_SHADOW_SIZE         0x22
> >
> >
> > Can you put this into the device structure instead? Let's minimize the
> number of kernel queries as much as possible.
>
> I guess, but one nice thing about this is that we can use the query as
> a way to determine if the kernel supports this functionality or not.
> If not, the query returns -ENOTSUP.
>

That should be another flag in the device info structure or the sizes
should be 0. There is never a reason to add a new single-value INFO query.

Marek

[-- Attachment #2: Type: text/html, Size: 2365 bytes --]

Re: [PATCH 07/11] drm/amdgpu: add UAPI to query GFX shadow sizes

[Marek Olšák]

[-- Attachment #1: Type: text/plain, Size: 2385 bytes --]

On Tue, Mar 21, 2023 at 3:51 PM Alex Deucher <alexdeucher@gmail.com> wrote:

> On Mon, Mar 20, 2023 at 8:30 PM Marek Olšák <maraeo@gmail.com> wrote:
> >
> >
> > On Mon, Mar 20, 2023 at 1:38 PM Alex Deucher <alexander.deucher@amd.com>
> wrote:
> >>
> >> Add UAPI to query the GFX shadow buffer requirements
> >> for preemption on GFX11.  UMDs need to specify the shadow
> >> areas for preemption.
> >>
> >> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> >> ---
> >>  include/uapi/drm/amdgpu_drm.h | 10 ++++++++++
> >>  1 file changed, 10 insertions(+)
> >>
> >> diff --git a/include/uapi/drm/amdgpu_drm.h
> b/include/uapi/drm/amdgpu_drm.h
> >> index 3d9474af6566..19a806145371 100644
> >> --- a/include/uapi/drm/amdgpu_drm.h
> >> +++ b/include/uapi/drm/amdgpu_drm.h
> >> @@ -886,6 +886,7 @@ struct drm_amdgpu_cs_chunk_cp_gfx_shadow {
> >>         #define AMDGPU_INFO_VIDEO_CAPS_DECODE           0
> >>         /* Subquery id: Encode */
> >>         #define AMDGPU_INFO_VIDEO_CAPS_ENCODE           1
> >> +#define AMDGPU_INFO_CP_GFX_SHADOW_SIZE         0x22
> >>
> >>  #define AMDGPU_INFO_MMR_SE_INDEX_SHIFT 0
> >>  #define AMDGPU_INFO_MMR_SE_INDEX_MASK  0xff
> >> @@ -1203,6 +1204,15 @@ struct drm_amdgpu_info_video_caps {
> >>         struct drm_amdgpu_info_video_codec_info
> codec_info[AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_COUNT];
> >>  };
> >>
> >> +struct drm_amdgpu_info_cp_gfx_shadow_size {
> >> +       __u32 shadow_size;
> >> +       __u32 shadow_alignment;
> >> +       __u32 csa_size;
> >> +       __u32 csa_alignment;
> >> +       __u32 gds_size;
> >> +       __u32 gds_alignment;
> >
> >
> > Can you document the fields? What is CSA? Also, why is GDS there when
> the hw deprecated it and replaced it with GDS registers?
>
> Will add documentation.  For reference:
> CSA (Context Save Area) - used as a scratch area for FW for saving
> various things
> Shadow - stores the pipeline state
> GDS backup - stores the GDS state used by the pipeline.  I'm not sure
> if this is registers or the old GDS memory.  Presumably the former.
>

1. The POR for gfx11 was not to use GDS memory. I don't know why it's
there, but it would be unused uapi.

2. Is it secure to give userspace write access to the CSA and shadow
buffers? In the case of CSA, it looks like userspace could break the
firmware.

Marek

[-- Attachment #2: Type: text/html, Size: 3377 bytes --]

Re: [PATCH 07/11] drm/amdgpu: add UAPI to query GFX shadow sizes

[Alex Deucher]

On Wed, Mar 22, 2023 at 10:12 AM Marek Olšák <maraeo@gmail.com> wrote:
>
> On Tue, Mar 21, 2023 at 3:51 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>>
>> On Mon, Mar 20, 2023 at 8:30 PM Marek Olšák <maraeo@gmail.com> wrote:
>> >
>> >
>> > On Mon, Mar 20, 2023 at 1:38 PM Alex Deucher <alexander.deucher@amd.com> wrote:
>> >>
>> >> Add UAPI to query the GFX shadow buffer requirements
>> >> for preemption on GFX11.  UMDs need to specify the shadow
>> >> areas for preemption.
>> >>
>> >> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> >> ---
>> >>  include/uapi/drm/amdgpu_drm.h | 10 ++++++++++
>> >>  1 file changed, 10 insertions(+)
>> >>
>> >> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
>> >> index 3d9474af6566..19a806145371 100644
>> >> --- a/include/uapi/drm/amdgpu_drm.h
>> >> +++ b/include/uapi/drm/amdgpu_drm.h
>> >> @@ -886,6 +886,7 @@ struct drm_amdgpu_cs_chunk_cp_gfx_shadow {
>> >>         #define AMDGPU_INFO_VIDEO_CAPS_DECODE           0
>> >>         /* Subquery id: Encode */
>> >>         #define AMDGPU_INFO_VIDEO_CAPS_ENCODE           1
>> >> +#define AMDGPU_INFO_CP_GFX_SHADOW_SIZE         0x22
>> >>
>> >>  #define AMDGPU_INFO_MMR_SE_INDEX_SHIFT 0
>> >>  #define AMDGPU_INFO_MMR_SE_INDEX_MASK  0xff
>> >> @@ -1203,6 +1204,15 @@ struct drm_amdgpu_info_video_caps {
>> >>         struct drm_amdgpu_info_video_codec_info codec_info[AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_COUNT];
>> >>  };
>> >>
>> >> +struct drm_amdgpu_info_cp_gfx_shadow_size {
>> >> +       __u32 shadow_size;
>> >> +       __u32 shadow_alignment;
>> >> +       __u32 csa_size;
>> >> +       __u32 csa_alignment;
>> >> +       __u32 gds_size;
>> >> +       __u32 gds_alignment;
>> >
>> >
>> > Can you document the fields? What is CSA? Also, why is GDS there when the hw deprecated it and replaced it with GDS registers?
>>
>> Will add documentation.  For reference:
>> CSA (Context Save Area) - used as a scratch area for FW for saving
>> various things
>> Shadow - stores the pipeline state
>> GDS backup - stores the GDS state used by the pipeline.  I'm not sure
>> if this is registers or the old GDS memory.  Presumably the former.
>
>
> 1. The POR for gfx11 was not to use GDS memory. I don't know why it's there, but it would be unused uapi.

It still needs to be allocated because the FW requires it.

>
> 2. Is it secure to give userspace write access to the CSA and shadow buffers? In the case of CSA, it looks like userspace could break the firmware.

Yes, it should be fine. It's the same way it has always been, it's
just that on older chips the kernel mapped them into all of the GPUVMs
because it was global. If the userspace screws it up, they are only
hurting themselves because the FW uses it to save the UMD's hardware
state.

Alex

>
> Marek