Re: [PATCH] drm/amdgpu/vcn:enable priority queues for encoder

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Satyajit,

On 8/10/2021 12:39 PM, Satyajit Sahu wrote:
VCN and VCE support multiple queues with different priority.
Use differnt encoder queue based on the priority set by UMD.

Signed-off-by: Satyajit Sahu <satyajit.sahu@xxxxxxx>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c   | 35 +++++++++++++++++++++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c   | 14 +++++++++
  drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h   |  9 ++++++
  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c   | 14 +++++++++
  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h   |  8 ++++++
  drivers/gpu/drm/amd/amdgpu/vce_v2_0.c     |  4 ++-
  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     |  4 ++-
  drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c     |  4 ++-
  drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c     |  4 ++-
  drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c     |  5 ++--
  13 files changed, 99 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index e7a010b7ca1f..b0ae2b45c7c3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -73,15 +73,44 @@ static enum gfx_pipe_priority amdgpu_ctx_sched_prio_to_compute_prio(enum drm_sch
  	}
  }
+static enum gfx_pipe_priority amdgpu_ctx_sched_prio_to_vcn_prio(enum drm_sched_priority prio)
+{
+	switch (prio) {
+	case DRM_SCHED_PRIORITY_HIGH:
+		return AMDGPU_VCN_ENC_PRIO_HIGH;
+	case DRM_SCHED_PRIORITY_KERNEL:
+		return AMDGPU_VCN_ENC_PRIO_VERY_HIGH;
+	default:
+		return AMDGPU_VCN_ENC_PRIO_NORMAL;
+	}
+}
+
+static enum gfx_pipe_priority amdgpu_ctx_sched_prio_to_vce_prio(enum drm_sched_priority prio)
+{
+	switch (prio) {
+	case DRM_SCHED_PRIORITY_HIGH:
+		return AMDGPU_VCE_ENC_PRIO_HIGH;
+	case DRM_SCHED_PRIORITY_KERNEL:
+		return AMDGPU_VCE_ENC_PRIO_VERY_HIGH;
+	default:
+		return AMDGPU_VCE_ENC_PRIO_NORMAL;
+	}
+}
+

As it is also mentioned by Nirmoy, we can't use kernel priority for VCE/VCN ENC work due to the delay/stale punishment.

But adding DRM_SCHED_PRIORITY_VERY_HIGH also might not be a good way as other drivers might not be very interested in that (or are they ?)

I can think of two ways to go ahead from here:

Mrthod 1:
=========
- Introduce DRM_SCHED_PRIORITY_VERY_HIGH in the DRM scheduler layer, and demo a usecase of the same in our high priority VCN queues (in fact this should work for high priority gfx queues as well)

- Check the feedback from the community

If there are not many consumer or there is not much interest around:

Mehod 2:
========
- Add a AMDGPU specific priority flag while submitting the work
- Use this flag to keep the DRM scheduler level: DRM_SCHED_PRIORITY_HIGH but the AMD scheduler level: AMDGPU_VCE_ENC_PRIO_VERY_HIGH - Pass this flag parameter to amdgpu_ctx_sched_prio_to_vcn_prio/amdgpu_ctx_sched_prio_to_vce_prio functions and set the proper scheduler level which will allow AMDGPU driver to schedule it in the correct ring

  static unsigned int amdgpu_ctx_prio_sched_to_hw(struct amdgpu_device *adev,
  						 enum drm_sched_priority prio,
  						 u32 hw_ip)
  {
  	unsigned int hw_prio;
- hw_prio = (hw_ip == AMDGPU_HW_IP_COMPUTE) ?
-			amdgpu_ctx_sched_prio_to_compute_prio(prio) :
-			AMDGPU_RING_PRIO_DEFAULT;
+	if (hw_ip == AMDGPU_HW_IP_COMPUTE)
+		hw_prio = amdgpu_ctx_sched_prio_to_compute_prio(prio);
+	else if (hw_ip == AMDGPU_HW_IP_VCN_ENC)
+		hw_prio = amdgpu_ctx_sched_prio_to_vcn_prio(prio);
+	else if (hw_ip == AMDGPU_HW_IP_VCE)
+		hw_prio = amdgpu_ctx_sched_prio_to_vce_prio(prio);
+	else
+		hw_prio = AMDGPU_RING_PRIO_DEFAULT;

Move to switch(case) probably for the better appearance ?

  	hw_ip = array_index_nospec(hw_ip, AMDGPU_HW_IP_NUM);
  	if (adev->gpu_sched[hw_ip][hw_prio].num_scheds == 0)
  		hw_prio = AMDGPU_RING_PRIO_DEFAULT;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
index b7d861ed5284..4087acb6b8bb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
@@ -37,7 +37,7 @@ int amdgpu_to_sched_priority(int amdgpu_priority,
  {
  	switch (amdgpu_priority) {
  	case AMDGPU_CTX_PRIORITY_VERY_HIGH:
-		*prio = DRM_SCHED_PRIORITY_HIGH;
+		*prio = DRM_SCHED_PRIORITY_KERNEL;
as mentioned above
  		break;
  	case AMDGPU_CTX_PRIORITY_HIGH:
  		*prio = DRM_SCHED_PRIORITY_HIGH;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
index 1ae7f824adc7..9d59ca31bc22 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
@@ -1168,3 +1168,17 @@ int amdgpu_vce_ring_test_ib(struct amdgpu_ring *ring, long timeout)
  	amdgpu_bo_free_kernel(&bo, NULL, NULL);
  	return r;
  }
+
+enum vce_enc_ring_priority get_vce_ring_prio(int index)
+{
+	switch(index) {
+	case 0:
+		return AMDGPU_VCE_ENC_PRIO_NORMAL;
+	case 1:
+		return AMDGPU_VCE_ENC_PRIO_HIGH;
+	case 2:
+		return AMDGPU_VCE_ENC_PRIO_VERY_HIGH;
Please use AMDGPU_VCE_ENC_PRIO_NORMAL/HIGH/VERY_HIGH instead of 0/1/2 for better readability.
+	default:
+		return AMDGPU_VCE_ENC_PRIO_NORMAL;
+	}
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
index d6d83a3ec803..f5265f385890 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
@@ -32,6 +32,13 @@
#define AMDGPU_VCE_FW_53_45 ((53 << 24) | (45 << 16)) +enum vce_enc_ring_priority {
+	AMDGPU_VCE_ENC_PRIO_NORMAL = 1,
+	AMDGPU_VCE_ENC_PRIO_HIGH,
+	AMDGPU_VCE_ENC_PRIO_VERY_HIGH,
+	AMDGPU_VCE_ENC_PRIO_MAX
+};
+
  struct amdgpu_vce {
  	struct amdgpu_bo	*vcpu_bo;
  	uint64_t		gpu_addr;
@@ -72,4 +79,6 @@ void amdgpu_vce_ring_end_use(struct amdgpu_ring *ring);
  unsigned amdgpu_vce_ring_get_emit_ib_size(struct amdgpu_ring *ring);
  unsigned amdgpu_vce_ring_get_dma_frame_size(struct amdgpu_ring *ring);
+enum vce_enc_ring_priority get_vce_ring_prio(int index);
+
  #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 6780df0fb265..ca6cc07a726b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -951,3 +951,17 @@ int amdgpu_vcn_enc_ring_test_ib(struct amdgpu_ring *ring, long timeout)
return r;
  }
+
+enum vcn_enc_ring_priority get_vcn_enc_ring_prio(int index)
+{
+	switch(index) {
+	case 0:
+		return AMDGPU_VCN_ENC_PRIO_NORMAL;
+	case 1:
+		return AMDGPU_VCN_ENC_PRIO_HIGH;
+	case 2:
+		return AMDGPU_VCN_ENC_PRIO_VERY_HIGH;
Same here
+	default:
+		return AMDGPU_VCN_ENC_PRIO_NORMAL;
+	}
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index d74c62b49795..bf14ee54f774 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -290,6 +290,13 @@ enum vcn_ring_type {
  	VCN_UNIFIED_RING,
  };
+enum vcn_enc_ring_priority {
+	AMDGPU_VCN_ENC_PRIO_NORMAL = 1,
+	AMDGPU_VCN_ENC_PRIO_HIGH,
+	AMDGPU_VCN_ENC_PRIO_VERY_HIGH,
+	AMDGPU_VCN_ENC_PRIO_MAX
+};
+
  int amdgpu_vcn_sw_init(struct amdgpu_device *adev);
  int amdgpu_vcn_sw_fini(struct amdgpu_device *adev);
  int amdgpu_vcn_suspend(struct amdgpu_device *adev);
@@ -307,5 +314,6 @@ int amdgpu_vcn_dec_sw_ring_test_ib(struct amdgpu_ring *ring, long timeout);
int amdgpu_vcn_enc_ring_test_ring(struct amdgpu_ring *ring);
  int amdgpu_vcn_enc_ring_test_ib(struct amdgpu_ring *ring, long timeout);
+enum vcn_enc_ring_priority get_vcn_enc_ring_prio(int index);
#endif
diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
index c7d28c169be5..2b6b7f1a77b9 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
@@ -431,10 +431,12 @@ static int vce_v2_0_sw_init(void *handle)
  		return r;
for (i = 0; i < adev->vce.num_rings; i++) {
+		unsigned int hw_prio = get_vce_ring_prio(i);
+
  		ring = &adev->vce.ring[i];
  		sprintf(ring->name, "vce%d", i);
  		r = amdgpu_ring_init(adev, ring, 512, &adev->vce.irq, 0,
-				     AMDGPU_RING_PRIO_DEFAULT, NULL);
+				     hw_prio, NULL);
  		if (r)
  			return r;
  	}
diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
index 3b82fb289ef6..5ce182a837f3 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
@@ -440,10 +440,12 @@ static int vce_v3_0_sw_init(void *handle)
  		return r;
for (i = 0; i < adev->vce.num_rings; i++) {
+		unsigned int hw_prio = get_vce_ring_prio(i);
+
  		ring = &adev->vce.ring[i];
  		sprintf(ring->name, "vce%d", i);
  		r = amdgpu_ring_init(adev, ring, 512, &adev->vce.irq, 0,
-				     AMDGPU_RING_PRIO_DEFAULT, NULL);
+				     hw_prio, NULL);
  		if (r)
  			return r;
  	}
diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
index 90910d19db12..c085defaabfe 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
@@ -463,6 +463,8 @@ static int vce_v4_0_sw_init(void *handle)
  	}
for (i = 0; i < adev->vce.num_rings; i++) {
+		unsigned int hw_prio = get_vce_ring_prio(i);
+
  		ring = &adev->vce.ring[i];
  		sprintf(ring->name, "vce%d", i);
  		if (amdgpu_sriov_vf(adev)) {
@@ -478,7 +480,7 @@ static int vce_v4_0_sw_init(void *handle)
  				ring->doorbell_index = adev->doorbell_index.uvd_vce.vce_ring2_3 * 2 + 1;
  		}
  		r = amdgpu_ring_init(adev, ring, 512, &adev->vce.irq, 0,
-				     AMDGPU_RING_PRIO_DEFAULT, NULL);
+				     hw_prio, NULL);
  		if (r)
  			return r;
  	}
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
index 121ee9f2b8d1..f471c65d78bc 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
@@ -145,10 +145,12 @@ static int vcn_v1_0_sw_init(void *handle)
  		SOC15_REG_OFFSET(UVD, 0, mmUVD_NO_OP);
for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
+		unsigned int hw_prio = get_vcn_enc_ring_prio(i);
+
  		ring = &adev->vcn.inst->ring_enc[i];
  		sprintf(ring->name, "vcn_enc%d", i);
  		r = amdgpu_ring_init(adev, ring, 512, &adev->vcn.inst->irq, 0,
-				     AMDGPU_RING_PRIO_DEFAULT, NULL);
+				     hw_prio, NULL);
  		if (r)
  			return r;
  	}
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
index f4686e918e0d..06978d5a93c7 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
@@ -159,6 +159,8 @@ static int vcn_v2_0_sw_init(void *handle)
  	adev->vcn.inst->external.nop = SOC15_REG_OFFSET(UVD, 0, mmUVD_NO_OP);
for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
+		unsigned int hw_prio = get_vcn_enc_ring_prio(i);
+
  		ring = &adev->vcn.inst->ring_enc[i];
  		ring->use_doorbell = true;
  		if (!amdgpu_sriov_vf(adev))
@@ -167,7 +169,7 @@ static int vcn_v2_0_sw_init(void *handle)
  			ring->doorbell_index = (adev->doorbell_index.vcn.vcn_ring0_1 << 1) + 1 + i;
  		sprintf(ring->name, "vcn_enc%d", i);
  		r = amdgpu_ring_init(adev, ring, 512, &adev->vcn.inst->irq, 0,
-				     AMDGPU_RING_PRIO_DEFAULT, NULL);
+				     hw_prio, NULL);
  		if (r)
  			return r;
  	}
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
index e0c0c3734432..fba453ca74fa 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
@@ -194,6 +194,8 @@ static int vcn_v2_5_sw_init(void *handle)
  			return r;
for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
+			unsigned int hw_prio = get_vcn_enc_ring_prio(i);
+
  			ring = &adev->vcn.inst[j].ring_enc[i];
  			ring->use_doorbell = true;
@@ -203,7 +205,7 @@ static int vcn_v2_5_sw_init(void *handle)
  			sprintf(ring->name, "vcn_enc_%d.%d", j, i);
  			r = amdgpu_ring_init(adev, ring, 512,
  					     &adev->vcn.inst[j].irq, 0,
-					     AMDGPU_RING_PRIO_DEFAULT, NULL);
+					     hw_prio, NULL);
  			if (r)
  				return r;
  		}
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
index 3d18aab88b4e..f5baf7321b0d 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
@@ -224,6 +224,8 @@ static int vcn_v3_0_sw_init(void *handle)
  			return r;
for (j = 0; j < adev->vcn.num_enc_rings; ++j) {
+			unsigned int hw_prio = get_vcn_enc_ring_prio(j);
+
  			/* VCN ENC TRAP */
  			r = amdgpu_irq_add_id(adev, amdgpu_ih_clientid_vcns[i],
  				j + VCN_2_0__SRCID__UVD_ENC_GENERAL_PURPOSE, &adev->vcn.inst[i].irq);
@@ -239,8 +241,7 @@ static int vcn_v3_0_sw_init(void *handle)
  			}
  			sprintf(ring->name, "vcn_enc_%d.%d", i, j);
  			r = amdgpu_ring_init(adev, ring, 512, &adev->vcn.inst[i].irq, 0,
-					     AMDGPU_RING_PRIO_DEFAULT,
-					     &adev->vcn.inst[i].sched_score);
+					     hw_prio, &adev->vcn.inst[i].sched_score);
  			if (r)
  				return r;
  		}

I think this patch can be split into 2,
 patch 1: introduce the priority core logic and functions
patch 2: consume priority logic across various vcn generations sw_init functions (vcn_v1_sw_init, vcn_v2_sw_init, vcn_v2_5_sw_init and so on)

- Shashank




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux