On Thu, Sep 07, 2023 at 08:37:55AM -0700, Lucas De Marchi wrote:
> From: Ravi Kumar Vodapalli <ravi.kumar.vodapalli@xxxxxxxxx>
>
> Add CDCLK initialization sequence changes and CDCLK set frequency
The title and first line of this commit message don't seem to really
match the patch anymore. The changes here aren't about the
"initialization sequence" (which I assume refers to something that
happens suring driver init), but rather to the steps we take every time
we reprogram the cdclk.
> sequence for LNL platform. It's mostly the same as MTL, but with some
> additional programming for the squash and crawling steps when
> when a change in mdclk/cdclk ratio is observed.
>
> v2: Remove wrong changes for bxt_cdclk_cd2x_pipe() (Matt Roper)
>
> BSpec: 68846, 68864
Related to the above, I don't see anything on 68846 that relates to the
changes in this patch.
> 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 | 51 +++++++++++++++++++++-
> 1 file changed, 49 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 677a50c28dae..dfefc971b733 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -40,6 +40,7 @@
> #include "intel_psr.h"
> #include "intel_vdsc.h"
> #include "skl_watermark.h"
> +#include "skl_watermark_regs.h"
> #include "vlv_sideband.h"
>
> /**
> @@ -1811,6 +1812,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);
> @@ -1913,8 +1955,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)
Since everything going forward is expected to support squashing, it may
be cleaner to flatten this into a single if/else ladder:
if (DISPLAY_VER(dev_priv) >= 20)
...lnl_cdclk_squash_program...
else if (HAS_CDCLK_SQUASH(dev_priv))
...dg2_cdclk_squash_program...
Aside from the commit message/title and the ladder flattening, the
actual logic of the patch looks correct, so
Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
with those fixed.
Matt
> + 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);
> --
> 2.40.1
>
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
