> -----Original Message----- > From: Intel-gfx <[email protected]> On Behalf Of Ville > Syrjala > Sent: Wednesday, March 27, 2024 11:16 PM > To: [email protected] > Cc: Ville Syrjälä <[email protected]> > Subject: [PATCH 11/13] drm/i915: Implement vblank synchronized MBUS join > changes > > From: Stanislav Lisovskiy <[email protected]> > > Currently we can't change MBUS join status without doing a modeset, because > we are lacking mechanism to synchronize those with vblank. > However then this means that we can't do a fastset, if there is a need to > change > MBUS join state. Fix that by implementing such change. > We already call correspondent check and update at pre_plane dbuf update, so > the > only thing left is to have a non-modeset version of that. > If active pipes stay the same then fastset is possible and only MBUS join > state/ddb allocation updates would be committed. > > The full mbus/cdclk sequence will look as follows: > 1. disable pipes > 2. increase cdclk if necessary > 2.1 reprogram cdclk > 2.2 update dbuf tracker value > 3. enable mbus joining if necessary > 3.1 update mbus_ctl > 3.2 update dbuf tracker value > 4. reallocate dbuf for planes on active pipes 5. disable mbus joining if > necessary > 5.1 update dbuf tracker value > 5.2 update mbus_ctl > 6. enable pipes > 7. decrease cdclk if necessary > 7.1 update dbuf tracker value > 7.2 reprogram cdclk > > And in order to keep things in sync we need: > Step 2: > - mbus_join == old > - mdclk/cdclk ratio == new > Step 3: > - mbus_join == new > - mdclk/cdclk ratio == old when cdclk is changing in step 7 > - mdclk/cdclk ratio == new when cdclk is changing in step 2 Step 5: > - mbus_join == new > - mdclk/cdclk ratio == old when cdclk is changing in step 7 > - mdclk/cdclk ratio == new when cdclk is changing in step 2 Step 7: > - mbus_join == new > - mdclk/cdclk ratio == new > > v2: - Removed redundant parentheses(Ville Syrjälä) > - Constified new_crtc_state in intel_mbus_joined_pipe(Ville Syrjälä) > - Removed pipe_select variable(Ville Syrjälä) > [v3: vsyrjala: Correctly sequence vs. cdclk updates, > properly describe the full sequence, > shuffle code around to make the diff more legible, > streamline a few things]
Looks Good to me. Reviewed-by: Uma Shankar <[email protected]> > Signed-off-by: Stanislav Lisovskiy <[email protected]> > Co-developed-by: Ville Syrjälä <[email protected]> > Signed-off-by: Ville Syrjälä <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_cdclk.c | 11 ++ > drivers/gpu/drm/i915/display/intel_cdclk.h | 1 + > drivers/gpu/drm/i915/display/intel_display.c | 5 +- > drivers/gpu/drm/i915/display/skl_watermark.c | 141 ++++++++++++------- > drivers/gpu/drm/i915/display/skl_watermark.h | 3 +- > 5 files changed, 112 insertions(+), 49 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c > b/drivers/gpu/drm/i915/display/intel_cdclk.c > index 4024118a7ffb..66c161d7b485 100644 > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > @@ -2576,6 +2576,17 @@ static void intel_cdclk_pcode_post_notify(struct > intel_atomic_state *state) > update_cdclk, update_pipe_count); } > > +bool intel_cdclk_is_decreasing_later(struct intel_atomic_state *state) > +{ > + const struct intel_cdclk_state *old_cdclk_state = > + intel_atomic_get_old_cdclk_state(state); > + const struct intel_cdclk_state *new_cdclk_state = > + intel_atomic_get_new_cdclk_state(state); > + > + return new_cdclk_state && !new_cdclk_state->disable_pipes && > + new_cdclk_state->actual.cdclk < old_cdclk_state->actual.cdclk; > } > + > /** > * intel_set_cdclk_pre_plane_update - Push the CDCLK state to the hardware > * @state: intel atomic state > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h > b/drivers/gpu/drm/i915/display/intel_cdclk.h > index 2843fc091086..5d4faf401774 100644 > --- a/drivers/gpu/drm/i915/display/intel_cdclk.h > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h > @@ -69,6 +69,7 @@ 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); > +bool intel_cdclk_is_decreasing_later(struct intel_atomic_state *state); > 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/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index 4d6668a5f1ab..023cf4a77e6f 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -6915,6 +6915,8 @@ static void skl_commit_modeset_enables(struct > intel_atomic_state *state) > intel_pre_update_crtc(state, crtc); > } > > + intel_dbuf_mbus_pre_ddb_update(state); > + > while (update_pipes) { > for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, > new_crtc_state, i) { > @@ -6945,6 +6947,8 @@ static void skl_commit_modeset_enables(struct > intel_atomic_state *state) > } > } > > + intel_dbuf_mbus_post_ddb_update(state); > + > update_pipes = modeset_pipes; > > /* > @@ -7191,7 +7195,6 @@ static void intel_atomic_commit_tail(struct > intel_atomic_state *state) > intel_encoders_update_prepare(state); > > intel_dbuf_pre_plane_update(state); > - intel_mbus_dbox_update(state); > > for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { > if (new_crtc_state->do_async_flip) > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c > b/drivers/gpu/drm/i915/display/skl_watermark.c > index a118ecf9e532..ca0f1f89e6d9 100644 > --- a/drivers/gpu/drm/i915/display/skl_watermark.c > +++ b/drivers/gpu/drm/i915/display/skl_watermark.c > @@ -2636,13 +2636,6 @@ skl_compute_ddb(struct intel_atomic_state *state) > if (ret) > return ret; > > - if (old_dbuf_state->joined_mbus != new_dbuf_state- > >joined_mbus) { > - /* TODO: Implement vblank synchronized MBUS joining > changes */ > - ret = intel_modeset_all_pipes_late(state, "MBUS joining > change"); > - if (ret) > - return ret; > - } > - > drm_dbg_kms(&i915->drm, > "Enabled dbuf slices 0x%x -> 0x%x (total dbuf slices > 0x%x), mbus joined? %s->%s\n", > old_dbuf_state->enabled_slices, > @@ -3559,7 +3552,7 @@ static bool xelpdp_is_only_pipe_per_dbuf_bank(enum > pipe pipe, u8 active_pipes) > return false; > } > > -void intel_mbus_dbox_update(struct intel_atomic_state *state) > +static void intel_mbus_dbox_update(struct intel_atomic_state *state) > { > struct drm_i915_private *i915 = to_i915(state->base.dev); > const struct intel_dbuf_state *new_dbuf_state, *old_dbuf_state; @@ - > 3640,6 +3633,9 @@ void intel_dbuf_mdclk_cdclk_ratio_update(struct > drm_i915_private *i915, u8 ratio { > enum dbuf_slice slice; > > + if (!HAS_MBUS_JOINING(i915)) > + return; > + > if (DISPLAY_VER(i915) >= 20) > intel_de_rmw(i915, MBUS_CTL, > MBUS_TRANSLATION_THROTTLE_MIN_MASK, > MBUS_TRANSLATION_THROTTLE_MIN(ratio - 1)); > @@ -3663,24 +3659,42 @@ static void > intel_dbuf_mdclk_min_tracker_update(struct intel_atomic_state *state > intel_atomic_get_old_dbuf_state(state); > const struct intel_dbuf_state *new_dbuf_state = > intel_atomic_get_new_dbuf_state(state); > + int mdclk_cdclk_ratio; > > - if (DISPLAY_VER(i915) >= 20 && > - old_dbuf_state->mdclk_cdclk_ratio != new_dbuf_state- > >mdclk_cdclk_ratio) { > - /* > - * For Xe2LPD and beyond, when there is a change in the ratio > - * between MDCLK and CDCLK, updates to related registers need > to > - * happen at a specific point in the CDCLK change sequence. In > - * that case, we defer to the call to > - * intel_dbuf_mdclk_cdclk_ratio_update() to the CDCLK logic. > - */ > - return; > + if (intel_cdclk_is_decreasing_later(state)) { > + /* cdclk/mdclk will be changed later by > intel_set_cdclk_post_plane_update() */ > + mdclk_cdclk_ratio = old_dbuf_state->mdclk_cdclk_ratio; > + } else { > + /* cdclk/mdclk already changed by > intel_set_cdclk_pre_plane_update() */ > + mdclk_cdclk_ratio = new_dbuf_state->mdclk_cdclk_ratio; > } > > - intel_dbuf_mdclk_cdclk_ratio_update(i915, new_dbuf_state- > >mdclk_cdclk_ratio, > + intel_dbuf_mdclk_cdclk_ratio_update(i915, mdclk_cdclk_ratio, > new_dbuf_state->joined_mbus); > } > > -static void intel_dbuf_mbus_join_update(struct intel_atomic_state *state) > +static enum pipe intel_mbus_joined_pipe(struct intel_atomic_state *state, > + const struct intel_dbuf_state > *dbuf_state) { > + struct drm_i915_private *i915 = to_i915(state->base.dev); > + enum pipe pipe = ffs(dbuf_state->active_pipes) - 1; > + const struct intel_crtc_state *new_crtc_state; > + struct intel_crtc *crtc; > + > + drm_WARN_ON(&i915->drm, !dbuf_state->joined_mbus); > + drm_WARN_ON(&i915->drm, !is_power_of_2(dbuf_state- > >active_pipes)); > + > + crtc = intel_crtc_for_pipe(i915, pipe); > + new_crtc_state = intel_atomic_get_new_crtc_state(state, crtc); > + > + if (new_crtc_state && !intel_crtc_needs_modeset(new_crtc_state)) > + return pipe; > + else > + return INVALID_PIPE; > +} > + > +static void intel_dbuf_mbus_join_update(struct intel_atomic_state *state, > + enum pipe pipe) > { > struct drm_i915_private *i915 = to_i915(state->base.dev); > const struct intel_dbuf_state *old_dbuf_state = @@ -3689,44 +3703,80 > @@ static void intel_dbuf_mbus_join_update(struct intel_atomic_state *state) > intel_atomic_get_new_dbuf_state(state); > u32 mbus_ctl; > > - drm_dbg_kms(&i915->drm, "Changing mbus joined: %s -> %s\n", > + drm_dbg_kms(&i915->drm, "Changing mbus joined: %s -> %s (pipe: > %c)\n", > str_yes_no(old_dbuf_state->joined_mbus), > - str_yes_no(new_dbuf_state->joined_mbus)); > + str_yes_no(new_dbuf_state->joined_mbus), > + pipe != INVALID_PIPE ? pipe_name(pipe) : '*'); > > - /* > - * TODO: Implement vblank synchronized MBUS joining changes. > - * Must be properly coordinated with dbuf reprogramming. > - */ > if (new_dbuf_state->joined_mbus) > - mbus_ctl = MBUS_HASHING_MODE_1x4 | MBUS_JOIN | > - MBUS_JOIN_PIPE_SELECT_NONE; > + mbus_ctl = MBUS_HASHING_MODE_1x4 | MBUS_JOIN; > else > - mbus_ctl = MBUS_HASHING_MODE_2x2 | > - MBUS_JOIN_PIPE_SELECT_NONE; > + mbus_ctl = MBUS_HASHING_MODE_2x2; > + > + if (pipe != INVALID_PIPE) > + mbus_ctl |= MBUS_JOIN_PIPE_SELECT(pipe); > + else > + mbus_ctl |= MBUS_JOIN_PIPE_SELECT_NONE; > > intel_de_rmw(i915, MBUS_CTL, > MBUS_HASHING_MODE_MASK | MBUS_JOIN | > MBUS_JOIN_PIPE_SELECT_MASK, mbus_ctl); } > > -/* > - * Configure MBUS_CTL and all DBUF_CTL_S of each slice to join_mbus state > before > - * update the request state of all DBUS slices. > - */ > -static void update_mbus_pre_enable(struct intel_atomic_state *state) > +void intel_dbuf_mbus_pre_ddb_update(struct intel_atomic_state *state) { > + const struct intel_dbuf_state *new_dbuf_state = > + intel_atomic_get_new_dbuf_state(state); > + const struct intel_dbuf_state *old_dbuf_state = > + intel_atomic_get_old_dbuf_state(state); > + > + if (!new_dbuf_state) > + return; > + > + if (!old_dbuf_state->joined_mbus && new_dbuf_state->joined_mbus) { > + enum pipe pipe = intel_mbus_joined_pipe(state, > new_dbuf_state); > + > + WARN_ON(!new_dbuf_state->base.changed); > + > + intel_dbuf_mbus_join_update(state, pipe); > + intel_mbus_dbox_update(state); > + intel_dbuf_mdclk_min_tracker_update(state); > + } > +} > + > +void intel_dbuf_mbus_post_ddb_update(struct intel_atomic_state *state) > { > struct drm_i915_private *i915 = to_i915(state->base.dev); > + const struct intel_dbuf_state *new_dbuf_state = > + intel_atomic_get_new_dbuf_state(state); > + const struct intel_dbuf_state *old_dbuf_state = > + intel_atomic_get_old_dbuf_state(state); > > - if (!HAS_MBUS_JOINING(i915)) > + if (!new_dbuf_state) > return; > > - /* > - * TODO: Implement vblank synchronized MBUS joining changes. > - * Must be properly coordinated with dbuf reprogramming. > - */ > - intel_dbuf_mbus_join_update(state); > + if (old_dbuf_state->joined_mbus && !new_dbuf_state->joined_mbus) { > + enum pipe pipe = intel_mbus_joined_pipe(state, old_dbuf_state); > + > + WARN_ON(!new_dbuf_state->base.changed); > + > + intel_dbuf_mdclk_min_tracker_update(state); > + intel_mbus_dbox_update(state); > + intel_dbuf_mbus_join_update(state, pipe); > + > + if (pipe != INVALID_PIPE) { > + struct intel_crtc *crtc = intel_crtc_for_pipe(i915, > pipe); > + > + intel_crtc_wait_for_next_vblank(crtc); > + } > + } else if (old_dbuf_state->joined_mbus == new_dbuf_state->joined_mbus > && > + old_dbuf_state->active_pipes != new_dbuf_state- > >active_pipes) { > + WARN_ON(!new_dbuf_state->base.changed); > + > + intel_dbuf_mdclk_min_tracker_update(state); > + intel_mbus_dbox_update(state); > + } > > - intel_dbuf_mdclk_min_tracker_update(state); > } > > void intel_dbuf_pre_plane_update(struct intel_atomic_state *state) @@ - > 3738,13 +3788,11 @@ void intel_dbuf_pre_plane_update(struct > intel_atomic_state *state) > intel_atomic_get_old_dbuf_state(state); > > if (!new_dbuf_state || > - (new_dbuf_state->enabled_slices == old_dbuf_state->enabled_slices > && > - new_dbuf_state->joined_mbus == old_dbuf_state->joined_mbus)) > + new_dbuf_state->enabled_slices == old_dbuf_state->enabled_slices) > return; > > WARN_ON(!new_dbuf_state->base.changed); > > - update_mbus_pre_enable(state); > gen9_dbuf_slices_update(i915, > old_dbuf_state->enabled_slices | > new_dbuf_state->enabled_slices); > @@ -3759,8 +3807,7 @@ void intel_dbuf_post_plane_update(struct > intel_atomic_state *state) > intel_atomic_get_old_dbuf_state(state); > > if (!new_dbuf_state || > - (new_dbuf_state->enabled_slices == old_dbuf_state->enabled_slices > && > - new_dbuf_state->joined_mbus == old_dbuf_state->joined_mbus)) > + new_dbuf_state->enabled_slices == old_dbuf_state->enabled_slices) > return; > > WARN_ON(!new_dbuf_state->base.changed); > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h > b/drivers/gpu/drm/i915/display/skl_watermark.h > index bf7516620ab6..3323a1d973f9 100644 > --- a/drivers/gpu/drm/i915/display/skl_watermark.h > +++ b/drivers/gpu/drm/i915/display/skl_watermark.h > @@ -79,7 +79,8 @@ int intel_dbuf_state_set_mdclk_cdclk_ratio(struct > intel_atomic_state *state, u8 void intel_dbuf_pre_plane_update(struct > intel_atomic_state *state); void intel_dbuf_post_plane_update(struct > intel_atomic_state *state); void intel_dbuf_mdclk_cdclk_ratio_update(struct > drm_i915_private *i915, u8 ratio, bool joined_mbus); -void > intel_mbus_dbox_update(struct intel_atomic_state *state); > +void intel_dbuf_mbus_pre_ddb_update(struct intel_atomic_state *state); > +void intel_dbuf_mbus_post_ddb_update(struct intel_atomic_state *state); > > #endif /* __SKL_WATERMARK_H__ */ > > -- > 2.43.2
