From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: Re: [PATCH] drm/amd/amdgpu: remove duplicated code in gfx_v11 and gfx_v12 Date: Tue, 05 May 2026 10:53:47 +1000 Message-ID: In-Reply-To: References: X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Overall Series Review Subject: Re: [PATCH] drm/amd/amdgpu: remove duplicated code in gfx_v11 and = gfx_v12 Author: =3D?UTF-8?Q?Christian_K=3DC3=3DB6nig?=3D Patches: 2 Reviewed: 2026-05-05T10:53:47.373114 --- This is a single-patch cleanup that consolidates identical `handle_priv_fau= lt` implementations from `gfx_v11_0.c` and `gfx_v12_0.c` into a shared `amd= gpu_gfx_handle_priv_fault()` in `amdgpu_gfx.c`. The consolidation is correc= t =E2=80=94 I verified that the two original functions are byte-for-byte id= entical in the current tree. The mechanical transformation is clean. However, there are a few issues: 1. **`Co-developed-by` tag ordering is wrong** =E2=80=94 kernel documentati= on requires each `Co-developed-by` to be immediately followed by its corres= ponding `Signed-off-by`, with the submitter's `Signed-off-by` last. 2. **`BUG()` in the default case** =E2=80=94 this is inherited from the ori= ginal code, but consolidating into shared infrastructure is a good opportun= ity to fix it. `BUG()` in an interrupt handler will panic the kernel. The n= ewer `gfx_v12_1` already uses `dev_dbg()` for this case instead. 3. **Incomplete consolidation** =E2=80=94 `gfx_v10_0.c` has a nearly identi= cal implementation (differing only in lacking the `disable_kq` guard), and = `gfx_v12_1.c` has a related variant. At minimum, the commit message should = acknowledge why these were excluded. 4. **Minor style issues** in the kernel-doc comment. Verdict: Good idea, needs a v2 to fix the tag ordering and ideally address = the `BUG()` concern. --- --- Generated by Claude Code Patch Reviewer