On Thu, Mar 13, 2014 at 09:30:17PM +0530, deepak.s@xxxxxxxxxxxxxxx wrote:
> From: Deepak S <deepak.s@xxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
