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

[Lisovskiy, Stanislav]

On Thu, Sep 07, 2023 at 03:25:41PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> 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 

> 
> Signed-off-by: Ville Syrjälä 
> ---
>  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
> 

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

[Ville Syrjala]

From: Ville Syrjälä 

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).

Signed-off-by: Ville Syrjälä 
---
 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