On Tue, Jul 08, 2025 at 07:03:18PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <[email protected]>
> 
> Extract the opregion runtime PM stuff to new functions. We'll
> need to add a bit more to the suspend side, and we don't want
> to clutter the top level runtime PM code with such details.
> 
> Signed-off-by: Ville Syrjälä <[email protected]>

Reviewed-by: Imre Deak <[email protected]>

> ---
>  drivers/gpu/drm/i915/display/intel_opregion.c | 41 +++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_opregion.h | 11 +++++
>  drivers/gpu/drm/i915/i915_driver.c            | 25 +----------
>  3 files changed, 54 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c 
> b/drivers/gpu/drm/i915/display/intel_opregion.c
> index 81efdb17fc0c..9e39ab55a099 100644
> --- a/drivers/gpu/drm/i915/display/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/display/intel_opregion.c
> @@ -1255,6 +1255,47 @@ void intel_opregion_suspend(struct intel_display 
> *display, pci_power_t state)
>               intel_opregion_suspend_display(display);
>  }
>  
> +void intel_opregion_runtime_resume(struct intel_display *display)
> +{
> +     struct intel_opregion *opregion = display->opregion;
> +
> +     if (!opregion)
> +             return;
> +
> +     intel_opregion_notify_adapter(display, PCI_D0);
> +}
> +
> +void intel_opregion_runtime_suspend(struct intel_display *display)
> +{
> +     struct intel_opregion *opregion = display->opregion;
> +
> +     if (!opregion)
> +             return;
> +
> +     /*
> +      * 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 intel_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);
> +     }
> +}
> +
>  void intel_opregion_unregister(struct intel_display *display)
>  {
>       struct intel_opregion *opregion = display->opregion;
> diff --git a/drivers/gpu/drm/i915/display/intel_opregion.h 
> b/drivers/gpu/drm/i915/display/intel_opregion.h
> index 8101eeebfd8b..7067ffe07744 100644
> --- a/drivers/gpu/drm/i915/display/intel_opregion.h
> +++ b/drivers/gpu/drm/i915/display/intel_opregion.h
> @@ -44,6 +44,9 @@ void intel_opregion_resume(struct intel_display *display);
>  void intel_opregion_suspend(struct intel_display *display,
>                           pci_power_t state);
>  
> +void intel_opregion_runtime_resume(struct intel_display *display);
> +void intel_opregion_runtime_suspend(struct intel_display *display);
> +
>  bool intel_opregion_asle_present(struct intel_display *display);
>  void intel_opregion_asle_intr(struct intel_display *display);
>  int intel_opregion_notify_encoder(struct intel_encoder *encoder,
> @@ -88,6 +91,14 @@ static inline void intel_opregion_suspend(struct 
> intel_display *display,
>  {
>  }
>  
> +static inline void intel_opregion_runtime_resume(struct intel_display 
> *display)
> +{
> +}
> +
> +static inline void intel_opregion_runtime_suspend(struct intel_display 
> *display)
> +{
> +}
> +
>  static inline bool intel_opregion_asle_present(struct intel_display *display)
>  {
>       return false;
> diff --git a/drivers/gpu/drm/i915/i915_driver.c 
> b/drivers/gpu/drm/i915/i915_driver.c
> index c6263c6d3384..da0b7d9da3d5 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -1553,28 +1553,7 @@ static int intel_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 intel_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);
> -     }
> +     intel_opregion_runtime_suspend(display);
>  
>       assert_forcewakes_inactive(&dev_priv->uncore);
>  
> @@ -1603,7 +1582,7 @@ static int intel_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);
> +     intel_opregion_runtime_resume(display);
>  
>       root_pdev = pcie_find_root_port(pdev);
>       if (root_pdev)
> -- 
> 2.49.0
> 

Reply via email to