On 2/13/2025 1:07 PM, Sundararaju, Sathishkumar wrote:
>
> 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@xxxxxxx>
>>> Acked-by: Christian König <christian.koenig@xxxxxxx>
>>> Reviewed-by: Leo Liu <leo.liu@xxxxxxx>
>>> ---
>>> 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.
>
Functionally, is an instance considered started even if its cores are
not initialized?
Thanks,
Lijo
> 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;
>>> }
>
