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

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

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

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 <paulo.r.zanoni@intel.com>
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: stable@vger.kernel.org
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67813
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 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.

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index b30404d..d2dc02b 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -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);
 
-- 
1.8.1.2

Re: [Intel-gfx] [PATCH] drm/i915: enable the power well before module unload

[Damien Lespiau]

On Wed, Aug 14, 2013 at 02:40:37PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> 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 <paulo.r.zanoni@intel.com>
> 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: stable@vger.kernel.org
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67813
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  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.
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index b30404d..d2dc02b 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -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);
>  
> -- 
> 1.8.1.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: enable the power well before module unload

[Daniel Vetter]

On Thu, Aug 15, 2013 at 12:07:44PM +0100, Damien Lespiau wrote:
> On Wed, Aug 14, 2013 at 02:40:37PM -0300, Paulo Zanoni wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > 
> > 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 <paulo.r.zanoni@intel.com>
> > 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: stable@vger.kernel.org
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67813
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

Queued for -next (with the cc: stable dropped since module unload is a
developer feature), thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch