On Mon, Oct 15, 2018 at 12:17:41PM +0100, Chris Wilson wrote:
> 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 <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Imre Deak <imre.deak@xxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
i915.disable_display was for those server chips where doing all the init
resulted in a dead machine. So not sure we want this.
What's the issue with power wells still being on and all that? On real hw
without display they won't exist, and I don't understand why we'd care for
testing.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.c | 53 +++++++++++++++++-------
> drivers/gpu/drm/i915/intel_device_info.c | 9 ++--
> drivers/gpu/drm/i915/intel_fbdev.c | 2 +-
> 3 files changed, 43 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index c14855f167b9..d71add64948b 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:
> @@ -1667,6 +1653,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
> @@ -1727,6 +1724,34 @@ 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) {
> + drm_mode_config_cleanup(&dev_priv->drm);
> + dev_priv->drm.driver_features &=
> + ~(DRIVER_MODESET | DRIVER_ATOMIC);
> + dev_priv->psr.sink_support = false;
> + }
> +
> + /* 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 03df4e33763d..5f6b12986ce9 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -775,12 +775,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 a75082813669..5442a13bba63 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -677,7 +677,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);
> --
> 2.19.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
