Re: [PATCH v2 9/9] drm/amdgpu: Enable devcoredump for JPEG2_5_0

["Sundararaju, Sathishkumar" <sasundar@xxxxxxx>]

Hi Leo,

Oh! I somehow missed it, I will enable it in 5_0_0 as well.
Initially I had some confusion around 4_0_5 and 5_0_0 as they both are 
in dpg mode, but after checking the existing register access I enabled 
it in 4_0_5, but missed to enable it in 5_0_0.

Regards,
Sathish

On 1/29/2025 10:06 PM, Leo Liu wrote:
> Looks pretty good to me. One question that why this is not added to 
> JPEG 5_0_0? Since the patch 3-9 cover the rest of versions, they are:
>
> Reviewed-by: Leo Liu <leo.liu@xxxxxxx>
>
> On 1/29/25 03:46, Sathishkumar S wrote:
> > Add register list and enable devcoredump for JPEG2_5_0
> >
> > V2: (Lijo)
> > - remove version specific callbacks and use simplified helper functions
> >
> > Signed-off-by: Sathishkumar S <sathishkumar.sundararaju@xxxxxxx>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/jpeg_v2_5.c | 26 ++++++++++++++++++++++++++
> >   1 file changed, 26 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v2_5.c 
> > b/drivers/gpu/drm/amd/amdgpu/jpeg_v2_5.c
> > index 11f6af2646e7..536a6101dcf9 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v2_5.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v2_5.c
> > @@ -36,6 +36,22 @@
> >     #define JPEG25_MAX_HW_INSTANCES_ARCTURUS            2
> >   +static const struct amdgpu_hwip_reg_entry jpeg_reg_list_2_5[] = {
> > +    SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_POWER_STATUS),
> > +    SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_INT_STAT),
> > +    SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_RPTR),
> > +    SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_WPTR),
> > +    SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_CNTL),
> > +    SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_SIZE),
> > +    SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_STATUS),
> > +    SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_ADDR_MODE),
> > +    SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_GFX10_ADDR_CONFIG),
> > +    SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_Y_GFX10_TILING_SURFACE),
> > +    SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_UV_GFX10_TILING_SURFACE),
> > +    SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_PITCH),
> > +    SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_UV_PITCH),
> > +};
> > +
> >   static void jpeg_v2_5_set_dec_ring_funcs(struct amdgpu_device *adev);
> >   static void jpeg_v2_5_set_irq_funcs(struct amdgpu_device *adev);
> >   static int jpeg_v2_5_set_powergating_state(struct amdgpu_ip_block 
> > *ip_block,
> > @@ -147,6 +163,10 @@ static int jpeg_v2_5_sw_init(struct 
> > amdgpu_ip_block *ip_block)
> >       if (r)
> >           return r;
> >   +    r = amdgpu_jpeg_reg_dump_init(adev, jpeg_reg_list_2_5, 
> > ARRAY_SIZE(jpeg_reg_list_2_5));
> > +    if (r)
> > +        return r;
> > +
> >       return 0;
> >   }
> >   @@ -168,6 +188,8 @@ static int jpeg_v2_5_sw_fini(struct 
> > amdgpu_ip_block *ip_block)
> >         r = amdgpu_jpeg_sw_fini(adev);
> >   +    amdgpu_jpeg_reg_dump_fini(adev);
> > +
> >       return r;
> >   }
> >   @@ -623,6 +645,8 @@ static const struct amd_ip_funcs 
> > jpeg_v2_5_ip_funcs = {
> >       .wait_for_idle = jpeg_v2_5_wait_for_idle,
> >       .set_clockgating_state = jpeg_v2_5_set_clockgating_state,
> >       .set_powergating_state = jpeg_v2_5_set_powergating_state,
> > +    .dump_ip_state = amdgpu_jpeg_dump_ip_state,
> > +    .print_ip_state = amdgpu_jpeg_print_ip_state,
> >   };
> >     static const struct amd_ip_funcs jpeg_v2_6_ip_funcs = {
> > @@ -638,6 +662,8 @@ static const struct amd_ip_funcs 
> > jpeg_v2_6_ip_funcs = {
> >       .wait_for_idle = jpeg_v2_5_wait_for_idle,
> >       .set_clockgating_state = jpeg_v2_5_set_clockgating_state,
> >       .set_powergating_state = jpeg_v2_5_set_powergating_state,
> > +    .dump_ip_state = amdgpu_jpeg_dump_ip_state,
> > +    .print_ip_state = amdgpu_jpeg_print_ip_state,
> >   };
> >     static const struct amdgpu_ring_funcs jpeg_v2_5_dec_ring_vm_funcs 
> > = {