From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/msm/a8xx: Preemption support for A840 Date: Wed, 25 Mar 2026 07:32:11 +1000 Message-ID: In-Reply-To: <20260324-a8xx-gpu-batch2-v1-14-fc95b8d9c017@oss.qualcomm.com> References: <20260324-a8xx-gpu-batch2-v1-0-fc95b8d9c017@oss.qualcomm.com> <20260324-a8xx-gpu-batch2-v1-14-fc95b8d9c017@oss.qualcomm.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review Splits common preemption helpers into `a6xx_preempt.h` and adds `a8xx_preem= pt.c`. **Header file concern:** `a6xx_preempt.h` contains `static` (non-inline) fu= nction `get_next_ring()`. This will cause "defined but not used" warnings o= r duplicate symbols if any translation unit includes the header but doesn't= call the function. It should be `static inline`. **In `a8xx_preempt_trigger()`:** The preemption timer is set to 10 seconds = (`msecs_to_jiffies(10000)`) =E2=80=94 the a6xx version uses the same, so th= is is consistent. **In `a8xx_gpu.c`:** `a8xx_preempt_start()` is called in `hw_init()` under = the `out:` label, meaning it runs even on error paths when `ret` is non-zer= o. This seems intentional (matching A7xx behavior) but could be problematic= if hw_init failed. **In `a7xx_submit()`:** The dispatch between `a8xx_preempt_trigger()` and `= a6xx_preempt_trigger()` is done with a runtime check. This could be a vtabl= e entry instead, but it's fine for now. **Duplicate error message in `a8xx_preempt_irq()`:** ```c DRM_DEV_ERROR(&gpu->pdev->dev, "!!!!!!!!!!!!!!!! preemption faulted !!!!!!!!!!!!!! irq\n"); ... dev_err(dev->dev, "%s: Preemption failed to complete\n", gpu->name); ``` Two error messages for the same condition is excessive. The first one with = the exclamation marks should probably be removed or at least toned down. --- Generated by Claude Code Patch Reviewer