* [Intel-gfx] [PATCH] drm/i915/display: pre-initialize some values in probe_gmdid_display()
@ 2023-06-01 9:03 Luca Coelho
2023-06-01 16:04 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Luca Coelho @ 2023-06-01 9:03 UTC (permalink / raw)
To: intel-gfx
When intel_display_device_probe() (and, subsequently,
probe_gmdid_display()) returns, the caller expects ver, rel and step
to be initialized. Since there's no way to check that there was a
failure and no_display was returned without some further refactoring,
pre-initiliaze all these values to zero to keep it simple and safe.
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
drivers/gpu/drm/i915/display/intel_display_device.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c b/drivers/gpu/drm/i915/display/intel_display_device.c
index 464df1764a86..fb6354e9e704 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.c
+++ b/drivers/gpu/drm/i915/display/intel_display_device.c
@@ -731,6 +731,15 @@ probe_gmdid_display(struct drm_i915_private *i915, u16 *ver, u16 *rel, u16 *step
u32 val;
int i;
+ /* The caller expects to ver, rel and step to be initialized
+ * here, and there's no good way to check when there was a
+ * failure and no_display was returned. So initialize all these
+ * values here zero, to be sure.
+ */
+ *ver = 0;
+ *rel = 0;
+ *step = 0;
+
addr = pci_iomap_range(pdev, 0, i915_mmio_reg_offset(GMD_ID_DISPLAY), sizeof(u32));
if (!addr) {
drm_err(&i915->drm, "Cannot map MMIO BAR to read display GMD_ID\n");
--
2.39.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/display: pre-initialize some values in probe_gmdid_display()
2023-06-01 9:03 [Intel-gfx] [PATCH] drm/i915/display: pre-initialize some values in probe_gmdid_display() Luca Coelho
@ 2023-06-01 16:04 ` Patchwork
2023-06-03 14:43 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2023-06-20 10:30 ` [Intel-gfx] [PATCH] " Kandpal, Suraj
2 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2023-06-01 16:04 UTC (permalink / raw)
To: Luca Coelho; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 10818 bytes --]
== Series Details ==
Series: drm/i915/display: pre-initialize some values in probe_gmdid_display()
URL : https://patchwork.freedesktop.org/series/118690/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_13215 -> Patchwork_118690v1
====================================================
Summary
-------
**WARNING**
Minor unknown changes coming with Patchwork_118690v1 need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_118690v1, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/index.html
Participating hosts (37 -> 38)
------------------------------
Additional (1): bat-dg1-5
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_118690v1:
### IGT changes ###
#### Warnings ####
* igt@kms_psr@primary_mmap_gtt:
- bat-rplp-1: [SKIP][1] ([i915#1072]) -> [ABORT][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/bat-rplp-1/igt@kms_psr@primary_mmap_gtt.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/bat-rplp-1/igt@kms_psr@primary_mmap_gtt.html
Known issues
------------
Here are the changes found in Patchwork_118690v1 that come from known issues:
### CI changes ###
#### Issues hit ####
* boot:
- fi-kbl-8809g: [PASS][3] -> [FAIL][4] ([i915#8293] / [i915#8298])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/fi-kbl-8809g/boot.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/fi-kbl-8809g/boot.html
### IGT changes ###
#### Issues hit ####
* igt@gem_mmap@basic:
- bat-dg1-5: NOTRUN -> [SKIP][5] ([i915#4083])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/bat-dg1-5/igt@gem_mmap@basic.html
* igt@gem_tiled_fence_blits@basic:
- bat-dg1-5: NOTRUN -> [SKIP][6] ([i915#4077]) +2 similar issues
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/bat-dg1-5/igt@gem_tiled_fence_blits@basic.html
* igt@gem_tiled_pread_basic:
- bat-dg1-5: NOTRUN -> [SKIP][7] ([i915#4079]) +1 similar issue
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/bat-dg1-5/igt@gem_tiled_pread_basic.html
* igt@i915_module_load@load:
- fi-kbl-soraka: [PASS][8] -> [DMESG-WARN][9] ([i915#1982])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/fi-kbl-soraka/igt@i915_module_load@load.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/fi-kbl-soraka/igt@i915_module_load@load.html
* igt@i915_pm_backlight@basic-brightness:
- bat-dg1-5: NOTRUN -> [SKIP][10] ([i915#7561])
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/bat-dg1-5/igt@i915_pm_backlight@basic-brightness.html
* igt@i915_pm_rps@basic-api:
- bat-dg1-5: NOTRUN -> [SKIP][11] ([i915#6621])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/bat-dg1-5/igt@i915_pm_rps@basic-api.html
* igt@i915_selftest@live@gt_heartbeat:
- fi-apl-guc: [PASS][12] -> [DMESG-FAIL][13] ([i915#5334])
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
* igt@kms_addfb_basic@basic-x-tiled-legacy:
- bat-dg1-5: NOTRUN -> [SKIP][14] ([i915#4212]) +7 similar issues
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/bat-dg1-5/igt@kms_addfb_basic@basic-x-tiled-legacy.html
* igt@kms_addfb_basic@basic-y-tiled-legacy:
- bat-dg1-5: NOTRUN -> [SKIP][15] ([i915#4215])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/bat-dg1-5/igt@kms_addfb_basic@basic-y-tiled-legacy.html
* igt@kms_chamelium_hpd@vga-hpd-fast:
- bat-dg1-5: NOTRUN -> [SKIP][16] ([i915#7828]) +8 similar issues
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/bat-dg1-5/igt@kms_chamelium_hpd@vga-hpd-fast.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
- bat-dg1-5: NOTRUN -> [SKIP][17] ([i915#4103] / [i915#4213]) +1 similar issue
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/bat-dg1-5/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
* igt@kms_force_connector_basic@force-load-detect:
- bat-dg1-5: NOTRUN -> [SKIP][18] ([fdo#109285])
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/bat-dg1-5/igt@kms_force_connector_basic@force-load-detect.html
* igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-d-dp-1:
- bat-dg2-8: [PASS][19] -> [FAIL][20] ([i915#7932])
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-d-dp-1.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-d-dp-1.html
* igt@kms_psr@sprite_plane_onoff:
- bat-dg1-5: NOTRUN -> [SKIP][21] ([i915#1072] / [i915#4078]) +3 similar issues
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/bat-dg1-5/igt@kms_psr@sprite_plane_onoff.html
* igt@kms_setmode@basic-clone-single-crtc:
- bat-dg1-5: NOTRUN -> [SKIP][22] ([i915#3555] / [i915#4579])
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/bat-dg1-5/igt@kms_setmode@basic-clone-single-crtc.html
* igt@prime_vgem@basic-fence-read:
- bat-dg1-5: NOTRUN -> [SKIP][23] ([i915#3708]) +3 similar issues
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/bat-dg1-5/igt@prime_vgem@basic-fence-read.html
* igt@prime_vgem@basic-gtt:
- bat-dg1-5: NOTRUN -> [SKIP][24] ([i915#3708] / [i915#4077]) +1 similar issue
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/bat-dg1-5/igt@prime_vgem@basic-gtt.html
#### Possible fixes ####
* igt@core_auth@basic-auth:
- {bat-adlp-11}: [ABORT][25] ([i915#8011]) -> [PASS][26]
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/bat-adlp-11/igt@core_auth@basic-auth.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/bat-adlp-11/igt@core_auth@basic-auth.html
* igt@i915_module_load@load:
- {bat-adlp-11}: [DMESG-WARN][27] ([i915#4423]) -> [PASS][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/bat-adlp-11/igt@i915_module_load@load.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/bat-adlp-11/igt@i915_module_load@load.html
* igt@i915_selftest@live@gt_mocs:
- bat-rpls-2: [DMESG-FAIL][29] ([i915#7059]) -> [PASS][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/bat-rpls-2/igt@i915_selftest@live@gt_mocs.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/bat-rpls-2/igt@i915_selftest@live@gt_mocs.html
* igt@i915_selftest@live@requests:
- {bat-mtlp-8}: [DMESG-FAIL][31] ([i915#8497]) -> [PASS][32]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/bat-mtlp-8/igt@i915_selftest@live@requests.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/bat-mtlp-8/igt@i915_selftest@live@requests.html
* igt@i915_selftest@live@slpc:
- bat-rpls-2: [DMESG-WARN][33] ([i915#6367]) -> [PASS][34]
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/bat-rpls-2/igt@i915_selftest@live@slpc.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/bat-rpls-2/igt@i915_selftest@live@slpc.html
- bat-rpls-1: [DMESG-WARN][35] ([i915#6367]) -> [PASS][36]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/bat-rpls-1/igt@i915_selftest@live@slpc.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/bat-rpls-1/igt@i915_selftest@live@slpc.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
[i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
[i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
[i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
[i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
[i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
[i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
[i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
[i915#4215]: https://gitlab.freedesktop.org/drm/intel/issues/4215
[i915#4423]: https://gitlab.freedesktop.org/drm/intel/issues/4423
[i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
[i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
[i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
[i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
[i915#7059]: https://gitlab.freedesktop.org/drm/intel/issues/7059
[i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456
[i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
[i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
[i915#7932]: https://gitlab.freedesktop.org/drm/intel/issues/7932
[i915#8011]: https://gitlab.freedesktop.org/drm/intel/issues/8011
[i915#8293]: https://gitlab.freedesktop.org/drm/intel/issues/8293
[i915#8298]: https://gitlab.freedesktop.org/drm/intel/issues/8298
[i915#8497]: https://gitlab.freedesktop.org/drm/intel/issues/8497
Build changes
-------------
* Linux: CI_DRM_13215 -> Patchwork_118690v1
CI-20190529: 20190529
CI_DRM_13215: 89016ff511ceed03fbfabdb1c4700bb2c87c0b88 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7317: c902b72df45aa49faa38205bc5be3c748d33a3e0 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_118690v1: 89016ff511ceed03fbfabdb1c4700bb2c87c0b88 @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
b88823954da6 drm/i915/display: pre-initialize some values in probe_gmdid_display()
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/index.html
[-- Attachment #2: Type: text/html, Size: 12185 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/display: pre-initialize some values in probe_gmdid_display()
2023-06-01 9:03 [Intel-gfx] [PATCH] drm/i915/display: pre-initialize some values in probe_gmdid_display() Luca Coelho
2023-06-01 16:04 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
@ 2023-06-03 14:43 ` Patchwork
2023-06-20 10:30 ` [Intel-gfx] [PATCH] " Kandpal, Suraj
2 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2023-06-03 14:43 UTC (permalink / raw)
To: Luca Coelho; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 15696 bytes --]
== Series Details ==
Series: drm/i915/display: pre-initialize some values in probe_gmdid_display()
URL : https://patchwork.freedesktop.org/series/118690/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_13215_full -> Patchwork_118690v1_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
Participating hosts (8 -> 7)
------------------------------
Missing (1): shard-rkl0
Known issues
------------
Here are the changes found in Patchwork_118690v1_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_fair@basic-pace-share@rcs0:
- shard-glk: NOTRUN -> [FAIL][1] ([i915#2842])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-glk4/igt@gem_exec_fair@basic-pace-share@rcs0.html
* igt@gem_lmem_swapping@heavy-verify-multi:
- shard-apl: NOTRUN -> [SKIP][2] ([fdo#109271] / [i915#4613])
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-apl7/igt@gem_lmem_swapping@heavy-verify-multi.html
* igt@gen9_exec_parse@allowed-single:
- shard-apl: [PASS][3] -> [ABORT][4] ([i915#5566])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/shard-apl3/igt@gen9_exec_parse@allowed-single.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-apl2/igt@gen9_exec_parse@allowed-single.html
* igt@kms_ccs@pipe-a-bad-aux-stride-y_tiled_gen12_mc_ccs:
- shard-apl: NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#3886])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-apl3/igt@kms_ccs@pipe-a-bad-aux-stride-y_tiled_gen12_mc_ccs.html
* igt@kms_ccs@pipe-c-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs_cc:
- shard-glk: NOTRUN -> [SKIP][6] ([fdo#109271] / [i915#3886]) +2 similar issues
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-glk4/igt@kms_ccs@pipe-c-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs_cc.html
* igt@kms_content_protection@atomic@pipe-a-dp-1:
- shard-apl: NOTRUN -> [TIMEOUT][7] ([i915#7173])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-apl7/igt@kms_content_protection@atomic@pipe-a-dp-1.html
* igt@kms_content_protection@uevent:
- shard-glk: NOTRUN -> [SKIP][8] ([fdo#109271] / [i915#4579]) +3 similar issues
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-glk4/igt@kms_content_protection@uevent.html
* igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
- shard-glk: [PASS][9] -> [FAIL][10] ([i915#2346])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/shard-glk9/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-glk5/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
* igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a1:
- shard-glk: [PASS][11] -> [FAIL][12] ([i915#2122])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/shard-glk9/igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a1.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-glk3/igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a1.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-onoff:
- shard-apl: NOTRUN -> [SKIP][13] ([fdo#109271]) +34 similar issues
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-apl3/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-onoff.html
* igt@kms_frontbuffer_tracking@fbcpsr-tiling-linear:
- shard-glk: NOTRUN -> [SKIP][14] ([fdo#109271]) +44 similar issues
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-glk4/igt@kms_frontbuffer_tracking@fbcpsr-tiling-linear.html
* igt@kms_plane_scaling@invalid-num-scalers@pipe-c-dp-1-invalid-num-scalers:
- shard-apl: NOTRUN -> [SKIP][15] ([fdo#109271] / [i915#4579])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-apl7/igt@kms_plane_scaling@invalid-num-scalers@pipe-c-dp-1-invalid-num-scalers.html
* igt@kms_plane_scaling@plane-downscale-with-modifiers-factor-0-5@pipe-b-vga-1:
- shard-snb: NOTRUN -> [SKIP][16] ([fdo#109271] / [i915#4579]) +11 similar issues
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-snb6/igt@kms_plane_scaling@plane-downscale-with-modifiers-factor-0-5@pipe-b-vga-1.html
* igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-25@pipe-a-vga-1:
- shard-snb: NOTRUN -> [SKIP][17] ([fdo#109271]) +14 similar issues
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-snb6/igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-25@pipe-a-vga-1.html
* igt@kms_psr2_sf@cursor-plane-move-continuous-exceed-fully-sf:
- shard-glk: NOTRUN -> [SKIP][18] ([fdo#109271] / [i915#658]) +1 similar issue
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-glk4/igt@kms_psr2_sf@cursor-plane-move-continuous-exceed-fully-sf.html
* igt@kms_writeback@writeback-fb-id:
- shard-glk: NOTRUN -> [SKIP][19] ([fdo#109271] / [i915#2437])
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-glk4/igt@kms_writeback@writeback-fb-id.html
* igt@kms_writeback@writeback-pixel-formats:
- shard-apl: NOTRUN -> [SKIP][20] ([fdo#109271] / [i915#2437])
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-apl3/igt@kms_writeback@writeback-pixel-formats.html
#### Possible fixes ####
* igt@core_hotunplug@unbind-rebind:
- shard-snb: [ABORT][21] ([i915#4528] / [i915#8213]) -> [PASS][22]
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/shard-snb5/igt@core_hotunplug@unbind-rebind.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-snb1/igt@core_hotunplug@unbind-rebind.html
* igt@gem_ctx_exec@basic-nohangcheck:
- {shard-tglu}: [FAIL][23] ([i915#6268]) -> [PASS][24]
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/shard-tglu-4/igt@gem_ctx_exec@basic-nohangcheck.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-tglu-10/igt@gem_ctx_exec@basic-nohangcheck.html
* igt@gem_eio@unwedge-stress:
- {shard-dg1}: [FAIL][25] ([i915#5784]) -> [PASS][26] +1 similar issue
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/shard-dg1-17/igt@gem_eio@unwedge-stress.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-dg1-15/igt@gem_eio@unwedge-stress.html
* igt@gem_exec_fair@basic-deadline:
- shard-glk: [FAIL][27] ([i915#2846]) -> [PASS][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/shard-glk5/igt@gem_exec_fair@basic-deadline.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-glk2/igt@gem_exec_fair@basic-deadline.html
* igt@gem_lmem_swapping@smem-oom@lmem0:
- {shard-dg1}: [DMESG-WARN][29] ([i915#4936] / [i915#5493]) -> [PASS][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/shard-dg1-16/igt@gem_lmem_swapping@smem-oom@lmem0.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-dg1-17/igt@gem_lmem_swapping@smem-oom@lmem0.html
* igt@gem_workarounds@suspend-resume-context:
- shard-apl: [ABORT][31] ([i915#180]) -> [PASS][32]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/shard-apl1/igt@gem_workarounds@suspend-resume-context.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-apl3/igt@gem_workarounds@suspend-resume-context.html
* igt@i915_pm_rc6_residency@rc6-idle@vecs0:
- {shard-dg1}: [FAIL][33] ([i915#3591]) -> [PASS][34]
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/shard-dg1-15/igt@i915_pm_rc6_residency@rc6-idle@vecs0.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-dg1-19/igt@i915_pm_rc6_residency@rc6-idle@vecs0.html
* igt@i915_pm_rpm@modeset-lpsp:
- {shard-rkl}: [SKIP][35] ([i915#1397]) -> [PASS][36]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/shard-rkl-4/igt@i915_pm_rpm@modeset-lpsp.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-rkl-7/igt@i915_pm_rpm@modeset-lpsp.html
* igt@i915_pm_rpm@modeset-non-lpsp-stress-no-wait:
- {shard-dg1}: [SKIP][37] ([i915#1397]) -> [PASS][38]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/shard-dg1-19/igt@i915_pm_rpm@modeset-non-lpsp-stress-no-wait.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-dg1-18/igt@i915_pm_rpm@modeset-non-lpsp-stress-no-wait.html
* igt@i915_selftest@live@gt_heartbeat:
- shard-glk: [DMESG-FAIL][39] ([i915#5334]) -> [PASS][40]
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/shard-glk2/igt@i915_selftest@live@gt_heartbeat.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-glk7/igt@i915_selftest@live@gt_heartbeat.html
* igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-hflip-async-flip:
- {shard-rkl}: [FAIL][41] ([i915#3743]) -> [PASS][42]
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/shard-rkl-7/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-hflip-async-flip.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-rkl-4/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-hflip-async-flip.html
* igt@kms_cursor_legacy@forked-bo@pipe-b:
- {shard-dg1}: [INCOMPLETE][43] ([i915#8011] / [i915#8347]) -> [PASS][44]
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/shard-dg1-19/igt@kms_cursor_legacy@forked-bo@pipe-b.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-dg1-18/igt@kms_cursor_legacy@forked-bo@pipe-b.html
* igt@kms_cursor_legacy@single-move@pipe-b:
- {shard-rkl}: [INCOMPLETE][45] ([i915#8011]) -> [PASS][46]
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/shard-rkl-7/igt@kms_cursor_legacy@single-move@pipe-b.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-rkl-4/igt@kms_cursor_legacy@single-move@pipe-b.html
* igt@perf_pmu@busy@bcs0:
- shard-apl: [DMESG-WARN][47] -> [PASS][48]
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/shard-apl3/igt@perf_pmu@busy@bcs0.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-apl7/igt@perf_pmu@busy@bcs0.html
* igt@perf_pmu@busy@rcs0:
- shard-apl: [ABORT][49] -> [PASS][50]
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13215/shard-apl3/igt@perf_pmu@busy@rcs0.html
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118690v1/shard-apl7/igt@perf_pmu@busy@rcs0.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#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
[fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
[fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
[fdo#110723]: https://bugs.freedesktop.org/show_bug.cgi?id=110723
[fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
[fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
[fdo#112283]: https://bugs.freedesktop.org/show_bug.cgi?id=112283
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
[i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
[i915#1825]: https://gitlab.freedesktop.org/drm/intel/issues/1825
[i915#1839]: https://gitlab.freedesktop.org/drm/intel/issues/1839
[i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
[i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
[i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
[i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
[i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
[i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
[i915#2846]: https://gitlab.freedesktop.org/drm/intel/issues/2846
[i915#3023]: https://gitlab.freedesktop.org/drm/intel/issues/3023
[i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
[i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
[i915#3591]: https://gitlab.freedesktop.org/drm/intel/issues/3591
[i915#3743]: https://gitlab.freedesktop.org/drm/intel/issues/3743
[i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
[i915#3955]: https://gitlab.freedesktop.org/drm/intel/issues/3955
[i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
[i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
[i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
[i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528
[i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4816]: https://gitlab.freedesktop.org/drm/intel/issues/4816
[i915#4936]: https://gitlab.freedesktop.org/drm/intel/issues/4936
[i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
[i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
[i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
[i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
[i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
[i915#5493]: https://gitlab.freedesktop.org/drm/intel/issues/5493
[i915#5566]: https://gitlab.freedesktop.org/drm/intel/issues/5566
[i915#5784]: https://gitlab.freedesktop.org/drm/intel/issues/5784
[i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
[i915#6268]: https://gitlab.freedesktop.org/drm/intel/issues/6268
[i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
[i915#6953]: https://gitlab.freedesktop.org/drm/intel/issues/6953
[i915#7173]: https://gitlab.freedesktop.org/drm/intel/issues/7173
[i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
[i915#7742]: https://gitlab.freedesktop.org/drm/intel/issues/7742
[i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
[i915#7975]: https://gitlab.freedesktop.org/drm/intel/issues/7975
[i915#8011]: https://gitlab.freedesktop.org/drm/intel/issues/8011
[i915#8213]: https://gitlab.freedesktop.org/drm/intel/issues/8213
[i915#8228]: https://gitlab.freedesktop.org/drm/intel/issues/8228
[i915#8247]: https://gitlab.freedesktop.org/drm/intel/issues/8247
[i915#8311]: https://gitlab.freedesktop.org/drm/intel/issues/8311
[i915#8347]: https://gitlab.freedesktop.org/drm/intel/issues/8347
Build changes
-------------
* Linux: CI_DRM_13215 -> Patchwork_118690v1
CI-20190529: 20190529
CI_DRM_13215: 89016ff511ceed03fbfabdb1c4700bb2c87c0b88 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7317: c902b72df45aa49faa38205bc5be3c748d33a3e0 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_118690v1: 89016ff511ceed03fbfabdb1c4700bb2c87c0b88 @ 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_118690v1/index.html
[-- Attachment #2: Type: text/html, Size: 15980 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/display: pre-initialize some values in probe_gmdid_display()
2023-06-01 9:03 [Intel-gfx] [PATCH] drm/i915/display: pre-initialize some values in probe_gmdid_display() Luca Coelho
2023-06-01 16:04 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
2023-06-03 14:43 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
@ 2023-06-20 10:30 ` Kandpal, Suraj
2023-06-22 9:50 ` Coelho, Luciano
2 siblings, 1 reply; 8+ messages in thread
From: Kandpal, Suraj @ 2023-06-20 10:30 UTC (permalink / raw)
To: Coelho, Luciano, intel-gfx
> When intel_display_device_probe() (and, subsequently,
> probe_gmdid_display()) returns, the caller expects ver, rel and step to be
> initialized. Since there's no way to check that there was a failure and
> no_display was returned without some further refactoring, pre-initiliaze all
> these values to zero to keep it simple and safe.
>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
Looks okay to me just a small suggestion/question below.
> ---
> drivers/gpu/drm/i915/display/intel_display_device.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c
> b/drivers/gpu/drm/i915/display/intel_display_device.c
> index 464df1764a86..fb6354e9e704 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.c
> @@ -731,6 +731,15 @@ probe_gmdid_display(struct drm_i915_private
> *i915, u16 *ver, u16 *rel, u16 *step
> u32 val;
> int i;
>
> + /* The caller expects to ver, rel and step to be initialized
> + * here, and there's no good way to check when there was a
> + * failure and no_display was returned. So initialize all these
> + * values here zero, to be sure.
> + */
> + *ver = 0;
> + *rel = 0;
> + *step = 0;
> +
From what I can see this is only called from intel_display_device_probe() which is in turn
called from intel_device_info_driver_create() where the above variables are defined maybe
we initialize these values there itself.
Regards,
Suraj Kandpal
> addr = pci_iomap_range(pdev, 0,
> i915_mmio_reg_offset(GMD_ID_DISPLAY), sizeof(u32));
> if (!addr) {
> drm_err(&i915->drm, "Cannot map MMIO BAR to read
> display GMD_ID\n");
> --
> 2.39.2
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/display: pre-initialize some values in probe_gmdid_display()
2023-06-20 10:30 ` [Intel-gfx] [PATCH] " Kandpal, Suraj
@ 2023-06-22 9:50 ` Coelho, Luciano
2023-06-22 10:08 ` Kandpal, Suraj
0 siblings, 1 reply; 8+ messages in thread
From: Coelho, Luciano @ 2023-06-22 9:50 UTC (permalink / raw)
To: Kandpal, Suraj, intel-gfx
On Tue, 2023-06-20 at 10:30 +0000, Kandpal, Suraj wrote:
> > When intel_display_device_probe() (and, subsequently,
> > probe_gmdid_display()) returns, the caller expects ver, rel and
> > step to be
> > initialized. Since there's no way to check that there was a
> > failure and
> > no_display was returned without some further refactoring, pre-
> > initiliaze all
> > these values to zero to keep it simple and safe.
> >
> > Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
>
> Looks okay to me just a small suggestion/question below.
>
> > ---
> > drivers/gpu/drm/i915/display/intel_display_device.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c
> > b/drivers/gpu/drm/i915/display/intel_display_device.c
> > index 464df1764a86..fb6354e9e704 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_device.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_device.c
> > @@ -731,6 +731,15 @@ probe_gmdid_display(struct drm_i915_private
> > *i915, u16 *ver, u16 *rel, u16 *step
> > u32 val;
> > int i;
> >
> > + /* The caller expects to ver, rel and step to be
> > initialized
> > + * here, and there's no good way to check when there was a
> > + * failure and no_display was returned. So initialize all
> > these
> > + * values here zero, to be sure.
> > + */
> > + *ver = 0;
> > + *rel = 0;
> > + *step = 0;
> > +
>
> From what I can see this is only called from
> intel_display_device_probe() which is in turn
> called from intel_device_info_driver_create() where the above
> variables are defined maybe
> we initialize these values there itself.
Thanks for the review!
I thought about initializing the variables on the caller side, but
reckoned that it would be more intuitive to initialize them in the
probe_gmdid_display() function instead, because the caller expects
those values to be set in successful cases and there's no way for it to
know whether there was a failure or not (because we return a pointer to
local no_display structure that the caller doesn't know about).
Obviously with the current code in the caller, that doesn't make much
difference, but I thought it was cleaner as I did.
But I'm okay to change it and initialize them at the caller, so just
let me know if you want that.
--
Cheers,
Luca.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/display: pre-initialize some values in probe_gmdid_display()
2023-06-22 9:50 ` Coelho, Luciano
@ 2023-06-22 10:08 ` Kandpal, Suraj
2023-06-22 12:09 ` Coelho, Luciano
0 siblings, 1 reply; 8+ messages in thread
From: Kandpal, Suraj @ 2023-06-22 10:08 UTC (permalink / raw)
To: Coelho, Luciano, intel-gfx
> On Tue, 2023-06-20 at 10:30 +0000, Kandpal, Suraj wrote:
> > > When intel_display_device_probe() (and, subsequently,
> > > probe_gmdid_display()) returns, the caller expects ver, rel and step
> > > to be initialized. Since there's no way to check that there was a
> > > failure and no_display was returned without some further
> > > refactoring, pre- initiliaze all these values to zero to keep it
> > > simple and safe.
> > >
> > > Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> >
> > Looks okay to me just a small suggestion/question below.
> >
> > > ---
> > > drivers/gpu/drm/i915/display/intel_display_device.c | 9 +++++++++
> > > 1 file changed, 9 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c
> > > b/drivers/gpu/drm/i915/display/intel_display_device.c
> > > index 464df1764a86..fb6354e9e704 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_device.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_device.c
> > > @@ -731,6 +731,15 @@ probe_gmdid_display(struct drm_i915_private
> > > *i915, u16 *ver, u16 *rel, u16 *step
> > > u32 val;
> > > int i;
> > >
> > > + /* The caller expects to ver, rel and step to be
> > > initialized
> > > + * here, and there's no good way to check when there was a
> > > + * failure and no_display was returned. So initialize all
> > > these
> > > + * values here zero, to be sure.
> > > + */
> > > + *ver = 0;
> > > + *rel = 0;
> > > + *step = 0;
> > > +
> >
> > From what I can see this is only called from
> > intel_display_device_probe() which is in turn called from
> > intel_device_info_driver_create() where the above variables are
> > defined maybe we initialize these values there itself.
>
> Thanks for the review!
>
> I thought about initializing the variables on the caller side, but reckoned that
> it would be more intuitive to initialize them in the
> probe_gmdid_display() function instead, because the caller expects those
> values to be set in successful cases and there's no way for it to know whether
> there was a failure or not (because we return a pointer to local no_display
> structure that the caller doesn't know about).
>
> Obviously with the current code in the caller, that doesn't make much
> difference, but I thought it was cleaner as I did.
>
> But I'm okay to change it and initialize them at the caller, so just let me know
> if you want that.
I don’t think it needs to be changed then and the explanation looks reasonable.
So this LGTM
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
>
> --
> Cheers,
> Luca.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/display: pre-initialize some values in probe_gmdid_display()
2023-06-22 10:08 ` Kandpal, Suraj
@ 2023-06-22 12:09 ` Coelho, Luciano
2023-08-08 13:41 ` Hogander, Jouni
0 siblings, 1 reply; 8+ messages in thread
From: Coelho, Luciano @ 2023-06-22 12:09 UTC (permalink / raw)
To: Kandpal, Suraj, intel-gfx
On Thu, 2023-06-22 at 10:08 +0000, Kandpal, Suraj wrote:
> > On Tue, 2023-06-20 at 10:30 +0000, Kandpal, Suraj wrote:
> > > > When intel_display_device_probe() (and, subsequently,
> > > > probe_gmdid_display()) returns, the caller expects ver, rel and
> > > > step
> > > > to be initialized. Since there's no way to check that there
> > > > was a
> > > > failure and no_display was returned without some further
> > > > refactoring, pre- initiliaze all these values to zero to keep
> > > > it
> > > > simple and safe.
> > > >
> > > > Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> > >
> > > Looks okay to me just a small suggestion/question below.
> > >
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_display_device.c | 9
> > > > +++++++++
> > > > 1 file changed, 9 insertions(+)
> > > >
> > > > diff --git
> > > > a/drivers/gpu/drm/i915/display/intel_display_device.c
> > > > b/drivers/gpu/drm/i915/display/intel_display_device.c
> > > > index 464df1764a86..fb6354e9e704 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display_device.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display_device.c
> > > > @@ -731,6 +731,15 @@ probe_gmdid_display(struct
> > > > drm_i915_private
> > > > *i915, u16 *ver, u16 *rel, u16 *step
> > > > u32 val;
> > > > int i;
> > > >
> > > > + /* The caller expects to ver, rel and step to be
> > > > initialized
> > > > + * here, and there's no good way to check when there
> > > > was a
> > > > + * failure and no_display was returned. So initialize
> > > > all
> > > > these
> > > > + * values here zero, to be sure.
> > > > + */
> > > > + *ver = 0;
> > > > + *rel = 0;
> > > > + *step = 0;
> > > > +
> > >
> > > From what I can see this is only called from
> > > intel_display_device_probe() which is in turn called from
> > > intel_device_info_driver_create() where the above variables are
> > > defined maybe we initialize these values there itself.
> >
> > Thanks for the review!
> >
> > I thought about initializing the variables on the caller side, but
> > reckoned that
> > it would be more intuitive to initialize them in the
> > probe_gmdid_display() function instead, because the caller expects
> > those
> > values to be set in successful cases and there's no way for it to
> > know whether
> > there was a failure or not (because we return a pointer to local
> > no_display
> > structure that the caller doesn't know about).
> >
> > Obviously with the current code in the caller, that doesn't make
> > much
> > difference, but I thought it was cleaner as I did.
> >
> > But I'm okay to change it and initialize them at the caller, so
> > just let me know
> > if you want that.
>
> I don’t think it needs to be changed then and the explanation looks
> reasonable.
> So this LGTM
>
> Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
>
Thanks, Suraj! Can someone merge this for me, please?
--
Cheers,
Luca.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/display: pre-initialize some values in probe_gmdid_display()
2023-06-22 12:09 ` Coelho, Luciano
@ 2023-08-08 13:41 ` Hogander, Jouni
0 siblings, 0 replies; 8+ messages in thread
From: Hogander, Jouni @ 2023-08-08 13:41 UTC (permalink / raw)
To: Kandpal, Suraj, intel-gfx, Coelho, Luciano
On Thu, 2023-06-22 at 12:09 +0000, Coelho, Luciano wrote:
> On Thu, 2023-06-22 at 10:08 +0000, Kandpal, Suraj wrote:
> > > On Tue, 2023-06-20 at 10:30 +0000, Kandpal, Suraj wrote:
> > > > > When intel_display_device_probe() (and, subsequently,
> > > > > probe_gmdid_display()) returns, the caller expects ver, rel
> > > > > and
> > > > > step
> > > > > to be initialized. Since there's no way to check that there
> > > > > was a
> > > > > failure and no_display was returned without some further
> > > > > refactoring, pre- initiliaze all these values to zero to keep
> > > > > it
> > > > > simple and safe.
> > > > >
> > > > > Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> > > >
> > > > Looks okay to me just a small suggestion/question below.
> > > >
> > > > > ---
> > > > > drivers/gpu/drm/i915/display/intel_display_device.c | 9
> > > > > +++++++++
> > > > > 1 file changed, 9 insertions(+)
> > > > >
> > > > > diff --git
> > > > > a/drivers/gpu/drm/i915/display/intel_display_device.c
> > > > > b/drivers/gpu/drm/i915/display/intel_display_device.c
> > > > > index 464df1764a86..fb6354e9e704 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_display_device.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_display_device.c
> > > > > @@ -731,6 +731,15 @@ probe_gmdid_display(struct
> > > > > drm_i915_private
> > > > > *i915, u16 *ver, u16 *rel, u16 *step
> > > > > u32 val;
> > > > > int i;
> > > > >
> > > > > + /* The caller expects to ver, rel and step to be
> > > > > initialized
> > > > > + * here, and there's no good way to check when there
> > > > > was a
> > > > > + * failure and no_display was returned. So
> > > > > initialize
> > > > > all
> > > > > these
> > > > > + * values here zero, to be sure.
> > > > > + */
> > > > > + *ver = 0;
> > > > > + *rel = 0;
> > > > > + *step = 0;
> > > > > +
> > > >
> > > > From what I can see this is only called from
> > > > intel_display_device_probe() which is in turn called from
> > > > intel_device_info_driver_create() where the above variables are
> > > > defined maybe we initialize these values there itself.
> > >
> > > Thanks for the review!
> > >
> > > I thought about initializing the variables on the caller side,
> > > but
> > > reckoned that
> > > it would be more intuitive to initialize them in the
> > > probe_gmdid_display() function instead, because the caller
> > > expects
> > > those
> > > values to be set in successful cases and there's no way for it to
> > > know whether
> > > there was a failure or not (because we return a pointer to local
> > > no_display
> > > structure that the caller doesn't know about).
> > >
> > > Obviously with the current code in the caller, that doesn't make
> > > much
> > > difference, but I thought it was cleaner as I did.
> > >
> > > But I'm okay to change it and initialize them at the caller, so
> > > just let me know
> > > if you want that.
> >
> > I don’t think it needs to be changed then and the explanation looks
> > reasonable.
> > So this LGTM
> >
> > Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
> >
>
> Thanks, Suraj! Can someone merge this for me, please?
This is now merged.
BR,
Jouni Högander
>
> --
> Cheers,
> Luca.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-08-08 13:41 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-01 9:03 [Intel-gfx] [PATCH] drm/i915/display: pre-initialize some values in probe_gmdid_display() Luca Coelho
2023-06-01 16:04 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
2023-06-03 14:43 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2023-06-20 10:30 ` [Intel-gfx] [PATCH] " Kandpal, Suraj
2023-06-22 9:50 ` Coelho, Luciano
2023-06-22 10:08 ` Kandpal, Suraj
2023-06-22 12:09 ` Coelho, Luciano
2023-08-08 13:41 ` Hogander, Jouni
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).