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

[Cavitt, Jonathan]

-----Original Message-----
From: Brost, Matthew <matthew.brost at intel.com> 
Sent: Thursday, June 6, 2024 10:38 AM
To: De Marchi, Lucas <lucas.demarchi at intel.com>
Cc: Cavitt, Jonathan <jonathan.cavitt at intel.com>; Vivi, Rodrigo <rodrigo.vivi at intel.com>; intel-xe at lists.freedesktop.org; Gupta, saurabhg <saurabhg.gupta at intel.com>; Harrison, John C <john.c.harrison at intel.com>; Summers, Stuart <stuart.summers at intel.com>
Subject: Re: [PATCH] drm/xe/xe_gt_debugfs: Add synchronous gt reset debugfs
> 
> On Thu, Jun 06, 2024 at 12:35:42PM -0500, Lucas De Marchi wrote:
> > On Thu, Jun 06, 2024 at 04:38:49PM GMT, Cavitt, Jonathan wrote:
> > > -----Original Message-----
> > > From: Vivi, Rodrigo <rodrigo.vivi at intel.com>
> > > Sent: Thursday, June 6, 2024 9:26 AM
> > > To: Cavitt, Jonathan <jonathan.cavitt at intel.com>
> > > Cc: intel-xe at lists.freedesktop.org; Gupta, saurabhg <saurabhg.gupta at intel.com>; Brost, Matthew <matthew.brost at intel.com>; Harrison, John C <john.c.harrison at intel.com>; Summers, Stuart <stuart.summers at 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 at intel.com>
> > > > > Sent: Wednesday, June 5, 2024 3:10 PM
> > > > > To: Cavitt, Jonathan <jonathan.cavitt at intel.com>
> > > > > Cc: intel-xe at lists.freedesktop.org; Gupta, saurabhg <saurabhg.gupta at intel.com>; Brost, Matthew <matthew.brost at intel.com>; Harrison, John C <john.c.harrison at intel.com>; Summers, Stuart <stuart.summers at 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/
> > 
> > great. Let's remove the Closes from this patch then.
> > We can do that while applying, but...
> > 
> 
> I think Rodrigo just pushed these...
> 
> > > > > > > @@ -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));
> > > > > > > +
> > 
> > why aren't we just calling force_reset(gt, p); here?
> > 
> 
> Opps, I missed that in my review and agree what you suggest is better.
> Given that Rodrigo just pushed this, let's do a quick follow up to fix?
> 

Will post follow-up later today.
-Jonathan Cavitt

> Matt 
> 
> > Lucas De Marchi
> > 
> > > > > > > +	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
> > > > > > >
> > > > > >
> > > > 
>