[Intel-gfx] [PATCH] drm/i915: Write zero wms if we disable planes for icl+

[Intel-gfx] [PATCH] drm/i915: Write zero wms if we disable planes for icl+

[Stanislav Lisovskiy]

Otherwise we seem to get FIFO underruns. It is being disabled
anyway, so kind of logical to write those as zeroes, even if
disabling is temporary.

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

diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index caa03324a733..c0251189c042 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -633,7 +633,7 @@ icl_plane_disable_arm(struct intel_plane *plane,
 	if (icl_is_hdr_plane(dev_priv, plane_id))
 		intel_de_write_fw(dev_priv, PLANE_CUS_CTL(pipe, plane_id), 0);
 
-	skl_write_plane_wm(plane, crtc_state);
+	skl_write_zero_plane_wm(plane, crtc_state);
 
 	intel_psr2_disable_plane_sel_fetch(plane, crtc_state);
 	intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), 0);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ee0047fdc95d..2470c037bfae 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5885,6 +5885,52 @@ void skl_write_plane_wm(struct intel_plane *plane,
 				    PLANE_NV12_BUF_CFG(pipe, plane_id), ddb_y);
 }
 
+void skl_write_zero_plane_wm(struct intel_plane *plane,
+			     const struct intel_crtc_state *crtc_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	int level, max_level = ilk_wm_max_level(dev_priv);
+	enum plane_id plane_id = plane->id;
+	enum pipe pipe = plane->pipe;
+	struct skl_pipe_wm pipe_wm;
+	const struct skl_ddb_entry *ddb =
+		&crtc_state->wm.skl.plane_ddb[plane_id];
+	const struct skl_ddb_entry *ddb_y =
+		&crtc_state->wm.skl.plane_ddb_y[plane_id];
+
+	for (level = 0; level <= max_level; level++) {
+		pipe_wm.planes[plane_id].wm[level].blocks = 0;
+		pipe_wm.planes[plane_id].wm[level].lines = 0;
+	}
+
+	pipe_wm.planes[plane_id].trans_wm.blocks = 0;
+	pipe_wm.planes[plane_id].trans_wm.lines = 0;
+
+	for (level = 0; level <= max_level; level++)
+		skl_write_wm_level(dev_priv, PLANE_WM(pipe, plane_id, level),
+				   skl_plane_wm_level(&pipe_wm, plane_id, level));
+
+	skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane_id),
+			   skl_plane_trans_wm(&pipe_wm, plane_id));
+
+	if (HAS_HW_SAGV_WM(dev_priv)) {
+		const struct skl_plane_wm *wm = &pipe_wm.planes[plane_id];
+
+		skl_write_wm_level(dev_priv, PLANE_WM_SAGV(pipe, plane_id),
+				   &wm->sagv.wm0);
+		skl_write_wm_level(dev_priv, PLANE_WM_SAGV_TRANS(pipe, plane_id),
+				   &wm->sagv.trans_wm);
+	}
+
+	skl_ddb_entry_write(dev_priv,
+			    PLANE_BUF_CFG(pipe, plane_id), ddb);
+
+	if (DISPLAY_VER(dev_priv) < 11)
+		skl_ddb_entry_write(dev_priv,
+				    PLANE_NV12_BUF_CFG(pipe, plane_id), ddb_y);
+}
+
+
 void skl_write_cursor_wm(struct intel_plane *plane,
 			 const struct intel_crtc_state *crtc_state)
 {
diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
index 50604cf7398c..fb0ac4f143ab 100644
--- a/drivers/gpu/drm/i915/intel_pm.h
+++ b/drivers/gpu/drm/i915/intel_pm.h
@@ -61,6 +61,8 @@ bool skl_wm_level_equals(const struct skl_wm_level *l1,
 bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry *ddb,
 				 const struct skl_ddb_entry *entries,
 				 int num_entries, int ignore_idx);
+void skl_write_zero_plane_wm(struct intel_plane *plane,
+			     const struct intel_crtc_state *crtc_state);
 void skl_write_plane_wm(struct intel_plane *plane,
 			const struct intel_crtc_state *crtc_state);
 void skl_write_cursor_wm(struct intel_plane *plane,
-- 
2.24.1.485.gad05a3d8e5

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Write zero wms if we disable planes for icl+

[Patchwork]

== Series Details ==

Series: drm/i915: Write zero wms if we disable planes for icl+
URL   : https://patchwork.freedesktop.org/series/104120/
State : warning

== Summary ==

Error: dim checkpatch failed
ad72e46079ae drm/i915: Write zero wms if we disable planes for icl+
-:78: CHECK:LINE_SPACING: Please don't use multiple blank lines
#78: FILE: drivers/gpu/drm/i915/intel_pm.c:5933:
+
+

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

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Write zero wms if we disable planes for icl+

[Patchwork]

== Series Details ==

Series: drm/i915: Write zero wms if we disable planes for icl+
URL   : https://patchwork.freedesktop.org/series/104120/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
+ ^
+ }
+drivers/gpu/drm/i915/intel_pm.c:5931:1: warning: the frame size of 1296 bytes is larger than 1024 bytes [-Wframe-larger-than=]
+drivers/gpu/drm/i915/intel_pm.c: In function ‘skl_write_zero_plane_wm’:

Re: [Intel-gfx] [PATCH] drm/i915: Write zero wms if we disable planes for icl+

[Jani Nikula]

On Wed, 18 May 2022, Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> wrote:
> Otherwise we seem to get FIFO underruns. It is being disabled
> anyway, so kind of logical to write those as zeroes, even if
> disabling is temporary.
>
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  .../drm/i915/display/skl_universal_plane.c    |  2 +-
>  drivers/gpu/drm/i915/intel_pm.c               | 46 +++++++++++++++++++
>  drivers/gpu/drm/i915/intel_pm.h               |  2 +
>  3 files changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index caa03324a733..c0251189c042 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -633,7 +633,7 @@ icl_plane_disable_arm(struct intel_plane *plane,
>  	if (icl_is_hdr_plane(dev_priv, plane_id))
>  		intel_de_write_fw(dev_priv, PLANE_CUS_CTL(pipe, plane_id), 0);
>  
> -	skl_write_plane_wm(plane, crtc_state);
> +	skl_write_zero_plane_wm(plane, crtc_state);
>  
>  	intel_psr2_disable_plane_sel_fetch(plane, crtc_state);
>  	intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), 0);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index ee0047fdc95d..2470c037bfae 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5885,6 +5885,52 @@ void skl_write_plane_wm(struct intel_plane *plane,
>  				    PLANE_NV12_BUF_CFG(pipe, plane_id), ddb_y);
>  }
>  
> +void skl_write_zero_plane_wm(struct intel_plane *plane,
> +			     const struct intel_crtc_state *crtc_state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> +	int level, max_level = ilk_wm_max_level(dev_priv);
> +	enum plane_id plane_id = plane->id;
> +	enum pipe pipe = plane->pipe;
> +	struct skl_pipe_wm pipe_wm;
> +	const struct skl_ddb_entry *ddb =
> +		&crtc_state->wm.skl.plane_ddb[plane_id];
> +	const struct skl_ddb_entry *ddb_y =
> +		&crtc_state->wm.skl.plane_ddb_y[plane_id];
> +
> +	for (level = 0; level <= max_level; level++) {

Not your doing here, but why have we adopted this error prone inclusive
max that always makes you take a double look in the for loops?!

BR,
Jani.

> +		pipe_wm.planes[plane_id].wm[level].blocks = 0;
> +		pipe_wm.planes[plane_id].wm[level].lines = 0;
> +	}
> +
> +	pipe_wm.planes[plane_id].trans_wm.blocks = 0;
> +	pipe_wm.planes[plane_id].trans_wm.lines = 0;
> +
> +	for (level = 0; level <= max_level; level++)
> +		skl_write_wm_level(dev_priv, PLANE_WM(pipe, plane_id, level),
> +				   skl_plane_wm_level(&pipe_wm, plane_id, level));
> +
> +	skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane_id),
> +			   skl_plane_trans_wm(&pipe_wm, plane_id));
> +
> +	if (HAS_HW_SAGV_WM(dev_priv)) {
> +		const struct skl_plane_wm *wm = &pipe_wm.planes[plane_id];
> +
> +		skl_write_wm_level(dev_priv, PLANE_WM_SAGV(pipe, plane_id),
> +				   &wm->sagv.wm0);
> +		skl_write_wm_level(dev_priv, PLANE_WM_SAGV_TRANS(pipe, plane_id),
> +				   &wm->sagv.trans_wm);
> +	}
> +
> +	skl_ddb_entry_write(dev_priv,
> +			    PLANE_BUF_CFG(pipe, plane_id), ddb);
> +
> +	if (DISPLAY_VER(dev_priv) < 11)
> +		skl_ddb_entry_write(dev_priv,
> +				    PLANE_NV12_BUF_CFG(pipe, plane_id), ddb_y);
> +}
> +
> +
>  void skl_write_cursor_wm(struct intel_plane *plane,
>  			 const struct intel_crtc_state *crtc_state)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
> index 50604cf7398c..fb0ac4f143ab 100644
> --- a/drivers/gpu/drm/i915/intel_pm.h
> +++ b/drivers/gpu/drm/i915/intel_pm.h
> @@ -61,6 +61,8 @@ bool skl_wm_level_equals(const struct skl_wm_level *l1,
>  bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry *ddb,
>  				 const struct skl_ddb_entry *entries,
>  				 int num_entries, int ignore_idx);
> +void skl_write_zero_plane_wm(struct intel_plane *plane,
> +			     const struct intel_crtc_state *crtc_state);
>  void skl_write_plane_wm(struct intel_plane *plane,
>  			const struct intel_crtc_state *crtc_state);
>  void skl_write_cursor_wm(struct intel_plane *plane,

-- 
Jani Nikula, Intel Open Source Graphics Center

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Write zero wms if we disable planes for icl+

[Patchwork]

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

== Series Details ==

Series: drm/i915: Write zero wms if we disable planes for icl+
URL   : https://patchwork.freedesktop.org/series/104120/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11672 -> Patchwork_104120v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_104120v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_104120v1, 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_104120v1/index.html

Participating hosts (44 -> 46)
------------------------------

  Additional (4): bat-rpls-1 fi-rkl-11600 fi-hsw-g3258 bat-adlm-1 
  Missing    (2): fi-cml-u2 bat-dg2-9 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@gem_exec_suspend@basic-s3@smem:
    - fi-rkl-guc:         NOTRUN -> [DMESG-WARN][1] +4 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-rkl-guc/igt@gem_exec_suspend@basic-s3@smem.html

  * igt@i915_suspend@basic-s2idle-without-i915:
    - fi-cfl-8109u:       [PASS][2] -> [DMESG-WARN][3]
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-cfl-8109u/igt@i915_suspend@basic-s2idle-without-i915.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-cfl-8109u/igt@i915_suspend@basic-s2idle-without-i915.html

  * igt@kms_busy@basic@flip:
    - fi-adl-ddr5:        [PASS][4] -> [DMESG-WARN][5] +34 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-adl-ddr5/igt@kms_busy@basic@flip.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-adl-ddr5/igt@kms_busy@basic@flip.html
    - bat-dg1-6:          [PASS][6] -> [DMESG-WARN][7] +29 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/bat-dg1-6/igt@kms_busy@basic@flip.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/bat-dg1-6/igt@kms_busy@basic@flip.html

  * igt@kms_flip@basic-flip-vs-dpms@b-dp1:
    - fi-tgl-1115g4:      [PASS][8] -> [DMESG-WARN][9] +30 similar issues
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-tgl-1115g4/igt@kms_flip@basic-flip-vs-dpms@b-dp1.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-tgl-1115g4/igt@kms_flip@basic-flip-vs-dpms@b-dp1.html

  * igt@kms_flip@basic-flip-vs-modeset@c-hdmi-a1:
    - fi-rkl-guc:         [PASS][10] -> [DMESG-WARN][11] +24 similar issues
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-rkl-guc/igt@kms_flip@basic-flip-vs-modeset@c-hdmi-a1.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-rkl-guc/igt@kms_flip@basic-flip-vs-modeset@c-hdmi-a1.html

  * igt@kms_flip@basic-plain-flip@a-hdmi-a1:
    - fi-rkl-11600:       NOTRUN -> [DMESG-WARN][12] +25 similar issues
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-rkl-11600/igt@kms_flip@basic-plain-flip@a-hdmi-a1.html

  * igt@kms_force_connector_basic@force-connector-state:
    - bat-dg1-5:          [PASS][13] -> [DMESG-WARN][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/bat-dg1-5/igt@kms_force_connector_basic@force-connector-state.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/bat-dg1-5/igt@kms_force_connector_basic@force-connector-state.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
    - fi-tgl-u2:          [PASS][15] -> [DMESG-WARN][16] +145 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-tgl-u2/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-tgl-u2/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a.html

  * igt@vgem_basic@dmabuf-fence:
    - fi-tgl-u2:          NOTRUN -> [DMESG-WARN][17]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-tgl-u2/igt@vgem_basic@dmabuf-fence.html

  
#### Warnings ####

  * igt@kms_flip@basic-flip-vs-modeset@a-edp1:
    - fi-tgl-u2:          [DMESG-WARN][18] ([i915#402]) -> [DMESG-WARN][19] +2 similar issues
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-tgl-u2/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-tgl-u2/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@core_hotunplug@unbind-rebind:
    - {bat-jsl-2}:        [PASS][20] -> [DMESG-WARN][21] +39 similar issues
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/bat-jsl-2/igt@core_hotunplug@unbind-rebind.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/bat-jsl-2/igt@core_hotunplug@unbind-rebind.html

  * igt@i915_suspend@basic-s2idle-without-i915:
    - {bat-adlp-6}:       [PASS][22] -> [DMESG-WARN][23] +47 similar issues
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/bat-adlp-6/igt@i915_suspend@basic-s2idle-without-i915.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/bat-adlp-6/igt@i915_suspend@basic-s2idle-without-i915.html
    - {fi-ehl-2}:         NOTRUN -> [DMESG-WARN][24]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-ehl-2/igt@i915_suspend@basic-s2idle-without-i915.html

  * igt@kms_flip@basic-flip-vs-dpms@b-edp1:
    - {bat-jsl-1}:        [PASS][25] -> [DMESG-WARN][26] +27 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/bat-jsl-1/igt@kms_flip@basic-flip-vs-dpms@b-edp1.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/bat-jsl-1/igt@kms_flip@basic-flip-vs-dpms@b-edp1.html

  * igt@kms_flip@basic-flip-vs-dpms@c-edp1:
    - {fi-ehl-2}:         [PASS][27] -> [DMESG-WARN][28] +35 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-ehl-2/igt@kms_flip@basic-flip-vs-dpms@c-edp1.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-ehl-2/igt@kms_flip@basic-flip-vs-dpms@c-edp1.html

  * igt@kms_flip@basic-flip-vs-dpms@c-hdmi-a1:
    - {bat-adln-1}:       [PASS][29] -> [DMESG-WARN][30] +24 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/bat-adln-1/igt@kms_flip@basic-flip-vs-dpms@c-hdmi-a1.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/bat-adln-1/igt@kms_flip@basic-flip-vs-dpms@c-hdmi-a1.html

  * igt@kms_flip@basic-flip-vs-dpms@d-edp1:
    - {bat-rpls-1}:       NOTRUN -> [DMESG-WARN][31] +58 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/bat-rpls-1/igt@kms_flip@basic-flip-vs-dpms@d-edp1.html

  * igt@kms_flip@basic-flip-vs-modeset@a-edp1:
    - {bat-adlp-6}:       [DMESG-WARN][32] ([i915#3576]) -> [DMESG-WARN][33]
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/bat-adlp-6/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/bat-adlp-6/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html

  * igt@kms_force_connector_basic@force-connector-state:
    - {bat-dg2-8}:        [PASS][34] -> [DMESG-WARN][35]
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/bat-dg2-8/igt@kms_force_connector_basic@force-connector-state.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/bat-dg2-8/igt@kms_force_connector_basic@force-connector-state.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-a:
    - {fi-tgl-dsi}:       [PASS][36] -> [DMESG-WARN][37] +26 similar issues
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-tgl-dsi/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-a.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-tgl-dsi/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-a.html
    - {bat-adlm-1}:       NOTRUN -> [DMESG-WARN][38] +29 similar issues
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/bat-adlm-1/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-a.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - {bat-rpls-2}:       [PASS][39] -> [DMESG-WARN][40] +26 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/bat-rpls-2/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/bat-rpls-2/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
    - {fi-jsl-1}:         [PASS][41] -> [DMESG-WARN][42] +39 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-jsl-1/igt@kms_pipe_crc_basic@hang-read-crc-pipe-a.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-jsl-1/igt@kms_pipe_crc_basic@hang-read-crc-pipe-a.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_huc_copy@huc-copy:
    - fi-rkl-11600:       NOTRUN -> [SKIP][43] ([i915#2190])
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-rkl-11600/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-rkl-11600:       NOTRUN -> [SKIP][44] ([i915#4613]) +3 similar issues
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-rkl-11600/igt@gem_lmem_swapping@basic.html

  * igt@gem_tiled_pread_basic:
    - fi-rkl-11600:       NOTRUN -> [SKIP][45] ([i915#3282])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-rkl-11600/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_backlight@basic-brightness:
    - fi-hsw-g3258:       NOTRUN -> [SKIP][46] ([fdo#109271] / [i915#3012])
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-hsw-g3258/igt@i915_pm_backlight@basic-brightness.html
    - fi-rkl-11600:       NOTRUN -> [SKIP][47] ([i915#3012])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-rkl-11600/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_pm_rpm@module-reload:
    - fi-cfl-8109u:       [PASS][48] -> [DMESG-FAIL][49] ([i915#62])
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-cfl-8109u/igt@i915_pm_rpm@module-reload.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-cfl-8109u/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live@hangcheck:
    - bat-dg1-5:          NOTRUN -> [DMESG-FAIL][50] ([i915#4494] / [i915#4957])
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/bat-dg1-5/igt@i915_selftest@live@hangcheck.html
    - fi-hsw-4770:        [PASS][51] -> [INCOMPLETE][52] ([i915#4785])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html

  * igt@i915_selftest@live@memory_region:
    - fi-cfl-8109u:       [PASS][53] -> [DMESG-WARN][54] ([i915#5904]) +11 similar issues
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-cfl-8109u/igt@i915_selftest@live@memory_region.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-cfl-8109u/igt@i915_selftest@live@memory_region.html

  * igt@i915_selftest@live@requests:
    - fi-blb-e6850:       [PASS][55] -> [DMESG-FAIL][56] ([i915#4528])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-blb-e6850/igt@i915_selftest@live@requests.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-blb-e6850/igt@i915_selftest@live@requests.html
    - fi-pnv-d510:        [PASS][57] -> [DMESG-FAIL][58] ([i915#4528])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-pnv-d510/igt@i915_selftest@live@requests.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-pnv-d510/igt@i915_selftest@live@requests.html

  * igt@i915_suspend@basic-s2idle-without-i915:
    - bat-dg1-5:          NOTRUN -> [INCOMPLETE][59] ([i915#6011])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/bat-dg1-5/igt@i915_suspend@basic-s2idle-without-i915.html

  * igt@i915_suspend@basic-s3-without-i915:
    - fi-rkl-11600:       NOTRUN -> [INCOMPLETE][60] ([i915#5982])
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-rkl-11600/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-rkl-guc:         NOTRUN -> [SKIP][61] ([fdo#111827])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-rkl-guc/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-rkl-11600:       NOTRUN -> [SKIP][62] ([fdo#111827]) +7 similar issues
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-rkl-11600/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_chamelium@vga-edid-read:
    - fi-hsw-g3258:       NOTRUN -> [SKIP][63] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-hsw-g3258/igt@kms_chamelium@vga-edid-read.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-rkl-11600:       NOTRUN -> [SKIP][64] ([i915#4070] / [i915#4103]) +1 similar issue
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-rkl-11600/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-rkl-11600:       NOTRUN -> [SKIP][65] ([fdo#109285] / [i915#4098])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-rkl-11600/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-b:
    - fi-cfl-8109u:       [PASS][66] -> [DMESG-WARN][67] ([i915#62]) +15 similar issues
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-cfl-8109u/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-b.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-cfl-8109u/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-b.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - fi-hsw-g3258:       NOTRUN -> [SKIP][68] ([fdo#109271] / [i915#533])
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-hsw-g3258/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html
    - fi-rkl-11600:       NOTRUN -> [SKIP][69] ([i915#4070] / [i915#533])
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-rkl-11600/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  * igt@kms_psr@primary_mmap_gtt:
    - fi-hsw-g3258:       NOTRUN -> [SKIP][70] ([fdo#109271] / [i915#1072]) +3 similar issues
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-hsw-g3258/igt@kms_psr@primary_mmap_gtt.html
    - fi-rkl-11600:       NOTRUN -> [SKIP][71] ([i915#1072]) +3 similar issues
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-rkl-11600/igt@kms_psr@primary_mmap_gtt.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - fi-hsw-g3258:       NOTRUN -> [SKIP][72] ([fdo#109271]) +9 similar issues
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-hsw-g3258/igt@kms_setmode@basic-clone-single-crtc.html
    - fi-rkl-11600:       NOTRUN -> [SKIP][73] ([i915#3555] / [i915#4098])
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-rkl-11600/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-userptr:
    - fi-rkl-11600:       NOTRUN -> [SKIP][74] ([i915#3301] / [i915#3708])
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-rkl-11600/igt@prime_vgem@basic-userptr.html

  * igt@prime_vgem@basic-write:
    - fi-rkl-11600:       NOTRUN -> [SKIP][75] ([i915#3291] / [i915#3708]) +2 similar issues
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-rkl-11600/igt@prime_vgem@basic-write.html

  * igt@runner@aborted:
    - fi-hsw-4770:        NOTRUN -> [FAIL][76] ([fdo#109271] / [i915#4312] / [i915#5594])
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-hsw-4770/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@gem_ringfill@basic-all:
    - {bat-dg2-8}:        [FAIL][77] ([i915#5886]) -> [PASS][78]
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/bat-dg2-8/igt@gem_ringfill@basic-all.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/bat-dg2-8/igt@gem_ringfill@basic-all.html

  * igt@i915_selftest@live@gt_engines:
    - bat-dg1-5:          [INCOMPLETE][79] ([i915#4418]) -> [PASS][80]
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/bat-dg1-5/igt@i915_selftest@live@gt_engines.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/bat-dg1-5/igt@i915_selftest@live@gt_engines.html

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-kbl-soraka:      [INCOMPLETE][81] -> [PASS][82]
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@gt_lrc:
    - fi-rkl-guc:         [INCOMPLETE][83] ([i915#4983]) -> [PASS][84]
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-rkl-guc/igt@i915_selftest@live@gt_lrc.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-rkl-guc/igt@i915_selftest@live@gt_lrc.html

  * igt@i915_selftest@live@hangcheck:
    - {fi-ehl-2}:         [INCOMPLETE][85] ([i915#5153]) -> [PASS][86]
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-ehl-2/igt@i915_selftest@live@hangcheck.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-ehl-2/igt@i915_selftest@live@hangcheck.html
    - bat-dg1-6:          [DMESG-FAIL][87] ([i915#4494] / [i915#4957]) -> [PASS][88]
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/bat-dg1-6/igt@i915_selftest@live@hangcheck.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/bat-dg1-6/igt@i915_selftest@live@hangcheck.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
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1155]: https://gitlab.freedesktop.org/drm/intel/issues/1155
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2411]: https://gitlab.freedesktop.org/drm/intel/issues/2411
  [i915#3003]: https://gitlab.freedesktop.org/drm/intel/issues/3003
  [i915#3012]: https://gitlab.freedesktop.org/drm/intel/issues/3012
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4391]: https://gitlab.freedesktop.org/drm/intel/issues/4391
  [i915#4418]: https://gitlab.freedesktop.org/drm/intel/issues/4418
  [i915#4494]: https://gitlab.freedesktop.org/drm/intel/issues/4494
  [i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785
  [i915#4957]: https://gitlab.freedesktop.org/drm/intel/issues/4957
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5122]: https://gitlab.freedesktop.org/drm/intel/issues/5122
  [i915#5153]: https://gitlab.freedesktop.org/drm/intel/issues/5153
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#5594]: https://gitlab.freedesktop.org/drm/intel/issues/5594
  [i915#5763]: https://gitlab.freedesktop.org/drm/intel/issues/5763
  [i915#5828]: https://gitlab.freedesktop.org/drm/intel/issues/5828
  [i915#5879]: https://gitlab.freedesktop.org/drm/intel/issues/5879
  [i915#5886]: https://gitlab.freedesktop.org/drm/intel/issues/5886
  [i915#5903]: https://gitlab.freedesktop.org/drm/intel/issues/5903
  [i915#5904]: https://gitlab.freedesktop.org/drm/intel/issues/5904
  [i915#5950]: https://gitlab.freedesktop.org/drm/intel/issues/5950
  [i915#5982]: https://gitlab.freedesktop.org/drm/intel/issues/5982
  [i915#5999]: https://gitlab.freedesktop.org/drm/intel/issues/5999
  [i915#6011]: https://gitlab.freedesktop.org/drm/intel/issues/6011
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62


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

  * Linux: CI_DRM_11672 -> Patchwork_104120v1

  CI-20190529: 20190529
  CI_DRM_11672: 63d4f934b573d277c1b43755245fef1baec728f5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6478: 7c3ceb08b633a66f77f42e596593de394da5dccc @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_104120v1: 63d4f934b573d277c1b43755245fef1baec728f5 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

a1c76961d64f drm/i915: Write zero wms if we disable planes for icl+

== Logs ==

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

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

Re: [Intel-gfx] [PATCH] drm/i915: Write zero wms if we disable planes for icl+

[Lisovskiy, Stanislav]

On Wed, May 18, 2022 at 02:44:30PM +0300, Jani Nikula wrote:
> On Wed, 18 May 2022, Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> wrote:
> > Otherwise we seem to get FIFO underruns. It is being disabled
> > anyway, so kind of logical to write those as zeroes, even if
> > disabling is temporary.
> >
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > ---
> >  .../drm/i915/display/skl_universal_plane.c    |  2 +-
> >  drivers/gpu/drm/i915/intel_pm.c               | 46 +++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_pm.h               |  2 +
> >  3 files changed, 49 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > index caa03324a733..c0251189c042 100644
> > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > @@ -633,7 +633,7 @@ icl_plane_disable_arm(struct intel_plane *plane,
> >  	if (icl_is_hdr_plane(dev_priv, plane_id))
> >  		intel_de_write_fw(dev_priv, PLANE_CUS_CTL(pipe, plane_id), 0);
> >  
> > -	skl_write_plane_wm(plane, crtc_state);
> > +	skl_write_zero_plane_wm(plane, crtc_state);
> >  
> >  	intel_psr2_disable_plane_sel_fetch(plane, crtc_state);
> >  	intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), 0);
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index ee0047fdc95d..2470c037bfae 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -5885,6 +5885,52 @@ void skl_write_plane_wm(struct intel_plane *plane,
> >  				    PLANE_NV12_BUF_CFG(pipe, plane_id), ddb_y);
> >  }
> >  
> > +void skl_write_zero_plane_wm(struct intel_plane *plane,
> > +			     const struct intel_crtc_state *crtc_state)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> > +	int level, max_level = ilk_wm_max_level(dev_priv);
> > +	enum plane_id plane_id = plane->id;
> > +	enum pipe pipe = plane->pipe;
> > +	struct skl_pipe_wm pipe_wm;
> > +	const struct skl_ddb_entry *ddb =
> > +		&crtc_state->wm.skl.plane_ddb[plane_id];
> > +	const struct skl_ddb_entry *ddb_y =
> > +		&crtc_state->wm.skl.plane_ddb_y[plane_id];
> > +
> > +	for (level = 0; level <= max_level; level++) {
> 
> Not your doing here, but why have we adopted this error prone inclusive
> max that always makes you take a double look in the for loops?!
> 
> BR,
> Jani.

No clue, really. It seems to be used same way all over the place in
intel_pm.c. I was suggesting there is some smart reasoning behind this,
so simply follow the common approach.

For me it is more confusing that apparently it is called "ilk_*" which
is IronLake probably, however we use it everywhere.
I would call it simple i915_wm_max_level and make it determine itself based on
dev_priv which platform it is..

Stan



> 
> > +		pipe_wm.planes[plane_id].wm[level].blocks = 0;
> > +		pipe_wm.planes[plane_id].wm[level].lines = 0;
> > +	}
> > +
> > +	pipe_wm.planes[plane_id].trans_wm.blocks = 0;
> > +	pipe_wm.planes[plane_id].trans_wm.lines = 0;
> > +
> > +	for (level = 0; level <= max_level; level++)
> > +		skl_write_wm_level(dev_priv, PLANE_WM(pipe, plane_id, level),
> > +				   skl_plane_wm_level(&pipe_wm, plane_id, level));
> > +
> > +	skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane_id),
> > +			   skl_plane_trans_wm(&pipe_wm, plane_id));
> > +
> > +	if (HAS_HW_SAGV_WM(dev_priv)) {
> > +		const struct skl_plane_wm *wm = &pipe_wm.planes[plane_id];
> > +
> > +		skl_write_wm_level(dev_priv, PLANE_WM_SAGV(pipe, plane_id),
> > +				   &wm->sagv.wm0);
> > +		skl_write_wm_level(dev_priv, PLANE_WM_SAGV_TRANS(pipe, plane_id),
> > +				   &wm->sagv.trans_wm);
> > +	}
> > +
> > +	skl_ddb_entry_write(dev_priv,
> > +			    PLANE_BUF_CFG(pipe, plane_id), ddb);
> > +
> > +	if (DISPLAY_VER(dev_priv) < 11)
> > +		skl_ddb_entry_write(dev_priv,
> > +				    PLANE_NV12_BUF_CFG(pipe, plane_id), ddb_y);
> > +}
> > +
> > +
> >  void skl_write_cursor_wm(struct intel_plane *plane,
> >  			 const struct intel_crtc_state *crtc_state)
> >  {
> > diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
> > index 50604cf7398c..fb0ac4f143ab 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.h
> > +++ b/drivers/gpu/drm/i915/intel_pm.h
> > @@ -61,6 +61,8 @@ bool skl_wm_level_equals(const struct skl_wm_level *l1,
> >  bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry *ddb,
> >  				 const struct skl_ddb_entry *entries,
> >  				 int num_entries, int ignore_idx);
> > +void skl_write_zero_plane_wm(struct intel_plane *plane,
> > +			     const struct intel_crtc_state *crtc_state);
> >  void skl_write_plane_wm(struct intel_plane *plane,
> >  			const struct intel_crtc_state *crtc_state);
> >  void skl_write_cursor_wm(struct intel_plane *plane,
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH] drm/i915: Write zero wms if we disable planes for icl+

[Ville Syrjälä]

On Wed, May 18, 2022 at 03:05:44PM +0300, Lisovskiy, Stanislav wrote:
> On Wed, May 18, 2022 at 02:44:30PM +0300, Jani Nikula wrote:
> > On Wed, 18 May 2022, Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> wrote:
> > > Otherwise we seem to get FIFO underruns. It is being disabled
> > > anyway, so kind of logical to write those as zeroes, even if
> > > disabling is temporary.
> > >
> > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > > ---
> > >  .../drm/i915/display/skl_universal_plane.c    |  2 +-
> > >  drivers/gpu/drm/i915/intel_pm.c               | 46 +++++++++++++++++++
> > >  drivers/gpu/drm/i915/intel_pm.h               |  2 +
> > >  3 files changed, 49 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > > index caa03324a733..c0251189c042 100644
> > > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > > @@ -633,7 +633,7 @@ icl_plane_disable_arm(struct intel_plane *plane,
> > >  	if (icl_is_hdr_plane(dev_priv, plane_id))
> > >  		intel_de_write_fw(dev_priv, PLANE_CUS_CTL(pipe, plane_id), 0);
> > >  
> > > -	skl_write_plane_wm(plane, crtc_state);
> > > +	skl_write_zero_plane_wm(plane, crtc_state);
> > >  
> > >  	intel_psr2_disable_plane_sel_fetch(plane, crtc_state);
> > >  	intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), 0);
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > index ee0047fdc95d..2470c037bfae 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -5885,6 +5885,52 @@ void skl_write_plane_wm(struct intel_plane *plane,
> > >  				    PLANE_NV12_BUF_CFG(pipe, plane_id), ddb_y);
> > >  }
> > >  
> > > +void skl_write_zero_plane_wm(struct intel_plane *plane,
> > > +			     const struct intel_crtc_state *crtc_state)
> > > +{
> > > +	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> > > +	int level, max_level = ilk_wm_max_level(dev_priv);
> > > +	enum plane_id plane_id = plane->id;
> > > +	enum pipe pipe = plane->pipe;
> > > +	struct skl_pipe_wm pipe_wm;
> > > +	const struct skl_ddb_entry *ddb =
> > > +		&crtc_state->wm.skl.plane_ddb[plane_id];
> > > +	const struct skl_ddb_entry *ddb_y =
> > > +		&crtc_state->wm.skl.plane_ddb_y[plane_id];
> > > +
> > > +	for (level = 0; level <= max_level; level++) {
> > 
> > Not your doing here, but why have we adopted this error prone inclusive
> > max that always makes you take a double look in the for loops?!
> > 
> > BR,
> > Jani.
> 
> No clue, really. It seems to be used same way all over the place in
> intel_pm.c. I was suggesting there is some smart reasoning behind this,
> so simply follow the common approach.
> 
> For me it is more confusing that apparently it is called "ilk_*" which
> is IronLake probably, however we use it everywhere.
> I would call it simple i915_wm_max_level and make it determine itself based on
> dev_priv which platform it is..

There are loads of (old by now) patches on the list for cleaning
up all kinds of stuff in the wm code:
https://patchwork.freedesktop.org/series/50802/
https://patchwork.freedesktop.org/series/83289/
https://patchwork.freedesktop.org/series/90164/

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH] drm/i915: Write zero wms if we disable planes for icl+

[Govindapillai, Vinod]

Hi Stan

Pls see some comments inline..

BR
Vinod

On Wed, 2022-05-18 at 13:59 +0300, Stanislav Lisovskiy wrote:
> Otherwise we seem to get FIFO underruns. It is being disabled
> anyway, so kind of logical to write those as zeroes, even if
> disabling is temporary.
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  .../drm/i915/display/skl_universal_plane.c    |  2 +-
>  drivers/gpu/drm/i915/intel_pm.c               | 46 +++++++++++++++++++
>  drivers/gpu/drm/i915/intel_pm.h               |  2 +
>  3 files changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index caa03324a733..c0251189c042 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -633,7 +633,7 @@ icl_plane_disable_arm(struct intel_plane *plane,
>  	if (icl_is_hdr_plane(dev_priv, plane_id))
>  		intel_de_write_fw(dev_priv, PLANE_CUS_CTL(pipe, plane_id), 0);
>  
> -	skl_write_plane_wm(plane, crtc_state);
> +	skl_write_zero_plane_wm(plane, crtc_state);
>  
>  	intel_psr2_disable_plane_sel_fetch(plane, crtc_state);
>  	intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), 0);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index ee0047fdc95d..2470c037bfae 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5885,6 +5885,52 @@ void skl_write_plane_wm(struct intel_plane *plane,
>  				    PLANE_NV12_BUF_CFG(pipe, plane_id), ddb_y);
>  }
>  
> +void skl_write_zero_plane_wm(struct intel_plane *plane,
> +			     const struct intel_crtc_state *crtc_state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> +	int level, max_level = ilk_wm_max_level(dev_priv);
> +	enum plane_id plane_id = plane->id;
> +	enum pipe pipe = plane->pipe;
> +	struct skl_pipe_wm pipe_wm;
> +	const struct skl_ddb_entry *ddb =
> +		&crtc_state->wm.skl.plane_ddb[plane_id];
> +	const struct skl_ddb_entry *ddb_y =
> +		&crtc_state->wm.skl.plane_ddb_y[plane_id];
> +
> +	for (level = 0; level <= max_level; level++) {
> +		pipe_wm.planes[plane_id].wm[level].blocks = 0;
> +		pipe_wm.planes[plane_id].wm[level].lines = 0;
> +	}
> +
> +	pipe_wm.planes[plane_id].trans_wm.blocks = 0;
> +	pipe_wm.planes[plane_id].trans_wm.lines = 0;

What about the other members of struct skl_plane_wm - uv_wm, sagv.wm, sagv.trans_wm?
Or memset to 0 to the sizeof(skl_wm_level) better?

> +
> +	for (level = 0; level <= max_level; level++)
> +		skl_write_wm_level(dev_priv, PLANE_WM(pipe, plane_id, level),
> +				   skl_plane_wm_level(&pipe_wm, plane_id, level));
> +
> +	skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane_id),
> +			   skl_plane_trans_wm(&pipe_wm, plane_id));
> +
> +	if (HAS_HW_SAGV_WM(dev_priv)) {
> +		const struct skl_plane_wm *wm = &pipe_wm.planes[plane_id];
> +
> +		skl_write_wm_level(dev_priv, PLANE_WM_SAGV(pipe, plane_id),
> +				   &wm->sagv.wm0);
> +		skl_write_wm_level(dev_priv, PLANE_WM_SAGV_TRANS(pipe, plane_id),
> +				   &wm->sagv.trans_wm);
> +	}
> +
> +	skl_ddb_entry_write(dev_priv,
> +			    PLANE_BUF_CFG(pipe, plane_id), ddb);

As the plane wm values are hardcode to 0 just before this line, these ddb entries might not be
representing the accurate information anymore. Does that even matter?

Or is it better if we ignore/skip zero-ing the wm values during the disable planes completely if it
doesnt matter and just rely on disabling the plane bit in plane_ctl?

> +
> +	if (DISPLAY_VER(dev_priv) < 11)
> +		skl_ddb_entry_write(dev_priv,
> +				    PLANE_NV12_BUF_CFG(pipe, plane_id), ddb_y);
> +}
> +
> +
>  void skl_write_cursor_wm(struct intel_plane *plane,
>  			 const struct intel_crtc_state *crtc_state)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
> index 50604cf7398c..fb0ac4f143ab 100644
> --- a/drivers/gpu/drm/i915/intel_pm.h
> +++ b/drivers/gpu/drm/i915/intel_pm.h
> @@ -61,6 +61,8 @@ bool skl_wm_level_equals(const struct skl_wm_level *l1,
>  bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry *ddb,
>  				 const struct skl_ddb_entry *entries,
>  				 int num_entries, int ignore_idx);
> +void skl_write_zero_plane_wm(struct intel_plane *plane,
> +			     const struct intel_crtc_state *crtc_state);
>  void skl_write_plane_wm(struct intel_plane *plane,
>  			const struct intel_crtc_state *crtc_state);
>  void skl_write_cursor_wm(struct intel_plane *plane,

Re: [Intel-gfx] [PATCH] drm/i915: Write zero wms if we disable planes for icl+

[Lisovskiy, Stanislav]

On Mon, May 23, 2022 at 11:06:05AM +0300, Govindapillai, Vinod wrote:
> Hi Stan
> 
> Pls see some comments inline..
> 
> BR
> Vinod
> 
> On Wed, 2022-05-18 at 13:59 +0300, Stanislav Lisovskiy wrote:
> > Otherwise we seem to get FIFO underruns. It is being disabled
> > anyway, so kind of logical to write those as zeroes, even if
> > disabling is temporary.
> >
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > ---
> >  .../drm/i915/display/skl_universal_plane.c    |  2 +-
> >  drivers/gpu/drm/i915/intel_pm.c               | 46 +++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_pm.h               |  2 +
> >  3 files changed, 49 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > index caa03324a733..c0251189c042 100644
> > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > @@ -633,7 +633,7 @@ icl_plane_disable_arm(struct intel_plane *plane,
> >       if (icl_is_hdr_plane(dev_priv, plane_id))
> >               intel_de_write_fw(dev_priv, PLANE_CUS_CTL(pipe, plane_id), 0);
> >
> > -     skl_write_plane_wm(plane, crtc_state);
> > +     skl_write_zero_plane_wm(plane, crtc_state);
> >
> >       intel_psr2_disable_plane_sel_fetch(plane, crtc_state);
> >       intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), 0);
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index ee0047fdc95d..2470c037bfae 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -5885,6 +5885,52 @@ void skl_write_plane_wm(struct intel_plane *plane,
> >                                   PLANE_NV12_BUF_CFG(pipe, plane_id), ddb_y);
> >  }
> >
> > +void skl_write_zero_plane_wm(struct intel_plane *plane,
> > +                          const struct intel_crtc_state *crtc_state)
> > +{
> > +     struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> > +     int level, max_level = ilk_wm_max_level(dev_priv);
> > +     enum plane_id plane_id = plane->id;
> > +     enum pipe pipe = plane->pipe;
> > +     struct skl_pipe_wm pipe_wm;
> > +     const struct skl_ddb_entry *ddb =
> > +             &crtc_state->wm.skl.plane_ddb[plane_id];
> > +     const struct skl_ddb_entry *ddb_y =
> > +             &crtc_state->wm.skl.plane_ddb_y[plane_id];
> > +
> > +     for (level = 0; level <= max_level; level++) {
> > +             pipe_wm.planes[plane_id].wm[level].blocks = 0;
> > +             pipe_wm.planes[plane_id].wm[level].lines = 0;
> > +     }
> > +
> > +     pipe_wm.planes[plane_id].trans_wm.blocks = 0;
> > +     pipe_wm.planes[plane_id].trans_wm.lines = 0;
> 
> What about the other members of struct skl_plane_wm - uv_wm, sagv.wm, sagv.trans_wm?
> Or memset to 0 to the sizeof(skl_wm_level) better?

Yes, you are right the whole structure should be zeroed. However that seems to be enough
in practice though..well theretically whatever we are writing here, shouldn't even
matter because we would be disabling the plane soon rightaway.

> 
> > +
> > +     for (level = 0; level <= max_level; level++)
> > +             skl_write_wm_level(dev_priv, PLANE_WM(pipe, plane_id, level),
> > +                                skl_plane_wm_level(&pipe_wm, plane_id, level));
> > +
> > +     skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane_id),
> > +                        skl_plane_trans_wm(&pipe_wm, plane_id));
> > +
> > +     if (HAS_HW_SAGV_WM(dev_priv)) {
> > +             const struct skl_plane_wm *wm = &pipe_wm.planes[plane_id];
> > +
> > +             skl_write_wm_level(dev_priv, PLANE_WM_SAGV(pipe, plane_id),
> > +                                &wm->sagv.wm0);
> > +             skl_write_wm_level(dev_priv, PLANE_WM_SAGV_TRANS(pipe, plane_id),
> > +                                &wm->sagv.trans_wm);
> > +     }
> > +
> > +     skl_ddb_entry_write(dev_priv,
> > +                         PLANE_BUF_CFG(pipe, plane_id), ddb);
> 
> As the plane wm values are hardcode to 0 just before this line, these ddb entries might not be
> representing the accurate information anymore. Does that even matter?

It shouldn't matter at all since the plane is disabled.

> 
> Or is it better if we ignore/skip zero-ing the wm values during the disable planes completely if it
> doesnt matter and just rely on disabling the plane bit in plane_ctl?


I now tend to do the same. Writing zeroed struct doesn't seem to be correct option
either, otherwise this should be reflected in state as well - otherwise we are getting
state mismatches.
I think we shouldn't even touch wms here - it doesn't make sense to update
them if we are disabling the plane anyway.
If it was enabled before - I think it should have those correct wms already,
which we are writing here(those from the state).
And once it is enabled -  we should anyway write new wm values there.

I will probably send a new version, where we don't write those at all,
when plane is disabled.

Also I have suspicion, that I can't reproduce this with intel_reg, because
it might be that in the driver we simply can be having race condition here - 
we have now removed spinlocks, in order to get rid of another bug(atomic
evasion time exceeded), so now it is more probable to get this kind of 
issue. That might explain, why writing wms here matters while it shouldn't really
and it is also not reproducible with direct register writes with intel_reg
tool. 

Stan

> 
> > +
> > +     if (DISPLAY_VER(dev_priv) < 11)
> > +             skl_ddb_entry_write(dev_priv,
> > +                                 PLANE_NV12_BUF_CFG(pipe, plane_id), ddb_y);
> > +}
> > +
> > +
> >  void skl_write_cursor_wm(struct intel_plane *plane,
> >                        const struct intel_crtc_state *crtc_state)
> >  {
> > diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
> > index 50604cf7398c..fb0ac4f143ab 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.h
> > +++ b/drivers/gpu/drm/i915/intel_pm.h
> > @@ -61,6 +61,8 @@ bool skl_wm_level_equals(const struct skl_wm_level *l1,
> >  bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry *ddb,
> >                                const struct skl_ddb_entry *entries,
> >                                int num_entries, int ignore_idx);
> > +void skl_write_zero_plane_wm(struct intel_plane *plane,
> > +                          const struct intel_crtc_state *crtc_state);
> >  void skl_write_plane_wm(struct intel_plane *plane,
> >                       const struct intel_crtc_state *crtc_state);
> >  void skl_write_cursor_wm(struct intel_plane *plane,