[PATCH 1/2] drm/amdgpu: Remove duplicate code in gfx_v8_0.c

Christian König ckoenig.leichtzumerken at gmail.com
Fri Aug 17 07:26:12 UTC 2018


Am 17.08.2018 um 09:06 schrieb Rex Zhu:
> There are no any logical changes here.
>
> 1. if kcq can be enabled via kiq, we don't need to
>     do kiq ring test.
> 2. amdgpu_ring_test_ring function can be used to
>     sync the ring complete, remove the duplicate code.
>
> v2: alloc 6 (not 7) dws for unmap_queues
>
> Signed-off-by: Rex Zhu <Rex.Zhu at amd.com>

At least of hand looks like a nice cleanup to be, but I can't fully 
judge because I'm not so familiar with the KIQ (I should probably change 
that at some point).

Series is Acked-by: Christian König <christian.koenig at amd.com>.

Regards,
Christian.

> ---
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 80 ++++++-----------------------------
>   1 file changed, 13 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 282dba6..906d4a5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -4604,7 +4604,6 @@ static void gfx_v8_0_kiq_setting(struct amdgpu_ring *ring)
>   static int gfx_v8_0_kiq_kcq_enable(struct amdgpu_device *adev)
>   {
>   	struct amdgpu_ring *kiq_ring = &adev->gfx.kiq.ring;
> -	uint32_t scratch, tmp = 0;
>   	uint64_t queue_mask = 0;
>   	int r, i;
>   
> @@ -4623,17 +4622,10 @@ static int gfx_v8_0_kiq_kcq_enable(struct amdgpu_device *adev)
>   		queue_mask |= (1ull << i);
>   	}
>   
> -	r = amdgpu_gfx_scratch_get(adev, &scratch);
> -	if (r) {
> -		DRM_ERROR("Failed to get scratch reg (%d).\n", r);
> -		return r;
> -	}
> -	WREG32(scratch, 0xCAFEDEAD);
> -
> -	r = amdgpu_ring_alloc(kiq_ring, (8 * adev->gfx.num_compute_rings) + 11);
> +	kiq_ring->ready = true;
> +	r = amdgpu_ring_alloc(kiq_ring, (8 * adev->gfx.num_compute_rings) + 8);
>   	if (r) {
>   		DRM_ERROR("Failed to lock KIQ (%d).\n", r);
> -		amdgpu_gfx_scratch_free(adev, scratch);
>   		return r;
>   	}
>   	/* set resources */
> @@ -4665,25 +4657,12 @@ static int gfx_v8_0_kiq_kcq_enable(struct amdgpu_device *adev)
>   		amdgpu_ring_write(kiq_ring, lower_32_bits(wptr_addr));
>   		amdgpu_ring_write(kiq_ring, upper_32_bits(wptr_addr));
>   	}
> -	/* write to scratch for completion */
> -	amdgpu_ring_write(kiq_ring, PACKET3(PACKET3_SET_UCONFIG_REG, 1));
> -	amdgpu_ring_write(kiq_ring, (scratch - PACKET3_SET_UCONFIG_REG_START));
> -	amdgpu_ring_write(kiq_ring, 0xDEADBEEF);
> -	amdgpu_ring_commit(kiq_ring);
>   
> -	for (i = 0; i < adev->usec_timeout; i++) {
> -		tmp = RREG32(scratch);
> -		if (tmp == 0xDEADBEEF)
> -			break;
> -		DRM_UDELAY(1);
> -	}
> -	if (i >= adev->usec_timeout) {
> -		DRM_ERROR("KCQ enable failed (scratch(0x%04X)=0x%08X)\n",
> -			  scratch, tmp);
> -		r = -EINVAL;
> +	r = amdgpu_ring_test_ring(kiq_ring);
> +	if (r) {
> +		DRM_ERROR("KCQ enable failed\n");
> +		kiq_ring->ready = false;
>   	}
> -	amdgpu_gfx_scratch_free(adev, scratch);
> -
>   	return r;
>   }
>   
> @@ -5014,15 +4993,6 @@ static int gfx_v8_0_kiq_resume(struct amdgpu_device *adev)
>   	if (r)
>   		goto done;
>   
> -	/* Test KIQ */
> -	ring = &adev->gfx.kiq.ring;
> -	ring->ready = true;
> -	r = amdgpu_ring_test_ring(ring);
> -	if (r) {
> -		ring->ready = false;
> -		goto done;
> -	}
> -
>   	/* Test KCQs */
>   	for (i = 0; i < adev->gfx.num_compute_rings; i++) {
>   		ring = &adev->gfx.compute_ring[i];
> @@ -5092,23 +5062,11 @@ static int gfx_v8_0_hw_init(void *handle)
>   
>   static int gfx_v8_0_kcq_disable(struct amdgpu_ring *kiq_ring,struct amdgpu_ring *ring)
>   {
> -	struct amdgpu_device *adev = kiq_ring->adev;
> -	uint32_t scratch, tmp = 0;
> -	int r, i;
> -
> -	r = amdgpu_gfx_scratch_get(adev, &scratch);
> -	if (r) {
> -		DRM_ERROR("Failed to get scratch reg (%d).\n", r);
> -		return r;
> -	}
> -	WREG32(scratch, 0xCAFEDEAD);
> +	int r;
>   
> -	r = amdgpu_ring_alloc(kiq_ring, 10);
> -	if (r) {
> +	r = amdgpu_ring_alloc(kiq_ring, 6);
> +	if (r)
>   		DRM_ERROR("Failed to lock KIQ (%d).\n", r);
> -		amdgpu_gfx_scratch_free(adev, scratch);
> -		return r;
> -	}
>   
>   	/* unmap queues */
>   	amdgpu_ring_write(kiq_ring, PACKET3(PACKET3_UNMAP_QUEUES, 4));
> @@ -5121,23 +5079,11 @@ static int gfx_v8_0_kcq_disable(struct amdgpu_ring *kiq_ring,struct amdgpu_ring
>   	amdgpu_ring_write(kiq_ring, 0);
>   	amdgpu_ring_write(kiq_ring, 0);
>   	amdgpu_ring_write(kiq_ring, 0);
> -	/* write to scratch for completion */
> -	amdgpu_ring_write(kiq_ring, PACKET3(PACKET3_SET_UCONFIG_REG, 1));
> -	amdgpu_ring_write(kiq_ring, (scratch - PACKET3_SET_UCONFIG_REG_START));
> -	amdgpu_ring_write(kiq_ring, 0xDEADBEEF);
> -	amdgpu_ring_commit(kiq_ring);
>   
> -	for (i = 0; i < adev->usec_timeout; i++) {
> -		tmp = RREG32(scratch);
> -		if (tmp == 0xDEADBEEF)
> -			break;
> -		DRM_UDELAY(1);
> -	}
> -	if (i >= adev->usec_timeout) {
> -		DRM_ERROR("KCQ disabled failed (scratch(0x%04X)=0x%08X)\n", scratch, tmp);
> -		r = -EINVAL;
> -	}
> -	amdgpu_gfx_scratch_free(adev, scratch);
> +	r = amdgpu_ring_test_ring(kiq_ring);
> +	if (r)
> +		DRM_ERROR("KCQ disable failed\n");
> +
>   	return r;
>   }
>   



More information about the amd-gfx mailing list