dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V1 RESEND] drm/amdgpu: deduplicate JPEG v5.0 interrupt routine
@ 2026-05-09  2:03 Andre Luiz Batista Bueno
  2026-05-11 14:55 ` Alex Deucher
  0 siblings, 1 reply; 8+ messages in thread
From: Andre Luiz Batista Bueno @ 2026-05-09  2:03 UTC (permalink / raw)
  To: amd-gfx, alexander.deucher, christian.koenig, airlied, simona
  Cc: enzo.spinella, Andre Luiz Batista Bueno, dri-devel

Both jpeg_v5_0_1.c and jpeg_v5_0_2.c implement identical
interrupt processing routines. To avoid code duplication,
extract this implementation into a shared common function
in new jpeg_v5_0_interrupt.c and .h files.

Signed-off-by: Andre Luiz Batista Bueno <andrebueno.mac@gmail.com>
Co-developed-by: Enzo Furegatti Spinella <enzo.spinella@usp.br>
Signed-off-by: Enzo Furegatti Spinella <enzo.spinella@usp.br>
---
 drivers/gpu/drm/amd/amdgpu/Makefile           |  1 +
 drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c      | 57 +----------------
 drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c      | 57 +----------------
 .../gpu/drm/amd/amdgpu/jpeg_v5_0_interrupt.c  | 64 +++++++++++++++++++
 .../gpu/drm/amd/amdgpu/jpeg_v5_0_interrupt.h  | 12 ++++
 5 files changed, 83 insertions(+), 108 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_interrupt.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_interrupt.h

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
index db66c6372199..69e5e55e1a2b 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -229,6 +229,7 @@ amdgpu-y += \
 	jpeg_v5_0_0.o \
 	jpeg_v5_0_1.o \
 	jpeg_v5_0_2.o \
+	jpeg_v5_0_interrupt.o \
 	jpeg_v5_3_0.o
 
 # add VPE block
diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
index edecbfe66c79..6e5611921eda 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
@@ -34,6 +34,8 @@
 #include "vcn/vcn_5_0_0_sh_mask.h"
 #include "ivsrcid/vcn/irqsrcs_vcn_5_0.h"
 
+#include "jpeg_v5_0_interrupt.h"
+
 static int jpeg_v5_0_1_start_sriov(struct amdgpu_device *adev);
 static void jpeg_v5_0_1_set_dec_ring_funcs(struct amdgpu_device *adev);
 static void jpeg_v5_0_1_set_irq_funcs(struct amdgpu_device *adev);
@@ -759,60 +761,7 @@ static int jpeg_v5_0_1_process_interrupt(struct amdgpu_device *adev,
 					 struct amdgpu_irq_src *source,
 					 struct amdgpu_iv_entry *entry)
 {
-	u32 i, inst;
-
-	i = node_id_to_phys_map[entry->node_id];
-	DRM_DEV_DEBUG(adev->dev, "IH: JPEG TRAP\n");
-
-	for (inst = 0; inst < adev->jpeg.num_jpeg_inst; ++inst)
-		if (adev->jpeg.inst[inst].aid_id == i)
-			break;
-
-	if (inst >= adev->jpeg.num_jpeg_inst) {
-		dev_WARN_ONCE(adev->dev, 1,
-			      "Interrupt received for unknown JPEG instance %d",
-			      entry->node_id);
-		return 0;
-	}
-
-	switch (entry->src_id) {
-	case VCN_5_0__SRCID__JPEG_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[0]);
-		break;
-	case VCN_5_0__SRCID__JPEG1_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[1]);
-		break;
-	case VCN_5_0__SRCID__JPEG2_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[2]);
-		break;
-	case VCN_5_0__SRCID__JPEG3_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[3]);
-		break;
-	case VCN_5_0__SRCID__JPEG4_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[4]);
-		break;
-	case VCN_5_0__SRCID__JPEG5_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[5]);
-		break;
-	case VCN_5_0__SRCID__JPEG6_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[6]);
-		break;
-	case VCN_5_0__SRCID__JPEG7_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[7]);
-		break;
-	case VCN_5_0__SRCID__JPEG8_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[8]);
-		break;
-	case VCN_5_0__SRCID__JPEG9_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[9]);
-		break;
-	default:
-		DRM_DEV_ERROR(adev->dev, "Unhandled interrupt: %d %d\n",
-			      entry->src_id, entry->src_data[0]);
-		break;
-	}
-
-	return 0;
+	return jpeg_v5_0_process_interrupt_common(adev, entry);
 }
 
 static void jpeg_v5_0_1_core_stall_reset(struct amdgpu_ring *ring)
diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c
index 285c459379c4..daea95907639 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c
@@ -34,6 +34,8 @@
 #include "vcn/vcn_5_0_0_sh_mask.h"
 #include "ivsrcid/vcn/irqsrcs_vcn_5_0.h"
 
+#include "jpeg_v5_0_interrupt.h"
+
 static void jpeg_v5_0_2_set_dec_ring_funcs(struct amdgpu_device *adev);
 static void jpeg_v5_0_2_set_irq_funcs(struct amdgpu_device *adev);
 static int jpeg_v5_0_2_set_powergating_state(struct amdgpu_ip_block *ip_block,
@@ -587,60 +589,7 @@ static int jpeg_v5_0_2_process_interrupt(struct amdgpu_device *adev,
 					 struct amdgpu_irq_src *source,
 					 struct amdgpu_iv_entry *entry)
 {
-	u32 i, inst;
-
-	i = node_id_to_phys_map[entry->node_id];
-	DRM_DEV_DEBUG(adev->dev, "IH: JPEG TRAP\n");
-
-	for (inst = 0; inst < adev->jpeg.num_jpeg_inst; ++inst)
-		if (adev->jpeg.inst[inst].aid_id == i)
-			break;
-
-	if (inst >= adev->jpeg.num_jpeg_inst) {
-		dev_WARN_ONCE(adev->dev, 1,
-			      "Interrupt received for unknown JPEG instance %d",
-			      entry->node_id);
-		return 0;
-	}
-
-	switch (entry->src_id) {
-	case VCN_5_0__SRCID__JPEG_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[0]);
-		break;
-	case VCN_5_0__SRCID__JPEG1_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[1]);
-		break;
-	case VCN_5_0__SRCID__JPEG2_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[2]);
-		break;
-	case VCN_5_0__SRCID__JPEG3_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[3]);
-		break;
-	case VCN_5_0__SRCID__JPEG4_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[4]);
-		break;
-	case VCN_5_0__SRCID__JPEG5_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[5]);
-		break;
-	case VCN_5_0__SRCID__JPEG6_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[6]);
-		break;
-	case VCN_5_0__SRCID__JPEG7_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[7]);
-		break;
-	case VCN_5_0__SRCID__JPEG8_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[8]);
-		break;
-	case VCN_5_0__SRCID__JPEG9_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[9]);
-		break;
-	default:
-		DRM_DEV_ERROR(adev->dev, "Unhandled interrupt: %d %d\n",
-			      entry->src_id, entry->src_data[0]);
-		break;
-	}
-
-	return 0;
+	return jpeg_v5_0_process_interrupt_common(adev, entry);
 }
 
 static void jpeg_v5_0_2_core_stall_reset(struct amdgpu_ring *ring)
diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_interrupt.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_interrupt.c
new file mode 100644
index 000000000000..a76ee6586b63
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_interrupt.c
@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+
+#include "amdgpu.h"
+#include "ivsrcid/vcn/irqsrcs_vcn_5_0.h"
+#include "jpeg_v5_0_interrupt.h"
+
+int jpeg_v5_0_process_interrupt_common(struct amdgpu_device *adev,
+				       struct amdgpu_iv_entry *entry)
+{
+	u32 i, inst;
+
+	i = node_id_to_phys_map[entry->node_id];
+	DRM_DEV_DEBUG(adev->dev, "IH: JPEG TRAP\n");
+
+	for (inst = 0; inst < adev->jpeg.num_jpeg_inst; ++inst)
+		if (adev->jpeg.inst[inst].aid_id == i)
+			break;
+
+	if (inst >= adev->jpeg.num_jpeg_inst) {
+		dev_WARN_ONCE(adev->dev, 1,
+			      "Interrupt received for unknown JPEG instance %d",
+			      entry->node_id);
+		return 0;
+	}
+
+	switch (entry->src_id) {
+	case VCN_5_0__SRCID__JPEG_DECODE:
+		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[0]);
+		break;
+	case VCN_5_0__SRCID__JPEG1_DECODE:
+		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[1]);
+		break;
+	case VCN_5_0__SRCID__JPEG2_DECODE:
+		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[2]);
+		break;
+	case VCN_5_0__SRCID__JPEG3_DECODE:
+		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[3]);
+		break;
+	case VCN_5_0__SRCID__JPEG4_DECODE:
+		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[4]);
+		break;
+	case VCN_5_0__SRCID__JPEG5_DECODE:
+		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[5]);
+		break;
+	case VCN_5_0__SRCID__JPEG6_DECODE:
+		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[6]);
+		break;
+	case VCN_5_0__SRCID__JPEG7_DECODE:
+		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[7]);
+		break;
+	case VCN_5_0__SRCID__JPEG8_DECODE:
+		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[8]);
+		break;
+	case VCN_5_0__SRCID__JPEG9_DECODE:
+		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[9]);
+		break;
+	default:
+		DRM_DEV_ERROR(adev->dev, "Unhandled interrupt: %d %d\n",
+			      entry->src_id, entry->src_data[0]);
+		break;
+	}
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_interrupt.h b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_interrupt.h
new file mode 100644
index 000000000000..046bcb4a3906
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_interrupt.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 OR MIT */
+
+#ifndef __JPEG_V5_0_INTERRUPT_H__
+#define __JPEG_V5_0_INTERRUPT_H__
+
+struct amdgpu_device;
+struct amdgpu_iv_entry;
+
+int jpeg_v5_0_process_interrupt_common(struct amdgpu_device *adev,
+				       struct amdgpu_iv_entry *entry);
+
+#endif /* __JPEG_V5_0_INTERRUPT_H__ */
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH V1 RESEND] drm/amdgpu: deduplicate JPEG v5.0 interrupt routine
  2026-05-09  2:03 [PATCH V1 RESEND] drm/amdgpu: deduplicate JPEG v5.0 interrupt routine Andre Luiz Batista Bueno
@ 2026-05-11 14:55 ` Alex Deucher
  2026-06-11  2:40   ` [PATCH v2] " Andre Luiz Batista Bueno
  2026-06-24  3:30   ` [PATCH V2 RESEND] " Andre Luiz Batista Bueno
  0 siblings, 2 replies; 8+ messages in thread
From: Alex Deucher @ 2026-05-11 14:55 UTC (permalink / raw)
  To: Andre Luiz Batista Bueno
  Cc: amd-gfx, alexander.deucher, christian.koenig, airlied, simona,
	enzo.spinella, dri-devel

On Sun, May 10, 2026 at 6:14 AM Andre Luiz Batista Bueno
<andrebueno.mac@gmail.com> wrote:
>
> Both jpeg_v5_0_1.c and jpeg_v5_0_2.c implement identical
> interrupt processing routines. To avoid code duplication,
> extract this implementation into a shared common function
> in new jpeg_v5_0_interrupt.c and .h files.
>
> Signed-off-by: Andre Luiz Batista Bueno <andrebueno.mac@gmail.com>
> Co-developed-by: Enzo Furegatti Spinella <enzo.spinella@usp.br>
> Signed-off-by: Enzo Furegatti Spinella <enzo.spinella@usp.br>
> ---
>  drivers/gpu/drm/amd/amdgpu/Makefile           |  1 +
>  drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c      | 57 +----------------
>  drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c      | 57 +----------------
>  .../gpu/drm/amd/amdgpu/jpeg_v5_0_interrupt.c  | 64 +++++++++++++++++++
>  .../gpu/drm/amd/amdgpu/jpeg_v5_0_interrupt.h  | 12 ++++
>  5 files changed, 83 insertions(+), 108 deletions(-)
>  create mode 100644 drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_interrupt.c
>  create mode 100644 drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_interrupt.h

This is not common to jpeg v5.  jpeg_v5_0_0.c has a different
interrupt handler, although it could probably be shared with that as
well.  I think the additional fields in the IH should be 0s so it
should work correctly, but we'd need to verify.  If you do want to
share this, I would just make the implementation in jpeg_v5_0_1.c
non-static and then just use it directly in jpeg_v5_0_2.c.

Alex

>
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
> index db66c6372199..69e5e55e1a2b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -229,6 +229,7 @@ amdgpu-y += \
>         jpeg_v5_0_0.o \
>         jpeg_v5_0_1.o \
>         jpeg_v5_0_2.o \
> +       jpeg_v5_0_interrupt.o \
>         jpeg_v5_3_0.o
>
>  # add VPE block
> diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
> index edecbfe66c79..6e5611921eda 100644
> --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
> @@ -34,6 +34,8 @@
>  #include "vcn/vcn_5_0_0_sh_mask.h"
>  #include "ivsrcid/vcn/irqsrcs_vcn_5_0.h"
>
> +#include "jpeg_v5_0_interrupt.h"
> +
>  static int jpeg_v5_0_1_start_sriov(struct amdgpu_device *adev);
>  static void jpeg_v5_0_1_set_dec_ring_funcs(struct amdgpu_device *adev);
>  static void jpeg_v5_0_1_set_irq_funcs(struct amdgpu_device *adev);
> @@ -759,60 +761,7 @@ static int jpeg_v5_0_1_process_interrupt(struct amdgpu_device *adev,
>                                          struct amdgpu_irq_src *source,
>                                          struct amdgpu_iv_entry *entry)
>  {
> -       u32 i, inst;
> -
> -       i = node_id_to_phys_map[entry->node_id];
> -       DRM_DEV_DEBUG(adev->dev, "IH: JPEG TRAP\n");
> -
> -       for (inst = 0; inst < adev->jpeg.num_jpeg_inst; ++inst)
> -               if (adev->jpeg.inst[inst].aid_id == i)
> -                       break;
> -
> -       if (inst >= adev->jpeg.num_jpeg_inst) {
> -               dev_WARN_ONCE(adev->dev, 1,
> -                             "Interrupt received for unknown JPEG instance %d",
> -                             entry->node_id);
> -               return 0;
> -       }
> -
> -       switch (entry->src_id) {
> -       case VCN_5_0__SRCID__JPEG_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[0]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG1_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[1]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG2_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[2]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG3_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[3]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG4_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[4]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG5_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[5]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG6_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[6]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG7_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[7]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG8_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[8]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG9_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[9]);
> -               break;
> -       default:
> -               DRM_DEV_ERROR(adev->dev, "Unhandled interrupt: %d %d\n",
> -                             entry->src_id, entry->src_data[0]);
> -               break;
> -       }
> -
> -       return 0;
> +       return jpeg_v5_0_process_interrupt_common(adev, entry);
>  }
>
>  static void jpeg_v5_0_1_core_stall_reset(struct amdgpu_ring *ring)
> diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c
> index 285c459379c4..daea95907639 100644
> --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c
> @@ -34,6 +34,8 @@
>  #include "vcn/vcn_5_0_0_sh_mask.h"
>  #include "ivsrcid/vcn/irqsrcs_vcn_5_0.h"
>
> +#include "jpeg_v5_0_interrupt.h"
> +
>  static void jpeg_v5_0_2_set_dec_ring_funcs(struct amdgpu_device *adev);
>  static void jpeg_v5_0_2_set_irq_funcs(struct amdgpu_device *adev);
>  static int jpeg_v5_0_2_set_powergating_state(struct amdgpu_ip_block *ip_block,
> @@ -587,60 +589,7 @@ static int jpeg_v5_0_2_process_interrupt(struct amdgpu_device *adev,
>                                          struct amdgpu_irq_src *source,
>                                          struct amdgpu_iv_entry *entry)
>  {
> -       u32 i, inst;
> -
> -       i = node_id_to_phys_map[entry->node_id];
> -       DRM_DEV_DEBUG(adev->dev, "IH: JPEG TRAP\n");
> -
> -       for (inst = 0; inst < adev->jpeg.num_jpeg_inst; ++inst)
> -               if (adev->jpeg.inst[inst].aid_id == i)
> -                       break;
> -
> -       if (inst >= adev->jpeg.num_jpeg_inst) {
> -               dev_WARN_ONCE(adev->dev, 1,
> -                             "Interrupt received for unknown JPEG instance %d",
> -                             entry->node_id);
> -               return 0;
> -       }
> -
> -       switch (entry->src_id) {
> -       case VCN_5_0__SRCID__JPEG_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[0]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG1_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[1]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG2_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[2]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG3_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[3]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG4_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[4]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG5_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[5]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG6_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[6]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG7_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[7]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG8_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[8]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG9_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[9]);
> -               break;
> -       default:
> -               DRM_DEV_ERROR(adev->dev, "Unhandled interrupt: %d %d\n",
> -                             entry->src_id, entry->src_data[0]);
> -               break;
> -       }
> -
> -       return 0;
> +       return jpeg_v5_0_process_interrupt_common(adev, entry);
>  }
>
>  static void jpeg_v5_0_2_core_stall_reset(struct amdgpu_ring *ring)
> diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_interrupt.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_interrupt.c
> new file mode 100644
> index 000000000000..a76ee6586b63
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_interrupt.c
> @@ -0,0 +1,64 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +
> +#include "amdgpu.h"
> +#include "ivsrcid/vcn/irqsrcs_vcn_5_0.h"
> +#include "jpeg_v5_0_interrupt.h"
> +
> +int jpeg_v5_0_process_interrupt_common(struct amdgpu_device *adev,
> +                                      struct amdgpu_iv_entry *entry)
> +{
> +       u32 i, inst;
> +
> +       i = node_id_to_phys_map[entry->node_id];
> +       DRM_DEV_DEBUG(adev->dev, "IH: JPEG TRAP\n");
> +
> +       for (inst = 0; inst < adev->jpeg.num_jpeg_inst; ++inst)
> +               if (adev->jpeg.inst[inst].aid_id == i)
> +                       break;
> +
> +       if (inst >= adev->jpeg.num_jpeg_inst) {
> +               dev_WARN_ONCE(adev->dev, 1,
> +                             "Interrupt received for unknown JPEG instance %d",
> +                             entry->node_id);
> +               return 0;
> +       }
> +
> +       switch (entry->src_id) {
> +       case VCN_5_0__SRCID__JPEG_DECODE:
> +               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[0]);
> +               break;
> +       case VCN_5_0__SRCID__JPEG1_DECODE:
> +               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[1]);
> +               break;
> +       case VCN_5_0__SRCID__JPEG2_DECODE:
> +               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[2]);
> +               break;
> +       case VCN_5_0__SRCID__JPEG3_DECODE:
> +               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[3]);
> +               break;
> +       case VCN_5_0__SRCID__JPEG4_DECODE:
> +               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[4]);
> +               break;
> +       case VCN_5_0__SRCID__JPEG5_DECODE:
> +               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[5]);
> +               break;
> +       case VCN_5_0__SRCID__JPEG6_DECODE:
> +               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[6]);
> +               break;
> +       case VCN_5_0__SRCID__JPEG7_DECODE:
> +               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[7]);
> +               break;
> +       case VCN_5_0__SRCID__JPEG8_DECODE:
> +               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[8]);
> +               break;
> +       case VCN_5_0__SRCID__JPEG9_DECODE:
> +               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[9]);
> +               break;
> +       default:
> +               DRM_DEV_ERROR(adev->dev, "Unhandled interrupt: %d %d\n",
> +                             entry->src_id, entry->src_data[0]);
> +               break;
> +       }
> +
> +       return 0;
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_interrupt.h b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_interrupt.h
> new file mode 100644
> index 000000000000..046bcb4a3906
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_interrupt.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0 OR MIT */
> +
> +#ifndef __JPEG_V5_0_INTERRUPT_H__
> +#define __JPEG_V5_0_INTERRUPT_H__
> +
> +struct amdgpu_device;
> +struct amdgpu_iv_entry;
> +
> +int jpeg_v5_0_process_interrupt_common(struct amdgpu_device *adev,
> +                                      struct amdgpu_iv_entry *entry);
> +
> +#endif /* __JPEG_V5_0_INTERRUPT_H__ */
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] drm/amdgpu: deduplicate JPEG v5.0 interrupt routine
  2026-05-11 14:55 ` Alex Deucher
@ 2026-06-11  2:40   ` Andre Luiz Batista Bueno
  2026-06-11 10:59     ` sashiko-bot
  2026-06-24  3:30   ` [PATCH V2 RESEND] " Andre Luiz Batista Bueno
  1 sibling, 1 reply; 8+ messages in thread
From: Andre Luiz Batista Bueno @ 2026-06-11  2:40 UTC (permalink / raw)
  To: alexander.deucher, christian.koenig, airlied, simona
  Cc: Andre Luiz Batista Bueno, Enzo Furegatti Spinella, amd-gfx,
	dri-devel

Both jpeg_v5_0_1.c and jpeg_v5_0_2.c implement identical
interrupt processing routines. To avoid code duplication,
make the implementation in jpeg_v5_0_1.c non-static and
call it directly from jpeg_v5_0_2.c.

Signed-off-by: Andre Luiz Batista Bueno <andrebueno.mac@gmail.com>
Co-developed-by: Enzo Furegatti Spinella <enzo.spinella@usp.br>
Signed-off-by: Enzo Furegatti Spinella <enzo.spinella@usp.br>
---

v2:
 - Dropped the new jpeg_v5_0_interrupt.c/h files and Makefile changes.
 - Kept the interrupt routine in jpeg_v5_0_1.c but removed the 'static' modifier.
 - Declared the function prototype in jpeg_v5_0_1.h and included it in jpeg_v5_0_2.c to call it directly, as suggested by Alex Deucher.

 drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.h |  8 +++
 drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c | 63 ++----------------------
 3 files changed, 12 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
index edecbfe66c79..46bcbecd89e8 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
@@ -755,7 +755,7 @@ static int jpeg_v5_0_1_set_ras_interrupt_state(struct amdgpu_device *adev,
 
 
 
-static int jpeg_v5_0_1_process_interrupt(struct amdgpu_device *adev,
+int jpeg_v5_0_1_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_v5_0_1.h b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.h
index a7e58d5fb246..67346faecb47 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.h
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.h
@@ -108,4 +108,12 @@ enum amdgpu_jpeg_v5_0_1_sub_block {
 	AMDGPU_JPEG_V5_0_1_MAX_SUB_BLOCK,
 };
 
+struct amdgpu_irq_src;
+struct amdgpu_iv_entry;
+struct amdgpu_device;
+
+int jpeg_v5_0_1_process_interrupt(struct amdgpu_device *adev,
+					struct amdgpu_irq_src *source,
+					struct amdgpu_iv_entry *entry);
+
 #endif /* __JPEG_V5_0_1_H__ */
diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c
index 285c459379c4..250e7f849037 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c
@@ -34,6 +34,8 @@
 #include "vcn/vcn_5_0_0_sh_mask.h"
 #include "ivsrcid/vcn/irqsrcs_vcn_5_0.h"
 
+#include "jpeg_v5_0_1.h"
+
 static void jpeg_v5_0_2_set_dec_ring_funcs(struct amdgpu_device *adev);
 static void jpeg_v5_0_2_set_irq_funcs(struct amdgpu_device *adev);
 static int jpeg_v5_0_2_set_powergating_state(struct amdgpu_ip_block *ip_block,
@@ -583,65 +585,6 @@ static int jpeg_v5_0_2_set_interrupt_state(struct amdgpu_device *adev,
 	return 0;
 }
 
-static int jpeg_v5_0_2_process_interrupt(struct amdgpu_device *adev,
-					 struct amdgpu_irq_src *source,
-					 struct amdgpu_iv_entry *entry)
-{
-	u32 i, inst;
-
-	i = node_id_to_phys_map[entry->node_id];
-	DRM_DEV_DEBUG(adev->dev, "IH: JPEG TRAP\n");
-
-	for (inst = 0; inst < adev->jpeg.num_jpeg_inst; ++inst)
-		if (adev->jpeg.inst[inst].aid_id == i)
-			break;
-
-	if (inst >= adev->jpeg.num_jpeg_inst) {
-		dev_WARN_ONCE(adev->dev, 1,
-			      "Interrupt received for unknown JPEG instance %d",
-			      entry->node_id);
-		return 0;
-	}
-
-	switch (entry->src_id) {
-	case VCN_5_0__SRCID__JPEG_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[0]);
-		break;
-	case VCN_5_0__SRCID__JPEG1_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[1]);
-		break;
-	case VCN_5_0__SRCID__JPEG2_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[2]);
-		break;
-	case VCN_5_0__SRCID__JPEG3_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[3]);
-		break;
-	case VCN_5_0__SRCID__JPEG4_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[4]);
-		break;
-	case VCN_5_0__SRCID__JPEG5_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[5]);
-		break;
-	case VCN_5_0__SRCID__JPEG6_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[6]);
-		break;
-	case VCN_5_0__SRCID__JPEG7_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[7]);
-		break;
-	case VCN_5_0__SRCID__JPEG8_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[8]);
-		break;
-	case VCN_5_0__SRCID__JPEG9_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[9]);
-		break;
-	default:
-		DRM_DEV_ERROR(adev->dev, "Unhandled interrupt: %d %d\n",
-			      entry->src_id, entry->src_data[0]);
-		break;
-	}
-
-	return 0;
-}
 
 static void jpeg_v5_0_2_core_stall_reset(struct amdgpu_ring *ring)
 {
@@ -749,7 +692,7 @@ static void jpeg_v5_0_2_set_dec_ring_funcs(struct amdgpu_device *adev)
 
 static const struct amdgpu_irq_src_funcs jpeg_v5_0_2_irq_funcs = {
 	.set = jpeg_v5_0_2_set_interrupt_state,
-	.process = jpeg_v5_0_2_process_interrupt,
+	.process = jpeg_v5_0_1_process_interrupt,
 };
 
 static void jpeg_v5_0_2_set_irq_funcs(struct amdgpu_device *adev)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] drm/amdgpu: deduplicate JPEG v5.0 interrupt routine
  2026-06-11  2:40   ` [PATCH v2] " Andre Luiz Batista Bueno
@ 2026-06-11 10:59     ` sashiko-bot
  0 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-06-11 10:59 UTC (permalink / raw)
  To: Andre Luiz Batista Bueno; +Cc: dri-devel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing bounds check on `entry->node_id` before array indexing on `node_id_to_phys_map` can lead to an out-of-bounds memory read.
--

commit 18102389ca6554c76ab0eea5128ed6304f9d3003
Author: Andre Luiz Batista Bueno <andrebueno.mac@gmail.com>

drm/amdgpu: deduplicate JPEG v5.0 interrupt routine

Both jpeg_v5_0_1.c and jpeg_v5_0_2.c implement identical interrupt
processing routines. To avoid code duplication, make the implementation
in jpeg_v5_0_1.c non-static and call it directly from jpeg_v5_0_2.c.

> diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
> index 250316704dfac..2b8d8f4ca30d2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
> @@ -755,7 +755,7 @@ static int jpeg_v5_0_1_set_ras_interrupt_state(struct amdgpu_device *adev,
>  
>  
>  
> -static int jpeg_v5_0_1_process_interrupt(struct amdgpu_device *adev,
> +int jpeg_v5_0_1_process_interrupt(struct amdgpu_device *adev,
>  					 struct amdgpu_irq_src *source,
>  					 struct amdgpu_iv_entry *entry)
>  {

[Severity: High]
This is a pre-existing issue, but since this function is being exported to be
shared by other JPEG v5.0 blocks, could an out-of-bounds array access occur
here?

Looking at the beginning of jpeg_v5_0_1_process_interrupt():

	u32 i, inst;

	i = node_id_to_phys_map[entry->node_id];

The entry->node_id is extracted from the GPU's Interrupt Ring buffer using
an 8-bit mask, allowing values up to 255. However, node_id_to_phys_map is
defined globally with a fixed size of NODEID_MAX (15 elements).

Without a bounds check verifying entry->node_id < NODEID_MAX before indexing
the array, could a malformed hardware interrupt payload cause the driver to
read out-of-bounds memory?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611024211.233876-1-andrebueno.mac@gmail.com?part=1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH V2 RESEND] drm/amdgpu: deduplicate JPEG v5.0 interrupt routine
  2026-05-11 14:55 ` Alex Deucher
  2026-06-11  2:40   ` [PATCH v2] " Andre Luiz Batista Bueno
@ 2026-06-24  3:30   ` Andre Luiz Batista Bueno
  2026-06-24  7:56     ` sashiko-bot
  1 sibling, 1 reply; 8+ messages in thread
From: Andre Luiz Batista Bueno @ 2026-06-24  3:30 UTC (permalink / raw)
  To: alexander.deucher, christian.koenig, airlied, simona
  Cc: Andre Luiz Batista Bueno, Enzo Furegatti Spinella, amd-gfx,
	dri-devel

Both jpeg_v5_0_1.c and jpeg_v5_0_2.c implement identical
interrupt processing routines. To avoid code duplication,
make the implementation in jpeg_v5_0_1.c non-static and
call it directly from jpeg_v5_0_2.c.

Signed-off-by: Andre Luiz Batista Bueno <andrebueno.mac@gmail.com>
Co-developed-by: Enzo Furegatti Spinella <enzo.spinella@usp.br>
Signed-off-by: Enzo Furegatti Spinella <enzo.spinella@usp.br>
---

v2:
 - Dropped the new jpeg_v5_0_interrupt.c/h files and Makefile changes.
 - Kept the interrupt routine in jpeg_v5_0_1.c but removed the 'static' modifier.
 - Declared the function prototype in jpeg_v5_0_1.h and included it in jpeg_v5_0_2.c to call it directly, as suggested by Alex Deucher.

 drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.h |  8 +++
 drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c | 63 ++----------------------
 3 files changed, 12 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
index edecbfe66c79..46bcbecd89e8 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
@@ -755,7 +755,7 @@ static int jpeg_v5_0_1_set_ras_interrupt_state(struct amdgpu_device *adev,
 
 
 
-static int jpeg_v5_0_1_process_interrupt(struct amdgpu_device *adev,
+int jpeg_v5_0_1_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_v5_0_1.h b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.h
index a7e58d5fb246..67346faecb47 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.h
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.h
@@ -108,4 +108,12 @@ enum amdgpu_jpeg_v5_0_1_sub_block {
 	AMDGPU_JPEG_V5_0_1_MAX_SUB_BLOCK,
 };
 
+struct amdgpu_irq_src;
+struct amdgpu_iv_entry;
+struct amdgpu_device;
+
+int jpeg_v5_0_1_process_interrupt(struct amdgpu_device *adev,
+					struct amdgpu_irq_src *source,
+					struct amdgpu_iv_entry *entry);
+
 #endif /* __JPEG_V5_0_1_H__ */
diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c
index 285c459379c4..250e7f849037 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c
@@ -34,6 +34,8 @@
 #include "vcn/vcn_5_0_0_sh_mask.h"
 #include "ivsrcid/vcn/irqsrcs_vcn_5_0.h"
 
+#include "jpeg_v5_0_1.h"
+
 static void jpeg_v5_0_2_set_dec_ring_funcs(struct amdgpu_device *adev);
 static void jpeg_v5_0_2_set_irq_funcs(struct amdgpu_device *adev);
 static int jpeg_v5_0_2_set_powergating_state(struct amdgpu_ip_block *ip_block,
@@ -583,65 +585,6 @@ static int jpeg_v5_0_2_set_interrupt_state(struct amdgpu_device *adev,
 	return 0;
 }
 
-static int jpeg_v5_0_2_process_interrupt(struct amdgpu_device *adev,
-					 struct amdgpu_irq_src *source,
-					 struct amdgpu_iv_entry *entry)
-{
-	u32 i, inst;
-
-	i = node_id_to_phys_map[entry->node_id];
-	DRM_DEV_DEBUG(adev->dev, "IH: JPEG TRAP\n");
-
-	for (inst = 0; inst < adev->jpeg.num_jpeg_inst; ++inst)
-		if (adev->jpeg.inst[inst].aid_id == i)
-			break;
-
-	if (inst >= adev->jpeg.num_jpeg_inst) {
-		dev_WARN_ONCE(adev->dev, 1,
-			      "Interrupt received for unknown JPEG instance %d",
-			      entry->node_id);
-		return 0;
-	}
-
-	switch (entry->src_id) {
-	case VCN_5_0__SRCID__JPEG_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[0]);
-		break;
-	case VCN_5_0__SRCID__JPEG1_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[1]);
-		break;
-	case VCN_5_0__SRCID__JPEG2_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[2]);
-		break;
-	case VCN_5_0__SRCID__JPEG3_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[3]);
-		break;
-	case VCN_5_0__SRCID__JPEG4_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[4]);
-		break;
-	case VCN_5_0__SRCID__JPEG5_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[5]);
-		break;
-	case VCN_5_0__SRCID__JPEG6_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[6]);
-		break;
-	case VCN_5_0__SRCID__JPEG7_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[7]);
-		break;
-	case VCN_5_0__SRCID__JPEG8_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[8]);
-		break;
-	case VCN_5_0__SRCID__JPEG9_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[9]);
-		break;
-	default:
-		DRM_DEV_ERROR(adev->dev, "Unhandled interrupt: %d %d\n",
-			      entry->src_id, entry->src_data[0]);
-		break;
-	}
-
-	return 0;
-}
 
 static void jpeg_v5_0_2_core_stall_reset(struct amdgpu_ring *ring)
 {
@@ -749,7 +692,7 @@ static void jpeg_v5_0_2_set_dec_ring_funcs(struct amdgpu_device *adev)
 
 static const struct amdgpu_irq_src_funcs jpeg_v5_0_2_irq_funcs = {
 	.set = jpeg_v5_0_2_set_interrupt_state,
-	.process = jpeg_v5_0_2_process_interrupt,
+	.process = jpeg_v5_0_1_process_interrupt,
 };
 
 static void jpeg_v5_0_2_set_irq_funcs(struct amdgpu_device *adev)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH V2 RESEND] drm/amdgpu: deduplicate JPEG v5.0 interrupt routine
  2026-06-24  3:30   ` [PATCH V2 RESEND] " Andre Luiz Batista Bueno
@ 2026-06-24  7:56     ` sashiko-bot
  0 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-06-24  7:56 UTC (permalink / raw)
  To: Andre Luiz Batista Bueno; +Cc: dri-devel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Potential out-of-bounds array access when indexing `node_id_to_phys_map` with unvalidated `entry->node_id`.
--

commit 9ee6645118b792b4fedbe83c62cc09cc3bce36b1
Author: Andre Luiz Batista Bueno <andrebueno.mac@gmail.com>

drm/amdgpu: deduplicate JPEG v5.0 interrupt routine

This commit deduplicates identical interrupt processing routines in
jpeg_v5_0_1.c and jpeg_v5_0_2.c. It updates jpeg_v5_0_1_process_interrupt()
to be non-static and calls it directly from the jpeg_v5_0_2 interrupt
handlers to avoid code duplication.

> diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
> index 250316704dfac..2b8d8f4ca30d2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
> @@ -755,7 +755,7 @@ static int jpeg_v5_0_1_set_ras_interrupt_state(struct amdgpu_device *adev,
>  
>  
>  
> -static int jpeg_v5_0_1_process_interrupt(struct amdgpu_device *adev,
> +int jpeg_v5_0_1_process_interrupt(struct amdgpu_device *adev,
>  					 struct amdgpu_irq_src *source,
>  					 struct amdgpu_iv_entry *entry)
>  {

[Severity: High]
This is a pre-existing issue, but does this function lack bounds checking on
entry->node_id before using it as an array index?

When the interrupt vector is read from the hardware-writeable IH ring,
entry->node_id can be an 8-bit value (0-255). Looking at how this is
handled inside jpeg_v5_0_1_process_interrupt():

int jpeg_v5_0_1_process_interrupt(...)
{
	u32 i, inst;

	i = node_id_to_phys_map[entry->node_id];
        ...
}

Could a malicious or malfunctioning PCIe device supply an interrupt vector
with a node_id >= NODEID_MAX (15), leading to an out-of-bounds read of up to
1020 bytes past the end of the node_id_to_phys_map array and a potential
kernel panic?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624033134.145561-1-andrebueno.mac@gmail.com?part=1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH V2] drm/amdgpu: deduplicate JPEG v5.0 interrupt routine
@ 2026-07-08 16:28 Andre Luiz Batista Bueno
  2026-07-10 13:20 ` Alex Deucher
  0 siblings, 1 reply; 8+ messages in thread
From: Andre Luiz Batista Bueno @ 2026-07-08 16:28 UTC (permalink / raw)
  To: amd-gfx, alexander.deucher, christian.koenig, airlied, simona
  Cc: Andre Luiz Batista Bueno, Enzo Furegatti Spinella, dri-devel

Both jpeg_v5_0_1.c and jpeg_v5_0_2.c implement identical
interrupt processing routines. To avoid code duplication,
make the implementation in jpeg_v5_0_1.c non-static and
call it directly from jpeg_v5_0_2.c.

Signed-off-by: Andre Luiz Batista Bueno <andrebueno.mac@gmail.com>
Co-developed-by: Enzo Furegatti Spinella <enzo.spinella@usp.br>
Signed-off-by: Enzo Furegatti Spinella <enzo.spinella@usp.br>
---

v2:
 - Dropped the new jpeg_v5_0_interrupt.c/h files and Makefile changes.
 - Kept the interrupt routine in jpeg_v5_0_1.c but removed the 'static' modifier.
 - Declared the function prototype in jpeg_v5_0_1.h and included it in jpeg_v5_0_2.c to call it directly, as suggested by Alex Deucher.

 drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.h |  8 +++
 drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c | 63 ++----------------------
 3 files changed, 12 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
index edecbfe66c79..46bcbecd89e8 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
@@ -755,7 +755,7 @@ static int jpeg_v5_0_1_set_ras_interrupt_state(struct amdgpu_device *adev,
 
 
 
-static int jpeg_v5_0_1_process_interrupt(struct amdgpu_device *adev,
+int jpeg_v5_0_1_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_v5_0_1.h b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.h
index a7e58d5fb246..67346faecb47 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.h
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.h
@@ -108,4 +108,12 @@ enum amdgpu_jpeg_v5_0_1_sub_block {
 	AMDGPU_JPEG_V5_0_1_MAX_SUB_BLOCK,
 };
 
+struct amdgpu_irq_src;
+struct amdgpu_iv_entry;
+struct amdgpu_device;
+
+int jpeg_v5_0_1_process_interrupt(struct amdgpu_device *adev,
+					struct amdgpu_irq_src *source,
+					struct amdgpu_iv_entry *entry);
+
 #endif /* __JPEG_V5_0_1_H__ */
diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c
index 285c459379c4..250e7f849037 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c
@@ -34,6 +34,8 @@
 #include "vcn/vcn_5_0_0_sh_mask.h"
 #include "ivsrcid/vcn/irqsrcs_vcn_5_0.h"
 
+#include "jpeg_v5_0_1.h"
+
 static void jpeg_v5_0_2_set_dec_ring_funcs(struct amdgpu_device *adev);
 static void jpeg_v5_0_2_set_irq_funcs(struct amdgpu_device *adev);
 static int jpeg_v5_0_2_set_powergating_state(struct amdgpu_ip_block *ip_block,
@@ -583,65 +585,6 @@ static int jpeg_v5_0_2_set_interrupt_state(struct amdgpu_device *adev,
 	return 0;
 }
 
-static int jpeg_v5_0_2_process_interrupt(struct amdgpu_device *adev,
-					 struct amdgpu_irq_src *source,
-					 struct amdgpu_iv_entry *entry)
-{
-	u32 i, inst;
-
-	i = node_id_to_phys_map[entry->node_id];
-	DRM_DEV_DEBUG(adev->dev, "IH: JPEG TRAP\n");
-
-	for (inst = 0; inst < adev->jpeg.num_jpeg_inst; ++inst)
-		if (adev->jpeg.inst[inst].aid_id == i)
-			break;
-
-	if (inst >= adev->jpeg.num_jpeg_inst) {
-		dev_WARN_ONCE(adev->dev, 1,
-			      "Interrupt received for unknown JPEG instance %d",
-			      entry->node_id);
-		return 0;
-	}
-
-	switch (entry->src_id) {
-	case VCN_5_0__SRCID__JPEG_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[0]);
-		break;
-	case VCN_5_0__SRCID__JPEG1_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[1]);
-		break;
-	case VCN_5_0__SRCID__JPEG2_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[2]);
-		break;
-	case VCN_5_0__SRCID__JPEG3_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[3]);
-		break;
-	case VCN_5_0__SRCID__JPEG4_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[4]);
-		break;
-	case VCN_5_0__SRCID__JPEG5_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[5]);
-		break;
-	case VCN_5_0__SRCID__JPEG6_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[6]);
-		break;
-	case VCN_5_0__SRCID__JPEG7_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[7]);
-		break;
-	case VCN_5_0__SRCID__JPEG8_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[8]);
-		break;
-	case VCN_5_0__SRCID__JPEG9_DECODE:
-		amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[9]);
-		break;
-	default:
-		DRM_DEV_ERROR(adev->dev, "Unhandled interrupt: %d %d\n",
-			      entry->src_id, entry->src_data[0]);
-		break;
-	}
-
-	return 0;
-}
 
 static void jpeg_v5_0_2_core_stall_reset(struct amdgpu_ring *ring)
 {
@@ -749,7 +692,7 @@ static void jpeg_v5_0_2_set_dec_ring_funcs(struct amdgpu_device *adev)
 
 static const struct amdgpu_irq_src_funcs jpeg_v5_0_2_irq_funcs = {
 	.set = jpeg_v5_0_2_set_interrupt_state,
-	.process = jpeg_v5_0_2_process_interrupt,
+	.process = jpeg_v5_0_1_process_interrupt,
 };
 
 static void jpeg_v5_0_2_set_irq_funcs(struct amdgpu_device *adev)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH V2] drm/amdgpu: deduplicate JPEG v5.0 interrupt routine
  2026-07-08 16:28 [PATCH V2] " Andre Luiz Batista Bueno
@ 2026-07-10 13:20 ` Alex Deucher
  0 siblings, 0 replies; 8+ messages in thread
From: Alex Deucher @ 2026-07-10 13:20 UTC (permalink / raw)
  To: Andre Luiz Batista Bueno
  Cc: amd-gfx, alexander.deucher, christian.koenig, airlied, simona,
	Enzo Furegatti Spinella, dri-devel

Applied.  Thanks!

On Thu, Jul 9, 2026 at 3:44 AM Andre Luiz Batista Bueno
<andrebueno.mac@gmail.com> wrote:
>
> Both jpeg_v5_0_1.c and jpeg_v5_0_2.c implement identical
> interrupt processing routines. To avoid code duplication,
> make the implementation in jpeg_v5_0_1.c non-static and
> call it directly from jpeg_v5_0_2.c.
>
> Signed-off-by: Andre Luiz Batista Bueno <andrebueno.mac@gmail.com>
> Co-developed-by: Enzo Furegatti Spinella <enzo.spinella@usp.br>
> Signed-off-by: Enzo Furegatti Spinella <enzo.spinella@usp.br>
> ---
>
> v2:
>  - Dropped the new jpeg_v5_0_interrupt.c/h files and Makefile changes.
>  - Kept the interrupt routine in jpeg_v5_0_1.c but removed the 'static' modifier.
>  - Declared the function prototype in jpeg_v5_0_1.h and included it in jpeg_v5_0_2.c to call it directly, as suggested by Alex Deucher.
>
>  drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c |  2 +-
>  drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.h |  8 +++
>  drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c | 63 ++----------------------
>  3 files changed, 12 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
> index edecbfe66c79..46bcbecd89e8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
> @@ -755,7 +755,7 @@ static int jpeg_v5_0_1_set_ras_interrupt_state(struct amdgpu_device *adev,
>
>
>
> -static int jpeg_v5_0_1_process_interrupt(struct amdgpu_device *adev,
> +int jpeg_v5_0_1_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_v5_0_1.h b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.h
> index a7e58d5fb246..67346faecb47 100644
> --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.h
> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.h
> @@ -108,4 +108,12 @@ enum amdgpu_jpeg_v5_0_1_sub_block {
>         AMDGPU_JPEG_V5_0_1_MAX_SUB_BLOCK,
>  };
>
> +struct amdgpu_irq_src;
> +struct amdgpu_iv_entry;
> +struct amdgpu_device;
> +
> +int jpeg_v5_0_1_process_interrupt(struct amdgpu_device *adev,
> +                                       struct amdgpu_irq_src *source,
> +                                       struct amdgpu_iv_entry *entry);
> +
>  #endif /* __JPEG_V5_0_1_H__ */
> diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c
> index 285c459379c4..250e7f849037 100644
> --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_2.c
> @@ -34,6 +34,8 @@
>  #include "vcn/vcn_5_0_0_sh_mask.h"
>  #include "ivsrcid/vcn/irqsrcs_vcn_5_0.h"
>
> +#include "jpeg_v5_0_1.h"
> +
>  static void jpeg_v5_0_2_set_dec_ring_funcs(struct amdgpu_device *adev);
>  static void jpeg_v5_0_2_set_irq_funcs(struct amdgpu_device *adev);
>  static int jpeg_v5_0_2_set_powergating_state(struct amdgpu_ip_block *ip_block,
> @@ -583,65 +585,6 @@ static int jpeg_v5_0_2_set_interrupt_state(struct amdgpu_device *adev,
>         return 0;
>  }
>
> -static int jpeg_v5_0_2_process_interrupt(struct amdgpu_device *adev,
> -                                        struct amdgpu_irq_src *source,
> -                                        struct amdgpu_iv_entry *entry)
> -{
> -       u32 i, inst;
> -
> -       i = node_id_to_phys_map[entry->node_id];
> -       DRM_DEV_DEBUG(adev->dev, "IH: JPEG TRAP\n");
> -
> -       for (inst = 0; inst < adev->jpeg.num_jpeg_inst; ++inst)
> -               if (adev->jpeg.inst[inst].aid_id == i)
> -                       break;
> -
> -       if (inst >= adev->jpeg.num_jpeg_inst) {
> -               dev_WARN_ONCE(adev->dev, 1,
> -                             "Interrupt received for unknown JPEG instance %d",
> -                             entry->node_id);
> -               return 0;
> -       }
> -
> -       switch (entry->src_id) {
> -       case VCN_5_0__SRCID__JPEG_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[0]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG1_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[1]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG2_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[2]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG3_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[3]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG4_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[4]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG5_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[5]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG6_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[6]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG7_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[7]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG8_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[8]);
> -               break;
> -       case VCN_5_0__SRCID__JPEG9_DECODE:
> -               amdgpu_fence_process(&adev->jpeg.inst[inst].ring_dec[9]);
> -               break;
> -       default:
> -               DRM_DEV_ERROR(adev->dev, "Unhandled interrupt: %d %d\n",
> -                             entry->src_id, entry->src_data[0]);
> -               break;
> -       }
> -
> -       return 0;
> -}
>
>  static void jpeg_v5_0_2_core_stall_reset(struct amdgpu_ring *ring)
>  {
> @@ -749,7 +692,7 @@ static void jpeg_v5_0_2_set_dec_ring_funcs(struct amdgpu_device *adev)
>
>  static const struct amdgpu_irq_src_funcs jpeg_v5_0_2_irq_funcs = {
>         .set = jpeg_v5_0_2_set_interrupt_state,
> -       .process = jpeg_v5_0_2_process_interrupt,
> +       .process = jpeg_v5_0_1_process_interrupt,
>  };
>
>  static void jpeg_v5_0_2_set_irq_funcs(struct amdgpu_device *adev)
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-07-10 13:21 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-09  2:03 [PATCH V1 RESEND] drm/amdgpu: deduplicate JPEG v5.0 interrupt routine Andre Luiz Batista Bueno
2026-05-11 14:55 ` Alex Deucher
2026-06-11  2:40   ` [PATCH v2] " Andre Luiz Batista Bueno
2026-06-11 10:59     ` sashiko-bot
2026-06-24  3:30   ` [PATCH V2 RESEND] " Andre Luiz Batista Bueno
2026-06-24  7:56     ` sashiko-bot
  -- strict thread matches above, loose matches on Subject: below --
2026-07-08 16:28 [PATCH V2] " Andre Luiz Batista Bueno
2026-07-10 13:20 ` Alex Deucher

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox