* [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@lists.freedesktop.org > 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@lists.freedesktop.org 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@lists.freedesktop.org > 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@lists.freedesktop.org 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@lists.freedesktop.org, 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