[PATCH v4 0/2] drm: bridge: cdns-mhdp8546: Add support for no-hpd

[PATCH v4 0/2] drm: bridge: cdns-mhdp8546: Add support for no-hpd

[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 HPD signal is routed to SoC pin AA24
(MCASP1_ACLKX/DP0_HPD). This pin is muxed with the McASP1 audio bit
clock; selecting DP0_HPD breaks audio. There is no alternative pin
carrying the HPD signal which makes it impossible to use HPD
without loosing the audio capabilities.

When 'no-hpd' is set, DRM_BRIDGE_OP_HPD is omitted so the framework
falls back to polling .detect() every ~10 seconds. Monitor presence is
determined via AUX DPCD reads instead of firmware HPD status registers.
The .detect() callback drives cdns_mhdp_update_link_status() on each
poll to keep mhdp->plugged current. At attach time, the driver waits
for firmware to be ready before performing the initial AUX poll since
no interrupt will trigger it.

Changes since v3:
  - DPCD read log level is now dev_dbg only in no-hpd path; HPD path
    keeps dev_err 
  - Added blank line and code comment in cdns_mhdp_attach()
  - Property name changed to 'no-hpd' without vendor prefix
  - Cleaned up the manual polling in detect since the framework
    already does it when OP_HPD is disabled
  - HPD detection and notification to userspace and drm clients 
    now happen through the framework callbacks

Link to v3: https://lore.kernel.org/all/20250205115025.3133487-1-h-shenoy@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   | 58 ++++++++++++++++---
 .../drm/bridge/cadence/cdns-mhdp8546-core.h   |  1 +
 3 files changed, 63 insertions(+), 7 deletions(-)

-- 
2.34.1

[PATCH v4 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 v4 1/2] dt-bindings: display/bridge: cdns-mhdp8546: Add no-hpd property to the cadence bridge

[Krzysztof Kozlowski]

On Tue, Jun 30, 2026 at 03:56:09PM +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.

Subject prefixes: There is no such file cdns-mhdp8546. I already pointed
this out at v2. This is a nit, but I do not understand why the same
feedback has to be repeated third time (v3 also ignored it).

Please use subject prefixes matching the subsystem. You can get them for
example with 'git log --oneline -- DIRECTORY_OR_FILE' on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

Best regards,
Krzysztof

[PATCH v4 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 signal has a MUX conflict with audio
functionality. While the HPD pin is physically connected to GPIO0_18,
routing it to the SoC requires sacrificing audio capability due to pin
muxing constraints. This board-level hardware limitation necessitates
an alternative detection mechanism.

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 (e.g., audio) 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   | 58 ++++++++++++++++---
 .../drm/bridge/cadence/cdns-mhdp8546-core.h   |  1 +
 2 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index 36c07b71fe04..5e1bad8fc73e 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);
 
@@ -788,9 +792,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 +1532,26 @@ static int cdns_mhdp_attach(struct drm_bridge *bridge,
 
 	spin_unlock(&mhdp->start_lock);
 
+	if (mhdp->no_hpd) {
+		/*
+		 * In no-hpd mode there is no interrupt to signal firmware
+		 * readiness. The firmware loads asynchronously after probe(),
+		 * so we must wait here until the uCPU is running before
+		 * attempting the first AUX channel poll for monitor presence.
+		 */
+		ret = wait_event_timeout(mhdp->fw_load_wq,
+					 mhdp->hw_state == MHDP_HW_READY,
+					 msecs_to_jiffies(100));
+		if (ret == 0) {
+			dev_err(mhdp->dev, "%s: Timeout waiting for fw loading\n",
+				__func__);
+			return -ETIMEDOUT;
+		}
+
+		cdns_mhdp_update_link_status(mhdp);
+		return 0;
+	}
+
 	/* Enable SW event interrupts */
 	if (hw_ready)
 		cdns_mhdp_bridge_hpd_enable(bridge);
@@ -2013,6 +2042,9 @@ cdns_mhdp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connect
 {
 	struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge);
 
+	if (mhdp->no_hpd)
+		cdns_mhdp_update_link_status(mhdp);
+
 	return cdns_mhdp_detect(mhdp);
 }
 
@@ -2100,7 +2132,16 @@ 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) {
+		ret = drm_dp_dpcd_read_link_status(&mhdp->aux, status);
+		hpd_pulse = false;
+		if (ret < 0)
+			mhdp->plugged = false;
+		else
+			mhdp->plugged = true;
+	} else {
+		mhdp->plugged = cdns_mhdp_detect_hpd(mhdp, &hpd_pulse);
+	}
 
 	if (!mhdp->plugged) {
 		cdns_mhdp_link_down(mhdp);
@@ -2288,6 +2329,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 +2403,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 v4 2/2] drm: bridge: cdns-mhdp8546: Add no-hpd property

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Polling monitor presence using `drm_dp_dpcd_read_link_status()` reads from link training status registers (DP_LANE0_1_STATUS), which can disrupt the DP sink's state machine.
- [High] Calling `cdns_mhdp_update_link_status()` from the `.detect()` callback continuously re-programs live video registers during DRM polling.
- [High] Missing `drm_dp_aux_unregister` on the firmware load timeout error path in `cdns_mhdp_attach`.
- [Medium] In no-hpd mode, link status evaluation is bypassed, preventing recovery from main link failures.
- [Medium] A hardcoded 100ms timeout in `cdns_mhdp_attach` to wait for asynchronous firmware loading is too short and will cause initialization failures.
--

commit 2c6e286375fe2e77fec97f0ea17066c50894dd26
Author: Rahul T R <r-ravikumar@ti.com>

drm: bridge: cdns-mhdp8546: Add no-hpd property

This patch adds a 'no-hpd' property to support boards where the HPD line cannot
be used for hotplug detection. It implements a polling mechanism using AUX reads
to detect monitor presence when hardware HPD signals are unavailable.

> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> index 36c07b71fe04b..5e1bad8fc73ea 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
[ ... ]
> @@ -1523,6 +1532,26 @@ static int cdns_mhdp_attach(struct drm_bridge *bridge,
>  
>  	spin_unlock(&mhdp->start_lock);
>  
> +	if (mhdp->no_hpd) {
> +		/*
> +		 * In no-hpd mode there is no interrupt to signal firmware
> +		 * readiness. The firmware loads asynchronously after probe(),
> +		 * so we must wait here until the uCPU is running before
> +		 * attempting the first AUX channel poll for monitor presence.
> +		 */
> +		ret = wait_event_timeout(mhdp->fw_load_wq,
> +					 mhdp->hw_state == MHDP_HW_READY,
> +					 msecs_to_jiffies(100));

[Severity: Medium]
Will this 100ms timeout cause initialization failures?

Async firmware loading can routinely take longer than 100ms on a busy system or
when reading from slow storage.

> +		if (ret == 0) {
> +			dev_err(mhdp->dev, "%s: Timeout waiting for fw loading\n",
> +				__func__);
> +			return -ETIMEDOUT;

[Severity: High]
Does this error path leak the registered AUX channel?

Earlier in cdns_mhdp_attach(), the DRM DP AUX channel is registered using
drm_dp_aux_register(). Returning directly here skips the aux_unregister error
path label at the end of the function, leaving the AUX channel registered but
in an invalid state.

> +		}
> +
> +		cdns_mhdp_update_link_status(mhdp);
> +		return 0;
> +	}
> +
[ ... ]
> @@ -2100,7 +2132,16 @@ 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) {
> +		ret = drm_dp_dpcd_read_link_status(&mhdp->aux, status);

[Severity: High]
Is it safe to read link training status registers to poll for monitor presence?

drm_dp_dpcd_read_link_status() reads the DP_LANE0_1_STATUS (0x202) register.
According to the DRM subsystem guidelines, reading link training status
registers (0x202-0x207) outside of the expected training sequence can trigger
unintended state machine transitions in DP/eDP sinks. Could this cause hardware
state corruption or screen flickering?

> +		hpd_pulse = false;

[Severity: Medium]
Does hardcoding hpd_pulse to false bypass link retraining?

Later in cdns_mhdp_update_link_status(), link validation logic checks
if (hpd_pulse && old_plugged == mhdp->plugged). By hardcoding this to false,
that logic is skipped. If the main high-speed link loses synchronization but
the monitor remains connected, wouldn't the driver fail to retrain the link?

[Severity: High]
Does this also cause live video stream registers to be reprogrammed on every
polling event?

Since hpd_pulse is false, the function skips the early return path for a healthy
link (goto out;). It then falls through to the end of the function where it
unconditionally executes cdns_mhdp_sst_enable(mhdp, current_mode) if
mhdp->bridge_enabled is true.

When called via periodic DRM polling via the .detect() callback, won't this
cause severe screen flickering or sync loss by continuously overwriting active
hardware display timing registers?

> +		if (ret < 0)
> +			mhdp->plugged = false;
> +		else
> +			mhdp->plugged = true;
> +	} else {
> +		mhdp->plugged = cdns_mhdp_detect_hpd(mhdp, &hpd_pulse);
> +	}

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