drm/i915: enable the power well before module unload

PATCH Applied Paulo Zanoni 3 participants 3 messages Aug 14, 2013 – Aug 15, 2013

Subsystems (2)

DRM drivers Intel i915

lore

Share & tools

lore

Paulo Zanoni

author

2013-08-14 17:40 UTC #0 lore raw

PATCH 1/1 drm/i915: enable the power well before module unload

From: Paulo Zanoni <[email protected]>

Our driver initialization doesn't seem to be ready to load when the power well is disabled: we hit a few "Unclaimed register" messages. So do just like we already do for the suspend/resume path: enable the power well before unloading.

At some point we'll want to be able to survive suspend/resume and load/unload with the power well disabled, but for now let's just fix the regression.

Regression introduced by the following commit:

commit bf51d5e2cda5d36d98e4b46ac7fca9461e512c41

Author: Paulo Zanoni <[email protected]>
Date:   Wed Jul 3 17:12:13 2013 -0300

drm/i915: switch disable_power_well default value to 1

Bug can be reproduced by running the "module_reload" script from intel-gpu-tools.

Cc: [email protected]
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67813

Signed-off-by Paulo Zanoni <[email protected]>

---
 drivers/gpu/drm/i915/i915_dma.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

QA (the bug reporter) still didn't confirm this patch fixes the bug, but I can reproduce it and I can confirm it fixes the problem at least on my machine.

Also please notice we have other problems that happen when we run module_reload (e.g., gpu hang, no backlight, lockdep error message, eventual oops), this patch fixes only the "Unclaimed register" errors.

drivers/gpu/drm/i915/i915_dma.c +6 -1

index b30404d..d2dc02b 100644
@@ -1688,8 +1688,13 @@ int i915_driver_unload(struct drm_device *dev)
 
 	intel_gpu_ips_teardown();
 
-	if (HAS_POWER_WELL(dev))
+	if (HAS_POWER_WELL(dev)) {
+		/* The i915.ko module is still not prepared to be loaded when
+		 * the power well is not enabled, so just enable it in case
+		 * we're going to unload/reload. */
+		intel_set_power_well(dev, true);
 		i915_remove_power_well(dev);
+	}
 
 	i915_teardown_sysfs(dev);

Signature · 1 line

1.8.1.2

Damien Lespiau

Reviewed-by on v1 1/1

2013-08-15 11:07 UTC #1 lore raw

Quoted 16 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>
Our driver initialization doesn't seem to be ready to load when the power well is disabled: we hit a few "Unclaimed register" messages. So do just like we already do for the suspend/resume path: enable the power well before unloading.
At some point we'll want to be able to survive suspend/resume and load/unload with the power well disabled, but for now let's just fix the regression.
Regression introduced by the following commit:
commit bf51d5e2cda5d36d98e4b46ac7fca9461e512c41
Author: Paulo Zanoni <[email protected]>
Date:   Wed Jul 3 17:12:13 2013 -0300

Quoted 8 lines

drm/i915: switch disable_power_well default value to 1
Bug can be reproduced by running the "module_reload" script from intel-gpu-tools.
Cc: [email protected]
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67813
Signed-off-by Paulo Zanoni <[email protected]>

Reviewed-by Damien Lespiau <[email protected]>

Signature · 1 line

Damien

Quoted 38 lines

---
 drivers/gpu/drm/i915/i915_dma.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
QA (the bug reporter) still didn't confirm this patch fixes the bug, but I can reproduce it and I can confirm it fixes the problem at least on my machine.
Also please notice we have other problems that happen when we run module_reload (e.g., gpu hang, no backlight, lockdep error message, eventual oops), this patch fixes only the "Unclaimed register" errors.
drivers/gpu/drm/i915/i915_dma.c +6 -1
index b30404d..d2dc02b 100644
@@ -1688,8 +1688,13 @@ int i915_driver_unload(struct drm_device *dev)
 
 	intel_gpu_ips_teardown();
 
-	if (HAS_POWER_WELL(dev))
+	if (HAS_POWER_WELL(dev)) {
+		/* The i915.ko module is still not prepared to be loaded when
+		 * the power well is not enabled, so just enable it in case
+		 * we're going to unload/reload. */
+		intel_set_power_well(dev, true);
 		i915_remove_power_well(dev);
+	}
 
 	i915_teardown_sysfs(dev);
 
Signature · 6 lines
1.8.1.2

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Daniel Vetter

Applied

2013-08-15 11:32 UTC #2 lore raw

Quoted 19 lines · Damien Lespiau

Quoted 18 lines · Paulo Zanoni
From: Paulo Zanoni <[email protected]>
Our driver initialization doesn't seem to be ready to load when the power well is disabled: we hit a few "Unclaimed register" messages. So do just like we already do for the suspend/resume path: enable the power well before unloading.
At some point we'll want to be able to survive suspend/resume and load/unload with the power well disabled, but for now let's just fix the regression.
Regression introduced by the following commit:
commit bf51d5e2cda5d36d98e4b46ac7fca9461e512c41
Author: Paulo Zanoni <[email protected]>
Date:   Wed Jul 3 17:12:13 2013 -0300
drm/i915: switch disable_power_well default value to 1

Quoted 8 lines

Quoted 6 lines
Bug can be reproduced by running the "module_reload" script from intel-gpu-tools.
Cc: [email protected]
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67813
Signed-off-by Paulo Zanoni <[email protected]>
Reviewed-by Damien Lespiau <[email protected]>

Queued for -next (with the cc: stable dropped since module unload is a developer feature), thanks for the patch.
-Daniel

Signature · 3 lines

Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch