From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/5] drm/i915: Rename HSW/BDW PLL bits
Date: Wed, 5 Jun 2019 18:53:19 +0300 [thread overview]
Message-ID: <20190605155319.GQ5942@intel.com> (raw)
In-Reply-To: <e5c28c0a-dd90-5cf8-686a-9f10b6cac6e0@linux.intel.com>
On Wed, Jun 05, 2019 at 04:24:25PM +0200, Maarten Lankhorst wrote:
> Op 04-06-2019 om 22:09 schreef Ville Syrjala:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Give the PLL control register bits better names on HSW/BDW.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 37 ++++++++++++++++++---------
> > drivers/gpu/drm/i915/intel_ddi.c | 16 ++++++------
> > drivers/gpu/drm/i915/intel_display.c | 8 +++---
> > drivers/gpu/drm/i915/intel_dpll_mgr.c | 4 +--
> > 4 files changed, 39 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index d5fee72fc079..b7dd42bfffaa 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -9465,24 +9465,33 @@ enum skl_power_gate {
> > /* SPLL */
> > #define SPLL_CTL _MMIO(0x46020)
> > #define SPLL_PLL_ENABLE (1 << 31)
> > -#define SPLL_PLL_SSC (1 << 28)
> > -#define SPLL_PLL_NON_SSC (2 << 28)
> > -#define SPLL_PLL_LCPLL (3 << 28)
> > -#define SPLL_PLL_REF_MASK (3 << 28)
> > -#define SPLL_PLL_FREQ_810MHz (0 << 26)
> > -#define SPLL_PLL_FREQ_1350MHz (1 << 26)
> > -#define SPLL_PLL_FREQ_2700MHz (2 << 26)
> > -#define SPLL_PLL_FREQ_MASK (3 << 26)
> > +#define SPLL_REF_BCLK (0 << 28)
> > +#define SPLL_REF_MUXED_SSC (1 << 28) /* CPU SSC if fused enabled, PCH SSC otherwise */
> > +#define SPLL_REF_NON_SSC_HSW (2 << 28)
> > +#define SPLL_REF_PCH_SSC_BDW (2 << 28)
> > +#define SPLL_REF_LCPLL (3 << 28)
> > +#define SPLL_REF_MASK (3 << 28)
> > +#define SPLL_REF_BCLK (0 << 28)
> > +#define SPLL_REF_SSC (1 << 28)
> > +#define SPLL_REF_NON_SSC (2 << 28)
> > +#define SPLL_REF_LCPLL (3 << 28)
>
> ? Bit unclear or double definitions, at least of SPLL_REF_MASK.
Oh, that one is a copy paste fail. The others are due to HSW
and BDW having slightly different meanings for the bits.
>
>
> > +#define SPLL_REF_MASK (3 << 28)
> > +#define SPLL_FREQ_810MHz (0 << 26)
> > +#define SPLL_FREQ_1350MHz (1 << 26)
> > +#define SPLL_FREQ_2700MHz (2 << 26)
> > +#define SPLL_FREQ_MASK (3 << 26)
> >
> > /* WRPLL */
> > #define _WRPLL_CTL1 0x46040
> > #define _WRPLL_CTL2 0x46060
> > #define WRPLL_CTL(pll) _MMIO_PIPE(pll, _WRPLL_CTL1, _WRPLL_CTL2)
> > #define WRPLL_PLL_ENABLE (1 << 31)
> > -#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)
> > +#define WRPLL_REF_BCLK (0 << 28)
> > +#define WRPLL_REF_PCH_SSC (1 << 28)
> > +#define WRPLL_REF_MUXED_SSC_BDW (2 << 28) /* CPU SSC if fused enabled, PCH SSC otherwise */
> > +#define WRPLL_REF_SPECIAL_HSW (2 << 28) /* muxed SSC (ULT), non-SSC (non-ULT) */
> > +#define WRPLL_REF_LCPLL (3 << 28)
> > +#define WRPLL_REF_MASK (3 << 28)
> > /* WRPLL divider programming */
> > #define WRPLL_DIVIDER_REFERENCE(x) ((x) << 0)
> > #define WRPLL_DIVIDER_REF_MASK (0xff)
> > @@ -9548,6 +9557,10 @@ enum skl_power_gate {
> > #define LCPLL_CTL _MMIO(0x130040)
> > #define LCPLL_PLL_DISABLE (1 << 31)
> > #define LCPLL_PLL_LOCK (1 << 30)
> > +#define LCPLL_REF_NON_SSC (0 << 28)
> > +#define LCPLL_REF_BCLK (2 << 28)
> > +#define LCPLL_REF_PCH_SSC (3 << 28)
> > +#define LCPLL_REF_MASK (3 << 28)
> > #define LCPLL_CLK_FREQ_MASK (3 << 26)
> > #define LCPLL_CLK_FREQ_450 (0 << 26)
> > #define LCPLL_CLK_FREQ_54O_BDW (1 << 26)
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 350eaf54f01f..183f91abda19 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1231,9 +1231,9 @@ static int hsw_ddi_calc_wrpll_link(struct drm_i915_private *dev_priv,
> > u32 wrpll;
> >
> > wrpll = I915_READ(reg);
> > - switch (wrpll & WRPLL_PLL_REF_MASK) {
> > - case WRPLL_PLL_SSC:
> > - case WRPLL_PLL_NON_SSC:
> > + switch (wrpll & WRPLL_REF_MASK) {
> > + case WRPLL_REF_SPECIAL_HSW:
> > + case WRPLL_REF_PCH_SSC:
> > /*
> > * We could calculate spread here, but our checking
> > * code only cares about 5% accuracy, and spread is a max of
> > @@ -1241,7 +1241,7 @@ static int hsw_ddi_calc_wrpll_link(struct drm_i915_private *dev_priv,
> > */
> > refclk = 135;
> > break;
> > - case WRPLL_PLL_LCPLL:
> > + case WRPLL_REF_LCPLL:
> > refclk = LC_FREQ;
> > break;
> > default:
> > @@ -1613,12 +1613,12 @@ static void hsw_ddi_clock_get(struct intel_encoder *encoder,
> > 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;
> > - if (pll == SPLL_PLL_FREQ_810MHz)
> > + pll = I915_READ(SPLL_CTL) & SPLL_FREQ_MASK;
> > + if (pll == SPLL_FREQ_810MHz)
> > link_clock = 81000;
> > - else if (pll == SPLL_PLL_FREQ_1350MHz)
> > + else if (pll == SPLL_FREQ_1350MHz)
> > link_clock = 135000;
> > - else if (pll == SPLL_PLL_FREQ_2700MHz)
> > + else if (pll == SPLL_FREQ_2700MHz)
> > link_clock = 270000;
> > else {
> > WARN(1, "bad spll freq\n");
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 75ca2030ffb0..393b19e53ba3 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9129,12 +9129,12 @@ static bool spll_uses_pch_ssc(struct drm_i915_private *dev_priv)
> > if ((ctl & SPLL_PLL_ENABLE) == 0)
> > return false;
> >
> > - if ((ctl & SPLL_PLL_REF_MASK) == SPLL_PLL_SSC &&
> > + if ((ctl & SPLL_REF_MASK) == SPLL_REF_MUXED_SSC &&
> > (fuse_strap & HSW_CPU_SSC_ENABLE) == 0)
> > return true;
> >
> > if (IS_BROADWELL(dev_priv) &&
> > - (ctl & SPLL_PLL_REF_MASK) == SPLL_PLL_NON_SSC)
> > + (ctl & SPLL_REF_MASK) == SPLL_REF_PCH_SSC_BDW)
> > return true;
> >
> > return false;
> > @@ -9149,11 +9149,11 @@ static bool wrpll_uses_pch_ssc(struct drm_i915_private *dev_priv,
> > if ((ctl & WRPLL_PLL_ENABLE) == 0)
> > return false;
> >
> > - if ((ctl & WRPLL_PLL_REF_MASK) == WRPLL_PLL_SSC)
> > + if ((ctl & WRPLL_REF_MASK) == WRPLL_REF_PCH_SSC)
> > return true;
> >
> > if ((IS_BROADWELL(dev_priv) || IS_HSW_ULT(dev_priv)) &&
> > - (ctl & WRPLL_PLL_REF_MASK) == WRPLL_PLL_NON_SSC &&
> > + (ctl & WRPLL_REF_MASK) == WRPLL_REF_MUXED_SSC_BDW &&
> > (fuse_strap & HSW_CPU_SSC_ENABLE) == 0)
> > return true;
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > index 69787f259677..2d4e7b9a7b9d 100644
> > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > @@ -775,7 +775,7 @@ static struct intel_shared_dpll *hsw_ddi_hdmi_get_dpll(struct intel_crtc_state *
> >
> > hsw_ddi_calculate_wrpll(crtc_state->port_clock * 1000, &r2, &n2, &p);
> >
> > - val = WRPLL_PLL_ENABLE | WRPLL_PLL_LCPLL |
> > + val = WRPLL_PLL_ENABLE | WRPLL_REF_LCPLL |
> > WRPLL_DIVIDER_REFERENCE(r2) | WRPLL_DIVIDER_FEEDBACK(n2) |
> > WRPLL_DIVIDER_POST(p);
> >
> > @@ -839,7 +839,7 @@ hsw_get_dpll(struct intel_crtc_state *crtc_state,
> > return NULL;
> >
> > crtc_state->dpll_hw_state.spll =
> > - SPLL_PLL_ENABLE | SPLL_PLL_FREQ_1350MHz | SPLL_PLL_SSC;
> > + SPLL_PLL_ENABLE | SPLL_FREQ_1350MHz | SPLL_REF_MUXED_SSC;
> >
> > pll = intel_find_shared_dpll(crtc_state,
> > DPLL_ID_SPLL, DPLL_ID_SPLL);
>
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2019-06-05 15:53 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-04 20:09 [PATCH 1/5] drm/i915: Do not touch the PCH SSC reference if a PLL is using it Ville Syrjala
2019-06-04 20:09 ` [PATCH 2/5] drm/i915: Rename HSW/BDW PLL bits Ville Syrjala
2019-06-05 14:24 ` Maarten Lankhorst
2019-06-05 15:53 ` Ville Syrjälä [this message]
2019-06-10 13:36 ` [PATCH v2 " Ville Syrjala
2019-06-04 20:09 ` [PATCH 3/5] drm/i915: Nuke LC_FREQ Ville Syrjala
2019-06-04 20:09 ` [PATCH 4/5] drm/i915: Assert that HSW/BDW LCPLL is using the non-SSC reference Ville Syrjala
2019-06-04 20:09 ` [PATCH 5/5] drm/i915: Improve WRPLL reference clock readout on HSW/BDW Ville Syrjala
2019-06-05 14:26 ` Maarten Lankhorst
2019-06-04 21:00 ` ✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915: Do not touch the PCH SSC reference if a PLL is using it Patchwork
2019-06-06 0:12 ` ✓ Fi.CI.IGT: " Patchwork
2019-06-10 17:01 ` ✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915: Do not touch the PCH SSC reference if a PLL is using it (rev2) Patchwork
2019-06-11 18:31 ` ✓ Fi.CI.IGT: " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190605155319.GQ5942@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=maarten.lankhorst@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.