* [Intel-gfx] [PATCH] drm/i915: do not reset PLANE_SURF on plane disable on older gens
@ 2022-09-05 8:05 Andrzej Hajda
2022-09-05 8:53 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Andrzej Hajda @ 2022-09-05 8:05 UTC (permalink / raw)
To: intel-gfx; +Cc: Andrzej Hajda, Rodrigo Vivi
In case of ICL and older generations disabling plane and/or disabling
async update is always performed on vblank, but if async update is enabled
PLANE_SURF register is updated asynchronously. Writing 0 to PLANE_SURF
when plane is still enabled can cause DMAR/PIPE errors.
On the other side PLANE_SURF is used to arm plane registers - we need to
write to it to trigger update on VBLANK, writting current value should
be safe - the buffer address is valid till vblank.
Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
---
drivers/gpu/drm/i915/display/skl_universal_plane.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index bcfde81e4d0866..bc9ed60a2d349e 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -615,11 +615,13 @@ skl_plane_disable_arm(struct intel_plane *plane,
struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
enum plane_id plane_id = plane->id;
enum pipe pipe = plane->pipe;
+ u32 plane_surf;
skl_write_plane_wm(plane, crtc_state);
intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), 0);
- intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), 0);
+ plane_surf = intel_de_read_fw(dev_priv, PLANE_SURF(pipe, plane_id));
+ intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), plane_surf);
}
static void
@@ -629,6 +631,7 @@ icl_plane_disable_arm(struct intel_plane *plane,
struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
enum plane_id plane_id = plane->id;
enum pipe pipe = plane->pipe;
+ u32 plane_surf;
if (icl_is_hdr_plane(dev_priv, plane_id))
intel_de_write_fw(dev_priv, PLANE_CUS_CTL(pipe, plane_id), 0);
@@ -637,7 +640,8 @@ icl_plane_disable_arm(struct intel_plane *plane,
intel_psr2_disable_plane_sel_fetch(plane, crtc_state);
intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), 0);
- intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), 0);
+ plane_surf = intel_de_read_fw(dev_priv, PLANE_SURF(pipe, plane_id));
+ intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), plane_surf);
}
static bool
--
2.25.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: do not reset PLANE_SURF on plane disable on older gens
2022-09-05 8:05 [Intel-gfx] [PATCH] drm/i915: do not reset PLANE_SURF on plane disable on older gens Andrzej Hajda
@ 2022-09-05 8:53 ` Patchwork
2022-09-05 9:07 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
` (2 subsequent siblings)
3 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2022-09-05 8:53 UTC (permalink / raw)
To: Andrzej Hajda; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: do not reset PLANE_SURF on plane disable on older gens
URL : https://patchwork.freedesktop.org/series/108133/
State : warning
== Summary ==
Error: dim checkpatch failed
574a7dd04303 drm/i915: do not reset PLANE_SURF on plane disable on older gens
-:12: WARNING:TYPO_SPELLING: 'writting' may be misspelled - perhaps 'writing'?
#12:
write to it to trigger update on VBLANK, writting current value should
^^^^^^^^
total: 0 errors, 1 warnings, 0 checks, 30 lines checked
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: do not reset PLANE_SURF on plane disable on older gens
2022-09-05 8:05 [Intel-gfx] [PATCH] drm/i915: do not reset PLANE_SURF on plane disable on older gens Andrzej Hajda
2022-09-05 8:53 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2022-09-05 9:07 ` Patchwork
2022-09-05 10:59 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2022-09-05 11:48 ` [Intel-gfx] [PATCH] " Ville Syrjälä
3 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2022-09-05 9:07 UTC (permalink / raw)
To: Andrzej Hajda; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 4153 bytes --]
== Series Details ==
Series: drm/i915: do not reset PLANE_SURF on plane disable on older gens
URL : https://patchwork.freedesktop.org/series/108133/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_12070 -> Patchwork_108133v1
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/index.html
Participating hosts (36 -> 34)
------------------------------
Missing (2): bat-rpls-2 fi-bdw-samus
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_108133v1:
### IGT changes ###
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@core_hotunplug@unbind-rebind:
- {fi-jsl-1}: [PASS][1] -> [INCOMPLETE][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/fi-jsl-1/igt@core_hotunplug@unbind-rebind.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/fi-jsl-1/igt@core_hotunplug@unbind-rebind.html
Known issues
------------
Here are the changes found in Patchwork_108133v1 that come from known issues:
### IGT changes ###
#### Possible fixes ####
* igt@i915_pm_rpm@module-reload:
- fi-cfl-8109u: [DMESG-FAIL][3] ([i915#62]) -> [PASS][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/fi-cfl-8109u/igt@i915_pm_rpm@module-reload.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/fi-cfl-8109u/igt@i915_pm_rpm@module-reload.html
* igt@i915_selftest@live@reset:
- {bat-rpls-1}: [DMESG-FAIL][5] ([i915#4983] / [i915#5828]) -> [PASS][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/bat-rpls-1/igt@i915_selftest@live@reset.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/bat-rpls-1/igt@i915_selftest@live@reset.html
* igt@i915_selftest@live@ring_submission:
- fi-cfl-8109u: [DMESG-WARN][7] ([i915#5904]) -> [PASS][8] +30 similar issues
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/fi-cfl-8109u/igt@i915_selftest@live@ring_submission.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/fi-cfl-8109u/igt@i915_selftest@live@ring_submission.html
* igt@i915_suspend@basic-s2idle-without-i915:
- fi-cfl-8109u: [DMESG-WARN][9] ([i915#5904] / [i915#62]) -> [PASS][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/fi-cfl-8109u/igt@i915_suspend@basic-s2idle-without-i915.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/fi-cfl-8109u/igt@i915_suspend@basic-s2idle-without-i915.html
* igt@kms_frontbuffer_tracking@basic:
- fi-cfl-8109u: [DMESG-WARN][11] ([i915#62]) -> [PASS][12] +12 similar issues
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/fi-cfl-8109u/igt@kms_frontbuffer_tracking@basic.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/fi-cfl-8109u/igt@kms_frontbuffer_tracking@basic.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
[i915#5828]: https://gitlab.freedesktop.org/drm/intel/issues/5828
[i915#5904]: https://gitlab.freedesktop.org/drm/intel/issues/5904
[i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
Build changes
-------------
* Linux: CI_DRM_12070 -> Patchwork_108133v1
CI-20190529: 20190529
CI_DRM_12070: 6f924a2ec43ac35516136a77b1adad0b6d8f8faa @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_6643: 9b2970a6d495ddd9ceb2487fc289105bf05812df @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_108133v1: 6f924a2ec43ac35516136a77b1adad0b6d8f8faa @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
4a298447c67b drm/i915: do not reset PLANE_SURF on plane disable on older gens
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/index.html
[-- Attachment #2: Type: text/html, Size: 5104 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: do not reset PLANE_SURF on plane disable on older gens
2022-09-05 8:05 [Intel-gfx] [PATCH] drm/i915: do not reset PLANE_SURF on plane disable on older gens Andrzej Hajda
2022-09-05 8:53 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2022-09-05 9:07 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2022-09-05 10:59 ` Patchwork
2022-09-05 11:48 ` [Intel-gfx] [PATCH] " Ville Syrjälä
3 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2022-09-05 10:59 UTC (permalink / raw)
To: Andrzej Hajda; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 23578 bytes --]
== Series Details ==
Series: drm/i915: do not reset PLANE_SURF on plane disable on older gens
URL : https://patchwork.freedesktop.org/series/108133/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_12070_full -> Patchwork_108133v1_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
Participating hosts (11 -> 12)
------------------------------
Additional (1): shard-rkl
Known issues
------------
Here are the changes found in Patchwork_108133v1_full that come from known issues:
### CI changes ###
#### Issues hit ####
* boot:
- shard-glk: ([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], [PASS][19], [PASS][20], [PASS][21], [PASS][22], [PASS][23], [PASS][24], [PASS][25]) -> ([PASS][26], [PASS][27], [PASS][28], [PASS][29], [PASS][30], [PASS][31], [PASS][32], [FAIL][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]) ([i915#4392])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk1/boot.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk1/boot.html
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk1/boot.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk2/boot.html
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk2/boot.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk2/boot.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk3/boot.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk3/boot.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk3/boot.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk5/boot.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk5/boot.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk5/boot.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk6/boot.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk6/boot.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk6/boot.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk7/boot.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk7/boot.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk7/boot.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk8/boot.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk8/boot.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk8/boot.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk8/boot.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk9/boot.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk9/boot.html
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk9/boot.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk1/boot.html
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk1/boot.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk1/boot.html
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk1/boot.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk2/boot.html
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk2/boot.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk2/boot.html
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk3/boot.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk3/boot.html
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk3/boot.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk5/boot.html
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk5/boot.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk5/boot.html
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk6/boot.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk6/boot.html
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk6/boot.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk7/boot.html
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk7/boot.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk7/boot.html
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk8/boot.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk8/boot.html
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk8/boot.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk9/boot.html
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk9/boot.html
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk9/boot.html
### IGT changes ###
#### Issues hit ####
* igt@gem_ctx_persistence@smoketest:
- shard-iclb: [PASS][51] -> [FAIL][52] ([i915#5099])
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-iclb7/igt@gem_ctx_persistence@smoketest.html
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-iclb8/igt@gem_ctx_persistence@smoketest.html
* igt@gem_exec_balancer@parallel-keep-in-fence:
- shard-iclb: [PASS][53] -> [SKIP][54] ([i915#4525]) +1 similar issue
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-iclb2/igt@gem_exec_balancer@parallel-keep-in-fence.html
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-iclb6/igt@gem_exec_balancer@parallel-keep-in-fence.html
* igt@gem_exec_fair@basic-flow@rcs0:
- shard-tglb: [PASS][55] -> [FAIL][56] ([i915#2842]) +1 similar issue
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-tglb3/igt@gem_exec_fair@basic-flow@rcs0.html
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-tglb2/igt@gem_exec_fair@basic-flow@rcs0.html
* igt@gem_exec_fair@basic-none-solo@rcs0:
- shard-apl: [PASS][57] -> [FAIL][58] ([i915#2842])
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-apl1/igt@gem_exec_fair@basic-none-solo@rcs0.html
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-apl3/igt@gem_exec_fair@basic-none-solo@rcs0.html
* igt@gem_huc_copy@huc-copy:
- shard-tglb: [PASS][59] -> [SKIP][60] ([i915#2190])
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-tglb2/igt@gem_huc_copy@huc-copy.html
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-tglb6/igt@gem_huc_copy@huc-copy.html
* igt@gen9_exec_parse@allowed-single:
- shard-apl: [PASS][61] -> [DMESG-WARN][62] ([i915#5566] / [i915#716])
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-apl7/igt@gen9_exec_parse@allowed-single.html
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-apl1/igt@gen9_exec_parse@allowed-single.html
* igt@kms_chamelium@dp-crc-single:
- shard-apl: NOTRUN -> [SKIP][63] ([fdo#109271] / [fdo#111827]) +1 similar issue
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-apl1/igt@kms_chamelium@dp-crc-single.html
* igt@kms_flip@flip-vs-expired-vblank-interruptible@c-dp1:
- shard-apl: [PASS][64] -> [FAIL][65] ([i915#79])
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-apl7/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-dp1.html
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-apl3/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-dp1.html
* igt@kms_flip_scaled_crc@flip-32bpp-4tile-to-64bpp-4tile-downscaling@pipe-a-valid-mode:
- shard-iclb: NOTRUN -> [SKIP][66] ([i915#2672]) +11 similar issues
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-iclb7/igt@kms_flip_scaled_crc@flip-32bpp-4tile-to-64bpp-4tile-downscaling@pipe-a-valid-mode.html
* igt@kms_flip_scaled_crc@flip-64bpp-xtile-to-16bpp-xtile-downscaling@pipe-a-default-mode:
- shard-iclb: NOTRUN -> [SKIP][67] ([i915#3555]) +1 similar issue
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-iclb2/igt@kms_flip_scaled_crc@flip-64bpp-xtile-to-16bpp-xtile-downscaling@pipe-a-default-mode.html
* igt@kms_flip_scaled_crc@flip-64bpp-ytile-to-32bpp-ytilegen12rcccs-upscaling@pipe-a-default-mode:
- shard-iclb: NOTRUN -> [SKIP][68] ([i915#2672] / [i915#3555])
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-iclb3/igt@kms_flip_scaled_crc@flip-64bpp-ytile-to-32bpp-ytilegen12rcccs-upscaling@pipe-a-default-mode.html
* igt@kms_psr@psr2_primary_mmap_cpu:
- shard-iclb: [PASS][69] -> [SKIP][70] ([fdo#109441]) +3 similar issues
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-iclb6/igt@kms_psr@psr2_primary_mmap_cpu.html
* igt@kms_psr@psr2_sprite_mmap_cpu:
- shard-apl: NOTRUN -> [SKIP][71] ([fdo#109271]) +10 similar issues
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-apl1/igt@kms_psr@psr2_sprite_mmap_cpu.html
* igt@kms_vblank@pipe-b-ts-continuation-suspend:
- shard-apl: [PASS][72] -> [DMESG-WARN][73] ([i915#180]) +3 similar issues
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-apl3/igt@kms_vblank@pipe-b-ts-continuation-suspend.html
[73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-apl8/igt@kms_vblank@pipe-b-ts-continuation-suspend.html
#### Possible fixes ####
* igt@gem_exec_fair@basic-pace-share@rcs0:
- shard-glk: [FAIL][74] ([i915#2842]) -> [PASS][75]
[74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk8/igt@gem_exec_fair@basic-pace-share@rcs0.html
[75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk1/igt@gem_exec_fair@basic-pace-share@rcs0.html
* igt@kms_cursor_crc@cursor-suspend@pipe-b-dp-1:
- shard-apl: [DMESG-WARN][76] ([i915#180]) -> [PASS][77]
[76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-apl3/igt@kms_cursor_crc@cursor-suspend@pipe-b-dp-1.html
[77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-apl1/igt@kms_cursor_crc@cursor-suspend@pipe-b-dp-1.html
* igt@kms_flip@2x-plain-flip-fb-recreate@ab-hdmi-a1-hdmi-a2:
- shard-glk: [FAIL][78] ([i915#2122]) -> [PASS][79]
[78]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-glk2/igt@kms_flip@2x-plain-flip-fb-recreate@ab-hdmi-a1-hdmi-a2.html
[79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-glk5/igt@kms_flip@2x-plain-flip-fb-recreate@ab-hdmi-a1-hdmi-a2.html
* igt@kms_plane_scaling@planes-upscale-20x20-downscale-factor-0-5@pipe-b-edp-1:
- shard-iclb: [SKIP][80] ([i915#5235]) -> [PASS][81] +2 similar issues
[80]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-iclb2/igt@kms_plane_scaling@planes-upscale-20x20-downscale-factor-0-5@pipe-b-edp-1.html
[81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-iclb7/igt@kms_plane_scaling@planes-upscale-20x20-downscale-factor-0-5@pipe-b-edp-1.html
* igt@kms_psr@psr2_no_drrs:
- shard-iclb: [SKIP][82] ([fdo#109441]) -> [PASS][83] +2 similar issues
[82]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-iclb8/igt@kms_psr@psr2_no_drrs.html
[83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-iclb2/igt@kms_psr@psr2_no_drrs.html
#### Warnings ####
* igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-fully-sf:
- shard-iclb: [SKIP][84] ([i915#658]) -> [SKIP][85] ([i915#2920]) +1 similar issue
[84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-iclb7/igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-fully-sf.html
[85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-iclb2/igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-fully-sf.html
* igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-big-fb:
- shard-iclb: [SKIP][86] ([i915#2920]) -> [SKIP][87] ([i915#658]) +1 similar issue
[86]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-iclb2/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-big-fb.html
[87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-iclb7/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-big-fb.html
* igt@kms_psr2_su@page_flip-nv12:
- shard-iclb: [FAIL][88] ([i915#5939]) -> [SKIP][89] ([fdo#109642] / [fdo#111068] / [i915#658]) +1 similar issue
[88]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-iclb2/igt@kms_psr2_su@page_flip-nv12.html
[89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-iclb6/igt@kms_psr2_su@page_flip-nv12.html
* igt@runner@aborted:
- shard-apl: ([FAIL][90], [FAIL][91], [FAIL][92], [FAIL][93]) ([i915#180] / [i915#3002] / [i915#4312] / [i915#5257] / [i915#6599]) -> ([FAIL][94], [FAIL][95], [FAIL][96], [FAIL][97], [FAIL][98], [FAIL][99], [FAIL][100]) ([fdo#109271] / [i915#180] / [i915#3002] / [i915#4312] / [i915#5257] / [i915#6599])
[90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-apl2/igt@runner@aborted.html
[91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-apl6/igt@runner@aborted.html
[92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-apl6/igt@runner@aborted.html
[93]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12070/shard-apl3/igt@runner@aborted.html
[94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-apl1/igt@runner@aborted.html
[95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-apl6/igt@runner@aborted.html
[96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-apl6/igt@runner@aborted.html
[97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-apl6/igt@runner@aborted.html
[98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-apl8/igt@runner@aborted.html
[99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-apl3/igt@runner@aborted.html
[100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108133v1/shard-apl3/igt@runner@aborted.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#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
[fdo#109283]: https://bugs.freedesktop.org/show_bug.cgi?id=109283
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[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#109302]: https://bugs.freedesktop.org/show_bug.cgi?id=109302
[fdo#109303]: https://bugs.freedesktop.org/show_bug.cgi?id=109303
[fdo#109308]: https://bugs.freedesktop.org/show_bug.cgi?id=109308
[fdo#109312]: https://bugs.freedesktop.org/show_bug.cgi?id=109312
[fdo#109314]: https://bugs.freedesktop.org/show_bug.cgi?id=109314
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[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#110542]: https://bugs.freedesktop.org/show_bug.cgi?id=110542
[fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
[fdo#111314]: https://bugs.freedesktop.org/show_bug.cgi?id=111314
[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#111656]: https://bugs.freedesktop.org/show_bug.cgi?id=111656
[fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
[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#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#1155]: https://gitlab.freedesktop.org/drm/intel/issues/1155
[i915#1257]: https://gitlab.freedesktop.org/drm/intel/issues/1257
[i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
[i915#1769]: https://gitlab.freedesktop.org/drm/intel/issues/1769
[i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
[i915#1839]: https://gitlab.freedesktop.org/drm/intel/issues/1839
[i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
[i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
[i915#1902]: https://gitlab.freedesktop.org/drm/intel/issues/1902
[i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
[i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
[i915#2530]: https://gitlab.freedesktop.org/drm/intel/issues/2530
[i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
[i915#2658]: https://gitlab.freedesktop.org/drm/intel/issues/2658
[i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
[i915#2705]: https://gitlab.freedesktop.org/drm/intel/issues/2705
[i915#280]: https://gitlab.freedesktop.org/drm/intel/issues/280
[i915#284]: https://gitlab.freedesktop.org/drm/intel/issues/284
[i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
[i915#2856]: https://gitlab.freedesktop.org/drm/intel/issues/2856
[i915#2920]: https://gitlab.freedesktop.org/drm/intel/issues/2920
[i915#2994]: https://gitlab.freedesktop.org/drm/intel/issues/2994
[i915#3002]: https://gitlab.freedesktop.org/drm/intel/issues/3002
[i915#3116]: https://gitlab.freedesktop.org/drm/intel/issues/3116
[i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
[i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
[i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
[i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
[i915#3299]: https://gitlab.freedesktop.org/drm/intel/issues/3299
[i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
[i915#3376]: https://gitlab.freedesktop.org/drm/intel/issues/3376
[i915#3528]: https://gitlab.freedesktop.org/drm/intel/issues/3528
[i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#3558]: https://gitlab.freedesktop.org/drm/intel/issues/3558
[i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
[i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
[i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
[i915#3742]: https://gitlab.freedesktop.org/drm/intel/issues/3742
[i915#3828]: https://gitlab.freedesktop.org/drm/intel/issues/3828
[i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
[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#426]: https://gitlab.freedesktop.org/drm/intel/issues/426
[i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
[i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
[i915#4369]: https://gitlab.freedesktop.org/drm/intel/issues/4369
[i915#4392]: https://gitlab.freedesktop.org/drm/intel/issues/4392
[i915#4525]: https://gitlab.freedesktop.org/drm/intel/issues/4525
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4877]: https://gitlab.freedesktop.org/drm/intel/issues/4877
[i915#4941]: https://gitlab.freedesktop.org/drm/intel/issues/4941
[i915#4991]: https://gitlab.freedesktop.org/drm/intel/issues/4991
[i915#5099]: https://gitlab.freedesktop.org/drm/intel/issues/5099
[i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
[i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
[i915#5257]: https://gitlab.freedesktop.org/drm/intel/issues/5257
[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#5325]: https://gitlab.freedesktop.org/drm/intel/issues/5325
[i915#5327]: https://gitlab.freedesktop.org/drm/intel/issues/5327
[i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
[i915#5439]: https://gitlab.freedesktop.org/drm/intel/issues/5439
[i915#5461]: https://gitlab.freedesktop.org/drm/intel/issues/5461
[i915#5566]: https://gitlab.freedesktop.org/drm/intel/issues/5566
[i915#5939]: https://gitlab.freedesktop.org/drm/intel/issues/5939
[i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
[i915#6117]: https://gitlab.freedesktop.org/drm/intel/issues/6117
[i915#6227]: https://gitlab.freedesktop.org/drm/intel/issues/6227
[i915#6230]: https://gitlab.freedesktop.org/drm/intel/issues/6230
[i915#6301]: https://gitlab.freedesktop.org/drm/intel/issues/6301
[i915#6344]: https://gitlab.freedesktop.org/drm/intel/issues/6344
[i915#6412]: https://gitlab.freedesktop.org/drm/intel/issues/6412
[i915#6433]: https://gitlab.freedesktop.org/drm/intel/issues/6433
[i915#6524]: https://gitlab.freedesktop.org/drm/intel/issues/6524
[i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
[i915#6599]: https://gitlab.freedesktop.org/drm/intel/issues/6599
[i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
[i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
Build changes
-------------
* Linux: CI_DRM_12070 -> Patchwork_108133v1
CI-20190529: 20190529
CI_DRM_12070: 6f924a2ec43ac35516136a77b1adad0b6d8f8faa @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_6643: 9b2970a6d495ddd9ceb2487fc289105bf05812df @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_108133v1: 6f924a2ec43ac35516136a77b1adad0b6d8f8faa @ 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_108133v1/index.html
[-- Attachment #2: Type: text/html, Size: 20112 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: do not reset PLANE_SURF on plane disable on older gens
2022-09-05 8:05 [Intel-gfx] [PATCH] drm/i915: do not reset PLANE_SURF on plane disable on older gens Andrzej Hajda
` (2 preceding siblings ...)
2022-09-05 10:59 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
@ 2022-09-05 11:48 ` Ville Syrjälä
2022-09-05 17:02 ` Andrzej Hajda
3 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2022-09-05 11:48 UTC (permalink / raw)
To: Andrzej Hajda; +Cc: intel-gfx, Rodrigo Vivi
On Mon, Sep 05, 2022 at 10:05:00AM +0200, Andrzej Hajda wrote:
> In case of ICL and older generations disabling plane and/or disabling
> async update is always performed on vblank,
It should only be broken on bdw-glk (see. need_async_flip_disable_wa).
> but if async update is enabled
> PLANE_SURF register is updated asynchronously. Writing 0 to PLANE_SURF
> when plane is still enabled can cause DMAR/PIPE errors.
> On the other side PLANE_SURF is used to arm plane registers - we need to
> write to it to trigger update on VBLANK, writting current value should
> be safe - the buffer address is valid till vblank.
I think you're effectively saying that somehow the async
flip disable w/a is not kicking in sometimes.
>
> Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
> ---
> drivers/gpu/drm/i915/display/skl_universal_plane.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index bcfde81e4d0866..bc9ed60a2d349e 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -615,11 +615,13 @@ skl_plane_disable_arm(struct intel_plane *plane,
> struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> enum plane_id plane_id = plane->id;
> enum pipe pipe = plane->pipe;
> + u32 plane_surf;
>
> skl_write_plane_wm(plane, crtc_state);
>
> intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), 0);
> - intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), 0);
> + plane_surf = intel_de_read_fw(dev_priv, PLANE_SURF(pipe, plane_id));
> + intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), plane_surf);
> }
>
> static void
> @@ -629,6 +631,7 @@ icl_plane_disable_arm(struct intel_plane *plane,
> struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> enum plane_id plane_id = plane->id;
> enum pipe pipe = plane->pipe;
> + u32 plane_surf;
>
> if (icl_is_hdr_plane(dev_priv, plane_id))
> intel_de_write_fw(dev_priv, PLANE_CUS_CTL(pipe, plane_id), 0);
> @@ -637,7 +640,8 @@ icl_plane_disable_arm(struct intel_plane *plane,
>
> intel_psr2_disable_plane_sel_fetch(plane, crtc_state);
> intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), 0);
> - intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), 0);
> + plane_surf = intel_de_read_fw(dev_priv, PLANE_SURF(pipe, plane_id));
> + intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), plane_surf);
> }
>
> static bool
> --
> 2.25.1
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: do not reset PLANE_SURF on plane disable on older gens
2022-09-05 11:48 ` [Intel-gfx] [PATCH] " Ville Syrjälä
@ 2022-09-05 17:02 ` Andrzej Hajda
2022-09-05 17:44 ` Ville Syrjälä
0 siblings, 1 reply; 16+ messages in thread
From: Andrzej Hajda @ 2022-09-05 17:02 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, Rodrigo Vivi
On 05.09.2022 13:48, Ville Syrjälä wrote:
> On Mon, Sep 05, 2022 at 10:05:00AM +0200, Andrzej Hajda wrote:
>> In case of ICL and older generations disabling plane and/or disabling
>> async update is always performed on vblank,
> It should only be broken on bdw-glk (see. need_async_flip_disable_wa).
On CFL it is reported every drmtip run:
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip.html?testfilter=tiled-max-hw
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html#dmesg-warnings402
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html#dmesg-warnings402
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1208/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
...
On APL it is less frequent, probably due to other bugs preventing run of
this test, last seen at:
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1190/fi-apl-guc/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
Similar for SKL:
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1181/fi-skl-guc/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
I am not sure if I correctly read the docs but [1] says that 9th bit of
PLANE_CFG (Async Address Update Enable) is "not double buffered and the
changes will apply immediately" only for ICL, JSL, LKF1.
So the change is not necessary in case of icl_plane_disable_arm.
[1]: https://gfxspecs.intel.com/Predator/Home/Index/7656
>
>> but if async update is enabled
>> PLANE_SURF register is updated asynchronously. Writing 0 to PLANE_SURF
>> when plane is still enabled can cause DMAR/PIPE errors.
>> On the other side PLANE_SURF is used to arm plane registers - we need to
>> write to it to trigger update on VBLANK, writting current value should
>> be safe - the buffer address is valid till vblank.
> I think you're effectively saying that somehow the async
> flip disable w/a is not kicking in sometimes.
I was not aware of existence of this w/a and I am little lost in
figuring out how this w/a can prevent zeroing PLANE_SURF too early.
Regards
Andrzej
>
>> Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
>> ---
>> drivers/gpu/drm/i915/display/skl_universal_plane.c | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> index bcfde81e4d0866..bc9ed60a2d349e 100644
>> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> @@ -615,11 +615,13 @@ skl_plane_disable_arm(struct intel_plane *plane,
>> struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
>> enum plane_id plane_id = plane->id;
>> enum pipe pipe = plane->pipe;
>> + u32 plane_surf;
>>
>> skl_write_plane_wm(plane, crtc_state);
>>
>> intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), 0);
>> - intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), 0);
>> + plane_surf = intel_de_read_fw(dev_priv, PLANE_SURF(pipe, plane_id));
>> + intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), plane_surf);
>> }
>>
>> static void
>> @@ -629,6 +631,7 @@ icl_plane_disable_arm(struct intel_plane *plane,
>> struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
>> enum plane_id plane_id = plane->id;
>> enum pipe pipe = plane->pipe;
>> + u32 plane_surf;
>>
>> if (icl_is_hdr_plane(dev_priv, plane_id))
>> intel_de_write_fw(dev_priv, PLANE_CUS_CTL(pipe, plane_id), 0);
>> @@ -637,7 +640,8 @@ icl_plane_disable_arm(struct intel_plane *plane,
>>
>> intel_psr2_disable_plane_sel_fetch(plane, crtc_state);
>> intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), 0);
>> - intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), 0);
>> + plane_surf = intel_de_read_fw(dev_priv, PLANE_SURF(pipe, plane_id));
>> + intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), plane_surf);
>> }
>>
>> static bool
>> --
>> 2.25.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: do not reset PLANE_SURF on plane disable on older gens
2022-09-05 17:02 ` Andrzej Hajda
@ 2022-09-05 17:44 ` Ville Syrjälä
2022-09-06 11:09 ` Andrzej Hajda
0 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2022-09-05 17:44 UTC (permalink / raw)
To: Andrzej Hajda; +Cc: intel-gfx, Rodrigo Vivi
On Mon, Sep 05, 2022 at 07:02:40PM +0200, Andrzej Hajda wrote:
>
>
> On 05.09.2022 13:48, Ville Syrjälä wrote:
> > On Mon, Sep 05, 2022 at 10:05:00AM +0200, Andrzej Hajda wrote:
> >> In case of ICL and older generations disabling plane and/or disabling
> >> async update is always performed on vblank,
> > It should only be broken on bdw-glk (see. need_async_flip_disable_wa).
>
> On CFL it is reported every drmtip run:
> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip.html?testfilter=tiled-max-hw
> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html#dmesg-warnings402
> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html#dmesg-warnings402
> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1208/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
> ...
> On APL it is less frequent, probably due to other bugs preventing run of
> this test, last seen at:
> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1190/fi-apl-guc/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
> Similar for SKL:
> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1181/fi-skl-guc/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
>
> I am not sure if I correctly read the docs but [1] says that 9th bit of
> PLANE_CFG (Async Address Update Enable) is "not double buffered and the
> changes will apply immediately" only for ICL, JSL, LKF1.
It got broken in bdw and fixed again in icl.
> So the change is not necessary in case of icl_plane_disable_arm.
>
> [1]: https://gfxspecs.intel.com/Predator/Home/Index/7656
> >
> >> but if async update is enabled
> >> PLANE_SURF register is updated asynchronously. Writing 0 to PLANE_SURF
> >> when plane is still enabled can cause DMAR/PIPE errors.
> >> On the other side PLANE_SURF is used to arm plane registers - we need to
> >> write to it to trigger update on VBLANK, writting current value should
> >> be safe - the buffer address is valid till vblank.
> > I think you're effectively saying that somehow the async
> > flip disable w/a is not kicking in sometimes.
>
> I was not aware of existence of this w/a and I am little lost in
> figuring out how this w/a can prevent zeroing PLANE_SURF too early.
When it works as designed it should:
1. turn off the async flip bit
2. wait for vblank so that gets latched
3. do the sync plane update/disable normally
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: do not reset PLANE_SURF on plane disable on older gens
2022-09-05 17:44 ` Ville Syrjälä
@ 2022-09-06 11:09 ` Andrzej Hajda
2022-09-06 11:22 ` Ville Syrjälä
0 siblings, 1 reply; 16+ messages in thread
From: Andrzej Hajda @ 2022-09-06 11:09 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, Rodrigo Vivi
On 05.09.2022 19:44, Ville Syrjälä wrote:
> On Mon, Sep 05, 2022 at 07:02:40PM +0200, Andrzej Hajda wrote:
>>
>> On 05.09.2022 13:48, Ville Syrjälä wrote:
>>> On Mon, Sep 05, 2022 at 10:05:00AM +0200, Andrzej Hajda wrote:
>>>> In case of ICL and older generations disabling plane and/or disabling
>>>> async update is always performed on vblank,
>>> It should only be broken on bdw-glk (see. need_async_flip_disable_wa).
>> On CFL it is reported every drmtip run:
>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip.html?testfilter=tiled-max-hw
>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html#dmesg-warnings402
>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html#dmesg-warnings402
>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1208/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
>> ...
>> On APL it is less frequent, probably due to other bugs preventing run of
>> this test, last seen at:
>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1190/fi-apl-guc/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
>> Similar for SKL:
>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1181/fi-skl-guc/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
>>
>> I am not sure if I correctly read the docs but [1] says that 9th bit of
>> PLANE_CFG (Async Address Update Enable) is "not double buffered and the
>> changes will apply immediately" only for ICL, JSL, LKF1.
> It got broken in bdw and fixed again in icl.
>
>> So the change is not necessary in case of icl_plane_disable_arm.
>>
>> [1]: https://gfxspecs.intel.com/Predator/Home/Index/7656
>>>> but if async update is enabled
>>>> PLANE_SURF register is updated asynchronously. Writing 0 to PLANE_SURF
>>>> when plane is still enabled can cause DMAR/PIPE errors.
>>>> On the other side PLANE_SURF is used to arm plane registers - we need to
>>>> write to it to trigger update on VBLANK, writting current value should
>>>> be safe - the buffer address is valid till vblank.
>>> I think you're effectively saying that somehow the async
>>> flip disable w/a is not kicking in sometimes.
>> I was not aware of existence of this w/a and I am little lost in
>> figuring out how this w/a can prevent zeroing PLANE_SURF too early.
> When it works as designed it should:
> 1. turn off the async flip bit
> 2. wait for vblank so that gets latched
> 3. do the sync plane update/disable normally
After debugging this terra incognita, I've figured out that plane states
are not populated in intel_crtc_async_flip_disable_wa
so for_each_old_intel_plane_in_state does not iterate over affected
planes and w/a does not work at all.
I have no idea where affected plane states should be added.
Adding them to the beginning of intel_atomic_check helped, but this is
just blind shot:
@@ -6778,10 +6778,14 @@ static int intel_atomic_check(struct drm_device
*dev,
new_crtc_state->uapi.mode_changed = true;
if (new_crtc_state->uapi.scaling_filter !=
old_crtc_state->uapi.scaling_filter)
new_crtc_state->uapi.mode_changed = true;
+
+ ret = intel_atomic_add_affected_planes(state, crtc);
+ if (ret)
+ goto fail;
}
intel_vrr_check_modeset(state);
ret = drm_atomic_helper_check_modeset(dev, &state->base);
Let me know if there is better place/way to handle it.
Regards
Andrzej
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: do not reset PLANE_SURF on plane disable on older gens
2022-09-06 11:09 ` Andrzej Hajda
@ 2022-09-06 11:22 ` Ville Syrjälä
2022-09-06 11:32 ` Ville Syrjälä
2022-09-06 13:57 ` Andrzej Hajda
0 siblings, 2 replies; 16+ messages in thread
From: Ville Syrjälä @ 2022-09-06 11:22 UTC (permalink / raw)
To: Andrzej Hajda; +Cc: intel-gfx, Rodrigo Vivi
On Tue, Sep 06, 2022 at 01:09:16PM +0200, Andrzej Hajda wrote:
>
>
> On 05.09.2022 19:44, Ville Syrjälä wrote:
> > On Mon, Sep 05, 2022 at 07:02:40PM +0200, Andrzej Hajda wrote:
> >>
> >> On 05.09.2022 13:48, Ville Syrjälä wrote:
> >>> On Mon, Sep 05, 2022 at 10:05:00AM +0200, Andrzej Hajda wrote:
> >>>> In case of ICL and older generations disabling plane and/or disabling
> >>>> async update is always performed on vblank,
> >>> It should only be broken on bdw-glk (see. need_async_flip_disable_wa).
> >> On CFL it is reported every drmtip run:
> >> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip.html?testfilter=tiled-max-hw
> >> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html#dmesg-warnings402
> >> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html#dmesg-warnings402
> >> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
> >> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1208/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
> >> ...
> >> On APL it is less frequent, probably due to other bugs preventing run of
> >> this test, last seen at:
> >> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1190/fi-apl-guc/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
> >> Similar for SKL:
> >> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1181/fi-skl-guc/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
> >>
> >> I am not sure if I correctly read the docs but [1] says that 9th bit of
> >> PLANE_CFG (Async Address Update Enable) is "not double buffered and the
> >> changes will apply immediately" only for ICL, JSL, LKF1.
> > It got broken in bdw and fixed again in icl.
> >
> >> So the change is not necessary in case of icl_plane_disable_arm.
> >>
> >> [1]: https://gfxspecs.intel.com/Predator/Home/Index/7656
> >>>> but if async update is enabled
> >>>> PLANE_SURF register is updated asynchronously. Writing 0 to PLANE_SURF
> >>>> when plane is still enabled can cause DMAR/PIPE errors.
> >>>> On the other side PLANE_SURF is used to arm plane registers - we need to
> >>>> write to it to trigger update on VBLANK, writting current value should
> >>>> be safe - the buffer address is valid till vblank.
> >>> I think you're effectively saying that somehow the async
> >>> flip disable w/a is not kicking in sometimes.
> >> I was not aware of existence of this w/a and I am little lost in
> >> figuring out how this w/a can prevent zeroing PLANE_SURF too early.
> > When it works as designed it should:
> > 1. turn off the async flip bit
> > 2. wait for vblank so that gets latched
> > 3. do the sync plane update/disable normally
>
> After debugging this terra incognita, I've figured out that plane states
> are not populated in intel_crtc_async_flip_disable_wa
> so for_each_old_intel_plane_in_state does not iterate over affected
> planes and w/a does not work at all.
> I have no idea where affected plane states should be added.
> Adding them to the beginning of intel_atomic_check helped, but this is
> just blind shot:
>
> @@ -6778,10 +6778,14 @@ static int intel_atomic_check(struct drm_device
> *dev,
> new_crtc_state->uapi.mode_changed = true;
>
> if (new_crtc_state->uapi.scaling_filter !=
> old_crtc_state->uapi.scaling_filter)
> new_crtc_state->uapi.mode_changed = true;
> +
> + ret = intel_atomic_add_affected_planes(state, crtc);
> + if (ret)
> + goto fail;
> }
>
> intel_vrr_check_modeset(state);
>
> ret = drm_atomic_helper_check_modeset(dev, &state->base);
^
This guy should be adding them for any crtc that has been flagged
for modeset ahead of time. For modesets flagged later we have to
add them by hand (eg. in intel_modeset_all_pipes()).
For normal plane updates the relevant planes are already added
when the property values are updated.
>
>
> Let me know if there is better place/way to handle it.
>
> Regards
> Andrzej
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: do not reset PLANE_SURF on plane disable on older gens
2022-09-06 11:22 ` Ville Syrjälä
@ 2022-09-06 11:32 ` Ville Syrjälä
2022-09-06 13:57 ` Andrzej Hajda
1 sibling, 0 replies; 16+ messages in thread
From: Ville Syrjälä @ 2022-09-06 11:32 UTC (permalink / raw)
To: Andrzej Hajda; +Cc: intel-gfx, Rodrigo Vivi
On Tue, Sep 06, 2022 at 02:22:38PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 06, 2022 at 01:09:16PM +0200, Andrzej Hajda wrote:
> >
> >
> > On 05.09.2022 19:44, Ville Syrjälä wrote:
> > > On Mon, Sep 05, 2022 at 07:02:40PM +0200, Andrzej Hajda wrote:
> > >>
> > >> On 05.09.2022 13:48, Ville Syrjälä wrote:
> > >>> On Mon, Sep 05, 2022 at 10:05:00AM +0200, Andrzej Hajda wrote:
> > >>>> In case of ICL and older generations disabling plane and/or disabling
> > >>>> async update is always performed on vblank,
> > >>> It should only be broken on bdw-glk (see. need_async_flip_disable_wa).
> > >> On CFL it is reported every drmtip run:
> > >> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip.html?testfilter=tiled-max-hw
> > >> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html#dmesg-warnings402
> > >> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html#dmesg-warnings402
> > >> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
> > >> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1208/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
> > >> ...
> > >> On APL it is less frequent, probably due to other bugs preventing run of
> > >> this test, last seen at:
> > >> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1190/fi-apl-guc/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
> > >> Similar for SKL:
> > >> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1181/fi-skl-guc/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
> > >>
> > >> I am not sure if I correctly read the docs but [1] says that 9th bit of
> > >> PLANE_CFG (Async Address Update Enable) is "not double buffered and the
> > >> changes will apply immediately" only for ICL, JSL, LKF1.
> > > It got broken in bdw and fixed again in icl.
> > >
> > >> So the change is not necessary in case of icl_plane_disable_arm.
> > >>
> > >> [1]: https://gfxspecs.intel.com/Predator/Home/Index/7656
> > >>>> but if async update is enabled
> > >>>> PLANE_SURF register is updated asynchronously. Writing 0 to PLANE_SURF
> > >>>> when plane is still enabled can cause DMAR/PIPE errors.
> > >>>> On the other side PLANE_SURF is used to arm plane registers - we need to
> > >>>> write to it to trigger update on VBLANK, writting current value should
> > >>>> be safe - the buffer address is valid till vblank.
> > >>> I think you're effectively saying that somehow the async
> > >>> flip disable w/a is not kicking in sometimes.
> > >> I was not aware of existence of this w/a and I am little lost in
> > >> figuring out how this w/a can prevent zeroing PLANE_SURF too early.
> > > When it works as designed it should:
> > > 1. turn off the async flip bit
> > > 2. wait for vblank so that gets latched
> > > 3. do the sync plane update/disable normally
> >
> > After debugging this terra incognita, I've figured out that plane states
> > are not populated in intel_crtc_async_flip_disable_wa
> > so for_each_old_intel_plane_in_state does not iterate over affected
> > planes and w/a does not work at all.
> > I have no idea where affected plane states should be added.
> > Adding them to the beginning of intel_atomic_check helped, but this is
> > just blind shot:
> >
> > @@ -6778,10 +6778,14 @@ static int intel_atomic_check(struct drm_device
> > *dev,
> > new_crtc_state->uapi.mode_changed = true;
> >
> > if (new_crtc_state->uapi.scaling_filter !=
> > old_crtc_state->uapi.scaling_filter)
> > new_crtc_state->uapi.mode_changed = true;
> > +
> > + ret = intel_atomic_add_affected_planes(state, crtc);
> > + if (ret)
> > + goto fail;
> > }
> >
> > intel_vrr_check_modeset(state);
> >
> > ret = drm_atomic_helper_check_modeset(dev, &state->base);
> ^
> This guy should be adding them for any crtc that has been flagged
> for modeset ahead of time. For modesets flagged later we have to
> add them by hand (eg. in intel_modeset_all_pipes()).
>
> For normal plane updates the relevant planes are already added
> when the property values are updated.
Hmm. Not having in the state doesn't really make sense
because then we wouldn't have called the disable hook for the
plane anyway. I guess one theory would be that update_mask is
somehow stale. I did see one weird assert_plane() failure on
one of my machines recently so that could also be pointing
to the same thing. But I've not had time to investigate that
one yet.
I did type up a preliminary patch to see if we might catch
something funny, but haven't tried feeding it to ci yet:
diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index dd876dbbaa39..f8ca3854357c 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -752,6 +752,9 @@ void intel_plane_disable_arm(struct intel_plane *plane,
void intel_crtc_planes_update_noarm(struct intel_atomic_state *state,
struct intel_crtc *crtc)
{
+ struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+ const struct intel_crtc_state *old_crtc_state =
+ intel_atomic_get_old_crtc_state(state, crtc);
struct intel_crtc_state *new_crtc_state =
intel_atomic_get_new_crtc_state(state, crtc);
u32 update_mask = new_crtc_state->update_planes;
@@ -762,6 +765,14 @@ void intel_crtc_planes_update_noarm(struct intel_atomic_state *state,
if (new_crtc_state->do_async_flip)
return;
+ drm_WARN(&dev_priv->drm, intel_crtc_needs_modeset(new_crtc_state) &&
+ ~update_mask & new_crtc_state->active_planes,
+ "[CRTC:%d:%s] active planes 0x%x -> 0x%x, update planes 0x%x\n",
+ crtc->base.base.id, crtc->base.name,
+ old_crtc_state->active_planes,
+ new_crtc_state->active_planes,
+ new_crtc_state->update_planes);
+
/*
* Since we only write non-arming registers here,
* the order does not matter even for skl+.
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a2ba7359ce8c..f9e2814a3ba1 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1426,6 +1426,8 @@ static void intel_crtc_disable_planes(struct intel_atomic_state *state,
struct intel_crtc *crtc)
{
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ const struct intel_crtc_state *old_crtc_state =
+ intel_atomic_get_old_crtc_state(state, crtc);
const struct intel_crtc_state *new_crtc_state =
intel_atomic_get_new_crtc_state(state, crtc);
unsigned int update_mask = new_crtc_state->update_planes;
@@ -1434,6 +1436,14 @@ static void intel_crtc_disable_planes(struct intel_atomic_state *state,
unsigned fb_bits = 0;
int i;
+ drm_WARN(&dev_priv->drm,
+ ~update_mask & old_crtc_state->active_planes,
+ "[CRTC:%d:%s] active planes 0x%x -> 0x%x, update planes 0x%x\n",
+ crtc->base.base.id, crtc->base.name,
+ old_crtc_state->active_planes,
+ new_crtc_state->active_planes,
+ new_crtc_state->update_planes);
+
intel_crtc_dpms_overlay_disable(crtc);
for_each_old_intel_plane_in_state(state, plane, old_plane_state, i) {
--
Ville Syrjälä
Intel
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: do not reset PLANE_SURF on plane disable on older gens
2022-09-06 11:22 ` Ville Syrjälä
2022-09-06 11:32 ` Ville Syrjälä
@ 2022-09-06 13:57 ` Andrzej Hajda
2022-09-06 14:14 ` Ville Syrjälä
1 sibling, 1 reply; 16+ messages in thread
From: Andrzej Hajda @ 2022-09-06 13:57 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, Rodrigo Vivi
On 06.09.2022 13:22, Ville Syrjälä wrote:
> On Tue, Sep 06, 2022 at 01:09:16PM +0200, Andrzej Hajda wrote:
>>
>> On 05.09.2022 19:44, Ville Syrjälä wrote:
>>> On Mon, Sep 05, 2022 at 07:02:40PM +0200, Andrzej Hajda wrote:
>>>> On 05.09.2022 13:48, Ville Syrjälä wrote:
>>>>> On Mon, Sep 05, 2022 at 10:05:00AM +0200, Andrzej Hajda wrote:
>>>>>> In case of ICL and older generations disabling plane and/or disabling
>>>>>> async update is always performed on vblank,
>>>>> It should only be broken on bdw-glk (see. need_async_flip_disable_wa).
>>>> On CFL it is reported every drmtip run:
>>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip.html?testfilter=tiled-max-hw
>>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html#dmesg-warnings402
>>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html#dmesg-warnings402
>>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
>>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1208/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
>>>> ...
>>>> On APL it is less frequent, probably due to other bugs preventing run of
>>>> this test, last seen at:
>>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1190/fi-apl-guc/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
>>>> Similar for SKL:
>>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1181/fi-skl-guc/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
>>>>
>>>> I am not sure if I correctly read the docs but [1] says that 9th bit of
>>>> PLANE_CFG (Async Address Update Enable) is "not double buffered and the
>>>> changes will apply immediately" only for ICL, JSL, LKF1.
>>> It got broken in bdw and fixed again in icl.
>>>
>>>> So the change is not necessary in case of icl_plane_disable_arm.
>>>>
>>>> [1]: https://gfxspecs.intel.com/Predator/Home/Index/7656
>>>>>> but if async update is enabled
>>>>>> PLANE_SURF register is updated asynchronously. Writing 0 to PLANE_SURF
>>>>>> when plane is still enabled can cause DMAR/PIPE errors.
>>>>>> On the other side PLANE_SURF is used to arm plane registers - we need to
>>>>>> write to it to trigger update on VBLANK, writting current value should
>>>>>> be safe - the buffer address is valid till vblank.
>>>>> I think you're effectively saying that somehow the async
>>>>> flip disable w/a is not kicking in sometimes.
>>>> I was not aware of existence of this w/a and I am little lost in
>>>> figuring out how this w/a can prevent zeroing PLANE_SURF too early.
>>> When it works as designed it should:
>>> 1. turn off the async flip bit
>>> 2. wait for vblank so that gets latched
>>> 3. do the sync plane update/disable normally
>> After debugging this terra incognita, I've figured out that plane states
>> are not populated in intel_crtc_async_flip_disable_wa
>> so for_each_old_intel_plane_in_state does not iterate over affected
>> planes and w/a does not work at all.
>> I have no idea where affected plane states should be added.
>> Adding them to the beginning of intel_atomic_check helped, but this is
>> just blind shot:
>>
>> @@ -6778,10 +6778,14 @@ static int intel_atomic_check(struct drm_device
>> *dev,
>> new_crtc_state->uapi.mode_changed = true;
>>
>> if (new_crtc_state->uapi.scaling_filter !=
>> old_crtc_state->uapi.scaling_filter)
>> new_crtc_state->uapi.mode_changed = true;
>> +
>> + ret = intel_atomic_add_affected_planes(state, crtc);
>> + if (ret)
>> + goto fail;
>> }
>>
>> intel_vrr_check_modeset(state);
>>
>> ret = drm_atomic_helper_check_modeset(dev, &state->base);
> ^
> This guy should be adding them for any crtc that has been flagged
> for modeset ahead of time. For modesets flagged later we have to
> add them by hand (eg. in intel_modeset_all_pipes()).
This is no-modeset scenario, drm_atomic_helper_check_modeset does not
add planes in this case.
Btw, tested your warns from another mail, none appeared.
Regards
Andrzej
>
> For normal plane updates the relevant planes are already added
> when the property values are updated.
>
>>
>> Let me know if there is better place/way to handle it.
>>
>> Regards
>> Andrzej
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: do not reset PLANE_SURF on plane disable on older gens
2022-09-06 13:57 ` Andrzej Hajda
@ 2022-09-06 14:14 ` Ville Syrjälä
2022-09-06 14:36 ` Ville Syrjälä
0 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2022-09-06 14:14 UTC (permalink / raw)
To: Andrzej Hajda; +Cc: intel-gfx, Rodrigo Vivi
On Tue, Sep 06, 2022 at 03:57:37PM +0200, Andrzej Hajda wrote:
>
>
> On 06.09.2022 13:22, Ville Syrjälä wrote:
> > On Tue, Sep 06, 2022 at 01:09:16PM +0200, Andrzej Hajda wrote:
> >>
> >> On 05.09.2022 19:44, Ville Syrjälä wrote:
> >>> On Mon, Sep 05, 2022 at 07:02:40PM +0200, Andrzej Hajda wrote:
> >>>> On 05.09.2022 13:48, Ville Syrjälä wrote:
> >>>>> On Mon, Sep 05, 2022 at 10:05:00AM +0200, Andrzej Hajda wrote:
> >>>>>> In case of ICL and older generations disabling plane and/or disabling
> >>>>>> async update is always performed on vblank,
> >>>>> It should only be broken on bdw-glk (see. need_async_flip_disable_wa).
> >>>> On CFL it is reported every drmtip run:
> >>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip.html?testfilter=tiled-max-hw
> >>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html#dmesg-warnings402
> >>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html#dmesg-warnings402
> >>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
> >>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1208/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
> >>>> ...
> >>>> On APL it is less frequent, probably due to other bugs preventing run of
> >>>> this test, last seen at:
> >>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1190/fi-apl-guc/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
> >>>> Similar for SKL:
> >>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1181/fi-skl-guc/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
> >>>>
> >>>> I am not sure if I correctly read the docs but [1] says that 9th bit of
> >>>> PLANE_CFG (Async Address Update Enable) is "not double buffered and the
> >>>> changes will apply immediately" only for ICL, JSL, LKF1.
> >>> It got broken in bdw and fixed again in icl.
> >>>
> >>>> So the change is not necessary in case of icl_plane_disable_arm.
> >>>>
> >>>> [1]: https://gfxspecs.intel.com/Predator/Home/Index/7656
> >>>>>> but if async update is enabled
> >>>>>> PLANE_SURF register is updated asynchronously. Writing 0 to PLANE_SURF
> >>>>>> when plane is still enabled can cause DMAR/PIPE errors.
> >>>>>> On the other side PLANE_SURF is used to arm plane registers - we need to
> >>>>>> write to it to trigger update on VBLANK, writting current value should
> >>>>>> be safe - the buffer address is valid till vblank.
> >>>>> I think you're effectively saying that somehow the async
> >>>>> flip disable w/a is not kicking in sometimes.
> >>>> I was not aware of existence of this w/a and I am little lost in
> >>>> figuring out how this w/a can prevent zeroing PLANE_SURF too early.
> >>> When it works as designed it should:
> >>> 1. turn off the async flip bit
> >>> 2. wait for vblank so that gets latched
> >>> 3. do the sync plane update/disable normally
> >> After debugging this terra incognita, I've figured out that plane states
> >> are not populated in intel_crtc_async_flip_disable_wa
> >> so for_each_old_intel_plane_in_state does not iterate over affected
> >> planes and w/a does not work at all.
> >> I have no idea where affected plane states should be added.
> >> Adding them to the beginning of intel_atomic_check helped, but this is
> >> just blind shot:
> >>
> >> @@ -6778,10 +6778,14 @@ static int intel_atomic_check(struct drm_device
> >> *dev,
> >> new_crtc_state->uapi.mode_changed = true;
> >>
> >> if (new_crtc_state->uapi.scaling_filter !=
> >> old_crtc_state->uapi.scaling_filter)
> >> new_crtc_state->uapi.mode_changed = true;
> >> +
> >> + ret = intel_atomic_add_affected_planes(state, crtc);
> >> + if (ret)
> >> + goto fail;
> >> }
> >>
> >> intel_vrr_check_modeset(state);
> >>
> >> ret = drm_atomic_helper_check_modeset(dev, &state->base);
> > ^
> > This guy should be adding them for any crtc that has been flagged
> > for modeset ahead of time. For modesets flagged later we have to
> > add them by hand (eg. in intel_modeset_all_pipes()).
>
> This is no-modeset scenario, drm_atomic_helper_check_modeset does not
> add planes in this case.
Then he mystery is how intel_crtc_async_flip_disable_wa() manages
to not disable async flip for some planes...
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: do not reset PLANE_SURF on plane disable on older gens
2022-09-06 14:14 ` Ville Syrjälä
@ 2022-09-06 14:36 ` Ville Syrjälä
2022-09-06 14:43 ` Ville Syrjälä
0 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2022-09-06 14:36 UTC (permalink / raw)
To: Andrzej Hajda; +Cc: intel-gfx, Rodrigo Vivi
On Tue, Sep 06, 2022 at 05:14:56PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 06, 2022 at 03:57:37PM +0200, Andrzej Hajda wrote:
> >
> >
> > On 06.09.2022 13:22, Ville Syrjälä wrote:
> > > On Tue, Sep 06, 2022 at 01:09:16PM +0200, Andrzej Hajda wrote:
> > >>
> > >> On 05.09.2022 19:44, Ville Syrjälä wrote:
> > >>> On Mon, Sep 05, 2022 at 07:02:40PM +0200, Andrzej Hajda wrote:
> > >>>> On 05.09.2022 13:48, Ville Syrjälä wrote:
> > >>>>> On Mon, Sep 05, 2022 at 10:05:00AM +0200, Andrzej Hajda wrote:
> > >>>>>> In case of ICL and older generations disabling plane and/or disabling
> > >>>>>> async update is always performed on vblank,
> > >>>>> It should only be broken on bdw-glk (see. need_async_flip_disable_wa).
> > >>>> On CFL it is reported every drmtip run:
> > >>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip.html?testfilter=tiled-max-hw
> > >>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html#dmesg-warnings402
> > >>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html#dmesg-warnings402
> > >>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
> > >>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1208/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
> > >>>> ...
> > >>>> On APL it is less frequent, probably due to other bugs preventing run of
> > >>>> this test, last seen at:
> > >>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1190/fi-apl-guc/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
> > >>>> Similar for SKL:
> > >>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1181/fi-skl-guc/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
> > >>>>
> > >>>> I am not sure if I correctly read the docs but [1] says that 9th bit of
> > >>>> PLANE_CFG (Async Address Update Enable) is "not double buffered and the
> > >>>> changes will apply immediately" only for ICL, JSL, LKF1.
> > >>> It got broken in bdw and fixed again in icl.
> > >>>
> > >>>> So the change is not necessary in case of icl_plane_disable_arm.
> > >>>>
> > >>>> [1]: https://gfxspecs.intel.com/Predator/Home/Index/7656
> > >>>>>> but if async update is enabled
> > >>>>>> PLANE_SURF register is updated asynchronously. Writing 0 to PLANE_SURF
> > >>>>>> when plane is still enabled can cause DMAR/PIPE errors.
> > >>>>>> On the other side PLANE_SURF is used to arm plane registers - we need to
> > >>>>>> write to it to trigger update on VBLANK, writting current value should
> > >>>>>> be safe - the buffer address is valid till vblank.
> > >>>>> I think you're effectively saying that somehow the async
> > >>>>> flip disable w/a is not kicking in sometimes.
> > >>>> I was not aware of existence of this w/a and I am little lost in
> > >>>> figuring out how this w/a can prevent zeroing PLANE_SURF too early.
> > >>> When it works as designed it should:
> > >>> 1. turn off the async flip bit
> > >>> 2. wait for vblank so that gets latched
> > >>> 3. do the sync plane update/disable normally
> > >> After debugging this terra incognita, I've figured out that plane states
> > >> are not populated in intel_crtc_async_flip_disable_wa
> > >> so for_each_old_intel_plane_in_state does not iterate over affected
> > >> planes and w/a does not work at all.
> > >> I have no idea where affected plane states should be added.
> > >> Adding them to the beginning of intel_atomic_check helped, but this is
> > >> just blind shot:
> > >>
> > >> @@ -6778,10 +6778,14 @@ static int intel_atomic_check(struct drm_device
> > >> *dev,
> > >> new_crtc_state->uapi.mode_changed = true;
> > >>
> > >> if (new_crtc_state->uapi.scaling_filter !=
> > >> old_crtc_state->uapi.scaling_filter)
> > >> new_crtc_state->uapi.mode_changed = true;
> > >> +
> > >> + ret = intel_atomic_add_affected_planes(state, crtc);
> > >> + if (ret)
> > >> + goto fail;
> > >> }
> > >>
> > >> intel_vrr_check_modeset(state);
> > >>
> > >> ret = drm_atomic_helper_check_modeset(dev, &state->base);
> > > ^
> > > This guy should be adding them for any crtc that has been flagged
> > > for modeset ahead of time. For modesets flagged later we have to
> > > add them by hand (eg. in intel_modeset_all_pipes()).
> >
> > This is no-modeset scenario, drm_atomic_helper_check_modeset does not
> > add planes in this case.
>
> Then he mystery is how intel_crtc_async_flip_disable_wa() manages
> to not disable async flip for some planes...
After a few minutes of pondering I have a theory:
1. async flip on plane 1
crtc_state.*async_flip: false -> true
2. sync flip on plane 2, plane 1 not include in state
crtc_state.*async_flip: true -> false, but plane 1 still remains in
async flip mode
3. sync update/disable plane 1
crtc_state.*async_flip = true -> true, so the async flip disable w/a
is not triggeed
Should be easy to type up a dedicated test case for that.
I think there are two options of handling this:
- Switch all planes out of async flip mode for any sync update.
Not great because then you can't mix async flips with any other
sync updates on the same crtc
- Start tracking which planes are in async flip mode vs. not
Should allow more freedom in mixing async flips with other updates
on the crtc
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: do not reset PLANE_SURF on plane disable on older gens
2022-09-06 14:36 ` Ville Syrjälä
@ 2022-09-06 14:43 ` Ville Syrjälä
2022-09-06 16:14 ` Andrzej Hajda
0 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2022-09-06 14:43 UTC (permalink / raw)
To: Andrzej Hajda; +Cc: intel-gfx, Rodrigo Vivi
On Tue, Sep 06, 2022 at 05:36:05PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 06, 2022 at 05:14:56PM +0300, Ville Syrjälä wrote:
> > On Tue, Sep 06, 2022 at 03:57:37PM +0200, Andrzej Hajda wrote:
> > >
> > >
> > > On 06.09.2022 13:22, Ville Syrjälä wrote:
> > > > On Tue, Sep 06, 2022 at 01:09:16PM +0200, Andrzej Hajda wrote:
> > > >>
> > > >> On 05.09.2022 19:44, Ville Syrjälä wrote:
> > > >>> On Mon, Sep 05, 2022 at 07:02:40PM +0200, Andrzej Hajda wrote:
> > > >>>> On 05.09.2022 13:48, Ville Syrjälä wrote:
> > > >>>>> On Mon, Sep 05, 2022 at 10:05:00AM +0200, Andrzej Hajda wrote:
> > > >>>>>> In case of ICL and older generations disabling plane and/or disabling
> > > >>>>>> async update is always performed on vblank,
> > > >>>>> It should only be broken on bdw-glk (see. need_async_flip_disable_wa).
> > > >>>> On CFL it is reported every drmtip run:
> > > >>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip.html?testfilter=tiled-max-hw
> > > >>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html#dmesg-warnings402
> > > >>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html#dmesg-warnings402
> > > >>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
> > > >>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1208/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
> > > >>>> ...
> > > >>>> On APL it is less frequent, probably due to other bugs preventing run of
> > > >>>> this test, last seen at:
> > > >>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1190/fi-apl-guc/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
> > > >>>> Similar for SKL:
> > > >>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1181/fi-skl-guc/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
> > > >>>>
> > > >>>> I am not sure if I correctly read the docs but [1] says that 9th bit of
> > > >>>> PLANE_CFG (Async Address Update Enable) is "not double buffered and the
> > > >>>> changes will apply immediately" only for ICL, JSL, LKF1.
> > > >>> It got broken in bdw and fixed again in icl.
> > > >>>
> > > >>>> So the change is not necessary in case of icl_plane_disable_arm.
> > > >>>>
> > > >>>> [1]: https://gfxspecs.intel.com/Predator/Home/Index/7656
> > > >>>>>> but if async update is enabled
> > > >>>>>> PLANE_SURF register is updated asynchronously. Writing 0 to PLANE_SURF
> > > >>>>>> when plane is still enabled can cause DMAR/PIPE errors.
> > > >>>>>> On the other side PLANE_SURF is used to arm plane registers - we need to
> > > >>>>>> write to it to trigger update on VBLANK, writting current value should
> > > >>>>>> be safe - the buffer address is valid till vblank.
> > > >>>>> I think you're effectively saying that somehow the async
> > > >>>>> flip disable w/a is not kicking in sometimes.
> > > >>>> I was not aware of existence of this w/a and I am little lost in
> > > >>>> figuring out how this w/a can prevent zeroing PLANE_SURF too early.
> > > >>> When it works as designed it should:
> > > >>> 1. turn off the async flip bit
> > > >>> 2. wait for vblank so that gets latched
> > > >>> 3. do the sync plane update/disable normally
> > > >> After debugging this terra incognita, I've figured out that plane states
> > > >> are not populated in intel_crtc_async_flip_disable_wa
> > > >> so for_each_old_intel_plane_in_state does not iterate over affected
> > > >> planes and w/a does not work at all.
> > > >> I have no idea where affected plane states should be added.
> > > >> Adding them to the beginning of intel_atomic_check helped, but this is
> > > >> just blind shot:
> > > >>
> > > >> @@ -6778,10 +6778,14 @@ static int intel_atomic_check(struct drm_device
> > > >> *dev,
> > > >> new_crtc_state->uapi.mode_changed = true;
> > > >>
> > > >> if (new_crtc_state->uapi.scaling_filter !=
> > > >> old_crtc_state->uapi.scaling_filter)
> > > >> new_crtc_state->uapi.mode_changed = true;
> > > >> +
> > > >> + ret = intel_atomic_add_affected_planes(state, crtc);
> > > >> + if (ret)
> > > >> + goto fail;
> > > >> }
> > > >>
> > > >> intel_vrr_check_modeset(state);
> > > >>
> > > >> ret = drm_atomic_helper_check_modeset(dev, &state->base);
> > > > ^
> > > > This guy should be adding them for any crtc that has been flagged
> > > > for modeset ahead of time. For modesets flagged later we have to
> > > > add them by hand (eg. in intel_modeset_all_pipes()).
> > >
> > > This is no-modeset scenario, drm_atomic_helper_check_modeset does not
> > > add planes in this case.
> >
> > Then he mystery is how intel_crtc_async_flip_disable_wa() manages
> > to not disable async flip for some planes...
>
> After a few minutes of pondering I have a theory:
> 1. async flip on plane 1
> crtc_state.*async_flip: false -> true
> 2. sync flip on plane 2, plane 1 not include in state
> crtc_state.*async_flip: true -> false, but plane 1 still remains in
> async flip mode
> 3. sync update/disable plane 1
> crtc_state.*async_flip = true -> true, so the async flip disable w/a
^^^^^^^^^^^^
Meant to write false->false there.
> is not triggeed
>
> Should be easy to type up a dedicated test case for that.
>
> I think there are two options of handling this:
> - Switch all planes out of async flip mode for any sync update.
> Not great because then you can't mix async flips with any other
> sync updates on the same crtc
> - Start tracking which planes are in async flip mode vs. not
> Should allow more freedom in mixing async flips with other updates
> on the crtc
>
> --
> Ville Syrjälä
> Intel
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: do not reset PLANE_SURF on plane disable on older gens
2022-09-06 14:43 ` Ville Syrjälä
@ 2022-09-06 16:14 ` Andrzej Hajda
2022-09-06 16:21 ` Ville Syrjälä
0 siblings, 1 reply; 16+ messages in thread
From: Andrzej Hajda @ 2022-09-06 16:14 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, Rodrigo Vivi
On 06.09.2022 16:43, Ville Syrjälä wrote:
> On Tue, Sep 06, 2022 at 05:36:05PM +0300, Ville Syrjälä wrote:
>> On Tue, Sep 06, 2022 at 05:14:56PM +0300, Ville Syrjälä wrote:
>>> On Tue, Sep 06, 2022 at 03:57:37PM +0200, Andrzej Hajda wrote:
>>>>
>>>> On 06.09.2022 13:22, Ville Syrjälä wrote:
>>>>> On Tue, Sep 06, 2022 at 01:09:16PM +0200, Andrzej Hajda wrote:
>>>>>> On 05.09.2022 19:44, Ville Syrjälä wrote:
>>>>>>> On Mon, Sep 05, 2022 at 07:02:40PM +0200, Andrzej Hajda wrote:
>>>>>>>> On 05.09.2022 13:48, Ville Syrjälä wrote:
>>>>>>>>> On Mon, Sep 05, 2022 at 10:05:00AM +0200, Andrzej Hajda wrote:
>>>>>>>>>> In case of ICL and older generations disabling plane and/or disabling
>>>>>>>>>> async update is always performed on vblank,
>>>>>>>>> It should only be broken on bdw-glk (see. need_async_flip_disable_wa).
>>>>>>>> On CFL it is reported every drmtip run:
>>>>>>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip.html?testfilter=tiled-max-hw
>>>>>>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html#dmesg-warnings402
>>>>>>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html#dmesg-warnings402
>>>>>>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
>>>>>>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1208/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
>>>>>>>> ...
>>>>>>>> On APL it is less frequent, probably due to other bugs preventing run of
>>>>>>>> this test, last seen at:
>>>>>>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1190/fi-apl-guc/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
>>>>>>>> Similar for SKL:
>>>>>>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1181/fi-skl-guc/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
>>>>>>>>
>>>>>>>> I am not sure if I correctly read the docs but [1] says that 9th bit of
>>>>>>>> PLANE_CFG (Async Address Update Enable) is "not double buffered and the
>>>>>>>> changes will apply immediately" only for ICL, JSL, LKF1.
>>>>>>> It got broken in bdw and fixed again in icl.
>>>>>>>
>>>>>>>> So the change is not necessary in case of icl_plane_disable_arm.
>>>>>>>>
>>>>>>>> [1]: https://gfxspecs.intel.com/Predator/Home/Index/7656
>>>>>>>>>> but if async update is enabled
>>>>>>>>>> PLANE_SURF register is updated asynchronously. Writing 0 to PLANE_SURF
>>>>>>>>>> when plane is still enabled can cause DMAR/PIPE errors.
>>>>>>>>>> On the other side PLANE_SURF is used to arm plane registers - we need to
>>>>>>>>>> write to it to trigger update on VBLANK, writting current value should
>>>>>>>>>> be safe - the buffer address is valid till vblank.
>>>>>>>>> I think you're effectively saying that somehow the async
>>>>>>>>> flip disable w/a is not kicking in sometimes.
>>>>>>>> I was not aware of existence of this w/a and I am little lost in
>>>>>>>> figuring out how this w/a can prevent zeroing PLANE_SURF too early.
>>>>>>> When it works as designed it should:
>>>>>>> 1. turn off the async flip bit
>>>>>>> 2. wait for vblank so that gets latched
>>>>>>> 3. do the sync plane update/disable normally
>>>>>> After debugging this terra incognita, I've figured out that plane states
>>>>>> are not populated in intel_crtc_async_flip_disable_wa
>>>>>> so for_each_old_intel_plane_in_state does not iterate over affected
>>>>>> planes and w/a does not work at all.
>>>>>> I have no idea where affected plane states should be added.
>>>>>> Adding them to the beginning of intel_atomic_check helped, but this is
>>>>>> just blind shot:
>>>>>>
>>>>>> @@ -6778,10 +6778,14 @@ static int intel_atomic_check(struct drm_device
>>>>>> *dev,
>>>>>> new_crtc_state->uapi.mode_changed = true;
>>>>>>
>>>>>> if (new_crtc_state->uapi.scaling_filter !=
>>>>>> old_crtc_state->uapi.scaling_filter)
>>>>>> new_crtc_state->uapi.mode_changed = true;
>>>>>> +
>>>>>> + ret = intel_atomic_add_affected_planes(state, crtc);
>>>>>> + if (ret)
>>>>>> + goto fail;
>>>>>> }
>>>>>>
>>>>>> intel_vrr_check_modeset(state);
>>>>>>
>>>>>> ret = drm_atomic_helper_check_modeset(dev, &state->base);
>>>>> ^
>>>>> This guy should be adding them for any crtc that has been flagged
>>>>> for modeset ahead of time. For modesets flagged later we have to
>>>>> add them by hand (eg. in intel_modeset_all_pipes()).
>>>> This is no-modeset scenario, drm_atomic_helper_check_modeset does not
>>>> add planes in this case.
>>> Then he mystery is how intel_crtc_async_flip_disable_wa() manages
>>> to not disable async flip for some planes...
>> After a few minutes of pondering I have a theory:
>> 1. async flip on plane 1
>> crtc_state.*async_flip: false -> true
>> 2. sync flip on plane 2, plane 1 not include in state
>> crtc_state.*async_flip: true -> false, but plane 1 still remains in
>> async flip mode
>> 3. sync update/disable plane 1
>> crtc_state.*async_flip = true -> true, so the async flip disable w/a
> ^^^^^^^^^^^^
> Meant to write false->false there.
There is only one plane in game.
Apparently there is an issue with intel_crtc_crc_setup_workarounds.
It calls intel_atomic_get_crtc_state for fresh state, causing state
duplication, but async_flip flag is set always to false in new state.
In cases full modeset is not performed hw and sw state of async_flip
will differ after commit of this state.
Ugly/racy workaround for this below:
---
@@ -316,10 +316,13 @@ intel_crtc_crc_setup_workarounds(struct intel_crtc
*crtc, bool enable)
if (IS_HASWELL(dev_priv) &&
pipe_config->hw.active && crtc->pipe == PIPE_A &&
pipe_config->cpu_transcoder == TRANSCODER_EDP)
pipe_config->uapi.mode_changed = true;
+ if (!pipe_config->uapi.mode_changed)
+ pipe_config->uapi.async_flip = crtc->base.state->async_flip;
+
ret = drm_atomic_commit(state);
put_state:
if (ret == -EDEADLK) {
drm_atomic_state_clear(state);
---
Regards
Andrzej
>
>> is not triggeed
>>
>> Should be easy to type up a dedicated test case for that.
>>
>> I think there are two options of handling this:
>> - Switch all planes out of async flip mode for any sync update.
>> Not great because then you can't mix async flips with any other
>> sync updates on the same crtc
>> - Start tracking which planes are in async flip mode vs. not
>> Should allow more freedom in mixing async flips with other updates
>> on the crtc
>>
>> --
>> Ville Syrjälä
>> Intel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: do not reset PLANE_SURF on plane disable on older gens
2022-09-06 16:14 ` Andrzej Hajda
@ 2022-09-06 16:21 ` Ville Syrjälä
0 siblings, 0 replies; 16+ messages in thread
From: Ville Syrjälä @ 2022-09-06 16:21 UTC (permalink / raw)
To: Andrzej Hajda; +Cc: intel-gfx, Rodrigo Vivi
On Tue, Sep 06, 2022 at 06:14:23PM +0200, Andrzej Hajda wrote:
>
>
> On 06.09.2022 16:43, Ville Syrjälä wrote:
> > On Tue, Sep 06, 2022 at 05:36:05PM +0300, Ville Syrjälä wrote:
> >> On Tue, Sep 06, 2022 at 05:14:56PM +0300, Ville Syrjälä wrote:
> >>> On Tue, Sep 06, 2022 at 03:57:37PM +0200, Andrzej Hajda wrote:
> >>>>
> >>>> On 06.09.2022 13:22, Ville Syrjälä wrote:
> >>>>> On Tue, Sep 06, 2022 at 01:09:16PM +0200, Andrzej Hajda wrote:
> >>>>>> On 05.09.2022 19:44, Ville Syrjälä wrote:
> >>>>>>> On Mon, Sep 05, 2022 at 07:02:40PM +0200, Andrzej Hajda wrote:
> >>>>>>>> On 05.09.2022 13:48, Ville Syrjälä wrote:
> >>>>>>>>> On Mon, Sep 05, 2022 at 10:05:00AM +0200, Andrzej Hajda wrote:
> >>>>>>>>>> In case of ICL and older generations disabling plane and/or disabling
> >>>>>>>>>> async update is always performed on vblank,
> >>>>>>>>> It should only be broken on bdw-glk (see. need_async_flip_disable_wa).
> >>>>>>>> On CFL it is reported every drmtip run:
> >>>>>>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip.html?testfilter=tiled-max-hw
> >>>>>>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html#dmesg-warnings402
> >>>>>>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html#dmesg-warnings402
> >>>>>>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1209/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
> >>>>>>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1208/fi-cfl-8109u/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
> >>>>>>>> ...
> >>>>>>>> On APL it is less frequent, probably due to other bugs preventing run of
> >>>>>>>> this test, last seen at:
> >>>>>>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1190/fi-apl-guc/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
> >>>>>>>> Similar for SKL:
> >>>>>>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1181/fi-skl-guc/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
> >>>>>>>>
> >>>>>>>> I am not sure if I correctly read the docs but [1] says that 9th bit of
> >>>>>>>> PLANE_CFG (Async Address Update Enable) is "not double buffered and the
> >>>>>>>> changes will apply immediately" only for ICL, JSL, LKF1.
> >>>>>>> It got broken in bdw and fixed again in icl.
> >>>>>>>
> >>>>>>>> So the change is not necessary in case of icl_plane_disable_arm.
> >>>>>>>>
> >>>>>>>> [1]: https://gfxspecs.intel.com/Predator/Home/Index/7656
> >>>>>>>>>> but if async update is enabled
> >>>>>>>>>> PLANE_SURF register is updated asynchronously. Writing 0 to PLANE_SURF
> >>>>>>>>>> when plane is still enabled can cause DMAR/PIPE errors.
> >>>>>>>>>> On the other side PLANE_SURF is used to arm plane registers - we need to
> >>>>>>>>>> write to it to trigger update on VBLANK, writting current value should
> >>>>>>>>>> be safe - the buffer address is valid till vblank.
> >>>>>>>>> I think you're effectively saying that somehow the async
> >>>>>>>>> flip disable w/a is not kicking in sometimes.
> >>>>>>>> I was not aware of existence of this w/a and I am little lost in
> >>>>>>>> figuring out how this w/a can prevent zeroing PLANE_SURF too early.
> >>>>>>> When it works as designed it should:
> >>>>>>> 1. turn off the async flip bit
> >>>>>>> 2. wait for vblank so that gets latched
> >>>>>>> 3. do the sync plane update/disable normally
> >>>>>> After debugging this terra incognita, I've figured out that plane states
> >>>>>> are not populated in intel_crtc_async_flip_disable_wa
> >>>>>> so for_each_old_intel_plane_in_state does not iterate over affected
> >>>>>> planes and w/a does not work at all.
> >>>>>> I have no idea where affected plane states should be added.
> >>>>>> Adding them to the beginning of intel_atomic_check helped, but this is
> >>>>>> just blind shot:
> >>>>>>
> >>>>>> @@ -6778,10 +6778,14 @@ static int intel_atomic_check(struct drm_device
> >>>>>> *dev,
> >>>>>> new_crtc_state->uapi.mode_changed = true;
> >>>>>>
> >>>>>> if (new_crtc_state->uapi.scaling_filter !=
> >>>>>> old_crtc_state->uapi.scaling_filter)
> >>>>>> new_crtc_state->uapi.mode_changed = true;
> >>>>>> +
> >>>>>> + ret = intel_atomic_add_affected_planes(state, crtc);
> >>>>>> + if (ret)
> >>>>>> + goto fail;
> >>>>>> }
> >>>>>>
> >>>>>> intel_vrr_check_modeset(state);
> >>>>>>
> >>>>>> ret = drm_atomic_helper_check_modeset(dev, &state->base);
> >>>>> ^
> >>>>> This guy should be adding them for any crtc that has been flagged
> >>>>> for modeset ahead of time. For modesets flagged later we have to
> >>>>> add them by hand (eg. in intel_modeset_all_pipes()).
> >>>> This is no-modeset scenario, drm_atomic_helper_check_modeset does not
> >>>> add planes in this case.
> >>> Then he mystery is how intel_crtc_async_flip_disable_wa() manages
> >>> to not disable async flip for some planes...
> >> After a few minutes of pondering I have a theory:
> >> 1. async flip on plane 1
> >> crtc_state.*async_flip: false -> true
> >> 2. sync flip on plane 2, plane 1 not include in state
> >> crtc_state.*async_flip: true -> false, but plane 1 still remains in
> >> async flip mode
> >> 3. sync update/disable plane 1
> >> crtc_state.*async_flip = true -> true, so the async flip disable w/a
> > ^^^^^^^^^^^^
> > Meant to write false->false there.
>
> There is only one plane in game.
> Apparently there is an issue with intel_crtc_crc_setup_workarounds.
> It calls intel_atomic_get_crtc_state for fresh state, causing state
> duplication, but async_flip flag is set always to false in new state.
Ah. We'll that's basically the same thing as what I listed, except
in step 2 we just do a commit w/o any planes. It still clears out the
async_flip flags and then step 3 gets confused.
> In cases full modeset is not performed hw and sw state of async_flip
> will differ after commit of this state.
> Ugly/racy workaround for this below:
> ---
> @@ -316,10 +316,13 @@ intel_crtc_crc_setup_workarounds(struct intel_crtc
> *crtc, bool enable)
> if (IS_HASWELL(dev_priv) &&
> pipe_config->hw.active && crtc->pipe == PIPE_A &&
> pipe_config->cpu_transcoder == TRANSCODER_EDP)
> pipe_config->uapi.mode_changed = true;
>
> + if (!pipe_config->uapi.mode_changed)
> + pipe_config->uapi.async_flip = crtc->base.state->async_flip;
> +
> ret = drm_atomic_commit(state);
>
> put_state:
> if (ret == -EDEADLK) {
> drm_atomic_state_clear(state);
> ---
>
> Regards
> Andrzej
> >
> >> is not triggeed
> >>
> >> Should be easy to type up a dedicated test case for that.
> >>
> >> I think there are two options of handling this:
> >> - Switch all planes out of async flip mode for any sync update.
> >> Not great because then you can't mix async flips with any other
> >> sync updates on the same crtc
> >> - Start tracking which planes are in async flip mode vs. not
> >> Should allow more freedom in mixing async flips with other updates
> >> on the crtc
> >>
> >> --
> >> Ville Syrjälä
> >> Intel
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2022-09-06 16:21 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-05 8:05 [Intel-gfx] [PATCH] drm/i915: do not reset PLANE_SURF on plane disable on older gens Andrzej Hajda
2022-09-05 8:53 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2022-09-05 9:07 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-09-05 10:59 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2022-09-05 11:48 ` [Intel-gfx] [PATCH] " Ville Syrjälä
2022-09-05 17:02 ` Andrzej Hajda
2022-09-05 17:44 ` Ville Syrjälä
2022-09-06 11:09 ` Andrzej Hajda
2022-09-06 11:22 ` Ville Syrjälä
2022-09-06 11:32 ` Ville Syrjälä
2022-09-06 13:57 ` Andrzej Hajda
2022-09-06 14:14 ` Ville Syrjälä
2022-09-06 14:36 ` Ville Syrjälä
2022-09-06 14:43 ` Ville Syrjälä
2022-09-06 16:14 ` Andrzej Hajda
2022-09-06 16:21 ` Ville Syrjälä
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.