* [PATCH] drm/xe: Make sysfs gt force reset synchronous
@ 2023-12-21 17:16 Karthik Poosa
2023-12-21 18:59 ` Matthew Brost
0 siblings, 1 reply; 4+ messages in thread
From: Karthik Poosa @ 2023-12-21 17:16 UTC (permalink / raw)
To: intel-xe; +Cc: Karthik Poosa
Wait for gt reset to complete before returning from force_reset
sysfs call. Without this igt test freq_reset_multiple fails
sporadically in case xe_guc_pc is not started.
v2:
- Rebase.
- Changed wait for completion to interruptible (Anshuman).
- Moved timeout to xe_gt.h (Anshuman).
Testcase: igt@xe_guc_pc@freq_reset_multiple
Signed-off-by: Karthik Poosa <karthik.poosa@intel.com>
---
drivers/gpu/drm/xe/xe_gt.c | 2 ++
drivers/gpu/drm/xe/xe_gt.h | 1 +
drivers/gpu/drm/xe/xe_gt_debugfs.c | 8 ++++++++
drivers/gpu/drm/xe/xe_gt_types.h | 3 +++
4 files changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
index 3af2adec1295..47abb9336c58 100644
--- a/drivers/gpu/drm/xe/xe_gt.c
+++ b/drivers/gpu/drm/xe/xe_gt.c
@@ -65,6 +65,7 @@ struct xe_gt *xe_gt_alloc(struct xe_tile *tile)
gt->tile = tile;
gt->ordered_wq = alloc_ordered_workqueue("gt-ordered-wq", 0);
+ init_completion(>->reset_done);
return gt;
}
@@ -633,6 +634,7 @@ static int gt_reset(struct xe_gt *gt)
xe_device_mem_access_put(gt_to_xe(gt));
XE_WARN_ON(err);
+ complete(>->reset_done);
xe_gt_info(gt, "reset done\n");
return 0;
diff --git a/drivers/gpu/drm/xe/xe_gt.h b/drivers/gpu/drm/xe/xe_gt.h
index 4486e083f5ef..591f5271c07d 100644
--- a/drivers/gpu/drm/xe/xe_gt.h
+++ b/drivers/gpu/drm/xe/xe_gt.h
@@ -17,6 +17,7 @@
xe_hw_engine_is_valid((hwe__)))
#define CCS_MASK(gt) (((gt)->info.engine_mask & XE_HW_ENGINE_CCS_MASK) >> XE_HW_ENGINE_CCS0)
+#define XE_GT_RESET_TIMEOUT_MS (msecs_to_jiffies(5 * 1000))
#ifdef CONFIG_FAULT_INJECTION
#include <linux/fault-inject.h> /* XXX: fault-inject.h is broken */
diff --git a/drivers/gpu/drm/xe/xe_gt_debugfs.c b/drivers/gpu/drm/xe/xe_gt_debugfs.c
index c4b67cf09f8f..543515e91c3d 100644
--- a/drivers/gpu/drm/xe/xe_gt_debugfs.c
+++ b/drivers/gpu/drm/xe/xe_gt_debugfs.c
@@ -58,8 +58,16 @@ static int hw_engines(struct seq_file *m, void *data)
static int force_reset(struct seq_file *m, void *data)
{
struct xe_gt *gt = node_to_gt(m->private);
+ struct xe_device *xe = gt_to_xe(gt);
+ long ret;
xe_gt_reset_async(gt);
+ ret = wait_for_completion_interruptible_timeout(>->reset_done,
+ XE_GT_RESET_TIMEOUT_MS);
+ if (ret <= 0) {
+ drm_err(&xe->drm, "gt reset timed out/interrputed, ret %ld\n", ret);
+ return -ETIMEDOUT;
+ }
return 0;
}
diff --git a/drivers/gpu/drm/xe/xe_gt_types.h b/drivers/gpu/drm/xe/xe_gt_types.h
index f74684660475..58396561fd6c 100644
--- a/drivers/gpu/drm/xe/xe_gt_types.h
+++ b/drivers/gpu/drm/xe/xe_gt_types.h
@@ -358,6 +358,9 @@ struct xe_gt {
/** @oob: bitmap with active OOB workaroudns */
unsigned long *oob;
} wa_active;
+
+ /** @reset_done : completion for GT reset */
+ struct completion reset_done;
};
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/xe: Make sysfs gt force reset synchronous
2023-12-21 17:16 [PATCH] drm/xe: Make sysfs gt force reset synchronous Karthik Poosa
@ 2023-12-21 18:59 ` Matthew Brost
0 siblings, 0 replies; 4+ messages in thread
From: Matthew Brost @ 2023-12-21 18:59 UTC (permalink / raw)
To: Karthik Poosa; +Cc: intel-xe
On Thu, Dec 21, 2023 at 10:46:41PM +0530, Karthik Poosa wrote:
> Wait for gt reset to complete before returning from force_reset
> sysfs call. Without this igt test freq_reset_multiple fails
> sporadically in case xe_guc_pc is not started.
>
> v2:
> - Rebase.
> - Changed wait for completion to interruptible (Anshuman).
> - Moved timeout to xe_gt.h (Anshuman).
>
The patch itself makes sense but small issue / NIT with variable
placement / name. Details below.
> Testcase: igt@xe_guc_pc@freq_reset_multiple
> Signed-off-by: Karthik Poosa <karthik.poosa@intel.com>
> ---
> drivers/gpu/drm/xe/xe_gt.c | 2 ++
> drivers/gpu/drm/xe/xe_gt.h | 1 +
> drivers/gpu/drm/xe/xe_gt_debugfs.c | 8 ++++++++
> drivers/gpu/drm/xe/xe_gt_types.h | 3 +++
> 4 files changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> index 3af2adec1295..47abb9336c58 100644
> --- a/drivers/gpu/drm/xe/xe_gt.c
> +++ b/drivers/gpu/drm/xe/xe_gt.c
> @@ -65,6 +65,7 @@ struct xe_gt *xe_gt_alloc(struct xe_tile *tile)
>
> gt->tile = tile;
> gt->ordered_wq = alloc_ordered_workqueue("gt-ordered-wq", 0);
> + init_completion(>->reset_done);
>
> return gt;
> }
> @@ -633,6 +634,7 @@ static int gt_reset(struct xe_gt *gt)
> xe_device_mem_access_put(gt_to_xe(gt));
> XE_WARN_ON(err);
>
> + complete(>->reset_done);
> xe_gt_info(gt, "reset done\n");
>
> return 0;
> diff --git a/drivers/gpu/drm/xe/xe_gt.h b/drivers/gpu/drm/xe/xe_gt.h
> index 4486e083f5ef..591f5271c07d 100644
> --- a/drivers/gpu/drm/xe/xe_gt.h
> +++ b/drivers/gpu/drm/xe/xe_gt.h
> @@ -17,6 +17,7 @@
> xe_hw_engine_is_valid((hwe__)))
>
> #define CCS_MASK(gt) (((gt)->info.engine_mask & XE_HW_ENGINE_CCS_MASK) >> XE_HW_ENGINE_CCS0)
> +#define XE_GT_RESET_TIMEOUT_MS (msecs_to_jiffies(5 * 1000))
>
> #ifdef CONFIG_FAULT_INJECTION
> #include <linux/fault-inject.h> /* XXX: fault-inject.h is broken */
> diff --git a/drivers/gpu/drm/xe/xe_gt_debugfs.c b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> index c4b67cf09f8f..543515e91c3d 100644
> --- a/drivers/gpu/drm/xe/xe_gt_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> @@ -58,8 +58,16 @@ static int hw_engines(struct seq_file *m, void *data)
> static int force_reset(struct seq_file *m, void *data)
> {
> struct xe_gt *gt = node_to_gt(m->private);
> + struct xe_device *xe = gt_to_xe(gt);
> + long ret;
>
> xe_gt_reset_async(gt);
> + ret = wait_for_completion_interruptible_timeout(>->reset_done,
> + XE_GT_RESET_TIMEOUT_MS);
> + if (ret <= 0) {
> + drm_err(&xe->drm, "gt reset timed out/interrputed, ret %ld\n", ret);
> + return -ETIMEDOUT;
> + }
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/xe/xe_gt_types.h b/drivers/gpu/drm/xe/xe_gt_types.h
> index f74684660475..58396561fd6c 100644
> --- a/drivers/gpu/drm/xe/xe_gt_types.h
> +++ b/drivers/gpu/drm/xe/xe_gt_types.h
> @@ -358,6 +358,9 @@ struct xe_gt {
> /** @oob: bitmap with active OOB workaroudns */
> unsigned long *oob;
> } wa_active;
> +
> + /** @reset_done : completion for GT reset */
> + struct completion reset_done;
This should be reset sub-structure:
144 /** @reset: state for GT resets */
145 struct {
146 /**
147 * @worker: work so GT resets can done async allowing to reset
148 * code to safely flush all code paths
149 */
150 struct work_struct worker;
151 } reset;
I'd move here and rename variable to 'done' so in the code it is
'reset.done'.
Matt
> };
>
> #endif
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] drm/xe: Make sysfs gt force reset synchronous
2023-12-16 5:04 Karthik Poosa
@ 2023-12-16 5:10 ` Gupta, Anshuman
0 siblings, 0 replies; 4+ messages in thread
From: Gupta, Anshuman @ 2023-12-16 5:10 UTC (permalink / raw)
To: Poosa, Karthik, intel-xe
> -----Original Message-----
> From: Poosa, Karthik <karthik.poosa@intel.com>
> Sent: Saturday, December 16, 2023 10:35 AM
> To: intel-xe@lists.freedesktop.org
> Cc: Gupta, Anshuman <anshuman.gupta@intel.com>; Nilawar, Badal
> <badal.nilawar@intel.com>; Brost, Matthew <matthew.brost@intel.com>; Poosa,
> Karthik <karthik.poosa@intel.com>
> Subject: [PATCH] drm/xe: Make sysfs gt force reset synchronous
>
> Wait for gt reset to complete before returning from force_reset sysfs call. Without
> this igt test freq_reset_multiple fails sporadically in case xe_guc_pc is not started.
>
> Testcase: igt@xe_guc_pc@freq_reset_multiple
> Signed-off-by: Karthik Poosa <karthik.poosa@intel.com>
> Reviewed-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
> drivers/gpu/drm/xe/xe_gt.c | 3 +++
> drivers/gpu/drm/xe/xe_gt_debugfs.c | 10 ++++++++++
> drivers/gpu/drm/xe/xe_gt_types.h | 3 +++
> 3 files changed, 16 insertions(+)
>
> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c index
> dfd9cf01a5d5..eb7552b6dfa5 100644
> --- a/drivers/gpu/drm/xe/xe_gt.c
> +++ b/drivers/gpu/drm/xe/xe_gt.c
> @@ -65,6 +65,7 @@ struct xe_gt *xe_gt_alloc(struct xe_tile *tile)
>
> gt->tile = tile;
> gt->ordered_wq = alloc_ordered_workqueue("gt-ordered-wq", 0);
> + init_completion(>->reset_done);
>
> return gt;
> }
> @@ -647,6 +648,8 @@ static int gt_reset(struct xe_gt *gt)
> xe_device_mem_access_put(gt_to_xe(gt));
> XE_WARN_ON(err);
>
> + complete(>->reset_done);
Don't use unnecessary whitespace, take a look at coding guidelines.
https://www.kernel.org/doc/html/v4.10/process/coding-style.html
> +
> xe_gt_info(gt, "reset done\n");
>
> return 0;
> diff --git a/drivers/gpu/drm/xe/xe_gt_debugfs.c
> b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> index c4b67cf09f8f..f4ff0eef0468 100644
> --- a/drivers/gpu/drm/xe/xe_gt_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> @@ -23,6 +23,8 @@
> #include "xe_uc_debugfs.h"
> #include "xe_wa.h"
>
> +#define XE_GT_RESET_TIMEOUT_MS (msecs_to_jiffies(5 * 1000))
We should define this to somewhere else, not in to debugfs headers.
> +
> static struct xe_gt *node_to_gt(struct drm_info_node *node) {
> return node->info_ent->data;
> @@ -58,9 +60,17 @@ static int hw_engines(struct seq_file *m, void *data) static
> int force_reset(struct seq_file *m, void *data) {
> struct xe_gt *gt = node_to_gt(m->private);
> + struct xe_device *xe = gt_to_xe(gt);
> + unsigned long timeout;
>
> xe_gt_reset_async(gt);
>
> + timeout = wait_for_completion_timeout(>->reset_done,
> XE_GT_RESET_TIMEOUT_MS);
Please use interruptible version of = wait_for_completion_timeout.
Thanks,
Anshuman.
> + if (timeout == 0) {
> + drm_err(&xe->drm, "gt reset timed out");
> + return -ETIMEDOUT;
> + }
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/xe/xe_gt_types.h
> b/drivers/gpu/drm/xe/xe_gt_types.h
> index f74684660475..6f2fb9e3cfea 100644
> --- a/drivers/gpu/drm/xe/xe_gt_types.h
> +++ b/drivers/gpu/drm/xe/xe_gt_types.h
> @@ -358,6 +358,9 @@ struct xe_gt {
> /** @oob: bitmap with active OOB workaroudns */
> unsigned long *oob;
> } wa_active;
> +
> + /** @reset_done : Completion of GT reset */
> + struct completion reset_done;
> };
>
> #endif
> --
> 2.25.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] drm/xe: Make sysfs gt force reset synchronous
@ 2023-12-16 5:04 Karthik Poosa
2023-12-16 5:10 ` Gupta, Anshuman
0 siblings, 1 reply; 4+ messages in thread
From: Karthik Poosa @ 2023-12-16 5:04 UTC (permalink / raw)
To: intel-xe; +Cc: Karthik Poosa
Wait for gt reset to complete before returning from force_reset
sysfs call. Without this igt test freq_reset_multiple fails
sporadically in case xe_guc_pc is not started.
Testcase: igt@xe_guc_pc@freq_reset_multiple
Signed-off-by: Karthik Poosa <karthik.poosa@intel.com>
Reviewed-by: Badal Nilawar <badal.nilawar@intel.com>
---
drivers/gpu/drm/xe/xe_gt.c | 3 +++
drivers/gpu/drm/xe/xe_gt_debugfs.c | 10 ++++++++++
drivers/gpu/drm/xe/xe_gt_types.h | 3 +++
3 files changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
index dfd9cf01a5d5..eb7552b6dfa5 100644
--- a/drivers/gpu/drm/xe/xe_gt.c
+++ b/drivers/gpu/drm/xe/xe_gt.c
@@ -65,6 +65,7 @@ struct xe_gt *xe_gt_alloc(struct xe_tile *tile)
gt->tile = tile;
gt->ordered_wq = alloc_ordered_workqueue("gt-ordered-wq", 0);
+ init_completion(>->reset_done);
return gt;
}
@@ -647,6 +648,8 @@ static int gt_reset(struct xe_gt *gt)
xe_device_mem_access_put(gt_to_xe(gt));
XE_WARN_ON(err);
+ complete(>->reset_done);
+
xe_gt_info(gt, "reset done\n");
return 0;
diff --git a/drivers/gpu/drm/xe/xe_gt_debugfs.c b/drivers/gpu/drm/xe/xe_gt_debugfs.c
index c4b67cf09f8f..f4ff0eef0468 100644
--- a/drivers/gpu/drm/xe/xe_gt_debugfs.c
+++ b/drivers/gpu/drm/xe/xe_gt_debugfs.c
@@ -23,6 +23,8 @@
#include "xe_uc_debugfs.h"
#include "xe_wa.h"
+#define XE_GT_RESET_TIMEOUT_MS (msecs_to_jiffies(5 * 1000))
+
static struct xe_gt *node_to_gt(struct drm_info_node *node)
{
return node->info_ent->data;
@@ -58,9 +60,17 @@ static int hw_engines(struct seq_file *m, void *data)
static int force_reset(struct seq_file *m, void *data)
{
struct xe_gt *gt = node_to_gt(m->private);
+ struct xe_device *xe = gt_to_xe(gt);
+ unsigned long timeout;
xe_gt_reset_async(gt);
+ timeout = wait_for_completion_timeout(>->reset_done, XE_GT_RESET_TIMEOUT_MS);
+ if (timeout == 0) {
+ drm_err(&xe->drm, "gt reset timed out");
+ return -ETIMEDOUT;
+ }
+
return 0;
}
diff --git a/drivers/gpu/drm/xe/xe_gt_types.h b/drivers/gpu/drm/xe/xe_gt_types.h
index f74684660475..6f2fb9e3cfea 100644
--- a/drivers/gpu/drm/xe/xe_gt_types.h
+++ b/drivers/gpu/drm/xe/xe_gt_types.h
@@ -358,6 +358,9 @@ struct xe_gt {
/** @oob: bitmap with active OOB workaroudns */
unsigned long *oob;
} wa_active;
+
+ /** @reset_done : Completion of GT reset */
+ struct completion reset_done;
};
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-12-21 19:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-21 17:16 [PATCH] drm/xe: Make sysfs gt force reset synchronous Karthik Poosa
2023-12-21 18:59 ` Matthew Brost
-- strict thread matches above, loose matches on Subject: below --
2023-12-16 5:04 Karthik Poosa
2023-12-16 5:10 ` Gupta, Anshuman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.