Re: [Intel-gfx] [PATCH 4/4] drm/i915: expose EU topology through sysfs

Mon, 20 Nov 2017 08:55:29 -0800 

On 20/11/17 16:13, Tvrtko Ursulin wrote:


On 20/11/2017 12:23, Lionel Landwerlin wrote:

With the introduction of asymetric slices in CNL, we cannot rely on
the previous SUBSLICE_MASK getparam. Here we introduce a more detailed
way of querying the Gen's GPU topology that doesn't aggregate numbers.

This is essential for monitoring parts of the GPU with the OA unit,
because signals need to be accounted properly based on whether part of
the GPU has been fused off. The current aggregated numbers like
EU_TOTAL do not gives us sufficient information.

Here is the sysfs layout on a Skylake GT4 :

/sys/devices/pci0000:00/0000:00:02.0/drm/card0/gt/rcs/0/
├── capabilities
├── class
├── id
├── max_eus_per_subslice
├── max_slices
├── max_subslices_per_slice
├── slice0
│   ├── subslice0
│   │   └── eus_enabled_mask
│   ├── subslice1
│   │   └── eus_enabled_mask
│   ├── subslice2
│   │   └── eus_enabled_mask
│   ├── subslice3
│   │   └── eus_enabled_mask
│   └── subslices_enabled_mask
├── slice1
│   ├── subslice0
│   │   └── eus_enabled_mask
│   ├── subslice1
│   │   └── eus_enabled_mask
│   ├── subslice2
│   │   └── eus_enabled_mask
│   ├── subslice3
│   │   └── eus_enabled_mask
│   └── subslices_enabled_mask
├── slice2
│   ├── subslice0
│   │   └── eus_enabled_mask
│   ├── subslice1
│   │   └── eus_enabled_mask
│   ├── subslice2
│   │   └── eus_enabled_mask
│   ├── subslice3
│   │   └── eus_enabled_mask
│   └── subslices_enabled_mask
└── slices_enabled_mask


For the topology layout I don't feel like an expert. :(

Is it more correct for it to be per engine instance, or per class?


I think you have a point here.

As far as I can tell this is only one sets of slices. So I guess it 
should be at the class level.




Should it go under the topology subdir?

Should there be a symlink from rcs/0/topology -> ../topology ?


Yeah, sounds good to me.




Should the organization be with more subdirs like 
topology/slice/0/subslice/0/eus_enabled_mask?



I would leave it slice0/subslice0/... for now and if it becomes more 
crowded, create slice/<id>/subslice/<id> with links for backward 
compatibility.

I don't think we're going to expose a lot of properties.




Is (sub)slices_enabled_mask required/helpful or it can be inferred 
from enumerating the slices?


For simplicity in the kernel & userspace, it's helpful.
It makes the reading of the structure simpler.




Same for max_slices? Or perhaps it is a different meaning than the 
enabled mask?


That is essentially to help out userspace.

Most application will use a flat structure to access this information 
with a couple of strides.

The max_* fields gives the strides.



Sorry, this is not very constructive, more questions than answers... :(


An outside opinion always raises interesting questions :)
Thanks!



Regards,

Tvrtko



Each enabled_mask file gives us a mask of the enabled units :


$ cat 
/sys/devices/pci0000\:00/0000\:00\:02.0/drm/card0/gt/rcs/0/slices_enabled_mask

0x7


$ cat 
/sys/devices/pci0000\:00/0000\:00\:02.0/drm/card0/gt/rcs/0/slice0/subslice2/eus_enabled_mask

0xff

v2: Move topology below rcs engine (Chris)

     Add max_eus_per_subslice/max_slices/max_subslices_per_slice 
(Lionel)


v3: Rename enabled_mask (Lionel)

Signed-off-by: Lionel Landwerlin <lionel.g.landwer...@intel.com>
---
  drivers/gpu/drm/i915/i915_drv.h   |  26 ++++++

  drivers/gpu/drm/i915/i915_sysfs.c | 188 
++++++++++++++++++++++++++++++++++++++

  2 files changed, 214 insertions(+)


diff --git a/drivers/gpu/drm/i915/i915_drv.h 
b/drivers/gpu/drm/i915/i915_drv.h

index db550322207c..1ac0a191e8fc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2259,6 +2259,24 @@ struct intel_cdclk_state {
      u8 voltage_level;
  };
  +struct intel_topology_kobject {
+    struct kobject kobj;
+    struct drm_i915_private *dev_priv;
+};
+
+struct intel_slice_kobject {
+    struct kobject kobj;
+    struct drm_i915_private *dev_priv;
+    u8 slice_index;
+};
+
+struct intel_subslice_kobject {
+    struct kobject kobj;
+    struct drm_i915_private *dev_priv;
+    u8 slice_index;
+    u8 subslice_index;
+};
+
  struct drm_i915_private {
      struct drm_device drm;
  @@ -2732,6 +2750,14 @@ struct drm_i915_private {
      struct {
          struct kobject kobj;
          struct kobject classes_kobjs[MAX_ENGINE_CLASS];
+
+        struct sysfs_slice {
+            struct intel_slice_kobject kobj;
+
+            struct sysfs_subslice {
+                struct intel_subslice_kobject kobj;
+            } subslices[GEN_MAX_SUBSLICES];
+        } slices[GEN_MAX_SLICES];
      } gt_topology;

        /* Abstract the submission mechanism (legacy ringbuffer or 
execlists) away */
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c 
b/drivers/gpu/drm/i915/i915_sysfs.c

index fd04d0b93eaf..df9d8fdbcb0a 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c

@@ -559,6 +559,174 @@ static void i915_setup_error_capture(struct 
device *kdev) {}

  static void i915_teardown_error_capture(struct device *kdev) {}
  #endif
  +static struct attribute slices_enabled_mask_attr = {
+    .name = "slices_enabled_mask",
+    .mode = 0444,
+};
+
+static struct attribute subslices_enabled_mask_attr = {
+    .name = "subslices_enabled_mask",
+    .mode = 0444,
+};
+
+static struct attribute eus_enabled_mask_attr = {
+    .name = "eus_enabled_mask",
+    .mode = 0444,
+};
+
+static struct attribute max_slices_attr = {
+    .name = "max_slices",
+    .mode = 0444,
+};
+
+static struct attribute max_subslices_per_slice_attr = {
+    .name = "max_subslices_per_slice",
+    .mode = 0444,
+};
+
+static struct attribute max_eus_per_subslice_attr = {
+    .name = "max_eus_per_subslice",
+    .mode = 0444,
+};
+
+static ssize_t

+show_slice_attr(struct kobject *kobj, struct attribute *attr, char 
*buf)

+{
+    struct intel_slice_kobject *kobj_wrapper =
+        container_of(kobj, struct intel_slice_kobject, kobj);
+    struct drm_i915_private *dev_priv = kobj_wrapper->dev_priv;
+    const struct sseu_dev_info *sseu = &INTEL_INFO(dev_priv)->sseu;
+
+    if (attr == &subslices_enabled_mask_attr) {
+        return sprintf(buf, "0x%hhx\n",
+ sseu->subslices_mask[kobj_wrapper->slice_index]);
+    }
+
+    return sprintf(buf, "0x0\n");
+}
+
+static const struct sysfs_ops slice_ops = {
+    .show = show_slice_attr,
+};
+
+static struct kobj_type slice_type = {
+    .sysfs_ops = &slice_ops,
+};
+
+static ssize_t

+show_subslice_attr(struct kobject *kobj, struct attribute *attr, 
char *buf)

+{
+    struct intel_subslice_kobject *kobj_wrapper =
+        container_of(kobj, struct intel_subslice_kobject, kobj);
+    struct drm_i915_private *dev_priv = kobj_wrapper->dev_priv;
+    const struct sseu_dev_info *sseu = &INTEL_INFO(dev_priv)->sseu;
+    int subslice_stride = ALIGN(sseu->max_eus_per_subslice, 8) / 8;
+    int slice_stride = sseu->max_subslices * subslice_stride;
+
+    if (attr == &eus_enabled_mask_attr)
+        return sprintf(buf, "0x%hhx\n",
+ sseu->eu_mask[kobj_wrapper->slice_index * slice_stride +

+                         kobj_wrapper->subslice_index * 
subslice_stride]);

+    return sprintf(buf, "0x0\n");
+}
+
+static const struct sysfs_ops subslice_ops = {
+    .show = show_subslice_attr,
+};
+
+static struct kobj_type subslice_type = {
+    .sysfs_ops = &subslice_ops,
+};
+

+static int i915_setup_rcs_topology_sysfs(struct drm_i915_private 
*dev_priv,

+                     struct kobject *engine_kobj)
+{
+    const struct sseu_dev_info *sseu = &INTEL_INFO(dev_priv)->sseu;
+    int ret, s, ss;
+
+    ret = sysfs_create_file(engine_kobj, &slices_enabled_mask_attr);
+    if (ret)
+        return ret;
+
+    ret = sysfs_create_file(engine_kobj, &max_slices_attr);
+    if (ret)
+        return ret;
+

+    ret = sysfs_create_file(engine_kobj, 
&max_subslices_per_slice_attr);

+    if (ret)
+        return ret;
+
+    ret = sysfs_create_file(engine_kobj, &max_eus_per_subslice_attr);
+    if (ret)
+        return ret;
+
+    for (s = 0; s < sseu->max_slices; s++) {
+        struct intel_slice_kobject *slice_kobj =
+            &dev_priv->gt_topology.slices[s].kobj;
+
+        slice_kobj->dev_priv = dev_priv;
+        slice_kobj->slice_index = s;
+        ret = kobject_init_and_add(&slice_kobj->kobj, &slice_type,
+                       engine_kobj, "slice%i", s);
+        if (ret)
+            return ret;
+
+        ret = sysfs_create_file(&slice_kobj->kobj,
+                    &subslices_enabled_mask_attr);
+        if (ret)
+            return ret;
+
+        for (ss = 0; ss < sseu->max_subslices; ss++) {
+            struct intel_subslice_kobject *subslice_kobj =
+ &dev_priv->gt_topology.slices[s].subslices[ss].kobj;
+
+            subslice_kobj->dev_priv = dev_priv;
+            subslice_kobj->slice_index = s;
+            subslice_kobj->subslice_index = ss;
+            ret = kobject_init_and_add(&subslice_kobj->kobj,
+                           &subslice_type,
+                           &slice_kobj->kobj,
+                           "subslice%i", ss);
+            if (ret)
+                return ret;
+
+            ret = sysfs_create_file(&subslice_kobj->kobj,
+                        &eus_enabled_mask_attr);
+            if (ret)
+                return ret;
+        }
+    }
+
+    return 0;
+}
+

+static void i915_teardown_rcs_topology_sysfs(struct drm_i915_private 
*dev_priv,

+                         struct kobject *engine_kobj)
+{
+    const struct sseu_dev_info *sseu = &INTEL_INFO(dev_priv)->sseu;
+    int s, ss;
+
+    for (s = 0; s < sseu->max_slices; s++) {
+        struct intel_slice_kobject *slice_kobj =
+            &dev_priv->gt_topology.slices[s].kobj;
+
+        for (ss = 0; ss < sseu->max_subslices; ss++) {
+            struct intel_subslice_kobject *subslice_kobj =
+ &dev_priv->gt_topology.slices[s].subslices[ss].kobj;
+
+            sysfs_remove_file(&subslice_kobj->kobj,
+                      &eus_enabled_mask_attr);
+        }
+
+        sysfs_remove_file(&slice_kobj->kobj,
+                  &subslices_enabled_mask_attr);
+    }
+    sysfs_remove_file(engine_kobj, &slices_enabled_mask_attr);
+    sysfs_remove_file(engine_kobj, &max_eus_per_subslice_attr);
+    sysfs_remove_file(engine_kobj, &max_subslices_per_slice_attr);
+    sysfs_remove_file(engine_kobj, &max_slices_attr);
+}
+
  static struct attribute engine_id_attr = {
      .name = "id",
      .mode = 0444,

@@ -574,11 +742,20 @@ show_engine_attr(struct kobject *kobj, struct 
attribute *attr, char *buf)

  {
      struct intel_engine_cs *engine =
          container_of(kobj, struct intel_engine_cs, instance_kobj);
+    const struct sseu_dev_info *sseu = &INTEL_INFO(engine->i915)->sseu;
        if (attr == &engine_id_attr)
          return sprintf(buf, "%hhu\n", engine->uabi_id);
      if (attr == &engine_class_attr)
          return sprintf(buf, "%hhu\n", engine->uabi_class);
+    if (attr == &slices_enabled_mask_attr)
+        return sprintf(buf, "0x%hhx\n", sseu->slice_mask);
+    if (attr == &max_eus_per_subslice_attr)
+        return sprintf(buf, "%hhd\n", sseu->max_eus_per_subslice);
+    if (attr == &max_subslices_per_slice_attr)
+        return sprintf(buf, "%hhd\n", sseu->max_subslices);
+    if (attr == &max_slices_attr)
+        return sprintf(buf, "%hhd\n", sseu->max_slices);
      return sprintf(buf, "\n");
  }

  @@ -671,6 +848,12 @@ static int i915_setup_engines_sysfs(struct 
drm_i915_private *dev_priv,

                  if (ret)
                      return ret;
              }
+            if (engine->id == RCS) {
+                ret = i915_setup_rcs_topology_sysfs(dev_priv,
+ &engine->instance_kobj);
+                if (ret)
+                    return ret;
+            }
          }
      }

  @@ -683,6 +866,11 @@ static void i915_teardown_engines_sysfs(struct 
drm_i915_private *dev_priv)

      enum intel_engine_id id;
        for_each_engine(engine, dev_priv, id) {
+        if (id == RCS) {
+            i915_teardown_rcs_topology_sysfs(dev_priv,
+                             &engine->instance_kobj);
+        }
+
          sysfs_remove_file(&engine->instance_kobj, &engine_id_attr);
          sysfs_remove_file(&engine->instance_kobj, &engine_class_attr);
          sysfs_remove_file(&engine->capabilities_kobj,





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























					Reply via email to