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 <jbarnes at 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 <ben at 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
