diff mbox series

[1/5] drm/i915/gt: Always track callers to intel_rps_mark_interactive()

Message ID 20191030103827.2413-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show
Series [1/5] drm/i915/gt: Always track callers to intel_rps_mark_interactive() | expand

Commit Message

Chris Wilson Oct. 30, 2019, 10:38 a.m. UTC 
During startup, we may find ourselves in an interesting position where
we haven't fully enabled RPS before the display starts trying to use it.
This may lead to an imbalance in our "interactive" counter:

<3>[    4.813326] intel_rps_mark_interactive:652 GEM_BUG_ON(!rps->power.interactive)
<4>[    4.813396] ------------[ cut here ]------------
<2>[    4.813398] kernel BUG at drivers/gpu/drm/i915/gt/intel_rps.c:652!
<4>[    4.813430] invalid opcode: 0000 [#1] PREEMPT SMP PTI
<4>[    4.813438] CPU: 1 PID: 18 Comm: kworker/1:0H Not tainted 5.4.0-rc5-CI-CI_DRM_7209+ #1
<4>[    4.813447] Hardware name:  /NUC7i5BNB, BIOS BNKBL357.86A.0054.2017.1025.1822 10/25/2017
<4>[    4.813525] Workqueue: events_highpri intel_atomic_cleanup_work [i915]
<4>[    4.813589] RIP: 0010:intel_rps_mark_interactive+0xb3/0xc0 [i915]
<4>[    4.813597] Code: bc 3f de e0 48 8b 35 84 2e 24 00 49 c7 c0 f3 d4 4e a0 b9 8c 02 00 00 48 c7 c2 80 9c 48 a0 48 c7 c7 3e 73 34 a0 e8 8d 3b e5 e0 <0f> 0b 90 66 2e 0f 1f 84 00 00 00 00 00 80 bf c0 00 00 00 00 74 32
<4>[    4.813616] RSP: 0018:ffffc900000efe00 EFLAGS: 00010286
<4>[    4.813623] RAX: 000000000000000e RBX: ffff8882583cc7f0 RCX: 0000000000000000
<4>[    4.813631] RDX: 0000000000000001 RSI: 0000000000000008 RDI: ffff888275969c00
<4>[    4.813639] RBP: 0000000000000000 R08: 0000000000000008 R09: ffff888275ace000
<4>[    4.813646] R10: ffffc900000efe00 R11: ffff888275969c00 R12: ffff8882583cc8d8
<4>[    4.813654] R13: ffff888276abce00 R14: 0000000000000000 R15: ffff88825e878860
<4>[    4.813662] FS:  0000000000000000(0000) GS:ffff888276a80000(0000) knlGS:0000000000000000
<4>[    4.813672] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
<4>[    4.813678] CR2: 00007f051d5ca0a8 CR3: 0000000262f48001 CR4: 00000000003606e0
<4>[    4.813686] Call Trace:
<4>[    4.813755]  intel_cleanup_plane_fb+0x4e/0x60 [i915]
<4>[    4.813764]  drm_atomic_helper_cleanup_planes+0x4d/0x70
<4>[    4.813833]  intel_atomic_cleanup_work+0x15/0x80 [i915]
<4>[    4.813842]  process_one_work+0x26a/0x620
<4>[    4.813850]  worker_thread+0x37/0x380
<4>[    4.813857]  ? process_one_work+0x620/0x620
<4>[    4.813864]  kthread+0x119/0x130
<4>[    4.813870]  ? kthread_park+0x80/0x80
<4>[    4.813878]  ret_from_fork+0x3a/0x50
<4>[    4.813887] Modules linked in: i915(+) mei_hdcp x86_pkg_temp_thermal coretemp crct10dif_pclmul crc32_pclmul btusb btrtl btbcm btintel snd_hda_intel snd_intel_nhlt snd_hda_codec bluetooth snd_hwdep snd_hda_core ghash_clmulni_intel snd_pcm e1000e ecdh_generic ecc ptp pps_core mei_me mei prime_numbers
<4>[    4.813934] ---[ end trace c13289af88174ffc ]---

The solution employed is to not worry about RPS state and keep the tally
of the interactive counter separate. When we do enable RPS, we will then
take the display activity into account.

Fixes: 3e7abf814193 ("drm/i915: Extract GT render power state management")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Andi Shyti <andi.shyti@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt.c  |  2 ++
 drivers/gpu/drm/i915/gt/intel_rps.c | 12 ++++++------
 drivers/gpu/drm/i915/gt/intel_rps.h |  1 +
 3 files changed, 9 insertions(+), 6 deletions(-)

Comments

Andi Shyti Oct. 30, 2019, 12:34 p.m. UTC | #1 
Hi Chris,

On Wed, Oct 30, 2019 at 10:38:23AM +0000, Chris Wilson wrote:
> During startup, we may find ourselves in an interesting position where
> we haven't fully enabled RPS before the display starts trying to use it.
> This may lead to an imbalance in our "interactive" counter:

yes, makes sense! Thanks!

Acked-by: Andi Shyti <andi.shyti@intel.com>

Andi
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index f2440113fdf8..898662c158ad 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -26,6 +26,8 @@  void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915)
 	intel_gt_init_reset(gt);
 	intel_gt_init_requests(gt);
 	intel_gt_pm_init_early(gt);
+
+	intel_rps_init_early(&gt->rps);
 	intel_uc_init_early(&gt->uc);
 }
 
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
index f6de19456c54..20d6ee148afc 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -641,9 +641,6 @@  static void gen6_rps_set_thresholds(struct intel_rps *rps, u8 val)
 
 void intel_rps_mark_interactive(struct intel_rps *rps, bool interactive)
 {
-	if (!rps->enabled)
-		return;
-
 	mutex_lock(&rps->power.mutex);
 	if (interactive) {
 		if (!rps->power.interactive++ && rps->active)
@@ -1609,16 +1606,19 @@  void gen5_rps_irq_handler(struct intel_rps *rps)
 	spin_unlock(&mchdev_lock);
 }
 
-void intel_rps_init(struct intel_rps *rps)
+void intel_rps_init_early(struct intel_rps *rps)
 {
-	struct drm_i915_private *i915 = rps_to_i915(rps);
-
 	mutex_init(&rps->lock);
 	mutex_init(&rps->power.mutex);
 
 	INIT_WORK(&rps->work, rps_work);
 
 	atomic_set(&rps->num_waiters, 0);
+}
+
+void intel_rps_init(struct intel_rps *rps)
+{
+	struct drm_i915_private *i915 = rps_to_i915(rps);
 
 	if (IS_CHERRYVIEW(i915))
 		chv_rps_init(rps);
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.h b/drivers/gpu/drm/i915/gt/intel_rps.h
index 997a4b4e0207..9518c66c9792 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.h
+++ b/drivers/gpu/drm/i915/gt/intel_rps.h
@@ -11,6 +11,7 @@ 
 
 struct i915_request;
 
+void intel_rps_init_early(struct intel_rps *rps);
 void intel_rps_init(struct intel_rps *rps);
 
 void intel_rps_driver_register(struct intel_rps *rps);