Re: [PATCH 12/19] drm/i915: sanitize PUNIT register macro definitions

Jesse Barnes
2014-02-20T19:46:02Z

On Tue, 18 Feb 2014 00:02:13 +0200
Imre Deak <imre.deak@intel.com> wrote:

> In the upcoming patches we'll need to access the rest of the fields in
> the punit power gating register, so prepare for that.
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 29 +++++++++++++++++++++++------
> drivers/gpu/drm/i915/intel_uncore.c | 4 +++-
> 2 files changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2f564ce..5a700e9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -377,14 +377,31 @@
> #define DSPFREQSTAT_MASK (0x3 << DSPFREQSTAT_SHIFT)
> #define DSPFREQGUAR_SHIFT 14
> #define DSPFREQGUAR_MASK (0x3 << DSPFREQGUAR_SHIFT)
> +
> +enum punit_power_well {
> + PUNIT_POWER_WELL_RENDER = 0,
> + PUNIT_POWER_WELL_MEDIA = 1,
> + PUNIT_POWER_WELL_DISP2D = 3,
> + PUNIT_POWER_WELL_DPIO_CMN_BC = 5,
> + PUNIT_POWER_WELL_DPIO_TX_B_LANES_01 = 6,
> + PUNIT_POWER_WELL_DPIO_TX_B_LANES_23 = 7,
> + PUNIT_POWER_WELL_DPIO_TX_C_LANES_01 = 8,
> + PUNIT_POWER_WELL_DPIO_TX_C_LANES_23 = 9,
> + PUNIT_POWER_WELL_DPIO_RX0 = 10,
> + PUNIT_POWER_WELL_DPIO_RX1 = 11,
> + PUNIT_POWER_WELL_DPIO_RX2 = 12,
> + PUNIT_POWER_WELL_DPIO_RX3 = 13,
> +
> + PUNIT_POWER_WELL_NUM,
> +};
> +
> #define PUNIT_REG_PWRGT_CTRL 0x60
> #define PUNIT_REG_PWRGT_STATUS 0x61
> -#define PUNIT_CLK_GATE 1
> -#define PUNIT_PWR_RESET 2
> -#define PUNIT_PWR_GATE 3
> -#define RENDER_PWRGT (PUNIT_PWR_GATE << 0)
> -#define MEDIA_PWRGT (PUNIT_PWR_GATE << 2)
> -#define DISP2D_PWRGT (PUNIT_PWR_GATE << 6)
> +#define PUNIT_PWRGT_MASK(power_well) (3 << ((power_well) * 2))
> +#define PUNIT_PWRGT_PWR_ON(power_well) (0 << ((power_well) * 2))
> +#define PUNIT_PWRGT_CLK_GATE(power_well) (1 << ((power_well) * 2))
> +#define PUNIT_PWRGT_RESET(power_well) (2 << ((power_well) * 2))
> +#define PUNIT_PWRGT_PWR_GATE(power_well) (3 << ((power_well) * 2))
>
> #define PUNIT_REG_GPU_LFM 0xd3
> #define PUNIT_REG_GPU_FREQ_REQ 0xd4
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index c628414..4aab7c2 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -354,7 +354,9 @@ void intel_uncore_sanitize(struct drm_device *dev)
> mutex_lock(&dev_priv->rps.hw_lock);
> reg_val = vlv_punit_read(dev_priv, PUNIT_REG_PWRGT_STATUS);
>
> - if (reg_val & (RENDER_PWRGT | MEDIA_PWRGT | DISP2D_PWRGT))
> + if (reg_val & (PUNIT_PWRGT_PWR_GATE(PUNIT_POWER_WELL_RENDER) |
> + PUNIT_PWRGT_PWR_GATE(PUNIT_POWER_WELL_MEDIA) |
> + PUNIT_PWRGT_PWR_GATE(PUNIT_POWER_WELL_DISP2D)))
> vlv_punit_write(dev_priv, PUNIT_REG_PWRGT_CTRL, 0x0);
>
> mutex_unlock(&dev_priv->rps.hw_lock);

I'd like to see a doc reference here, as I never remember which one has
these bits... Also, are you sure about the RX bits? The
PUNIT_HAS_0.8 doc says only subsystems 10-11 cover RX, maybe in a ganged
config?

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

--
Jesse Barnes, Intel Open Source Technology Center