Re: [PATCH 4/4] drm/i915: expose engine availability through sysfs

[Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>]

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@xxxxxxxxx>
> > ---
> >   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;

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