Re: [Intel-gfx] [PATCH 3/3] drm/i915: Do plane/etc. updates more atomically across pipes

["Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com>]

On Thu, Sep 07, 2023 at 03:25:41PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Perform all the intel_pre_update_crtc() stuff for all pipes first,
> and only then do the intel_update_crtc() vblank evasion stuff for
> every pipe back to back. This should make it more likely that
> the plane updates from multiple pipes happen on the same frame
> (assuming the pipes are running in sync, eg. due to bigjoiner
> or port sync).

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 26 ++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 7c19a0f380ca..f96230232a47 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6711,6 +6711,12 @@ static void intel_commit_modeset_enables(struct intel_atomic_state *state)
>  
>  		intel_enable_crtc(state, crtc);
>  		intel_pre_update_crtc(state, crtc);
> +	}
> +
> +	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
> +		if (!new_crtc_state->hw.active)
> +			continue;
> +
>  		intel_update_crtc(state, crtc);
>  	}
>  }
> @@ -6748,6 +6754,15 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
>  	 * So first lets enable all pipes that do not need a fullmodeset as
>  	 * those don't have any external dependency.
>  	 */
> +	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
> +		enum pipe pipe = crtc->pipe;
> +
> +		if ((update_pipes & BIT(pipe)) == 0)
> +			continue;
> +
> +		intel_pre_update_crtc(state, crtc);
> +	}
> +
>  	while (update_pipes) {
>  		for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>  						    new_crtc_state, i) {
> @@ -6763,7 +6778,6 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
>  			entries[pipe] = new_crtc_state->wm.skl.ddb;
>  			update_pipes &= ~BIT(pipe);
>  
> -			intel_pre_update_crtc(state, crtc);
>  			intel_update_crtc(state, crtc);
>  
>  			/*
> @@ -6819,6 +6833,15 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
>  	/*
>  	 * Finally we do the plane updates/etc. for all pipes that got enabled.
>  	 */
> +	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
> +		enum pipe pipe = crtc->pipe;
> +
> +		if ((update_pipes & BIT(pipe)) == 0)
> +			continue;
> +
> +		intel_pre_update_crtc(state, crtc);
> +	}
> +
>  	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
>  		enum pipe pipe = crtc->pipe;
>  
> @@ -6831,7 +6854,6 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
>  		entries[pipe] = new_crtc_state->wm.skl.ddb;
>  		update_pipes &= ~BIT(pipe);
>  
> -		intel_pre_update_crtc(state, crtc);
>  		intel_update_crtc(state, crtc);
>  	}
>  
> -- 
> 2.41.0
>