Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

drm/i915: Fix wrong CDCLK adjustment changes

Previous patch didn't take into account all pipes
but only those in state, which could cause wrong
CDCLK conclcusions and calculations.
Also there was a severe issue with min_cdclk being
assigned to 0 every compare cycle.

Too bad this was found by me only after merge.
This could be also causing the issues in test, however
not clear - anyway marking this as fixing the
"Adjust CDCLK accordingly to our DBuf bw needs".

v2: - s/pipe/crtc->pipe/
- save a bit of instructions by
skipping inactive pipes, without
getting 0 DBuf slice mask for it.

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Fixes: cd1915460861 ("drm/i915: Adjust CDCLK accordingly to our DBuf bw needs")
Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200601173058.5084-1-stanislav.lisovskiy@intel.com

authored by

Stanislav Lisovskiy and committed by
Manasi Navare
19aefbc7 b8226d62

+57 -44
+33 -19
drivers/gpu/drm/i915/display/intel_bw.c
··· 437 437 struct intel_crtc *crtc; 438 438 int max_bw = 0; 439 439 int slice_id; 440 + enum pipe pipe; 440 441 int i; 441 442 442 443 for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) { ··· 448 447 if (IS_ERR(new_bw_state)) 449 448 return PTR_ERR(new_bw_state); 450 449 450 + old_bw_state = intel_atomic_get_old_bw_state(state); 451 + 451 452 crtc_bw = &new_bw_state->dbuf_bw[crtc->pipe]; 452 453 453 454 memset(&crtc_bw->used_bw, 0, sizeof(crtc_bw->used_bw)); 455 + 456 + if (!crtc_state->hw.active) 457 + continue; 454 458 455 459 for_each_plane_id_on_crtc(crtc, plane_id) { 456 460 const struct skl_ddb_entry *plane_alloc = ··· 484 478 for_each_dbuf_slice_in_mask(slice_id, dbuf_mask) 485 479 crtc_bw->used_bw[slice_id] += data_rate; 486 480 } 481 + } 482 + 483 + if (!old_bw_state) 484 + return 0; 485 + 486 + for_each_pipe(dev_priv, pipe) { 487 + struct intel_dbuf_bw *crtc_bw; 488 + 489 + crtc_bw = &new_bw_state->dbuf_bw[pipe]; 487 490 488 491 for_each_dbuf_slice(slice_id) { 489 492 /* ··· 505 490 */ 506 491 max_bw += crtc_bw->used_bw[slice_id]; 507 492 } 508 - 509 - new_bw_state->min_cdclk = max_bw / 64; 510 - 511 - old_bw_state = intel_atomic_get_old_bw_state(state); 512 493 } 513 494 514 - if (!old_bw_state) 515 - return 0; 495 + new_bw_state->min_cdclk = max_bw / 64; 516 496 517 497 if (new_bw_state->min_cdclk != old_bw_state->min_cdclk) { 518 498 int ret = intel_atomic_lock_global_state(&new_bw_state->base); ··· 521 511 522 512 int intel_bw_calc_min_cdclk(struct intel_atomic_state *state) 523 513 { 524 - int i; 514 + struct drm_i915_private *dev_priv = to_i915(state->base.dev); 515 + struct intel_bw_state *new_bw_state = NULL; 516 + struct intel_bw_state *old_bw_state = NULL; 525 517 const struct intel_crtc_state *crtc_state; 526 518 struct intel_crtc *crtc; 527 519 int min_cdclk = 0; 528 - struct intel_bw_state *new_bw_state = NULL; 529 - struct intel_bw_state *old_bw_state = NULL; 520 + enum pipe pipe; 521 + int i; 530 522 531 523 for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) { 532 - struct intel_cdclk_state *cdclk_state; 533 - 534 524 new_bw_state = intel_atomic_get_bw_state(state); 535 525 if (IS_ERR(new_bw_state)) 536 526 return PTR_ERR(new_bw_state); 537 - 538 - cdclk_state = intel_atomic_get_cdclk_state(state); 539 - if (IS_ERR(cdclk_state)) 540 - return PTR_ERR(cdclk_state); 541 - 542 - min_cdclk = max(cdclk_state->min_cdclk[crtc->pipe], min_cdclk); 543 - 544 - new_bw_state->min_cdclk = min_cdclk; 545 527 546 528 old_bw_state = intel_atomic_get_old_bw_state(state); 547 529 } 548 530 549 531 if (!old_bw_state) 550 532 return 0; 533 + 534 + for_each_pipe(dev_priv, pipe) { 535 + struct intel_cdclk_state *cdclk_state; 536 + 537 + cdclk_state = intel_atomic_get_new_cdclk_state(state); 538 + if (!cdclk_state) 539 + return 0; 540 + 541 + min_cdclk = max(cdclk_state->min_cdclk[pipe], min_cdclk); 542 + } 543 + 544 + new_bw_state->min_cdclk = min_cdclk; 551 545 552 546 if (new_bw_state->min_cdclk != old_bw_state->min_cdclk) { 553 547 int ret = intel_atomic_lock_global_state(&new_bw_state->base);
+11 -8
drivers/gpu/drm/i915/display/intel_cdclk.c
··· 2084 2084 static int intel_compute_min_cdclk(struct intel_cdclk_state *cdclk_state) 2085 2085 { 2086 2086 struct intel_atomic_state *state = cdclk_state->base.state; 2087 + struct drm_i915_private *dev_priv = to_i915(state->base.dev); 2088 + struct intel_bw_state *bw_state = NULL; 2087 2089 struct intel_crtc *crtc; 2088 2090 struct intel_crtc_state *crtc_state; 2089 2091 int min_cdclk, i; 2092 + enum pipe pipe; 2090 2093 2091 2094 for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) { 2092 2095 int ret; ··· 2097 2094 min_cdclk = intel_crtc_compute_min_cdclk(crtc_state); 2098 2095 if (min_cdclk < 0) 2099 2096 return min_cdclk; 2097 + 2098 + bw_state = intel_atomic_get_bw_state(state); 2099 + if (IS_ERR(bw_state)) 2100 + return PTR_ERR(bw_state); 2100 2101 2101 2102 if (cdclk_state->min_cdclk[i] == min_cdclk) 2102 2103 continue; ··· 2113 2106 } 2114 2107 2115 2108 min_cdclk = cdclk_state->force_min_cdclk; 2109 + for_each_pipe(dev_priv, pipe) { 2110 + min_cdclk = max(cdclk_state->min_cdclk[pipe], min_cdclk); 2116 2111 2117 - for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) { 2118 - struct intel_bw_state *bw_state; 2119 - 2120 - min_cdclk = max(cdclk_state->min_cdclk[crtc->pipe], min_cdclk); 2121 - 2122 - bw_state = intel_atomic_get_bw_state(state); 2123 - if (IS_ERR(bw_state)) 2124 - return PTR_ERR(bw_state); 2112 + if (!bw_state) 2113 + continue; 2125 2114 2126 2115 min_cdclk = max(bw_state->min_cdclk, min_cdclk); 2127 2116 }
+13 -17
drivers/gpu/drm/i915/display/intel_display.c
··· 14716 14716 bool *need_cdclk_calc) 14717 14717 { 14718 14718 struct drm_i915_private *dev_priv = to_i915(state->base.dev); 14719 - int i; 14720 - struct intel_plane_state *plane_state; 14721 - struct intel_plane *plane; 14722 - int ret; 14723 14719 struct intel_cdclk_state *new_cdclk_state; 14724 - struct intel_crtc_state *new_crtc_state; 14725 - struct intel_crtc *crtc; 14720 + struct intel_plane_state *plane_state; 14721 + struct intel_bw_state *new_bw_state; 14722 + struct intel_plane *plane; 14723 + int min_cdclk = 0; 14724 + enum pipe pipe; 14725 + int ret; 14726 + int i; 14726 14727 /* 14727 14728 * active_planes bitmask has been updated, and potentially 14728 14729 * affected planes are part of the state. We can now ··· 14744 14743 if (ret) 14745 14744 return ret; 14746 14745 14747 - if (!new_cdclk_state) 14746 + new_bw_state = intel_atomic_get_new_bw_state(state); 14747 + 14748 + if (!new_cdclk_state || !new_bw_state) 14748 14749 return 0; 14749 14750 14750 - for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { 14751 - struct intel_bw_state *bw_state; 14752 - int min_cdclk = 0; 14753 - 14754 - min_cdclk = max(new_cdclk_state->min_cdclk[crtc->pipe], min_cdclk); 14755 - 14756 - bw_state = intel_atomic_get_bw_state(state); 14757 - if (IS_ERR(bw_state)) 14758 - return PTR_ERR(bw_state); 14751 + for_each_pipe(dev_priv, pipe) { 14752 + min_cdclk = max(new_cdclk_state->min_cdclk[pipe], min_cdclk); 14759 14753 14760 14754 /* 14761 14755 * Currently do this change only if we need to increase 14762 14756 */ 14763 - if (bw_state->min_cdclk > min_cdclk) 14757 + if (new_bw_state->min_cdclk > min_cdclk) 14764 14758 *need_cdclk_calc = true; 14765 14759 } 14766 14760