[Intel-gfx] [PATCH 0/2] Implement MBUS state changes according to spec

[Intel-gfx] [PATCH 0/2] Implement MBUS state changes according to spec

[Stanislav Lisovskiy]

We were previsously updating MBUS/DBOX/DBUF CTL registers incorrectly,
doing it in a single place, even though BSpec instructs to do it before
or after DDB reallocation depending on the scenario, also we missed
a vblank wait in one scenario, which forced us to use a full modeset
in all cases when MBUS join state needed to be changed, preventing
fastset to be done in certain cases.
This patch series attempts to fix that.

Stanislav Lisovskiy (2):
  drm/i915: Update mbus in intel_dbuf_mbus_update and do it properly
  drm/i915: Implement vblank synchronized MBUS join changes

 drivers/gpu/drm/i915/display/intel_display.c |  2 -
 drivers/gpu/drm/i915/display/skl_watermark.c | 41 +++++++++++++++-----
 2 files changed, 32 insertions(+), 11 deletions(-)

-- 
2.37.3

[Intel-gfx] [PATCH 1/2] drm/i915: Update mbus in intel_dbuf_mbus_update and do it properly

[Stanislav Lisovskiy]

According to BSpec we need to do correspondent MBUS updates before
or after DBUF reallocation, depending on whether we are reducing
or increasing amount of pipes(typical scenario is swithing between
multiple and single displays).

As of BSpec 49213 if we are swithing from multiple to single display
MBUS registers should be updated with correspondent values _before_
Dbuf reallocation happens, however if we are switching from single
display to multiple then it should happen _after_ DDB reallocation(i.e
plane programming).

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 drivers/gpu/drm/i915/display/skl_watermark.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
index 063929a42a42f..af99f2abd8446 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.c
+++ b/drivers/gpu/drm/i915/display/skl_watermark.c
@@ -3474,7 +3474,7 @@ int intel_dbuf_init(struct drm_i915_private *i915)
  * Configure MBUS_CTL and all DBUF_CTL_S of each slice to join_mbus state before
  * update the request state of all DBUS slices.
  */
-static void update_mbus_pre_enable(struct intel_atomic_state *state)
+static void intel_dbuf_mbus_update(struct intel_atomic_state *state)
 {
 	struct drm_i915_private *i915 = to_i915(state->base.dev);
 	u32 mbus_ctl, dbuf_min_tracker_val;
@@ -3524,7 +3524,9 @@ void intel_dbuf_pre_plane_update(struct intel_atomic_state *state)
 
 	WARN_ON(!new_dbuf_state->base.changed);
 
-	update_mbus_pre_enable(state);
+	if (hweight8(new_dbuf_state->active_pipes) <= hweight8(old_dbuf_state->active_pipes))
+		intel_dbuf_mbus_update(state);
+
 	gen9_dbuf_slices_update(i915,
 				old_dbuf_state->enabled_slices |
 				new_dbuf_state->enabled_slices);
@@ -3545,6 +3547,9 @@ void intel_dbuf_post_plane_update(struct intel_atomic_state *state)
 
 	WARN_ON(!new_dbuf_state->base.changed);
 
+	if (hweight8(new_dbuf_state->active_pipes) > hweight8(old_dbuf_state->active_pipes))
+		intel_dbuf_mbus_update(state);
+
 	gen9_dbuf_slices_update(i915,
 				new_dbuf_state->enabled_slices);
 }
-- 
2.37.3

[Intel-gfx] [PATCH 2/2] drm/i915: Implement vblank synchronized MBUS join changes

[Stanislav Lisovskiy]

Currently we can't change MBUS join status without doing a modeset,
because we are lacking mechanism to synchronize those with vblank.
However then this means that we can't do a fastset, if there is a need
to change MBUS join state. Fix that by implementing such change.
We already call correspondent check and update at pre_plane dbuf update,
so the only thing left is to have a non-modeset version of that.
If active pipes stay the same then fastset is possible and only MBUS
join state/ddb allocation updates would be committed.

v2: Implement additional changes according to BSpec.
    Vblank wait is needed after MBus/Dbuf programming in case if
    no modeset is done and we are switching from single to multiple
    displays, i.e mbus join state switches from "joined" to  "non-joined"
    state. Otherwise vblank wait is not needed according to spec.

v3: Split mbus and dbox programming into to pre/post plane update parts,
    how it should be done according to BSpec.

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Tested-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c |  2 --
 drivers/gpu/drm/i915/display/skl_watermark.c | 36 +++++++++++++++-----
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 8c81206ce90d7..249ba955cce2a 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7041,9 +7041,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 	}
 
 	intel_encoders_update_prepare(state);
-
 	intel_dbuf_pre_plane_update(state);
-	intel_mbus_dbox_update(state);
 
 	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
 		if (new_crtc_state->do_async_flip)
diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
index af99f2abd8446..b5c5fa9ecf43c 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.c
+++ b/drivers/gpu/drm/i915/display/skl_watermark.c
@@ -2614,13 +2614,6 @@ skl_compute_ddb(struct intel_atomic_state *state)
 		if (ret)
 			return ret;
 
-		if (old_dbuf_state->joined_mbus != new_dbuf_state->joined_mbus) {
-			/* TODO: Implement vblank synchronized MBUS joining changes */
-			ret = intel_modeset_all_pipes(state, "MBUS joining change");
-			if (ret)
-				return ret;
-		}
-
 		drm_dbg_kms(&i915->drm,
 			    "Enabled dbuf slices 0x%x -> 0x%x (total dbuf slices 0x%x), mbus joined? %s->%s\n",
 			    old_dbuf_state->enabled_slices,
@@ -3524,8 +3517,15 @@ void intel_dbuf_pre_plane_update(struct intel_atomic_state *state)
 
 	WARN_ON(!new_dbuf_state->base.changed);
 
-	if (hweight8(new_dbuf_state->active_pipes) <= hweight8(old_dbuf_state->active_pipes))
+	/*
+	 * Switching from multiple to single display scenario.
+	 * Also we put here "<=" instead of "<" for suboptimal cases, when
+	 * we switch from single => single display, enabling mbus join.
+	 */
+	if (hweight8(new_dbuf_state->active_pipes) <= hweight8(old_dbuf_state->active_pipes)) {
 		intel_dbuf_mbus_update(state);
+		intel_mbus_dbox_update(state);
+	}
 
 	gen9_dbuf_slices_update(i915,
 				old_dbuf_state->enabled_slices |
@@ -3547,8 +3547,26 @@ void intel_dbuf_post_plane_update(struct intel_atomic_state *state)
 
 	WARN_ON(!new_dbuf_state->base.changed);
 
-	if (hweight8(new_dbuf_state->active_pipes) > hweight8(old_dbuf_state->active_pipes))
+	/*
+	 * Switching from single to multiple display scenario
+	 */
+	if (hweight8(new_dbuf_state->active_pipes) > hweight8(old_dbuf_state->active_pipes)) {
+		struct intel_crtc *crtc;
+		struct intel_crtc_state *old_crtc_state;
+		int i;
 		intel_dbuf_mbus_update(state);
+		intel_mbus_dbox_update(state);
+
+		for_each_old_intel_crtc_in_state(state, crtc, old_crtc_state, i) {
+			/*
+			 * According to BSpec we should wait vblank on previously single display
+			 */
+			if (!old_crtc_state->hw.active)
+				continue;
+
+			intel_crtc_wait_for_next_vblank(crtc);
+		}
+	}
 
 	gen9_dbuf_slices_update(i915,
 				new_dbuf_state->enabled_slices);
-- 
2.37.3

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Implement MBUS state changes according to spec

[Patchwork]

== Series Details ==

Series: Implement MBUS state changes according to spec
URL   : https://patchwork.freedesktop.org/series/122641/
State : warning

== Summary ==

Error: dim checkpatch failed
/home/kbuild/linux/maintainer-tools/dim: line 50: /home/kbuild/.dimrc: No such file or directory

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for Implement MBUS state changes according to spec

[Patchwork]

== Series Details ==

Series: Implement MBUS state changes according to spec
URL   : https://patchwork.freedesktop.org/series/122641/
State : warning

== Summary ==

Error: dim sparse failed
/home/kbuild2/linux/maintainer-tools/dim: line 50: /home/kbuild2/.dimrc: No such file or directory

[Intel-gfx] ✗ Fi.CI.BAT: failure for Implement MBUS state changes according to spec

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 10190 bytes --]

== Series Details ==

Series: Implement MBUS state changes according to spec
URL   : https://patchwork.freedesktop.org/series/122641/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_13536 -> Patchwork_122641v1
====================================================

Summary
-------

  **FAILURE**

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

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122641v1/index.html

Participating hosts (38 -> 38)
------------------------------

  Additional (1): fi-pnv-d510 
  Missing    (1): fi-snb-2520m 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size:
    - bat-rplp-1:         [PASS][1] -> [DMESG-WARN][2] +30 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13536/bat-rplp-1/igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122641v1/bat-rplp-1/igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size.html

  * igt@kms_cursor_legacy@basic-flip-before-cursor-legacy:
    - bat-mtlp-8:         [PASS][3] -> [DMESG-FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13536/bat-mtlp-8/igt@kms_cursor_legacy@basic-flip-before-cursor-legacy.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122641v1/bat-mtlp-8/igt@kms_cursor_legacy@basic-flip-before-cursor-legacy.html

  * igt@kms_cursor_legacy@basic-flip-before-cursor-varying-size:
    - bat-mtlp-8:         [PASS][5] -> [DMESG-WARN][6] +44 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13536/bat-mtlp-8/igt@kms_cursor_legacy@basic-flip-before-cursor-varying-size.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122641v1/bat-mtlp-8/igt@kms_cursor_legacy@basic-flip-before-cursor-varying-size.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@d-dp3:
    - bat-adlp-6:         [PASS][7] -> [ABORT][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13536/bat-adlp-6/igt@kms_flip@basic-flip-vs-wf_vblank@d-dp3.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122641v1/bat-adlp-6/igt@kms_flip@basic-flip-vs-wf_vblank@d-dp3.html

  * igt@kms_force_connector_basic@force-connector-state:
    - bat-mtlp-6:         [PASS][9] -> [DMESG-WARN][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13536/bat-mtlp-6/igt@kms_force_connector_basic@force-connector-state.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122641v1/bat-mtlp-6/igt@kms_force_connector_basic@force-connector-state.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_module_load@load:
    - bat-adlp-6:         [PASS][11] -> [DMESG-WARN][12] ([i915#7507]) +15 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13536/bat-adlp-6/igt@i915_module_load@load.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122641v1/bat-adlp-6/igt@i915_module_load@load.html

  * igt@i915_selftest@live@execlists:
    - fi-bsw-n3050:       [PASS][13] -> [ABORT][14] ([i915#7911] / [i915#7913])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13536/fi-bsw-n3050/igt@i915_selftest@live@execlists.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122641v1/fi-bsw-n3050/igt@i915_selftest@live@execlists.html

  * igt@i915_selftest@live@mman:
    - bat-rpls-2:         [PASS][15] -> [TIMEOUT][16] ([i915#6794] / [i915#7392])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13536/bat-rpls-2/igt@i915_selftest@live@mman.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122641v1/bat-rpls-2/igt@i915_selftest@live@mman.html

  * igt@i915_suspend@basic-s2idle-without-i915:
    - bat-rpls-2:         [PASS][17] -> [WARN][18] ([i915#8747])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13536/bat-rpls-2/igt@i915_suspend@basic-s2idle-without-i915.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122641v1/bat-rpls-2/igt@i915_suspend@basic-s2idle-without-i915.html

  * igt@kms_cursor_legacy@basic-flip-before-cursor-legacy:
    - bat-adlp-9:         [PASS][19] -> [DMESG-WARN][20] ([i915#7507]) +25 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13536/bat-adlp-9/igt@kms_cursor_legacy@basic-flip-before-cursor-legacy.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122641v1/bat-adlp-9/igt@kms_cursor_legacy@basic-flip-before-cursor-legacy.html

  * igt@kms_flip@basic-plain-flip@a-dp1:
    - bat-adlp-9:         [PASS][21] -> [DMESG-WARN][22] ([i915#5954] / [i915#7507]) +9 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13536/bat-adlp-9/igt@kms_flip@basic-plain-flip@a-dp1.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122641v1/bat-adlp-9/igt@kms_flip@basic-plain-flip@a-dp1.html

  * igt@kms_force_connector_basic@force-connector-state:
    - bat-adlm-1:         [PASS][23] -> [DMESG-WARN][24] ([i915#7507])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13536/bat-adlm-1/igt@kms_force_connector_basic@force-connector-state.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122641v1/bat-adlm-1/igt@kms_force_connector_basic@force-connector-state.html

  * igt@kms_pipe_crc_basic@hang-read-crc@pipe-c-edp-1:
    - bat-adln-1:         [PASS][25] -> [DMESG-WARN][26] ([i915#6020] / [i915#7507]) +2 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13536/bat-adln-1/igt@kms_pipe_crc_basic@hang-read-crc@pipe-c-edp-1.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122641v1/bat-adln-1/igt@kms_pipe_crc_basic@hang-read-crc@pipe-c-edp-1.html

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1:
    - bat-rplp-1:         [PASS][27] -> [ABORT][28] ([i915#8442] / [i915#8668])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13536/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122641v1/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html

  * igt@kms_pipe_crc_basic@suspend-read-crc@pipe-a-edp-1:
    - bat-mtlp-8:         [PASS][29] -> [DMESG-WARN][30] ([i915#1982]) +1 similar issue
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13536/bat-mtlp-8/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-a-edp-1.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122641v1/bat-mtlp-8/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-a-edp-1.html

  * igt@kms_psr@cursor_plane_move:
    - bat-adln-1:         [PASS][31] -> [DMESG-WARN][32] ([i915#7507]) +29 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13536/bat-adln-1/igt@kms_psr@cursor_plane_move.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122641v1/bat-adln-1/igt@kms_psr@cursor_plane_move.html

  * igt@kms_psr@primary_page_flip:
    - fi-pnv-d510:        NOTRUN -> [SKIP][33] ([fdo#109271]) +30 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122641v1/fi-pnv-d510/igt@kms_psr@primary_page_flip.html

  
#### Warnings ####

  * igt@core_auth@basic-auth:
    - bat-adlp-11:        [ABORT][34] ([i915#8011]) -> [ABORT][35] ([i915#4423] / [i915#8011])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13536/bat-adlp-11/igt@core_auth@basic-auth.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122641v1/bat-adlp-11/igt@core_auth@basic-auth.html

  * igt@kms_psr@sprite_plane_onoff:
    - bat-adln-1:         [ABORT][36] ([i915#8442] / [i915#8668]) -> [ABORT][37] ([i915#7507] / [i915#8442] / [i915#8668])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13536/bat-adln-1/igt@kms_psr@sprite_plane_onoff.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122641v1/bat-adln-1/igt@kms_psr@sprite_plane_onoff.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#4423]: https://gitlab.freedesktop.org/drm/intel/issues/4423
  [i915#5954]: https://gitlab.freedesktop.org/drm/intel/issues/5954
  [i915#6020]: https://gitlab.freedesktop.org/drm/intel/issues/6020
  [i915#6794]: https://gitlab.freedesktop.org/drm/intel/issues/6794
  [i915#7359]: https://gitlab.freedesktop.org/drm/intel/issues/7359
  [i915#7392]: https://gitlab.freedesktop.org/drm/intel/issues/7392
  [i915#7507]: https://gitlab.freedesktop.org/drm/intel/issues/7507
  [i915#7911]: https://gitlab.freedesktop.org/drm/intel/issues/7911
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7953]: https://gitlab.freedesktop.org/drm/intel/issues/7953
  [i915#8011]: https://gitlab.freedesktop.org/drm/intel/issues/8011
  [i915#8442]: https://gitlab.freedesktop.org/drm/intel/issues/8442
  [i915#8668]: https://gitlab.freedesktop.org/drm/intel/issues/8668
  [i915#8747]: https://gitlab.freedesktop.org/drm/intel/issues/8747


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

  * Linux: CI_DRM_13536 -> Patchwork_122641v1

  CI-20190529: 20190529
  CI_DRM_13536: 7a825a06c6ee60a6586ddf8b4adb03ea5262bda7 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7444: 7444
  Patchwork_122641v1: 7a825a06c6ee60a6586ddf8b4adb03ea5262bda7 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

9380bc0e096b drm/i915: Implement vblank synchronized MBUS join changes
cfb7c43931ee drm/i915: Update mbus in intel_dbuf_mbus_update and do it properly

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122641v1/index.html

[-- Attachment #2: Type: text/html, Size: 11898 bytes --]