Re: [PATCH] drm/i915: Remove overzealous fence warn on runtime suspend

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On pe, 2017-02-03 at 12:57 +0000, Chris Wilson wrote:
> 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: 7c108fd8feac ("drm/i915: Move fence cancellation to runtime suspend")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98804
> Reported-by: Lionel Landwerlin <lionel.g.landwerlin@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Imre Deak <imre.deak@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+

<SNIP>

> @@ -2024,8 +2024,16 @@ void i915_gem_runtime_suspend(struct drm_i915_private *dev_priv)
>  	for (i = 0; i < dev_priv->num_fence_regs; i++) {
>  		struct drm_i915_fence_reg *reg = &dev_priv->fence_regs[i];
>  
> -		if (WARN_ON(reg->pin_count))
> -			continue;
> +		/* Ideally we want to assert that the fence register is not
> +		 * live at this point (i.e. that no piece of code will is

s/will is/will be/

With that,

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx