diff mbox series

[6/6] drm/i915/pm: Use pci_dev->skip_bus_pm for hibernate vs. D3 workaround

Message ID 20240925144526.2482-7-ville.syrjala@linux.intel.com (mailing list archive)
State Changes Requested
Headers show
Series drm/i915/pm: Clean up the hibernate vs. PCI D3 quirk | expand

Commit Message

Ville Syrjälä Sept. 25, 2024, 2:45 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

On some older laptops we have to leave the device in D0
during hibernation, or else the BIOS just hangs and never
finishes the hibernation.

Currently we are achieving that by skipping the
pci_set_power_state(D3). However we also need to call
pci_save_state() ahead of time, or else
pci_pm_suspend_noirq() will do the pci_set_power_state(D3)
anyway.

This is all rather ugly, and might cause us to deviate from
standard pci pm behaviour in unknown ways since we always
call pci_save_state() for any kind of suspend operation.

Stop calling pci_save_state()+pci_set_power_state() entirely
(apart from the switcheroo paths) and instead set
pci_dev->skip_bus_pm=true to prevent the D3 during hibernation
on old machines. Apart from that we'll just let the normal
pci pm code take care of everything for us.

Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: linux-pci@vger.kernel.org
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_driver.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Bjorn Helgaas Sept. 25, 2024, 7:28 p.m. UTC | #1
On Wed, Sep 25, 2024 at 05:45:26PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> On some older laptops we have to leave the device in D0
> during hibernation, or else the BIOS just hangs and never
> finishes the hibernation.
> 
> Currently we are achieving that by skipping the
> pci_set_power_state(D3). However we also need to call
> pci_save_state() ahead of time, or else
> pci_pm_suspend_noirq() will do the pci_set_power_state(D3)
> anyway.
> 
> This is all rather ugly, and might cause us to deviate from
> standard pci pm behaviour in unknown ways since we always
> call pci_save_state() for any kind of suspend operation.
> 
> Stop calling pci_save_state()+pci_set_power_state() entirely
> (apart from the switcheroo paths) and instead set
> pci_dev->skip_bus_pm=true to prevent the D3 during hibernation
> on old machines. Apart from that we'll just let the normal
> pci pm code take care of everything for us.
> 
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: linux-pci@vger.kernel.org
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_driver.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index c3e7225ea1ba..05948d00a874 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -1123,13 +1123,9 @@ static int i915_drm_suspend_noirq(struct drm_device *dev, bool hibernation)
>  	 * Lenovo Thinkpad X301, X61s, X60, T60, X41
>  	 * Fujitsu FSC S7110
>  	 * Acer Aspire 1830T
> -	 *
> -	 * pci_save_state() is needed to prevent driver/pci from
> -	 * automagically putting the device into D3.
>  	 */
> -	pci_save_state(pdev);
> -	if (!(hibernation && GRAPHICS_VER(dev_priv) < 6))
> -		pci_set_power_state(pdev, PCI_D3hot);
> +	if (hibernation && GRAPHICS_VER(dev_priv) < 6)
> +		pdev->skip_bus_pm = true;

.skip_bus_pm was previously strictly internal to
drivers/pci/pci-driver.c.  Not sure about using it outside
drivers/pci/; would want Rafael to chime in.

>  	return 0;
>  }
> @@ -1137,6 +1133,7 @@ static int i915_drm_suspend_noirq(struct drm_device *dev, bool hibernation)
>  int i915_driver_suspend_switcheroo(struct drm_i915_private *i915,
>  				   pm_message_t state)
>  {
> +	struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
>  	int error;
>  
>  	if (drm_WARN_ON_ONCE(&i915->drm, state.event != PM_EVENT_SUSPEND &&
> @@ -1158,6 +1155,9 @@ int i915_driver_suspend_switcheroo(struct drm_i915_private *i915,
>  	if (error)
>  		return error;
>  
> +	pci_save_state(pdev);
> +	pci_set_power_state(pdev, PCI_D3hot);
> +
>  	return 0;
>  }
>  
> -- 
> 2.44.2
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index c3e7225ea1ba..05948d00a874 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -1123,13 +1123,9 @@  static int i915_drm_suspend_noirq(struct drm_device *dev, bool hibernation)
 	 * Lenovo Thinkpad X301, X61s, X60, T60, X41
 	 * Fujitsu FSC S7110
 	 * Acer Aspire 1830T
-	 *
-	 * pci_save_state() is needed to prevent driver/pci from
-	 * automagically putting the device into D3.
 	 */
-	pci_save_state(pdev);
-	if (!(hibernation && GRAPHICS_VER(dev_priv) < 6))
-		pci_set_power_state(pdev, PCI_D3hot);
+	if (hibernation && GRAPHICS_VER(dev_priv) < 6)
+		pdev->skip_bus_pm = true;
 
 	return 0;
 }
@@ -1137,6 +1133,7 @@  static int i915_drm_suspend_noirq(struct drm_device *dev, bool hibernation)
 int i915_driver_suspend_switcheroo(struct drm_i915_private *i915,
 				   pm_message_t state)
 {
+	struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
 	int error;
 
 	if (drm_WARN_ON_ONCE(&i915->drm, state.event != PM_EVENT_SUSPEND &&
@@ -1158,6 +1155,9 @@  int i915_driver_suspend_switcheroo(struct drm_i915_private *i915,
 	if (error)
 		return error;
 
+	pci_save_state(pdev);
+	pci_set_power_state(pdev, PCI_D3hot);
+
 	return 0;
 }