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

[PATCH v5 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

Limitations:
- DP "attention" IRQs from sink will not be received
- Hotplug detection latency increases to ~10 seconds (polling interval)

This follows the same pattern as 'no-hpd' in ti-sn65dsi86 and panel-edp
bindings, providing consistent semantics across the DRM subsystem.

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 504a3186ebb3..73d5c718c7db 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

[PATCH v5 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 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 v4:
- Corrected the dt-binding Subject

Link to v4: https://lore.kernel.org/all/20260630102610.1849902-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         | 17 ++++++
 .../drm/bridge/cadence/cdns-mhdp8546-core.c   | 58 ++++++++++++++++---
 .../drm/bridge/cadence/cdns-mhdp8546-core.h   |  1 +
 3 files changed, 69 insertions(+), 7 deletions(-)

-- 
2.34.1

[PATCH v5 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  | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml
index c2b369456e4e..f19a4900ecd8 100644
--- a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml
@@ -57,6 +57,23 @@ 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 (e.g., audio) on the SoC,
+        making it unavailable for hotplug detection
+      - Hardware design where HPD cannot reliably detect monitor presence
+
+      When this property is set, the driver will use auxiliary channel (AUX)
+      DPCD reads to detect monitor connection instead of relying on the
+      hardware HPD signal. Monitor detection will use polling rather than
+      interrupt-driven detection.
+
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
 
-- 
2.34.1

[PATCH v5 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

Limitations:
- DP "attention" IRQs from sink will not be received
- Hotplug detection latency increases to ~10 seconds (polling interval)

This follows the same pattern as 'no-hpd' in ti-sn65dsi86 and panel-edp
bindings, providing consistent semantics across the DRM subsystem.

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 504a3186ebb3..73d5c718c7db 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 v5 2/2] drm: bridge: cdns-mhdp8546: Add no-hpd property

[sashiko-bot]

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

New issues:
- [High] Arbitrary 100ms synchronous timeout in `cdns_mhdp_attach()` for asynchronous firmware loading permanently fails DRM display initialization.
- [High] Disabling `cdns_mhdp_bridge_hpd_enable()` in `no-hpd` mode unintentionally masks all software event interrupts, breaking HDCP.
- [High] Returning `-ETIMEDOUT` directly on firmware load timeout in `cdns_mhdp_attach()` leaks the AUX device and leaves the bridge in an invalid state.
- [High] Periodic `.detect()` polling continuously reprograms display hardware stream registers every 10 seconds.
- [Medium] Polling monitor presence using the DP_LANE0_1_STATUS (0x202) register triggers unintended DP sink state transitions.

Pre-existing issues:
- [Critical] Unsafe unlocked access to `bridge.base.state` inside `cdns_mhdp_update_link_status()`.
- [High] The driver never powers down the DP link on disconnect because it checks the newly updated disconnected state instead of the previous state.
--

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

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 504a3186ebb3a..73d5c718c7db5 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 this function unintentionally disable other features like HDCP?

Despite the name cdns_mhdp_bridge_hpd_enable(), it unmasks the
CDNS_APB_INT_MASK_SW_EVENT_INT interrupt. This interrupt handles all software
events from the firmware, not just HPD.

Without it, will operations waiting for software events, such as
cdns_mhdp_wait_for_sw_event(), silently timeout and fail?

[ ... ]
> @@ -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;
> +		}

[Severity: High]
Will this hard-coded 100ms timeout cause permanent probe failures?

Since the firmware loads asynchronously, file system latency or user-space
helper delays could easily exceed 100ms. Returning -ETIMEDOUT directly bypasses
the standard -EPROBE_DEFER mechanism and permanently aborts the display
initialization pipeline.

[Severity: High]
Could this direct error return leak resources and cause a use-after-free?

Earlier in cdns_mhdp_attach(), the code registers the AUX device and sets
mhdp->bridge_attached to true:
    drm_dp_aux_register(&mhdp->aux);
    ...
    mhdp->bridge_attached = true;

Returning -ETIMEDOUT directly bypasses the goto aux_unregister cleanup path.

Additionally, it leaves mhdp->bridge_attached set to true. When the
asynchronous firmware load eventually completes, cdns_mhdp_fw_cb() will see
bridge_attached == true and call drm_bridge_hpd_notify() on a bridge that
failed to attach, which might lead to a crash.

[ ... ]
> @@ -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);
> +

[Severity: High]
Could calling cdns_mhdp_update_link_status() directly from the detect
callback cause display flickering or tearing?

The DRM framework polls the detect callback every 10 seconds. Inside
cdns_mhdp_update_link_status(), if the monitor is connected and the bridge
is enabled, it unconditionally calls cdns_mhdp_sst_enable().

This reprograms the hardware stream registers (video timing, TU sizes, MSA)
on an active stream. Will this disrupt the display periodically during
routine polling?

>  	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);

[Severity: Medium]
Is it safe to read the link status register for periodic presence detection?

drm_dp_dpcd_read_link_status() reads from DP_LANE0_1_STATUS (0x202), which
is in the link training status range.

Reading these registers outside of the link training sequence can trigger
unintended state machine transitions in DP sinks, potentially causing screen
flickering or link training failures.

> +		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);

[Severity: High]
This is a pre-existing issue, but does cdns_mhdp_link_down() fail to power
down the DP link when disconnected?

When a monitor is disconnected, the connection state is updated:
    mhdp->plugged = false;

Then the code enters the disconnect path and calls cdns_mhdp_link_down():
    if (!mhdp->plugged) {
        cdns_mhdp_link_down(mhdp);

Inside cdns_mhdp_link_down(), the power down is guarded by the plugged state:
    if (mhdp->plugged)
        drm_dp_link_power_down(&mhdp->aux, mhdp->link.revision);

Since mhdp->plugged was just cleared, it appears the link is never actually
powered down.

[Severity: Critical]
This is a pre-existing issue, but is it safe to access the atomic bridge
state without holding the necessary DRM modeset locks here?

Later in cdns_mhdp_update_link_status(), the code accesses the state pointer:
    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;

Because cdns_mhdp_update_link_status() is now called periodically from the
unlocked .detect() callback, could an atomic commit concurrently swap and free
the bridge state, leading to a use-after-free?

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `no-hpd` property is documented in the device tree bindings but has no corresponding implementation in the driver.
--

--- Patch [1]: [PATCH v5 1/2] dt-bindings: display: bridge: cdns, mhdp8546: Add no-hpd property to the cadence bridge ---
commit 4ac60fbbcafba148711703269045306014cd935a
Author: Rahul T R <r-ravikumar@ti.com>

dt-bindings: display: bridge: cdns, mhdp8546: Add no-hpd property to the cadence bridge

This commit introduces the no-hpd property to the mhdp8546 device tree
bindings. It provides a way to indicate when the hardware HPD line is not
available, specifying that the driver should fall back to auxiliary channel
DPCD reads for hotplug detection.

> diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml
> --- a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml

[ ... ]

> @@ -57,6 +57,23 @@ 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 (e.g., audio) on the SoC,
> +        making it unavailable for hotplug detection
> +      - Hardware design where HPD cannot reliably detect monitor presence
> +
> +      When this property is set, the driver will use auxiliary channel (AUX)
> +      DPCD reads to detect monitor connection instead of relying on the
> +      hardware HPD signal. Monitor detection will use polling rather than
> +      interrupt-driven detection.
> +

[Severity: Medium]
Is the driver implementation missing for this new property?

The binding documentation states that the driver will use polling rather than
interrupt-driven detection when this property is set. However, looking at the
driver in cdns_mhdp_probe(), it unconditionally requires hardware HPD
operations:

drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c:cdns_mhdp_probe() {
	...
	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.type = DRM_MODE_CONNECTOR_DisplayPort;
	...
}

Does the driver need logic to parse the no-hpd property and conditionally
adjust these operations to enable polling instead of enforcing hardware HPD?

>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
>

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

Re: [PATCH v5 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] Reading `DP_LANE0_1_STATUS` (0x202) for connection polling violates DRM guidelines and disrupts link training state machines.
- [High] The `.detect()` callback unconditionally triggers hardware stream re-enablement during periodic polling in `no-hpd` mode.
- [High] Resource leak of `drm_dp_aux` if firmware loading times out during `cdns_mhdp_attach()`, leading to a Use-After-Free.
- [Medium] Hardcoded 100ms timeout for asynchronous firmware loading in `cdns_mhdp_attach()` can spuriously fail bridge attachment.
- [Medium] Changing DPCD read failures to `dev_err()` will flood the kernel log with false-positive errors during normal operations.
--

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

This commit adds a 'no-hpd' property to support boards where the HPD line
cannot be used for hotplug detection due to hardware limitations. When set,
the driver uses auxiliary channel (AUX) DPCD reads to detect monitor presence
by polling the connection status via the .detect() callback.

> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
[ ... ]
> @@ -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);

[Severity: Medium]
Will this change spam the kernel log with false-positive errors during normal
operations?

DP AUX channel transactions routinely fail during normal operations, such as
when a disconnected monitor is probed, during DP sleep states, or when the
DRM DP helper retries transient link training failures. Emitting a dev_err()
on every AUX read failure might flood the kernel log on standard hardware.

[ ... ]
> @@ -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]
Could this hardcoded 100ms timeout spuriously fail bridge attachment?

Firmware loading from a filesystem (via request_firmware_nowait) can easily
take hundreds of milliseconds or longer depending on storage performance and
system load. If the timeout expires prematurely, the driver aborts with
-ETIMEDOUT, which could permanently break the display pipeline for that boot.

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

[Severity: High]
Does this direct return leak the drm_dp_aux resource?

If the firmware loading timeout expires, this returns -ETIMEDOUT directly
instead of jumping to the aux_unregister error handling label.

Since drm_dp_aux_register() sets up an I2C adapter and the DRM core does not
call .detach() if attach fails, bypassing drm_dp_aux_unregister() leaks the
I2C adapter. Because the memory backing the drm_dp_aux structure is
devm-managed, unloading the module frees the memory while the adapter is still
registered, which can cause a use-after-free when the I2C subsystem accesses
it.

> +		}
> +
> +		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]
Does reading the link status here violate DRM subsystem guidelines and disrupt
link training state machines?

The DRM core polls .detect() every 10 seconds when no-hpd is set, which
calls cdns_mhdp_update_link_status(). This unconditionally calls
drm_dp_dpcd_read_link_status() which reads DP_LANE0_1_STATUS (0x202).

According to DRM guidelines, registers in the link training status range
(0x202-0x207) participate in the CR/EQ training state machine and are not
safe for arbitrary read access outside of a link training sequence. Reading
this register continuously for presence detection can induce hardware
instability, screen flickering, and link drops. Should a safe register like
DP_DPCD_REV (0x000) be used instead?

> +		hpd_pulse = false;

[Severity: High]
Does hardcoding hpd_pulse to false cause the active display stream to be
re-initialized on every 10-second poll?

In no-hpd mode, .detect() is polled every 10 seconds. Because hpd_pulse is
hardcoded to false here, the later logic bypasses the check that validates
link status without state mutation:

    if (hpd_pulse && old_plugged == mhdp->plugged) {
        ret = drm_dp_dpcd_read_link_status(&mhdp->aux, status);
        if (!ret && ...)
            goto out;
        ...
    }

Instead, execution falls through to the end of the function. If
mhdp->bridge_enabled is true, this causes cdns_mhdp_sst_enable() to be
called:

    if (mhdp->bridge_enabled) {
        ...
        cdns_mhdp_sst_enable(mhdp, current_mode);
    }

This repeatedly re-programs active stream and framer configuration registers
on every poll, which can cause the active video stream to reset and glitch
every 10 seconds.

> +		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/20260728125336.776116-1-y-d@ti.com?part=2