Re: [PATCH] drm/i915: Expose engine properties via sysfs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 18/09/2019 10:23, Chris Wilson wrote:
Preliminary stub to add engines underneath /sys/class/drm/cardN/, so
that we can expose properties on each engine to the sysadmin.

Do we also envisage a need for these future things we'll expose to be per-context-engine and not just per physical engine?


To start with we have basic analogues of the i915_query ioctl so that we
can pretty print engine discovery from the shell, and flesh out the
directory structure. Later we will add writeable sysadmin properties such
as per-engine timeout controls.

It would be good to show an example of the layout in commit text.

Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
---
  drivers/gpu/drm/i915/Makefile                |   3 +-
  drivers/gpu/drm/i915/gt/intel_engine_sysfs.c | 113 +++++++++++++++++++
  drivers/gpu/drm/i915/gt/intel_engine_sysfs.h |  14 +++
  drivers/gpu/drm/i915/i915_sysfs.c            |   4 +
  4 files changed, 133 insertions(+), 1 deletion(-)
  create mode 100644 drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
  create mode 100644 drivers/gpu/drm/i915/gt/intel_engine_sysfs.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 658b930d34a8..bbea0d4dadd6 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -76,8 +76,9 @@ gt-y += \
  	gt/intel_breadcrumbs.o \
  	gt/intel_context.o \
  	gt/intel_engine_cs.o \
-	gt/intel_engine_pool.o \
  	gt/intel_engine_pm.o \
+	gt/intel_engine_pool.o \
+	gt/intel_engine_sysfs.o \
  	gt/intel_engine_user.o \
  	gt/intel_gt.o \
  	gt/intel_gt_irq.o \
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
new file mode 100644
index 000000000000..51b4b3f2a808
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
@@ -0,0 +1,113 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2019 Intel Corporation
+ */
+
+#include <linux/kobject.h>
+#include <linux/sysfs.h>
+
+#include "i915_drv.h"
+#include "intel_engine.h"
+#include "intel_engine_sysfs.h"
+
+struct kobj_engine {
+	struct kobject base;
+	struct intel_engine_cs *engine;
+};
+
+static struct intel_engine_cs *kobj_to_engine(struct kobject *kobj)
+{
+	return container_of(kobj, struct kobj_engine, base)->engine;
+}
+
+static ssize_t
+name_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%s\n", kobj_to_engine(kobj)->name);

Name contains our internal instance number which I think we don't want to export.

On the other hand we could think of dmesg as user visible so could consider tweaking engine->name to be built from uabi components.

+}
+
+static ssize_t
+class_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%d\n", kobj_to_engine(kobj)->uabi_class);
+}
+
+static ssize_t
+inst_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%d\n", kobj_to_engine(kobj)->uabi_instance);
+}
+
+static ssize_t
+mmio_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	return sprintf(buf, "0x%x\n", kobj_to_engine(kobj)->mmio_base);
+}

Nice try ;) but I suggest you leave adding mmio for a separate patch.

+
+static struct kobj_attribute name_attr = __ATTR(name, 0444, name_show, NULL);
+static struct kobj_attribute class_attr = __ATTR(class, 0444, class_show, NULL);
+static struct kobj_attribute inst_attr = __ATTR(instance, 0444, inst_show, NULL);
+static struct kobj_attribute mmio_attr = __ATTR(instance, 0444, mmio_show, NULL);
+
+static void kobj_engine_release(struct kobject *kobj)
+{
+	kfree(kobj);
+}
+
+static struct kobj_type kobj_engine_type = {
+	.release = kobj_engine_release,
+	.sysfs_ops = &kobj_sysfs_ops
+};
+
+static struct kobject *
+kobj_engine(struct kobject *dir, struct intel_engine_cs *engine)
+{
+	struct kobj_engine *ke;
+
+	ke = kzalloc(sizeof(*ke), GFP_KERNEL);
+	if (!ke)
+		return NULL;

Could embed kobj into the engine?

+
+	kobject_init(&ke->base, &kobj_engine_type);
+	ke->engine = engine;
+
+	if (kobject_add(&ke->base, dir, "%s", engine->name)) {
+		kobject_put(&ke->base);
+		return NULL;
+	}
+
+	return &ke->base;
+}
+
+void intel_engines_add_sysfs(struct drm_i915_private *i915)
+{
+	static const struct attribute *files[] = {
+		&name_attr.attr,
+		&class_attr.attr,
+		&inst_attr.attr,
+		&mmio_attr.attr,
+	};
+
+	struct device *kdev = i915->drm.primary->kdev;
+	struct intel_engine_cs *engine;
+	struct kobject *dir;
+
+	dir = kobject_create_and_add("engine", &kdev->kobj);
+	if (!dir)
+		return;
+
+	for_each_uabi_engine(engine, i915) {
+		struct kobject *kobj;
+
+		kobj = kobj_engine(dir, engine);
+		if (!kobj)
+			continue;

Could be consistent and log an error in this case as well.

+
+		if (sysfs_create_files(kobj, files)) {
+			dev_err(kdev, "Failed to add sysfs engine '%s'\n",
+				engine->name);
+			break;

Leaks kobj.

+		}
+	}
+}
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_sysfs.h b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.h
new file mode 100644
index 000000000000..ef44a745b70a
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.h
@@ -0,0 +1,14 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2019 Intel Corporation
+ */
+
+#ifndef INTEL_ENGINE_SYSFS_H
+#define INTEL_ENGINE_SYSFS_H
+
+struct drm_i915_private;
+
+void intel_engines_add_sysfs(struct drm_i915_private *i915);
+
+#endif /* INTEL_ENGINE_SYSFS_H */
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index d8a3b180c084..6b88d934927a 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -30,6 +30,8 @@
  #include <linux/stat.h>
  #include <linux/sysfs.h>
+#include "gt/intel_engine_sysfs.h"
+
  #include "i915_drv.h"
  #include "i915_sysfs.h"
  #include "intel_pm.h"
@@ -618,6 +620,8 @@ void i915_setup_sysfs(struct drm_i915_private *dev_priv)
  		DRM_ERROR("RPS sysfs setup failed\n");
i915_setup_error_capture(kdev);
+
+	intel_engines_add_sysfs(dev_priv);

Or gt?

  }
void i915_teardown_sysfs(struct drm_i915_private *dev_priv)


Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux