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

[Daniel Vetter]

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 at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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