* [PATCH] drm/xe: Inline gt_reset in the worker
@ 2025-10-29 19:24 Lucas De Marchi
2025-10-29 19:51 ` Matthew Brost
0 siblings, 1 reply; 3+ messages in thread
From: Lucas De Marchi @ 2025-10-29 19:24 UTC (permalink / raw)
To: intel-xe; +Cc: Lucas De Marchi, Matthew Brost
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>
---
| 32 ++++++++------------------------
1 file changed, 8 insertions(+), 24 deletions(-)
--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(>->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)
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] drm/xe: Inline gt_reset in the worker
2025-10-29 19:24 [PATCH] drm/xe: Inline gt_reset in the worker Lucas De Marchi
@ 2025-10-29 19:51 ` Matthew Brost
2025-10-30 20:05 ` Lucas De Marchi
0 siblings, 1 reply; 3+ messages in thread
From: Matthew Brost @ 2025-10-29 19:51 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-xe
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(>->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)
>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] drm/xe: Inline gt_reset in the worker
2025-10-29 19:51 ` Matthew Brost
@ 2025-10-30 20:05 ` Lucas De Marchi
0 siblings, 0 replies; 3+ messages in thread
From: Lucas De Marchi @ 2025-10-30 20:05 UTC (permalink / raw)
To: Matthew Brost; +Cc: intel-xe
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(>->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)
>>
>>
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-10-30 20:06 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-29 19:24 [PATCH] drm/xe: Inline gt_reset in the worker Lucas De Marchi
2025-10-29 19:51 ` Matthew Brost
2025-10-30 20:05 ` Lucas De Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox