On Tue, 2025-03-11 at 21:56 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrj...@linux.intel.com>
>
> If the driver doesn't call pci_save_state() drivers/pci will
> normally save+power manage the device from the _noirq() pm hooks.
>
> We can't let that happen as some old BIOSes fail to hibernate
> when the device is in D3. However, we can get very close to
> the standard behaviour by doing our explicit pci_save_state()
> and pci_set_power_state() stuff from driver provided _noirq()
> hooks.
>
> This results in a change of behaviour where we no longer go
> into D3 at the end of freeze_late, so when it comes time
> to thaw() we'll already be in D0, and thus we can drop the
> explicit pci_set_power_state(D0) call.
>
> Presumably switcheroo suspend will want to go into D3 so
> call the _noirq() stuff from the switcheroo suspend hook,
> and since we dropped the pci_set_power_state(D0) from
> resume_early() we'll need to add one back into the
> switcheroo resume hook.
>
> Cc: Rodrigo Vivi <rodrigo.v...@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
Reviewed-by: Jouni Högander <jouni.hogan...@intel.com>
> ---
> drivers/gpu/drm/i915/i915_driver.c | 76 ++++++++++++++++++++--------
> --
> 1 file changed, 51 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_driver.c
> b/drivers/gpu/drm/i915/i915_driver.c
> index e06f2956382c..995205e24ebf 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -1124,6 +1124,21 @@ static int i915_drm_suspend_late(struct
> drm_device *dev, bool hibernation)
>
> pci_disable_device(pdev);
>
> + return 0;
> +
> +fail:
> + enable_rpm_wakeref_asserts(rpm);
> + if (!dev_priv->uncore.user_forcewake_count)
> + intel_runtime_pm_driver_release(rpm);
> +
> + return ret;
> +}
> +
> +static int i915_drm_suspend_noirq(struct drm_device *dev, bool
> hibernation)
> +{
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
> +
> /*
> * During hibernation on some platforms the BIOS may try to
> access
> * the device even though it's already in D3 and hang the
> machine. So
> @@ -1144,13 +1159,6 @@ static int i915_drm_suspend_late(struct
> drm_device *dev, bool hibernation)
> pci_set_power_state(pdev, PCI_D3hot);
>
> return 0;
> -
> -fail:
> - enable_rpm_wakeref_asserts(rpm);
> - if (!dev_priv->uncore.user_forcewake_count)
> - intel_runtime_pm_driver_release(rpm);
> -
> - return ret;
> }
>
> int i915_driver_suspend_switcheroo(struct drm_i915_private *i915,
> @@ -1169,7 +1177,15 @@ int i915_driver_suspend_switcheroo(struct
> drm_i915_private *i915,
> if (error)
> return error;
>
> - return i915_drm_suspend_late(&i915->drm, false);
> + error = i915_drm_suspend_late(&i915->drm, false);
> + if (error)
> + return error;
> +
> + error = i915_drm_suspend_noirq(&i915->drm, false);
> + if (error)
> + return error;
> +
> + return 0;
> }
>
> static int i915_drm_resume(struct drm_device *dev)
> @@ -1277,23 +1293,6 @@ static int i915_drm_resume_early(struct
> drm_device *dev)
> * similar so that power domains can be employed.
> */
>
> - /*
> - * Note that we need to set the power state explicitly,
> since we
> - * powered off the device during freeze and the PCI core
> won't power
> - * it back up for us during thaw. Powering off the device
> during
> - * freeze is not a hard requirement though, and during the
> - * suspend/resume phases the PCI core makes sure we get here
> with the
> - * device powered on. So in case we change our freeze logic
> and keep
> - * the device powered we can also remove the following set
> power state
> - * call.
> - */
> - ret = pci_set_power_state(pdev, PCI_D0);
> - if (ret) {
> - drm_err(&dev_priv->drm,
> - "failed to set PCI D0 power state (%d)\n",
> ret);
> - return ret;
> - }
> -
> /*
> * Note that pci_enable_device() first enables any parent
> bridge
> * device and only then sets the power state for this
> device. The
> @@ -1331,11 +1330,16 @@ static int i915_drm_resume_early(struct
> drm_device *dev)
>
> int i915_driver_resume_switcheroo(struct drm_i915_private *i915)
> {
> + struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
> int ret;
>
> if (i915->drm.switch_power_state == DRM_SWITCH_POWER_OFF)
> return 0;
>
> + ret = pci_set_power_state(pdev, PCI_D0);
> + if (ret)
> + return ret;
> +
> ret = i915_drm_resume_early(&i915->drm);
> if (ret)
> return ret;
> @@ -1392,6 +1396,16 @@ static int i915_pm_suspend_late(struct device
> *kdev)
> return i915_drm_suspend_late(&i915->drm, false);
> }
>
> +static int i915_pm_suspend_noirq(struct device *kdev)
> +{
> + struct drm_i915_private *i915 = kdev_to_i915(kdev);
> +
> + if (i915->drm.switch_power_state == DRM_SWITCH_POWER_OFF)
> + return 0;
> +
> + return i915_drm_suspend_noirq(&i915->drm, false);
> +}
> +
> static int i915_pm_poweroff_late(struct device *kdev)
> {
> struct drm_i915_private *i915 = kdev_to_i915(kdev);
> @@ -1402,6 +1416,16 @@ static int i915_pm_poweroff_late(struct device
> *kdev)
> return i915_drm_suspend_late(&i915->drm, true);
> }
>
> +static int i915_pm_poweroff_noirq(struct device *kdev)
> +{
> + struct drm_i915_private *i915 = kdev_to_i915(kdev);
> +
> + if (i915->drm.switch_power_state == DRM_SWITCH_POWER_OFF)
> + return 0;
> +
> + return i915_drm_suspend_noirq(&i915->drm, true);
> +}
> +
> static int i915_pm_resume_early(struct device *kdev)
> {
> struct drm_i915_private *i915 = kdev_to_i915(kdev);
> @@ -1667,6 +1691,7 @@ const struct dev_pm_ops i915_pm_ops = {
> .prepare = i915_pm_prepare,
> .suspend = i915_pm_suspend,
> .suspend_late = i915_pm_suspend_late,
> + .suspend_noirq = i915_pm_suspend_noirq,
> .resume_early = i915_pm_resume_early,
> .resume = i915_pm_resume,
> .complete = i915_pm_complete,
> @@ -1692,6 +1717,7 @@ const struct dev_pm_ops i915_pm_ops = {
> .thaw = i915_pm_thaw,
> .poweroff = i915_pm_suspend,
> .poweroff_late = i915_pm_poweroff_late,
> + .poweroff_noirq = i915_pm_poweroff_noirq,
> .restore_early = i915_pm_restore_early,
> .restore = i915_pm_restore,
>
