Chris Wilson <[email protected]> writes: > Exploit that reads of the ring registers return 0 from the engine when > it is idle and we do not apply forcewake to know that if the engine is > idle then both reads will be identical (and so we interpret the ring as > idle). > > The ulterior motive is to try and reduce the number of spurious wakeups > to avoid untimely death, such as: > > <3> [85.046836] [drm:fw_domains_get [i915]] *ERROR* render: timed out waiting > for forcewake ack request. > <4> [85.051916] ------------[ cut here ]------------ > <4> [85.051917] GT thread status wait timed out > <4> [85.051963] WARNING: CPU: 2 PID: 2195 at > drivers/gpu/drm/i915/intel_uncore.c:303 > __gen6_gt_wait_for_thread_c0+0x6e/0xa0 [i915] > <4> [85.051964] Modules linked in: snd_hda_codec_hdmi snd_hda_codec_realtek > snd_hda_codec_generic i915 x86_pkg_temp_thermal coretemp mei_hdcp > crct10dif_pclmul crc32_pclmul snd_hda_intel ghash_clmulni_intel snd_hda_codec > broadcom bcm_phy_lib i2c_i801 snd_hwdep snd_hda_core tg3 snd_pcm ptp pps_core > mei_me mei prime_numbers lpc_ich > <4> [85.051980] CPU: 2 PID: 2195 Comm: drm_read Tainted: G U > 5.0.0-rc8-CI-CI_DRM_5662+ #1 > <4> [85.051981] Hardware name: Dell Inc. XPS 8300 /0Y2MRG, BIOS A06 > 10/17/2011 > <4> [85.052012] RIP: 0010:__gen6_gt_wait_for_thread_c0+0x6e/0xa0 [i915] > <4> [85.052015] Code: 8b 92 5c 80 13 00 83 e2 07 75 d5 5b 5d c3 80 3d 5b 6a > 1a 00 00 75 f4 48 c7 c7 38 21 31 a0 c6 05 4b 6a 1a 00 01 e8 e2 84 ea e0 <0f> > 0b eb dd 80 3d 3a 6a 1a 00 00 75 98 48 c7 c6 08 21 31 a0 48 c7 > <4> [85.052016] RSP: 0018:ffffc9000043bd00 EFLAGS: 00010086 > <4> [85.052019] RAX: 0000000000000000 RBX: ffff888217c50000 RCX: > 0000000000000000 > <4> [85.052020] RDX: 0000000000000007 RSI: ffffffff820cb141 RDI: > 00000000ffffffff > <4> [85.052022] RBP: 00000013cd30f2fb R08: 0000000000000000 R09: > 0000000000000001 > <4> [85.052024] R10: ffffc9000043bce0 R11: 0000000000000000 R12: > ffff888217c50ee0 > <4> [85.052025] R13: 0000000000000001 R14: 00000000ffffffff R15: > ffff888218076530 > <4> [85.052028] FS: 00007fc79d049980(0000) GS:ffff888227a80000(0000) > knlGS:0000000000000000 > <4> [85.052029] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > <4> [85.052031] CR2: 00007f782e2940f8 CR3: 000000022458e006 CR4: > 00000000000606e0 > <4> [85.052033] Call Trace: > <4> [85.052064] gen6_read32+0x14e/0x250 [i915] > <4> [85.052096] intel_engine_is_idle+0x7d/0x180 [i915] > <4> [85.052126] intel_engines_are_idle+0x29/0x50 [i915] > <4> [85.052153] i915_drop_caches_set+0x21c/0x290 [i915] > <4> [85.052160] simple_attr_write+0xb0/0xd0 > <4> [85.052165] full_proxy_write+0x51/0x80 > <4> [85.052170] __vfs_write+0x31/0x190 > <4> [85.052176] ? rcu_read_lock_sched_held+0x6f/0x80 > <4> [85.052178] ? rcu_sync_lockdep_assert+0x29/0x50 > <4> [85.052181] ? __sb_start_write+0x152/0x1f0 > <4> [85.052183] ? __sb_start_write+0x163/0x1f0 > <4> [85.052187] vfs_write+0xbd/0x1b0 > <4> [85.052191] ksys_write+0x50/0xc0 > <4> [85.052196] do_syscall_64+0x55/0x190 > <4> [85.052200] entry_SYSCALL_64_after_hwframe+0x49/0xbe > <4> [85.052202] RIP: 0033:0x7fc79c9d3281 > <4> [85.052204] Code: c3 0f 1f 84 00 00 00 00 00 48 8b 05 59 8d 20 00 c3 0f > 1f 84 00 00 00 00 00 8b 05 8a d1 20 00 85 c0 75 16 b8 01 00 00 00 0f 05 <48> > 3d 00 f0 ff ff 77 57 f3 c3 0f 1f 44 00 00 41 54 55 49 89 d4 53 > <4> [85.052206] RSP: 002b:00007fffa4a0a7f8 EFLAGS: 00000246 ORIG_RAX: > 0000000000000001 > <4> [85.052208] RAX: ffffffffffffffda RBX: 0000000000000001 RCX: > 00007fc79c9d3281 > <4> [85.052210] RDX: 0000000000000005 RSI: 00007fffa4a0a880 RDI: > 0000000000000008 > <4> [85.052212] RBP: 00007fffa4a0a820 R08: 0000000000000000 R09: > 0000000000000000 > <4> [85.052213] R10: 0000000000000000 R11: 0000000000000246 R12: > 00007fc79c9bc718 > <4> [85.052215] R13: 0000000000000003 R14: 00007fc79c9c1628 R15: > 00007fc79c9bdd80 > <4> [85.052223] irq event stamp: 71630 > <4> [85.052226] hardirqs last enabled at (71629): [<ffffffff8197b64c>] > _raw_spin_unlock_irqrestore+0x4c/0x60 > <4> [85.052228] hardirqs last disabled at (71630): [<ffffffff8197b4bd>] > _raw_spin_lock_irqsave+0xd/0x50 > <4> [85.052231] softirqs last enabled at (70444): [<ffffffff81c0033a>] > __do_softirq+0x33a/0x4b9 > <4> [85.052234] softirqs last disabled at (70433): [<ffffffff810b51b1>] > irq_exit+0xd1/0xe0 > <4> [85.052264] WARNING: CPU: 2 PID: 2195 at > drivers/gpu/drm/i915/intel_uncore.c:303 > __gen6_gt_wait_for_thread_c0+0x6e/0xa0 [i915] > > Signed-off-by: Chris Wilson <[email protected]> > Cc: Mika Kuoppala <[email protected]> > --- > drivers/gpu/drm/i915/intel_engine_cs.c | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c > b/drivers/gpu/drm/i915/intel_engine_cs.c > index b7b626195eda..4f244019560d 100644 > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > @@ -968,6 +968,7 @@ static bool ring_is_idle(struct intel_engine_cs *engine) > { > struct drm_i915_private *dev_priv = engine->i915; > intel_wakeref_t wakeref; > + unsigned long flags; > bool idle = true; > > if (I915_SELFTEST_ONLY(!engine->mmio_base)) > @@ -978,15 +979,19 @@ static bool ring_is_idle(struct intel_engine_cs *engine) > if (!wakeref) > return true; > > - /* First check that no commands are left in the ring */ > - if ((I915_READ_HEAD(engine) & HEAD_ADDR) != > - (I915_READ_TAIL(engine) & TAIL_ADDR)) > - idle = false; > + spin_lock_irqsave(&dev_priv->uncore.lock, flags); > > - /* No bit for gen2, so assume the CS parser is idle */ > - if (INTEL_GEN(dev_priv) > 2 && !(I915_READ_MODE(engine) & MODE_IDLE))
I agree on the sentiment that in here we should have a one marker only. And that is idle == (head == tail). We have MODE_IDLE checks on other parts but they dont affect the flow. Reviewed-by: Mika Kuoppala <[email protected]> > + /* > + * Check that no commands are left in the ring. > + * > + * If the engine is not awake, both reads return 0 as we do so without > + * forcewake. > + */ > + if ((I915_READ_FW(RING_HEAD(engine->mmio_base)) & HEAD_ADDR) != > + (I915_READ_FW(RING_TAIL(engine->mmio_base)) & TAIL_ADDR)) > idle = false; > > + spin_unlock_irqrestore(&dev_priv->uncore.lock, flags); > intel_runtime_pm_put(dev_priv, wakeref); > > return idle; > -- > 2.20.1 > > _______________________________________________ > Intel-gfx mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
