On Thu, Apr 19, 2012 at 06:10:18PM +0200, Takashi Iwai wrote:
> The hotplug work can be still kicked off via irq during PM, and this
> may conflict with the resume procedure. For example, eDP on SNB
> machine shows WARNING like below during the resume:
>
> WARNING: at /usr/src/packages/BUILD/kernel-default-3.0.13/linux-3.0/drivers/gpu/drm/i915/intel_dp.c:332 intel_dp_check_edp+0x73/0xd0 [i915]()
> Hardware name: HP Z1 Workstation
> eDP powered off while attempting aux channel communication.
> Supported: Yes
> Pid: 3210, comm: kworker/u:49 Tainted: G C N 3.0.13-0.27-default #1
> Call Trace:
> [<ffffffff810048b5>] dump_trace+0x75/0x300
> [<ffffffff8143ea0f>] dump_stack+0x69/0x6f
> [<ffffffff81059e2b>] warn_slowpath_common+0x7b/0xc0
> [<ffffffff81059f25>] warn_slowpath_fmt+0x45/0x50
> [<ffffffffa01fa9f3>] intel_dp_check_edp+0x73/0xd0 [i915]
> [<ffffffffa01fae4b>] intel_dp_aux_native_write+0x1b/0xe0 [i915]
> [<ffffffffa01fb033>] intel_dp_set_link_train+0x73/0xa0 [i915]
> [<ffffffffa01fb58e>] intel_dp_start_link_train+0x16e/0x400 [i915]
> [<ffffffffa01fbc6c>] intel_dp_complete_link_train+0x1fc/0x3d0 [i915]
> [<ffffffffa01fcf4c>] intel_dp_check_link_status+0x12c/0x1d0 [i915]
> [<ffffffffa01cf22e>] i915_hotplug_work_func+0x6e/0xa0 [i915]
> [<ffffffff810747bc>] process_one_work+0x16c/0x350
> [<ffffffff8107734a>] worker_thread+0x17a/0x410
> [<ffffffff8107b676>] kthread+0x96/0xa0
> [<ffffffff8144a7c4>] kernel_thread_helper+0x4/0x10
> DWARF2 unwinder stuck at kernel_thread_helper+0x4/0x10
>
> This patch adds a flag to disable the hotplug during PM operation for
> avoiding such a race.
>
> Cc: <stable at kernel.org>
> Signed-off-by: Takashi Iwai <tiwai at suse.de>
I haven't looked to closely, but isn't cancelling the hotplug work after
we disable the irqs in the suspend path good enough? This here feels a bit
like ducttapeing over the problem.
I'm asking because we seem to have other problems with work queue items
that leak across s/r and cause havoc on resume. So extracting this
quiescenting code from module unload and also running it at suspend time
sounds more like the right thing.
-Daniel
> ---
>
> It's a pretty simplistic solution that just ignores the hotplug work.
> We may keep some pending flag and process it later, if ignoring the
> event matters...
>
> drivers/gpu/drm/i915/i915_drv.c | 2 ++
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/i915_irq.c | 9 ++++++---
> 3 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index ae8a64f..dcbc8be 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -471,6 +471,7 @@ static int i915_drm_freeze(struct drm_device *dev)
>
> /* Modeset on resume, not lid events */
> dev_priv->modeset_on_lid = 0;
> + dev_priv->disable_hotplug_in_pm = true;
>
> console_lock();
> intel_fbdev_set_suspend(dev, 1);
> @@ -548,6 +549,7 @@ static int i915_drm_thaw(struct drm_device *dev)
>
> intel_opregion_init(dev);
>
> + dev_priv->disable_hotplug_in_pm = false;
> dev_priv->modeset_on_lid = 0;
>
> console_lock();
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5fabc6c..80d38ce 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -360,6 +360,7 @@ typedef struct drm_i915_private {
> u32 gt_irq_mask;
> u32 pch_irq_mask;
>
> + bool disable_hotplug_in_pm;
> u32 hotplug_supported_mask;
> struct work_struct hotplug_work;
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index afd4e03..f56f4bc 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -527,7 +527,8 @@ static irqreturn_t ivybridge_irq_handler(DRM_IRQ_ARGS)
>
> /* check event from PCH */
> if (de_iir & DE_PCH_EVENT_IVB) {
> - if (pch_iir & SDE_HOTPLUG_MASK_CPT)
> + if (!dev_priv->disable_hotplug_in_pm &&
> + (pch_iir & SDE_HOTPLUG_MASK_CPT))
> queue_work(dev_priv->wq, &dev_priv->hotplug_work);
> pch_irq_handler(dev);
> }
> @@ -627,7 +628,8 @@ static irqreturn_t ironlake_irq_handler(DRM_IRQ_ARGS)
>
> /* check event from PCH */
> if (de_iir & DE_PCH_EVENT) {
> - if (pch_iir & hotplug_mask)
> + if (!dev_priv->disable_hotplug_in_pm &&
> + (pch_iir & hotplug_mask))
> queue_work(dev_priv->wq, &dev_priv->hotplug_work);
> pch_irq_handler(dev);
> }
> @@ -1345,7 +1347,8 @@ static irqreturn_t i915_driver_irq_handler(DRM_IRQ_ARGS)
>
> DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n",
> hotplug_status);
> - if (hotplug_status & dev_priv->hotplug_supported_mask)
> + if (!dev_priv->disable_hotplug_in_pm &&
> + (hotplug_status & dev_priv->hotplug_supported_mask))
> queue_work(dev_priv->wq,
> &dev_priv->hotplug_work);
>
> --
> 1.7.9.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48
