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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).