All of lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND] drm/i915: stop conflating HAS_DISPLAY() and disabled display
@ 2019-09-16  9:29 Jani Nikula
  2019-09-16 10:29 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Jani Nikula @ 2019-09-16  9:29 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Stop setting ->pipe_mask to zero when display is disabled, allowing us
to have different code paths for not actually having display hardware,
and having display hardware disabled. This lets us develop those two
avenues independently.

There are no functional changes for when there is no display. However,
all uses of for_each_pipe() and for_each_pipe_masked() will start
running for the disabled display case. Put one of the more significant
ones behind checks for INTEL_DISPLAY_ENABLED(), otherwise the cases
should not be hit with disabled display, or they seem benign. Fingers
crossed.

All in all, this might not be the ideal solution. In fact we may have
had something along the lines of this in the past, but we ended up
conflating the two cases. Possibly even by recommendation by yours
truly; I did not dare dig up that part of the history. But the perfect
is the enemy of the good, this is a straightforward change, and lets us
get actual work done in both fronts without interfering with each other.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 12 +++++++-----
 drivers/gpu/drm/i915/intel_device_info.c     |  8 ++------
 2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index e75945a53e06..ac24f96582ca 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -16281,11 +16281,13 @@ int intel_modeset_init(struct drm_device *dev)
 		      INTEL_NUM_PIPES(dev_priv),
 		      INTEL_NUM_PIPES(dev_priv) > 1 ? "s" : "");
 
-	for_each_pipe(dev_priv, pipe) {
-		ret = intel_crtc_init(dev_priv, pipe);
-		if (ret) {
-			drm_mode_config_cleanup(dev);
-			return ret;
+	if (HAS_DISPLAY(dev_priv) && INTEL_DISPLAY_ENABLED(dev_priv)) {
+		for_each_pipe(dev_priv, pipe) {
+			ret = intel_crtc_init(dev_priv, pipe);
+			if (ret) {
+				drm_mode_config_cleanup(dev);
+				return ret;
+			}
 		}
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index 727089dcd280..728c881718a2 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -894,12 +894,8 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
 			runtime->num_sprites[pipe] = 1;
 	}
 
-	if (i915_modparams.disable_display) {
-		DRM_INFO("Display disabled (module parameter)\n");
-		info->pipe_mask = 0;
-	} else if (HAS_DISPLAY(dev_priv) &&
-		   (IS_GEN_RANGE(dev_priv, 7, 8)) &&
-		   HAS_PCH_SPLIT(dev_priv)) {
+	if (HAS_DISPLAY(dev_priv) && IS_GEN_RANGE(dev_priv, 7, 8) &&
+	    HAS_PCH_SPLIT(dev_priv)) {
 		u32 fuse_strap = I915_READ(FUSE_STRAP);
 		u32 sfuse_strap = I915_READ(SFUSE_STRAP);
 
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: stop conflating HAS_DISPLAY() and disabled display
  2019-09-16  9:29 [RESEND] drm/i915: stop conflating HAS_DISPLAY() and disabled display Jani Nikula
@ 2019-09-16 10:29 ` Patchwork
  2019-09-16 12:46 ` ✓ Fi.CI.IGT: " Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2019-09-16 10:29 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: stop conflating HAS_DISPLAY() and disabled display
URL   : https://patchwork.freedesktop.org/series/66749/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6897 -> Patchwork_14418
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       [PASS][1] -> [INCOMPLETE][2] ([fdo#107718])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html

  * igt@gem_flink_basic@bad-open:
    - fi-icl-u3:          [PASS][3] -> [DMESG-WARN][4] ([fdo#107724]) +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/fi-icl-u3/igt@gem_flink_basic@bad-open.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/fi-icl-u3/igt@gem_flink_basic@bad-open.html

  * igt@gem_mmap_gtt@basic:
    - fi-glk-dsi:         [PASS][5] -> [INCOMPLETE][6] ([fdo#103359] / [k.org#198133])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/fi-glk-dsi/igt@gem_mmap_gtt@basic.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/fi-glk-dsi/igt@gem_mmap_gtt@basic.html

  * igt@i915_selftest@live_reset:
    - fi-icl-u2:          [PASS][7] -> [INCOMPLETE][8] ([fdo#107713])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/fi-icl-u2/igt@i915_selftest@live_reset.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/fi-icl-u2/igt@i915_selftest@live_reset.html

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - {fi-icl-u4}:        [INCOMPLETE][9] ([fdo#107713] / [fdo#109100]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/fi-icl-u4/igt@gem_ctx_create@basic-files.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/fi-icl-u4/igt@gem_ctx_create@basic-files.html

  * igt@gem_ctx_switch@legacy-render:
    - {fi-icl-guc}:       [INCOMPLETE][11] ([fdo#107713] / [fdo#111381]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/fi-icl-guc/igt@gem_ctx_switch@legacy-render.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/fi-icl-guc/igt@gem_ctx_switch@legacy-render.html

  * igt@gem_exec_fence@nb-await-default:
    - {fi-tgl-u}:         [FAIL][13] ([fdo#111562] / [fdo#111597]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/fi-tgl-u/igt@gem_exec_fence@nb-await-default.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/fi-tgl-u/igt@gem_exec_fence@nb-await-default.html

  * {igt@i915_selftest@live_gt_timelines}:
    - fi-bsw-kefka:       [DMESG-WARN][15] ([fdo#111373]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/fi-bsw-kefka/igt@i915_selftest@live_gt_timelines.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/fi-bsw-kefka/igt@i915_selftest@live_gt_timelines.html

  * igt@vgem_basic@dmabuf-fence-before:
    - fi-icl-u3:          [DMESG-WARN][17] ([fdo#107724]) -> [PASS][18] +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/fi-icl-u3/igt@vgem_basic@dmabuf-fence-before.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/fi-icl-u3/igt@vgem_basic@dmabuf-fence-before.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#111373]: https://bugs.freedesktop.org/show_bug.cgi?id=111373
  [fdo#111381]: https://bugs.freedesktop.org/show_bug.cgi?id=111381
  [fdo#111562]: https://bugs.freedesktop.org/show_bug.cgi?id=111562
  [fdo#111597]: https://bugs.freedesktop.org/show_bug.cgi?id=111597
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (55 -> 49)
------------------------------

  Additional (1): fi-apl-guc 
  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6897 -> Patchwork_14418

  CI-20190529: 20190529
  CI_DRM_6897: 8ee6ca77b45a87f294a219eca4b467425a3ddc73 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5183: 6ddc1a143495baa68dbc909f2a8819ec03c31c8e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14418: e61e022df89dd4a0058e6efe0d5828fdddf43b35 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

e61e022df89d drm/i915: stop conflating HAS_DISPLAY() and disabled display

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915: stop conflating HAS_DISPLAY() and disabled display
  2019-09-16  9:29 [RESEND] drm/i915: stop conflating HAS_DISPLAY() and disabled display Jani Nikula
  2019-09-16 10:29 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2019-09-16 12:46 ` Patchwork
  2019-09-16 13:27 ` [RESEND] " Chris Wilson
  2019-09-16 14:16 ` Chris Wilson
  3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2019-09-16 12:46 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: stop conflating HAS_DISPLAY() and disabled display
URL   : https://patchwork.freedesktop.org/series/66749/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6897_full -> Patchwork_14418_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [PASS][1] -> [SKIP][2] ([fdo#110854])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-iclb2/igt@gem_exec_balancer@smoke.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-iclb5/igt@gem_exec_balancer@smoke.html

  * igt@gem_exec_schedule@preempt-bsd:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#111325]) +4 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-iclb5/igt@gem_exec_schedule@preempt-bsd.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-iclb4/igt@gem_exec_schedule@preempt-bsd.html

  * igt@i915_pm_rc6_residency@rc6-accuracy:
    - shard-kbl:          [PASS][5] -> [SKIP][6] ([fdo#109271])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-kbl1/igt@i915_pm_rc6_residency@rc6-accuracy.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-kbl2/igt@i915_pm_rc6_residency@rc6-accuracy.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          [PASS][7] -> [DMESG-WARN][8] ([fdo#108566]) +3 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-apl2/igt@i915_suspend@fence-restore-tiled2untiled.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-apl2/igt@i915_suspend@fence-restore-tiled2untiled.html
    - shard-kbl:          [PASS][9] -> [DMESG-WARN][10] ([fdo#108566])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-kbl3/igt@i915_suspend@fence-restore-tiled2untiled.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-kbl2/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
    - shard-iclb:         [PASS][11] -> [INCOMPLETE][12] ([fdo#107713]) +1 similar issue
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-iclb3/igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-iclb7/igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a.html

  * igt@kms_color@pipe-a-ctm-green-to-red:
    - shard-skl:          [PASS][13] -> [FAIL][14] ([fdo#107201])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-skl7/igt@kms_color@pipe-a-ctm-green-to-red.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-skl3/igt@kms_color@pipe-a-ctm-green-to-red.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-glk:          [PASS][15] -> [FAIL][16] ([fdo#105363])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-glk9/igt@kms_flip@flip-vs-expired-vblank.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-glk1/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-hsw:          [PASS][17] -> [INCOMPLETE][18] ([fdo#103540])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-hsw8/igt@kms_flip@flip-vs-suspend.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-hsw1/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-msflip-blt:
    - shard-iclb:         [PASS][19] -> [FAIL][20] ([fdo#103167]) +1 similar issue
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-iclb3/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-msflip-blt.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-msflip-blt.html

  * igt@kms_frontbuffer_tracking@fbc-1p-rte:
    - shard-iclb:         [PASS][21] -> [FAIL][22] ([fdo#103167] / [fdo#110378])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-1p-rte.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-1p-rte.html

  * igt@kms_frontbuffer_tracking@psr-1p-pri-indfb-multidraw:
    - shard-skl:          [PASS][23] -> [FAIL][24] ([fdo#103167])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-skl7/igt@kms_frontbuffer_tracking@psr-1p-pri-indfb-multidraw.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-skl3/igt@kms_frontbuffer_tracking@psr-1p-pri-indfb-multidraw.html

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          [PASS][25] -> [FAIL][26] ([fdo#108145])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-skl7/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-skl3/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         [PASS][27] -> [FAIL][28] ([fdo#103166])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-iclb3/igt@kms_plane_lowres@pipe-a-tiling-y.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-iclb7/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * igt@kms_psr@psr2_basic:
    - shard-iclb:         [PASS][29] -> [SKIP][30] ([fdo#109441]) +1 similar issue
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-iclb2/igt@kms_psr@psr2_basic.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-iclb7/igt@kms_psr@psr2_basic.html

  * igt@kms_setmode@basic:
    - shard-hsw:          [PASS][31] -> [FAIL][32] ([fdo#99912])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-hsw7/igt@kms_setmode@basic.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-hsw2/igt@kms_setmode@basic.html

  * igt@prime_busy@hang-bsd2:
    - shard-iclb:         [PASS][33] -> [SKIP][34] ([fdo#109276]) +15 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-iclb4/igt@prime_busy@hang-bsd2.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-iclb3/igt@prime_busy@hang-bsd2.html

  
#### Possible fixes ####

  * igt@gem_ctx_switch@legacy-render-heavy:
    - shard-apl:          [INCOMPLETE][35] ([fdo#103927]) -> [PASS][36] +1 similar issue
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-apl2/igt@gem_ctx_switch@legacy-render-heavy.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-apl4/igt@gem_ctx_switch@legacy-render-heavy.html

  * igt@gem_eio@in-flight-immediate:
    - shard-skl:          [FAIL][37] ([fdo#105957]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-skl7/igt@gem_eio@in-flight-immediate.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-skl4/igt@gem_eio@in-flight-immediate.html

  * igt@gem_exec_schedule@in-order-bsd:
    - shard-iclb:         [SKIP][39] ([fdo#111325]) -> [PASS][40] +6 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-iclb2/igt@gem_exec_schedule@in-order-bsd.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-iclb8/igt@gem_exec_schedule@in-order-bsd.html

  * igt@gem_exec_schedule@independent-bsd2:
    - shard-iclb:         [SKIP][41] ([fdo#109276]) -> [PASS][42] +18 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-iclb8/igt@gem_exec_schedule@independent-bsd2.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-iclb2/igt@gem_exec_schedule@independent-bsd2.html

  * igt@i915_suspend@sysfs-reader:
    - shard-apl:          [DMESG-WARN][43] ([fdo#108566]) -> [PASS][44] +5 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-apl8/igt@i915_suspend@sysfs-reader.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-apl3/igt@i915_suspend@sysfs-reader.html

  * igt@kms_draw_crc@draw-method-xrgb8888-mmap-cpu-ytiled:
    - shard-iclb:         [FAIL][45] ([fdo#103184] / [fdo#103232]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-iclb1/igt@kms_draw_crc@draw-method-xrgb8888-mmap-cpu-ytiled.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-iclb6/igt@kms_draw_crc@draw-method-xrgb8888-mmap-cpu-ytiled.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          [DMESG-WARN][47] ([fdo#108566]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-kbl2/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-kbl3/igt@kms_frontbuffer_tracking@fbc-suspend.html
    - shard-iclb:         [FAIL][49] ([fdo#103167]) -> [PASS][50] +6 similar issues
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-iclb3/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-skl:          [INCOMPLETE][51] ([fdo#104108]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-skl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-skl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [FAIL][53] ([fdo#108145] / [fdo#110403]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-skl6/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-skl9/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_psr@psr2_cursor_plane_move:
    - shard-iclb:         [SKIP][55] ([fdo#109441]) -> [PASS][56] +1 similar issue
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-iclb3/igt@kms_psr@psr2_cursor_plane_move.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-iclb2/igt@kms_psr@psr2_cursor_plane_move.html

  * igt@kms_setmode@basic:
    - shard-apl:          [FAIL][57] ([fdo#99912]) -> [PASS][58]
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-apl2/igt@kms_setmode@basic.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-apl2/igt@kms_setmode@basic.html
    - shard-kbl:          [FAIL][59] ([fdo#99912]) -> [PASS][60]
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-kbl3/igt@kms_setmode@basic.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-kbl2/igt@kms_setmode@basic.html

  * igt@perf@polling:
    - shard-skl:          [FAIL][61] ([fdo#110728]) -> [PASS][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-skl2/igt@perf@polling.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-skl9/igt@perf@polling.html

  
#### Warnings ####

  * igt@gem_mocs_settings@mocs-rc6-bsd2:
    - shard-iclb:         [SKIP][63] ([fdo#109276]) -> [FAIL][64] ([fdo#111330])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-iclb5/igt@gem_mocs_settings@mocs-rc6-bsd2.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-iclb1/igt@gem_mocs_settings@mocs-rc6-bsd2.html

  * igt@gem_mocs_settings@mocs-reset-bsd2:
    - shard-iclb:         [FAIL][65] ([fdo#111330]) -> [SKIP][66] ([fdo#109276])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6897/shard-iclb2/igt@gem_mocs_settings@mocs-reset-bsd2.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/shard-iclb8/igt@gem_mocs_settings@mocs-reset-bsd2.html

  
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105957]: https://bugs.freedesktop.org/show_bug.cgi?id=105957
  [fdo#107201]: https://bugs.freedesktop.org/show_bug.cgi?id=107201
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110378]: https://bugs.freedesktop.org/show_bug.cgi?id=110378
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
  [fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
  [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
  [fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6897 -> Patchwork_14418

  CI-20190529: 20190529
  CI_DRM_6897: 8ee6ca77b45a87f294a219eca4b467425a3ddc73 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5183: 6ddc1a143495baa68dbc909f2a8819ec03c31c8e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14418: e61e022df89dd4a0058e6efe0d5828fdddf43b35 @ 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_14418/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RESEND] drm/i915: stop conflating HAS_DISPLAY() and disabled display
  2019-09-16  9:29 [RESEND] drm/i915: stop conflating HAS_DISPLAY() and disabled display Jani Nikula
  2019-09-16 10:29 ` ✓ Fi.CI.BAT: success for " Patchwork
  2019-09-16 12:46 ` ✓ Fi.CI.IGT: " Patchwork
@ 2019-09-16 13:27 ` Chris Wilson
  2019-09-16 14:05   ` Jani Nikula
  2019-09-16 14:16 ` Chris Wilson
  3 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2019-09-16 13:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Quoting Jani Nikula (2019-09-16 10:29:01)
> Stop setting ->pipe_mask to zero when display is disabled, allowing us
> to have different code paths for not actually having display hardware,
> and having display hardware disabled. This lets us develop those two
> avenues independently.
> 
> There are no functional changes for when there is no display. However,
> all uses of for_each_pipe() and for_each_pipe_masked() will start
> running for the disabled display case. Put one of the more significant
> ones behind checks for INTEL_DISPLAY_ENABLED(), otherwise the cases
> should not be hit with disabled display, or they seem benign. Fingers
> crossed.
> 
> All in all, this might not be the ideal solution. In fact we may have
> had something along the lines of this in the past, but we ended up
> conflating the two cases. Possibly even by recommendation by yours
> truly; I did not dare dig up that part of the history. But the perfect
> is the enemy of the good, this is a straightforward change, and lets us
> get actual work done in both fronts without interfering with each other.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 12 +++++++-----
>  drivers/gpu/drm/i915/intel_device_info.c     |  8 ++------
>  2 files changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index e75945a53e06..ac24f96582ca 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -16281,11 +16281,13 @@ int intel_modeset_init(struct drm_device *dev)
>                       INTEL_NUM_PIPES(dev_priv),
>                       INTEL_NUM_PIPES(dev_priv) > 1 ? "s" : "");
>  
> -       for_each_pipe(dev_priv, pipe) {
> -               ret = intel_crtc_init(dev_priv, pipe);
> -               if (ret) {
> -                       drm_mode_config_cleanup(dev);
> -                       return ret;
> +       if (HAS_DISPLAY(dev_priv) && INTEL_DISPLAY_ENABLED(dev_priv)) {
> +               for_each_pipe(dev_priv, pipe) {
> +                       ret = intel_crtc_init(dev_priv, pipe);

What direction are you planning to take, avoid enabling anything related
to display? My worry is that in

https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/fi-bsw-kefka/igt@i915_selftest@live_gt_timelines.html

we still see weird events like

<7> [444.313823] [drm:i915_redisable_vga_power_on [i915]] Something enabled VGA plane, disabling it

and I'm not sure how you intend to curtail that. (Or if that's even
possible.)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RESEND] drm/i915: stop conflating HAS_DISPLAY() and disabled display
  2019-09-16 13:27 ` [RESEND] " Chris Wilson
@ 2019-09-16 14:05   ` Jani Nikula
  2019-09-16 14:27     ` Ville Syrjälä
  0 siblings, 1 reply; 9+ messages in thread
From: Jani Nikula @ 2019-09-16 14:05 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On Mon, 16 Sep 2019, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Jani Nikula (2019-09-16 10:29:01)
>> Stop setting ->pipe_mask to zero when display is disabled, allowing us
>> to have different code paths for not actually having display hardware,
>> and having display hardware disabled. This lets us develop those two
>> avenues independently.
>> 
>> There are no functional changes for when there is no display. However,
>> all uses of for_each_pipe() and for_each_pipe_masked() will start
>> running for the disabled display case. Put one of the more significant
>> ones behind checks for INTEL_DISPLAY_ENABLED(), otherwise the cases
>> should not be hit with disabled display, or they seem benign. Fingers
>> crossed.
>> 
>> All in all, this might not be the ideal solution. In fact we may have
>> had something along the lines of this in the past, but we ended up
>> conflating the two cases. Possibly even by recommendation by yours
>> truly; I did not dare dig up that part of the history. But the perfect
>> is the enemy of the good, this is a straightforward change, and lets us
>> get actual work done in both fronts without interfering with each other.
>> 
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: José Roberto de Souza <jose.souza@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_display.c | 12 +++++++-----
>>  drivers/gpu/drm/i915/intel_device_info.c     |  8 ++------
>>  2 files changed, 9 insertions(+), 11 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index e75945a53e06..ac24f96582ca 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -16281,11 +16281,13 @@ int intel_modeset_init(struct drm_device *dev)
>>                       INTEL_NUM_PIPES(dev_priv),
>>                       INTEL_NUM_PIPES(dev_priv) > 1 ? "s" : "");
>>  
>> -       for_each_pipe(dev_priv, pipe) {
>> -               ret = intel_crtc_init(dev_priv, pipe);
>> -               if (ret) {
>> -                       drm_mode_config_cleanup(dev);
>> -                       return ret;
>> +       if (HAS_DISPLAY(dev_priv) && INTEL_DISPLAY_ENABLED(dev_priv)) {
>> +               for_each_pipe(dev_priv, pipe) {
>> +                       ret = intel_crtc_init(dev_priv, pipe);
>
> What direction are you planning to take, avoid enabling anything related
> to display? My worry is that in
>
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/fi-bsw-kefka/igt@i915_selftest@live_gt_timelines.html
>
> we still see weird events like
>
> <7> [444.313823] [drm:i915_redisable_vga_power_on [i915]] Something enabled VGA plane, disabling it
>
> and I'm not sure how you intend to curtail that. (Or if that's even
> possible.)

The main goal here (in this specific patch) is to decouple disabled but
existing display from non-existing display. That lets us develop the two
cases independently, and I acknowledge I may have been simple minded
enough at some point to believe they could be put in the same bucket.

This patch tries to do the decoupling with the least amount of
functional changes to the disabled display case (no matter how broken
the status quo is), but also without sprinkling INTEL_DISPLAY_ENABLED()
checks at every for_each_display{,_masked}. Some arbitrarily chosen
balance.

After this, I'm hoping we can develop the cases independently, without
each of them stomping on the other's feet. Who knows, maybe the next
patch should revert the hunk you quote above. I do know I'm not signing
up for fixing everything about the disabled display path, at least right
away, but the point is to remove obstacles.

BR,
Jani.



-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RESEND] drm/i915: stop conflating HAS_DISPLAY() and disabled display
  2019-09-16  9:29 [RESEND] drm/i915: stop conflating HAS_DISPLAY() and disabled display Jani Nikula
                   ` (2 preceding siblings ...)
  2019-09-16 13:27 ` [RESEND] " Chris Wilson
@ 2019-09-16 14:16 ` Chris Wilson
  3 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2019-09-16 14:16 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Quoting Jani Nikula (2019-09-16 10:29:01)
> Stop setting ->pipe_mask to zero when display is disabled, allowing us
> to have different code paths for not actually having display hardware,
> and having display hardware disabled. This lets us develop those two
> avenues independently.
> 
> There are no functional changes for when there is no display. However,
> all uses of for_each_pipe() and for_each_pipe_masked() will start
> running for the disabled display case. Put one of the more significant
> ones behind checks for INTEL_DISPLAY_ENABLED(), otherwise the cases
> should not be hit with disabled display, or they seem benign. Fingers
> crossed.
> 
> All in all, this might not be the ideal solution. In fact we may have
> had something along the lines of this in the past, but we ended up
> conflating the two cases. Possibly even by recommendation by yours
> truly; I did not dare dig up that part of the history. But the perfect
> is the enemy of the good, this is a straightforward change, and lets us
> get actual work done in both fronts without interfering with each other.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

It doesn't fall over, which is impressive enough.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RESEND] drm/i915: stop conflating HAS_DISPLAY() and disabled display
  2019-09-16 14:05   ` Jani Nikula
@ 2019-09-16 14:27     ` Ville Syrjälä
  2019-09-16 14:33       ` Chris Wilson
  2019-09-16 18:22       ` Jani Nikula
  0 siblings, 2 replies; 9+ messages in thread
From: Ville Syrjälä @ 2019-09-16 14:27 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Mon, Sep 16, 2019 at 05:05:10PM +0300, Jani Nikula wrote:
> On Mon, 16 Sep 2019, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > Quoting Jani Nikula (2019-09-16 10:29:01)
> >> Stop setting ->pipe_mask to zero when display is disabled, allowing us
> >> to have different code paths for not actually having display hardware,
> >> and having display hardware disabled. This lets us develop those two
> >> avenues independently.
> >> 
> >> There are no functional changes for when there is no display. However,
> >> all uses of for_each_pipe() and for_each_pipe_masked() will start
> >> running for the disabled display case. Put one of the more significant
> >> ones behind checks for INTEL_DISPLAY_ENABLED(), otherwise the cases
> >> should not be hit with disabled display, or they seem benign. Fingers
> >> crossed.
> >> 
> >> All in all, this might not be the ideal solution. In fact we may have
> >> had something along the lines of this in the past, but we ended up
> >> conflating the two cases. Possibly even by recommendation by yours
> >> truly; I did not dare dig up that part of the history. But the perfect
> >> is the enemy of the good, this is a straightforward change, and lets us
> >> get actual work done in both fronts without interfering with each other.
> >> 
> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >> Cc: José Roberto de Souza <jose.souza@intel.com>
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/display/intel_display.c | 12 +++++++-----
> >>  drivers/gpu/drm/i915/intel_device_info.c     |  8 ++------
> >>  2 files changed, 9 insertions(+), 11 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> >> index e75945a53e06..ac24f96582ca 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> >> @@ -16281,11 +16281,13 @@ int intel_modeset_init(struct drm_device *dev)
> >>                       INTEL_NUM_PIPES(dev_priv),
> >>                       INTEL_NUM_PIPES(dev_priv) > 1 ? "s" : "");
> >>  
> >> -       for_each_pipe(dev_priv, pipe) {
> >> -               ret = intel_crtc_init(dev_priv, pipe);
> >> -               if (ret) {
> >> -                       drm_mode_config_cleanup(dev);
> >> -                       return ret;
> >> +       if (HAS_DISPLAY(dev_priv) && INTEL_DISPLAY_ENABLED(dev_priv)) {
> >> +               for_each_pipe(dev_priv, pipe) {
> >> +                       ret = intel_crtc_init(dev_priv, pipe);
> >
> > What direction are you planning to take, avoid enabling anything related
> > to display? My worry is that in
> >
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/fi-bsw-kefka/igt@i915_selftest@live_gt_timelines.html
> >
> > we still see weird events like
> >
> > <7> [444.313823] [drm:i915_redisable_vga_power_on [i915]] Something enabled VGA plane, disabling it
> >
> > and I'm not sure how you intend to curtail that. (Or if that's even
> > possible.)
> 
> The main goal here (in this specific patch) is to decouple disabled but
> existing display from non-existing display. That lets us develop the two
> cases independently, and I acknowledge I may have been simple minded
> enough at some point to believe they could be put in the same bucket.

What's the actual use case for the "disabled but existing display"?

So far I've thought that the only use case is regression testing
of the "hw has no display" case on hw which in fact has a display.
If we have separate codepaths we can't do that effectively. At
which point we might as well get rid of the "disable display"
capability entirely.

But maybe I'm missing something...

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RESEND] drm/i915: stop conflating HAS_DISPLAY() and disabled display
  2019-09-16 14:27     ` Ville Syrjälä
@ 2019-09-16 14:33       ` Chris Wilson
  2019-09-16 18:22       ` Jani Nikula
  1 sibling, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2019-09-16 14:33 UTC (permalink / raw)
  To: Jani Nikula, Ville Syrjälä; +Cc: intel-gfx

Quoting Ville Syrjälä (2019-09-16 15:27:40)
> On Mon, Sep 16, 2019 at 05:05:10PM +0300, Jani Nikula wrote:
> > The main goal here (in this specific patch) is to decouple disabled but
> > existing display from non-existing display. That lets us develop the two
> > cases independently, and I acknowledge I may have been simple minded
> > enough at some point to believe they could be put in the same bucket.
> 
> What's the actual use case for the "disabled but existing display"?

There are 2 reasons why I have it enabled for the live gem selftests. Not
setting up the display makes module reload faster, and the other reason
is that I hoped to avoid any spurious interactions (random hotplug
events) in the middle of the stress tests.

The latter usecase I would suggest applies to headless servers, where we
want to minimise random events.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RESEND] drm/i915: stop conflating HAS_DISPLAY() and disabled display
  2019-09-16 14:27     ` Ville Syrjälä
  2019-09-16 14:33       ` Chris Wilson
@ 2019-09-16 18:22       ` Jani Nikula
  1 sibling, 0 replies; 9+ messages in thread
From: Jani Nikula @ 2019-09-16 18:22 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Mon, 16 Sep 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Mon, Sep 16, 2019 at 05:05:10PM +0300, Jani Nikula wrote:
>> On Mon, 16 Sep 2019, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> > Quoting Jani Nikula (2019-09-16 10:29:01)
>> >> Stop setting ->pipe_mask to zero when display is disabled, allowing us
>> >> to have different code paths for not actually having display hardware,
>> >> and having display hardware disabled. This lets us develop those two
>> >> avenues independently.
>> >> 
>> >> There are no functional changes for when there is no display. However,
>> >> all uses of for_each_pipe() and for_each_pipe_masked() will start
>> >> running for the disabled display case. Put one of the more significant
>> >> ones behind checks for INTEL_DISPLAY_ENABLED(), otherwise the cases
>> >> should not be hit with disabled display, or they seem benign. Fingers
>> >> crossed.
>> >> 
>> >> All in all, this might not be the ideal solution. In fact we may have
>> >> had something along the lines of this in the past, but we ended up
>> >> conflating the two cases. Possibly even by recommendation by yours
>> >> truly; I did not dare dig up that part of the history. But the perfect
>> >> is the enemy of the good, this is a straightforward change, and lets us
>> >> get actual work done in both fronts without interfering with each other.
>> >> 
>> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> >> Cc: José Roberto de Souza <jose.souza@intel.com>
>> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> >> ---
>> >>  drivers/gpu/drm/i915/display/intel_display.c | 12 +++++++-----
>> >>  drivers/gpu/drm/i915/intel_device_info.c     |  8 ++------
>> >>  2 files changed, 9 insertions(+), 11 deletions(-)
>> >> 
>> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> >> index e75945a53e06..ac24f96582ca 100644
>> >> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> >> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> >> @@ -16281,11 +16281,13 @@ int intel_modeset_init(struct drm_device *dev)
>> >>                       INTEL_NUM_PIPES(dev_priv),
>> >>                       INTEL_NUM_PIPES(dev_priv) > 1 ? "s" : "");
>> >>  
>> >> -       for_each_pipe(dev_priv, pipe) {
>> >> -               ret = intel_crtc_init(dev_priv, pipe);
>> >> -               if (ret) {
>> >> -                       drm_mode_config_cleanup(dev);
>> >> -                       return ret;
>> >> +       if (HAS_DISPLAY(dev_priv) && INTEL_DISPLAY_ENABLED(dev_priv)) {
>> >> +               for_each_pipe(dev_priv, pipe) {
>> >> +                       ret = intel_crtc_init(dev_priv, pipe);
>> >
>> > What direction are you planning to take, avoid enabling anything related
>> > to display? My worry is that in
>> >
>> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14418/fi-bsw-kefka/igt@i915_selftest@live_gt_timelines.html
>> >
>> > we still see weird events like
>> >
>> > <7> [444.313823] [drm:i915_redisable_vga_power_on [i915]] Something enabled VGA plane, disabling it
>> >
>> > and I'm not sure how you intend to curtail that. (Or if that's even
>> > possible.)
>> 
>> The main goal here (in this specific patch) is to decouple disabled but
>> existing display from non-existing display. That lets us develop the two
>> cases independently, and I acknowledge I may have been simple minded
>> enough at some point to believe they could be put in the same bucket.
>
> What's the actual use case for the "disabled but existing display"?
>
> So far I've thought that the only use case is regression testing
> of the "hw has no display" case on hw which in fact has a display.
> If we have separate codepaths we can't do that effectively. At
> which point we might as well get rid of the "disable display"
> capability entirely.

The problem seems to be that we simply can't have the same code paths,
with e.g. bios enabling display hw behind our backs. And patching either
code path with just one knob causes problems to the other. So I want to
decouple the two to make our lives easier for the immediate future.

If we can think of better ways to do this, and better utilize shared
code paths, the decoupling doesn't really prevent us from doing that
either. One idea was probing, then disabling everything, and either
using -EPROBE_DEFER or reprobing and then pretending there is no
display.

But right now this is getting in the way. Need to unblock work. Later we
can re-evaluate whether we need display disable or not and how to best
support it.

Pushed the patch, thanks for review. Does not have to mean end of
discussion though.


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-09-16 18:20 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-16  9:29 [RESEND] drm/i915: stop conflating HAS_DISPLAY() and disabled display Jani Nikula
2019-09-16 10:29 ` ✓ Fi.CI.BAT: success for " Patchwork
2019-09-16 12:46 ` ✓ Fi.CI.IGT: " Patchwork
2019-09-16 13:27 ` [RESEND] " Chris Wilson
2019-09-16 14:05   ` Jani Nikula
2019-09-16 14:27     ` Ville Syrjälä
2019-09-16 14:33       ` Chris Wilson
2019-09-16 18:22       ` Jani Nikula
2019-09-16 14:16 ` Chris Wilson

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.