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

[Jani Nikula <jani.nikula@intel.com>]

On Tue, 26 Feb 2019, Imre Deak <imre.deak@intel.com> 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@linux.intel.com>
>> Cc: Imre Deak <imre.deak@intel.com>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Signed-off-by: José Roberto de Souza <jose.souza@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 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?
>
> For HDMI we'd also need to handle this in intel_hdmi.c.
>
> 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.

Guang Bai also kept referencing a pathological customer test case which
we're not privy to.

BR,
Jani.

>
> --Imre
>
>> +		state = INTEL_HOTPLUG_RETRY;
>> +
>>  	return state;
>>  }
>>  
>> -- 
>> 2.20.1
>> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx