On 7/1/26 20:22, Alex Deucher wrote: > On Wed, Jul 1, 2026 at 2:14 PM Tiago Gomes Dourado de Oliveira > <[email protected]> wrote: >> >> On Fri, Jun 26, 2026 at 6:49 PM Alex Deucher <[email protected]> wrote: >>> >>> On Fri, Jun 26, 2026 at 8:14 AM Christian König >>> <[email protected]> wrote: >>>> >>>> On 6/26/26 04:59, Tiago Dourado wrote: >>>>> The jpeg_v3_0_process_interrupt function is identical to >>>>> jpeg_v2_0_process_interrupt. Remove the duplicate implementation >>>>> in jpeg_v3_0 and assign the jpeg_v2_0 version directly to the irq_funcs >>>>> struct. >>>>> Export jpeg_v2_0_process_interrupt through jpeg_v2_0.h to allow >>>>> cross-version reuse. >>>> >>>> Usually we intentionally don't do any cross IP version reuse, that has >>>> cause tons of problems in the past. >>>> >>>> @Leo any particular reason why we don't have a separate SRCID file for >>>> VCN3? >>>> >>>> My educated guess is that nothing changed compared to VCN2 and nobody >>>> cared to re-generate the file from the HW definition. >>>> >>> >>> I think it's the same so no need for a separate one. There are a >>> number of existing places where VCN or jpeg use one function >>> implementation across multiple generations. >>> >> >> Thanks for the feedback. Given what both of you pointed out, what do >> you suggest that we can do? > > I've applied the series. Thanks.
I think we should add a comment to functions which are used for more than one IP generation. Just to make sure that we don't accidentially add modifications which are specific to one generation. Christian. > > Alex > >> >> Regards, >> Tiago >> >>> Alex >>> >>>> Regards, >>>> Christian. >>>> >>>>> >>>>> Signed-off-by: Tiago Dourado <[email protected]> >>>>> Co-developed-by: Luiz Fernandes <[email protected]> >>>>> Signed-off-by: Luiz Fernandes <[email protected]> >>>>> --- >>>>> >>>>> v2: >>>>> - Assigned function directly to irq_funcs instead of using a macro. >>>>> >>>>> drivers/gpu/drm/amd/amdgpu/jpeg_v2_0.c | 2 +- >>>>> drivers/gpu/drm/amd/amdgpu/jpeg_v2_0.h | 4 ++++ >>>>> drivers/gpu/drm/amd/amdgpu/jpeg_v3_0.c | 21 +-------------------- >>>>> 3 files changed, 6 insertions(+), 21 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v2_0.c >>>>> b/drivers/gpu/drm/amd/amdgpu/jpeg_v2_0.c >>>>> index 9fe8d10ab..9006fc57e 100644 >>>>> --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v2_0.c >>>>> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v2_0.c >>>>> @@ -747,7 +747,7 @@ static int jpeg_v2_0_set_interrupt_state(struct >>>>> amdgpu_device *adev, >>>>> return 0; >>>>> } >>>>> >>>>> -static int jpeg_v2_0_process_interrupt(struct amdgpu_device *adev, >>>>> +int jpeg_v2_0_process_interrupt(struct amdgpu_device *adev, >>>>> struct amdgpu_irq_src *source, >>>>> struct amdgpu_iv_entry *entry) >>>>> { >>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v2_0.h >>>>> b/drivers/gpu/drm/amd/amdgpu/jpeg_v2_0.h >>>>> index 654e43e83..4f400fb47 100644 >>>>> --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v2_0.h >>>>> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v2_0.h >>>>> @@ -58,6 +58,10 @@ void jpeg_v2_0_dec_ring_emit_vm_flush(struct >>>>> amdgpu_ring *ring, >>>>> void jpeg_v2_0_dec_ring_emit_wreg(struct amdgpu_ring *ring, uint32_t >>>>> reg, uint32_t val); >>>>> void jpeg_v2_0_dec_ring_nop(struct amdgpu_ring *ring, uint32_t count); >>>>> >>>>> +int jpeg_v2_0_process_interrupt(struct amdgpu_device *adev, >>>>> + struct amdgpu_irq_src *source, >>>>> + struct amdgpu_iv_entry *entry); >>>>> + >>>>> extern const struct amdgpu_ip_block_version jpeg_v2_0_ip_block; >>>>> >>>>> #endif /* __JPEG_V2_0_H__ */ >>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v3_0.c >>>>> b/drivers/gpu/drm/amd/amdgpu/jpeg_v3_0.c >>>>> index 98f5e0622..2f3a5a17e 100644 >>>>> --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v3_0.c >>>>> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v3_0.c >>>>> @@ -539,25 +539,6 @@ static int jpeg_v3_0_set_interrupt_state(struct >>>>> amdgpu_device *adev, >>>>> return 0; >>>>> } >>>>> >>>>> -static int jpeg_v3_0_process_interrupt(struct amdgpu_device *adev, >>>>> - struct amdgpu_irq_src *source, >>>>> - struct amdgpu_iv_entry *entry) >>>>> -{ >>>>> - DRM_DEBUG("IH: JPEG TRAP\n"); >>>>> - >>>>> - switch (entry->src_id) { >>>>> - case VCN_2_0__SRCID__JPEG_DECODE: >>>>> - amdgpu_fence_process(adev->jpeg.inst->ring_dec); >>>>> - break; >>>>> - default: >>>>> - DRM_ERROR("Unhandled interrupt: %d %d\n", >>>>> - entry->src_id, entry->src_data[0]); >>>>> - break; >>>>> - } >>>>> - >>>>> - return 0; >>>>> -} >>>>> - >>>>> static int jpeg_v3_0_ring_reset(struct amdgpu_ring *ring, >>>>> unsigned int vmid, >>>>> struct amdgpu_fence *timedout_fence) >>>>> @@ -629,7 +610,7 @@ static void jpeg_v3_0_set_dec_ring_funcs(struct >>>>> amdgpu_device *adev) >>>>> >>>>> static const struct amdgpu_irq_src_funcs jpeg_v3_0_irq_funcs = { >>>>> .set = jpeg_v3_0_set_interrupt_state, >>>>> - .process = jpeg_v3_0_process_interrupt, >>>>> + .process = jpeg_v2_0_process_interrupt, >>>>> }; >>>>> >>>>> static void jpeg_v3_0_set_irq_funcs(struct amdgpu_device *adev) >>>>
