[PATCH] drm/i915: Squelch WARN for VLV_COUNTER_CONTROL

[PATCH] drm/i915: Squelch WARN for VLV_COUNTER_CONTROL

[Chris Wilson]

Before rc6 is initialised (after driver load or resume), the value inside
VLV_COUNTER_CONTROL is undefined so we cannot make an assertion that is
in HIGH_RANGE mode.

Fixes: 6b7f6aa75e38 ("drm/i915: Use coarse grained residency counter with byt")
Testcase: igt/drv_suspend/debugfs-reader
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d55bf882d1aa..aece0ff88a5d 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -8354,22 +8354,24 @@ void intel_pm_setup(struct drm_i915_private *dev_priv)
 static u64 vlv_residency_raw(struct drm_i915_private *dev_priv,
 			     const i915_reg_t reg)
 {
-	u32 lower, upper, tmp, saved_ctl;
+	u32 lower, upper, tmp;
 
 	/* The register accessed do not need forcewake. We borrow
 	 * uncore lock to prevent concurrent access to range reg.
 	 */
 	spin_lock_irq(&dev_priv->uncore.lock);
-	saved_ctl = I915_READ_FW(VLV_COUNTER_CONTROL);
-
-	if (WARN_ON(!(saved_ctl & VLV_COUNT_RANGE_HIGH)))
-		I915_WRITE_FW(VLV_COUNTER_CONTROL,
-			      _MASKED_BIT_ENABLE(VLV_COUNT_RANGE_HIGH));
 
 	/* vlv and chv residency counters are 40 bits in width.
 	 * With a control bit, we can choose between upper or lower
 	 * 32bit window into this counter.
+	 *
+	 * Although we always use the counter in high-range mode elsewhere,
+	 * userspace may attempt to read the value before rc6 is initialised,
+	 * before we have set the default VLV_COUNTER_CONTROL value. So always
+	 * set the high bit to be safe.
 	 */
+	I915_WRITE_FW(VLV_COUNTER_CONTROL,
+		      _MASKED_BIT_ENABLE(VLV_COUNT_RANGE_HIGH));
 	upper = I915_READ_FW(reg);
 	do {
 		tmp = upper;
@@ -8383,6 +8385,11 @@ static u64 vlv_residency_raw(struct drm_i915_private *dev_priv,
 		upper = I915_READ_FW(reg);
 	} while (upper != tmp);
 
+	/* Everywhere else we always use VLV_COUNTER_CONTROL with the
+	 * VLV_COUNT_RANGE_HIGH bit set - so it is safe to leave it set
+	 * now.
+	 */
+
 	spin_unlock_irq(&dev_priv->uncore.lock);
 
 	return lower | (u64)upper << 8;
-- 
2.11.0

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

Re: [PATCH] drm/i915: Squelch WARN for VLV_COUNTER_CONTROL

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Before rc6 is initialised (after driver load or resume), the value inside
> VLV_COUNTER_CONTROL is undefined so we cannot make an assertion that is
> in HIGH_RANGE mode.
>
> Fixes: 6b7f6aa75e38 ("drm/i915: Use coarse grained residency counter with byt")
> Testcase: igt/drv_suspend/debugfs-reader
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d55bf882d1aa..aece0ff88a5d 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -8354,22 +8354,24 @@ void intel_pm_setup(struct drm_i915_private *dev_priv)
>  static u64 vlv_residency_raw(struct drm_i915_private *dev_priv,
>  			     const i915_reg_t reg)
>  {
> -	u32 lower, upper, tmp, saved_ctl;
> +	u32 lower, upper, tmp;
>  
>  	/* The register accessed do not need forcewake. We borrow
>  	 * uncore lock to prevent concurrent access to range reg.
>  	 */
>  	spin_lock_irq(&dev_priv->uncore.lock);
> -	saved_ctl = I915_READ_FW(VLV_COUNTER_CONTROL);
> -
> -	if (WARN_ON(!(saved_ctl & VLV_COUNT_RANGE_HIGH)))
> -		I915_WRITE_FW(VLV_COUNTER_CONTROL,
> -			      _MASKED_BIT_ENABLE(VLV_COUNT_RANGE_HIGH));
>  
>  	/* vlv and chv residency counters are 40 bits in width.
>  	 * With a control bit, we can choose between upper or lower
>  	 * 32bit window into this counter.
> +	 *
> +	 * Although we always use the counter in high-range mode elsewhere,
> +	 * userspace may attempt to read the value before rc6 is initialised,
> +	 * before we have set the default VLV_COUNTER_CONTROL value. So always
> +	 * set the high bit to be safe.
>  	 */
> +	I915_WRITE_FW(VLV_COUNTER_CONTROL,
> +		      _MASKED_BIT_ENABLE(VLV_COUNT_RANGE_HIGH));
>  	upper = I915_READ_FW(reg);
>  	do {
>  		tmp = upper;
> @@ -8383,6 +8385,11 @@ static u64 vlv_residency_raw(struct drm_i915_private *dev_priv,
>  		upper = I915_READ_FW(reg);
>  	} while (upper != tmp);
>  
> +	/* Everywhere else we always use VLV_COUNTER_CONTROL with the
> +	 * VLV_COUNT_RANGE_HIGH bit set - so it is safe to leave it set
> +	 * now.
> +	 */
> +
>  	spin_unlock_irq(&dev_priv->uncore.lock);
>  
>  	return lower | (u64)upper << 8;
> -- 
> 2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx