On Fri, 18 Sep 2015, [email protected] wrote: > From: Ville Syrjälä <[email protected]> > > Signed-off-by: Ville Syrjälä <[email protected]> > --- > drivers/gpu/drm/i915/i915_reg.h | 2 +- > drivers/gpu/drm/i915/intel_ddi.c | 8 ++++---- > drivers/gpu/drm/i915/intel_display.c | 4 ++-- > 3 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 25864ae..a8fb5f7 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7253,7 +7253,7 @@ enum skl_disp_power_wells { > /* WRPLL */ > #define WRPLL_CTL1 0x46040 > #define WRPLL_CTL2 0x46060 > -#define WRPLL_CTL(pll) (pll == 0 ? WRPLL_CTL1 : > WRPLL_CTL2) > +#define WRPLL_CTL(pll) _PIPE(pll, WRPLL_CTL1, > WRPLL_CTL2)
I hope we never accidentally pass pll >= 2 here... I guess we'll find out. ;) I know we do reuse _PIPE like this, but maybe we should have a different macro that emphasizes the parameter is really not pipe based, but rather an ID or enumeration. There's value in self-documentating code, and *gasp* maybe we'll eventually make this stuff type safe too! Basically it would be an alias for _PIPE(); see _PLANE(). Or maybe _PIPE and _PLANE could be based on the new one. Anyway, just a thought, this is Reviewed-by: Jani Nikula <[email protected]> > #define WRPLL_PLL_ENABLE (1<<31) > #define WRPLL_PLL_SSC (1<<28) > #define WRPLL_PLL_NON_SSC (2<<28) > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index 399e70e..fb456a4 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1094,10 +1094,10 @@ static void hsw_ddi_clock_get(struct intel_encoder > *encoder, > link_clock = 270000; > break; > case PORT_CLK_SEL_WRPLL1: > - link_clock = hsw_ddi_calc_wrpll_link(dev_priv, WRPLL_CTL1); > + link_clock = hsw_ddi_calc_wrpll_link(dev_priv, WRPLL_CTL(0)); > break; > case PORT_CLK_SEL_WRPLL2: > - link_clock = hsw_ddi_calc_wrpll_link(dev_priv, WRPLL_CTL2); > + link_clock = hsw_ddi_calc_wrpll_link(dev_priv, WRPLL_CTL(1)); > break; > case PORT_CLK_SEL_SPLL: > pll = I915_READ(SPLL_CTL) & SPLL_PLL_FREQ_MASK; > @@ -2485,13 +2485,13 @@ static const struct skl_dpll_regs skl_dpll_regs[3] = { > }, > { > /* DPLL 2 */ > - .ctl = WRPLL_CTL1, > + .ctl = WRPLL_CTL(0), > .cfgcr1 = DPLL_CFGCR1(2), > .cfgcr2 = DPLL_CFGCR2(2), > }, > { > /* DPLL 3 */ > - .ctl = WRPLL_CTL2, > + .ctl = WRPLL_CTL(1), > .cfgcr1 = DPLL_CFGCR1(3), > .cfgcr2 = DPLL_CFGCR2(3), > }, > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 63cf5eb..fa7c10fa 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9237,8 +9237,8 @@ static void assert_can_disable_lcpll(struct > drm_i915_private *dev_priv) > > I915_STATE_WARN(I915_READ(HSW_PWR_WELL_DRIVER), "Power well on\n"); > I915_STATE_WARN(I915_READ(SPLL_CTL) & SPLL_PLL_ENABLE, "SPLL > enabled\n"); > - I915_STATE_WARN(I915_READ(WRPLL_CTL1) & WRPLL_PLL_ENABLE, "WRPLL1 > enabled\n"); > - I915_STATE_WARN(I915_READ(WRPLL_CTL2) & WRPLL_PLL_ENABLE, "WRPLL2 > enabled\n"); > + I915_STATE_WARN(I915_READ(WRPLL_CTL(0)) & WRPLL_PLL_ENABLE, "WRPLL1 > enabled\n"); > + I915_STATE_WARN(I915_READ(WRPLL_CTL(1)) & WRPLL_PLL_ENABLE, "WRPLL2 > enabled\n"); > I915_STATE_WARN(I915_READ(PCH_PP_STATUS) & PP_ON, "Panel power on\n"); > I915_STATE_WARN(I915_READ(BLC_PWM_CPU_CTL2) & BLM_PWM_ENABLE, > "CPU PWM1 enabled\n"); > -- > 2.4.6 > > _______________________________________________ > Intel-gfx mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
