[Intel-gfx] [PATCH 2/2] drm/i915: Do not get aux power for disconnected DP ports

[Imre Deak]

On Wed, Sep 26, 2018 at 10:42:25PM +0000, Souza, Jose wrote:
> On Tue, 2018-09-25 at 15:17 +0300, Ville Syrjälä wrote:
> > On Mon, Sep 24, 2018 at 06:16:49PM -0700, José Roberto de Souza
> > wrote:
> > > For ICL type-c ports there is a aux power restriction, it can only
> > > be
> > > enabled while there is sink connected.
> > 
> > That can't be entirely true because it's super racy. I was talking
> > with Imre about this mess at some point, and IIRC we came up with
> > some half decent theories on how we might handle this mess in a
> > sane manner. IIRC it was something along the lines of: the tbt vs.
> > other mode knob defines which power well we can enable, and so
> > while we're doing something with the port in one mode or the other
> > we must not try to use the port in the other mode. Ie. we need to
> > properly track which mode the port is in and if there's an event
> > that requires switching modes we have to wait until the previous
> > use of the other mode has stopped.
> 
> I can't think how would we use the same TBT3 port in TBT3 and USB-C
> mode without going to the disconnection and connection flow.

There's no thunderbolt support added yet, for instance all the TypeC
ports are set up statically to work in USB-C mode. So a proper way to
switch from one mode to other with all the proper waits for any pending
connect/disconnect event needs to be implemented as described in BSpec.

The above also means that when switching from one mode to the other we
have to wait for any pending operation keeping the AUX power domain for
the other mode enabled (meaning detect and AUX transfers for any other
reason).

> This statment about aux power restriction is on BSec also in the past
> we were having some timeouts warnings while waiting for USB-C/TBT aux
> power wells to be turn or off, Imre fixed that by not adding
> POWER_DOMAIN_INIT to those power well domains.
> 
> > 
> > > 
> > > BSpec: 21750
> > > 
> > > Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c | 19 ++++++++++++++-----
> > >  1 file changed, 14 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index 6b4c19123f2a..48fd38cd4ba4 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -5019,19 +5019,27 @@ intel_dp_long_pulse(struct intel_connector
> > > *connector,
> > >  	struct intel_dp *intel_dp = intel_attached_dp(&connector-
> > > >base);
> > >  	enum drm_connector_status status;
> > >  	u8 sink_irq_vector = 0;
> > > +	bool got_aux_power;
> > >  
> > >  	WARN_ON(!drm_modeset_is_locked(&dev_priv-
> > > >drm.mode_config.connection_mutex));
> > >  
> > > +	/* Can't disconnect eDP */
> > > +	if (!intel_dp_is_edp(intel_dp) &&
> > > +	    !intel_digital_port_connected(&dp_to_dig_port(intel_dp)-
> > > >base)) {
> > > +		status = connector_status_disconnected;
> > > +		got_aux_power = false;
> > > +		goto port_disconnected;
> > > +	}
> > > +
> > >  	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
> > > +	got_aux_power = true;
> > >  
> > > -	/* Can't disconnect eDP */
> > >  	if (intel_dp_is_edp(intel_dp))
> > >  		status = edp_detect(intel_dp);
> > > -	else if
> > > (intel_digital_port_connected(&dp_to_dig_port(intel_dp)->base))
> > > -		status = intel_dp_detect_dpcd(intel_dp);
> > >  	else
> > > -		status = connector_status_disconnected;
> > > +		status = intel_dp_detect_dpcd(intel_dp);
> > >  
> > > +port_disconnected:
> > >  	if (status == connector_status_disconnected) {
> > >  		memset(&intel_dp->compliance, 0, sizeof(intel_dp-
> > > >compliance));
> > >  
> > > @@ -5122,7 +5130,8 @@ intel_dp_long_pulse(struct intel_connector
> > > *connector,
> > >  	if (status != connector_status_connected && !intel_dp->is_mst)
> > >  		intel_dp_unset_edid(intel_dp);
> > >  
> > > -	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> > > +	if (got_aux_power)
> > > +		intel_display_power_put(dev_priv, intel_dp-
> > > >aux_power_domain);
> > >  	return status;
> > >  }
> > >  
> > > -- 
> > > 2.19.0
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel