diff mbox series

[v2,7/8] drm/i915/tc: Catch TC users accessing FIA registers without enable aux

Message ID 20200407011157.362092-7-jose.souza@intel.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/8] drm/i915/display: Move out code to return the digital_port of the aux ch | expand

Commit Message

Souza, Jose April 7, 2020, 1:11 a.m. UTC 
As described in "drm/i915/tc/icl: Implement TC cold sequences" users
of TC functions should held aux power well during access to avoid
read garbage due HW in TC cold state.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_tc.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

Comments

Imre Deak April 7, 2020, 4:04 p.m. UTC | #1 
On Mon, Apr 06, 2020 at 06:11:56PM -0700, José Roberto de Souza wrote:
> As described in "drm/i915/tc/icl: Implement TC cold sequences" users
> of TC functions should held aux power well during access to avoid
> read garbage due HW in TC cold state.
> 
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_tc.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
> index 83861653768d..e473bb4a9b0b 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.c
> +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> @@ -95,6 +95,20 @@ tc_cold_unblock(struct intel_digital_port *dig_port, intel_wakeref_t wakeref)
>  	intel_display_power_put_async(i915, domain, wakeref);
>  }
>  
> +static void
> +is_tc_cold_blocked(struct intel_digital_port *dig_port)

assert_tc_cold_blocked()?

Reviewed-by: Imre Deak <imre.deak@intel.com>

> +{
> +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> +	bool enabled;
> +
> +	if (INTEL_GEN(i915) == 11 && !dig_port->tc_legacy_port)
> +		return;
> +
> +	enabled = intel_display_power_is_enabled(i915,
> +						 tc_cold_get_power_domain(dig_port));
> +	drm_WARN_ON(&i915->drm, !enabled);
> +}
> +
>  u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port)
>  {
>  	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> @@ -104,7 +118,7 @@ u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port)
>  	lane_mask = intel_uncore_read(uncore,
>  				      PORT_TX_DFLEXDPSP(dig_port->tc_phy_fia));
>  
> -	drm_WARN_ON(&i915->drm, lane_mask == 0xffffffff);
> +	is_tc_cold_blocked(dig_port);
>  
>  	lane_mask &= DP_LANE_ASSIGNMENT_MASK(dig_port->tc_phy_fia_idx);
>  	return lane_mask >> DP_LANE_ASSIGNMENT_SHIFT(dig_port->tc_phy_fia_idx);
> @@ -119,7 +133,7 @@ u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port)
>  	pin_mask = intel_uncore_read(uncore,
>  				     PORT_TX_DFLEXPA1(dig_port->tc_phy_fia));
>  
> -	drm_WARN_ON(&i915->drm, pin_mask == 0xffffffff);
> +	is_tc_cold_blocked(dig_port);
>  
>  	return (pin_mask & DP_PIN_ASSIGNMENT_MASK(dig_port->tc_phy_fia_idx)) >>
>  	       DP_PIN_ASSIGNMENT_SHIFT(dig_port->tc_phy_fia_idx);
> @@ -134,6 +148,8 @@ int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port)
>  	if (dig_port->tc_mode != TC_PORT_DP_ALT)
>  		return 4;
>  
> +	is_tc_cold_blocked(dig_port);
> +
>  	lane_mask = 0;
>  	with_intel_display_power(i915, POWER_DOMAIN_DISPLAY_CORE, wakeref)
>  		lane_mask = intel_tc_port_get_lane_mask(dig_port);
> @@ -166,6 +182,8 @@ void intel_tc_port_set_fia_lane_count(struct intel_digital_port *dig_port,
>  	drm_WARN_ON(&i915->drm,
>  		    lane_reversal && dig_port->tc_mode != TC_PORT_LEGACY);
>  
> +	is_tc_cold_blocked(dig_port);
> +
>  	val = intel_uncore_read(uncore,
>  				PORT_TX_DFLEXDPMLE1(dig_port->tc_phy_fia));
>  	val &= ~DFLEXDPMLE1_DPMLETC_MASK(dig_port->tc_phy_fia_idx);
> -- 
> 2.26.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Imre Deak April 7, 2020, 6:20 p.m. UTC | #2 
On Tue, Apr 07, 2020 at 07:04:55PM +0300, Imre Deak wrote:
> On Mon, Apr 06, 2020 at 06:11:56PM -0700, José Roberto de Souza wrote:
> > As described in "drm/i915/tc/icl: Implement TC cold sequences" users
> > of TC functions should held aux power well during access to avoid
> > read garbage due HW in TC cold state.
> > 
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_tc.c | 22 ++++++++++++++++++++--
> >  1 file changed, 20 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
> > index 83861653768d..e473bb4a9b0b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_tc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> > @@ -95,6 +95,20 @@ tc_cold_unblock(struct intel_digital_port *dig_port, intel_wakeref_t wakeref)
> >  	intel_display_power_put_async(i915, domain, wakeref);
> >  }
> >  
> > +static void
> > +is_tc_cold_blocked(struct intel_digital_port *dig_port)
> 
> assert_tc_cold_blocked()?
> 
> Reviewed-by: Imre Deak <imre.deak@intel.com>
> 
> > +{
> > +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> > +	bool enabled;
> > +
> > +	if (INTEL_GEN(i915) == 11 && !dig_port->tc_legacy_port)
> > +		return;
> > +
> > +	enabled = intel_display_power_is_enabled(i915,
> > +						 tc_cold_get_power_domain(dig_port));
> > +	drm_WARN_ON(&i915->drm, !enabled);
> > +}
> > +
> >  u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port)
> >  {
> >  	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> > @@ -104,7 +118,7 @@ u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port)
> >  	lane_mask = intel_uncore_read(uncore,
> >  				      PORT_TX_DFLEXDPSP(dig_port->tc_phy_fia));
> >  
> > -	drm_WARN_ON(&i915->drm, lane_mask == 0xffffffff);

You could also keep the checks like the above one, since reading all
0xff is still bogus.

> > +	is_tc_cold_blocked(dig_port);
> >  
> >  	lane_mask &= DP_LANE_ASSIGNMENT_MASK(dig_port->tc_phy_fia_idx);
> >  	return lane_mask >> DP_LANE_ASSIGNMENT_SHIFT(dig_port->tc_phy_fia_idx);
> > @@ -119,7 +133,7 @@ u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port)
> >  	pin_mask = intel_uncore_read(uncore,
> >  				     PORT_TX_DFLEXPA1(dig_port->tc_phy_fia));
> >  
> > -	drm_WARN_ON(&i915->drm, pin_mask == 0xffffffff);
> > +	is_tc_cold_blocked(dig_port);
> >  
> >  	return (pin_mask & DP_PIN_ASSIGNMENT_MASK(dig_port->tc_phy_fia_idx)) >>
> >  	       DP_PIN_ASSIGNMENT_SHIFT(dig_port->tc_phy_fia_idx);
> > @@ -134,6 +148,8 @@ int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port)
> >  	if (dig_port->tc_mode != TC_PORT_DP_ALT)
> >  		return 4;
> >  
> > +	is_tc_cold_blocked(dig_port);
> > +
> >  	lane_mask = 0;
> >  	with_intel_display_power(i915, POWER_DOMAIN_DISPLAY_CORE, wakeref)
> >  		lane_mask = intel_tc_port_get_lane_mask(dig_port);
> > @@ -166,6 +182,8 @@ void intel_tc_port_set_fia_lane_count(struct intel_digital_port *dig_port,
> >  	drm_WARN_ON(&i915->drm,
> >  		    lane_reversal && dig_port->tc_mode != TC_PORT_LEGACY);
> >  
> > +	is_tc_cold_blocked(dig_port);
> > +
> >  	val = intel_uncore_read(uncore,
> >  				PORT_TX_DFLEXDPMLE1(dig_port->tc_phy_fia));
> >  	val &= ~DFLEXDPMLE1_DPMLETC_MASK(dig_port->tc_phy_fia_idx);
> > -- 
> > 2.26.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
index 83861653768d..e473bb4a9b0b 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.c
+++ b/drivers/gpu/drm/i915/display/intel_tc.c
@@ -95,6 +95,20 @@  tc_cold_unblock(struct intel_digital_port *dig_port, intel_wakeref_t wakeref)
 	intel_display_power_put_async(i915, domain, wakeref);
 }
 
+static void
+is_tc_cold_blocked(struct intel_digital_port *dig_port)
+{
+	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+	bool enabled;
+
+	if (INTEL_GEN(i915) == 11 && !dig_port->tc_legacy_port)
+		return;
+
+	enabled = intel_display_power_is_enabled(i915,
+						 tc_cold_get_power_domain(dig_port));
+	drm_WARN_ON(&i915->drm, !enabled);
+}
+
 u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port)
 {
 	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
@@ -104,7 +118,7 @@  u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port)
 	lane_mask = intel_uncore_read(uncore,
 				      PORT_TX_DFLEXDPSP(dig_port->tc_phy_fia));
 
-	drm_WARN_ON(&i915->drm, lane_mask == 0xffffffff);
+	is_tc_cold_blocked(dig_port);
 
 	lane_mask &= DP_LANE_ASSIGNMENT_MASK(dig_port->tc_phy_fia_idx);
 	return lane_mask >> DP_LANE_ASSIGNMENT_SHIFT(dig_port->tc_phy_fia_idx);
@@ -119,7 +133,7 @@  u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port)
 	pin_mask = intel_uncore_read(uncore,
 				     PORT_TX_DFLEXPA1(dig_port->tc_phy_fia));
 
-	drm_WARN_ON(&i915->drm, pin_mask == 0xffffffff);
+	is_tc_cold_blocked(dig_port);
 
 	return (pin_mask & DP_PIN_ASSIGNMENT_MASK(dig_port->tc_phy_fia_idx)) >>
 	       DP_PIN_ASSIGNMENT_SHIFT(dig_port->tc_phy_fia_idx);
@@ -134,6 +148,8 @@  int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port)
 	if (dig_port->tc_mode != TC_PORT_DP_ALT)
 		return 4;
 
+	is_tc_cold_blocked(dig_port);
+
 	lane_mask = 0;
 	with_intel_display_power(i915, POWER_DOMAIN_DISPLAY_CORE, wakeref)
 		lane_mask = intel_tc_port_get_lane_mask(dig_port);
@@ -166,6 +182,8 @@  void intel_tc_port_set_fia_lane_count(struct intel_digital_port *dig_port,
 	drm_WARN_ON(&i915->drm,
 		    lane_reversal && dig_port->tc_mode != TC_PORT_LEGACY);
 
+	is_tc_cold_blocked(dig_port);
+
 	val = intel_uncore_read(uncore,
 				PORT_TX_DFLEXDPMLE1(dig_port->tc_phy_fia));
 	val &= ~DFLEXDPMLE1_DPMLETC_MASK(dig_port->tc_phy_fia_idx);