public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Lionel Landwerlin <lionel.g.landwerlin@intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 4/4] drm/i915: expose EU topology through sysfs
Date: Mon, 20 Nov 2017 16:13:19 +0000	[thread overview]
Message-ID: <2c007984-be6e-5160-1903-2ab7d0a57e22@linux.intel.com> (raw)
In-Reply-To: <20171120122316.25830-5-lionel.g.landwerlin@intel.com>


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?

Should it go under the topology subdir?

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

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

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

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

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

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.landwerlin@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	other threads:[~2017-11-20 16:13 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-20 12:23 [PATCH 0/4] drm/i915: Expose more GPU properties through sysfs Lionel Landwerlin
2017-11-20 12:23 ` [PATCH 1/4] drm/i915: store all subslice masks Lionel Landwerlin
2017-11-20 12:23 ` [PATCH 2/4] drm/i915/debugfs: reuse max slice/subslices already stored in sseu Lionel Landwerlin
2017-11-20 12:23 ` [PATCH 3/4] drm/i915: expose engine availability through sysfs Lionel Landwerlin
2017-11-20 15:57   ` Tvrtko Ursulin
2017-11-21 12:41     ` Ewelina Musial
2017-11-20 16:03   ` Tvrtko Ursulin
2017-11-20 16:33     ` Lionel Landwerlin
2017-11-20 17:08       ` Tvrtko Ursulin
2017-11-28 18:17   ` Tvrtko Ursulin
2017-11-28 20:56     ` Chris Wilson
2017-11-28 21:39       ` Chris Wilson
2017-11-29  8:39         ` Tvrtko Ursulin
2017-11-20 12:23 ` [PATCH 4/4] drm/i915: expose EU topology " Lionel Landwerlin
2017-11-20 16:13   ` Tvrtko Ursulin [this message]
2017-11-20 16:54     ` Lionel Landwerlin
2017-11-20 12:45 ` ✓ Fi.CI.BAT: success for drm/i915: Expose more GPU properties through sysfs (rev3) Patchwork
2017-11-20 13:33 ` ✓ Fi.CI.IGT: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2c007984-be6e-5160-1903-2ab7d0a57e22@linux.intel.com \
    --to=tvrtko.ursulin@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=lionel.g.landwerlin@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox