diff mbox series

[3/3] drm/xe: Use i915-display shutdown sequence directly

Message ID 20250117220943.506991-3-rodrigo.vivi@intel.com (mailing list archive)
State New
Headers show
Series [1/3] drm/i915/display: Move shutdown sequences under display driver | expand

Commit Message

Rodrigo Vivi Jan. 17, 2025, 10:09 p.m. UTC
Start the xe-i915-display reconciliation by using the same
shutdown sequences.

v2: include the stubs for !CONFIG_DRM_XE_DISPLAY (Kunit)

Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/xe/display/xe_display.c | 48 +++++++------------------
 drivers/gpu/drm/xe/display/xe_display.h | 10 +++---
 drivers/gpu/drm/xe/xe_device.c          |  4 ++-
 3 files changed, 22 insertions(+), 40 deletions(-)

Comments

Maarten Lankhorst Jan. 20, 2025, 1:42 p.m. UTC | #1
Hey,

Den 2025-01-17 kl. 23:09, skrev Rodrigo Vivi:
> Start the xe-i915-display reconciliation by using the same
> shutdown sequences.
> 
> v2: include the stubs for !CONFIG_DRM_XE_DISPLAY (Kunit)
> 
> Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>   drivers/gpu/drm/xe/display/xe_display.c | 48 +++++++------------------
>   drivers/gpu/drm/xe/display/xe_display.h | 10 +++---
>   drivers/gpu/drm/xe/xe_device.c          |  4 ++-
>   3 files changed, 22 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> index 4f60d7bd7742..e1ce9eb3332d 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.c
> +++ b/drivers/gpu/drm/xe/display/xe_display.c
> @@ -10,7 +10,6 @@
>   
>   #include <drm/drm_drv.h>
>   #include <drm/drm_managed.h>
> -#include <drm/drm_atomic_helper.h>
>   #include <drm/drm_probe_helper.h>
>   #include <uapi/drm/xe_drm.h>
>   
> @@ -369,32 +368,26 @@ void xe_display_pm_suspend(struct xe_device *xe)
>   
>   void xe_display_pm_shutdown(struct xe_device *xe)
>   {
> -	struct intel_display *display = &xe->display;
> -
>   	if (!xe->info.probe_display)
>   		return;
>   
> -	intel_power_domains_disable(display);
> -	intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_SUSPENDED, true);
> -	if (has_display(xe)) {
> -		drm_kms_helper_poll_disable(&xe->drm);
> -		intel_display_driver_disable_user_access(display);
> -
> -		drm_atomic_helper_shutdown(display->drm);
> -	}
> -
> -	intel_dp_mst_suspend(display);
> -	intel_hpd_cancel_work(xe);
> +	intel_display_driver_shutdown(&xe->display);
> +}
>   
> -	if (has_display(xe))
> -		intel_display_driver_suspend_access(display);
> +void xe_display_pm_shutdown_noirq(struct xe_device *xe)
> +{
> +	if (!xe->info.probe_display)
> +		return;
>   
> -	intel_encoder_suspend_all(display);
> -	intel_encoder_shutdown_all(display);
> +	intel_display_driver_shutdown_noirq(&xe->display);
> +}
>   
> -	intel_opregion_suspend(display, PCI_D3cold);
> +void xe_display_pm_shutdown_noaccel(struct xe_device *xe)
> +{
> +	if (!xe->info.probe_display)
> +		return;
>   
> -	intel_dmc_suspend(display);
> +	intel_display_driver_shutdown_nogem(&xe->display);
>   }
>   
>   void xe_display_pm_runtime_suspend(struct xe_device *xe)
> @@ -439,21 +432,6 @@ void xe_display_pm_runtime_suspend_late(struct xe_device *xe)
>   	intel_dmc_wl_flush_release_work(display);
>   }
>   
> -void xe_display_pm_shutdown_late(struct xe_device *xe)
> -{
> -	struct intel_display *display = &xe->display;
> -
> -	if (!xe->info.probe_display)
> -		return;
> -
> -	/*
> -	 * The only requirement is to reboot with display DC states disabled,
> -	 * for now leaving all display power wells in the INIT power domain
> -	 * enabled.
> -	 */
> -	intel_power_domains_driver_remove(display);
> -}
> -
>   void xe_display_pm_resume_early(struct xe_device *xe)
>   {
>   	struct intel_display *display = &xe->display;
> diff --git a/drivers/gpu/drm/xe/display/xe_display.h b/drivers/gpu/drm/xe/display/xe_display.h
> index 233f81a26c25..a15ec29b862b 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.h
> +++ b/drivers/gpu/drm/xe/display/xe_display.h
> @@ -35,9 +35,10 @@ void xe_display_irq_reset(struct xe_device *xe);
>   void xe_display_irq_postinstall(struct xe_device *xe, struct xe_gt *gt);
>   
>   void xe_display_pm_suspend(struct xe_device *xe);
> -void xe_display_pm_shutdown(struct xe_device *xe);
>   void xe_display_pm_suspend_late(struct xe_device *xe);
> -void xe_display_pm_shutdown_late(struct xe_device *xe);
> +void xe_display_pm_shutdown(struct xe_device *xe);
> +void xe_display_pm_shutdown_noirq(struct xe_device *xe);
> +void xe_display_pm_shutdown_noaccel(struct xe_device *xe);
>   void xe_display_pm_resume_early(struct xe_device *xe);
>   void xe_display_pm_resume(struct xe_device *xe);
>   void xe_display_pm_runtime_suspend(struct xe_device *xe);
> @@ -69,9 +70,10 @@ static inline void xe_display_irq_reset(struct xe_device *xe) {}
>   static inline void xe_display_irq_postinstall(struct xe_device *xe, struct xe_gt *gt) {}
>   
>   static inline void xe_display_pm_suspend(struct xe_device *xe) {}
> -static inline void xe_display_pm_shutdown(struct xe_device *xe) {}
>   static inline void xe_display_pm_suspend_late(struct xe_device *xe) {}
> -static inline void xe_display_pm_shutdown_late(struct xe_device *xe) {}
> +static inline void xe_display_pm_shutdown(struct xe_device *xe) {}
> +static inline void xe_display_pm_shutdown_noirq(struct xe_device *xe) {}
> +static inline void xe_display_pm_shutdown_noaccel(struct xe_device *xe) {}
>   static inline void xe_display_pm_resume_early(struct xe_device *xe) {}
>   static inline void xe_display_pm_resume(struct xe_device *xe) {}
>   static inline void xe_display_pm_runtime_suspend(struct xe_device *xe) {}
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 0966d9697caf..53cac055a2a9 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -934,10 +934,12 @@ void xe_device_shutdown(struct xe_device *xe)
>   
>   		xe_irq_suspend(xe);
>   
> +		xe_display_pm_shutdown_noirq(xe);
> +
>   		for_each_gt(gt, xe, id)
>   			xe_gt_shutdown(gt);
>   
> -		xe_display_pm_shutdown_late(xe);
> +		xe_display_pm_shutdown_noaccel(xe);
 From the xe point of view, it shouldn't matter whether we call the 
noirq part before or after gt shutdown. I like the integration into 
xe_device to be as simple as possible, so could we keep the single 
xe_display_pm_shutdown_late() call?

Which reminds me to send out xe_display simplification once more to do 
the same for init..
>   	} else {
>   		/* BOOM! */
>   		__xe_driver_flr(xe);
Completely unrelated, do you happen to know if we need to call 
encoder_suspend/shutdown in the FLR path, whether it affects logic not 
on the chip itself?

Cheers,
~Maarten
Rodrigo Vivi Jan. 21, 2025, 7:37 p.m. UTC | #2
On Mon, Jan 20, 2025 at 02:42:14PM +0100, Maarten Lankhorst wrote:
> Hey,
> 
> Den 2025-01-17 kl. 23:09, skrev Rodrigo Vivi:
> > Start the xe-i915-display reconciliation by using the same
> > shutdown sequences.
> > 
> > v2: include the stubs for !CONFIG_DRM_XE_DISPLAY (Kunit)
> > 
> > Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >   drivers/gpu/drm/xe/display/xe_display.c | 48 +++++++------------------
> >   drivers/gpu/drm/xe/display/xe_display.h | 10 +++---
> >   drivers/gpu/drm/xe/xe_device.c          |  4 ++-
> >   3 files changed, 22 insertions(+), 40 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> > index 4f60d7bd7742..e1ce9eb3332d 100644
> > --- a/drivers/gpu/drm/xe/display/xe_display.c
> > +++ b/drivers/gpu/drm/xe/display/xe_display.c
> > @@ -10,7 +10,6 @@
> >   #include <drm/drm_drv.h>
> >   #include <drm/drm_managed.h>
> > -#include <drm/drm_atomic_helper.h>
> >   #include <drm/drm_probe_helper.h>
> >   #include <uapi/drm/xe_drm.h>
> > @@ -369,32 +368,26 @@ void xe_display_pm_suspend(struct xe_device *xe)
> >   void xe_display_pm_shutdown(struct xe_device *xe)
> >   {
> > -	struct intel_display *display = &xe->display;
> > -
> >   	if (!xe->info.probe_display)
> >   		return;
> > -	intel_power_domains_disable(display);
> > -	intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_SUSPENDED, true);
> > -	if (has_display(xe)) {
> > -		drm_kms_helper_poll_disable(&xe->drm);
> > -		intel_display_driver_disable_user_access(display);
> > -
> > -		drm_atomic_helper_shutdown(display->drm);
> > -	}
> > -
> > -	intel_dp_mst_suspend(display);
> > -	intel_hpd_cancel_work(xe);
> > +	intel_display_driver_shutdown(&xe->display);
> > +}
> > -	if (has_display(xe))
> > -		intel_display_driver_suspend_access(display);
> > +void xe_display_pm_shutdown_noirq(struct xe_device *xe)
> > +{
> > +	if (!xe->info.probe_display)
> > +		return;
> > -	intel_encoder_suspend_all(display);
> > -	intel_encoder_shutdown_all(display);
> > +	intel_display_driver_shutdown_noirq(&xe->display);
> > +}
> > -	intel_opregion_suspend(display, PCI_D3cold);
> > +void xe_display_pm_shutdown_noaccel(struct xe_device *xe)
> > +{
> > +	if (!xe->info.probe_display)
> > +		return;
> > -	intel_dmc_suspend(display);
> > +	intel_display_driver_shutdown_nogem(&xe->display);
> >   }
> >   void xe_display_pm_runtime_suspend(struct xe_device *xe)
> > @@ -439,21 +432,6 @@ void xe_display_pm_runtime_suspend_late(struct xe_device *xe)
> >   	intel_dmc_wl_flush_release_work(display);
> >   }
> > -void xe_display_pm_shutdown_late(struct xe_device *xe)
> > -{
> > -	struct intel_display *display = &xe->display;
> > -
> > -	if (!xe->info.probe_display)
> > -		return;
> > -
> > -	/*
> > -	 * The only requirement is to reboot with display DC states disabled,
> > -	 * for now leaving all display power wells in the INIT power domain
> > -	 * enabled.
> > -	 */
> > -	intel_power_domains_driver_remove(display);
> > -}
> > -
> >   void xe_display_pm_resume_early(struct xe_device *xe)
> >   {
> >   	struct intel_display *display = &xe->display;
> > diff --git a/drivers/gpu/drm/xe/display/xe_display.h b/drivers/gpu/drm/xe/display/xe_display.h
> > index 233f81a26c25..a15ec29b862b 100644
> > --- a/drivers/gpu/drm/xe/display/xe_display.h
> > +++ b/drivers/gpu/drm/xe/display/xe_display.h
> > @@ -35,9 +35,10 @@ void xe_display_irq_reset(struct xe_device *xe);
> >   void xe_display_irq_postinstall(struct xe_device *xe, struct xe_gt *gt);
> >   void xe_display_pm_suspend(struct xe_device *xe);
> > -void xe_display_pm_shutdown(struct xe_device *xe);
> >   void xe_display_pm_suspend_late(struct xe_device *xe);
> > -void xe_display_pm_shutdown_late(struct xe_device *xe);
> > +void xe_display_pm_shutdown(struct xe_device *xe);
> > +void xe_display_pm_shutdown_noirq(struct xe_device *xe);
> > +void xe_display_pm_shutdown_noaccel(struct xe_device *xe);
> >   void xe_display_pm_resume_early(struct xe_device *xe);
> >   void xe_display_pm_resume(struct xe_device *xe);
> >   void xe_display_pm_runtime_suspend(struct xe_device *xe);
> > @@ -69,9 +70,10 @@ static inline void xe_display_irq_reset(struct xe_device *xe) {}
> >   static inline void xe_display_irq_postinstall(struct xe_device *xe, struct xe_gt *gt) {}
> >   static inline void xe_display_pm_suspend(struct xe_device *xe) {}
> > -static inline void xe_display_pm_shutdown(struct xe_device *xe) {}
> >   static inline void xe_display_pm_suspend_late(struct xe_device *xe) {}
> > -static inline void xe_display_pm_shutdown_late(struct xe_device *xe) {}
> > +static inline void xe_display_pm_shutdown(struct xe_device *xe) {}
> > +static inline void xe_display_pm_shutdown_noirq(struct xe_device *xe) {}
> > +static inline void xe_display_pm_shutdown_noaccel(struct xe_device *xe) {}
> >   static inline void xe_display_pm_resume_early(struct xe_device *xe) {}
> >   static inline void xe_display_pm_resume(struct xe_device *xe) {}
> >   static inline void xe_display_pm_runtime_suspend(struct xe_device *xe) {}
> > diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> > index 0966d9697caf..53cac055a2a9 100644
> > --- a/drivers/gpu/drm/xe/xe_device.c
> > +++ b/drivers/gpu/drm/xe/xe_device.c
> > @@ -934,10 +934,12 @@ void xe_device_shutdown(struct xe_device *xe)
> >   		xe_irq_suspend(xe);
> > +		xe_display_pm_shutdown_noirq(xe);
> > +
> >   		for_each_gt(gt, xe, id)
> >   			xe_gt_shutdown(gt);
> > -		xe_display_pm_shutdown_late(xe);
> > +		xe_display_pm_shutdown_noaccel(xe);
> From the xe point of view, it shouldn't matter whether we call the noirq
> part before or after gt shutdown. I like the integration into xe_device to
> be as simple as possible, so could we keep the single
> xe_display_pm_shutdown_late() call?

I prefer that we make the xe_display entirely an wrapper to i915/display,
It should only check for the Xe's display module parameter, and then call
the equivalent function there directly.

Then, whatever differences we might have we move to xe_device itself.

And in this case _noaccel is just a generic name for _nogem which is a name
that would works for both i915 and xe...

> 
> Which reminds me to send out xe_display simplification once more to do the
> same for init..

On that too, I know I reviewed, but Jani also had concerns with that on the
sense that that deviates from removing display differences between drivers.

Although I still believe it is possible to take that patch in, but later
move the differences out of xe_display ?!

> >   	} else {
> >   		/* BOOM! */
> >   		__xe_driver_flr(xe);
> Completely unrelated, do you happen to know if we need to call
> encoder_suspend/shutdown in the FLR path, whether it affects logic not on
> the chip itself?
> 
> Cheers,
> ~Maarten
>
Rodrigo Vivi Jan. 28, 2025, 11:27 p.m. UTC | #3
On Mon, Jan 20, 2025 at 02:42:14PM +0100, Maarten Lankhorst wrote:
> Hey,
> 
> Den 2025-01-17 kl. 23:09, skrev Rodrigo Vivi:
> > Start the xe-i915-display reconciliation by using the same
> > shutdown sequences.
> > 
> > v2: include the stubs for !CONFIG_DRM_XE_DISPLAY (Kunit)
> > 
> > Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >   drivers/gpu/drm/xe/display/xe_display.c | 48 +++++++------------------
> >   drivers/gpu/drm/xe/display/xe_display.h | 10 +++---
> >   drivers/gpu/drm/xe/xe_device.c          |  4 ++-
> >   3 files changed, 22 insertions(+), 40 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> > index 4f60d7bd7742..e1ce9eb3332d 100644
> > --- a/drivers/gpu/drm/xe/display/xe_display.c
> > +++ b/drivers/gpu/drm/xe/display/xe_display.c
> > @@ -10,7 +10,6 @@
> >   #include <drm/drm_drv.h>
> >   #include <drm/drm_managed.h>
> > -#include <drm/drm_atomic_helper.h>
> >   #include <drm/drm_probe_helper.h>
> >   #include <uapi/drm/xe_drm.h>
> > @@ -369,32 +368,26 @@ void xe_display_pm_suspend(struct xe_device *xe)
> >   void xe_display_pm_shutdown(struct xe_device *xe)
> >   {
> > -	struct intel_display *display = &xe->display;
> > -
> >   	if (!xe->info.probe_display)
> >   		return;
> > -	intel_power_domains_disable(display);
> > -	intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_SUSPENDED, true);
> > -	if (has_display(xe)) {
> > -		drm_kms_helper_poll_disable(&xe->drm);
> > -		intel_display_driver_disable_user_access(display);
> > -
> > -		drm_atomic_helper_shutdown(display->drm);
> > -	}
> > -
> > -	intel_dp_mst_suspend(display);
> > -	intel_hpd_cancel_work(xe);
> > +	intel_display_driver_shutdown(&xe->display);
> > +}
> > -	if (has_display(xe))
> > -		intel_display_driver_suspend_access(display);
> > +void xe_display_pm_shutdown_noirq(struct xe_device *xe)
> > +{
> > +	if (!xe->info.probe_display)
> > +		return;
> > -	intel_encoder_suspend_all(display);
> > -	intel_encoder_shutdown_all(display);
> > +	intel_display_driver_shutdown_noirq(&xe->display);
> > +}
> > -	intel_opregion_suspend(display, PCI_D3cold);
> > +void xe_display_pm_shutdown_noaccel(struct xe_device *xe)
> > +{
> > +	if (!xe->info.probe_display)
> > +		return;
> > -	intel_dmc_suspend(display);
> > +	intel_display_driver_shutdown_nogem(&xe->display);
> >   }
> >   void xe_display_pm_runtime_suspend(struct xe_device *xe)
> > @@ -439,21 +432,6 @@ void xe_display_pm_runtime_suspend_late(struct xe_device *xe)
> >   	intel_dmc_wl_flush_release_work(display);
> >   }
> > -void xe_display_pm_shutdown_late(struct xe_device *xe)
> > -{
> > -	struct intel_display *display = &xe->display;
> > -
> > -	if (!xe->info.probe_display)
> > -		return;
> > -
> > -	/*
> > -	 * The only requirement is to reboot with display DC states disabled,
> > -	 * for now leaving all display power wells in the INIT power domain
> > -	 * enabled.
> > -	 */
> > -	intel_power_domains_driver_remove(display);
> > -}
> > -
> >   void xe_display_pm_resume_early(struct xe_device *xe)
> >   {
> >   	struct intel_display *display = &xe->display;
> > diff --git a/drivers/gpu/drm/xe/display/xe_display.h b/drivers/gpu/drm/xe/display/xe_display.h
> > index 233f81a26c25..a15ec29b862b 100644
> > --- a/drivers/gpu/drm/xe/display/xe_display.h
> > +++ b/drivers/gpu/drm/xe/display/xe_display.h
> > @@ -35,9 +35,10 @@ void xe_display_irq_reset(struct xe_device *xe);
> >   void xe_display_irq_postinstall(struct xe_device *xe, struct xe_gt *gt);
> >   void xe_display_pm_suspend(struct xe_device *xe);
> > -void xe_display_pm_shutdown(struct xe_device *xe);
> >   void xe_display_pm_suspend_late(struct xe_device *xe);
> > -void xe_display_pm_shutdown_late(struct xe_device *xe);
> > +void xe_display_pm_shutdown(struct xe_device *xe);
> > +void xe_display_pm_shutdown_noirq(struct xe_device *xe);
> > +void xe_display_pm_shutdown_noaccel(struct xe_device *xe);
> >   void xe_display_pm_resume_early(struct xe_device *xe);
> >   void xe_display_pm_resume(struct xe_device *xe);
> >   void xe_display_pm_runtime_suspend(struct xe_device *xe);
> > @@ -69,9 +70,10 @@ static inline void xe_display_irq_reset(struct xe_device *xe) {}
> >   static inline void xe_display_irq_postinstall(struct xe_device *xe, struct xe_gt *gt) {}
> >   static inline void xe_display_pm_suspend(struct xe_device *xe) {}
> > -static inline void xe_display_pm_shutdown(struct xe_device *xe) {}
> >   static inline void xe_display_pm_suspend_late(struct xe_device *xe) {}
> > -static inline void xe_display_pm_shutdown_late(struct xe_device *xe) {}
> > +static inline void xe_display_pm_shutdown(struct xe_device *xe) {}
> > +static inline void xe_display_pm_shutdown_noirq(struct xe_device *xe) {}
> > +static inline void xe_display_pm_shutdown_noaccel(struct xe_device *xe) {}
> >   static inline void xe_display_pm_resume_early(struct xe_device *xe) {}
> >   static inline void xe_display_pm_resume(struct xe_device *xe) {}
> >   static inline void xe_display_pm_runtime_suspend(struct xe_device *xe) {}
> > diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> > index 0966d9697caf..53cac055a2a9 100644
> > --- a/drivers/gpu/drm/xe/xe_device.c
> > +++ b/drivers/gpu/drm/xe/xe_device.c
> > @@ -934,10 +934,12 @@ void xe_device_shutdown(struct xe_device *xe)
> >   		xe_irq_suspend(xe);
> > +		xe_display_pm_shutdown_noirq(xe);
> > +
> >   		for_each_gt(gt, xe, id)
> >   			xe_gt_shutdown(gt);
> > -		xe_display_pm_shutdown_late(xe);
> > +		xe_display_pm_shutdown_noaccel(xe);
> From the xe point of view, it shouldn't matter whether we call the noirq
> part before or after gt shutdown. I like the integration into xe_device to
> be as simple as possible, so could we keep the single
> xe_display_pm_shutdown_late() call?

As I had told you offline I was seeing if we could have a naming in i915
more generic like shutdown, shutdown_late, shutdown_early...
However I didn't like that that much... From the i915/display side, the
names still matters... that function can only be called at shutdown when
there's no irq anymore...

And from the xe side, I'm still working towards make the xe_display only
a thin layer for checking xe.enable_display parameter and nothing else.

So I still pretty much prefer the _noirq and _noaccel naming...

> 
> Which reminds me to send out xe_display simplification once more to do the
> same for init..
> >   	} else {
> >   		/* BOOM! */
> >   		__xe_driver_flr(xe);
> Completely unrelated, do you happen to know if we need to call
> encoder_suspend/shutdown in the FLR path, whether it affects logic not on
> the chip itself?
> 
> Cheers,
> ~Maarten
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
index 4f60d7bd7742..e1ce9eb3332d 100644
--- a/drivers/gpu/drm/xe/display/xe_display.c
+++ b/drivers/gpu/drm/xe/display/xe_display.c
@@ -10,7 +10,6 @@ 
 
 #include <drm/drm_drv.h>
 #include <drm/drm_managed.h>
-#include <drm/drm_atomic_helper.h>
 #include <drm/drm_probe_helper.h>
 #include <uapi/drm/xe_drm.h>
 
@@ -369,32 +368,26 @@  void xe_display_pm_suspend(struct xe_device *xe)
 
 void xe_display_pm_shutdown(struct xe_device *xe)
 {
-	struct intel_display *display = &xe->display;
-
 	if (!xe->info.probe_display)
 		return;
 
-	intel_power_domains_disable(display);
-	intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_SUSPENDED, true);
-	if (has_display(xe)) {
-		drm_kms_helper_poll_disable(&xe->drm);
-		intel_display_driver_disable_user_access(display);
-
-		drm_atomic_helper_shutdown(display->drm);
-	}
-
-	intel_dp_mst_suspend(display);
-	intel_hpd_cancel_work(xe);
+	intel_display_driver_shutdown(&xe->display);
+}
 
-	if (has_display(xe))
-		intel_display_driver_suspend_access(display);
+void xe_display_pm_shutdown_noirq(struct xe_device *xe)
+{
+	if (!xe->info.probe_display)
+		return;
 
-	intel_encoder_suspend_all(display);
-	intel_encoder_shutdown_all(display);
+	intel_display_driver_shutdown_noirq(&xe->display);
+}
 
-	intel_opregion_suspend(display, PCI_D3cold);
+void xe_display_pm_shutdown_noaccel(struct xe_device *xe)
+{
+	if (!xe->info.probe_display)
+		return;
 
-	intel_dmc_suspend(display);
+	intel_display_driver_shutdown_nogem(&xe->display);
 }
 
 void xe_display_pm_runtime_suspend(struct xe_device *xe)
@@ -439,21 +432,6 @@  void xe_display_pm_runtime_suspend_late(struct xe_device *xe)
 	intel_dmc_wl_flush_release_work(display);
 }
 
-void xe_display_pm_shutdown_late(struct xe_device *xe)
-{
-	struct intel_display *display = &xe->display;
-
-	if (!xe->info.probe_display)
-		return;
-
-	/*
-	 * The only requirement is to reboot with display DC states disabled,
-	 * for now leaving all display power wells in the INIT power domain
-	 * enabled.
-	 */
-	intel_power_domains_driver_remove(display);
-}
-
 void xe_display_pm_resume_early(struct xe_device *xe)
 {
 	struct intel_display *display = &xe->display;
diff --git a/drivers/gpu/drm/xe/display/xe_display.h b/drivers/gpu/drm/xe/display/xe_display.h
index 233f81a26c25..a15ec29b862b 100644
--- a/drivers/gpu/drm/xe/display/xe_display.h
+++ b/drivers/gpu/drm/xe/display/xe_display.h
@@ -35,9 +35,10 @@  void xe_display_irq_reset(struct xe_device *xe);
 void xe_display_irq_postinstall(struct xe_device *xe, struct xe_gt *gt);
 
 void xe_display_pm_suspend(struct xe_device *xe);
-void xe_display_pm_shutdown(struct xe_device *xe);
 void xe_display_pm_suspend_late(struct xe_device *xe);
-void xe_display_pm_shutdown_late(struct xe_device *xe);
+void xe_display_pm_shutdown(struct xe_device *xe);
+void xe_display_pm_shutdown_noirq(struct xe_device *xe);
+void xe_display_pm_shutdown_noaccel(struct xe_device *xe);
 void xe_display_pm_resume_early(struct xe_device *xe);
 void xe_display_pm_resume(struct xe_device *xe);
 void xe_display_pm_runtime_suspend(struct xe_device *xe);
@@ -69,9 +70,10 @@  static inline void xe_display_irq_reset(struct xe_device *xe) {}
 static inline void xe_display_irq_postinstall(struct xe_device *xe, struct xe_gt *gt) {}
 
 static inline void xe_display_pm_suspend(struct xe_device *xe) {}
-static inline void xe_display_pm_shutdown(struct xe_device *xe) {}
 static inline void xe_display_pm_suspend_late(struct xe_device *xe) {}
-static inline void xe_display_pm_shutdown_late(struct xe_device *xe) {}
+static inline void xe_display_pm_shutdown(struct xe_device *xe) {}
+static inline void xe_display_pm_shutdown_noirq(struct xe_device *xe) {}
+static inline void xe_display_pm_shutdown_noaccel(struct xe_device *xe) {}
 static inline void xe_display_pm_resume_early(struct xe_device *xe) {}
 static inline void xe_display_pm_resume(struct xe_device *xe) {}
 static inline void xe_display_pm_runtime_suspend(struct xe_device *xe) {}
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index 0966d9697caf..53cac055a2a9 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -934,10 +934,12 @@  void xe_device_shutdown(struct xe_device *xe)
 
 		xe_irq_suspend(xe);
 
+		xe_display_pm_shutdown_noirq(xe);
+
 		for_each_gt(gt, xe, id)
 			xe_gt_shutdown(gt);
 
-		xe_display_pm_shutdown_late(xe);
+		xe_display_pm_shutdown_noaccel(xe);
 	} else {
 		/* BOOM! */
 		__xe_driver_flr(xe);