All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: Modify the argument of emit_ib interface
@ 2018-10-23 12:01 Rex Zhu
       [not found] ` <1540296077-9971-1-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Rex Zhu @ 2018-10-23 12:01 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Rex Zhu

use the point of struct amdgpu_job as the function
argument instand of vmid, so the other members of
struct amdgpu_job can be visit in emit_ib function.

Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c   |  3 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h  |  2 +-
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c    |  3 ++-
 drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c    |  3 ++-
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c    |  6 ++++--
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c    |  6 ++++--
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c    | 24 +++++++++++++-----------
 drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c   |  4 +++-
 drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c   |  4 +++-
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c   |  4 +++-
 drivers/gpu/drm/amd/amdgpu/si_dma.c      |  3 ++-
 drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c    |  2 +-
 drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c    |  2 +-
 drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c    |  8 ++++++--
 drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c    |  7 +++++--
 drivers/gpu/drm/amd/amdgpu/vce_v3_0.c    |  4 +++-
 drivers/gpu/drm/amd/amdgpu/vce_v4_0.c    |  4 +++-
 drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c    | 10 +++++++---
 20 files changed, 66 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index b8963b7..0b227ca 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -221,8 +221,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 			!amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble CE ib must be inserted anyway */
 			continue;
 
-		amdgpu_ring_emit_ib(ring, ib, job ? job->vmid : 0,
-				    need_ctx_switch);
+		amdgpu_ring_emit_ib(ring, ib, job, need_ctx_switch);
 		need_ctx_switch = false;
 	}
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index 3cb7fb8..0f0f8fb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -229,7 +229,7 @@ struct amdgpu_ring {
 #define amdgpu_ring_get_rptr(r) (r)->funcs->get_rptr((r))
 #define amdgpu_ring_get_wptr(r) (r)->funcs->get_wptr((r))
 #define amdgpu_ring_set_wptr(r) (r)->funcs->set_wptr((r))
-#define amdgpu_ring_emit_ib(r, ib, vmid, c) (r)->funcs->emit_ib((r), (ib), (vmid), (c))
+#define amdgpu_ring_emit_ib(r, ib, job, c) ((r)->funcs->emit_ib((r), (ib), (job), (c)))
 #define amdgpu_ring_emit_pipeline_sync(r) (r)->funcs->emit_pipeline_sync((r))
 #define amdgpu_ring_emit_vm_flush(r, vmid, addr) (r)->funcs->emit_vm_flush((r), (vmid), (addr))
 #define amdgpu_ring_emit_fence(r, addr, seq, flags) (r)->funcs->emit_fence((r), (addr), (seq), (flags))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
index 84dd550..8f98641 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
@@ -1032,7 +1032,7 @@ int amdgpu_vce_ring_parse_cs_vm(struct amdgpu_cs_parser *p, uint32_t ib_idx)
  *
  */
 void amdgpu_vce_ring_emit_ib(struct amdgpu_ring *ring, struct amdgpu_ib *ib,
-			     unsigned vmid, bool ctx_switch)
+					struct amdgpu_job *job, bool ctx_switch)
 {
 	amdgpu_ring_write(ring, VCE_CMD_IB);
 	amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
index a1f209e..06d6d87 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
@@ -66,7 +66,7 @@ int amdgpu_vce_get_destroy_msg(struct amdgpu_ring *ring, uint32_t handle,
 int amdgpu_vce_ring_parse_cs(struct amdgpu_cs_parser *p, uint32_t ib_idx);
 int amdgpu_vce_ring_parse_cs_vm(struct amdgpu_cs_parser *p, uint32_t ib_idx);
 void amdgpu_vce_ring_emit_ib(struct amdgpu_ring *ring, struct amdgpu_ib *ib,
-			     unsigned vmid, bool ctx_switch);
+				struct amdgpu_job *job, bool ctx_switch);
 void amdgpu_vce_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64 seq,
 				unsigned flags);
 int amdgpu_vce_ring_test_ring(struct amdgpu_ring *ring);
diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
index 32eb43d..70d4419 100644
--- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
@@ -219,8 +219,9 @@ static void cik_sdma_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
  */
 static void cik_sdma_ring_emit_ib(struct amdgpu_ring *ring,
 				  struct amdgpu_ib *ib,
-				  unsigned vmid, bool ctx_switch)
+				  struct amdgpu_job *job, bool ctx_switch)
 {
+	unsigned vmid = job ? job->vmid : 0;
 	u32 extra_bits = vmid & 0xf;
 
 	/* IB packet must end on a 8 DW boundary */
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
index 622dd70..266482f6 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
@@ -1846,8 +1846,9 @@ static void gfx_v6_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
 
 static void gfx_v6_0_ring_emit_ib(struct amdgpu_ring *ring,
 				  struct amdgpu_ib *ib,
-				  unsigned vmid, bool ctx_switch)
+				  struct amdgpu_job *job, bool ctx_switch)
 {
+	unsigned vmid = job ? job->vmid : 0;
 	u32 header, control = 0;
 
 	/* insert SWITCH_BUFFER packet before first IB in the ring frame */
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index cfa45d9..dbcd9cc 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -2234,8 +2234,9 @@ static void gfx_v7_0_ring_emit_fence_compute(struct amdgpu_ring *ring,
  */
 static void gfx_v7_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
 				      struct amdgpu_ib *ib,
-				      unsigned vmid, bool ctx_switch)
+				      struct amdgpu_job *job, bool ctx_switch)
 {
+	unsigned vmid = job ? job->vmid : 0;
 	u32 header, control = 0;
 
 	/* insert SWITCH_BUFFER packet before first IB in the ring frame */
@@ -2263,8 +2264,9 @@ static void gfx_v7_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
 
 static void gfx_v7_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
 					  struct amdgpu_ib *ib,
-					  unsigned vmid, bool ctx_switch)
+				  struct amdgpu_job *job, bool ctx_switch)
 {
+	unsigned vmid = job ? job->vmid : 0;
 	u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
 
 	amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index f6cc0d3..355679a 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -6130,8 +6130,9 @@ static void gfx_v8_0_ring_emit_vgt_flush(struct amdgpu_ring *ring)
 
 static void gfx_v8_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
 				      struct amdgpu_ib *ib,
-				      unsigned vmid, bool ctx_switch)
+				  struct amdgpu_job *job, bool ctx_switch)
 {
+	unsigned vmid = job ? job->vmid : 0;
 	u32 header, control = 0;
 
 	if (ib->flags & AMDGPU_IB_FLAG_CE)
@@ -6160,8 +6161,9 @@ static void gfx_v8_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
 
 static void gfx_v8_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
 					  struct amdgpu_ib *ib,
-					  unsigned vmid, bool ctx_switch)
+				  struct amdgpu_job *job, bool ctx_switch)
 {
+	unsigned vmid = job ? job->vmid : 0;
 	u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
 
 	amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index f1bf0b7..34bf45c 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -4072,9 +4072,10 @@ static void gfx_v9_0_ring_emit_hdp_flush(struct amdgpu_ring *ring)
 }
 
 static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
-                                      struct amdgpu_ib *ib,
-                                      unsigned vmid, bool ctx_switch)
+					struct amdgpu_ib *ib,
+					struct amdgpu_job *job, bool ctx_switch)
 {
+	unsigned vmid = job ? job->vmid : 0;
 	u32 header, control = 0;
 
 	if (ib->flags & AMDGPU_IB_FLAG_CE)
@@ -4103,20 +4104,21 @@ static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
 }
 
 static void gfx_v9_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
-                                          struct amdgpu_ib *ib,
-                                          unsigned vmid, bool ctx_switch)
+					struct amdgpu_ib *ib,
+					struct amdgpu_job *job, bool ctx_switch)
 {
-        u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
+	unsigned vmid = job ? job->vmid : 0;
+	u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
 
-        amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
+	amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
 	BUG_ON(ib->gpu_addr & 0x3); /* Dword align */
-        amdgpu_ring_write(ring,
+	amdgpu_ring_write(ring,
 #ifdef __BIG_ENDIAN
-                                (2 << 0) |
+				(2 << 0) |
 #endif
-                                lower_32_bits(ib->gpu_addr));
-        amdgpu_ring_write(ring, upper_32_bits(ib->gpu_addr));
-        amdgpu_ring_write(ring, control);
+				lower_32_bits(ib->gpu_addr));
+	amdgpu_ring_write(ring, upper_32_bits(ib->gpu_addr));
+	amdgpu_ring_write(ring, control);
 }
 
 static void gfx_v9_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
index bedbd5f..d29f126 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
@@ -246,8 +246,10 @@ static void sdma_v2_4_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
  */
 static void sdma_v2_4_ring_emit_ib(struct amdgpu_ring *ring,
 				   struct amdgpu_ib *ib,
-				   unsigned vmid, bool ctx_switch)
+				  struct amdgpu_job *job, bool ctx_switch)
 {
+	unsigned vmid = job ? job->vmid : 0;
+
 	/* IB packet must end on a 8 DW boundary */
 	sdma_v2_4_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7)) % 8);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
index 6d039b6..061ef5f 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
@@ -420,8 +420,10 @@ static void sdma_v3_0_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
  */
 static void sdma_v3_0_ring_emit_ib(struct amdgpu_ring *ring,
 				   struct amdgpu_ib *ib,
-				   unsigned vmid, bool ctx_switch)
+				  struct amdgpu_job *job, bool ctx_switch)
 {
+	unsigned vmid = job ? job->vmid : 0;
+
 	/* IB packet must end on a 8 DW boundary */
 	sdma_v3_0_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7)) % 8);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index 2b97224..452fef3 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -503,8 +503,10 @@ static void sdma_v4_0_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
  */
 static void sdma_v4_0_ring_emit_ib(struct amdgpu_ring *ring,
 					struct amdgpu_ib *ib,
-					unsigned vmid, bool ctx_switch)
+				  struct amdgpu_job *job, bool ctx_switch)
 {
+	unsigned vmid = job ? job->vmid : 0;
+
 	/* IB packet must end on a 8 DW boundary */
 	sdma_v4_0_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7)) % 8);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/si_dma.c b/drivers/gpu/drm/amd/amdgpu/si_dma.c
index d9b27d7..1bf54b2 100644
--- a/drivers/gpu/drm/amd/amdgpu/si_dma.c
+++ b/drivers/gpu/drm/amd/amdgpu/si_dma.c
@@ -62,8 +62,9 @@ static void si_dma_ring_set_wptr(struct amdgpu_ring *ring)
 
 static void si_dma_ring_emit_ib(struct amdgpu_ring *ring,
 				struct amdgpu_ib *ib,
-				unsigned vmid, bool ctx_switch)
+				  struct amdgpu_job *job, bool ctx_switch)
 {
+	unsigned vmid = job ? job->vmid : 0;
 	/* The indirect buffer packet must end on an 8 DW boundary in the DMA ring.
 	 * Pad as necessary with NOPs.
 	 */
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
index 1fc17bf..4706c91 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
@@ -520,7 +520,7 @@ static int uvd_v4_2_ring_test_ring(struct amdgpu_ring *ring)
  */
 static void uvd_v4_2_ring_emit_ib(struct amdgpu_ring *ring,
 				  struct amdgpu_ib *ib,
-				  unsigned vmid, bool ctx_switch)
+				  struct amdgpu_job *job, bool ctx_switch)
 {
 	amdgpu_ring_write(ring, PACKET0(mmUVD_RBC_IB_BASE, 0));
 	amdgpu_ring_write(ring, ib->gpu_addr);
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
index fde6ad5..b69c2ac 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
@@ -536,7 +536,7 @@ static int uvd_v5_0_ring_test_ring(struct amdgpu_ring *ring)
  */
 static void uvd_v5_0_ring_emit_ib(struct amdgpu_ring *ring,
 				  struct amdgpu_ib *ib,
-				  unsigned vmid, bool ctx_switch)
+				  struct amdgpu_job *job, bool ctx_switch)
 {
 	amdgpu_ring_write(ring, PACKET0(mmUVD_LMI_RBC_IB_64BIT_BAR_LOW, 0));
 	amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
index 7a5b402..36f6b96 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
@@ -1005,8 +1005,10 @@ static int uvd_v6_0_ring_test_ring(struct amdgpu_ring *ring)
  */
 static void uvd_v6_0_ring_emit_ib(struct amdgpu_ring *ring,
 				  struct amdgpu_ib *ib,
-				  unsigned vmid, bool ctx_switch)
+				  struct amdgpu_job *job, bool ctx_switch)
 {
+	unsigned vmid = job ? job->vmid : 0;
+
 	amdgpu_ring_write(ring, PACKET0(mmUVD_LMI_RBC_IB_VMID, 0));
 	amdgpu_ring_write(ring, vmid);
 
@@ -1027,8 +1029,10 @@ static void uvd_v6_0_ring_emit_ib(struct amdgpu_ring *ring,
  * Write enc ring commands to execute the indirect buffer
  */
 static void uvd_v6_0_enc_ring_emit_ib(struct amdgpu_ring *ring,
-		struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
+		struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
 {
+	unsigned vmid = job ? job->vmid : 0;
+
 	amdgpu_ring_write(ring, HEVC_ENC_CMD_IB_VM);
 	amdgpu_ring_write(ring, vmid);
 	amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
index 58b39af..beda492 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
@@ -1301,9 +1301,10 @@ static int uvd_v7_0_ring_patch_cs_in_place(struct amdgpu_cs_parser *p,
  */
 static void uvd_v7_0_ring_emit_ib(struct amdgpu_ring *ring,
 				  struct amdgpu_ib *ib,
-				  unsigned vmid, bool ctx_switch)
+				  struct amdgpu_job *job, bool ctx_switch)
 {
 	struct amdgpu_device *adev = ring->adev;
+	unsigned vmid = job ? job->vmid : 0;
 
 	amdgpu_ring_write(ring,
 		PACKET0(SOC15_REG_OFFSET(UVD, ring->me, mmUVD_LMI_RBC_IB_VMID), 0));
@@ -1329,8 +1330,10 @@ static void uvd_v7_0_ring_emit_ib(struct amdgpu_ring *ring,
  * Write enc ring commands to execute the indirect buffer
  */
 static void uvd_v7_0_enc_ring_emit_ib(struct amdgpu_ring *ring,
-		struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
+		struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
 {
+	unsigned vmid = job ? job->vmid : 0;
+
 	amdgpu_ring_write(ring, HEVC_ENC_CMD_IB_VM);
 	amdgpu_ring_write(ring, vmid);
 	amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
index a1feea0..26661bf 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
@@ -849,8 +849,10 @@ static void vce_v3_0_get_clockgating_state(void *handle, u32 *flags)
 }
 
 static void vce_v3_0_ring_emit_ib(struct amdgpu_ring *ring,
-		struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
+		struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
 {
+	unsigned vmid = job ? job->vmid : 0;
+
 	amdgpu_ring_write(ring, VCE_CMD_IB_VM);
 	amdgpu_ring_write(ring, vmid);
 	amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
index 1c94718..e531cfe 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
@@ -952,8 +952,10 @@ static int vce_v4_0_set_powergating_state(void *handle,
 #endif
 
 static void vce_v4_0_ring_emit_ib(struct amdgpu_ring *ring,
-		struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
+		struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
 {
+	unsigned vmid = job ? job->vmid : 0;
+
 	amdgpu_ring_write(ring, VCE_CMD_IB_VM);
 	amdgpu_ring_write(ring, vmid);
 	amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
index eae9092..f7abf5b 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
@@ -1367,9 +1367,10 @@ static void vcn_v1_0_dec_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64
  */
 static void vcn_v1_0_dec_ring_emit_ib(struct amdgpu_ring *ring,
 				  struct amdgpu_ib *ib,
-				  unsigned vmid, bool ctx_switch)
+				  struct amdgpu_job *job, bool ctx_switch)
 {
 	struct amdgpu_device *adev = ring->adev;
+	unsigned vmid = job ? job->vmid : 0;
 
 	amdgpu_ring_write(ring,
 		PACKET0(SOC15_REG_OFFSET(UVD, 0, mmUVD_LMI_RBC_IB_VMID), 0));
@@ -1524,8 +1525,10 @@ static void vcn_v1_0_enc_ring_insert_end(struct amdgpu_ring *ring)
  * Write enc ring commands to execute the indirect buffer
  */
 static void vcn_v1_0_enc_ring_emit_ib(struct amdgpu_ring *ring,
-		struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
+		struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
 {
+	unsigned vmid = job ? job->vmid : 0;
+
 	amdgpu_ring_write(ring, VCN_ENC_CMD_IB);
 	amdgpu_ring_write(ring, vmid);
 	amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
@@ -1726,9 +1729,10 @@ static void vcn_v1_0_jpeg_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u6
  */
 static void vcn_v1_0_jpeg_ring_emit_ib(struct amdgpu_ring *ring,
 				  struct amdgpu_ib *ib,
-				  unsigned vmid, bool ctx_switch)
+				  struct amdgpu_job *job, bool ctx_switch)
 {
 	struct amdgpu_device *adev = ring->adev;
+	unsigned vmid = job ? job->vmid : 0;
 
 	amdgpu_ring_write(ring,
 		PACKETJ(SOC15_REG_OFFSET(UVD, 0, mmUVD_LMI_JRBC_IB_VMID), 0, 0, PACKETJ_TYPE0));
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/amdgpu: Modify the argument of emit_ib interface
       [not found] ` <1540296077-9971-1-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
@ 2018-10-23 12:09   ` Christian König
       [not found]     ` <331a74c1-2dac-52e4-363f-eaed2b1c9aa3-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Christian König @ 2018-10-23 12:09 UTC (permalink / raw)
  To: Rex Zhu, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Am 23.10.18 um 14:01 schrieb Rex Zhu:
> use the point of struct amdgpu_job as the function
> argument instand of vmid, so the other members of
> struct amdgpu_job can be visit in emit_ib function.
>
> Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c   |  3 +--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c  |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h  |  2 +-
>   drivers/gpu/drm/amd/amdgpu/cik_sdma.c    |  3 ++-
>   drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c    |  3 ++-
>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c    |  6 ++++--
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c    |  6 ++++--
>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c    | 24 +++++++++++++-----------
>   drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c   |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c   |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c   |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/si_dma.c      |  3 ++-
>   drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c    |  2 +-
>   drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c    |  2 +-
>   drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c    |  8 ++++++--
>   drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c    |  7 +++++--
>   drivers/gpu/drm/amd/amdgpu/vce_v3_0.c    |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/vce_v4_0.c    |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c    | 10 +++++++---
>   20 files changed, 66 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index b8963b7..0b227ca 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -221,8 +221,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>   			!amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble CE ib must be inserted anyway */
>   			continue;
>   
> -		amdgpu_ring_emit_ib(ring, ib, job ? job->vmid : 0,
> -				    need_ctx_switch);
> +		amdgpu_ring_emit_ib(ring, ib, job, need_ctx_switch);
>   		need_ctx_switch = false;
>   	}
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index 3cb7fb8..0f0f8fb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -229,7 +229,7 @@ struct amdgpu_ring {
>   #define amdgpu_ring_get_rptr(r) (r)->funcs->get_rptr((r))
>   #define amdgpu_ring_get_wptr(r) (r)->funcs->get_wptr((r))
>   #define amdgpu_ring_set_wptr(r) (r)->funcs->set_wptr((r))
> -#define amdgpu_ring_emit_ib(r, ib, vmid, c) (r)->funcs->emit_ib((r), (ib), (vmid), (c))
> +#define amdgpu_ring_emit_ib(r, ib, job, c) ((r)->funcs->emit_ib((r), (ib), (job), (c)))
>   #define amdgpu_ring_emit_pipeline_sync(r) (r)->funcs->emit_pipeline_sync((r))
>   #define amdgpu_ring_emit_vm_flush(r, vmid, addr) (r)->funcs->emit_vm_flush((r), (vmid), (addr))
>   #define amdgpu_ring_emit_fence(r, addr, seq, flags) (r)->funcs->emit_fence((r), (addr), (seq), (flags))
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> index 84dd550..8f98641 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> @@ -1032,7 +1032,7 @@ int amdgpu_vce_ring_parse_cs_vm(struct amdgpu_cs_parser *p, uint32_t ib_idx)
>    *
>    */
>   void amdgpu_vce_ring_emit_ib(struct amdgpu_ring *ring, struct amdgpu_ib *ib,
> -			     unsigned vmid, bool ctx_switch)
> +					struct amdgpu_job *job, bool ctx_switch)

The indentation here looks wrong on first glance.

Additional to that I would put the job before the IB in the parameter 
list because it is the containing object.

>   {
>   	amdgpu_ring_write(ring, VCE_CMD_IB);
>   	amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> index a1f209e..06d6d87 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> @@ -66,7 +66,7 @@ int amdgpu_vce_get_destroy_msg(struct amdgpu_ring *ring, uint32_t handle,
>   int amdgpu_vce_ring_parse_cs(struct amdgpu_cs_parser *p, uint32_t ib_idx);
>   int amdgpu_vce_ring_parse_cs_vm(struct amdgpu_cs_parser *p, uint32_t ib_idx);
>   void amdgpu_vce_ring_emit_ib(struct amdgpu_ring *ring, struct amdgpu_ib *ib,
> -			     unsigned vmid, bool ctx_switch);
> +				struct amdgpu_job *job, bool ctx_switch);
>   void amdgpu_vce_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64 seq,
>   				unsigned flags);
>   int amdgpu_vce_ring_test_ring(struct amdgpu_ring *ring);
> diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> index 32eb43d..70d4419 100644
> --- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> @@ -219,8 +219,9 @@ static void cik_sdma_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void cik_sdma_ring_emit_ib(struct amdgpu_ring *ring,
>   				  struct amdgpu_ib *ib,
> -				  unsigned vmid, bool ctx_switch)
> +				  struct amdgpu_job *job, bool ctx_switch)
>   {
> +	unsigned vmid = job ? job->vmid : 0;

I'm not 100% sure if duplicating this logic over and over again is such 
a good idea.

Maybe indeed provide the CSA address as parameter instead. It doesn't 
really matter that it is unused by most IP block implementations.

Alternative is to use a wrapper function to get the VMID from the job.

Regards,
Christian.

>   	u32 extra_bits = vmid & 0xf;
>   
>   	/* IB packet must end on a 8 DW boundary */
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> index 622dd70..266482f6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> @@ -1846,8 +1846,9 @@ static void gfx_v6_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
>   
>   static void gfx_v6_0_ring_emit_ib(struct amdgpu_ring *ring,
>   				  struct amdgpu_ib *ib,
> -				  unsigned vmid, bool ctx_switch)
> +				  struct amdgpu_job *job, bool ctx_switch)
>   {
> +	unsigned vmid = job ? job->vmid : 0;
>   	u32 header, control = 0;
>   
>   	/* insert SWITCH_BUFFER packet before first IB in the ring frame */
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> index cfa45d9..dbcd9cc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> @@ -2234,8 +2234,9 @@ static void gfx_v7_0_ring_emit_fence_compute(struct amdgpu_ring *ring,
>    */
>   static void gfx_v7_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>   				      struct amdgpu_ib *ib,
> -				      unsigned vmid, bool ctx_switch)
> +				      struct amdgpu_job *job, bool ctx_switch)
>   {
> +	unsigned vmid = job ? job->vmid : 0;
>   	u32 header, control = 0;
>   
>   	/* insert SWITCH_BUFFER packet before first IB in the ring frame */
> @@ -2263,8 +2264,9 @@ static void gfx_v7_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>   
>   static void gfx_v7_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
>   					  struct amdgpu_ib *ib,
> -					  unsigned vmid, bool ctx_switch)
> +				  struct amdgpu_job *job, bool ctx_switch)
>   {
> +	unsigned vmid = job ? job->vmid : 0;
>   	u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
>   
>   	amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index f6cc0d3..355679a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -6130,8 +6130,9 @@ static void gfx_v8_0_ring_emit_vgt_flush(struct amdgpu_ring *ring)
>   
>   static void gfx_v8_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>   				      struct amdgpu_ib *ib,
> -				      unsigned vmid, bool ctx_switch)
> +				  struct amdgpu_job *job, bool ctx_switch)
>   {
> +	unsigned vmid = job ? job->vmid : 0;
>   	u32 header, control = 0;
>   
>   	if (ib->flags & AMDGPU_IB_FLAG_CE)
> @@ -6160,8 +6161,9 @@ static void gfx_v8_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>   
>   static void gfx_v8_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
>   					  struct amdgpu_ib *ib,
> -					  unsigned vmid, bool ctx_switch)
> +				  struct amdgpu_job *job, bool ctx_switch)
>   {
> +	unsigned vmid = job ? job->vmid : 0;
>   	u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
>   
>   	amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index f1bf0b7..34bf45c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -4072,9 +4072,10 @@ static void gfx_v9_0_ring_emit_hdp_flush(struct amdgpu_ring *ring)
>   }
>   
>   static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
> -                                      struct amdgpu_ib *ib,
> -                                      unsigned vmid, bool ctx_switch)
> +					struct amdgpu_ib *ib,
> +					struct amdgpu_job *job, bool ctx_switch)
>   {
> +	unsigned vmid = job ? job->vmid : 0;
>   	u32 header, control = 0;
>   
>   	if (ib->flags & AMDGPU_IB_FLAG_CE)
> @@ -4103,20 +4104,21 @@ static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>   }
>   
>   static void gfx_v9_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
> -                                          struct amdgpu_ib *ib,
> -                                          unsigned vmid, bool ctx_switch)
> +					struct amdgpu_ib *ib,
> +					struct amdgpu_job *job, bool ctx_switch)
>   {
> -        u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
> +	unsigned vmid = job ? job->vmid : 0;
> +	u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
>   
> -        amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
> +	amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
>   	BUG_ON(ib->gpu_addr & 0x3); /* Dword align */
> -        amdgpu_ring_write(ring,
> +	amdgpu_ring_write(ring,
>   #ifdef __BIG_ENDIAN
> -                                (2 << 0) |
> +				(2 << 0) |
>   #endif
> -                                lower_32_bits(ib->gpu_addr));
> -        amdgpu_ring_write(ring, upper_32_bits(ib->gpu_addr));
> -        amdgpu_ring_write(ring, control);
> +				lower_32_bits(ib->gpu_addr));
> +	amdgpu_ring_write(ring, upper_32_bits(ib->gpu_addr));
> +	amdgpu_ring_write(ring, control);
>   }
>   
>   static void gfx_v9_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> index bedbd5f..d29f126 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> @@ -246,8 +246,10 @@ static void sdma_v2_4_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void sdma_v2_4_ring_emit_ib(struct amdgpu_ring *ring,
>   				   struct amdgpu_ib *ib,
> -				   unsigned vmid, bool ctx_switch)
> +				  struct amdgpu_job *job, bool ctx_switch)
>   {
> +	unsigned vmid = job ? job->vmid : 0;
> +
>   	/* IB packet must end on a 8 DW boundary */
>   	sdma_v2_4_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7)) % 8);
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> index 6d039b6..061ef5f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> @@ -420,8 +420,10 @@ static void sdma_v3_0_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void sdma_v3_0_ring_emit_ib(struct amdgpu_ring *ring,
>   				   struct amdgpu_ib *ib,
> -				   unsigned vmid, bool ctx_switch)
> +				  struct amdgpu_job *job, bool ctx_switch)
>   {
> +	unsigned vmid = job ? job->vmid : 0;
> +
>   	/* IB packet must end on a 8 DW boundary */
>   	sdma_v3_0_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7)) % 8);
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index 2b97224..452fef3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -503,8 +503,10 @@ static void sdma_v4_0_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void sdma_v4_0_ring_emit_ib(struct amdgpu_ring *ring,
>   					struct amdgpu_ib *ib,
> -					unsigned vmid, bool ctx_switch)
> +				  struct amdgpu_job *job, bool ctx_switch)
>   {
> +	unsigned vmid = job ? job->vmid : 0;
> +
>   	/* IB packet must end on a 8 DW boundary */
>   	sdma_v4_0_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7)) % 8);
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/si_dma.c b/drivers/gpu/drm/amd/amdgpu/si_dma.c
> index d9b27d7..1bf54b2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/si_dma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/si_dma.c
> @@ -62,8 +62,9 @@ static void si_dma_ring_set_wptr(struct amdgpu_ring *ring)
>   
>   static void si_dma_ring_emit_ib(struct amdgpu_ring *ring,
>   				struct amdgpu_ib *ib,
> -				unsigned vmid, bool ctx_switch)
> +				  struct amdgpu_job *job, bool ctx_switch)
>   {
> +	unsigned vmid = job ? job->vmid : 0;
>   	/* The indirect buffer packet must end on an 8 DW boundary in the DMA ring.
>   	 * Pad as necessary with NOPs.
>   	 */
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> index 1fc17bf..4706c91 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> @@ -520,7 +520,7 @@ static int uvd_v4_2_ring_test_ring(struct amdgpu_ring *ring)
>    */
>   static void uvd_v4_2_ring_emit_ib(struct amdgpu_ring *ring,
>   				  struct amdgpu_ib *ib,
> -				  unsigned vmid, bool ctx_switch)
> +				  struct amdgpu_job *job, bool ctx_switch)
>   {
>   	amdgpu_ring_write(ring, PACKET0(mmUVD_RBC_IB_BASE, 0));
>   	amdgpu_ring_write(ring, ib->gpu_addr);
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> index fde6ad5..b69c2ac 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> @@ -536,7 +536,7 @@ static int uvd_v5_0_ring_test_ring(struct amdgpu_ring *ring)
>    */
>   static void uvd_v5_0_ring_emit_ib(struct amdgpu_ring *ring,
>   				  struct amdgpu_ib *ib,
> -				  unsigned vmid, bool ctx_switch)
> +				  struct amdgpu_job *job, bool ctx_switch)
>   {
>   	amdgpu_ring_write(ring, PACKET0(mmUVD_LMI_RBC_IB_64BIT_BAR_LOW, 0));
>   	amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> index 7a5b402..36f6b96 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> @@ -1005,8 +1005,10 @@ static int uvd_v6_0_ring_test_ring(struct amdgpu_ring *ring)
>    */
>   static void uvd_v6_0_ring_emit_ib(struct amdgpu_ring *ring,
>   				  struct amdgpu_ib *ib,
> -				  unsigned vmid, bool ctx_switch)
> +				  struct amdgpu_job *job, bool ctx_switch)
>   {
> +	unsigned vmid = job ? job->vmid : 0;
> +
>   	amdgpu_ring_write(ring, PACKET0(mmUVD_LMI_RBC_IB_VMID, 0));
>   	amdgpu_ring_write(ring, vmid);
>   
> @@ -1027,8 +1029,10 @@ static void uvd_v6_0_ring_emit_ib(struct amdgpu_ring *ring,
>    * Write enc ring commands to execute the indirect buffer
>    */
>   static void uvd_v6_0_enc_ring_emit_ib(struct amdgpu_ring *ring,
> -		struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +		struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +	unsigned vmid = job ? job->vmid : 0;
> +
>   	amdgpu_ring_write(ring, HEVC_ENC_CMD_IB_VM);
>   	amdgpu_ring_write(ring, vmid);
>   	amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> index 58b39af..beda492 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> @@ -1301,9 +1301,10 @@ static int uvd_v7_0_ring_patch_cs_in_place(struct amdgpu_cs_parser *p,
>    */
>   static void uvd_v7_0_ring_emit_ib(struct amdgpu_ring *ring,
>   				  struct amdgpu_ib *ib,
> -				  unsigned vmid, bool ctx_switch)
> +				  struct amdgpu_job *job, bool ctx_switch)
>   {
>   	struct amdgpu_device *adev = ring->adev;
> +	unsigned vmid = job ? job->vmid : 0;
>   
>   	amdgpu_ring_write(ring,
>   		PACKET0(SOC15_REG_OFFSET(UVD, ring->me, mmUVD_LMI_RBC_IB_VMID), 0));
> @@ -1329,8 +1330,10 @@ static void uvd_v7_0_ring_emit_ib(struct amdgpu_ring *ring,
>    * Write enc ring commands to execute the indirect buffer
>    */
>   static void uvd_v7_0_enc_ring_emit_ib(struct amdgpu_ring *ring,
> -		struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +		struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +	unsigned vmid = job ? job->vmid : 0;
> +
>   	amdgpu_ring_write(ring, HEVC_ENC_CMD_IB_VM);
>   	amdgpu_ring_write(ring, vmid);
>   	amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> index a1feea0..26661bf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> @@ -849,8 +849,10 @@ static void vce_v3_0_get_clockgating_state(void *handle, u32 *flags)
>   }
>   
>   static void vce_v3_0_ring_emit_ib(struct amdgpu_ring *ring,
> -		struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +		struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +	unsigned vmid = job ? job->vmid : 0;
> +
>   	amdgpu_ring_write(ring, VCE_CMD_IB_VM);
>   	amdgpu_ring_write(ring, vmid);
>   	amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> index 1c94718..e531cfe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> @@ -952,8 +952,10 @@ static int vce_v4_0_set_powergating_state(void *handle,
>   #endif
>   
>   static void vce_v4_0_ring_emit_ib(struct amdgpu_ring *ring,
> -		struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +		struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +	unsigned vmid = job ? job->vmid : 0;
> +
>   	amdgpu_ring_write(ring, VCE_CMD_IB_VM);
>   	amdgpu_ring_write(ring, vmid);
>   	amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> index eae9092..f7abf5b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> @@ -1367,9 +1367,10 @@ static void vcn_v1_0_dec_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64
>    */
>   static void vcn_v1_0_dec_ring_emit_ib(struct amdgpu_ring *ring,
>   				  struct amdgpu_ib *ib,
> -				  unsigned vmid, bool ctx_switch)
> +				  struct amdgpu_job *job, bool ctx_switch)
>   {
>   	struct amdgpu_device *adev = ring->adev;
> +	unsigned vmid = job ? job->vmid : 0;
>   
>   	amdgpu_ring_write(ring,
>   		PACKET0(SOC15_REG_OFFSET(UVD, 0, mmUVD_LMI_RBC_IB_VMID), 0));
> @@ -1524,8 +1525,10 @@ static void vcn_v1_0_enc_ring_insert_end(struct amdgpu_ring *ring)
>    * Write enc ring commands to execute the indirect buffer
>    */
>   static void vcn_v1_0_enc_ring_emit_ib(struct amdgpu_ring *ring,
> -		struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +		struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +	unsigned vmid = job ? job->vmid : 0;
> +
>   	amdgpu_ring_write(ring, VCN_ENC_CMD_IB);
>   	amdgpu_ring_write(ring, vmid);
>   	amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> @@ -1726,9 +1729,10 @@ static void vcn_v1_0_jpeg_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u6
>    */
>   static void vcn_v1_0_jpeg_ring_emit_ib(struct amdgpu_ring *ring,
>   				  struct amdgpu_ib *ib,
> -				  unsigned vmid, bool ctx_switch)
> +				  struct amdgpu_job *job, bool ctx_switch)
>   {
>   	struct amdgpu_device *adev = ring->adev;
> +	unsigned vmid = job ? job->vmid : 0;
>   
>   	amdgpu_ring_write(ring,
>   		PACKETJ(SOC15_REG_OFFSET(UVD, 0, mmUVD_LMI_JRBC_IB_VMID), 0, 0, PACKETJ_TYPE0));

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/amdgpu: Modify the argument of emit_ib interface
       [not found]     ` <331a74c1-2dac-52e4-363f-eaed2b1c9aa3-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2018-10-23 12:43       ` Zhu, Rex
       [not found]         ` <BYAPR12MB2775CA38B33B516CF35C61B5FBF50-ZGDeBxoHBPmJeBUhB162ZQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Zhu, Rex @ 2018-10-23 12:43 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Koenig, Christian


[-- Attachment #1.1: Type: text/plain, Size: 23470 bytes --]

>Maybe indeed provide the CSA address as parameter instead. It doesn't
>really matter that it is unused by most IP block implementations.

>Alternative is to use a wrapper function to get the VMID from the job.


I preferred to use a wrapper function to get vmid.


Because if use CSA address as parameter, we need to check which ring it is.

The csa address is different between sdma/gfx/compute.


Best Regards

Rex






________________________________
From: Christian König <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Sent: Tuesday, October 23, 2018 8:09 PM
To: Zhu, Rex; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH] drm/amdgpu: Modify the argument of emit_ib interface

Am 23.10.18 um 14:01 schrieb Rex Zhu:
> use the point of struct amdgpu_job as the function
> argument instand of vmid, so the other members of
> struct amdgpu_job can be visit in emit_ib function.
>
> Signed-off-by: Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c   |  3 +--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c  |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h  |  2 +-
>   drivers/gpu/drm/amd/amdgpu/cik_sdma.c    |  3 ++-
>   drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c    |  3 ++-
>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c    |  6 ++++--
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c    |  6 ++++--
>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c    | 24 +++++++++++++-----------
>   drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c   |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c   |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c   |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/si_dma.c      |  3 ++-
>   drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c    |  2 +-
>   drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c    |  2 +-
>   drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c    |  8 ++++++--
>   drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c    |  7 +++++--
>   drivers/gpu/drm/amd/amdgpu/vce_v3_0.c    |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/vce_v4_0.c    |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c    | 10 +++++++---
>   20 files changed, 66 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index b8963b7..0b227ca 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -221,8 +221,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>                        !amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble CE ib must be inserted anyway */
>                        continue;
>
> -             amdgpu_ring_emit_ib(ring, ib, job ? job->vmid : 0,
> -                                 need_ctx_switch);
> +             amdgpu_ring_emit_ib(ring, ib, job, need_ctx_switch);
>                need_ctx_switch = false;
>        }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index 3cb7fb8..0f0f8fb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -229,7 +229,7 @@ struct amdgpu_ring {
>   #define amdgpu_ring_get_rptr(r) (r)->funcs->get_rptr((r))
>   #define amdgpu_ring_get_wptr(r) (r)->funcs->get_wptr((r))
>   #define amdgpu_ring_set_wptr(r) (r)->funcs->set_wptr((r))
> -#define amdgpu_ring_emit_ib(r, ib, vmid, c) (r)->funcs->emit_ib((r), (ib), (vmid), (c))
> +#define amdgpu_ring_emit_ib(r, ib, job, c) ((r)->funcs->emit_ib((r), (ib), (job), (c)))
>   #define amdgpu_ring_emit_pipeline_sync(r) (r)->funcs->emit_pipeline_sync((r))
>   #define amdgpu_ring_emit_vm_flush(r, vmid, addr) (r)->funcs->emit_vm_flush((r), (vmid), (addr))
>   #define amdgpu_ring_emit_fence(r, addr, seq, flags) (r)->funcs->emit_fence((r), (addr), (seq), (flags))
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> index 84dd550..8f98641 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> @@ -1032,7 +1032,7 @@ int amdgpu_vce_ring_parse_cs_vm(struct amdgpu_cs_parser *p, uint32_t ib_idx)
>    *
>    */
>   void amdgpu_vce_ring_emit_ib(struct amdgpu_ring *ring, struct amdgpu_ib *ib,
> -                          unsigned vmid, bool ctx_switch)
> +                                     struct amdgpu_job *job, bool ctx_switch)

The indentation here looks wrong on first glance.

Additional to that I would put the job before the IB in the parameter
list because it is the containing object.

>   {
>        amdgpu_ring_write(ring, VCE_CMD_IB);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> index a1f209e..06d6d87 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> @@ -66,7 +66,7 @@ int amdgpu_vce_get_destroy_msg(struct amdgpu_ring *ring, uint32_t handle,
>   int amdgpu_vce_ring_parse_cs(struct amdgpu_cs_parser *p, uint32_t ib_idx);
>   int amdgpu_vce_ring_parse_cs_vm(struct amdgpu_cs_parser *p, uint32_t ib_idx);
>   void amdgpu_vce_ring_emit_ib(struct amdgpu_ring *ring, struct amdgpu_ib *ib,
> -                          unsigned vmid, bool ctx_switch);
> +                             struct amdgpu_job *job, bool ctx_switch);
>   void amdgpu_vce_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64 seq,
>                                unsigned flags);
>   int amdgpu_vce_ring_test_ring(struct amdgpu_ring *ring);
> diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> index 32eb43d..70d4419 100644
> --- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> @@ -219,8 +219,9 @@ static void cik_sdma_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void cik_sdma_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;

I'm not 100% sure if duplicating this logic over and over again is such
a good idea.

Maybe indeed provide the CSA address as parameter instead. It doesn't
really matter that it is unused by most IP block implementations.

Alternative is to use a wrapper function to get the VMID from the job.

Regards,
Christian.

>        u32 extra_bits = vmid & 0xf;
>
>        /* IB packet must end on a 8 DW boundary */
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> index 622dd70..266482f6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> @@ -1846,8 +1846,9 @@ static void gfx_v6_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
>
>   static void gfx_v6_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 header, control = 0;
>
>        /* insert SWITCH_BUFFER packet before first IB in the ring frame */
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> index cfa45d9..dbcd9cc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> @@ -2234,8 +2234,9 @@ static void gfx_v7_0_ring_emit_fence_compute(struct amdgpu_ring *ring,
>    */
>   static void gfx_v7_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>                                      struct amdgpu_ib *ib,
> -                                   unsigned vmid, bool ctx_switch)
> +                                   struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 header, control = 0;
>
>        /* insert SWITCH_BUFFER packet before first IB in the ring frame */
> @@ -2263,8 +2264,9 @@ static void gfx_v7_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>
>   static void gfx_v7_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
>                                          struct amdgpu_ib *ib,
> -                                       unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
>
>        amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index f6cc0d3..355679a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -6130,8 +6130,9 @@ static void gfx_v8_0_ring_emit_vgt_flush(struct amdgpu_ring *ring)
>
>   static void gfx_v8_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>                                      struct amdgpu_ib *ib,
> -                                   unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 header, control = 0;
>
>        if (ib->flags & AMDGPU_IB_FLAG_CE)
> @@ -6160,8 +6161,9 @@ static void gfx_v8_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>
>   static void gfx_v8_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
>                                          struct amdgpu_ib *ib,
> -                                       unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
>
>        amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index f1bf0b7..34bf45c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -4072,9 +4072,10 @@ static void gfx_v9_0_ring_emit_hdp_flush(struct amdgpu_ring *ring)
>   }
>
>   static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
> -                                      struct amdgpu_ib *ib,
> -                                      unsigned vmid, bool ctx_switch)
> +                                     struct amdgpu_ib *ib,
> +                                     struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 header, control = 0;
>
>        if (ib->flags & AMDGPU_IB_FLAG_CE)
> @@ -4103,20 +4104,21 @@ static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>   }
>
>   static void gfx_v9_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
> -                                          struct amdgpu_ib *ib,
> -                                          unsigned vmid, bool ctx_switch)
> +                                     struct amdgpu_ib *ib,
> +                                     struct amdgpu_job *job, bool ctx_switch)
>   {
> -        u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
> +     unsigned vmid = job ? job->vmid : 0;
> +     u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
>
> -        amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
> +     amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
>        BUG_ON(ib->gpu_addr & 0x3); /* Dword align */
> -        amdgpu_ring_write(ring,
> +     amdgpu_ring_write(ring,
>   #ifdef __BIG_ENDIAN
> -                                (2 << 0) |
> +                             (2 << 0) |
>   #endif
> -                                lower_32_bits(ib->gpu_addr));
> -        amdgpu_ring_write(ring, upper_32_bits(ib->gpu_addr));
> -        amdgpu_ring_write(ring, control);
> +                             lower_32_bits(ib->gpu_addr));
> +     amdgpu_ring_write(ring, upper_32_bits(ib->gpu_addr));
> +     amdgpu_ring_write(ring, control);
>   }
>
>   static void gfx_v9_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> index bedbd5f..d29f126 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> @@ -246,8 +246,10 @@ static void sdma_v2_4_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void sdma_v2_4_ring_emit_ib(struct amdgpu_ring *ring,
>                                   struct amdgpu_ib *ib,
> -                                unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        /* IB packet must end on a 8 DW boundary */
>        sdma_v2_4_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7)) % 8);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> index 6d039b6..061ef5f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> @@ -420,8 +420,10 @@ static void sdma_v3_0_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void sdma_v3_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                   struct amdgpu_ib *ib,
> -                                unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        /* IB packet must end on a 8 DW boundary */
>        sdma_v3_0_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7)) % 8);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index 2b97224..452fef3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -503,8 +503,10 @@ static void sdma_v4_0_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void sdma_v4_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                        struct amdgpu_ib *ib,
> -                                     unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        /* IB packet must end on a 8 DW boundary */
>        sdma_v4_0_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7)) % 8);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/si_dma.c b/drivers/gpu/drm/amd/amdgpu/si_dma.c
> index d9b27d7..1bf54b2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/si_dma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/si_dma.c
> @@ -62,8 +62,9 @@ static void si_dma_ring_set_wptr(struct amdgpu_ring *ring)
>
>   static void si_dma_ring_emit_ib(struct amdgpu_ring *ring,
>                                struct amdgpu_ib *ib,
> -                             unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        /* The indirect buffer packet must end on an 8 DW boundary in the DMA ring.
>         * Pad as necessary with NOPs.
>         */
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> index 1fc17bf..4706c91 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> @@ -520,7 +520,7 @@ static int uvd_v4_2_ring_test_ring(struct amdgpu_ring *ring)
>    */
>   static void uvd_v4_2_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        amdgpu_ring_write(ring, PACKET0(mmUVD_RBC_IB_BASE, 0));
>        amdgpu_ring_write(ring, ib->gpu_addr);
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> index fde6ad5..b69c2ac 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> @@ -536,7 +536,7 @@ static int uvd_v5_0_ring_test_ring(struct amdgpu_ring *ring)
>    */
>   static void uvd_v5_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        amdgpu_ring_write(ring, PACKET0(mmUVD_LMI_RBC_IB_64BIT_BAR_LOW, 0));
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> index 7a5b402..36f6b96 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> @@ -1005,8 +1005,10 @@ static int uvd_v6_0_ring_test_ring(struct amdgpu_ring *ring)
>    */
>   static void uvd_v6_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, PACKET0(mmUVD_LMI_RBC_IB_VMID, 0));
>        amdgpu_ring_write(ring, vmid);
>
> @@ -1027,8 +1029,10 @@ static void uvd_v6_0_ring_emit_ib(struct amdgpu_ring *ring,
>    * Write enc ring commands to execute the indirect buffer
>    */
>   static void uvd_v6_0_enc_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, HEVC_ENC_CMD_IB_VM);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> index 58b39af..beda492 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> @@ -1301,9 +1301,10 @@ static int uvd_v7_0_ring_patch_cs_in_place(struct amdgpu_cs_parser *p,
>    */
>   static void uvd_v7_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        struct amdgpu_device *adev = ring->adev;
> +     unsigned vmid = job ? job->vmid : 0;
>
>        amdgpu_ring_write(ring,
>                PACKET0(SOC15_REG_OFFSET(UVD, ring->me, mmUVD_LMI_RBC_IB_VMID), 0));
> @@ -1329,8 +1330,10 @@ static void uvd_v7_0_ring_emit_ib(struct amdgpu_ring *ring,
>    * Write enc ring commands to execute the indirect buffer
>    */
>   static void uvd_v7_0_enc_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, HEVC_ENC_CMD_IB_VM);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> index a1feea0..26661bf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> @@ -849,8 +849,10 @@ static void vce_v3_0_get_clockgating_state(void *handle, u32 *flags)
>   }
>
>   static void vce_v3_0_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, VCE_CMD_IB_VM);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> index 1c94718..e531cfe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> @@ -952,8 +952,10 @@ static int vce_v4_0_set_powergating_state(void *handle,
>   #endif
>
>   static void vce_v4_0_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, VCE_CMD_IB_VM);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> index eae9092..f7abf5b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> @@ -1367,9 +1367,10 @@ static void vcn_v1_0_dec_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64
>    */
>   static void vcn_v1_0_dec_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        struct amdgpu_device *adev = ring->adev;
> +     unsigned vmid = job ? job->vmid : 0;
>
>        amdgpu_ring_write(ring,
>                PACKET0(SOC15_REG_OFFSET(UVD, 0, mmUVD_LMI_RBC_IB_VMID), 0));
> @@ -1524,8 +1525,10 @@ static void vcn_v1_0_enc_ring_insert_end(struct amdgpu_ring *ring)
>    * Write enc ring commands to execute the indirect buffer
>    */
>   static void vcn_v1_0_enc_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, VCN_ENC_CMD_IB);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> @@ -1726,9 +1729,10 @@ static void vcn_v1_0_jpeg_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u6
>    */
>   static void vcn_v1_0_jpeg_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        struct amdgpu_device *adev = ring->adev;
> +     unsigned vmid = job ? job->vmid : 0;
>
>        amdgpu_ring_write(ring,
>                PACKETJ(SOC15_REG_OFFSET(UVD, 0, mmUVD_LMI_JRBC_IB_VMID), 0, 0, PACKETJ_TYPE0));


[-- Attachment #1.2: Type: text/html, Size: 45691 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/amdgpu: Modify the argument of emit_ib interface
       [not found]         ` <BYAPR12MB2775CA38B33B516CF35C61B5FBF50-ZGDeBxoHBPmJeBUhB162ZQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2018-10-23 12:47           ` Koenig, Christian
       [not found]             ` <61db8416-8167-e150-c06e-48fd613fa6c8-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Koenig, Christian @ 2018-10-23 12:47 UTC (permalink / raw)
  To: Zhu, Rex, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW


[-- Attachment #1.1: Type: text/plain, Size: 23743 bytes --]

Am 23.10.18 um 14:43 schrieb Zhu, Rex:

>Maybe indeed provide the CSA address as parameter instead. It doesn't
>really matter that it is unused by most IP block implementations.

>Alternative is to use a wrapper function to get the VMID from the job.


I preferred to use a wrapper function to get vmid.


Because if use CSA address as parameter, we need to check which ring it is.

The csa address is different between sdma/gfx/compute.

Yeah, but that can be calculated in the ring specific function. Can't it?

I mean if I'm not completely mistaken it is just an offset which needs to be added to the base CSA address.

Christian.



Best Regards

Rex






________________________________
From: Christian König <ckoenig.leichtzumerken@gmail.com><mailto:ckoenig.leichtzumerken@gmail.com>
Sent: Tuesday, October 23, 2018 8:09 PM
To: Zhu, Rex; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/amdgpu: Modify the argument of emit_ib interface

Am 23.10.18 um 14:01 schrieb Rex Zhu:
> use the point of struct amdgpu_job as the function
> argument instand of vmid, so the other members of
> struct amdgpu_job can be visit in emit_ib function.
>
> Signed-off-by: Rex Zhu <Rex.Zhu@amd.com><mailto:Rex.Zhu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c   |  3 +--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c  |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h  |  2 +-
>   drivers/gpu/drm/amd/amdgpu/cik_sdma.c    |  3 ++-
>   drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c    |  3 ++-
>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c    |  6 ++++--
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c    |  6 ++++--
>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c    | 24 +++++++++++++-----------
>   drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c   |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c   |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c   |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/si_dma.c      |  3 ++-
>   drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c    |  2 +-
>   drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c    |  2 +-
>   drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c    |  8 ++++++--
>   drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c    |  7 +++++--
>   drivers/gpu/drm/amd/amdgpu/vce_v3_0.c    |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/vce_v4_0.c    |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c    | 10 +++++++---
>   20 files changed, 66 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index b8963b7..0b227ca 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -221,8 +221,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>                        !amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble CE ib must be inserted anyway */
>                        continue;
>
> -             amdgpu_ring_emit_ib(ring, ib, job ? job->vmid : 0,
> -                                 need_ctx_switch);
> +             amdgpu_ring_emit_ib(ring, ib, job, need_ctx_switch);
>                need_ctx_switch = false;
>        }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index 3cb7fb8..0f0f8fb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -229,7 +229,7 @@ struct amdgpu_ring {
>   #define amdgpu_ring_get_rptr(r) (r)->funcs->get_rptr((r))
>   #define amdgpu_ring_get_wptr(r) (r)->funcs->get_wptr((r))
>   #define amdgpu_ring_set_wptr(r) (r)->funcs->set_wptr((r))
> -#define amdgpu_ring_emit_ib(r, ib, vmid, c) (r)->funcs->emit_ib((r), (ib), (vmid), (c))
> +#define amdgpu_ring_emit_ib(r, ib, job, c) ((r)->funcs->emit_ib((r), (ib), (job), (c)))
>   #define amdgpu_ring_emit_pipeline_sync(r) (r)->funcs->emit_pipeline_sync((r))
>   #define amdgpu_ring_emit_vm_flush(r, vmid, addr) (r)->funcs->emit_vm_flush((r), (vmid), (addr))
>   #define amdgpu_ring_emit_fence(r, addr, seq, flags) (r)->funcs->emit_fence((r), (addr), (seq), (flags))
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> index 84dd550..8f98641 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> @@ -1032,7 +1032,7 @@ int amdgpu_vce_ring_parse_cs_vm(struct amdgpu_cs_parser *p, uint32_t ib_idx)
>    *
>    */
>   void amdgpu_vce_ring_emit_ib(struct amdgpu_ring *ring, struct amdgpu_ib *ib,
> -                          unsigned vmid, bool ctx_switch)
> +                                     struct amdgpu_job *job, bool ctx_switch)

The indentation here looks wrong on first glance.

Additional to that I would put the job before the IB in the parameter
list because it is the containing object.

>   {
>        amdgpu_ring_write(ring, VCE_CMD_IB);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> index a1f209e..06d6d87 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> @@ -66,7 +66,7 @@ int amdgpu_vce_get_destroy_msg(struct amdgpu_ring *ring, uint32_t handle,
>   int amdgpu_vce_ring_parse_cs(struct amdgpu_cs_parser *p, uint32_t ib_idx);
>   int amdgpu_vce_ring_parse_cs_vm(struct amdgpu_cs_parser *p, uint32_t ib_idx);
>   void amdgpu_vce_ring_emit_ib(struct amdgpu_ring *ring, struct amdgpu_ib *ib,
> -                          unsigned vmid, bool ctx_switch);
> +                             struct amdgpu_job *job, bool ctx_switch);
>   void amdgpu_vce_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64 seq,
>                                unsigned flags);
>   int amdgpu_vce_ring_test_ring(struct amdgpu_ring *ring);
> diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> index 32eb43d..70d4419 100644
> --- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> @@ -219,8 +219,9 @@ static void cik_sdma_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void cik_sdma_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;

I'm not 100% sure if duplicating this logic over and over again is such
a good idea.

Maybe indeed provide the CSA address as parameter instead. It doesn't
really matter that it is unused by most IP block implementations.

Alternative is to use a wrapper function to get the VMID from the job.

Regards,
Christian.

>        u32 extra_bits = vmid & 0xf;
>
>        /* IB packet must end on a 8 DW boundary */
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> index 622dd70..266482f6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> @@ -1846,8 +1846,9 @@ static void gfx_v6_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
>
>   static void gfx_v6_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 header, control = 0;
>
>        /* insert SWITCH_BUFFER packet before first IB in the ring frame */
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> index cfa45d9..dbcd9cc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> @@ -2234,8 +2234,9 @@ static void gfx_v7_0_ring_emit_fence_compute(struct amdgpu_ring *ring,
>    */
>   static void gfx_v7_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>                                      struct amdgpu_ib *ib,
> -                                   unsigned vmid, bool ctx_switch)
> +                                   struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 header, control = 0;
>
>        /* insert SWITCH_BUFFER packet before first IB in the ring frame */
> @@ -2263,8 +2264,9 @@ static void gfx_v7_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>
>   static void gfx_v7_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
>                                          struct amdgpu_ib *ib,
> -                                       unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
>
>        amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index f6cc0d3..355679a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -6130,8 +6130,9 @@ static void gfx_v8_0_ring_emit_vgt_flush(struct amdgpu_ring *ring)
>
>   static void gfx_v8_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>                                      struct amdgpu_ib *ib,
> -                                   unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 header, control = 0;
>
>        if (ib->flags & AMDGPU_IB_FLAG_CE)
> @@ -6160,8 +6161,9 @@ static void gfx_v8_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>
>   static void gfx_v8_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
>                                          struct amdgpu_ib *ib,
> -                                       unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
>
>        amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index f1bf0b7..34bf45c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -4072,9 +4072,10 @@ static void gfx_v9_0_ring_emit_hdp_flush(struct amdgpu_ring *ring)
>   }
>
>   static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
> -                                      struct amdgpu_ib *ib,
> -                                      unsigned vmid, bool ctx_switch)
> +                                     struct amdgpu_ib *ib,
> +                                     struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 header, control = 0;
>
>        if (ib->flags & AMDGPU_IB_FLAG_CE)
> @@ -4103,20 +4104,21 @@ static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>   }
>
>   static void gfx_v9_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
> -                                          struct amdgpu_ib *ib,
> -                                          unsigned vmid, bool ctx_switch)
> +                                     struct amdgpu_ib *ib,
> +                                     struct amdgpu_job *job, bool ctx_switch)
>   {
> -        u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
> +     unsigned vmid = job ? job->vmid : 0;
> +     u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
>
> -        amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
> +     amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
>        BUG_ON(ib->gpu_addr & 0x3); /* Dword align */
> -        amdgpu_ring_write(ring,
> +     amdgpu_ring_write(ring,
>   #ifdef __BIG_ENDIAN
> -                                (2 << 0) |
> +                             (2 << 0) |
>   #endif
> -                                lower_32_bits(ib->gpu_addr));
> -        amdgpu_ring_write(ring, upper_32_bits(ib->gpu_addr));
> -        amdgpu_ring_write(ring, control);
> +                             lower_32_bits(ib->gpu_addr));
> +     amdgpu_ring_write(ring, upper_32_bits(ib->gpu_addr));
> +     amdgpu_ring_write(ring, control);
>   }
>
>   static void gfx_v9_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> index bedbd5f..d29f126 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> @@ -246,8 +246,10 @@ static void sdma_v2_4_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void sdma_v2_4_ring_emit_ib(struct amdgpu_ring *ring,
>                                   struct amdgpu_ib *ib,
> -                                unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        /* IB packet must end on a 8 DW boundary */
>        sdma_v2_4_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7)) % 8);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> index 6d039b6..061ef5f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> @@ -420,8 +420,10 @@ static void sdma_v3_0_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void sdma_v3_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                   struct amdgpu_ib *ib,
> -                                unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        /* IB packet must end on a 8 DW boundary */
>        sdma_v3_0_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7)) % 8);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index 2b97224..452fef3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -503,8 +503,10 @@ static void sdma_v4_0_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void sdma_v4_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                        struct amdgpu_ib *ib,
> -                                     unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        /* IB packet must end on a 8 DW boundary */
>        sdma_v4_0_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7)) % 8);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/si_dma.c b/drivers/gpu/drm/amd/amdgpu/si_dma.c
> index d9b27d7..1bf54b2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/si_dma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/si_dma.c
> @@ -62,8 +62,9 @@ static void si_dma_ring_set_wptr(struct amdgpu_ring *ring)
>
>   static void si_dma_ring_emit_ib(struct amdgpu_ring *ring,
>                                struct amdgpu_ib *ib,
> -                             unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        /* The indirect buffer packet must end on an 8 DW boundary in the DMA ring.
>         * Pad as necessary with NOPs.
>         */
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> index 1fc17bf..4706c91 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> @@ -520,7 +520,7 @@ static int uvd_v4_2_ring_test_ring(struct amdgpu_ring *ring)
>    */
>   static void uvd_v4_2_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        amdgpu_ring_write(ring, PACKET0(mmUVD_RBC_IB_BASE, 0));
>        amdgpu_ring_write(ring, ib->gpu_addr);
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> index fde6ad5..b69c2ac 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> @@ -536,7 +536,7 @@ static int uvd_v5_0_ring_test_ring(struct amdgpu_ring *ring)
>    */
>   static void uvd_v5_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        amdgpu_ring_write(ring, PACKET0(mmUVD_LMI_RBC_IB_64BIT_BAR_LOW, 0));
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> index 7a5b402..36f6b96 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> @@ -1005,8 +1005,10 @@ static int uvd_v6_0_ring_test_ring(struct amdgpu_ring *ring)
>    */
>   static void uvd_v6_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, PACKET0(mmUVD_LMI_RBC_IB_VMID, 0));
>        amdgpu_ring_write(ring, vmid);
>
> @@ -1027,8 +1029,10 @@ static void uvd_v6_0_ring_emit_ib(struct amdgpu_ring *ring,
>    * Write enc ring commands to execute the indirect buffer
>    */
>   static void uvd_v6_0_enc_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, HEVC_ENC_CMD_IB_VM);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> index 58b39af..beda492 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> @@ -1301,9 +1301,10 @@ static int uvd_v7_0_ring_patch_cs_in_place(struct amdgpu_cs_parser *p,
>    */
>   static void uvd_v7_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        struct amdgpu_device *adev = ring->adev;
> +     unsigned vmid = job ? job->vmid : 0;
>
>        amdgpu_ring_write(ring,
>                PACKET0(SOC15_REG_OFFSET(UVD, ring->me, mmUVD_LMI_RBC_IB_VMID), 0));
> @@ -1329,8 +1330,10 @@ static void uvd_v7_0_ring_emit_ib(struct amdgpu_ring *ring,
>    * Write enc ring commands to execute the indirect buffer
>    */
>   static void uvd_v7_0_enc_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, HEVC_ENC_CMD_IB_VM);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> index a1feea0..26661bf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> @@ -849,8 +849,10 @@ static void vce_v3_0_get_clockgating_state(void *handle, u32 *flags)
>   }
>
>   static void vce_v3_0_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, VCE_CMD_IB_VM);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> index 1c94718..e531cfe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> @@ -952,8 +952,10 @@ static int vce_v4_0_set_powergating_state(void *handle,
>   #endif
>
>   static void vce_v4_0_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, VCE_CMD_IB_VM);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> index eae9092..f7abf5b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> @@ -1367,9 +1367,10 @@ static void vcn_v1_0_dec_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64
>    */
>   static void vcn_v1_0_dec_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        struct amdgpu_device *adev = ring->adev;
> +     unsigned vmid = job ? job->vmid : 0;
>
>        amdgpu_ring_write(ring,
>                PACKET0(SOC15_REG_OFFSET(UVD, 0, mmUVD_LMI_RBC_IB_VMID), 0));
> @@ -1524,8 +1525,10 @@ static void vcn_v1_0_enc_ring_insert_end(struct amdgpu_ring *ring)
>    * Write enc ring commands to execute the indirect buffer
>    */
>   static void vcn_v1_0_enc_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, VCN_ENC_CMD_IB);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> @@ -1726,9 +1729,10 @@ static void vcn_v1_0_jpeg_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u6
>    */
>   static void vcn_v1_0_jpeg_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        struct amdgpu_device *adev = ring->adev;
> +     unsigned vmid = job ? job->vmid : 0;
>
>        amdgpu_ring_write(ring,
>                PACKETJ(SOC15_REG_OFFSET(UVD, 0, mmUVD_LMI_JRBC_IB_VMID), 0, 0, PACKETJ_TYPE0));



[-- Attachment #1.2: Type: text/html, Size: 46582 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/amdgpu: Modify the argument of emit_ib interface
       [not found]             ` <61db8416-8167-e150-c06e-48fd613fa6c8-5C7GfCeVMHo@public.gmane.org>
@ 2018-10-23 12:55               ` Zhu, Rex
       [not found]                 ` <BYAPR12MB2775566EEEB2472311E90EEDFBF50-ZGDeBxoHBPmJeBUhB162ZQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Zhu, Rex @ 2018-10-23 12:55 UTC (permalink / raw)
  To: Koenig, Christian, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW


[-- Attachment #1.1: Type: text/plain, Size: 24560 bytes --]

>Yeah, but that can be calculated in the ring specific function. Can't it?


Yes, we should reserve and map the CSA buffer for gfx/compute/sdma together.


Another concern is more and more parameters may be added to the the common interface.


Best Regards

Rex


________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>
Sent: Tuesday, October 23, 2018 8:47 PM
To: Zhu, Rex; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH] drm/amdgpu: Modify the argument of emit_ib interface

Am 23.10.18 um 14:43 schrieb Zhu, Rex:

>Maybe indeed provide the CSA address as parameter instead. It doesn't
>really matter that it is unused by most IP block implementations.

>Alternative is to use a wrapper function to get the VMID from the job.


I preferred to use a wrapper function to get vmid.


Because if use CSA address as parameter, we need to check which ring it is.

The csa address is different between sdma/gfx/compute.

Yeah, but that can be calculated in the ring specific function. Can't it?

I mean if I'm not completely mistaken it is just an offset which needs to be added to the base CSA address.

Christian.



Best Regards

Rex






________________________________
From: Christian König <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org><mailto:ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Sent: Tuesday, October 23, 2018 8:09 PM
To: Zhu, Rex; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9w@public.gmane.orgp.org>
Subject: Re: [PATCH] drm/amdgpu: Modify the argument of emit_ib interface

Am 23.10.18 um 14:01 schrieb Rex Zhu:
> use the point of struct amdgpu_job as the function
> argument instand of vmid, so the other members of
> struct amdgpu_job can be visit in emit_ib function.
>
> Signed-off-by: Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org><mailto:Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c   |  3 +--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c  |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h  |  2 +-
>   drivers/gpu/drm/amd/amdgpu/cik_sdma.c    |  3 ++-
>   drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c    |  3 ++-
>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c    |  6 ++++--
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c    |  6 ++++--
>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c    | 24 +++++++++++++-----------
>   drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c   |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c   |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c   |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/si_dma.c      |  3 ++-
>   drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c    |  2 +-
>   drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c    |  2 +-
>   drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c    |  8 ++++++--
>   drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c    |  7 +++++--
>   drivers/gpu/drm/amd/amdgpu/vce_v3_0.c    |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/vce_v4_0.c    |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c    | 10 +++++++---
>   20 files changed, 66 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index b8963b7..0b227ca 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -221,8 +221,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>                        !amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble CE ib must be inserted anyway */
>                        continue;
>
> -             amdgpu_ring_emit_ib(ring, ib, job ? job->vmid : 0,
> -                                 need_ctx_switch);
> +             amdgpu_ring_emit_ib(ring, ib, job, need_ctx_switch);
>                need_ctx_switch = false;
>        }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index 3cb7fb8..0f0f8fb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -229,7 +229,7 @@ struct amdgpu_ring {
>   #define amdgpu_ring_get_rptr(r) (r)->funcs->get_rptr((r))
>   #define amdgpu_ring_get_wptr(r) (r)->funcs->get_wptr((r))
>   #define amdgpu_ring_set_wptr(r) (r)->funcs->set_wptr((r))
> -#define amdgpu_ring_emit_ib(r, ib, vmid, c) (r)->funcs->emit_ib((r), (ib), (vmid), (c))
> +#define amdgpu_ring_emit_ib(r, ib, job, c) ((r)->funcs->emit_ib((r), (ib), (job), (c)))
>   #define amdgpu_ring_emit_pipeline_sync(r) (r)->funcs->emit_pipeline_sync((r))
>   #define amdgpu_ring_emit_vm_flush(r, vmid, addr) (r)->funcs->emit_vm_flush((r), (vmid), (addr))
>   #define amdgpu_ring_emit_fence(r, addr, seq, flags) (r)->funcs->emit_fence((r), (addr), (seq), (flags))
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> index 84dd550..8f98641 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> @@ -1032,7 +1032,7 @@ int amdgpu_vce_ring_parse_cs_vm(struct amdgpu_cs_parser *p, uint32_t ib_idx)
>    *
>    */
>   void amdgpu_vce_ring_emit_ib(struct amdgpu_ring *ring, struct amdgpu_ib *ib,
> -                          unsigned vmid, bool ctx_switch)
> +                                     struct amdgpu_job *job, bool ctx_switch)

The indentation here looks wrong on first glance.

Additional to that I would put the job before the IB in the parameter
list because it is the containing object.

>   {
>        amdgpu_ring_write(ring, VCE_CMD_IB);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> index a1f209e..06d6d87 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> @@ -66,7 +66,7 @@ int amdgpu_vce_get_destroy_msg(struct amdgpu_ring *ring, uint32_t handle,
>   int amdgpu_vce_ring_parse_cs(struct amdgpu_cs_parser *p, uint32_t ib_idx);
>   int amdgpu_vce_ring_parse_cs_vm(struct amdgpu_cs_parser *p, uint32_t ib_idx);
>   void amdgpu_vce_ring_emit_ib(struct amdgpu_ring *ring, struct amdgpu_ib *ib,
> -                          unsigned vmid, bool ctx_switch);
> +                             struct amdgpu_job *job, bool ctx_switch);
>   void amdgpu_vce_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64 seq,
>                                unsigned flags);
>   int amdgpu_vce_ring_test_ring(struct amdgpu_ring *ring);
> diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> index 32eb43d..70d4419 100644
> --- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> @@ -219,8 +219,9 @@ static void cik_sdma_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void cik_sdma_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;

I'm not 100% sure if duplicating this logic over and over again is such
a good idea.

Maybe indeed provide the CSA address as parameter instead. It doesn't
really matter that it is unused by most IP block implementations.

Alternative is to use a wrapper function to get the VMID from the job.

Regards,
Christian.

>        u32 extra_bits = vmid & 0xf;
>
>        /* IB packet must end on a 8 DW boundary */
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> index 622dd70..266482f6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> @@ -1846,8 +1846,9 @@ static void gfx_v6_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
>
>   static void gfx_v6_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 header, control = 0;
>
>        /* insert SWITCH_BUFFER packet before first IB in the ring frame */
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> index cfa45d9..dbcd9cc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> @@ -2234,8 +2234,9 @@ static void gfx_v7_0_ring_emit_fence_compute(struct amdgpu_ring *ring,
>    */
>   static void gfx_v7_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>                                      struct amdgpu_ib *ib,
> -                                   unsigned vmid, bool ctx_switch)
> +                                   struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 header, control = 0;
>
>        /* insert SWITCH_BUFFER packet before first IB in the ring frame */
> @@ -2263,8 +2264,9 @@ static void gfx_v7_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>
>   static void gfx_v7_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
>                                          struct amdgpu_ib *ib,
> -                                       unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
>
>        amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index f6cc0d3..355679a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -6130,8 +6130,9 @@ static void gfx_v8_0_ring_emit_vgt_flush(struct amdgpu_ring *ring)
>
>   static void gfx_v8_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>                                      struct amdgpu_ib *ib,
> -                                   unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 header, control = 0;
>
>        if (ib->flags & AMDGPU_IB_FLAG_CE)
> @@ -6160,8 +6161,9 @@ static void gfx_v8_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>
>   static void gfx_v8_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
>                                          struct amdgpu_ib *ib,
> -                                       unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
>
>        amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index f1bf0b7..34bf45c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -4072,9 +4072,10 @@ static void gfx_v9_0_ring_emit_hdp_flush(struct amdgpu_ring *ring)
>   }
>
>   static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
> -                                      struct amdgpu_ib *ib,
> -                                      unsigned vmid, bool ctx_switch)
> +                                     struct amdgpu_ib *ib,
> +                                     struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 header, control = 0;
>
>        if (ib->flags & AMDGPU_IB_FLAG_CE)
> @@ -4103,20 +4104,21 @@ static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>   }
>
>   static void gfx_v9_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
> -                                          struct amdgpu_ib *ib,
> -                                          unsigned vmid, bool ctx_switch)
> +                                     struct amdgpu_ib *ib,
> +                                     struct amdgpu_job *job, bool ctx_switch)
>   {
> -        u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
> +     unsigned vmid = job ? job->vmid : 0;
> +     u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
>
> -        amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
> +     amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
>        BUG_ON(ib->gpu_addr & 0x3); /* Dword align */
> -        amdgpu_ring_write(ring,
> +     amdgpu_ring_write(ring,
>   #ifdef __BIG_ENDIAN
> -                                (2 << 0) |
> +                             (2 << 0) |
>   #endif
> -                                lower_32_bits(ib->gpu_addr));
> -        amdgpu_ring_write(ring, upper_32_bits(ib->gpu_addr));
> -        amdgpu_ring_write(ring, control);
> +                             lower_32_bits(ib->gpu_addr));
> +     amdgpu_ring_write(ring, upper_32_bits(ib->gpu_addr));
> +     amdgpu_ring_write(ring, control);
>   }
>
>   static void gfx_v9_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> index bedbd5f..d29f126 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> @@ -246,8 +246,10 @@ static void sdma_v2_4_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void sdma_v2_4_ring_emit_ib(struct amdgpu_ring *ring,
>                                   struct amdgpu_ib *ib,
> -                                unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        /* IB packet must end on a 8 DW boundary */
>        sdma_v2_4_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7)) % 8);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> index 6d039b6..061ef5f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> @@ -420,8 +420,10 @@ static void sdma_v3_0_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void sdma_v3_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                   struct amdgpu_ib *ib,
> -                                unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        /* IB packet must end on a 8 DW boundary */
>        sdma_v3_0_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7)) % 8);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index 2b97224..452fef3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -503,8 +503,10 @@ static void sdma_v4_0_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void sdma_v4_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                        struct amdgpu_ib *ib,
> -                                     unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        /* IB packet must end on a 8 DW boundary */
>        sdma_v4_0_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7)) % 8);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/si_dma.c b/drivers/gpu/drm/amd/amdgpu/si_dma.c
> index d9b27d7..1bf54b2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/si_dma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/si_dma.c
> @@ -62,8 +62,9 @@ static void si_dma_ring_set_wptr(struct amdgpu_ring *ring)
>
>   static void si_dma_ring_emit_ib(struct amdgpu_ring *ring,
>                                struct amdgpu_ib *ib,
> -                             unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        /* The indirect buffer packet must end on an 8 DW boundary in the DMA ring.
>         * Pad as necessary with NOPs.
>         */
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> index 1fc17bf..4706c91 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> @@ -520,7 +520,7 @@ static int uvd_v4_2_ring_test_ring(struct amdgpu_ring *ring)
>    */
>   static void uvd_v4_2_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        amdgpu_ring_write(ring, PACKET0(mmUVD_RBC_IB_BASE, 0));
>        amdgpu_ring_write(ring, ib->gpu_addr);
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> index fde6ad5..b69c2ac 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> @@ -536,7 +536,7 @@ static int uvd_v5_0_ring_test_ring(struct amdgpu_ring *ring)
>    */
>   static void uvd_v5_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        amdgpu_ring_write(ring, PACKET0(mmUVD_LMI_RBC_IB_64BIT_BAR_LOW, 0));
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> index 7a5b402..36f6b96 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> @@ -1005,8 +1005,10 @@ static int uvd_v6_0_ring_test_ring(struct amdgpu_ring *ring)
>    */
>   static void uvd_v6_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, PACKET0(mmUVD_LMI_RBC_IB_VMID, 0));
>        amdgpu_ring_write(ring, vmid);
>
> @@ -1027,8 +1029,10 @@ static void uvd_v6_0_ring_emit_ib(struct amdgpu_ring *ring,
>    * Write enc ring commands to execute the indirect buffer
>    */
>   static void uvd_v6_0_enc_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, HEVC_ENC_CMD_IB_VM);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> index 58b39af..beda492 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> @@ -1301,9 +1301,10 @@ static int uvd_v7_0_ring_patch_cs_in_place(struct amdgpu_cs_parser *p,
>    */
>   static void uvd_v7_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        struct amdgpu_device *adev = ring->adev;
> +     unsigned vmid = job ? job->vmid : 0;
>
>        amdgpu_ring_write(ring,
>                PACKET0(SOC15_REG_OFFSET(UVD, ring->me, mmUVD_LMI_RBC_IB_VMID), 0));
> @@ -1329,8 +1330,10 @@ static void uvd_v7_0_ring_emit_ib(struct amdgpu_ring *ring,
>    * Write enc ring commands to execute the indirect buffer
>    */
>   static void uvd_v7_0_enc_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, HEVC_ENC_CMD_IB_VM);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> index a1feea0..26661bf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> @@ -849,8 +849,10 @@ static void vce_v3_0_get_clockgating_state(void *handle, u32 *flags)
>   }
>
>   static void vce_v3_0_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, VCE_CMD_IB_VM);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> index 1c94718..e531cfe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> @@ -952,8 +952,10 @@ static int vce_v4_0_set_powergating_state(void *handle,
>   #endif
>
>   static void vce_v4_0_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, VCE_CMD_IB_VM);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> index eae9092..f7abf5b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> @@ -1367,9 +1367,10 @@ static void vcn_v1_0_dec_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64
>    */
>   static void vcn_v1_0_dec_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        struct amdgpu_device *adev = ring->adev;
> +     unsigned vmid = job ? job->vmid : 0;
>
>        amdgpu_ring_write(ring,
>                PACKET0(SOC15_REG_OFFSET(UVD, 0, mmUVD_LMI_RBC_IB_VMID), 0));
> @@ -1524,8 +1525,10 @@ static void vcn_v1_0_enc_ring_insert_end(struct amdgpu_ring *ring)
>    * Write enc ring commands to execute the indirect buffer
>    */
>   static void vcn_v1_0_enc_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, VCN_ENC_CMD_IB);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> @@ -1726,9 +1729,10 @@ static void vcn_v1_0_jpeg_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u6
>    */
>   static void vcn_v1_0_jpeg_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        struct amdgpu_device *adev = ring->adev;
> +     unsigned vmid = job ? job->vmid : 0;
>
>        amdgpu_ring_write(ring,
>                PACKETJ(SOC15_REG_OFFSET(UVD, 0, mmUVD_LMI_JRBC_IB_VMID), 0, 0, PACKETJ_TYPE0));



[-- Attachment #1.2: Type: text/html, Size: 48268 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/amdgpu: Modify the argument of emit_ib interface
       [not found]                 ` <BYAPR12MB2775566EEEB2472311E90EEDFBF50-ZGDeBxoHBPmJeBUhB162ZQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2018-10-23 13:05                   ` Koenig, Christian
       [not found]                     ` <7ddb16a6-a91f-4cce-7ac6-e3a3158f1df6-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Koenig, Christian @ 2018-10-23 13:05 UTC (permalink / raw)
  To: Zhu, Rex, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW


[-- Attachment #1.1: Type: text/plain, Size: 24757 bytes --]

Another concern is more and more parameters may be added to the the common interface.

Yeah, that is a rather good argument.

Well in this case please add a wrapper for figuring out the VMID and add the job before the ib on the parameter list.

Christian.

Am 23.10.18 um 14:55 schrieb Zhu, Rex:

>Yeah, but that can be calculated in the ring specific function. Can't it?


Yes, we should reserve and map the CSA buffer for gfx/compute/sdma together.


Another concern is more and more parameters may be added to the the common interface.


Best Regards

Rex


________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org><mailto:amd-gfx-bounces@lists.freedesktop.org> on behalf of Koenig, Christian <Christian.Koenig@amd.com><mailto:Christian.Koenig@amd.com>
Sent: Tuesday, October 23, 2018 8:47 PM
To: Zhu, Rex; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/amdgpu: Modify the argument of emit_ib interface

Am 23.10.18 um 14:43 schrieb Zhu, Rex:

>Maybe indeed provide the CSA address as parameter instead. It doesn't
>really matter that it is unused by most IP block implementations.

>Alternative is to use a wrapper function to get the VMID from the job.


I preferred to use a wrapper function to get vmid.


Because if use CSA address as parameter, we need to check which ring it is.

The csa address is different between sdma/gfx/compute.

Yeah, but that can be calculated in the ring specific function. Can't it?

I mean if I'm not completely mistaken it is just an offset which needs to be added to the base CSA address.

Christian.



Best Regards

Rex






________________________________
From: Christian König <ckoenig.leichtzumerken@gmail.com><mailto:ckoenig.leichtzumerken@gmail.com>
Sent: Tuesday, October 23, 2018 8:09 PM
To: Zhu, Rex; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/amdgpu: Modify the argument of emit_ib interface

Am 23.10.18 um 14:01 schrieb Rex Zhu:
> use the point of struct amdgpu_job as the function
> argument instand of vmid, so the other members of
> struct amdgpu_job can be visit in emit_ib function.
>
> Signed-off-by: Rex Zhu <Rex.Zhu@amd.com><mailto:Rex.Zhu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c   |  3 +--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c  |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h  |  2 +-
>   drivers/gpu/drm/amd/amdgpu/cik_sdma.c    |  3 ++-
>   drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c    |  3 ++-
>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c    |  6 ++++--
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c    |  6 ++++--
>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c    | 24 +++++++++++++-----------
>   drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c   |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c   |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c   |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/si_dma.c      |  3 ++-
>   drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c    |  2 +-
>   drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c    |  2 +-
>   drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c    |  8 ++++++--
>   drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c    |  7 +++++--
>   drivers/gpu/drm/amd/amdgpu/vce_v3_0.c    |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/vce_v4_0.c    |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c    | 10 +++++++---
>   20 files changed, 66 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index b8963b7..0b227ca 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -221,8 +221,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>                        !amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble CE ib must be inserted anyway */
>                        continue;
>
> -             amdgpu_ring_emit_ib(ring, ib, job ? job->vmid : 0,
> -                                 need_ctx_switch);
> +             amdgpu_ring_emit_ib(ring, ib, job, need_ctx_switch);
>                need_ctx_switch = false;
>        }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index 3cb7fb8..0f0f8fb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -229,7 +229,7 @@ struct amdgpu_ring {
>   #define amdgpu_ring_get_rptr(r) (r)->funcs->get_rptr((r))
>   #define amdgpu_ring_get_wptr(r) (r)->funcs->get_wptr((r))
>   #define amdgpu_ring_set_wptr(r) (r)->funcs->set_wptr((r))
> -#define amdgpu_ring_emit_ib(r, ib, vmid, c) (r)->funcs->emit_ib((r), (ib), (vmid), (c))
> +#define amdgpu_ring_emit_ib(r, ib, job, c) ((r)->funcs->emit_ib((r), (ib), (job), (c)))
>   #define amdgpu_ring_emit_pipeline_sync(r) (r)->funcs->emit_pipeline_sync((r))
>   #define amdgpu_ring_emit_vm_flush(r, vmid, addr) (r)->funcs->emit_vm_flush((r), (vmid), (addr))
>   #define amdgpu_ring_emit_fence(r, addr, seq, flags) (r)->funcs->emit_fence((r), (addr), (seq), (flags))
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> index 84dd550..8f98641 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> @@ -1032,7 +1032,7 @@ int amdgpu_vce_ring_parse_cs_vm(struct amdgpu_cs_parser *p, uint32_t ib_idx)
>    *
>    */
>   void amdgpu_vce_ring_emit_ib(struct amdgpu_ring *ring, struct amdgpu_ib *ib,
> -                          unsigned vmid, bool ctx_switch)
> +                                     struct amdgpu_job *job, bool ctx_switch)

The indentation here looks wrong on first glance.

Additional to that I would put the job before the IB in the parameter
list because it is the containing object.

>   {
>        amdgpu_ring_write(ring, VCE_CMD_IB);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> index a1f209e..06d6d87 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> @@ -66,7 +66,7 @@ int amdgpu_vce_get_destroy_msg(struct amdgpu_ring *ring, uint32_t handle,
>   int amdgpu_vce_ring_parse_cs(struct amdgpu_cs_parser *p, uint32_t ib_idx);
>   int amdgpu_vce_ring_parse_cs_vm(struct amdgpu_cs_parser *p, uint32_t ib_idx);
>   void amdgpu_vce_ring_emit_ib(struct amdgpu_ring *ring, struct amdgpu_ib *ib,
> -                          unsigned vmid, bool ctx_switch);
> +                             struct amdgpu_job *job, bool ctx_switch);
>   void amdgpu_vce_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64 seq,
>                                unsigned flags);
>   int amdgpu_vce_ring_test_ring(struct amdgpu_ring *ring);
> diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> index 32eb43d..70d4419 100644
> --- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> @@ -219,8 +219,9 @@ static void cik_sdma_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void cik_sdma_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;

I'm not 100% sure if duplicating this logic over and over again is such
a good idea.

Maybe indeed provide the CSA address as parameter instead. It doesn't
really matter that it is unused by most IP block implementations.

Alternative is to use a wrapper function to get the VMID from the job.

Regards,
Christian.

>        u32 extra_bits = vmid & 0xf;
>
>        /* IB packet must end on a 8 DW boundary */
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> index 622dd70..266482f6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> @@ -1846,8 +1846,9 @@ static void gfx_v6_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
>
>   static void gfx_v6_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 header, control = 0;
>
>        /* insert SWITCH_BUFFER packet before first IB in the ring frame */
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> index cfa45d9..dbcd9cc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> @@ -2234,8 +2234,9 @@ static void gfx_v7_0_ring_emit_fence_compute(struct amdgpu_ring *ring,
>    */
>   static void gfx_v7_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>                                      struct amdgpu_ib *ib,
> -                                   unsigned vmid, bool ctx_switch)
> +                                   struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 header, control = 0;
>
>        /* insert SWITCH_BUFFER packet before first IB in the ring frame */
> @@ -2263,8 +2264,9 @@ static void gfx_v7_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>
>   static void gfx_v7_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
>                                          struct amdgpu_ib *ib,
> -                                       unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
>
>        amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index f6cc0d3..355679a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -6130,8 +6130,9 @@ static void gfx_v8_0_ring_emit_vgt_flush(struct amdgpu_ring *ring)
>
>   static void gfx_v8_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>                                      struct amdgpu_ib *ib,
> -                                   unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 header, control = 0;
>
>        if (ib->flags & AMDGPU_IB_FLAG_CE)
> @@ -6160,8 +6161,9 @@ static void gfx_v8_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>
>   static void gfx_v8_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
>                                          struct amdgpu_ib *ib,
> -                                       unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
>
>        amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index f1bf0b7..34bf45c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -4072,9 +4072,10 @@ static void gfx_v9_0_ring_emit_hdp_flush(struct amdgpu_ring *ring)
>   }
>
>   static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
> -                                      struct amdgpu_ib *ib,
> -                                      unsigned vmid, bool ctx_switch)
> +                                     struct amdgpu_ib *ib,
> +                                     struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 header, control = 0;
>
>        if (ib->flags & AMDGPU_IB_FLAG_CE)
> @@ -4103,20 +4104,21 @@ static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>   }
>
>   static void gfx_v9_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
> -                                          struct amdgpu_ib *ib,
> -                                          unsigned vmid, bool ctx_switch)
> +                                     struct amdgpu_ib *ib,
> +                                     struct amdgpu_job *job, bool ctx_switch)
>   {
> -        u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
> +     unsigned vmid = job ? job->vmid : 0;
> +     u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
>
> -        amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
> +     amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
>        BUG_ON(ib->gpu_addr & 0x3); /* Dword align */
> -        amdgpu_ring_write(ring,
> +     amdgpu_ring_write(ring,
>   #ifdef __BIG_ENDIAN
> -                                (2 << 0) |
> +                             (2 << 0) |
>   #endif
> -                                lower_32_bits(ib->gpu_addr));
> -        amdgpu_ring_write(ring, upper_32_bits(ib->gpu_addr));
> -        amdgpu_ring_write(ring, control);
> +                             lower_32_bits(ib->gpu_addr));
> +     amdgpu_ring_write(ring, upper_32_bits(ib->gpu_addr));
> +     amdgpu_ring_write(ring, control);
>   }
>
>   static void gfx_v9_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> index bedbd5f..d29f126 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> @@ -246,8 +246,10 @@ static void sdma_v2_4_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void sdma_v2_4_ring_emit_ib(struct amdgpu_ring *ring,
>                                   struct amdgpu_ib *ib,
> -                                unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        /* IB packet must end on a 8 DW boundary */
>        sdma_v2_4_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7)) % 8);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> index 6d039b6..061ef5f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> @@ -420,8 +420,10 @@ static void sdma_v3_0_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void sdma_v3_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                   struct amdgpu_ib *ib,
> -                                unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        /* IB packet must end on a 8 DW boundary */
>        sdma_v3_0_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7)) % 8);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index 2b97224..452fef3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -503,8 +503,10 @@ static void sdma_v4_0_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void sdma_v4_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                        struct amdgpu_ib *ib,
> -                                     unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        /* IB packet must end on a 8 DW boundary */
>        sdma_v4_0_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7)) % 8);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/si_dma.c b/drivers/gpu/drm/amd/amdgpu/si_dma.c
> index d9b27d7..1bf54b2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/si_dma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/si_dma.c
> @@ -62,8 +62,9 @@ static void si_dma_ring_set_wptr(struct amdgpu_ring *ring)
>
>   static void si_dma_ring_emit_ib(struct amdgpu_ring *ring,
>                                struct amdgpu_ib *ib,
> -                             unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        /* The indirect buffer packet must end on an 8 DW boundary in the DMA ring.
>         * Pad as necessary with NOPs.
>         */
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> index 1fc17bf..4706c91 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> @@ -520,7 +520,7 @@ static int uvd_v4_2_ring_test_ring(struct amdgpu_ring *ring)
>    */
>   static void uvd_v4_2_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        amdgpu_ring_write(ring, PACKET0(mmUVD_RBC_IB_BASE, 0));
>        amdgpu_ring_write(ring, ib->gpu_addr);
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> index fde6ad5..b69c2ac 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> @@ -536,7 +536,7 @@ static int uvd_v5_0_ring_test_ring(struct amdgpu_ring *ring)
>    */
>   static void uvd_v5_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        amdgpu_ring_write(ring, PACKET0(mmUVD_LMI_RBC_IB_64BIT_BAR_LOW, 0));
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> index 7a5b402..36f6b96 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> @@ -1005,8 +1005,10 @@ static int uvd_v6_0_ring_test_ring(struct amdgpu_ring *ring)
>    */
>   static void uvd_v6_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, PACKET0(mmUVD_LMI_RBC_IB_VMID, 0));
>        amdgpu_ring_write(ring, vmid);
>
> @@ -1027,8 +1029,10 @@ static void uvd_v6_0_ring_emit_ib(struct amdgpu_ring *ring,
>    * Write enc ring commands to execute the indirect buffer
>    */
>   static void uvd_v6_0_enc_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, HEVC_ENC_CMD_IB_VM);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> index 58b39af..beda492 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> @@ -1301,9 +1301,10 @@ static int uvd_v7_0_ring_patch_cs_in_place(struct amdgpu_cs_parser *p,
>    */
>   static void uvd_v7_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        struct amdgpu_device *adev = ring->adev;
> +     unsigned vmid = job ? job->vmid : 0;
>
>        amdgpu_ring_write(ring,
>                PACKET0(SOC15_REG_OFFSET(UVD, ring->me, mmUVD_LMI_RBC_IB_VMID), 0));
> @@ -1329,8 +1330,10 @@ static void uvd_v7_0_ring_emit_ib(struct amdgpu_ring *ring,
>    * Write enc ring commands to execute the indirect buffer
>    */
>   static void uvd_v7_0_enc_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, HEVC_ENC_CMD_IB_VM);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> index a1feea0..26661bf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> @@ -849,8 +849,10 @@ static void vce_v3_0_get_clockgating_state(void *handle, u32 *flags)
>   }
>
>   static void vce_v3_0_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, VCE_CMD_IB_VM);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> index 1c94718..e531cfe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> @@ -952,8 +952,10 @@ static int vce_v4_0_set_powergating_state(void *handle,
>   #endif
>
>   static void vce_v4_0_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, VCE_CMD_IB_VM);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> index eae9092..f7abf5b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> @@ -1367,9 +1367,10 @@ static void vcn_v1_0_dec_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64
>    */
>   static void vcn_v1_0_dec_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        struct amdgpu_device *adev = ring->adev;
> +     unsigned vmid = job ? job->vmid : 0;
>
>        amdgpu_ring_write(ring,
>                PACKET0(SOC15_REG_OFFSET(UVD, 0, mmUVD_LMI_RBC_IB_VMID), 0));
> @@ -1524,8 +1525,10 @@ static void vcn_v1_0_enc_ring_insert_end(struct amdgpu_ring *ring)
>    * Write enc ring commands to execute the indirect buffer
>    */
>   static void vcn_v1_0_enc_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, VCN_ENC_CMD_IB);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> @@ -1726,9 +1729,10 @@ static void vcn_v1_0_jpeg_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u6
>    */
>   static void vcn_v1_0_jpeg_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        struct amdgpu_device *adev = ring->adev;
> +     unsigned vmid = job ? job->vmid : 0;
>
>        amdgpu_ring_write(ring,
>                PACKETJ(SOC15_REG_OFFSET(UVD, 0, mmUVD_LMI_JRBC_IB_VMID), 0, 0, PACKETJ_TYPE0));




[-- Attachment #1.2: Type: text/html, Size: 49024 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/amdgpu: Modify the argument of emit_ib interface
       [not found]                     ` <7ddb16a6-a91f-4cce-7ac6-e3a3158f1df6-5C7GfCeVMHo@public.gmane.org>
@ 2018-10-23 13:32                       ` Zhu, Rex
  0 siblings, 0 replies; 7+ messages in thread
From: Zhu, Rex @ 2018-10-23 13:32 UTC (permalink / raw)
  To: Koenig, Christian, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW


[-- Attachment #1.1: Type: text/plain, Size: 25489 bytes --]

Ok, I will refine the patch as you suggested.


Thanks.


Best Regards

Rex



________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>
Sent: Tuesday, October 23, 2018 9:05 PM
To: Zhu, Rex; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH] drm/amdgpu: Modify the argument of emit_ib interface


Another concern is more and more parameters may be added to the the common interface.

Yeah, that is a rather good argument.

Well in this case please add a wrapper for figuring out the VMID and add the job before the ib on the parameter list.

Christian.

Am 23.10.18 um 14:55 schrieb Zhu, Rex:

>Yeah, but that can be calculated in the ring specific function. Can't it?


Yes, we should reserve and map the CSA buffer for gfx/compute/sdma together.


Another concern is more and more parameters may be added to the the common interface.


Best Regards

Rex


________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org><mailto:amd-gfx-bounces@lists.freedesktop.org> on behalf of Koenig, Christian <Christian.Koenig@amd.com><mailto:Christian.Koenig-5C7GfCeVMHo@public.gmane.org>
Sent: Tuesday, October 23, 2018 8:47 PM
To: Zhu, Rex; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9w@public.gmane.orgp.org>
Subject: Re: [PATCH] drm/amdgpu: Modify the argument of emit_ib interface

Am 23.10.18 um 14:43 schrieb Zhu, Rex:

>Maybe indeed provide the CSA address as parameter instead. It doesn't
>really matter that it is unused by most IP block implementations.

>Alternative is to use a wrapper function to get the VMID from the job.


I preferred to use a wrapper function to get vmid.


Because if use CSA address as parameter, we need to check which ring it is.

The csa address is different between sdma/gfx/compute.

Yeah, but that can be calculated in the ring specific function. Can't it?

I mean if I'm not completely mistaken it is just an offset which needs to be added to the base CSA address.

Christian.



Best Regards

Rex






________________________________
From: Christian König <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org><mailto:ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Sent: Tuesday, October 23, 2018 8:09 PM
To: Zhu, Rex; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9w@public.gmane.orgp.org>
Subject: Re: [PATCH] drm/amdgpu: Modify the argument of emit_ib interface

Am 23.10.18 um 14:01 schrieb Rex Zhu:
> use the point of struct amdgpu_job as the function
> argument instand of vmid, so the other members of
> struct amdgpu_job can be visit in emit_ib function.
>
> Signed-off-by: Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org><mailto:Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c   |  3 +--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c  |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h  |  2 +-
>   drivers/gpu/drm/amd/amdgpu/cik_sdma.c    |  3 ++-
>   drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c    |  3 ++-
>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c    |  6 ++++--
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c    |  6 ++++--
>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c    | 24 +++++++++++++-----------
>   drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c   |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c   |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c   |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/si_dma.c      |  3 ++-
>   drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c    |  2 +-
>   drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c    |  2 +-
>   drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c    |  8 ++++++--
>   drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c    |  7 +++++--
>   drivers/gpu/drm/amd/amdgpu/vce_v3_0.c    |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/vce_v4_0.c    |  4 +++-
>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c    | 10 +++++++---
>   20 files changed, 66 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index b8963b7..0b227ca 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -221,8 +221,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>                        !amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble CE ib must be inserted anyway */
>                        continue;
>
> -             amdgpu_ring_emit_ib(ring, ib, job ? job->vmid : 0,
> -                                 need_ctx_switch);
> +             amdgpu_ring_emit_ib(ring, ib, job, need_ctx_switch);
>                need_ctx_switch = false;
>        }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index 3cb7fb8..0f0f8fb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -229,7 +229,7 @@ struct amdgpu_ring {
>   #define amdgpu_ring_get_rptr(r) (r)->funcs->get_rptr((r))
>   #define amdgpu_ring_get_wptr(r) (r)->funcs->get_wptr((r))
>   #define amdgpu_ring_set_wptr(r) (r)->funcs->set_wptr((r))
> -#define amdgpu_ring_emit_ib(r, ib, vmid, c) (r)->funcs->emit_ib((r), (ib), (vmid), (c))
> +#define amdgpu_ring_emit_ib(r, ib, job, c) ((r)->funcs->emit_ib((r), (ib), (job), (c)))
>   #define amdgpu_ring_emit_pipeline_sync(r) (r)->funcs->emit_pipeline_sync((r))
>   #define amdgpu_ring_emit_vm_flush(r, vmid, addr) (r)->funcs->emit_vm_flush((r), (vmid), (addr))
>   #define amdgpu_ring_emit_fence(r, addr, seq, flags) (r)->funcs->emit_fence((r), (addr), (seq), (flags))
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> index 84dd550..8f98641 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> @@ -1032,7 +1032,7 @@ int amdgpu_vce_ring_parse_cs_vm(struct amdgpu_cs_parser *p, uint32_t ib_idx)
>    *
>    */
>   void amdgpu_vce_ring_emit_ib(struct amdgpu_ring *ring, struct amdgpu_ib *ib,
> -                          unsigned vmid, bool ctx_switch)
> +                                     struct amdgpu_job *job, bool ctx_switch)

The indentation here looks wrong on first glance.

Additional to that I would put the job before the IB in the parameter
list because it is the containing object.

>   {
>        amdgpu_ring_write(ring, VCE_CMD_IB);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> index a1f209e..06d6d87 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> @@ -66,7 +66,7 @@ int amdgpu_vce_get_destroy_msg(struct amdgpu_ring *ring, uint32_t handle,
>   int amdgpu_vce_ring_parse_cs(struct amdgpu_cs_parser *p, uint32_t ib_idx);
>   int amdgpu_vce_ring_parse_cs_vm(struct amdgpu_cs_parser *p, uint32_t ib_idx);
>   void amdgpu_vce_ring_emit_ib(struct amdgpu_ring *ring, struct amdgpu_ib *ib,
> -                          unsigned vmid, bool ctx_switch);
> +                             struct amdgpu_job *job, bool ctx_switch);
>   void amdgpu_vce_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64 seq,
>                                unsigned flags);
>   int amdgpu_vce_ring_test_ring(struct amdgpu_ring *ring);
> diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> index 32eb43d..70d4419 100644
> --- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> @@ -219,8 +219,9 @@ static void cik_sdma_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void cik_sdma_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;

I'm not 100% sure if duplicating this logic over and over again is such
a good idea.

Maybe indeed provide the CSA address as parameter instead. It doesn't
really matter that it is unused by most IP block implementations.

Alternative is to use a wrapper function to get the VMID from the job.

Regards,
Christian.

>        u32 extra_bits = vmid & 0xf;
>
>        /* IB packet must end on a 8 DW boundary */
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> index 622dd70..266482f6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> @@ -1846,8 +1846,9 @@ static void gfx_v6_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
>
>   static void gfx_v6_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 header, control = 0;
>
>        /* insert SWITCH_BUFFER packet before first IB in the ring frame */
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> index cfa45d9..dbcd9cc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> @@ -2234,8 +2234,9 @@ static void gfx_v7_0_ring_emit_fence_compute(struct amdgpu_ring *ring,
>    */
>   static void gfx_v7_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>                                      struct amdgpu_ib *ib,
> -                                   unsigned vmid, bool ctx_switch)
> +                                   struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 header, control = 0;
>
>        /* insert SWITCH_BUFFER packet before first IB in the ring frame */
> @@ -2263,8 +2264,9 @@ static void gfx_v7_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>
>   static void gfx_v7_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
>                                          struct amdgpu_ib *ib,
> -                                       unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
>
>        amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index f6cc0d3..355679a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -6130,8 +6130,9 @@ static void gfx_v8_0_ring_emit_vgt_flush(struct amdgpu_ring *ring)
>
>   static void gfx_v8_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>                                      struct amdgpu_ib *ib,
> -                                   unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 header, control = 0;
>
>        if (ib->flags & AMDGPU_IB_FLAG_CE)
> @@ -6160,8 +6161,9 @@ static void gfx_v8_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>
>   static void gfx_v8_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
>                                          struct amdgpu_ib *ib,
> -                                       unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
>
>        amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index f1bf0b7..34bf45c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -4072,9 +4072,10 @@ static void gfx_v9_0_ring_emit_hdp_flush(struct amdgpu_ring *ring)
>   }
>
>   static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
> -                                      struct amdgpu_ib *ib,
> -                                      unsigned vmid, bool ctx_switch)
> +                                     struct amdgpu_ib *ib,
> +                                     struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        u32 header, control = 0;
>
>        if (ib->flags & AMDGPU_IB_FLAG_CE)
> @@ -4103,20 +4104,21 @@ static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>   }
>
>   static void gfx_v9_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
> -                                          struct amdgpu_ib *ib,
> -                                          unsigned vmid, bool ctx_switch)
> +                                     struct amdgpu_ib *ib,
> +                                     struct amdgpu_job *job, bool ctx_switch)
>   {
> -        u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
> +     unsigned vmid = job ? job->vmid : 0;
> +     u32 control = INDIRECT_BUFFER_VALID | ib->length_dw | (vmid << 24);
>
> -        amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
> +     amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
>        BUG_ON(ib->gpu_addr & 0x3); /* Dword align */
> -        amdgpu_ring_write(ring,
> +     amdgpu_ring_write(ring,
>   #ifdef __BIG_ENDIAN
> -                                (2 << 0) |
> +                             (2 << 0) |
>   #endif
> -                                lower_32_bits(ib->gpu_addr));
> -        amdgpu_ring_write(ring, upper_32_bits(ib->gpu_addr));
> -        amdgpu_ring_write(ring, control);
> +                             lower_32_bits(ib->gpu_addr));
> +     amdgpu_ring_write(ring, upper_32_bits(ib->gpu_addr));
> +     amdgpu_ring_write(ring, control);
>   }
>
>   static void gfx_v9_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> index bedbd5f..d29f126 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> @@ -246,8 +246,10 @@ static void sdma_v2_4_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void sdma_v2_4_ring_emit_ib(struct amdgpu_ring *ring,
>                                   struct amdgpu_ib *ib,
> -                                unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        /* IB packet must end on a 8 DW boundary */
>        sdma_v2_4_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7)) % 8);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> index 6d039b6..061ef5f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> @@ -420,8 +420,10 @@ static void sdma_v3_0_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void sdma_v3_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                   struct amdgpu_ib *ib,
> -                                unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        /* IB packet must end on a 8 DW boundary */
>        sdma_v3_0_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7)) % 8);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index 2b97224..452fef3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -503,8 +503,10 @@ static void sdma_v4_0_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>    */
>   static void sdma_v4_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                        struct amdgpu_ib *ib,
> -                                     unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        /* IB packet must end on a 8 DW boundary */
>        sdma_v4_0_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7)) % 8);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/si_dma.c b/drivers/gpu/drm/amd/amdgpu/si_dma.c
> index d9b27d7..1bf54b2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/si_dma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/si_dma.c
> @@ -62,8 +62,9 @@ static void si_dma_ring_set_wptr(struct amdgpu_ring *ring)
>
>   static void si_dma_ring_emit_ib(struct amdgpu_ring *ring,
>                                struct amdgpu_ib *ib,
> -                             unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
>        /* The indirect buffer packet must end on an 8 DW boundary in the DMA ring.
>         * Pad as necessary with NOPs.
>         */
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> index 1fc17bf..4706c91 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> @@ -520,7 +520,7 @@ static int uvd_v4_2_ring_test_ring(struct amdgpu_ring *ring)
>    */
>   static void uvd_v4_2_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        amdgpu_ring_write(ring, PACKET0(mmUVD_RBC_IB_BASE, 0));
>        amdgpu_ring_write(ring, ib->gpu_addr);
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> index fde6ad5..b69c2ac 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> @@ -536,7 +536,7 @@ static int uvd_v5_0_ring_test_ring(struct amdgpu_ring *ring)
>    */
>   static void uvd_v5_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        amdgpu_ring_write(ring, PACKET0(mmUVD_LMI_RBC_IB_64BIT_BAR_LOW, 0));
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> index 7a5b402..36f6b96 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> @@ -1005,8 +1005,10 @@ static int uvd_v6_0_ring_test_ring(struct amdgpu_ring *ring)
>    */
>   static void uvd_v6_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, PACKET0(mmUVD_LMI_RBC_IB_VMID, 0));
>        amdgpu_ring_write(ring, vmid);
>
> @@ -1027,8 +1029,10 @@ static void uvd_v6_0_ring_emit_ib(struct amdgpu_ring *ring,
>    * Write enc ring commands to execute the indirect buffer
>    */
>   static void uvd_v6_0_enc_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, HEVC_ENC_CMD_IB_VM);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> index 58b39af..beda492 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> @@ -1301,9 +1301,10 @@ static int uvd_v7_0_ring_patch_cs_in_place(struct amdgpu_cs_parser *p,
>    */
>   static void uvd_v7_0_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        struct amdgpu_device *adev = ring->adev;
> +     unsigned vmid = job ? job->vmid : 0;
>
>        amdgpu_ring_write(ring,
>                PACKET0(SOC15_REG_OFFSET(UVD, ring->me, mmUVD_LMI_RBC_IB_VMID), 0));
> @@ -1329,8 +1330,10 @@ static void uvd_v7_0_ring_emit_ib(struct amdgpu_ring *ring,
>    * Write enc ring commands to execute the indirect buffer
>    */
>   static void uvd_v7_0_enc_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, HEVC_ENC_CMD_IB_VM);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> index a1feea0..26661bf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> @@ -849,8 +849,10 @@ static void vce_v3_0_get_clockgating_state(void *handle, u32 *flags)
>   }
>
>   static void vce_v3_0_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, VCE_CMD_IB_VM);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> index 1c94718..e531cfe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> @@ -952,8 +952,10 @@ static int vce_v4_0_set_powergating_state(void *handle,
>   #endif
>
>   static void vce_v4_0_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, VCE_CMD_IB_VM);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> index eae9092..f7abf5b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> @@ -1367,9 +1367,10 @@ static void vcn_v1_0_dec_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64
>    */
>   static void vcn_v1_0_dec_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        struct amdgpu_device *adev = ring->adev;
> +     unsigned vmid = job ? job->vmid : 0;
>
>        amdgpu_ring_write(ring,
>                PACKET0(SOC15_REG_OFFSET(UVD, 0, mmUVD_LMI_RBC_IB_VMID), 0));
> @@ -1524,8 +1525,10 @@ static void vcn_v1_0_enc_ring_insert_end(struct amdgpu_ring *ring)
>    * Write enc ring commands to execute the indirect buffer
>    */
>   static void vcn_v1_0_enc_ring_emit_ib(struct amdgpu_ring *ring,
> -             struct amdgpu_ib *ib, unsigned int vmid, bool ctx_switch)
> +             struct amdgpu_ib *ib, struct amdgpu_job *job, bool ctx_switch)
>   {
> +     unsigned vmid = job ? job->vmid : 0;
> +
>        amdgpu_ring_write(ring, VCN_ENC_CMD_IB);
>        amdgpu_ring_write(ring, vmid);
>        amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> @@ -1726,9 +1729,10 @@ static void vcn_v1_0_jpeg_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u6
>    */
>   static void vcn_v1_0_jpeg_ring_emit_ib(struct amdgpu_ring *ring,
>                                  struct amdgpu_ib *ib,
> -                               unsigned vmid, bool ctx_switch)
> +                               struct amdgpu_job *job, bool ctx_switch)
>   {
>        struct amdgpu_device *adev = ring->adev;
> +     unsigned vmid = job ? job->vmid : 0;
>
>        amdgpu_ring_write(ring,
>                PACKETJ(SOC15_REG_OFFSET(UVD, 0, mmUVD_LMI_JRBC_IB_VMID), 0, 0, PACKETJ_TYPE0));




[-- Attachment #1.2: Type: text/html, Size: 50591 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-10-23 13:32 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-23 12:01 [PATCH] drm/amdgpu: Modify the argument of emit_ib interface Rex Zhu
     [not found] ` <1540296077-9971-1-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
2018-10-23 12:09   ` Christian König
     [not found]     ` <331a74c1-2dac-52e4-363f-eaed2b1c9aa3-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-10-23 12:43       ` Zhu, Rex
     [not found]         ` <BYAPR12MB2775CA38B33B516CF35C61B5FBF50-ZGDeBxoHBPmJeBUhB162ZQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2018-10-23 12:47           ` Koenig, Christian
     [not found]             ` <61db8416-8167-e150-c06e-48fd613fa6c8-5C7GfCeVMHo@public.gmane.org>
2018-10-23 12:55               ` Zhu, Rex
     [not found]                 ` <BYAPR12MB2775566EEEB2472311E90EEDFBF50-ZGDeBxoHBPmJeBUhB162ZQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2018-10-23 13:05                   ` Koenig, Christian
     [not found]                     ` <7ddb16a6-a91f-4cce-7ac6-e3a3158f1df6-5C7GfCeVMHo@public.gmane.org>
2018-10-23 13:32                       ` Zhu, Rex

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.