Re: [Intel-gfx] [PATCH v4 08/23] drm/i915/adl_p: Handle TC cold

From: "Souza, Jose" <jose.souza@intel.com>
To: "Deak, Imre" <imre.deak@intel.com>
Cc: "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH v4 08/23] drm/i915/adl_p: Handle TC cold
Date: Mon, 17 May 2021 23:15:05 +0000	[thread overview]
Message-ID: <4b0abab8e08a1218ed776899a978b99cb49a4347.camel@intel.com> (raw)
In-Reply-To: <20210517145359.GA1367033@ideak-desk.fi.intel.com>

[-- Attachment #1: Type: text/plain, Size: 3451 bytes --]

On Mon, 2021-05-17 at 17:53 +0300, Imre Deak wrote:
> On Fri, May 14, 2021 at 08:10:20PM -0700, Matt Roper wrote:
> > From: José Roberto de Souza <jose.souza@intel.com>
> > 
> > On ADL-P TC cold is exited and blocked when legacy aux is powered,
> > that is exacly the same of what ICL need for static TC ports.
> > 
> > TODO: When a TBT hub or monitor is connected it will cause TBT and
> > legacy aux to be powered at the same time, hopefully this will not
> > cause any issues but if it do, some rework will be needed.
> > 
> > v2:
> >  - skip icl_tc_port_assert_ref_held() warn on, adl-p uses aux to
> >    block TC cold
> > 
> > BSpec: 55480
> > Cc: Imre Deak <imre.deak@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > Signed-off-by: Clinton Taylor <Clinton.A.Taylor@intel.com>
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display_power.c | 3 ++-
> >  drivers/gpu/drm/i915/display/intel_tc.c            | 2 +-
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> > index 54c6d65011ee..29d2f1d0cffd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -551,7 +551,8 @@ static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv,
> >  	if (drm_WARN_ON(&dev_priv->drm, !dig_port))
> >  		return;
> >  
> > -	if (DISPLAY_VER(dev_priv) == 11 && dig_port->tc_legacy_port)
> > +	if (IS_ALDERLAKE_P(dev_priv) ||
> 
> A TC port reference is held whenever enabling the port's AUX power
> domain (so whenever blocking TC-cold for instance), so this shouldn't be
> needed.

I'm kinda of rusty in this area so I could be making a mistake but I don't see how tc_lock is held or tc_link_refcount is != 0 when enabling aux to do
a DPCD transaction when the port is no driving image to the sink.
Will do some experiments and update here.

> 
> OTOH, the !aux_powered check in intel_tc_port_reset_mode() needs this
> exception, since there TC-cold must be blocked and so AUX will be
> enabled as opposed to other platforms.
> 
> Also, in icl_tc_phy_aux_power_well_enable() we need to avoid the power
> well enabling timeout error message, since it won't get enabled unless
> something is actually plugged to the TC connector.

Agree with both above, planning to squash patch.txt to fix it and make sure we don't have similar issues with future platforms.

> 
> > +	    (DISPLAY_VER(dev_priv) == 11 && dig_port->tc_legacy_port))
> >  		return;
> >  
> >  	drm_WARN_ON(&dev_priv->drm, !intel_tc_port_ref_held(dig_port));
> > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
> > index e325463acddd..1b108dea5fed 100644
> > --- a/drivers/gpu/drm/i915/display/intel_tc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> > @@ -28,7 +28,7 @@ tc_cold_get_power_domain(struct intel_digital_port *dig_port)
> >  {
> >  	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> >  
> > -	if (DISPLAY_VER(i915) == 11)
> > +	if (IS_ALDERLAKE_P(i915) || DISPLAY_VER(i915) == 11)
> >  		return intel_legacy_aux_to_power_domain(dig_port->aux_ch);
> >  	else
> >  		return POWER_DOMAIN_TC_COLD_OFF;
> > -- 
> > 2.25.4
> > 


[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 3079 bytes --]

commit 2119a9417715932031fcf50e7c76f158febc6c7c
Author: JosÃ© Roberto de Souza <jose.souza@intel.com>
Date:   Mon May 17 16:11:22 2021 -0700

    squash

diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index feab8468d272..cf6488a33192 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -620,11 +620,9 @@ icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
 	 * or need to enable AUX on a legacy TypeC port as part of the TC-cold
 	 * exit sequence.
 	 */
-	timeout_expected = is_tbt;
-	if (DISPLAY_VER(dev_priv) == 11 && dig_port->tc_legacy_port) {
+	timeout_expected = is_tbt || intel_tc_cold_requires_aux_pw(dig_port);
+	if (DISPLAY_VER(dev_priv) == 11 && dig_port->tc_legacy_port)
 		icl_tc_cold_exit(dev_priv);
-		timeout_expected = true;
-	}
 
 	hsw_wait_for_power_well_enable(dev_priv, power_well, timeout_expected);
 
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
index 2af4d8d5a983..3c23346aeef3 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.c
+++ b/drivers/gpu/drm/i915/display/intel_tc.c
@@ -26,9 +26,7 @@ static const char *tc_port_mode_name(enum tc_port_mode mode)
 static enum intel_display_power_domain
 tc_cold_get_power_domain(struct intel_digital_port *dig_port)
 {
-	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
-
-	if (IS_ALDERLAKE_P(i915) || DISPLAY_VER(i915) == 11)
+	if (intel_tc_cold_requires_aux_pw(dig_port))
 		return intel_legacy_aux_to_power_domain(dig_port->aux_ch);
 	else
 		return POWER_DOMAIN_TC_COLD_OFF;
@@ -454,7 +452,7 @@ static void intel_tc_port_reset_mode(struct intel_digital_port *dig_port,
 	enum tc_port_mode old_tc_mode = dig_port->tc_mode;
 
 	intel_display_power_flush_work(i915);
-	if (DISPLAY_VER(i915) != 11 || !dig_port->tc_legacy_port) {
+	if (!intel_tc_cold_requires_aux_pw(dig_port)) {
 		enum intel_display_power_domain aux_domain;
 		bool aux_powered;
 
@@ -669,3 +667,15 @@ void intel_tc_port_init(struct intel_digital_port *dig_port, bool is_legacy)
 	dig_port->tc_link_refcount = 0;
 	tc_port_load_fia_params(i915, dig_port);
 }
+
+bool intel_tc_cold_requires_aux_pw(struct intel_digital_port *dig_port)
+{
+	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+	enum phy phy = intel_port_to_phy(i915, dig_port->base.port);
+
+	if (!intel_phy_is_tc(i915, phy))
+		return false;
+
+	return (DISPLAY_VER(i915) == 11 && dig_port->tc_legacy_port) ||
+		IS_ALDERLAKE_P(i915);
+}
diff --git a/drivers/gpu/drm/i915/display/intel_tc.h b/drivers/gpu/drm/i915/display/intel_tc.h
index b619e4736f85..0eacbd76ec15 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.h
+++ b/drivers/gpu/drm/i915/display/intel_tc.h
@@ -29,4 +29,6 @@ bool intel_tc_port_ref_held(struct intel_digital_port *dig_port);
 
 void intel_tc_port_init(struct intel_digital_port *dig_port, bool is_legacy);
 
+bool intel_tc_cold_requires_aux_pw(struct intel_digital_port *dig_port);
+
 #endif /* __INTEL_TC_H__ */

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
