[PATCH v6 19/22] drm: bridge: dw_hdmi: Rework HDP and RXSENSE interrupt handling

[Jonas Karlman]

On 5/16/2026 9:43 PM, sashiko-bot at kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Clearing the W1C HPD interrupt after reading the live status and setting the new polarity creates a race condition that can permanently break hotplug detection.
> --
> 
> commit 40973548d3ce7bfe1722c0d53b828f8ef0ca4d81
> Author: Jonas Karlman <jonas at kwiboo.se>
> 
> drm: bridge: dw_hdmi: Rework HDP and RXSENSE interrupt handling
> 
> This commit removes RXSENSE handling to simplify the HPD interrupt
> handling, depending instead on a delayed work to detect connection
> status or EDID changes. It also ensures the initial HPD interrupt
> polarity is based on the current HPD status.
> 
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> index 300dac4a6c358..39e94d22249b2 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> 
> [ ... ]
> 
>> @@ -3060,78 +3011,29 @@ static irqreturn_t dw_hdmi_hardirq(int irq, void *dev_id)
>>  
>>  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);
>> -	phy_stat = hdmi_readb(hdmi, HDMI_PHY_STAT0);
>> -
>> -	phy_pol_mask = 0;
>> -	if (intr_stat & HDMI_IH_PHY_STAT0_HPD)
>> -		phy_pol_mask |= HDMI_PHY_HPD;
>> -	if (intr_stat & HDMI_IH_PHY_STAT0_RX_SENSE0)
>> -		phy_pol_mask |= HDMI_PHY_RX_SENSE0;
>> -	if (intr_stat & HDMI_IH_PHY_STAT0_RX_SENSE1)
>> -		phy_pol_mask |= HDMI_PHY_RX_SENSE1;
>> -	if (intr_stat & HDMI_IH_PHY_STAT0_RX_SENSE2)
>> -		phy_pol_mask |= HDMI_PHY_RX_SENSE2;
>> -	if (intr_stat & HDMI_IH_PHY_STAT0_RX_SENSE3)
>> -		phy_pol_mask |= HDMI_PHY_RX_SENSE3;
>> -
>> -	if (phy_pol_mask)
>> -		hdmi_modb(hdmi, ~phy_int_pol, phy_pol_mask, HDMI_PHY_POL0);
>> +	u8 intr_stat;
>>  
>>  	/*
>> -	 * RX sense tells us whether the TDMS transmitters are detecting
>> -	 * load - in other words, there's something listening on the
>> -	 * other end of the link.  Use this to decide whether we should
>> -	 * power on the phy as HPD may be toggled by the sink to merely
>> -	 * ask the source to re-read the EDID.
>> +	 * Interrupt generation is accomplished in the following way:
>> +	 *   interrupt = (mask == 0) && (polarity == status)
>> +	 * All interrupts are forwarded to the Interrupt Handler sticky bit
>> +	 * register ih_phy_stat0 and muted using the register ih_mute_phy_stat0.
>>  	 */
>> -	if (intr_stat &
>> -	    (HDMI_IH_PHY_STAT0_RX_SENSE | HDMI_IH_PHY_STAT0_HPD)) {
>> -		dw_hdmi_setup_rx_sense(hdmi,
>> -				       phy_stat & HDMI_PHY_HPD,
>> -				       phy_stat & HDMI_PHY_RX_SENSE);
>> -
>> -		if ((intr_stat & HDMI_IH_PHY_STAT0_HPD) &&
>> -		    (phy_stat & HDMI_PHY_HPD))
>> -			status = connector_status_connected;
>> +	intr_stat = hdmi_readb(hdmi, HDMI_IH_PHY_STAT0);
>> +	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
>> +		enum drm_connector_status status;
>>  
>> -		if (!(phy_stat & (HDMI_PHY_HPD | HDMI_PHY_RX_SENSE)))
>> -			status = connector_status_disconnected;
>> -	}
>> +		/* Set HPD interrupt polarity based on current HPD status. */
>> +		status = dw_hdmi_phy_read_hpd(hdmi, hdmi->phy.data);
>> +		hdmi_modb(hdmi, status == connector_status_connected ?
>> +			  0 : HDMI_PHY_HPD, HDMI_PHY_HPD, HDMI_PHY_POL0);
>>  
>> -	if (status != connector_status_unknown) {
>>  		dev_dbg(hdmi->dev, "EVENT=%s\n",
>>  			status == connector_status_connected ?
>>  			"plugin" : "plugout");
>> @@ -3141,8 +3043,7 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
>>  	}
>>  
>>  	hdmi_writeb(hdmi, intr_stat, HDMI_IH_PHY_STAT0);
> 
> Can this sequence create a race condition that breaks hotplug detection?
> 
> If the HPD line toggles (for example, due to contact bounce) after
> dw_hdmi_phy_read_hpd() but before this final hdmi_writeb(), the hardware
> could generate a new interrupt and set the sticky bit again.
> 
> Since the final hdmi_writeb() writes the original intr_stat (which has the
> HPD bit set) back to the register, and the register is write-1-to-clear, it
> will clear this newly generated interrupt.

No, the sticky bit is muted, meaning that no interrupt should be generated
until this run has completed, the sticky bit is cleared and then unmuted.

Regards,
Jonas

> This could leave the driver and hardware out of sync, with the hardware
> waiting for a polarity transition that has already occurred, causing all
> future plug/unplug events to be ignored.
> 
> Should the interrupt status be cleared before reading the live state and
> updating the polarity to close this race window?
> 
>> -	hdmi_writeb(hdmi, ~(HDMI_IH_PHY_STAT0_HPD | HDMI_IH_PHY_STAT0_RX_SENSE),
>> -		    HDMI_IH_MUTE_PHY_STAT0);
>> +	hdmi_writeb(hdmi, ~HDMI_IH_PHY_STAT0_HPD, HDMI_IH_MUTE_PHY_STAT0);
>>  
>>  	return IRQ_HANDLED;
>>  }
>