On 1/28/2025 2:39 PM, Sathishkumar S wrote: > Add register list and enable devcoredump for JPEG4_0_3 > > Signed-off-by: Sathishkumar S <sathishkumar.sundararaju@xxxxxxx> > --- > drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c | 58 ++++++++++++++++++++++++ > 1 file changed, 58 insertions(+) > > 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 bc21f12daea8..39fd678dd874 100644 > --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c > +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c > @@ -47,6 +47,8 @@ static int jpeg_v4_0_3_set_powergating_state(struct amdgpu_ip_block *ip_block, > enum amd_powergating_state state); > static void jpeg_v4_0_3_set_ras_funcs(struct amdgpu_device *adev); > static void jpeg_v4_0_3_dec_ring_set_wptr(struct amdgpu_ring *ring); > +static void jpeg_v4_0_3_dump_ip_state(struct amdgpu_ip_block *ip_block); > +static void jpeg_v4_0_3_print_ip_state(struct amdgpu_ip_block *ip_block, struct drm_printer *p); > > static int amdgpu_ih_srcid_jpeg[] = { > VCN_4_0__SRCID__JPEG_DECODE, > @@ -59,6 +61,42 @@ static int amdgpu_ih_srcid_jpeg[] = { > VCN_4_0__SRCID__JPEG7_DECODE > }; > > +static const struct amdgpu_hwip_reg_entry jpeg_reg_list_4_0_3[] = { > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JPEG_POWER_STATUS), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JPEG_INT_STAT), > + SOC15_REG_ENTRY_STR(VCN, 0, regJPEG_SYS_INT_STATUS), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC0_UVD_JRBC_RB_RPTR), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC0_UVD_JRBC_RB_WPTR), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC0_UVD_JRBC_STATUS), > + SOC15_REG_ENTRY_STR(VCN, 0, regJPEG_DEC_ADDR_MODE), > + SOC15_REG_ENTRY_STR(VCN, 0, regJPEG_DEC_GFX10_ADDR_CONFIG), > + SOC15_REG_ENTRY_STR(VCN, 0, regJPEG_DEC_Y_GFX10_TILING_SURFACE), > + SOC15_REG_ENTRY_STR(VCN, 0, regJPEG_DEC_UV_GFX10_TILING_SURFACE), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JPEG_PITCH), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JPEG_UV_PITCH), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC1_UVD_JRBC_RB_RPTR), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC1_UVD_JRBC_RB_WPTR), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC1_UVD_JRBC_STATUS), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC2_UVD_JRBC_RB_RPTR), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC2_UVD_JRBC_RB_WPTR), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC2_UVD_JRBC_STATUS), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC3_UVD_JRBC_RB_RPTR), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC3_UVD_JRBC_RB_WPTR), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC3_UVD_JRBC_STATUS), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC4_UVD_JRBC_RB_RPTR), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC4_UVD_JRBC_RB_WPTR), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC4_UVD_JRBC_STATUS), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC5_UVD_JRBC_RB_RPTR), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC5_UVD_JRBC_RB_WPTR), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC5_UVD_JRBC_STATUS), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC6_UVD_JRBC_RB_RPTR), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC6_UVD_JRBC_RB_WPTR), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC6_UVD_JRBC_STATUS), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC7_UVD_JRBC_RB_RPTR), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC7_UVD_JRBC_RB_WPTR), > + SOC15_REG_ENTRY_STR(VCN, 0, regUVD_JRBC7_UVD_JRBC_STATUS), > +}; > + > static inline bool jpeg_v4_0_3_normalizn_reqd(struct amdgpu_device *adev) > { > return (adev->jpeg.caps & AMDGPU_JPEG_CAPS(RRMT_ENABLED)) == 0; > @@ -164,6 +202,11 @@ static int jpeg_v4_0_3_sw_init(struct amdgpu_ip_block *ip_block) > } > } > > + adev->jpeg.ip_dump = kcalloc(adev->jpeg.num_jpeg_inst * ARRAY_SIZE(jpeg_reg_list_4_0_3), > + sizeof(uint32_t), GFP_KERNEL); Probably, this can be simplified and kept under common logic amdgpu_jpeg_reg_dump { struct amdgpu_hwip_reg_entry *reg_list; int num_regs; }; amdgpu_jpeg_reg_dump_init(adev, jpeg_reg_list_4_0_3, ARRAY_SIZE(jpeg_reg_list_4_0_3)); amdgpu_jpeg_reg_dump_fini(adev); In that case, amdgpu_jpeg_dump_ip_state/amdgpu_jpeg_print_ip_state will be good enough instead of keeping jpeg_<ver>_dump/print_ip_state Thanks, Lijo > + if (!adev->jpeg.ip_dump) > + DRM_ERROR("Failed to allocate memory for JPEG IP Dump\n"); > + > /* TODO: Add queue reset mask when FW fully supports it */ > adev->jpeg.supported_reset = > amdgpu_get_soft_full_reset_mask(&adev->jpeg.inst[0].ring_dec[0]); > @@ -193,6 +236,8 @@ static int jpeg_v4_0_3_sw_fini(struct amdgpu_ip_block *ip_block) > amdgpu_jpeg_sysfs_reset_mask_fini(adev); > r = amdgpu_jpeg_sw_fini(adev); > > + kfree(adev->jpeg.ip_dump); > + > return r; > } > > @@ -1053,6 +1098,17 @@ static int jpeg_v4_0_3_process_interrupt(struct amdgpu_device *adev, > return 0; > } > > +static void jpeg_v4_0_3_dump_ip_state(struct amdgpu_ip_block *ip_block) > +{ > + amdgpu_jpeg_dump_ip_state(ip_block, jpeg_reg_list_4_0_3, ARRAY_SIZE(jpeg_reg_list_4_0_3)); > +} > + > +static void jpeg_v4_0_3_print_ip_state(struct amdgpu_ip_block *ip_block, struct drm_printer *p) > +{ > + amdgpu_jpeg_print_ip_state(ip_block, p, jpeg_reg_list_4_0_3, > + ARRAY_SIZE(jpeg_reg_list_4_0_3)); > +} > + > static const struct amd_ip_funcs jpeg_v4_0_3_ip_funcs = { > .name = "jpeg_v4_0_3", > .early_init = jpeg_v4_0_3_early_init, > @@ -1066,6 +1122,8 @@ static const struct amd_ip_funcs jpeg_v4_0_3_ip_funcs = { > .wait_for_idle = jpeg_v4_0_3_wait_for_idle, > .set_clockgating_state = jpeg_v4_0_3_set_clockgating_state, > .set_powergating_state = jpeg_v4_0_3_set_powergating_state, > + .dump_ip_state = jpeg_v4_0_3_dump_ip_state, > + .print_ip_state = jpeg_v4_0_3_print_ip_state, > }; > > static const struct amdgpu_ring_funcs jpeg_v4_0_3_dec_ring_vm_funcs = {