diff mbox series

[v9,20/61] drm/bridge: dw-hdmi-qp: Provide .{enable,disable}_hpd() PHY ops

Message ID 20260723-dw-hdmi-qp-scramb-v9-20-4fb12ea22ac9@collabora.com (mailing list archive)
State New
Headers
Series Add HDMI 2.0 support to DW HDMI QP TX |

Commit Message

Cristian Ciocaltea July 23, 2026, 1:35 a.m. UTC 
Implement the .hpd_enable() and .hpd_disable() bridge callbacks and
extend dw_hdmi_qp_phy_ops with corresponding hooks.

This enables the DRM core to control when HPD interrupts are armed,
which is needed to prevent premature interrupt delivery before the
connector is fully initialized, and to properly quiesce HPD during
suspend.

Tested-by: Maud Spierings <maud_spierings@hotmail.com>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 18 ++++++++++++++++++
 include/drm/bridge/dw_hdmi_qp.h              |  2 ++
 2 files changed, 20 insertions(+)

Comments

sashiko-bot@kernel.org July 23, 2026, 1:52 a.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The patch fails to achieve its stated goal of preventing premature HPD interrupt delivery because the underlying PHY driver was not updated and HPD remains statically armed.
- [Medium] The bridge implements `.hpd_enable` but never calls `drm_bridge_hpd_notify()`, bypassing the DRM core's hot-plug notification callback.
--

--- Patch [20]: [PATCH v9 20/61] drm/bridge: dw-hdmi-qp: Provide .{enable,disable}_hpd() PHY ops ---
commit 1e237fa22e4f4ffd6fd8e88a7191823f910b5d7a
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/bridge: dw-hdmi-qp: Provide .{enable,disable}_hpd() PHY ops

This commit implements the .hpd_enable() and .hpd_disable() bridge callbacks
for the dw-hdmi-qp driver and extends dw_hdmi_qp_phy_ops with corresponding
hooks. The goal is to allow the DRM core to control when HPD interrupts are
armed to prevent premature delivery.

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
> index 7475b6b71836b..a0613f0d2ebcf 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
> @@ -1189,6 +1189,22 @@ static int dw_hdmi_qp_cec_transmit(struct drm_bridge *bridge, u8 attempts,
>  #define dw_hdmi_qp_cec_transmit NULL
>  #endif /* CONFIG_DRM_DW_HDMI_QP_CEC */
>  
> +static void dw_hdmi_qp_bridge_hpd_enable(struct drm_bridge *bridge)
> +{
> +	struct dw_hdmi_qp *hdmi = bridge->driver_private;
> +
> +	if (hdmi->phy.ops->enable_hpd)
> +		hdmi->phy.ops->enable_hpd(hdmi, hdmi->phy.data);
> +}

[Severity: Medium]
The commit message states this change enables the DRM core to control when HPD
interrupts are armed to prevent premature interrupt delivery.

Does this actually prevent premature delivery? Looking at dw_hdmi_qp_init_hw(),
it still unconditionally calls the setup_hpd hook, which statically unmasks
the HPD interrupt before the DRM core assumes control.

Additionally, if the underlying PHY driver hasn't been updated to implement
these new enable_hpd/disable_hpd hooks, won't these callbacks just be silent
no-ops, leaving HPD statically armed?

[Severity: Medium]
By implementing .hpd_enable, the driver opts into the dynamic HPD
notification API.

This API contract expects the driver to call drm_bridge_hpd_notify() when an
HPD event occurs, so the DRM core can execute its registered callback. Since
dw-hdmi-qp.c neither calls drm_bridge_hpd_notify() nor provides a way for the
platform wrapper to trigger it, won't the DRM core's hot-plug notification
callback remain unexecuted?

[ ... ]
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
index 7475b6b71836..a0613f0d2ebc 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
@@ -1189,6 +1189,22 @@  static int dw_hdmi_qp_cec_transmit(struct drm_bridge *bridge, u8 attempts,
 #define dw_hdmi_qp_cec_transmit NULL
 #endif /* CONFIG_DRM_DW_HDMI_QP_CEC */
 
+static void dw_hdmi_qp_bridge_hpd_enable(struct drm_bridge *bridge)
+{
+	struct dw_hdmi_qp *hdmi = bridge->driver_private;
+
+	if (hdmi->phy.ops->enable_hpd)
+		hdmi->phy.ops->enable_hpd(hdmi, hdmi->phy.data);
+}
+
+static void dw_hdmi_qp_bridge_hpd_disable(struct drm_bridge *bridge)
+{
+	struct dw_hdmi_qp *hdmi = bridge->driver_private;
+
+	if (hdmi->phy.ops->disable_hpd)
+		hdmi->phy.ops->disable_hpd(hdmi, hdmi->phy.data);
+}
+
 static const struct drm_bridge_funcs dw_hdmi_qp_bridge_funcs = {
 	.atomic_get_output_bus_fmts = drm_atomic_helper_bridge_get_hdmi_output_bus_fmts,
 	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
@@ -1197,6 +1213,8 @@  static const struct drm_bridge_funcs dw_hdmi_qp_bridge_funcs = {
 	.atomic_enable = dw_hdmi_qp_bridge_atomic_enable,
 	.atomic_disable = dw_hdmi_qp_bridge_atomic_disable,
 	.detect = dw_hdmi_qp_bridge_detect,
+	.hpd_enable = dw_hdmi_qp_bridge_hpd_enable,
+	.hpd_disable = dw_hdmi_qp_bridge_hpd_disable,
 	.edid_read = dw_hdmi_qp_bridge_edid_read,
 	.hdmi_tmds_char_rate_valid = dw_hdmi_qp_bridge_tmds_char_rate_valid,
 	.hdmi_clear_avi_infoframe = dw_hdmi_qp_bridge_clear_avi_infoframe,
diff --git a/include/drm/bridge/dw_hdmi_qp.h b/include/drm/bridge/dw_hdmi_qp.h
index 6ea9c561cfef..b80fceffc315 100644
--- a/include/drm/bridge/dw_hdmi_qp.h
+++ b/include/drm/bridge/dw_hdmi_qp.h
@@ -17,6 +17,8 @@  struct dw_hdmi_qp_phy_ops {
 	void (*disable)(struct dw_hdmi_qp *hdmi, void *data);
 	enum drm_connector_status (*read_hpd)(struct dw_hdmi_qp *hdmi, void *data);
 	void (*setup_hpd)(struct dw_hdmi_qp *hdmi, void *data);
+	void (*enable_hpd)(struct dw_hdmi_qp *hdmi, void *data);
+	void (*disable_hpd)(struct dw_hdmi_qp *hdmi, void *data);
 };
 
 struct dw_hdmi_qp_plat_data {