* [RFC 0/3] Engine utilization tracking @ 2017-05-09 14:09 Tvrtko Ursulin 2017-05-09 14:09 ` [RFC 1/3] drm/i915: Wrap context schedule notification Tvrtko Ursulin ` (4 more replies) 0 siblings, 5 replies; 19+ messages in thread From: Tvrtko Ursulin @ 2017-05-09 14:09 UTC (permalink / raw) To: Intel-gfx 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC 1/3] drm/i915: Wrap context schedule notification 2017-05-09 14:09 [RFC 0/3] Engine utilization tracking Tvrtko Ursulin @ 2017-05-09 14:09 ` Tvrtko Ursulin 2017-05-09 14:09 ` [RFC 2/3] drm/i915: Engine busy time tracking Tvrtko Ursulin ` (3 subsequent siblings) 4 siblings, 0 replies; 19+ messages in thread From: Tvrtko Ursulin @ 2017-05-09 14:09 UTC (permalink / raw) To: Intel-gfx 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC 2/3] drm/i915: Engine busy time tracking 2017-05-09 14:09 [RFC 0/3] Engine utilization tracking Tvrtko Ursulin 2017-05-09 14:09 ` [RFC 1/3] drm/i915: Wrap context schedule notification Tvrtko Ursulin @ 2017-05-09 14:09 ` Tvrtko Ursulin 2017-05-10 12:41 ` Joonas Lahtinen 2017-05-09 14:09 ` [RFC 3/3] drm/i915: Export engine busy stats in debugfs Tvrtko Ursulin ` (2 subsequent siblings) 4 siblings, 1 reply; 19+ messages in thread From: Tvrtko Ursulin @ 2017-05-09 14:09 UTC (permalink / raw) To: Intel-gfx 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [RFC 2/3] drm/i915: Engine busy time tracking 2017-05-09 14:09 ` [RFC 2/3] drm/i915: Engine busy time tracking Tvrtko Ursulin @ 2017-05-10 12:41 ` Joonas Lahtinen 0 siblings, 0 replies; 19+ messages in thread From: Joonas Lahtinen @ 2017-05-10 12:41 UTC (permalink / raw) To: Tvrtko Ursulin, Intel-gfx 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC 3/3] drm/i915: Export engine busy stats in debugfs 2017-05-09 14:09 [RFC 0/3] Engine utilization tracking Tvrtko Ursulin 2017-05-09 14:09 ` [RFC 1/3] drm/i915: Wrap context schedule notification Tvrtko Ursulin 2017-05-09 14:09 ` [RFC 2/3] drm/i915: Engine busy time tracking Tvrtko Ursulin @ 2017-05-09 14:09 ` Tvrtko Ursulin 2017-05-09 18:17 ` Dmitry Rogozhkin 2017-05-09 14:26 ` [RFC 0/3] Engine utilization tracking Chris Wilson 2017-05-09 14:51 ` ✓ Fi.CI.BAT: success for " Patchwork 4 siblings, 1 reply; 19+ messages in thread From: Tvrtko Ursulin @ 2017-05-09 14:09 UTC (permalink / raw) To: Intel-gfx 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [RFC 3/3] drm/i915: Export engine busy stats in debugfs 2017-05-09 14:09 ` [RFC 3/3] drm/i915: Export engine busy stats in debugfs Tvrtko Ursulin @ 2017-05-09 18:17 ` Dmitry Rogozhkin 2017-05-10 8:30 ` Tvrtko Ursulin 0 siblings, 1 reply; 19+ messages in thread From: Dmitry Rogozhkin @ 2017-05-09 18:17 UTC (permalink / raw) To: Tvrtko Ursulin, Intel-gfx 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC 3/3] drm/i915: Export engine busy stats in debugfs 2017-05-09 18:17 ` Dmitry Rogozhkin @ 2017-05-10 8:30 ` Tvrtko Ursulin 2017-05-10 15:57 ` Dmitry Rogozhkin 0 siblings, 1 reply; 19+ messages in thread From: Tvrtko Ursulin @ 2017-05-10 8:30 UTC (permalink / raw) To: Dmitry Rogozhkin, Tvrtko Ursulin, Intel-gfx 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC 3/3] drm/i915: Export engine busy stats in debugfs 2017-05-10 8:30 ` Tvrtko Ursulin @ 2017-05-10 15:57 ` Dmitry Rogozhkin 0 siblings, 0 replies; 19+ messages in thread From: Dmitry Rogozhkin @ 2017-05-10 15:57 UTC (permalink / raw) To: Tvrtko Ursulin, Tvrtko Ursulin, Intel-gfx 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC 0/3] Engine utilization tracking 2017-05-09 14:09 [RFC 0/3] Engine utilization tracking Tvrtko Ursulin ` (2 preceding siblings ...) 2017-05-09 14:09 ` [RFC 3/3] drm/i915: Export engine busy stats in debugfs Tvrtko Ursulin @ 2017-05-09 14:26 ` Chris Wilson 2017-05-09 15:16 ` Tvrtko Ursulin 2017-05-09 14:51 ` ✓ Fi.CI.BAT: success for " Patchwork 4 siblings, 1 reply; 19+ messages in thread From: Chris Wilson @ 2017-05-09 14:26 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Intel-gfx 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC 0/3] Engine utilization tracking 2017-05-09 14:26 ` [RFC 0/3] Engine utilization tracking Chris Wilson @ 2017-05-09 15:16 ` Tvrtko Ursulin 2017-05-09 15:29 ` Chris Wilson 0 siblings, 1 reply; 19+ messages in thread From: Tvrtko Ursulin @ 2017-05-09 15:16 UTC (permalink / raw) To: Chris Wilson, Tvrtko Ursulin, Intel-gfx 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC 0/3] Engine utilization tracking 2017-05-09 15:16 ` Tvrtko Ursulin @ 2017-05-09 15:29 ` Chris Wilson 2017-05-09 15:51 ` Tvrtko Ursulin 0 siblings, 1 reply; 19+ messages in thread From: Chris Wilson @ 2017-05-09 15:29 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Intel-gfx 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC 0/3] Engine utilization tracking 2017-05-09 15:29 ` Chris Wilson @ 2017-05-09 15:51 ` Tvrtko Ursulin 2017-05-09 18:11 ` Dmitry Rogozhkin 2017-05-10 12:31 ` Chris Wilson 0 siblings, 2 replies; 19+ messages in thread From: Tvrtko Ursulin @ 2017-05-09 15:51 UTC (permalink / raw) To: Chris Wilson, Tvrtko Ursulin, Intel-gfx 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC 0/3] Engine utilization tracking 2017-05-09 15:51 ` Tvrtko Ursulin @ 2017-05-09 18:11 ` Dmitry Rogozhkin 2017-05-10 8:38 ` Tvrtko Ursulin 2017-05-10 12:31 ` Chris Wilson 1 sibling, 1 reply; 19+ messages in thread From: Dmitry Rogozhkin @ 2017-05-09 18:11 UTC (permalink / raw) To: Tvrtko Ursulin, Chris Wilson, Tvrtko Ursulin, Intel-gfx 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC 0/3] Engine utilization tracking 2017-05-09 18:11 ` Dmitry Rogozhkin @ 2017-05-10 8:38 ` Tvrtko Ursulin 2017-05-10 15:50 ` Dmitry Rogozhkin 2017-05-10 19:45 ` Daniel Vetter 0 siblings, 2 replies; 19+ messages in thread From: Tvrtko Ursulin @ 2017-05-10 8:38 UTC (permalink / raw) To: Dmitry Rogozhkin, Chris Wilson, Tvrtko Ursulin, Intel-gfx 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC 0/3] Engine utilization tracking 2017-05-10 8:38 ` Tvrtko Ursulin @ 2017-05-10 15:50 ` Dmitry Rogozhkin 2017-05-10 19:45 ` Daniel Vetter 1 sibling, 0 replies; 19+ messages in thread From: Dmitry Rogozhkin @ 2017-05-10 15:50 UTC (permalink / raw) To: Tvrtko Ursulin, Chris Wilson, Tvrtko Ursulin, Intel-gfx 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC 0/3] Engine utilization tracking 2017-05-10 8:38 ` Tvrtko Ursulin 2017-05-10 15:50 ` Dmitry Rogozhkin @ 2017-05-10 19:45 ` Daniel Vetter 2017-05-12 17:40 ` Dmitry Rogozhkin 1 sibling, 1 reply; 19+ messages in thread From: Daniel Vetter @ 2017-05-10 19:45 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC 0/3] Engine utilization tracking 2017-05-10 19:45 ` Daniel Vetter @ 2017-05-12 17:40 ` Dmitry Rogozhkin 0 siblings, 0 replies; 19+ messages in thread From: Dmitry Rogozhkin @ 2017-05-12 17:40 UTC (permalink / raw) To: Daniel Vetter, Tvrtko Ursulin; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC 0/3] Engine utilization tracking 2017-05-09 15:51 ` Tvrtko Ursulin 2017-05-09 18:11 ` Dmitry Rogozhkin @ 2017-05-10 12:31 ` Chris Wilson 1 sibling, 0 replies; 19+ messages in thread From: Chris Wilson @ 2017-05-10 12:31 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Intel-gfx 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* ✓ Fi.CI.BAT: success for Engine utilization tracking 2017-05-09 14:09 [RFC 0/3] Engine utilization tracking Tvrtko Ursulin ` (3 preceding siblings ...) 2017-05-09 14:26 ` [RFC 0/3] Engine utilization tracking Chris Wilson @ 2017-05-09 14:51 ` Patchwork 4 siblings, 0 replies; 19+ messages in thread From: Patchwork @ 2017-05-09 14:51 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: intel-gfx == 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2017-05-12 17:40 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-05-09 14:09 [RFC 0/3] Engine utilization tracking Tvrtko Ursulin 2017-05-09 14:09 ` [RFC 1/3] drm/i915: Wrap context schedule notification Tvrtko Ursulin 2017-05-09 14:09 ` [RFC 2/3] drm/i915: Engine busy time tracking Tvrtko Ursulin 2017-05-10 12:41 ` Joonas Lahtinen 2017-05-09 14:09 ` [RFC 3/3] drm/i915: Export engine busy stats in debugfs Tvrtko Ursulin 2017-05-09 18:17 ` Dmitry Rogozhkin 2017-05-10 8:30 ` Tvrtko Ursulin 2017-05-10 15:57 ` Dmitry Rogozhkin 2017-05-09 14:26 ` [RFC 0/3] Engine utilization tracking Chris Wilson 2017-05-09 15:16 ` Tvrtko Ursulin 2017-05-09 15:29 ` Chris Wilson 2017-05-09 15:51 ` Tvrtko Ursulin 2017-05-09 18:11 ` Dmitry Rogozhkin 2017-05-10 8:38 ` Tvrtko Ursulin 2017-05-10 15:50 ` Dmitry Rogozhkin 2017-05-10 19:45 ` Daniel Vetter 2017-05-12 17:40 ` Dmitry Rogozhkin 2017-05-10 12:31 ` Chris Wilson 2017-05-09 14:51 ` ✓ Fi.CI.BAT: success for " Patchwork
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.