| Message ID | 20240924204222.246862-32-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 31/31] drm/{i915, xe}/display: Consolidade entire runtime pm sequence > > No functional change. > > Consolidate the entire runtime pm sequences under > intel_display_driver. > > Simplifications and optimizations around the D3cold sequences are > likely still possible. But before that can be done, consolidate > everything at the intel_display_driver side. > > Xe display power management functions are now only a wrapper > checking for xe's display probe parameter. > > 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 | 66 ++++++++++++++++--- > .../drm/i915/display/intel_display_driver.h | 12 ++-- > drivers/gpu/drm/i915/i915_driver.c | 8 +-- > drivers/gpu/drm/xe/display/xe_display.c | 56 ++-------------- > 4 files changed, 74 insertions(+), 68 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c > index 62a7aa56f0da..3861fdbefaff 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_driver.c > +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c > @@ -856,12 +856,45 @@ 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) > +static void intel_display_to_d3cold(struct drm_i915_private *i915) > { > - intel_display_power_suspend(i915); > + struct intel_display *display = &i915->display; > + > + /* We do a lot of poking in a lot of registers, make sure they work properly. */ > + intel_power_domains_disable(i915); > + > + intel_hpd_cancel_work(i915); > + > + intel_opregion_suspend(display, PCI_D3cold); > + > + intel_dmc_suspend(display); > +} > + > +static void intel_display_from_d3cold(struct drm_i915_private *i915) > +{ > + struct intel_display *display = &i915->display; > + > + intel_dmc_resume(display); > + > + if (HAS_DISPLAY(i915)) > + drm_mode_config_reset(&i915->drm); > + > + intel_display_driver_init_hw(i915); > + > + intel_opregion_resume(display); > + > + intel_power_domains_enable(i915); > +} > + > +void intel_display_driver_runtime_suspend(struct drm_i915_private *i915, bool d3cold_allowed) > +{ > + if (d3cold_allowed) > + intel_display_to_d3cold(i915); > + else > + intel_display_power_suspend(i915); > } > > -void intel_display_driver_runtime_suspend_late(struct drm_i915_private *i915) > +static void display_runtime_suspend_notify_opregion(struct drm_i915_private *i915) > { > struct intel_display *display = &i915->display; > > @@ -887,20 +920,37 @@ void intel_display_driver_runtime_suspend_late(struct drm_i915_private *i915) > */ > intel_opregion_notify_adapter(display, PCI_D1); > } > +} > + > +void intel_display_driver_runtime_suspend_late(struct drm_i915_private *i915, > + bool d3cold_allowed) > +{ > + if (d3cold_allowed) > + intel_display_power_suspend_late(i915, false); > + else > + display_runtime_suspend_notify_opregion(i915); > > if (!IS_VALLEYVIEW(i915) && !IS_CHERRYVIEW(i915)) > intel_hpd_poll_enable(i915); > } > > -void intel_display_driver_runtime_resume_early(struct drm_i915_private *i915) > +void intel_display_driver_runtime_resume_early(struct drm_i915_private *i915, > + bool d3cold_allowed) > { > - intel_opregion_notify_adapter(&i915->display, PCI_D0); > - > - intel_display_power_resume(i915); > + if (d3cold_allowed) { > + intel_display_power_resume_early(i915); > + } else { > + intel_opregion_notify_adapter(&i915->display, PCI_D0); > + intel_display_power_resume(i915); > + } > } > > -void intel_display_driver_runtime_resume(struct drm_i915_private *i915) > +void intel_display_driver_runtime_resume(struct drm_i915_private *i915, > + bool d3cold_allowed) > { > + if (d3cold_allowed) > + intel_display_from_d3cold(i915); > + > /* > * On VLV/CHV display interrupts are part of the display > * power well, so hpd is reinitialized from there. For > diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.h b/drivers/gpu/drm/i915/display/intel_display_driver.h > index b1441a55d72d..21aa0e551898 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_driver.h > +++ b/drivers/gpu/drm/i915/display/intel_display_driver.h > @@ -33,10 +33,14 @@ 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_runtime_suspend(struct drm_i915_private *i915, > + bool d3cold_allowed); > +void intel_display_driver_runtime_suspend_late(struct drm_i915_private *i915, > + bool d3cold_allowed); > +void intel_display_driver_runtime_resume_early(struct drm_i915_private *i915, > + bool d3cold_allowed); > +void intel_display_driver_runtime_resume(struct drm_i915_private *i915, > + bool d3cold_allowed); > 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 b3eaa55ebacb..719b1c21b695 100644 > --- a/drivers/gpu/drm/i915/i915_driver.c > +++ b/drivers/gpu/drm/i915/i915_driver.c > @@ -1402,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_driver_runtime_suspend(dev_priv); > + intel_display_driver_runtime_suspend(dev_priv, false); > > ret = vlv_suspend_complete(dev_priv); > if (ret) { > @@ -1436,7 +1436,7 @@ static int intel_runtime_suspend(struct device *kdev) > if (root_pdev) > pci_d3cold_disable(root_pdev); > > - intel_display_driver_runtime_suspend_late(dev_priv); > + intel_display_driver_runtime_suspend_late(dev_priv, false); > > assert_forcewakes_inactive(&dev_priv->uncore); > > @@ -1469,7 +1469,7 @@ static int intel_runtime_resume(struct device *kdev) > drm_dbg(&dev_priv->drm, > "Unclaimed access during suspend, bios?\n"); > > - intel_display_driver_runtime_resume_early(dev_priv); > + intel_display_driver_runtime_resume_early(dev_priv, false); > > ret = vlv_resume_prepare(dev_priv, true); > > @@ -1487,7 +1487,7 @@ static int intel_runtime_resume(struct device *kdev) > > intel_pxp_runtime_resume(dev_priv->pxp); > > - intel_display_driver_runtime_resume_early(dev_priv); > + intel_display_driver_runtime_resume_early(dev_priv, false); > > enable_rpm_wakeref_asserts(rpm); > > diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c > index ab85c7fb217a..9a652292d988 100644 > --- a/drivers/gpu/drm/xe/display/xe_display.c > +++ b/drivers/gpu/drm/xe/display/xe_display.c > @@ -283,36 +283,6 @@ static bool suspend_to_idle(void) > return false; > } > > -static void xe_display_to_d3cold(struct xe_device *xe) > -{ > - struct intel_display *display = &xe->display; > - > - /* We do a lot of poking in a lot of registers, make sure they work properly. */ > - intel_power_domains_disable(xe); > - > - intel_hpd_cancel_work(xe); > - > - intel_opregion_suspend(display, PCI_D3cold); > - > - intel_dmc_suspend(display); > -} > - > -static void xe_display_from_d3cold(struct xe_device *xe) > -{ > - struct intel_display *display = &xe->display; > - > - intel_dmc_resume(display); > - > - if (has_display(xe)) > - drm_mode_config_reset(&xe->drm); > - > - intel_display_driver_init_hw(xe); > - > - intel_opregion_resume(display); > - > - intel_power_domains_enable(xe); > -} > - > void xe_display_pm_suspend(struct xe_device *xe) > { > if (!xe->info.probe_display) > @@ -413,10 +383,7 @@ void xe_display_pm_runtime_suspend(struct xe_device *xe) > if (!xe->info.probe_display) > return; > > - if (xe->d3cold.allowed) > - xe_display_to_d3cold(xe); > - else > - intel_display_power_suspend(xe); > + intel_display_driver_runtime_suspend(xe, xe->d3cold.allowed); > } > > void xe_display_pm_runtime_suspend_late(struct xe_device *xe) > @@ -424,12 +391,7 @@ void xe_display_pm_runtime_suspend_late(struct xe_device *xe) > if (!xe->info.probe_display) > return; > > - if (xe->d3cold.allowed) > - intel_display_power_suspend_late(xe, false); > - else > - intel_opregion_notify_adapter(&xe->display, PCI_D1); > - > - intel_hpd_poll_enable(xe); > + intel_display_driver_runtime_suspend_late(xe, xe->d3cold.allowed); > } > > void xe_display_pm_runtime_resume_early(struct xe_device *xe) > @@ -437,12 +399,7 @@ void xe_display_pm_runtime_resume_early(struct xe_device *xe) > if (!xe->info.probe_display) > return; > > - if (xe->d3cold.allowed) { > - intel_display_power_resume_early(xe); > - } else { > - intel_opregion_notify_adapter(&xe->display, PCI_D0); > - intel_display_power_resume(xe); > - } > + intel_display_driver_runtime_resume_early(xe, xe->d3cold.allowed); > } > > void xe_display_pm_runtime_resume(struct xe_device *xe) > @@ -450,12 +407,7 @@ void xe_display_pm_runtime_resume(struct xe_device *xe) > if (!xe->info.probe_display) > return; > > - if (xe->d3cold.allowed) > - xe_display_from_d3cold(xe); > - > - intel_hpd_init(xe); > - intel_hpd_poll_disable(xe); > - skl_watermark_ipc_update(xe); > + intel_display_driver_runtime_resume(xe, xe->d3cold.allowed); > } > > static void display_device_remove(struct drm_device *dev, void *arg) > -- > 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 62a7aa56f0da..3861fdbefaff 100644 --- a/drivers/gpu/drm/i915/display/intel_display_driver.c +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c @@ -856,12 +856,45 @@ 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) +static void intel_display_to_d3cold(struct drm_i915_private *i915) { - intel_display_power_suspend(i915); + struct intel_display *display = &i915->display; + + /* We do a lot of poking in a lot of registers, make sure they work properly. */ + intel_power_domains_disable(i915); + + intel_hpd_cancel_work(i915); + + intel_opregion_suspend(display, PCI_D3cold); + + intel_dmc_suspend(display); +} + +static void intel_display_from_d3cold(struct drm_i915_private *i915) +{ + struct intel_display *display = &i915->display; + + intel_dmc_resume(display); + + if (HAS_DISPLAY(i915)) + drm_mode_config_reset(&i915->drm); + + intel_display_driver_init_hw(i915); + + intel_opregion_resume(display); + + intel_power_domains_enable(i915); +} + +void intel_display_driver_runtime_suspend(struct drm_i915_private *i915, bool d3cold_allowed) +{ + if (d3cold_allowed) + intel_display_to_d3cold(i915); + else + intel_display_power_suspend(i915); } -void intel_display_driver_runtime_suspend_late(struct drm_i915_private *i915) +static void display_runtime_suspend_notify_opregion(struct drm_i915_private *i915) { struct intel_display *display = &i915->display; @@ -887,20 +920,37 @@ void intel_display_driver_runtime_suspend_late(struct drm_i915_private *i915) */ intel_opregion_notify_adapter(display, PCI_D1); } +} + +void intel_display_driver_runtime_suspend_late(struct drm_i915_private *i915, + bool d3cold_allowed) +{ + if (d3cold_allowed) + intel_display_power_suspend_late(i915, false); + else + display_runtime_suspend_notify_opregion(i915); if (!IS_VALLEYVIEW(i915) && !IS_CHERRYVIEW(i915)) intel_hpd_poll_enable(i915); } -void intel_display_driver_runtime_resume_early(struct drm_i915_private *i915) +void intel_display_driver_runtime_resume_early(struct drm_i915_private *i915, + bool d3cold_allowed) { - intel_opregion_notify_adapter(&i915->display, PCI_D0); - - intel_display_power_resume(i915); + if (d3cold_allowed) { + intel_display_power_resume_early(i915); + } else { + intel_opregion_notify_adapter(&i915->display, PCI_D0); + intel_display_power_resume(i915); + } } -void intel_display_driver_runtime_resume(struct drm_i915_private *i915) +void intel_display_driver_runtime_resume(struct drm_i915_private *i915, + bool d3cold_allowed) { + if (d3cold_allowed) + intel_display_from_d3cold(i915); + /* * On VLV/CHV display interrupts are part of the display * power well, so hpd is reinitialized from there. For diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.h b/drivers/gpu/drm/i915/display/intel_display_driver.h index b1441a55d72d..21aa0e551898 100644 --- a/drivers/gpu/drm/i915/display/intel_display_driver.h +++ b/drivers/gpu/drm/i915/display/intel_display_driver.h @@ -33,10 +33,14 @@ 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_runtime_suspend(struct drm_i915_private *i915, + bool d3cold_allowed); +void intel_display_driver_runtime_suspend_late(struct drm_i915_private *i915, + bool d3cold_allowed); +void intel_display_driver_runtime_resume_early(struct drm_i915_private *i915, + bool d3cold_allowed); +void intel_display_driver_runtime_resume(struct drm_i915_private *i915, + bool d3cold_allowed); 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 b3eaa55ebacb..719b1c21b695 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -1402,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_driver_runtime_suspend(dev_priv); + intel_display_driver_runtime_suspend(dev_priv, false); ret = vlv_suspend_complete(dev_priv); if (ret) { @@ -1436,7 +1436,7 @@ static int intel_runtime_suspend(struct device *kdev) if (root_pdev) pci_d3cold_disable(root_pdev); - intel_display_driver_runtime_suspend_late(dev_priv); + intel_display_driver_runtime_suspend_late(dev_priv, false); assert_forcewakes_inactive(&dev_priv->uncore); @@ -1469,7 +1469,7 @@ static int intel_runtime_resume(struct device *kdev) drm_dbg(&dev_priv->drm, "Unclaimed access during suspend, bios?\n"); - intel_display_driver_runtime_resume_early(dev_priv); + intel_display_driver_runtime_resume_early(dev_priv, false); ret = vlv_resume_prepare(dev_priv, true); @@ -1487,7 +1487,7 @@ static int intel_runtime_resume(struct device *kdev) intel_pxp_runtime_resume(dev_priv->pxp); - intel_display_driver_runtime_resume_early(dev_priv); + intel_display_driver_runtime_resume_early(dev_priv, false); enable_rpm_wakeref_asserts(rpm); diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c index ab85c7fb217a..9a652292d988 100644 --- a/drivers/gpu/drm/xe/display/xe_display.c +++ b/drivers/gpu/drm/xe/display/xe_display.c @@ -283,36 +283,6 @@ static bool suspend_to_idle(void) return false; } -static void xe_display_to_d3cold(struct xe_device *xe) -{ - struct intel_display *display = &xe->display; - - /* We do a lot of poking in a lot of registers, make sure they work properly. */ - intel_power_domains_disable(xe); - - intel_hpd_cancel_work(xe); - - intel_opregion_suspend(display, PCI_D3cold); - - intel_dmc_suspend(display); -} - -static void xe_display_from_d3cold(struct xe_device *xe) -{ - struct intel_display *display = &xe->display; - - intel_dmc_resume(display); - - if (has_display(xe)) - drm_mode_config_reset(&xe->drm); - - intel_display_driver_init_hw(xe); - - intel_opregion_resume(display); - - intel_power_domains_enable(xe); -} - void xe_display_pm_suspend(struct xe_device *xe) { if (!xe->info.probe_display) @@ -413,10 +383,7 @@ void xe_display_pm_runtime_suspend(struct xe_device *xe) if (!xe->info.probe_display) return; - if (xe->d3cold.allowed) - xe_display_to_d3cold(xe); - else - intel_display_power_suspend(xe); + intel_display_driver_runtime_suspend(xe, xe->d3cold.allowed); } void xe_display_pm_runtime_suspend_late(struct xe_device *xe) @@ -424,12 +391,7 @@ void xe_display_pm_runtime_suspend_late(struct xe_device *xe) if (!xe->info.probe_display) return; - if (xe->d3cold.allowed) - intel_display_power_suspend_late(xe, false); - else - intel_opregion_notify_adapter(&xe->display, PCI_D1); - - intel_hpd_poll_enable(xe); + intel_display_driver_runtime_suspend_late(xe, xe->d3cold.allowed); } void xe_display_pm_runtime_resume_early(struct xe_device *xe) @@ -437,12 +399,7 @@ void xe_display_pm_runtime_resume_early(struct xe_device *xe) if (!xe->info.probe_display) return; - if (xe->d3cold.allowed) { - intel_display_power_resume_early(xe); - } else { - intel_opregion_notify_adapter(&xe->display, PCI_D0); - intel_display_power_resume(xe); - } + intel_display_driver_runtime_resume_early(xe, xe->d3cold.allowed); } void xe_display_pm_runtime_resume(struct xe_device *xe) @@ -450,12 +407,7 @@ void xe_display_pm_runtime_resume(struct xe_device *xe) if (!xe->info.probe_display) return; - if (xe->d3cold.allowed) - xe_display_from_d3cold(xe); - - intel_hpd_init(xe); - intel_hpd_poll_disable(xe); - skl_watermark_ipc_update(xe); + intel_display_driver_runtime_resume(xe, xe->d3cold.allowed); } static void display_device_remove(struct drm_device *dev, void *arg)
No functional change. Consolidate the entire runtime pm sequences under intel_display_driver. Simplifications and optimizations around the D3cold sequences are likely still possible. But before that can be done, consolidate everything at the intel_display_driver side. Xe display power management functions are now only a wrapper checking for xe's display probe parameter. Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- .../drm/i915/display/intel_display_driver.c | 66 ++++++++++++++++--- .../drm/i915/display/intel_display_driver.h | 12 ++-- drivers/gpu/drm/i915/i915_driver.c | 8 +-- drivers/gpu/drm/xe/display/xe_display.c | 56 ++-------------- 4 files changed, 74 insertions(+), 68 deletions(-)