Re: [Intel-gfx] [PATCH v2 1/6] drm/i915: Shut down displays gracefully on reboot

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 01 Oct 2020, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Implement the pci .shutdown() hook in order to quiesce the
> hardware prior to reboot. The main purpose here is to turn
> all displays off. Some displays/other drivers tend to get
> confused if the state after reboot isn't exactly as they
> expected.
>
> One specific example was the Dell UP2414Q in MST mode.
> It would require me to pull the power cord after a reboot
> or else it would just not come back to life. Sadly I don't
> have that at hand anymore so not sure if it's still
> misbehaving without the graceful shutdown, or if we
> managed to fix something else since I last tested it.
>
> For good measure we do a gem suspend as well, so that
> we match the suspend flow more closely. Also stopping
> all DMA and whatnot is probably a good idea for kexec.
> I would expect that some kind of GT reset happens on
> normal reboot so probably not totally necessary there.
>
> v2: Use the pci .shutdown() hook instead of a reboot notifier (Lukas)
>     Do the gem suspend for kexec (Chris)
>
> Cc: Lukas Wunner <lukas@wunner.de>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 16 ++++++++++++++++
>  drivers/gpu/drm/i915/i915_drv.h |  1 +
>  drivers/gpu/drm/i915/i915_pci.c |  8 ++++++++
>  3 files changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 45e719c79183..062b61ebd9c4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1036,6 +1036,22 @@ static void intel_suspend_encoders(struct drm_i915_private *dev_priv)
>  	drm_modeset_unlock_all(dev);
>  }
>  
> +void i915_driver_shutdown(struct drm_i915_private *i915)
> +{
> +	i915_gem_suspend(i915);
> +
> +	drm_kms_helper_poll_disable(&i915->drm);
> +
> +	drm_atomic_helper_shutdown(&i915->drm);
> +
> +	intel_dp_mst_suspend(i915);
> +
> +	intel_runtime_pm_disable_interrupts(i915);
> +	intel_hpd_cancel_work(i915);
> +
> +	intel_suspend_encoders(i915);

I wish we could directly split this to gem and display parts.

*shrug*

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> +}
> +
>  static bool suspend_to_idle(struct drm_i915_private *dev_priv)
>  {
>  #if IS_ENABLED(CONFIG_ACPI_SLEEP)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index eef9a821c49c..9c2672c56cc1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1779,6 +1779,7 @@ extern const struct dev_pm_ops i915_pm_ops;
>  
>  int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
>  void i915_driver_remove(struct drm_i915_private *i915);
> +void i915_driver_shutdown(struct drm_i915_private *i915);
>  
>  int i915_resume_switcheroo(struct drm_i915_private *i915);
>  int i915_suspend_switcheroo(struct drm_i915_private *i915, pm_message_t state);
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 366ddfc8df6b..249730561b6c 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -1090,11 +1090,19 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	return 0;
>  }
>  
> +static void i915_pci_shutdown(struct pci_dev *pdev)
> +{
> +	struct drm_i915_private *i915 = pci_get_drvdata(pdev);
> +
> +	i915_driver_shutdown(i915);
> +}
> +
>  static struct pci_driver i915_pci_driver = {
>  	.name = DRIVER_NAME,
>  	.id_table = pciidlist,
>  	.probe = i915_pci_probe,
>  	.remove = i915_pci_remove,
> +	.shutdown = i915_pci_shutdown,
>  	.driver.pm = &i915_pm_ops,
>  };

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