[PATCH v6 0/2] Add no-hpd property to the cadence bridge

[PATCH v6 0/2] Add no-hpd property to the cadence bridge

[Yashas D]

This series adds 'no-hpd' device tree property support to the Cadence
MHDP8546 bridge driver for boards where the HPD line cannot be used for
hotplug detection.

On TI J721S2 EVMs, the DP0 HPD resistor is DNI from factory, so the HPD
signal is not physically connected to SoC pin AA24 by default. AA24 must
still be in DP0_HPD mux mode for the MHDP firmware to operate.

When 'no-hpd' is set, DRM_BRIDGE_OP_HPD is omitted and the framework
falls back to polling .detect() every ~10 seconds. Presence is detected
by reading DP_DPCD_REV (0x000) over AUX, with an early exit when the
monitor is stably connected to avoid re-programming the stream each poll.

Changes since v5:
  - Corrected hardware description: conflict is with RGMII1_RD0 on GESI
    ethernet board, not MCASP1 audio
  - Fixed YAML parse error caused by bullet list in binding description
  - Removed 100ms firmware timeout
  - Fixed sst_enable() called on every poll: added early exit when link
    is already up
  - Switched presence probe to DP_DPCD_REV (0x000) to avoid reading link
    training status registers unnecessarily
  - Fixed lockless hw_state read in bridge_detect(): now reads under
    start_lock
  - Fixed data race in attach(): use hw_ready snapshot, not bare
    mhdp->hw_state after lock release

Link to v5: https://lore.kernel.org/all/20260728125336.776116-1-y-d@ti.com/

Rahul T R (2):
  dt-bindings: display: bridge: cdns,mhdp8546: Add no-hpd property to
    the cadence bridge
  drm: bridge: cdns-mhdp8546: Add no-hpd property

 .../display/bridge/cdns,mhdp8546.yaml         | 11 +++
 .../drm/bridge/cadence/cdns-mhdp8546-core.c   | 79 +++++++++++++++++--
 .../drm/bridge/cadence/cdns-mhdp8546-core.h   |  1 +
 3 files changed, 83 insertions(+), 8 deletions(-)

-- 
2.34.1

[PATCH v6 1/2] dt-bindings: display: bridge: cdns,mhdp8546: Add no-hpd property to the cadence bridge

[Yashas D]

From: Rahul T R <r-ravikumar@ti.com>

The mhdp bridge can work without its HPD pin hooked up to the connector,
but the current bridge driver throws an error when hpd line is not
connected to the connector. For such cases, we need an indication for
no-hpd, using which we can bypass the hpd detection and instead use the
auxiliary channels connected to the DP connector to confirm the
connection.
So add no-hpd property to the bindings, to disable hpd when not
connected or cannot be used for hotplug detection.

Signed-off-by: Rahul T R <r-ravikumar@ti.com>
Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
Signed-off-by: Yashas D <y-d@ti.com>
---
 .../bindings/display/bridge/cdns,mhdp8546.yaml        | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml
index c2b369456e4e..56ce3f65ff49 100644
--- a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml
@@ -57,6 +57,17 @@ properties:
   interrupts:
     maxItems: 1
 
+  no-hpd:
+    type: boolean
+    description:
+      Set if the HPD line on the bridge isn't physically connected to the
+      DisplayPort connector or cannot be used for hotplug detection.
+
+      Valid use cases include HPD pin not routed to the connector on the PCB,
+      HPD signal muxed with another function on the SoC making it unavailable
+      for hotplug detection, or hardware design where HPD cannot reliably
+      detect monitor presence.
+
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
 
-- 
2.34.1

Re: [PATCH v6 1/2] dt-bindings: display: bridge: cdns,mhdp8546: Add no-hpd property to the cadence bridge

[Krzysztof Kozlowski]

On Sun, Aug 02, 2026 at 09:08:24PM +0530, Yashas D wrote:
> From: Rahul T R <r-ravikumar@ti.com>
> 
> The mhdp bridge can work without its HPD pin hooked up to the connector,
> but the current bridge driver throws an error when hpd line is not
> connected to the connector. For such cases, we need an indication for
> no-hpd, using which we can bypass the hpd detection and instead use the
> auxiliary channels connected to the DP connector to confirm the
> connection.
> So add no-hpd property to the bindings, to disable hpd when not
> connected or cannot be used for hotplug detection.
> 
> Signed-off-by: Rahul T R <r-ravikumar@ti.com>
> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
> Signed-off-by: Yashas D <y-d@ti.com>

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

[PATCH v6 2/2] drm: bridge: cdns-mhdp8546: Add no-hpd property

[Yashas D]

From: Rahul T R <r-ravikumar@ti.com>

Add a 'no-hpd' boolean property to support boards where the HPD line
cannot be used for hotplug detection due to hardware limitations.

On TI J721S2 EVMs, the DP0 HPD resistor is not populated from factory
(DNI), so the HPD signal is not physically connected to SoC pin AA24
by default which makes HPD unavailable but AA24 must be in DP0_HPD
mux mode for the MHDP firmware to operate.

When this property is set, the driver uses auxiliary channel (AUX) DPCD
reads to detect monitor presence instead of hardware HPD signals. The
DRM framework polls the connection status via the .detect() callback,
providing hotplug detection without requiring the HPD pin.

Valid use cases:
- HPD pin not routed to connector on PCB
- HPD signal muxed with another function on SoC
- Hardware designs where HPD cannot reliably detect monitor presence

Signed-off-by: Rahul T R <r-ravikumar@ti.com>
Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
Signed-off-by: Harikrishna Shenoy <h-shenoy@ti.com>
Signed-off-by: Yashas D <y-d@ti.com>
---
 .../drm/bridge/cadence/cdns-mhdp8546-core.c   | 79 +++++++++++++++++--
 .../drm/bridge/cadence/cdns-mhdp8546-core.h   |  1 +
 2 files changed, 72 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index 504a3186ebb3..ae9bbec855f3 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -53,6 +53,8 @@
 #include "cdns-mhdp8546-hdcp.h"
 #include "cdns-mhdp8546-j721e.h"
 
+static int cdns_mhdp_update_link_status(struct cdns_mhdp_device *mhdp);
+
 static void cdns_mhdp_bridge_hpd_enable(struct drm_bridge *bridge)
 {
 	struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge);
@@ -698,7 +700,9 @@ static int cdns_mhdp_fw_activate(const struct firmware *fw,
 	 * MHDP_HW_STOPPED happens only due to driver removal when
 	 * bridge should already be detached.
 	 */
-	cdns_mhdp_bridge_hpd_enable(&mhdp->bridge);
+
+	if (!mhdp->no_hpd)
+		cdns_mhdp_bridge_hpd_enable(&mhdp->bridge);
 
 	spin_unlock(&mhdp->start_lock);
 
@@ -739,7 +743,13 @@ static void cdns_mhdp_fw_cb(const struct firmware *fw, void *context)
 	spin_lock(&mhdp->start_lock);
 	bridge_attached = mhdp->bridge_attached;
 	spin_unlock(&mhdp->start_lock);
-	if (bridge_attached)
+
+	if (!bridge_attached)
+		return;
+
+	if (mhdp->no_hpd)
+		cdns_mhdp_update_link_status(mhdp);
+	else
 		drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
 }
 
@@ -788,9 +798,14 @@ static ssize_t cdns_mhdp_transfer(struct drm_dp_aux *aux,
 		ret = cdns_mhdp_dpcd_read(mhdp, msg->address,
 					  msg->buffer, msg->size);
 		if (ret) {
-			dev_dbg(mhdp->dev,
-				"Failed to read DPCD addr %u\n",
-				msg->address);
+			if (mhdp->no_hpd)
+				dev_dbg(mhdp->dev,
+					"Failed to read DPCD addr %u\n",
+					msg->address);
+			else
+				dev_err(mhdp->dev,
+					"Failed to read DPCD addr %u\n",
+					msg->address);
 
 			return ret;
 		}
@@ -1523,6 +1538,19 @@ static int cdns_mhdp_attach(struct drm_bridge *bridge,
 
 	spin_unlock(&mhdp->start_lock);
 
+	if (mhdp->no_hpd) {
+		/*
+		 * In no-hpd mode there are no HPD interrupts to trigger
+		 * detection. If firmware is already ready, do the initial
+		 * AUX poll immediately. Otherwise fw_cb() will call
+		 * cdns_mhdp_update_link_status() once firmware finishes
+		 * loading and sees bridge_attached is true.
+		 */
+		if (hw_ready)
+			cdns_mhdp_update_link_status(mhdp);
+		return 0;
+	}
+
 	/* Enable SW event interrupts */
 	if (hw_ready)
 		cdns_mhdp_bridge_hpd_enable(bridge);
@@ -2012,6 +2040,16 @@ static enum drm_connector_status
 cdns_mhdp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connector)
 {
 	struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge);
+	bool hw_ready;
+
+	if (mhdp->no_hpd) {
+		spin_lock(&mhdp->start_lock);
+		hw_ready = mhdp->hw_state == MHDP_HW_READY;
+		spin_unlock(&mhdp->start_lock);
+
+		if (hw_ready)
+			cdns_mhdp_update_link_status(mhdp);
+	}
 
 	return cdns_mhdp_detect(mhdp);
 }
@@ -2100,7 +2138,29 @@ static int cdns_mhdp_update_link_status(struct cdns_mhdp_device *mhdp)
 
 	mutex_lock(&mhdp->link_mutex);
 
-	mhdp->plugged = cdns_mhdp_detect_hpd(mhdp, &hpd_pulse);
+	if (mhdp->no_hpd) {
+		u8 rev;
+
+		/*
+		 * Use a side-effect-free capability register for presence
+		 * detection. A successful AUX read means a sink is present;
+		 * we do not need link training status registers here.
+		 */
+		ret = drm_dp_dpcd_read(&mhdp->aux, DP_DPCD_REV, &rev, 1);
+		mhdp->plugged = (ret == 1);
+		ret = mhdp->plugged ? 0 : -EIO;
+		hpd_pulse = false;
+
+		/*
+		 * If the monitor is still connected and the link is already
+		 * up, there is nothing to do. Avoid falling through to
+		 * cdns_mhdp_sst_enable() on every poll cycle.
+		 */
+		if (mhdp->plugged && old_plugged && mhdp->link_up)
+			goto out;
+	} else {
+		mhdp->plugged = cdns_mhdp_detect_hpd(mhdp, &hpd_pulse);
+	}
 
 	if (!mhdp->plugged) {
 		cdns_mhdp_link_down(mhdp);
@@ -2288,6 +2348,8 @@ static int cdns_mhdp_probe(struct platform_device *pdev)
 	mhdp->aux.dev = dev;
 	mhdp->aux.transfer = cdns_mhdp_transfer;
 
+	mhdp->no_hpd = of_property_read_bool(dev->of_node, "no-hpd");
+
 	mhdp->regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(mhdp->regs)) {
 		dev_err(dev, "Failed to get memory resource\n");
@@ -2360,8 +2422,9 @@ static int cdns_mhdp_probe(struct platform_device *pdev)
 	mhdp->display_fmt.bpc = 8;
 
 	mhdp->bridge.of_node = pdev->dev.of_node;
-	mhdp->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID |
-			   DRM_BRIDGE_OP_HPD;
+	mhdp->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID;
+	if (!mhdp->no_hpd)
+		mhdp->bridge.ops |= DRM_BRIDGE_OP_HPD;
 	mhdp->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
 
 	ret = phy_init(mhdp->phy);
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
index b53335b0d22c..24ffb732a207 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
@@ -388,6 +388,7 @@ struct cdns_mhdp_device {
 
 	bool link_up;
 	bool plugged;
+	bool no_hpd;
 
 	/*
 	 * "start_lock" protects the access to bridge_attached and
-- 
2.34.1

Re: [PATCH v6 2/2] drm: bridge: cdns-mhdp8546: Add no-hpd property

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Polling `.detect()` breaks DPMS and wastes power by continuously retraining the link after the display is disabled.
- [High] Skipping `cdns_mhdp_bridge_hpd_enable()` in no-hpd mode permanently masks SW event interrupts, breaking HDCP.

Pre-existing issues:
- [Critical] Accessing `bridge.base.state` outside of atomic commit context without modeset locks leads to a Use-After-Free.
--

--- Patch [2]: [PATCH v6 2/2] drm: bridge: cdns-mhdp8546: Add no-hpd property ---
commit 33969e025e143110f913b15c51739edb9cc917f9
Author: Rahul T R <r-ravikumar@ti.com>
Subject: drm: bridge: cdns-mhdp8546: Add no-hpd property

Add a 'no-hpd' boolean property to support boards where the HPD line
cannot be used for hotplug detection due to hardware limitations.

> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> index 36c07b71fe04b..c81c742e56ac8 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c

[ ... ]

> @@ -698,7 +700,9 @@ static int cdns_mhdp_fw_activate(const struct firmware *fw,
>  	 * MHDP_HW_STOPPED happens only due to driver removal when
>  	 * bridge should already be detached.
>  	 */
> -	cdns_mhdp_bridge_hpd_enable(&mhdp->bridge);
> +
> +	if (!mhdp->no_hpd)
> +		cdns_mhdp_bridge_hpd_enable(&mhdp->bridge);

[Severity: High]
Does skipping cdns_mhdp_bridge_hpd_enable() here and in cdns_mhdp_attach()
permanently mask software event interrupts?

The CDNS_APB_INT_MASK_SW_EVENT_INT interrupt is multiplexed for multiple
events, including HDCP status (CDNS_HDCP_TX_STATUS).

If it is never unmasked in no-hpd mode, it looks like HDCP initialization
and communication will timeout and fail completely.

[ ... ]

> @@ -2012,6 +2040,16 @@ static enum drm_connector_status
>  cdns_mhdp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connector)
>  {
>  	struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge);
> +	bool hw_ready;
> +
> +	if (mhdp->no_hpd) {
> +		spin_lock(&mhdp->start_lock);
> +		hw_ready = mhdp->hw_state == MHDP_HW_READY;
> +		spin_unlock(&mhdp->start_lock);
> +
> +		if (hw_ready)
> +			cdns_mhdp_update_link_status(mhdp);
> +	}

[Severity: High]
Will this polling cause the link to be continuously retrained even when the
display is intentionally disabled?

When the display is disabled for power management (DPMS blanking),
cdns_mhdp_update_link_status() will see the link is down and execute
cdns_mhdp_link_up():

cdns_mhdp_update_link_status() {
    ...
    if (!mhdp->link_up) {
        ret = cdns_mhdp_link_up(mhdp);
        if (ret < 0)
            goto out;
    }
}

Since the .detect() callback is periodically invoked by the DRM core (every
10 seconds), does this unconditionally wake the monitor up from sleep and waste
power by ignoring the mhdp->bridge_enabled state?

[Severity: Critical]
This is a pre-existing issue, but does exposing cdns_mhdp_update_link_status()
to periodic polling significantly expand a use-after-free race window?

Later in cdns_mhdp_update_link_status(), the code accesses the atomic state
holding only the link_mutex:

cdns_mhdp_update_link_status() {
    ...
    state = drm_priv_to_bridge_state(mhdp->bridge.base.state);
    ...
    cdns_bridge_state = to_cdns_mhdp_bridge_state(state);
    ...
    current_mode = cdns_bridge_state->current_mode;
}

Since drm_private_obj.state is managed by the DRM atomic framework, if a
concurrent non-blocking atomic commit completes during this poll, the old
state can be swapped and freed asynchronously by
drm_atomic_state_default_clear(), leaving this thread dereferencing freed
memory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260802153825.1570435-1-y-d@ti.com?part=2