2014-04-24 18:55 GMT-03:00 Daniel Vetter <daniel.vetter@xxxxxxxx>:
> Luckily the bit definitions match, but it's still confusing
> to use one when handling the other. So sprinkle some OCD over
> the #defines to make them match and use the right version in
> each place.
>
> Maybe we should unify these definitions completely, but that
> can always be done sometime in the future.
I agree that using SPLL definitions on WRPLL code was very confusing.
But I think I still prefer the longer field names, since they are more
meaningful. If "select" is too big, we could at least use "sel".
With or without that: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 7 ++++---
> drivers/gpu/drm/i915/intel_ddi.c | 12 ++++++------
> 2 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8f845556503e..64d40f22e708 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5505,9 +5505,10 @@ enum punit_power_well {
> #define WRPLL_CTL1 0x46040
> #define WRPLL_CTL2 0x46060
> #define WRPLL_PLL_ENABLE (1<<31)
> -#define WRPLL_PLL_SELECT_SSC (0x01<<28)
> -#define WRPLL_PLL_SELECT_NON_SSC (0x02<<28)
> -#define WRPLL_PLL_SELECT_LCPLL_2700 (0x03<<28)
> +#define WRPLL_PLL_SSC (1<<28)
> +#define WRPLL_PLL_NON_SSC (2<<28)
> +#define WRPLL_PLL_LCPLL (3<<28)
> +#define WRPLL_PLL_REF_MASK (3<<28)
> /* WRPLL divider programming */
> #define WRPLL_DIVIDER_REFERENCE(x) ((x)<<0)
> #define WRPLL_DIVIDER_REF_MASK (0xff)
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 16ec6aee3df7..09ae104d9c2b 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -588,9 +588,9 @@ static int intel_ddi_calc_wrpll_link(struct drm_i915_private *dev_priv,
> u32 wrpll;
>
> wrpll = I915_READ(reg);
> - switch (wrpll & SPLL_PLL_REF_MASK) {
> - case SPLL_PLL_SSC:
> - case SPLL_PLL_NON_SSC:
> + switch (wrpll & WRPLL_PLL_REF_MASK) {
> + case WRPLL_PLL_SSC:
> + case WRPLL_PLL_NON_SSC:
> /*
> * We could calculate spread here, but our checking
> * code only cares about 5% accuracy, and spread is a max of
> @@ -598,7 +598,7 @@ static int intel_ddi_calc_wrpll_link(struct drm_i915_private *dev_priv,
> */
> refclk = 135;
> break;
> - case SPLL_PLL_LCPLL:
> + case WRPLL_PLL_LCPLL:
> refclk = LC_FREQ;
> break;
> default:
> @@ -780,7 +780,7 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
>
> intel_ddi_calculate_wrpll(clock * 1000, &r2, &n2, &p);
>
> - val = WRPLL_PLL_ENABLE | WRPLL_PLL_SELECT_LCPLL_2700 |
> + val = WRPLL_PLL_ENABLE | WRPLL_PLL_LCPLL |
> WRPLL_DIVIDER_REFERENCE(r2) | WRPLL_DIVIDER_FEEDBACK(n2) |
> WRPLL_DIVIDER_POST(p);
>
> @@ -879,7 +879,7 @@ void intel_ddi_pll_enable(struct intel_crtc *crtc)
>
> intel_ddi_calculate_wrpll(clock * 1000, &r2, &n2, &p);
>
> - new_val = WRPLL_PLL_ENABLE | WRPLL_PLL_SELECT_LCPLL_2700 |
> + new_val = WRPLL_PLL_ENABLE | WRPLL_PLL_LCPLL |
> WRPLL_DIVIDER_REFERENCE(r2) |
> WRPLL_DIVIDER_FEEDBACK(n2) | WRPLL_DIVIDER_POST(p);
>
> --
> 1.8.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
