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

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Wed, Feb 12, 2025 at 10:33:18PM +0200, Imre Deak wrote:
> 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.

my next step is to split this into 2 different functions
getting rid of this runtime bool.

> 
> 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.

hmm indeed... I will take care of that while working on
this reconciliation with i915 order...

Thank you so much

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