Skip to content

Commit e0ec3ec

Browse files
committed
drm/i915: Remove overzealous fence warn on runtime suspend
The goal of the WARN was to catch when we are still actively using the fence as we go into the runtime suspend. However, the reg->pin_count is too coarse as it does not distinguish between exclusive ownership of the fence register from activity. I've not improved on the WARN, nor have we captured this WARN in an exact igt, but it is showing up regularly in the wild: [ 1915.935332] WARNING: CPU: 1 PID: 10861 at drivers/gpu/drm/i915/i915_gem.c:2022 i915_gem_runtime_suspend+0x116/0x130 [i915] [ 1915.935383] WARN_ON(reg->pin_count)[ 1915.935399] Modules linked in: snd_hda_intel i915 drm_kms_helper vgem netconsole scsi_transport_iscsi fuse vfat fat x86_pkg_temp_thermal coretemp intel_cstate intel_uncore snd_hda_codec_hdmi snd_hda_codec_generic snd_hda_codec snd_hwdep snd_hda_core snd_pcm snd_timer snd mei_me mei serio_raw intel_rapl_perf intel_pch_thermal soundcore wmi acpi_pad i2c_algo_bit syscopyarea sysfillrect sysimgblt fb_sys_fops drm r8169 mii video [last unloaded: drm_kms_helper] [ 1915.935785] CPU: 1 PID: 10861 Comm: kworker/1:0 Tainted: G U W 4.9.0-rc5+ #170 [ 1915.935799] Hardware name: LENOVO 80MX/Lenovo E31-80, BIOS DCCN34WW(V2.03) 12/01/2015 [ 1915.935822] Workqueue: pm pm_runtime_work [ 1915.935845] ffffc900044fbbf0 ffffffffac3220bc ffffc900044fbc40 0000000000000000 [ 1915.935890] ffffc900044fbc30 ffffffffac059bcb 000007e6044fbc60 ffff8801626e3198 [ 1915.935937] ffff8801626e0000 0000000000000002 ffffffffc05e5d4e 0000000000000000 [ 1915.935985] Call Trace: [ 1915.936013] [<ffffffffac3220bc>] dump_stack+0x4f/0x73 [ 1915.936038] [<ffffffffac059bcb>] __warn+0xcb/0xf0 [ 1915.936060] [<ffffffffac059c4f>] warn_slowpath_fmt+0x5f/0x80 [ 1915.936158] [<ffffffffc052d916>] i915_gem_runtime_suspend+0x116/0x130 [i915] [ 1915.936251] [<ffffffffc04f1c74>] intel_runtime_suspend+0x64/0x280 [i915] [ 1915.936277] [<ffffffffac0926f1>] ? dequeue_entity+0x241/0xbc0 [ 1915.936298] [<ffffffffac36bb85>] pci_pm_runtime_suspend+0x55/0x180 [ 1915.936317] [<ffffffffac36bb30>] ? pci_pm_runtime_resume+0xa0/0xa0 [ 1915.936339] [<ffffffffac4514e2>] __rpm_callback+0x32/0x70 [ 1915.936356] [<ffffffffac451544>] rpm_callback+0x24/0x80 [ 1915.936375] [<ffffffffac36bb30>] ? pci_pm_runtime_resume+0xa0/0xa0 [ 1915.936392] [<ffffffffac45222d>] rpm_suspend+0x12d/0x680 [ 1915.936415] [<ffffffffac69f6d7>] ? _raw_spin_unlock_irq+0x17/0x30 [ 1915.936435] [<ffffffffac0810b8>] ? finish_task_switch+0x88/0x220 [ 1915.936455] [<ffffffffac4534bf>] pm_runtime_work+0x6f/0xb0 [ 1915.936477] [<ffffffffac074353>] process_one_work+0x1f3/0x4d0 [ 1915.936501] [<ffffffffac074678>] worker_thread+0x48/0x4e0 [ 1915.936523] [<ffffffffac074630>] ? process_one_work+0x4d0/0x4d0 [ 1915.936542] [<ffffffffac074630>] ? process_one_work+0x4d0/0x4d0 [ 1915.936559] [<ffffffffac07a2c9>] kthread+0xd9/0xf0 [ 1915.936580] [<ffffffffac07a1f0>] ? kthread_park+0x60/0x60 [ 1915.936600] [<ffffffffac69fe62>] ret_from_fork+0x22/0x30 In the case the register is pinned, it should be present and we will need to invalidate them to be restored upon resume as we cannot expect the owner of the pin to call get_fence prior to use after resume. Fixes: 7c108fd ("drm/i915: Move fence cancellation to runtime suspend") Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98804 Reported-by: Lionel Landwerlin <[email protected]> Signed-off-by: Chris Wilson <[email protected]> Cc: Daniel Vetter <[email protected]> Cc: Imre Deak <[email protected]> Cc: Jani Nikula <[email protected]> Cc: <[email protected]> # v4.10-rc1+ Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Reviewed-by: Joonas Lahtinen <[email protected]>
1 parent 418e3cd commit e0ec3ec

File tree

2 files changed

+11
-2
lines changed

2 files changed

+11
-2
lines changed

drivers/gpu/drm/i915/i915_drv.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2475,6 +2475,7 @@ static int intel_runtime_resume(struct device *kdev)
24752475
* we can do is to hope that things will still work (and disable RPM).
24762476
*/
24772477
i915_gem_init_swizzling(dev_priv);
2478+
i915_gem_restore_fences(dev_priv);
24782479

24792480
intel_runtime_pm_enable_interrupts(dev_priv);
24802481

drivers/gpu/drm/i915/i915_gem.c

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2011,8 +2011,16 @@ void i915_gem_runtime_suspend(struct drm_i915_private *dev_priv)
20112011
for (i = 0; i < dev_priv->num_fence_regs; i++) {
20122012
struct drm_i915_fence_reg *reg = &dev_priv->fence_regs[i];
20132013

2014-
if (WARN_ON(reg->pin_count))
2015-
continue;
2014+
/* Ideally we want to assert that the fence register is not
2015+
* live at this point (i.e. that no piece of code will be
2016+
* trying to write through fence + GTT, as that both violates
2017+
* our tracking of activity and associated locking/barriers,
2018+
* but also is illegal given that the hw is powered down).
2019+
*
2020+
* Previously we used reg->pin_count as a "liveness" indicator.
2021+
* That is not sufficient, and we need a more fine-grained
2022+
* tool if we want to have a sanity check here.
2023+
*/
20162024

20172025
if (!reg->vma)
20182026
continue;

0 commit comments

Comments
 (0)