[Intel-gfx] [PATCH 1/2] drm/i915: VLV GPU frequency to opcode functions

[Ben Widawsky]

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