[Intel-xe] [PATCH V3 5/6] drm/xe: Add sysfs for preempt reset timeout

[Niranjana Vishwanathapura]

On Wed, Jun 28, 2023 at 05:17:31PM +0530, Tejas Upadhyay wrote:
>The preemption request and timeout is used for
>higher priority context or kill hung context and reset
>gpu.
>
>The preempt timeout can be adjusted per-engine class using,
>
>/sys/class/drm/cardX/device/gtN/engines/ccs/preempt_timeout_us
>
>and can be disabled by setting it to 0.
>
>Signed-off-by: Tejas Upadhyay <tejas.upadhyay at intel.com>
>---
> drivers/gpu/drm/xe/xe_gt_sysfs.c | 44 +++++++++++++++++++++++++++++---
> 1 file changed, 40 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_gt_sysfs.c b/drivers/gpu/drm/xe/xe_gt_sysfs.c
>index 6ca77c5917cc..4c8f589870ba 100644
>--- a/drivers/gpu/drm/xe/xe_gt_sysfs.c
>+++ b/drivers/gpu/drm/xe/xe_gt_sysfs.c
>@@ -34,6 +34,37 @@ static const struct kobj_type kobj_xe_engine_type = {
> 	.sysfs_ops = &kobj_sysfs_ops
> };
>
>+static ssize_t preempt_timeout_store(struct kobject *kobj,
>+				     struct kobj_attribute *attr,
>+				     const char *buf, size_t count)
>+{
>+	struct xe_hw_engine *hwe = kobj_to_hwe(kobj);
>+	u32 timeout;
>+	int err;
>+
>+	err = kstrtou32(buf, 0, &timeout);
>+	if (err)
>+		return err;
>+
>+	if (timeout > jiffies_to_usecs(MAX_SCHEDULE_TIMEOUT))
>+		return -EINVAL;
>+
>+	WRITE_ONCE(hwe->sched_props.preempt_timeout_us, timeout);
>+
>+	return count;
>+}

So, these updated values does not affect the already created user engines.
Maybe we can add that to commit description?

>+
>+static ssize_t preempt_timeout_show(struct kobject *kobj,
>+				    struct kobj_attribute *attr, char *buf)
>+{
>+	struct xe_hw_engine *hwe = kobj_to_hwe(kobj);
>+
>+	return sprintf(buf, "%u\n", hwe->sched_props.preempt_timeout_us);
>+}
>+
>+static struct kobj_attribute preempt_timeout_attr =
>+__ATTR(preempt_timeout_us, 0644, preempt_timeout_show, preempt_timeout_store);
>+
> static ssize_t timeslice_duration_store(struct kobject *kobj,
> 					struct kobj_attribute *attr,
> 					const char *buf, size_t count)
>@@ -100,6 +131,7 @@ __ATTR(job_timeout_ms, 0644, job_timeout_show, job_timeout_store);
> static const struct attribute *files[] = {
> 	&job_timeout_attr.attr,
> 	&timeslice_duration_attr.attr,
>+	&preempt_timeout_attr.attr,
> 	NULL
> };
>
>@@ -261,10 +293,14 @@ int xe_gt_sysfs_engines(struct xe_gt *gt)
> 		}
>
> 		err = sysfs_create_files(&khwe->base, files);
>-		if (err) {
>-			kobject_put(kobj);
>-			return err;
>-		}
>+		if (err)
>+			goto err_object;
>+	}
>+
>+	if (0) {
>+err_object:
>+		kobject_put(kobj);
>+		return err;
> 	}

I am not sure if we really need this 'if (0)' construct. We could instead do,

      return 0;
err_object:
      kobject_put(kobj);
      return err;

In any case, we should do it in patch #3 (where this code is added) instead of here.

Niranjana

>
> 	return err;
>-- 
>2.25.1
>