Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Imre Deak <imre.deak@intel.com>
To: Jani Nikula <jani.nikula@intel.com>
Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org
Subject: Re: [PATCH 2/3] drm/{i915, xe}: move xe display shutdown and pm hooks to intel_display_driver.c
Date: Wed, 27 May 2026 21:52:08 +0300	[thread overview]
Message-ID: <ahc9WCALisq7cyrd@ideak-desk.lan> (raw)
In-Reply-To: <7bd1fbcda840f3f382f29e67592b587ab844905d@intel.com>

On Wed, May 27, 2026 at 07:35:32PM +0300, Jani Nikula wrote:
> On Wed, 27 May 2026, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Wed, May 27, 2026 at 04:06:25PM +0300, Jani Nikula wrote:
> >> Move the xe display glue code for shutdown and pm hooks from
> >> xe_display.c to intel_display_driver.c. This is a small step towards
> >> unifying the display interfaces between i915 and xe drivers. The code
> >> belongs in display, not in i915 or xe driver. Neither the xe nor i915
> >> core code should be calling deep into display functionality.
> >> 
> >> The high level functions are obviously modeled after the xe driver
> >> now. The i915 driver needs to start calling them as well. For this, they
> >> may need to be further changed and refactored, but this needs to happen
> >> in display side.
> >> 
> >> Clean up xe_display.c includes as many of them become unnecessary.
> >> 
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >>  .../drm/i915/display/intel_display_driver.c   | 189 ++++++++++++++++++
> >>  .../drm/i915/display/intel_display_driver.h   |  12 ++
> >>  drivers/gpu/drm/xe/display/xe_display.c       | 185 ++---------------
> >>  3 files changed, 214 insertions(+), 172 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
> >> index d0729936f681..15ba4c2ac985 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display_driver.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
> >> @@ -43,6 +43,7 @@
> >>  #include "intel_dp_tunnel.h"
> >>  #include "intel_dpll.h"
> >>  #include "intel_dpll_mgr.h"
> >> +#include "intel_encoder.h"
> >>  #include "intel_fb.h"
> >>  #include "intel_fbc.h"
> >>  #include "intel_fbdev.h"
> >> @@ -780,3 +781,191 @@ void intel_display_driver_resume(struct intel_display *display)
> >>  	if (state)
> >>  		drm_atomic_commit_put(state);
> >>  }
> >> +
> >> +/*
> >> + * FIXME: The below interfaces are currently only being called from the xe
> >> + * driver code. They need to be unified with the needs of the i915 driver hooks,
> >> + * and i915 needs to migrate over to them.
> >> + */
> >> +
> >> +void intel_display_driver_shutdown(struct intel_display *display)
> >> +{
> >> +	intel_display_power_disable(display);
> >> +	drm_client_dev_suspend(display->drm);
> >> +
> >> +	if (intel_display_device_present(display)) {
> >> +		drm_kms_helper_poll_disable(display->drm);
> >> +		intel_display_driver_disable_user_access(display);
> >> +		intel_display_driver_suspend(display);
> >> +	}
> >> +
> >> +	intel_display_flush_cleanup_work(display);
> >> +	intel_dp_mst_suspend(display);
> >> +	intel_encoder_block_all_hpds(display);
> >> +	intel_hpd_cancel_work(display);
> >> +
> >> +	if (intel_display_device_present(display))
> >> +		intel_display_driver_suspend_access(display);
> >> +
> >> +	intel_encoder_suspend_all(display);
> >> +	intel_encoder_shutdown_all(display);
> >> +
> >> +	intel_opregion_suspend(display, PCI_D3cold);
> >> +
> >> +	intel_dmc_suspend(display);
> >> +}
> >> +
> >> +void intel_display_driver_shutdown_late(struct intel_display *display)
> >> +{
> >> +	/*
> >> +	 * 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_display_power_driver_remove(display);
> >> +}
> >> +
> >> +static bool suspend_to_idle(void)
> >> +{
> >> +#if IS_ENABLED(CONFIG_ACPI_SLEEP)
> >> +	if (acpi_target_system_state() < ACPI_STATE_S3)
> >> +		return true;
> >> +#endif
> >> +	return false;
> >> +}
> >> +
> >> +void intel_display_driver_pm_enable_d3cold(struct intel_display *display)
> >> +{
> >> +	/*
> >> +	 * We do a lot of poking in a lot of registers, make sure they work
> >> +	 * properly.
> >> +	 */
> >> +	intel_display_power_disable(display);
> >
> > This stuff is not meant for runtime pm. xe just has some
> > obnoxious hacks in its runtime pm code to allow it to call
> > incorrect functions from its runtime pm paths without
> > deadlocks/etc.
> >
> > I think the xe hacks need to be killed and runtime pm
> > implemented there *correctly* before we base any common
> > code on the xe implementation.
> >
> > We should perhaps start from the i915 implementation
> > instead. That might help properly highlight all the
> > bogus things that xe is doing.
> 
> There are a few reasons why I chose to start off with xe like this.
> 
> The granularity of functions are a fairly good starting point for a
> shared implementation. Simply moving them over from xe to display in a
> non-functional way reduces xe_display.c dependency deep into display
> functionality. It's forward progress with no risk for regressions.
> 
> Sure, we could define similar functions for i915 to call, but that's
> going to contain functional changes from about patch #1, because i915
> calls deep into display in a very scattered way. With the approach at
> hand, we can gradually move i915 over to the new stuff, even function by
> function, comparing the sequences, making small changes to either along
> the way, as the case may be.
> 
> From my POV the end result is going to be the same. The difference is in
> the path we choose.
> 
> Of course, there's also the problem that I don't know for sure what all
> the hacks are that you refer to, or what implementing runtime PM
> correctly there means.

From the peanut gallery: the hack I suppose is the mechanism in the xe
driver to allow the driver's runtime suspend/resume hooks to get a
runtime PM reference. Everywhere else in the kernel (at least to my
knowledge) this is forbidden, since getting an RPM reference itself
requires 1. waiting for any pending runtime suspend hook to finish, 2.
run and complete the runtime resume hook. Hence getting an RPM reference
from the suspend/resume hooks themselves is a re-entrancy problem (and
others, like unexpectedly not actually enabling some power resource,
depending on the exact point in the hooks the RPM reference is
acquired).

One reason for adding the above mechanism (I assume) was the
implemenation of the D3cold enabling/disabling during runtime
suspend/resume, which do get an RPM reference. The solution would be -
for this one particular part - to change the D3cold enabling/disabling
sequence not to acquire any RPM reference. The rest of the runtime
suspend/resume sequence should be also converted not to rely on getting
an RPM reference.

> The d3cold stuff (including those intel_display_power_disable/enable()
> calls) is hidden behind a flag that only gets called for xe, which I
> guess is a bit lame, but also isolates it from the rest.
> 
> I guess to me it's often more important to be able to make meaningful
> forward progress without stalling right in the beginning. It does defer
> tackling the hard problems instead of confronting them right away, but
> it also makes it possible for the solutions to present themselves while
> making progress, without banging head on the wall so much.
> 
> *shrug*
> 
> I can also start looking at going the i915 route. But Someone(tm) needs
> to look at xe runtime suspend/resume/etc. in the mean time.
> 
> 
> BR,
> Jani.
> 
> 
> >
> >> +
> >> +	intel_display_flush_cleanup_work(display);
> >> +
> >> +	intel_opregion_suspend(display, PCI_D3cold);
> >> +
> >> +	intel_dmc_suspend(display);
> >> +
> >> +	if (intel_display_device_present(display))
> >> +		intel_hpd_poll_enable(display);
> >> +}
> >> +
> >> +void intel_display_driver_pm_disable_d3cold(struct intel_display *display)
> >> +{
> >> +	intel_dmc_resume(display);
> >> +
> >> +	if (intel_display_device_present(display))
> >> +		drm_mode_config_reset(display->drm);
> >> +
> >> +	intel_display_driver_init_hw(display);
> >> +
> >> +	intel_hpd_init(display);
> >> +
> >> +	if (intel_display_device_present(display))
> >> +		intel_hpd_poll_disable(display);
> >> +
> >> +	intel_opregion_resume(display);
> >> +
> >> +	intel_display_power_enable(display);
> >> +}
> >> +
> >> +void intel_display_driver_pm_suspend(struct intel_display *display)
> >> +{
> >> +	bool s2idle = suspend_to_idle();
> >> +
> >> +	/*
> >> +	 * We do a lot of poking in a lot of registers, make sure they work
> >> +	 * properly.
> >> +	 */
> >> +	intel_display_power_disable(display);
> >> +	drm_client_dev_suspend(display->drm);
> >> +
> >> +	if (intel_display_device_present(display)) {
> >> +		drm_kms_helper_poll_disable(display->drm);
> >> +		intel_display_driver_disable_user_access(display);
> >> +		intel_display_driver_suspend(display);
> >> +	}
> >> +
> >> +	intel_display_flush_cleanup_work(display);
> >> +
> >> +	intel_encoder_block_all_hpds(display);
> >> +
> >> +	intel_hpd_cancel_work(display);
> >> +
> >> +	if (intel_display_device_present(display)) {
> >> +		intel_display_driver_suspend_access(display);
> >> +		intel_encoder_suspend_all(display);
> >> +	}
> >> +
> >> +	intel_opregion_suspend(display, s2idle ? PCI_D1 : PCI_D3cold);
> >> +
> >> +	intel_dmc_suspend(display);
> >> +}
> >> +
> >> +void intel_display_driver_pm_suspend_late(struct intel_display *display)
> >> +{
> >> +	bool s2idle = suspend_to_idle();
> >> +
> >> +	intel_display_power_suspend_late(display, s2idle);
> >> +}
> >> +
> >> +void intel_display_driver_pm_resume_early(struct intel_display *display)
> >> +{
> >> +	intel_display_power_resume_early(display);
> >> +}
> >> +
> >> +void intel_display_driver_pm_resume(struct intel_display *display)
> >> +{
> >> +	intel_dmc_resume(display);
> >> +
> >> +	if (intel_display_device_present(display))
> >> +		drm_mode_config_reset(display->drm);
> >> +
> >> +	intel_display_driver_init_hw(display);
> >> +
> >> +	if (intel_display_device_present(display))
> >> +		intel_display_driver_resume_access(display);
> >> +
> >> +	intel_hpd_init(display);
> >> +
> >> +	intel_encoder_unblock_all_hpds(display);
> >> +
> >> +	if (intel_display_device_present(display)) {
> >> +		intel_display_driver_resume(display);
> >> +		drm_kms_helper_poll_enable(display->drm);
> >> +		intel_display_driver_enable_user_access(display);
> >> +	}
> >> +
> >> +	if (intel_display_device_present(display))
> >> +		intel_hpd_poll_disable(display);
> >> +
> >> +	intel_opregion_resume(display);
> >> +
> >> +	drm_client_dev_resume(display->drm);
> >> +
> >> +	intel_display_power_enable(display);
> >> +}
> >> +
> >> +void intel_display_driver_pm_runtime_suspend(struct intel_display *display)
> >> +{
> >> +	intel_hpd_poll_enable(display);
> >> +}
> >> +
> >> +void intel_display_driver_pm_runtime_suspend_late(struct intel_display *display)
> >> +{
> >> +	/*
> >> +	 * If xe_display_pm_suspend_late() is not called, it is likely
> >> +	 * that we will be on dynamic DC states with DMC wakelock enabled. We
> >> +	 * need to flush the release work in that case.
> >> +	 */
> >> +	intel_dmc_wl_flush_release_work(display);
> >> +}
> >> +
> >> +void intel_display_driver_pm_runtime_resume(struct intel_display *display)
> >> +{
> >> +	intel_hpd_init(display);
> >> +	intel_hpd_poll_disable(display);
> >> +	skl_watermark_ipc_update(display);
> >> +}
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.h b/drivers/gpu/drm/i915/display/intel_display_driver.h
> >> index 5270c26a32e0..e4ce17efe793 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display_driver.h
> >> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.h
> >> @@ -26,6 +26,8 @@ void intel_display_driver_remove_nogem(struct intel_display *display);
> >>  void intel_display_driver_unregister(struct intel_display *display);
> >>  int intel_display_driver_suspend(struct intel_display *display);
> >>  void intel_display_driver_resume(struct intel_display *display);
> >> +void intel_display_driver_shutdown(struct intel_display *display);
> >> +void intel_display_driver_shutdown_late(struct intel_display *display);
> >>  
> >>  /* interface for intel_display_reset.c */
> >>  int __intel_display_driver_resume(struct intel_display *display,
> >> @@ -38,5 +40,15 @@ void intel_display_driver_suspend_access(struct intel_display *display);
> >>  void intel_display_driver_resume_access(struct intel_display *display);
> >>  bool intel_display_driver_check_access(struct intel_display *display);
> >>  
> >> +void intel_display_driver_pm_enable_d3cold(struct intel_display *display);
> >> +void intel_display_driver_pm_disable_d3cold(struct intel_display *display);
> >> +void intel_display_driver_pm_suspend(struct intel_display *display);
> >> +void intel_display_driver_pm_suspend_late(struct intel_display *display);
> >> +void intel_display_driver_pm_resume_early(struct intel_display *display);
> >> +void intel_display_driver_pm_resume(struct intel_display *display);
> >> +void intel_display_driver_pm_runtime_suspend(struct intel_display *display);
> >> +void intel_display_driver_pm_runtime_suspend_late(struct intel_display *display);
> >> +void intel_display_driver_pm_runtime_resume(struct intel_display *display);
> >> +
> >>  #endif /* __INTEL_DISPLAY_DRIVER_H__ */
> >>  
> >> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> >> index a18af4d96dd1..6aba5668e4df 100644
> >> --- a/drivers/gpu/drm/xe/display/xe_display.c
> >> +++ b/drivers/gpu/drm/xe/display/xe_display.c
> >> @@ -6,36 +6,22 @@
> >>  #include "xe_display.h"
> >>  #include "regs/xe_irq_regs.h"
> >>  
> >> -#include <linux/fb.h>
> >> -
> >> -#include <drm/drm_client.h>
> >> -#include <drm/drm_client_event.h>
> >>  #include <drm/drm_drv.h>
> >>  #include <drm/drm_managed.h>
> >> -#include <drm/drm_probe_helper.h>
> >>  #include <drm/intel/display_member.h>
> >>  #include <drm/intel/display_parent_interface.h>
> >> -#include <uapi/drm/xe_drm.h>
> >>  
> >> -#include "intel_acpi.h"
> >>  #include "intel_audio.h"
> >>  #include "intel_bw.h"
> >> -#include "intel_display.h"
> >> -#include "intel_display_core.h"
> >>  #include "intel_display_device.h"
> >>  #include "intel_display_driver.h"
> >>  #include "intel_display_irq.h"
> >> -#include "intel_display_types.h"
> >> -#include "intel_dmc.h"
> >> -#include "intel_dmc_wl.h"
> >> -#include "intel_dp.h"
> >> +#include "intel_display_power.h"
> >>  #include "intel_dram.h"
> >> -#include "intel_encoder.h"
> >>  #include "intel_fbdev.h"
> >>  #include "intel_hdcp.h"
> >>  #include "intel_hotplug.h"
> >>  #include "intel_opregion.h"
> >> -#include "skl_watermark.h"
> >>  #include "xe_device.h"
> >>  #include "xe_display_bo.h"
> >>  #include "xe_display_pcode.h"
> >> @@ -235,97 +221,14 @@ void xe_display_irq_postinstall(struct xe_device *xe)
> >>  	intel_display_irq_postinstall(display);
> >>  }
> >>  
> >> -static bool suspend_to_idle(void)
> >> -{
> >> -#if IS_ENABLED(CONFIG_ACPI_SLEEP)
> >> -	if (acpi_target_system_state() < ACPI_STATE_S3)
> >> -		return true;
> >> -#endif
> >> -	return false;
> >> -}
> >> -
> >> -static void xe_display_enable_d3cold(struct xe_device *xe)
> >> -{
> >> -	struct intel_display *display = xe->display;
> >> -
> >> -	if (!xe->info.probe_display)
> >> -		return;
> >> -
> >> -	/*
> >> -	 * We do a lot of poking in a lot of registers, make sure they work
> >> -	 * properly.
> >> -	 */
> >> -	intel_display_power_disable(display);
> >> -
> >> -	intel_display_flush_cleanup_work(display);
> >> -
> >> -	intel_opregion_suspend(display, PCI_D3cold);
> >> -
> >> -	intel_dmc_suspend(display);
> >> -
> >> -	if (intel_display_device_present(display))
> >> -		intel_hpd_poll_enable(display);
> >> -}
> >> -
> >> -static void xe_display_disable_d3cold(struct xe_device *xe)
> >> -{
> >> -	struct intel_display *display = xe->display;
> >> -
> >> -	if (!xe->info.probe_display)
> >> -		return;
> >> -
> >> -	intel_dmc_resume(display);
> >> -
> >> -	if (intel_display_device_present(display))
> >> -		drm_mode_config_reset(&xe->drm);
> >> -
> >> -	intel_display_driver_init_hw(display);
> >> -
> >> -	intel_hpd_init(display);
> >> -
> >> -	if (intel_display_device_present(display))
> >> -		intel_hpd_poll_disable(display);
> >> -
> >> -	intel_opregion_resume(display);
> >> -
> >> -	intel_display_power_enable(display);
> >> -}
> >> -
> >>  void xe_display_pm_suspend(struct xe_device *xe)
> >>  {
> >>  	struct intel_display *display = xe->display;
> >> -	bool s2idle = suspend_to_idle();
> >>  
> >>  	if (!xe->info.probe_display)
> >>  		return;
> >>  
> >> -	/*
> >> -	 * We do a lot of poking in a lot of registers, make sure they work
> >> -	 * properly.
> >> -	 */
> >> -	intel_display_power_disable(display);
> >> -	drm_client_dev_suspend(&xe->drm);
> >> -
> >> -	if (intel_display_device_present(display)) {
> >> -		drm_kms_helper_poll_disable(&xe->drm);
> >> -		intel_display_driver_disable_user_access(display);
> >> -		intel_display_driver_suspend(display);
> >> -	}
> >> -
> >> -	intel_display_flush_cleanup_work(display);
> >> -
> >> -	intel_encoder_block_all_hpds(display);
> >> -
> >> -	intel_hpd_cancel_work(display);
> >> -
> >> -	if (intel_display_device_present(display)) {
> >> -		intel_display_driver_suspend_access(display);
> >> -		intel_encoder_suspend_all(display);
> >> -	}
> >> -
> >> -	intel_opregion_suspend(display, s2idle ? PCI_D1 : PCI_D3cold);
> >> -
> >> -	intel_dmc_suspend(display);
> >> +	intel_display_driver_pm_suspend(display);
> >>  }
> >>  
> >>  void xe_display_shutdown(struct xe_device *xe)
> >> @@ -335,29 +238,7 @@ void xe_display_shutdown(struct xe_device *xe)
> >>  	if (!xe->info.probe_display)
> >>  		return;
> >>  
> >> -	intel_display_power_disable(display);
> >> -	drm_client_dev_suspend(&xe->drm);
> >> -
> >> -	if (intel_display_device_present(display)) {
> >> -		drm_kms_helper_poll_disable(&xe->drm);
> >> -		intel_display_driver_disable_user_access(display);
> >> -		intel_display_driver_suspend(display);
> >> -	}
> >> -
> >> -	intel_display_flush_cleanup_work(display);
> >> -	intel_dp_mst_suspend(display);
> >> -	intel_encoder_block_all_hpds(display);
> >> -	intel_hpd_cancel_work(display);
> >> -
> >> -	if (intel_display_device_present(display))
> >> -		intel_display_driver_suspend_access(display);
> >> -
> >> -	intel_encoder_suspend_all(display);
> >> -	intel_encoder_shutdown_all(display);
> >> -
> >> -	intel_opregion_suspend(display, PCI_D3cold);
> >> -
> >> -	intel_dmc_suspend(display);
> >> +	intel_display_driver_shutdown(display);
> >>  }
> >>  
> >>  void xe_display_pm_runtime_suspend(struct xe_device *xe)
> >> @@ -368,22 +249,21 @@ void xe_display_pm_runtime_suspend(struct xe_device *xe)
> >>  		return;
> >>  
> >>  	if (xe->d3cold.allowed) {
> >> -		xe_display_enable_d3cold(xe);
> >> +		intel_display_driver_pm_enable_d3cold(display);
> >>  		return;
> >>  	}
> >>  
> >> -	intel_hpd_poll_enable(display);
> >> +	intel_display_driver_pm_runtime_suspend(display);
> >>  }
> >>  
> >>  void xe_display_pm_suspend_late(struct xe_device *xe)
> >>  {
> >>  	struct intel_display *display = xe->display;
> >> -	bool s2idle = suspend_to_idle();
> >>  
> >>  	if (!xe->info.probe_display)
> >>  		return;
> >>  
> >> -	intel_display_power_suspend_late(display, s2idle);
> >> +	intel_display_driver_pm_suspend_late(display);
> >>  }
> >>  
> >>  void xe_display_pm_runtime_suspend_late(struct xe_device *xe)
> >> @@ -394,14 +274,9 @@ void xe_display_pm_runtime_suspend_late(struct xe_device *xe)
> >>  		return;
> >>  
> >>  	if (xe->d3cold.allowed)
> >> -		xe_display_pm_suspend_late(xe);
> >> +		intel_display_driver_pm_suspend_late(display);
> >>  
> >> -	/*
> >> -	 * If xe_display_pm_suspend_late() is not called, it is likely
> >> -	 * that we will be on dynamic DC states with DMC wakelock enabled. We
> >> -	 * need to flush the release work in that case.
> >> -	 */
> >> -	intel_dmc_wl_flush_release_work(display);
> >> +	intel_display_driver_pm_runtime_suspend_late(display);
> >>  }
> >>  
> >>  void xe_display_shutdown_late(struct xe_device *xe)
> >> @@ -411,12 +286,7 @@ void xe_display_shutdown_late(struct xe_device *xe)
> >>  	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_display_power_driver_remove(display);
> >> +	intel_display_driver_shutdown_late(display);
> >>  }
> >>  
> >>  void xe_display_pm_resume_early(struct xe_device *xe)
> >> @@ -426,7 +296,7 @@ void xe_display_pm_resume_early(struct xe_device *xe)
> >>  	if (!xe->info.probe_display)
> >>  		return;
> >>  
> >> -	intel_display_power_resume_early(display);
> >> +	intel_display_driver_pm_resume_early(display);
> >>  }
> >>  
> >>  void xe_display_pm_resume(struct xe_device *xe)
> >> @@ -436,34 +306,7 @@ void xe_display_pm_resume(struct xe_device *xe)
> >>  	if (!xe->info.probe_display)
> >>  		return;
> >>  
> >> -	intel_dmc_resume(display);
> >> -
> >> -	if (intel_display_device_present(display))
> >> -		drm_mode_config_reset(&xe->drm);
> >> -
> >> -	intel_display_driver_init_hw(display);
> >> -
> >> -	if (intel_display_device_present(display))
> >> -		intel_display_driver_resume_access(display);
> >> -
> >> -	intel_hpd_init(display);
> >> -
> >> -	intel_encoder_unblock_all_hpds(display);
> >> -
> >> -	if (intel_display_device_present(display)) {
> >> -		intel_display_driver_resume(display);
> >> -		drm_kms_helper_poll_enable(&xe->drm);
> >> -		intel_display_driver_enable_user_access(display);
> >> -	}
> >> -
> >> -	if (intel_display_device_present(display))
> >> -		intel_hpd_poll_disable(display);
> >> -
> >> -	intel_opregion_resume(display);
> >> -
> >> -	drm_client_dev_resume(&xe->drm);
> >> -
> >> -	intel_display_power_enable(display);
> >> +	intel_display_driver_pm_resume(display);
> >>  }
> >>  
> >>  void xe_display_pm_runtime_resume(struct xe_device *xe)
> >> @@ -474,13 +317,11 @@ void xe_display_pm_runtime_resume(struct xe_device *xe)
> >>  		return;
> >>  
> >>  	if (xe->d3cold.allowed) {
> >> -		xe_display_disable_d3cold(xe);
> >> +		intel_display_driver_pm_disable_d3cold(display);
> >>  		return;
> >>  	}
> >>  
> >> -	intel_hpd_init(display);
> >> -	intel_hpd_poll_disable(display);
> >> -	skl_watermark_ipc_update(display);
> >> +	intel_display_driver_pm_runtime_resume(display);
> >>  }
> >>  
> >>  
> >> -- 
> >> 2.47.3
> 
> -- 
> Jani Nikula, Intel

  reply	other threads:[~2026-05-27 18:52 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-27 13:06 [PATCH 0/3] drm/{i915, xe}: relocate shutdown and pm hooks from xe to display Jani Nikula
2026-05-27 13:06 ` [PATCH 1/3] drm/xe/display: rename xe_display_pm_shutdown*() to xe_display_shutdown*() Jani Nikula
2026-05-27 13:06 ` [PATCH 2/3] drm/{i915, xe}: move xe display shutdown and pm hooks to intel_display_driver.c Jani Nikula
2026-05-27 14:59   ` Ville Syrjälä
2026-05-27 16:35     ` Jani Nikula
2026-05-27 18:52       ` Imre Deak [this message]
2026-05-27 19:01       ` Ville Syrjälä
2026-05-29 11:09         ` Jani Nikula
2026-05-27 13:06 ` [PATCH 3/3] drm/i915/display: move d3cold allowed handling to parent interface Jani Nikula
2026-05-27 13:55 ` ✓ i915.CI.BAT: success for drm/{i915, xe}: relocate shutdown and pm hooks from xe to display Patchwork
2026-05-27 20:56 ` ✓ i915.CI.Full: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ahc9WCALisq7cyrd@ideak-desk.lan \
    --to=imre.deak@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox