| Message ID | 20171116160004.22720-5-lionel.g.landwerlin@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 16/11/2017 16:00, Lionel Landwerlin wrote: > This enables userspace to discover the engines available on the GPU. > Here is the layout : > > /sys/devices/pci0000:00/0000:00:02.0/drm/card0/engines > ├── bcs0 > │ ├── class > │ └── instance > ├── rcs0 > │ ├── class > │ └── instance > ├── vcs0 > │ ├── class > │ ├── hevc For engine capabilities I was considering two alternatives since I think both would be better than stuffing individual files on the same level as class and instance. Option 1: $ cat vcs0/capabilities hevc Essentially a list of tokens similar to the cpu info string in /proc/cpuinfo. Option 2, more subdirectories: $ ls -1 vcs0/capabilities hevc Option 2 might be easier for userspace to parse? But more annoying to implement in i915 I think. So not sure. Otherwise I am quite happy with the sysfs approach. Can't think of any real downsides at the moment. Regards, Tvrtko > │ └── instance > ├── vcs1 > │ ├── class > │ └── instance > └── vecs0 > ├── class > └── instance > > Further capabilities can be added later as attributes of each engine. > > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_sysfs.c | 92 +++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_ringbuffer.h | 2 + > 3 files changed, 95 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 44b807421af8..c4f6e8ecf212 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2749,6 +2749,7 @@ struct drm_i915_private { > > struct { > struct intel_topology_kobject kobj; > + struct kobject engines_kobj; > > struct sysfs_slice { > struct intel_slice_kobject kobj; > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c > index 1d835f164d80..992aeaa91565 100644 > --- a/drivers/gpu/drm/i915/i915_sysfs.c > +++ b/drivers/gpu/drm/i915/i915_sysfs.c > @@ -712,6 +712,92 @@ static void i915_teardown_topology_sysfs(struct drm_i915_private *dev_priv) > kobject_del(&topology_kobj->kobj); > } > > +static struct attribute engine_instance_attr = { > + .name = "instance", > + .mode = 0444, > +}; > + > +static struct attribute engine_class_attr = { > + .name = "class", > + .mode = 0444, > +}; > + > +static struct attribute engine_hevc_attr = { > + .name = "hevc", > + .mode = 0444, > +}; > + > +static ssize_t > +show_engine_attr(struct kobject *kobj, struct attribute *attr, char *buf) > +{ > + struct intel_engine_cs *engine = > + container_of(kobj, struct intel_engine_cs, kobj); > + > + if (attr == &engine_instance_attr) > + return sprintf(buf, "%hhu\n", engine->uabi_id); > + if (attr == &engine_class_attr) > + return sprintf(buf, "%hhu\n", engine->uabi_class); > + if (attr == &engine_hevc_attr) > + return sprintf(buf, "%i\n", INTEL_GEN(engine->i915) >= 8); > + return sprintf(buf, "\n"); > +} > + > +static const struct sysfs_ops engine_ops = { > + .show = show_engine_attr, > +}; > + > +static struct kobj_type engine_type = { > + .sysfs_ops = &engine_ops, > +}; > + > +static int i915_setup_engines_sysfs(struct drm_i915_private *dev_priv) > +{ > + struct intel_engine_cs *engine; > + enum intel_engine_id id; > + > + kobject_init_and_add(&dev_priv->topology.engines_kobj, > + dev_priv->drm.primary->kdev->kobj.ktype, > + &dev_priv->drm.primary->kdev->kobj, > + "engines"); > + > + for_each_engine(engine, dev_priv, id) { > + int ret; > + > + kobject_init_and_add(&engine->kobj, &engine_type, > + &dev_priv->topology.engines_kobj, > + engine->name); > + > + ret = sysfs_create_file(&engine->kobj, &engine_instance_attr); > + if (ret) > + return ret; > + ret = sysfs_create_file(&engine->kobj, &engine_class_attr); > + if (ret) > + return ret; > + if (engine->id == VCS) { > + ret = sysfs_create_file(&engine->kobj, &engine_hevc_attr); > + if (ret) > + return ret; > + } > + } > + > + return 0; > +} > + > +static void i915_teardown_engines_sysfs(struct drm_i915_private *dev_priv) > +{ > + struct intel_engine_cs *engine; > + enum intel_engine_id id; > + > + for_each_engine(engine, dev_priv, id) { > + sysfs_remove_file(&engine->kobj, &engine_instance_attr); > + sysfs_remove_file(&engine->kobj, &engine_class_attr); > + sysfs_remove_file(&engine->kobj, &engine_hevc_attr); > + } > + > + kobject_get(&dev_priv->topology.engines_kobj); > + kobject_del(&dev_priv->topology.engines_kobj); > +} > + > void i915_setup_sysfs(struct drm_i915_private *dev_priv) > { > struct device *kdev = dev_priv->drm.primary->kdev; > @@ -762,6 +848,10 @@ void i915_setup_sysfs(struct drm_i915_private *dev_priv) > if (ret) > DRM_ERROR("Topology sysfs setup failed\n"); > > + ret = i915_setup_engines_sysfs(dev_priv); > + if (ret) > + DRM_ERROR("Engines sysfs setup failed\n"); > + > i915_setup_error_capture(kdev); > } > > @@ -771,6 +861,8 @@ void i915_teardown_sysfs(struct drm_i915_private *dev_priv) > > i915_teardown_error_capture(kdev); > > + i915_teardown_engines_sysfs(dev_priv); > + > i915_teardown_topology_sysfs(dev_priv); > > if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h > index 5f96533e5341..0243a9e620da 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -290,6 +290,8 @@ struct intel_engine_cs { > struct drm_i915_private *i915; > char name[INTEL_ENGINE_CS_MAX_NAME]; > > + struct kobject kobj; > + > enum intel_engine_id id; > unsigned int hw_id; > unsigned int guc_id; >
On 17/11/17 09:51, Tvrtko Ursulin wrote: > > On 16/11/2017 16:00, Lionel Landwerlin wrote: >> This enables userspace to discover the engines available on the GPU. >> Here is the layout : >> >> /sys/devices/pci0000:00/0000:00:02.0/drm/card0/engines >> ├── bcs0 >> │ ├── class >> │ └── instance >> ├── rcs0 >> │ ├── class >> │ └── instance >> ├── vcs0 >> │ ├── class >> │ ├── hevc > > For engine capabilities I was considering two alternatives since I > think both would be better than stuffing individual files on the same > level as class and instance. > > Option 1: > > $ cat vcs0/capabilities > hevc > > Essentially a list of tokens similar to the cpu info string in > /proc/cpuinfo. > > Option 2, more subdirectories: > > $ ls -1 vcs0/capabilities > hevc > > Option 2 might be easier for userspace to parse? But more annoying to > implement in i915 I think. So not sure. > > Otherwise I am quite happy with the sysfs approach. Can't think of any > real downsides at the moment. > > Regards, > > Tvrtko Now that I'm familiar with kobject/sysfs, option 1 or 2 is about the same amount of work. Just let me know what you prefer. I would also take suggestions about the layout, is card0/engines the right places? Should I rename card0/topology to card0/eu_topology? You could argue that both engines & eus are part of the topology of the GPU... > >> │ └── instance >> ├── vcs1 >> │ ├── class >> │ └── instance >> └── vecs0 >> ├── class >> └── instance >> >> Further capabilities can be added later as attributes of each engine. >> >> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >> --- >> drivers/gpu/drm/i915/i915_drv.h | 1 + >> drivers/gpu/drm/i915/i915_sysfs.c | 92 >> +++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/i915/intel_ringbuffer.h | 2 + >> 3 files changed, 95 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h >> b/drivers/gpu/drm/i915/i915_drv.h >> index 44b807421af8..c4f6e8ecf212 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -2749,6 +2749,7 @@ struct drm_i915_private { >> struct { >> struct intel_topology_kobject kobj; >> + struct kobject engines_kobj; >> struct sysfs_slice { >> struct intel_slice_kobject kobj; >> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c >> b/drivers/gpu/drm/i915/i915_sysfs.c >> index 1d835f164d80..992aeaa91565 100644 >> --- a/drivers/gpu/drm/i915/i915_sysfs.c >> +++ b/drivers/gpu/drm/i915/i915_sysfs.c >> @@ -712,6 +712,92 @@ static void i915_teardown_topology_sysfs(struct >> drm_i915_private *dev_priv) >> kobject_del(&topology_kobj->kobj); >> } >> +static struct attribute engine_instance_attr = { >> + .name = "instance", >> + .mode = 0444, >> +}; >> + >> +static struct attribute engine_class_attr = { >> + .name = "class", >> + .mode = 0444, >> +}; >> + >> +static struct attribute engine_hevc_attr = { >> + .name = "hevc", >> + .mode = 0444, >> +}; >> + >> +static ssize_t >> +show_engine_attr(struct kobject *kobj, struct attribute *attr, char >> *buf) >> +{ >> + struct intel_engine_cs *engine = >> + container_of(kobj, struct intel_engine_cs, kobj); >> + >> + if (attr == &engine_instance_attr) >> + return sprintf(buf, "%hhu\n", engine->uabi_id); >> + if (attr == &engine_class_attr) >> + return sprintf(buf, "%hhu\n", engine->uabi_class); >> + if (attr == &engine_hevc_attr) >> + return sprintf(buf, "%i\n", INTEL_GEN(engine->i915) >= 8); >> + return sprintf(buf, "\n"); >> +} >> + >> +static const struct sysfs_ops engine_ops = { >> + .show = show_engine_attr, >> +}; >> + >> +static struct kobj_type engine_type = { >> + .sysfs_ops = &engine_ops, >> +}; >> + >> +static int i915_setup_engines_sysfs(struct drm_i915_private *dev_priv) >> +{ >> + struct intel_engine_cs *engine; >> + enum intel_engine_id id; >> + >> + kobject_init_and_add(&dev_priv->topology.engines_kobj, >> + dev_priv->drm.primary->kdev->kobj.ktype, >> + &dev_priv->drm.primary->kdev->kobj, >> + "engines"); >> + >> + for_each_engine(engine, dev_priv, id) { >> + int ret; >> + >> + kobject_init_and_add(&engine->kobj, &engine_type, >> + &dev_priv->topology.engines_kobj, >> + engine->name); >> + >> + ret = sysfs_create_file(&engine->kobj, &engine_instance_attr); >> + if (ret) >> + return ret; >> + ret = sysfs_create_file(&engine->kobj, &engine_class_attr); >> + if (ret) >> + return ret; >> + if (engine->id == VCS) { >> + ret = sysfs_create_file(&engine->kobj, &engine_hevc_attr); >> + if (ret) >> + return ret; >> + } >> + } >> + >> + return 0; >> +} >> + >> +static void i915_teardown_engines_sysfs(struct drm_i915_private >> *dev_priv) >> +{ >> + struct intel_engine_cs *engine; >> + enum intel_engine_id id; >> + >> + for_each_engine(engine, dev_priv, id) { >> + sysfs_remove_file(&engine->kobj, &engine_instance_attr); >> + sysfs_remove_file(&engine->kobj, &engine_class_attr); >> + sysfs_remove_file(&engine->kobj, &engine_hevc_attr); >> + } >> + >> + kobject_get(&dev_priv->topology.engines_kobj); >> + kobject_del(&dev_priv->topology.engines_kobj); >> +} >> + >> void i915_setup_sysfs(struct drm_i915_private *dev_priv) >> { >> struct device *kdev = dev_priv->drm.primary->kdev; >> @@ -762,6 +848,10 @@ void i915_setup_sysfs(struct drm_i915_private >> *dev_priv) >> if (ret) >> DRM_ERROR("Topology sysfs setup failed\n"); >> + ret = i915_setup_engines_sysfs(dev_priv); >> + if (ret) >> + DRM_ERROR("Engines sysfs setup failed\n"); >> + >> i915_setup_error_capture(kdev); >> } >> @@ -771,6 +861,8 @@ void i915_teardown_sysfs(struct >> drm_i915_private *dev_priv) >> i915_teardown_error_capture(kdev); >> + i915_teardown_engines_sysfs(dev_priv); >> + >> i915_teardown_topology_sysfs(dev_priv); >> if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h >> b/drivers/gpu/drm/i915/intel_ringbuffer.h >> index 5f96533e5341..0243a9e620da 100644 >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h >> @@ -290,6 +290,8 @@ struct intel_engine_cs { >> struct drm_i915_private *i915; >> char name[INTEL_ENGINE_CS_MAX_NAME]; >> + struct kobject kobj; >> + >> enum intel_engine_id id; >> unsigned int hw_id; >> unsigned int guc_id; >> >
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 44b807421af8..c4f6e8ecf212 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2749,6 +2749,7 @@ struct drm_i915_private { struct { struct intel_topology_kobject kobj; + struct kobject engines_kobj; struct sysfs_slice { struct intel_slice_kobject kobj; diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c index 1d835f164d80..992aeaa91565 100644 --- a/drivers/gpu/drm/i915/i915_sysfs.c +++ b/drivers/gpu/drm/i915/i915_sysfs.c @@ -712,6 +712,92 @@ static void i915_teardown_topology_sysfs(struct drm_i915_private *dev_priv) kobject_del(&topology_kobj->kobj); } +static struct attribute engine_instance_attr = { + .name = "instance", + .mode = 0444, +}; + +static struct attribute engine_class_attr = { + .name = "class", + .mode = 0444, +}; + +static struct attribute engine_hevc_attr = { + .name = "hevc", + .mode = 0444, +}; + +static ssize_t +show_engine_attr(struct kobject *kobj, struct attribute *attr, char *buf) +{ + struct intel_engine_cs *engine = + container_of(kobj, struct intel_engine_cs, kobj); + + if (attr == &engine_instance_attr) + return sprintf(buf, "%hhu\n", engine->uabi_id); + if (attr == &engine_class_attr) + return sprintf(buf, "%hhu\n", engine->uabi_class); + if (attr == &engine_hevc_attr) + return sprintf(buf, "%i\n", INTEL_GEN(engine->i915) >= 8); + return sprintf(buf, "\n"); +} + +static const struct sysfs_ops engine_ops = { + .show = show_engine_attr, +}; + +static struct kobj_type engine_type = { + .sysfs_ops = &engine_ops, +}; + +static int i915_setup_engines_sysfs(struct drm_i915_private *dev_priv) +{ + struct intel_engine_cs *engine; + enum intel_engine_id id; + + kobject_init_and_add(&dev_priv->topology.engines_kobj, + dev_priv->drm.primary->kdev->kobj.ktype, + &dev_priv->drm.primary->kdev->kobj, + "engines"); + + for_each_engine(engine, dev_priv, id) { + int ret; + + kobject_init_and_add(&engine->kobj, &engine_type, + &dev_priv->topology.engines_kobj, + engine->name); + + ret = sysfs_create_file(&engine->kobj, &engine_instance_attr); + if (ret) + return ret; + ret = sysfs_create_file(&engine->kobj, &engine_class_attr); + if (ret) + return ret; + if (engine->id == VCS) { + ret = sysfs_create_file(&engine->kobj, &engine_hevc_attr); + if (ret) + return ret; + } + } + + return 0; +} + +static void i915_teardown_engines_sysfs(struct drm_i915_private *dev_priv) +{ + struct intel_engine_cs *engine; + enum intel_engine_id id; + + for_each_engine(engine, dev_priv, id) { + sysfs_remove_file(&engine->kobj, &engine_instance_attr); + sysfs_remove_file(&engine->kobj, &engine_class_attr); + sysfs_remove_file(&engine->kobj, &engine_hevc_attr); + } + + kobject_get(&dev_priv->topology.engines_kobj); + kobject_del(&dev_priv->topology.engines_kobj); +} + void i915_setup_sysfs(struct drm_i915_private *dev_priv) { struct device *kdev = dev_priv->drm.primary->kdev; @@ -762,6 +848,10 @@ void i915_setup_sysfs(struct drm_i915_private *dev_priv) if (ret) DRM_ERROR("Topology sysfs setup failed\n"); + ret = i915_setup_engines_sysfs(dev_priv); + if (ret) + DRM_ERROR("Engines sysfs setup failed\n"); + i915_setup_error_capture(kdev); } @@ -771,6 +861,8 @@ void i915_teardown_sysfs(struct drm_i915_private *dev_priv) i915_teardown_error_capture(kdev); + i915_teardown_engines_sysfs(dev_priv); + i915_teardown_topology_sysfs(dev_priv); if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 5f96533e5341..0243a9e620da 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -290,6 +290,8 @@ struct intel_engine_cs { struct drm_i915_private *i915; char name[INTEL_ENGINE_CS_MAX_NAME]; + struct kobject kobj; + enum intel_engine_id id; unsigned int hw_id; unsigned int guc_id;
This enables userspace to discover the engines available on the GPU. Here is the layout : /sys/devices/pci0000:00/0000:00:02.0/drm/card0/engines ├── bcs0 │ ├── class │ └── instance ├── rcs0 │ ├── class │ └── instance ├── vcs0 │ ├── class │ ├── hevc │ └── instance ├── vcs1 │ ├── class │ └── instance └── vecs0 ├── class └── instance Further capabilities can be added later as attributes of each engine. Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_sysfs.c | 92 +++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_ringbuffer.h | 2 + 3 files changed, 95 insertions(+)