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

[Takashi Iwai <tiwai@suse.de>]

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