[PATCH] drm: bridge: dw_hdmi: only trigger hotplug event on real link state change

[Neil Armstrong]

Hi,

On 27/06/2022 14:47, Lucas Stach wrote:
> There are two events that signal a real change of the link state: HPD going
> high means the sink is newly connected or wants the source to re-read the
> EDID, RX sense going low is a indication that the link has been disconnected.
> 
> Ignore the other two events that also trigger interrupts, but don't need
> immediate attention: HPD going low does not necessarily mean the link has
> been lost and should not trigger a immediate read of the status. RX sense
> going high also does not require a detect cycle, as HPD going high is the
> right point in time to read the EDID.
> 
> Ignoring the negative HPD edge does make the detection much more robust
> against spurious link status changes due to EMI or marginal signal levels.

Fair enough, but it means RX Sense must be totally functional with this change.

> 
> Signed-off-by: Lucas Stach <l.stach at pengutronix.de>
> ---
>   drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 13 ++++++++-----
>   1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 3e1be9894ed1..24f991b5248d 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -3095,6 +3095,7 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
>   {
>   	struct dw_hdmi *hdmi = dev_id;
>   	u8 intr_stat, phy_int_pol, phy_pol_mask, phy_stat;
> +	enum drm_connector_status status = connector_status_unknown;
>   
>   	intr_stat = hdmi_readb(hdmi, HDMI_IH_PHY_STAT0);
>   	phy_int_pol = hdmi_readb(hdmi, HDMI_PHY_POL0);
> @@ -3133,13 +3134,15 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
>   			cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
>   			mutex_unlock(&hdmi->cec_notifier_mutex);
>   		}
> -	}
>   
> -	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
> -		enum drm_connector_status status = phy_int_pol & HDMI_PHY_HPD
> -						 ? connector_status_connected
> -						 : connector_status_disconnected;
> +		if (phy_stat & HDMI_PHY_HPD)
> +			status = connector_status_connected;
> +
> +		if (!( phy_stat & HDMI_PHY_RX_SENSE))
> +			status = connector_status_disconnected;
> +	}
>   
> +	if (status != connector_status_unknown) {
>   		dev_dbg(hdmi->dev, "EVENT=%s\n",
>   			status == connector_status_connected ?
>   			"plugin" : "plugout");

Reviewed-by: Neil Armstrong <narmstrong at baylibre.com>

It would be nice to have this tested on another platform using the Synopsys PHY (unlike Amlogic platforms)
like Renesas, Rockchip, Allwinner or Ingenic SoCs.

Neil