Re: [PATCH 1/3] drm/amdgpu: Enable only one high prio compute queue

[Christian König <christian.koenig@xxxxxxx>]

Am 01.02.21 um 13:07 schrieb Nirmoy Das:
> For high priority compute to work properly we need to enable
> wave limiting on gfx pipe. Wave limiting is done through writing
> into mmSPI_WCL_PIPE_PERCENT_GFX register. Enable only one high
> priority compute queue to avoid race condition between multiple
> high priority compute queues writing that register simultaneously.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@xxxxxxx>

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

Looks good, but you might ask somebody which is more familiar with the 
compute ring background for a review.

Christian.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 15 ++++++++-------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h |  2 +-
>   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c  |  6 ++----
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c   |  6 ++----
>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c   |  7 ++-----
>   5 files changed, 15 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index cd2c676a2797..8e0a6c62322e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -193,15 +193,16 @@ static bool amdgpu_gfx_is_multipipe_capable(struct amdgpu_device *adev)
>   }
>   
>   bool amdgpu_gfx_is_high_priority_compute_queue(struct amdgpu_device *adev,
> -					       int pipe, int queue)
> +					       struct amdgpu_ring *ring)
>   {
> -	bool multipipe_policy = amdgpu_gfx_is_multipipe_capable(adev);
> -	int cond;
> -	/* Policy: alternate between normal and high priority */
> -	cond = multipipe_policy ? pipe : queue;
> -
> -	return ((cond % 2) != 0);
> +	/* Policy: use 1st queue as high priority compute queue if we
> +	 * have more than one compute queue.
> +	 */
> +	if (adev->gfx.num_compute_rings > 1 &&
> +	    ring == &adev->gfx.compute_ring[0])
> +		return true;
>   
> +	return false;
>   }
>   
>   void amdgpu_gfx_compute_queue_acquire(struct amdgpu_device *adev)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> index 6b5a8f4642cc..72dbcd2bc6a6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> @@ -380,7 +380,7 @@ void amdgpu_queue_mask_bit_to_mec_queue(struct amdgpu_device *adev, int bit,
>   bool amdgpu_gfx_is_mec_queue_enabled(struct amdgpu_device *adev, int mec,
>   				     int pipe, int queue);
>   bool amdgpu_gfx_is_high_priority_compute_queue(struct amdgpu_device *adev,
> -					       int pipe, int queue);
> +					       struct amdgpu_ring *ring);
>   int amdgpu_gfx_me_queue_to_bit(struct amdgpu_device *adev, int me,
>   			       int pipe, int queue);
>   void amdgpu_gfx_bit_to_me_queue(struct amdgpu_device *adev, int bit,
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> index 426a217ce83f..0987890d3e51 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> @@ -4487,8 +4487,7 @@ static int gfx_v10_0_compute_ring_init(struct amdgpu_device *adev, int ring_id,
>   	irq_type = AMDGPU_CP_IRQ_COMPUTE_MEC1_PIPE0_EOP
>   		+ ((ring->me - 1) * adev->gfx.mec.num_pipe_per_mec)
>   		+ ring->pipe;
> -	hw_prio = amdgpu_gfx_is_high_priority_compute_queue(adev, ring->pipe,
> -							    ring->queue) ?
> +	hw_prio = amdgpu_gfx_is_high_priority_compute_queue(adev, ring) ?
>   			AMDGPU_GFX_PIPE_PRIO_HIGH : AMDGPU_GFX_PIPE_PRIO_NORMAL;
>   	/* type-2 packets are deprecated on MEC, use type-3 instead */
>   	r = amdgpu_ring_init(adev, ring, 1024,
> @@ -6530,8 +6529,7 @@ static void gfx_v10_0_compute_mqd_set_priority(struct amdgpu_ring *ring, struct
>   	struct amdgpu_device *adev = ring->adev;
>   
>   	if (ring->funcs->type == AMDGPU_RING_TYPE_COMPUTE) {
> -		if (amdgpu_gfx_is_high_priority_compute_queue(adev, ring->pipe,
> -							      ring->queue)) {
> +		if (amdgpu_gfx_is_high_priority_compute_queue(adev, ring)) {
>   			mqd->cp_hqd_pipe_priority = AMDGPU_GFX_PIPE_PRIO_HIGH;
>   			mqd->cp_hqd_queue_priority =
>   				AMDGPU_GFX_QUEUE_PRIORITY_MAXIMUM;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 37639214cbbb..b0284c4659ba 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -1923,8 +1923,7 @@ static int gfx_v8_0_compute_ring_init(struct amdgpu_device *adev, int ring_id,
>   		+ ((ring->me - 1) * adev->gfx.mec.num_pipe_per_mec)
>   		+ ring->pipe;
>   
> -	hw_prio = amdgpu_gfx_is_high_priority_compute_queue(adev, ring->pipe,
> -							    ring->queue) ?
> +	hw_prio = amdgpu_gfx_is_high_priority_compute_queue(adev, ring) ?
>   			AMDGPU_GFX_PIPE_PRIO_HIGH : AMDGPU_RING_PRIO_DEFAULT;
>   	/* type-2 packets are deprecated on MEC, use type-3 instead */
>   	r = amdgpu_ring_init(adev, ring, 1024,
> @@ -4442,8 +4441,7 @@ static void gfx_v8_0_mqd_set_priority(struct amdgpu_ring *ring, struct vi_mqd *m
>   	struct amdgpu_device *adev = ring->adev;
>   
>   	if (ring->funcs->type == AMDGPU_RING_TYPE_COMPUTE) {
> -		if (amdgpu_gfx_is_high_priority_compute_queue(adev, ring->pipe,
> -							      ring->queue)) {
> +		if (amdgpu_gfx_is_high_priority_compute_queue(adev, ring)) {
>   			mqd->cp_hqd_pipe_priority = AMDGPU_GFX_PIPE_PRIO_HIGH;
>   			mqd->cp_hqd_queue_priority =
>   				AMDGPU_GFX_QUEUE_PRIORITY_MAXIMUM;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index a896e3d0fcf8..7b13f9dd965a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -2227,8 +2227,7 @@ static int gfx_v9_0_compute_ring_init(struct amdgpu_device *adev, int ring_id,
>   	irq_type = AMDGPU_CP_IRQ_COMPUTE_MEC1_PIPE0_EOP
>   		+ ((ring->me - 1) * adev->gfx.mec.num_pipe_per_mec)
>   		+ ring->pipe;
> -	hw_prio = amdgpu_gfx_is_high_priority_compute_queue(adev, ring->pipe,
> -							    ring->queue) ?
> +	hw_prio = amdgpu_gfx_is_high_priority_compute_queue(adev, ring) ?
>   			AMDGPU_GFX_PIPE_PRIO_HIGH : AMDGPU_GFX_PIPE_PRIO_NORMAL;
>   	/* type-2 packets are deprecated on MEC, use type-3 instead */
>   	return amdgpu_ring_init(adev, ring, 1024,
> @@ -3390,9 +3389,7 @@ static void gfx_v9_0_mqd_set_priority(struct amdgpu_ring *ring, struct v9_mqd *m
>   	struct amdgpu_device *adev = ring->adev;
>   
>   	if (ring->funcs->type == AMDGPU_RING_TYPE_COMPUTE) {
> -		if (amdgpu_gfx_is_high_priority_compute_queue(adev,
> -							      ring->pipe,
> -							      ring->queue)) {
> +		if (amdgpu_gfx_is_high_priority_compute_queue(adev, ring)) {
>   			mqd->cp_hqd_pipe_priority = AMDGPU_GFX_PIPE_PRIO_HIGH;
>   			mqd->cp_hqd_queue_priority =
>   				AMDGPU_GFX_QUEUE_PRIORITY_MAXIMUM;

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