On Fri, 2019-10-11 at 23:09 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> skl_commit_modeset_enables() is a bit of mess. Let's streamline
> it by simply tracking which pipes still need to be updated.
> As a bonus we get rid of the state->wm_results.dirty_pipes usage.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 39 +++++++++---------
> --
> 1 file changed, 17 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index b9b51bd0c956..9eab67bbf61d 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -13881,17 +13881,19 @@ static void
> skl_commit_modeset_enables(struct intel_atomic_state *state)
> struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> struct intel_crtc *crtc;
> struct intel_crtc_state *old_crtc_state, *new_crtc_state;
> - unsigned int updated = 0;
> - bool progress;
> - int i;
> u8 hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices;
> u8 required_slices = state->wm_results.ddb.enabled_slices;
> struct skl_ddb_entry entries[I915_MAX_PIPES] = {};
> + u8 dirty_pipes = 0;
> + int i;
>
> - for_each_oldnew_intel_crtc_in_state(state, crtc,
> old_crtc_state, new_crtc_state, i)
Really "love" this medieval C standard thing for that nice "i" variable
:)
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>
> + for_each_oldnew_intel_crtc_in_state(state, crtc,
> old_crtc_state, new_crtc_state, i) {
> /* ignore allocations for crtc's that have been turned
> off. */
> - if (new_crtc_state->base.active)
> + if (new_crtc_state->base.active) {
> entries[i] = old_crtc_state->wm.skl.ddb;
> + dirty_pipes |= BIT(crtc->pipe);
> + }
> + }
>
> /* If 2nd DBuf slice required, enable it here */
> if (INTEL_GEN(dev_priv) >= 11 && required_slices >
> hw_enabled_slices)
> @@ -13903,14 +13905,12 @@ static void
> skl_commit_modeset_enables(struct intel_atomic_state *state)
> * never overlap with eachother inbetween CRTC updates.
> Otherwise we'll
> * cause pipe underruns and other bad stuff.
> */
> - do {
> - progress = false;
> -
> - for_each_oldnew_intel_crtc_in_state(state, crtc,
> old_crtc_state, new_crtc_state, i) {
> + while (dirty_pipes) {
> + for_each_oldnew_intel_crtc_in_state(state, crtc,
> old_crtc_state,
> + new_crtc_state, i)
> {
> enum pipe pipe = crtc->pipe;
> - bool vbl_wait = false;
>
> - if (updated & BIT(crtc->pipe) ||
> !new_crtc_state->base.active)
> + if ((dirty_pipes & BIT(pipe)) == 0)
> continue;
>
> if
> (skl_ddb_allocation_overlaps(&new_crtc_state->wm.skl.ddb,
> @@ -13918,8 +13918,11 @@ static void
> skl_commit_modeset_enables(struct intel_atomic_state *state)
> INTEL_NUM_PIPES
> (dev_priv), i))
> continue;
>
> - updated |= BIT(pipe);
> entries[i] = new_crtc_state->wm.skl.ddb;
> + dirty_pipes &= ~BIT(pipe);
> +
> + intel_update_crtc(crtc, state, old_crtc_state,
> + new_crtc_state);
>
> /*
> * If this is an already active pipe, it's DDB
> changed,
> @@ -13930,18 +13933,10 @@ static void
> skl_commit_modeset_enables(struct intel_atomic_state *state)
> if (!skl_ddb_entry_equal(&new_crtc_state-
> >wm.skl.ddb,
> &old_crtc_state-
> >wm.skl.ddb) &&
> !new_crtc_state->base.active_changed &&
> - state->wm_results.dirty_pipes != updated)
> - vbl_wait = true;
> -
> - intel_update_crtc(crtc, state, old_crtc_state,
> - new_crtc_state);
> -
> - if (vbl_wait)
> + dirty_pipes)
> intel_wait_for_vblank(dev_priv, pipe);
> -
> - progress = true;
> }
> - } while (progress);
> + }
>
> /* If 2nd DBuf slice is no more required disable it */
> if (INTEL_GEN(dev_priv) >= 11 && required_slices <
> hw_enabled_slices)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
