Re: [PATCH] drm/xe/xe_gt_debugfs: Add synchronous gt reset debugfs

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

On Thu, Jun 06, 2024 at 02:11:02PM +0000, Cavitt, Jonathan wrote:
> -----Original Message-----
> From: Vivi, Rodrigo <rodrigo.vivi@intel.com> 
> Sent: Wednesday, June 5, 2024 3:10 PM
> To: Cavitt, Jonathan <jonathan.cavitt@intel.com>
> Cc: intel-xe@lists.freedesktop.org; Gupta, saurabhg <saurabhg.gupta@intel.com>; Brost, Matthew <matthew.brost@intel.com>; Harrison, John C <john.c.harrison@intel.com>; Summers, Stuart <stuart.summers@intel.com>
> Subject: Re: [PATCH] drm/xe/xe_gt_debugfs: Add synchronous gt reset debugfs
> > 
> > On Wed, Jun 05, 2024 at 08:08:28AM -0700, Jonathan Cavitt wrote:
> > > We currently have debugfs support that allows the userspace to initiate
> > > an asynchronous gt reset on command.  However, userspace may also wish
> > > to wait for the completion of the gt reset before performing any
> > > additional work.  To that end, add a version of the force_reset gt
> > > debugfs function that operates synchronously.
> > > 
> > > Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1068
> > 
> > do we already have the igt changes that really closes this?
> 
> The IGT change that closes this issue has not yet landed.  It relies on
> using the new debugfs, so if it landed first, we'd be attempting to call
> a nonexistent debugfs function.

Please provide the IGT and the kernel patch at the same time.
you can copy patchwork or lore links in the one or the other cover letter
to explain and justify each other.

In the current way we cannot simply merge this because the 'Closes:' tag
is a lie.

Also, there's always the risk if doing the IGT in a later stage you realize
that you need slightly different and cause unnecessary rounds.

> -Jonathan Cavitt
> 
> > 
> > > Suggested-by: Matthew Brost <matthew.brost@intel.com>
> > > Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > > CC: John Harrison <john.c.harrison@intel.com>
> > > CC: Stuart Summers <stuart.summers@intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/xe_gt_debugfs.c | 12 ++++++++++++
> > >  1 file changed, 12 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_gt_debugfs.c b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> > > index 66f897a9b6ca9..5e7fd937917a1 100644
> > > --- a/drivers/gpu/drm/xe/xe_gt_debugfs.c
> > > +++ b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> > > @@ -116,6 +116,17 @@ static int force_reset(struct xe_gt *gt, struct drm_printer *p)
> > >  	return 0;
> > >  }
> > >  
> > > +static int force_reset_sync(struct xe_gt *gt, struct drm_printer *p)
> > > +{
> > > +	xe_pm_runtime_get(gt_to_xe(gt));
> > > +	xe_gt_reset_async(gt);
> > > +	xe_pm_runtime_put(gt_to_xe(gt));
> > > +
> > > +	flush_work(&gt->reset.worker);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >  static int sa_info(struct xe_gt *gt, struct drm_printer *p)
> > >  {
> > >  	struct xe_tile *tile = gt_to_tile(gt);
> > > @@ -261,6 +272,7 @@ static int vecs_default_lrc(struct xe_gt *gt, struct drm_printer *p)
> > >  static const struct drm_info_list debugfs_list[] = {
> > >  	{"hw_engines", .show = xe_gt_debugfs_simple_show, .data = hw_engines},
> > >  	{"force_reset", .show = xe_gt_debugfs_simple_show, .data = force_reset},
> > > +	{"force_reset_sync", .show = xe_gt_debugfs_simple_show, .data = force_reset_sync},
> > >  	{"sa_info", .show = xe_gt_debugfs_simple_show, .data = sa_info},
> > >  	{"topology", .show = xe_gt_debugfs_simple_show, .data = topology},
> > >  	{"steering", .show = xe_gt_debugfs_simple_show, .data = steering},
> > > -- 
> > > 2.25.1
> > > 
> >