[RFC 0/3] Engine utilization tracking

[RFC 0/3] Engine utilization tracking

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

By popular customer demand here is the prototype for cheap engine utilization
tracking.

It uses static branches so in the default off case it really should be cheap.

It exports the per engine total time something has been executing (in nano-
seconds) in debugfs/i915_engine_stats.

This is a problem for something which would preferrably be a stable ABI but I
needed all of the open, release and read hooks for it to work as I imagined it.
So far I did not come up with a different location which would support that.

Userspace is supposed to open the file and keep re-reading it by seeking to its
beginning. This is because first open and last close are a costly operations,
and also because the most interesting is the relative change between two
sampling periods.

Example code which does this looks like this:

	#include <stdio.h>
	#include <sys/types.h>
	#include <sys/stat.h>
	#include <fcntl.h>
	#include <unistd.h>

	#include "drmtest.h"
	#include "igt_debugfs.h"

	int main(void)
	{
		int drm_fd, fd;

		drm_fd = drm_open_driver(DRIVER_INTEL);

		fd = igt_debugfs_open(drm_fd, "i915_engine_stats", O_RDONLY);
		igt_assert(fd >= 0);

		for (;;) {
			char buf[4096];
			ssize_t ret;
			off_t off;

			ret = read(fd, buf, sizeof(buf));
			igt_assert(ret > 0);

			ret = write(1, buf, ret);
			printf("\n");

			off = lseek(fd, 0, SEEK_SET);
			igt_assert_eq(off, 0);

			sleep(1);
		}

		close(fd);
		close(drm_fd);

		return 0;
	}

Comments, flames, ideas welcome!

Tvrtko Ursulin (3):
  drm/i915: Wrap context schedule notification
  drm/i915: Engine busy time tracking
  drm/i915: Export engine busy stats in debugfs

 drivers/gpu/drm/i915/i915_debugfs.c     | 120 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_engine_cs.c  |   6 ++
 drivers/gpu/drm/i915/intel_lrc.c        |  24 +++++--
 drivers/gpu/drm/i915/intel_ringbuffer.h |  39 +++++++++++
 4 files changed, 182 insertions(+), 7 deletions(-)

-- 
2.9.3

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

[RFC 1/3] drm/i915: Wrap context schedule notification

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

No functional change just something which will be handy in the
following patch.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 319d9a8f37ca..4c37e94c4d3d 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -306,6 +306,18 @@ execlists_context_status_change(struct drm_i915_gem_request *rq,
 				   status, rq);
 }
 
+static inline void
+execlists_context_schedule_in(struct drm_i915_gem_request *rq)
+{
+	execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN);
+}
+
+static inline void
+execlists_context_schedule_out(struct drm_i915_gem_request *rq)
+{
+	execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
+}
+
 static void
 execlists_update_context_pdps(struct i915_hw_ppgtt *ppgtt, u32 *reg_state)
 {
@@ -345,16 +357,14 @@ static void execlists_submit_ports(struct intel_engine_cs *engine)
 
 	GEM_BUG_ON(port[0].count > 1);
 	if (!port[0].count)
-		execlists_context_status_change(port[0].request,
-						INTEL_CONTEXT_SCHEDULE_IN);
+		execlists_context_schedule_in(port[0].request);
 	desc[0] = execlists_update_context(port[0].request);
 	GEM_DEBUG_EXEC(port[0].context_id = upper_32_bits(desc[0]));
 	port[0].count++;
 
 	if (port[1].request) {
 		GEM_BUG_ON(port[1].count);
-		execlists_context_status_change(port[1].request,
-						INTEL_CONTEXT_SCHEDULE_IN);
+		execlists_context_schedule_in(port[1].request);
 		desc[1] = execlists_update_context(port[1].request);
 		GEM_DEBUG_EXEC(port[1].context_id = upper_32_bits(desc[1]));
 		port[1].count = 1;
@@ -581,9 +591,7 @@ static void intel_lrc_irq_handler(unsigned long data)
 			if (--port[0].count == 0) {
 				GEM_BUG_ON(status & GEN8_CTX_STATUS_PREEMPTED);
 				GEM_BUG_ON(!i915_gem_request_completed(port[0].request));
-				execlists_context_status_change(port[0].request,
-								INTEL_CONTEXT_SCHEDULE_OUT);
-
+				execlists_context_schedule_out(port[0].request);
 				trace_i915_gem_request_out(port[0].request);
 				i915_gem_request_put(port[0].request);
 				port[0] = port[1];
-- 
2.9.3

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

[RFC 2/3] drm/i915: Engine busy time tracking

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Track total time requests have been executing on the hardware.

To make this cheap it is hidden behind a static branch with the
intention that it is only enabled when there is a consumer
listening. This means that in the default off case the total
cost of the tracking is just a few no-op instructions on the
fast paths.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_engine_cs.c  |  6 +++++
 drivers/gpu/drm/i915/intel_lrc.c        |  2 ++
 drivers/gpu/drm/i915/intel_ringbuffer.h | 39 +++++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 483ed7635692..4d42e86ad5de 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -22,10 +22,14 @@
  *
  */
 
+#include <linux/static_key.h>
+
 #include "i915_drv.h"
 #include "intel_ringbuffer.h"
 #include "intel_lrc.h"
 
+DEFINE_STATIC_KEY_FALSE(i915_engine_stats_key);
+
 /* Haswell does have the CXT_SIZE register however it does not appear to be
  * valid. Now, docs explain in dwords what is in the context object. The full
  * size is 70720 bytes, however, the power context and execlist context will
@@ -222,6 +226,8 @@ intel_engine_setup(struct drm_i915_private *dev_priv,
 	/* Nothing to do here, execute in order of dependencies */
 	engine->schedule = NULL;
 
+	spin_lock_init(&engine->stats.lock);
+
 	ATOMIC_INIT_NOTIFIER_HEAD(&engine->context_status_notifier);
 
 	dev_priv->engine[id] = engine;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 4c37e94c4d3d..92da6d1676ce 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -309,12 +309,14 @@ execlists_context_status_change(struct drm_i915_gem_request *rq,
 static inline void
 execlists_context_schedule_in(struct drm_i915_gem_request *rq)
 {
+	intel_engine_context_in(rq->engine);
 	execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN);
 }
 
 static inline void
 execlists_context_schedule_out(struct drm_i915_gem_request *rq)
 {
+	intel_engine_context_out(rq->engine);
 	execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index ec16fb6fde62..dd007dae6533 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -425,6 +425,13 @@ struct intel_engine_cs {
 	 * certain bits to encode the command length in the header).
 	 */
 	u32 (*get_cmd_length_mask)(u32 cmd_header);
+
+       struct {
+               spinlock_t lock;
+               unsigned int ref;
+               u64 start; /* Timestamp of the last idle to active transition. */
+               u64 total; /* Total time engined was busy. */
+       } stats;
 };
 
 static inline unsigned int
@@ -718,4 +725,36 @@ bool intel_engines_are_idle(struct drm_i915_private *dev_priv);
 
 void intel_engines_reset_default_submission(struct drm_i915_private *i915);
 
+DECLARE_STATIC_KEY_FALSE(i915_engine_stats_key);
+
+static inline void intel_engine_context_in(struct intel_engine_cs *engine)
+{
+	if (static_branch_unlikely(&i915_engine_stats_key)) {
+		unsigned long flags;
+
+		spin_lock_irqsave(&engine->stats.lock, flags);
+		if (engine->stats.ref++ == 0)
+			engine->stats.start = ktime_get_real_ns();
+		GEM_BUG_ON(engine->stats.ref == 0);
+		spin_unlock_irqrestore(&engine->stats.lock, flags);
+	}
+}
+
+static inline void intel_engine_context_out(struct intel_engine_cs *engine)
+{
+	if (static_branch_unlikely(&i915_engine_stats_key)) {
+		unsigned long flags;
+
+		spin_lock_irqsave(&engine->stats.lock, flags);
+		/*
+		 * After turning on the static key context out might be the
+		 * first event which then needs to be ignored (ref == 0).
+		 */
+		if (engine->stats.ref && --engine->stats.ref == 0)
+			engine->stats.total += ktime_get_real_ns() -
+					       engine->stats.start;
+		spin_unlock_irqrestore(&engine->stats.lock, flags);
+	}
+}
+
 #endif /* _INTEL_RINGBUFFER_H_ */
-- 
2.9.3

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

[RFC 3/3] drm/i915: Export engine busy stats in debugfs

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Export the stats added in the previous patch in debugfs.

Number of active clients reading this data is tracked and the
static key is only enabled whilst there are some.

Userspace is intended to keep the file descriptor open, seeking
to the beginning of the file periodically, and re-reading the
stats.

This is because the underlying implementation is costly on every
first open/last close transition, and also, because the stats
exported mostly make sense when they are considered relative to
the previous sample.

File lists nanoseconds each engine was active since the tracking
has started.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 120 ++++++++++++++++++++++++++++++++++++
 1 file changed, 120 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 1003511f28cc..db588ef858cb 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4752,6 +4752,120 @@ static const struct file_operations i915_hpd_storm_ctl_fops = {
 	.write = i915_hpd_storm_ctl_write
 };
 
+DECLARE_STATIC_KEY_FALSE(i915_engine_stats_key);
+static DEFINE_MUTEX(i915_engine_stats_mutex);
+static int i915_engine_stats_ref;
+
+struct i915_engine_stats_buf {
+	unsigned int len;
+	size_t available;
+	char buf[0];
+};
+
+static int i915_engine_stats_open(struct inode *inode, struct file *file)
+{
+	const unsigned int engine_name_len =
+		sizeof(((struct intel_engine_cs *)0)->name);
+	struct i915_engine_stats_buf *buf;
+	const unsigned int buf_size =
+		(engine_name_len + 2 + 19 + 1) * I915_NUM_ENGINES + 1 +
+		sizeof(*buf);
+	int ret;
+
+	buf = kzalloc(buf_size, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	buf->len = buf_size;
+	file->private_data = buf;
+
+	ret = mutex_lock_interruptible(&i915_engine_stats_mutex);
+	if (ret)
+		return ret;
+
+	if (i915_engine_stats_ref++ == 0) {
+		struct drm_i915_private *dev_priv = file->f_inode->i_private;
+		struct intel_engine_cs *engine;
+		enum intel_engine_id id;
+
+		for_each_engine(engine, dev_priv, id) {
+			memset(&engine->stats, 0, sizeof(engine->stats));
+			spin_lock_init(&engine->stats.lock);
+		}
+
+		static_branch_enable(&i915_engine_stats_key);
+	}
+
+	mutex_unlock(&i915_engine_stats_mutex);
+
+	return 0;
+}
+
+static int i915_engine_stats_release(struct inode *inode, struct file *file)
+{
+	mutex_lock(&i915_engine_stats_mutex);
+	if (--i915_engine_stats_ref == 0)
+		static_branch_disable(&i915_engine_stats_key);
+	mutex_unlock(&i915_engine_stats_mutex);
+
+	kfree(file->private_data);
+
+	return 0;
+}
+
+static ssize_t i915_engine_stats_read(struct file *file, char __user *ubuf,
+				      size_t count, loff_t *pos)
+{
+	struct i915_engine_stats_buf *buf =
+		(struct i915_engine_stats_buf *)file->private_data;
+
+	if (*pos == 0) {
+		struct drm_i915_private *dev_priv = file->f_inode->i_private;
+		char *ptr = &buf->buf[0];
+		int left = buf->len;
+		struct intel_engine_cs *engine;
+		enum intel_engine_id id;
+
+		buf->available = 0;
+
+		for_each_engine(engine, dev_priv, id) {
+			u64 total;
+			int len;
+
+			spin_lock_irq(&engine->stats.lock);
+			total = engine->stats.total;
+			/*
+			 * If the engine is executing something at the moment
+			 * add it to the total.
+			 */
+			if (engine->stats.ref)
+				total += ktime_get_real_ns() -
+					 engine->stats.start;
+			spin_unlock_irq(&engine->stats.lock);
+
+			len = snprintf(ptr, left, "%s: %llu\n",
+				       engine->name, total);
+			buf->available += len;
+			left -= len;
+			ptr += len;
+
+			if (len == 0)
+				return -EFBIG;
+		}
+	}
+
+	return simple_read_from_buffer(ubuf, count, pos, &buf->buf[0],
+				       buf->available);
+}
+
+static const struct file_operations i915_engine_stats_fops = {
+	.owner = THIS_MODULE,
+	.open = i915_engine_stats_open,
+	.release = i915_engine_stats_release,
+	.read = i915_engine_stats_read,
+	.llseek = default_llseek,
+};
+
 static const struct drm_info_list i915_debugfs_list[] = {
 	{"i915_capabilities", i915_capabilities, 0},
 	{"i915_gem_objects", i915_gem_object_info, 0},
@@ -4839,6 +4953,12 @@ int i915_debugfs_register(struct drm_i915_private *dev_priv)
 	struct dentry *ent;
 	int ret, i;
 
+	ent = debugfs_create_file("i915_engine_stats", S_IRUGO,
+				  minor->debugfs_root, to_i915(minor->dev),
+				  &i915_engine_stats_fops);
+	if (!ent)
+		return -ENOMEM;
+
 	ent = debugfs_create_file("i915_forcewake_user", S_IRUSR,
 				  minor->debugfs_root, to_i915(minor->dev),
 				  &i915_forcewake_fops);
-- 
2.9.3

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

Re: [RFC 0/3] Engine utilization tracking

[Chris Wilson]

On Tue, May 09, 2017 at 03:09:33PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> By popular customer demand here is the prototype for cheap engine utilization
> tracking.

customer and debugfs?

> It uses static branches so in the default off case it really should be cheap.

Not as cheap (for the off case) as simply sampling RING_HEAD/RING_TAIL
which looks to be the same level of detail. I wrapped all this up in a
perf interface once up a time...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for Engine utilization tracking

[Patchwork]

== Series Details ==

Series: Engine utilization tracking
URL   : https://patchwork.freedesktop.org/series/24177/
State : success

== Summary ==

Series 24177v1 Engine utilization tracking
https://patchwork.freedesktop.org/api/1.0/series/24177/revisions/1/mbox/

Test gem_exec_fence:
        Subgroup await-hang-default:
                pass       -> SKIP       (fi-elk-e7500) fdo#100942
Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                dmesg-warn -> PASS       (fi-kbl-7560u) fdo#100125

fdo#100942 https://bugs.freedesktop.org/show_bug.cgi?id=100942
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:444s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:432s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:573s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:520s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:492s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:486s
fi-elk-e7500     total:278  pass:9    dwarn:1   dfail:0   fail:0   skip:72 
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:418s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:406s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:421s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:494s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:466s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:461s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:574s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:457s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:579s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:465s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:499s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:436s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:537s
fi-snb-2600      total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  time:410s
fi-bxt-t5700 failed to collect. IGT log at Patchwork_4650/fi-bxt-t5700/igt.log

417babaa12ad98dad2c39f361612f1afe6894816 drm-tip: 2017y-05m-09d-13h-13m-23s UTC integration manifest
f561414 drm/i915: Export engine busy stats in debugfs
3a7db31 drm/i915: Engine busy time tracking
fa5b8d0 drm/i915: Wrap context schedule notification

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4650/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 0/3] Engine utilization tracking

[Tvrtko Ursulin]

On 09/05/2017 15:26, Chris Wilson wrote:
> On Tue, May 09, 2017 at 03:09:33PM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> By popular customer demand here is the prototype for cheap engine utilization
>> tracking.
>
> customer and debugfs?

Well I did write in one of the following paragraphs on this topic. 
Perhaps I should have put it in procfs. :) Sysfs API looks restrictive 
or perhaps I missed a way to get low level (fops) access to it.

>> It uses static branches so in the default off case it really should be cheap.
>
> Not as cheap (for the off case) as simply sampling RING_HEAD/RING_TAIL

Off case are three no-op instructions in three places in the irq 
tasklet. And a little bit of object size growth, if you worry about that 
aspect?

> which looks to be the same level of detail. I wrapped all this up in a
> perf interface once up a time...

How does that work? Via periodic sampling? Accuracy sounds like it would 
be proportionate to the sampling frequency, no?

Regards,

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

Re: [RFC 0/3] Engine utilization tracking

[Chris Wilson]

On Tue, May 09, 2017 at 04:16:41PM +0100, Tvrtko Ursulin wrote:
> 
> On 09/05/2017 15:26, Chris Wilson wrote:
> >On Tue, May 09, 2017 at 03:09:33PM +0100, Tvrtko Ursulin wrote:
> >>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >>By popular customer demand here is the prototype for cheap engine utilization
> >>tracking.
> >
> >customer and debugfs?
> 
> Well I did write in one of the following paragraphs on this topic.
> Perhaps I should have put it in procfs. :) Sysfs API looks
> restrictive or perhaps I missed a way to get low level (fops) access
> to it.
> 
> >>It uses static branches so in the default off case it really should be cheap.
> >
> >Not as cheap (for the off case) as simply sampling RING_HEAD/RING_TAIL
> 
> Off case are three no-op instructions in three places in the irq
> tasklet. And a little bit of object size growth, if you worry about
> that aspect?

It's just how the snowball begins.
 
> >which looks to be the same level of detail. I wrapped all this up in a
> >perf interface once up a time...
> 
> How does that work? Via periodic sampling? Accuracy sounds like it
> would be proportionate to the sampling frequency, no?

Right, and the sampling frequency is under user control (via perf) with
a default of around 1000, gives a small systematic error when dealing with %

I included power, interrupts, rc6, frequency (and the statistics but I
never used those and dropped them once oa landed), as well as
utilisation, just for the convenience of having sane interface :)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 0/3] Engine utilization tracking

[Tvrtko Ursulin]

On 09/05/2017 16:29, Chris Wilson wrote:
> On Tue, May 09, 2017 at 04:16:41PM +0100, Tvrtko Ursulin wrote:
>>
>> On 09/05/2017 15:26, Chris Wilson wrote:
>>> On Tue, May 09, 2017 at 03:09:33PM +0100, Tvrtko Ursulin wrote:
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> By popular customer demand here is the prototype for cheap engine utilization
>>>> tracking.
>>>
>>> customer and debugfs?
>>
>> Well I did write in one of the following paragraphs on this topic.
>> Perhaps I should have put it in procfs. :) Sysfs API looks
>> restrictive or perhaps I missed a way to get low level (fops) access
>> to it.
>>
>>>> It uses static branches so in the default off case it really should be cheap.
>>>
>>> Not as cheap (for the off case) as simply sampling RING_HEAD/RING_TAIL
>>
>> Off case are three no-op instructions in three places in the irq
>> tasklet. And a little bit of object size growth, if you worry about
>> that aspect?
>
> It's just how the snowball begins.

We should be able to control it. We also have to consider which one is 
lighter for this particular use case.

>>> which looks to be the same level of detail. I wrapped all this up in a
>>> perf interface once up a time...
>>
>> How does that work? Via periodic sampling? Accuracy sounds like it
>> would be proportionate to the sampling frequency, no?
>
> Right, and the sampling frequency is under user control (via perf) with
> a default of around 1000, gives a small systematic error when dealing with %
>
> I included power, interrupts, rc6, frequency (and the statistics but I
> never used those and dropped them once oa landed), as well as
> utilisation, just for the convenience of having sane interface :)

Can you resurrect those patches? Don't have to rebase and all but I 
would like to see them at least.

Regards,

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

Re: [RFC 0/3] Engine utilization tracking

[Dmitry Rogozhkin]

On 5/9/2017 8:51 AM, Tvrtko Ursulin wrote:
>
> On 09/05/2017 16:29, Chris Wilson wrote:
>> On Tue, May 09, 2017 at 04:16:41PM +0100, Tvrtko Ursulin wrote:
>>>
>>> On 09/05/2017 15:26, Chris Wilson wrote:
>>>> On Tue, May 09, 2017 at 03:09:33PM +0100, Tvrtko Ursulin wrote:
>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>
>>>>> By popular customer demand here is the prototype for cheap engine 
>>>>> utilization
>>>>> tracking.
>>>>
>>>> customer and debugfs?
>>>
>>> Well I did write in one of the following paragraphs on this topic.
>>> Perhaps I should have put it in procfs. :) Sysfs API looks
>>> restrictive or perhaps I missed a way to get low level (fops) access
>>> to it.
>>>
>>>>> It uses static branches so in the default off case it really 
>>>>> should be cheap.
>>>>
>>>> Not as cheap (for the off case) as simply sampling RING_HEAD/RING_TAIL
>>>
>>> Off case are three no-op instructions in three places in the irq
>>> tasklet. And a little bit of object size growth, if you worry about
>>> that aspect?
>>
>> It's just how the snowball begins.
>
> We should be able to control it. We also have to consider which one is 
> lighter for this particular use case.
>
>>>> which looks to be the same level of detail. I wrapped all this up in a
>>>> perf interface once up a time...
>>>
>>> How does that work? Via periodic sampling? Accuracy sounds like it
>>> would be proportionate to the sampling frequency, no?
>>
>> Right, and the sampling frequency is under user control (via perf) with
>> a default of around 1000, gives a small systematic error when dealing 
>> with %
>>
>> I included power, interrupts, rc6, frequency (and the statistics but I
>> never used those and dropped them once oa landed), as well as
>> utilisation, just for the convenience of having sane interface :)
>
> Can you resurrect those patches? Don't have to rebase and all but I 
> would like to see them at least.
Mind that the idea behind the requested kind of stats is primary usage 
by the customers in the _product_ environment to track GPU occupancy and 
predict based on this stats whether they can execute something else. 
Which means that 1) debugfs and any kind of debug-like infrastructure is 
really a no-option, 2) any kind of restrictions are no-option (like 
disable RC6 states). Also, there is no need to expose low-level detailed 
information like how many EUs and VMEs were in use - this belongs to the 
debug things. As for now i915 driver exposes only single required 
metric: gt_act_freq_mhz.

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

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

Re: [RFC 3/3] drm/i915: Export engine busy stats in debugfs

[Dmitry Rogozhkin]

On 5/9/2017 7:09 AM, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Export the stats added in the previous patch in debugfs.
>
> Number of active clients reading this data is tracked and the
> static key is only enabled whilst there are some.
>
> Userspace is intended to keep the file descriptor open, seeking
> to the beginning of the file periodically, and re-reading the
> stats.
>
> This is because the underlying implementation is costly on every
> first open/last close transition, and also, because the stats
> exported mostly make sense when they are considered relative to
> the previous sample.
>
> File lists nanoseconds each engine was active since the tracking
> has started.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c | 120 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 120 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 1003511f28cc..db588ef858cb 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4752,6 +4752,120 @@ static const struct file_operations i915_hpd_storm_ctl_fops = {
>   	.write = i915_hpd_storm_ctl_write
>   };
>   
> +DECLARE_STATIC_KEY_FALSE(i915_engine_stats_key);
> +static DEFINE_MUTEX(i915_engine_stats_mutex);
> +static int i915_engine_stats_ref;
> +
> +struct i915_engine_stats_buf {
> +	unsigned int len;
> +	size_t available;
> +	char buf[0];
> +};
> +
> +static int i915_engine_stats_open(struct inode *inode, struct file *file)
> +{
> +	const unsigned int engine_name_len =
> +		sizeof(((struct intel_engine_cs *)0)->name);
> +	struct i915_engine_stats_buf *buf;
> +	const unsigned int buf_size =
> +		(engine_name_len + 2 + 19 + 1) * I915_NUM_ENGINES + 1 +
> +		sizeof(*buf);
> +	int ret;
> +
> +	buf = kzalloc(buf_size, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	buf->len = buf_size;
> +	file->private_data = buf;
> +
> +	ret = mutex_lock_interruptible(&i915_engine_stats_mutex);
> +	if (ret)
> +		return ret;
> +
> +	if (i915_engine_stats_ref++ == 0) {
> +		struct drm_i915_private *dev_priv = file->f_inode->i_private;
> +		struct intel_engine_cs *engine;
> +		enum intel_engine_id id;
> +
> +		for_each_engine(engine, dev_priv, id) {
> +			memset(&engine->stats, 0, sizeof(engine->stats));
> +			spin_lock_init(&engine->stats.lock);
> +		}
> +
> +		static_branch_enable(&i915_engine_stats_key);
> +	}
> +
> +	mutex_unlock(&i915_engine_stats_mutex);
> +
> +	return 0;
> +}
> +
> +static int i915_engine_stats_release(struct inode *inode, struct file *file)
> +{
> +	mutex_lock(&i915_engine_stats_mutex);
> +	if (--i915_engine_stats_ref == 0)
> +		static_branch_disable(&i915_engine_stats_key);
> +	mutex_unlock(&i915_engine_stats_mutex);
> +
> +	kfree(file->private_data);
> +
> +	return 0;
> +}
> +
> +static ssize_t i915_engine_stats_read(struct file *file, char __user *ubuf,
> +				      size_t count, loff_t *pos)
> +{
> +	struct i915_engine_stats_buf *buf =
> +		(struct i915_engine_stats_buf *)file->private_data;
> +
> +	if (*pos == 0) {
> +		struct drm_i915_private *dev_priv = file->f_inode->i_private;
> +		char *ptr = &buf->buf[0];
> +		int left = buf->len;
> +		struct intel_engine_cs *engine;
> +		enum intel_engine_id id;
> +
> +		buf->available = 0;
> +
> +		for_each_engine(engine, dev_priv, id) {
> +			u64 total;
> +			int len;
> +
> +			spin_lock_irq(&engine->stats.lock);
> +			total = engine->stats.total;
> +			/*
> +			 * If the engine is executing something at the moment
> +			 * add it to the total.
> +			 */
> +			if (engine->stats.ref)
> +				total += ktime_get_real_ns() -
> +					 engine->stats.start;
> +			spin_unlock_irq(&engine->stats.lock);
> +
> +			len = snprintf(ptr, left, "%s: %llu\n",
> +				       engine->name, total);
If I caught it right, file format is:
   render ring: 12345
   bsd ring: 12345
   ...
where numbers are busy clocks (ns) from the system boot time. Is that 
right? What if we will want to expose some other statistics information 
later, not only busy clocks? For example, engines i915 queues depths is 
a next interest. Maybe later we will find something else interesting. 
So, do we want to consider this file to contain all kind of statistics 
in the future, and hence it should be of somewhat different format, or 
it will have only busy clocks, and maybe we need other file name then?
> +			buf->available += len;
> +			left -= len;
> +			ptr += len;
> +
> +			if (len == 0)
> +				return -EFBIG;
> +		}
> +	}
> +
> +	return simple_read_from_buffer(ubuf, count, pos, &buf->buf[0],
> +				       buf->available);
> +}
> +
> +static const struct file_operations i915_engine_stats_fops = {
> +	.owner = THIS_MODULE,
> +	.open = i915_engine_stats_open,
> +	.release = i915_engine_stats_release,
> +	.read = i915_engine_stats_read,
> +	.llseek = default_llseek,
> +};
> +
>   static const struct drm_info_list i915_debugfs_list[] = {
>   	{"i915_capabilities", i915_capabilities, 0},
>   	{"i915_gem_objects", i915_gem_object_info, 0},
> @@ -4839,6 +4953,12 @@ int i915_debugfs_register(struct drm_i915_private *dev_priv)
>   	struct dentry *ent;
>   	int ret, i;
>   
> +	ent = debugfs_create_file("i915_engine_stats", S_IRUGO,
> +				  minor->debugfs_root, to_i915(minor->dev),
> +				  &i915_engine_stats_fops);
> +	if (!ent)
> +		return -ENOMEM;
> +
>   	ent = debugfs_create_file("i915_forcewake_user", S_IRUSR,
>   				  minor->debugfs_root, to_i915(minor->dev),
>   				  &i915_forcewake_fops);

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

Re: [RFC 3/3] drm/i915: Export engine busy stats in debugfs

[Tvrtko Ursulin]

On 09/05/2017 19:17, Dmitry Rogozhkin wrote:
> On 5/9/2017 7:09 AM, Tvrtko Ursulin wrote:

[snip]

>> +static ssize_t i915_engine_stats_read(struct file *file, char __user
>> *ubuf,
>> +                      size_t count, loff_t *pos)
>> +{
>> +    struct i915_engine_stats_buf *buf =
>> +        (struct i915_engine_stats_buf *)file->private_data;
>> +
>> +    if (*pos == 0) {
>> +        struct drm_i915_private *dev_priv = file->f_inode->i_private;
>> +        char *ptr = &buf->buf[0];
>> +        int left = buf->len;
>> +        struct intel_engine_cs *engine;
>> +        enum intel_engine_id id;
>> +
>> +        buf->available = 0;
>> +
>> +        for_each_engine(engine, dev_priv, id) {
>> +            u64 total;
>> +            int len;
>> +
>> +            spin_lock_irq(&engine->stats.lock);
>> +            total = engine->stats.total;
>> +            /*
>> +             * If the engine is executing something at the moment
>> +             * add it to the total.
>> +             */
>> +            if (engine->stats.ref)
>> +                total += ktime_get_real_ns() -
>> +                     engine->stats.start;
>> +            spin_unlock_irq(&engine->stats.lock);
>> +
>> +            len = snprintf(ptr, left, "%s: %llu\n",
>> +                       engine->name, total);
> If I caught it right, file format is:
>   render ring: 12345
>   bsd ring: 12345
>   ...

Yes almost, just that the engine names have been changed to likes of 
rcs0, vcs0, vcs1, vecs0 and bcs0 in the meantime.

> where numbers are busy clocks (ns) from the system boot time. Is that

Nanoseconds, but not since boot time but since the last time tracking 
got enabled.

Because the most important thing in this version, from the point of view 
of overhead in interrupt tasklet, is that the tracking is not done 
unless somebody is listening (has the file open).

As I wrote in the cover letter and the 2nd patch, when nobody has the 
file open the only thing which exists in the interrupt tasklets are 
three no-nop instructions. They only get patched to jumps (to sections 
actually collecting the stats) for as long as someone has the file open.

> right? What if we will want to expose some other statistics information
> later, not only busy clocks? For example, engines i915 queues depths is
> a next interest. Maybe later we will find something else interesting.
> So, do we want to consider this file to contain all kind of statistics
> in the future, and hence it should be of somewhat different format, or
> it will have only busy clocks, and maybe we need other file name then?

It can be either of the two, or some third option. It sounds like it is 
too early to discuss those level of detail. At this point it was an RFC 
only to gather some opinions on the overall idea.

Regards,

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

Re: [RFC 0/3] Engine utilization tracking

[Tvrtko Ursulin]

On 09/05/2017 19:11, Dmitry Rogozhkin wrote:
> On 5/9/2017 8:51 AM, Tvrtko Ursulin wrote:
>> On 09/05/2017 16:29, Chris Wilson wrote:
>>> On Tue, May 09, 2017 at 04:16:41PM +0100, Tvrtko Ursulin wrote:
>>>>
>>>> On 09/05/2017 15:26, Chris Wilson wrote:
>>>>> On Tue, May 09, 2017 at 03:09:33PM +0100, Tvrtko Ursulin wrote:
>>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>>
>>>>>> By popular customer demand here is the prototype for cheap engine
>>>>>> utilization
>>>>>> tracking.
>>>>>
>>>>> customer and debugfs?
>>>>
>>>> Well I did write in one of the following paragraphs on this topic.
>>>> Perhaps I should have put it in procfs. :) Sysfs API looks
>>>> restrictive or perhaps I missed a way to get low level (fops) access
>>>> to it.
>>>>
>>>>>> It uses static branches so in the default off case it really
>>>>>> should be cheap.
>>>>>
>>>>> Not as cheap (for the off case) as simply sampling RING_HEAD/RING_TAIL
>>>>
>>>> Off case are three no-op instructions in three places in the irq
>>>> tasklet. And a little bit of object size growth, if you worry about
>>>> that aspect?
>>>
>>> It's just how the snowball begins.
>>
>> We should be able to control it. We also have to consider which one is
>> lighter for this particular use case.
>>
>>>>> which looks to be the same level of detail. I wrapped all this up in a
>>>>> perf interface once up a time...
>>>>
>>>> How does that work? Via periodic sampling? Accuracy sounds like it
>>>> would be proportionate to the sampling frequency, no?
>>>
>>> Right, and the sampling frequency is under user control (via perf) with
>>> a default of around 1000, gives a small systematic error when dealing
>>> with %
>>>
>>> I included power, interrupts, rc6, frequency (and the statistics but I
>>> never used those and dropped them once oa landed), as well as
>>> utilisation, just for the convenience of having sane interface :)
>>
>> Can you resurrect those patches? Don't have to rebase and all but I
>> would like to see them at least.
> Mind that the idea behind the requested kind of stats is primary usage
> by the customers in the _product_ environment to track GPU occupancy and
> predict based on this stats whether they can execute something else.
> Which means that 1) debugfs and any kind of debug-like infrastructure is

Yeah I acknowledged in the cover letter debugfs is not ideal.

I could implement it in sysfs I suppose by doing time based transitions 
as opposed to having explicit open/release hooks. It wouldn't make a 
fundamental different to this RFC from the overhead point of view.

But most importantly we need to see in detail how does Chris' perf based 
idea looks like and does it fit your requirements.

> really a no-option, 2) any kind of restrictions are no-option (like
> disable RC6 states). Also, there is no need to expose low-level detailed
> information like how many EUs and VMEs were in use - this belongs to the
> debug things. As for now i915 driver exposes only single required
> metric: gt_act_freq_mhz.

I suppose it doesn't matter if the perf based solution (or any really) 
exports more than what you want/need since it is such that you can 
select the events you are interested in.

But the overhead and accuracy of both solutions, plus some other 
considerations like maintainability, need to be looked at.

Regards,

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

Re: [RFC 0/3] Engine utilization tracking

[Chris Wilson]

On Tue, May 09, 2017 at 04:51:05PM +0100, Tvrtko Ursulin wrote:
> On 09/05/2017 16:29, Chris Wilson wrote:
> >On Tue, May 09, 2017 at 04:16:41PM +0100, Tvrtko Ursulin wrote:
> Can you resurrect those patches? Don't have to rebase and all but I
> would like to see them at least.

https://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=prescheduler&id=19c209495dcbfc9f7a7930e7b7d7109e3a348264
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 2/3] drm/i915: Engine busy time tracking

[Joonas Lahtinen]

On ti, 2017-05-09 at 15:09 +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Track total time requests have been executing on the hardware.
> 
> To make this cheap it is hidden behind a static branch with the
> intention that it is only enabled when there is a consumer
> listening. This means that in the default off case the total
> cost of the tracking is just a few no-op instructions on the
> fast paths.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

<SNIP>

> @@ -309,12 +309,14 @@ execlists_context_status_change(struct drm_i915_gem_request *rq,
>  static inline void
>  execlists_context_schedule_in(struct drm_i915_gem_request *rq)
>  {
> +	intel_engine_context_in(rq->engine);
>  	execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN);
>  }
>  
>  static inline void
>  execlists_context_schedule_out(struct drm_i915_gem_request *rq)
>  {
> +	intel_engine_context_out(rq->engine);
>  	execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
>  }

It's the perfect opportunity to actually use the notifier chain and
optimize its "empty" case instead of rolling our own.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 0/3] Engine utilization tracking

[Dmitry Rogozhkin]

On 5/10/2017 1:38 AM, Tvrtko Ursulin wrote:
>
> On 09/05/2017 19:11, Dmitry Rogozhkin wrote:
>> On 5/9/2017 8:51 AM, Tvrtko Ursulin wrote:
>>> On 09/05/2017 16:29, Chris Wilson wrote:
>>>> On Tue, May 09, 2017 at 04:16:41PM +0100, Tvrtko Ursulin wrote:
>>>>>
>>>>> On 09/05/2017 15:26, Chris Wilson wrote:
>>>>>> On Tue, May 09, 2017 at 03:09:33PM +0100, Tvrtko Ursulin wrote:
>>>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>>>
>>>>>>> By popular customer demand here is the prototype for cheap engine
>>>>>>> utilization
>>>>>>> tracking.
>>>>>>
>>>>>> customer and debugfs?
>>>>>
>>>>> Well I did write in one of the following paragraphs on this topic.
>>>>> Perhaps I should have put it in procfs. :) Sysfs API looks
>>>>> restrictive or perhaps I missed a way to get low level (fops) access
>>>>> to it.
>>>>>
>>>>>>> It uses static branches so in the default off case it really
>>>>>>> should be cheap.
>>>>>>
>>>>>> Not as cheap (for the off case) as simply sampling 
>>>>>> RING_HEAD/RING_TAIL
>>>>>
>>>>> Off case are three no-op instructions in three places in the irq
>>>>> tasklet. And a little bit of object size growth, if you worry about
>>>>> that aspect?
>>>>
>>>> It's just how the snowball begins.
>>>
>>> We should be able to control it. We also have to consider which one is
>>> lighter for this particular use case.
>>>
>>>>>> which looks to be the same level of detail. I wrapped all this up 
>>>>>> in a
>>>>>> perf interface once up a time...
>>>>>
>>>>> How does that work? Via periodic sampling? Accuracy sounds like it
>>>>> would be proportionate to the sampling frequency, no?
>>>>
>>>> Right, and the sampling frequency is under user control (via perf) 
>>>> with
>>>> a default of around 1000, gives a small systematic error when dealing
>>>> with %
>>>>
>>>> I included power, interrupts, rc6, frequency (and the statistics but I
>>>> never used those and dropped them once oa landed), as well as
>>>> utilisation, just for the convenience of having sane interface :)
>>>
>>> Can you resurrect those patches? Don't have to rebase and all but I
>>> would like to see them at least.
>> Mind that the idea behind the requested kind of stats is primary usage
>> by the customers in the _product_ environment to track GPU occupancy and
>> predict based on this stats whether they can execute something else.
>> Which means that 1) debugfs and any kind of debug-like infrastructure is
>
> Yeah I acknowledged in the cover letter debugfs is not ideal.
>
> I could implement it in sysfs I suppose by doing time based 
> transitions as opposed to having explicit open/release hooks. It 
> wouldn't make a fundamental different to this RFC from the overhead 
> point of view.
>
> But most importantly we need to see in detail how does Chris' perf 
> based idea looks like and does it fit your requirements.
>
>> really a no-option, 2) any kind of restrictions are no-option (like
>> disable RC6 states). Also, there is no need to expose low-level detailed
>> information like how many EUs and VMEs were in use - this belongs to the
>> debug things. As for now i915 driver exposes only single required
>> metric: gt_act_freq_mhz.
>
> I suppose it doesn't matter if the perf based solution (or any really) 
> exports more than what you want/need since it is such that you can 
> select the events you are interested in.
>
> But the overhead and accuracy of both solutions, plus some other 
> considerations like maintainability, need to be looked at.
>

Let's review both solutions. I am not against.

> Regards,
>
> Tvrtko

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

Re: [RFC 3/3] drm/i915: Export engine busy stats in debugfs

[Dmitry Rogozhkin]

On 5/10/2017 1:30 AM, Tvrtko Ursulin wrote:
>
> On 09/05/2017 19:17, Dmitry Rogozhkin wrote:
>> On 5/9/2017 7:09 AM, Tvrtko Ursulin wrote:
>
> [snip]
>
>>> +static ssize_t i915_engine_stats_read(struct file *file, char __user
>>> *ubuf,
>>> +                      size_t count, loff_t *pos)
>>> +{
>>> +    struct i915_engine_stats_buf *buf =
>>> +        (struct i915_engine_stats_buf *)file->private_data;
>>> +
>>> +    if (*pos == 0) {
>>> +        struct drm_i915_private *dev_priv = file->f_inode->i_private;
>>> +        char *ptr = &buf->buf[0];
>>> +        int left = buf->len;
>>> +        struct intel_engine_cs *engine;
>>> +        enum intel_engine_id id;
>>> +
>>> +        buf->available = 0;
>>> +
>>> +        for_each_engine(engine, dev_priv, id) {
>>> +            u64 total;
>>> +            int len;
>>> +
>>> +            spin_lock_irq(&engine->stats.lock);
>>> +            total = engine->stats.total;
>>> +            /*
>>> +             * If the engine is executing something at the moment
>>> +             * add it to the total.
>>> +             */
>>> +            if (engine->stats.ref)
>>> +                total += ktime_get_real_ns() -
>>> +                     engine->stats.start;
>>> +            spin_unlock_irq(&engine->stats.lock);
>>> +
>>> +            len = snprintf(ptr, left, "%s: %llu\n",
>>> +                       engine->name, total);
>> If I caught it right, file format is:
>>   render ring: 12345
>>   bsd ring: 12345
>>   ...
>
> Yes almost, just that the engine names have been changed to likes of 
> rcs0, vcs0, vcs1, vecs0 and bcs0 in the meantime.
>
>> where numbers are busy clocks (ns) from the system boot time. Is that
>
> Nanoseconds, but not since boot time but since the last time tracking 
> got enabled.

 From my perspective that's bad: clocks from the boot time is more 
natural metric. And with it you will be able to definitely know what you 
did with GPU on the boot time. For certain customers like on Android and 
other embedded devices this is critically important. Just recently we 
worked in one of our project on the boot time optimization. Thus, I 
would recommend to have this metric permanently available.

Now, if we will still fall to the clocks from the some moment in time, I 
do not like "the last time tracking got enabled" approach. You did not 
count on the few consumers of the metric? Why? What if there are few 
independent clients requesting the access to the metric in parallel? If 
you track from the moment when last client requested an access, then you 
will damage data for the clients already having access.

>
> Because the most important thing in this version, from the point of 
> view of overhead in interrupt tasklet, is that the tracking is not 
> done unless somebody is listening (has the file open).
>
> As I wrote in the cover letter and the 2nd patch, when nobody has the 
> file open the only thing which exists in the interrupt tasklets are 
> three no-nop instructions. They only get patched to jumps (to sections 
> actually collecting the stats) for as long as someone has the file open.
>
>> right? What if we will want to expose some other statistics information
>> later, not only busy clocks? For example, engines i915 queues depths is
>> a next interest. Maybe later we will find something else interesting.
>> So, do we want to consider this file to contain all kind of statistics
>> in the future, and hence it should be of somewhat different format, or
>> it will have only busy clocks, and maybe we need other file name then?
>
> It can be either of the two, or some third option. It sounds like it 
> is too early to discuss those level of detail. At this point it was an 
> RFC only to gather some opinions on the overall idea.
Yep, agree. Just something to remember going forward...
>
> Regards,
>
> Tvrtko

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

Re: [RFC 0/3] Engine utilization tracking

[Daniel Vetter]

On Wed, May 10, 2017 at 10:38 AM, Tvrtko Ursulin
<tvrtko.ursulin@linux.intel.com> wrote:
>
> On 09/05/2017 19:11, Dmitry Rogozhkin wrote:
>>
>> On 5/9/2017 8:51 AM, Tvrtko Ursulin wrote:
>>>
>>> On 09/05/2017 16:29, Chris Wilson wrote:
>>>>
>>>> On Tue, May 09, 2017 at 04:16:41PM +0100, Tvrtko Ursulin wrote:
>>>>>
>>>>>
>>>>> On 09/05/2017 15:26, Chris Wilson wrote:
>>>>>>
>>>>>> On Tue, May 09, 2017 at 03:09:33PM +0100, Tvrtko Ursulin wrote:
>>>>>>>
>>>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>>>
>>>>>>> By popular customer demand here is the prototype for cheap engine
>>>>>>> utilization
>>>>>>> tracking.
>>>>>>
>>>>>>
>>>>>> customer and debugfs?
>>>>>
>>>>>
>>>>> Well I did write in one of the following paragraphs on this topic.
>>>>> Perhaps I should have put it in procfs. :) Sysfs API looks
>>>>> restrictive or perhaps I missed a way to get low level (fops) access
>>>>> to it.
>>>>>
>>>>>>> It uses static branches so in the default off case it really
>>>>>>> should be cheap.
>>>>>>
>>>>>>
>>>>>> Not as cheap (for the off case) as simply sampling RING_HEAD/RING_TAIL
>>>>>
>>>>>
>>>>> Off case are three no-op instructions in three places in the irq
>>>>> tasklet. And a little bit of object size growth, if you worry about
>>>>> that aspect?
>>>>
>>>>
>>>> It's just how the snowball begins.
>>>
>>>
>>> We should be able to control it. We also have to consider which one is
>>> lighter for this particular use case.
>>>
>>>>>> which looks to be the same level of detail. I wrapped all this up in a
>>>>>> perf interface once up a time...
>>>>>
>>>>>
>>>>> How does that work? Via periodic sampling? Accuracy sounds like it
>>>>> would be proportionate to the sampling frequency, no?
>>>>
>>>>
>>>> Right, and the sampling frequency is under user control (via perf) with
>>>> a default of around 1000, gives a small systematic error when dealing
>>>> with %
>>>>
>>>> I included power, interrupts, rc6, frequency (and the statistics but I
>>>> never used those and dropped them once oa landed), as well as
>>>> utilisation, just for the convenience of having sane interface :)
>>>
>>>
>>> Can you resurrect those patches? Don't have to rebase and all but I
>>> would like to see them at least.
>>
>> Mind that the idea behind the requested kind of stats is primary usage
>> by the customers in the _product_ environment to track GPU occupancy and
>> predict based on this stats whether they can execute something else.
>> Which means that 1) debugfs and any kind of debug-like infrastructure is
>
>
> Yeah I acknowledged in the cover letter debugfs is not ideal.
>
> I could implement it in sysfs I suppose by doing time based transitions as
> opposed to having explicit open/release hooks. It wouldn't make a
> fundamental different to this RFC from the overhead point of view.
>
> But most importantly we need to see in detail how does Chris' perf based
> idea looks like and does it fit your requirements.

+1 on perf pmu, that sounds much more like the userspace interface
you're looking for. If it's not that, then perhaps hand-rolled like
the i915 OA stuff we now have (but starting out with a perf pmu sounds
much better, at least for anything global which doesn't need to be
per-context or per-batch).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 0/3] Engine utilization tracking

[Dmitry Rogozhkin]

On 5/10/2017 12:45 PM, Daniel Vetter wrote:
> On Wed, May 10, 2017 at 10:38 AM, Tvrtko Ursulin
> <tvrtko.ursulin@linux.intel.com> wrote:
>> On 09/05/2017 19:11, Dmitry Rogozhkin wrote:
>>> On 5/9/2017 8:51 AM, Tvrtko Ursulin wrote:
>>>> On 09/05/2017 16:29, Chris Wilson wrote:
>>>>> On Tue, May 09, 2017 at 04:16:41PM +0100, Tvrtko Ursulin wrote:
>>>>>>
>>>>>> On 09/05/2017 15:26, Chris Wilson wrote:
>>>>>>> On Tue, May 09, 2017 at 03:09:33PM +0100, Tvrtko Ursulin wrote:
>>>>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>>>>
>>>>>>>> By popular customer demand here is the prototype for cheap engine
>>>>>>>> utilization
>>>>>>>> tracking.
>>>>>>>
>>>>>>> customer and debugfs?
>>>>>>
>>>>>> Well I did write in one of the following paragraphs on this topic.
>>>>>> Perhaps I should have put it in procfs. :) Sysfs API looks
>>>>>> restrictive or perhaps I missed a way to get low level (fops) access
>>>>>> to it.
>>>>>>
>>>>>>>> It uses static branches so in the default off case it really
>>>>>>>> should be cheap.
>>>>>>>
>>>>>>> Not as cheap (for the off case) as simply sampling RING_HEAD/RING_TAIL
>>>>>>
>>>>>> Off case are three no-op instructions in three places in the irq
>>>>>> tasklet. And a little bit of object size growth, if you worry about
>>>>>> that aspect?
>>>>>
>>>>> It's just how the snowball begins.
>>>>
>>>> We should be able to control it. We also have to consider which one is
>>>> lighter for this particular use case.
>>>>
>>>>>>> which looks to be the same level of detail. I wrapped all this up in a
>>>>>>> perf interface once up a time...
>>>>>>
>>>>>> How does that work? Via periodic sampling? Accuracy sounds like it
>>>>>> would be proportionate to the sampling frequency, no?
>>>>>
>>>>> Right, and the sampling frequency is under user control (via perf) with
>>>>> a default of around 1000, gives a small systematic error when dealing
>>>>> with %
>>>>>
>>>>> I included power, interrupts, rc6, frequency (and the statistics but I
>>>>> never used those and dropped them once oa landed), as well as
>>>>> utilisation, just for the convenience of having sane interface :)
>>>>
>>>> Can you resurrect those patches? Don't have to rebase and all but I
>>>> would like to see them at least.
>>> Mind that the idea behind the requested kind of stats is primary usage
>>> by the customers in the _product_ environment to track GPU occupancy and
>>> predict based on this stats whether they can execute something else.
>>> Which means that 1) debugfs and any kind of debug-like infrastructure is
>>
>> Yeah I acknowledged in the cover letter debugfs is not ideal.
>>
>> I could implement it in sysfs I suppose by doing time based transitions as
>> opposed to having explicit open/release hooks. It wouldn't make a
>> fundamental different to this RFC from the overhead point of view.
>>
>> But most importantly we need to see in detail how does Chris' perf based
>> idea looks like and does it fit your requirements.
> +1 on perf pmu, that sounds much more like the userspace interface
> you're looking for. If it's not that, then perhaps hand-rolled like
> the i915 OA stuff we now have (but starting out with a perf pmu sounds
> much better, at least for anything global which doesn't need to be
> per-context or per-batch).
> -Daniel
You know, thinking once more time which interface I would like to see as 
a user, I would say the following. As a user I expect to have easy 
access to the basic GPU information and current characteristics. This 
information includes:
1. GPU frequency characteristics including: current running frequency, 
min/max SW limits, min/max HW limits, boost frequency settings (if any), 
driver power/performance preset (if any)
2. Basic information of GPU high level structure, I specifically mean 
engines capable to work in parallel: number of VDBOX engines, number of 
VEBOX engines, etc.
3. High level metric to understand how GPU was busy over time: each 
engine busy clocks
I would assume that there will be users who will simply log to the 
system and want to quickly get the above info with the cat /sysfs file. 
I would assume that some programmatic usages are possible to parse sysfs 
and take certain actions if, for example, current GPU support single 
VDBOX only (for example, run some operation as SW decoding, rather than 
HW). So, I would suggest to have /sysfs files for the information above.

Perf subsystem indeed looks attractive to expose such a metrics, but I 
think we need to target lower level metrics with Perf. From my 
perspective right now i915 misses exposure of certain key information 
which is natively expected by any user and developer. Using perf to 
expose it will force users to use special tools or write own programs to 
query them - this will simply reduce usability. After all, why you 
expose /sys/class/drm/card0/power/rc6_residency_ms and you do not expose 
how much time GPGPU or VDBOX did its job?! Honestly, RC6 is a second 
level of details for the significant part of the users and customers.

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