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

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 <[email protected]>

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?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=20