Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] drm/amdgpu/jpeg: inaccurate commit message claim about other JPEG sw_init() --
commit 286291864ed14a6432cfeee63e75b0d8982f6065 Author: Wentao Liang <[email protected]> drm/amdgpu/jpeg: unwind the rings when sw init fails This commit updates jpeg_v4_0_5_sw_init() to unwind initialized JPEG resources via amdgpu_jpeg_suspend() and amdgpu_jpeg_sw_fini() when amdgpu_jpeg_reg_dump_init() or amdgpu_jpeg_sysfs_reset_mask_init() fails. > drm/amdgpu/jpeg: unwind the rings when sw init fails > > jpeg_v4_0_5_sw_init() returns directly when > amdgpu_jpeg_reg_dump_init() or amdgpu_jpeg_sysfs_reset_mask_init() > fails, after all JPEG rings have been initialized successfully. > amdgpu_ring_init() allocated the ring buffers, writeback slots and the > vmid fences, and only amdgpu_ring_fini(), called through > amdgpu_jpeg_sw_fini(), releases them. Because the IP block's status.sw is > only set after sw_init() succeeds, the upper layers skip sw_fini() for > this block and the resources are leaked. > > Unwind with amdgpu_jpeg_sw_fini(), as the other JPEG implementations do. > > Fixes: 2b0ccf3923b5 ("drm/amdgpu: Enable devcoredump for JPEG4_0_5") > Cc: [email protected] > Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex > Signed-off-by: Wentao Liang <[email protected]> [Severity: Low] This isn't a bug in the code, but is the statement that the other JPEG implementations unwind with amdgpu_jpeg_sw_fini() accurate? Looking at the other JPEG IP block implementations in drivers/gpu/drm/amd/amdgpu/ (jpeg_v1_0.c, jpeg_v2_0.c, jpeg_v2_5.c, jpeg_v3_0.c, jpeg_v4_0.c, jpeg_v4_0_3.c, jpeg_v5_0_0.c, jpeg_v5_0_1.c, jpeg_v5_0_2.c, and jpeg_v5_3_0.c), none of their sw_init callbacks call amdgpu_jpeg_sw_fini() or amdgpu_jpeg_suspend() on error. For example, in drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c, jpeg_v4_0_sw_init() returns r directly when amdgpu_jpeg_reg_dump_init() or amdgpu_jpeg_sysfs_reset_mask_init() fails: r = amdgpu_jpeg_reg_dump_init(adev, jpeg_reg_list_4_0, ARRAY_SIZE(jpeg_reg_list_4_0)); if (r) return r; adev->jpeg.supported_reset = amdgpu_get_soft_full_reset_mask(adev->jpeg.inst[0].ring_dec); if (!amdgpu_sriov_vf(adev)) adev->jpeg.supported_reset |= AMDGPU_RESET_TYPE_PER_QUEUE; r = amdgpu_jpeg_sysfs_reset_mask_init(adev); return r; Also, since the direct error return on amdgpu_jpeg_sysfs_reset_mask_init() in jpeg_v4_0_5_sw_init() in drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c was introduced earlier in commit 96f0b56c34d2 ("drm/amdgpu: add sysfs interfaces for jpeg reset mask"), should that commit be included in the Fixes tags as well? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
