Re: [Intel-gfx] [PATCH 03/10] drm/i915: Expose engine properties via sysfs
On 14/10/2019 23:05, 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. 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. An example tree of the engine properties on Braswell: /sys/class/drm/card0 └── engine ├── bcs0 │ ├── capabilities │ ├── class │ ├── instance │ ├── known_capabilities │ └── name ├── rcs0 │ ├── capabilities │ ├── class │ ├── instance │ ├── known_capabilities │ └── name ├── vcs0 │ ├── capabilities │ ├── class │ ├── instance │ ├── known_capabilities │ └── name └── vecs0 ├── capabilities ├── class ├── instance ├── known_capabilities └── name v2: Include stringified capabilities v3: Include all known capabilities for futureproofing. v4: Combine the two caps loops into one Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Cc: Tvrtko Ursulin Cc: Daniele Ceraolo Spurio Cc: Rodrigo Vivi Acked-by: Rodrigo Vivi --- drivers/gpu/drm/i915/Makefile| 3 +- drivers/gpu/drm/i915/gt/intel_engine_sysfs.c | 205 +++ drivers/gpu/drm/i915/gt/intel_engine_sysfs.h | 14 ++ drivers/gpu/drm/i915/i915_sysfs.c| 3 + 4 files changed, 224 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 e791d9323b51..cd9a10ba2516 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -78,8 +78,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 ..823153e56c67 --- /dev/null +++ b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c @@ -0,0 +1,205 @@ +/* + * SPDX-License-Identifier: MIT + * + * Copyright © 2019 Intel Corporation + */ + +#include +#include + +#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); +} + +static struct kobj_attribute name_attr = +__ATTR(name, 0444, name_show, NULL); + +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 struct kobj_attribute class_attr = +__ATTR(class, 0444, class_show, NULL); + +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 struct kobj_attribute inst_attr = +__ATTR(instance, 0444, inst_show, NULL); + +static const char * const vcs_caps[] = { + [ilog2(I915_VIDEO_CLASS_CAPABILITY_HEVC)] = "hevc", + [ilog2(I915_VIDEO_AND_ENHANCE_CLASS_CAPABILITY_SFC)] = "sfc", +}; + +static const char * const vecs_caps[] = { + [ilog2(I915_VIDEO_AND_ENHANCE_CLASS_CAPABILITY_SFC)] = "sfc", +}; + +static ssize_t repr_trim(char *buf, ssize_t len) +{ + /* Trim off the trailing space and replace with a newline */ + if (len > PAGE_SIZE) + len = PAGE_SIZE; + if (len > 0) + buf[len - 1] = '\n'; + + return len; +} + +static ssize_t +__caps_show(struct intel_engine_cs *engine, + typeof(engine->uabi_capabilities) caps, What's the point, its one of the less likely fields to change type. You just have to try and push in this concept, don't you. :))) + char *buf, bool show_unknown) +{ + const char * const *repr; + int count, n; + ssize_t len; + + switch (engine->class) { + case VIDEO_DECODE_CLASS: + repr = vcs_caps; + count = ARRAY_SIZE(vcs_caps); + break; + + case VIDEO_ENHANCEMENT_CLASS: + repr = vecs_caps; +
[Intel-gfx] [PATCH 03/10] drm/i915: Expose engine properties via sysfs
Preliminary stub to add engines underneath /sys/class/drm/cardN/, so that we can expose properties on each engine to the sysadmin. 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. An example tree of the engine properties on Braswell: /sys/class/drm/card0 └── engine ├── bcs0 │ ├── capabilities │ ├── class │ ├── instance │ ├── known_capabilities │ └── name ├── rcs0 │ ├── capabilities │ ├── class │ ├── instance │ ├── known_capabilities │ └── name ├── vcs0 │ ├── capabilities │ ├── class │ ├── instance │ ├── known_capabilities │ └── name └── vecs0 ├── capabilities ├── class ├── instance ├── known_capabilities └── name v2: Include stringified capabilities v3: Include all known capabilities for futureproofing. v4: Combine the two caps loops into one Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Cc: Tvrtko Ursulin Cc: Daniele Ceraolo Spurio Cc: Rodrigo Vivi Acked-by: Rodrigo Vivi --- drivers/gpu/drm/i915/Makefile| 3 +- drivers/gpu/drm/i915/gt/intel_engine_sysfs.c | 205 +++ drivers/gpu/drm/i915/gt/intel_engine_sysfs.h | 14 ++ drivers/gpu/drm/i915/i915_sysfs.c| 3 + 4 files changed, 224 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 e791d9323b51..cd9a10ba2516 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -78,8 +78,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 ..823153e56c67 --- /dev/null +++ b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c @@ -0,0 +1,205 @@ +/* + * SPDX-License-Identifier: MIT + * + * Copyright © 2019 Intel Corporation + */ + +#include +#include + +#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); +} + +static struct kobj_attribute name_attr = +__ATTR(name, 0444, name_show, NULL); + +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 struct kobj_attribute class_attr = +__ATTR(class, 0444, class_show, NULL); + +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 struct kobj_attribute inst_attr = +__ATTR(instance, 0444, inst_show, NULL); + +static const char * const vcs_caps[] = { + [ilog2(I915_VIDEO_CLASS_CAPABILITY_HEVC)] = "hevc", + [ilog2(I915_VIDEO_AND_ENHANCE_CLASS_CAPABILITY_SFC)] = "sfc", +}; + +static const char * const vecs_caps[] = { + [ilog2(I915_VIDEO_AND_ENHANCE_CLASS_CAPABILITY_SFC)] = "sfc", +}; + +static ssize_t repr_trim(char *buf, ssize_t len) +{ + /* Trim off the trailing space and replace with a newline */ + if (len > PAGE_SIZE) + len = PAGE_SIZE; + if (len > 0) + buf[len - 1] = '\n'; + + return len; +} + +static ssize_t +__caps_show(struct intel_engine_cs *engine, + typeof(engine->uabi_capabilities) caps, + char *buf, bool show_unknown) +{ + const char * const *repr; + int count, n; + ssize_t len; + + switch (engine->class) { + case VIDEO_DECODE_CLASS: + repr = vcs_caps; + count = ARRAY_SIZE(vcs_caps); + break; + + case VIDEO_ENHANCEMENT_CLASS: + repr = vecs_caps; + count = ARRAY_SIZE(vecs_caps); + break; + + default: + repr = NULL; + count = 0; + break; + } + GEM_BUG_ON(count >
Re: [Intel-gfx] [PATCH 03/10] drm/i915: Expose engine properties via sysfs
On 11/10/2019 09:49, Chris Wilson wrote: Quoting Tvrtko Ursulin (2019-10-11 09:44:16) On 10/10/2019 08:14, 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. 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. An example tree of the engine properties on Braswell: /sys/class/drm/card0 └── engine ├── bcs0 │ ├── class │ ├── heartbeat_interval_ms Not present in this patch. I did say an example tree, not this tree :) │ ├── instance │ ├── mmio_base I vote for putting mmio_base in a followup patch. Darn your eagle eyes ;) And how about we add capabilities in the first patch? So we get another way of engine discovery. Ideally with mapping of bits to user friendly strings. Right, I was about to ask if we should do a /proc/cpuinfo style capabilities. Do we need both? Or just stick to the more human readable output for sysfs? Interesting question and I am not sure. I'd definitely have human readable and that even being an aggregation of engine->flags and engine->uabi_capabilities. Whether or not to also put hex in there.. For uabi_capabilities it's possible, but for the rest not so much. So that probably means only human readable? Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/10] drm/i915: Expose engine properties via sysfs
Quoting Tvrtko Ursulin (2019-10-11 09:44:16) > > On 10/10/2019 08:14, 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. > > > > 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. > > > > An example tree of the engine properties on Braswell: > > /sys/class/drm/card0 > > └── engine > > ├── bcs0 > > │ ├── class > > │ ├── heartbeat_interval_ms > > Not present in this patch. I did say an example tree, not this tree :) > > │ ├── instance > > │ ├── mmio_base > > I vote for putting mmio_base in a followup patch. Darn your eagle eyes ;) > > And how about we add capabilities in the first patch? So we get another > way of engine discovery. Ideally with mapping of bits to user friendly > strings. Right, I was about to ask if we should do a /proc/cpuinfo style capabilities. Do we need both? Or just stick to the more human readable output for sysfs? -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/10] drm/i915: Expose engine properties via sysfs
On 10/10/2019 08:14, 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. 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. An example tree of the engine properties on Braswell: /sys/class/drm/card0 └── engine ├── bcs0 │ ├── class │ ├── heartbeat_interval_ms Not present in this patch. │ ├── instance │ ├── mmio_base I vote for putting mmio_base in a followup patch. And how about we add capabilities in the first patch? So we get another way of engine discovery. Ideally with mapping of bits to user friendly strings. Regards, Tvrtko │ └── name ├── rcs0 │ ├── class │ ├── heartbeat_interval_ms │ ├── instance │ ├── mmio_base │ └── name ├── vcs0 │ ├── class │ ├── heartbeat_interval_ms │ ├── instance │ ├── mmio_base │ └── name └── vecs0 ├── class ├── heartbeat_interval_ms ├── instance ├── mmio_base └── name Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Cc: Tvrtko Ursulin Cc: Daniele Ceraolo Spurio Cc: Rodrigo Vivi Acked-by: Rodrigo Vivi --- drivers/gpu/drm/i915/Makefile| 3 +- drivers/gpu/drm/i915/gt/intel_engine_sysfs.c | 119 +++ drivers/gpu/drm/i915/gt/intel_engine_sysfs.h | 14 +++ drivers/gpu/drm/i915/i915_sysfs.c| 3 + 4 files changed, 138 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 e791d9323b51..cd9a10ba2516 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -78,8 +78,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 ..cbe9ec59beeb --- /dev/null +++ b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c @@ -0,0 +1,119 @@ +/* + * SPDX-License-Identifier: MIT + * + * Copyright © 2019 Intel Corporation + */ + +#include +#include + +#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); +} + +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); +} + +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(mmio_base, 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 = _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; + + kobject_init(>base, _engine_type); + ke->engine = engine; + + if (kobject_add(>base, dir, "%s", engine->name)) { + kobject_put(>base); + return NULL; + } + + /* xfer ownership to sysfs tree */ + return >base; +} + +void intel_engines_add_sysfs(struct drm_i915_private *i915) +{ + static const struct attribute
[Intel-gfx] [PATCH 03/10] drm/i915: Expose engine properties via sysfs
Preliminary stub to add engines underneath /sys/class/drm/cardN/, so that we can expose properties on each engine to the sysadmin. 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. An example tree of the engine properties on Braswell: /sys/class/drm/card0 └── engine ├── bcs0 │ ├── class │ ├── heartbeat_interval_ms │ ├── instance │ ├── mmio_base │ └── name ├── rcs0 │ ├── class │ ├── heartbeat_interval_ms │ ├── instance │ ├── mmio_base │ └── name ├── vcs0 │ ├── class │ ├── heartbeat_interval_ms │ ├── instance │ ├── mmio_base │ └── name └── vecs0 ├── class ├── heartbeat_interval_ms ├── instance ├── mmio_base └── name Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Cc: Tvrtko Ursulin Cc: Daniele Ceraolo Spurio Cc: Rodrigo Vivi Acked-by: Rodrigo Vivi --- drivers/gpu/drm/i915/Makefile| 3 +- drivers/gpu/drm/i915/gt/intel_engine_sysfs.c | 119 +++ drivers/gpu/drm/i915/gt/intel_engine_sysfs.h | 14 +++ drivers/gpu/drm/i915/i915_sysfs.c| 3 + 4 files changed, 138 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 e791d9323b51..cd9a10ba2516 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -78,8 +78,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 ..cbe9ec59beeb --- /dev/null +++ b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c @@ -0,0 +1,119 @@ +/* + * SPDX-License-Identifier: MIT + * + * Copyright © 2019 Intel Corporation + */ + +#include +#include + +#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); +} + +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); +} + +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(mmio_base, 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 = _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; + + kobject_init(>base, _engine_type); + ke->engine = engine; + + if (kobject_add(>base, dir, "%s", engine->name)) { + kobject_put(>base); + return NULL; + } + + /* xfer ownership to sysfs tree */ + return >base; +} + +void intel_engines_add_sysfs(struct drm_i915_private *i915) +{ + static const struct attribute *files[] = { + _attr.attr, + _attr.attr, + _attr.attr, + _attr.attr, + NULL + }; + + struct device *kdev = i915->drm.primary->kdev; + struct intel_engine_cs *engine; + struct kobject *dir; + + dir = kobject_create_and_add("engine", >kobj);