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/amd/display: update outdated comments for renamed vblank_control_worker() Date: Sun, 22 Mar 2026 03:07:22 +1000 Message-ID: In-Reply-To: <20260321105717.6212-1-kexinsun@smail.nju.edu.cn> References: <20260321105717.6212-1-kexinsun@smail.nju.edu.cn> <20260321105717.6212-1-kexinsun@smail.nju.edu.cn> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Correctness**: The patch is correct. The function `amdgpu_dm_crtc_vblank_= control_worker()` is defined at `amdgpu_dm_crtc.c:246`, confirming the rena= me happened and the new name in the comments is accurate. **Minor nits**: 1. The `Assisted-by: unnamed:deepseek-v3.2 coccinelle` tag is non-standard.= Typically for tool-assisted changes, the tag format would be something lik= e `Reported-by:` or just mentioned in the commit message body. The kernel c= ommunity has discussed `Assisted-by:` but it is not yet a widely recognized= trailer. The maintainer may ask for this to be adjusted. 2. The line wrapping in the second hunk is slightly awkward: ``` + * PSR-SU will be enabled in + * amdgpu_dm_crtc_vblank_control_worker() if user + * pause the video during the PSR-SU was disabled. ``` This could read more naturally as a single reflow, but given the 80-colu= mn constraint and the long function name, it's acceptable. 3. Pre-existing grammar issue (not introduced by this patch): "if user paus= e the video during the PSR-SU was disabled" reads awkwardly =E2=80=94 it sh= ould be "if the user pauses the video while PSR-SU is disabled." Since the = patch is touching these lines anyway, it would be a nice cleanup to fix the= grammar, but this is optional. **Verdict**: The patch is straightforward and correct. Acceptable as-is, wi= th optional grammar cleanup. --- Generated by Claude Code Patch Reviewer