* [Intel-gfx] [PATCH] drm/i915: Move hotplug inversion logic into separate helper
@ 2022-09-19 14:56 Gustavo Sousa
2022-09-19 22:58 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Gustavo Sousa @ 2022-09-19 14:56 UTC (permalink / raw)
To: intel-gfx
Make the code more readable, which will be more apparent as new
platforms with different hotplug inversion needs are added.
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index de06f293e173..c53d21ae197f 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3263,6 +3263,21 @@ static void cherryview_irq_reset(struct drm_i915_private *dev_priv)
spin_unlock_irq(&dev_priv->irq_lock);
}
+static void setup_hotplug_inversion(struct drm_i915_private *dev_priv)
+{
+ u32 invert_bits;
+
+ if (HAS_PCH_DG1(dev_priv))
+ invert_bits = INVERT_DDIA_HPD |
+ INVERT_DDIB_HPD |
+ INVERT_DDIC_HPD |
+ INVERT_DDID_HPD;
+ else
+ return;
+
+ intel_uncore_rmw(&dev_priv->uncore, SOUTH_CHICKEN1, 0, invert_bits);
+}
+
static u32 ibx_hotplug_enables(struct drm_i915_private *i915,
enum hpd_pin pin)
{
@@ -3413,15 +3428,7 @@ static u32 gen11_hotplug_enables(struct drm_i915_private *i915,
static void dg1_hpd_irq_setup(struct drm_i915_private *dev_priv)
{
- u32 val;
-
- val = intel_uncore_read(&dev_priv->uncore, SOUTH_CHICKEN1);
- val |= (INVERT_DDIA_HPD |
- INVERT_DDIB_HPD |
- INVERT_DDIC_HPD |
- INVERT_DDID_HPD);
- intel_uncore_write(&dev_priv->uncore, SOUTH_CHICKEN1, val);
-
+ setup_hotplug_inversion(dev_priv);
icp_hpd_irq_setup(dev_priv);
}
--
2.37.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Move hotplug inversion logic into separate helper
2022-09-19 14:56 [Intel-gfx] [PATCH] drm/i915: Move hotplug inversion logic into separate helper Gustavo Sousa
@ 2022-09-19 22:58 ` Patchwork
2022-09-20 6:47 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2022-09-19 22:58 UTC (permalink / raw)
To: Gustavo Sousa; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 4889 bytes --]
== Series Details ==
Series: drm/i915: Move hotplug inversion logic into separate helper
URL : https://patchwork.freedesktop.org/series/108742/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_12158 -> Patchwork_108742v1
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/index.html
Participating hosts (42 -> 38)
------------------------------
Missing (4): fi-ctg-p8600 fi-cfl-8109u bat-jsl-3 bat-dg1-5
Known issues
------------
Here are the changes found in Patchwork_108742v1 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@kms_chamelium@common-hpd-after-suspend:
- fi-hsw-4770: NOTRUN -> [SKIP][1] ([fdo#109271] / [fdo#111827])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/fi-hsw-4770/igt@kms_chamelium@common-hpd-after-suspend.html
- fi-rkl-11600: NOTRUN -> [SKIP][2] ([fdo#111827])
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/fi-rkl-11600/igt@kms_chamelium@common-hpd-after-suspend.html
#### Possible fixes ####
* igt@fbdev@read:
- {fi-tgl-mst}: [SKIP][3] ([i915#2582]) -> [PASS][4] +2 similar issues
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/fi-tgl-mst/igt@fbdev@read.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/fi-tgl-mst/igt@fbdev@read.html
* igt@gem_ctx_create@basic-files:
- {fi-tgl-mst}: [DMESG-WARN][5] -> [PASS][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/fi-tgl-mst/igt@gem_ctx_create@basic-files.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/fi-tgl-mst/igt@gem_ctx_create@basic-files.html
* igt@gem_exec_suspend@basic-s0@smem:
- {bat-adlm-1}: [DMESG-WARN][7] ([i915#2867]) -> [PASS][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/bat-adlm-1/igt@gem_exec_suspend@basic-s0@smem.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/bat-adlm-1/igt@gem_exec_suspend@basic-s0@smem.html
* igt@i915_module_load@reload:
- {fi-tgl-mst}: [WARN][9] ([i915#6596]) -> [PASS][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/fi-tgl-mst/igt@i915_module_load@reload.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/fi-tgl-mst/igt@i915_module_load@reload.html
* igt@i915_selftest@live@hangcheck:
- fi-hsw-4770: [INCOMPLETE][11] ([i915#4785]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
* igt@i915_suspend@basic-s3-without-i915:
- fi-rkl-11600: [INCOMPLETE][13] ([i915#5982]) -> [PASS][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/fi-rkl-11600/igt@i915_suspend@basic-s3-without-i915.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/fi-rkl-11600/igt@i915_suspend@basic-s3-without-i915.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#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
[i915#2867]: https://gitlab.freedesktop.org/drm/intel/issues/2867
[i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
[i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785
[i915#5122]: https://gitlab.freedesktop.org/drm/intel/issues/5122
[i915#5257]: https://gitlab.freedesktop.org/drm/intel/issues/5257
[i915#5537]: https://gitlab.freedesktop.org/drm/intel/issues/5537
[i915#5982]: https://gitlab.freedesktop.org/drm/intel/issues/5982
[i915#6434]: https://gitlab.freedesktop.org/drm/intel/issues/6434
[i915#6596]: https://gitlab.freedesktop.org/drm/intel/issues/6596
[i915#6599]: https://gitlab.freedesktop.org/drm/intel/issues/6599
[i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
Build changes
-------------
* Linux: CI_DRM_12158 -> Patchwork_108742v1
CI-20190529: 20190529
CI_DRM_12158: 3bde74f15d452bf788ecab8933ee802b2ee9e673 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_6656: 24100c4e181c50e3678aeca9c641b8a43555ad73 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_108742v1: 3bde74f15d452bf788ecab8933ee802b2ee9e673 @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
0f142ed49d1e drm/i915: Move hotplug inversion logic into separate helper
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/index.html
[-- Attachment #2: Type: text/html, Size: 5283 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915: Move hotplug inversion logic into separate helper
2022-09-19 14:56 [Intel-gfx] [PATCH] drm/i915: Move hotplug inversion logic into separate helper Gustavo Sousa
2022-09-19 22:58 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
@ 2022-09-20 6:47 ` Patchwork
2022-09-20 7:01 ` [Intel-gfx] [PATCH] " Lucas De Marchi
2022-09-20 7:19 ` Jani Nikula
3 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2022-09-20 6:47 UTC (permalink / raw)
To: Gustavo Sousa; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 29516 bytes --]
== Series Details ==
Series: drm/i915: Move hotplug inversion logic into separate helper
URL : https://patchwork.freedesktop.org/series/108742/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_12158_full -> Patchwork_108742v1_full
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_108742v1_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_108742v1_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
Participating hosts (10 -> 10)
------------------------------
No changes in participating hosts
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_108742v1_full:
### IGT changes ###
#### Possible regressions ####
* igt@kms_plane_scaling@planes-scaler-unity-scaling@pipe-d-edp-1:
- shard-tglb: [PASS][1] -> [INCOMPLETE][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-tglb1/igt@kms_plane_scaling@planes-scaler-unity-scaling@pipe-d-edp-1.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-tglb8/igt@kms_plane_scaling@planes-scaler-unity-scaling@pipe-d-edp-1.html
#### Warnings ####
* igt@runner@aborted:
- shard-skl: ([FAIL][3], [FAIL][4], [FAIL][5], [FAIL][6], [FAIL][7], [FAIL][8], [FAIL][9], [FAIL][10], [FAIL][11], [FAIL][12], [FAIL][13], [FAIL][14], [FAIL][15], [FAIL][16], [FAIL][17], [FAIL][18], [FAIL][19], [FAIL][20], [FAIL][21], [FAIL][22], [FAIL][23], [FAIL][24], [FAIL][25], [FAIL][26], [FAIL][27]) ([i915#6599]) -> ([FAIL][28], [FAIL][29], [FAIL][30], [FAIL][31], [FAIL][32], [FAIL][33], [FAIL][34], [FAIL][35], [FAIL][36], [FAIL][37], [FAIL][38], [FAIL][39], [FAIL][40], [FAIL][41], [FAIL][42], [FAIL][43], [FAIL][44], [FAIL][45], [FAIL][46], [FAIL][47], [FAIL][48], [FAIL][49], [FAIL][50], [FAIL][51], [FAIL][52])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl1/igt@runner@aborted.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl4/igt@runner@aborted.html
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl1/igt@runner@aborted.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl10/igt@runner@aborted.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl1/igt@runner@aborted.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl10/igt@runner@aborted.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl10/igt@runner@aborted.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl10/igt@runner@aborted.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl1/igt@runner@aborted.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl10/igt@runner@aborted.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl7/igt@runner@aborted.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl4/igt@runner@aborted.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl6/igt@runner@aborted.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl6/igt@runner@aborted.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl4/igt@runner@aborted.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl6/igt@runner@aborted.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl9/igt@runner@aborted.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl6/igt@runner@aborted.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl7/igt@runner@aborted.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl4/igt@runner@aborted.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl9/igt@runner@aborted.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl7/igt@runner@aborted.html
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl7/igt@runner@aborted.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl9/igt@runner@aborted.html
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-skl9/igt@runner@aborted.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl7/igt@runner@aborted.html
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl1/igt@runner@aborted.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl10/igt@runner@aborted.html
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl9/igt@runner@aborted.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl6/igt@runner@aborted.html
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl10/igt@runner@aborted.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl4/igt@runner@aborted.html
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl1/igt@runner@aborted.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl9/igt@runner@aborted.html
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl1/igt@runner@aborted.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl4/igt@runner@aborted.html
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl9/igt@runner@aborted.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl6/igt@runner@aborted.html
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl4/igt@runner@aborted.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl10/igt@runner@aborted.html
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl7/igt@runner@aborted.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl4/igt@runner@aborted.html
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl1/igt@runner@aborted.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl10/igt@runner@aborted.html
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl9/igt@runner@aborted.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl6/igt@runner@aborted.html
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl7/igt@runner@aborted.html
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl6/igt@runner@aborted.html
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl7/igt@runner@aborted.html
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-skl10/igt@runner@aborted.html
Known issues
------------
Here are the changes found in Patchwork_108742v1_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_balancer@parallel-balancer:
- shard-iclb: [PASS][53] -> [SKIP][54] ([i915#4525])
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-iclb1/igt@gem_exec_balancer@parallel-balancer.html
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-iclb5/igt@gem_exec_balancer@parallel-balancer.html
* igt@gem_exec_fair@basic-none@vecs0:
- shard-glk: [PASS][55] -> [FAIL][56] ([i915#2842])
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-glk9/igt@gem_exec_fair@basic-none@vecs0.html
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-glk6/igt@gem_exec_fair@basic-none@vecs0.html
* igt@gem_exec_fair@basic-pace-share@rcs0:
- shard-apl: [PASS][57] -> [FAIL][58] ([i915#2842])
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-apl6/igt@gem_exec_fair@basic-pace-share@rcs0.html
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-apl1/igt@gem_exec_fair@basic-pace-share@rcs0.html
* igt@gem_exec_fair@basic-throttle@rcs0:
- shard-iclb: [PASS][59] -> [FAIL][60] ([i915#2842])
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-iclb1/igt@gem_exec_fair@basic-throttle@rcs0.html
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-iclb1/igt@gem_exec_fair@basic-throttle@rcs0.html
* igt@gem_exec_flush@basic-uc-set-default:
- shard-apl: [PASS][61] -> [DMESG-FAIL][62] ([i915#6864])
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-apl8/igt@gem_exec_flush@basic-uc-set-default.html
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-apl6/igt@gem_exec_flush@basic-uc-set-default.html
* igt@gem_userptr_blits@input-checking:
- shard-apl: NOTRUN -> [DMESG-WARN][63] ([i915#4991])
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-apl2/igt@gem_userptr_blits@input-checking.html
* igt@gen7_exec_parse@oacontrol-tracking:
- shard-apl: NOTRUN -> [SKIP][64] ([fdo#109271]) +32 similar issues
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-apl2/igt@gen7_exec_parse@oacontrol-tracking.html
* igt@kms_big_fb@linear-max-hw-stride-64bpp-rotate-180:
- shard-iclb: [PASS][65] -> [DMESG-WARN][66] ([i915#402])
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-iclb7/igt@kms_big_fb@linear-max-hw-stride-64bpp-rotate-180.html
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-iclb2/igt@kms_big_fb@linear-max-hw-stride-64bpp-rotate-180.html
* igt@kms_ccs@pipe-a-bad-aux-stride-y_tiled_gen12_rc_ccs_cc:
- shard-apl: NOTRUN -> [SKIP][67] ([fdo#109271] / [i915#3886])
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-apl2/igt@kms_ccs@pipe-a-bad-aux-stride-y_tiled_gen12_rc_ccs_cc.html
* igt@kms_chamelium@dp-hpd-fast:
- shard-apl: NOTRUN -> [SKIP][68] ([fdo#109271] / [fdo#111827]) +1 similar issue
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-apl2/igt@kms_chamelium@dp-hpd-fast.html
* igt@kms_dither@fb-8bpc-vs-panel-6bpc@pipe-a-hdmi-a-1:
- shard-glk: NOTRUN -> [SKIP][69] ([fdo#109271])
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-glk1/igt@kms_dither@fb-8bpc-vs-panel-6bpc@pipe-a-hdmi-a-1.html
* igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@ab-hdmi-a1-hdmi-a2:
- shard-glk: [PASS][70] -> [FAIL][71] ([i915#2122]) +1 similar issue
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-glk8/igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@ab-hdmi-a1-hdmi-a2.html
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-glk6/igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@ab-hdmi-a1-hdmi-a2.html
* igt@kms_flip_scaled_crc@flip-32bpp-4tile-to-32bpp-4tiledg2rcccs-downscaling@pipe-a-valid-mode:
- shard-iclb: NOTRUN -> [SKIP][72] ([i915#2587] / [i915#2672]) +1 similar issue
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-iclb7/igt@kms_flip_scaled_crc@flip-32bpp-4tile-to-32bpp-4tiledg2rcccs-downscaling@pipe-a-valid-mode.html
* igt@kms_flip_scaled_crc@flip-32bpp-4tile-to-32bpp-4tiledg2rcccs-upscaling@pipe-a-default-mode:
- shard-iclb: NOTRUN -> [SKIP][73] ([i915#2672]) +3 similar issues
[73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-iclb3/igt@kms_flip_scaled_crc@flip-32bpp-4tile-to-32bpp-4tiledg2rcccs-upscaling@pipe-a-default-mode.html
* igt@kms_flip_scaled_crc@flip-32bpp-yftile-to-32bpp-yftileccs-downscaling@pipe-a-default-mode:
- shard-iclb: [PASS][74] -> [SKIP][75] ([i915#6375])
[74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-iclb3/igt@kms_flip_scaled_crc@flip-32bpp-yftile-to-32bpp-yftileccs-downscaling@pipe-a-default-mode.html
[75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-iclb2/igt@kms_flip_scaled_crc@flip-32bpp-yftile-to-32bpp-yftileccs-downscaling@pipe-a-default-mode.html
* igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytileccs-downscaling@pipe-a-default-mode:
- shard-iclb: NOTRUN -> [SKIP][76] ([i915#3555])
[76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-iclb2/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytileccs-downscaling@pipe-a-default-mode.html
* igt@kms_plane_scaling@plane-scaler-with-clipping-clamping-pixel-formats@pipe-b-edp-1:
- shard-iclb: [PASS][77] -> [SKIP][78] ([i915#5176]) +1 similar issue
[77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-iclb5/igt@kms_plane_scaling@plane-scaler-with-clipping-clamping-pixel-formats@pipe-b-edp-1.html
[78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-iclb3/igt@kms_plane_scaling@plane-scaler-with-clipping-clamping-pixel-formats@pipe-b-edp-1.html
* igt@kms_psr2_sf@cursor-plane-move-continuous-exceed-fully-sf:
- shard-apl: NOTRUN -> [SKIP][79] ([fdo#109271] / [i915#658])
[79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-apl2/igt@kms_psr2_sf@cursor-plane-move-continuous-exceed-fully-sf.html
* igt@kms_psr@psr2_sprite_plane_onoff:
- shard-iclb: [PASS][80] -> [SKIP][81] ([fdo#109441]) +2 similar issues
[80]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-iclb2/igt@kms_psr@psr2_sprite_plane_onoff.html
[81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-iclb7/igt@kms_psr@psr2_sprite_plane_onoff.html
* igt@kms_vblank@pipe-b-ts-continuation-suspend:
- shard-apl: [PASS][82] -> [DMESG-WARN][83] ([i915#180]) +2 similar issues
[82]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-apl7/igt@kms_vblank@pipe-b-ts-continuation-suspend.html
[83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-apl1/igt@kms_vblank@pipe-b-ts-continuation-suspend.html
#### Possible fixes ####
* igt@drm_import_export@prime:
- shard-apl: [DMESG-WARN][84] ([i915#6864]) -> [PASS][85]
[84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-apl8/igt@drm_import_export@prime.html
[85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-apl7/igt@drm_import_export@prime.html
* igt@gem_ctx_exec@basic-nohangcheck:
- shard-tglb: [FAIL][86] ([i915#6268]) -> [PASS][87]
[86]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-tglb3/igt@gem_ctx_exec@basic-nohangcheck.html
[87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-tglb2/igt@gem_ctx_exec@basic-nohangcheck.html
* igt@gem_eio@in-flight-suspend:
- shard-apl: [DMESG-WARN][88] ([i915#180]) -> [PASS][89]
[88]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-apl6/igt@gem_eio@in-flight-suspend.html
[89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-apl2/igt@gem_eio@in-flight-suspend.html
* igt@gem_exec_balancer@parallel-keep-in-fence:
- shard-iclb: [SKIP][90] ([i915#4525]) -> [PASS][91] +1 similar issue
[90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-iclb7/igt@gem_exec_balancer@parallel-keep-in-fence.html
[91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-iclb4/igt@gem_exec_balancer@parallel-keep-in-fence.html
* igt@gem_exec_fair@basic-none-share@rcs0:
- {shard-tglu}: [FAIL][92] ([i915#2842]) -> [PASS][93]
[92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-tglu-6/igt@gem_exec_fair@basic-none-share@rcs0.html
[93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-tglu-4/igt@gem_exec_fair@basic-none-share@rcs0.html
* igt@gem_exec_fair@basic-none@vcs0:
- shard-glk: [FAIL][94] ([i915#2842]) -> [PASS][95]
[94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-glk9/igt@gem_exec_fair@basic-none@vcs0.html
[95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-glk6/igt@gem_exec_fair@basic-none@vcs0.html
* igt@i915_pm_dc@dc5-psr:
- shard-iclb: [FAIL][96] ([i915#3989]) -> [PASS][97]
[96]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-iclb8/igt@i915_pm_dc@dc5-psr.html
[97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-iclb5/igt@i915_pm_dc@dc5-psr.html
* igt@i915_selftest@live@hangcheck:
- shard-tglb: [DMESG-WARN][98] ([i915#5591]) -> [PASS][99]
[98]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-tglb5/igt@i915_selftest@live@hangcheck.html
[99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-tglb5/igt@i915_selftest@live@hangcheck.html
* igt@kms_frontbuffer_tracking@fbc-badstride:
- shard-snb: [SKIP][100] ([fdo#109271]) -> [PASS][101]
[100]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-snb4/igt@kms_frontbuffer_tracking@fbc-badstride.html
[101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-snb4/igt@kms_frontbuffer_tracking@fbc-badstride.html
* igt@kms_psr@psr2_primary_blt:
- shard-iclb: [SKIP][102] ([fdo#109441]) -> [PASS][103] +1 similar issue
[102]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-iclb7/igt@kms_psr@psr2_primary_blt.html
[103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-iclb2/igt@kms_psr@psr2_primary_blt.html
* igt@kms_vblank@pipe-b-accuracy-idle:
- shard-glk: [FAIL][104] ([i915#43]) -> [PASS][105]
[104]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-glk6/igt@kms_vblank@pipe-b-accuracy-idle.html
[105]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-glk2/igt@kms_vblank@pipe-b-accuracy-idle.html
#### Warnings ####
* igt@gem_exec_balancer@parallel-ordering:
- shard-iclb: [FAIL][106] ([i915#6117]) -> [SKIP][107] ([i915#4525])
[106]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-iclb2/igt@gem_exec_balancer@parallel-ordering.html
[107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-iclb7/igt@gem_exec_balancer@parallel-ordering.html
* igt@i915_pm_rc6_residency@rc6-idle@vcs0:
- shard-iclb: [FAIL][108] ([i915#2684]) -> [WARN][109] ([i915#2684])
[108]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-iclb2/igt@i915_pm_rc6_residency@rc6-idle@vcs0.html
[109]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-iclb7/igt@i915_pm_rc6_residency@rc6-idle@vcs0.html
* igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-sf:
- shard-iclb: [SKIP][110] ([i915#658]) -> [SKIP][111] ([i915#2920]) +1 similar issue
[110]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-iclb7/igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-sf.html
[111]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-iclb2/igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-sf.html
* igt@kms_psr2_sf@primary-plane-update-sf-dmg-area:
- shard-iclb: [SKIP][112] ([fdo#111068] / [i915#658]) -> [SKIP][113] ([i915#2920]) +1 similar issue
[112]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-iclb3/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area.html
[113]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-iclb2/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area.html
* igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-big-fb:
- shard-iclb: [SKIP][114] ([i915#2920]) -> [SKIP][115] ([i915#658])
[114]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-iclb2/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-big-fb.html
[115]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-iclb7/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-big-fb.html
* igt@runner@aborted:
- shard-apl: ([FAIL][116], [FAIL][117], [FAIL][118], [FAIL][119]) ([fdo#109271] / [i915#180] / [i915#3002] / [i915#4312] / [i915#5257] / [i915#6599]) -> ([FAIL][120], [FAIL][121], [FAIL][122], [FAIL][123], [FAIL][124], [FAIL][125], [FAIL][126]) ([fdo#109271] / [i915#180] / [i915#3002] / [i915#4312] / [i915#5257])
[116]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-apl8/igt@runner@aborted.html
[117]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-apl1/igt@runner@aborted.html
[118]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-apl3/igt@runner@aborted.html
[119]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-apl6/igt@runner@aborted.html
[120]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-apl1/igt@runner@aborted.html
[121]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-apl1/igt@runner@aborted.html
[122]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-apl8/igt@runner@aborted.html
[123]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-apl6/igt@runner@aborted.html
[124]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-apl3/igt@runner@aborted.html
[125]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-apl6/igt@runner@aborted.html
[126]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-apl2/igt@runner@aborted.html
- shard-tglb: ([FAIL][127], [FAIL][128]) ([i915#3002] / [i915#4312] / [i915#5257] / [i915#6599]) -> ([FAIL][129], [FAIL][130]) ([i915#3002] / [i915#4312] / [i915#5257])
[127]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-tglb8/igt@runner@aborted.html
[128]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-tglb1/igt@runner@aborted.html
[129]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-tglb1/igt@runner@aborted.html
[130]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-tglb8/igt@runner@aborted.html
- shard-glk: ([FAIL][131], [FAIL][132], [FAIL][133], [FAIL][134]) ([i915#3002] / [i915#4312] / [i915#5257] / [i915#6599]) -> ([FAIL][135], [FAIL][136], [FAIL][137], [FAIL][138]) ([i915#3002] / [i915#4312] / [i915#5257])
[131]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-glk6/igt@runner@aborted.html
[132]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-glk2/igt@runner@aborted.html
[133]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-glk2/igt@runner@aborted.html
[134]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-glk1/igt@runner@aborted.html
[135]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-glk5/igt@runner@aborted.html
[136]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-glk5/igt@runner@aborted.html
[137]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-glk2/igt@runner@aborted.html
[138]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-glk7/igt@runner@aborted.html
- shard-iclb: ([FAIL][139], [FAIL][140]) ([i915#3002] / [i915#4312] / [i915#5257] / [i915#6599]) -> ([FAIL][141], [FAIL][142]) ([i915#3002] / [i915#4312] / [i915#5257])
[139]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-iclb7/igt@runner@aborted.html
[140]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12158/shard-iclb5/igt@runner@aborted.html
[141]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-iclb2/igt@runner@aborted.html
[142]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108742v1/shard-iclb3/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#109279]: https://bugs.freedesktop.org/show_bug.cgi?id=109279
[fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
[fdo#109283]: https://bugs.freedesktop.org/show_bug.cgi?id=109283
[fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
[fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
[fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
[fdo#109312]: https://bugs.freedesktop.org/show_bug.cgi?id=109312
[fdo#109313]: https://bugs.freedesktop.org/show_bug.cgi?id=109313
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#109506]: https://bugs.freedesktop.org/show_bug.cgi?id=109506
[fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
[fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
[fdo#111614]: https://bugs.freedesktop.org/show_bug.cgi?id=111614
[fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
[fdo#111644]: https://bugs.freedesktop.org/show_bug.cgi?id=111644
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[fdo#112283]: https://bugs.freedesktop.org/show_bug.cgi?id=112283
[i915#1063]: https://gitlab.freedesktop.org/drm/intel/issues/1063
[i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
[i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
[i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
[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#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
[i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
[i915#2681]: https://gitlab.freedesktop.org/drm/intel/issues/2681
[i915#2684]: https://gitlab.freedesktop.org/drm/intel/issues/2684
[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#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
[i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
[i915#3469]: https://gitlab.freedesktop.org/drm/intel/issues/3469
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
[i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
[i915#3804]: https://gitlab.freedesktop.org/drm/intel/issues/3804
[i915#3840]: https://gitlab.freedesktop.org/drm/intel/issues/3840
[i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
[i915#3989]: https://gitlab.freedesktop.org/drm/intel/issues/3989
[i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
[i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
[i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
[i915#43]: https://gitlab.freedesktop.org/drm/intel/issues/43
[i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
[i915#4525]: https://gitlab.freedesktop.org/drm/intel/issues/4525
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4991]: https://gitlab.freedesktop.org/drm/intel/issues/4991
[i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
[i915#5257]: https://gitlab.freedesktop.org/drm/intel/issues/5257
[i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
[i915#5289]: https://gitlab.freedesktop.org/drm/intel/issues/5289
[i915#5325]: https://gitlab.freedesktop.org/drm/intel/issues/5325
[i915#5461]: https://gitlab.freedesktop.org/drm/intel/issues/5461
[i915#5591]: https://gitlab.freedesktop.org/drm/intel/issues/5591
[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#6268]: https://gitlab.freedesktop.org/drm/intel/issues/6268
[i915#6344]: https://gitlab.freedesktop.org/drm/intel/issues/6344
[i915#6375]: https://gitlab.freedesktop.org/drm/intel/issues/6375
[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#6864]: https://gitlab.freedesktop.org/drm/intel/issues/6864
Build changes
-------------
* Linux: CI_DRM_12158 -> Patchwork_108742v1
CI-20190529: 20190529
CI_DRM_12158: 3bde74f15d452bf788ecab8933ee802b2ee9e673 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_6656: 24100c4e181c50e3678aeca9c641b8a43555ad73 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_108742v1: 3bde74f15d452bf788ecab8933ee802b2ee9e673 @ 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_108742v1/index.html
[-- Attachment #2: Type: text/html, Size: 31726 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Move hotplug inversion logic into separate helper
2022-09-19 14:56 [Intel-gfx] [PATCH] drm/i915: Move hotplug inversion logic into separate helper Gustavo Sousa
2022-09-19 22:58 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
2022-09-20 6:47 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
@ 2022-09-20 7:01 ` Lucas De Marchi
2022-09-20 16:27 ` Gustavo Sousa
2022-09-20 7:19 ` Jani Nikula
3 siblings, 1 reply; 14+ messages in thread
From: Lucas De Marchi @ 2022-09-20 7:01 UTC (permalink / raw)
To: Gustavo Sousa; +Cc: intel-gfx
On Mon, Sep 19, 2022 at 11:56:59AM -0300, Gustavo Sousa wrote:
>Make the code more readable, which will be more apparent as new
>platforms with different hotplug inversion needs are added.
>
>Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>---
> drivers/gpu/drm/i915/i915_irq.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>index de06f293e173..c53d21ae197f 100644
>--- a/drivers/gpu/drm/i915/i915_irq.c
>+++ b/drivers/gpu/drm/i915/i915_irq.c
>@@ -3263,6 +3263,21 @@ static void cherryview_irq_reset(struct drm_i915_private *dev_priv)
> spin_unlock_irq(&dev_priv->irq_lock);
> }
>
>+static void setup_hotplug_inversion(struct drm_i915_private *dev_priv)
new users of drm_i915_private should use "i915" as variable name rather
than dev_priv.
other than that, Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
Lucas De Marchi
>+{
>+ u32 invert_bits;
>+
>+ if (HAS_PCH_DG1(dev_priv))
>+ invert_bits = INVERT_DDIA_HPD |
>+ INVERT_DDIB_HPD |
>+ INVERT_DDIC_HPD |
>+ INVERT_DDID_HPD;
>+ else
>+ return;
>+
>+ intel_uncore_rmw(&dev_priv->uncore, SOUTH_CHICKEN1, 0, invert_bits);
>+}
>+
> static u32 ibx_hotplug_enables(struct drm_i915_private *i915,
> enum hpd_pin pin)
> {
>@@ -3413,15 +3428,7 @@ static u32 gen11_hotplug_enables(struct drm_i915_private *i915,
>
> static void dg1_hpd_irq_setup(struct drm_i915_private *dev_priv)
> {
>- u32 val;
>-
>- val = intel_uncore_read(&dev_priv->uncore, SOUTH_CHICKEN1);
>- val |= (INVERT_DDIA_HPD |
>- INVERT_DDIB_HPD |
>- INVERT_DDIC_HPD |
>- INVERT_DDID_HPD);
>- intel_uncore_write(&dev_priv->uncore, SOUTH_CHICKEN1, val);
>-
>+ setup_hotplug_inversion(dev_priv);
> icp_hpd_irq_setup(dev_priv);
> }
>
>--
>2.37.3
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Move hotplug inversion logic into separate helper
2022-09-19 14:56 [Intel-gfx] [PATCH] drm/i915: Move hotplug inversion logic into separate helper Gustavo Sousa
` (2 preceding siblings ...)
2022-09-20 7:01 ` [Intel-gfx] [PATCH] " Lucas De Marchi
@ 2022-09-20 7:19 ` Jani Nikula
2022-09-20 17:04 ` Gustavo Sousa
3 siblings, 1 reply; 14+ messages in thread
From: Jani Nikula @ 2022-09-20 7:19 UTC (permalink / raw)
To: Gustavo Sousa, intel-gfx
On Mon, 19 Sep 2022, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> Make the code more readable, which will be more apparent as new
> platforms with different hotplug inversion needs are added.
>
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index de06f293e173..c53d21ae197f 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -3263,6 +3263,21 @@ static void cherryview_irq_reset(struct drm_i915_private *dev_priv)
> spin_unlock_irq(&dev_priv->irq_lock);
> }
>
> +static void setup_hotplug_inversion(struct drm_i915_private *dev_priv)
> +{
> + u32 invert_bits;
> +
> + if (HAS_PCH_DG1(dev_priv))
> + invert_bits = INVERT_DDIA_HPD |
> + INVERT_DDIB_HPD |
> + INVERT_DDIC_HPD |
> + INVERT_DDID_HPD;
Nitpick, the indentation will be off compared to automated indentation.
> + else
> + return;
> +
> + intel_uncore_rmw(&dev_priv->uncore, SOUTH_CHICKEN1, 0, invert_bits);
> +}
> +
> static u32 ibx_hotplug_enables(struct drm_i915_private *i915,
> enum hpd_pin pin)
> {
> @@ -3413,15 +3428,7 @@ static u32 gen11_hotplug_enables(struct drm_i915_private *i915,
>
> static void dg1_hpd_irq_setup(struct drm_i915_private *dev_priv)
> {
> - u32 val;
> -
> - val = intel_uncore_read(&dev_priv->uncore, SOUTH_CHICKEN1);
> - val |= (INVERT_DDIA_HPD |
> - INVERT_DDIB_HPD |
> - INVERT_DDIC_HPD |
> - INVERT_DDID_HPD);
> - intel_uncore_write(&dev_priv->uncore, SOUTH_CHICKEN1, val);
> -
> + setup_hotplug_inversion(dev_priv);
Since you're already in a platform specific function here, seems a bit
odd to call a new generic function that needs to have another if ladder
platform check. What are we gaining here? The end result is
de-duplicating just one line of intel_uncore_rmw(). I'm not convinced.
BR,
Jani.
> icp_hpd_irq_setup(dev_priv);
> }
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Move hotplug inversion logic into separate helper
2022-09-20 7:01 ` [Intel-gfx] [PATCH] " Lucas De Marchi
@ 2022-09-20 16:27 ` Gustavo Sousa
2022-09-21 9:59 ` Jani Nikula
0 siblings, 1 reply; 14+ messages in thread
From: Gustavo Sousa @ 2022-09-20 16:27 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
On Tue, Sep 20, 2022 at 12:01:36AM -0700, Lucas De Marchi wrote:
> On Mon, Sep 19, 2022 at 11:56:59AM -0300, Gustavo Sousa wrote:
> > Make the code more readable, which will be more apparent as new
> > platforms with different hotplug inversion needs are added.
> >
> > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 25 ++++++++++++++++---------
> > 1 file changed, 16 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index de06f293e173..c53d21ae197f 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -3263,6 +3263,21 @@ static void cherryview_irq_reset(struct drm_i915_private *dev_priv)
> > spin_unlock_irq(&dev_priv->irq_lock);
> > }
> >
> > +static void setup_hotplug_inversion(struct drm_i915_private *dev_priv)
>
> new users of drm_i915_private should use "i915" as variable name rather
> than dev_priv.
Thanks. I will update this.
Is there any documentation where we can find information like this?
>
> other than that, Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
>
>
> Lucas De Marchi
>
> > +{
> > + u32 invert_bits;
> > +
> > + if (HAS_PCH_DG1(dev_priv))
> > + invert_bits = INVERT_DDIA_HPD |
> > + INVERT_DDIB_HPD |
> > + INVERT_DDIC_HPD |
> > + INVERT_DDID_HPD;
> > + else
> > + return;
> > +
> > + intel_uncore_rmw(&dev_priv->uncore, SOUTH_CHICKEN1, 0, invert_bits);
> > +}
> > +
> > static u32 ibx_hotplug_enables(struct drm_i915_private *i915,
> > enum hpd_pin pin)
> > {
> > @@ -3413,15 +3428,7 @@ static u32 gen11_hotplug_enables(struct drm_i915_private *i915,
> >
> > static void dg1_hpd_irq_setup(struct drm_i915_private *dev_priv)
> > {
> > - u32 val;
> > -
> > - val = intel_uncore_read(&dev_priv->uncore, SOUTH_CHICKEN1);
> > - val |= (INVERT_DDIA_HPD |
> > - INVERT_DDIB_HPD |
> > - INVERT_DDIC_HPD |
> > - INVERT_DDID_HPD);
> > - intel_uncore_write(&dev_priv->uncore, SOUTH_CHICKEN1, val);
> > -
> > + setup_hotplug_inversion(dev_priv);
> > icp_hpd_irq_setup(dev_priv);
> > }
> >
> > --
> > 2.37.3
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Move hotplug inversion logic into separate helper
2022-09-20 7:19 ` Jani Nikula
@ 2022-09-20 17:04 ` Gustavo Sousa
2022-09-20 17:56 ` Lucas De Marchi
2022-09-21 8:21 ` Jani Nikula
0 siblings, 2 replies; 14+ messages in thread
From: Gustavo Sousa @ 2022-09-20 17:04 UTC (permalink / raw)
To: Jani Nikula, intel-gfx; +Cc: Lucas De Marchi
Hi, Jani.
On Tue, Sep 20, 2022 at 10:19:53AM +0300, Jani Nikula wrote:
> On Mon, 19 Sep 2022, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> > Make the code more readable, which will be more apparent as new
> > platforms with different hotplug inversion needs are added.
> >
> > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 25 ++++++++++++++++---------
> > 1 file changed, 16 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index de06f293e173..c53d21ae197f 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -3263,6 +3263,21 @@ static void cherryview_irq_reset(struct drm_i915_private *dev_priv)
> > spin_unlock_irq(&dev_priv->irq_lock);
> > }
> >
> > +static void setup_hotplug_inversion(struct drm_i915_private *dev_priv)
> > +{
> > + u32 invert_bits;
> > +
> > + if (HAS_PCH_DG1(dev_priv))
> > + invert_bits = INVERT_DDIA_HPD |
> > + INVERT_DDIB_HPD |
> > + INVERT_DDIC_HPD |
> > + INVERT_DDID_HPD;
>
> Nitpick, the indentation will be off compared to automated indentation.
What do you mean by automated indentation?
>
> > + else
> > + return;
> > +
> > + intel_uncore_rmw(&dev_priv->uncore, SOUTH_CHICKEN1, 0, invert_bits);
> > +}
> > +
> > static u32 ibx_hotplug_enables(struct drm_i915_private *i915,
> > enum hpd_pin pin)
> > {
> > @@ -3413,15 +3428,7 @@ static u32 gen11_hotplug_enables(struct drm_i915_private *i915,
> >
> > static void dg1_hpd_irq_setup(struct drm_i915_private *dev_priv)
> > {
> > - u32 val;
> > -
> > - val = intel_uncore_read(&dev_priv->uncore, SOUTH_CHICKEN1);
> > - val |= (INVERT_DDIA_HPD |
> > - INVERT_DDIB_HPD |
> > - INVERT_DDIC_HPD |
> > - INVERT_DDID_HPD);
> > - intel_uncore_write(&dev_priv->uncore, SOUTH_CHICKEN1, val);
> > -
> > + setup_hotplug_inversion(dev_priv);
>
> Since you're already in a platform specific function here, seems a bit
> odd to call a new generic function that needs to have another if ladder
> platform check. What are we gaining here? The end result is
> de-duplicating just one line of intel_uncore_rmw(). I'm not convinced.
It is true that the proposed refactor repeats a platform check, but the proposed
refactor has its benefits. As more platforms with hotplug inversion needs are
added (e.g. MTL), we will have a common place for the logic of hotplug
inversion. That arguably makes the code more readable and makes future refactors
easier when we need split a function that has become too complex due to platform
checks.
To make that last point clearer, I am quoting Lucas' (copied here as well)
comment (which was what convinced me) from a discussion regarding the advantage
of using such a helper:
that is what helpers are for, so you don't have to open code it in every
platform-fork of the function that needs it. See how the various
"Sequences to initialize display" are done in the driver... When we are
extending it to a future platform, if the change is small enough we just
add e few if/else in the same function. But it doesn't take too long for
those functions to become unreadable if there are several branches the
code path may take. So then we "fork" the function for a new platform,
but reuse the helpers doing the individual steps.
--
Gustavo Sousa
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Move hotplug inversion logic into separate helper
2022-09-20 17:04 ` Gustavo Sousa
@ 2022-09-20 17:56 ` Lucas De Marchi
2022-09-21 10:23 ` Jani Nikula
2022-09-21 8:21 ` Jani Nikula
1 sibling, 1 reply; 14+ messages in thread
From: Lucas De Marchi @ 2022-09-20 17:56 UTC (permalink / raw)
To: Gustavo Sousa; +Cc: intel-gfx
On Tue, Sep 20, 2022 at 02:04:33PM -0300, Gustavo Sousa wrote:
>Hi, Jani.
>
>On Tue, Sep 20, 2022 at 10:19:53AM +0300, Jani Nikula wrote:
>> On Mon, 19 Sep 2022, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
>> > Make the code more readable, which will be more apparent as new
>> > platforms with different hotplug inversion needs are added.
>> >
>> > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>> > ---
>> > drivers/gpu/drm/i915/i915_irq.c | 25 ++++++++++++++++---------
>> > 1 file changed, 16 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> > index de06f293e173..c53d21ae197f 100644
>> > --- a/drivers/gpu/drm/i915/i915_irq.c
>> > +++ b/drivers/gpu/drm/i915/i915_irq.c
>> > @@ -3263,6 +3263,21 @@ static void cherryview_irq_reset(struct drm_i915_private *dev_priv)
>> > spin_unlock_irq(&dev_priv->irq_lock);
>> > }
>> >
>> > +static void setup_hotplug_inversion(struct drm_i915_private *dev_priv)
>> > +{
>> > + u32 invert_bits;
>> > +
>> > + if (HAS_PCH_DG1(dev_priv))
>> > + invert_bits = INVERT_DDIA_HPD |
>> > + INVERT_DDIB_HPD |
>> > + INVERT_DDIC_HPD |
>> > + INVERT_DDID_HPD;
>>
>> Nitpick, the indentation will be off compared to automated indentation.
>
>What do you mean by automated indentation?
>
>>
>> > + else
>> > + return;
>> > +
>> > + intel_uncore_rmw(&dev_priv->uncore, SOUTH_CHICKEN1, 0, invert_bits);
>> > +}
>> > +
>> > static u32 ibx_hotplug_enables(struct drm_i915_private *i915,
>> > enum hpd_pin pin)
>> > {
>> > @@ -3413,15 +3428,7 @@ static u32 gen11_hotplug_enables(struct drm_i915_private *i915,
>> >
>> > static void dg1_hpd_irq_setup(struct drm_i915_private *dev_priv)
>> > {
>> > - u32 val;
>> > -
>> > - val = intel_uncore_read(&dev_priv->uncore, SOUTH_CHICKEN1);
>> > - val |= (INVERT_DDIA_HPD |
>> > - INVERT_DDIB_HPD |
>> > - INVERT_DDIC_HPD |
>> > - INVERT_DDID_HPD);
>> > - intel_uncore_write(&dev_priv->uncore, SOUTH_CHICKEN1, val);
>> > -
>> > + setup_hotplug_inversion(dev_priv);
>>
>> Since you're already in a platform specific function here, seems a bit
>> odd to call a new generic function that needs to have another if ladder
>> platform check. What are we gaining here? The end result is
>> de-duplicating just one line of intel_uncore_rmw(). I'm not convinced.
>
>It is true that the proposed refactor repeats a platform check, but the proposed
>refactor has its benefits. As more platforms with hotplug inversion needs are
>added (e.g. MTL), we will have a common place for the logic of hotplug
>inversion. That arguably makes the code more readable and makes future refactors
>easier when we need split a function that has become too complex due to platform
>checks.
>
>To make that last point clearer, I am quoting Lucas' (copied here as well)
>comment (which was what convinced me) from a discussion regarding the advantage
>of using such a helper:
>
> that is what helpers are for, so you don't have to open code it in every
> platform-fork of the function that needs it. See how the various
> "Sequences to initialize display" are done in the driver... When we are
> extending it to a future platform, if the change is small enough we just
> add e few if/else in the same function. But it doesn't take too long for
> those functions to become unreadable if there are several branches the
> code path may take. So then we "fork" the function for a new platform,
> but reuse the helpers doing the individual steps.
the missing information here is that there are changes in the pipeline
for platforms that have different bits to be inverted, or none at
all, with a different register to program. Adding the if/else in this
function seems unrelated churn.
Another possibility would be to just let the caller handle the if/else
decision, passing the bits (and possibly register) to invert. The noise
in xxx_hpd_irq_setup() function may be avoid by
#define INVERT_DII_HPD (INVERT_DDIA_HPD | INVERT_DDIB_HPD | INVERT_DDIC_HPD | INVERT_DDID_HPD)
#define XXX_INVERT_DII_HPD (...)
Third possibility since the function is already very small is to just go
ahead and use another _setup() for the next platforms.
Lucas De Marchi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Move hotplug inversion logic into separate helper
2022-09-20 17:04 ` Gustavo Sousa
2022-09-20 17:56 ` Lucas De Marchi
@ 2022-09-21 8:21 ` Jani Nikula
2022-09-21 12:05 ` Gustavo Sousa
1 sibling, 1 reply; 14+ messages in thread
From: Jani Nikula @ 2022-09-21 8:21 UTC (permalink / raw)
To: Gustavo Sousa, intel-gfx; +Cc: Lucas De Marchi
On Tue, 20 Sep 2022, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> Hi, Jani.
>
> On Tue, Sep 20, 2022 at 10:19:53AM +0300, Jani Nikula wrote:
>> On Mon, 19 Sep 2022, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
>> > Make the code more readable, which will be more apparent as new
>> > platforms with different hotplug inversion needs are added.
>> >
>> > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>> > ---
>> > drivers/gpu/drm/i915/i915_irq.c | 25 ++++++++++++++++---------
>> > 1 file changed, 16 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> > index de06f293e173..c53d21ae197f 100644
>> > --- a/drivers/gpu/drm/i915/i915_irq.c
>> > +++ b/drivers/gpu/drm/i915/i915_irq.c
>> > @@ -3263,6 +3263,21 @@ static void cherryview_irq_reset(struct drm_i915_private *dev_priv)
>> > spin_unlock_irq(&dev_priv->irq_lock);
>> > }
>> >
>> > +static void setup_hotplug_inversion(struct drm_i915_private *dev_priv)
>> > +{
>> > + u32 invert_bits;
>> > +
>> > + if (HAS_PCH_DG1(dev_priv))
>> > + invert_bits = INVERT_DDIA_HPD |
>> > + INVERT_DDIB_HPD |
>> > + INVERT_DDIC_HPD |
>> > + INVERT_DDID_HPD;
>>
>> Nitpick, the indentation will be off compared to automated indentation.
>
> What do you mean by automated indentation?
For example, hit TAB on the lines using a smart enough editor, which has
been configured to follow kernel coding style. ;)
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Move hotplug inversion logic into separate helper
2022-09-20 16:27 ` Gustavo Sousa
@ 2022-09-21 9:59 ` Jani Nikula
0 siblings, 0 replies; 14+ messages in thread
From: Jani Nikula @ 2022-09-21 9:59 UTC (permalink / raw)
To: Gustavo Sousa, Lucas De Marchi; +Cc: intel-gfx
On Tue, 20 Sep 2022, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> On Tue, Sep 20, 2022 at 12:01:36AM -0700, Lucas De Marchi wrote:
>> On Mon, Sep 19, 2022 at 11:56:59AM -0300, Gustavo Sousa wrote:
>> > Make the code more readable, which will be more apparent as new
>> > platforms with different hotplug inversion needs are added.
>> >
>> > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>> > ---
>> > drivers/gpu/drm/i915/i915_irq.c | 25 ++++++++++++++++---------
>> > 1 file changed, 16 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> > index de06f293e173..c53d21ae197f 100644
>> > --- a/drivers/gpu/drm/i915/i915_irq.c
>> > +++ b/drivers/gpu/drm/i915/i915_irq.c
>> > @@ -3263,6 +3263,21 @@ static void cherryview_irq_reset(struct drm_i915_private *dev_priv)
>> > spin_unlock_irq(&dev_priv->irq_lock);
>> > }
>> >
>> > +static void setup_hotplug_inversion(struct drm_i915_private *dev_priv)
>>
>> new users of drm_i915_private should use "i915" as variable name rather
>> than dev_priv.
>
> Thanks. I will update this.
>
> Is there any documentation where we can find information like this?
WIP:
https://gitlab.freedesktop.org/jani/i915-check/-/blob/main/i915-style-guide.rst
BR,
Jani.
>
>>
>> other than that, Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
>>
>>
>> Lucas De Marchi
>>
>> > +{
>> > + u32 invert_bits;
>> > +
>> > + if (HAS_PCH_DG1(dev_priv))
>> > + invert_bits = INVERT_DDIA_HPD |
>> > + INVERT_DDIB_HPD |
>> > + INVERT_DDIC_HPD |
>> > + INVERT_DDID_HPD;
>> > + else
>> > + return;
>> > +
>> > + intel_uncore_rmw(&dev_priv->uncore, SOUTH_CHICKEN1, 0, invert_bits);
>> > +}
>> > +
>> > static u32 ibx_hotplug_enables(struct drm_i915_private *i915,
>> > enum hpd_pin pin)
>> > {
>> > @@ -3413,15 +3428,7 @@ static u32 gen11_hotplug_enables(struct drm_i915_private *i915,
>> >
>> > static void dg1_hpd_irq_setup(struct drm_i915_private *dev_priv)
>> > {
>> > - u32 val;
>> > -
>> > - val = intel_uncore_read(&dev_priv->uncore, SOUTH_CHICKEN1);
>> > - val |= (INVERT_DDIA_HPD |
>> > - INVERT_DDIB_HPD |
>> > - INVERT_DDIC_HPD |
>> > - INVERT_DDID_HPD);
>> > - intel_uncore_write(&dev_priv->uncore, SOUTH_CHICKEN1, val);
>> > -
>> > + setup_hotplug_inversion(dev_priv);
>> > icp_hpd_irq_setup(dev_priv);
>> > }
>> >
>> > --
>> > 2.37.3
>> >
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Move hotplug inversion logic into separate helper
2022-09-20 17:56 ` Lucas De Marchi
@ 2022-09-21 10:23 ` Jani Nikula
0 siblings, 0 replies; 14+ messages in thread
From: Jani Nikula @ 2022-09-21 10:23 UTC (permalink / raw)
To: Lucas De Marchi, Gustavo Sousa; +Cc: intel-gfx
On Tue, 20 Sep 2022, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> On Tue, Sep 20, 2022 at 02:04:33PM -0300, Gustavo Sousa wrote:
>>Hi, Jani.
>>
>>On Tue, Sep 20, 2022 at 10:19:53AM +0300, Jani Nikula wrote:
>>> On Mon, 19 Sep 2022, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
>>> > Make the code more readable, which will be more apparent as new
>>> > platforms with different hotplug inversion needs are added.
>>> >
>>> > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>>> > ---
>>> > drivers/gpu/drm/i915/i915_irq.c | 25 ++++++++++++++++---------
>>> > 1 file changed, 16 insertions(+), 9 deletions(-)
>>> >
>>> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>>> > index de06f293e173..c53d21ae197f 100644
>>> > --- a/drivers/gpu/drm/i915/i915_irq.c
>>> > +++ b/drivers/gpu/drm/i915/i915_irq.c
>>> > @@ -3263,6 +3263,21 @@ static void cherryview_irq_reset(struct drm_i915_private *dev_priv)
>>> > spin_unlock_irq(&dev_priv->irq_lock);
>>> > }
>>> >
>>> > +static void setup_hotplug_inversion(struct drm_i915_private *dev_priv)
>>> > +{
>>> > + u32 invert_bits;
>>> > +
>>> > + if (HAS_PCH_DG1(dev_priv))
>>> > + invert_bits = INVERT_DDIA_HPD |
>>> > + INVERT_DDIB_HPD |
>>> > + INVERT_DDIC_HPD |
>>> > + INVERT_DDID_HPD;
>>>
>>> Nitpick, the indentation will be off compared to automated indentation.
>>
>>What do you mean by automated indentation?
>>
>>>
>>> > + else
>>> > + return;
>>> > +
>>> > + intel_uncore_rmw(&dev_priv->uncore, SOUTH_CHICKEN1, 0, invert_bits);
>>> > +}
>>> > +
>>> > static u32 ibx_hotplug_enables(struct drm_i915_private *i915,
>>> > enum hpd_pin pin)
>>> > {
>>> > @@ -3413,15 +3428,7 @@ static u32 gen11_hotplug_enables(struct drm_i915_private *i915,
>>> >
>>> > static void dg1_hpd_irq_setup(struct drm_i915_private *dev_priv)
>>> > {
>>> > - u32 val;
>>> > -
>>> > - val = intel_uncore_read(&dev_priv->uncore, SOUTH_CHICKEN1);
>>> > - val |= (INVERT_DDIA_HPD |
>>> > - INVERT_DDIB_HPD |
>>> > - INVERT_DDIC_HPD |
>>> > - INVERT_DDID_HPD);
>>> > - intel_uncore_write(&dev_priv->uncore, SOUTH_CHICKEN1, val);
>>> > -
>>> > + setup_hotplug_inversion(dev_priv);
>>>
>>> Since you're already in a platform specific function here, seems a bit
>>> odd to call a new generic function that needs to have another if ladder
>>> platform check. What are we gaining here? The end result is
>>> de-duplicating just one line of intel_uncore_rmw(). I'm not convinced.
>>
>>It is true that the proposed refactor repeats a platform check, but the proposed
>>refactor has its benefits. As more platforms with hotplug inversion needs are
>>added (e.g. MTL), we will have a common place for the logic of hotplug
>>inversion. That arguably makes the code more readable and makes future refactors
>>easier when we need split a function that has become too complex due to platform
>>checks.
>>
>>To make that last point clearer, I am quoting Lucas' (copied here as well)
>>comment (which was what convinced me) from a discussion regarding the advantage
>>of using such a helper:
>>
>> that is what helpers are for, so you don't have to open code it in every
>> platform-fork of the function that needs it. See how the various
>> "Sequences to initialize display" are done in the driver... When we are
>> extending it to a future platform, if the change is small enough we just
>> add e few if/else in the same function. But it doesn't take too long for
>> those functions to become unreadable if there are several branches the
>> code path may take. So then we "fork" the function for a new platform,
>> but reuse the helpers doing the individual steps.
>
> the missing information here is that there are changes in the pipeline
> for platforms that have different bits to be inverted, or none at
> all, with a different register to program. Adding the if/else in this
> function seems unrelated churn.
>
> Another possibility would be to just let the caller handle the if/else
> decision, passing the bits (and possibly register) to invert. The noise
> in xxx_hpd_irq_setup() function may be avoid by
>
> #define INVERT_DII_HPD (INVERT_DDIA_HPD | INVERT_DDIB_HPD | INVERT_DDIC_HPD | INVERT_DDID_HPD)
> #define XXX_INVERT_DII_HPD (...)
>
> Third possibility since the function is already very small is to just go
> ahead and use another _setup() for the next platforms.
IMO if you already have platform specific functions, it can get
confusing if you call generic helpers that again spread out to platform
specific functions, possibly with different conditions than the first
one. We've had a bunch of those where you eventually realize there's
conditions in the helper that never happen.
I'd probably just add small *platform specific* hpd invert
functions. dg1_hpd_invert() and mtl_hpd_invert() etc.
Compare with *_hpd_detection_setup(), and wonder what that would look
like if it were a generic helper. Pretty bad, huh?
Also note how bxt actually handles hpd invert within
bxt_hpd_detection_setup().
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Move hotplug inversion logic into separate helper
2022-09-21 8:21 ` Jani Nikula
@ 2022-09-21 12:05 ` Gustavo Sousa
2022-09-22 8:17 ` Jani Nikula
0 siblings, 1 reply; 14+ messages in thread
From: Gustavo Sousa @ 2022-09-21 12:05 UTC (permalink / raw)
To: Jani Nikula, intel-gfx
On Wed, Sep 21, 2022 at 11:21:00AM +0300, Jani Nikula wrote:
> On Tue, 20 Sep 2022, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> > Hi, Jani.
> >
> > On Tue, Sep 20, 2022 at 10:19:53AM +0300, Jani Nikula wrote:
> >> On Mon, 19 Sep 2022, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> >> > Make the code more readable, which will be more apparent as new
> >> > platforms with different hotplug inversion needs are added.
> >> >
> >> > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> >> > ---
> >> > drivers/gpu/drm/i915/i915_irq.c | 25 ++++++++++++++++---------
> >> > 1 file changed, 16 insertions(+), 9 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> >> > index de06f293e173..c53d21ae197f 100644
> >> > --- a/drivers/gpu/drm/i915/i915_irq.c
> >> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> >> > @@ -3263,6 +3263,21 @@ static void cherryview_irq_reset(struct drm_i915_private *dev_priv)
> >> > spin_unlock_irq(&dev_priv->irq_lock);
> >> > }
> >> >
> >> > +static void setup_hotplug_inversion(struct drm_i915_private *dev_priv)
> >> > +{
> >> > + u32 invert_bits;
> >> > +
> >> > + if (HAS_PCH_DG1(dev_priv))
> >> > + invert_bits = INVERT_DDIA_HPD |
> >> > + INVERT_DDIB_HPD |
> >> > + INVERT_DDIC_HPD |
> >> > + INVERT_DDID_HPD;
> >>
> >> Nitpick, the indentation will be off compared to automated indentation.
> >
> > What do you mean by automated indentation?
>
> For example, hit TAB on the lines using a smart enough editor, which has
> been configured to follow kernel coding style. ;)
>
I'm not sure I completely understand the issue. Could you provide an example of
such a configuration?
Is the problem here the spaces after the tabs to align each INVERT_DDI*? Would
you suggest I just use tabs and do not align them to the first one?
E.g.:
invert_bits = INVERT_DDIA_HPD |
INVERT_DDIB_HPD |
INVERT_DDIC_HPD |
INVERT_DDID_HPD;
Another one:
invert_bits = INVERT_DDIA_HPD |
INVERT_DDIB_HPD |
INVERT_DDIC_HPD |
INVERT_DDID_HPD;
Note: I'm using 8 spaces for instead tabs in the above examples for proper
visual, but they would be tab characters in the source.
--
Gustavo Sousa
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Move hotplug inversion logic into separate helper
2022-09-21 12:05 ` Gustavo Sousa
@ 2022-09-22 8:17 ` Jani Nikula
2022-09-22 10:53 ` Gustavo Sousa
0 siblings, 1 reply; 14+ messages in thread
From: Jani Nikula @ 2022-09-22 8:17 UTC (permalink / raw)
To: Gustavo Sousa, intel-gfx
On Wed, 21 Sep 2022, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> On Wed, Sep 21, 2022 at 11:21:00AM +0300, Jani Nikula wrote:
>> On Tue, 20 Sep 2022, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
>> > Hi, Jani.
>> >
>> > On Tue, Sep 20, 2022 at 10:19:53AM +0300, Jani Nikula wrote:
>> >> On Mon, 19 Sep 2022, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
>> >> > Make the code more readable, which will be more apparent as new
>> >> > platforms with different hotplug inversion needs are added.
>> >> >
>> >> > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>> >> > ---
>> >> > drivers/gpu/drm/i915/i915_irq.c | 25 ++++++++++++++++---------
>> >> > 1 file changed, 16 insertions(+), 9 deletions(-)
>> >> >
>> >> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> >> > index de06f293e173..c53d21ae197f 100644
>> >> > --- a/drivers/gpu/drm/i915/i915_irq.c
>> >> > +++ b/drivers/gpu/drm/i915/i915_irq.c
>> >> > @@ -3263,6 +3263,21 @@ static void cherryview_irq_reset(struct drm_i915_private *dev_priv)
>> >> > spin_unlock_irq(&dev_priv->irq_lock);
>> >> > }
>> >> >
>> >> > +static void setup_hotplug_inversion(struct drm_i915_private *dev_priv)
>> >> > +{
>> >> > + u32 invert_bits;
>> >> > +
>> >> > + if (HAS_PCH_DG1(dev_priv))
>> >> > + invert_bits = INVERT_DDIA_HPD |
>> >> > + INVERT_DDIB_HPD |
>> >> > + INVERT_DDIC_HPD |
>> >> > + INVERT_DDID_HPD;
>> >>
>> >> Nitpick, the indentation will be off compared to automated indentation.
>> >
>> > What do you mean by automated indentation?
>>
>> For example, hit TAB on the lines using a smart enough editor, which has
>> been configured to follow kernel coding style. ;)
>>
>
> I'm not sure I completely understand the issue. Could you provide an example of
> such a configuration?
I never indent anything manually. If I hit TAB in emacs (or run M-x
indent-region), it'll indent the current line (or region) using the
built-in "linux" style [1].
[1] https://www.gnu.org/software/emacs/manual/html_node/ccmode/Built_002din-Styles.html
> Is the problem here the spaces after the tabs to align each INVERT_DDI*? Would
> you suggest I just use tabs and do not align them to the first one?
If a parenthesized element in round brackets is split to multiple lines,
the indent is usually tabs first and then spaces to align everything
after the open parenthesis. If there are no parenthesis, the indent is
just tabs.
Guess why the original dg1_hpd_irq_setup() has round brackets in the
statement. ;) Though not everyone likes superfluous parenthesis for
style.
> E.g.:
>
> invert_bits = INVERT_DDIA_HPD |
> INVERT_DDIB_HPD |
> INVERT_DDIC_HPD |
> INVERT_DDID_HPD;
>
> Another one:
>
> invert_bits = INVERT_DDIA_HPD |
> INVERT_DDIB_HPD |
> INVERT_DDIC_HPD |
> INVERT_DDID_HPD;
Your patch applied, re-indented, and tabs converted to spaces for visual
here:
if (HAS_PCH_DG1(dev_priv))
invert_bits = INVERT_DDIA_HPD |
INVERT_DDIB_HPD |
INVERT_DDIC_HPD |
INVERT_DDID_HPD;
Anyway, indentation isn't a very fruitful conversation in itself. The
original nitpick was an off the cuff remark. I'm mostly trying to
suggest to use tools that will do indentation and other things for you,
so you don't have to worry about getting it right.
BR,
Jani.
>
>
> Note: I'm using 8 spaces for instead tabs in the above examples for proper
> visual, but they would be tab characters in the source.
>
> --
> Gustavo Sousa
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Move hotplug inversion logic into separate helper
2022-09-22 8:17 ` Jani Nikula
@ 2022-09-22 10:53 ` Gustavo Sousa
0 siblings, 0 replies; 14+ messages in thread
From: Gustavo Sousa @ 2022-09-22 10:53 UTC (permalink / raw)
To: Jani Nikula, intel-gfx
On Thu, Sep 22, 2022 at 11:17:01AM +0300, Jani Nikula wrote:
> On Wed, 21 Sep 2022, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> > On Wed, Sep 21, 2022 at 11:21:00AM +0300, Jani Nikula wrote:
> >> On Tue, 20 Sep 2022, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> >> > Hi, Jani.
> >> >
> >> > On Tue, Sep 20, 2022 at 10:19:53AM +0300, Jani Nikula wrote:
> >> >> On Mon, 19 Sep 2022, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> >> >> > Make the code more readable, which will be more apparent as new
> >> >> > platforms with different hotplug inversion needs are added.
> >> >> >
> >> >> > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> >> >> > ---
> >> >> > drivers/gpu/drm/i915/i915_irq.c | 25 ++++++++++++++++---------
> >> >> > 1 file changed, 16 insertions(+), 9 deletions(-)
> >> >> >
> >> >> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> >> >> > index de06f293e173..c53d21ae197f 100644
> >> >> > --- a/drivers/gpu/drm/i915/i915_irq.c
> >> >> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> >> >> > @@ -3263,6 +3263,21 @@ static void cherryview_irq_reset(struct drm_i915_private *dev_priv)
> >> >> > spin_unlock_irq(&dev_priv->irq_lock);
> >> >> > }
> >> >> >
> >> >> > +static void setup_hotplug_inversion(struct drm_i915_private *dev_priv)
> >> >> > +{
> >> >> > + u32 invert_bits;
> >> >> > +
> >> >> > + if (HAS_PCH_DG1(dev_priv))
> >> >> > + invert_bits = INVERT_DDIA_HPD |
> >> >> > + INVERT_DDIB_HPD |
> >> >> > + INVERT_DDIC_HPD |
> >> >> > + INVERT_DDID_HPD;
Just checked the source and it seems I'm the only oddball aligning stuff like
this :-)
> >> >>
> >> >> Nitpick, the indentation will be off compared to automated indentation.
> >> >
> >> > What do you mean by automated indentation?
> >>
> >> For example, hit TAB on the lines using a smart enough editor, which has
> >> been configured to follow kernel coding style. ;)
> >>
> >
> > I'm not sure I completely understand the issue. Could you provide an example of
> > such a configuration?
>
> I never indent anything manually. If I hit TAB in emacs (or run M-x
> indent-region), it'll indent the current line (or region) using the
> built-in "linux" style [1].
>
> [1] https://www.gnu.org/software/emacs/manual/html_node/ccmode/Built_002din-Styles.html
>
> > Is the problem here the spaces after the tabs to align each INVERT_DDI*? Would
> > you suggest I just use tabs and do not align them to the first one?
>
> If a parenthesized element in round brackets is split to multiple lines,
> the indent is usually tabs first and then spaces to align everything
> after the open parenthesis. If there are no parenthesis, the indent is
> just tabs.
>
> Guess why the original dg1_hpd_irq_setup() has round brackets in the
> statement. ;) Though not everyone likes superfluous parenthesis for
> style.
>
> > E.g.:
> >
> > invert_bits = INVERT_DDIA_HPD |
> > INVERT_DDIB_HPD |
> > INVERT_DDIC_HPD |
> > INVERT_DDID_HPD;
> >
> > Another one:
> >
> > invert_bits = INVERT_DDIA_HPD |
> > INVERT_DDIB_HPD |
> > INVERT_DDIC_HPD |
> > INVERT_DDID_HPD;
>
> Your patch applied, re-indented, and tabs converted to spaces for visual
> here:
>
> if (HAS_PCH_DG1(dev_priv))
> invert_bits = INVERT_DDIA_HPD |
> INVERT_DDIB_HPD |
> INVERT_DDIC_HPD |
> INVERT_DDID_HPD;
>
> Anyway, indentation isn't a very fruitful conversation in itself. The
> original nitpick was an off the cuff remark. I'm mostly trying to
> suggest to use tools that will do indentation and other things for you,
> so you don't have to worry about getting it right.
>
Thank you for the tips! I find them very helpful. I'll make sure to use proper
style when sending a new version.
--
Gustavo Sousa
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-09-22 10:52 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-19 14:56 [Intel-gfx] [PATCH] drm/i915: Move hotplug inversion logic into separate helper Gustavo Sousa
2022-09-19 22:58 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
2022-09-20 6:47 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2022-09-20 7:01 ` [Intel-gfx] [PATCH] " Lucas De Marchi
2022-09-20 16:27 ` Gustavo Sousa
2022-09-21 9:59 ` Jani Nikula
2022-09-20 7:19 ` Jani Nikula
2022-09-20 17:04 ` Gustavo Sousa
2022-09-20 17:56 ` Lucas De Marchi
2022-09-21 10:23 ` Jani Nikula
2022-09-21 8:21 ` Jani Nikula
2022-09-21 12:05 ` Gustavo Sousa
2022-09-22 8:17 ` Jani Nikula
2022-09-22 10:53 ` Gustavo Sousa
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.