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

[Lucas De Marchi]

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

>> > > > @@ -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?

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