Nouveau - Nov 2016 - [PATCH 2/2] drm/nouveau: Queue hpd_work on (runtime) resume

We need to call drm_helper_hpd_irq_event() on resume to properly detect
monitor connection / disconnection on some laptops. For runtime-resume
(which gets called on resume from normal suspend too) we must call
drm_helper_hpd_irq_event() from a workqueue to avoid a deadlock.

Rename acpi_work to hpd_work, and move it out of the #ifdef CONFIG_ACPI
blocks to make it suitable for generic work.

Signed-off-by: Hans de Goede <hdegoede at redhat.com>
---
 drivers/gpu/drm/nouveau/nouveau_display.c | 32 +++++++++++++++----------------
 drivers/gpu/drm/nouveau/nouveau_drv.h     |  2 +-
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c
b/drivers/gpu/drm/nouveau/nouveau_display.c
index a0be029..3cd2b8a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -359,21 +359,10 @@ static struct nouveau_drm_prop_enum_list dither_depth[] =
{
 	}                                                                      \
 } while(0)
 
-#ifdef CONFIG_ACPI
-
-/*
- * Hans de Goede: This define belongs in acpi/video.h, I've submitted a
patch
- * to the acpi subsys to move it there from drivers/acpi/acpi_video.c .
- * This should be dropped once that is merged.
- */
-#ifndef ACPI_VIDEO_NOTIFY_PROBE
-#define ACPI_VIDEO_NOTIFY_PROBE			0x81
-#endif
-
 static void
-nouveau_display_acpi_work(struct work_struct *work)
+nouveau_display_hpd_work(struct work_struct *work)
 {
-	struct nouveau_drm *drm = container_of(work, typeof(*drm), acpi_work);
+	struct nouveau_drm *drm = container_of(work, typeof(*drm), hpd_work);
 
 	pm_runtime_get_sync(drm->dev->dev);
 
@@ -383,6 +372,17 @@ nouveau_display_acpi_work(struct work_struct *work)
 	pm_runtime_put_sync(drm->dev->dev);
 }
 
+#ifdef CONFIG_ACPI
+
+/*
+ * Hans de Goede: This define belongs in acpi/video.h, I've submitted a
patch
+ * to the acpi subsys to move it there from drivers/acpi/acpi_video.c .
+ * This should be dropped once that is merged.
+ */
+#ifndef ACPI_VIDEO_NOTIFY_PROBE
+#define ACPI_VIDEO_NOTIFY_PROBE			0x81
+#endif
+
 static int
 nouveau_display_acpi_ntfy(struct notifier_block *nb, unsigned long val,
 			  void *data)
@@ -395,9 +395,9 @@ nouveau_display_acpi_ntfy(struct notifier_block *nb,
unsigned long val,
 			/*
 			 * This may be the only indication we receive of a
 			 * connector hotplug on a runtime suspended GPU,
-			 * schedule acpi_work to check.
+			 * schedule hpd_work to check.
 			 */
-			schedule_work(&drm->acpi_work);
+			schedule_work(&drm->hpd_work);
 
 			/* acpi-video should not generate keypresses for this */
 			return NOTIFY_BAD;
@@ -587,8 +587,8 @@ nouveau_display_create(struct drm_device *dev)
 	}
 
 	nouveau_backlight_init(dev);
+	INIT_WORK(&drm->hpd_work, nouveau_display_hpd_work);
 #ifdef CONFIG_ACPI
-	INIT_WORK(&drm->acpi_work, nouveau_display_acpi_work);
 	drm->acpi_nb.notifier_call = nouveau_display_acpi_ntfy;
 	register_acpi_notifier(&drm->acpi_nb);
 #endif
diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h
b/drivers/gpu/drm/nouveau/nouveau_drv.h
index 71d4532..0c17ca1 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -163,9 +163,9 @@ struct nouveau_drm {
 	struct nvbios vbios;
 	struct nouveau_display *display;
 	struct backlight_device *backlight;
+	struct work_struct hpd_work;
 #ifdef CONFIG_ACPI
 	struct notifier_block acpi_nb;
-	struct work_struct acpi_work;
 #endif
 
 	/* power management */
-- 
2.9.3

We need to call drm_helper_hpd_irq_event() on resume to properly detect
monitor connection / disconnection on some laptops, use hpd_work for
this to avoid deadlocks.

Signed-off-by: Hans de Goede <hdegoede at redhat.com>
---
 drivers/gpu/drm/nouveau/nouveau_drm.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 3100fd88..b564ab8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -692,7 +692,12 @@ nouveau_pmops_resume(struct device *dev)
 		return ret;
 	pci_set_master(pdev);
 
-	return nouveau_do_resume(drm_dev, false);
+	ret = nouveau_do_resume(drm_dev, false);
+
+	/* Monitors may have been connected / disconnected during suspend */
+	schedule_work(&nouveau_drm(drm_dev)->hpd_work);
+
+	return ret;
 }
 
 static int
@@ -766,6 +771,10 @@ nouveau_pmops_runtime_resume(struct device *dev)
 	nvif_mask(&device->object, 0x088488, (1 << 25), (1 << 25));
 	vga_switcheroo_set_dynamic_switch(pdev, VGA_SWITCHEROO_ON);
 	drm_dev->switch_power_state = DRM_SWITCH_POWER_ON;
+
+	/* Monitors may have been connected / disconnected during suspend */
+	schedule_work(&nouveau_drm(drm_dev)->hpd_work);
+
 	return ret;
 }
 
-- 
2.9.3

On 11/21/2016 05:50 PM, Hans de Goede wrote:
> We need to call drm_helper_hpd_irq_event() on resume to properly detect
> monitor connection / disconnection on some laptops, use hpd_work for
> this to avoid deadlocks.
>
Hi,

this seems to introduce a hang of nouveau in 4.10-rc if the gpu is 
runtime resumed while no displays are connected at all.

I get a permanent hang - need to power cycle to recover - if i either

a) Boot a MacPro test machine which has two discrete cards, one radeon, 
one geforce, but the displays are only connected to the radeon, whereas 
the nvidia gpu has no displays connected during boot.

b) On a gmux'ed MacBookPro 2010 intel + nvidia if i switch to the intel 
card via vgaswitcheroo (echo IGD > ...vgaswitcheroo/switch) and then 
after nouveau has powered down the nvidia, i use echo ON > 
...vgaswitcheroo/switch) to power up the nvidia again, but now with 
nothing connected to its outputs.

I can prevent the hang if i either boot with nouveau.runpm=0, or connect 
displays to the nvidia in case a), or if i remove the new 
schedule_work() for hpd_work in the nouveau_pmops_runtime_resume() function.

Otherwise i get a hanging gpu-manager process on Ubuntu and this 
repeating in my kernel log:

[  246.899424] INFO: task kworker/7:1:127 blocked for more than 120 seconds.
[  246.899476]       Tainted: G          I     4.9.0-rc8 #60
[  246.899511] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" 
disables this message.
[  246.899561] kworker/7:1     D    0   127      2 0x00000000
[  246.899573] Workqueue: pm pm_runtime_work
[  246.899576]  ffff917bcbeb2680 0000000000000000 ffff917c0e8d0000 
ffff917c0d890000
[  246.899582]  ffff917c165da058 ffff9c86866c7b70 ffffffff8c920fab 
ffff917c0de3d360
[  246.899587]  0000000000000086 000000000de3d360 ffff917c165da058 
0000000000000000
[  246.899593] Call Trace:
[  246.899601]  [<ffffffff8c920fab>] ? __schedule+0x2fb/0xb30
[  246.899605]  [<ffffffff8c921820>] schedule+0x40/0x90
[  246.899608]  [<ffffffff8c5ff8da>] rpm_resume+0x14a/0x740
[  246.899614]  [<ffffffff8c0e1c10>] ? wake_atomic_t_function+0x60/0x60
[  246.899617]  [<ffffffff8c6012e3>] pm_runtime_forbid+0x43/0x50
[  246.899678]  [<ffffffffc08f67c5>] 
nouveau_pmops_runtime_suspend+0xc5/0xd0 [nouveau]
[  246.899684]  [<ffffffff8c4ced8d>] pci_pm_runtime_suspend+0x5d/0x190
[  246.899687]  [<ffffffff8c4ced30>] ? pci_pm_runtime_resume+0xa0/0xa0
[  246.899690]  [<ffffffff8c5ff252>] __rpm_callback+0x32/0x70
[  246.899693]  [<ffffffff8c5ff2b4>] rpm_callback+0x24/0x80
[  246.899695]  [<ffffffff8c4ced30>] ? pci_pm_runtime_resume+0xa0/0xa0
[  246.899698]  [<ffffffff8c5fffee>] rpm_suspend+0x11e/0x6f0
[  246.899701]  [<ffffffff8c60149b>] pm_runtime_work+0x7b/0xc0
[  246.899707]  [<ffffffff8c0afe58>] process_one_work+0x1f8/0x750
[  246.899710]  [<ffffffff8c0afdd9>] ? process_one_work+0x179/0x750
[  246.899713]  [<ffffffff8c0b03fb>] worker_thread+0x4b/0x4f0
[  246.899717]  [<ffffffff8c0bf8fc>] ? preempt_count_sub+0x4c/0x80
[  246.899720]  [<ffffffff8c0b03b0>] ? process_one_work+0x750/0x750
[  246.899723]  [<ffffffff8c0b7212>] kthread+0x102/0x120
[  246.899728]  [<ffffffff8c0ef546>] ? trace_hardirqs_on_caller+0x16/0x1c0
[  246.899732]  [<ffffffff8c0b7110>] ? kthread_park+0x60/0x60
[  246.899735]  [<ffffffff8c929b2a>] ret_from_fork+0x2a/0x40
[  246.899738] INFO: lockdep is turned off.
[  246.899751] INFO: task gpu-manager:1173 blocked for more than 120 
seconds.
[  246.899796]       Tainted: G          I     4.9.0-rc8 #60
[  246.899832] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" 
disables this message.
[  246.899880] gpu-manager     D    0  1173      1 0x00000000
[  246.899884]  ffff917bcbd38580 0000000000000000 ffff917c0e8d5180 
ffff917bcbced180
[  246.899889]  ffff917c169da058 ffff9c8688573ce0 ffffffff8c920fab 
ffff917c0de3d360
[  246.899894]  0000000000000086 000000000de3d360 ffff917c169da058 
0000000000000000
[  246.899899] Call Trace:
[  246.899903]  [<ffffffff8c920fab>] ? __schedule+0x2fb/0xb30
[  246.899906]  [<ffffffff8c921820>] schedule+0x40/0x90
[  246.899909]  [<ffffffff8c5ff595>] __pm_runtime_barrier+0x95/0x140
[  246.899913]  [<ffffffff8c0e1c10>] ? wake_atomic_t_function+0x60/0x60
[  246.899915]  [<ffffffff8c60109b>] pm_runtime_barrier+0x5b/0xc0
[  246.899919]  [<ffffffff8c4cce6b>] pci_config_pm_runtime_get+0x3b/0x60
[  246.899922]  [<ffffffff8c4d1bac>] pci_read_config+0x7c/0x260
[  246.899927]  [<ffffffff8c30fe60>] sysfs_kf_bin_read+0x50/0x80
[  246.899929]  [<ffffffff8c30f91a>] kernfs_fop_read+0xba/0x1c0
[  246.899934]  [<ffffffff8c277d98>] __vfs_read+0x28/0x130
[  246.899939]  [<ffffffff8c3cb7be>] ? security_file_permission+0x9e/0xc0
[  246.899942]  [<ffffffff8c278dee>] ? rw_verify_area+0x4e/0xc0
[  246.899945]  [<ffffffff8c278ef6>] vfs_read+0x96/0x140
[  246.899948]  [<ffffffff8c27a4ba>] SyS_pread64+0x7a/0x90
[  246.899952]  [<ffffffff8c929885>] entry_SYSCALL_64_fastpath+0x23/0xc6
[  246.899955]  [<ffffffff8c0eb61f>] ? trace_hardirqs_off_caller+0x1f/0xc0
[  246.899958] INFO: lockdep is turned off.
[  369.775936] INFO: task kworker/7:1:127 blocked for more than 120 seconds.
[  369.775989]       Tainted: G          I     4.9.0-rc8 #60
[  369.776024] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" 
disables this message.
[  369.776074] kworker/7:1     D    0   127      2 0x00000000
[  369.776086] Workqueue: pm pm_runtime_work
[  369.776090]  ffff917bcbeb2680 0000000000000000 ffff917c0e8d0000 
ffff917c0d890000
[  369.776096]  ffff917c165da058 ffff9c86866c7b70 ffffffff8c920fab 
ffff917c0de3d360
[  369.776101]  0000000000000086 000000000de3d360 ffff917c165da058 
0000000000000000
[  369.776106] Call Trace:
[  369.776114]  [<ffffffff8c920fab>] ? __schedule+0x2fb/0xb30
[  369.776118]  [<ffffffff8c921820>] schedule+0x40/0x90
[  369.776121]  [<ffffffff8c5ff8da>] rpm_resume+0x14a/0x740
[  369.776127]  [<ffffffff8c0e1c10>] ? wake_atomic_t_function+0x60/0x60
[  369.776130]  [<ffffffff8c6012e3>] pm_runtime_forbid+0x43/0x50
[  369.776192]  [<ffffffffc08f67c5>] 
nouveau_pmops_runtime_suspend+0xc5/0xd0 [nouveau]
[  369.776199]  [<ffffffff8c4ced8d>] pci_pm_runtime_suspend+0x5d/0x190
[  369.776202]  [<ffffffff8c4ced30>] ? pci_pm_runtime_resume+0xa0/0xa0
[  369.776205]  [<ffffffff8c5ff252>] __rpm_callback+0x32/0x70
[  369.776207]  [<ffffffff8c5ff2b4>] rpm_callback+0x24/0x80
[  369.776210]  [<ffffffff8c4ced30>] ? pci_pm_runtime_resume+0xa0/0xa0
[  369.776212]  [<ffffffff8c5fffee>] rpm_suspend+0x11e/0x6f0
[  369.776216]  [<ffffffff8c60149b>] pm_runtime_work+0x7b/0xc0
[  369.776221]  [<ffffffff8c0afe58>] process_one_work+0x1f8/0x750
[  369.776224]  [<ffffffff8c0afdd9>] ? process_one_work+0x179/0x750
[  369.776228]  [<ffffffff8c0b03fb>] worker_thread+0x4b/0x4f0
[  369.776231]  [<ffffffff8c0bf8fc>] ? preempt_count_sub+0x4c/0x80
[  369.776234]  [<ffffffff8c0b03b0>] ? process_one_work+0x750/0x750
[  369.776237]  [<ffffffff8c0b7212>] kthread+0x102/0x120
[  369.776242]  [<ffffffff8c0ef546>] ? trace_hardirqs_on_caller+0x16/0x1c0
[  369.776246]  [<ffffffff8c0b7110>] ? kthread_park+0x60/0x60
[  369.776249]  [<ffffffff8c929b2a>] ret_from_fork+0x2a/0x40
[  369.776252] INFO: lockdep is turned off.

thanks,
-mario

> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>  drivers/gpu/drm/nouveau/nouveau_drm.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 3100fd88..b564ab8 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -692,7 +692,12 @@ nouveau_pmops_resume(struct device *dev)
>  		return ret;
>  	pci_set_master(pdev);
>
> -	return nouveau_do_resume(drm_dev, false);
> +	ret = nouveau_do_resume(drm_dev, false);
> +
> +	/* Monitors may have been connected / disconnected during suspend */
> +	schedule_work(&nouveau_drm(drm_dev)->hpd_work);
> +
> +	return ret;
>  }
>
>  static int
> @@ -766,6 +771,10 @@ nouveau_pmops_runtime_resume(struct device *dev)
>  	nvif_mask(&device->object, 0x088488, (1 << 25), (1 <<
25));
>  	vga_switcheroo_set_dynamic_switch(pdev, VGA_SWITCHEROO_ON);
>  	drm_dev->switch_power_state = DRM_SWITCH_POWER_ON;
> +
> +	/* Monitors may have been connected / disconnected during suspend */
> +	schedule_work(&nouveau_drm(drm_dev)->hpd_work);
> +
>  	return ret;
>  }
>
>