[PATCH] drm/xe: Inline gt_reset in the worker

[PATCH] drm/xe: Inline gt_reset in the worker

[Lucas De Marchi]

gt_reset() doesn't make sense by itself: it can only be called as part
of the worker. Inline it there to avoid it being called from elsewhere
and clarify the gt_reset() vs do_gt_reset() paths. Note that the error
return from gt_reset() was just being ignored.

Also add a comment to the xe_pm_runtime_put() to make sure the
get()/put() pair is clear.

Cc: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/xe/xe_gt.c | 32 ++++++++------------------------
 1 file changed, 8 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
index 89808b33d0a8b..51d6b4e4e1311 100644
--- a/drivers/gpu/drm/xe/xe_gt.c
+++ b/drivers/gpu/drm/xe/xe_gt.c
@@ -813,36 +813,29 @@ static int do_gt_restart(struct xe_gt *gt)
 	return 0;
 }
 
-static int gt_reset(struct xe_gt *gt)
+static void gt_reset_worker(struct work_struct *w)
 {
+	struct xe_gt *gt = container_of(w, typeof(*gt), reset.worker);
 	unsigned int fw_ref;
 	int err;
 
-	if (xe_device_wedged(gt_to_xe(gt))) {
-		err = -ECANCELED;
+	if (xe_device_wedged(gt_to_xe(gt)))
 		goto err_pm_put;
-	}
 
 	/* We only support GT resets with GuC submission */
-	if (!xe_device_uc_enabled(gt_to_xe(gt))) {
-		err = -ENODEV;
+	if (!xe_device_uc_enabled(gt_to_xe(gt)))
 		goto err_pm_put;
-	}
 
 	xe_gt_info(gt, "reset started\n");
 
-	if (xe_fault_inject_gt_reset()) {
-		err = -ECANCELED;
+	if (xe_fault_inject_gt_reset())
 		goto err_fail;
-	}
 
 	xe_gt_sanitize(gt);
 
 	fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
-	if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL)) {
-		err = -ETIMEDOUT;
+	if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL))
 		goto err_out;
-	}
 
 	if (IS_SRIOV_PF(gt_to_xe(gt)))
 		xe_gt_sriov_pf_stop_prepare(gt);
@@ -864,12 +857,12 @@ static int gt_reset(struct xe_gt *gt)
 		goto err_out;
 
 	xe_force_wake_put(gt_to_fw(gt), fw_ref);
+
+	/* Pair with get while enqueueing the work in xe_gt_reset_async() */
 	xe_pm_runtime_put(gt_to_xe(gt));
 
 	xe_gt_info(gt, "reset done\n");
 
-	return 0;
-
 err_out:
 	xe_force_wake_put(gt_to_fw(gt), fw_ref);
 	XE_WARN_ON(xe_uc_start(&gt->uc));
@@ -879,15 +872,6 @@ static int gt_reset(struct xe_gt *gt)
 	xe_device_declare_wedged(gt_to_xe(gt));
 err_pm_put:
 	xe_pm_runtime_put(gt_to_xe(gt));
-
-	return err;
-}
-
-static void gt_reset_worker(struct work_struct *w)
-{
-	struct xe_gt *gt = container_of(w, typeof(*gt), reset.worker);
-
-	gt_reset(gt);
 }
 
 void xe_gt_reset_async(struct xe_gt *gt)

Re: [PATCH] drm/xe: Inline gt_reset in the worker

[Matthew Brost]

On Wed, Oct 29, 2025 at 12:24:59PM -0700, Lucas De Marchi wrote:
> gt_reset() doesn't make sense by itself: it can only be called as part
> of the worker. Inline it there to avoid it being called from elsewhere
> and clarify the gt_reset() vs do_gt_reset() paths. Note that the error
> return from gt_reset() was just being ignored.
> 
> Also add a comment to the xe_pm_runtime_put() to make sure the
> get()/put() pair is clear.
> 
> Cc: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_gt.c | 32 ++++++++------------------------
>  1 file changed, 8 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> index 89808b33d0a8b..51d6b4e4e1311 100644
> --- a/drivers/gpu/drm/xe/xe_gt.c
> +++ b/drivers/gpu/drm/xe/xe_gt.c
> @@ -813,36 +813,29 @@ static int do_gt_restart(struct xe_gt *gt)
>  	return 0;
>  }
>  
> -static int gt_reset(struct xe_gt *gt)
> +static void gt_reset_worker(struct work_struct *w)
>  {
> +	struct xe_gt *gt = container_of(w, typeof(*gt), reset.worker);
>  	unsigned int fw_ref;
>  	int err;
>  
> -	if (xe_device_wedged(gt_to_xe(gt))) {
> -		err = -ECANCELED;
> +	if (xe_device_wedged(gt_to_xe(gt)))
>  		goto err_pm_put;
> -	}
>  
>  	/* We only support GT resets with GuC submission */
> -	if (!xe_device_uc_enabled(gt_to_xe(gt))) {
> -		err = -ENODEV;
> +	if (!xe_device_uc_enabled(gt_to_xe(gt)))
>  		goto err_pm_put;
> -	}
>  
>  	xe_gt_info(gt, "reset started\n");
>  
> -	if (xe_fault_inject_gt_reset()) {
> -		err = -ECANCELED;
> +	if (xe_fault_inject_gt_reset())
>  		goto err_fail;
> -	}
>  
>  	xe_gt_sanitize(gt);
>  
>  	fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> -	if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL)) {
> -		err = -ETIMEDOUT;
> +	if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL))
>  		goto err_out;
> -	}
>  
>  	if (IS_SRIOV_PF(gt_to_xe(gt)))
>  		xe_gt_sriov_pf_stop_prepare(gt);
> @@ -864,12 +857,12 @@ static int gt_reset(struct xe_gt *gt)
>  		goto err_out;
>  
>  	xe_force_wake_put(gt_to_fw(gt), fw_ref);
> +
> +	/* Pair with get while enqueueing the work in xe_gt_reset_async() */

Nit, I'd add commit on other side of this (i.e., the get) too.

Feell free to add that when merging. Good cleanup.

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

>  	xe_pm_runtime_put(gt_to_xe(gt));
>  
>  	xe_gt_info(gt, "reset done\n");
>  
> -	return 0;
> -
>  err_out:
>  	xe_force_wake_put(gt_to_fw(gt), fw_ref);
>  	XE_WARN_ON(xe_uc_start(&gt->uc));
> @@ -879,15 +872,6 @@ static int gt_reset(struct xe_gt *gt)
>  	xe_device_declare_wedged(gt_to_xe(gt));
>  err_pm_put:
>  	xe_pm_runtime_put(gt_to_xe(gt));
> -
> -	return err;
> -}
> -
> -static void gt_reset_worker(struct work_struct *w)
> -{
> -	struct xe_gt *gt = container_of(w, typeof(*gt), reset.worker);
> -
> -	gt_reset(gt);
>  }
>  
>  void xe_gt_reset_async(struct xe_gt *gt)
> 
> 
> 

Re: [PATCH] drm/xe: Inline gt_reset in the worker

[Lucas De Marchi]

On Wed, Oct 29, 2025 at 12:51:23PM -0700, Matthew Brost wrote:
>On Wed, Oct 29, 2025 at 12:24:59PM -0700, Lucas De Marchi wrote:
>> gt_reset() doesn't make sense by itself: it can only be called as part
>> of the worker. Inline it there to avoid it being called from elsewhere
>> and clarify the gt_reset() vs do_gt_reset() paths. Note that the error
>> return from gt_reset() was just being ignored.
>>
>> Also add a comment to the xe_pm_runtime_put() to make sure the
>> get()/put() pair is clear.
>>
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_gt.c | 32 ++++++++------------------------
>>  1 file changed, 8 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
>> index 89808b33d0a8b..51d6b4e4e1311 100644
>> --- a/drivers/gpu/drm/xe/xe_gt.c
>> +++ b/drivers/gpu/drm/xe/xe_gt.c
>> @@ -813,36 +813,29 @@ static int do_gt_restart(struct xe_gt *gt)
>>  	return 0;
>>  }
>>
>> -static int gt_reset(struct xe_gt *gt)
>> +static void gt_reset_worker(struct work_struct *w)
>>  {
>> +	struct xe_gt *gt = container_of(w, typeof(*gt), reset.worker);
>>  	unsigned int fw_ref;
>>  	int err;
>>
>> -	if (xe_device_wedged(gt_to_xe(gt))) {
>> -		err = -ECANCELED;
>> +	if (xe_device_wedged(gt_to_xe(gt)))
>>  		goto err_pm_put;
>> -	}
>>
>>  	/* We only support GT resets with GuC submission */
>> -	if (!xe_device_uc_enabled(gt_to_xe(gt))) {
>> -		err = -ENODEV;
>> +	if (!xe_device_uc_enabled(gt_to_xe(gt)))
>>  		goto err_pm_put;
>> -	}
>>
>>  	xe_gt_info(gt, "reset started\n");
>>
>> -	if (xe_fault_inject_gt_reset()) {
>> -		err = -ECANCELED;
>> +	if (xe_fault_inject_gt_reset())
>>  		goto err_fail;
>> -	}
>>
>>  	xe_gt_sanitize(gt);
>>
>>  	fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
>> -	if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL)) {
>> -		err = -ETIMEDOUT;
>> +	if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL))
>>  		goto err_out;
>> -	}
>>
>>  	if (IS_SRIOV_PF(gt_to_xe(gt)))
>>  		xe_gt_sriov_pf_stop_prepare(gt);
>> @@ -864,12 +857,12 @@ static int gt_reset(struct xe_gt *gt)
>>  		goto err_out;
>>
>>  	xe_force_wake_put(gt_to_fw(gt), fw_ref);
>> +
>> +	/* Pair with get while enqueueing the work in xe_gt_reset_async() */
>
>Nit, I'd add commit on other side of this (i.e., the get) too.

will do

>
>Feell free to add that when merging. Good cleanup.

I will send a [CI]  patch, because it seems patchwork was dead when I
sent so there's no CI results.

>
>Reviewed-by: Matthew Brost <matthew.brost@intel.com>

thanks
Lucas De Marchi

>
>>  	xe_pm_runtime_put(gt_to_xe(gt));
>>
>>  	xe_gt_info(gt, "reset done\n");
>>
>> -	return 0;
>> -
>>  err_out:
>>  	xe_force_wake_put(gt_to_fw(gt), fw_ref);
>>  	XE_WARN_ON(xe_uc_start(&gt->uc));
>> @@ -879,15 +872,6 @@ static int gt_reset(struct xe_gt *gt)
>>  	xe_device_declare_wedged(gt_to_xe(gt));
>>  err_pm_put:
>>  	xe_pm_runtime_put(gt_to_xe(gt));
>> -
>> -	return err;
>> -}
>> -
>> -static void gt_reset_worker(struct work_struct *w)
>> -{
>> -	struct xe_gt *gt = container_of(w, typeof(*gt), reset.worker);
>> -
>> -	gt_reset(gt);
>>  }
>>
>>  void xe_gt_reset_async(struct xe_gt *gt)
>>
>>
>>