* [PATCH 1/6] drm/i915: Track per-context engine busyness
2018-01-19 13:45 [PATCH v2 0/6] Per-context and per-client engine busyness Tvrtko Ursulin
@ 2018-01-19 13:45 ` Tvrtko Ursulin
2018-01-19 16:26 ` [PATCH v5 " Tvrtko Ursulin
2018-01-19 13:45 ` [PATCH 2/6] drm/i915: Allow clients to query own per-engine busyness Tvrtko Ursulin
` (6 subsequent siblings)
7 siblings, 1 reply; 18+ messages in thread
From: Tvrtko Ursulin @ 2018-01-19 13:45 UTC (permalink / raw)
To: Intel-gfx; +Cc: gordon.kelly
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Some customers want to know how much of the GPU time are their clients
using in order to make dynamic load balancing decisions.
With the hooks already in place which track the overall engine busyness,
we can extend that slightly to split that time between contexts.
v2: Fix accounting for tail updates.
v3: Rebase.
v4: Mark currently running contexts as active on stats enable.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: gordon.kelly@intel.com
---
drivers/gpu/drm/i915/i915_gem_context.h | 5 ++++
drivers/gpu/drm/i915/intel_engine_cs.c | 32 +++++++++++++++++++++
drivers/gpu/drm/i915/intel_lrc.c | 14 +++++----
drivers/gpu/drm/i915/intel_ringbuffer.h | 50 +++++++++++++++++++++++++++++----
4 files changed, 90 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
index 4bfb72f8e1cb..0ce8b9bf0f32 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -157,6 +157,11 @@ struct i915_gem_context {
u32 *lrc_reg_state;
u64 lrc_desc;
int pin_count;
+ struct {
+ bool active;
+ ktime_t start;
+ ktime_t total;
+ } stats;
} engine[I915_NUM_ENGINES];
/** ring_size: size for allocating the per-engine ring buffer */
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index d572b18d39eb..9907ceedfa90 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -1966,6 +1966,16 @@ int intel_enable_engine_stats(struct intel_engine_cs *engine)
engine->stats.enabled_at = ktime_get();
+ /* Mark currently running context as active. */
+ if (port_isset(port)) {
+ struct drm_i915_gem_request *req = port_request(port);
+ struct intel_context *ce =
+ &req->ctx->engine[engine->id];
+
+ ce->stats.start = engine->stats.enabled_at;
+ ce->stats.active = true;
+ }
+
/* XXX submission method oblivious? */
while (num_ports-- && port_isset(port)) {
engine->stats.active++;
@@ -2038,6 +2048,28 @@ void intel_disable_engine_stats(struct intel_engine_cs *engine)
spin_unlock_irqrestore(&engine->stats.lock, flags);
}
+ktime_t intel_context_engine_get_busy_time(struct i915_gem_context *ctx,
+ struct intel_engine_cs *engine)
+{
+ struct intel_context *ce;
+ unsigned long flags;
+ ktime_t total;
+
+ ce = &ctx->engine[engine->id];
+
+ spin_lock_irqsave(&engine->stats.lock, flags);
+
+ total = ce->stats.total;
+
+ if (ce->stats.active)
+ total = ktime_add(total,
+ ktime_sub(ktime_get(), ce->stats.start));
+
+ spin_unlock_irqrestore(&engine->stats.lock, flags);
+
+ return total;
+}
+
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
#include "selftests/mock_engine.c"
#endif
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 24ce781d39b7..a82ad5da6090 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -380,16 +380,19 @@ execlists_context_status_change(struct drm_i915_gem_request *rq,
}
static inline void
-execlists_context_schedule_in(struct drm_i915_gem_request *rq)
+execlists_context_schedule_in(struct drm_i915_gem_request *rq,
+ unsigned int port)
{
execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN);
- intel_engine_context_in(rq->engine);
+ intel_engine_context_in(rq->engine,
+ &rq->ctx->engine[rq->engine->id],
+ port == 0);
}
static inline void
execlists_context_schedule_out(struct drm_i915_gem_request *rq)
{
- intel_engine_context_out(rq->engine);
+ intel_engine_context_out(rq->engine, &rq->ctx->engine[rq->engine->id]);
execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
}
@@ -442,7 +445,7 @@ static void execlists_submit_ports(struct intel_engine_cs *engine)
if (rq) {
GEM_BUG_ON(count > !n);
if (!count++)
- execlists_context_schedule_in(rq);
+ execlists_context_schedule_in(rq, n);
port_set(&port[n], port_pack(rq, count));
desc = execlists_update_context(rq);
GEM_DEBUG_EXEC(port[n].context_id = upper_32_bits(desc));
@@ -703,7 +706,8 @@ execlists_cancel_port_requests(struct intel_engine_execlists * const execlists)
struct drm_i915_gem_request *rq = port_request(port);
GEM_BUG_ON(!execlists->active);
- intel_engine_context_out(rq->engine);
+ intel_engine_context_out(rq->engine,
+ &rq->ctx->engine[rq->engine->id]);
execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_PREEMPTED);
i915_gem_request_put(rq);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index b198df1f248c..27b727cf4017 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -4,6 +4,7 @@
#include <linux/hashtable.h>
#include "i915_gem_batch_pool.h"
+#include "i915_gem_context.h"
#include "i915_gem_request.h"
#include "i915_gem_timeline.h"
#include "i915_pmu.h"
@@ -1036,25 +1037,42 @@ void intel_engine_dump(struct intel_engine_cs *engine,
struct intel_engine_cs *
intel_engine_lookup_user(struct drm_i915_private *i915, u8 class, u8 instance);
-static inline void intel_engine_context_in(struct intel_engine_cs *engine)
+static inline void
+intel_engine_context_in(struct intel_engine_cs *engine,
+ struct intel_context *ce,
+ bool submit)
{
unsigned long flags;
+ ktime_t now;
if (READ_ONCE(engine->stats.enabled) == 0)
return;
spin_lock_irqsave(&engine->stats.lock, flags);
+ if (submit) {
+ now = ktime_get();
+ ce->stats.start = now;
+ ce->stats.active = true;
+ } else {
+ now = 0;
+ }
+
if (engine->stats.enabled > 0) {
- if (engine->stats.active++ == 0)
- engine->stats.start = ktime_get();
+ if (engine->stats.active++ == 0) {
+ if (!now)
+ now = ktime_get();
+ engine->stats.start = now;
+ }
GEM_BUG_ON(engine->stats.active == 0);
}
spin_unlock_irqrestore(&engine->stats.lock, flags);
}
-static inline void intel_engine_context_out(struct intel_engine_cs *engine)
+static inline void
+intel_engine_context_out(struct intel_engine_cs *engine,
+ struct intel_context *ce)
{
unsigned long flags;
@@ -1064,14 +1082,31 @@ static inline void intel_engine_context_out(struct intel_engine_cs *engine)
spin_lock_irqsave(&engine->stats.lock, flags);
if (engine->stats.enabled > 0) {
+ struct execlist_port *next_port = &engine->execlists.port[1];
+ ktime_t now = ktime_get();
ktime_t last;
+ GEM_BUG_ON(!ce->stats.start);
+ ce->stats.total = ktime_add(ce->stats.total,
+ ktime_sub(now, ce->stats.start));
+ ce->stats.active = false;
+
+ if (port_isset(next_port)) {
+ struct drm_i915_gem_request *next_req =
+ port_request(next_port);
+ struct intel_context *next_ce =
+ &next_req->ctx->engine[engine->id];
+
+ next_ce->stats.start = now;
+ next_ce->stats.active = true;
+ }
+
if (engine->stats.active && --engine->stats.active == 0) {
/*
* Decrement the active context count and in case GPU
* is now idle add up to the running total.
*/
- last = ktime_sub(ktime_get(), engine->stats.start);
+ last = ktime_sub(now, engine->stats.start);
engine->stats.total = ktime_add(engine->stats.total,
last);
@@ -1081,7 +1116,7 @@ static inline void intel_engine_context_out(struct intel_engine_cs *engine)
* the first event in which case we account from the
* time stats gathering was turned on.
*/
- last = ktime_sub(ktime_get(), engine->stats.enabled_at);
+ last = ktime_sub(now, engine->stats.enabled_at);
engine->stats.total = ktime_add(engine->stats.total,
last);
@@ -1091,6 +1126,9 @@ static inline void intel_engine_context_out(struct intel_engine_cs *engine)
spin_unlock_irqrestore(&engine->stats.lock, flags);
}
+ktime_t intel_context_engine_get_busy_time(struct i915_gem_context *ctx,
+ struct intel_engine_cs *engine);
+
int intel_enable_engine_stats(struct intel_engine_cs *engine);
void intel_disable_engine_stats(struct intel_engine_cs *engine);
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v5 1/6] drm/i915: Track per-context engine busyness
2018-01-19 13:45 ` [PATCH 1/6] drm/i915: Track per-context " Tvrtko Ursulin
@ 2018-01-19 16:26 ` Tvrtko Ursulin
2018-01-19 21:02 ` Chris Wilson
0 siblings, 1 reply; 18+ messages in thread
From: Tvrtko Ursulin @ 2018-01-19 16:26 UTC (permalink / raw)
To: Intel-gfx; +Cc: gordon.kelly
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Some customers want to know how much of the GPU time are their clients
using in order to make dynamic load balancing decisions.
With the hooks already in place which track the overall engine busyness,
we can extend that slightly to split that time between contexts.
v2: Fix accounting for tail updates.
v3: Rebase.
v4: Mark currently running contexts as active on stats enable.
v5: Include some headers to fix the build.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: gordon.kelly@intel.com
---
drivers/gpu/drm/i915/i915_gem_context.h | 8 ++++++
drivers/gpu/drm/i915/intel_engine_cs.c | 32 +++++++++++++++++++++
drivers/gpu/drm/i915/intel_lrc.c | 14 +++++----
drivers/gpu/drm/i915/intel_ringbuffer.h | 50 +++++++++++++++++++++++++++++----
4 files changed, 93 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
index 4bfb72f8e1cb..7f5eebb67167 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -29,6 +29,9 @@
#include <linux/list.h>
#include <linux/radix-tree.h>
+#include "i915_gem.h"
+#include "i915_gem_request.h"
+
struct pid;
struct drm_device;
@@ -157,6 +160,11 @@ struct i915_gem_context {
u32 *lrc_reg_state;
u64 lrc_desc;
int pin_count;
+ struct {
+ bool active;
+ ktime_t start;
+ ktime_t total;
+ } stats;
} engine[I915_NUM_ENGINES];
/** ring_size: size for allocating the per-engine ring buffer */
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index d572b18d39eb..9907ceedfa90 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -1966,6 +1966,16 @@ int intel_enable_engine_stats(struct intel_engine_cs *engine)
engine->stats.enabled_at = ktime_get();
+ /* Mark currently running context as active. */
+ if (port_isset(port)) {
+ struct drm_i915_gem_request *req = port_request(port);
+ struct intel_context *ce =
+ &req->ctx->engine[engine->id];
+
+ ce->stats.start = engine->stats.enabled_at;
+ ce->stats.active = true;
+ }
+
/* XXX submission method oblivious? */
while (num_ports-- && port_isset(port)) {
engine->stats.active++;
@@ -2038,6 +2048,28 @@ void intel_disable_engine_stats(struct intel_engine_cs *engine)
spin_unlock_irqrestore(&engine->stats.lock, flags);
}
+ktime_t intel_context_engine_get_busy_time(struct i915_gem_context *ctx,
+ struct intel_engine_cs *engine)
+{
+ struct intel_context *ce;
+ unsigned long flags;
+ ktime_t total;
+
+ ce = &ctx->engine[engine->id];
+
+ spin_lock_irqsave(&engine->stats.lock, flags);
+
+ total = ce->stats.total;
+
+ if (ce->stats.active)
+ total = ktime_add(total,
+ ktime_sub(ktime_get(), ce->stats.start));
+
+ spin_unlock_irqrestore(&engine->stats.lock, flags);
+
+ return total;
+}
+
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
#include "selftests/mock_engine.c"
#endif
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 24ce781d39b7..a82ad5da6090 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -380,16 +380,19 @@ execlists_context_status_change(struct drm_i915_gem_request *rq,
}
static inline void
-execlists_context_schedule_in(struct drm_i915_gem_request *rq)
+execlists_context_schedule_in(struct drm_i915_gem_request *rq,
+ unsigned int port)
{
execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN);
- intel_engine_context_in(rq->engine);
+ intel_engine_context_in(rq->engine,
+ &rq->ctx->engine[rq->engine->id],
+ port == 0);
}
static inline void
execlists_context_schedule_out(struct drm_i915_gem_request *rq)
{
- intel_engine_context_out(rq->engine);
+ intel_engine_context_out(rq->engine, &rq->ctx->engine[rq->engine->id]);
execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
}
@@ -442,7 +445,7 @@ static void execlists_submit_ports(struct intel_engine_cs *engine)
if (rq) {
GEM_BUG_ON(count > !n);
if (!count++)
- execlists_context_schedule_in(rq);
+ execlists_context_schedule_in(rq, n);
port_set(&port[n], port_pack(rq, count));
desc = execlists_update_context(rq);
GEM_DEBUG_EXEC(port[n].context_id = upper_32_bits(desc));
@@ -703,7 +706,8 @@ execlists_cancel_port_requests(struct intel_engine_execlists * const execlists)
struct drm_i915_gem_request *rq = port_request(port);
GEM_BUG_ON(!execlists->active);
- intel_engine_context_out(rq->engine);
+ intel_engine_context_out(rq->engine,
+ &rq->ctx->engine[rq->engine->id]);
execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_PREEMPTED);
i915_gem_request_put(rq);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index b198df1f248c..27b727cf4017 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -4,6 +4,7 @@
#include <linux/hashtable.h>
#include "i915_gem_batch_pool.h"
+#include "i915_gem_context.h"
#include "i915_gem_request.h"
#include "i915_gem_timeline.h"
#include "i915_pmu.h"
@@ -1036,25 +1037,42 @@ void intel_engine_dump(struct intel_engine_cs *engine,
struct intel_engine_cs *
intel_engine_lookup_user(struct drm_i915_private *i915, u8 class, u8 instance);
-static inline void intel_engine_context_in(struct intel_engine_cs *engine)
+static inline void
+intel_engine_context_in(struct intel_engine_cs *engine,
+ struct intel_context *ce,
+ bool submit)
{
unsigned long flags;
+ ktime_t now;
if (READ_ONCE(engine->stats.enabled) == 0)
return;
spin_lock_irqsave(&engine->stats.lock, flags);
+ if (submit) {
+ now = ktime_get();
+ ce->stats.start = now;
+ ce->stats.active = true;
+ } else {
+ now = 0;
+ }
+
if (engine->stats.enabled > 0) {
- if (engine->stats.active++ == 0)
- engine->stats.start = ktime_get();
+ if (engine->stats.active++ == 0) {
+ if (!now)
+ now = ktime_get();
+ engine->stats.start = now;
+ }
GEM_BUG_ON(engine->stats.active == 0);
}
spin_unlock_irqrestore(&engine->stats.lock, flags);
}
-static inline void intel_engine_context_out(struct intel_engine_cs *engine)
+static inline void
+intel_engine_context_out(struct intel_engine_cs *engine,
+ struct intel_context *ce)
{
unsigned long flags;
@@ -1064,14 +1082,31 @@ static inline void intel_engine_context_out(struct intel_engine_cs *engine)
spin_lock_irqsave(&engine->stats.lock, flags);
if (engine->stats.enabled > 0) {
+ struct execlist_port *next_port = &engine->execlists.port[1];
+ ktime_t now = ktime_get();
ktime_t last;
+ GEM_BUG_ON(!ce->stats.start);
+ ce->stats.total = ktime_add(ce->stats.total,
+ ktime_sub(now, ce->stats.start));
+ ce->stats.active = false;
+
+ if (port_isset(next_port)) {
+ struct drm_i915_gem_request *next_req =
+ port_request(next_port);
+ struct intel_context *next_ce =
+ &next_req->ctx->engine[engine->id];
+
+ next_ce->stats.start = now;
+ next_ce->stats.active = true;
+ }
+
if (engine->stats.active && --engine->stats.active == 0) {
/*
* Decrement the active context count and in case GPU
* is now idle add up to the running total.
*/
- last = ktime_sub(ktime_get(), engine->stats.start);
+ last = ktime_sub(now, engine->stats.start);
engine->stats.total = ktime_add(engine->stats.total,
last);
@@ -1081,7 +1116,7 @@ static inline void intel_engine_context_out(struct intel_engine_cs *engine)
* the first event in which case we account from the
* time stats gathering was turned on.
*/
- last = ktime_sub(ktime_get(), engine->stats.enabled_at);
+ last = ktime_sub(now, engine->stats.enabled_at);
engine->stats.total = ktime_add(engine->stats.total,
last);
@@ -1091,6 +1126,9 @@ static inline void intel_engine_context_out(struct intel_engine_cs *engine)
spin_unlock_irqrestore(&engine->stats.lock, flags);
}
+ktime_t intel_context_engine_get_busy_time(struct i915_gem_context *ctx,
+ struct intel_engine_cs *engine);
+
int intel_enable_engine_stats(struct intel_engine_cs *engine);
void intel_disable_engine_stats(struct intel_engine_cs *engine);
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v5 1/6] drm/i915: Track per-context engine busyness
2018-01-19 16:26 ` [PATCH v5 " Tvrtko Ursulin
@ 2018-01-19 21:02 ` Chris Wilson
0 siblings, 0 replies; 18+ messages in thread
From: Chris Wilson @ 2018-01-19 21:02 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx; +Cc: gordon.kelly
Quoting Tvrtko Ursulin (2018-01-19 16:26:16)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Some customers want to know how much of the GPU time are their clients
> using in order to make dynamic load balancing decisions.
>
> With the hooks already in place which track the overall engine busyness,
> we can extend that slightly to split that time between contexts.
>
> v2: Fix accounting for tail updates.
> v3: Rebase.
> v4: Mark currently running contexts as active on stats enable.
> v5: Include some headers to fix the build.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: gordon.kelly@intel.com
> ---
> drivers/gpu/drm/i915/i915_gem_context.h | 8 ++++++
> drivers/gpu/drm/i915/intel_engine_cs.c | 32 +++++++++++++++++++++
> drivers/gpu/drm/i915/intel_lrc.c | 14 +++++----
> drivers/gpu/drm/i915/intel_ringbuffer.h | 50 +++++++++++++++++++++++++++++----
> 4 files changed, 93 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
> index 4bfb72f8e1cb..7f5eebb67167 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.h
> +++ b/drivers/gpu/drm/i915/i915_gem_context.h
> @@ -29,6 +29,9 @@
> #include <linux/list.h>
> #include <linux/radix-tree.h>
>
> +#include "i915_gem.h"
> +#include "i915_gem_request.h"
Yup, we need a patch for tip for
#include "i915_gem.h"
struct drm_i915_gem_request;
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/6] drm/i915: Allow clients to query own per-engine busyness
2018-01-19 13:45 [PATCH v2 0/6] Per-context and per-client engine busyness Tvrtko Ursulin
2018-01-19 13:45 ` [PATCH 1/6] drm/i915: Track per-context " Tvrtko Ursulin
@ 2018-01-19 13:45 ` Tvrtko Ursulin
2018-01-19 21:08 ` Chris Wilson
2018-01-19 13:45 ` [PATCH 3/6] drm/i915: Expose list of clients in sysfs Tvrtko Ursulin
` (5 subsequent siblings)
7 siblings, 1 reply; 18+ messages in thread
From: Tvrtko Ursulin @ 2018-01-19 13:45 UTC (permalink / raw)
To: Intel-gfx; +Cc: gordon.kelly
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Some customers want to know how much of the GPU time are their clients
using in order to make dynamic load balancing decisions.
With the accounting infrastructure in place in the previous patch, we add
a new context param (I915_CONTEXT_GET_ENGINE_BUSY) which takes a class and
instance of an engine for which the client would like to know its
utilization.
Utilization is reported as monotonically increasing number of nano-
seconds the engine spent executing jobs belonging to this context.
v2:
* Use intel_context_engine_get_busy_time.
* Refactor to only use struct_mutex while initially enabling engine
stats.
v3:
* Fix stats enabling.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: gordon.kelly@intel.com
---
drivers/gpu/drm/i915/i915_gem_context.c | 41 ++++++++++++++++++++++++++++++---
drivers/gpu/drm/i915/i915_gem_context.h | 1 +
include/uapi/drm/i915_drm.h | 11 ++++++++-
3 files changed, 49 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 648e7536ff51..256800fb1a3b 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -126,6 +126,9 @@ static void i915_gem_context_free(struct i915_gem_context *ctx)
for (i = 0; i < I915_NUM_ENGINES; i++) {
struct intel_context *ce = &ctx->engine[i];
+ if (ce->stats.enabled)
+ intel_disable_engine_stats(ctx->i915->engine[i]);
+
if (!ce->state)
continue;
@@ -713,7 +716,10 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
{
struct drm_i915_file_private *file_priv = file->driver_priv;
struct drm_i915_gem_context_param *args = data;
+ struct drm_i915_private *i915 = to_i915(dev);
+ struct intel_engine_cs *engine;
struct i915_gem_context *ctx;
+ struct intel_context *ce;
int ret = 0;
ctx = i915_gem_context_lookup(file_priv, args->ctx_id);
@@ -731,10 +737,10 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
case I915_CONTEXT_PARAM_GTT_SIZE:
if (ctx->ppgtt)
args->value = ctx->ppgtt->base.total;
- else if (to_i915(dev)->mm.aliasing_ppgtt)
- args->value = to_i915(dev)->mm.aliasing_ppgtt->base.total;
+ else if (i915->mm.aliasing_ppgtt)
+ args->value = i915->mm.aliasing_ppgtt->base.total;
else
- args->value = to_i915(dev)->ggtt.base.total;
+ args->value = i915->ggtt.base.total;
break;
case I915_CONTEXT_PARAM_NO_ERROR_CAPTURE:
args->value = i915_gem_context_no_error_capture(ctx);
@@ -745,6 +751,34 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
case I915_CONTEXT_PARAM_PRIORITY:
args->value = ctx->priority;
break;
+ case I915_CONTEXT_GET_ENGINE_BUSY:
+ engine = intel_engine_lookup_user(i915, args->class,
+ args->instance);
+ if (!engine) {
+ ret = -EINVAL;
+ break;
+ }
+
+ ce = &ctx->engine[engine->id];
+ if (!READ_ONCE(ce->stats.enabled)) {
+ ret = i915_mutex_lock_interruptible(dev);
+ if (!ret)
+ break;
+
+ if (!ce->stats.enabled) {
+ ret = intel_enable_engine_stats(engine);
+ if (!ret)
+ break;
+ ce->stats.enabled = true;
+ }
+
+ mutex_unlock(&dev->struct_mutex);
+ }
+
+ args->value =
+ ktime_to_ns(intel_context_engine_get_busy_time(ctx,
+ engine));
+ break;
default:
ret = -EINVAL;
break;
@@ -820,6 +854,7 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
}
break;
+ case I915_CONTEXT_GET_ENGINE_BUSY:
default:
ret = -EINVAL;
break;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
index 0ce8b9bf0f32..2312967a5890 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -158,6 +158,7 @@ struct i915_gem_context {
u64 lrc_desc;
int pin_count;
struct {
+ bool enabled;
bool active;
ktime_t start;
ktime_t total;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index c3b98fb8b691..642a7c15e9d9 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1468,7 +1468,16 @@ struct drm_i915_gem_context_param {
#define I915_CONTEXT_MAX_USER_PRIORITY 1023 /* inclusive */
#define I915_CONTEXT_DEFAULT_PRIORITY 0
#define I915_CONTEXT_MIN_USER_PRIORITY -1023 /* inclusive */
- __u64 value;
+#define I915_CONTEXT_GET_ENGINE_BUSY 0x7
+ union {
+ __u64 value;
+ struct {
+ __u8 pad[6]; /* unused */
+
+ __u8 class;
+ __u8 instance;
+ };
+ };
};
enum drm_i915_oa_format {
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 2/6] drm/i915: Allow clients to query own per-engine busyness
2018-01-19 13:45 ` [PATCH 2/6] drm/i915: Allow clients to query own per-engine busyness Tvrtko Ursulin
@ 2018-01-19 21:08 ` Chris Wilson
2018-01-22 9:53 ` Tvrtko Ursulin
0 siblings, 1 reply; 18+ messages in thread
From: Chris Wilson @ 2018-01-19 21:08 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx; +Cc: gordon.kelly
Quoting Tvrtko Ursulin (2018-01-19 13:45:24)
> + case I915_CONTEXT_GET_ENGINE_BUSY:
> + engine = intel_engine_lookup_user(i915, args->class,
> + args->instance);
> + if (!engine) {
> + ret = -EINVAL;
> + break;
> + }
> +
> + ce = &ctx->engine[engine->id];
> + if (!READ_ONCE(ce->stats.enabled)) {
> + ret = i915_mutex_lock_interruptible(dev);
> + if (!ret)
> + break;
> +
> + if (!ce->stats.enabled) {
> + ret = intel_enable_engine_stats(engine);
* Blink.
This caught me by surprise. (Other than struct_mutex) Not too offensive,
but surprising. At the very least call out to a function to handle the
request. Where did args->class, args->instance come from? You surely
didn't extend the ioctl struct just for that?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 2/6] drm/i915: Allow clients to query own per-engine busyness
2018-01-19 21:08 ` Chris Wilson
@ 2018-01-22 9:53 ` Tvrtko Ursulin
2018-01-22 10:00 ` Chris Wilson
0 siblings, 1 reply; 18+ messages in thread
From: Tvrtko Ursulin @ 2018-01-22 9:53 UTC (permalink / raw)
To: Chris Wilson, Tvrtko Ursulin, Intel-gfx; +Cc: gordon.kelly
On 19/01/2018 21:08, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-01-19 13:45:24)
>> + case I915_CONTEXT_GET_ENGINE_BUSY:
>> + engine = intel_engine_lookup_user(i915, args->class,
>> + args->instance);
>> + if (!engine) {
>> + ret = -EINVAL;
>> + break;
>> + }
>> +
>> + ce = &ctx->engine[engine->id];
>> + if (!READ_ONCE(ce->stats.enabled)) {
>> + ret = i915_mutex_lock_interruptible(dev);
>> + if (!ret)
>> + break;
>> +
>> + if (!ce->stats.enabled) {
>> + ret = intel_enable_engine_stats(engine);
>
> * Blink.
>
> This caught me by surprise. (Other than struct_mutex) Not too offensive,
> but surprising. At the very least call out to a function to handle the
> request. Where did args->class, args->instance come from? You surely
> didn't extend the ioctl struct just for that?
Haven't extended it no, just did this:
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1468,7 +1468,16 @@ struct drm_i915_gem_context_param {
#define I915_CONTEXT_MAX_USER_PRIORITY 1023 /* inclusive */
#define I915_CONTEXT_DEFAULT_PRIORITY 0
#define I915_CONTEXT_MIN_USER_PRIORITY -1023 /* inclusive */
- __u64 value;
+#define I915_CONTEXT_GET_ENGINE_BUSY 0x7
+ union {
+ __u64 value;
+ struct {
+ __u8 pad[6]; /* unused */
+
+ __u8 class;
+ __u8 instance;
+ };
+ };
};
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 2/6] drm/i915: Allow clients to query own per-engine busyness
2018-01-22 9:53 ` Tvrtko Ursulin
@ 2018-01-22 10:00 ` Chris Wilson
2018-01-22 11:45 ` Tvrtko Ursulin
0 siblings, 1 reply; 18+ messages in thread
From: Chris Wilson @ 2018-01-22 10:00 UTC (permalink / raw)
To: Tvrtko Ursulin, Tvrtko Ursulin, Intel-gfx; +Cc: gordon.kelly
Quoting Tvrtko Ursulin (2018-01-22 09:53:27)
>
> On 19/01/2018 21:08, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2018-01-19 13:45:24)
> >> + case I915_CONTEXT_GET_ENGINE_BUSY:
> >> + engine = intel_engine_lookup_user(i915, args->class,
> >> + args->instance);
> >> + if (!engine) {
> >> + ret = -EINVAL;
> >> + break;
> >> + }
> >> +
> >> + ce = &ctx->engine[engine->id];
> >> + if (!READ_ONCE(ce->stats.enabled)) {
> >> + ret = i915_mutex_lock_interruptible(dev);
> >> + if (!ret)
> >> + break;
> >> +
> >> + if (!ce->stats.enabled) {
> >> + ret = intel_enable_engine_stats(engine);
> >
> > * Blink.
> >
> > This caught me by surprise. (Other than struct_mutex) Not too offensive,
> > but surprising. At the very least call out to a function to handle the
> > request. Where did args->class, args->instance come from? You surely
> > didn't extend the ioctl struct just for that?
>
> Haven't extended it no, just did this:
>
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -1468,7 +1468,16 @@ struct drm_i915_gem_context_param {
> #define I915_CONTEXT_MAX_USER_PRIORITY 1023 /* inclusive */
> #define I915_CONTEXT_DEFAULT_PRIORITY 0
> #define I915_CONTEXT_MIN_USER_PRIORITY -1023 /* inclusive */
> - __u64 value;
> +#define I915_CONTEXT_GET_ENGINE_BUSY 0x7
> + union {
> + __u64 value;
> + struct {
> + __u8 pad[6]; /* unused */
> +
> + __u8 class;
> + __u8 instance;
> + };
> + };
> };
Not entirely happy about mixing in/out parameters. It's already
complicated by being either an out value or an out pointer.
Closer to the original idea for context_getparam would be to return the
array of engine values.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 2/6] drm/i915: Allow clients to query own per-engine busyness
2018-01-22 10:00 ` Chris Wilson
@ 2018-01-22 11:45 ` Tvrtko Ursulin
2018-01-22 12:32 ` Chris Wilson
0 siblings, 1 reply; 18+ messages in thread
From: Tvrtko Ursulin @ 2018-01-22 11:45 UTC (permalink / raw)
To: Chris Wilson, Tvrtko Ursulin, Intel-gfx; +Cc: gordon.kelly
On 22/01/2018 10:00, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-01-22 09:53:27)
>>
>> On 19/01/2018 21:08, Chris Wilson wrote:
>>> Quoting Tvrtko Ursulin (2018-01-19 13:45:24)
>>>> + case I915_CONTEXT_GET_ENGINE_BUSY:
>>>> + engine = intel_engine_lookup_user(i915, args->class,
>>>> + args->instance);
>>>> + if (!engine) {
>>>> + ret = -EINVAL;
>>>> + break;
>>>> + }
>>>> +
>>>> + ce = &ctx->engine[engine->id];
>>>> + if (!READ_ONCE(ce->stats.enabled)) {
>>>> + ret = i915_mutex_lock_interruptible(dev);
>>>> + if (!ret)
>>>> + break;
>>>> +
>>>> + if (!ce->stats.enabled) {
>>>> + ret = intel_enable_engine_stats(engine);
>>>
>>> * Blink.
>>>
>>> This caught me by surprise. (Other than struct_mutex) Not too offensive,
>>> but surprising. At the very least call out to a function to handle the
>>> request. Where did args->class, args->instance come from? You surely
>>> didn't extend the ioctl struct just for that?
>>
>> Haven't extended it no, just did this:
>>
>> --- a/include/uapi/drm/i915_drm.h
>> +++ b/include/uapi/drm/i915_drm.h
>> @@ -1468,7 +1468,16 @@ struct drm_i915_gem_context_param {
>> #define I915_CONTEXT_MAX_USER_PRIORITY 1023 /* inclusive */
>> #define I915_CONTEXT_DEFAULT_PRIORITY 0
>> #define I915_CONTEXT_MIN_USER_PRIORITY -1023 /* inclusive */
>> - __u64 value;
>> +#define I915_CONTEXT_GET_ENGINE_BUSY 0x7
>> + union {
>> + __u64 value;
>> + struct {
>> + __u8 pad[6]; /* unused */
>> +
>> + __u8 class;
>> + __u8 instance;
>> + };
>> + };
>> };
>
> Not entirely happy about mixing in/out parameters. It's already
> complicated by being either an out value or an out pointer.
>
> Closer to the original idea for context_getparam would be to return the
> array of engine values.
It would have the advantage that multiple engines could be queried
(more) atomically. How about then:
I915_CONTEXT_ENABLE_ENGINE_STATS:
value = &{
__u32 num_engines;
__u32 pad;
struct {
__u8 class;
__u8 instance;
__u8 pad[6];
} [num_engines];
};
I915_CONTEXT_GET_ENGINE_STATS:
value = &{
__u32 num_engines;
__u32 pad;
struct {
__u8 class;
__u8 instance;
__u8 pad[6];
__u64 busy;
} [num_engines];
};
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 2/6] drm/i915: Allow clients to query own per-engine busyness
2018-01-22 11:45 ` Tvrtko Ursulin
@ 2018-01-22 12:32 ` Chris Wilson
2018-01-22 17:11 ` Tvrtko Ursulin
0 siblings, 1 reply; 18+ messages in thread
From: Chris Wilson @ 2018-01-22 12:32 UTC (permalink / raw)
To: Tvrtko Ursulin, Tvrtko Ursulin, Intel-gfx; +Cc: gordon.kelly
Quoting Tvrtko Ursulin (2018-01-22 11:45:04)
>
> On 22/01/2018 10:00, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2018-01-22 09:53:27)
> >>
> >> On 19/01/2018 21:08, Chris Wilson wrote:
> >>> Quoting Tvrtko Ursulin (2018-01-19 13:45:24)
> >>>> + case I915_CONTEXT_GET_ENGINE_BUSY:
> >>>> + engine = intel_engine_lookup_user(i915, args->class,
> >>>> + args->instance);
> >>>> + if (!engine) {
> >>>> + ret = -EINVAL;
> >>>> + break;
> >>>> + }
> >>>> +
> >>>> + ce = &ctx->engine[engine->id];
> >>>> + if (!READ_ONCE(ce->stats.enabled)) {
> >>>> + ret = i915_mutex_lock_interruptible(dev);
> >>>> + if (!ret)
> >>>> + break;
> >>>> +
> >>>> + if (!ce->stats.enabled) {
> >>>> + ret = intel_enable_engine_stats(engine);
> >>>
> >>> * Blink.
> >>>
> >>> This caught me by surprise. (Other than struct_mutex) Not too offensive,
> >>> but surprising. At the very least call out to a function to handle the
> >>> request. Where did args->class, args->instance come from? You surely
> >>> didn't extend the ioctl struct just for that?
> >>
> >> Haven't extended it no, just did this:
> >>
> >> --- a/include/uapi/drm/i915_drm.h
> >> +++ b/include/uapi/drm/i915_drm.h
> >> @@ -1468,7 +1468,16 @@ struct drm_i915_gem_context_param {
> >> #define I915_CONTEXT_MAX_USER_PRIORITY 1023 /* inclusive */
> >> #define I915_CONTEXT_DEFAULT_PRIORITY 0
> >> #define I915_CONTEXT_MIN_USER_PRIORITY -1023 /* inclusive */
> >> - __u64 value;
> >> +#define I915_CONTEXT_GET_ENGINE_BUSY 0x7
> >> + union {
> >> + __u64 value;
> >> + struct {
> >> + __u8 pad[6]; /* unused */
> >> +
> >> + __u8 class;
> >> + __u8 instance;
> >> + };
> >> + };
> >> };
> >
> > Not entirely happy about mixing in/out parameters. It's already
> > complicated by being either an out value or an out pointer.
> >
> > Closer to the original idea for context_getparam would be to return the
> > array of engine values.
>
> It would have the advantage that multiple engines could be queried
> (more) atomically. How about then:
>
> I915_CONTEXT_ENABLE_ENGINE_STATS:
> value = &{
> __u32 num_engines;
> __u32 pad;
>
> struct {
> __u8 class;
> __u8 instance;
> __u8 pad[6];
> } [num_engines];
> };
>
> I915_CONTEXT_GET_ENGINE_STATS:
> value = &{
> __u32 num_engines;
> __u32 pad;
>
> struct {
> __u8 class;
> __u8 instance;
> __u8 pad[6];
>
> __u64 busy;
> } [num_engines];
> };
Yes, that's what I had in mind. How does that work in practice? Does it
make userspace too clumsy?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 2/6] drm/i915: Allow clients to query own per-engine busyness
2018-01-22 12:32 ` Chris Wilson
@ 2018-01-22 17:11 ` Tvrtko Ursulin
0 siblings, 0 replies; 18+ messages in thread
From: Tvrtko Ursulin @ 2018-01-22 17:11 UTC (permalink / raw)
To: Chris Wilson, Tvrtko Ursulin, Intel-gfx; +Cc: gordon.kelly
On 22/01/2018 12:32, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-01-22 11:45:04)
>>
>> On 22/01/2018 10:00, Chris Wilson wrote:
>>> Quoting Tvrtko Ursulin (2018-01-22 09:53:27)
>>>>
>>>> On 19/01/2018 21:08, Chris Wilson wrote:
>>>>> Quoting Tvrtko Ursulin (2018-01-19 13:45:24)
>>>>>> + case I915_CONTEXT_GET_ENGINE_BUSY:
>>>>>> + engine = intel_engine_lookup_user(i915, args->class,
>>>>>> + args->instance);
>>>>>> + if (!engine) {
>>>>>> + ret = -EINVAL;
>>>>>> + break;
>>>>>> + }
>>>>>> +
>>>>>> + ce = &ctx->engine[engine->id];
>>>>>> + if (!READ_ONCE(ce->stats.enabled)) {
>>>>>> + ret = i915_mutex_lock_interruptible(dev);
>>>>>> + if (!ret)
>>>>>> + break;
>>>>>> +
>>>>>> + if (!ce->stats.enabled) {
>>>>>> + ret = intel_enable_engine_stats(engine);
>>>>>
>>>>> * Blink.
>>>>>
>>>>> This caught me by surprise. (Other than struct_mutex) Not too offensive,
>>>>> but surprising. At the very least call out to a function to handle the
>>>>> request. Where did args->class, args->instance come from? You surely
>>>>> didn't extend the ioctl struct just for that?
>>>>
>>>> Haven't extended it no, just did this:
>>>>
>>>> --- a/include/uapi/drm/i915_drm.h
>>>> +++ b/include/uapi/drm/i915_drm.h
>>>> @@ -1468,7 +1468,16 @@ struct drm_i915_gem_context_param {
>>>> #define I915_CONTEXT_MAX_USER_PRIORITY 1023 /* inclusive */
>>>> #define I915_CONTEXT_DEFAULT_PRIORITY 0
>>>> #define I915_CONTEXT_MIN_USER_PRIORITY -1023 /* inclusive */
>>>> - __u64 value;
>>>> +#define I915_CONTEXT_GET_ENGINE_BUSY 0x7
>>>> + union {
>>>> + __u64 value;
>>>> + struct {
>>>> + __u8 pad[6]; /* unused */
>>>> +
>>>> + __u8 class;
>>>> + __u8 instance;
>>>> + };
>>>> + };
>>>> };
>>>
>>> Not entirely happy about mixing in/out parameters. It's already
>>> complicated by being either an out value or an out pointer.
>>>
>>> Closer to the original idea for context_getparam would be to return the
>>> array of engine values.
>>
>> It would have the advantage that multiple engines could be queried
>> (more) atomically. How about then:
>>
>> I915_CONTEXT_ENABLE_ENGINE_STATS:
>> value = &{
>> __u32 num_engines;
>> __u32 pad;
>>
>> struct {
>> __u8 class;
>> __u8 instance;
>> __u8 pad[6];
>> } [num_engines];
>> };
We could also get away without explicit stats enable step if so is
desired (like the posted RFC). Enable on first query, disable on context
destroy.
>>
>> I915_CONTEXT_GET_ENGINE_STATS:
>> value = &{
>> __u32 num_engines;
>> __u32 pad;
Will need length here so we don't overwrite users memory.
>>
>> struct {
>> __u8 class;
>> __u8 instance;
>> __u8 pad[6];
>>
>> __u64 busy;
>> } [num_engines];
>> };
>
> Yes, that's what I had in mind. How does that work in practice? Does it
> make userspace too clumsy?
I wouldn't expect it to be a problem. Gordon?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/6] drm/i915: Expose list of clients in sysfs
2018-01-19 13:45 [PATCH v2 0/6] Per-context and per-client engine busyness Tvrtko Ursulin
2018-01-19 13:45 ` [PATCH 1/6] drm/i915: Track per-context " Tvrtko Ursulin
2018-01-19 13:45 ` [PATCH 2/6] drm/i915: Allow clients to query own per-engine busyness Tvrtko Ursulin
@ 2018-01-19 13:45 ` Tvrtko Ursulin
2018-01-19 13:45 ` [PATCH 4/6] drm/i915: Update client name on context create Tvrtko Ursulin
` (4 subsequent siblings)
7 siblings, 0 replies; 18+ messages in thread
From: Tvrtko Ursulin @ 2018-01-19 13:45 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Expose a list of clients with open file handles in sysfs.
This will be a basis for a top-like utility showing per-client and per-
engine GPU load.
Currently we only expose each client's pid and name under opaque numbered
directories in /sys/class/drm/card0/clients/.
For instance:
/sys/class/drm/card0/clients/3/name: Xorg
/sys/class/drm/card0/clients/3/pid: 5664
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 13 +++++
drivers/gpu/drm/i915/i915_gem.c | 112 +++++++++++++++++++++++++++++++++++---
drivers/gpu/drm/i915/i915_sysfs.c | 8 +++
3 files changed, 126 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 317953484fec..2414aa430f65 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -341,6 +341,16 @@ struct drm_i915_file_private {
*/
#define I915_MAX_CLIENT_CONTEXT_BANS 3
atomic_t context_bans;
+
+ unsigned int client_sysfs_id;
+ unsigned int client_pid;
+ char *client_name;
+ struct kobject *client_root;
+
+ struct {
+ struct device_attribute pid;
+ struct device_attribute name;
+ } attr;
};
/* Interface history:
@@ -2348,6 +2358,9 @@ struct drm_i915_private {
struct i915_pmu pmu;
+ struct kobject *clients_root;
+ atomic_t client_serial;
+
/*
* NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
* will be rejected. Instead look for a better place.
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7f0684ccc724..3625c0d4b039 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5520,6 +5520,89 @@ int i915_gem_freeze_late(struct drm_i915_private *dev_priv)
return 0;
}
+static ssize_t
+show_client_name(struct device *kdev, struct device_attribute *attr, char *buf)
+{
+ struct drm_i915_file_private *file_priv =
+ container_of(attr, struct drm_i915_file_private, attr.name);
+
+ return snprintf(buf, PAGE_SIZE, "%s", file_priv->client_name);
+}
+
+static ssize_t
+show_client_pid(struct device *kdev, struct device_attribute *attr, char *buf)
+{
+ struct drm_i915_file_private *file_priv =
+ container_of(attr, struct drm_i915_file_private, attr.pid);
+
+ return snprintf(buf, PAGE_SIZE, "%u", file_priv->client_pid);
+}
+
+static int
+i915_gem_add_client(struct drm_i915_private *i915,
+ struct drm_i915_file_private *file_priv,
+ struct task_struct *task,
+ unsigned int serial)
+{
+ int ret = -ENOMEM;
+ struct device_attribute *attr;
+ char id[32];
+
+ file_priv->client_name = kstrdup(task->comm, GFP_KERNEL);
+ if (!file_priv->client_name)
+ goto err_name;
+
+ snprintf(id, sizeof(id), "%u", serial);
+ file_priv->client_root = kobject_create_and_add(id,
+ i915->clients_root);
+ if (!file_priv->client_root)
+ goto err_client;
+
+ attr = &file_priv->attr.name;
+ attr->attr.name = "name";
+ attr->attr.mode = 0444;
+ attr->show = show_client_name;
+
+ ret = sysfs_create_file(file_priv->client_root,
+ (struct attribute *)attr);
+ if (ret)
+ goto err_attr_name;
+
+ attr = &file_priv->attr.pid;
+ attr->attr.name = "pid";
+ attr->attr.mode = 0444;
+ attr->show = show_client_pid;
+
+ ret = sysfs_create_file(file_priv->client_root,
+ (struct attribute *)attr);
+ if (ret)
+ goto err_attr_pid;
+
+ file_priv->client_pid = pid_nr(get_task_pid(task, PIDTYPE_PID));
+
+ return 0;
+
+err_attr_pid:
+ sysfs_remove_file(file_priv->client_root,
+ (struct attribute *)&file_priv->attr.name);
+err_attr_name:
+ kobject_put(file_priv->client_root);
+err_client:
+ kfree(file_priv->client_name);
+err_name:
+ return ret;
+}
+
+static void i915_gem_remove_client(struct drm_i915_file_private *file_priv)
+{
+ sysfs_remove_file(file_priv->client_root,
+ (struct attribute *)&file_priv->attr.pid);
+ sysfs_remove_file(file_priv->client_root,
+ (struct attribute *)&file_priv->attr.name);
+ kobject_put(file_priv->client_root);
+ kfree(file_priv->client_name);
+}
+
void i915_gem_release(struct drm_device *dev, struct drm_file *file)
{
struct drm_i915_file_private *file_priv = file->driver_priv;
@@ -5533,32 +5616,47 @@ void i915_gem_release(struct drm_device *dev, struct drm_file *file)
list_for_each_entry(request, &file_priv->mm.request_list, client_link)
request->file_priv = NULL;
spin_unlock(&file_priv->mm.lock);
+
+ i915_gem_remove_client(file_priv);
}
int i915_gem_open(struct drm_i915_private *i915, struct drm_file *file)
{
+ int ret = -ENOMEM;
struct drm_i915_file_private *file_priv;
- int ret;
DRM_DEBUG("\n");
file_priv = kzalloc(sizeof(*file_priv), GFP_KERNEL);
if (!file_priv)
- return -ENOMEM;
+ goto err_alloc;
+
+ file_priv->client_sysfs_id = atomic_inc_return(&i915->client_serial);
+ ret = i915_gem_add_client(i915, file_priv, current,
+ file_priv->client_sysfs_id);
+ if (ret)
+ goto err_client;
file->driver_priv = file_priv;
+ ret = i915_gem_context_open(i915, file);
+ if (ret)
+ goto err_context;
+
file_priv->dev_priv = i915;
file_priv->file = file;
+ file_priv->bsd_engine = -1;
spin_lock_init(&file_priv->mm.lock);
INIT_LIST_HEAD(&file_priv->mm.request_list);
- file_priv->bsd_engine = -1;
-
- ret = i915_gem_context_open(i915, file);
- if (ret)
- kfree(file_priv);
+ return 0;
+err_context:
+ i915_gem_remove_client(file_priv);
+err_client:
+ atomic_dec(&i915->client_serial);
+ kfree(file_priv);
+err_alloc:
return ret;
}
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index c74a20b80182..ae875c8686a3 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -575,6 +575,11 @@ void i915_setup_sysfs(struct drm_i915_private *dev_priv)
struct device *kdev = dev_priv->drm.primary->kdev;
int ret;
+ dev_priv->clients_root =
+ kobject_create_and_add("clients", &kdev->kobj);
+ if (!dev_priv->clients_root)
+ DRM_ERROR("Per-client sysfs setup failed\n");
+
#ifdef CONFIG_PM
if (HAS_RC6(dev_priv)) {
ret = sysfs_merge_group(&kdev->kobj,
@@ -635,4 +640,7 @@ void i915_teardown_sysfs(struct drm_i915_private *dev_priv)
sysfs_unmerge_group(&kdev->kobj, &rc6_attr_group);
sysfs_unmerge_group(&kdev->kobj, &rc6p_attr_group);
#endif
+
+ if (dev_priv->clients_root)
+ kobject_put(dev_priv->clients_root);
}
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 4/6] drm/i915: Update client name on context create
2018-01-19 13:45 [PATCH v2 0/6] Per-context and per-client engine busyness Tvrtko Ursulin
` (2 preceding siblings ...)
2018-01-19 13:45 ` [PATCH 3/6] drm/i915: Expose list of clients in sysfs Tvrtko Ursulin
@ 2018-01-19 13:45 ` Tvrtko Ursulin
2018-01-19 13:45 ` [PATCH 5/6] drm/i915: Expose per-engine client busyness Tvrtko Ursulin
` (3 subsequent siblings)
7 siblings, 0 replies; 18+ messages in thread
From: Tvrtko Ursulin @ 2018-01-19 13:45 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Some clients have the DRM fd passed to them over a socket by the X server.
Grab the real client and pid when they create their first context and
update the exposed data for more useful enumeration.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 8 ++++++++
drivers/gpu/drm/i915/i915_gem.c | 4 ++--
drivers/gpu/drm/i915/i915_gem_context.c | 15 +++++++++++++--
3 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2414aa430f65..a2a883aaa59e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3398,6 +3398,14 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
void i915_gem_object_unpin_from_display_plane(struct i915_vma *vma);
int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj,
int align);
+
+int
+i915_gem_add_client(struct drm_i915_private *i915,
+ struct drm_i915_file_private *file_priv,
+ struct task_struct *task,
+ unsigned int serial);
+void i915_gem_remove_client(struct drm_i915_file_private *file_priv);
+
int i915_gem_open(struct drm_i915_private *i915, struct drm_file *file);
void i915_gem_release(struct drm_device *dev, struct drm_file *file);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3625c0d4b039..7fdd892aa32a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5538,7 +5538,7 @@ show_client_pid(struct device *kdev, struct device_attribute *attr, char *buf)
return snprintf(buf, PAGE_SIZE, "%u", file_priv->client_pid);
}
-static int
+int
i915_gem_add_client(struct drm_i915_private *i915,
struct drm_i915_file_private *file_priv,
struct task_struct *task,
@@ -5593,7 +5593,7 @@ i915_gem_add_client(struct drm_i915_private *i915,
return ret;
}
-static void i915_gem_remove_client(struct drm_i915_file_private *file_priv)
+void i915_gem_remove_client(struct drm_i915_file_private *file_priv)
{
sysfs_remove_file(file_priv->client_root,
(struct attribute *)&file_priv->attr.pid);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 256800fb1a3b..0f828318a4f9 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -644,6 +644,7 @@ static bool client_is_banned(struct drm_i915_file_private *file_priv)
int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
struct drm_file *file)
{
+ unsigned int pid = pid_nr(get_task_pid(current, PIDTYPE_PID));
struct drm_i915_private *dev_priv = to_i915(dev);
struct drm_i915_gem_context_create *args = data;
struct drm_i915_file_private *file_priv = file->driver_priv;
@@ -658,8 +659,7 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
if (client_is_banned(file_priv)) {
DRM_DEBUG("client %s[%d] banned from creating ctx\n",
- current->comm,
- pid_nr(get_task_pid(current, PIDTYPE_PID)));
+ current->comm, pid);
return -EIO;
}
@@ -668,6 +668,17 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
if (ret)
return ret;
+ if (file_priv->client_pid != pid) {
+ unsigned int id = file_priv->client_sysfs_id;
+
+ i915_gem_remove_client(file_priv);
+ ret = i915_gem_add_client(dev_priv, file_priv, current, id);
+ if (ret) {
+ mutex_unlock(&dev->struct_mutex);
+ return ret;
+ }
+ }
+
ctx = i915_gem_create_context(dev_priv, file_priv);
mutex_unlock(&dev->struct_mutex);
if (IS_ERR(ctx))
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 5/6] drm/i915: Expose per-engine client busyness
2018-01-19 13:45 [PATCH v2 0/6] Per-context and per-client engine busyness Tvrtko Ursulin
` (3 preceding siblings ...)
2018-01-19 13:45 ` [PATCH 4/6] drm/i915: Update client name on context create Tvrtko Ursulin
@ 2018-01-19 13:45 ` Tvrtko Ursulin
2018-01-22 9:59 ` Tvrtko Ursulin
2018-01-19 13:45 ` [PATCH 6/6] drm/i915: Add sysfs toggle to enable per-client engine stats Tvrtko Ursulin
` (2 subsequent siblings)
7 siblings, 1 reply; 18+ messages in thread
From: Tvrtko Ursulin @ 2018-01-19 13:45 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Expose per-client and per-engine busyness under the previously added sysfs
client root.
The new file is named 'busy' and contains a list of, one line for each
engine, monotonically increasing nano-second resolution times each
client's jobs were executing on the GPU.
$ cat /sys/class/drm/card0/clients/5/busy
32516602
0
0
0
This data can serve as an interface to implement a top like utility for
GPU jobs. For instance I have prototyped a tool in IGT which produces
periodic output like:
neverball[ 6011]: rcs0: 41.01% bcs0: 0.00% vcs0: 0.00% vecs0: 0.00%
Xorg[ 5664]: rcs0: 31.16% bcs0: 0.00% vcs0: 0.00% vecs0: 0.00%
xfwm4[ 5727]: rcs0: 0.00% bcs0: 0.00% vcs0: 0.00% vecs0: 0.00%
This tools can also be extended to use the i915 PMU and show overall engine
busyness, and engine loads using the queue depth metric.
v2: Use intel_context_engine_get_busy_time.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem.c | 66 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 67 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a2a883aaa59e..a96cfdfcba03 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -350,6 +350,7 @@ struct drm_i915_file_private {
struct {
struct device_attribute pid;
struct device_attribute name;
+ struct device_attribute busy;
} attr;
};
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7fdd892aa32a..c8753e4d66c4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5538,6 +5538,57 @@ show_client_pid(struct device *kdev, struct device_attribute *attr, char *buf)
return snprintf(buf, PAGE_SIZE, "%u", file_priv->client_pid);
}
+struct busy_ctx {
+ u64 total[I915_NUM_ENGINES];
+};
+
+static int busy_add(int _id, void *p, void *data)
+{
+ struct i915_gem_context *ctx = p;
+ struct busy_ctx *bc = data;
+ struct intel_engine_cs *engine;
+ enum intel_engine_id id;
+
+ for_each_engine(engine, ctx->i915, id)
+ bc->total[id] +=
+ ktime_to_ns(intel_context_engine_get_busy_time(ctx, engine));
+
+ return 0;
+}
+
+static ssize_t
+show_client_busy(struct device *kdev, struct device_attribute *attr, char *buf)
+{
+ struct drm_i915_file_private *file_priv =
+ container_of(attr, struct drm_i915_file_private, attr.busy);
+ struct drm_i915_private *i915 = file_priv->dev_priv;
+ unsigned int len = PAGE_SIZE;
+ struct busy_ctx bc = { };
+ ssize_t res = 0;
+ struct intel_engine_cs *engine;
+ enum intel_engine_id id;
+ ssize_t ret;
+
+ ret = i915_mutex_lock_interruptible(&i915->drm);
+ if (ret)
+ return ret;
+
+ idr_for_each(&file_priv->context_idr, busy_add, &bc);
+
+ mutex_unlock(&i915->drm.struct_mutex);
+
+ for_each_engine(engine, i915, id) {
+ ret = snprintf(buf, len, "%llu\n", bc.total[id]);
+ if (ret <= 0)
+ break;
+ res += ret;
+ len -= ret;
+ buf += ret;
+ }
+
+ return res;
+}
+
int
i915_gem_add_client(struct drm_i915_private *i915,
struct drm_i915_file_private *file_priv,
@@ -5578,10 +5629,23 @@ i915_gem_add_client(struct drm_i915_private *i915,
if (ret)
goto err_attr_pid;
+ attr = &file_priv->attr.busy;
+ attr->attr.name = "busy";
+ attr->attr.mode = 0444;
+ attr->show = show_client_busy;
+
+ ret = sysfs_create_file(file_priv->client_root,
+ (struct attribute *)attr);
+ if (ret)
+ goto err_attr_busy;
+
file_priv->client_pid = pid_nr(get_task_pid(task, PIDTYPE_PID));
return 0;
+err_attr_busy:
+ sysfs_remove_file(file_priv->client_root,
+ (struct attribute *)&file_priv->attr.pid);
err_attr_pid:
sysfs_remove_file(file_priv->client_root,
(struct attribute *)&file_priv->attr.name);
@@ -5595,6 +5659,8 @@ i915_gem_add_client(struct drm_i915_private *i915,
void i915_gem_remove_client(struct drm_i915_file_private *file_priv)
{
+ sysfs_remove_file(file_priv->client_root,
+ (struct attribute *)&file_priv->attr.busy);
sysfs_remove_file(file_priv->client_root,
(struct attribute *)&file_priv->attr.pid);
sysfs_remove_file(file_priv->client_root,
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 5/6] drm/i915: Expose per-engine client busyness
2018-01-19 13:45 ` [PATCH 5/6] drm/i915: Expose per-engine client busyness Tvrtko Ursulin
@ 2018-01-22 9:59 ` Tvrtko Ursulin
0 siblings, 0 replies; 18+ messages in thread
From: Tvrtko Ursulin @ 2018-01-22 9:59 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx
On 19/01/2018 13:45, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Expose per-client and per-engine busyness under the previously added sysfs
> client root.
>
> The new file is named 'busy' and contains a list of, one line for each
> engine, monotonically increasing nano-second resolution times each
> client's jobs were executing on the GPU.
>
> $ cat /sys/class/drm/card0/clients/5/busy
> 32516602
> 0
> 0
> 0
>
> This data can serve as an interface to implement a top like utility for
> GPU jobs. For instance I have prototyped a tool in IGT which produces
> periodic output like:
>
> neverball[ 6011]: rcs0: 41.01% bcs0: 0.00% vcs0: 0.00% vecs0: 0.00%
> Xorg[ 5664]: rcs0: 31.16% bcs0: 0.00% vcs0: 0.00% vecs0: 0.00%
> xfwm4[ 5727]: rcs0: 0.00% bcs0: 0.00% vcs0: 0.00% vecs0: 0.00%
>
> This tools can also be extended to use the i915 PMU and show overall engine
> busyness, and engine loads using the queue depth metric.
>
> v2: Use intel_context_engine_get_busy_time.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/i915_gem.c | 66 +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 67 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a2a883aaa59e..a96cfdfcba03 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -350,6 +350,7 @@ struct drm_i915_file_private {
> struct {
> struct device_attribute pid;
> struct device_attribute name;
> + struct device_attribute busy;
> } attr;
> };
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 7fdd892aa32a..c8753e4d66c4 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5538,6 +5538,57 @@ show_client_pid(struct device *kdev, struct device_attribute *attr, char *buf)
> return snprintf(buf, PAGE_SIZE, "%u", file_priv->client_pid);
> }
>
> +struct busy_ctx {
> + u64 total[I915_NUM_ENGINES];
> +};
> +
> +static int busy_add(int _id, void *p, void *data)
> +{
> + struct i915_gem_context *ctx = p;
> + struct busy_ctx *bc = data;
> + struct intel_engine_cs *engine;
> + enum intel_engine_id id;
> +
> + for_each_engine(engine, ctx->i915, id)
> + bc->total[id] +=
> + ktime_to_ns(intel_context_engine_get_busy_time(ctx, engine));
> +
> + return 0;
> +}
> +
> +static ssize_t
> +show_client_busy(struct device *kdev, struct device_attribute *attr, char *buf)
> +{
> + struct drm_i915_file_private *file_priv =
> + container_of(attr, struct drm_i915_file_private, attr.busy);
> + struct drm_i915_private *i915 = file_priv->dev_priv;
> + unsigned int len = PAGE_SIZE;
> + struct busy_ctx bc = { };
> + ssize_t res = 0;
> + struct intel_engine_cs *engine;
> + enum intel_engine_id id;
> + ssize_t ret;
> +
> + ret = i915_mutex_lock_interruptible(&i915->drm);
> + if (ret)
> + return ret;
> +
> + idr_for_each(&file_priv->context_idr, busy_add, &bc);
> +
> + mutex_unlock(&i915->drm.struct_mutex);
This is the part which could be done more efficiently by adding an
aggregated busyness to file_priv as a couple atomic_t (busy count and
active count I think), and incrementing it from the
context_in/context_out hooks as they go. Then the client busyness query
would be both lockless and wouldn't need to iterate the
file_priv->context_id.
It would probably be preferable to do it like that instead of having to
take struct_mutex here. But until there is some idea if people are
warming up to this feature in general I did not bother fully polishing it.
Regards,
Tvrtko
> +
> + for_each_engine(engine, i915, id) {
> + ret = snprintf(buf, len, "%llu\n", bc.total[id]);
> + if (ret <= 0)
> + break;
> + res += ret;
> + len -= ret;
> + buf += ret;
> + }
> +
> + return res;
> +}
> +
> int
> i915_gem_add_client(struct drm_i915_private *i915,
> struct drm_i915_file_private *file_priv,
> @@ -5578,10 +5629,23 @@ i915_gem_add_client(struct drm_i915_private *i915,
> if (ret)
> goto err_attr_pid;
>
> + attr = &file_priv->attr.busy;
> + attr->attr.name = "busy";
> + attr->attr.mode = 0444;
> + attr->show = show_client_busy;
> +
> + ret = sysfs_create_file(file_priv->client_root,
> + (struct attribute *)attr);
> + if (ret)
> + goto err_attr_busy;
> +
> file_priv->client_pid = pid_nr(get_task_pid(task, PIDTYPE_PID));
>
> return 0;
>
> +err_attr_busy:
> + sysfs_remove_file(file_priv->client_root,
> + (struct attribute *)&file_priv->attr.pid);
> err_attr_pid:
> sysfs_remove_file(file_priv->client_root,
> (struct attribute *)&file_priv->attr.name);
> @@ -5595,6 +5659,8 @@ i915_gem_add_client(struct drm_i915_private *i915,
>
> void i915_gem_remove_client(struct drm_i915_file_private *file_priv)
> {
> + sysfs_remove_file(file_priv->client_root,
> + (struct attribute *)&file_priv->attr.busy);
> sysfs_remove_file(file_priv->client_root,
> (struct attribute *)&file_priv->attr.pid);
> sysfs_remove_file(file_priv->client_root,
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 6/6] drm/i915: Add sysfs toggle to enable per-client engine stats
2018-01-19 13:45 [PATCH v2 0/6] Per-context and per-client engine busyness Tvrtko Ursulin
` (4 preceding siblings ...)
2018-01-19 13:45 ` [PATCH 5/6] drm/i915: Expose per-engine client busyness Tvrtko Ursulin
@ 2018-01-19 13:45 ` Tvrtko Ursulin
2018-01-19 15:31 ` ✗ Fi.CI.BAT: failure for Per-context and per-client engine busyness (rev2) Patchwork
2018-01-19 16:54 ` ✗ Fi.CI.BAT: failure for Per-context and per-client engine busyness (rev3) Patchwork
7 siblings, 0 replies; 18+ messages in thread
From: Tvrtko Ursulin @ 2018-01-19 13:45 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
By default we are not collecting any per-engine and per-context
statistcs.
Add a new sysfs toggle to enable this facility:
$ echo 1 >/sys/class/drm/card0/clients/enable_stats
v2: Rebase.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_sysfs.c | 72 +++++++++++++++++++++++++++++++++++++++
2 files changed, 74 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a96cfdfcba03..63f96e8fe3b7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2361,6 +2361,8 @@ struct drm_i915_private {
struct kobject *clients_root;
atomic_t client_serial;
+ struct device_attribute client_stats_attr;
+ bool client_stats_enabled;
/*
* NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index ae875c8686a3..f250547af1ef 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -570,9 +570,67 @@ static void i915_setup_error_capture(struct device *kdev) {}
static void i915_teardown_error_capture(struct device *kdev) {}
#endif
+static ssize_t
+show_client_stats(struct device *kdev, struct device_attribute *attr, char *buf)
+{
+ struct drm_i915_private *i915 =
+ container_of(attr, struct drm_i915_private, client_stats_attr);
+
+ return snprintf(buf, PAGE_SIZE, "%u\n", i915->client_stats_enabled);
+}
+
+static ssize_t
+store_client_stats(struct device *kdev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct drm_i915_private *i915 =
+ container_of(attr, struct drm_i915_private, client_stats_attr);
+ bool disable = false;
+ bool enable = false;
+ bool val = false;
+ struct intel_engine_cs *engine;
+ enum intel_engine_id id;
+ int ret;
+
+ /* Use RCS as proxy for all engines. */
+ if (!intel_engine_supports_stats(i915->engine[RCS]))
+ return -EINVAL;
+
+ ret = kstrtobool(buf, &val);
+ if (ret)
+ return ret;
+
+ ret = i915_mutex_lock_interruptible(&i915->drm);
+ if (ret)
+ return ret;
+
+ if (val && !i915->client_stats_enabled)
+ enable = true;
+ else if (!val && i915->client_stats_enabled)
+ disable = true;
+
+ if (!enable && !disable)
+ goto out;
+
+ for_each_engine(engine, i915, id) {
+ if (enable)
+ WARN_ON_ONCE(intel_enable_engine_stats(engine));
+ else if (disable)
+ intel_disable_engine_stats(engine);
+ }
+
+ i915->client_stats_enabled = val;
+
+out:
+ mutex_unlock(&i915->drm.struct_mutex);
+
+ return count;
+}
+
void i915_setup_sysfs(struct drm_i915_private *dev_priv)
{
struct device *kdev = dev_priv->drm.primary->kdev;
+ struct device_attribute *attr;
int ret;
dev_priv->clients_root =
@@ -580,6 +638,17 @@ void i915_setup_sysfs(struct drm_i915_private *dev_priv)
if (!dev_priv->clients_root)
DRM_ERROR("Per-client sysfs setup failed\n");
+ attr = &dev_priv->client_stats_attr;
+ attr->attr.name = "enable_stats";
+ attr->attr.mode = 0664;
+ attr->show = show_client_stats;
+ attr->store = store_client_stats;
+
+ ret = sysfs_create_file(dev_priv->clients_root,
+ (struct attribute *)attr);
+ if (ret)
+ DRM_ERROR("Per-client sysfs setup failed! (%d)\n", ret);
+
#ifdef CONFIG_PM
if (HAS_RC6(dev_priv)) {
ret = sysfs_merge_group(&kdev->kobj,
@@ -641,6 +710,9 @@ void i915_teardown_sysfs(struct drm_i915_private *dev_priv)
sysfs_unmerge_group(&kdev->kobj, &rc6p_attr_group);
#endif
+ sysfs_remove_file(dev_priv->clients_root,
+ (struct attribute *)&dev_priv->client_stats_attr);
+
if (dev_priv->clients_root)
kobject_put(dev_priv->clients_root);
}
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread* ✗ Fi.CI.BAT: failure for Per-context and per-client engine busyness (rev2)
2018-01-19 13:45 [PATCH v2 0/6] Per-context and per-client engine busyness Tvrtko Ursulin
` (5 preceding siblings ...)
2018-01-19 13:45 ` [PATCH 6/6] drm/i915: Add sysfs toggle to enable per-client engine stats Tvrtko Ursulin
@ 2018-01-19 15:31 ` Patchwork
2018-01-19 16:54 ` ✗ Fi.CI.BAT: failure for Per-context and per-client engine busyness (rev3) Patchwork
7 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-01-19 15:31 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
== Series Details ==
Series: Per-context and per-client engine busyness (rev2)
URL : https://patchwork.freedesktop.org/series/32645/
State : failure
== Summary ==
CHK include/config/kernel.release
CHK include/generated/uapi/linux/version.h
CHK include/generated/utsrelease.h
CHK include/generated/bounds.h
CHK include/generated/timeconst.h
CHK include/generated/asm-offsets.h
CALL scripts/checksyscalls.sh
DESCEND objtool
CHK scripts/mod/devicetable-offsets.h
CHK include/generated/compile.h
CC init/version.o
AR init/built-in.o
CC arch/x86/kernel/early-quirks.o
AR arch/x86/kernel/built-in.o
AR arch/x86/built-in.o
CC kernel/sys.o
CC kernel/trace/trace.o
AR kernel/trace/built-in.o
CC kernel/module.o
AR kernel/built-in.o
GZIP kernel/config_data.gz
CHK kernel/config_data.h
UPD kernel/config_data.h
CC [M] kernel/configs.o
CC drivers/base/firmware_class.o
AR drivers/base/built-in.o
CC [M] drivers/gpu/drm/i915/i915_drv.o
In file included from drivers/gpu/drm/i915/intel_ringbuffer.h:7:0,
from drivers/gpu/drm/i915/intel_lrc.h:27,
from drivers/gpu/drm/i915/i915_drv.h:63,
from drivers/gpu/drm/i915/i915_drv.c:49:
drivers/gpu/drm/i915/i915_gem_context.h:166:11: error: ‘I915_NUM_ENGINES’ undeclared here (not in a function)
} engine[I915_NUM_ENGINES];
^~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/i915_gem_context.h: In function ‘i915_gem_context_set_closed’:
drivers/gpu/drm/i915/i915_gem_context.h:209:2: error: implicit declaration of function ‘GEM_BUG_ON’ [-Werror=implicit-function-declaration]
GEM_BUG_ON(i915_gem_context_is_closed(ctx));
^~~~~~~~~~
drivers/gpu/drm/i915/i915_gem_context.h: At top level:
drivers/gpu/drm/i915/i915_gem_context.h:282:32: error: ‘struct drm_i915_gem_request’ declared inside parameter list will not be visible outside of this definition or declaration [-Werror]
int i915_switch_context(struct drm_i915_gem_request *req);
^~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
scripts/Makefile.build:310: recipe for target 'drivers/gpu/drm/i915/i915_drv.o' failed
make[4]: *** [drivers/gpu/drm/i915/i915_drv.o] Error 1
scripts/Makefile.build:569: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:569: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:569: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1018: recipe for target 'drivers' failed
make: *** [drivers] Error 2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread* ✗ Fi.CI.BAT: failure for Per-context and per-client engine busyness (rev3)
2018-01-19 13:45 [PATCH v2 0/6] Per-context and per-client engine busyness Tvrtko Ursulin
` (6 preceding siblings ...)
2018-01-19 15:31 ` ✗ Fi.CI.BAT: failure for Per-context and per-client engine busyness (rev2) Patchwork
@ 2018-01-19 16:54 ` Patchwork
7 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-01-19 16:54 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
== Series Details ==
Series: Per-context and per-client engine busyness (rev3)
URL : https://patchwork.freedesktop.org/series/32645/
State : failure
== Summary ==
Series 32645v3 Per-context and per-client engine busyness
https://patchwork.freedesktop.org/api/1.0/series/32645/revisions/3/mbox/
Test debugfs_test:
Subgroup read_all_entries:
dmesg-warn -> DMESG-FAIL (fi-elk-e7500) fdo#103989
Test gem_exec_flush:
Subgroup basic-wb-prw-default:
pass -> INCOMPLETE (fi-snb-2600)
Test gem_ringfill:
Subgroup basic-default-hang:
dmesg-warn -> PASS (fi-pnv-d510) fdo#101600
fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#101600 https://bugs.freedesktop.org/show_bug.cgi?id=101600
fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:435s
fi-bdw-gvtdvm total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:428s
fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:372s
fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:512s
fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:275s
fi-bxt-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:526s
fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:526s
fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:532s
fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:473s
fi-elk-e7500 total:224 pass:168 dwarn:9 dfail:1 fail:0 skip:45
fi-gdg-551 total:288 pass:179 dwarn:0 dfail:0 fail:1 skip:108 time:278s
fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:543s
fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:422s
fi-hsw-4770r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:430s
fi-ilk-650 total:288 pass:228 dwarn:0 dfail:0 fail:0 skip:60 time:411s
fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:492s
fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:0 skip:33 time:445s
fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:468s
fi-kbl-7560u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:511s
fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:463s
fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:538s
fi-pnv-d510 total:288 pass:223 dwarn:0 dfail:0 fail:0 skip:65 time:529s
fi-skl-6260u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:451s
fi-skl-6600u total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:524s
fi-skl-6700hq total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:575s
fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:509s
fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:500s
fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:431s
fi-snb-2520m total:245 pass:211 dwarn:0 dfail:0 fail:0 skip:33
fi-snb-2600 total:59 pass:48 dwarn:0 dfail:0 fail:0 skip:10
Blacklisted hosts:
fi-cfl-s2 total:288 pass:256 dwarn:0 dfail:0 fail:3 skip:26 time:610s
fi-glk-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:506s
fi-skl-guc total:288 pass:211 dwarn:48 dfail:0 fail:1 skip:28 time:453s
fd10fae2b0f673861816511f99d4798d6b619b40 drm-tip: 2018y-01m-19d-14h-22m-15s UTC integration manifest
1d9909fd0965 drm/i915: Add sysfs toggle to enable per-client engine stats
45aee892c9ad drm/i915: Expose per-engine client busyness
813f0228546b drm/i915: Update client name on context create
9e245f148ad3 drm/i915: Expose list of clients in sysfs
1a6e191c83f8 drm/i915: Allow clients to query own per-engine busyness
01e95b47013e drm/i915: Track per-context engine busyness
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7726/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread