New DDB allocation algorithm
Subsystems (2)
cover New DDB allocation algorithm
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(-)
Signature · 6 lines
2.14.4 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 1/3 drm/i915: Remove a very stale FIXME
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.
--- drivers/gpu/drm/i915/intel_pm.c | 4 ---- 1 file changed, 4 deletions(-)
drivers/gpu/drm/i915/intel_pm.c +0 -4
@@ -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 =
Signature · 6 lines
2.14.4 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 1/3] drm/i915: Remove a very stale FIXME
Quoted 3 lines · 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.
Identical patch already pushed.
Quoted 28 lines
Signed-off-by Matt Roper <[email protected]>--- drivers/gpu/drm/i915/intel_pm.c | 4 ---- 1 file changed, 4 deletions(-)drivers/gpu/drm/i915/intel_pm.c +0
-4@@ -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 =Signature · 6 lines
2.14.4 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 6 lines
Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 2/3 drm/i915: Don't use DDB allocation when choosing gen9 watermark method
In reply to Matt RoperThe 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).
--- drivers/gpu/drm/i915/intel_pm.c | 7 ------- 1 file changed, 7 deletions(-)
drivers/gpu/drm/i915/intel_pm.c +0 -7
@@ -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))
Signature · 6 lines
2.14.4 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 3/3 drm/i915: Switch to level-based DDB allocation algorithm
In reply to Matt RoperThe 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
--- drivers/gpu/drm/i915/intel_pm.c | 345 ++++++++++++++-------------------------- 1 file changed, 121 insertions(+), 224 deletions(-)
drivers/gpu/drm/i915/intel_pm.c +121 -224
@@ -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 ;
Signature · 6 lines
2.14.4 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 3/3 drm/i915: Switch to level-based DDB allocation algorithm (v2)
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
--- drivers/gpu/drm/i915/intel_pm.c | 350 +++++++++++++++------------------------- 1 file changed, 129 insertions(+), 221 deletions(-)
drivers/gpu/drm/i915/intel_pm.c +129 -221
@@ -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 ;
Signature · 6 lines
2.14.4 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 3/3] drm/i915: Switch to level-based DDB allocation algorithm (v2)
Quoted 12 lines · 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.
Quoted 8 lines
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.
Quoted 129 lines
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 <[email protected]>--- drivers/gpu/drm/i915/intel_pm.c | 350 +++++++++++++++------------------------- 1 file changed, 129 insertions(+), 221 deletions(-)drivers/gpu/drm/i915/intel_pm.c +0
-96@@ -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 */ )
drivers/gpu/drm/i915/intel_pm.c skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, · line 4319
Quoted 8 lines
drivers/gpu/drm/i915/intel_pm.c +2
-3@@ -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.
Quoted 93 lines
drivers/gpu/drm/i915/intel_pm.c +37
-26+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];
drivers/gpu/drm/i915/intel_pm.c skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, · line 4411
Quoted 10 lines
drivers/gpu/drm/i915/intel_pm.c +3
-7-/* -* 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.
Quoted 43 lines
drivers/gpu/drm/i915/intel_pm.c +21
-16-/* 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];
drivers/gpu/drm/i915/intel_pm.c skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, · line 4448
Quoted 10 lines
drivers/gpu/drm/i915/intel_pm.c +10
-0+ } + } + +/* +* 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.
Quoted 7 lines
drivers/gpu/drm/i915/intel_pm.c +6
-0+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 ; } + }
drivers/gpu/drm/i915/intel_pm.c skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, · line 4464
Quoted 10 lines
drivers/gpu/drm/i915/intel_pm.c +8
-1- 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.
Quoted 38 lines
drivers/gpu/drm/i915/intel_pm.c +6
-11}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)) &&
drivers/gpu/drm/i915/intel_pm.c static int skl_compute_plane_wm(const struct intel_crtc_state *cstate, · line 4833
Quoted 6 lines
drivers/gpu/drm/i915/intel_pm.c +0
-3@@ -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.
Quoted 105 lines
drivers/gpu/drm/i915/intel_pm.c +21
-41-/* -* 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];
drivers/gpu/drm/i915/intel_pm.c static void skl_compute_transition_wm(const struct intel_crtc_state *cstate, · line 4896
Quoted 10 lines
drivers/gpu/drm/i915/intel_pm.c +6
-1-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".
Quoted 70 lines
drivers/gpu/drm/i915/intel_pm.c +9
-16+ 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 ;Signature · 6 lines
2.14.4 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 6 lines
Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Patchwork
CI✗ Fi.CI.CHECKPATCH: warning for New DDB allocation algorithm
In reply to Matt Roper== Series Details ==
== 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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Patchwork
CI✗ Fi.CI.SPARSE: warning for New DDB allocation algorithm
In reply to Matt Roper== Series Details ==
== 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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Patchwork
CI✗ Fi.CI.BAT: failure for New DDB allocation algorithm
In reply to Matt Roper== Series Details ==
== 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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Patchwork
CI✗ Fi.CI.SPARSE: warning for New DDB allocation algorithm (rev2)
In reply to Matt Roper== Series Details ==
== 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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Patchwork
CI✓ Fi.CI.BAT: success for New DDB allocation algorithm (rev2)
In reply to Matt Roper== Series Details ==
== 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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Patchwork
CI✓ Fi.CI.IGT: success for New DDB allocation algorithm (rev2)
In reply to Matt Roper== Series Details ==
== 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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx