Re: [PATCH] drm/xe/display: Remove hpd cancel work sync from runtime pm path

[Imre Deak <imre.deak@intel.com>]

On Wed, Feb 12, 2025 at 02:24:47PM -0500, Rodrigo Vivi wrote:
> This function will synchronously cancel and wait for many display
> work queue items, which might try to take the runtime pm reference
> causing a bad deadlock. So, remove it from the runtime_pm suspend patch.
> 
> Reported-by: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Makes sense, since instead of trying to cancel a pending a work from
here, the work should instead wake up the device if needed via RPM get:

Reviewed-by: Imre Deak <imre.deak@intel.com>

See some notes below.

> ---
>  drivers/gpu/drm/xe/display/xe_display.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> index 651799c946ac..f0f427689a56 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.c
> +++ b/drivers/gpu/drm/xe/display/xe_display.c
> @@ -311,7 +311,8 @@ static void __xe_display_pm_suspend(struct xe_device *xe, bool runtime)
>  
>  	xe_display_flush_cleanup_work(xe);
>  
> -	intel_hpd_cancel_work(xe);
> +	if (!runtime)
> +		intel_hpd_cancel_work(xe);

Could be moved to the block below.

AFAICS it's still in the wrong spot for system suspend, as in
xe_pm_suspend() the above function will be called before
xe_irq_suspend(), so a new interrupt/work could be scheduled
here right after intel_hpd_cancel_work(). But that's a separate issue.

>  
>  	if (!runtime && has_display(xe)) {
>  		intel_display_driver_suspend_access(display);
> -- 
> 2.48.1
>