Re: [PATCH v2 1/9] drm/amdgpu: Add a func for core specific reg offset

[Leo Liu <leo.liu@amd.com>]

This patch is:

Reviewed-by: Leo Liu <leo.liu@amd.com>

On 1/29/25 03:46, Sathishkumar S wrote:
> Add an inline function to calculate core specific register offsets for
> JPEG v4.0.3 and reuse it, makes code more readable and easier to align.
>
> Signed-off-by: Sathishkumar S <sathishkumar.sundararaju@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c | 54 +++++++++++-------------
>   1 file changed, 24 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c
> index 9459e8cc7413..bc21f12daea8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c
> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c
> @@ -64,6 +64,14 @@ static inline bool jpeg_v4_0_3_normalizn_reqd(struct amdgpu_device *adev)
>   	return (adev->jpeg.caps & AMDGPU_JPEG_CAPS(RRMT_ENABLED)) == 0;
>   }
>   
> +static inline int jpeg_v4_0_3_core_reg_offset(u32 pipe)
> +{
> +	if (pipe)
> +		return ((0x40 * pipe) - 0xc80);
> +	else
> +		return 0;
> +}
> +
>   /**
>    * jpeg_v4_0_3_early_init - set function pointers
>    *
> @@ -143,10 +151,8 @@ static int jpeg_v4_0_3_sw_init(struct amdgpu_ip_block *ip_block)
>   			adev->jpeg.internal.jpeg_pitch[j] =
>   				regUVD_JRBC0_UVD_JRBC_SCRATCH0_INTERNAL_OFFSET;
>   			adev->jpeg.inst[i].external.jpeg_pitch[j] =
> -				SOC15_REG_OFFSET1(
> -					JPEG, jpeg_inst,
> -					regUVD_JRBC0_UVD_JRBC_SCRATCH0,
> -					(j ? (0x40 * j - 0xc80) : 0));
> +				SOC15_REG_OFFSET1(JPEG, jpeg_inst, regUVD_JRBC0_UVD_JRBC_SCRATCH0,
> +						  jpeg_v4_0_3_core_reg_offset(j));
>   		}
>   	}
>   
> @@ -521,7 +527,7 @@ static int jpeg_v4_0_3_start(struct amdgpu_device *adev)
>   			 ~UVD_JMI_CNTL__SOFT_RESET_MASK);
>   
>   		for (j = 0; j < adev->jpeg.num_jpeg_rings; ++j) {
> -			unsigned int reg_offset = (j?(0x40 * j - 0xc80):0);
> +			int reg_offset = jpeg_v4_0_3_core_reg_offset(j);
>   
>   			ring = &adev->jpeg.inst[i].ring_dec[j];
>   
> @@ -616,9 +622,8 @@ static uint64_t jpeg_v4_0_3_dec_ring_get_rptr(struct amdgpu_ring *ring)
>   {
>   	struct amdgpu_device *adev = ring->adev;
>   
> -	return RREG32_SOC15_OFFSET(
> -		JPEG, GET_INST(JPEG, ring->me), regUVD_JRBC0_UVD_JRBC_RB_RPTR,
> -		ring->pipe ? (0x40 * ring->pipe - 0xc80) : 0);
> +	return RREG32_SOC15_OFFSET(JPEG, GET_INST(JPEG, ring->me), regUVD_JRBC0_UVD_JRBC_RB_RPTR,
> +				   jpeg_v4_0_3_core_reg_offset(ring->pipe));
>   }
>   
>   /**
> @@ -634,11 +639,9 @@ static uint64_t jpeg_v4_0_3_dec_ring_get_wptr(struct amdgpu_ring *ring)
>   
>   	if (ring->use_doorbell)
>   		return adev->wb.wb[ring->wptr_offs];
> -	else
> -		return RREG32_SOC15_OFFSET(
> -			JPEG, GET_INST(JPEG, ring->me),
> -			regUVD_JRBC0_UVD_JRBC_RB_WPTR,
> -			ring->pipe ? (0x40 * ring->pipe - 0xc80) : 0);
> +
> +	return RREG32_SOC15_OFFSET(JPEG, GET_INST(JPEG, ring->me), regUVD_JRBC0_UVD_JRBC_RB_WPTR,
> +				   jpeg_v4_0_3_core_reg_offset(ring->pipe));
>   }
>   
>   static void jpeg_v4_0_3_ring_emit_hdp_flush(struct amdgpu_ring *ring)
> @@ -663,10 +666,8 @@ static void jpeg_v4_0_3_dec_ring_set_wptr(struct amdgpu_ring *ring)
>   		adev->wb.wb[ring->wptr_offs] = lower_32_bits(ring->wptr);
>   		WDOORBELL32(ring->doorbell_index, lower_32_bits(ring->wptr));
>   	} else {
> -		WREG32_SOC15_OFFSET(JPEG, GET_INST(JPEG, ring->me),
> -				    regUVD_JRBC0_UVD_JRBC_RB_WPTR,
> -				    (ring->pipe ? (0x40 * ring->pipe - 0xc80) :
> -						  0),
> +		WREG32_SOC15_OFFSET(JPEG, GET_INST(JPEG, ring->me), regUVD_JRBC0_UVD_JRBC_RB_WPTR,
> +				    jpeg_v4_0_3_core_reg_offset(ring->pipe),
>   				    lower_32_bits(ring->wptr));
>   	}
>   }
> @@ -919,13 +920,9 @@ static bool jpeg_v4_0_3_is_idle(void *handle)
>   
>   	for (i = 0; i < adev->jpeg.num_jpeg_inst; ++i) {
>   		for (j = 0; j < adev->jpeg.num_jpeg_rings; ++j) {
> -			unsigned int reg_offset = (j?(0x40 * j - 0xc80):0);
> -
> -			ret &= ((RREG32_SOC15_OFFSET(
> -					 JPEG, GET_INST(JPEG, i),
> -					 regUVD_JRBC0_UVD_JRBC_STATUS,
> -					 reg_offset) &
> -				 UVD_JRBC0_UVD_JRBC_STATUS__RB_JOB_DONE_MASK) ==
> +			ret &= ((RREG32_SOC15_OFFSET(JPEG, GET_INST(JPEG, i),
> +				regUVD_JRBC0_UVD_JRBC_STATUS, jpeg_v4_0_3_core_reg_offset(j)) &
> +				UVD_JRBC0_UVD_JRBC_STATUS__RB_JOB_DONE_MASK) ==
>   				UVD_JRBC0_UVD_JRBC_STATUS__RB_JOB_DONE_MASK);
>   		}
>   	}
> @@ -941,13 +938,10 @@ static int jpeg_v4_0_3_wait_for_idle(struct amdgpu_ip_block *ip_block)
>   
>   	for (i = 0; i < adev->jpeg.num_jpeg_inst; ++i) {
>   		for (j = 0; j < adev->jpeg.num_jpeg_rings; ++j) {
> -			unsigned int reg_offset = (j?(0x40 * j - 0xc80):0);
> -
> -			ret &= SOC15_WAIT_ON_RREG_OFFSET(
> -				JPEG, GET_INST(JPEG, i),
> -				regUVD_JRBC0_UVD_JRBC_STATUS, reg_offset,
> +			ret &= (SOC15_WAIT_ON_RREG_OFFSET(JPEG, GET_INST(JPEG, i),
> +				regUVD_JRBC0_UVD_JRBC_STATUS, jpeg_v4_0_3_core_reg_offset(j),
>   				UVD_JRBC0_UVD_JRBC_STATUS__RB_JOB_DONE_MASK,
> -				UVD_JRBC0_UVD_JRBC_STATUS__RB_JOB_DONE_MASK);
> +				UVD_JRBC0_UVD_JRBC_STATUS__RB_JOB_DONE_MASK));
>   		}
>   	}
>   	return ret;