[PATCH 1/4] drm/i915: Drop stray clearing of rps->last_adj

[PATCH 1/4] drm/i915: Drop stray clearing of rps->last_adj

[Chris Wilson]

We used to reset last_adj to 0 on crossing a power domain boundary, to
slow down our rate of change. However, commit 60548c554be2 ("drm/i915:
Interactive RPS mode") accidentally caused it to be reset on every
frequency update, nerfing the fast response granted by the slow start
algorithm.

Fixes: 60548c554be2 ("drm/i915: Interactive RPS mode")
Testcase: igt/pm_rps/mix-max-config-loaded
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2531eb75bdce..f90a3c7f1c40 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6371,7 +6371,6 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
 		new_power = HIGH_POWER;
 	rps_set_power(dev_priv, new_power);
 	mutex_unlock(&rps->power.mutex);
-	rps->last_adj = 0;
 }
 
 void intel_rps_mark_interactive(struct drm_i915_private *i915, bool interactive)
-- 
2.18.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/4] drm/i915: Unconditionally clear the pm/guc GT IIR upon acking

[Chris Wilson]

Having stored the IIR for action, we should always clear it.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 90628a47ae17..e37e3ec22a79 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1534,11 +1534,8 @@ static void gen8_gt_irq_ack(struct drm_i915_private *i915,
 
 	if (master_ctl & (GEN8_GT_PM_IRQ | GEN8_GT_GUC_IRQ)) {
 		gt_iir[2] = raw_reg_read(regs, GEN8_GT_IIR(2));
-		if (likely(gt_iir[2] & (i915->pm_rps_events |
-					i915->pm_guc_events)))
-			raw_reg_write(regs, GEN8_GT_IIR(2),
-				      gt_iir[2] & (i915->pm_rps_events |
-						   i915->pm_guc_events));
+		if (likely(gt_iir[2]))
+			raw_reg_write(regs, GEN8_GT_IIR(2), gt_iir[2]);
 	}
 
 	if (master_ctl & GEN8_GT_VECS_IRQ) {
-- 
2.18.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/4] drm/i915: Clear all residual RPS events on disabling interrupts

[Chris Wilson]

Make sure that the RPS IIR is completely clear on disabling so we should
not get any more interrupts after idling. Since the IIR is shared with
the guc, we have to be careful to only clobber RPS events.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 8 +++++---
 drivers/gpu/drm/i915/i915_reg.h | 6 ++++--
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index e37e3ec22a79..8084e35b25c5 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -478,7 +478,7 @@ void gen11_reset_rps_interrupts(struct drm_i915_private *dev_priv)
 void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
 {
 	spin_lock_irq(&dev_priv->irq_lock);
-	gen6_reset_pm_iir(dev_priv, dev_priv->pm_rps_events);
+	gen6_reset_pm_iir(dev_priv, GEN6_PM_RPS_EVENTS);
 	dev_priv->gt_pm.rps.pm_iir = 0;
 	spin_unlock_irq(&dev_priv->irq_lock);
 }
@@ -516,7 +516,7 @@ void gen6_disable_rps_interrupts(struct drm_i915_private *dev_priv)
 
 	I915_WRITE(GEN6_PMINTRMSK, gen6_sanitize_rps_pm_mask(dev_priv, ~0u));
 
-	gen6_disable_pm_irq(dev_priv, dev_priv->pm_rps_events);
+	gen6_disable_pm_irq(dev_priv, GEN6_PM_RPS_EVENTS);
 
 	spin_unlock_irq(&dev_priv->irq_lock);
 	synchronize_irq(dev_priv->drm.irq);
@@ -4778,7 +4778,9 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
 		/* WaGsvRC0ResidencyMethod:vlv */
 		dev_priv->pm_rps_events = GEN6_PM_RP_UP_EI_EXPIRED;
 	else
-		dev_priv->pm_rps_events = GEN6_PM_RPS_EVENTS;
+		dev_priv->pm_rps_events = (GEN6_PM_RP_UP_THRESHOLD |
+					   GEN6_PM_RP_DOWN_THRESHOLD |
+					   GEN6_PM_RP_DOWN_TIMEOUT);
 
 	rps->pm_intrmsk_mbz = 0;
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e0f5999fff07..4b656f31fde9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8582,8 +8582,10 @@ enum {
 #define  GEN6_PM_RP_DOWN_THRESHOLD		(1 << 4)
 #define  GEN6_PM_RP_UP_EI_EXPIRED		(1 << 2)
 #define  GEN6_PM_RP_DOWN_EI_EXPIRED		(1 << 1)
-#define  GEN6_PM_RPS_EVENTS			(GEN6_PM_RP_UP_THRESHOLD | \
-						 GEN6_PM_RP_DOWN_THRESHOLD | \
+#define  GEN6_PM_RPS_EVENTS			(GEN6_PM_RP_UP_EI_EXPIRED   | \
+						 GEN6_PM_RP_UP_THRESHOLD    | \
+						 GEN6_PM_RP_DOWN_EI_EXPIRED | \
+						 GEN6_PM_RP_DOWN_THRESHOLD  | \
 						 GEN6_PM_RP_DOWN_TIMEOUT)
 
 #define GEN7_GT_SCRATCH(i)			_MMIO(0x4F100 + (i) * 4)
-- 
2.18.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/4] drm/i915: Dampen RPS slow start

[Chris Wilson]

Currently, we note congestion for the slow start ramping up of RPS only
when we overshoot the target workload and have to reverse direction for
our reclocking. That is, if we have a period where the current GPU
frequency is enough to sustain the workload within our target
utilisation, we should not trigger any RPS EI interrupts, and then may
continue again with the previous last_adj after multiple periods causing
us to dramatically overreact. To prevent us not noticing a period where
the system is behaving correctly, we can schedule an extra interrupt
that will not be associated with either an up or down event causing to
reset last_adj back to zero, cancelling the slow start due to the
congestion.

v2: Separate up/down EI
v3: Reset rps events upon enabling

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 43 +++++++++++++++++++++------------
 drivers/gpu/drm/i915/intel_pm.c | 14 ++++++++---
 2 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 8084e35b25c5..69919a97ec2e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -493,6 +493,14 @@ void gen6_enable_rps_interrupts(struct drm_i915_private *dev_priv)
 	spin_lock_irq(&dev_priv->irq_lock);
 	WARN_ON_ONCE(rps->pm_iir);
 
+	if (IS_VALLEYVIEW(dev_priv))
+		/* WaGsvRC0ResidencyMethod:vlv */
+		dev_priv->pm_rps_events = GEN6_PM_RP_UP_EI_EXPIRED;
+	else
+		dev_priv->pm_rps_events = (GEN6_PM_RP_UP_THRESHOLD |
+					   GEN6_PM_RP_DOWN_THRESHOLD |
+					   GEN6_PM_RP_DOWN_TIMEOUT);
+
 	if (INTEL_GEN(dev_priv) >= 11)
 		WARN_ON_ONCE(gen11_reset_one_iir(dev_priv, 0, GEN11_GTPM));
 	else
@@ -1298,7 +1306,13 @@ static void gen6_pm_rps_work(struct work_struct *work)
 
 	mutex_lock(&dev_priv->pcu_lock);
 
-	pm_iir |= vlv_wa_c0_ei(dev_priv, pm_iir);
+	dev_priv->pm_rps_events &=
+		~(GEN6_PM_RP_DOWN_EI_EXPIRED | GEN6_PM_RP_UP_EI_EXPIRED);
+
+	if (IS_VALLEYVIEW(dev_priv)) {
+		dev_priv->pm_rps_events |= GEN6_PM_RP_UP_EI_EXPIRED;
+		pm_iir |= vlv_wa_c0_ei(dev_priv, pm_iir);
+	}
 
 	adj = rps->last_adj;
 	new_delay = rps->cur_freq;
@@ -1310,10 +1324,12 @@ static void gen6_pm_rps_work(struct work_struct *work)
 		new_delay = rps->boost_freq;
 		adj = 0;
 	} else if (pm_iir & GEN6_PM_RP_UP_THRESHOLD) {
-		if (adj > 0)
+		if (adj > 0) {
+			dev_priv->pm_rps_events |= GEN6_PM_RP_UP_EI_EXPIRED;
 			adj *= 2;
-		else /* CHV needs even encode values */
+		} else { /* CHV needs even encode values */
 			adj = IS_CHERRYVIEW(dev_priv) ? 2 : 1;
+		}
 
 		if (new_delay >= rps->max_freq_softlimit)
 			adj = 0;
@@ -1326,15 +1342,21 @@ static void gen6_pm_rps_work(struct work_struct *work)
 			new_delay = rps->min_freq_softlimit;
 		adj = 0;
 	} else if (pm_iir & GEN6_PM_RP_DOWN_THRESHOLD) {
-		if (adj < 0)
+		if (adj < 0) {
+			dev_priv->pm_rps_events |= GEN6_PM_RP_DOWN_EI_EXPIRED;
 			adj *= 2;
-		else /* CHV needs even encode values */
+		} else { /* CHV needs even encode values */
 			adj = IS_CHERRYVIEW(dev_priv) ? -2 : -1;
+		}
 
 		if (new_delay <= rps->min_freq_softlimit)
 			adj = 0;
-	} else { /* unknown event */
+	} else if (pm_iir & GEN6_PM_RP_UP_THRESHOLD && adj > 0) {
+		adj = 0;
+	} else if (pm_iir & GEN6_PM_RP_DOWN_THRESHOLD && adj < 0) {
 		adj = 0;
+	} else {
+		/* unknown event */
 	}
 
 	rps->last_adj = adj;
@@ -4773,15 +4795,6 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
 	if (HAS_GUC_SCHED(dev_priv))
 		dev_priv->pm_guc_events = GEN9_GUC_TO_HOST_INT_EVENT;
 
-	/* Let's track the enabled rps events */
-	if (IS_VALLEYVIEW(dev_priv))
-		/* WaGsvRC0ResidencyMethod:vlv */
-		dev_priv->pm_rps_events = GEN6_PM_RP_UP_EI_EXPIRED;
-	else
-		dev_priv->pm_rps_events = (GEN6_PM_RP_UP_THRESHOLD |
-					   GEN6_PM_RP_DOWN_THRESHOLD |
-					   GEN6_PM_RP_DOWN_TIMEOUT);
-
 	rps->pm_intrmsk_mbz = 0;
 
 	/*
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f90a3c7f1c40..d71a498ee3a1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6397,10 +6397,16 @@ static u32 gen6_rps_pm_mask(struct drm_i915_private *dev_priv, u8 val)
 	u32 mask = 0;
 
 	/* We use UP_EI_EXPIRED interupts for both up/down in manual mode */
-	if (val > rps->min_freq_softlimit)
-		mask |= GEN6_PM_RP_UP_EI_EXPIRED | GEN6_PM_RP_DOWN_THRESHOLD | GEN6_PM_RP_DOWN_TIMEOUT;
-	if (val < rps->max_freq_softlimit)
-		mask |= GEN6_PM_RP_UP_EI_EXPIRED | GEN6_PM_RP_UP_THRESHOLD;
+	if (val > rps->min_freq_softlimit) {
+		mask |= (GEN6_PM_RP_UP_EI_EXPIRED |
+			 GEN6_PM_RP_DOWN_EI_EXPIRED |
+			 GEN6_PM_RP_DOWN_THRESHOLD |
+			 GEN6_PM_RP_DOWN_TIMEOUT);
+	}
+	if (val < rps->max_freq_softlimit) {
+		mask |= (GEN6_PM_RP_UP_EI_EXPIRED |
+			 GEN6_PM_RP_UP_THRESHOLD);
+	}
 
 	mask &= dev_priv->pm_rps_events;
 
-- 
2.18.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915: Drop stray clearing of rps->last_adj

[Patchwork]

== Series Details ==

Series: series starting with [1/4] drm/i915: Drop stray clearing of rps->last_adj
URL   : https://patchwork.freedesktop.org/series/47600/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4605 -> Patchwork_9841 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/47600/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_9841 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_guc:
      fi-skl-guc:         NOTRUN -> DMESG-WARN (fdo#107258, fdo#107175)

    igt@drv_selftest@live_workarounds:
      {fi-cfl-8109u}:     PASS -> DMESG-FAIL (fdo#107292)
      fi-cnl-psr:         PASS -> DMESG-FAIL (fdo#107292)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_workarounds:
      {fi-bdw-samus}:     DMESG-FAIL (fdo#107292) -> PASS

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         FAIL (fdo#104008) -> PASS

    
    ==== Warnings ====

    {igt@kms_psr@primary_page_flip}:
      fi-cnl-psr:         DMESG-FAIL (fdo#107372) -> DMESG-WARN (fdo#107372)

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#107175 https://bugs.freedesktop.org/show_bug.cgi?id=107175
  fdo#107258 https://bugs.freedesktop.org/show_bug.cgi?id=107258
  fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292
  fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372


== Participating hosts (50 -> 44) ==

  Additional (1): fi-skl-guc 
  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-kbl-7560u fi-byt-clapper 


== Build changes ==

    * Linux: CI_DRM_4605 -> Patchwork_9841

  CI_DRM_4605: 50098198da758bdd54245d511f4f97236c296c72 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4582: 263ca16e4d8909f475d32a28fc0e5972bac214fb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9841: 6508fa1fc8c9799bceee3853bc7025c254c11afb @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

6508fa1fc8c9 drm/i915: Dampen RPS slow start
3e13f77aebce drm/i915: Clear all residual RPS events on disabling interrupts
b66d6b661fde drm/i915: Unconditionally clear the pm/guc GT IIR upon acking
8618cf85463b drm/i915: Drop stray clearing of rps->last_adj

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9841/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [1/4] drm/i915: Drop stray clearing of rps->last_adj

[Patchwork]

== Series Details ==

Series: series starting with [1/4] drm/i915: Drop stray clearing of rps->last_adj
URL   : https://patchwork.freedesktop.org/series/47600/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4605_full -> Patchwork_9841_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9841_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9841_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9841_full:

  === IGT changes ===

    ==== Warnings ====

    igt@kms_frontbuffer_tracking@fbc-1p-rte:
      shard-snb:          PASS -> SKIP +2

    
== Known issues ==

  Here are the changes found in Patchwork_9841_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_userptr_blits@create-destroy-sync:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411)

    igt@kms_flip@modeset-vs-vblank-race:
      shard-glk:          PASS -> FAIL (fdo#103060)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665)

    
    ==== Possible fixes ====

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS

    igt@pm_rps@min-max-config-loaded:
      shard-apl:          FAIL (fdo#102250) -> PASS
      shard-glk:          FAIL -> PASS

    
  fdo#102250 https://bugs.freedesktop.org/show_bug.cgi?id=102250
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4605 -> Patchwork_9841

  CI_DRM_4605: 50098198da758bdd54245d511f4f97236c296c72 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4582: 263ca16e4d8909f475d32a28fc0e5972bac214fb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9841: 6508fa1fc8c9799bceee3853bc7025c254c11afb @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9841/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] drm/i915: Drop stray clearing of rps->last_adj

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> We used to reset last_adj to 0 on crossing a power domain boundary, to
> slow down our rate of change. However, commit 60548c554be2 ("drm/i915:
> Interactive RPS mode") accidentally caused it to be reset on every
> frequency update, nerfing the fast response granted by the slow start
> algorithm.
>
> Fixes: 60548c554be2 ("drm/i915: Interactive RPS mode")
> Testcase: igt/pm_rps/mix-max-config-loaded
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 2531eb75bdce..f90a3c7f1c40 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6371,7 +6371,6 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
>  		new_power = HIGH_POWER;
>  	rps_set_power(dev_priv, new_power);
>  	mutex_unlock(&rps->power.mutex);
> -	rps->last_adj = 0;
>  }
>  
>  void intel_rps_mark_interactive(struct drm_i915_private *i915, bool interactive)
> -- 
> 2.18.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] drm/i915: Drop stray clearing of rps->last_adj

[Chris Wilson]

Quoting Mika Kuoppala (2018-08-02 16:30:24)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > We used to reset last_adj to 0 on crossing a power domain boundary, to
> > slow down our rate of change. However, commit 60548c554be2 ("drm/i915:
> > Interactive RPS mode") accidentally caused it to be reset on every
> > frequency update, nerfing the fast response granted by the slow start
> > algorithm.
> >
> > Fixes: 60548c554be2 ("drm/i915: Interactive RPS mode")
> > Testcase: igt/pm_rps/mix-max-config-loaded
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

Plonked this one as fixes the regression. 2&3 are nice cleanups, 4 seems
like an unlikely boundary case, but I suspect kodi might be a good
example.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Unconditionally clear the pm/guc GT IIR upon acking

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Having stored the IIR for action, we should always clear it.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>

As getting stray unexpected intrs usually is
driver vs hw misconfiguration, should we at some point
start to complain if we see one?

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 90628a47ae17..e37e3ec22a79 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1534,11 +1534,8 @@ static void gen8_gt_irq_ack(struct drm_i915_private *i915,
>  
>  	if (master_ctl & (GEN8_GT_PM_IRQ | GEN8_GT_GUC_IRQ)) {
>  		gt_iir[2] = raw_reg_read(regs, GEN8_GT_IIR(2));
> -		if (likely(gt_iir[2] & (i915->pm_rps_events |
> -					i915->pm_guc_events)))
> -			raw_reg_write(regs, GEN8_GT_IIR(2),
> -				      gt_iir[2] & (i915->pm_rps_events |
> -						   i915->pm_guc_events));
> +		if (likely(gt_iir[2]))
> +			raw_reg_write(regs, GEN8_GT_IIR(2), gt_iir[2]);
>  	}
>  
>  	if (master_ctl & GEN8_GT_VECS_IRQ) {
> -- 
> 2.18.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] drm/i915: Clear all residual RPS events on disabling interrupts

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Make sure that the RPS IIR is completely clear on disabling so we should
> not get any more interrupts after idling. Since the IIR is shared with
> the guc, we have to be careful to only clobber RPS events.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 8 +++++---
>  drivers/gpu/drm/i915/i915_reg.h | 6 ++++--
>  2 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index e37e3ec22a79..8084e35b25c5 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -478,7 +478,7 @@ void gen11_reset_rps_interrupts(struct drm_i915_private *dev_priv)
>  void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
>  {
>  	spin_lock_irq(&dev_priv->irq_lock);
> -	gen6_reset_pm_iir(dev_priv, dev_priv->pm_rps_events);
> +	gen6_reset_pm_iir(dev_priv, GEN6_PM_RPS_EVENTS);
>  	dev_priv->gt_pm.rps.pm_iir = 0;
>  	spin_unlock_irq(&dev_priv->irq_lock);
>  }
> @@ -516,7 +516,7 @@ void gen6_disable_rps_interrupts(struct drm_i915_private *dev_priv)
>  
>  	I915_WRITE(GEN6_PMINTRMSK, gen6_sanitize_rps_pm_mask(dev_priv, ~0u));
>  
> -	gen6_disable_pm_irq(dev_priv, dev_priv->pm_rps_events);
> +	gen6_disable_pm_irq(dev_priv, GEN6_PM_RPS_EVENTS);
>  
>  	spin_unlock_irq(&dev_priv->irq_lock);
>  	synchronize_irq(dev_priv->drm.irq);
> @@ -4778,7 +4778,9 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
>  		/* WaGsvRC0ResidencyMethod:vlv */
>  		dev_priv->pm_rps_events = GEN6_PM_RP_UP_EI_EXPIRED;
>  	else
> -		dev_priv->pm_rps_events = GEN6_PM_RPS_EVENTS;
> +		dev_priv->pm_rps_events = (GEN6_PM_RP_UP_THRESHOLD |
> +					   GEN6_PM_RP_DOWN_THRESHOLD |
> +					   GEN6_PM_RP_DOWN_TIMEOUT);
>  
>  	rps->pm_intrmsk_mbz = 0;
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index e0f5999fff07..4b656f31fde9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8582,8 +8582,10 @@ enum {
>  #define  GEN6_PM_RP_DOWN_THRESHOLD		(1 << 4)
>  #define  GEN6_PM_RP_UP_EI_EXPIRED		(1 << 2)
>  #define  GEN6_PM_RP_DOWN_EI_EXPIRED		(1 << 1)
> -#define  GEN6_PM_RPS_EVENTS			(GEN6_PM_RP_UP_THRESHOLD | \
> -						 GEN6_PM_RP_DOWN_THRESHOLD | \
> +#define  GEN6_PM_RPS_EVENTS			(GEN6_PM_RP_UP_EI_EXPIRED   | \
> +						 GEN6_PM_RP_UP_THRESHOLD    | \
> +						 GEN6_PM_RP_DOWN_EI_EXPIRED | \
> +						 GEN6_PM_RP_DOWN_THRESHOLD  | \
>  						 GEN6_PM_RP_DOWN_TIMEOUT)

GEN6_PM_RPS_MASK ?

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

>  
>  #define GEN7_GT_SCRATCH(i)			_MMIO(0x4F100 + (i) * 4)
> -- 
> 2.18.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] drm/i915: Clear all residual RPS events on disabling interrupts

[Chris Wilson]

Quoting Mika Kuoppala (2018-08-03 14:59:58)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index e0f5999fff07..4b656f31fde9 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -8582,8 +8582,10 @@ enum {
> >  #define  GEN6_PM_RP_DOWN_THRESHOLD           (1 << 4)
> >  #define  GEN6_PM_RP_UP_EI_EXPIRED            (1 << 2)
> >  #define  GEN6_PM_RP_DOWN_EI_EXPIRED          (1 << 1)
> > -#define  GEN6_PM_RPS_EVENTS                  (GEN6_PM_RP_UP_THRESHOLD | \
> > -                                              GEN6_PM_RP_DOWN_THRESHOLD | \
> > +#define  GEN6_PM_RPS_EVENTS                  (GEN6_PM_RP_UP_EI_EXPIRED   | \
> > +                                              GEN6_PM_RP_UP_THRESHOLD    | \
> > +                                              GEN6_PM_RP_DOWN_EI_EXPIRED | \
> > +                                              GEN6_PM_RP_DOWN_THRESHOLD  | \
> >                                                GEN6_PM_RP_DOWN_TIMEOUT)
> 
> GEN6_PM_RPS_MASK ?

I think we are still missing a few, so events as a superset of
pm_rps_events seems like a reasonable minimum change :)

Bonus, add the ones we are missing.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Unconditionally clear the pm/guc GT IIR upon acking

[Chris Wilson]

Quoting Mika Kuoppala (2018-08-03 14:56:54)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Having stored the IIR for action, we should always clear it.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> 
> As getting stray unexpected intrs usually is
> driver vs hw misconfiguration, should we at some point
> start to complain if we see one?

We have a history of doing strange things, sometimes even intentionally
;)

Maybe worth the effort in presilicon, but for the most part we are
trying desperately to minimise cost here.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx