-----Original Message-----
From: Intel-gfx <intel-gfx-boun...@lists.freedesktop.org> On Behalf Of Rodrigo
Vivi
Sent: Tuesday, September 24, 2024 1:36 PM
To: intel-gfx@lists.freedesktop.org; intel...@lists.freedesktop.org
Cc: Deak, Imre <imre.d...@intel.com>; Vivi, Rodrigo <rodrigo.v...@intel.com>
Subject: [PATCH 22/31] drm/i915/display: Move runtime pm related calls under
intel_display_driver
>
> Continue moving more display stuff from i915_driver to intel_display.
> With the end goal of reconciling xe and i915 sequences.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.v...@intel.com>
LGTM.
Reviewed-by: Jonathan Cavitt <jonathan.cav...@intel.com>
-Jonathan Cavitt
> ---
> .../drm/i915/display/intel_display_driver.c | 58 +++++++++++++++++++
> .../drm/i915/display/intel_display_driver.h | 4 ++
> drivers/gpu/drm/i915/i915_driver.c | 46 ++-------------
> 3 files changed, 66 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c
> b/drivers/gpu/drm/i915/display/intel_display_driver.c
> index f3a586913c6f..62a7aa56f0da 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_driver.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
> @@ -856,6 +856,64 @@ void intel_display_driver_resume(struct drm_i915_private
> *i915)
> intel_power_domains_enable(i915);
> }
>
> +void intel_display_driver_runtime_suspend(struct drm_i915_private *i915)
> +{
> + intel_display_power_suspend(i915);
> +}
> +
> +void intel_display_driver_runtime_suspend_late(struct drm_i915_private *i915)
> +{
> + struct intel_display *display = &i915->display;
> +
> + /*
> + * FIXME: We really should find a document that references the arguments
> + * used below!
> + */
> + if (IS_BROADWELL(i915)) {
> + /*
> + * 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);
> + }
> +
> + if (!IS_VALLEYVIEW(i915) && !IS_CHERRYVIEW(i915))
> + intel_hpd_poll_enable(i915);
> +}
> +
> +void intel_display_driver_runtime_resume_early(struct drm_i915_private *i915)
> +{
> + intel_opregion_notify_adapter(&i915->display, PCI_D0);
> +
> + intel_display_power_resume(i915);
> +}
> +
> +void intel_display_driver_runtime_resume(struct drm_i915_private *i915)
> +{
> + /*
> + * 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(i915) && !IS_CHERRYVIEW(i915)) {
> + intel_hpd_init(i915);
> + intel_hpd_poll_disable(i915);
> + }
> +
> + skl_watermark_ipc_update(i915);
> +}
> +
> void intel_display_driver_shutdown(struct drm_i915_private *i915)
> {
> intel_power_domains_disable(i915);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.h
> b/drivers/gpu/drm/i915/display/intel_display_driver.h
> index e287574fcd35..b1441a55d72d 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_driver.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.h
> @@ -33,6 +33,10 @@ void intel_display_driver_resume(struct drm_i915_private
> *i915);
> void intel_display_driver_resume_noirq(struct drm_i915_private *i915);
> void intel_display_driver_resume_noirq_legacy(struct drm_i915_private *i915);
> void intel_display_driver_resume_nogem(struct intel_display *display);
> +void intel_display_driver_runtime_suspend(struct drm_i915_private *i915);
> +void intel_display_driver_runtime_suspend_late(struct drm_i915_private
> *i915);
> +void intel_display_driver_runtime_resume_early(struct drm_i915_private
> *i915);
> +void intel_display_driver_runtime_resume(struct drm_i915_private *i915);
> void intel_display_driver_shutdown(struct drm_i915_private *i915);
> void intel_display_driver_shutdown_noirq(struct drm_i915_private *i915);
> void intel_display_driver_shutdown_nogem(struct drm_i915_private *i915);
> diff --git a/drivers/gpu/drm/i915/i915_driver.c
> b/drivers/gpu/drm/i915/i915_driver.c
> index 7fce210d355d..b3eaa55ebacb 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -1373,7 +1373,6 @@ static int i915_pm_restore(struct device *kdev)
> static int intel_runtime_suspend(struct device *kdev)
> {
> struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
> - struct intel_display *display = &dev_priv->display;
> struct intel_runtime_pm *rpm = &dev_priv->runtime_pm;
> struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
> struct pci_dev *root_pdev;
> @@ -1403,7 +1402,7 @@ static int intel_runtime_suspend(struct device *kdev)
> for_each_gt(gt, dev_priv, i)
> intel_uncore_suspend(gt->uncore);
>
> - intel_display_power_suspend(dev_priv);
> + intel_display_driver_runtime_suspend(dev_priv);
>
> ret = vlv_suspend_complete(dev_priv);
> if (ret) {
> @@ -1437,34 +1436,10 @@ 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_display_driver_runtime_suspend_late(dev_priv);
>
> assert_forcewakes_inactive(&dev_priv->uncore);
>
> - if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
> - intel_hpd_poll_enable(dev_priv);
> -
> drm_dbg(&dev_priv->drm, "Device suspended\n");
> return 0;
> }
> @@ -1472,7 +1447,6 @@ static int intel_runtime_suspend(struct device *kdev)
> static int intel_runtime_resume(struct device *kdev)
> {
> struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
> - struct intel_display *display = &dev_priv->display;
> struct intel_runtime_pm *rpm = &dev_priv->runtime_pm;
> struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
> struct pci_dev *root_pdev;
> @@ -1487,8 +1461,6 @@ 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);
> -
> root_pdev = pcie_find_root_port(pdev);
> if (root_pdev)
> pci_d3cold_enable(root_pdev);
> @@ -1497,7 +1469,7 @@ static int intel_runtime_resume(struct device *kdev)
> drm_dbg(&dev_priv->drm,
> "Unclaimed access during suspend, bios?\n");
>
> - intel_display_power_resume(dev_priv);
> + intel_display_driver_runtime_resume_early(dev_priv);
>
> ret = vlv_resume_prepare(dev_priv, true);
>
> @@ -1515,17 +1487,7 @@ static int intel_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(dev_priv);
> - intel_hpd_poll_disable(dev_priv);
> - }
> -
> - skl_watermark_ipc_update(dev_priv);
> + intel_display_driver_runtime_resume_early(dev_priv);
>
> enable_rpm_wakeref_asserts(rpm);
>
> --
> 2.46.0
>
>
