diff mbox

[2/3] drm/i915: put ring frequency and turbo setup into a work queue

Message ID 1350267038-3599-2-git-send-email-jbarnes@virtuousgeek.org (mailing list archive)
State New, archived
Headers show

Commit Message

Jesse Barnes Oct. 15, 2012, 2:10 a.m. UTC 
Communicating via the mailbox registers with the PCU can take quite
awhile.  And updating the ring frequency or enabling turbo is not
something that needs to happen synchronously, so take it out of our init
and resume paths to speed things up (~200ms on my T420).

Signed-of-by: Jesse Barnes <jbarnes@virtuougseek.org>
---
 drivers/gpu/drm/i915/i915_dma.c |    1 +
 drivers/gpu/drm/i915/i915_drv.h |    3 +++
 drivers/gpu/drm/i915/intel_pm.c |   25 +++++++++++++++++++++++--
 3 files changed, 27 insertions(+), 2 deletions(-)

Comments

Daniel Vetter Oct. 15, 2012, 7:46 a.m. UTC | #1 
On Sun, Oct 14, 2012 at 07:10:37PM -0700, Jesse Barnes wrote:
> Communicating via the mailbox registers with the PCU can take quite
> awhile.  And updating the ring frequency or enabling turbo is not
> something that needs to happen synchronously, so take it out of our init
> and resume paths to speed things up (~200ms on my T420).
> 
> Signed-of-by: Jesse Barnes <jbarnes@virtuougseek.org>

A few minor things:
- Please add a short comment why we schedule_work instead of calling these
  things directly.
- imo calling intel_gt_cleanup in the suspend function can't hurt.
- schedule_delayed_work sounds even better, since as long as the cpu is
  fully busy we don't need to set up anything for power saving, really.
  Also, a short delay of e.g. 1s could paper over a reported regression,
  see:

  https://bugs.freedesktop.org/show_bug.cgi?id=54089

Cheers, Daniel
> ---
>  drivers/gpu/drm/i915/i915_dma.c |    1 +
>  drivers/gpu/drm/i915/i915_drv.h |    3 +++
>  drivers/gpu/drm/i915/intel_pm.c |   25 +++++++++++++++++++++++--
>  3 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index f88bfa6..9780845 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1723,6 +1723,7 @@ int i915_driver_unload(struct drm_device *dev)
>  	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
>  		intel_fbdev_fini(dev);
>  		intel_modeset_cleanup(dev);
> +		intel_gt_cleanup(dev);
>  		cancel_work_sync(&dev_priv->console_resume_work);
>  
>  		/*
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e22b9e3..8418345 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -875,6 +875,8 @@ typedef struct drm_i915_private {
>  		int r_t;
>  	} ips;
>  
> +	struct work_struct gen6_power_work;
> +
>  	enum no_fbc_reason no_fbc_reason;
>  
>  	struct drm_mm_node *compressed_fb;
> @@ -1271,6 +1273,7 @@ void i915_handle_error(struct drm_device *dev, bool wedged);
>  
>  extern void intel_irq_init(struct drm_device *dev);
>  extern void intel_gt_init(struct drm_device *dev);
> +extern void intel_gt_cleanup(struct drm_device *dev);
>  
>  void i915_error_state_free(struct kref *error_ref);
>  
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 07da990..fbf10b8 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3286,15 +3286,28 @@ void intel_disable_gt_powersave(struct drm_device *dev)
>  	}
>  }
>  
> +static void intel_gen6_powersave_work(struct work_struct *work)
> +{
> +	struct drm_i915_private *dev_priv =
> +		container_of(work, struct drm_i915_private, gen6_power_work);
> +	struct drm_device *dev = dev_priv->dev;
> +
> +	mutex_lock(&dev->struct_mutex);
> +	gen6_enable_rps(dev);
> +	gen6_update_ring_freq(dev);
> +	mutex_unlock(&dev->struct_mutex);
> +}
> +
>  void intel_enable_gt_powersave(struct drm_device *dev)
>  {
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
>  	if (IS_IRONLAKE_M(dev)) {
>  		ironlake_enable_drps(dev);
>  		ironlake_enable_rc6(dev);
>  		intel_init_emon(dev);
>  	} else if ((IS_GEN6(dev) || IS_GEN7(dev)) && !IS_VALLEYVIEW(dev)) {
> -		gen6_enable_rps(dev);
> -		gen6_update_ring_freq(dev);
> +		schedule_work(&dev_priv->gen6_power_work);
>  	}
>  }
>  
> @@ -4149,6 +4162,14 @@ void intel_gt_init(struct drm_device *dev)
>  					__gen6_gt_force_wake_mt_put;
>  			}
>  		}
> +		INIT_WORK(&dev_priv->gen6_power_work,
> +			  intel_gen6_powersave_work);
>  	}
>  }
>  
> +void intel_gt_cleanup(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	cancel_work_sync(&dev_priv->gen6_power_work);
> +}
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index f88bfa6..9780845 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1723,6 +1723,7 @@  int i915_driver_unload(struct drm_device *dev)
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
 		intel_fbdev_fini(dev);
 		intel_modeset_cleanup(dev);
+		intel_gt_cleanup(dev);
 		cancel_work_sync(&dev_priv->console_resume_work);
 
 		/*
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e22b9e3..8418345 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -875,6 +875,8 @@  typedef struct drm_i915_private {
 		int r_t;
 	} ips;
 
+	struct work_struct gen6_power_work;
+
 	enum no_fbc_reason no_fbc_reason;
 
 	struct drm_mm_node *compressed_fb;
@@ -1271,6 +1273,7 @@  void i915_handle_error(struct drm_device *dev, bool wedged);
 
 extern void intel_irq_init(struct drm_device *dev);
 extern void intel_gt_init(struct drm_device *dev);
+extern void intel_gt_cleanup(struct drm_device *dev);
 
 void i915_error_state_free(struct kref *error_ref);
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 07da990..fbf10b8 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3286,15 +3286,28 @@  void intel_disable_gt_powersave(struct drm_device *dev)
 	}
 }
 
+static void intel_gen6_powersave_work(struct work_struct *work)
+{
+	struct drm_i915_private *dev_priv =
+		container_of(work, struct drm_i915_private, gen6_power_work);
+	struct drm_device *dev = dev_priv->dev;
+
+	mutex_lock(&dev->struct_mutex);
+	gen6_enable_rps(dev);
+	gen6_update_ring_freq(dev);
+	mutex_unlock(&dev->struct_mutex);
+}
+
 void intel_enable_gt_powersave(struct drm_device *dev)
 {
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
 	if (IS_IRONLAKE_M(dev)) {
 		ironlake_enable_drps(dev);
 		ironlake_enable_rc6(dev);
 		intel_init_emon(dev);
 	} else if ((IS_GEN6(dev) || IS_GEN7(dev)) && !IS_VALLEYVIEW(dev)) {
-		gen6_enable_rps(dev);
-		gen6_update_ring_freq(dev);
+		schedule_work(&dev_priv->gen6_power_work);
 	}
 }
 
@@ -4149,6 +4162,14 @@  void intel_gt_init(struct drm_device *dev)
 					__gen6_gt_force_wake_mt_put;
 			}
 		}
+		INIT_WORK(&dev_priv->gen6_power_work,
+			  intel_gen6_powersave_work);
 	}
 }
 
+void intel_gt_cleanup(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	cancel_work_sync(&dev_priv->gen6_power_work);
+}