[PATCH] drm/i915: Suppress hotplug work during PM suspend/resume

[PATCH] drm/i915: Suppress hotplug work during PM suspend/resume

[Takashi Iwai]

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@kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---

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

Re: [PATCH] drm/i915: Suppress hotplug work during PM suspend/resume

[Chris Wilson]

On Thu, 19 Apr 2012 18:10:18 +0200, Takashi Iwai <tiwai@suse.de> 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:

Daniel and I were just discussing the number of work queues we have
active across suspend that like to poke hardware as we either try to
sleep or resume. Since we already have to quiesce the workers to disable
the device, I suggest we reuse the same code for suspend.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Suppress hotplug work during PM suspend/resume

[Takashi Iwai]

At Thu, 19 Apr 2012 17:22:47 +0100,
Chris Wilson wrote:
> 
> On Thu, 19 Apr 2012 18:10:18 +0200, Takashi Iwai <tiwai@suse.de> 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:
> 
> Daniel and I were just discussing the number of work queues we have
> active across suspend that like to poke hardware as we either try to
> sleep or resume. Since we already have to quiesce the workers to disable
> the device, I suggest we reuse the same code for suspend.

Hm, sorry it's not clear to me which code you are referring to...
Is it only in Daniel's next tree, or am I missing in Linus tree?


thanks,

Takashi

Re: [PATCH] drm/i915: Suppress hotplug work during PM suspend/resume

[Daniel Vetter]

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@kernel.org>
> Signed-off-by: Takashi Iwai <tiwai@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] drm/i915: Suppress hotplug work during PM suspend/resume

[Takashi Iwai]

At Thu, 19 Apr 2012 18:29:47 +0200,
Daniel Vetter wrote:
> 
> 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@kernel.org>
> > Signed-off-by: Takashi Iwai <tiwai@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.

This doesn't look like a leftover work.  Judging from the log I got,
the hotplug event is kicked really from the irq handler in the resume
phase.

[   53.424757] ehci_hcd 0000:00:1d.0: cache line size of 64 is not supported
[   53.452721] [drm:intel_enable_rc6], Sandybridge: RC6 disabled
[   53.477048] firewire_core: skipped bus generations, destroying all nodes
[   53.504839] [drm:intel_opregion_setup], graphic opregion physical addr: 0x73d1c018
[   53.504929] [drm:intel_opregion_setup], Public ACPI methods supported
[   53.504930] [drm:intel_opregion_setup], SWSCI supported
[   53.504931] [drm:intel_opregion_setup], ASLE supported
[   53.504962] [drm:init_status_page], render ring hws offset: 0x00000000
[   53.505047] [drm:init_status_page], gen6 bsd ring hws offset: 0x00022000
[   53.505118] [drm:init_status_page], blt ring hws offset: 0x00043000
[   53.505185] [drm:ironlake_init_pch_refclk], has_panel 1 has_lvds 0 has_pch_edp 1 has_cpu_edp 0 has_ck505 0
[   53.505188] [drm:ironlake_init_pch_refclk], Using SSC on panel
[   53.505611] [drm:intel_dp_mode_fixup], Display port link bw 0a lane count 4 clock 270000
[   53.505613] [drm:drm_crtc_helper_set_mode], [CRTC:3]
[   53.505615] [drm:ironlake_edp_backlight_off], 
[   53.512832] [drm:ironlake_edp_panel_off], Turn eDP power off
[   53.512836] [drm:ironlake_wait_panel_off], Wait for panel power off time
[   53.512839] [drm:ironlake_wait_panel_status], mask b000000f value 00000000 status c0000008 control abcd0000
[   53.652153] [drm:pch_irq_handler], PCH HDCP audio interrupt
[   53.652159] [drm:i915_hotplug_work_func], running encoder hotplug functions
[   53.653780] [drm:intel_dp_check_link_status], TMDS-6: channel EQ not ok, retraining
[   53.654188] [drm:intel_dp_start_link_train], training pattern 1 signal levels 00000000
....
[   53.680513] [drm:intel_dp_start_link_train], too many full retries, give up
...
[   53.714791] [drm:intel_dp_start_link_train], training pattern 1 signal levels 06000000
[   53.715198] ------------[ cut here ]------------
[   53.715230] 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]()
...

> 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.

Yes, this should be needed anyway, I think.  But the new hotplug event
can be still generated, as it seems, and this would conflict with the
initialization beind done in the resume.


thanks,

Takashi

Re: [PATCH] drm/i915: Suppress hotplug work during PM suspend/resume

[Daniel Vetter]

On Thu, Apr 19, 2012 at 06:38:13PM +0200, Takashi Iwai wrote:
> At Thu, 19 Apr 2012 18:29:47 +0200,
> Daniel Vetter wrote:
> > 
> > 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@kernel.org>
> > > Signed-off-by: Takashi Iwai <tiwai@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.
> 
> This doesn't look like a leftover work.  Judging from the log I got,
> the hotplug event is kicked really from the irq handler in the resume
> phase.

In that case I guess we have a setup ordering issue on the resume side. If
we enable irqs before everything is set up again, we will fail because the
resume path doesn't grab any locks ...

> [   53.424757] ehci_hcd 0000:00:1d.0: cache line size of 64 is not supported
> [   53.452721] [drm:intel_enable_rc6], Sandybridge: RC6 disabled
> [   53.477048] firewire_core: skipped bus generations, destroying all nodes
> [   53.504839] [drm:intel_opregion_setup], graphic opregion physical addr: 0x73d1c018
> [   53.504929] [drm:intel_opregion_setup], Public ACPI methods supported
> [   53.504930] [drm:intel_opregion_setup], SWSCI supported
> [   53.504931] [drm:intel_opregion_setup], ASLE supported
> [   53.504962] [drm:init_status_page], render ring hws offset: 0x00000000
> [   53.505047] [drm:init_status_page], gen6 bsd ring hws offset: 0x00022000
> [   53.505118] [drm:init_status_page], blt ring hws offset: 0x00043000
> [   53.505185] [drm:ironlake_init_pch_refclk], has_panel 1 has_lvds 0 has_pch_edp 1 has_cpu_edp 0 has_ck505 0
> [   53.505188] [drm:ironlake_init_pch_refclk], Using SSC on panel
> [   53.505611] [drm:intel_dp_mode_fixup], Display port link bw 0a lane count 4 clock 270000
> [   53.505613] [drm:drm_crtc_helper_set_mode], [CRTC:3]
> [   53.505615] [drm:ironlake_edp_backlight_off], 
> [   53.512832] [drm:ironlake_edp_panel_off], Turn eDP power off
> [   53.512836] [drm:ironlake_wait_panel_off], Wait for panel power off time
> [   53.512839] [drm:ironlake_wait_panel_status], mask b000000f value 00000000 status c0000008 control abcd0000
> [   53.652153] [drm:pch_irq_handler], PCH HDCP audio interrupt
> [   53.652159] [drm:i915_hotplug_work_func], running encoder hotplug functions
> [   53.653780] [drm:intel_dp_check_link_status], TMDS-6: channel EQ not ok, retraining
> [   53.654188] [drm:intel_dp_start_link_train], training pattern 1 signal levels 00000000
> ....
> [   53.680513] [drm:intel_dp_start_link_train], too many full retries, give up
> ...
> [   53.714791] [drm:intel_dp_start_link_train], training pattern 1 signal levels 06000000
> [   53.715198] ------------[ cut here ]------------
> [   53.715230] 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]()
> ...
> 
> > 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.
> 
> Yes, this should be needed anyway, I think.  But the new hotplug event
> can be still generated, as it seems, and this would conflict with the
> initialization beind done in the resume.

Yeah, we have a few holes to plug in s/r ...
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] drm/i915: Suppress hotplug work during PM suspend/resume

[Adam Jackson]

[-- Attachment #1.1: Type: text/plain, Size: 943 bytes --]

On Thu, 2012-04-19 at 18:10 +0200, Takashi Iwai wrote:

> This patch adds a flag to disable the hotplug during PM operation for
> avoiding such a race.
> 
> Cc: <stable@kernel.org>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

This seems simpler (untested):

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1a7559b..1cd77a4 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -530,11 +530,12 @@ static int i915_drm_thaw(struct drm_device *dev)
                        ironlake_init_pch_refclk(dev);
 
                drm_mode_config_reset(dev);
-               drm_irq_install(dev);
 
                /* Resume the modeset for every activated CRTC */
                drm_helper_resume_force_mode(dev);
 
+               drm_irq_install(dev);
+
                if (IS_IRONLAKE_M(dev))
                        ironlake_enable_rc6(dev);

- ajax

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Suppress hotplug work during PM suspend/resume

[Takashi Iwai]

At Thu, 19 Apr 2012 13:55:04 -0400,
Adam Jackson wrote:
> 
> On Thu, 2012-04-19 at 18:10 +0200, Takashi Iwai wrote:
> 
> > This patch adds a flag to disable the hotplug during PM operation for
> > avoiding such a race.
> > 
> > Cc: <stable@kernel.org>
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> 
> This seems simpler (untested):

This looks promising.  I'll ask a test with your patch.


thanks,

Takashi


> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 1a7559b..1cd77a4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -530,11 +530,12 @@ static int i915_drm_thaw(struct drm_device *dev)
>                         ironlake_init_pch_refclk(dev);
>  
>                 drm_mode_config_reset(dev);
> -               drm_irq_install(dev);
>  
>                 /* Resume the modeset for every activated CRTC */
>                 drm_helper_resume_force_mode(dev);
>  
> +               drm_irq_install(dev);
> +
>                 if (IS_IRONLAKE_M(dev))
>                         ironlake_enable_rc6(dev);
> 
> - ajax

Re: [PATCH] drm/i915: Suppress hotplug work during PM suspend/resume

[Takashi Iwai]

At Thu, 19 Apr 2012 20:11:53 +0200,
Takashi Iwai wrote:
> 
> At Thu, 19 Apr 2012 13:55:04 -0400,
> Adam Jackson wrote:
> > 
> > On Thu, 2012-04-19 at 18:10 +0200, Takashi Iwai wrote:
> > 
> > > This patch adds a flag to disable the hotplug during PM operation for
> > > avoiding such a race.
> > > 
> > > Cc: <stable@kernel.org>
> > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > 
> > This seems simpler (untested):
> 
> This looks promising.  I'll ask a test with your patch.

Tester reported a positive feedback.  But he also experienced with a
blank screen after a couple of S4 resumes.  Now it's being checked
whether it's a regression by the patch or not.

The machine has a problem with S3 anyway, also showing a blank screen
after S3 resume.  So, the problem is likely irrelevant from your
patch, but just to be sure.


thanks,

Takashi

> 
> 
> thanks,
> 
> Takashi
> 
> 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 1a7559b..1cd77a4 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -530,11 +530,12 @@ static int i915_drm_thaw(struct drm_device *dev)
> >                         ironlake_init_pch_refclk(dev);
> >  
> >                 drm_mode_config_reset(dev);
> > -               drm_irq_install(dev);
> >  
> >                 /* Resume the modeset for every activated CRTC */
> >                 drm_helper_resume_force_mode(dev);
> >  
> > +               drm_irq_install(dev);
> > +
> >                 if (IS_IRONLAKE_M(dev))
> >                         ironlake_enable_rc6(dev);
> > 
> > - ajax
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 

Re: [PATCH] drm/i915: Suppress hotplug work during PM suspend/resume

[Takashi Iwai]

At Wed, 25 Apr 2012 10:14:51 +0200,
Takashi Iwai wrote:
> 
> At Thu, 19 Apr 2012 20:11:53 +0200,
> Takashi Iwai wrote:
> > 
> > At Thu, 19 Apr 2012 13:55:04 -0400,
> > Adam Jackson wrote:
> > > 
> > > On Thu, 2012-04-19 at 18:10 +0200, Takashi Iwai wrote:
> > > 
> > > > This patch adds a flag to disable the hotplug during PM operation for
> > > > avoiding such a race.
> > > > 
> > > > Cc: <stable@kernel.org>
> > > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > > 
> > > This seems simpler (untested):
> > 
> > This looks promising.  I'll ask a test with your patch.
> 
> Tester reported a positive feedback.  But he also experienced with a
> blank screen after a couple of S4 resumes.  Now it's being checked
> whether it's a regression by the patch or not.

It seems unrelated with the patch itself.  So, from my side,
	Reviewed-by: Takashi Iwai <tiwai@suse.de>

Adam, could you resubmit it with a proper sign-off so that it can be
merged for 3.4 or 3.5, preferably with Cc to stable kernel?


thanks,

Takashi

> 
> The machine has a problem with S3 anyway, also showing a blank screen
> after S3 resume.  So, the problem is likely irrelevant from your
> patch, but just to be sure.
> 
> 
> thanks,
> 
> Takashi
> 
> > 
> > 
> > thanks,
> > 
> > Takashi
> > 
> > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > index 1a7559b..1cd77a4 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -530,11 +530,12 @@ static int i915_drm_thaw(struct drm_device *dev)
> > >                         ironlake_init_pch_refclk(dev);
> > >  
> > >                 drm_mode_config_reset(dev);
> > > -               drm_irq_install(dev);
> > >  
> > >                 /* Resume the modeset for every activated CRTC */
> > >                 drm_helper_resume_force_mode(dev);
> > >  
> > > +               drm_irq_install(dev);
> > > +
> > >                 if (IS_IRONLAKE_M(dev))
> > >                         ironlake_enable_rc6(dev);
> > > 
> > > - ajax
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 

Re: [PATCH] drm/i915: Suppress hotplug work during PM suspend/resume

[Daniel Vetter]

On Thu, May 10, 2012 at 08:34:43AM +0200, Takashi Iwai wrote:
> At Wed, 25 Apr 2012 10:14:51 +0200,
> Takashi Iwai wrote:
> > 
> > At Thu, 19 Apr 2012 20:11:53 +0200,
> > Takashi Iwai wrote:
> > > 
> > > At Thu, 19 Apr 2012 13:55:04 -0400,
> > > Adam Jackson wrote:
> > > > 
> > > > On Thu, 2012-04-19 at 18:10 +0200, Takashi Iwai wrote:
> > > > 
> > > > > This patch adds a flag to disable the hotplug during PM operation for
> > > > > avoiding such a race.
> > > > > 
> > > > > Cc: <stable@kernel.org>
> > > > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > > > 
> > > > This seems simpler (untested):
> > > 
> > > This looks promising.  I'll ask a test with your patch.
> > 
> > Tester reported a positive feedback.  But he also experienced with a
> > blank screen after a couple of S4 resumes.  Now it's being checked
> > whether it's a regression by the patch or not.
> 
> It seems unrelated with the patch itself.  So, from my side,
> 	Reviewed-by: Takashi Iwai <tiwai@suse.de>
> 
> Adam, could you resubmit it with a proper sign-off so that it can be
> merged for 3.4 or 3.5, preferably with Cc to stable kernel?

Note that

daniel@phenom:~/linux/src$ git show
8b2e326dc7c5aa6952c88656d04d0d81fd85a6f8
commit 8b2e326dc7c5aa6952c88656d04d0d81fd85a6f8
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Tue Apr 24 22:59:41 2012 +0100

    drm/i915: Unconditionally initialise the interrupt workers

in drm-intel-next is closes a race around suspend/resume that could lead
to an oops somewhere on resume.

-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] drm/i915: Suppress hotplug work during PM suspend/resume

[Takashi Iwai]

At Thu, 10 May 2012 10:29:19 +0200,
Daniel Vetter wrote:
> 
> On Thu, May 10, 2012 at 08:34:43AM +0200, Takashi Iwai wrote:
> > At Wed, 25 Apr 2012 10:14:51 +0200,
> > Takashi Iwai wrote:
> > > 
> > > At Thu, 19 Apr 2012 20:11:53 +0200,
> > > Takashi Iwai wrote:
> > > > 
> > > > At Thu, 19 Apr 2012 13:55:04 -0400,
> > > > Adam Jackson wrote:
> > > > > 
> > > > > On Thu, 2012-04-19 at 18:10 +0200, Takashi Iwai wrote:
> > > > > 
> > > > > > This patch adds a flag to disable the hotplug during PM operation for
> > > > > > avoiding such a race.
> > > > > > 
> > > > > > Cc: <stable@kernel.org>
> > > > > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > > > > 
> > > > > This seems simpler (untested):
> > > > 
> > > > This looks promising.  I'll ask a test with your patch.
> > > 
> > > Tester reported a positive feedback.  But he also experienced with a
> > > blank screen after a couple of S4 resumes.  Now it's being checked
> > > whether it's a regression by the patch or not.
> > 
> > It seems unrelated with the patch itself.  So, from my side,
> > 	Reviewed-by: Takashi Iwai <tiwai@suse.de>
> > 
> > Adam, could you resubmit it with a proper sign-off so that it can be
> > merged for 3.4 or 3.5, preferably with Cc to stable kernel?
> 
> Note that
> 
> daniel@phenom:~/linux/src$ git show
> 8b2e326dc7c5aa6952c88656d04d0d81fd85a6f8
> commit 8b2e326dc7c5aa6952c88656d04d0d81fd85a6f8
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Tue Apr 24 22:59:41 2012 +0100
> 
>     drm/i915: Unconditionally initialise the interrupt workers
> 
> in drm-intel-next is closes a race around suspend/resume that could lead
> to an oops somewhere on resume.

I don't think it's missing hotplug initializations. It's no Oops but
the race between the resume procedure being processed and the hotplug
work triggered during the resume procedure.


thanks,

Takashi

Re: [PATCH] drm/i915: Suppress hotplug work during PM suspend/resume

[Daniel Vetter]

On Thu, May 10, 2012 at 10:40:52AM +0200, Takashi Iwai wrote:
> At Thu, 10 May 2012 10:29:19 +0200,
> Daniel Vetter wrote:
> > 
> > On Thu, May 10, 2012 at 08:34:43AM +0200, Takashi Iwai wrote:
> > > At Wed, 25 Apr 2012 10:14:51 +0200,
> > > Takashi Iwai wrote:
> > > > 
> > > > At Thu, 19 Apr 2012 20:11:53 +0200,
> > > > Takashi Iwai wrote:
> > > > > 
> > > > > At Thu, 19 Apr 2012 13:55:04 -0400,
> > > > > Adam Jackson wrote:
> > > > > > 
> > > > > > On Thu, 2012-04-19 at 18:10 +0200, Takashi Iwai wrote:
> > > > > > 
> > > > > > > This patch adds a flag to disable the hotplug during PM operation for
> > > > > > > avoiding such a race.
> > > > > > > 
> > > > > > > Cc: <stable@kernel.org>
> > > > > > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > > > > > 
> > > > > > This seems simpler (untested):
> > > > > 
> > > > > This looks promising.  I'll ask a test with your patch.
> > > > 
> > > > Tester reported a positive feedback.  But he also experienced with a
> > > > blank screen after a couple of S4 resumes.  Now it's being checked
> > > > whether it's a regression by the patch or not.
> > > 
> > > It seems unrelated with the patch itself.  So, from my side,
> > > 	Reviewed-by: Takashi Iwai <tiwai@suse.de>
> > > 
> > > Adam, could you resubmit it with a proper sign-off so that it can be
> > > merged for 3.4 or 3.5, preferably with Cc to stable kernel?
> > 
> > Note that
> > 
> > daniel@phenom:~/linux/src$ git show
> > 8b2e326dc7c5aa6952c88656d04d0d81fd85a6f8
> > commit 8b2e326dc7c5aa6952c88656d04d0d81fd85a6f8
> > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > Date:   Tue Apr 24 22:59:41 2012 +0100
> > 
> >     drm/i915: Unconditionally initialise the interrupt workers
> > 
> > in drm-intel-next is closes a race around suspend/resume that could lead
> > to an oops somewhere on resume.
> 
> I don't think it's missing hotplug initializations. It's no Oops but
> the race between the resume procedure being processed and the hotplug
> work triggered during the resume procedure.

This patch is not just for hotplug, but for all the delayed work and timer
stuff the driver does. And we _do_ have a bug report that leaking the rps
work (for snb+ turbo mode) across either a s/r cycle or a gpu reset kills
the driver.

-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] drm/i915: Suppress hotplug work during PM suspend/resume

[Takashi Iwai]

At Thu, 10 May 2012 11:06:46 +0200,
Daniel Vetter wrote:
> 
> On Thu, May 10, 2012 at 10:40:52AM +0200, Takashi Iwai wrote:
> > At Thu, 10 May 2012 10:29:19 +0200,
> > Daniel Vetter wrote:
> > > 
> > > On Thu, May 10, 2012 at 08:34:43AM +0200, Takashi Iwai wrote:
> > > > At Wed, 25 Apr 2012 10:14:51 +0200,
> > > > Takashi Iwai wrote:
> > > > > 
> > > > > At Thu, 19 Apr 2012 20:11:53 +0200,
> > > > > Takashi Iwai wrote:
> > > > > > 
> > > > > > At Thu, 19 Apr 2012 13:55:04 -0400,
> > > > > > Adam Jackson wrote:
> > > > > > > 
> > > > > > > On Thu, 2012-04-19 at 18:10 +0200, Takashi Iwai wrote:
> > > > > > > 
> > > > > > > > This patch adds a flag to disable the hotplug during PM operation for
> > > > > > > > avoiding such a race.
> > > > > > > > 
> > > > > > > > Cc: <stable@kernel.org>
> > > > > > > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > > > > > > 
> > > > > > > This seems simpler (untested):
> > > > > > 
> > > > > > This looks promising.  I'll ask a test with your patch.
> > > > > 
> > > > > Tester reported a positive feedback.  But he also experienced with a
> > > > > blank screen after a couple of S4 resumes.  Now it's being checked
> > > > > whether it's a regression by the patch or not.
> > > > 
> > > > It seems unrelated with the patch itself.  So, from my side,
> > > > 	Reviewed-by: Takashi Iwai <tiwai@suse.de>
> > > > 
> > > > Adam, could you resubmit it with a proper sign-off so that it can be
> > > > merged for 3.4 or 3.5, preferably with Cc to stable kernel?
> > > 
> > > Note that
> > > 
> > > daniel@phenom:~/linux/src$ git show
> > > 8b2e326dc7c5aa6952c88656d04d0d81fd85a6f8
> > > commit 8b2e326dc7c5aa6952c88656d04d0d81fd85a6f8
> > > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > > Date:   Tue Apr 24 22:59:41 2012 +0100
> > > 
> > >     drm/i915: Unconditionally initialise the interrupt workers
> > > 
> > > in drm-intel-next is closes a race around suspend/resume that could lead
> > > to an oops somewhere on resume.
> > 
> > I don't think it's missing hotplug initializations. It's no Oops but
> > the race between the resume procedure being processed and the hotplug
> > work triggered during the resume procedure.
> 
> This patch is not just for hotplug, but for all the delayed work and timer
> stuff the driver does. And we _do_ have a bug report that leaking the rps
> work (for snb+ turbo mode) across either a s/r cycle or a gpu reset kills
> the driver.

Yes, but my point is that Chris's patch (at least the commit above
alone) won't fix the problem we faced.


Takashi

Re: [PATCH] drm/i915: Suppress hotplug work during PM suspend/resume

[Chris Wilson]

On Thu, 10 May 2012 11:19:11 +0200, Takashi Iwai <tiwai@suse.de> wrote:
> At Thu, 10 May 2012 11:06:46 +0200,
> Daniel Vetter wrote:
> > This patch is not just for hotplug, but for all the delayed work and timer
> > stuff the driver does. And we _do_ have a bug report that leaking the rps
> > work (for snb+ turbo mode) across either a s/r cycle or a gpu reset kills
> > the driver.
> 
> Yes, but my point is that Chris's patch (at least the commit above
> alone) won't fix the problem we faced.

The patch I thought you two were talking about was:
http://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=fastboot&id=a5f91dcc3cb7a9dc1214d5014ea6f0338824ad8e
which is what I thought we had planned to fix the residual issue.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Suppress hotplug work during PM suspend/resume

[Takashi Iwai]

At Thu, 10 May 2012 10:25:44 +0100,
Chris Wilson wrote:
> 
> On Thu, 10 May 2012 11:19:11 +0200, Takashi Iwai <tiwai@suse.de> wrote:
> > At Thu, 10 May 2012 11:06:46 +0200,
> > Daniel Vetter wrote:
> > > This patch is not just for hotplug, but for all the delayed work and timer
> > > stuff the driver does. And we _do_ have a bug report that leaking the rps
> > > work (for snb+ turbo mode) across either a s/r cycle or a gpu reset kills
> > > the driver.
> > 
> > Yes, but my point is that Chris's patch (at least the commit above
> > alone) won't fix the problem we faced.
> 
> The patch I thought you two were talking about was:
> http://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=fastboot&id=a5f91dcc3cb7a9dc1214d5014ea6f0338824ad8e
> which is what I thought we had planned to fix the residual issue.

Thanks, this explains better :)

But, as far as I checked, the hotplug event wasn't a leftover but
newly triggered during the resume procedure.  Thus there is still a
race even with the patch [drm/i915: Cancel outstanding modeset workers
before suspend].


Takashi

Re: [PATCH] drm/i915: Suppress hotplug work during PM suspend/resume

[Daniel Vetter]

On Thu, May 10, 2012 at 11:39:49AM +0200, Takashi Iwai wrote:
> At Thu, 10 May 2012 10:25:44 +0100,
> Chris Wilson wrote:
> > 
> > On Thu, 10 May 2012 11:19:11 +0200, Takashi Iwai <tiwai@suse.de> wrote:
> > > At Thu, 10 May 2012 11:06:46 +0200,
> > > Daniel Vetter wrote:
> > > > This patch is not just for hotplug, but for all the delayed work and timer
> > > > stuff the driver does. And we _do_ have a bug report that leaking the rps
> > > > work (for snb+ turbo mode) across either a s/r cycle or a gpu reset kills
> > > > the driver.
> > > 
> > > Yes, but my point is that Chris's patch (at least the commit above
> > > alone) won't fix the problem we faced.
> > 
> > The patch I thought you two were talking about was:
> > http://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=fastboot&id=a5f91dcc3cb7a9dc1214d5014ea6f0338824ad8e
> > which is what I thought we had planned to fix the residual issue.
> 
> Thanks, this explains better :)
> 
> But, as far as I checked, the hotplug event wasn't a leftover but
> newly triggered during the resume procedure.  Thus there is still a
> race even with the patch [drm/i915: Cancel outstanding modeset workers
> before suspend].

Sorry for the confusion, I've thought the patch I've quoted might fix
another bug around s/r, in addition to the hotplug race.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] drm/i915: Suppress hotplug work during PM suspend/resume

[Chris Wilson]

On Thu, 10 May 2012 11:39:49 +0200, Takashi Iwai <tiwai@suse.de> wrote:
> At Thu, 10 May 2012 10:25:44 +0100,
> Chris Wilson wrote:
> > 
> > On Thu, 10 May 2012 11:19:11 +0200, Takashi Iwai <tiwai@suse.de> wrote:
> > > At Thu, 10 May 2012 11:06:46 +0200,
> > > Daniel Vetter wrote:
> > > > This patch is not just for hotplug, but for all the delayed work and timer
> > > > stuff the driver does. And we _do_ have a bug report that leaking the rps
> > > > work (for snb+ turbo mode) across either a s/r cycle or a gpu reset kills
> > > > the driver.
> > > 
> > > Yes, but my point is that Chris's patch (at least the commit above
> > > alone) won't fix the problem we faced.
> > 
> > The patch I thought you two were talking about was:
> > http://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=fastboot&id=a5f91dcc3cb7a9dc1214d5014ea6f0338824ad8e
> > which is what I thought we had planned to fix the residual issue.
> 
> Thanks, this explains better :)
> 
> But, as far as I checked, the hotplug event wasn't a leftover but
> newly triggered during the resume procedure.  Thus there is still a
> race even with the patch [drm/i915: Cancel outstanding modeset workers
> before suspend].

Which should be fixed with Adam's patch to only restore IRQs after doing
the modeset upon resume; if I am following all the threads of this bug
correctly.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Suppress hotplug work during PM suspend/resume

[Chris Wilson]

On Thu, 19 Apr 2012 13:55:04 -0400, Adam Jackson <ajax@redhat.com> wrote:
> On Thu, 2012-04-19 at 18:10 +0200, Takashi Iwai wrote:
> 
> > This patch adds a flag to disable the hotplug during PM operation for
> > avoiding such a race.
> > 
> > Cc: <stable@kernel.org>
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> 
> This seems simpler (untested):
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 1a7559b..1cd77a4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -530,11 +530,12 @@ static int i915_drm_thaw(struct drm_device *dev)
>                         ironlake_init_pch_refclk(dev);
>  
>                 drm_mode_config_reset(dev);
> -               drm_irq_install(dev);
>  
>                 /* Resume the modeset for every activated CRTC */
>                 drm_helper_resume_force_mode(dev);
>  
> +               drm_irq_install(dev);
> +
>                 if (IS_IRONLAKE_M(dev))
>                         ironlake_enable_rc6(dev);

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre