[Intel-gfx] [PATCH 2/2] drm/i915/icl: Probe again type-c connectors that failed

[Souza, Jose]

On Tue, 2019-02-26 at 16:08 +0200, Imre Deak wrote:
> On Fri, Feb 22, 2019 at 01:08:34PM -0800, José Roberto de Souza
> wrote:
> > Unpowered type-c dongles can take some time to boot and be
> > responsible, causing the probe to fail and sink never be detected
> > without further actions from userspace.
> > 
> > It was not a issue for older platforms because there was a hardware
> > bridge between DDI/DP ports and type-c controller adding a implicit
> > delay that hid this issue but ICL have type-c controllers
> > integrated
> > to the SOC bring this issue to users.
> > 
> > So here if the probe failed when coming from a IRQ it returns
> > INTEL_HOTPLUG_RETRY that will schedule another run of
> > i915_hotplug_work_func() after 1 second what is time enough for
> > those type-c dongles to boot.
> > 
> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Cc: Imre Deak <imre.deak at intel.com>
> > Cc: Jani Nikula <jani.nikula at intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index 1676a87f18cb..96bbcf5c9787 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -4056,6 +4056,8 @@ intel_ddi_hotplug(struct intel_encoder
> > *encoder,
> >  		  struct intel_connector *connector,
> >  		  bool irq_received)
> >  {
> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder-
> > >base);
> >  	struct drm_modeset_acquire_ctx ctx;
> >  	enum intel_hotplug_state state;
> >  	int ret;
> > @@ -4082,6 +4084,17 @@ intel_ddi_hotplug(struct intel_encoder
> > *encoder,
> >  	drm_modeset_acquire_fini(&ctx);
> >  	WARN(ret, "Acquiring modeset locks failed with %i\n", ret);
> >  
> > +	/*
> > +	 * Unpowered type-c dongles can take some time to boot and be
> > +	 * responsible, so here giving some type to those dongles to 

s/type/time
So I don't loose it when doing the next version.

> > power up
> > +	 * and then retrying the probe.
> > +	 */
> > +	if (state == INTEL_HOTPLUG_NOCHANGE &&
> > +	    connector->base.status != connector_status_connected &&
> > +	    irq_received && intel_port_is_tc(dev_priv, encoder->port)
> > &&
> > +	    !dig_port->tc_legacy_port && !dig_port->dp.is_mst)
> 
> Based on the investigation by Jani et al, we have the similar problem
> with
> HDMI, only during disconnect. So I think we could generalize by
> retrying
> any time there is no change (except for MST where the driver always
> keeps
> the connector in a disconnected state), regardless of the type of the
> sink, since a no-change is suspect in any case: why would the sink
> signal
> (with a long pulse) if there is no change?

The only case that I can think of is the cases were sink do a short
pulse but we don't handle it in the short pulse handler and schedule a
long pulse handler but it should not cause any drawback retry everytime
there is no change in the state and irq_received is set.

What comment could we add about the HDMI here? Something like this
would be fine?

"HDMI sinks are reported as connected by hardware right after unpluging
it, so here also giving some time for hardware to process the unplug so
driver can read it and do the unplug sequence and notify userspace
about the absence of the HDMI sink"

> 
> For HDMI we'd also need to handle this in intel_hdmi.c.

It happens in older gens that don't have DDI? Otherwise just the update
above should take care of DP and HDMI DDI ports.

> 
> Then Ville suggested to add a Chamelium test for this to IGT, could
> you
> Jose look into that as well? Both the connect and disconnect races
> could
> be tested, both on HDMI and DP, by generating the HPD early/late wrt.
> to
> AUX/DDC starting/stopping to return valid data. I don't know if
> Chamelium can do this, you'd have to find that out first.

I will try put my hands in a Chamelium board otherwise I will play with
trybot to add this tests.

> 
> --Imre
> 
> > +		state = INTEL_HOTPLUG_RETRY;
> > +
> >  	return state;
> >  }
> >  
> > -- 
> > 2.20.1
> > 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20190228/67cd97d5/attachment.sig>