[Intel-gfx] [PATCH v2 3/4] drm/i915/chv: Set min freq to efficient frequency on chv

[Paulo Zanoni]

2015-03-19 11:14 GMT-03:00  <deepak.s at linux.intel.com>:
> From: Deepak S <deepak.s at linux.intel.com>
>
> After feedback from the hardware team, now we set the GPU min/idel freq to RPe.
> Punit is expecting us to operate GPU between Rpe & Rp0. If we drop the
> frequency to RPn, punit is failing to change the input voltage to
> minimum :(

Since this is far away from the obvious, I am imagining some
programmer from the future looking at this code and imagining
min_freq_softlimit was "accidentally" set to RPe instead of RPn. Can't
we add a comment in the code - not just the commit message -, to make
it clear that we're doing this since the punit is weird?

Another thing which I noticed is that your patch title mentions CHV,
but your patch touches the VLV function instead of the CHV one. This
also leads me to think that maybe the power measurement experiments
you did were done using the non-patched CHV code... Can you please
clarify your intentions here? And also maybe redo the power
measurements if needed.

Also, I think we need at least an ACK from Chris here, especially
since he was already discussing the previous version of this patch.

Thanks,
Paulo

>
> v2: Change commit message
>
> Signed-off-by: Deepak S <deepak.s at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 6d04147..b9b4d16 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4859,7 +4859,7 @@ static void valleyview_init_gt_powersave(struct drm_device *dev)
>                 dev_priv->rps.max_freq_softlimit = dev_priv->rps.max_freq;
>
>         if (dev_priv->rps.min_freq_softlimit == 0)
> -               dev_priv->rps.min_freq_softlimit = dev_priv->rps.min_freq;
> +               dev_priv->rps.min_freq_softlimit = dev_priv->rps.efficient_freq;
>
>         mutex_unlock(&dev_priv->rps.hw_lock);
>  }
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni