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

[Daniel Vetter <daniel@ffwll.ch>]

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@chris-wilson.co.uk>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx