* [PATCH 0/2] drm/amdgpu/jpeg: deduplicate process_interrupt across JPEG versions
@ 2026-04-22 0:38 Tiago Dourado
2026-04-22 0:38 ` [PATCH 1/2] drm/amdgpu/jpeg: deduplicate jpeg_v3_0 process_interrupt Tiago Dourado
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Tiago Dourado @ 2026-04-22 0:38 UTC (permalink / raw)
To: airlied, alexander.deucher, christian.koenig, simona
Cc: luiz.f.f.fernandes, Tiago Dourado, amd-gfx, dri-devel
The JPEG versions v3_0 and v5_3_0 have process_interrupt implementations identical to their predecessor versions (v2_0, v5_0_0 respectively). This patchset removes these duplicates by exporting the original functions and reusing them via macro aliases.
Tiago Dourado (2):
drm/amdgpu/jpeg: deduplicate jpeg_v3_0 process_interrupt
drm/amdgpu/jpeg: deduplicate jpeg_v5_3_0 process_interrupt
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 ++-------------------
drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.c | 2 +-
drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.h | 4 ++++
drivers/gpu/drm/amd/amdgpu/jpeg_v5_3_0.c | 22 +++-------------------
6 files changed, 15 insertions(+), 40 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] drm/amdgpu/jpeg: deduplicate jpeg_v3_0 process_interrupt
2026-04-22 0:38 [PATCH 0/2] drm/amdgpu/jpeg: deduplicate process_interrupt across JPEG versions Tiago Dourado
@ 2026-04-22 0:38 ` Tiago Dourado
2026-04-22 21:56 ` Claude review: " Claude Code Review Bot
2026-04-22 0:38 ` [PATCH 2/2] drm/amdgpu/jpeg: deduplicate jpeg_v5_3_0 process_interrupt Tiago Dourado
2026-04-22 21:56 ` Claude review: drm/amdgpu/jpeg: deduplicate process_interrupt across JPEG versions Claude Code Review Bot
2 siblings, 1 reply; 6+ messages in thread
From: Tiago Dourado @ 2026-04-22 0:38 UTC (permalink / raw)
To: airlied, alexander.deucher, christian.koenig, simona
Cc: luiz.f.f.fernandes, Tiago Dourado, amd-gfx, dri-devel
The jpeg_v3_0_process_interrupt function is identical to
jpeg_v2_0_process_interrupt. Remove the duplicate implementation
in jpeg_v3_0 and reuse the jpeg_v2_0 version via a macro alias.
Export jpeg_v2_0_process_interrupt through jpeg_v2_0.h to allow
cross-version reuse.
Signed-off-by: Tiago Dourado <tiagodourado@usp.br>
Co-developed-by: Luiz Fernandes <luiz.f.f.fernandes@usp.br>
Signed-off-by: Luiz Fernandes <luiz.f.f.fernandes@usp.br>
---
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, 7 insertions(+), 20 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..5010dafc2 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v3_0.c
@@ -34,6 +34,8 @@
#define mmUVD_JPEG_PITCH_INTERNAL_OFFSET 0x401f
+#define jpeg_v3_0_process_interrupt jpeg_v2_0_process_interrupt
+
static const struct amdgpu_hwip_reg_entry jpeg_reg_list_3_0[] = {
SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_POWER_STATUS),
SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_INT_STAT),
@@ -539,25 +541,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)
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] drm/amdgpu/jpeg: deduplicate jpeg_v5_3_0 process_interrupt
2026-04-22 0:38 [PATCH 0/2] drm/amdgpu/jpeg: deduplicate process_interrupt across JPEG versions Tiago Dourado
2026-04-22 0:38 ` [PATCH 1/2] drm/amdgpu/jpeg: deduplicate jpeg_v3_0 process_interrupt Tiago Dourado
@ 2026-04-22 0:38 ` Tiago Dourado
2026-04-22 21:56 ` Claude review: " Claude Code Review Bot
2026-04-22 21:56 ` Claude review: drm/amdgpu/jpeg: deduplicate process_interrupt across JPEG versions Claude Code Review Bot
2 siblings, 1 reply; 6+ messages in thread
From: Tiago Dourado @ 2026-04-22 0:38 UTC (permalink / raw)
To: airlied, alexander.deucher, christian.koenig, simona
Cc: luiz.f.f.fernandes, Tiago Dourado, amd-gfx, dri-devel
The jpeg_v5_3_0_process_interrupt function is identical to
jpeg_v5_0_0_process_interrupt. Remove the duplicate implementation
in jpeg_v5_3_0 and reuse the jpeg_v5_0_0 version via a macro alias.
Export jpeg_v5_0_0_process_interrupt through jpeg_v5_0_0.h to allow
cross-version reuse.
Signed-off-by: Tiago Dourado <tiagodourado@usp.br>
Co-developed-by: Luiz Fernandes <luiz.f.f.fernandes@usp.br>
Signed-off-by: Luiz Fernandes <luiz.f.f.fernandes@usp.br>
---
drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.c | 2 +-
drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.h | 4 ++++
drivers/gpu/drm/amd/amdgpu/jpeg_v5_3_0.c | 22 +++-------------------
3 files changed, 8 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.c
index 46bf15dce..4575d1f9d 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.c
@@ -625,7 +625,7 @@ static int jpeg_v5_0_0_set_interrupt_state(struct amdgpu_device *adev,
return 0;
}
-static int jpeg_v5_0_0_process_interrupt(struct amdgpu_device *adev,
+int jpeg_v5_0_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_v5_0_0.h b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.h
index 5abb96159..4eeb0c147 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.h
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.h
@@ -32,4 +32,8 @@
extern const struct amdgpu_ip_block_version jpeg_v5_0_0_ip_block;
+int jpeg_v5_0_0_process_interrupt(struct amdgpu_device *adev,
+ struct amdgpu_irq_src *source,
+ struct amdgpu_iv_entry *entry);
+
#endif /* __JPEG_V5_0_0_H__ */
diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_3_0.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_3_0.c
index 1821dced9..92aaf5dea 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_3_0.c
@@ -32,8 +32,11 @@
#include "vcn/vcn_5_3_0_offset.h"
#include "vcn/vcn_5_3_0_sh_mask.h"
#include "ivsrcid/vcn/irqsrcs_vcn_5_0.h"
+#include "jpeg_v5_0_0.h"
#include "jpeg_v5_3_0.h"
+#define jpeg_v5_3_0_process_interrupt jpeg_v5_0_0_process_interrupt
+
static void jpeg_v5_3_0_set_dec_ring_funcs(struct amdgpu_device *adev);
static void jpeg_v5_3_0_set_irq_funcs(struct amdgpu_device *adev);
static int jpeg_v5_3_0_set_powergating_state(struct amdgpu_ip_block *ip_block,
@@ -608,25 +611,6 @@ static int jpeg_v5_3_0_set_interrupt_state(struct amdgpu_device *adev,
return 0;
}
-static int jpeg_v5_3_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_5_0__SRCID__JPEG_DECODE:
- amdgpu_fence_process(adev->jpeg.inst->ring_dec);
- break;
- default:
- DRM_DEV_ERROR(adev->dev, "Unhandled interrupt: %d %d\n",
- entry->src_id, entry->src_data[0]);
- break;
- }
-
- return 0;
-}
-
static int jpeg_v5_3_0_ring_reset(struct amdgpu_ring *ring,
unsigned int vmid,
struct amdgpu_fence *timedout_fence)
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Claude review: drm/amdgpu/jpeg: deduplicate process_interrupt across JPEG versions
2026-04-22 0:38 [PATCH 0/2] drm/amdgpu/jpeg: deduplicate process_interrupt across JPEG versions Tiago Dourado
2026-04-22 0:38 ` [PATCH 1/2] drm/amdgpu/jpeg: deduplicate jpeg_v3_0 process_interrupt Tiago Dourado
2026-04-22 0:38 ` [PATCH 2/2] drm/amdgpu/jpeg: deduplicate jpeg_v5_3_0 process_interrupt Tiago Dourado
@ 2026-04-22 21:56 ` Claude Code Review Bot
2 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-04-22 21:56 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/amdgpu/jpeg: deduplicate process_interrupt across JPEG versions
Author: Tiago Dourado <tiagodourado@usp.br>
Patches: 3
Reviewed: 2026-04-23T07:56:58.862961
---
This is a straightforward deduplication series removing two copy-paste `process_interrupt` implementations in JPEG v3.0 and v5.3.0 that are identical to their predecessor versions (v2.0 and v5.0.0 respectively). The functional change is correct — the deleted code is genuinely identical to the reused functions. However, the implementation uses an unnecessary macro alias pattern instead of simply assigning the function pointer directly, and the Signed-off-by/Co-developed-by tag ordering is wrong per kernel documentation.
**Recommendation: Needs minor rework.**
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 6+ messages in thread
* Claude review: drm/amdgpu/jpeg: deduplicate jpeg_v3_0 process_interrupt
2026-04-22 0:38 ` [PATCH 1/2] drm/amdgpu/jpeg: deduplicate jpeg_v3_0 process_interrupt Tiago Dourado
@ 2026-04-22 21:56 ` Claude Code Review Bot
0 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-04-22 21:56 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Correctness:** The deleted `jpeg_v3_0_process_interrupt` is verified identical to `jpeg_v2_0_process_interrupt` — both use `VCN_2_0__SRCID__JPEG_DECODE`, `DRM_ERROR`, and `adev->jpeg.inst->ring_dec`. The deduplication is valid.
**Style concern — macro alias is unnecessary indirection:**
```c
#define jpeg_v3_0_process_interrupt jpeg_v2_0_process_interrupt
```
This macro exists solely so the struct initializer can still say `.process = jpeg_v3_0_process_interrupt`. This is over-engineering — it hides the actual function being called and makes code search harder (grepping for the definition of `jpeg_v3_0_process_interrupt` now lands on a macro, not a function). It would be simpler and more readable to just use the function directly in the struct initializer:
```c
static const struct amdgpu_irq_src_funcs jpeg_v3_0_irq_funcs = {
.set = jpeg_v3_0_set_interrupt_state,
.process = jpeg_v2_0_process_interrupt,
};
```
This is the standard kernel pattern for reusing a function from another version — no macro needed.
**Tag ordering issue:** The Signed-off-by / Co-developed-by order is wrong per `Documentation/process/submitting-patches.rst`. Currently:
```
Signed-off-by: Tiago Dourado <tiagodourado@usp.br>
Co-developed-by: Luiz Fernandes <luiz.f.f.fernandes@usp.br>
Signed-off-by: Luiz Fernandes <luiz.f.f.fernandes@usp.br>
```
The submitter's `Signed-off-by` must be **last**, and every `Co-developed-by` must be immediately followed by its matching `Signed-off-by`. The correct order is:
```
Co-developed-by: Luiz Fernandes <luiz.f.f.fernandes@usp.br>
Signed-off-by: Luiz Fernandes <luiz.f.f.fernandes@usp.br>
Signed-off-by: Tiago Dourado <tiagodourado@usp.br>
```
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 6+ messages in thread
* Claude review: drm/amdgpu/jpeg: deduplicate jpeg_v5_3_0 process_interrupt
2026-04-22 0:38 ` [PATCH 2/2] drm/amdgpu/jpeg: deduplicate jpeg_v5_3_0 process_interrupt Tiago Dourado
@ 2026-04-22 21:56 ` Claude Code Review Bot
0 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-04-22 21:56 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Correctness:** The deleted `jpeg_v5_3_0_process_interrupt` is verified identical to `jpeg_v5_0_0_process_interrupt` — both use `VCN_5_0__SRCID__JPEG_DECODE`, `DRM_DEV_ERROR`, and `adev->jpeg.inst->ring_dec`. The deduplication is valid.
**Same style concern — unnecessary macro alias:**
```c
#define jpeg_v5_3_0_process_interrupt jpeg_v5_0_0_process_interrupt
```
Same feedback as patch 1: drop the macro and assign `.process = jpeg_v5_0_0_process_interrupt` directly in the `jpeg_v5_3_0_irq_funcs` struct.
**Same tag ordering issue** as patch 1 — submitter's `Signed-off-by` should be last.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-04-22 21:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-22 0:38 [PATCH 0/2] drm/amdgpu/jpeg: deduplicate process_interrupt across JPEG versions Tiago Dourado
2026-04-22 0:38 ` [PATCH 1/2] drm/amdgpu/jpeg: deduplicate jpeg_v3_0 process_interrupt Tiago Dourado
2026-04-22 21:56 ` Claude review: " Claude Code Review Bot
2026-04-22 0:38 ` [PATCH 2/2] drm/amdgpu/jpeg: deduplicate jpeg_v5_3_0 process_interrupt Tiago Dourado
2026-04-22 21:56 ` Claude review: " Claude Code Review Bot
2026-04-22 21:56 ` Claude review: drm/amdgpu/jpeg: deduplicate process_interrupt across JPEG versions Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox