-----Original Message----- From: Intel-gfx <[email protected]> On Behalf Of Rodrigo Vivi Sent: Tuesday, September 24, 2024 1:35 PM To: [email protected]; [email protected] Cc: Deak, Imre <[email protected]>; Vivi, Rodrigo <[email protected]> Subject: [PATCH 08/31] drm/i915/display: Move suspend sequences to intel_display_driver > > The goal is to reconcile the Xe and i915 PM functions. > Start by moving the display sequences from i915_drv towards > intel_display_driver. > > Signed-off-by: Rodrigo Vivi <[email protected]> > --- > .../drm/i915/display/intel_display_driver.c | 20 +++++++++++++++++++ > .../drm/i915/display/intel_display_driver.h | 3 +++ > drivers/gpu/drm/i915/i915_driver.c | 14 ++----------- > drivers/gpu/drm/xe/display/xe_fb_pin.c | 4 ++++ > 4 files changed, 29 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c > b/drivers/gpu/drm/i915/display/intel_display_driver.c > index 51fc1c148283..42624bf80f91 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_driver.c > +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c > @@ -36,6 +36,7 @@ > #include "intel_dkl_phy.h" > #include "intel_dmc.h" > #include "intel_dp.h" > +#include "intel_dpt.h" > #include "intel_dp_tunnel.h" > #include "intel_dpll.h" > #include "intel_dpll_mgr.h" > @@ -682,6 +683,25 @@ int intel_display_driver_suspend(struct drm_i915_private > *i915) > return ret; > } > > +void intel_display_driver_suspend_noirq(struct drm_i915_private *i915) > +{ > + intel_hpd_cancel_work(i915); > + > + if (HAS_DISPLAY(i915)) > + intel_display_driver_suspend_access(i915); > + > + intel_encoder_suspend_all(&i915->display); > + > + /* Must be called before GGTT is suspended. */ > + intel_dpt_suspend(i915); > +} > + > +void intel_display_driver_suspend_noggtt(struct intel_display *display, bool > s2idle) > +{ > + intel_opregion_suspend(display, s2idle ? PCI_D1 : PCI_D3cold); > + intel_dmc_suspend(display); > +} > + > int > __intel_display_driver_resume(struct drm_i915_private *i915, > struct drm_atomic_state *state, > diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.h > b/drivers/gpu/drm/i915/display/intel_display_driver.h > index 1ee37fb58d38..179fbb86923a 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_driver.h > +++ b/drivers/gpu/drm/i915/display/intel_display_driver.h > @@ -12,6 +12,7 @@ struct drm_atomic_state; > struct drm_i915_private; > struct drm_modeset_acquire_ctx; > struct pci_dev; > +struct intel_display; > > bool intel_display_driver_probe_defer(struct pci_dev *pdev); > void intel_display_driver_init_hw(struct drm_i915_private *i915); > @@ -25,6 +26,8 @@ void intel_display_driver_remove_noirq(struct > drm_i915_private *i915); > void intel_display_driver_remove_nogem(struct drm_i915_private *i915); > void intel_display_driver_unregister(struct drm_i915_private *i915); > int intel_display_driver_suspend(struct drm_i915_private *i915); > +void intel_display_driver_suspend_noirq(struct drm_i915_private *i915); > +void intel_display_driver_suspend_noggtt(struct intel_display *display, bool > s2idle); > void intel_display_driver_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); > diff --git a/drivers/gpu/drm/i915/i915_driver.c > b/drivers/gpu/drm/i915/i915_driver.c > index d166a8164b42..ac8bf00458b5 100644 > --- a/drivers/gpu/drm/i915/i915_driver.c > +++ b/drivers/gpu/drm/i915/i915_driver.c > @@ -996,7 +996,6 @@ static int i915_drm_suspend(struct drm_device *dev) > struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_display *display = &dev_priv->display; > struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev); > - pci_power_t opregion_target_state; > > disable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > > @@ -1014,26 +1013,17 @@ static int i915_drm_suspend(struct drm_device *dev) > intel_display_driver_suspend(dev_priv); > > intel_irq_suspend(dev_priv); > - intel_hpd_cancel_work(dev_priv); > > - if (HAS_DISPLAY(dev_priv)) > - intel_display_driver_suspend_access(dev_priv); > - > - intel_encoder_suspend_all(&dev_priv->display); > + intel_display_driver_suspend_noirq(dev_priv); > > - /* Must be called before GGTT is suspended. */ > - intel_dpt_suspend(dev_priv); > i915_ggtt_suspend(to_gt(dev_priv)->ggtt); > > i9xx_display_sr_save(dev_priv); > > - opregion_target_state = suspend_to_idle(dev_priv) ? PCI_D1 : PCI_D3cold; > - intel_opregion_suspend(display, opregion_target_state); > + intel_display_driver_suspend_noggtt(display, suspend_to_idle(dev_priv)); > > dev_priv->suspend_count++; > > - intel_dmc_suspend(display); > - > enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > > i915_gem_drain_freed_objects(dev_priv); > diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c > b/drivers/gpu/drm/xe/display/xe_fb_pin.c > index 79dbbbe03c7f..49dc91bdbcb0 100644 > --- a/drivers/gpu/drm/xe/display/xe_fb_pin.c > +++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c > @@ -408,3 +408,7 @@ u64 intel_dpt_offset(struct i915_vma *dpt_vma) > { > return 0; > } > + > +void intel_dpt_suspend(struct xe_device *xe) > +{ > +}
I can't really tell why this is being disabled here. Maybe this can be broken into a separate patch with an explanation for why this is being done? Something similar is done in patch 14 with intel_dpt_resume, so perhaps both changes can be collected together in the same patch? I won't block on it, but I am curious. Reviewed-by: Jonathan Cavitt <[email protected]> -Jonathan Cavitt > -- > 2.46.0 > >
