From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/msm/dp: turn link_ready into plugged
Date: Mon, 25 May 2026 17:13:09 +1000 [thread overview]
Message-ID: <review-patch9-20260524-hpd-refactor-v6-9-cf3ab488dd7b@oss.qualcomm.com> (raw)
In-Reply-To: <20260524-hpd-refactor-v6-9-cf3ab488dd7b@oss.qualcomm.com>
Patch Review
Replaces the public `link_ready` boolean with a private `plugged` boolean protected by `plugged_lock` mutex.
**Bug (logic error):** In `msm_dp_bridge_get_modes()`, removing the `link_ready` guard breaks the if/else structure:
```c
- if (dp->link_ready) {
- rc = msm_dp_display_get_modes(dp);
- if (rc <= 0) {
- DRM_ERROR("failed to get DP sink modes, rc=%d\n", rc);
- return rc;
- }
+ rc = msm_dp_display_get_modes(dp);
+ if (rc <= 0) {
+ DRM_ERROR("failed to get DP sink modes, rc=%d\n", rc);
+ return rc;
} else {
drm_dbg_dp(connector->dev, "No sink connected\n");
}
```
The `else` now pairs with `if (rc <= 0)`, so when modes ARE successfully retrieved (`rc > 0`), it prints "No sink connected" -- exactly backwards. The `else` branch should be removed entirely since `get_modes` is now always called.
**Bug (PM refcount leak):** In `msm_dp_bridge_detect()`, when the device is plugged, the function does `pm_runtime_resume_and_get()` at the top but the cleanup block only calls `pm_runtime_put_sync()` when `!priv->plugged`:
```c
+ if (!priv->plugged) {
+ if (phy_deinit) {
+ msm_dp_aux_enable_xfers(priv->aux, false);
+ msm_dp_display_host_phy_exit(priv);
+ }
+ pm_runtime_put_sync(&dp->pdev->dev);
+ }
```
Each call to `detect()` when plugged increments the PM refcount without a corresponding put. Since `detect()` can be called multiple times (e.g., during probe, modeset, userspace polling), this will leak PM references and prevent the device from suspending.
**Issue (missing early-out):** `msm_dp_hpd_plug_handle()` removes the `if (dp->msm_dp_display.link_ready) return 0` check and doesn't replace it with a `if (dp->plugged) return 0` equivalent. Now every plug event from HPD does full re-initialization even if already plugged, including `pm_runtime_resume_and_get()` which adds another PM ref.
**Typo:** "initliazed" should be "initialized" in the kerneldoc:
```c
+ * Returns: true if this call has initliazed the PHY
```
**Typo:** "loose" should be "lose" in the comment:
```c
+ * If we detected the DPRX, leave the controller on so that it doesn't
+ * loose the state.
```
---
---
Generated by Claude Code Patch Reviewer
next prev parent reply other threads:[~2026-05-25 7:13 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-24 10:33 [PATCH v6 00/10] drm/msm/dp: Drop the HPD state machine Dmitry Baryshkov
2026-05-24 10:33 ` [PATCH v6 01/10] drm/msm/dp: fix HPD state status bit shift value Dmitry Baryshkov
2026-05-25 7:13 ` Claude review: " Claude Code Review Bot
2026-05-24 10:33 ` [PATCH v6 02/10] drm/msm/dp: Fix the ISR_* enum values Dmitry Baryshkov
2026-05-25 7:13 ` Claude review: " Claude Code Review Bot
2026-05-24 10:33 ` [PATCH v6 03/10] drm/msm/dp: Read DPCD and sink count in bridge detect() Dmitry Baryshkov
2026-05-25 7:13 ` Claude review: " Claude Code Review Bot
2026-05-24 10:33 ` [PATCH v6 04/10] drm/msm/dp: Move link training to atomic_enable() Dmitry Baryshkov
2026-05-25 7:13 ` Claude review: " Claude Code Review Bot
2026-05-24 10:33 ` [PATCH v6 05/10] drm/msm/dp: Drop EV_USER_NOTIFICATION Dmitry Baryshkov
2026-05-25 7:13 ` Claude review: " Claude Code Review Bot
2026-05-24 10:33 ` [PATCH v6 06/10] drm/msm/dp: drop event data Dmitry Baryshkov
2026-05-25 7:13 ` Claude review: " Claude Code Review Bot
2026-05-24 10:33 ` [PATCH v6 07/10] drm/msm/dp: rework HPD handling Dmitry Baryshkov
2026-05-25 7:13 ` Claude review: " Claude Code Review Bot
2026-05-24 10:33 ` [PATCH v6 08/10] drm/msm/dp: Add sink_count to debug logs Dmitry Baryshkov
2026-05-25 7:13 ` Claude review: " Claude Code Review Bot
2026-05-24 10:33 ` [PATCH v6 09/10] drm/msm/dp: turn link_ready into plugged Dmitry Baryshkov
2026-05-25 7:13 ` Claude Code Review Bot [this message]
2026-05-24 10:33 ` [PATCH v6 10/10] drm/msm/dp: clear EDID on display unplug Dmitry Baryshkov
2026-05-25 7:13 ` Claude review: " Claude Code Review Bot
2026-05-24 21:32 ` [PATCH v6 00/10] drm/msm/dp: Drop the HPD state machine Val Packett
2026-05-25 7:13 ` Claude review: " Claude Code Review Bot
-- strict thread matches above, loose matches on Subject: below --
2026-03-14 1:09 [PATCH v5 00/10] " Dmitry Baryshkov
2026-03-14 1:09 ` [PATCH v5 09/10] drm/msm/dp: turn link_ready into plugged Dmitry Baryshkov
2026-03-16 2:12 ` Claude review: " Claude Code Review Bot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=review-patch9-20260524-hpd-refactor-v6-9-cf3ab488dd7b@oss.qualcomm.com \
--to=claude-review@example.com \
--cc=dri-devel-reviews@example.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox