Quoting Ville Syrjala (2024-03-26 17:31:26-03:00)
>From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
>Currently we always reprogram CDCLK from the
>intel_set_cdclk_post_plane_update() when using squahs/crawl.
Hm... I'm not following this sentence. Looks like cdclk_state->pipe will
be INVALID_PIPE unless it is a cd2x update, right?
Did you mean intel_set_cdclk_pre_plane_update() above? That would make
sense, because it seems we would be doing the update there even when
decreasing the cdclk because of the condition pipe == INVALID_PIPE.
--
Gustavo Sousa
>The code only works correctly for the cd2x update or full
>modeset cases, and it was simply never updated to deal with
>squash/crawl.
>
>If the CDCLK frequency is increasing we must reprogram it
>before we do anything else that might depend on the new
>higher frequency, and conversely we must not decrease
>the frequency until everything that might still depend
>on the old higher frequency has been dealt with.
>
>So let us only consider the relationship of the old and
>new CDCLK frequencies when determining where to do
>the reprogramming, regarless of whether all pipes might
>be off or not at the time.
>
>If the CDCLK freqiency remains unchanges we may still have to
>do the reprogramming to change the voltage_level. Currently
>we do that from intel_set_cdclk_pre_plane_update() which
>probably is the right choice most of the time. The only
>counterexample is when the voltage_level needs to increase
>due to a DDI port, but the CDCLK frequency is decreasing.
>The current approach will not bump the voltage level up until
>after the port has already been enabled, which is too late.
>But we'll take care of that case separately.
>
>Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>---
> drivers/gpu/drm/i915/display/intel_cdclk.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
>index 31aaa9780dfc..49e2f09a796a 100644
>--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
>+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
>@@ -2600,7 +2600,6 @@ intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state)
> intel_atomic_get_old_cdclk_state(state);
> const struct intel_cdclk_state *new_cdclk_state =
> intel_atomic_get_new_cdclk_state(state);
>- enum pipe pipe = new_cdclk_state->pipe;
>
> if (!intel_cdclk_changed(&old_cdclk_state->actual,
> &new_cdclk_state->actual))
>@@ -2609,11 +2608,11 @@ intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state)
> if (IS_DG2(i915))
> intel_cdclk_pcode_pre_notify(state);
>
>- if (pipe == INVALID_PIPE ||
>- old_cdclk_state->actual.cdclk <= new_cdclk_state->actual.cdclk) {
>+ if (old_cdclk_state->actual.cdclk <= new_cdclk_state->actual.cdclk) {
> drm_WARN_ON(&i915->drm, !new_cdclk_state->base.changed);
>
>- intel_set_cdclk(i915, &new_cdclk_state->actual, pipe);
>+ intel_set_cdclk(i915, &new_cdclk_state->actual,
>+ new_cdclk_state->pipe);
> }
> }
>
>@@ -2632,7 +2631,6 @@ intel_set_cdclk_post_plane_update(struct intel_atomic_state *state)
> intel_atomic_get_old_cdclk_state(state);
> const struct intel_cdclk_state *new_cdclk_state =
> intel_atomic_get_new_cdclk_state(state);
>- enum pipe pipe = new_cdclk_state->pipe;
>
> if (!intel_cdclk_changed(&old_cdclk_state->actual,
> &new_cdclk_state->actual))
>@@ -2641,11 +2639,11 @@ intel_set_cdclk_post_plane_update(struct intel_atomic_state *state)
> if (IS_DG2(i915))
> intel_cdclk_pcode_post_notify(state);
>
>- if (pipe != INVALID_PIPE &&
>- old_cdclk_state->actual.cdclk > new_cdclk_state->actual.cdclk) {
>+ if (old_cdclk_state->actual.cdclk > new_cdclk_state->actual.cdclk) {
> drm_WARN_ON(&i915->drm, !new_cdclk_state->base.changed);
>
>- intel_set_cdclk(i915, &new_cdclk_state->actual, pipe);
>+ intel_set_cdclk(i915, &new_cdclk_state->actual,
>+ new_cdclk_state->pipe);
> }
> }
>
>--
>2.43.2
>
