Re: [PATCH 4/4] drm/i915: Reduce frequency of unspecific HSW reg debugging

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Sep 03, 2015 at 04:51:45PM -0300, Paulo Zanoni wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Delay the expensive read on the FPGA_DBG register from once per mmio to
> once per forcewake section when we are doing the general wellbeing
> check rather than the targetted error detection. This almost reduces
> the overhead of the debug facility (for example when submitting execlists)
> to zero whilst keeping the debug checks around.
> 
> v2: Enable one-shot mmio debugging from the interrupt check as well as a
>     safeguard to catch invalid display writes from outside the powerwell.
> v3 (from Paulo): rebase since gen9 addition and intel_uncore_check_errors
>     removal
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

I'm unclear how this interacts (or how it sould interact) with patch 2:
Forcwake is mostly for GT registers, but patch 2 also tries to optimize
forcwake for GT registers. Do we really need both?
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 52 +++++++++++++++++++++----------------
>  1 file changed, 29 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 8844c314..1fe63fc 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -148,6 +148,31 @@ fw_domains_put(struct drm_i915_private *dev_priv, enum forcewake_domains fw_doma
>  }
>  
>  static void
> +hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv)
> +{
> +	static bool mmio_debug_once = true;
> +
> +	if (i915.mmio_debug || !mmio_debug_once)
> +		return;
> +
> +	if (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM) {
> +		DRM_ERROR("Unclaimed register detected, "
> +			  "enabling oneshot unclaimed register reporting. "
> +			  "Please use i915.mmio_debug=N for more information.\n");
> +		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
> +		i915.mmio_debug = mmio_debug_once--;
> +	}
> +}
> +
> +static void
> +fw_domains_put_debug(struct drm_i915_private *dev_priv,
> +		     enum forcewake_domains fw_domains)
> +{
> +	hsw_unclaimed_reg_detect(dev_priv);
> +	fw_domains_put(dev_priv, fw_domains);
> +}
> +
> +static void
>  fw_domains_posting_read(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_uncore_forcewake_domain *d;
> @@ -627,26 +652,6 @@ hsw_unclaimed_reg_debug(struct drm_i915_private *dev_priv, u32 reg, bool read,
>  	}
>  }
>  
> -static void
> -hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv, u32 reg)
> -{
> -	static bool mmio_debug_once = true;
> -
> -	if (!UNCLAIMED_CHECK_RANGE(reg))
> -		return;
> -
> -	if (i915.mmio_debug || !mmio_debug_once)
> -		return;
> -
> -	if (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM) {
> -		DRM_ERROR("Unclaimed register detected, "
> -			  "enabling oneshot unclaimed register reporting. "
> -			  "Please use i915.mmio_debug=N for more information.\n");
> -		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
> -		i915.mmio_debug = mmio_debug_once--;
> -	}
> -}
> -
>  #define GEN2_READ_HEADER(x) \
>  	u##x val = 0; \
>  	assert_device_not_suspended(dev_priv);
> @@ -900,7 +905,6 @@ hsw_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace)
>  		gen6_gt_check_fifodbg(dev_priv); \
>  	} \
>  	hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \
> -	hsw_unclaimed_reg_detect(dev_priv, reg); \
>  	GEN6_WRITE_FOOTER; \
>  }
>  
> @@ -942,7 +946,6 @@ gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace
>  		__force_wake_get(dev_priv, FORCEWAKE_RENDER); \
>  	__raw_i915_write##x(dev_priv, reg, val); \
>  	hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \
> -	hsw_unclaimed_reg_detect(dev_priv, reg); \
>  	GEN6_WRITE_FOOTER; \
>  }
>  
> @@ -1008,7 +1011,6 @@ gen9_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, \
>  		__force_wake_get(dev_priv, fw_engine); \
>  	__raw_i915_write##x(dev_priv, reg, val); \
>  	hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \
> -	hsw_unclaimed_reg_detect(dev_priv, reg); \
>  	GEN6_WRITE_FOOTER; \
>  }
>  
> @@ -1194,6 +1196,10 @@ static void intel_uncore_fw_domains_init(struct drm_device *dev)
>  			       FORCEWAKE, FORCEWAKE_ACK);
>  	}
>  
> +	if (HAS_FPGA_DBG_UNCLAIMED(dev) &&
> +	    dev_priv->uncore.funcs.force_wake_put == fw_domains_put)
> +		dev_priv->uncore.funcs.force_wake_put = fw_domains_put_debug;
> +
>  	/* All future platforms are expected to require complex power gating */
>  	WARN_ON(dev_priv->uncore.fw_domains == 0);
>  }
> -- 
> 2.5.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx