-----Original Message----- From: Intel-gfx <[email protected]> On Behalf Of Rodrigo Vivi Sent: Tuesday, September 24, 2024 1:36 PM To: [email protected]; [email protected] Cc: Deak, Imre <[email protected]>; Vivi, Rodrigo <[email protected]> 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 <[email protected]>
LGTM. Reviewed-by: Jonathan Cavitt <[email protected]> -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 > >
