| Message ID | 20240924204222.246862-23-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Reconcile i915's and xe's display power mgt sequences | expand |
-----Original Message----- From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Rodrigo Vivi Sent: Tuesday, September 24, 2024 1:36 PM To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org Cc: Deak, Imre <imre.deak@intel.com>; Vivi, Rodrigo <rodrigo.vivi@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.vivi@intel.com> LGTM. Reviewed-by: Jonathan Cavitt <jonathan.cavitt@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 > >
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);
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.vivi@intel.com> --- .../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(-)