[Intel-gfx] [PATCH 4/4] drm/i915: expose engine availability through sysfs

[Lionel Landwerlin]

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 at 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;
>>
>