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

[Daniel Vetter <daniel@ffwll.ch>]

On Sun, Mar 22, 2015 at 03:41:04PM +0000, Chris Wilson wrote:
> 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.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>

Unfortunately the unclaimed reg stuff is most useful for debugging display
power well issues (it catches those while nothing else really does), and
this removes that facility. Can't we do tricks with using raw reads/writes
for forcewaked registers or something like that?
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 47 ++++++++++++++++++++-----------------
>  1 file changed, 26 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index ab5cc94588e1..6b065592aede 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -149,6 +149,30 @@ 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_DEBUG("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;
> @@ -561,23 +585,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)
> -{
> -	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_DEBUG("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);
> @@ -829,7 +836,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); \
>  	GEN6_WRITE_FOOTER; \
>  }
>  
> @@ -871,7 +877,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); \
>  	GEN6_WRITE_FOOTER; \
>  }
>  
> @@ -1043,7 +1048,7 @@ static void intel_uncore_fw_domains_init(struct drm_device *dev)
>  
>  	if (IS_GEN9(dev)) {
>  		dev_priv->uncore.funcs.force_wake_get = fw_domains_get;
> -		dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
> +		dev_priv->uncore.funcs.force_wake_put = fw_domains_put_debug;
>  		fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
>  			       FORCEWAKE_RENDER_GEN9,
>  			       FORCEWAKE_ACK_RENDER_GEN9);
> @@ -1066,7 +1071,7 @@ static void intel_uncore_fw_domains_init(struct drm_device *dev)
>  	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
>  		dev_priv->uncore.funcs.force_wake_get =
>  			fw_domains_get_with_thread_status;
> -		dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
> +		dev_priv->uncore.funcs.force_wake_put = fw_domains_put_debug;
>  		fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
>  			       FORCEWAKE_MT, FORCEWAKE_ACK_HSW);
>  	} else if (IS_IVYBRIDGE(dev)) {
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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