Re: [PATCH 7/7] drm/i915/chv: Add non claimed mmio checking

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 7/7] drm/i915/chv: Add non claimed mmio checking
Date: Tue, 15 Dec 2015 16:44:25 +0200	[thread overview]
Message-ID: <20151215144425.GH4437@intel.com> (raw)
In-Reply-To: <1450189512-30360-7-git-send-email-mika.kuoppala@intel.com>

On Tue, Dec 15, 2015 at 04:25:12PM +0200, Mika Kuoppala wrote:
> Imre mentioned that chv might also have capability to
> track unclaimed mmio accesses. And it has, so take it
> into use.

VLV too.

My old patch:
http://lists.freedesktop.org/archives/intel-gfx/2013-May/027599.html

> 
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h     |  4 ++++
>  drivers/gpu/drm/i915/intel_uncore.c | 34 ++++++++++++++++++++++++++++++----
>  2 files changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 007ae83..24686ab 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1711,6 +1711,10 @@ enum skl_disp_power_wells {
>  #define FPGA_DBG		_MMIO(0x42300)
>  #define   FPGA_DBG_RM_NOCLAIM	(1<<31)
>  
> +#define CLAIM_ER		_MMIO(0x182028)
> +#define   CLAIM_ER_CLR		(1<<31)

Looks like you forgot the overflow bit.

> +#define   CLAIM_ER_CTR_MASK	(0xffff)

Needless parens.

> +
>  #define DERRMR		_MMIO(0x44050)
>  /* Note that HBLANK events are reserved on bdw+ */
>  #define   DERRMR_PIPEA_SCANLINE		(1<<0)
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 02bad32..ea8fcd4 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -335,13 +335,10 @@ static void intel_uncore_ellc_detect(struct drm_device *dev)
>  }
>  
>  static bool
> -check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
> +fpga_check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
>  {
>  	u32 dbg;
>  
> -	if (!HAS_FPGA_DBG_UNCLAIMED(dev_priv))
> -		return false;
> -
>  	dbg = __raw_i915_read32(dev_priv, FPGA_DBG);
>  	if (likely(!(dbg & FPGA_DBG_RM_NOCLAIM)))
>  		return false;
> @@ -354,6 +351,35 @@ check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
>  	return true;
>  }
>  
> +static bool
> +chv_check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
> +{
> +	u32 cer;
> +
> +	cer = __raw_i915_read32(dev_priv, CLAIM_ER);
> +	if (likely(!(cer & CLAIM_ER_CTR_MASK)))
> +		return false;
> +
> +	__raw_i915_write32(dev_priv, CLAIM_ER, CLAIM_ER_CLR);
> +
> +	/* We want to clear it asap, so post */
> +	__raw_i915_read32(dev_priv, CLAIM_ER);

__raw_posting_read()

But not sure why we'd want to do this read really. Seems like pointless
overhead.

The other thing is that this only detect unclaimed RM cycles, so display
registers only. Might be nice to only do the checks when accessing
display registers so that we don't add overhead to the GT stuff.
I think the same holds for HSW+ actually.

> +
> +	return true;
> +}
> +
> +static bool
> +check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
> +{
> +	if (HAS_FPGA_DBG_UNCLAIMED(dev_priv))
> +		return fpga_check_for_unclaimed_mmio(dev_priv);
> +
> +	if (IS_CHERRYVIEW(dev_priv))
> +		return chv_check_for_unclaimed_mmio(dev_priv);
> +
> +	return false;
> +}
> +
>  static void __intel_uncore_early_sanitize(struct drm_device *dev,
>  					  bool restore_forcewake)
>  {
> -- 
> 2.5.0

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx