[Intel-gfx] [PATCH 1/4] drm/i915: Pass plane id to watermark calculation functions

[Intel-gfx] [PATCH 1/4] drm/i915: Pass plane id to watermark calculation functions

[Stanislav Lisovskiy]

Sometimes we might need to change the way we calculate
watermarks, based on which particular plane it is calculated
for. Thus it would be convenient to pass plane_id to those
functions.

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2ff260117476..c5c1b6bef9a2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4274,6 +4274,7 @@ static int skl_compute_wm_params(const struct intel_crtc_state *crtc_state,
 				 u32 plane_pixel_rate, struct skl_wm_params *wp,
 				 int color_plane);
 static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
+				 enum plane_id plane_id,
 				 int level,
 				 unsigned int latency,
 				 const struct skl_wm_params *wp,
@@ -4300,7 +4301,7 @@ skl_cursor_allocation(const struct intel_crtc_state *crtc_state,
 	for (level = 0; level <= max_level; level++) {
 		unsigned int latency = dev_priv->wm.skl_latency[level];
 
-		skl_compute_plane_wm(crtc_state, level, latency, &wp, &wm, &wm);
+		skl_compute_plane_wm(crtc_state, PLANE_CURSOR, level, latency, &wp, &wm, &wm);
 		if (wm.min_ddb_alloc == U16_MAX)
 			break;
 
@@ -5530,6 +5531,7 @@ static int skl_wm_max_lines(struct drm_i915_private *dev_priv)
 }
 
 static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
+				 enum plane_id plane_id,
 				 int level,
 				 unsigned int latency,
 				 const struct skl_wm_params *wp,
@@ -5657,6 +5659,7 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
 
 static void
 skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
+		      enum plane_id plane_id,
 		      const struct skl_wm_params *wm_params,
 		      struct skl_wm_level *levels)
 {
@@ -5668,7 +5671,7 @@ skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
 		struct skl_wm_level *result = &levels[level];
 		unsigned int latency = dev_priv->wm.skl_latency[level];
 
-		skl_compute_plane_wm(crtc_state, level, latency,
+		skl_compute_plane_wm(crtc_state, plane_id, level, latency,
 				     wm_params, result_prev, result);
 
 		result_prev = result;
@@ -5676,6 +5679,7 @@ skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
 }
 
 static void tgl_compute_sagv_wm(const struct intel_crtc_state *crtc_state,
+				enum plane_id plane_id,
 				const struct skl_wm_params *wm_params,
 				struct skl_plane_wm *plane_wm)
 {
@@ -5684,7 +5688,7 @@ static void tgl_compute_sagv_wm(const struct intel_crtc_state *crtc_state,
 	struct skl_wm_level *levels = plane_wm->wm;
 	unsigned int latency = dev_priv->wm.skl_latency[0] + dev_priv->sagv_block_time_us;
 
-	skl_compute_plane_wm(crtc_state, 0, latency,
+	skl_compute_plane_wm(crtc_state, plane_id, 0, latency,
 			     wm_params, &levels[0],
 			     sagv_wm);
 }
@@ -5767,13 +5771,13 @@ static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
 	if (ret)
 		return ret;
 
-	skl_compute_wm_levels(crtc_state, &wm_params, wm->wm);
+	skl_compute_wm_levels(crtc_state, plane_id, &wm_params, wm->wm);
 
 	skl_compute_transition_wm(dev_priv, &wm->trans_wm,
 				  &wm->wm[0], &wm_params);
 
 	if (DISPLAY_VER(dev_priv) >= 12) {
-		tgl_compute_sagv_wm(crtc_state, &wm_params, wm);
+		tgl_compute_sagv_wm(crtc_state, plane_id, &wm_params, wm);
 
 		skl_compute_transition_wm(dev_priv, &wm->sagv.trans_wm,
 					  &wm->sagv.wm0, &wm_params);
@@ -5798,7 +5802,7 @@ static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state,
 	if (ret)
 		return ret;
 
-	skl_compute_wm_levels(crtc_state, &wm_params, wm->uv_wm);
+	skl_compute_wm_levels(crtc_state, plane_id, &wm_params, wm->uv_wm);
 
 	return 0;
 }
-- 
2.24.1.485.gad05a3d8e5

[Intel-gfx] [PATCH 2/4] drm/i915: Introduce do_async_flip flag to intel_plane_state

[Stanislav Lisovskiy]

There might be various logical contructs when we might want
to enable async flip, so lets calculate those and set this
flag, so that there is no need in long conditions in other
places.

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 drivers/gpu/drm/i915/display/intel_atomic_plane.c  | 2 +-
 drivers/gpu/drm/i915/display/intel_display.c       | 3 +++
 drivers/gpu/drm/i915/display/intel_display_types.h | 3 +++
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index a5bfc047162b..9dceb952009c 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -502,7 +502,7 @@ void intel_update_plane(struct intel_plane *plane,
 
 	trace_intel_update_plane(&plane->base, crtc);
 
-	if (crtc_state->uapi.async_flip && plane->async_flip)
+	if (plane_state->do_async_flip)
 		plane->async_flip(plane, crtc_state, plane_state, true);
 	else
 		plane->update_plane(plane, crtc_state, plane_state);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a610087e6885..b8fe40050463 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5558,6 +5558,9 @@ int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_stat
 			 needs_scaling(new_plane_state))))
 		new_crtc_state->disable_lp_wm = true;
 
+	if (new_crtc_state->uapi.async_flip && plane->async_flip)
+		new_plane_state->do_async_flip = true;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 26744f8713ab..d60adcd75757 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -632,6 +632,9 @@ struct intel_plane_state {
 
 	struct intel_fb_view view;
 
+	/* Indicates if async flip is required */
+	bool do_async_flip;
+
 	/* Plane pxp decryption state */
 	bool decrypt;
 
-- 
2.24.1.485.gad05a3d8e5

[Intel-gfx] [PATCH 3/4] drm/i915: Use wm0 only during async flips for DG2

[Stanislav Lisovskiy]

This optimization allows to achieve higher perfomance
during async flips.
For the first async flip we have to still temporarily
switch to sync flip, in order to reprogram plane
watermarks, so this requires taking into account
old plane state's do_async_flip flag.

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 13 ++++++++++++-
 drivers/gpu/drm/i915/intel_pm.c              |  5 ++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index b8fe40050463..872bbb965992 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5439,6 +5439,16 @@ static bool needs_scaling(const struct intel_plane_state *state)
 	return (src_w != dst_w || src_h != dst_h);
 }
 
+static bool needs_async_flip_wm_override(struct intel_plane *plane,
+					 const struct intel_plane_state *new_plane_state,
+					 const struct intel_plane_state *old_plane_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+
+	return (DISPLAY_VER(dev_priv) >= 13) && !old_plane_state->do_async_flip &&
+	       new_plane_state->do_async_flip;
+}
+
 int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_state,
 				    struct intel_crtc_state *new_crtc_state,
 				    const struct intel_plane_state *old_plane_state,
@@ -5558,7 +5568,8 @@ int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_stat
 			 needs_scaling(new_plane_state))))
 		new_crtc_state->disable_lp_wm = true;
 
-	if (new_crtc_state->uapi.async_flip && plane->async_flip)
+	if (new_crtc_state->uapi.async_flip && new_plane_state->do_async_flip &&
+	    !needs_async_flip_wm_override(plane, new_plane_state, old_plane_state))
 		new_plane_state->do_async_flip = true;
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index c5c1b6bef9a2..0b45d1d61d0f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5542,8 +5542,11 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
 	uint_fixed_16_16_t method1, method2;
 	uint_fixed_16_16_t selected_result;
 	u32 blocks, lines, min_ddb_alloc = 0;
+	bool dg2_async_flip_optimization = (DISPLAY_VER(dev_priv) >= 13) &&
+					   crtc_state->uapi.async_flip &&
+					   plane_id != PLANE_CURSOR;
 
-	if (latency == 0) {
+	if (latency == 0 || (dg2_async_flip_optimization && (level > 0))) {
 		/* reject it */
 		result->min_ddb_alloc = U16_MAX;
 		return;
-- 
2.24.1.485.gad05a3d8e5

[Intel-gfx] [PATCH 4/4] drm/i915: Don't allocate extra ddb during async flip for DG2

[Stanislav Lisovskiy]

In terms of async flip optimization we don't to allocate
extra ddb space, so lets skip it.

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0b45d1d61d0f..e1594f43bb1b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5245,9 +5245,15 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
 			break;
 
 		rate = crtc_state->plane_data_rate[plane_id];
-		extra = min_t(u16, alloc_size,
-			      DIV64_U64_ROUND_UP(alloc_size * rate,
-						 total_data_rate));
+
+		if (IS_DG2(dev_priv) && crtc_state->uapi.async_flip) {
+			extra = 0;
+		} else {
+			extra = min_t(u16, alloc_size,
+				      DIV64_U64_ROUND_UP(alloc_size * rate,
+							 total_data_rate));
+		}
+
 		total[plane_id] = wm->wm[level].min_ddb_alloc + extra;
 		alloc_size -= extra;
 		total_data_rate -= rate;
@@ -5256,9 +5262,15 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
 			break;
 
 		rate = crtc_state->uv_plane_data_rate[plane_id];
-		extra = min_t(u16, alloc_size,
-			      DIV64_U64_ROUND_UP(alloc_size * rate,
-						 total_data_rate));
+
+		if (IS_DG2(dev_priv) && crtc_state->uapi.async_flip) {
+			extra = 0;
+		} else {
+			extra = min_t(u16, alloc_size,
+				      DIV64_U64_ROUND_UP(alloc_size * rate,
+							 total_data_rate));
+		}
+
 		uv_total[plane_id] = wm->uv_wm[level].min_ddb_alloc + extra;
 		alloc_size -= extra;
 		total_data_rate -= rate;
-- 
2.24.1.485.gad05a3d8e5

Re: [Intel-gfx] [PATCH 3/4] drm/i915: Use wm0 only during async flips for DG2

[Ville Syrjälä]

On Fri, Dec 03, 2021 at 11:40:40AM +0200, Stanislav Lisovskiy wrote:
> This optimization allows to achieve higher perfomance
> during async flips.
> For the first async flip we have to still temporarily
> switch to sync flip, in order to reprogram plane
> watermarks, so this requires taking into account
> old plane state's do_async_flip flag.
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 13 ++++++++++++-
>  drivers/gpu/drm/i915/intel_pm.c              |  5 ++++-
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index b8fe40050463..872bbb965992 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -5439,6 +5439,16 @@ static bool needs_scaling(const struct intel_plane_state *state)
>  	return (src_w != dst_w || src_h != dst_h);
>  }
>  
> +static bool needs_async_flip_wm_override(struct intel_plane *plane,
> +					 const struct intel_plane_state *new_plane_state,
> +					 const struct intel_plane_state *old_plane_state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> +
> +	return (DISPLAY_VER(dev_priv) >= 13) && !old_plane_state->do_async_flip &&
> +	       new_plane_state->do_async_flip;
> +}
> +
>  int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_state,
>  				    struct intel_crtc_state *new_crtc_state,
>  				    const struct intel_plane_state *old_plane_state,
> @@ -5558,7 +5568,8 @@ int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_stat
>  			 needs_scaling(new_plane_state))))
>  		new_crtc_state->disable_lp_wm = true;
>  
> -	if (new_crtc_state->uapi.async_flip && plane->async_flip)
> +	if (new_crtc_state->uapi.async_flip && new_plane_state->do_async_flip &&
> +	    !needs_async_flip_wm_override(plane, new_plane_state, old_plane_state))
>  		new_plane_state->do_async_flip = true;

That code looks super confused.

>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index c5c1b6bef9a2..0b45d1d61d0f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5542,8 +5542,11 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
>  	uint_fixed_16_16_t method1, method2;
>  	uint_fixed_16_16_t selected_result;
>  	u32 blocks, lines, min_ddb_alloc = 0;
> +	bool dg2_async_flip_optimization = (DISPLAY_VER(dev_priv) >= 13) &&
> +					   crtc_state->uapi.async_flip &&
> +					   plane_id != PLANE_CURSOR;

Function please. Hmm. And rather than checking the plane_id we should
probably just check plane->async_flip since we don't want to minimize
the watermarks for any other plane than the one doing the async flips.

>  
> -	if (latency == 0) {
> +	if (latency == 0 || (dg2_async_flip_optimization && (level > 0))) {
>  		/* reject it */
>  		result->min_ddb_alloc = U16_MAX;
>  		return;
> -- 
> 2.24.1.485.gad05a3d8e5

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Don't allocate extra ddb during async flip for DG2

[Ville Syrjälä]

On Fri, Dec 03, 2021 at 11:40:41AM +0200, Stanislav Lisovskiy wrote:
> In terms of async flip optimization we don't to allocate
> extra ddb space, so lets skip it.
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 0b45d1d61d0f..e1594f43bb1b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5245,9 +5245,15 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
>  			break;
>  
>  		rate = crtc_state->plane_data_rate[plane_id];
> -		extra = min_t(u16, alloc_size,
> -			      DIV64_U64_ROUND_UP(alloc_size * rate,
> -						 total_data_rate));
> +
> +		if (IS_DG2(dev_priv) && crtc_state->uapi.async_flip) {

We should have a sensible function for this.

> +			extra = 0;

Aren't we going to get the WARN(alloc_size != 0) if this
is the only enabled plane?

> +		} else {
> +			extra = min_t(u16, alloc_size,
> +				      DIV64_U64_ROUND_UP(alloc_size * rate,
> +							 total_data_rate));
> +		}
> +
>  		total[plane_id] = wm->wm[level].min_ddb_alloc + extra;
>  		alloc_size -= extra;
>  		total_data_rate -= rate;
> @@ -5256,9 +5262,15 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
>  			break;
>  
>  		rate = crtc_state->uv_plane_data_rate[plane_id];
> -		extra = min_t(u16, alloc_size,
> -			      DIV64_U64_ROUND_UP(alloc_size * rate,
> -						 total_data_rate));
> +
> +		if (IS_DG2(dev_priv) && crtc_state->uapi.async_flip) {
> +			extra = 0;
> +		} else {
> +			extra = min_t(u16, alloc_size,
> +				      DIV64_U64_ROUND_UP(alloc_size * rate,
> +							 total_data_rate));
> +		}
> +
>  		uv_total[plane_id] = wm->uv_wm[level].min_ddb_alloc + extra;
>  		alloc_size -= extra;
>  		total_data_rate -= rate;
> -- 
> 2.24.1.485.gad05a3d8e5

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 3/4] drm/i915: Use wm0 only during async flips for DG2

[Lisovskiy, Stanislav]

On Fri, Dec 03, 2021 at 12:00:42PM +0200, Ville Syrjälä wrote:
> On Fri, Dec 03, 2021 at 11:40:40AM +0200, Stanislav Lisovskiy wrote:
> > This optimization allows to achieve higher perfomance
> > during async flips.
> > For the first async flip we have to still temporarily
> > switch to sync flip, in order to reprogram plane
> > watermarks, so this requires taking into account
> > old plane state's do_async_flip flag.
> > 
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 13 ++++++++++++-
> >  drivers/gpu/drm/i915/intel_pm.c              |  5 ++++-
> >  2 files changed, 16 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index b8fe40050463..872bbb965992 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -5439,6 +5439,16 @@ static bool needs_scaling(const struct intel_plane_state *state)
> >  	return (src_w != dst_w || src_h != dst_h);
> >  }
> >  
> > +static bool needs_async_flip_wm_override(struct intel_plane *plane,
> > +					 const struct intel_plane_state *new_plane_state,
> > +					 const struct intel_plane_state *old_plane_state)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> > +
> > +	return (DISPLAY_VER(dev_priv) >= 13) && !old_plane_state->do_async_flip &&
> > +	       new_plane_state->do_async_flip;
> > +}
> > +
> >  int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_state,
> >  				    struct intel_crtc_state *new_crtc_state,
> >  				    const struct intel_plane_state *old_plane_state,
> > @@ -5558,7 +5568,8 @@ int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_stat
> >  			 needs_scaling(new_plane_state))))
> >  		new_crtc_state->disable_lp_wm = true;
> >  
> > -	if (new_crtc_state->uapi.async_flip && plane->async_flip)
> > +	if (new_crtc_state->uapi.async_flip && new_plane_state->do_async_flip &&
> > +	    !needs_async_flip_wm_override(plane, new_plane_state, old_plane_state))
> >  		new_plane_state->do_async_flip = true;
> 
> That code looks super confused.

Oh.. thanks for spotting new_plane_state->do_async_flip is redundant here.
The logic is such that if old_plane_state didn't have do_async_flip flag set,
but the new_plane_state has, it means we do first async flip here, thus
it requires temporary switching to sync flip as we discussed.

So I actually meant something like this here:

> > +   if (new_crtc_state->uapi.async_flip && !needs_async_flip_wm_override(plane, 
						new_plane_state, old_plane_state))


> 
> >  
> >  	return 0;
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index c5c1b6bef9a2..0b45d1d61d0f 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -5542,8 +5542,11 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
> >  	uint_fixed_16_16_t method1, method2;
> >  	uint_fixed_16_16_t selected_result;
> >  	u32 blocks, lines, min_ddb_alloc = 0;
> > +	bool dg2_async_flip_optimization = (DISPLAY_VER(dev_priv) >= 13) &&
> > +					   crtc_state->uapi.async_flip &&
> > +					   plane_id != PLANE_CURSOR;
> 
> Function please. Hmm. And rather than checking the plane_id we should
> probably just check plane->async_flip since we don't want to minimize
> the watermarks for any other plane than the one doing the async flips.

Function sounds like good idea.

Stan

> 
> >  
> > -	if (latency == 0) {
> > +	if (latency == 0 || (dg2_async_flip_optimization && (level > 0))) {
> >  		/* reject it */
> >  		result->min_ddb_alloc = U16_MAX;
> >  		return;
> > -- 
> > 2.24.1.485.gad05a3d8e5
> 
> -- 
> Ville Syrjälä
> Intel

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Don't allocate extra ddb during async flip for DG2

[Lisovskiy, Stanislav]

On Fri, Dec 03, 2021 at 12:03:41PM +0200, Ville Syrjälä wrote:
> On Fri, Dec 03, 2021 at 11:40:41AM +0200, Stanislav Lisovskiy wrote:
> > In terms of async flip optimization we don't to allocate
> > extra ddb space, so lets skip it.
> > 
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 24 ++++++++++++++++++------
> >  1 file changed, 18 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 0b45d1d61d0f..e1594f43bb1b 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -5245,9 +5245,15 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
> >  			break;
> >  
> >  		rate = crtc_state->plane_data_rate[plane_id];
> > -		extra = min_t(u16, alloc_size,
> > -			      DIV64_U64_ROUND_UP(alloc_size * rate,
> > -						 total_data_rate));
> > +
> > +		if (IS_DG2(dev_priv) && crtc_state->uapi.async_flip) {
> 
> We should have a sensible function for this.

Agree, tbh was thinking about this as well.

> 
> > +			extra = 0;
> 
> Aren't we going to get the WARN(alloc_size != 0) if this
> is the only enabled plane?

Actually true as well, I remember I did get this warn when
I was testing it. 
I guess we shouldn't trigger WARN for that case then?

> 
> > +		} else {
> > +			extra = min_t(u16, alloc_size,
> > +				      DIV64_U64_ROUND_UP(alloc_size * rate,
> > +							 total_data_rate));
> > +		}
> > +
> >  		total[plane_id] = wm->wm[level].min_ddb_alloc + extra;
> >  		alloc_size -= extra;
> >  		total_data_rate -= rate;
> > @@ -5256,9 +5262,15 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
> >  			break;
> >  
> >  		rate = crtc_state->uv_plane_data_rate[plane_id];
> > -		extra = min_t(u16, alloc_size,
> > -			      DIV64_U64_ROUND_UP(alloc_size * rate,
> > -						 total_data_rate));
> > +
> > +		if (IS_DG2(dev_priv) && crtc_state->uapi.async_flip) {
> > +			extra = 0;
> > +		} else {
> > +			extra = min_t(u16, alloc_size,
> > +				      DIV64_U64_ROUND_UP(alloc_size * rate,
> > +							 total_data_rate));
> > +		}
> > +
> >  		uv_total[plane_id] = wm->uv_wm[level].min_ddb_alloc + extra;
> >  		alloc_size -= extra;
> >  		total_data_rate -= rate;
> > -- 
> > 2.24.1.485.gad05a3d8e5
> 
> -- 
> Ville Syrjälä
> Intel

[Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with [1/4] drm/i915: Pass plane id to watermark calculation functions

[Patchwork]

== Series Details ==

Series: series starting with [1/4] drm/i915: Pass plane id to watermark calculation functions
URL   : https://patchwork.freedesktop.org/series/97536/
State : failure

== Summary ==

Applying: drm/i915: Pass plane id to watermark calculation functions
Applying: drm/i915: Introduce do_async_flip flag to intel_plane_state
error: sha1 information is lacking or useless (drivers/gpu/drm/i915/display/intel_atomic_plane.c).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 drm/i915: Introduce do_async_flip flag to intel_plane_state
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".