On Wed, Aug 23, 2023 at 10:07:34AM -0700, Lucas De Marchi wrote: > From: Ravi Kumar Vodapalli <ravi.kumar.vodapalli@xxxxxxxxx> > > Add CDCLK initialization sequence changes and CDCLK set frequency > sequence for LNL platform. > > CDCLK frequency change sequence is different for LNL compared to MTL > when a change in mdclk/cdclk ratio is observed. Below are changes to be > made: > > 1. In MBUS_CTL register translation Throttle Min value. > 2. In DBUF_CTL_S* register Min Tracker State Service value. The previous patch just did these same changes, but made the changes to the existing functions. It looks like we wound up with two patches doing the same thing? > > BSpec: 68846, 68864 > Signed-off-by: Ravi Kumar Vodapalli <ravi.kumar.vodapalli@xxxxxxxxx> > Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx> > --- > drivers/gpu/drm/i915/display/intel_cdclk.c | 58 ++++++++++++++++++++-- > drivers/gpu/drm/i915/i915_reg.h | 2 + > 2 files changed, 57 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c > index aa1000db3cb9..4d8b960389ec 100644 > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > @@ -38,6 +38,7 @@ > #include "intel_pcode.h" > #include "intel_psr.h" > #include "skl_watermark.h" > +#include "skl_watermark_regs.h" > #include "vlv_sideband.h" > > /** > @@ -1727,7 +1728,12 @@ static void adlp_cdclk_pll_crawl(struct drm_i915_private *dev_priv, int vco) > > static u32 bxt_cdclk_cd2x_pipe(struct drm_i915_private *dev_priv, enum pipe pipe) > { > - if (DISPLAY_VER(dev_priv) >= 12) { > + if (DISPLAY_VER(dev_priv) >= 20) { > + if (pipe == INVALID_PIPE) > + return LNL_CDCLK_CD2X_PIPE_NONE; > + else > + return LNL_CDCLK_CD2X_PIPE(pipe); I don't think this change is correct; see note farther down on the register definitions. > + } else if (DISPLAY_VER(dev_priv) >= 12) { > if (pipe == INVALID_PIPE) > return TGL_CDCLK_CD2X_PIPE_NONE; > else > @@ -1837,6 +1843,47 @@ static int get_mdclk_cdclk_ratio(struct drm_i915_private *i915, > return 1; > } > > +static void lnl_prog_mbus_dbuf_ctrl(struct drm_i915_private *i915, > + const struct intel_cdclk_config *cdclk_config) > +{ > + int min_throttle_val; > + int min_tracker_state; > + enum dbuf_slice slice; > + int mdclk_cdclk_div_ratio; > + int mbus_join = intel_de_read(i915, MBUS_CTL) & MBUS_JOIN; > + > + mdclk_cdclk_div_ratio = get_mdclk_cdclk_ratio(i915, cdclk_config); > + > + min_throttle_val = MBUS_TRANS_THROTTLE_MIN_SELECT(mdclk_cdclk_div_ratio); > + > + intel_de_rmw(i915, MBUS_CTL, MBUS_TRANS_THROTTLE_MIN_MASK, min_throttle_val); > + > + if (mbus_join) > + mdclk_cdclk_div_ratio = (mdclk_cdclk_div_ratio << 1) + 1; > + > + min_tracker_state = DBUF_MIN_TRACKER_STATE_SERVICE(mdclk_cdclk_div_ratio); > + > + for_each_dbuf_slice(i915, slice) > + intel_de_rmw(i915, DBUF_CTL_S(slice), > + DBUF_MIN_TRACKER_STATE_SERVICE_MASK, > + min_tracker_state); > +} > + > +static void lnl_cdclk_squash_program(struct drm_i915_private *i915, > + const struct intel_cdclk_config *cdclk_config, > + u16 waveform) > +{ > + if (cdclk_config->cdclk < i915->display.cdclk.hw.cdclk) > + /* Program mbus_ctrl and dbuf_ctrl registers as Pre hook */ > + lnl_prog_mbus_dbuf_ctrl(i915, cdclk_config); > + > + dg2_cdclk_squash_program(i915, waveform); > + > + if (cdclk_config->cdclk > i915->display.cdclk.hw.cdclk) > + /* Program mbus_ctrl and dbuf_ctrl registers as Post hook */ > + lnl_prog_mbus_dbuf_ctrl(i915, cdclk_config); > +} > + > static int cdclk_squash_divider(u16 waveform) > { > return hweight16(waveform ?: 0xffff); > @@ -1938,8 +1985,13 @@ static void _bxt_set_cdclk(struct drm_i915_private *dev_priv, > else > clock = cdclk; > > - if (HAS_CDCLK_SQUASH(dev_priv)) > - dg2_cdclk_squash_program(dev_priv, waveform); > + if (HAS_CDCLK_SQUASH(dev_priv)) { > + if (DISPLAY_VER(dev_priv) >= 20) > + lnl_cdclk_squash_program(dev_priv, cdclk_config, > + waveform); > + else > + dg2_cdclk_squash_program(dev_priv, waveform); > + } > > val = bxt_cdclk_cd2x_div_sel(dev_priv, clock, vco) | > bxt_cdclk_cd2x_pipe(dev_priv, pipe); > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index d5850761a75a..c9639f0f4f49 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -5944,6 +5944,8 @@ enum skl_power_gate { > #define BXT_CDCLK_CD2X_PIPE(pipe) ((pipe) << 20) > #define CDCLK_DIVMUX_CD_OVERRIDE (1 << 19) > #define BXT_CDCLK_CD2X_PIPE_NONE BXT_CDCLK_CD2X_PIPE(3) > +#define LNL_CDCLK_CD2X_PIPE(pipe) ((pipe) << 19) This doesn't match what I see on bspec 69090: Bits 21:19 000 => Pipe A 010 => Pipe B 100 => Pipe C 110 => Pipe D So the pipe ID (0-3) should actually be shifted by 20 since bit 19 is always 0 (except for the "none" case). I think Matt > +#define LNL_CDCLK_CD2X_PIPE_NONE (7 << 19) > #define ICL_CDCLK_CD2X_PIPE(pipe) (_PICK(pipe, 0, 2, 6) << 19) > #define ICL_CDCLK_CD2X_PIPE_NONE (7 << 19) > #define TGL_CDCLK_CD2X_PIPE(pipe) BXT_CDCLK_CD2X_PIPE(pipe) > -- > 2.40.1 > -- Matt Roper Graphics Software Engineer Linux GPU Platform Enablement Intel Corporation