[3/4] drm/i915: Disable displays at the user's request

Commit Message

Chris Wilson Sept. 13, 2018, 1:16 p.m. UTC

If the user passes i915.disable_display=1 we want to disable all the
displays and associated HW like the powerwells on their behalf. Instead
of short circuiting the HW probe, let it run and setup all the
bookkeeping for the known HW. Afterwards, instead of taking over the
BIOS fb and installing the fbcon, we shutdown all the outputs and
teardown the bookkeeping, leaving us with no attached outputs or crtcs,
and all the HW powered down.

Open: wq flushes should be required but seem to deadlock the modprobe
under CI.

Signed-off-by: Chris Wilson <[e-mail]>
Cc: Imre Deak <[e-mail]>
Cc: Ville Syrjälä <[e-mail]>
---
 drivers/gpu/drm/i915/i915_drv.c          | 52 +++++++++++++++++-------
 drivers/gpu/drm/i915/intel_device_info.c |  9 ++--
 drivers/gpu/drm/i915/intel_fbdev.c       |  2 +-
 3 files changed, 42 insertions(+), 21 deletions(-)

Comments

Chris Wilson Sept. 13, 2018, 1:19 p.m. UTC | #1

Quoting Chris Wilson (2018-09-13 14:16:28)
> +       /*
> +        * After completing our HW probe; tear it all down again (at the
> +        * user's request)!
> +        *
> +        * Along side the CRTCs and connectors, there is a medley of
> +        * auxiliary HW which control various powerwells and interact with
> +        * other state (such as the BIOS framebuffer occupying a portion
> +        * of reserved memory). If the user tells us to run without any
> +        * displays enabled, we still need to register all the display and
> +        * auxiliary HW in order to safely disable them.
> +        */
> +       if (i915_modparams.disable_display) {
> +               DRM_INFO("Display disabled (module parameter)\n");
> +               disable_display(dev_priv);
> +               mkwrite_device_info(dev_priv)->num_pipes = 0;
> +       }
> +
> +       if (INTEL_INFO(dev_priv)->num_pipes == 0) {
> +               dev_priv->psr.sink_support = false;
> +               drm_mode_config_cleanup(&dev_priv->drm);
> +               driver.driver_features &= ~(DRIVER_MODESET | DRIVER_ATOMIC);

Removing the feature seems like a good idea on surface (as we then don't
have to worry about hitting internals that expect crtcs and planes and
such), however more work required in declaring igt requirements than
simply having no pipes :)
-Chris

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a74de4428c79..f34a1f5b99e5 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -689,22 +689,8 @@  static int i915_load_modeset_init(struct drm_device *dev)
 
 	intel_setup_overlay(dev_priv);
 
-	if (INTEL_INFO(dev_priv)->num_pipes == 0)
-		return 0;
-
-	ret = intel_fbdev_init(dev_priv);
-	if (ret)
-		goto cleanup_gem;
-
-	/* Only enable hotplug handling once the fbdev is fully set up. */
-	intel_hpd_init(dev_priv);
-
 	return 0;
 
-cleanup_gem:
-	if (i915_gem_suspend(dev_priv))
-		DRM_ERROR("failed to idle hardware; continuing to unload!\n");
-	i915_gem_fini(dev_priv);
 cleanup_modeset:
 	intel_modeset_cleanup(dev);
 cleanup_irq:
@@ -1366,6 +1352,17 @@  static void i915_driver_destroy(struct drm_i915_private *i915)
 	pci_set_drvdata(pdev, NULL);
 }
 
+static void disable_display(struct drm_i915_private *i915)
+{
+	drm_atomic_helper_shutdown(&i915->drm);
+
+#if 0 /* XXX flushes deadlock under modprobe??? */
+	flush_workqueue(i915->modeset_wq);
+	flush_work(&i915->atomic_helper.free_work);
+	flush_scheduled_work();
+#endif
+}
+
 /**
  * i915_driver_load - setup chip and create an initial config
  * @pdev: PCI device
@@ -1426,6 +1423,33 @@  int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (ret < 0)
 		goto out_cleanup_hw;
 
+	/*
+	 * After completing our HW probe; tear it all down again (at the
+	 * user's request)!
+	 *
+	 * Along side the CRTCs and connectors, there is a medley of
+	 * auxiliary HW which control various powerwells and interact with
+	 * other state (such as the BIOS framebuffer occupying a portion
+	 * of reserved memory). If the user tells us to run without any
+	 * displays enabled, we still need to register all the display and
+	 * auxiliary HW in order to safely disable them.
+	 */
+	if (i915_modparams.disable_display) {
+		DRM_INFO("Display disabled (module parameter)\n");
+		disable_display(dev_priv);
+		mkwrite_device_info(dev_priv)->num_pipes = 0;
+	}
+
+	if (INTEL_INFO(dev_priv)->num_pipes == 0) {
+		dev_priv->psr.sink_support = false;
+		drm_mode_config_cleanup(&dev_priv->drm);
+		driver.driver_features &= ~(DRIVER_MODESET | DRIVER_ATOMIC);
+	}
+
+	/* Only enable hotplug handling once the fbdev is fully set up. */
+	if (intel_fbdev_init(dev_priv) == 0)
+		intel_hpd_init(dev_priv);
+
 	i915_driver_register(dev_priv);
 
 	intel_init_ipc(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index 0ef0c6448d53..e50ea5b2576b 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -776,12 +776,9 @@  void intel_device_info_runtime_init(struct intel_device_info *info)
 			info->num_sprites[pipe] = 1;
 	}
 
-	if (i915_modparams.disable_display) {
-		DRM_INFO("Display disabled (module parameter)\n");
-		info->num_pipes = 0;
-	} else if (info->num_pipes > 0 &&
-		   (IS_GEN7(dev_priv) || IS_GEN8(dev_priv)) &&
-		   HAS_PCH_SPLIT(dev_priv)) {
+	if (info->num_pipes > 0 &&
+	    (IS_GEN7(dev_priv) || IS_GEN8(dev_priv)) &&
+	    HAS_PCH_SPLIT(dev_priv)) {
 		u32 fuse_strap = I915_READ(FUSE_STRAP);
 		u32 sfuse_strap = I915_READ(SFUSE_STRAP);
 
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 84ebd8102215..9dd50c5ec558 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -668,7 +668,7 @@  int intel_fbdev_init(struct drm_i915_private *i915)
 	struct intel_fbdev *ifbdev;
 	int ret;
 
-	if (WARN_ON(INTEL_INFO(i915)->num_pipes == 0))
+	if (INTEL_INFO(i915)->num_pipes == 0)
 		return -ENODEV;
 
 	ifbdev = kzalloc(sizeof(struct intel_fbdev), GFP_KERNEL);