Quoting Matt Roper (2024-03-04 20:25:31-03:00) >On Mon, Mar 04, 2024 at 03:30:24PM -0300, Gustavo Sousa wrote: >> CDCLK programming Xe2LPD always selects the CDCLK PLL as source for the > >I think something got a bit muddled while rewriting this sentence. >Maybe the first two words were supposed to be dropped? Yeah. The original intention here was "CDCLK programming for Xe2LPD ...". Dropping the first two words also works :-) Will reword this in v2. > >Otherwise, > >Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx> Thanks! -- Gustavo Sousa > >> MDCLK. Because of that, the ratio between MDCLK and CDCLK is not be >> constant anymore. As such, make sure to have the current ratio available >> in intel_dbuf_state so that it can be used during dbuf programming. >> >> Note that we write-lock the global state instead of serializing to a >> hardware commit because a change in the ratio should be rather handled >> in the CDCLK change sequence, which will need to take care of updating >> the necessary registers in that case. We will implement that in upcoming >> changes. >> >> That said, changes in the MBus joining state should be handled by the >> DBUF/MBUS logic, just like it is already done, but the logic will need >> to know the ratio to properly update the registers. >> >> Signed-off-by: Gustavo Sousa <gustavo.sousa@xxxxxxxxx> >> --- >> drivers/gpu/drm/i915/display/intel_cdclk.c | 26 ++++++++++++++++++++ >> drivers/gpu/drm/i915/display/intel_cdclk.h | 2 ++ >> drivers/gpu/drm/i915/display/skl_watermark.c | 18 +++++++++++++- >> drivers/gpu/drm/i915/display/skl_watermark.h | 3 +++ >> 4 files changed, 48 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c >> index cdf3ae766f9e..04a6e9806254 100644 >> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c >> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c >> @@ -39,6 +39,7 @@ >> #include "intel_pcode.h" >> #include "intel_psr.h" >> #include "intel_vdsc.h" >> +#include "skl_watermark.h" >> #include "vlv_sideband.h" >> >> /** >> @@ -1891,6 +1892,22 @@ static u32 xe2lpd_mdclk_source_sel(struct drm_i915_private *i915) >> return MDCLK_SOURCE_SEL_CD2XCLK; >> } >> >> +u8 intel_mdclk_cdclk_ratio(struct drm_i915_private *i915, >> + const struct intel_cdclk_config *cdclk_config) >> +{ >> + u32 source_sel = xe2lpd_mdclk_source_sel(i915); >> + >> + switch (source_sel) { >> + case MDCLK_SOURCE_SEL_CD2XCLK: >> + return 2; >> + case MDCLK_SOURCE_SEL_CDCLK_PLL: >> + return DIV_ROUND_UP(cdclk_config->vco, cdclk_config->cdclk); >> + default: >> + MISSING_CASE(source_sel); >> + return 2; >> + } >> +} >> + >> static bool cdclk_compute_crawl_and_squash_midpoint(struct drm_i915_private *i915, >> const struct intel_cdclk_config *old_cdclk_config, >> const struct intel_cdclk_config *new_cdclk_config, >> @@ -3281,6 +3298,15 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state) >> "Modeset required for cdclk change\n"); >> } >> >> + if (intel_mdclk_cdclk_ratio(dev_priv, &old_cdclk_state->actual) != >> + intel_mdclk_cdclk_ratio(dev_priv, &new_cdclk_state->actual)) { >> + u8 ratio = intel_mdclk_cdclk_ratio(dev_priv, &new_cdclk_state->actual); >> + >> + ret = intel_dbuf_state_set_mdclk_cdclk_ratio(state, ratio); >> + if (ret) >> + return ret; >> + } >> + >> drm_dbg_kms(&dev_priv->drm, >> "New cdclk calculated to be logical %u kHz, actual %u kHz\n", >> new_cdclk_state->logical.cdclk, >> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h >> index fa301495e7f1..8e6e302bd599 100644 >> --- a/drivers/gpu/drm/i915/display/intel_cdclk.h >> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h >> @@ -62,6 +62,8 @@ void intel_update_cdclk(struct drm_i915_private *dev_priv); >> u32 intel_read_rawclk(struct drm_i915_private *dev_priv); >> bool intel_cdclk_clock_changed(const struct intel_cdclk_config *a, >> const struct intel_cdclk_config *b); >> +u8 intel_mdclk_cdclk_ratio(struct drm_i915_private *i915, >> + const struct intel_cdclk_config *cdclk_config); >> void intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state); >> void intel_set_cdclk_post_plane_update(struct intel_atomic_state *state); >> void intel_cdclk_dump_config(struct drm_i915_private *i915, >> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c >> index d9e49cd60d3a..4410e21888ad 100644 >> --- a/drivers/gpu/drm/i915/display/skl_watermark.c >> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c >> @@ -3057,6 +3057,8 @@ static void skl_wm_get_hw_state(struct drm_i915_private *i915) >> if (HAS_MBUS_JOINING(i915)) >> dbuf_state->joined_mbus = intel_de_read(i915, MBUS_CTL) & MBUS_JOIN; >> >> + dbuf_state->mdclk_cdclk_ratio = intel_mdclk_cdclk_ratio(i915, &i915->display.cdclk.hw); >> + >> for_each_intel_crtc(&i915->drm, crtc) { >> struct intel_crtc_state *crtc_state = >> to_intel_crtc_state(crtc->base.state); >> @@ -3530,6 +3532,19 @@ int intel_dbuf_init(struct drm_i915_private *i915) >> return 0; >> } >> >> +int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8 ratio) >> +{ >> + struct intel_dbuf_state *dbuf_state; >> + >> + dbuf_state = intel_atomic_get_dbuf_state(state); >> + if (IS_ERR(dbuf_state)) >> + return PTR_ERR(dbuf_state); >> + >> + dbuf_state->mdclk_cdclk_ratio = ratio; >> + >> + return intel_atomic_lock_global_state(&dbuf_state->base); >> +} >> + >> static void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, >> u8 ratio, >> bool joined_mbus) >> @@ -3574,7 +3589,8 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state) >> MBUS_HASHING_MODE_MASK | MBUS_JOIN | >> MBUS_JOIN_PIPE_SELECT_MASK, mbus_ctl); >> >> - intel_dbuf_mdclk_cdclk_ratio_update(i915, 2, dbuf_state->joined_mbus); >> + intel_dbuf_mdclk_cdclk_ratio_update(i915, dbuf_state->mdclk_cdclk_ratio, >> + dbuf_state->joined_mbus); >> } >> >> void intel_dbuf_pre_plane_update(struct intel_atomic_state *state) >> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h b/drivers/gpu/drm/i915/display/skl_watermark.h >> index e3d1d74a7b17..fed4d12df584 100644 >> --- a/drivers/gpu/drm/i915/display/skl_watermark.h >> +++ b/drivers/gpu/drm/i915/display/skl_watermark.h >> @@ -58,6 +58,7 @@ struct intel_dbuf_state { >> u8 slices[I915_MAX_PIPES]; >> u8 enabled_slices; >> u8 active_pipes; >> + u8 mdclk_cdclk_ratio; >> bool joined_mbus; >> }; >> >> @@ -71,6 +72,8 @@ intel_atomic_get_dbuf_state(struct intel_atomic_state *state); >> to_intel_dbuf_state(intel_atomic_get_new_global_obj_state(state, &to_i915(state->base.dev)->display.dbuf.obj)) >> >> int intel_dbuf_init(struct drm_i915_private *i915); >> +int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8 ratio); >> + >> void intel_dbuf_pre_plane_update(struct intel_atomic_state *state); >> void intel_dbuf_post_plane_update(struct intel_atomic_state *state); >> void intel_mbus_dbox_update(struct intel_atomic_state *state); >> -- >> 2.44.0 >> > >-- >Matt Roper >Graphics Software Engineer >Linux GPU Platform Enablement >Intel Corporation