Re: [PATCH] drm/xe: Move drm_client_dev_suspend/resume outside pm block signalling

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 17 Jun 2026, Tejas Upadhyay <tejas.upadhyay@intel.com> wrote:
> A circular locking dependency is reported between xe_pm_block_map and
> dev->clientlist_mutex:
>
>   &dev->clientlist_mutex --> &vm->lock --> xe_pm_block_map --> &dev->clientlist_mutex
>
> The dependency chain is:
> 1. xe_pm_suspend() calls xe_pm_block_begin_signalling(), then
>    xe_display_pm_suspend() which calls drm_client_dev_suspend()
>    acquiring clientlist_mutex. (xe_pm_block_map -> clientlist_mutex)
>
> 2. drm_client_dev_unregister() -> drm_file_free() -> xe_file_close() ->
>    prelim_xe_eudebug_file_close() acquires discovery_lock.
>    (clientlist_mutex -> discovery_lock)
>
> 3. xe_vm_close_and_put() acquires vm->lock under discovery_lock.
>    (discovery_lock -> vm->lock)
>
> 4. xe_exec_ioctl() holds vm->lock and calls xe_pm_block_on_suspend()
>    which waits on xe_pm_block_map. (vm->lock -> xe_pm_block_map)
>
> Fix this by moving drm_client_dev_suspend() before
> xe_pm_block_begin_signalling() in xe_pm_suspend(), and
> drm_client_dev_resume() after xe_pm_block_end_signalling() in
> xe_pm_resume(). This breaks the xe_pm_block_map -> clientlist_mutex
> edge in the dependency chain.
>
> The drm_client_dev_suspend/resume calls are safe to move because they
> only blank/unblank the fbdev console and don't depend on display power
> state or any operations within the block signalling scope.

What baseline are you using?

See f597d4b2fd0d ("drm/{i915, xe}: move more calls inside
intel_display_driver_pm_resume()"), pushed almost two weeks ago.

The direction must be to move display related calls to display, and
neither i915 nor xe core driver should be messing with anything to do
with display directly.

Need to find another solution.

BR,
Jani.

>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Badal Nilawar <badal.nilawar@intel.com>
> Cc: Raag Jadav <raag.jadav@intel.com>
> Signed-off-by: Tejas Upadhyay <tejas.upadhyay@intel.com>
> ---
>  drivers/gpu/drm/xe/display/xe_display.c | 3 ---
>  drivers/gpu/drm/xe/xe_pm.c              | 5 +++++
>  2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> index 810d93fefcbc..ec65dcbc3fdc 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.c
> +++ b/drivers/gpu/drm/xe/display/xe_display.c
> @@ -304,7 +304,6 @@ void xe_display_pm_suspend(struct xe_device *xe)
>  	 * 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);
> @@ -461,8 +460,6 @@ void xe_display_pm_resume(struct xe_device *xe)
>  
>  	intel_opregion_resume(display);
>  
> -	drm_client_dev_resume(&xe->drm);
> -
>  	intel_display_power_enable(display);
>  }
>  
> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> index 99562f691080..d00c620d0926 100644
> --- a/drivers/gpu/drm/xe/xe_pm.c
> +++ b/drivers/gpu/drm/xe/xe_pm.c
> @@ -10,6 +10,7 @@
>  #include <linux/suspend.h>
>  #include <linux/dmi.h>
>  
> +#include <drm/drm_client_event.h>
>  #include <drm/drm_managed.h>
>  #include <drm/ttm/ttm_placement.h>
>  
> @@ -177,6 +178,7 @@ int xe_pm_suspend(struct xe_device *xe)
>  	int err;
>  
>  	drm_dbg(&xe->drm, "Suspending device\n");
> +	drm_client_dev_suspend(&xe->drm);
>  	xe_pm_block_begin_signalling();
>  	trace_xe_pm_suspend(xe, __builtin_return_address(0));
>  
> @@ -219,6 +221,7 @@ int xe_pm_suspend(struct xe_device *xe)
>  err:
>  	drm_dbg(&xe->drm, "Device suspend failed %d\n", err);
>  	xe_pm_block_end_signalling();
> +	drm_client_dev_resume(&xe->drm);
>  	return err;
>  }
>  
> @@ -292,10 +295,12 @@ int xe_pm_resume(struct xe_device *xe)
>  
>  	drm_dbg(&xe->drm, "Device resumed\n");
>  	xe_pm_block_end_signalling();
> +	drm_client_dev_resume(&xe->drm);
>  	return 0;
>  err:
>  	drm_dbg(&xe->drm, "Device resume failed %d\n", err);
>  	xe_pm_block_end_signalling();
> +	drm_client_dev_resume(&xe->drm);
>  	return err;
>  }

-- 
Jani Nikula, Intel