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

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

On Thu, Jun 06, 2024 at 04:38:49PM +0000, Cavitt, Jonathan wrote:
> -----Original Message-----
> From: Vivi, Rodrigo <rodrigo.vivi@intel.com> 
> Sent: Thursday, June 6, 2024 9:26 AM
> 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 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.
> 
> Here's the related IGT change:
> https://patchwork.freedesktop.org/series/134509/

doh! I had missed that, I'm sorry.
I just pushed both sides, kernel and igt, since they were all reviewed.

> -Jonathan Cavitt
> 
> > 
> > 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
> > > > > 
> > > > 
> >