Re: [RESEND v2 3/8] drm/i915: move some display runtime suspend operations earlier

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Wed, Jul 01, 2026 at 04:53:01PM +0300, Jani Nikula wrote:
> In preparation for abstracting display runtime suspend/resume calls,
> relocate opregion adapter notification and hotplug poll enable slightly
> earlier. Even though this is intended to be a non-functional change, do
> it as a separate step to pinpoint any issues here.

fingers crossed! :)

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_driver.c | 54 ++++++++++++++++--------------
>  1 file changed, 28 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index 10427f8f5699..b3d87acc142c 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -1518,11 +1518,39 @@ static int i915_pm_runtime_suspend(struct device *kdev)
>  
>  	intel_display_power_runtime_suspend(display);
>  
> +	/*
> +	 * FIXME: We really should find a document that references the arguments
> +	 * used below!
> +	 */
> +	if (IS_BROADWELL(dev_priv)) {
> +		/*
> +		 * On Broadwell, if we use PCI_D1 the PCH DDI ports will stop
> +		 * being detected, and the call we do at i915_pm_runtime_resume()
> +		 * won't be able to restore them. Since PCI_D3hot matches the
> +		 * actual specification and appears to be working, use it.
> +		 */
> +		intel_opregion_notify_adapter(display, PCI_D3hot);
> +	} else {
> +		/*
> +		 * current versions of firmware which depend on this opregion
> +		 * notification have repurposed the D1 definition to mean
> +		 * "runtime suspended" vs. what you would normally expect (D3)
> +		 * to distinguish it from notifications that might be sent via
> +		 * the suspend path.
> +		 */
> +		intel_opregion_notify_adapter(display, PCI_D1);
> +	}
> +
> +	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
> +		intel_hpd_poll_enable(display);
> +
>  	ret = vlv_suspend_complete(dev_priv);
>  	if (ret) {
>  		drm_err(&dev_priv->drm,
>  			"Runtime suspend failed, disabling it (%d)\n", ret);
>  
> +		intel_opregion_notify_adapter(display, PCI_D0);
> +
>  		intel_display_power_runtime_resume(display);
>  
>  		for_each_gt(gt, dev_priv, i)
> @@ -1554,34 +1582,8 @@ static int i915_pm_runtime_suspend(struct device *kdev)
>  	if (root_pdev)
>  		pci_d3cold_disable(root_pdev);
>  
> -	/*
> -	 * FIXME: We really should find a document that references the arguments
> -	 * used below!
> -	 */
> -	if (IS_BROADWELL(dev_priv)) {
> -		/*
> -		 * On Broadwell, if we use PCI_D1 the PCH DDI ports will stop
> -		 * being detected, and the call we do at i915_pm_runtime_resume()
> -		 * won't be able to restore them. Since PCI_D3hot matches the
> -		 * actual specification and appears to be working, use it.
> -		 */
> -		intel_opregion_notify_adapter(display, PCI_D3hot);
> -	} else {
> -		/*
> -		 * current versions of firmware which depend on this opregion
> -		 * notification have repurposed the D1 definition to mean
> -		 * "runtime suspended" vs. what you would normally expect (D3)
> -		 * to distinguish it from notifications that might be sent via
> -		 * the suspend path.
> -		 */
> -		intel_opregion_notify_adapter(display, PCI_D1);
> -	}
> -
>  	assert_forcewakes_inactive(&dev_priv->uncore);
>  
> -	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
> -		intel_hpd_poll_enable(display);
> -
>  	drm_dbg(&dev_priv->drm, "Device suspended\n");
>  	return 0;
>  }
> -- 
> 2.47.3
>