All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jani Nikula <jani.nikula@intel.com>
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
	Imre Deak <imre.deak@intel.com>
Subject: Re: [CI] drm/{i915, xe}: move intel_opregion_suspend() to intel_display_driver_pm_suspend_late()
Date: Thu, 8 Oct 2026 15:12:49 +0300	[thread overview]
Message-ID: <aseIwWEv58T-wioc@intel.com> (raw)
In-Reply-To: <20261008114735.2662427-1-jani.nikula@intel.com>

On Thu, Oct 08, 2026 at 02:47:35PM +0300, Jani Nikula wrote:
> Try to unify the suspend paths and behaviour by moving the
> intel_opregion_suspend() calls to
> intel_display_driver_pm_suspend_late().
> 
> This is a change in the suspend sequences. The idea behind adding this
> to intel_display_driver_pm_suspend_late() is primarily based on not
> having to pass the s2idle parameter to more functions than that.
> 
> This also changes behaviour for i915 hibernation, going for PCI_D3cold
> instead of PCI_D1 on hibernate.

Pretty sure I pointed out before that this isn't true. PCI_D3cold is
what the current code also does.

I suppose it *might* be the case if one does something like 
'echo reboot > /sys/power/disk', but I've not actually checked
what that sort of thing does to acpi_target_system_state()...

> It's probably the correct thing to do
> anyway, I don't think it should matter all that much, but fingers
> crossed.
> 
> Reviewed-by: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_driver.c | 2 ++
>  drivers/gpu/drm/i915/i915_driver.c                  | 4 ----
>  drivers/gpu/drm/xe/display/xe_display.c             | 3 ---
>  3 files changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
> index 77009dca7d0d..abab457bce3a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_driver.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
> @@ -783,6 +783,8 @@ void intel_display_driver_pm_suspend_late(struct intel_display *display, bool s2
>  	if (!HAS_DISPLAY(display))
>  		return;
>  
> +	intel_opregion_suspend(display, s2idle ? PCI_D1 : PCI_D3cold);
> +
>  	intel_display_power_suspend_late(display, s2idle);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index 9e047c27a153..293099fe6639 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -1075,7 +1075,6 @@ static int i915_drm_suspend(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_display *display = dev_priv->display;
> -	pci_power_t opregion_target_state;
>  
>  	disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>  
> @@ -1089,9 +1088,6 @@ static int i915_drm_suspend(struct drm_device *dev)
>  
>  	i9xx_display_sr_save(display);
>  
> -	opregion_target_state = suspend_to_idle(dev_priv) ? PCI_D1 : PCI_D3cold;
> -	intel_opregion_suspend(display, opregion_target_state);
> -
>  	dev_priv->suspend_count++;
>  
>  	enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> index aab416485e3e..7d19f3c42e0e 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.c
> +++ b/drivers/gpu/drm/xe/display/xe_display.c
> @@ -238,14 +238,11 @@ static bool suspend_to_idle(void)
>  void xe_display_pm_suspend(struct xe_device *xe)
>  {
>  	struct intel_display *display = xe->display;
> -	bool s2idle = suspend_to_idle();
>  
>  	if (!xe->info.probe_display)
>  		return;
>  
>  	intel_display_driver_pm_suspend(display);
> -
> -	intel_opregion_suspend(display, s2idle ? PCI_D1 : PCI_D3cold);
>  }
>  
>  void xe_display_pm_suspend_late(struct xe_device *xe)
> -- 
> 2.47.3

-- 
Ville Syrjälä
Intel

  parent reply	other threads:[~2026-10-08 12:12 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-08 11:47 [CI] drm/{i915, xe}: move intel_opregion_suspend() to intel_display_driver_pm_suspend_late() Jani Nikula
2026-10-08 11:56 ` ✓ CI.KUnit: success for " Patchwork
2026-10-08 12:12 ` Ville Syrjälä [this message]
2026-10-08 14:00   ` [CI] " Jani Nikula
2026-10-08 23:03     ` Ville Syrjälä
2026-10-08 12:37 ` ✓ Xe.CI.BAT: success for " Patchwork
2026-10-08 13:53 ` ✓ Xe.CI.FULL: " Patchwork
2026-10-08 14:32 ` ✓ i915.CI.BAT: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aseIwWEv58T-wioc@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=imre.deak@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.