Am Dienstag, dem 28.06.2022 um 09:16 +0200 schrieb 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.
At least in my testing the condition used in this check of RX sense on
all lanes going low is very reliable. During plugin/-out sometimes the
RX sense status of some of lanes bounces a bit, but all of them going
low is a very reliable indicator.
>
> >
> > Signed-off-by: Lucas Stach <l.st...@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 <narmstr...@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.
For reference, I tested this change on i.MX6 (Synopsys PHY) and i.MX8MP
(Samsung PHY).
Regards,
Lucas
