From mboxrd@z Thu Jan  1 00:00:00 1970
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
Message-ID: <review-patch9-20260524-hpd-refactor-v6-9-cf3ab488dd7b@oss.qualcomm.com>
In-Reply-To: <20260524-hpd-refactor-v6-9-cf3ab488dd7b@oss.qualcomm.com>
References: <20260524-hpd-refactor-v6-0-cf3ab488dd7b@oss.qualcomm.com> <20260524-hpd-refactor-v6-9-cf3ab488dd7b@oss.qualcomm.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

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