[PATCH v3 1/6] drm/amdgpu: Per-instance init func for JPEG4_0_3

Sundararaju, Sathishkumar sasundar at amd.com
Thu Feb 13 07:37:29 UTC 2025


On 2/13/2025 12:16 PM, Lazar, Lijo wrote:
>
> On 2/13/2025 8:24 AM, Sathishkumar S wrote:
>> Add helper functions to handle per-instance and per-core
>> initialization and deinitialization in JPEG4_0_3.
>>
>> Signed-off-by: Sathishkumar S <sathishkumar.sundararaju at amd.com>
>> Acked-by: Christian König <christian.koenig at amd.com>
>> Reviewed-by: Leo Liu <leo.liu at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c | 190 ++++++++++++-----------
>>   1 file changed, 98 insertions(+), 92 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 2a97302a22d3..e355febd9b82 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c
>> @@ -525,6 +525,75 @@ static void jpeg_v4_0_3_enable_clock_gating(struct amdgpu_device *adev, int inst
>>   	WREG32_SOC15(JPEG, jpeg_inst, regJPEG_CGC_GATE, data);
>>   }
>>   
>> +static void jpeg_v4_0_3_start_inst(struct amdgpu_device *adev, int inst)
>> +{
>> +	int jpeg_inst = GET_INST(JPEG, inst);
>> +
>> +	WREG32_SOC15(JPEG, jpeg_inst, regUVD_PGFSM_CONFIG,
>> +		     1 << UVD_PGFSM_CONFIG__UVDJ_PWR_CONFIG__SHIFT);
>> +	SOC15_WAIT_ON_RREG(JPEG, jpeg_inst, regUVD_PGFSM_STATUS,
>> +			   UVD_PGFSM_STATUS__UVDJ_PWR_ON <<
>> +			   UVD_PGFSM_STATUS__UVDJ_PWR_STATUS__SHIFT,
>> +			   UVD_PGFSM_STATUS__UVDJ_PWR_STATUS_MASK);
>> +
>> +	/* disable anti hang mechanism */
>> +	WREG32_P(SOC15_REG_OFFSET(JPEG, jpeg_inst, regUVD_JPEG_POWER_STATUS),
>> +		 0, ~UVD_JPEG_POWER_STATUS__JPEG_POWER_STATUS_MASK);
>> +
>> +	/* JPEG disable CGC */
>> +	jpeg_v4_0_3_disable_clock_gating(adev, inst);
>> +
>> +	/* MJPEG global tiling registers */
>> +	WREG32_SOC15(JPEG, jpeg_inst, regJPEG_DEC_GFX8_ADDR_CONFIG,
>> +		     adev->gfx.config.gb_addr_config);
>> +	WREG32_SOC15(JPEG, jpeg_inst, regJPEG_DEC_GFX10_ADDR_CONFIG,
>> +		     adev->gfx.config.gb_addr_config);
>> +
>> +	/* enable JMI channel */
>> +	WREG32_P(SOC15_REG_OFFSET(JPEG, jpeg_inst, regUVD_JMI_CNTL), 0,
>> +		 ~UVD_JMI_CNTL__SOFT_RESET_MASK);
>> +}
>> +
>> +static void jpeg_v4_0_3_start_jrbc(struct amdgpu_ring *ring)
>> +{
>> +	struct amdgpu_device *adev = ring->adev;
>> +	int jpeg_inst = GET_INST(JPEG, ring->me);
>> +	int reg_offset = jpeg_v4_0_3_core_reg_offset(ring->pipe);
>> +
>> +	/* enable System Interrupt for JRBC */
>> +	WREG32_P(SOC15_REG_OFFSET(JPEG, jpeg_inst, regJPEG_SYS_INT_EN),
>> +		 JPEG_SYS_INT_EN__DJRBC0_MASK << ring->pipe,
>> +		 ~(JPEG_SYS_INT_EN__DJRBC0_MASK << ring->pipe));
>> +
>> +	WREG32_SOC15_OFFSET(JPEG, jpeg_inst,
>> +			    regUVD_JMI0_UVD_LMI_JRBC_RB_VMID,
>> +			    reg_offset, 0);
>> +	WREG32_SOC15_OFFSET(JPEG, jpeg_inst,
>> +			    regUVD_JRBC0_UVD_JRBC_RB_CNTL,
>> +			    reg_offset,
>> +			    (0x00000001L | 0x00000002L));
>> +	WREG32_SOC15_OFFSET(JPEG, jpeg_inst,
>> +			    regUVD_JMI0_UVD_LMI_JRBC_RB_64BIT_BAR_LOW,
>> +			    reg_offset, lower_32_bits(ring->gpu_addr));
>> +	WREG32_SOC15_OFFSET(JPEG, jpeg_inst,
>> +			    regUVD_JMI0_UVD_LMI_JRBC_RB_64BIT_BAR_HIGH,
>> +			    reg_offset, upper_32_bits(ring->gpu_addr));
>> +	WREG32_SOC15_OFFSET(JPEG, jpeg_inst,
>> +			    regUVD_JRBC0_UVD_JRBC_RB_RPTR,
>> +			    reg_offset, 0);
>> +	WREG32_SOC15_OFFSET(JPEG, jpeg_inst,
>> +			    regUVD_JRBC0_UVD_JRBC_RB_WPTR,
>> +			    reg_offset, 0);
>> +	WREG32_SOC15_OFFSET(JPEG, jpeg_inst,
>> +			    regUVD_JRBC0_UVD_JRBC_RB_CNTL,
>> +			    reg_offset, 0x00000002L);
>> +	WREG32_SOC15_OFFSET(JPEG, jpeg_inst,
>> +			    regUVD_JRBC0_UVD_JRBC_RB_SIZE,
>> +			    reg_offset, ring->ring_size / 4);
>> +	ring->wptr = RREG32_SOC15_OFFSET(JPEG, jpeg_inst, regUVD_JRBC0_UVD_JRBC_RB_WPTR,
>> +					 reg_offset);
>> +}
>> +
>>   /**
>>    * jpeg_v4_0_3_start - start JPEG block
>>    *
>> @@ -535,84 +604,42 @@ static void jpeg_v4_0_3_enable_clock_gating(struct amdgpu_device *adev, int inst
>>   static int jpeg_v4_0_3_start(struct amdgpu_device *adev)
>>   {
>>   	struct amdgpu_ring *ring;
>> -	int i, j, jpeg_inst;
>> +	int i, j;
>>   
>>   	for (i = 0; i < adev->jpeg.num_jpeg_inst; ++i) {
>> -		jpeg_inst = GET_INST(JPEG, i);
>> -
>> -		WREG32_SOC15(JPEG, jpeg_inst, regUVD_PGFSM_CONFIG,
>> -			     1 << UVD_PGFSM_CONFIG__UVDJ_PWR_CONFIG__SHIFT);
>> -		SOC15_WAIT_ON_RREG(
>> -			JPEG, jpeg_inst, regUVD_PGFSM_STATUS,
>> -			UVD_PGFSM_STATUS__UVDJ_PWR_ON
>> -				<< UVD_PGFSM_STATUS__UVDJ_PWR_STATUS__SHIFT,
>> -			UVD_PGFSM_STATUS__UVDJ_PWR_STATUS_MASK);
>> -
>> -		/* disable anti hang mechanism */
>> -		WREG32_P(SOC15_REG_OFFSET(JPEG, jpeg_inst,
>> -					  regUVD_JPEG_POWER_STATUS),
>> -			 0, ~UVD_JPEG_POWER_STATUS__JPEG_POWER_STATUS_MASK);
>> -
>> -		/* JPEG disable CGC */
>> -		jpeg_v4_0_3_disable_clock_gating(adev, i);
>> -
>> -		/* MJPEG global tiling registers */
>> -		WREG32_SOC15(JPEG, jpeg_inst, regJPEG_DEC_GFX8_ADDR_CONFIG,
>> -			     adev->gfx.config.gb_addr_config);
>> -		WREG32_SOC15(JPEG, jpeg_inst, regJPEG_DEC_GFX10_ADDR_CONFIG,
>> -			     adev->gfx.config.gb_addr_config);
>> -
>> -		/* enable JMI channel */
>> -		WREG32_P(SOC15_REG_OFFSET(JPEG, jpeg_inst, regUVD_JMI_CNTL), 0,
>> -			 ~UVD_JMI_CNTL__SOFT_RESET_MASK);
>> -
>> +		jpeg_v4_0_3_start_inst(adev, i);
>>   		for (j = 0; j < adev->jpeg.num_jpeg_rings; ++j) {
> It's better to move this inside the instance function. Instance takes
> care of all cores within the instance.
The separation in the helper functions was done to decouple core 
specific configs away from instance
specific configs to have the degree of freedom to control them 
independently without meddling with
each other, so having them done separately kind of helps to align better 
with that choice.

Regards,
Sathish
>
> Thanks,
> Lijo
>
>> -			int reg_offset = jpeg_v4_0_3_core_reg_offset(j);
>> -
>>   			ring = &adev->jpeg.inst[i].ring_dec[j];
>> -
>> -			/* enable System Interrupt for JRBC */
>> -			WREG32_P(SOC15_REG_OFFSET(JPEG, jpeg_inst,
>> -						  regJPEG_SYS_INT_EN),
>> -				 JPEG_SYS_INT_EN__DJRBC0_MASK << j,
>> -				 ~(JPEG_SYS_INT_EN__DJRBC0_MASK << j));
>> -
>> -			WREG32_SOC15_OFFSET(JPEG, jpeg_inst,
>> -					    regUVD_JMI0_UVD_LMI_JRBC_RB_VMID,
>> -					    reg_offset, 0);
>> -			WREG32_SOC15_OFFSET(JPEG, jpeg_inst,
>> -					    regUVD_JRBC0_UVD_JRBC_RB_CNTL,
>> -					    reg_offset,
>> -					    (0x00000001L | 0x00000002L));
>> -			WREG32_SOC15_OFFSET(
>> -				JPEG, jpeg_inst,
>> -				regUVD_JMI0_UVD_LMI_JRBC_RB_64BIT_BAR_LOW,
>> -				reg_offset, lower_32_bits(ring->gpu_addr));
>> -			WREG32_SOC15_OFFSET(
>> -				JPEG, jpeg_inst,
>> -				regUVD_JMI0_UVD_LMI_JRBC_RB_64BIT_BAR_HIGH,
>> -				reg_offset, upper_32_bits(ring->gpu_addr));
>> -			WREG32_SOC15_OFFSET(JPEG, jpeg_inst,
>> -					    regUVD_JRBC0_UVD_JRBC_RB_RPTR,
>> -					    reg_offset, 0);
>> -			WREG32_SOC15_OFFSET(JPEG, jpeg_inst,
>> -					    regUVD_JRBC0_UVD_JRBC_RB_WPTR,
>> -					    reg_offset, 0);
>> -			WREG32_SOC15_OFFSET(JPEG, jpeg_inst,
>> -					    regUVD_JRBC0_UVD_JRBC_RB_CNTL,
>> -					    reg_offset, 0x00000002L);
>> -			WREG32_SOC15_OFFSET(JPEG, jpeg_inst,
>> -					    regUVD_JRBC0_UVD_JRBC_RB_SIZE,
>> -					    reg_offset, ring->ring_size / 4);
>> -			ring->wptr = RREG32_SOC15_OFFSET(
>> -				JPEG, jpeg_inst, regUVD_JRBC0_UVD_JRBC_RB_WPTR,
>> -				reg_offset);
>> +			jpeg_v4_0_3_start_jrbc(ring);
>>   		}
>>   	}
>>   
>>   	return 0;
>>   }
>>   
>> +static void jpeg_v4_0_3_stop_inst(struct amdgpu_device *adev, int inst)
>> +{
>> +	int jpeg_inst = GET_INST(JPEG, inst);
>> +	/* reset JMI */
>> +	WREG32_P(SOC15_REG_OFFSET(JPEG, jpeg_inst, regUVD_JMI_CNTL),
>> +		 UVD_JMI_CNTL__SOFT_RESET_MASK,
>> +		 ~UVD_JMI_CNTL__SOFT_RESET_MASK);
>> +
>> +	jpeg_v4_0_3_enable_clock_gating(adev, inst);
>> +
>> +	/* enable anti hang mechanism */
>> +	WREG32_P(SOC15_REG_OFFSET(JPEG, jpeg_inst, regUVD_JPEG_POWER_STATUS),
>> +		 UVD_JPEG_POWER_STATUS__JPEG_POWER_STATUS_MASK,
>> +		 ~UVD_JPEG_POWER_STATUS__JPEG_POWER_STATUS_MASK);
>> +
>> +	WREG32_SOC15(JPEG, jpeg_inst, regUVD_PGFSM_CONFIG,
>> +		     2 << UVD_PGFSM_CONFIG__UVDJ_PWR_CONFIG__SHIFT);
>> +	SOC15_WAIT_ON_RREG
>> +		(JPEG, jpeg_inst, regUVD_PGFSM_STATUS,
>> +		 UVD_PGFSM_STATUS__UVDJ_PWR_OFF << UVD_PGFSM_STATUS__UVDJ_PWR_STATUS__SHIFT,
>> +		 UVD_PGFSM_STATUS__UVDJ_PWR_STATUS_MASK);
>> +}
>> +
>>   /**
>>    * jpeg_v4_0_3_stop - stop JPEG block
>>    *
>> @@ -622,31 +649,10 @@ static int jpeg_v4_0_3_start(struct amdgpu_device *adev)
>>    */
>>   static int jpeg_v4_0_3_stop(struct amdgpu_device *adev)
>>   {
>> -	int i, jpeg_inst;
>> +	int i;
>>   
>> -	for (i = 0; i < adev->jpeg.num_jpeg_inst; ++i) {
>> -		jpeg_inst = GET_INST(JPEG, i);
>> -		/* reset JMI */
>> -		WREG32_P(SOC15_REG_OFFSET(JPEG, jpeg_inst, regUVD_JMI_CNTL),
>> -			 UVD_JMI_CNTL__SOFT_RESET_MASK,
>> -			 ~UVD_JMI_CNTL__SOFT_RESET_MASK);
>> -
>> -		jpeg_v4_0_3_enable_clock_gating(adev, i);
>> -
>> -		/* enable anti hang mechanism */
>> -		WREG32_P(SOC15_REG_OFFSET(JPEG, jpeg_inst,
>> -					  regUVD_JPEG_POWER_STATUS),
>> -			 UVD_JPEG_POWER_STATUS__JPEG_POWER_STATUS_MASK,
>> -			 ~UVD_JPEG_POWER_STATUS__JPEG_POWER_STATUS_MASK);
>> -
>> -		WREG32_SOC15(JPEG, jpeg_inst, regUVD_PGFSM_CONFIG,
>> -			     2 << UVD_PGFSM_CONFIG__UVDJ_PWR_CONFIG__SHIFT);
>> -		SOC15_WAIT_ON_RREG(
>> -			JPEG, jpeg_inst, regUVD_PGFSM_STATUS,
>> -			UVD_PGFSM_STATUS__UVDJ_PWR_OFF
>> -				<< UVD_PGFSM_STATUS__UVDJ_PWR_STATUS__SHIFT,
>> -			UVD_PGFSM_STATUS__UVDJ_PWR_STATUS_MASK);
>> -	}
>> +	for (i = 0; i < adev->jpeg.num_jpeg_inst; ++i)
>> +		jpeg_v4_0_3_stop_inst(adev, i);
>>   
>>   	return 0;
>>   }



More information about the amd-gfx mailing list