[PATCH 0/3] New DDB allocation algorithm

[PATCH 0/3] New DDB allocation algorithm

[Matt Roper]

Our current algorithm for partitioning a pipe's DDB allocation bases
plane allocations exclusively on the proportion of the pipe's data rate
that the plane contributes.  This is a simple heuristic that works
pretty well in a lot of cases, but it breaks down when there's a large
difference in plane sizes (e.g., one full-screen plane and another
100x100 plane).  When sizes are very different, using proportional data
rate can leave the smaller planes without enough DDB allocation to even
hit their level 0 watermarks (which means the whole configuration
becomes impossible), or can artificially limit how high of a watermark
level the smaller planes can achieve.  These problems are most visible
on APL (due to the platform's smaller DDB in general).

A better algorithm is to calculate watermarks first, DDB second, and use
the block counts from the watermark calculation to give planes exactly
the allocation they need to hit the highest possible global watermark
level.  Mahesh took a stab at implementing this algorithm about 1.5
years ago, but there were some bugs that led to regressions and nobody
had time to track them down.  Our watermark code has evolved
significantly since that time, but the problems with the original
datarate-proportional algorithm remain.

Matt Roper (3):
  drm/i915: Remove a very stale FIXME
  drm/i915: Don't use DDB allocation when choosing gen9 watermark method
  drm/i915: Switch to level-based DDB allocation algorithm

 drivers/gpu/drm/i915/intel_pm.c | 356 ++++++++++++++--------------------------
 1 file changed, 121 insertions(+), 235 deletions(-)

-- 
2.14.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/3] drm/i915: Remove a very stale FIXME

[Matt Roper]

SKL watermark calculations can and do trigger atomic transaction
rejection if no valid set of watermarks can be found.  This FIXME
comment in the code hasn't been relevant for a very long time.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a26b4eddda25..9500bda64f26 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5548,10 +5548,6 @@ skl_compute_wm(struct drm_atomic_state *state)
 	 * Note that the DDB allocation above may have added more CRTC's that
 	 * weren't otherwise being modified (and set bits in dirty_pipes) if
 	 * pipe allocations had to change.
-	 *
-	 * FIXME:  Now that we're doing this in the atomic check phase, we
-	 * should allow skl_update_pipe_wm() to return failure in cases where
-	 * no suitable watermark values can be found.
 	 */
 	for_each_new_crtc_in_state(state, crtc, cstate, i) {
 		struct intel_crtc_state *intel_cstate =
-- 
2.14.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Remove a very stale FIXME

[Ville Syrjälä]

On Thu, Dec 06, 2018 at 09:09:42AM -0800, Matt Roper wrote:
> SKL watermark calculations can and do trigger atomic transaction
> rejection if no valid set of watermarks can be found.  This FIXME
> comment in the code hasn't been relevant for a very long time.

Identical patch already pushed.

> 
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a26b4eddda25..9500bda64f26 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5548,10 +5548,6 @@ skl_compute_wm(struct drm_atomic_state *state)
>  	 * Note that the DDB allocation above may have added more CRTC's that
>  	 * weren't otherwise being modified (and set bits in dirty_pipes) if
>  	 * pipe allocations had to change.
> -	 *
> -	 * FIXME:  Now that we're doing this in the atomic check phase, we
> -	 * should allow skl_update_pipe_wm() to return failure in cases where
> -	 * no suitable watermark values can be found.
>  	 */
>  	for_each_new_crtc_in_state(state, crtc, cstate, i) {
>  		struct intel_crtc_state *intel_cstate =
> -- 
> 2.14.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915: Don't use DDB allocation when choosing gen9 watermark method

[Matt Roper]

The bspec gives an if/else chain for choosing whether to use "method 1"
or "method 2" for calculating the watermark "Selected Result Blocks"
value for a plane.  One of the branches of the if chain is:

        "Else If ('plane buffer allocation' is known and (plane buffer
        allocation / plane blocks per line) >=1)"

Since our driver currently calculates DDB allocations first and the
actual watermark values second, the plane buffer allocation is known at
this point in our code and we include this test in our driver's logic.
However we plan to soon move to a "watermarks first, ddb allocation
second" sequence where we won't know the DDB allocation at this point.
Let's drop this arm of the if/else statement (effectively considering
the DDB allocation unknown) as an independent patch so that any
regressions can be more accurately bisected to either the different
watermark value (in this patch) or the new DDB allocation (in the next
patch).

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9500bda64f26..b09c2a257ff1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4766,13 +4766,6 @@ static int skl_compute_plane_wm(const struct intel_crtc_state *cstate,
 		     wp->dbuf_block_size < 1) &&
 		     (wp->plane_bytes_per_line / wp->dbuf_block_size < 1)) {
 			selected_result = method2;
-		} else if (ddb_allocation >=
-			 fixed16_to_u32_round_up(wp->plane_blocks_per_line)) {
-			if (IS_GEN9(dev_priv) &&
-			    !IS_GEMINILAKE(dev_priv))
-				selected_result = min_fixed16(method1, method2);
-			else
-				selected_result = method2;
 		} else if (latency >= wp->linetime_us) {
 			if (IS_GEN9(dev_priv) &&
 			    !IS_GEMINILAKE(dev_priv))
-- 
2.14.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915: Switch to level-based DDB allocation algorithm

[Matt Roper]

The DDB allocation algorithm currently used by the driver grants each
plane a very small minimum allocation of DDB blocks and then divies up
all of the remaining blocks based on the percentage of the total data
rate that the plane makes up.  It turns out that this proportional
allocation approach is overly-generous with the larger planes and can
leave very small planes wthout a big enough allocation to even hit their
level 0 watermark requirements (especially on APL, which has a smaller
DDB in general than other gen9 platforms).  Or there can be situations
where the smallest planes hit a lower watermark level than they should
have been able to hit with a more equitable division of DDB blocks, thus
limiting the overall system sleep state that can be achieved.

The bspec now describes an alternate algorithm that can be used to
overcome these types of issues.  With the new algorithm, we calculate
all plane watermark values for all wm levels first, then go back and
partition a pipe's DDB space second.  The DDB allocation will calculate
what the highest watermark level that can be achieved on *all* active
planes, and then grant the blocks necessary to hit that level to each
plane.  Any remaining blocks are then divided up proportionally
according to data rate, similar to the old algorithm.

There was a previous attempt to implement this algorithm a couple years
ago in bb9d85f6e9d ("drm/i915/skl: New ddb allocation algorithm"), but
some regressions were reported, the patch was reverted, and nobody
ever got around to figuring out exactly where the bug was in that
version.  Our watermark code has evolved significantly in the meantime,
but we're still getting bug reports caused by the unfair proportional
algorithm, so let's give this another shot.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105458
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 345 ++++++++++++++--------------------------
 1 file changed, 121 insertions(+), 224 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b09c2a257ff1..6c60a668c383 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4306,102 +4306,6 @@ icl_get_total_relative_data_rate(struct intel_crtc_state *intel_cstate,
 	return total_data_rate;
 }
 
-static uint16_t
-skl_ddb_min_alloc(const struct drm_plane_state *pstate, const int plane)
-{
-	struct drm_framebuffer *fb = pstate->fb;
-	struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
-	uint32_t src_w, src_h;
-	uint32_t min_scanlines = 8;
-	uint8_t plane_bpp;
-
-	if (WARN_ON(!fb))
-		return 0;
-
-	/* For packed formats, and uv-plane, return 0 */
-	if (plane == 1 && fb->format->format != DRM_FORMAT_NV12)
-		return 0;
-
-	/* For Non Y-tile return 8-blocks */
-	if (fb->modifier != I915_FORMAT_MOD_Y_TILED &&
-	    fb->modifier != I915_FORMAT_MOD_Yf_TILED &&
-	    fb->modifier != I915_FORMAT_MOD_Y_TILED_CCS &&
-	    fb->modifier != I915_FORMAT_MOD_Yf_TILED_CCS)
-		return 8;
-
-	/*
-	 * Src coordinates are already rotated by 270 degrees for
-	 * the 90/270 degree plane rotation cases (to match the
-	 * GTT mapping), hence no need to account for rotation here.
-	 */
-	src_w = drm_rect_width(&intel_pstate->base.src) >> 16;
-	src_h = drm_rect_height(&intel_pstate->base.src) >> 16;
-
-	/* Halve UV plane width and height for NV12 */
-	if (plane == 1) {
-		src_w /= 2;
-		src_h /= 2;
-	}
-
-	plane_bpp = fb->format->cpp[plane];
-
-	if (drm_rotation_90_or_270(pstate->rotation)) {
-		switch (plane_bpp) {
-		case 1:
-			min_scanlines = 32;
-			break;
-		case 2:
-			min_scanlines = 16;
-			break;
-		case 4:
-			min_scanlines = 8;
-			break;
-		case 8:
-			min_scanlines = 4;
-			break;
-		default:
-			WARN(1, "Unsupported pixel depth %u for rotation",
-			     plane_bpp);
-			min_scanlines = 32;
-		}
-	}
-
-	return DIV_ROUND_UP((4 * src_w * plane_bpp), 512) * min_scanlines/4 + 3;
-}
-
-static void
-skl_ddb_calc_min(const struct intel_crtc_state *cstate, int num_active,
-		 uint16_t *minimum, uint16_t *uv_minimum)
-{
-	const struct drm_plane_state *pstate;
-	struct drm_plane *plane;
-
-	drm_atomic_crtc_state_for_each_plane_state(plane, pstate, &cstate->base) {
-		enum plane_id plane_id = to_intel_plane(plane)->id;
-		struct intel_plane_state *plane_state = to_intel_plane_state(pstate);
-
-		if (plane_id == PLANE_CURSOR)
-			continue;
-
-		/* slave plane must be invisible and calculated from master */
-		if (!pstate->visible || WARN_ON(plane_state->slave))
-			continue;
-
-		if (!plane_state->linked_plane) {
-			minimum[plane_id] = skl_ddb_min_alloc(pstate, 0);
-			uv_minimum[plane_id] = skl_ddb_min_alloc(pstate, 1);
-		} else {
-			enum plane_id y_plane_id =
-				plane_state->linked_plane->id;
-
-			minimum[y_plane_id] = skl_ddb_min_alloc(pstate, 0);
-			minimum[plane_id] = skl_ddb_min_alloc(pstate, 1);
-		}
-	}
-
-	minimum[PLANE_CURSOR] = skl_cursor_allocation(num_active);
-}
-
 static int
 skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 		      struct skl_ddb_allocation *ddb /* out */)
@@ -4411,15 +4315,18 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct skl_ddb_entry *alloc = &cstate->wm.skl.ddb;
-	uint16_t alloc_size, start;
-	uint16_t minimum[I915_MAX_PLANES] = {};
-	uint16_t uv_minimum[I915_MAX_PLANES] = {};
+	struct skl_ddb_entry *plane_alloc, *uv_plane_alloc;
+	struct skl_plane_wm *wm;
+	uint16_t alloc_size, start = 0;
+	uint16_t total[I915_MAX_PLANES] = {};
+	uint16_t uv_total[I915_MAX_PLANES] = {};
 	u64 total_data_rate;
 	enum plane_id plane_id;
 	int num_active;
 	u64 plane_data_rate[I915_MAX_PLANES] = {};
 	u64 uv_plane_data_rate[I915_MAX_PLANES] = {};
-	uint16_t total_min_blocks = 0;
+	uint16_t blocks = 0;
+	int level;
 
 	/* Clear the partitioning for disabled planes. */
 	memset(cstate->wm.skl.plane_ddb_y, 0, sizeof(cstate->wm.skl.plane_ddb_y));
@@ -4449,81 +4356,100 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 	if (alloc_size == 0)
 		return 0;
 
-	skl_ddb_calc_min(cstate, num_active, minimum, uv_minimum);
+	/* Allocate fixed number of blocks for cursor. */
+	total[PLANE_CURSOR] = skl_cursor_allocation(num_active);
+	alloc_size -= total[PLANE_CURSOR];
 
 	/*
-	 * 1. Allocate the mininum required blocks for each active plane
-	 * and allocate the cursor, it doesn't require extra allocation
-	 * proportional to the data rate.
+	 * Find the highest watermark level for which we can satisfy the block
+	 * requirement of active planes.
 	 */
+	for (level = ilk_wm_max_level(dev_priv) - 1; level >= 0; level--) {
+		for_each_plane_id_on_crtc(intel_crtc, plane_id) {
+			wm = &cstate->wm.skl.optimal.planes[plane_id];
+			blocks += wm->wm[level].plane_res_b;
+			blocks += wm->uv_wm[level].plane_res_b;
+		}
 
-	for_each_plane_id_on_crtc(intel_crtc, plane_id) {
-		total_min_blocks += minimum[plane_id];
-		total_min_blocks += uv_minimum[plane_id];
+		if (blocks < alloc_size) {
+			alloc_size -= blocks;
+			break;
+		}
 	}
 
-	if (total_min_blocks > alloc_size) {
+	if (level < 0) {
 		DRM_DEBUG_KMS("Requested display configuration exceeds system DDB limitations");
-		DRM_DEBUG_KMS("minimum required %d/%d\n", total_min_blocks,
-							alloc_size);
+		DRM_DEBUG_KMS("minimum required %d/%d\n", blocks,
+			      alloc_size);
 		return -EINVAL;
 	}
 
-	alloc_size -= total_min_blocks;
-	cstate->wm.skl.plane_ddb_y[PLANE_CURSOR].start = alloc->end - minimum[PLANE_CURSOR];
-	cstate->wm.skl.plane_ddb_y[PLANE_CURSOR].end = alloc->end;
-
 	/*
-	 * 2. Distribute the remaining space in proportion to the amount of
-	 * data each plane needs to fetch from memory.
-	 *
-	 * FIXME: we may not allocate every single block here.
+	 * If any non-cursor planes are turned on (i.e., total_data_rate != 0),
+	 * grant each plane the blocks it requires at the highest achievable
+	 * watermark level, plus an extra share of the leftover blocks
+	 * proportional to its relative data rate.
 	 */
-	if (total_data_rate == 0)
-		return 0;
+	if (total_data_rate) {
+		for_each_plane_id_on_crtc(intel_crtc, plane_id) {
+			u64 rate;
 
-	start = alloc->start;
-	for_each_plane_id_on_crtc(intel_crtc, plane_id) {
-		u64 data_rate, uv_data_rate;
-		uint16_t plane_blocks, uv_plane_blocks;
-
-		if (plane_id == PLANE_CURSOR)
-			continue;
+			wm = &cstate->wm.skl.optimal.planes[plane_id];
 
-		data_rate = plane_data_rate[plane_id];
+			rate = plane_data_rate[plane_id];
+			total[plane_id] = wm->wm[level].plane_res_b +
+				div64_u64(alloc_size * rate, total_data_rate);
 
-		/*
-		 * allocation for (packed formats) or (uv-plane part of planar format):
-		 * promote the expression to 64 bits to avoid overflowing, the
-		 * result is < available as data_rate / total_data_rate < 1
-		 */
-		plane_blocks = minimum[plane_id];
-		plane_blocks += div64_u64(alloc_size * data_rate, total_data_rate);
-
-		/* Leave disabled planes at (0,0) */
-		if (data_rate) {
-			cstate->wm.skl.plane_ddb_y[plane_id].start = start;
-			cstate->wm.skl.plane_ddb_y[plane_id].end = start + plane_blocks;
+			rate = uv_plane_data_rate[plane_id];
+			uv_total[plane_id] = wm->uv_wm[level].plane_res_b +
+				div64_u64(alloc_size * rate, total_data_rate);
 		}
+	}
 
-		start += plane_blocks;
+	/* Set the actual DDB start/end points for each plane */
+	start = alloc->start;
+	for_each_plane_id_on_crtc(intel_crtc, plane_id) {
+		plane_alloc = &cstate->wm.skl.plane_ddb_y[plane_id];
+		uv_plane_alloc = &cstate->wm.skl.plane_ddb_uv[plane_id];
 
-		/* Allocate DDB for UV plane for planar format/NV12 */
-		uv_data_rate = uv_plane_data_rate[plane_id];
+		/* Gen11+ uses a separate plane for UV watermarks */
+		WARN_ON(INTEL_GEN(dev_priv) >= 11 && uv_total[plane_id]);
 
-		uv_plane_blocks = uv_minimum[plane_id];
-		uv_plane_blocks += div64_u64(alloc_size * uv_data_rate, total_data_rate);
+		/* Leave disabled planes at (0,0) */
+		if (total[plane_id]) {
+			plane_alloc->start = start;
+			plane_alloc->end = start += total[plane_id];
+		}
 
-		/* Gen11+ uses a separate plane for UV watermarks */
-		WARN_ON(INTEL_GEN(dev_priv) >= 11 && uv_plane_blocks);
+		if (uv_total[plane_id]) {
+			uv_plane_alloc->start = start;
+			uv_plane_alloc->end = start + uv_total[plane_id];
+		}
+	}
 
-		if (uv_data_rate) {
-			cstate->wm.skl.plane_ddb_uv[plane_id].start = start;
-			cstate->wm.skl.plane_ddb_uv[plane_id].end =
-				start + uv_plane_blocks;
+	/*
+	 * When we calculated watermark values we didn't know how high
+	 * of a level we'd actually be able to hit, so we just marked
+	 * all levels as "enabled."  Go back now and disable the ones
+	 * that aren't actually possible.
+	 */
+	for ( ; level < ilk_wm_max_level(dev_priv); level++) {
+		for_each_plane_id_on_crtc(intel_crtc, plane_id) {
+			wm = &cstate->wm.skl.optimal.planes[plane_id];
+			wm->wm[level].plane_en = false;
+			wm->wm[level].plane_res_b = 0;
+			wm->wm[level].plane_res_l = 0;
 		}
+	}
 
-		start += uv_plane_blocks;
+	/*
+	 * Go back and disable the transition watermark if it turns out we
+	 * don't have enough DDB blocks for it.
+	 */
+	for_each_plane_id_on_crtc(intel_crtc, plane_id) {
+		wm = &cstate->wm.skl.optimal.planes[plane_id];
+		if (wm->trans_wm.plane_res_b > total[plane_id])
+			wm->trans_wm.plane_en = false;
 	}
 
 	return 0;
@@ -4720,17 +4646,15 @@ skl_compute_plane_wm_params(const struct intel_crtc_state *cstate,
 	return 0;
 }
 
-static int skl_compute_plane_wm(const struct intel_crtc_state *cstate,
-				const struct intel_plane_state *intel_pstate,
-				uint16_t ddb_allocation,
-				int level,
-				const struct skl_wm_params *wp,
-				const struct skl_wm_level *result_prev,
-				struct skl_wm_level *result /* out */)
+static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
+				 const struct intel_plane_state *intel_pstate,
+				 int level,
+				 const struct skl_wm_params *wp,
+				 const struct skl_wm_level *result_prev,
+				 struct skl_wm_level *result /* out */)
 {
 	struct drm_i915_private *dev_priv =
 		to_i915(intel_pstate->base.plane->dev);
-	const struct drm_plane_state *pstate = &intel_pstate->base;
 	uint32_t latency = dev_priv->wm.skl_latency[level];
 	uint_fixed_16_16_t method1, method2;
 	uint_fixed_16_16_t selected_result;
@@ -4740,9 +4664,6 @@ static int skl_compute_plane_wm(const struct intel_crtc_state *cstate,
 	bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state);
 	uint32_t min_disp_buf_needed;
 
-	if (latency == 0)
-		return level == 0 ? -EINVAL : 0;
-
 	/* Display WA #1141: kbl,cfl */
 	if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
 	    IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_B0)) &&
@@ -4828,38 +4749,24 @@ static int skl_compute_plane_wm(const struct intel_crtc_state *cstate,
 		min_disp_buf_needed = res_blocks;
 	}
 
-	if ((level > 0 && res_lines > 31) ||
-	    res_blocks >= ddb_allocation ||
-	    min_disp_buf_needed >= ddb_allocation) {
-		/*
-		 * If there are no valid level 0 watermarks, then we can't
-		 * support this display configuration.
-		 */
-		if (level) {
-			return 0;
-		} else {
-			struct drm_plane *plane = pstate->plane;
-
-			DRM_DEBUG_KMS("Requested display configuration exceeds system watermark limitations\n");
-			DRM_DEBUG_KMS("[PLANE:%d:%s] blocks required = %u/%u, lines required = %u/31\n",
-				      plane->base.id, plane->name,
-				      res_blocks, ddb_allocation, res_lines);
-			return -EINVAL;
-		}
-	}
-
 	/* The number of lines are ignored for the level 0 watermark. */
+	if (level > 0 && res_lines > 31)
+		return;
+
+	/*
+	 * If res_lines is valid, assume we can use this watermark level
+	 * for now.  We'll come back and disable it after we calculate the
+	 * DDB allocation if it turns out we don't actually have enough
+	 * blocks to satisfy it.
+	 */
 	result->plane_res_b = res_blocks;
 	result->plane_res_l = res_lines;
 	result->plane_en = true;
-
-	return 0;
 }
 
-static int
+static void
 skl_compute_wm_levels(const struct intel_crtc_state *cstate,
 		      const struct intel_plane_state *intel_pstate,
-		      uint16_t ddb_blocks,
 		      const struct skl_wm_params *wm_params,
 		      struct skl_wm_level *levels)
 {
@@ -4867,25 +4774,15 @@ skl_compute_wm_levels(const struct intel_crtc_state *cstate,
 		to_i915(intel_pstate->base.plane->dev);
 	int level, max_level = ilk_wm_max_level(dev_priv);
 	struct skl_wm_level *result_prev = &levels[0];
-	int ret;
 
 	for (level = 0; level <= max_level; level++) {
 		struct skl_wm_level *result = &levels[level];
 
-		ret = skl_compute_plane_wm(cstate,
-					   intel_pstate,
-					   ddb_blocks,
-					   level,
-					   wm_params,
-					   result_prev,
-					   result);
-		if (ret)
-			return ret;
+		skl_compute_plane_wm(cstate, intel_pstate, level, wm_params,
+				     result_prev, result);
 
 		result_prev = result;
 	}
-
-	return 0;
 }
 
 static uint32_t
@@ -4913,8 +4810,7 @@ skl_compute_linetime_wm(const struct intel_crtc_state *cstate)
 
 static void skl_compute_transition_wm(const struct intel_crtc_state *cstate,
 				      const struct skl_wm_params *wp,
-				      struct skl_plane_wm *wm,
-				      uint16_t ddb_allocation)
+				      struct skl_plane_wm *wm)
 {
 	struct drm_device *dev = cstate->base.crtc->dev;
 	const struct drm_i915_private *dev_priv = to_i915(dev);
@@ -4962,34 +4858,38 @@ static void skl_compute_transition_wm(const struct intel_crtc_state *cstate,
 
 	}
 
-	res_blocks += 1;
-
-	if (res_blocks < ddb_allocation) {
-		wm->trans_wm.plane_res_b = res_blocks;
-		wm->trans_wm.plane_en = true;
-	}
+	/*
+	 * Just assume we can enable the transition watermark.  After
+	 * computing the DDB we'll come back and disable it if that
+	 * assumption turns out to be false.
+	 */
+	wm->trans_wm.plane_res_b = res_blocks + 1;
+	wm->trans_wm.plane_en = true;
 }
 
 static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
 				     const struct intel_plane_state *plane_state,
 				     enum plane_id plane_id, int color_plane)
 {
+	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
 	struct skl_plane_wm *wm = &crtc_state->wm.skl.optimal.planes[plane_id];
-	u16 ddb_blocks = skl_ddb_entry_size(&crtc_state->wm.skl.plane_ddb_y[plane_id]);
 	struct skl_wm_params wm_params;
 	int ret;
 
+	/*
+	 * This can only happen if someone overrides the latency to an invalid
+	 * value of 0 in debugfs.
+	 */
+	if (dev_priv->wm.skl_latency[0] == 0)
+		return -EINVAL;
+
 	ret = skl_compute_plane_wm_params(crtc_state, plane_state,
 					  &wm_params, color_plane);
 	if (ret)
 		return ret;
 
-	ret = skl_compute_wm_levels(crtc_state, plane_state,
-				    ddb_blocks, &wm_params, wm->wm);
-	if (ret)
-		return ret;
-
-	skl_compute_transition_wm(crtc_state, &wm_params, wm, ddb_blocks);
+	skl_compute_wm_levels(crtc_state, plane_state, &wm_params, wm->wm);
+	skl_compute_transition_wm(crtc_state, &wm_params, wm);
 
 	return 0;
 }
@@ -4999,7 +4899,6 @@ static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state,
 				 enum plane_id plane_id)
 {
 	struct skl_plane_wm *wm = &crtc_state->wm.skl.optimal.planes[plane_id];
-	u16 ddb_blocks = skl_ddb_entry_size(&crtc_state->wm.skl.plane_ddb_uv[plane_id]);
 	struct skl_wm_params wm_params;
 	int ret;
 
@@ -5011,10 +4910,7 @@ static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state,
 	if (ret)
 		return ret;
 
-	ret = skl_compute_wm_levels(crtc_state, plane_state,
-				    ddb_blocks, &wm_params, wm->uv_wm);
-	if (ret)
-		return ret;
+	skl_compute_wm_levels(crtc_state, plane_state, &wm_params, wm->uv_wm);
 
 	return 0;
 }
@@ -5532,13 +5428,9 @@ skl_compute_wm(struct drm_atomic_state *state)
 	if (ret || !changed)
 		return ret;
 
-	ret = skl_compute_ddb(state);
-	if (ret)
-		return ret;
-
 	/*
 	 * Calculate WM's for all pipes that are part of this transaction.
-	 * Note that the DDB allocation above may have added more CRTC's that
+	 * Note that skl_ddb_add_affected_pipes may have added more CRTC's that
 	 * weren't otherwise being modified (and set bits in dirty_pipes) if
 	 * pipe allocations had to change.
 	 */
@@ -5568,6 +5460,11 @@ skl_compute_wm(struct drm_atomic_state *state)
 		intel_cstate->update_wm_pre = true;
 	}
 
+	ret = skl_compute_ddb(state);
+	if (ret)
+		return ret;
+
+
 	skl_print_wm_changes(intel_state);
 
 	return 0;
-- 
2.14.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915: Switch to level-based DDB allocation algorithm (v2)

[Matt Roper]

The DDB allocation algorithm currently used by the driver grants each
plane a very small minimum allocation of DDB blocks and then divies up
all of the remaining blocks based on the percentage of the total data
rate that the plane makes up.  It turns out that this proportional
allocation approach is overly-generous with the larger planes and can
leave very small planes wthout a big enough allocation to even hit their
level 0 watermark requirements (especially on APL, which has a smaller
DDB in general than other gen9 platforms).  Or there can be situations
where the smallest planes hit a lower watermark level than they should
have been able to hit with a more equitable division of DDB blocks, thus
limiting the overall system sleep state that can be achieved.

The bspec now describes an alternate algorithm that can be used to
overcome these types of issues.  With the new algorithm, we calculate
all plane watermark values for all wm levels first, then go back and
partition a pipe's DDB space second.  The DDB allocation will calculate
what the highest watermark level that can be achieved on *all* active
planes, and then grant the blocks necessary to hit that level to each
plane.  Any remaining blocks are then divided up proportionally
according to data rate, similar to the old algorithm.

There was a previous attempt to implement this algorithm a couple years
ago in bb9d85f6e9d ("drm/i915/skl: New ddb allocation algorithm"), but
some regressions were reported, the patch was reverted, and nobody
ever got around to figuring out exactly where the bug was in that
version.  Our watermark code has evolved significantly in the meantime,
but we're still getting bug reports caused by the unfair proportional
algorithm, so let's give this another shot.

v2:
 - Make sure cursor allocation stays constant and fixed at the end of
   the pipe allocation.
 - Fix some watermark level iterators that weren't handling the max
   level.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105458
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 350 +++++++++++++++-------------------------
 1 file changed, 129 insertions(+), 221 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b09c2a257ff1..6db2bf7b037c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4306,102 +4306,6 @@ icl_get_total_relative_data_rate(struct intel_crtc_state *intel_cstate,
 	return total_data_rate;
 }
 
-static uint16_t
-skl_ddb_min_alloc(const struct drm_plane_state *pstate, const int plane)
-{
-	struct drm_framebuffer *fb = pstate->fb;
-	struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
-	uint32_t src_w, src_h;
-	uint32_t min_scanlines = 8;
-	uint8_t plane_bpp;
-
-	if (WARN_ON(!fb))
-		return 0;
-
-	/* For packed formats, and uv-plane, return 0 */
-	if (plane == 1 && fb->format->format != DRM_FORMAT_NV12)
-		return 0;
-
-	/* For Non Y-tile return 8-blocks */
-	if (fb->modifier != I915_FORMAT_MOD_Y_TILED &&
-	    fb->modifier != I915_FORMAT_MOD_Yf_TILED &&
-	    fb->modifier != I915_FORMAT_MOD_Y_TILED_CCS &&
-	    fb->modifier != I915_FORMAT_MOD_Yf_TILED_CCS)
-		return 8;
-
-	/*
-	 * Src coordinates are already rotated by 270 degrees for
-	 * the 90/270 degree plane rotation cases (to match the
-	 * GTT mapping), hence no need to account for rotation here.
-	 */
-	src_w = drm_rect_width(&intel_pstate->base.src) >> 16;
-	src_h = drm_rect_height(&intel_pstate->base.src) >> 16;
-
-	/* Halve UV plane width and height for NV12 */
-	if (plane == 1) {
-		src_w /= 2;
-		src_h /= 2;
-	}
-
-	plane_bpp = fb->format->cpp[plane];
-
-	if (drm_rotation_90_or_270(pstate->rotation)) {
-		switch (plane_bpp) {
-		case 1:
-			min_scanlines = 32;
-			break;
-		case 2:
-			min_scanlines = 16;
-			break;
-		case 4:
-			min_scanlines = 8;
-			break;
-		case 8:
-			min_scanlines = 4;
-			break;
-		default:
-			WARN(1, "Unsupported pixel depth %u for rotation",
-			     plane_bpp);
-			min_scanlines = 32;
-		}
-	}
-
-	return DIV_ROUND_UP((4 * src_w * plane_bpp), 512) * min_scanlines/4 + 3;
-}
-
-static void
-skl_ddb_calc_min(const struct intel_crtc_state *cstate, int num_active,
-		 uint16_t *minimum, uint16_t *uv_minimum)
-{
-	const struct drm_plane_state *pstate;
-	struct drm_plane *plane;
-
-	drm_atomic_crtc_state_for_each_plane_state(plane, pstate, &cstate->base) {
-		enum plane_id plane_id = to_intel_plane(plane)->id;
-		struct intel_plane_state *plane_state = to_intel_plane_state(pstate);
-
-		if (plane_id == PLANE_CURSOR)
-			continue;
-
-		/* slave plane must be invisible and calculated from master */
-		if (!pstate->visible || WARN_ON(plane_state->slave))
-			continue;
-
-		if (!plane_state->linked_plane) {
-			minimum[plane_id] = skl_ddb_min_alloc(pstate, 0);
-			uv_minimum[plane_id] = skl_ddb_min_alloc(pstate, 1);
-		} else {
-			enum plane_id y_plane_id =
-				plane_state->linked_plane->id;
-
-			minimum[y_plane_id] = skl_ddb_min_alloc(pstate, 0);
-			minimum[plane_id] = skl_ddb_min_alloc(pstate, 1);
-		}
-	}
-
-	minimum[PLANE_CURSOR] = skl_cursor_allocation(num_active);
-}
-
 static int
 skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 		      struct skl_ddb_allocation *ddb /* out */)
@@ -4411,15 +4315,18 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct skl_ddb_entry *alloc = &cstate->wm.skl.ddb;
-	uint16_t alloc_size, start;
-	uint16_t minimum[I915_MAX_PLANES] = {};
-	uint16_t uv_minimum[I915_MAX_PLANES] = {};
+	struct skl_ddb_entry *plane_alloc, *uv_plane_alloc;
+	struct skl_plane_wm *wm;
+	uint16_t alloc_size, start = 0;
+	uint16_t total[I915_MAX_PLANES] = {};
+	uint16_t uv_total[I915_MAX_PLANES] = {};
 	u64 total_data_rate;
 	enum plane_id plane_id;
 	int num_active;
 	u64 plane_data_rate[I915_MAX_PLANES] = {};
 	u64 uv_plane_data_rate[I915_MAX_PLANES] = {};
-	uint16_t total_min_blocks = 0;
+	uint16_t blocks = 0;
+	int level;
 
 	/* Clear the partitioning for disabled planes. */
 	memset(cstate->wm.skl.plane_ddb_y, 0, sizeof(cstate->wm.skl.plane_ddb_y));
@@ -4449,81 +4356,112 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 	if (alloc_size == 0)
 		return 0;
 
-	skl_ddb_calc_min(cstate, num_active, minimum, uv_minimum);
+	/* Allocate fixed number of blocks for cursor. */
+	total[PLANE_CURSOR] = skl_cursor_allocation(num_active);
+	alloc_size -= total[PLANE_CURSOR];
+	cstate->wm.skl.plane_ddb_y[PLANE_CURSOR].start =
+		alloc->end - total[PLANE_CURSOR];
+	cstate->wm.skl.plane_ddb_y[PLANE_CURSOR].end = alloc->end;
+
+	if (total_data_rate == 0)
+		return 0;
 
 	/*
-	 * 1. Allocate the mininum required blocks for each active plane
-	 * and allocate the cursor, it doesn't require extra allocation
-	 * proportional to the data rate.
+	 * Find the highest watermark level for which we can satisfy the block
+	 * requirement of active planes.
 	 */
+	for (level = ilk_wm_max_level(dev_priv); level >= 0; level--) {
+		for_each_plane_id_on_crtc(intel_crtc, plane_id) {
+			if (plane_id == PLANE_CURSOR)
+				continue;
 
-	for_each_plane_id_on_crtc(intel_crtc, plane_id) {
-		total_min_blocks += minimum[plane_id];
-		total_min_blocks += uv_minimum[plane_id];
+			wm = &cstate->wm.skl.optimal.planes[plane_id];
+			blocks += wm->wm[level].plane_res_b;
+			blocks += wm->uv_wm[level].plane_res_b;
+		}
+
+		if (blocks < alloc_size) {
+			alloc_size -= blocks;
+			break;
+		}
 	}
 
-	if (total_min_blocks > alloc_size) {
+	if (level < 0) {
 		DRM_DEBUG_KMS("Requested display configuration exceeds system DDB limitations");
-		DRM_DEBUG_KMS("minimum required %d/%d\n", total_min_blocks,
-							alloc_size);
+		DRM_DEBUG_KMS("minimum required %d/%d\n", blocks,
+			      alloc_size);
 		return -EINVAL;
 	}
 
-	alloc_size -= total_min_blocks;
-	cstate->wm.skl.plane_ddb_y[PLANE_CURSOR].start = alloc->end - minimum[PLANE_CURSOR];
-	cstate->wm.skl.plane_ddb_y[PLANE_CURSOR].end = alloc->end;
-
 	/*
-	 * 2. Distribute the remaining space in proportion to the amount of
-	 * data each plane needs to fetch from memory.
-	 *
-	 * FIXME: we may not allocate every single block here.
+	 * Grant each plane the blocks it requires at the highest achievable
+	 * watermark level, plus an extra share of the leftover blocks
+	 * proportional to its relative data rate.
 	 */
-	if (total_data_rate == 0)
-		return 0;
-
-	start = alloc->start;
 	for_each_plane_id_on_crtc(intel_crtc, plane_id) {
-		u64 data_rate, uv_data_rate;
-		uint16_t plane_blocks, uv_plane_blocks;
+		u64 rate;
 
 		if (plane_id == PLANE_CURSOR)
 			continue;
 
-		data_rate = plane_data_rate[plane_id];
+		wm = &cstate->wm.skl.optimal.planes[plane_id];
 
-		/*
-		 * allocation for (packed formats) or (uv-plane part of planar format):
-		 * promote the expression to 64 bits to avoid overflowing, the
-		 * result is < available as data_rate / total_data_rate < 1
-		 */
-		plane_blocks = minimum[plane_id];
-		plane_blocks += div64_u64(alloc_size * data_rate, total_data_rate);
+		rate = plane_data_rate[plane_id];
+		total[plane_id] = wm->wm[level].plane_res_b +
+			div64_u64(alloc_size * rate, total_data_rate);
 
-		/* Leave disabled planes at (0,0) */
-		if (data_rate) {
-			cstate->wm.skl.plane_ddb_y[plane_id].start = start;
-			cstate->wm.skl.plane_ddb_y[plane_id].end = start + plane_blocks;
-		}
-
-		start += plane_blocks;
+		rate = uv_plane_data_rate[plane_id];
+		uv_total[plane_id] = wm->uv_wm[level].plane_res_b +
+			div64_u64(alloc_size * rate, total_data_rate);
+	}
 
-		/* Allocate DDB for UV plane for planar format/NV12 */
-		uv_data_rate = uv_plane_data_rate[plane_id];
+	/* Set the actual DDB start/end points for each plane */
+	start = alloc->start;
+	for_each_plane_id_on_crtc(intel_crtc, plane_id) {
+		if (plane_id == PLANE_CURSOR)
+			continue;
 
-		uv_plane_blocks = uv_minimum[plane_id];
-		uv_plane_blocks += div64_u64(alloc_size * uv_data_rate, total_data_rate);
+		plane_alloc = &cstate->wm.skl.plane_ddb_y[plane_id];
+		uv_plane_alloc = &cstate->wm.skl.plane_ddb_uv[plane_id];
 
 		/* Gen11+ uses a separate plane for UV watermarks */
-		WARN_ON(INTEL_GEN(dev_priv) >= 11 && uv_plane_blocks);
+		WARN_ON(INTEL_GEN(dev_priv) >= 11 && uv_total[plane_id]);
 
-		if (uv_data_rate) {
-			cstate->wm.skl.plane_ddb_uv[plane_id].start = start;
-			cstate->wm.skl.plane_ddb_uv[plane_id].end =
-				start + uv_plane_blocks;
+		/* Leave disabled planes at (0,0) */
+		if (total[plane_id]) {
+			plane_alloc->start = start;
+			plane_alloc->end = start += total[plane_id];
+		}
+
+		if (uv_total[plane_id]) {
+			uv_plane_alloc->start = start;
+			uv_plane_alloc->end = start + uv_total[plane_id];
+		}
+	}
+
+	/*
+	 * When we calculated watermark values we didn't know how high
+	 * of a level we'd actually be able to hit, so we just marked
+	 * all levels as "enabled."  Go back now and disable the ones
+	 * that aren't actually possible.
+	 */
+	while (level++ <= ilk_wm_max_level(dev_priv)) {
+		for_each_plane_id_on_crtc(intel_crtc, plane_id) {
+			wm = &cstate->wm.skl.optimal.planes[plane_id];
+			wm->wm[level].plane_en = false;
+			wm->wm[level].plane_res_b = 0;
+			wm->wm[level].plane_res_l = 0;
 		}
+	}
 
-		start += uv_plane_blocks;
+	/*
+	 * Go back and disable the transition watermark if it turns out we
+	 * don't have enough DDB blocks for it.
+	 */
+	for_each_plane_id_on_crtc(intel_crtc, plane_id) {
+		wm = &cstate->wm.skl.optimal.planes[plane_id];
+		if (wm->trans_wm.plane_res_b > total[plane_id])
+			wm->trans_wm.plane_en = false;
 	}
 
 	return 0;
@@ -4720,17 +4658,15 @@ skl_compute_plane_wm_params(const struct intel_crtc_state *cstate,
 	return 0;
 }
 
-static int skl_compute_plane_wm(const struct intel_crtc_state *cstate,
-				const struct intel_plane_state *intel_pstate,
-				uint16_t ddb_allocation,
-				int level,
-				const struct skl_wm_params *wp,
-				const struct skl_wm_level *result_prev,
-				struct skl_wm_level *result /* out */)
+static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
+				 const struct intel_plane_state *intel_pstate,
+				 int level,
+				 const struct skl_wm_params *wp,
+				 const struct skl_wm_level *result_prev,
+				 struct skl_wm_level *result /* out */)
 {
 	struct drm_i915_private *dev_priv =
 		to_i915(intel_pstate->base.plane->dev);
-	const struct drm_plane_state *pstate = &intel_pstate->base;
 	uint32_t latency = dev_priv->wm.skl_latency[level];
 	uint_fixed_16_16_t method1, method2;
 	uint_fixed_16_16_t selected_result;
@@ -4740,9 +4676,6 @@ static int skl_compute_plane_wm(const struct intel_crtc_state *cstate,
 	bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state);
 	uint32_t min_disp_buf_needed;
 
-	if (latency == 0)
-		return level == 0 ? -EINVAL : 0;
-
 	/* Display WA #1141: kbl,cfl */
 	if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
 	    IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_B0)) &&
@@ -4828,38 +4761,24 @@ static int skl_compute_plane_wm(const struct intel_crtc_state *cstate,
 		min_disp_buf_needed = res_blocks;
 	}
 
-	if ((level > 0 && res_lines > 31) ||
-	    res_blocks >= ddb_allocation ||
-	    min_disp_buf_needed >= ddb_allocation) {
-		/*
-		 * If there are no valid level 0 watermarks, then we can't
-		 * support this display configuration.
-		 */
-		if (level) {
-			return 0;
-		} else {
-			struct drm_plane *plane = pstate->plane;
-
-			DRM_DEBUG_KMS("Requested display configuration exceeds system watermark limitations\n");
-			DRM_DEBUG_KMS("[PLANE:%d:%s] blocks required = %u/%u, lines required = %u/31\n",
-				      plane->base.id, plane->name,
-				      res_blocks, ddb_allocation, res_lines);
-			return -EINVAL;
-		}
-	}
-
 	/* The number of lines are ignored for the level 0 watermark. */
+	if (level > 0 && res_lines > 31)
+		return;
+
+	/*
+	 * If res_lines is valid, assume we can use this watermark level
+	 * for now.  We'll come back and disable it after we calculate the
+	 * DDB allocation if it turns out we don't actually have enough
+	 * blocks to satisfy it.
+	 */
 	result->plane_res_b = res_blocks;
 	result->plane_res_l = res_lines;
 	result->plane_en = true;
-
-	return 0;
 }
 
-static int
+static void
 skl_compute_wm_levels(const struct intel_crtc_state *cstate,
 		      const struct intel_plane_state *intel_pstate,
-		      uint16_t ddb_blocks,
 		      const struct skl_wm_params *wm_params,
 		      struct skl_wm_level *levels)
 {
@@ -4867,25 +4786,15 @@ skl_compute_wm_levels(const struct intel_crtc_state *cstate,
 		to_i915(intel_pstate->base.plane->dev);
 	int level, max_level = ilk_wm_max_level(dev_priv);
 	struct skl_wm_level *result_prev = &levels[0];
-	int ret;
 
 	for (level = 0; level <= max_level; level++) {
 		struct skl_wm_level *result = &levels[level];
 
-		ret = skl_compute_plane_wm(cstate,
-					   intel_pstate,
-					   ddb_blocks,
-					   level,
-					   wm_params,
-					   result_prev,
-					   result);
-		if (ret)
-			return ret;
+		skl_compute_plane_wm(cstate, intel_pstate, level, wm_params,
+				     result_prev, result);
 
 		result_prev = result;
 	}
-
-	return 0;
 }
 
 static uint32_t
@@ -4913,8 +4822,7 @@ skl_compute_linetime_wm(const struct intel_crtc_state *cstate)
 
 static void skl_compute_transition_wm(const struct intel_crtc_state *cstate,
 				      const struct skl_wm_params *wp,
-				      struct skl_plane_wm *wm,
-				      uint16_t ddb_allocation)
+				      struct skl_plane_wm *wm)
 {
 	struct drm_device *dev = cstate->base.crtc->dev;
 	const struct drm_i915_private *dev_priv = to_i915(dev);
@@ -4962,34 +4870,38 @@ static void skl_compute_transition_wm(const struct intel_crtc_state *cstate,
 
 	}
 
-	res_blocks += 1;
-
-	if (res_blocks < ddb_allocation) {
-		wm->trans_wm.plane_res_b = res_blocks;
-		wm->trans_wm.plane_en = true;
-	}
+	/*
+	 * Just assume we can enable the transition watermark.  After
+	 * computing the DDB we'll come back and disable it if that
+	 * assumption turns out to be false.
+	 */
+	wm->trans_wm.plane_res_b = res_blocks + 1;
+	wm->trans_wm.plane_en = true;
 }
 
 static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
 				     const struct intel_plane_state *plane_state,
 				     enum plane_id plane_id, int color_plane)
 {
+	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
 	struct skl_plane_wm *wm = &crtc_state->wm.skl.optimal.planes[plane_id];
-	u16 ddb_blocks = skl_ddb_entry_size(&crtc_state->wm.skl.plane_ddb_y[plane_id]);
 	struct skl_wm_params wm_params;
 	int ret;
 
+	/*
+	 * This can only happen if someone overrides the latency to an invalid
+	 * value of 0 in debugfs.
+	 */
+	if (dev_priv->wm.skl_latency[0] == 0)
+		return -EINVAL;
+
 	ret = skl_compute_plane_wm_params(crtc_state, plane_state,
 					  &wm_params, color_plane);
 	if (ret)
 		return ret;
 
-	ret = skl_compute_wm_levels(crtc_state, plane_state,
-				    ddb_blocks, &wm_params, wm->wm);
-	if (ret)
-		return ret;
-
-	skl_compute_transition_wm(crtc_state, &wm_params, wm, ddb_blocks);
+	skl_compute_wm_levels(crtc_state, plane_state, &wm_params, wm->wm);
+	skl_compute_transition_wm(crtc_state, &wm_params, wm);
 
 	return 0;
 }
@@ -4999,7 +4911,6 @@ static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state,
 				 enum plane_id plane_id)
 {
 	struct skl_plane_wm *wm = &crtc_state->wm.skl.optimal.planes[plane_id];
-	u16 ddb_blocks = skl_ddb_entry_size(&crtc_state->wm.skl.plane_ddb_uv[plane_id]);
 	struct skl_wm_params wm_params;
 	int ret;
 
@@ -5011,10 +4922,7 @@ static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state,
 	if (ret)
 		return ret;
 
-	ret = skl_compute_wm_levels(crtc_state, plane_state,
-				    ddb_blocks, &wm_params, wm->uv_wm);
-	if (ret)
-		return ret;
+	skl_compute_wm_levels(crtc_state, plane_state, &wm_params, wm->uv_wm);
 
 	return 0;
 }
@@ -5532,13 +5440,9 @@ skl_compute_wm(struct drm_atomic_state *state)
 	if (ret || !changed)
 		return ret;
 
-	ret = skl_compute_ddb(state);
-	if (ret)
-		return ret;
-
 	/*
 	 * Calculate WM's for all pipes that are part of this transaction.
-	 * Note that the DDB allocation above may have added more CRTC's that
+	 * Note that skl_ddb_add_affected_pipes may have added more CRTC's that
 	 * weren't otherwise being modified (and set bits in dirty_pipes) if
 	 * pipe allocations had to change.
 	 */
@@ -5568,6 +5472,10 @@ skl_compute_wm(struct drm_atomic_state *state)
 		intel_cstate->update_wm_pre = true;
 	}
 
+	ret = skl_compute_ddb(state);
+	if (ret)
+		return ret;
+
 	skl_print_wm_changes(intel_state);
 
 	return 0;
-- 
2.14.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Switch to level-based DDB allocation algorithm (v2)

[Ville Syrjälä]

On Thu, Dec 06, 2018 at 03:55:41PM -0800, Matt Roper wrote:
> The DDB allocation algorithm currently used by the driver grants each
> plane a very small minimum allocation of DDB blocks and then divies up
> all of the remaining blocks based on the percentage of the total data
> rate that the plane makes up.  It turns out that this proportional
> allocation approach is overly-generous with the larger planes and can
> leave very small planes wthout a big enough allocation to even hit their
> level 0 watermark requirements (especially on APL, which has a smaller
> DDB in general than other gen9 platforms).  Or there can be situations
> where the smallest planes hit a lower watermark level than they should
> have been able to hit with a more equitable division of DDB blocks, thus
> limiting the overall system sleep state that can be achieved.
> 
> The bspec now describes an alternate algorithm that can be used to
> overcome these types of issues.  With the new algorithm, we calculate
> all plane watermark values for all wm levels first, then go back and
> partition a pipe's DDB space second.  The DDB allocation will calculate
> what the highest watermark level that can be achieved on *all* active
> planes, and then grant the blocks necessary to hit that level to each
> plane.  Any remaining blocks are then divided up proportionally
> according to data rate, similar to the old algorithm.

OK, so it's somewhat similar to what I already did for VLV/CHV. The
difference being that on VLV/CHV I distribute purely based on the
level 0 watermark and don't even consider the data rates. Not sure
if there's a significant difference between the two approaches.

> 
> There was a previous attempt to implement this algorithm a couple years
> ago in bb9d85f6e9d ("drm/i915/skl: New ddb allocation algorithm"), but
> some regressions were reported, the patch was reverted, and nobody
> ever got around to figuring out exactly where the bug was in that
> version.  Our watermark code has evolved significantly in the meantime,
> but we're still getting bug reports caused by the unfair proportional
> algorithm, so let's give this another shot.
> 
> v2:
>  - Make sure cursor allocation stays constant and fixed at the end of
>    the pipe allocation.
>  - Fix some watermark level iterators that weren't handling the max
>    level.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105458
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 350 +++++++++++++++-------------------------
>  1 file changed, 129 insertions(+), 221 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index b09c2a257ff1..6db2bf7b037c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4306,102 +4306,6 @@ icl_get_total_relative_data_rate(struct intel_crtc_state *intel_cstate,
>  	return total_data_rate;
>  }
>  
> -static uint16_t
> -skl_ddb_min_alloc(const struct drm_plane_state *pstate, const int plane)
> -{
> -	struct drm_framebuffer *fb = pstate->fb;
> -	struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
> -	uint32_t src_w, src_h;
> -	uint32_t min_scanlines = 8;
> -	uint8_t plane_bpp;
> -
> -	if (WARN_ON(!fb))
> -		return 0;
> -
> -	/* For packed formats, and uv-plane, return 0 */
> -	if (plane == 1 && fb->format->format != DRM_FORMAT_NV12)
> -		return 0;
> -
> -	/* For Non Y-tile return 8-blocks */
> -	if (fb->modifier != I915_FORMAT_MOD_Y_TILED &&
> -	    fb->modifier != I915_FORMAT_MOD_Yf_TILED &&
> -	    fb->modifier != I915_FORMAT_MOD_Y_TILED_CCS &&
> -	    fb->modifier != I915_FORMAT_MOD_Yf_TILED_CCS)
> -		return 8;
> -
> -	/*
> -	 * Src coordinates are already rotated by 270 degrees for
> -	 * the 90/270 degree plane rotation cases (to match the
> -	 * GTT mapping), hence no need to account for rotation here.
> -	 */
> -	src_w = drm_rect_width(&intel_pstate->base.src) >> 16;
> -	src_h = drm_rect_height(&intel_pstate->base.src) >> 16;
> -
> -	/* Halve UV plane width and height for NV12 */
> -	if (plane == 1) {
> -		src_w /= 2;
> -		src_h /= 2;
> -	}
> -
> -	plane_bpp = fb->format->cpp[plane];
> -
> -	if (drm_rotation_90_or_270(pstate->rotation)) {
> -		switch (plane_bpp) {
> -		case 1:
> -			min_scanlines = 32;
> -			break;
> -		case 2:
> -			min_scanlines = 16;
> -			break;
> -		case 4:
> -			min_scanlines = 8;
> -			break;
> -		case 8:
> -			min_scanlines = 4;
> -			break;
> -		default:
> -			WARN(1, "Unsupported pixel depth %u for rotation",
> -			     plane_bpp);
> -			min_scanlines = 32;
> -		}
> -	}
> -
> -	return DIV_ROUND_UP((4 * src_w * plane_bpp), 512) * min_scanlines/4 + 3;
> -}
> -
> -static void
> -skl_ddb_calc_min(const struct intel_crtc_state *cstate, int num_active,
> -		 uint16_t *minimum, uint16_t *uv_minimum)
> -{
> -	const struct drm_plane_state *pstate;
> -	struct drm_plane *plane;
> -
> -	drm_atomic_crtc_state_for_each_plane_state(plane, pstate, &cstate->base) {
> -		enum plane_id plane_id = to_intel_plane(plane)->id;
> -		struct intel_plane_state *plane_state = to_intel_plane_state(pstate);
> -
> -		if (plane_id == PLANE_CURSOR)
> -			continue;
> -
> -		/* slave plane must be invisible and calculated from master */
> -		if (!pstate->visible || WARN_ON(plane_state->slave))
> -			continue;
> -
> -		if (!plane_state->linked_plane) {
> -			minimum[plane_id] = skl_ddb_min_alloc(pstate, 0);
> -			uv_minimum[plane_id] = skl_ddb_min_alloc(pstate, 1);
> -		} else {
> -			enum plane_id y_plane_id =
> -				plane_state->linked_plane->id;
> -
> -			minimum[y_plane_id] = skl_ddb_min_alloc(pstate, 0);
> -			minimum[plane_id] = skl_ddb_min_alloc(pstate, 1);
> -		}
> -	}
> -
> -	minimum[PLANE_CURSOR] = skl_cursor_allocation(num_active);
> -}
> -
>  static int
>  skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>  		      struct skl_ddb_allocation *ddb /* out */)
> @@ -4411,15 +4315,18 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>  	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct skl_ddb_entry *alloc = &cstate->wm.skl.ddb;
> -	uint16_t alloc_size, start;
> -	uint16_t minimum[I915_MAX_PLANES] = {};
> -	uint16_t uv_minimum[I915_MAX_PLANES] = {};
> +	struct skl_ddb_entry *plane_alloc, *uv_plane_alloc;
> +	struct skl_plane_wm *wm;

This one at least could be moved into a tighter scope AFAICS.

> +	uint16_t alloc_size, start = 0;
> +	uint16_t total[I915_MAX_PLANES] = {};
> +	uint16_t uv_total[I915_MAX_PLANES] = {};
>  	u64 total_data_rate;
>  	enum plane_id plane_id;
>  	int num_active;
>  	u64 plane_data_rate[I915_MAX_PLANES] = {};
>  	u64 uv_plane_data_rate[I915_MAX_PLANES] = {};
> -	uint16_t total_min_blocks = 0;
> +	uint16_t blocks = 0;
> +	int level;
>  
>  	/* Clear the partitioning for disabled planes. */
>  	memset(cstate->wm.skl.plane_ddb_y, 0, sizeof(cstate->wm.skl.plane_ddb_y));
> @@ -4449,81 +4356,112 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>  	if (alloc_size == 0)
>  		return 0;
>  
> -	skl_ddb_calc_min(cstate, num_active, minimum, uv_minimum);
> +	/* Allocate fixed number of blocks for cursor. */
> +	total[PLANE_CURSOR] = skl_cursor_allocation(num_active);
> +	alloc_size -= total[PLANE_CURSOR];
> +	cstate->wm.skl.plane_ddb_y[PLANE_CURSOR].start =
> +		alloc->end - total[PLANE_CURSOR];
> +	cstate->wm.skl.plane_ddb_y[PLANE_CURSOR].end = alloc->end;
> +
> +	if (total_data_rate == 0)
> +		return 0;
>  
>  	/*
> -	 * 1. Allocate the mininum required blocks for each active plane
> -	 * and allocate the cursor, it doesn't require extra allocation
> -	 * proportional to the data rate.
> +	 * Find the highest watermark level for which we can satisfy the block
> +	 * requirement of active planes.
>  	 */
> +	for (level = ilk_wm_max_level(dev_priv); level >= 0; level--) {
> +		for_each_plane_id_on_crtc(intel_crtc, plane_id) {
> +			if (plane_id == PLANE_CURSOR)
> +				continue;
>  
> -	for_each_plane_id_on_crtc(intel_crtc, plane_id) {
> -		total_min_blocks += minimum[plane_id];
> -		total_min_blocks += uv_minimum[plane_id];
> +			wm = &cstate->wm.skl.optimal.planes[plane_id];
> +			blocks += wm->wm[level].plane_res_b;
> +			blocks += wm->uv_wm[level].plane_res_b;
> +		}
> +
> +		if (blocks < alloc_size) {
> +			alloc_size -= blocks;
> +			break;
> +		}
>  	}
>  
> -	if (total_min_blocks > alloc_size) {
> +	if (level < 0) {
>  		DRM_DEBUG_KMS("Requested display configuration exceeds system DDB limitations");
> -		DRM_DEBUG_KMS("minimum required %d/%d\n", total_min_blocks,
> -							alloc_size);
> +		DRM_DEBUG_KMS("minimum required %d/%d\n", blocks,
> +			      alloc_size);
>  		return -EINVAL;
>  	}
>  
> -	alloc_size -= total_min_blocks;
> -	cstate->wm.skl.plane_ddb_y[PLANE_CURSOR].start = alloc->end - minimum[PLANE_CURSOR];
> -	cstate->wm.skl.plane_ddb_y[PLANE_CURSOR].end = alloc->end;
> -
>  	/*
> -	 * 2. Distribute the remaining space in proportion to the amount of
> -	 * data each plane needs to fetch from memory.
> -	 *
> -	 * FIXME: we may not allocate every single block here.
> +	 * Grant each plane the blocks it requires at the highest achievable
> +	 * watermark level, plus an extra share of the leftover blocks
> +	 * proportional to its relative data rate.
>  	 */
> -	if (total_data_rate == 0)
> -		return 0;
> -
> -	start = alloc->start;
>  	for_each_plane_id_on_crtc(intel_crtc, plane_id) {
> -		u64 data_rate, uv_data_rate;
> -		uint16_t plane_blocks, uv_plane_blocks;
> +		u64 rate;
>  
>  		if (plane_id == PLANE_CURSOR)
>  			continue;
>  
> -		data_rate = plane_data_rate[plane_id];
> +		wm = &cstate->wm.skl.optimal.planes[plane_id];
>  
> -		/*
> -		 * allocation for (packed formats) or (uv-plane part of planar format):
> -		 * promote the expression to 64 bits to avoid overflowing, the
> -		 * result is < available as data_rate / total_data_rate < 1
> -		 */
> -		plane_blocks = minimum[plane_id];
> -		plane_blocks += div64_u64(alloc_size * data_rate, total_data_rate);
> +		rate = plane_data_rate[plane_id];
> +		total[plane_id] = wm->wm[level].plane_res_b +
> +			div64_u64(alloc_size * rate, total_data_rate);

This might still leave some blocks unused no? On VLV/CHV I used
DIV_ROUND_UP()+min()+WARN() to make sure we never a block behind.

>  
> -		/* Leave disabled planes at (0,0) */
> -		if (data_rate) {
> -			cstate->wm.skl.plane_ddb_y[plane_id].start = start;
> -			cstate->wm.skl.plane_ddb_y[plane_id].end = start + plane_blocks;
> -		}
> -
> -		start += plane_blocks;
> +		rate = uv_plane_data_rate[plane_id];
> +		uv_total[plane_id] = wm->uv_wm[level].plane_res_b +
> +			div64_u64(alloc_size * rate, total_data_rate);
> +	}
>  
> -		/* Allocate DDB for UV plane for planar format/NV12 */
> -		uv_data_rate = uv_plane_data_rate[plane_id];
> +	/* Set the actual DDB start/end points for each plane */
> +	start = alloc->start;
> +	for_each_plane_id_on_crtc(intel_crtc, plane_id) {
> +		if (plane_id == PLANE_CURSOR)
> +			continue;
>  
> -		uv_plane_blocks = uv_minimum[plane_id];
> -		uv_plane_blocks += div64_u64(alloc_size * uv_data_rate, total_data_rate);
> +		plane_alloc = &cstate->wm.skl.plane_ddb_y[plane_id];
> +		uv_plane_alloc = &cstate->wm.skl.plane_ddb_uv[plane_id];
>  
>  		/* Gen11+ uses a separate plane for UV watermarks */
> -		WARN_ON(INTEL_GEN(dev_priv) >= 11 && uv_plane_blocks);
> +		WARN_ON(INTEL_GEN(dev_priv) >= 11 && uv_total[plane_id]);
>  
> -		if (uv_data_rate) {
> -			cstate->wm.skl.plane_ddb_uv[plane_id].start = start;
> -			cstate->wm.skl.plane_ddb_uv[plane_id].end =
> -				start + uv_plane_blocks;
> +		/* Leave disabled planes at (0,0) */
> +		if (total[plane_id]) {
> +			plane_alloc->start = start;
> +			plane_alloc->end = start += total[plane_id];
> +		}
> +
> +		if (uv_total[plane_id]) {
> +			uv_plane_alloc->start = start;
> +			uv_plane_alloc->end = start + uv_total[plane_id];
> +		}
> +	}
> +
> +	/*
> +	 * When we calculated watermark values we didn't know how high
> +	 * of a level we'd actually be able to hit, so we just marked
> +	 * all levels as "enabled."  Go back now and disable the ones
> +	 * that aren't actually possible.
> +	 */
> +	while (level++ <= ilk_wm_max_level(dev_priv)) {

I'd prefer a for-loop.

> +		for_each_plane_id_on_crtc(intel_crtc, plane_id) {
> +			wm = &cstate->wm.skl.optimal.planes[plane_id];
> +			wm->wm[level].plane_en = false;
> +			wm->wm[level].plane_res_b = 0;
> +			wm->wm[level].plane_res_l = 0;
>  		}
> +	}
>  
> -		start += uv_plane_blocks;
> +	/*
> +	 * Go back and disable the transition watermark if it turns out we
> +	 * don't have enough DDB blocks for it.
> +	 */
> +	for_each_plane_id_on_crtc(intel_crtc, plane_id) {
> +		wm = &cstate->wm.skl.optimal.planes[plane_id];
> +		if (wm->trans_wm.plane_res_b > total[plane_id])
> +			wm->trans_wm.plane_en = false;

Since we use memcmp() to check for dirty watermarks we should
probably zero out plane_res_b/l here too. Maybe even use memset()
for clearing each disabled trans/normal wm.

>  	}
>  
>  	return 0;
> @@ -4720,17 +4658,15 @@ skl_compute_plane_wm_params(const struct intel_crtc_state *cstate,
>  	return 0;
>  }
>  
> -static int skl_compute_plane_wm(const struct intel_crtc_state *cstate,
> -				const struct intel_plane_state *intel_pstate,
> -				uint16_t ddb_allocation,
> -				int level,
> -				const struct skl_wm_params *wp,
> -				const struct skl_wm_level *result_prev,
> -				struct skl_wm_level *result /* out */)
> +static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
> +				 const struct intel_plane_state *intel_pstate,
> +				 int level,
> +				 const struct skl_wm_params *wp,
> +				 const struct skl_wm_level *result_prev,
> +				 struct skl_wm_level *result /* out */)
>  {
>  	struct drm_i915_private *dev_priv =
>  		to_i915(intel_pstate->base.plane->dev);
> -	const struct drm_plane_state *pstate = &intel_pstate->base;
>  	uint32_t latency = dev_priv->wm.skl_latency[level];
>  	uint_fixed_16_16_t method1, method2;
>  	uint_fixed_16_16_t selected_result;
> @@ -4740,9 +4676,6 @@ static int skl_compute_plane_wm(const struct intel_crtc_state *cstate,
>  	bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state);
>  	uint32_t min_disp_buf_needed;
>  
> -	if (latency == 0)
> -		return level == 0 ? -EINVAL : 0;
> -
>  	/* Display WA #1141: kbl,cfl */
>  	if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
>  	    IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_B0)) &&
> @@ -4828,38 +4761,24 @@ static int skl_compute_plane_wm(const struct intel_crtc_state *cstate,
>  		min_disp_buf_needed = res_blocks;
>  	}
>  
> -	if ((level > 0 && res_lines > 31) ||
> -	    res_blocks >= ddb_allocation ||
> -	    min_disp_buf_needed >= ddb_allocation) {

Since you're removing this thing the calculation of min_disp_buf_needed
is now pointless AFAICS.

> -		/*
> -		 * If there are no valid level 0 watermarks, then we can't
> -		 * support this display configuration.
> -		 */
> -		if (level) {
> -			return 0;
> -		} else {
> -			struct drm_plane *plane = pstate->plane;
> -
> -			DRM_DEBUG_KMS("Requested display configuration exceeds system watermark limitations\n");
> -			DRM_DEBUG_KMS("[PLANE:%d:%s] blocks required = %u/%u, lines required = %u/31\n",
> -				      plane->base.id, plane->name,
> -				      res_blocks, ddb_allocation, res_lines);
> -			return -EINVAL;
> -		}
> -	}
> -
>  	/* The number of lines are ignored for the level 0 watermark. */
> +	if (level > 0 && res_lines > 31)
> +		return;
> +
> +	/*
> +	 * If res_lines is valid, assume we can use this watermark level
> +	 * for now.  We'll come back and disable it after we calculate the
> +	 * DDB allocation if it turns out we don't actually have enough
> +	 * blocks to satisfy it.
> +	 */
>  	result->plane_res_b = res_blocks;
>  	result->plane_res_l = res_lines;
>  	result->plane_en = true;
> -
> -	return 0;
>  }
>  
> -static int
> +static void
>  skl_compute_wm_levels(const struct intel_crtc_state *cstate,
>  		      const struct intel_plane_state *intel_pstate,
> -		      uint16_t ddb_blocks,
>  		      const struct skl_wm_params *wm_params,
>  		      struct skl_wm_level *levels)
>  {
> @@ -4867,25 +4786,15 @@ skl_compute_wm_levels(const struct intel_crtc_state *cstate,
>  		to_i915(intel_pstate->base.plane->dev);
>  	int level, max_level = ilk_wm_max_level(dev_priv);
>  	struct skl_wm_level *result_prev = &levels[0];
> -	int ret;
>  
>  	for (level = 0; level <= max_level; level++) {
>  		struct skl_wm_level *result = &levels[level];
>  
> -		ret = skl_compute_plane_wm(cstate,
> -					   intel_pstate,
> -					   ddb_blocks,
> -					   level,
> -					   wm_params,
> -					   result_prev,
> -					   result);
> -		if (ret)
> -			return ret;
> +		skl_compute_plane_wm(cstate, intel_pstate, level, wm_params,
> +				     result_prev, result);
>  
>  		result_prev = result;
>  	}
> -
> -	return 0;
>  }
>  
>  static uint32_t
> @@ -4913,8 +4822,7 @@ skl_compute_linetime_wm(const struct intel_crtc_state *cstate)
>  
>  static void skl_compute_transition_wm(const struct intel_crtc_state *cstate,
>  				      const struct skl_wm_params *wp,
> -				      struct skl_plane_wm *wm,
> -				      uint16_t ddb_allocation)
> +				      struct skl_plane_wm *wm)
>  {
>  	struct drm_device *dev = cstate->base.crtc->dev;
>  	const struct drm_i915_private *dev_priv = to_i915(dev);
> @@ -4962,34 +4870,38 @@ static void skl_compute_transition_wm(const struct intel_crtc_state *cstate,
>  
>  	}
>  
> -	res_blocks += 1;
> -
> -	if (res_blocks < ddb_allocation) {
> -		wm->trans_wm.plane_res_b = res_blocks;
> -		wm->trans_wm.plane_en = true;
> -	}
> +	/*
> +	 * Just assume we can enable the transition watermark.  After
> +	 * computing the DDB we'll come back and disable it if that
> +	 * assumption turns out to be false.
> +	 */
> +	wm->trans_wm.plane_res_b = res_blocks + 1;
> +	wm->trans_wm.plane_en = true;
>  }
>  
>  static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
>  				     const struct intel_plane_state *plane_state,
>  				     enum plane_id plane_id, int color_plane)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
>  	struct skl_plane_wm *wm = &crtc_state->wm.skl.optimal.planes[plane_id];
> -	u16 ddb_blocks = skl_ddb_entry_size(&crtc_state->wm.skl.plane_ddb_y[plane_id]);
>  	struct skl_wm_params wm_params;
>  	int ret;
>  
> +	/*
> +	 * This can only happen if someone overrides the latency to an invalid
> +	 * value of 0 in debugfs.
> +	 */
> +	if (dev_priv->wm.skl_latency[0] == 0)
> +		return -EINVAL;

I guess we don't actually need this anymore. I think it should end
up automagically handled by the ddb allocation failing on account
of skl_wm_method{1,2}() returning "some big number".

> +
>  	ret = skl_compute_plane_wm_params(crtc_state, plane_state,
>  					  &wm_params, color_plane);
>  	if (ret)
>  		return ret;
>  
> -	ret = skl_compute_wm_levels(crtc_state, plane_state,
> -				    ddb_blocks, &wm_params, wm->wm);
> -	if (ret)
> -		return ret;
> -
> -	skl_compute_transition_wm(crtc_state, &wm_params, wm, ddb_blocks);
> +	skl_compute_wm_levels(crtc_state, plane_state, &wm_params, wm->wm);
> +	skl_compute_transition_wm(crtc_state, &wm_params, wm);
>  
>  	return 0;
>  }
> @@ -4999,7 +4911,6 @@ static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state,
>  				 enum plane_id plane_id)
>  {
>  	struct skl_plane_wm *wm = &crtc_state->wm.skl.optimal.planes[plane_id];
> -	u16 ddb_blocks = skl_ddb_entry_size(&crtc_state->wm.skl.plane_ddb_uv[plane_id]);
>  	struct skl_wm_params wm_params;
>  	int ret;
>  
> @@ -5011,10 +4922,7 @@ static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state,
>  	if (ret)
>  		return ret;
>  
> -	ret = skl_compute_wm_levels(crtc_state, plane_state,
> -				    ddb_blocks, &wm_params, wm->uv_wm);
> -	if (ret)
> -		return ret;
> +	skl_compute_wm_levels(crtc_state, plane_state, &wm_params, wm->uv_wm);
>  
>  	return 0;
>  }
> @@ -5532,13 +5440,9 @@ skl_compute_wm(struct drm_atomic_state *state)
>  	if (ret || !changed)
>  		return ret;
>  
> -	ret = skl_compute_ddb(state);
> -	if (ret)
> -		return ret;
> -
>  	/*
>  	 * Calculate WM's for all pipes that are part of this transaction.
> -	 * Note that the DDB allocation above may have added more CRTC's that
> +	 * Note that skl_ddb_add_affected_pipes may have added more CRTC's that
>  	 * weren't otherwise being modified (and set bits in dirty_pipes) if
>  	 * pipe allocations had to change.
>  	 */
> @@ -5568,6 +5472,10 @@ skl_compute_wm(struct drm_atomic_state *state)
>  		intel_cstate->update_wm_pre = true;
>  	}
>  
> +	ret = skl_compute_ddb(state);
> +	if (ret)
> +		return ret;
> +
>  	skl_print_wm_changes(intel_state);
>  
>  	return 0;
> -- 
> 2.14.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for New DDB allocation algorithm

[Patchwork]

== Series Details ==

Series: New DDB allocation algorithm
URL   : https://patchwork.freedesktop.org/series/53682/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
2a7cd471873a drm/i915: Remove a very stale FIXME
e52095e73e0d drm/i915: Don't use DDB allocation when choosing gen9 watermark method
193e7356179f drm/i915: Switch to level-based DDB allocation algorithm
-:537: CHECK:LINE_SPACING: Please don't use multiple blank lines
#537: FILE: drivers/gpu/drm/i915/intel_pm.c:5467:
+
+

total: 0 errors, 0 warnings, 1 checks, 486 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.SPARSE: warning for New DDB allocation algorithm

[Patchwork]

== Series Details ==

Series: New DDB allocation algorithm
URL   : https://patchwork.freedesktop.org/series/53682/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: Remove a very stale FIXME
Okay!

Commit: drm/i915: Don't use DDB allocation when choosing gen9 watermark method
-drivers/gpu/drm/i915/i915_fixed.h:42:43: warning: expression using sizeof(void)
-drivers/gpu/drm/i915/i915_fixed.h:42:43: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_pm.c:6752:35: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_pm.c:6752:35: warning: expression using sizeof(void)

Commit: drm/i915: Switch to level-based DDB allocation algorithm
Okay!

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for New DDB allocation algorithm

[Patchwork]

== Series Details ==

Series: New DDB allocation algorithm
URL   : https://patchwork.freedesktop.org/series/53682/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5277 -> Patchwork_11038
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_11038 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_11038, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/53682/revisions/1/mbox/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_11038:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-glk-j4005:       PASS -> DMESG-FAIL +1
    - fi-skl-iommu:       PASS -> DMESG-FAIL +1
    - fi-whl-u:           PASS -> DMESG-FAIL +1
    - fi-skl-6700hq:      PASS -> FAIL +2
    - fi-kbl-7560u:       PASS -> DMESG-FAIL +1
    - fi-cfl-8109u:       PASS -> DMESG-FAIL +1
    - fi-cfl-guc:         PASS -> DMESG-FAIL +1
    - fi-skl-6260u:       PASS -> DMESG-FAIL +1
    - fi-bxt-j4205:       PASS -> DMESG-FAIL +1
    - {fi-icl-u3}:        PASS -> DMESG-FAIL +1
    - fi-skl-6770hq:      PASS -> DMESG-FAIL

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - fi-kbl-x1275:       PASS -> DMESG-FAIL +1
    - fi-cfl-8700k:       PASS -> DMESG-FAIL +1
    - fi-apl-guc:         PASS -> DMESG-FAIL +1
    - fi-glk-dsi:         NOTRUN -> DMESG-FAIL +1
    - fi-skl-6700hq:      PASS -> DMESG-FAIL
    - fi-skl-gvtdvm:      PASS -> DMESG-FAIL +1
    - fi-bxt-dsi:         PASS -> DMESG-FAIL +1
    - {fi-kbl-7567u}:     SKIP -> DMESG-FAIL +1
    - fi-kbl-r:           PASS -> DMESG-FAIL +1

  * igt@kms_cursor_legacy@basic-flip-before-cursor-atomic:
    - fi-bxt-j4205:       PASS -> FAIL +1
    - fi-glk-j4005:       PASS -> FAIL +1
    - fi-cfl-8109u:       PASS -> FAIL +1
    - fi-whl-u:           PASS -> FAIL +1
    - fi-bxt-dsi:         PASS -> FAIL +1
    - fi-cfl-8700k:       PASS -> FAIL +1
    - fi-skl-iommu:       PASS -> FAIL +1
    - fi-kbl-x1275:       PASS -> FAIL +1

  * igt@kms_cursor_legacy@basic-flip-before-cursor-legacy:
    - fi-cfl-guc:         PASS -> FAIL +1
    - fi-glk-dsi:         NOTRUN -> FAIL +1
    - {fi-kbl-7567u}:     SKIP -> FAIL +1
    - fi-apl-guc:         PASS -> FAIL +1
    - fi-kbl-7560u:       PASS -> FAIL +1
    - fi-skl-6770hq:      PASS -> FAIL +2
    - fi-skl-6260u:       PASS -> FAIL +1
    - fi-skl-gvtdvm:      PASS -> FAIL +1
    - fi-kbl-r:           PASS -> FAIL +1

  
#### Warnings ####

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
    - {fi-kbl-7567u}:     SKIP -> PASS +29

  
Known issues
------------

  Here are the changes found in Patchwork_11038 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@cs-compute:
    - fi-kbl-8809g:       NOTRUN -> FAIL [fdo#108094]

  * igt@amdgpu/amd_prime@amd-to-i915:
    - fi-kbl-8809g:       NOTRUN -> FAIL [fdo#107341]

  * igt@i915_selftest@live_hangcheck:
    - fi-bwr-2160:        PASS -> DMESG-FAIL [fdo#108735]

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-skl-6700k2:      PASS -> INCOMPLETE [fdo#104108] / [fdo#105524] / [k.org#199541]

  * igt@kms_cursor_legacy@basic-flip-before-cursor-atomic:
    - {fi-icl-u3}:        PASS -> FAIL [fdo#107736] +1

  * {igt@runner@aborted}:
    - {fi-icl-u3}:        NOTRUN -> FAIL [fdo#108315]

  
#### Possible fixes ####

  * igt@amdgpu/amd_basic@userptr:
    - fi-kbl-8809g:       DMESG-WARN -> PASS

  * igt@gem_ctx_create@basic-files:
    - fi-bsw-kefka:       FAIL [fdo#108656] -> PASS

  * igt@gem_mmap_gtt@basic-small-copy:
    - fi-glk-dsi:         INCOMPLETE [fdo#103359] / [k.org#198133] -> PASS

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       DMESG-WARN [fdo#102614] -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
    - fi-cfl-8109u:       INCOMPLETE [fdo#106070] / [fdo#108126] -> PASS

  
#### Warnings ####

  * igt@i915_selftest@live_contexts:
    - {fi-icl-u3}:        DMESG-FAIL [fdo#108569] -> INCOMPLETE [fdo#108315]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105524]: https://bugs.freedesktop.org/show_bug.cgi?id=105524
  [fdo#106070]: https://bugs.freedesktop.org/show_bug.cgi?id=106070
  [fdo#107341]: https://bugs.freedesktop.org/show_bug.cgi?id=107341
  [fdo#107736]: https://bugs.freedesktop.org/show_bug.cgi?id=107736
  [fdo#108094]: https://bugs.freedesktop.org/show_bug.cgi?id=108094
  [fdo#108126]: https://bugs.freedesktop.org/show_bug.cgi?id=108126
  [fdo#108315]: https://bugs.freedesktop.org/show_bug.cgi?id=108315
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108656]: https://bugs.freedesktop.org/show_bug.cgi?id=108656
  [fdo#108735]: https://bugs.freedesktop.org/show_bug.cgi?id=108735
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133
  [k.org#199541]: https://bugzilla.kernel.org/show_bug.cgi?id=199541


Participating hosts (48 -> 42)
------------------------------

  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-skl-guc fi-byt-squawks fi-bsw-cyan fi-pnv-d510 


Build changes
-------------

    * Linux: CI_DRM_5277 -> Patchwork_11038

  CI_DRM_5277: 13e7a0e59420cbdafca1cedbe2d9c136180aa689 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4743: edb2db2cf2b6665d7ba3fa9117263302f6307a4f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11038: 193e7356179f4f20f01a3401bb1a1be712ef7910 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

193e7356179f drm/i915: Switch to level-based DDB allocation algorithm
e52095e73e0d drm/i915: Don't use DDB allocation when choosing gen9 watermark method
2a7cd471873a drm/i915: Remove a very stale FIXME

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11038/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.SPARSE: warning for New DDB allocation algorithm (rev2)

[Patchwork]

== Series Details ==

Series: New DDB allocation algorithm (rev2)
URL   : https://patchwork.freedesktop.org/series/53682/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: Remove a very stale FIXME
Okay!

Commit: drm/i915: Don't use DDB allocation when choosing gen9 watermark method
-drivers/gpu/drm/i915/i915_fixed.h:42:43: warning: expression using sizeof(void)
-drivers/gpu/drm/i915/i915_fixed.h:42:43: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_pm.c:6752:35: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_pm.c:6752:35: warning: expression using sizeof(void)

Commit: drm/i915: Switch to level-based DDB allocation algorithm (v2)
Okay!

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for New DDB allocation algorithm (rev2)

[Patchwork]

== Series Details ==

Series: New DDB allocation algorithm (rev2)
URL   : https://patchwork.freedesktop.org/series/53682/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5279 -> Patchwork_11041
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/53682/revisions/2/mbox/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_11041:

### IGT changes ###

#### Possible regressions ####

  * {igt@runner@aborted}:
    - fi-cfl-8109u:       NOTRUN -> FAIL

  
Known issues
------------

  Here are the changes found in Patchwork_11041 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@cs-compute:
    - fi-kbl-8809g:       NOTRUN -> FAIL [fdo#108094]

  * igt@amdgpu/amd_prime@amd-to-i915:
    - fi-kbl-8809g:       NOTRUN -> FAIL [fdo#107341]

  * igt@gem_ctx_create@basic-files:
    - fi-bsw-kefka:       PASS -> FAIL [fdo#108656]

  * igt@i915_module_load@reload:
    - fi-blb-e6850:       NOTRUN -> INCOMPLETE [fdo#107718]

  * {igt@runner@aborted}:
    - {fi-icl-y}:         NOTRUN -> FAIL [fdo#108070]

  
#### Possible fixes ####

  * igt@amdgpu/amd_basic@userptr:
    - fi-kbl-8809g:       DMESG-WARN -> PASS

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#107341]: https://bugs.freedesktop.org/show_bug.cgi?id=107341
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108070]: https://bugs.freedesktop.org/show_bug.cgi?id=108070
  [fdo#108094]: https://bugs.freedesktop.org/show_bug.cgi?id=108094
  [fdo#108656]: https://bugs.freedesktop.org/show_bug.cgi?id=108656


Participating hosts (46 -> 42)
------------------------------

  Additional (1): fi-icl-y 
  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-u3 


Build changes
-------------

    * Linux: CI_DRM_5279 -> Patchwork_11041

  CI_DRM_5279: 8c3dbdac21ef5357bfa9b11da9b2bd1baedc4962 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4743: edb2db2cf2b6665d7ba3fa9117263302f6307a4f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11041: 0df16827bdf5f904893def72fcd9f2aab3f7fec0 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

0df16827bdf5 drm/i915: Switch to level-based DDB allocation algorithm (v2)
c26e16a3f466 drm/i915: Don't use DDB allocation when choosing gen9 watermark method
041050b5e617 drm/i915: Remove a very stale FIXME

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11041/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for New DDB allocation algorithm (rev2)

[Patchwork]

== Series Details ==

Series: New DDB allocation algorithm (rev2)
URL   : https://patchwork.freedesktop.org/series/53682/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5279_full -> Patchwork_11041_full
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with Patchwork_11041_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_11041_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_11041_full:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_plane_alpha_blend@pipe-c-alpha-transparant-fb:
    - {shard-iclb}:       PASS -> FAIL +11

  
#### Warnings ####

  * igt@kms_plane_lowres@pipe-c-tiling-yf:
    - {shard-iclb}:       PASS -> SKIP +1

  * igt@tools_test@tools_test:
    - shard-glk:          PASS -> SKIP

  
Known issues
------------

  Here are the changes found in Patchwork_11041_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@pi-ringfull-blt:
    - shard-skl:          NOTRUN -> FAIL [fdo#103158]

  * igt@gem_softpin@noreloc-s3:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108] / [fdo#107773]

  * igt@i915_suspend@fence-restore-untiled:
    - {shard-iclb}:       PASS -> INCOMPLETE [fdo#107713]

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-glk:          PASS -> FAIL [fdo#108145]

  * igt@kms_cursor_crc@cursor-128x128-sliding:
    - shard-apl:          PASS -> FAIL [fdo#103232] +1

  * igt@kms_cursor_crc@cursor-256x256-sliding:
    - {shard-iclb}:       NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-256x85-sliding:
    - shard-glk:          PASS -> FAIL [fdo#103232] +2

  * igt@kms_cursor_crc@cursor-size-change:
    - {shard-iclb}:       PASS -> FAIL [fdo#103232]

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
    - shard-hsw:          PASS -> FAIL [fdo#105767]

  * igt@kms_draw_crc@draw-method-rgb565-mmap-wc-ytiled:
    - shard-skl:          PASS -> FAIL [fdo#103184]

  * igt@kms_draw_crc@draw-method-xrgb8888-render-untiled:
    - shard-skl:          NOTRUN -> FAIL [fdo#103184]

  * igt@kms_fbcon_fbt@psr:
    - shard-skl:          NOTRUN -> FAIL [fdo#107882]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
    - shard-glk:          PASS -> FAIL [fdo#103167] +1

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-mmap-gtt:
    - shard-apl:          SKIP -> INCOMPLETE [fdo#103927]

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-cpu:
    - shard-skl:          NOTRUN -> FAIL [fdo#105682] +1

  * igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-render:
    - shard-skl:          PASS -> FAIL [fdo#103167] +3

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-mmap-cpu:
    - shard-skl:          NOTRUN -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-move:
    - {shard-iclb}:       PASS -> FAIL [fdo#103167] +3

  * igt@kms_plane@pixel-format-pipe-a-planes:
    - shard-skl:          NOTRUN -> FAIL [fdo#103166]

  * igt@kms_plane@pixel-format-pipe-c-planes:
    - {shard-iclb}:       NOTRUN -> FAIL [fdo#103166]

  * {igt@kms_plane@pixel-format-pipe-c-planes-source-clamping}:
    - shard-apl:          PASS -> FAIL [fdo#108948]

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          PASS -> FAIL [fdo#107815] / [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-b-alpha-transparant-fb:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] +2

  * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          PASS -> FAIL [fdo#107815]

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-yf:
    - shard-glk:          PASS -> FAIL [fdo#103166]

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
    - shard-apl:          PASS -> FAIL [fdo#103166]
    - {shard-iclb}:       PASS -> FAIL [fdo#103166]

  * igt@kms_sysfs_edid_timing:
    - shard-skl:          NOTRUN -> FAIL [fdo#100047]

  * igt@pm_backlight@fade_with_suspend:
    - shard-skl:          NOTRUN -> FAIL [fdo#107847]

  * igt@pm_rpm@basic-rte:
    - {shard-iclb}:       PASS -> DMESG-WARN [fdo#108654]

  * igt@pm_rpm@debugfs-read:
    - shard-skl:          PASS -> INCOMPLETE [fdo#107807] +2

  
#### Possible fixes ####

  * igt@gem_userptr_blits@readonly-unsync:
    - shard-skl:          TIMEOUT [fdo#108887] -> PASS

  * igt@gem_workarounds@suspend-resume-fd:
    - shard-skl:          INCOMPLETE [fdo#104108] / [fdo#107773] -> PASS

  * igt@kms_atomic@test_only:
    - {shard-iclb}:       DMESG-WARN [fdo#107724] -> PASS +22

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-apl:          FAIL [fdo#105458] / [fdo#106510] -> PASS +1

  * igt@kms_cursor_crc@cursor-128x128-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS

  * igt@kms_draw_crc@draw-method-xrgb8888-mmap-gtt-ytiled:
    - {shard-iclb}:       WARN [fdo#108336] -> PASS +3

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-move:
    - {shard-iclb}:       DMESG-FAIL [fdo#107724] -> PASS +10

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-cpu:
    - shard-glk:          FAIL [fdo#103167] -> PASS

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-mmap-wc:
    - {shard-iclb}:       FAIL [fdo#103167] -> PASS +3

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-cpu:
    - {shard-iclb}:       DMESG-WARN [fdo#107724] / [fdo#108336] -> PASS +10

  * {igt@kms_plane@pixel-format-pipe-b-planes-source-clamping}:
    - shard-apl:          FAIL [fdo#108948] -> PASS

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - {shard-iclb}:       INCOMPLETE [fdo#107713] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
    - {shard-iclb}:       FAIL [fdo#103166] -> PASS

  * igt@kms_setmode@basic:
    - shard-apl:          FAIL [fdo#99912] -> PASS

  * igt@pm_rpm@legacy-planes-dpms:
    - {shard-iclb}:       INCOMPLETE [fdo#108840] -> PASS

  * igt@pm_rpm@modeset-lpsp:
    - shard-skl:          INCOMPLETE [fdo#107807] -> PASS +1

  * igt@pm_rpm@modeset-non-lpsp-stress-no-wait:
    - {shard-iclb}:       INCOMPLETE [fdo#108840] -> SKIP

  
#### Warnings ####

  * igt@kms_cursor_crc@cursor-256x85-onscreen:
    - {shard-iclb}:       DMESG-WARN [fdo#107724] / [fdo#108336] -> FAIL [fdo#103232]

  * igt@kms_plane@pixel-format-pipe-a-planes:
    - {shard-iclb}:       DMESG-WARN [fdo#107724] / [fdo#108336] -> FAIL [fdo#103166]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#100047]: https://bugs.freedesktop.org/show_bug.cgi?id=100047
  [fdo#103158]: https://bugs.freedesktop.org/show_bug.cgi?id=103158
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105458]: https://bugs.freedesktop.org/show_bug.cgi?id=105458
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
  [fdo#106510]: https://bugs.freedesktop.org/show_bug.cgi?id=106510
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#107847]: https://bugs.freedesktop.org/show_bug.cgi?id=107847
  [fdo#107882]: https://bugs.freedesktop.org/show_bug.cgi?id=107882
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108336]: https://bugs.freedesktop.org/show_bug.cgi?id=108336
  [fdo#108654]: https://bugs.freedesktop.org/show_bug.cgi?id=108654
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#108887]: https://bugs.freedesktop.org/show_bug.cgi?id=108887
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts


Build changes
-------------

    * Linux: CI_DRM_5279 -> Patchwork_11041

  CI_DRM_5279: 8c3dbdac21ef5357bfa9b11da9b2bd1baedc4962 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4743: edb2db2cf2b6665d7ba3fa9117263302f6307a4f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11041: 0df16827bdf5f904893def72fcd9f2aab3f7fec0 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11041/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx