On Fri, Apr 12, 2013 at 10:50:44AM -0700, Jesse Barnes wrote:
> When requesting frequency changes or querying status from the Punit, we
> need to use an opcode that corresponds to the frequency, taking into
> account the memory frequency.
> 
> Signed-off-by: Jesse Barnes <jbar...@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_drv.h |    2 ++
>  drivers/gpu/drm/i915/intel_pm.c |   56 
> +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 58 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e4fa36d..db7d0be 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1873,6 +1873,8 @@ int sandybridge_pcode_read(struct drm_i915_private 
> *dev_priv, u8 mbox, u32 *val)
>  int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 
> val);
>  int valleyview_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 
> *val);
>  int valleyview_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 
> val);
> +int vlv_gpu_freq(int ddr_freq, int val);
> +int vlv_freq_opcode(int ddr_freq, int val);
>  
>  #define __i915_read(x, y) \
>       u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 13a0666..7f313c0 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4579,3 +4579,59 @@ int valleyview_punit_write(struct drm_i915_private 
> *dev_priv, u8 addr, u32 val)
>  {
>       return vlv_punit_rw(dev_priv, PUNIT_OPCODE_REG_WRITE, addr, &val);
>  }
> +
> +int vlv_gpu_freq(int ddr_freq, int val)
> +{
> +     int mult, base;
> +
> +     switch (ddr_freq) {
> +     case 800:
> +             mult = 20;
> +             base = 120;
> +             break;
> +     case 1066:
> +             mult = 22;
> +             base = 133;
> +             break;
> +     case 1333:
> +             mult = 21;
> +             base = 125;
> +             break;
> +     default:
> +             return -1;
> +     }
> +
> +     return ((val - 0xbd) * mult) + base;
> +}

> +
> +int vlv_freq_opcode(int ddr_freq, int val)
> +{
> +     int mult, base;
> +
> +     switch (ddr_freq) {
> +     case 800:
> +             mult = 20;
> +             base = 120;
> +             break;
> +     case 1066:
> +             mult = 22;
> +             base = 133;
> +             break;
> +     case 1333:
> +             mult = 21;
> +             base = 125;
> +             break;
> +     default:
> +             return -1;
> +     }
> +
> +     val /= mult;
> +     val -= base / mult;
> +     val += 0xbd;
> +
> +     if (val > 0xea)
> +             val = 0xea;
> +
> +     return val;
> +}
> +

I can't find this stupid doc, again. But with or without comments below:
Acked-by: Ben Widawsky <b...@bwidawsk.net>

As we just discussed on IRC, I remember a much simpler formula. I assume
you have a reason for not using it.

Without seeing the callers it's hard to tell, but using unsigned values,
and asserting that val > 0xbd would seem to make more sense.

Also, I'd think it's reasonable to WARN_ON_ONCE() or something in the
default case since it means we probably need to support something new.

Finally, I think doing the math in the reverse order of freq() is easier
to read, ie.

val -= base;
val /= mult;
val += 0xbd;


-- 
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to