[PATCH v7 20/23] drm: bridge: dw_hdmi: Rework HDP and RXSENSE interrupt handling
Neil Armstrong
neil.armstrong at linaro.org
Wed May 20 02:59:53 PDT 2026
On 5/18/26 20:01, Jonas Karlman wrote:
> The commit aeac23bda87f ("drm: bridge/dw_hdmi: improve HDMI
> enable/disable handling") added use of PHY RXSENSE indications to avoid
> triggering a full enable/disable of the HDMI block when a sink use a HPD
> low voltage level pulse to indicate changes of the EDID.
>
> HDMI Specification Version 1.4b chapter 8.5 mentions:
>
> An HDMI Sink shall indicate any change to the contents of the E-EDID
> by driving a low voltage level pulse on the Hot Plug Detect pin. This
> pulse shall be at least 100 msec.
>
> A delayed work is now used to debounce reacting on a HPD low voltage
> level pulse when a sink changes the EDID. The delayed work triggers a
> hotplug uevent every time the connection status or EDID has changed.
>
> Remove RXSENSE handling to simplify the HPD interrupt handling and
> instead depend on the delayed work to detect any connection status or
> EDID changes.
>
> This also ensures the initial HPD interrupt polarity is based on current
> HPD status to avoid an unnecessary interrupt from being triggered
> immediately at probe or resume when a sink is connected.
I'm still puzzled of the removal of RX_SENSE entirely as v1, and I since the
rx_sense code is not easy to understand I don't have an opinion on that.
Can someone with more knowledge can comment on that ?
Neil
>
> Tested-by: Diederik de Haas <diederik at cknow-tech.com> # Rock64, RockPro64, Quartz64-B
> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
> ---
> v7: Remove clear of STAT0_RX_SENSE in dw_hdmi_remove() added in prior
> patch
> v6: Update commit message,
> Collect t-b tag
> v5: Add comment about interrupt generation
> v4: New patch
> ---
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 147 ++++------------------
> 1 file changed, 22 insertions(+), 125 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 270db58a0e7c..2e09bff5faf7 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -161,11 +161,7 @@ struct dw_hdmi {
> struct pinctrl_state *unwedge_state;
>
> struct mutex mutex; /* for state below */
> - enum drm_connector_force force; /* mutex-protected force state */
> struct drm_connector *curr_conn;/* current connector (only valid when !disabled) */
> - bool disabled; /* DRM has disabled our bridge */
> - bool rxsense; /* rxsense state */
> - u8 phy_mask; /* desired phy int mask settings */
> u8 mc_clkdis; /* clock disable register */
>
> spinlock_t audio_lock;
> @@ -196,14 +192,6 @@ const struct dw_hdmi_plat_data *dw_hdmi_to_plat_data(struct dw_hdmi *hdmi)
> }
> EXPORT_SYMBOL_GPL(dw_hdmi_to_plat_data);
>
> -#define HDMI_IH_PHY_STAT0_RX_SENSE \
> - (HDMI_IH_PHY_STAT0_RX_SENSE0 | HDMI_IH_PHY_STAT0_RX_SENSE1 | \
> - HDMI_IH_PHY_STAT0_RX_SENSE2 | HDMI_IH_PHY_STAT0_RX_SENSE3)
> -
> -#define HDMI_PHY_RX_SENSE \
> - (HDMI_PHY_RX_SENSE0 | HDMI_PHY_RX_SENSE1 | \
> - HDMI_PHY_RX_SENSE2 | HDMI_PHY_RX_SENSE3)
> -
> static inline void hdmi_writeb(struct dw_hdmi *hdmi, u8 val, int offset)
> {
> regmap_write(hdmi->regm, offset << hdmi->reg_shift, val);
> @@ -1702,36 +1690,25 @@ EXPORT_SYMBOL_GPL(dw_hdmi_phy_read_hpd);
> void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
> bool force, bool disabled, bool rxsense)
> {
> - u8 old_mask = hdmi->phy_mask;
> -
> - if (force || disabled || !rxsense)
> - hdmi->phy_mask |= HDMI_PHY_RX_SENSE;
> - else
> - hdmi->phy_mask &= ~HDMI_PHY_RX_SENSE;
> -
> - if (old_mask != hdmi->phy_mask)
> - hdmi_writeb(hdmi, hdmi->phy_mask, HDMI_PHY_MASK0);
> }
> EXPORT_SYMBOL_GPL(dw_hdmi_phy_update_hpd);
>
> void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data)
> {
> /*
> - * Configure the PHY RX SENSE and HPD interrupts polarities and clear
> - * any pending interrupt.
> + * Configure the PHY HPD interrupt polarity based on current HPD status
> + * and clear any pending interrupt.
> */
> - hdmi_writeb(hdmi, HDMI_PHY_HPD | HDMI_PHY_RX_SENSE, HDMI_PHY_POL0);
> - hdmi_writeb(hdmi, HDMI_IH_PHY_STAT0_HPD | HDMI_IH_PHY_STAT0_RX_SENSE,
> - HDMI_IH_PHY_STAT0);
> + hdmi_modb(hdmi, hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_HPD ?
> + 0 : HDMI_PHY_HPD, HDMI_PHY_HPD, HDMI_PHY_POL0);
> + hdmi_writeb(hdmi, HDMI_IH_PHY_STAT0_HPD, HDMI_IH_PHY_STAT0);
>
> /* Enable cable hot plug irq. */
> - hdmi_writeb(hdmi, hdmi->phy_mask, HDMI_PHY_MASK0);
> + hdmi_writeb(hdmi, ~HDMI_PHY_HPD, HDMI_PHY_MASK0);
>
> /* Clear and unmute interrupts. */
> - hdmi_writeb(hdmi, HDMI_IH_PHY_STAT0_HPD | HDMI_IH_PHY_STAT0_RX_SENSE,
> - HDMI_IH_PHY_STAT0);
> - 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_PHY_STAT0);
> + hdmi_writeb(hdmi, ~HDMI_IH_PHY_STAT0_HPD, HDMI_IH_MUTE_PHY_STAT0);
> }
> EXPORT_SYMBOL_GPL(dw_hdmi_phy_setup_hpd);
>
> @@ -2395,26 +2372,6 @@ static void dw_hdmi_poweroff(struct dw_hdmi *hdmi)
> }
> }
>
> -/*
> - * Adjust the detection of RXSENSE according to whether we have a forced
> - * connection mode enabled, or whether we have been disabled. There is
> - * no point processing RXSENSE interrupts if we have a forced connection
> - * state, or DRM has us disabled.
> - *
> - * We also disable rxsense interrupts when we think we're disconnected
> - * to avoid floating TDMS signals giving false rxsense interrupts.
> - *
> - * Note: we still need to listen for HPD interrupts even when DRM has us
> - * disabled so that we can detect a connect event.
> - */
> -static void dw_hdmi_update_phy_mask(struct dw_hdmi *hdmi)
> -{
> - if (hdmi->phy.ops->update_hpd)
> - hdmi->phy.ops->update_hpd(hdmi, hdmi->phy.data,
> - hdmi->force, hdmi->disabled,
> - hdmi->rxsense);
> -}
> -
> static enum drm_connector_status dw_hdmi_detect(struct dw_hdmi *hdmi)
> {
> enum drm_connector_status result;
> @@ -2512,9 +2469,7 @@ static void dw_hdmi_connector_force(struct drm_connector *connector)
> struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi, connector);
>
> mutex_lock(&hdmi->mutex);
> - hdmi->force = connector->force;
> hdmi->last_connector_result = connector->status;
> - dw_hdmi_update_phy_mask(hdmi);
> mutex_unlock(&hdmi->mutex);
>
> dw_hdmi_connector_status_update(hdmi, connector, connector->status);
> @@ -2932,10 +2887,8 @@ static void dw_hdmi_bridge_atomic_disable(struct drm_bridge *bridge,
> struct dw_hdmi *hdmi = bridge->driver_private;
>
> mutex_lock(&hdmi->mutex);
> - hdmi->disabled = true;
> hdmi->curr_conn = NULL;
> dw_hdmi_poweroff(hdmi);
> - dw_hdmi_update_phy_mask(hdmi);
> handle_plugged_change(hdmi, false);
> mutex_unlock(&hdmi->mutex);
> }
> @@ -2954,10 +2907,8 @@ static void dw_hdmi_bridge_atomic_enable(struct drm_bridge *bridge,
> mode = &drm_atomic_get_new_crtc_state(state, crtc)->adjusted_mode;
>
> mutex_lock(&hdmi->mutex);
> - hdmi->disabled = false;
> hdmi->curr_conn = connector;
> dw_hdmi_poweron(hdmi, connector, mode);
> - dw_hdmi_update_phy_mask(hdmi);
> handle_plugged_change(hdmi, true);
> mutex_unlock(&hdmi->mutex);
> }
> @@ -3060,78 +3011,29 @@ static irqreturn_t dw_hdmi_hardirq(int irq, void *dev_id)
>
> void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
> {
> - mutex_lock(&hdmi->mutex);
> -
> - if (!hdmi->force) {
> - /*
> - * If the RX sense status indicates we're disconnected,
> - * clear the software rxsense status.
> - */
> - if (!rx_sense)
> - hdmi->rxsense = false;
> -
> - /*
> - * Only set the software rxsense status when both
> - * rxsense and hpd indicates we're connected.
> - * This avoids what seems to be bad behaviour in
> - * at least iMX6S versions of the phy.
> - */
> - if (hpd)
> - hdmi->rxsense = true;
> -
> - dw_hdmi_update_phy_mask(hdmi);
> - }
> - mutex_unlock(&hdmi->mutex);
> }
> EXPORT_SYMBOL_GPL(dw_hdmi_setup_rx_sense);
>
> 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);
> + intr_stat = hdmi_readb(hdmi, HDMI_IH_PHY_STAT0);
> + if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
> + enum drm_connector_status status;
>
> - if ((intr_stat & HDMI_IH_PHY_STAT0_HPD) &&
> - (phy_stat & HDMI_PHY_HPD))
> - status = connector_status_connected;
> + /* 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 (!(phy_stat & (HDMI_PHY_HPD | 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");
> @@ -3141,8 +3043,7 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
> }
>
> hdmi_writeb(hdmi, intr_stat, HDMI_IH_PHY_STAT0);
> - 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;
> }
> @@ -3343,9 +3244,6 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
> hdmi->dev = dev;
> hdmi->sample_rate = 48000;
> hdmi->channels = 2;
> - hdmi->disabled = true;
> - hdmi->rxsense = true;
> - hdmi->phy_mask = (u8)~(HDMI_PHY_HPD | HDMI_PHY_RX_SENSE);
> hdmi->mc_clkdis = 0x7f;
> hdmi->last_connector_result = connector_status_disconnected;
>
> @@ -3599,8 +3497,7 @@ void dw_hdmi_remove(struct dw_hdmi *hdmi)
> /* Free, mute and clear phy interrupts */
> devm_free_irq(hdmi->dev, irq, hdmi);
> hdmi_writeb(hdmi, ~0, HDMI_IH_MUTE_PHY_STAT0);
> - hdmi_writeb(hdmi, HDMI_IH_PHY_STAT0_HPD | HDMI_IH_PHY_STAT0_RX_SENSE,
> - HDMI_IH_PHY_STAT0);
> + hdmi_writeb(hdmi, HDMI_IH_PHY_STAT0_HPD, HDMI_IH_PHY_STAT0);
>
> /* Cancel any pending hot plug work */
> cancel_delayed_work_sync(&hdmi->hpd_work);
More information about the Linux-rockchip
mailing list