Message ID | 20191014090757.32111-8-chris@chris-wilson.co.uk (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [01/15] drm/i915/display: Squelch kerneldoc warnings | expand |
On 14/10/2019 10:07, 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. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > drivers/gpu/drm/i915/Makefile | 3 +- > drivers/gpu/drm/i915/gt/intel_engine_sysfs.c | 223 +++++++++++++++++++ > drivers/gpu/drm/i915/gt/intel_engine_sysfs.h | 14 ++ > drivers/gpu/drm/i915/i915_sysfs.c | 3 + > 4 files changed, 242 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 000000000000..4e403a1b069a > --- /dev/null > +++ b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c > @@ -0,0 +1,223 @@ > +/* > + * 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); > +} > + > +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 kobject *kobj, struct kobj_attribute *attr, char *buf) > +{ > + struct intel_engine_cs *engine = kobj_to_engine(kobj); > + const char * const *repr; > + int num_repr, n; > + ssize_t len; > + > + switch (engine->class) { > + case VIDEO_DECODE_CLASS: > + repr = vcs_caps; > + num_repr = ARRAY_SIZE(vcs_caps); > + break; > + > + case VIDEO_ENHANCEMENT_CLASS: > + repr = vecs_caps; > + num_repr = ARRAY_SIZE(vecs_caps); > + break; > + > + default: > + repr = NULL; > + num_repr = 0; > + break; > + } > + > + len = 0; > + for_each_set_bit(n, > + (unsigned long *)&engine->uabi_capabilities, > + BITS_PER_TYPE(typeof(engine->uabi_capabilities))) { > + if (GEM_WARN_ON(n >= num_repr || !repr[n])) > + len += snprintf(buf + len, PAGE_SIZE - len, > + "[%x] ", n); > + else > + len += snprintf(buf + len, PAGE_SIZE - len, > + "%s ", repr[n]); > + if (GEM_WARN_ON(len >= PAGE_SIZE)) > + break; > + } > + return repr_trim(buf, len); > +} > + > +static struct kobj_attribute caps_attr = > +__ATTR(capabilities, 0444, caps_show, NULL); > + > +static ssize_t > +all_caps_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf) > +{ > + struct intel_engine_cs *engine = kobj_to_engine(kobj); > + const char * const *repr; > + int num_repr, n; > + ssize_t len; > + > + switch (engine->class) { > + case VIDEO_DECODE_CLASS: > + repr = vcs_caps; > + num_repr = ARRAY_SIZE(vcs_caps); > + break; > + > + case VIDEO_ENHANCEMENT_CLASS: > + repr = vecs_caps; > + num_repr = ARRAY_SIZE(vecs_caps); > + break; > + > + default: > + repr = NULL; > + num_repr = 0; > + break; > + } > + > + len = 0; > + for (n = 0; n < num_repr; n++) { > + if (!repr[n]) > + continue; > + > + len += snprintf(buf + len, PAGE_SIZE - len, "%s ", repr[n]); > + if (GEM_WARN_ON(len >= PAGE_SIZE)) > + break; > + } > + return repr_trim(buf, len); > +} Would it be worth consolidating like: __caps_show(engine, buf, len, mask, warn_unknown) { ... switch(engine->class... ... for_each_set_bit(...mask...) { if (n >= repr || !repr[n]) { if (warn_unknown) GEM_WARN_ON(...); else len += snprinf... } else { len += snprintf... } } ... } caps_show() { ... len = __caps_show(engine, buf, len, engine->uabi_capabilities, true); ... } all_caps_show() { ... len = __caps_show(engine, buf, len, ~0, false); ... } Regards, Tvrtko > + > +static struct kobj_attribute all_caps_attr = > +__ATTR(known_capabilities, 0444, all_caps_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; > + > + 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; > + } > + > + /* xfer ownership to sysfs tree */ > + 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, > + &caps_attr.attr, > + &all_caps_attr.attr, > + NULL > + }; > + > + 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) > + goto err_engine; > + > + if (sysfs_create_files(kobj, files)) > + goto err_object; > + > + if (0) { > +err_object: > + kobject_put(kobj); > +err_engine: > + dev_err(kdev, "Failed to add sysfs engine '%s'\n", > + engine->name); > + break; > + } > + } > +} > 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 bf039b8ba593..7b665f69f301 100644 > --- a/drivers/gpu/drm/i915/i915_sysfs.c > +++ b/drivers/gpu/drm/i915/i915_sysfs.c > @@ -30,6 +30,7 @@ > #include <linux/stat.h> > #include <linux/sysfs.h> > > +#include "gt/intel_engine_sysfs.h" > #include "gt/intel_rc6.h" > > #include "i915_drv.h" > @@ -616,6 +617,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); > } > > void i915_teardown_sysfs(struct drm_i915_private *dev_priv) >
Quoting Tvrtko Ursulin (2019-10-14 11:17:54) > > On 14/10/2019 10:07, Chris Wilson wrote: > > +static ssize_t > > +caps_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf) > > +{ > > + struct intel_engine_cs *engine = kobj_to_engine(kobj); > > + const char * const *repr; > > + int num_repr, n; > > + ssize_t len; > > + > > + switch (engine->class) { > > + case VIDEO_DECODE_CLASS: > > + repr = vcs_caps; > > + num_repr = ARRAY_SIZE(vcs_caps); > > + break; > > + > > + case VIDEO_ENHANCEMENT_CLASS: > > + repr = vecs_caps; > > + num_repr = ARRAY_SIZE(vecs_caps); > > + break; > > + > > + default: > > + repr = NULL; > > + num_repr = 0; > > + break; > > + } > > + > > + len = 0; > > + for_each_set_bit(n, > > + (unsigned long *)&engine->uabi_capabilities, > > + BITS_PER_TYPE(typeof(engine->uabi_capabilities))) { > > + if (GEM_WARN_ON(n >= num_repr || !repr[n])) > > + len += snprintf(buf + len, PAGE_SIZE - len, > > + "[%x] ", n); > > + else > > + len += snprintf(buf + len, PAGE_SIZE - len, > > + "%s ", repr[n]); > > + if (GEM_WARN_ON(len >= PAGE_SIZE)) > > + break; > > + } > > + return repr_trim(buf, len); > > +} > > + > > +static struct kobj_attribute caps_attr = > > +__ATTR(capabilities, 0444, caps_show, NULL); > > + > > +static ssize_t > > +all_caps_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf) > > +{ > > + struct intel_engine_cs *engine = kobj_to_engine(kobj); > > + const char * const *repr; > > + int num_repr, n; > > + ssize_t len; > > + > > + switch (engine->class) { > > + case VIDEO_DECODE_CLASS: > > + repr = vcs_caps; > > + num_repr = ARRAY_SIZE(vcs_caps); > > + break; > > + > > + case VIDEO_ENHANCEMENT_CLASS: > > + repr = vecs_caps; > > + num_repr = ARRAY_SIZE(vecs_caps); > > + break; > > + > > + default: > > + repr = NULL; > > + num_repr = 0; > > + break; > > + } > > + > > + len = 0; > > + for (n = 0; n < num_repr; n++) { > > + if (!repr[n]) > > + continue; > > + > > + len += snprintf(buf + len, PAGE_SIZE - len, "%s ", repr[n]); > > + if (GEM_WARN_ON(len >= PAGE_SIZE)) > > + break; > > + } > > + return repr_trim(buf, len); > > +} > > Would it be worth consolidating like: > > __caps_show(engine, buf, len, mask, warn_unknown) > { > ... > switch(engine->class... > ... > for_each_set_bit(...mask...) { > if (n >= repr || !repr[n]) { > if (warn_unknown) > GEM_WARN_ON(...); > else > len += snprinf... > } else { > len += snprintf... > } > } > ... > } > > caps_show() > { > ... > len = __caps_show(engine, buf, len, engine->uabi_capabilities, true); > ... > } > > all_caps_show() > { > ... > len = __caps_show(engine, buf, len, ~0, false); > ... > } I thought it would look ugly and distract from the intent. One is to list the bits, the other the array of string. But there is a certain appeal to having just one setup and loop. -Chris
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 000000000000..4e403a1b069a --- /dev/null +++ b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c @@ -0,0 +1,223 @@ +/* + * 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); +} + +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 kobject *kobj, struct kobj_attribute *attr, char *buf) +{ + struct intel_engine_cs *engine = kobj_to_engine(kobj); + const char * const *repr; + int num_repr, n; + ssize_t len; + + switch (engine->class) { + case VIDEO_DECODE_CLASS: + repr = vcs_caps; + num_repr = ARRAY_SIZE(vcs_caps); + break; + + case VIDEO_ENHANCEMENT_CLASS: + repr = vecs_caps; + num_repr = ARRAY_SIZE(vecs_caps); + break; + + default: + repr = NULL; + num_repr = 0; + break; + } + + len = 0; + for_each_set_bit(n, + (unsigned long *)&engine->uabi_capabilities, + BITS_PER_TYPE(typeof(engine->uabi_capabilities))) { + if (GEM_WARN_ON(n >= num_repr || !repr[n])) + len += snprintf(buf + len, PAGE_SIZE - len, + "[%x] ", n); + else + len += snprintf(buf + len, PAGE_SIZE - len, + "%s ", repr[n]); + if (GEM_WARN_ON(len >= PAGE_SIZE)) + break; + } + return repr_trim(buf, len); +} + +static struct kobj_attribute caps_attr = +__ATTR(capabilities, 0444, caps_show, NULL); + +static ssize_t +all_caps_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf) +{ + struct intel_engine_cs *engine = kobj_to_engine(kobj); + const char * const *repr; + int num_repr, n; + ssize_t len; + + switch (engine->class) { + case VIDEO_DECODE_CLASS: + repr = vcs_caps; + num_repr = ARRAY_SIZE(vcs_caps); + break; + + case VIDEO_ENHANCEMENT_CLASS: + repr = vecs_caps; + num_repr = ARRAY_SIZE(vecs_caps); + break; + + default: + repr = NULL; + num_repr = 0; + break; + } + + len = 0; + for (n = 0; n < num_repr; n++) { + if (!repr[n]) + continue; + + len += snprintf(buf + len, PAGE_SIZE - len, "%s ", repr[n]); + if (GEM_WARN_ON(len >= PAGE_SIZE)) + break; + } + return repr_trim(buf, len); +} + +static struct kobj_attribute all_caps_attr = +__ATTR(known_capabilities, 0444, all_caps_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; + + 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; + } + + /* xfer ownership to sysfs tree */ + 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, + &caps_attr.attr, + &all_caps_attr.attr, + NULL + }; + + 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) + goto err_engine; + + if (sysfs_create_files(kobj, files)) + goto err_object; + + if (0) { +err_object: + kobject_put(kobj); +err_engine: + dev_err(kdev, "Failed to add sysfs engine '%s'\n", + engine->name); + break; + } + } +} 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 bf039b8ba593..7b665f69f301 100644 --- a/drivers/gpu/drm/i915/i915_sysfs.c +++ b/drivers/gpu/drm/i915/i915_sysfs.c @@ -30,6 +30,7 @@ #include <linux/stat.h> #include <linux/sysfs.h> +#include "gt/intel_engine_sysfs.h" #include "gt/intel_rc6.h" #include "i915_drv.h" @@ -616,6 +617,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); } void i915_teardown_sysfs(struct drm_i915_private *dev_priv)