On Fri, 18 Sep 2015, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> 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 <jani.nikula@xxxxxxxxx>
> #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
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
