[PATCH 3/9] drm/amdgpu: Enable devcoredump for JPEG4_0_3
Sundararaju, Sathishkumar
sasundar at amd.com
Tue Jan 28 10:24:59 UTC 2025
Hi Lijo,
On 1/28/2025 3:16 PM, Lazar, Lijo wrote:
>
> 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 at amd.com>
>> ---
>> 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);
okay, got it, will rework this.
>
>
> 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
Right, this can use the reg_list to dump/print and can get rid of
version specific dump/print callbacks.
would prefer to maintain the *reg_list in adev->jpeg and have it
initialized in amdgpu_jpeg_reg_dump_init() as suggested. But regarding
the other suggestion, w.r.t to per instance dump/print, there are some
limitations, which I will try to explain in the other patch. Brought it
up here as there can be relevance here based on that decision. Regards,
Sathish
>
> 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 = {
More information about the amd-gfx
mailing list