Re: [PATCH] drm/amdgpu: make scratch_get/free more general

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

Am 05.05.22 um 05:20 schrieb Lang Yu:
> Then other IPs can use this utility.

Oh, we intentionally move the scratch register handling from the device 
code into the GFX code a few years ago.

Why is that suddenly necessary here again?

Regards,
Christian.

> Signed-off-by: Lang Yu <Lang.Yu@xxxxxxx>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  3 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 36 ++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c    | 36 ----------------------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h    | 12 --------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h   |  9 ++++++
>   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c     |  6 ++--
>   drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c     |  6 ++--
>   drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c      |  8 ++---
>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c      |  8 ++---
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c      |  4 +--
>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c      |  4 +--
>   11 files changed, 66 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 578a405f777c..2f85d5a228a9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1272,6 +1272,9 @@ u32 amdgpu_device_pcie_port_rreg(struct amdgpu_device *adev,
>   void amdgpu_device_pcie_port_wreg(struct amdgpu_device *adev,
>   				u32 reg, u32 v);
>   
> +int amdgpu_device_scratch_get(struct amdgpu_scratch *scratch, uint32_t *reg);
> +void amdgpu_device_scratch_free(struct amdgpu_scratch *scratch, uint32_t reg);
> +
>   /* atpx handler */
>   #if defined(CONFIG_VGA_SWITCHEROO)
>   void amdgpu_register_atpx_handler(void);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index e582f1044c0f..abbccede3586 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -5846,3 +5846,39 @@ void amdgpu_device_pcie_port_wreg(struct amdgpu_device *adev,
>   	(void)RREG32(data);
>   	spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
>   }
> +
> +/**
> + * amdgpu_device_scratch_get - Allocate a scratch register
> + *
> + * @scratch: amdgpu_scratch pointer
> + * @reg: scratch register mmio offset
> + *
> + * Allocate a scratch register for use by the driver (all asics).
> + * Returns 0 on success or -EINVAL on failure.
> + */
> +int amdgpu_device_scratch_get(struct amdgpu_scratch *scratch, uint32_t *reg)
> +{
> +	int i;
> +
> +	i = ffs(scratch->free_mask);
> +	if (i != 0 && i <= scratch->num_reg) {
> +		i--;
> +		scratch->free_mask &= ~(1u << i);
> +		*reg = scratch->reg_base + i;
> +		return 0;
> +	}
> +	return -EINVAL;
> +}
> +
> +/**
> + * amdgpu_device_scratch_free - Free a scratch register
> + *
> + * @scratch: amdgpu_scratch pointer
> + * @reg: scratch register mmio offset
> + *
> + * Free a scratch register allocated for use by the driver (all asics)
> + */
> +void amdgpu_device_scratch_free(struct amdgpu_scratch *scratch, uint32_t reg)
> +{
> +	scratch->free_mask |= 1u << (reg - scratch->reg_base);
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index 5d6b04fc6206..ede2fa56f6c9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -98,42 +98,6 @@ bool amdgpu_gfx_is_me_queue_enabled(struct amdgpu_device *adev,
>   			adev->gfx.me.queue_bitmap);
>   }
>   
> -/**
> - * amdgpu_gfx_scratch_get - Allocate a scratch register
> - *
> - * @adev: amdgpu_device pointer
> - * @reg: scratch register mmio offset
> - *
> - * Allocate a CP scratch register for use by the driver (all asics).
> - * Returns 0 on success or -EINVAL on failure.
> - */
> -int amdgpu_gfx_scratch_get(struct amdgpu_device *adev, uint32_t *reg)
> -{
> -	int i;
> -
> -	i = ffs(adev->gfx.scratch.free_mask);
> -	if (i != 0 && i <= adev->gfx.scratch.num_reg) {
> -		i--;
> -		adev->gfx.scratch.free_mask &= ~(1u << i);
> -		*reg = adev->gfx.scratch.reg_base + i;
> -		return 0;
> -	}
> -	return -EINVAL;
> -}
> -
> -/**
> - * amdgpu_gfx_scratch_free - Free a scratch register
> - *
> - * @adev: amdgpu_device pointer
> - * @reg: scratch register mmio offset
> - *
> - * Free a CP scratch register allocated for use by the driver (all asics)
> - */
> -void amdgpu_gfx_scratch_free(struct amdgpu_device *adev, uint32_t reg)
> -{
> -	adev->gfx.scratch.free_mask |= 1u << (reg - adev->gfx.scratch.reg_base);
> -}
> -
>   /**
>    * amdgpu_gfx_parse_disable_cu - Parse the disable_cu module parameter
>    *
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> index 45522609d4b4..54455961720b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> @@ -110,15 +110,6 @@ struct amdgpu_kiq {
>   	const struct kiq_pm4_funcs *pmf;
>   };
>   
> -/*
> - * GPU scratch registers structures, functions & helpers
> - */
> -struct amdgpu_scratch {
> -	unsigned		num_reg;
> -	uint32_t                reg_base;
> -	uint32_t		free_mask;
> -};
> -
>   /*
>    * GFX configurations
>    */
> @@ -376,9 +367,6 @@ static inline u32 amdgpu_gfx_create_bitmask(u32 bit_width)
>   	return (u32)((1ULL << bit_width) - 1);
>   }
>   
> -int amdgpu_gfx_scratch_get(struct amdgpu_device *adev, uint32_t *reg);
> -void amdgpu_gfx_scratch_free(struct amdgpu_device *adev, uint32_t reg);
> -
>   void amdgpu_gfx_parse_disable_cu(unsigned *mask, unsigned max_se,
>   				 unsigned max_sh);
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index 7d89a52091c0..7d09842730cd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -102,6 +102,15 @@ struct amdgpu_sched {
>   	struct drm_gpu_scheduler	*sched[AMDGPU_MAX_HWIP_RINGS];
>   };
>   
> +/*
> + * GPU scratch registers structures, functions & helpers
> + */
> +struct amdgpu_scratch {
> +	uint32_t		num_reg;
> +	uint32_t                reg_base;
> +	uint32_t		free_mask;
> +};
> +
>   /*
>    * Fences.
>    */
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> index 64d36622ee23..bd5b2e1ab2c3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> @@ -3792,7 +3792,7 @@ static int gfx_v10_0_ring_test_ring(struct amdgpu_ring *ring)
>   	unsigned i;
>   	int r;
>   
> -	r = amdgpu_gfx_scratch_get(adev, &scratch);
> +	r = amdgpu_device_scratch_get(&adev->gfx.scratch, &scratch);
>   	if (r) {
>   		DRM_ERROR("amdgpu: cp failed to get scratch reg (%d).\n", r);
>   		return r;
> @@ -3804,7 +3804,7 @@ static int gfx_v10_0_ring_test_ring(struct amdgpu_ring *ring)
>   	if (r) {
>   		DRM_ERROR("amdgpu: cp failed to lock ring %d (%d).\n",
>   			  ring->idx, r);
> -		amdgpu_gfx_scratch_free(adev, scratch);
> +		amdgpu_device_scratch_free(&adev->gfx.scratch, scratch);
>   		return r;
>   	}
>   
> @@ -3826,7 +3826,7 @@ static int gfx_v10_0_ring_test_ring(struct amdgpu_ring *ring)
>   	if (i >= adev->usec_timeout)
>   		r = -ETIMEDOUT;
>   
> -	amdgpu_gfx_scratch_free(adev, scratch);
> +	amdgpu_device_scratch_free(&adev->gfx.scratch, scratch);
>   
>   	return r;
>   }
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> index c5655128fd9c..c6d33d6be778 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> @@ -313,7 +313,7 @@ static int gfx_v11_0_ring_test_ring(struct amdgpu_ring *ring)
>   	unsigned i;
>   	int r;
>   
> -	r = amdgpu_gfx_scratch_get(adev, &scratch);
> +	r = amdgpu_device_scratch_get(&adev->gfx.scratch, &scratch);
>   	if (r) {
>   		DRM_ERROR("amdgpu: cp failed to get scratch reg (%d).\n", r);
>   		return r;
> @@ -325,7 +325,7 @@ static int gfx_v11_0_ring_test_ring(struct amdgpu_ring *ring)
>   	if (r) {
>   		DRM_ERROR("amdgpu: cp failed to lock ring %d (%d).\n",
>   			  ring->idx, r);
> -		amdgpu_gfx_scratch_free(adev, scratch);
> +		amdgpu_device_scratch_free(&adev->gfx.scratch, scratch);
>   		return r;
>   	}
>   
> @@ -351,7 +351,7 @@ static int gfx_v11_0_ring_test_ring(struct amdgpu_ring *ring)
>   	if (i >= adev->usec_timeout)
>   		r = -ETIMEDOUT;
>   
> -	amdgpu_gfx_scratch_free(adev, scratch);
> +	amdgpu_device_scratch_free(&adev->gfx.scratch, scratch);
>   
>   	return r;
>   }
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> index 29a91b320d4f..896bbf50a494 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> @@ -1794,7 +1794,7 @@ static int gfx_v6_0_ring_test_ring(struct amdgpu_ring *ring)
>   	unsigned i;
>   	int r;
>   
> -	r = amdgpu_gfx_scratch_get(adev, &scratch);
> +	r = amdgpu_device_scratch_get(&adev->gfx.scratch, &scratch);
>   	if (r)
>   		return r;
>   
> @@ -1820,7 +1820,7 @@ static int gfx_v6_0_ring_test_ring(struct amdgpu_ring *ring)
>   		r = -ETIMEDOUT;
>   
>   error_free_scratch:
> -	amdgpu_gfx_scratch_free(adev, scratch);
> +	amdgpu_device_scratch_free(&adev->gfx.scratch, scratch);
>   	return r;
>   }
>   
> @@ -1909,7 +1909,7 @@ static int gfx_v6_0_ring_test_ib(struct amdgpu_ring *ring, long timeout)
>   	uint32_t tmp = 0;
>   	long r;
>   
> -	r = amdgpu_gfx_scratch_get(adev, &scratch);
> +	r = amdgpu_device_scratch_get(&adev->gfx.scratch, &scratch);
>   	if (r)
>   		return r;
>   
> @@ -1946,7 +1946,7 @@ static int gfx_v6_0_ring_test_ib(struct amdgpu_ring *ring, long timeout)
>   	amdgpu_ib_free(adev, &ib, NULL);
>   	dma_fence_put(f);
>   err1:
> -	amdgpu_gfx_scratch_free(adev, scratch);
> +	amdgpu_device_scratch_free(&adev->gfx.scratch, scratch);
>   	return r;
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> index ac3f2dbba726..b60e3bcba050 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> @@ -2087,7 +2087,7 @@ static int gfx_v7_0_ring_test_ring(struct amdgpu_ring *ring)
>   	unsigned i;
>   	int r;
>   
> -	r = amdgpu_gfx_scratch_get(adev, &scratch);
> +	r = amdgpu_device_scratch_get(&adev->gfx.scratch, &scratch);
>   	if (r)
>   		return r;
>   
> @@ -2111,7 +2111,7 @@ static int gfx_v7_0_ring_test_ring(struct amdgpu_ring *ring)
>   		r = -ETIMEDOUT;
>   
>   error_free_scratch:
> -	amdgpu_gfx_scratch_free(adev, scratch);
> +	amdgpu_device_scratch_free(&adev->gfx.scratch, scratch);
>   	return r;
>   }
>   
> @@ -2359,7 +2359,7 @@ static int gfx_v7_0_ring_test_ib(struct amdgpu_ring *ring, long timeout)
>   	uint32_t tmp = 0;
>   	long r;
>   
> -	r = amdgpu_gfx_scratch_get(adev, &scratch);
> +	r = amdgpu_device_scratch_get(&adev->gfx.scratch, &scratch);
>   	if (r)
>   		return r;
>   
> @@ -2396,7 +2396,7 @@ static int gfx_v7_0_ring_test_ib(struct amdgpu_ring *ring, long timeout)
>   	amdgpu_ib_free(adev, &ib, NULL);
>   	dma_fence_put(f);
>   err1:
> -	amdgpu_gfx_scratch_free(adev, scratch);
> +	amdgpu_device_scratch_free(&adev->gfx.scratch, scratch);
>   	return r;
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index e4e779a19c20..d268522b743c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -850,7 +850,7 @@ static int gfx_v8_0_ring_test_ring(struct amdgpu_ring *ring)
>   	unsigned i;
>   	int r;
>   
> -	r = amdgpu_gfx_scratch_get(adev, &scratch);
> +	r = amdgpu_device_scratch_get(&adev->gfx.scratch, &scratch);
>   	if (r)
>   		return r;
>   
> @@ -875,7 +875,7 @@ static int gfx_v8_0_ring_test_ring(struct amdgpu_ring *ring)
>   		r = -ETIMEDOUT;
>   
>   error_free_scratch:
> -	amdgpu_gfx_scratch_free(adev, scratch);
> +	amdgpu_device_scratch_free(&adev->gfx.scratch, scratch);
>   	return r;
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 06182b7e4351..a383808b566f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -999,7 +999,7 @@ static int gfx_v9_0_ring_test_ring(struct amdgpu_ring *ring)
>   	unsigned i;
>   	int r;
>   
> -	r = amdgpu_gfx_scratch_get(adev, &scratch);
> +	r = amdgpu_device_scratch_get(&adev->gfx.scratch, &scratch);
>   	if (r)
>   		return r;
>   
> @@ -1024,7 +1024,7 @@ static int gfx_v9_0_ring_test_ring(struct amdgpu_ring *ring)
>   		r = -ETIMEDOUT;
>   
>   error_free_scratch:
> -	amdgpu_gfx_scratch_free(adev, scratch);
> +	amdgpu_device_scratch_free(&adev->gfx.scratch, scratch);
>   	return r;
>   }
>