[Intel-gfx] [PATCH 0/2] Fixes for selective fetch area calculation

[Intel-gfx] [PATCH 0/2] Fixes for selective fetch area calculation

[Jouni Högander]

Currently selective fetch area calculation ends up as bogus area in
following cases:

1. Updated plane is partially or fully outside pipe area
2. Big fb with only part of memory area used for plane

These end up as y1 = 0, y2 = 4 or y2 being outside pipe area. This
patch set addresses these by ensuring update area is within pipe area
and falling back to full update. 

Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Mika Kahola <mika.kahola@intel.com>
Cc: Mark Pearson <markpearson@lenovo.com>

Jouni Högander (2):
  drm/i915/psr: Use full update In case of area calculation fails
  drm/i915: Ensure damage clip area is within pipe area

 drivers/gpu/drm/i915/display/intel_psr.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

-- 
2.25.1

[Intel-gfx] [PATCH 1/2] drm/i915/psr: Use full update In case of area calculation fails

[Jouni Högander]

Currently we have some corner cases where area calculation fails.  For
these sel fetch are calculation ends up having update area as y1 = 0,
y2 = 4. Instead of these values safer option is full update.

Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Mika Kahola <mika.kahola@intel.com>
Tested-by: Mark Pearson <markpearson@lenovo.com>
Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 06db407e2749..8c099d24de86 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -1770,6 +1770,9 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
 		clip_area_update(&pipe_clip, &damaged_area);
 	}
 
+	if (pipe_clip.y1 == -1)
+		full_update = true;
+
 	if (full_update)
 		goto skip_sel_fetch_set_loop;
 
-- 
2.25.1

[Intel-gfx] [PATCH 2/2] drm/i915: Ensure damage clip area is within pipe area

[Jouni Högander]

Current update area calculation is not handling situation where
e.g. cursor plane is fully or partially outside pipe area.

Fix this by checking damage area against pipe_src area using
drm_rect_intersect.

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5440
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Mika Kahola <mika.kahola@intel.com>
Tested-by: Mark Pearson <markpearson@lenovo.com>
Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 8c099d24de86..5229ba89a079 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -1724,6 +1724,10 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
 			break;
 		}
 
+		/* Set x1 and x2 for drm_rect_intersect usage */
+		damaged_area.x1 = 0;
+		damaged_area.x2 = INT_MAX;
+
 		/*
 		 * If visibility or plane moved, mark the whole plane area as
 		 * damaged as it needs to be complete redraw in the new and old
@@ -1735,20 +1739,23 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
 			if (old_plane_state->uapi.visible) {
 				damaged_area.y1 = old_plane_state->uapi.dst.y1;
 				damaged_area.y2 = old_plane_state->uapi.dst.y2;
-				clip_area_update(&pipe_clip, &damaged_area);
+				if (drm_rect_intersect(&damaged_area, &crtc_state->pipe_src))
+					clip_area_update(&pipe_clip, &damaged_area);
 			}
 
 			if (new_plane_state->uapi.visible) {
 				damaged_area.y1 = new_plane_state->uapi.dst.y1;
 				damaged_area.y2 = new_plane_state->uapi.dst.y2;
-				clip_area_update(&pipe_clip, &damaged_area);
+				if (drm_rect_intersect(&damaged_area, &crtc_state->pipe_src))
+					clip_area_update(&pipe_clip, &damaged_area);
 			}
 			continue;
 		} else if (new_plane_state->uapi.alpha != old_plane_state->uapi.alpha) {
 			/* If alpha changed mark the whole plane area as damaged */
 			damaged_area.y1 = new_plane_state->uapi.dst.y1;
 			damaged_area.y2 = new_plane_state->uapi.dst.y2;
-			clip_area_update(&pipe_clip, &damaged_area);
+			if (drm_rect_intersect(&damaged_area, &crtc_state->pipe_src))
+				clip_area_update(&pipe_clip, &damaged_area);
 			continue;
 		}
 
@@ -1767,7 +1774,9 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
 
 		damaged_area.y1 += new_plane_state->uapi.dst.y1 - src.y1;
 		damaged_area.y2 += new_plane_state->uapi.dst.y1 - src.y1;
-		clip_area_update(&pipe_clip, &damaged_area);
+
+		if (drm_rect_intersect(&damaged_area, &crtc_state->pipe_src))
+			clip_area_update(&pipe_clip, &damaged_area);
 	}
 
 	if (pipe_clip.y1 == -1)
-- 
2.25.1

[Intel-gfx] ✓ Fi.CI.BAT: success for Fixes for selective fetch area calculation

[Patchwork]

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

== Series Details ==

Series: Fixes for selective fetch area calculation
URL   : https://patchwork.freedesktop.org/series/103659/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_11616 -> Patchwork_103659v1
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Participating hosts (42 -> 36)
------------------------------

  Additional (1): fi-icl-u2 
  Missing    (7): bat-adls-5 bat-adlm-1 fi-bsw-cyan bat-adlp-6 bat-adln-1 bat-rpls-1 bat-rpls-2 

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

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

### CI changes ###

#### Issues hit ####

  * boot:
    - fi-snb-2600:        [PASS][1] -> [FAIL][2] ([i915#4338])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/fi-snb-2600/boot.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/fi-snb-2600/boot.html

  

### IGT changes ###

#### Issues hit ####

  * igt@runner@aborted:
    - fi-icl-u2:          NOTRUN -> [FAIL][3] ([i915#3690])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/fi-icl-u2/igt@runner@aborted.html

  
  [i915#3690]: https://gitlab.freedesktop.org/drm/intel/issues/3690
  [i915#4338]: https://gitlab.freedesktop.org/drm/intel/issues/4338


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

  * Linux: CI_DRM_11616 -> Patchwork_103659v1

  CI-20190529: 20190529
  CI_DRM_11616: 65a5fe9ac96c60bd6dfcc44a0bb8d584912ea53d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6467: 929abc51cdd48d673efa03e025b1f31b557972ed @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_103659v1: 65a5fe9ac96c60bd6dfcc44a0bb8d584912ea53d @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

699ac7b731d4 drm/i915: Ensure damage clip area is within pipe area
259af3dc5feb drm/i915/psr: Use full update In case of area calculation fails

== Logs ==

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

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

[Intel-gfx] ✓ Fi.CI.IGT: success for Fixes for selective fetch area calculation

[Patchwork]

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

== Series Details ==

Series: Fixes for selective fetch area calculation
URL   : https://patchwork.freedesktop.org/series/103659/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_11616_full -> Patchwork_103659v1_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Participating hosts (12 -> 8)
------------------------------

  Missing    (4): pig-skl-6260u pig-kbl-iris shard-rkl pig-glk-j5005 

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

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

### CI changes ###

#### Possible fixes ####

  * boot:
    - shard-snb:          ([PASS][1], [PASS][2], [PASS][3], [PASS][4], [PASS][5], [PASS][6], [PASS][7], [PASS][8], [PASS][9], [PASS][10], [PASS][11], [PASS][12], [PASS][13], [PASS][14], [PASS][15], [PASS][16], [PASS][17], [PASS][18], [FAIL][19], [PASS][20], [PASS][21], [PASS][22], [PASS][23], [PASS][24], [PASS][25]) ([i915#4338]) -> ([PASS][26], [PASS][27], [PASS][28], [PASS][29], [PASS][30], [PASS][31], [PASS][32], [PASS][33], [PASS][34], [PASS][35], [PASS][36], [PASS][37], [PASS][38], [PASS][39], [PASS][40], [PASS][41], [PASS][42], [PASS][43], [PASS][44], [PASS][45], [PASS][46], [PASS][47], [PASS][48], [PASS][49], [PASS][50])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb4/boot.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb2/boot.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb2/boot.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb2/boot.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb2/boot.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb7/boot.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb7/boot.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb7/boot.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb7/boot.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb7/boot.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb6/boot.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb6/boot.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb6/boot.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb6/boot.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb6/boot.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb5/boot.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb5/boot.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb5/boot.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb5/boot.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb5/boot.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb5/boot.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb4/boot.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb4/boot.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb4/boot.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb4/boot.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb6/boot.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb5/boot.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb5/boot.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb5/boot.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb5/boot.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb5/boot.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb4/boot.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb4/boot.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb4/boot.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb4/boot.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb4/boot.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb2/boot.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb2/boot.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb2/boot.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb2/boot.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb2/boot.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb6/boot.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb6/boot.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb6/boot.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb6/boot.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb7/boot.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb7/boot.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb7/boot.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb7/boot.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb7/boot.html

  

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_persistence@engines-mixed:
    - shard-snb:          NOTRUN -> [SKIP][51] ([fdo#109271] / [i915#1099]) +1 similar issue
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb2/igt@gem_ctx_persistence@engines-mixed.html

  * igt@gem_exec_flush@basic-uc-prw-default:
    - shard-snb:          [PASS][52] -> [SKIP][53] ([fdo#109271]) +5 similar issues
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb5/igt@gem_exec_flush@basic-uc-prw-default.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb6/igt@gem_exec_flush@basic-uc-prw-default.html

  * igt@kms_color@pipe-c-degamma:
    - shard-snb:          NOTRUN -> [SKIP][54] ([fdo#109271]) +75 similar issues
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb2/igt@kms_color@pipe-c-degamma.html

  * igt@kms_color_chamelium@pipe-c-ctm-green-to-red:
    - shard-snb:          NOTRUN -> [SKIP][55] ([fdo#109271] / [fdo#111827]) +3 similar issues
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb2/igt@kms_color_chamelium@pipe-c-ctm-green-to-red.html

  
#### Possible fixes ####

  * igt@gem_exec_fair@basic-none-share@rcs0:
    - {shard-tglu}:       [FAIL][56] ([i915#2842]) -> [PASS][57]
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-tglu-2/igt@gem_exec_fair@basic-none-share@rcs0.html
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-tglu-2/igt@gem_exec_fair@basic-none-share@rcs0.html

  * igt@gem_exec_flush@basic-wb-ro-before-default:
    - shard-snb:          [SKIP][58] ([fdo#109271]) -> [PASS][59] +2 similar issues
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11616/shard-snb6/igt@gem_exec_flush@basic-wb-ro-before-default.html
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103659v1/shard-snb2/igt@gem_exec_flush@basic-wb-ro-before-default.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#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109279]: https://bugs.freedesktop.org/show_bug.cgi?id=109279
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#109312]: https://bugs.freedesktop.org/show_bug.cgi?id=109312
  [fdo#109506]: https://bugs.freedesktop.org/show_bug.cgi?id=109506
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111614]: https://bugs.freedesktop.org/show_bug.cgi?id=111614
  [fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
  [fdo#111644]: https://bugs.freedesktop.org/show_bug.cgi?id=111644
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [fdo#112283]: https://bugs.freedesktop.org/show_bug.cgi?id=112283
  [i915#1063]: https://gitlab.freedesktop.org/drm/intel/issues/1063
  [i915#1099]: https://gitlab.freedesktop.org/drm/intel/issues/1099
  [i915#1155]: https://gitlab.freedesktop.org/drm/intel/issues/1155
  [i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
  [i915#1769]: https://gitlab.freedesktop.org/drm/intel/issues/1769
  [i915#1836]: https://gitlab.freedesktop.org/drm/intel/issues/1836
  [i915#1839]: https://gitlab.freedesktop.org/drm/intel/issues/1839
  [i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
  [i915#2530]: https://gitlab.freedesktop.org/drm/intel/issues/2530
  [i915#2658]: https://gitlab.freedesktop.org/drm/intel/issues/2658
  [i915#280]: https://gitlab.freedesktop.org/drm/intel/issues/280
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2856]: https://gitlab.freedesktop.org/drm/intel/issues/2856
  [i915#2994]: https://gitlab.freedesktop.org/drm/intel/issues/2994
  [i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3318]: https://gitlab.freedesktop.org/drm/intel/issues/3318
  [i915#3319]: https://gitlab.freedesktop.org/drm/intel/issues/3319
  [i915#3323]: https://gitlab.freedesktop.org/drm/intel/issues/3323
  [i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
  [i915#3469]: https://gitlab.freedesktop.org/drm/intel/issues/3469
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
  [i915#3828]: https://gitlab.freedesktop.org/drm/intel/issues/3828
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#3966]: https://gitlab.freedesktop.org/drm/intel/issues/3966
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#433]: https://gitlab.freedesktop.org/drm/intel/issues/433
  [i915#4338]: https://gitlab.freedesktop.org/drm/intel/issues/4338
  [i915#4387]: https://gitlab.freedesktop.org/drm/intel/issues/4387
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#5076]: https://gitlab.freedesktop.org/drm/intel/issues/5076
  [i915#5098]: https://gitlab.freedesktop.org/drm/intel/issues/5098
  [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
  [i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
  [i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
  [i915#5287]: https://gitlab.freedesktop.org/drm/intel/issues/5287
  [i915#5288]: https://gitlab.freedesktop.org/drm/intel/issues/5288
  [i915#5289]: https://gitlab.freedesktop.org/drm/intel/issues/5289
  [i915#5325]: https://gitlab.freedesktop.org/drm/intel/issues/5325
  [i915#5614]: https://gitlab.freedesktop.org/drm/intel/issues/5614
  [i915#5723]: https://gitlab.freedesktop.org/drm/intel/issues/5723
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658


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

  * Linux: CI_DRM_11616 -> Patchwork_103659v1
  * Piglit: piglit_4509 -> None

  CI-20190529: 20190529
  CI_DRM_11616: 65a5fe9ac96c60bd6dfcc44a0bb8d584912ea53d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6467: 929abc51cdd48d673efa03e025b1f31b557972ed @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_103659v1: 65a5fe9ac96c60bd6dfcc44a0bb8d584912ea53d @ 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_103659v1/index.html

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

Re: [Intel-gfx] [PATCH 1/2] drm/i915/psr: Use full update In case of area calculation fails

[Souza, Jose]

On Fri, 2022-05-06 at 08:48 +0300, Jouni Högander wrote:
> Currently we have some corner cases where area calculation fails.  For
> these sel fetch are calculation ends up having update area as y1 = 0,
> y2 = 4. Instead of these values safer option is full update.

Aren't you able to reproduce this scenarios with IGT? So why not probably fix the calculations?

> 
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Mika Kahola <mika.kahola@intel.com>
> Tested-by: Mark Pearson <markpearson@lenovo.com>
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 06db407e2749..8c099d24de86 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1770,6 +1770,9 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
>  		clip_area_update(&pipe_clip, &damaged_area);
>  	}
>  
> +	if (pipe_clip.y1 == -1)
> +		full_update = true;
> +
>  	if (full_update)
>  		goto skip_sel_fetch_set_loop;
>  

Re: [Intel-gfx] [PATCH 2/2] drm/i915: Ensure damage clip area is within pipe area

[Souza, Jose]

On Fri, 2022-05-06 at 08:48 +0300, Jouni Högander wrote:
> Current update area calculation is not handling situation where
> e.g. cursor plane is fully or partially outside pipe area.
> 
> Fix this by checking damage area against pipe_src area using
> drm_rect_intersect.
> 
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5440
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Mika Kahola <mika.kahola@intel.com>
> Tested-by: Mark Pearson <markpearson@lenovo.com>
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 8c099d24de86..5229ba89a079 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1724,6 +1724,10 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
>  			break;
>  		}
>  
> +		/* Set x1 and x2 for drm_rect_intersect usage */
> +		damaged_area.x1 = 0;
> +		damaged_area.x2 = INT_MAX;

Move the above to the variable definition and initialization.

> +
>  		/*
>  		 * If visibility or plane moved, mark the whole plane area as
>  		 * damaged as it needs to be complete redraw in the new and old
> @@ -1735,20 +1739,23 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
>  			if (old_plane_state->uapi.visible) {
>  				damaged_area.y1 = old_plane_state->uapi.dst.y1;
>  				damaged_area.y2 = old_plane_state->uapi.dst.y2;
> -				clip_area_update(&pipe_clip, &damaged_area);
> +				if (drm_rect_intersect(&damaged_area, &crtc_state->pipe_src))
> +					clip_area_update(&pipe_clip, &damaged_area);
>  			}
>  
>  			if (new_plane_state->uapi.visible) {
>  				damaged_area.y1 = new_plane_state->uapi.dst.y1;
>  				damaged_area.y2 = new_plane_state->uapi.dst.y2;
> -				clip_area_update(&pipe_clip, &damaged_area);
> +				if (drm_rect_intersect(&damaged_area, &crtc_state->pipe_src))
> +					clip_area_update(&pipe_clip, &damaged_area);
>  			}
>  			continue;
>  		} else if (new_plane_state->uapi.alpha != old_plane_state->uapi.alpha) {
>  			/* If alpha changed mark the whole plane area as damaged */
>  			damaged_area.y1 = new_plane_state->uapi.dst.y1;
>  			damaged_area.y2 = new_plane_state->uapi.dst.y2;
> -			clip_area_update(&pipe_clip, &damaged_area);
> +			if (drm_rect_intersect(&damaged_area, &crtc_state->pipe_src))
> +				clip_area_update(&pipe_clip, &damaged_area);
>  			continue;
>  		}
>  
> @@ -1767,7 +1774,9 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
>  
>  		damaged_area.y1 += new_plane_state->uapi.dst.y1 - src.y1;
>  		damaged_area.y2 += new_plane_state->uapi.dst.y1 - src.y1;
> -		clip_area_update(&pipe_clip, &damaged_area);
> +
> +		if (drm_rect_intersect(&damaged_area, &crtc_state->pipe_src))
> +			clip_area_update(&pipe_clip, &damaged_area);

As it is repeating move the drm_rect_intersect() call to clip_area_update(), adding a crtc_state parameter or pipe_src, your call.

Also please include a Fixes tag to some commit that makes sense so this patch is backported to older kernels with selective fetch enabled.

>  	}
>  
>  	if (pipe_clip.y1 == -1)

Re: [Intel-gfx] [PATCH 1/2] drm/i915/psr: Use full update In case of area calculation fails

[Hogander, Jouni]

On Fri, 2022-05-06 at 15:29 +0000, Souza, Jose wrote:
> On Fri, 2022-05-06 at 08:48 +0300, Jouni Högander wrote:
> > Currently we have some corner cases where area calculation
> > fails.  For
> > these sel fetch are calculation ends up having update area as y1 =
> > 0,
> > y2 = 4. Instead of these values safer option is full update.
> 
> Aren't you able to reproduce this scenarios with IGT? So why not
> probably fix the calculations?

There were some discussion with Ville Syrjälä that the proper fix for
this would be to move psr update area calculation into where other
calculations for planes are done. Currently we don't have e.g. proper
offset information available here. I have this in my tasklist, but been
busy with other tracks.

I'm also concerned generally on the first loop possibly ending up with
y1=-1,y2=-1 values due to other reasons as well. So using that
full_update prevents this posibility completely.

If I forget how I originally found this problem with bigfb I think this
backup using full update if something goes wrong is generally a good
idea. Currently it's just using y1=0,y2=4.

> 
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Cc: Mika Kahola <mika.kahola@intel.com>
> > Tested-by: Mark Pearson <markpearson@lenovo.com>
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 06db407e2749..8c099d24de86 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -1770,6 +1770,9 @@ int intel_psr2_sel_fetch_update(struct
> > intel_atomic_state *state,
> >  clip_area_update(&pipe_clip, &damaged_area);
> >  }
> > 
> > +if (pipe_clip.y1 == -1)
> > +full_update = true;
> > +
> >  if (full_update)
> >  goto skip_sel_fetch_set_loop;
> > 

Re: [Intel-gfx] [PATCH 1/2] drm/i915/psr: Use full update In case of area calculation fails

[Souza, Jose]

On Fri, 2022-05-06 at 18:28 +0000, Hogander, Jouni wrote:
> On Fri, 2022-05-06 at 15:29 +0000, Souza, Jose wrote:
> > On Fri, 2022-05-06 at 08:48 +0300, Jouni Högander wrote:
> > > Currently we have some corner cases where area calculation
> > > fails.  For
> > > these sel fetch are calculation ends up having update area as y1 =
> > > 0,
> > > y2 = 4. Instead of these values safer option is full update.
> > 
> > Aren't you able to reproduce this scenarios with IGT? So why not
> > probably fix the calculations?
> 
> There were some discussion with Ville Syrjälä that the proper fix for
> this would be to move psr update area calculation into where other
> calculations for planes are done. Currently we don't have e.g. proper
> offset information available here. I have this in my tasklist, but been
> busy with other tracks.

Okay so please add some of that to the commit description.

> 
> I'm also concerned generally on the first loop possibly ending up with
> y1=-1,y2=-1 values due to other reasons as well. So using that
> full_update prevents this posibility completely.
> 
> If I forget how I originally found this problem with bigfb I think this
> backup using full update if something goes wrong is generally a good
> idea. Currently it's just using y1=0,y2=4.
> 
> > 
> > > Cc: José Roberto de Souza <jose.souza@intel.com>
> > > Cc: Mika Kahola <mika.kahola@intel.com>
> > > Tested-by: Mark Pearson <markpearson@lenovo.com>
> > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_psr.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index 06db407e2749..8c099d24de86 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -1770,6 +1770,9 @@ int intel_psr2_sel_fetch_update(struct
> > > intel_atomic_state *state,
> > >  clip_area_update(&pipe_clip, &damaged_area);
> > >  }
> > > 

Add a TODO and a "drm_DEBUG_ONCE()"(check drm_WARN_ONCE) here so we get a warning about this at least once and this is not forgot.

> > > +if (pipe_clip.y1 == -1)
> > > +full_update = true;
> > > +
> > >  if (full_update)
> > >  goto skip_sel_fetch_set_loop;
> > > 
> 

Re: [Intel-gfx] [PATCH 2/2] drm/i915: Ensure damage clip area is within pipe area

[Hogander, Jouni]

On Fri, 2022-05-06 at 15:43 +0000, Souza, Jose wrote:
> On Fri, 2022-05-06 at 08:48 +0300, Jouni Högander wrote:
> > Current update area calculation is not handling situation where
> > e.g. cursor plane is fully or partially outside pipe area.
> > 
> > Fix this by checking damage area against pipe_src area using
> > drm_rect_intersect.
> > 
> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5440
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Cc: Mika Kahola <mika.kahola@intel.com>
> > Tested-by: Mark Pearson <markpearson@lenovo.com>
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 17 +++++++++++++----
> >  1 file changed, 13 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 8c099d24de86..5229ba89a079 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -1724,6 +1724,10 @@ int intel_psr2_sel_fetch_update(struct
> > intel_atomic_state *state,
> >  break;
> >  }
> > 
> > +/* Set x1 and x2 for drm_rect_intersect usage */
> > +damaged_area.x1 = 0;
> > +damaged_area.x2 = INT_MAX;
> 
> Move the above to the variable definition and initialization.
> 

Fixed, please check new version.

> > +
> >  /*
> >   * If visibility or plane moved, mark the whole plane area as
> >   * damaged as it needs to be complete redraw in the new and old
> > @@ -1735,20 +1739,23 @@ int intel_psr2_sel_fetch_update(struct
> > intel_atomic_state *state,
> >  if (old_plane_state->uapi.visible) {
> >  damaged_area.y1 = old_plane_state->uapi.dst.y1;
> >  damaged_area.y2 = old_plane_state->uapi.dst.y2;
> > -clip_area_update(&pipe_clip, &damaged_area);
> > +if (drm_rect_intersect(&damaged_area, &crtc_state->pipe_src))
> > +clip_area_update(&pipe_clip, &damaged_area);
> >  }
> > 
> >  if (new_plane_state->uapi.visible) {
> >  damaged_area.y1 = new_plane_state->uapi.dst.y1;
> >  damaged_area.y2 = new_plane_state->uapi.dst.y2;
> > -clip_area_update(&pipe_clip, &damaged_area);
> > +if (drm_rect_intersect(&damaged_area, &crtc_state->pipe_src))
> > +clip_area_update(&pipe_clip, &damaged_area);
> >  }
> >  continue;
> >  } else if (new_plane_state->uapi.alpha != old_plane_state-
> > >uapi.alpha) {
> >  /* If alpha changed mark the whole plane area as damaged */
> >  damaged_area.y1 = new_plane_state->uapi.dst.y1;
> >  damaged_area.y2 = new_plane_state->uapi.dst.y2;
> > -clip_area_update(&pipe_clip, &damaged_area);
> > +if (drm_rect_intersect(&damaged_area, &crtc_state->pipe_src))
> > +clip_area_update(&pipe_clip, &damaged_area);
> >  continue;
> >  }
> > 
> > @@ -1767,7 +1774,9 @@ int intel_psr2_sel_fetch_update(struct
> > intel_atomic_state *state,
> > 
> >  damaged_area.y1 += new_plane_state->uapi.dst.y1 - src.y1;
> >  damaged_area.y2 += new_plane_state->uapi.dst.y1 - src.y1;
> > -clip_area_update(&pipe_clip, &damaged_area);
> > +
> > +if (drm_rect_intersect(&damaged_area, &crtc_state->pipe_src))
> > +clip_area_update(&pipe_clip, &damaged_area);
> 
> As it is repeating move the drm_rect_intersect() call to
> clip_area_update(), adding a crtc_state parameter or pipe_src, your
> call.
> 
> Also please include a Fixes tag to some commit that makes sense so
> this patch is backported to older kernels with selective fetch
> enabled.
> 
> >  }
> > 
> >  if (pipe_clip.y1 == -1)

Re: [Intel-gfx] [PATCH 1/2] drm/i915/psr: Use full update In case of area calculation fails

[Hogander, Jouni]

On Fri, 2022-05-06 at 18:40 +0000, Souza, Jose wrote:
> On Fri, 2022-05-06 at 18:28 +0000, Hogander, Jouni wrote:
> > On Fri, 2022-05-06 at 15:29 +0000, Souza, Jose wrote:
> > > On Fri, 2022-05-06 at 08:48 +0300, Jouni Högander wrote:
> > > > Currently we have some corner cases where area calculation
> > > > fails.  For
> > > > these sel fetch are calculation ends up having update area as
> > > > y1 =
> > > > 0,
> > > > y2 = 4. Instead of these values safer option is full update.
> > > 
> > > Aren't you able to reproduce this scenarios with IGT? So why not
> > > probably fix the calculations?
> > 
> > There were some discussion with Ville Syrjälä that the proper fix
> > for
> > this would be to move psr update area calculation into where other
> > calculations for planes are done. Currently we don't have e.g.
> > proper
> > offset information available here. I have this in my tasklist, but
> > been
> > busy with other tracks.
> 
> Okay so please add some of that to the commit description.

Added some of this into commit message in new version, please check.

> 
> > I'm also concerned generally on the first loop possibly ending up
> > with
> > y1=-1,y2=-1 values due to other reasons as well. So using that
> > full_update prevents this posibility completely.
> > 
> > If I forget how I originally found this problem with bigfb I think
> > this
> > backup using full update if something goes wrong is generally a
> > good
> > idea. Currently it's just using y1=0,y2=4.
> > 
> > > > Cc: José Roberto de Souza <jose.souza@intel.com>
> > > > Cc: Mika Kahola <mika.kahola@intel.com>
> > > > Tested-by: Mark Pearson <markpearson@lenovo.com>
> > > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_psr.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > index 06db407e2749..8c099d24de86 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > @@ -1770,6 +1770,9 @@ int intel_psr2_sel_fetch_update(struct
> > > > intel_atomic_state *state,
> > > >  clip_area_update(&pipe_clip, &damaged_area);
> > > >  }
> > > > 
> 
> Add a TODO and a "drm_DEBUG_ONCE()"(check drm_WARN_ONCE) here so we
> get a warning about this at least once and this is not forgot.

I left the warn out. There is some case during boot-up at least in
Fedora35. I.e. This warning would be there always.

How about if I just file own bug for this in gitlab and assign it to
myself?

> 
> > > > +if (pipe_clip.y1 == -1)
> > > > +full_update = true;
> > > > +
> > > >  if (full_update)
> > > >  goto skip_sel_fetch_set_loop;
> > > >