[Intel-gfx] [PATCH v3 2/3] drm/i915/vlv: WA for Turbo and RC6 to work together.

Ville Syrjälä ville.syrjala at linux.intel.com
Thu Mar 13 19:17:30 CET 2014


On Thu, Mar 13, 2014 at 09:30:17PM +0530, deepak.s at linux.intel.com wrote:
> From: Deepak S <deepak.s at intel.com>
> 
> With RC6 enabled, BYT has an HW issue in determining the right
> Gfx busyness.
> WA for Turbo + RC6: Use SW based Gfx busy-ness detection to decide
> on increasing/decreasing the freq. This logic will monitor C0
> counters of render/media power-wells over EI period and takes
> necessary action based on these values
> 
> v2: Refactor duplicate code. (Ville)
> 
> v3: Reformat the comments. (Ville)
> 
> Signed-off-by: Deepak S <deepak.s at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  17 +++++
>  drivers/gpu/drm/i915/i915_irq.c | 133 ++++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_reg.h |  13 +++-
>  drivers/gpu/drm/i915/intel_pm.c |  19 +++---
>  4 files changed, 173 insertions(+), 9 deletions(-)
> 
<snip>
> @@ -1564,6 +1683,16 @@ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)
>  		queue_work(dev_priv->wq, &dev_priv->rps.work);
>  	}
>  
> +	if (pm_iir & GEN6_PM_RP_UP_EI_EXPIRED) {
> +		spin_lock(&dev_priv->irq_lock);
> +		dev_priv->rps.pm_iir |= pm_iir & GEN6_PM_RP_UP_EI_EXPIRED;
> +		snb_disable_pm_irq(dev_priv, pm_iir & GEN6_PM_RP_UP_EI_EXPIRED);
> +		spin_unlock(&dev_priv->irq_lock);
> +		DRM_DEBUG_DRIVER("\nQueueing RPS Work - RC6 WA Turbo");

This debug message still seems rather pointless. Just drop it.

Oh actually isn't this entire block of code useless now that
pm_rps_events is used? The previous if block already checked
pm_rps_events which will include GEN6_PM_RP_UP_EI_EXPIRED on
VLV, so this code here will just repeat the work already done.

> +
> +		queue_work(dev_priv->wq, &dev_priv->rps.work);
> +	}
> +
>  	if (HAS_VEBOX(dev_priv->dev)) {
>  		if (pm_iir & PM_VEBOX_USER_INTERRUPT)
>  			notify_ring(dev_priv->dev, &dev_priv->ring[VECS]);
> @@ -2989,6 +3118,10 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev)
>  			pm_irqs |= PM_VEBOX_USER_INTERRUPT;
>  
>  		dev_priv->pm_irq_mask = 0xffffffff;
> +
> +		dev_priv->pm_irq_mask &= ~dev_priv->pm_rps_events;
> +		pm_irqs |= dev_priv->pm_rps_events;

What's this stuff doing here?

> +
>  		I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR));
>  		I915_WRITE(GEN6_PMIMR, dev_priv->pm_irq_mask);
>  		I915_WRITE(GEN6_PMIER, pm_irqs);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 6174fda..d978b46 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -419,6 +419,7 @@ enum punit_power_well {
>  #define PUNIT_REG_GPU_FREQ_STS			0xd8
>  #define   GENFREQSTATUS				(1<<0)
>  #define PUNIT_REG_MEDIA_TURBO_FREQ_REQ		0xdc
> +#define PUNIT_REG_CZ_TIMESTAMP			0xce
>  
>  #define PUNIT_FUSE_BUS2				0xf6 /* bits 47:40 */
>  #define PUNIT_FUSE_BUS1				0xf5 /* bits 55:48 */
> @@ -434,6 +435,11 @@ enum punit_power_well {
>  #define   FB_FMAX_VMIN_FREQ_LO_SHIFT		27
>  #define   FB_FMAX_VMIN_FREQ_LO_MASK		0xf8000000
>  
> +#define VLV_CZ_CLOCK_TO_MILLI_SEC		100000
> +#define VLV_RP_UP_EI_THRESHOLD			90
> +#define VLV_RP_DOWN_EI_THRESHOLD		70
> +#define VLV_INT_COUNT_FOR_DOWN_EI		5
> +
>  /* vlv2 north clock has */
>  #define CCK_FUSE_REG				0x8
>  #define  CCK_FUSE_HPLL_FREQ_MASK		0x3
> @@ -4892,6 +4898,7 @@ enum punit_power_well {
>  #define  VLV_GTLC_PW_STATUS			0x130094
>  #define VLV_GTLC_PW_RENDER_STATUS_MASK		0x80
>  #define VLV_GTLC_PW_MEDIA_STATUS_MASK		0x20
> +#define VLV_GTLC_SURVIVABILITY_REG              0x130098
>  #define  FORCEWAKE_MT				0xa188 /* multi-threaded */
>  #define   FORCEWAKE_KERNEL			0x1
>  #define   FORCEWAKE_USER			0x2
> @@ -5019,13 +5026,17 @@ enum punit_power_well {
>  
>  #define GEN6_GT_GFX_RC6_LOCKED			0x138104
>  #define VLV_COUNTER_CONTROL			0x138104
> +#define VLV_RC_COUNTER_CONTROL                  0xFFFF00FF

I'd still like to see names for all the bits we frob, and I'd
still like to have some kind of an answer to the question whether
we really need to enable them all when the w/a is only interested
in the rc0 counters.

>  #define   VLV_COUNT_RANGE_HIGH			(1<<15)
> +#define   VLV_MEDIA_RC0_COUNT_EN		(1<<5)
> +#define   VLV_RENDER_RC0_COUNT_EN		(1<<4)
>  #define   VLV_MEDIA_RC6_COUNT_EN		(1<<1)
>  #define   VLV_RENDER_RC6_COUNT_EN		(1<<0)
>  #define GEN6_GT_GFX_RC6				0x138108
>  #define GEN6_GT_GFX_RC6p			0x13810C
>  #define GEN6_GT_GFX_RC6pp			0x138110
> -
> +#define VLV_RENDER_C0_COUNT_REG		0x138118
> +#define VLV_MEDIA_C0_COUNT_REG			0x13811C
>  #define GEN6_PCODE_MAILBOX			0x138124
>  #define   GEN6_PCODE_READY			(1<<31)
>  #define   GEN6_READ_OC_PARAMS			0xc
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index bf6baa6..8a791b7 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3096,10 +3096,14 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
>  		I915_READ(VLV_GTLC_SURVIVABILITY_REG) &
>  				~VLV_GFX_CLK_FORCE_ON_BIT);
>  
> -	/* Unmask Up interrupts */
> -	dev_priv->rps.rp_up_masked = true;
> -	gen6_set_pm_mask(dev_priv, GEN6_PM_RP_DOWN_THRESHOLD,
> +	/* Unmask Turbo interrupts */
> +	if (dev_priv->pm_rps_events & GEN6_PM_RP_UP_EI_EXPIRED)
> +		I915_WRITE(GEN6_PMINTRMSK, ~dev_priv->pm_rps_events);
> +	else {
> +		dev_priv->rps.rp_up_masked = true;
> +		gen6_set_pm_mask(dev_priv, GEN6_PM_RP_DOWN_THRESHOLD,
>  						dev_priv->rps.min_delay);
> +	}
>  }
>  
>  void gen6_rps_idle(struct drm_i915_private *dev_priv)
> @@ -3620,6 +3624,7 @@ static void valleyview_enable_rps(struct drm_device *dev)
>  	I915_WRITE(GEN6_RP_DOWN_EI, 350000);
>  
>  	I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
> +	I915_WRITE(GEN6_RP_DOWN_TIMEOUT, 0xf4240);
>  
>  	I915_WRITE(GEN6_RP_CONTROL,
>  		   GEN6_RP_MEDIA_TURBO |
> @@ -3639,10 +3644,7 @@ static void valleyview_enable_rps(struct drm_device *dev)
>  	I915_WRITE(GEN6_RC6_THRESHOLD, 0x557);
>  
>  	/* allows RC6 residency counter to work */
> -	I915_WRITE(VLV_COUNTER_CONTROL,
> -		   _MASKED_BIT_ENABLE(VLV_COUNT_RANGE_HIGH |
> -				      VLV_MEDIA_RC6_COUNT_EN |
> -				      VLV_RENDER_RC6_COUNT_EN));
> +	I915_WRITE(VLV_COUNTER_CONTROL, VLV_RC_COUNTER_CONTROL);
>  	if (intel_enable_rc6(dev) & INTEL_RC6_ENABLE)
>  		rc6_mode = GEN7_RC_CTL_TO_MODE | VLV_RC_CTL_CTX_RST_PARALLEL;
>  
> @@ -3691,7 +3693,8 @@ static void valleyview_enable_rps(struct drm_device *dev)
>  	dev_priv->rps.rp_up_masked = false;
>  	dev_priv->rps.rp_down_masked = false;
>  
> -	dev_priv->pm_rps_events = GEN6_PM_RPS_EVENTS;
> +	/* WAUseRC0ResidenncyTurbo:VLV */
> +	dev_priv->pm_rps_events = GEN6_PM_RP_UP_EI_EXPIRED;

I'm still wondering if we should have the option of using the old
fashioned method...

>  	gen6_enable_rps_interrupts(dev);
>  
>  	gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
> -- 
> 1.8.4.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC



More information about the Intel-gfx mailing list