[PATCH v3 1/6] drm/amdgpu: Per-instance init func for JPEG4_0_3
Sundararaju, Sathishkumar
sasundar at amd.com
Thu Feb 13 09:15:40 UTC 2025
On 2/13/2025 1:35 PM, Lazar, Lijo wrote:
>
> 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 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.
>>
> Functionally, is an instance considered started even if its cores are
> not initialized?
was that a question ? _start() does it.
The current design aligns better for the initialization and also future
work planned, like per core reset
and full instance recovery, and this separation is a must for that, I do
not want them together as it
hinders an axis of freedom in configuration and recovery process with
validation at every step.
Regards,
Sathish
> 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;
>>>> }
More information about the amd-gfx
mailing list