[PATCH v2 1/2] drm/amdgpu/jpeg: deduplicate jpeg_v3_0 process_interrupt

[Christian König]

On 7/1/26 20:22, Alex Deucher wrote:
> On Wed, Jul 1, 2026 at 2:14 PM Tiago Gomes Dourado de Oliveira
> <tiagodourado em usp.br> wrote:
>>
>> On Fri, Jun 26, 2026 at 6:49 PM Alex Deucher <alexdeucher em gmail.com> wrote:
>>>
>>> On Fri, Jun 26, 2026 at 8:14 AM Christian König
>>> <christian.koenig em amd.com> 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 <tiagodourado em usp.br>
>>>>> Co-developed-by: Luiz Fernandes <luiz.f.f.fernandes em usp.br>
>>>>> Signed-off-by: Luiz Fernandes <luiz.f.f.fernandes em usp.br>
>>>>> ---
>>>>>
>>>>> 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)
>>>>