[PATCH 1/2] drm/i915: use runtime irq suspend/resume in freeze/thaw

[PATCH 1/2] drm/i915: use runtime irq suspend/resume in freeze/thaw

[Jesse Barnes]

We don't need to uninstall the full handler, simply disabling interrupts
ought to be enough.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.c | 5 ++---
 drivers/gpu/drm/i915/intel_pm.c | 4 ++--
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 7ae4e2a..be97741 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -522,7 +522,7 @@ static int i915_drm_freeze(struct drm_device *dev)
 			return error;
 		}
 
-		drm_irq_uninstall(dev);
+		intel_runtime_pm_disable_interrupts(dev);
 		dev_priv->enable_hotplug_processing = false;
 
 		intel_suspend_gt_powersave(dev);
@@ -646,8 +646,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
 		}
 		mutex_unlock(&dev->struct_mutex);
 
-		/* We need working interrupts for modeset enabling ... */
-		drm_irq_install(dev, dev->pdev->irq);
+		intel_runtime_pm_restore_interrupts(dev);
 
 		intel_modeset_init_hw(dev);
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2043c4b..0bf9b0c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4879,7 +4879,7 @@ void intel_suspend_gt_powersave(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	/* Interrupts should be disabled already to avoid re-arming. */
-	WARN_ON(dev->irq_enabled);
+	WARN_ON(dev->irq_enabled && !dev_priv->pm.irqs_disabled);
 
 	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
 
@@ -4891,7 +4891,7 @@ void intel_disable_gt_powersave(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	/* Interrupts should be disabled already to avoid re-arming. */
-	WARN_ON(dev->irq_enabled);
+	WARN_ON(dev->irq_enabled && !dev_priv->pm.irqs_disabled);
 
 	if (IS_IRONLAKE_M(dev)) {
 		ironlake_disable_drps(dev);
-- 
1.8.3.2

[PATCH 2/2] drm/i915: don't take runtime PM reference around freeze/thaw

[Jesse Barnes]

We should be taking the right power well refs these days, so this
shouldn't be necessary.  It also gets in the way of re-using these
routines for S0iX states, as those need all the power saving features
enabled.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index be97741..6eb45ac 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -496,8 +496,6 @@ static int i915_drm_freeze(struct drm_device *dev)
 	struct drm_crtc *crtc;
 	pci_power_t opregion_target_state;
 
-	intel_runtime_pm_get(dev_priv);
-
 	/* ignore lid events during suspend */
 	mutex_lock(&dev_priv->modeset_restore_lock);
 	dev_priv->modeset_restore = MODESET_SUSPENDED;
@@ -686,7 +684,6 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
 
 	intel_opregion_notify_adapter(dev, PCI_D0);
 
-	intel_runtime_pm_put(dev_priv);
 	return 0;
 }
 
-- 
1.8.3.2

Re: [PATCH 1/2] drm/i915: use runtime irq suspend/resume in freeze/thaw

[Imre Deak]

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

On Wed, 2014-06-18 at 09:52 -0700, Jesse Barnes wrote:
> We don't need to uninstall the full handler, simply disabling interrupts
> ought to be enough.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Reviewed-by: Imre Deak <imre.deak@intel.com>

For follow-up:
I agree with Daniel that we should add a new helper and use it instead
of the current dev->irq_enabled checks elsewhere.

Also we should add this check to the irq handlers themselves.

--Imre

> ---
>  drivers/gpu/drm/i915/i915_drv.c | 5 ++---
>  drivers/gpu/drm/i915/intel_pm.c | 4 ++--
>  2 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 7ae4e2a..be97741 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -522,7 +522,7 @@ static int i915_drm_freeze(struct drm_device *dev)
>  			return error;
>  		}
>  
> -		drm_irq_uninstall(dev);
> +		intel_runtime_pm_disable_interrupts(dev);
>  		dev_priv->enable_hotplug_processing = false;
>  
>  		intel_suspend_gt_powersave(dev);
> @@ -646,8 +646,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
>  		}
>  		mutex_unlock(&dev->struct_mutex);
>  
> -		/* We need working interrupts for modeset enabling ... */
> -		drm_irq_install(dev, dev->pdev->irq);
> +		intel_runtime_pm_restore_interrupts(dev);
>  
>  		intel_modeset_init_hw(dev);
>  
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 2043c4b..0bf9b0c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4879,7 +4879,7 @@ void intel_suspend_gt_powersave(struct drm_device *dev)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
>  	/* Interrupts should be disabled already to avoid re-arming. */
> -	WARN_ON(dev->irq_enabled);
> +	WARN_ON(dev->irq_enabled && !dev_priv->pm.irqs_disabled);
>  
>  	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
>  
> @@ -4891,7 +4891,7 @@ void intel_disable_gt_powersave(struct drm_device *dev)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
>  	/* Interrupts should be disabled already to avoid re-arming. */
> -	WARN_ON(dev->irq_enabled);
> +	WARN_ON(dev->irq_enabled && !dev_priv->pm.irqs_disabled);
>  
>  	if (IS_IRONLAKE_M(dev)) {
>  		ironlake_disable_drps(dev);


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 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 2/2] drm/i915: don't take runtime PM reference around freeze/thaw

[Imre Deak]

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

On Wed, 2014-06-18 at 09:52 -0700, Jesse Barnes wrote:
> We should be taking the right power well refs these days, so this
> shouldn't be necessary.  It also gets in the way of re-using these
> routines for S0iX states, as those need all the power saving features
> enabled.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Reviewed-by: Imre Deak <imre.deak@intel.com>

Btw, we also have the same redundancy in intel_dp_detect().

--Imre

> ---
>  drivers/gpu/drm/i915/i915_drv.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index be97741..6eb45ac 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -496,8 +496,6 @@ static int i915_drm_freeze(struct drm_device *dev)
>  	struct drm_crtc *crtc;
>  	pci_power_t opregion_target_state;
>  
> -	intel_runtime_pm_get(dev_priv);
> -
>  	/* ignore lid events during suspend */
>  	mutex_lock(&dev_priv->modeset_restore_lock);
>  	dev_priv->modeset_restore = MODESET_SUSPENDED;
> @@ -686,7 +684,6 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
>  
>  	intel_opregion_notify_adapter(dev, PCI_D0);
>  
> -	intel_runtime_pm_put(dev_priv);
>  	return 0;
>  }
>  


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 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 2/2] drm/i915: don't take runtime PM reference around freeze/thaw

[Daniel Vetter]

On Thu, Jun 19, 2014 at 04:25:55PM +0300, Imre Deak wrote:
> On Wed, 2014-06-18 at 09:52 -0700, Jesse Barnes wrote:
> > We should be taking the right power well refs these days, so this
> > shouldn't be necessary.  It also gets in the way of re-using these
> > routines for S0iX states, as those need all the power saving features
> > enabled.
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> Reviewed-by: Imre Deak <imre.deak@intel.com>
> 
> Btw, we also have the same redundancy in intel_dp_detect().

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