Re: [PATCH v2] drm/i915: move power domain init earlier during system resume

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Apr 01, 2014 at 07:55:22PM +0300, Imre Deak wrote:
> During resume the intel hda audio driver depends on the i915 driver
> reinitializing the audio power domain. Since the order of calling the
> i915 resume handler wrt. that of the audio driver is not guaranteed,
> move the power domain reinitialization step to the resume_early
> handler. This is guaranteed to run before the resume handler of any
> other driver.
> 
> The power domain initialization in turn requires us to enable the i915
> pci device first, so move that part earlier too.
> 
> Accordingly disabling of the i915 pci device should happen after the
> audio suspend handler ran. So move the disabling later from the i915
> resume handler to the resume_late handler.
> 
> v2:
> - move intel_uncore_sanitize/early_sanitize earlier too, so they don't
>   get reordered wrt. intel_power_domains_init_hw()
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76152
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>

So this is kinda why we should have gone with something proper, like a new
hdmi sink platform device created by i915 and registered as a driver by
snd-hda. Then the power domains stuff in the device core should take care
of these kinds of ordering issues. Or at least snd-hda can tell it that it
needs to wait for the hdmi-sink power domain to go on first before it can
resume, I'm not really fluent on the details here.

And having a hdmi sink bus would allow us to throw all kinds of crap into
a clearly-defined interface, e.g. eld handling, hdcp synchronization, hpd
forwarding and all the other fun stuff.

So not sure what I should do with this here now.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.c | 72 +++++++++++++++++++++++++++++++++--------
>  1 file changed, 58 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 11f77a8..90c399d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -537,14 +537,21 @@ static void intel_resume_hotplug(struct drm_device *dev)
>  	drm_helper_hpd_irq_event(dev);
>  }
>  
> -static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
> +static int i915_drm_thaw_early(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int error = 0;
>  
>  	intel_uncore_early_sanitize(dev);
> -
>  	intel_uncore_sanitize(dev);
> +	intel_power_domains_init_hw(dev_priv);
> +
> +	return 0;
> +}
> +
> +static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int error = 0;
>  
>  	if (drm_core_check_feature(dev, DRIVER_MODESET) &&
>  	    restore_gtt_mappings) {
> @@ -553,8 +560,6 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
>  		mutex_unlock(&dev->struct_mutex);
>  	}
>  
> -	intel_power_domains_init_hw(dev_priv);
> -
>  	i915_restore_state(dev);
>  	intel_opregion_setup(dev);
>  
> @@ -619,11 +624,8 @@ static int i915_drm_thaw(struct drm_device *dev)
>  	return __i915_drm_thaw(dev, true);
>  }
>  
> -int i915_resume(struct drm_device *dev)
> +static int i915_resume_early(struct drm_device *dev)
>  {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int ret;
> -
>  	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
>  		return 0;
>  
> @@ -632,6 +634,14 @@ int i915_resume(struct drm_device *dev)
>  
>  	pci_set_master(dev->pdev);
>  
> +	return i915_drm_thaw_early(dev);
> +}
> +
> +int i915_resume(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int ret;
> +
>  	/*
>  	 * Platforms with opregion should have sane BIOS, older ones (gen3 and
>  	 * earlier) need to restore the GTT mappings since the BIOS might clear
> @@ -645,6 +655,14 @@ int i915_resume(struct drm_device *dev)
>  	return 0;
>  }
>  
> +int i915_resume_legacy(struct drm_device *dev)
> +{
> +	i915_resume_early(dev);
> +	i915_resume(dev);
> +
> +	return 0;
> +}
> +
>  /**
>   * i915_reset - reset chip after a hang
>   * @dev: drm device to reset
> @@ -776,7 +794,6 @@ static int i915_pm_suspend(struct device *dev)
>  {
>  	struct pci_dev *pdev = to_pci_dev(dev);
>  	struct drm_device *drm_dev = pci_get_drvdata(pdev);
> -	int error;
>  
>  	if (!drm_dev || !drm_dev->dev_private) {
>  		dev_err(dev, "DRM not initialized, aborting suspend.\n");
> @@ -786,9 +803,16 @@ static int i915_pm_suspend(struct device *dev)
>  	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
>  		return 0;
>  
> -	error = i915_drm_freeze(drm_dev);
> -	if (error)
> -		return error;
> +	return i915_drm_freeze(drm_dev);
> +}
> +
> +static int i915_pm_suspend_late(struct device *dev)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	struct drm_device *drm_dev = pci_get_drvdata(pdev);
> +
> +	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
> +		return 0;
>  
>  	pci_disable_device(pdev);
>  	pci_set_power_state(pdev, PCI_D3hot);
> @@ -796,6 +820,14 @@ static int i915_pm_suspend(struct device *dev)
>  	return 0;
>  }
>  
> +static int i915_pm_resume_early(struct device *dev)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	struct drm_device *drm_dev = pci_get_drvdata(pdev);
> +
> +	return i915_resume_early(drm_dev);
> +}
> +
>  static int i915_pm_resume(struct device *dev)
>  {
>  	struct pci_dev *pdev = to_pci_dev(dev);
> @@ -817,6 +849,14 @@ static int i915_pm_freeze(struct device *dev)
>  	return i915_drm_freeze(drm_dev);
>  }
>  
> +static int i915_pm_thaw_early(struct device *dev)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	struct drm_device *drm_dev = pci_get_drvdata(pdev);
> +
> +	return i915_drm_thaw_early(drm_dev);
> +}
> +
>  static int i915_pm_thaw(struct device *dev)
>  {
>  	struct pci_dev *pdev = to_pci_dev(dev);
> @@ -887,10 +927,14 @@ static int i915_runtime_resume(struct device *device)
>  
>  static const struct dev_pm_ops i915_pm_ops = {
>  	.suspend = i915_pm_suspend,
> +	.suspend_late = i915_pm_suspend_late,
> +	.resume_early = i915_pm_resume_early,
>  	.resume = i915_pm_resume,
>  	.freeze = i915_pm_freeze,
> +	.thaw_early = i915_pm_thaw_early,
>  	.thaw = i915_pm_thaw,
>  	.poweroff = i915_pm_poweroff,
> +	.restore_early = i915_pm_resume_early,
>  	.restore = i915_pm_resume,
>  	.runtime_suspend = i915_runtime_suspend,
>  	.runtime_resume = i915_runtime_resume,
> @@ -933,7 +977,7 @@ static struct drm_driver driver = {
>  
>  	/* Used in place of i915_pm_ops for non-DRIVER_MODESET */
>  	.suspend = i915_suspend,
> -	.resume = i915_resume,
> +	.resume = i915_resume_legacy,
>  
>  	.device_is_agp = i915_driver_device_is_agp,
>  	.master_create = i915_master_create,
> -- 
> 1.8.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux