Re: [PATCH 1/2] drm/xe: introduce xe_gt_reset_wait to wait for async gt reset

[Maciej Patelczyk <maciej.patelczyk@intel.com>]

On 9.12.2024 17:22, Lucas De Marchi wrote:

> On Mon, Dec 09, 2024 at 04:54:45PM +0200, Raag Jadav wrote:
>> On Mon, Dec 09, 2024 at 04:42:49PM +0200, Raag Jadav wrote:
>>> On Mon, Dec 09, 2024 at 03:12:09PM +0100, Maciej Patelczyk wrote:
>>> > The reset of the GT is asynchronous.
>>> > Adding a wait helper to wait until gt reset is done.
>>> >
>>> > Signed-off-by: Maciej Patelczyk <maciej.patelczyk@intel.com>
>>> > ---
>>> >  drivers/gpu/drm/xe/tests/xe_bo.c   |  2 +-
>>> >  drivers/gpu/drm/xe/tests/xe_mocs.c |  2 +-
>>> >  drivers/gpu/drm/xe/xe_gt.h         | 12 ++++++++++++
>>> >  drivers/gpu/drm/xe/xe_gt_debugfs.c |  2 +-
>>> >  4 files changed, 15 insertions(+), 3 deletions(-)
>>> >
>>> > diff --git a/drivers/gpu/drm/xe/tests/xe_bo.c 
>>> b/drivers/gpu/drm/xe/tests/xe_bo.c
>>> > index c9ec7a313c6b..b51434fc5bd1 100644
>>> > --- a/drivers/gpu/drm/xe/tests/xe_bo.c
>>> > +++ b/drivers/gpu/drm/xe/tests/xe_bo.c
>>> > @@ -266,7 +266,7 @@ static int evict_test_run_tile(struct 
>>> xe_device *xe, struct xe_tile *tile, struc
>>> >           */
>>> >          for_each_gt(__gt, xe, id) {
>>> >              xe_gt_reset_async(__gt);
>>> > -            flush_work(&__gt->reset.worker);
>>> > +            xe_gt_reset_wait(__gt);
>>>
>>> Why not just create a
>>>
>>> void xe_gt_reset_sync(gt)
>>> {
>>>     xe_gt_reset_async()
>>>     flush_work()
>>> }
>
>
> maybe, the problem I think is the 1 letter difference between
>
> xe_gt_reset_async
> xe_gt_reset_sync
>
> that may be confusing/error-prone.
>
> I like the helper, just think we need the naming to be polished.
>
> +Thomas
>
> maybe having this as xe_gt_reset() / xe_gt_reset_async()?
>
>
> Lucas De Marchi
>
xe_gt_reset(), xe_gt_reset_async() would be my vote.


I proposed only the wait helper as I did not want to block under mutex 
for the whole period of time when the reset worker will be scheduler and 
executed.

That could be seconds.

Maciej


>
>>>
>>> and use it instead?
>>
>> Or perhaps reuse force_reset_sync()?
>>
>> Raag