On Fri, May 29, 2026 at 02:04:06PM +0300, Jani Nikula wrote:
> Add new functions intel_display_driver_pm_runtime_suspend(),
> intel_display_driver_pm_runtime_suspend_late(),
> intel_display_driver_pm_runtime_resume_early(), and
> intel_display_driver_pm_runtime_resume(). Initially, only migrate i915,
> as there are some differences with xe that will be addressed later.
> 
> There are a few functional changes, which should be benign:
> 
> - i915_pm_runtime_suspend() moves assert_forcewakes_inactive() call
>   before opregion calls.
> 
> - i915_pm_runtime_resume() moves intel_opregion_notify_adapter() call
>   slightly later.
> 
> In the interest of not introducing more severe functional changes, the
> calls become slightly asymmetric. We might want to address this later.
> 
> Signed-off-by: Jani Nikula <[email protected]>

Reviewed-by: Ville Syrjälä <[email protected]>

> ---
>  .../drm/i915/display/intel_display_driver.c   | 56 +++++++++++++++++++
>  .../drm/i915/display/intel_display_driver.h   |  5 ++
>  drivers/gpu/drm/i915/i915_driver.c            | 44 ++-------------
>  3 files changed, 65 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c 
> b/drivers/gpu/drm/i915/display/intel_display_driver.c
> index 84807a9bff2b..a27ddf21b08b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_driver.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
> @@ -884,3 +884,59 @@ void intel_display_driver_pm_resume(struct intel_display 
> *display)
>  
>       intel_display_power_enable(display);
>  }
> +
> +void intel_display_driver_pm_runtime_suspend(struct intel_display *display)
> +{
> +     intel_display_power_runtime_suspend(display);
> +}
> +
> +void intel_display_driver_pm_runtime_suspend_late(struct intel_display 
> *display)
> +{
> +     /*
> +      * FIXME: We really should find a document that references the arguments
> +      * used below!
> +      */
> +     if (display->platform.broadwell) {
> +             /*
> +              * 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 (!display->platform.valleyview && !display->platform.cherryview)
> +             intel_hpd_poll_enable(display);
> +}
> +
> +void intel_display_driver_pm_runtime_resume_early(struct intel_display 
> *display)
> +{
> +     intel_opregion_notify_adapter(display, PCI_D0);
> +
> +     intel_display_power_runtime_resume(display);
> +}
> +
> +void intel_display_driver_pm_runtime_resume(struct intel_display *display)
> +{
> +     /*
> +      * On VLV/CHV display interrupts are part of the display
> +      * power well, so hpd is reinitialized from there. For
> +      * everyone else do it here.
> +      */
> +     if (!display->platform.valleyview && !display->platform.cherryview) {
> +             intel_hpd_init(display);
> +             intel_hpd_poll_disable(display);
> +     }
> +
> +     skl_watermark_ipc_update(display);
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.h 
> b/drivers/gpu/drm/i915/display/intel_display_driver.h
> index 7eca3d17dd82..1b494337d629 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_driver.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.h
> @@ -43,5 +43,10 @@ void intel_display_driver_suspend_access(struct 
> intel_display *display);
>  void intel_display_driver_resume_access(struct intel_display *display);
>  bool intel_display_driver_check_access(struct intel_display *display);
>  
> +void intel_display_driver_pm_runtime_suspend(struct intel_display *display);
> +void intel_display_driver_pm_runtime_suspend_late(struct intel_display 
> *display);
> +void intel_display_driver_pm_runtime_resume_early(struct intel_display 
> *display);
> +void intel_display_driver_pm_runtime_resume(struct intel_display *display);
> +
>  #endif /* __INTEL_DISPLAY_DRIVER_H__ */
>  
> diff --git a/drivers/gpu/drm/i915/i915_driver.c 
> b/drivers/gpu/drm/i915/i915_driver.c
> index 6b9e1b268a89..519a519d2d96 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -1521,7 +1521,7 @@ static int i915_pm_runtime_suspend(struct device *kdev)
>       for_each_gt(gt, dev_priv, i)
>               intel_uncore_suspend(gt->uncore);
>  
> -     intel_display_power_runtime_suspend(display);
> +     intel_display_driver_pm_runtime_suspend(display);
>  
>       ret = vlv_suspend_complete(dev_priv);
>       if (ret) {
> @@ -1555,33 +1555,9 @@ 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);
> +     intel_display_driver_pm_runtime_suspend_late(display);
>  
>       drm_dbg(&dev_priv->drm, "Device suspended\n");
>       return 0;
> @@ -1605,8 +1581,6 @@ static int i915_pm_runtime_resume(struct device *kdev)
>       drm_WARN_ON_ONCE(&dev_priv->drm, atomic_read(&rpm->wakeref_count));
>       disable_rpm_wakeref_asserts(rpm);
>  
> -     intel_opregion_notify_adapter(display, PCI_D0);
> -
>       root_pdev = pcie_find_root_port(pdev);
>       if (root_pdev)
>               pci_d3cold_enable(root_pdev);
> @@ -1615,7 +1589,7 @@ static int i915_pm_runtime_resume(struct device *kdev)
>               drm_dbg(&dev_priv->drm,
>                       "Unclaimed access during suspend, bios?\n");
>  
> -     intel_display_power_runtime_resume(display);
> +     intel_display_driver_pm_runtime_resume_early(display);
>  
>       ret = vlv_resume_prepare(dev_priv, true);
>  
> @@ -1633,17 +1607,7 @@ static int i915_pm_runtime_resume(struct device *kdev)
>  
>       intel_pxp_runtime_resume(dev_priv->pxp);
>  
> -     /*
> -      * On VLV/CHV display interrupts are part of the display
> -      * power well, so hpd is reinitialized from there. For
> -      * everyone else do it here.
> -      */
> -     if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv)) {
> -             intel_hpd_init(display);
> -             intel_hpd_poll_disable(display);
> -     }
> -
> -     skl_watermark_ipc_update(display);
> +     intel_display_driver_pm_runtime_resume(display);
>  
>       enable_rpm_wakeref_asserts(rpm);
>  
> -- 
> 2.47.3

-- 
Ville Syrjälä
Intel

Reply via email to