Re: [PATCH 2/2] drm/xe: Add dbg messages on the suspend resume functions.

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

On Mon, Mar 18, 2024 at 06:12:44PM +0000, Matthew Auld wrote:
> On 18/03/2024 18:01, Rodrigo Vivi wrote:
> > In case of the suspend/resume flow getting locked up we
> > can get reports with some useful hints on where it might
> > get locked and if that has failed.
> > 
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> Makes sense. What about maybe also adding that to the rpm versions? Those
> can also be fun, and so would be useful to get hints when inside the
> callbacks.

I'm planning to get that on RPM next... just was trying to avoid
conflicting with myself ;)
The bug that I'm targeting with this right now is a suspend to memory.

And I was afraid that someone might complain about verbosity in the rpm
path on cases where gnome-shell keeps doing some ioctl and waking up
the device.

> 
> > ---
> >   drivers/gpu/drm/xe/xe_pm.c | 22 +++++++++++++++++-----
> >   1 file changed, 17 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> > index 9fbb6f6c598a..cc650a92c2fc 100644
> > --- a/drivers/gpu/drm/xe/xe_pm.c
> > +++ b/drivers/gpu/drm/xe/xe_pm.c
> > @@ -80,13 +80,15 @@ int xe_pm_suspend(struct xe_device *xe)
> >   	u8 id;
> >   	int err;
> > +	drm_dbg(&xe->drm, "Suspending device\n");
> > +
> >   	for_each_gt(gt, xe, id)
> >   		xe_gt_suspend_prepare(gt);
> >   	/* FIXME: Super racey... */
> >   	err = xe_bo_evict_all(xe);
> >   	if (err)
> > -		return err;
> > +		goto err;
> >   	xe_display_pm_suspend(xe);
> > @@ -94,7 +96,7 @@ int xe_pm_suspend(struct xe_device *xe)
> >   		err = xe_gt_suspend(gt);
> >   		if (err) {
> >   			xe_display_pm_resume(xe);
> > -			return err;
> > +			goto err;
> >   		}
> >   	}
> > @@ -102,7 +104,11 @@ int xe_pm_suspend(struct xe_device *xe)
> >   	xe_display_pm_suspend_late(xe);
> > +	drm_dbg(&xe->drm, "Device suspended\n");
> >   	return 0;
> > +err:
> > +	drm_dbg(&xe->drm, "Device suspend failed %d\n", err);
> > +	return err;
> >   }
> >   /**
> > @@ -118,13 +124,15 @@ int xe_pm_resume(struct xe_device *xe)
> >   	u8 id;
> >   	int err;
> > +	drm_dbg(&xe->drm, "Resuming device\n");
> > +
> >   	for_each_tile(tile, xe, id)
> >   		xe_wa_apply_tile_workarounds(tile);
> >   	for_each_gt(gt, xe, id) {
> >   		err = xe_pcode_init(gt);
> >   		if (err)
> > -			return err;
> > +			goto err;
> >   	}
> >   	xe_display_pm_resume_early(xe);
> > @@ -135,7 +143,7 @@ int xe_pm_resume(struct xe_device *xe)
> >   	 */
> >   	err = xe_bo_restore_kernel(xe);
> >   	if (err)
> > -		return err;
> > +		goto err;
> >   	xe_irq_resume(xe);
> > @@ -146,9 +154,13 @@ int xe_pm_resume(struct xe_device *xe)
> >   	err = xe_bo_restore_user(xe);
> >   	if (err)
> > -		return err;
> > +		goto err;
> > +	drm_dbg(&xe->drm, "Device resumed\n");
> >   	return 0;
> > +err:
> > +	drm_dbg(&xe->drm, "Device resume failed %d\n", err);
> > +	return err;
> >   }
> >   static bool xe_pm_pci_d3cold_capable(struct xe_device *xe)