Re: [PATCH] drm/i915: Only enable IPS polling for gen5

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Mon, 30 Apr 2012 19:35:02 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> On SandyBridge IPS was entirely implemented in hardware and not reliant
> on the driver monitoring power consumption and feeding back desired run
> states, so the hardware is able to adapt quicker and more flexibly. Which
> is a huge relief for us as we no longer have to carry empirically
> derived magic algorithms.
> 
> Yet despite the advance in technology, the driver was still doing its
> IPS polling on all machines. Restrict it to the only supported hardware,
> Clarkdale/Arrandale.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |    3 +++
>  drivers/gpu/drm/i915/i915_dma.c     |   15 ++++++++++-----
>  2 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 1ff6ec7..b9bd9e8 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1212,6 +1212,9 @@ static int i915_emon_status(struct seq_file *m, void *unused)
>  	unsigned long temp, chipset, gfx;
>  	int ret;
>  
> +	if (!IS_GEN5(dev))
> +		return -ENODEV;
> +
>  	ret = mutex_lock_interruptible(&dev->struct_mutex);
>  	if (ret)
>  		return ret;
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 4c01a47..3d03f3a 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1527,6 +1527,9 @@ void i915_update_gfx_val(struct drm_i915_private *dev_priv)
>  	unsigned long diffms;
>  	u32 count;
>  
> +	if (dev_priv->info->gen != 5)
> +		return;
> +
>  	getrawmonotonic(&now);
>  	diff1 = timespec_sub(now, dev_priv->last_time2);

Looks ok, but I think just update_gfx_val is called in the idle
routine?  The other isn't I think...

>  
> @@ -1966,12 +1969,14 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  	setup_timer(&dev_priv->hangcheck_timer, i915_hangcheck_elapsed,
>  		    (unsigned long) dev);
>  
> -	spin_lock(&mchdev_lock);
> -	i915_mch_dev = dev_priv;
> -	dev_priv->mchdev_lock = &mchdev_lock;
> -	spin_unlock(&mchdev_lock);
> +	if (IS_GEN5(dev)) {
> +		spin_lock(&mchdev_lock);
> +		i915_mch_dev = dev_priv;
> +		dev_priv->mchdev_lock = &mchdev_lock;
> +		spin_unlock(&mchdev_lock);
>  
> -	ips_ping_for_i915_load();
> +		ips_ping_for_i915_load();
> +	}
>  
>  	return 0;

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>


-- 
Jesse Barnes, Intel Open Source Technology Center