Re: [Intel-gfx] [PATCH v3 6/6] drm/i915/dp: Get TC link reference during DP detection

["Souza, Jose" <jose.souza@intel.com>]

On Sat, 2020-03-28 at 12:26 +0200, Imre Deak wrote:
> On Tue, Mar 24, 2020 at 01:14:29PM -0700, José Roberto de Souza
> wrote:
> > As now the cost to lock and use a TC port is higher due the
> > implementation of the TCCOLD sequences it is worty to hold a
> > reference
> > of the TC port to avoid all this locking at every aux transaction
> > part of the DisplayPort detection.
> 
> The problem with locking the port for detection is that it would
> block
> modesets on the port, which we should avoid. By blocking tc-cold

It will not block modesets on the port, intel_tc_port_get_link and
intel_tc_port_put_link gets locks tc_lock, increment or decrement
tc_link_refcount and then unlock, it would only avoid the TC cold
sequences over and over.

> whenever enabling an AUX power well you would avoid the overhead of
> the
> PCODE requests for each AUX transfer, since the AUX power refs are
> dropped asynchronously with a delay.

Left comments on your proposal in patch 1.

> 
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Cooper Chiou <cooper.chiou@intel.com>
> > Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 19 ++++++++++++++-----
> >  1 file changed, 14 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 7f1a4e55cda1..6fbf3beee544 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -6041,6 +6041,7 @@ intel_dp_detect(struct drm_connector
> > *connector,
> >  	struct intel_dp *intel_dp =
> > intel_attached_dp(to_intel_connector(connector));
> >  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> >  	struct intel_encoder *encoder = &dig_port->base;
> > +	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
> >  	enum drm_connector_status status;
> >  
> >  	drm_dbg_kms(&dev_priv->drm, "[CONNECTOR:%d:%s]\n",
> > @@ -6049,12 +6050,17 @@ intel_dp_detect(struct drm_connector
> > *connector,
> >  		    !drm_modeset_is_locked(&dev_priv-
> > >drm.mode_config.connection_mutex));
> >  
> >  	/* Can't disconnect eDP */
> > -	if (intel_dp_is_edp(intel_dp))
> > +	if (intel_dp_is_edp(intel_dp)) {
> >  		status = edp_detect(intel_dp);
> > -	else if (intel_digital_port_connected(encoder))
> > -		status = intel_dp_detect_dpcd(intel_dp);
> > -	else
> > -		status = connector_status_disconnected;
> > +	} else {
> > +		if (intel_phy_is_tc(dev_priv, phy))
> > +			intel_tc_port_get_link(dig_port, 1);
> > +
> > +		if (intel_digital_port_connected(encoder))
> > +			status = intel_dp_detect_dpcd(intel_dp);
> > +		else
> > +			status = connector_status_disconnected;
> > +	}
> >  
> >  	if (status == connector_status_disconnected) {
> >  		memset(&intel_dp->compliance, 0, sizeof(intel_dp-
> > >compliance));
> > @@ -6132,6 +6138,9 @@ intel_dp_detect(struct drm_connector
> > *connector,
> >  	if (status != connector_status_connected && !intel_dp->is_mst)
> >  		intel_dp_unset_edid(intel_dp);
> >  
> > +	if (intel_phy_is_tc(dev_priv, phy))
> > +		intel_tc_port_put_link(dig_port);
> > +
> >  	/*
> >  	 * Make sure the refs for power wells enabled during detect are
> >  	 * dropped to avoid a new detect cycle triggered by HPD
> > polling.
> > -- 
> > 2.26.0
> > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx