Add no-hpd property to the cadence bridge
Subsystems (4)
v5 cover Add no-hpd property to the cadence bridge
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/[email protected]/
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(-)
Signature · 1 line
2.34.1
PATCH v5 1/2 dt-bindings: display: bridge: cdns,mhdp8546: Add no-hpd property to the cadence bridge
From: Rahul T R <[email protected]>
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.
--- .../bindings/display/bridge/cdns,mhdp8546.yaml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml +17 -0
@@ -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
Signature · 1 line
2.34.1
Rob Herring (Arm)
maintainerRe: [PATCH v5 1/2] dt-bindings: display: bridge: cdns,mhdp8546: Add no-hpd property to the cadence bridge
Quoted 11 lines · Yashas D
From: Rahul T R <[email protected]>
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.
Quoted 7 lines
Signed-off-by Rahul T R <[email protected]>Signed-off-by Jayesh Choudhary <[email protected]>Signed-off-by Yashas D <[email protected]>--- .../bindings/display/bridge/cdns,mhdp8546.yaml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:./Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml:66:30: [error] syntax error: mapping values are not allowed here (syntax)
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml: ignoring, error parsing file ./Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml: error loading YAML: mapping values are not allowed here
in "./Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml", line 66, column 30./Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml:66:30: mapping values are not allowed here make[2]: *** Deleting file 'Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.example.dts' Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml:66:30: mapping values are not allowed here
make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.example.dts] Error 1 make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1703: dt_binding_check] Error 2
make: *** [Makefile:248: __sub-make] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.kernel.org/project/devicetree/patch/[email protected]
The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Re: Re: [PATCH v5 1/2] dt-bindings: display: bridge: cdns,mhdp8546: Add no-hpd property to the cadence bridge
Quoted 51 lines · Rob Herring (Arm)
On Tue, 28 Jul 2026 18: 12: 23 +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
Quoted 11 lines · Yashas D
From: Rahul T R <[email protected]>
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.Quoted 7 lines
Signed-off-by Rahul T R <[email protected]>Signed-off-by Jayesh Choudhary <[email protected]>Signed-off-by Yashas D <[email protected]>--- .../bindings/display/bridge/cdns,mhdp8546.yaml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)My bot found errors running 'make
dt_binding_check' on your patch:yamllint warnings/errors:
./Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml:66:30: [error] syntax error: mapping values are not allowed here (syntax)dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml: ignoring, error parsing file./Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml: error loading YAML: mapping values are not allowed here
in "./Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml", line 66, column 30./Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml:66:30: mapping values are not allowed here make[2]: *** Deleting file 'Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.example.dts'Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml:66:30: mapping values are not allowed here
make[2]: *** [Documentation/devicetree/bindings/Makefile:26:Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.example.dts] Error 1 make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1703:dt_binding_check] Error 2
make: *** [Makefile:248: __sub-make] Error 2doc reference errors (make refcheckdocs):
Seehttps://urldefense.com/v3/__https://patchwork.kernel.org/project/ devicetree/patch/[email protected]__;!!G3vK!W- t7VTYjMiEiiyA5HnNwMPXZXIDl6mIiVFQjSxqG20XM1FTyTXYmNfyADL2cjwLH7WdrUA$ <https://urldefense.com/v3/__https://patchwork.kernel.org/project/devicetree/patch/[email protected]__;!!G3vK!W-t7VTYjMiEiiyA5HnNwMPXZXIDl6mIiVFQjSxqG20XM1FTyTXYmNfyADL2cjwLH7WdrUA$>
The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch.
If you already ran 'make
dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:
Quoted 6 lines
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note that
DT_SCHEMA_FILEScan be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Hi Rob,
My dt_binding_check had passed, will upgrade and run it again and send a new patch.
Thanks,
Yashas D
Re: [PATCH v5 1/2] dt-bindings: display: bridge: cdns,mhdp8546: Add no-hpd property to the cadence bridge
Quoted 61 lines · Yashas D
Quoted 58 lines · Rob Herring (Arm)
On Tue, 28 Jul 2026 18: 12: 23 +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
Quoted 11 lines · Yashas D
From: Rahul T R <[email protected]>
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.Quoted 7 lines
Signed-off-by Rahul T R <[email protected]>Signed-off-by Jayesh Choudhary <[email protected]>Signed-off-by Yashas D <[email protected]>--- .../bindings/display/bridge/cdns,mhdp8546.yaml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)My bot found errors running 'make
dt_binding_check' on your patch:yamllint warnings/errors:
./Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml:66:30: [error] syntax error: mapping values are not allowed here (syntax)dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml: ignoring, error parsing file./Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml: error loading YAML: mapping values are not allowed here
in "./Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml", line 66, column 30./Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml:66:30: mapping values are not allowed here make[2]: *** Deleting file 'Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.example.dts'Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml:66:30: mapping values are not allowed here
make[2]: *** [Documentation/devicetree/bindings/Makefile:26:Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.example.dts] Error 1 make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1703:dt_binding_check] Error 2
make: *** [Makefile:248: __sub-make] Error 2doc reference errors (make refcheckdocs):
Seehttps://urldefense.com/v3/__https://patchwork.kernel.org/project/
devicetree/patch/[email protected]__;!!G3vK!W-
t7VTYjMiEiiyA5HnNwMPXZXIDl6mIiVFQjSxqG20XM1FTyTXYmNfyADL2cjwLH7WdrUA$ <https://urldefense.com/v3/__https://patchwork.kernel.org/project/devicetree/patch/[email protected]__;!!G3vK!W-t7VTYjMiEiiyA5HnNwMPXZXIDl6mIiVFQjSxqG20XM1FTyTXYmNfyADL2cjwLH7WdrUA$>The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch.
If you already ran 'make
dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:pip3 install dtschema --upgrade
Quoted 8 lines
Quoted 4 lines
Please check and re-submit after running the above command yourself. Note that
DT_SCHEMA_FILEScan be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.Hi Rob,
Mydt_binding_checkhad passed, will upgrade and run it again and send a new patch.
I doubt, code has obvious syntax issues...
Best regards,
Krzysztof
PATCH v5 2/2 drm: bridge: cdns-mhdp8546: Add no-hpd property
In reply to Yashas DFrom: Rahul T R <[email protected]>
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.
--- .../drm/bridge/cadence/cdns-mhdp8546-core.c | 58 ++++++++++++++++--- .../drm/bridge/cadence/cdns-mhdp8546-core.h | 1 + 2 files changed, 52 insertions(+), 7 deletions(-)
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +51 -7
@@ -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);
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h +1 -0
@@ -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
Signature · 1 line
2.34.1
[email protected]
AI reviewRe: [PATCH v5 2/2] drm: bridge: cdns-mhdp8546: Add no-hpd property
Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
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 <[email protected]>
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.
Quoted 4 lines
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +0
-0
[ ... ]
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c static int cdns_mhdp_fw_activate(const struct firmware *fw, · line 705
Quoted 8 lines
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +3
-1@@ -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?
[ ... ]
Quoted 9 lines
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +5
-0@@ -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
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c static int cdns_mhdp_attach(struct drm_bridge *bridge, · line 1549
Quoted 10 lines
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +10
-0+* 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.
[ ... ]
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c cdns_mhdp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connect · line 2047
Quoted 7 lines
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +3
-0@@ -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?
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c static int cdns_mhdp_update_link_status(struct cdns_mhdp_device *mhdp) · line 2136
Quoted 10 lines
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +2
-1return 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.
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c static int cdns_mhdp_update_link_status(struct cdns_mhdp_device *mhdp) · line 2147
Quoted 11 lines
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +8
-0+ 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?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2