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/dpu: fix mismatch between power and frequency Date: Tue, 10 Mar 2026 12:42:47 +1000 Message-ID: In-Reply-To: <20260309063720.13572-1-yuanjie.yang@oss.qualcomm.com> References: <20260309063720.13572-1-yuanjie.yang@oss.qualcomm.com> <20260309063720.13572-1-yuanjie.yang@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 **Commit message:** The commit message is well-written with a clear explana= tion of the race condition between `rpmhpd_sync_state` and `dpu_kms_hw_init= `, and the resulting voltage/clock mismatch. The ASCII timeline diagram is = a nice touch for illustrating the concurrency issue. **Code change:** ```c - /* Drop the performance state vote */ - dev_pm_opp_set_rate(dev, 0); clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks); ``` The removal is clean and correct. Two observations: 1. **Asymmetry concern is addressed:** The resume path at `dpu_kms.c:1485` = only calls `clk_bulk_prepare_enable()` and never restores the OPP rate, so = removing the drop in suspend eliminates the unpaired voltage reduction. 2. **Potential concern =E2=80=94 performance state leak during suspend:** B= y not calling `dev_pm_opp_set_rate(dev, 0)`, the performance state vote fro= m the last `dev_pm_opp_set_rate(&kms->pdev->dev, clk_rate)` call in `dpu_co= re_perf.c:398` remains active even during suspend. However, since the clock= s are being disabled and the ICC bandwidth is zeroed (`icc_set_bw(dpu_kms->= path[i], 0, 0)` at line 1469), runtime PM should still allow the power doma= in to scale down appropriately once the device is fully suspended. The OPP = framework will reconcile when genpd aggregates votes. This should be fine i= n practice, but **the maintainers should confirm** that not explicitly drop= ping the OPP vote doesn't prevent the power domain from reaching a low-powe= r state during suspend on all affected platforms. 3. **Fixes tag is correct:** The referenced commit `b0530eb11913` is the on= e that introduced the OPP API usage in this driver. **Verdict:** The patch looks correct and addresses a real bug. The fix is m= inimal and appropriate. The only question worth raising with the maintainer= s is whether the stale OPP vote could prevent optimal power savings during = runtime suspend, but given that genpd aggregation handles this when the dev= ice is fully powered off, this should be fine. --- Generated by Claude Code Patch Reviewer