| Message ID | 20210515031035.2561658-9-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Alder Lake-P Support | expand |
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. 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. > + (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 >
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 > > 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__ */
On Mon, 2021-05-17 at 16:18 -0700, José Roberto de Souza wrote: > 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. Ops please ignore the above. Yep we don't need it, when coming from tc_cold_block() the mutex is held and when doing a DPCD transaction intel_tc_port_lock() is called before get the aux power well. So will drop it, thanks for pointing it out. > > > > > 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 > > > >
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) || + (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;