* Context churn
@ 2016-05-23 11:34 Chris Wilson
2016-05-23 11:34 ` [PATCH v2 01/11] drm/i915: Rename struct intel_context Chris Wilson
` (14 more replies)
0 siblings, 15 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-23 11:34 UTC (permalink / raw)
To: intel-gfx
The essence of this series is to just rename the anonymous struct inside
intel_context. But since it involves churn, I added some sweetner into
the miex (and more churn)!
[PATCH v2 04/11] drm/i915: Rename i915_gem_context_reference/unreference()
Long term plan is to migrate all of our reference manipulation over to
the core kref_get/kref_put idiom. Whilst I was changing contexts, adding
the churn here seemed sensible.
Wants somebody to second my crazy plan for consistent naming.
[PATCH v2 06/11] drm/i915: Move pinning of dev_priv->kernel_context
Tvrtko spotted a missed idr_remove on an error path, v2 needs an r-b.
[PATCH v2 07/11] drm/i915: Show i915_gem_context owner in debugfs
Tvrtko spotted that we never mark the file_priv as closed, brought that
earlier in the queue (to this v2).
[PATCH v2 10/11] drm/i915: Rearrange i915_gem_context
Tvrkto has r-b this, I've just managed to forget to add it.
[PATCH v2 11/11] drm/i915: Show context objects in
Any takers?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 01/11] drm/i915: Rename struct intel_context
2016-05-23 11:34 Context churn Chris Wilson
@ 2016-05-23 11:34 ` Chris Wilson
2016-05-23 11:34 ` [PATCH v2 02/11] drm/i915: Apply lockdep annotations to i915_gem_context.c Chris Wilson
` (13 subsequent siblings)
14 siblings, 0 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-23 11:34 UTC (permalink / raw)
To: intel-gfx
Our goal is to rename the anonymous per-engine struct beneath the
current intel_context. However, after a lively debate resolving around
the confusion between intel_context_engine and intel_engine_context, the
realisation is that the two structs target different users. The outer
struct is API / user facing, and so carries the higher level GEM
information. The inner struct is hw facing. Thus we want to name the
inner struct intel_context and the outer one i915_gem_context. As the
first step, we need to rename the current struct:
s/struct intel_context/struct i915_gem_context/
which fits much better with its constructors already conveying the
i915_gem_context prefix!
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Dave Gordon <david.s.gordon@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 10 +++---
drivers/gpu/drm/i915/i915_drv.h | 22 ++++++-------
drivers/gpu/drm/i915/i915_gem.c | 8 ++---
drivers/gpu/drm/i915/i915_gem_context.c | 52 +++++++++++++++---------------
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 10 +++---
drivers/gpu/drm/i915/i915_guc_submission.c | 12 +++----
drivers/gpu/drm/i915/i915_sysfs.c | 2 +-
drivers/gpu/drm/i915/i915_trace.h | 12 +++----
drivers/gpu/drm/i915/intel_guc.h | 2 +-
drivers/gpu/drm/i915/intel_lrc.c | 22 ++++++-------
drivers/gpu/drm/i915/intel_lrc.h | 10 +++---
drivers/gpu/drm/i915/intel_ringbuffer.h | 4 +--
12 files changed, 84 insertions(+), 82 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 4c6b48dbd6e2..ae28e6e9d603 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -199,7 +199,7 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
seq_printf(m, " (frontbuffer: 0x%03x)", obj->frontbuffer_bits);
}
-static void describe_ctx(struct seq_file *m, struct intel_context *ctx)
+static void describe_ctx(struct seq_file *m, struct i915_gem_context *ctx)
{
seq_putc(m, ctx->legacy_hw_ctx.initialized ? 'I' : 'i');
seq_putc(m, ctx->remap_slice ? 'R' : 'r');
@@ -2000,7 +2000,7 @@ static int i915_context_status(struct seq_file *m, void *unused)
struct drm_device *dev = node->minor->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_engine_cs *engine;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
enum intel_engine_id id;
int ret;
@@ -2046,7 +2046,7 @@ static int i915_context_status(struct seq_file *m, void *unused)
}
static void i915_dump_lrc_obj(struct seq_file *m,
- struct intel_context *ctx,
+ struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
struct page *page;
@@ -2094,7 +2094,7 @@ static int i915_dump_lrc(struct seq_file *m, void *unused)
struct drm_device *dev = node->minor->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_engine_cs *engine;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
int ret;
if (!i915.enable_execlists) {
@@ -2274,7 +2274,7 @@ static int i915_swizzle_info(struct seq_file *m, void *data)
static int per_file_ctx(int id, void *ptr, void *data)
{
- struct intel_context *ctx = ptr;
+ struct i915_gem_context *ctx = ptr;
struct seq_file *m = data;
struct i915_hw_ppgtt *ppgtt = ctx->ppgtt;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 255d4c3e3eea..ff35e36599de 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -831,7 +831,7 @@ struct i915_ctx_hang_stats {
#define CONTEXT_NO_ZEROMAP (1<<0)
/**
- * struct intel_context - as the name implies, represents a context.
+ * struct i915_gem_context - as the name implies, represents a context.
* @ref: reference count.
* @user_handle: userspace tracking identity for this context.
* @remap_slice: l3 row remapping information.
@@ -849,7 +849,7 @@ struct i915_ctx_hang_stats {
* Contexts are memory images used by the hardware to store copies of their
* internal state.
*/
-struct intel_context {
+struct i915_gem_context {
struct kref ref;
int user_handle;
uint8_t remap_slice;
@@ -1710,7 +1710,7 @@ struct i915_execbuffer_params {
uint64_t batch_obj_vm_offset;
struct intel_engine_cs *engine;
struct drm_i915_gem_object *batch_obj;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
struct drm_i915_gem_request *request;
};
@@ -2013,7 +2013,7 @@ struct drm_i915_private {
void (*stop_engine)(struct intel_engine_cs *engine);
} gt;
- struct intel_context *kernel_context;
+ struct i915_gem_context *kernel_context;
/* perform PHY state sanity checks? */
bool chv_phy_assert[2];
@@ -2381,7 +2381,7 @@ struct drm_i915_gem_request {
* i915_gem_request_free() will then decrement the refcount on the
* context.
*/
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
struct intel_ringbuffer *ringbuf;
/**
@@ -2393,7 +2393,7 @@ struct drm_i915_gem_request {
* we keep the previous context pinned until the following (this)
* request is retired.
*/
- struct intel_context *previous_context;
+ struct i915_gem_context *previous_context;
/** Batch buffer related to this request if any (used for
error state dump only) */
@@ -2437,7 +2437,7 @@ struct drm_i915_gem_request {
struct drm_i915_gem_request * __must_check
i915_gem_request_alloc(struct intel_engine_cs *engine,
- struct intel_context *ctx);
+ struct i915_gem_context *ctx);
void i915_gem_request_free(struct kref *req_ref);
int i915_gem_request_add_to_client(struct drm_i915_gem_request *req,
struct drm_file *file);
@@ -3417,22 +3417,22 @@ void i915_gem_context_reset(struct drm_device *dev);
int i915_gem_context_open(struct drm_device *dev, struct drm_file *file);
void i915_gem_context_close(struct drm_device *dev, struct drm_file *file);
int i915_switch_context(struct drm_i915_gem_request *req);
-struct intel_context *
+struct i915_gem_context *
i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id);
void i915_gem_context_free(struct kref *ctx_ref);
struct drm_i915_gem_object *
i915_gem_alloc_context_obj(struct drm_device *dev, size_t size);
-static inline void i915_gem_context_reference(struct intel_context *ctx)
+static inline void i915_gem_context_reference(struct i915_gem_context *ctx)
{
kref_get(&ctx->ref);
}
-static inline void i915_gem_context_unreference(struct intel_context *ctx)
+static inline void i915_gem_context_unreference(struct i915_gem_context *ctx)
{
kref_put(&ctx->ref, i915_gem_context_free);
}
-static inline bool i915_gem_context_is_default(const struct intel_context *c)
+static inline bool i915_gem_context_is_default(const struct i915_gem_context *c)
{
return c->user_handle == DEFAULT_CONTEXT_HANDLE;
}
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c4af57ada378..728b66911840 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2689,7 +2689,7 @@ void __i915_add_request(struct drm_i915_gem_request *request,
}
static bool i915_context_is_banned(struct drm_i915_private *dev_priv,
- const struct intel_context *ctx)
+ const struct i915_gem_context *ctx)
{
unsigned long elapsed;
@@ -2714,7 +2714,7 @@ static bool i915_context_is_banned(struct drm_i915_private *dev_priv,
}
static void i915_set_reset_status(struct drm_i915_private *dev_priv,
- struct intel_context *ctx,
+ struct i915_gem_context *ctx,
const bool guilty)
{
struct i915_ctx_hang_stats *hs;
@@ -2742,7 +2742,7 @@ void i915_gem_request_free(struct kref *req_ref)
static inline int
__i915_gem_request_alloc(struct intel_engine_cs *engine,
- struct intel_context *ctx,
+ struct i915_gem_context *ctx,
struct drm_i915_gem_request **req_out)
{
struct drm_i915_private *dev_priv = engine->i915;
@@ -2818,7 +2818,7 @@ err:
*/
struct drm_i915_gem_request *
i915_gem_request_alloc(struct intel_engine_cs *engine,
- struct intel_context *ctx)
+ struct i915_gem_context *ctx)
{
struct drm_i915_gem_request *req;
int err;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 2aedd188473d..8484da26b5d4 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -134,7 +134,7 @@ static int get_context_size(struct drm_i915_private *dev_priv)
return ret;
}
-static void i915_gem_context_clean(struct intel_context *ctx)
+static void i915_gem_context_clean(struct i915_gem_context *ctx)
{
struct i915_hw_ppgtt *ppgtt = ctx->ppgtt;
struct i915_vma *vma, *next;
@@ -151,7 +151,7 @@ static void i915_gem_context_clean(struct intel_context *ctx)
void i915_gem_context_free(struct kref *ctx_ref)
{
- struct intel_context *ctx = container_of(ctx_ref, typeof(*ctx), ref);
+ struct i915_gem_context *ctx = container_of(ctx_ref, typeof(*ctx), ref);
trace_i915_context_free(ctx);
@@ -234,12 +234,12 @@ static int assign_hw_id(struct drm_i915_private *dev_priv, unsigned *out)
return 0;
}
-static struct intel_context *
+static struct i915_gem_context *
__create_hw_context(struct drm_device *dev,
struct drm_i915_file_private *file_priv)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
int ret;
ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
@@ -296,12 +296,12 @@ err_out:
* context state of the GPU for applications that don't utilize HW contexts, as
* well as an idle case.
*/
-static struct intel_context *
+static struct i915_gem_context *
i915_gem_create_context(struct drm_device *dev,
struct drm_i915_file_private *file_priv)
{
const bool is_global_default_ctx = file_priv == NULL;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
int ret = 0;
BUG_ON(!mutex_is_locked(&dev->struct_mutex));
@@ -352,7 +352,7 @@ err_destroy:
return ERR_PTR(ret);
}
-static void i915_gem_context_unpin(struct intel_context *ctx,
+static void i915_gem_context_unpin(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
if (i915.enable_execlists) {
@@ -369,7 +369,7 @@ void i915_gem_context_reset(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
if (i915.enable_execlists) {
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
list_for_each_entry(ctx, &dev_priv->context_list, link)
intel_lr_context_reset(dev_priv, ctx);
@@ -381,7 +381,7 @@ void i915_gem_context_reset(struct drm_device *dev)
int i915_gem_context_init(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
/* Init should only be called once per module load. Eventually the
* restriction on the context_disabled check can be loosened. */
@@ -449,7 +449,7 @@ void i915_gem_context_lost(struct drm_i915_private *dev_priv)
void i915_gem_context_fini(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_context *dctx = dev_priv->kernel_context;
+ struct i915_gem_context *dctx = dev_priv->kernel_context;
if (dctx->legacy_hw_ctx.rcs_state)
i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
@@ -462,7 +462,7 @@ void i915_gem_context_fini(struct drm_device *dev)
static int context_idr_cleanup(int id, void *p, void *data)
{
- struct intel_context *ctx = p;
+ struct i915_gem_context *ctx = p;
i915_gem_context_unreference(ctx);
return 0;
@@ -471,7 +471,7 @@ static int context_idr_cleanup(int id, void *p, void *data)
int i915_gem_context_open(struct drm_device *dev, struct drm_file *file)
{
struct drm_i915_file_private *file_priv = file->driver_priv;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
idr_init(&file_priv->context_idr);
@@ -495,12 +495,12 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file)
idr_destroy(&file_priv->context_idr);
}
-struct intel_context *
+struct i915_gem_context *
i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id)
{
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
- ctx = (struct intel_context *)idr_find(&file_priv->context_idr, id);
+ ctx = idr_find(&file_priv->context_idr, id);
if (!ctx)
return ERR_PTR(-ENOENT);
@@ -641,7 +641,7 @@ static int remap_l3(struct drm_i915_gem_request *req, int slice)
static inline bool skip_rcs_switch(struct i915_hw_ppgtt *ppgtt,
struct intel_engine_cs *engine,
- struct intel_context *to)
+ struct i915_gem_context *to)
{
if (to->remap_slice)
return false;
@@ -658,7 +658,7 @@ static inline bool skip_rcs_switch(struct i915_hw_ppgtt *ppgtt,
static bool
needs_pd_load_pre(struct i915_hw_ppgtt *ppgtt,
struct intel_engine_cs *engine,
- struct intel_context *to)
+ struct i915_gem_context *to)
{
if (!ppgtt)
return false;
@@ -683,7 +683,7 @@ needs_pd_load_pre(struct i915_hw_ppgtt *ppgtt,
static bool
needs_pd_load_post(struct i915_hw_ppgtt *ppgtt,
- struct intel_context *to,
+ struct i915_gem_context *to,
u32 hw_flags)
{
if (!ppgtt)
@@ -700,10 +700,10 @@ needs_pd_load_post(struct i915_hw_ppgtt *ppgtt,
static int do_rcs_switch(struct drm_i915_gem_request *req)
{
- struct intel_context *to = req->ctx;
+ struct i915_gem_context *to = req->ctx;
struct intel_engine_cs *engine = req->engine;
struct i915_hw_ppgtt *ppgtt = to->ppgtt ?: req->i915->mm.aliasing_ppgtt;
- struct intel_context *from;
+ struct i915_gem_context *from;
u32 hw_flags;
int ret, i;
@@ -859,7 +859,7 @@ int i915_switch_context(struct drm_i915_gem_request *req)
if (engine->id != RCS ||
req->ctx->legacy_hw_ctx.rcs_state == NULL) {
- struct intel_context *to = req->ctx;
+ struct i915_gem_context *to = req->ctx;
struct i915_hw_ppgtt *ppgtt =
to->ppgtt ?: req->i915->mm.aliasing_ppgtt;
@@ -897,7 +897,7 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
{
struct drm_i915_gem_context_create *args = data;
struct drm_i915_file_private *file_priv = file->driver_priv;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
int ret;
if (!contexts_enabled(dev))
@@ -926,7 +926,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
{
struct drm_i915_gem_context_destroy *args = data;
struct drm_i915_file_private *file_priv = file->driver_priv;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
int ret;
if (args->pad != 0)
@@ -958,7 +958,7 @@ 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 intel_context *ctx;
+ struct i915_gem_context *ctx;
int ret;
ret = i915_mutex_lock_interruptible(dev);
@@ -1001,7 +1001,7 @@ int i915_gem_context_setparam_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 intel_context *ctx;
+ struct i915_gem_context *ctx;
int ret;
ret = i915_mutex_lock_interruptible(dev);
@@ -1047,7 +1047,7 @@ int i915_gem_context_reset_stats_ioctl(struct drm_device *dev,
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_reset_stats *args = data;
struct i915_ctx_hang_stats *hs;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
int ret;
if (args->flags || args->pad)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index a54a243ccaac..e61b92d7b0bc 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -716,7 +716,7 @@ eb_vma_misplaced(struct i915_vma *vma)
static int
i915_gem_execbuffer_reserve(struct intel_engine_cs *engine,
struct list_head *vmas,
- struct intel_context *ctx,
+ struct i915_gem_context *ctx,
bool *need_relocs)
{
struct drm_i915_gem_object *obj;
@@ -828,7 +828,7 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev,
struct intel_engine_cs *engine,
struct eb_vmas *eb,
struct drm_i915_gem_exec_object2 *exec,
- struct intel_context *ctx)
+ struct i915_gem_context *ctx)
{
struct drm_i915_gem_relocation_entry *reloc;
struct i915_address_space *vm;
@@ -1065,11 +1065,11 @@ validate_exec_list(struct drm_device *dev,
return 0;
}
-static struct intel_context *
+static struct i915_gem_context *
i915_gem_validate_context(struct drm_device *dev, struct drm_file *file,
struct intel_engine_cs *engine, const u32 ctx_id)
{
- struct intel_context *ctx = NULL;
+ struct i915_gem_context *ctx = NULL;
struct i915_ctx_hang_stats *hs;
if (engine->id != RCS && ctx_id != DEFAULT_CONTEXT_HANDLE)
@@ -1430,7 +1430,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
struct drm_i915_gem_object *batch_obj;
struct drm_i915_gem_exec_object2 shadow_exec_entry;
struct intel_engine_cs *engine;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
struct i915_address_space *vm;
struct i915_execbuffer_params params_master; /* XXX: will be removed later */
struct i915_execbuffer_params *params = ¶ms_master;
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 169242a8adff..a292672f36d5 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -360,7 +360,7 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
struct drm_i915_gem_object *client_obj = client->client_obj;
struct drm_i915_private *dev_priv = guc_to_i915(guc);
struct intel_engine_cs *engine;
- struct intel_context *ctx = client->owner;
+ struct i915_gem_context *ctx = client->owner;
struct guc_context_desc desc;
struct sg_table *sg;
enum intel_engine_id id;
@@ -426,7 +426,7 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
desc.wq_size = client->wq_size;
/*
- * XXX: Take LRCs from an existing intel_context if this is not an
+ * XXX: Take LRCs from an existing context if this is not an
* IsKMDCreatedContext client
*/
desc.desc_private = (uintptr_t)client;
@@ -677,7 +677,7 @@ static void guc_client_free(struct drm_device *dev,
*/
static struct i915_guc_client *guc_client_alloc(struct drm_device *dev,
uint32_t priority,
- struct intel_context *ctx)
+ struct i915_gem_context *ctx)
{
struct i915_guc_client *client;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -915,7 +915,7 @@ int i915_guc_submission_enable(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_guc *guc = &dev_priv->guc;
- struct intel_context *ctx = dev_priv->kernel_context;
+ struct i915_gem_context *ctx = dev_priv->kernel_context;
struct i915_guc_client *client;
/* client for execbuf submission */
@@ -966,7 +966,7 @@ int intel_guc_suspend(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_guc *guc = &dev_priv->guc;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
u32 data[3];
if (!i915.enable_guc_submission)
@@ -992,7 +992,7 @@ int intel_guc_resume(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_guc *guc = &dev_priv->guc;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
u32 data[3];
if (!i915.enable_guc_submission)
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index 37b6444b8e22..02507bfc8def 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -203,7 +203,7 @@ i915_l3_write(struct file *filp, struct kobject *kobj,
struct drm_minor *dminor = dev_to_drm_minor(dev);
struct drm_device *drm_dev = dminor->dev;
struct drm_i915_private *dev_priv = drm_dev->dev_private;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
u32 *temp = NULL; /* Just here to make handling failures easy */
int slice = (int)(uintptr_t)attr->private;
int ret;
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index 20b2e4039792..6768db032f84 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -734,12 +734,12 @@ DEFINE_EVENT(i915_ppgtt, i915_ppgtt_release,
* the context.
*/
DECLARE_EVENT_CLASS(i915_context,
- TP_PROTO(struct intel_context *ctx),
+ TP_PROTO(struct i915_gem_context *ctx),
TP_ARGS(ctx),
TP_STRUCT__entry(
__field(u32, dev)
- __field(struct intel_context *, ctx)
+ __field(struct i915_gem_context *, ctx)
__field(struct i915_address_space *, vm)
),
@@ -754,12 +754,12 @@ DECLARE_EVENT_CLASS(i915_context,
)
DEFINE_EVENT(i915_context, i915_context_create,
- TP_PROTO(struct intel_context *ctx),
+ TP_PROTO(struct i915_gem_context *ctx),
TP_ARGS(ctx)
);
DEFINE_EVENT(i915_context, i915_context_free,
- TP_PROTO(struct intel_context *ctx),
+ TP_PROTO(struct i915_gem_context *ctx),
TP_ARGS(ctx)
);
@@ -771,13 +771,13 @@ DEFINE_EVENT(i915_context, i915_context_free,
* called only if full ppgtt is enabled.
*/
TRACE_EVENT(switch_mm,
- TP_PROTO(struct intel_engine_cs *engine, struct intel_context *to),
+ TP_PROTO(struct intel_engine_cs *engine, struct i915_gem_context *to),
TP_ARGS(engine, to),
TP_STRUCT__entry(
__field(u32, ring)
- __field(struct intel_context *, to)
+ __field(struct i915_gem_context *, to)
__field(struct i915_address_space *, vm)
__field(u32, dev)
),
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 9d79c4c3e256..c9c757ef003f 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -55,7 +55,7 @@ struct drm_i915_gem_request;
struct i915_guc_client {
struct drm_i915_gem_object *client_obj;
void *client_base; /* first page (only) of above */
- struct intel_context *owner;
+ struct i915_gem_context *owner;
struct intel_guc *guc;
uint32_t priority;
uint32_t ctx_index;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index affca6d5ce7e..426dc90aceed 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -231,9 +231,9 @@ enum {
/* Typical size of the average request (2 pipecontrols and a MI_BB) */
#define EXECLISTS_REQUEST_SIZE 64 /* bytes */
-static int execlists_context_deferred_alloc(struct intel_context *ctx,
+static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
struct intel_engine_cs *engine);
-static int intel_lr_context_pin(struct intel_context *ctx,
+static int intel_lr_context_pin(struct i915_gem_context *ctx,
struct intel_engine_cs *engine);
/**
@@ -315,7 +315,7 @@ logical_ring_init_platform_invariants(struct intel_engine_cs *engine)
* bits 55-63: group ID, currently unused and set to 0
*/
static void
-intel_lr_context_descriptor_update(struct intel_context *ctx,
+intel_lr_context_descriptor_update(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
u64 desc;
@@ -330,7 +330,7 @@ intel_lr_context_descriptor_update(struct intel_context *ctx,
ctx->engine[engine->id].lrc_desc = desc;
}
-uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
+uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
return ctx->engine[engine->id].lrc_desc;
@@ -932,7 +932,7 @@ int logical_ring_flush_all_caches(struct drm_i915_gem_request *req)
return 0;
}
-static int intel_lr_context_pin(struct intel_context *ctx,
+static int intel_lr_context_pin(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
struct drm_i915_private *dev_priv = ctx->i915;
@@ -988,7 +988,7 @@ err:
return ret;
}
-void intel_lr_context_unpin(struct intel_context *ctx,
+void intel_lr_context_unpin(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
struct drm_i915_gem_object *ctx_obj;
@@ -2049,7 +2049,7 @@ logical_ring_setup(struct drm_device *dev, enum intel_engine_id id)
static int
logical_ring_init(struct intel_engine_cs *engine)
{
- struct intel_context *dctx = engine->i915->kernel_context;
+ struct i915_gem_context *dctx = engine->i915->kernel_context;
int ret;
ret = i915_cmd_parser_init_ring(engine);
@@ -2273,7 +2273,7 @@ static u32 intel_lr_indirect_ctx_offset(struct intel_engine_cs *engine)
}
static int
-populate_lr_context(struct intel_context *ctx,
+populate_lr_context(struct i915_gem_context *ctx,
struct drm_i915_gem_object *ctx_obj,
struct intel_engine_cs *engine,
struct intel_ringbuffer *ringbuf)
@@ -2421,7 +2421,7 @@ populate_lr_context(struct intel_context *ctx,
* takes care of the bits that are LRC related: the per-engine backing
* objects and the logical ringbuffer.
*/
-void intel_lr_context_free(struct intel_context *ctx)
+void intel_lr_context_free(struct i915_gem_context *ctx)
{
int i;
@@ -2489,7 +2489,7 @@ uint32_t intel_lr_context_size(struct intel_engine_cs *engine)
*
* Return: non-zero on error.
*/
-static int execlists_context_deferred_alloc(struct intel_context *ctx,
+static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
struct drm_i915_gem_object *ctx_obj;
@@ -2539,7 +2539,7 @@ error_deref_obj:
}
void intel_lr_context_reset(struct drm_i915_private *dev_priv,
- struct intel_context *ctx)
+ struct i915_gem_context *ctx)
{
struct intel_engine_cs *engine;
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 1afba0331dc6..eb2e1d157fed 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -99,16 +99,18 @@ static inline void intel_logical_ring_emit_reg(struct intel_ringbuffer *ringbuf,
#define LRC_PPHWSP_PN (LRC_GUCSHR_PN + 1)
#define LRC_STATE_PN (LRC_PPHWSP_PN + 1)
-void intel_lr_context_free(struct intel_context *ctx);
+struct i915_gem_context;
+
+void intel_lr_context_free(struct i915_gem_context *ctx);
uint32_t intel_lr_context_size(struct intel_engine_cs *engine);
-void intel_lr_context_unpin(struct intel_context *ctx,
+void intel_lr_context_unpin(struct i915_gem_context *ctx,
struct intel_engine_cs *engine);
struct drm_i915_private;
void intel_lr_context_reset(struct drm_i915_private *dev_priv,
- struct intel_context *ctx);
-uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
+ struct i915_gem_context *ctx);
+uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx,
struct intel_engine_cs *engine);
/* Execlists */
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 929e7b4af2a4..b33c876fed20 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -119,7 +119,7 @@ struct intel_ringbuffer {
u32 last_retired_head;
};
-struct intel_context;
+struct i915_gem_context;
struct drm_i915_reg_table;
/*
@@ -310,7 +310,7 @@ struct intel_engine_cs {
wait_queue_head_t irq_queue;
- struct intel_context *last_context;
+ struct i915_gem_context *last_context;
struct intel_ring_hangcheck hangcheck;
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 02/11] drm/i915: Apply lockdep annotations to i915_gem_context.c
2016-05-23 11:34 Context churn Chris Wilson
2016-05-23 11:34 ` [PATCH v2 01/11] drm/i915: Rename struct intel_context Chris Wilson
@ 2016-05-23 11:34 ` Chris Wilson
2016-05-23 11:34 ` [PATCH v2 03/11] drm/i915: Rename and inline i915_gem_context_get() Chris Wilson
` (12 subsequent siblings)
14 siblings, 0 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-23 11:34 UTC (permalink / raw)
To: intel-gfx
Markup the functions that require the caller to hold struct_mutex with
lockdep_assert_held(). In the hopefully not-too-distant future we will
split the struct_mutex up, and in doing so we need to be sure that we
know what it protects - here the lockdep annotations are invaluable.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem_context.c | 16 ++++++++++++++--
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ff35e36599de..a0f5f00f552f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3429,6 +3429,7 @@ static inline void i915_gem_context_reference(struct i915_gem_context *ctx)
static inline void i915_gem_context_unreference(struct i915_gem_context *ctx)
{
+ lockdep_assert_held(&ctx->i915->dev->struct_mutex);
kref_put(&ctx->ref, i915_gem_context_free);
}
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 8484da26b5d4..af747f14522e 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -153,6 +153,7 @@ void i915_gem_context_free(struct kref *ctx_ref)
{
struct i915_gem_context *ctx = container_of(ctx_ref, typeof(*ctx), ref);
+ lockdep_assert_held(&ctx->i915->dev->struct_mutex);
trace_i915_context_free(ctx);
if (i915.enable_execlists)
@@ -181,6 +182,8 @@ i915_gem_alloc_context_obj(struct drm_device *dev, size_t size)
struct drm_i915_gem_object *obj;
int ret;
+ lockdep_assert_held(&dev->struct_mutex);
+
obj = i915_gem_object_create(dev, size);
if (IS_ERR(obj))
return obj;
@@ -368,6 +371,8 @@ void i915_gem_context_reset(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
+ lockdep_assert_held(&dev->struct_mutex);
+
if (i915.enable_execlists) {
struct i915_gem_context *ctx;
@@ -433,6 +438,8 @@ void i915_gem_context_lost(struct drm_i915_private *dev_priv)
{
struct intel_engine_cs *engine;
+ lockdep_assert_held(&dev_priv->dev->struct_mutex);
+
for_each_engine(engine, dev_priv) {
if (engine->last_context == NULL)
continue;
@@ -451,6 +458,8 @@ void i915_gem_context_fini(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
struct i915_gem_context *dctx = dev_priv->kernel_context;
+ lockdep_assert_held(&dev->struct_mutex);
+
if (dctx->legacy_hw_ctx.rcs_state)
i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
@@ -491,6 +500,8 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file)
{
struct drm_i915_file_private *file_priv = file->driver_priv;
+ lockdep_assert_held(&dev->struct_mutex);
+
idr_for_each(&file_priv->context_idr, context_idr_cleanup, NULL);
idr_destroy(&file_priv->context_idr);
}
@@ -500,6 +511,8 @@ i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id)
{
struct i915_gem_context *ctx;
+ lockdep_assert_held(&file_priv->dev_priv->dev->struct_mutex);
+
ctx = idr_find(&file_priv->context_idr, id);
if (!ctx)
return ERR_PTR(-ENOENT);
@@ -852,10 +865,9 @@ unpin_out:
int i915_switch_context(struct drm_i915_gem_request *req)
{
struct intel_engine_cs *engine = req->engine;
- struct drm_i915_private *dev_priv = req->i915;
WARN_ON(i915.enable_execlists);
- WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex));
+ lockdep_assert_held(&req->i915->dev->struct_mutex);
if (engine->id != RCS ||
req->ctx->legacy_hw_ctx.rcs_state == NULL) {
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 03/11] drm/i915: Rename and inline i915_gem_context_get()
2016-05-23 11:34 Context churn Chris Wilson
2016-05-23 11:34 ` [PATCH v2 01/11] drm/i915: Rename struct intel_context Chris Wilson
2016-05-23 11:34 ` [PATCH v2 02/11] drm/i915: Apply lockdep annotations to i915_gem_context.c Chris Wilson
@ 2016-05-23 11:34 ` Chris Wilson
2016-05-23 11:34 ` [PATCH v2 04/11] drm/i915: Rename i915_gem_context_reference/unreference() Chris Wilson
` (11 subsequent siblings)
14 siblings, 0 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-23 11:34 UTC (permalink / raw)
To: intel-gfx
i915_gem_context_get() is a very simple wrapper around idr_find(), so
simple that it would be smaller to do the lookup inline. Also we use the
verb 'lookup' to return a pointer from a handle, freeing 'get' to imply
obtaining a reference to the context.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 17 +++++++++++++++--
drivers/gpu/drm/i915/i915_gem_context.c | 22 ++++------------------
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
3 files changed, 20 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a0f5f00f552f..d162bf86db7e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3417,11 +3417,24 @@ void i915_gem_context_reset(struct drm_device *dev);
int i915_gem_context_open(struct drm_device *dev, struct drm_file *file);
void i915_gem_context_close(struct drm_device *dev, struct drm_file *file);
int i915_switch_context(struct drm_i915_gem_request *req);
-struct i915_gem_context *
-i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id);
void i915_gem_context_free(struct kref *ctx_ref);
struct drm_i915_gem_object *
i915_gem_alloc_context_obj(struct drm_device *dev, size_t size);
+
+static inline struct i915_gem_context *
+i915_gem_context_lookup(struct drm_i915_file_private *file_priv, u32 id)
+{
+ struct i915_gem_context *ctx;
+
+ lockdep_assert_held(&file_priv->dev_priv->dev->struct_mutex);
+
+ ctx = idr_find(&file_priv->context_idr, id);
+ if (!ctx)
+ return ERR_PTR(-ENOENT);
+
+ return ctx;
+}
+
static inline void i915_gem_context_reference(struct i915_gem_context *ctx)
{
kref_get(&ctx->ref);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index af747f14522e..aa329175f73a 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -506,20 +506,6 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file)
idr_destroy(&file_priv->context_idr);
}
-struct i915_gem_context *
-i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id)
-{
- struct i915_gem_context *ctx;
-
- lockdep_assert_held(&file_priv->dev_priv->dev->struct_mutex);
-
- ctx = idr_find(&file_priv->context_idr, id);
- if (!ctx)
- return ERR_PTR(-ENOENT);
-
- return ctx;
-}
-
static inline int
mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags)
{
@@ -951,7 +937,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
if (ret)
return ret;
- ctx = i915_gem_context_get(file_priv, args->ctx_id);
+ ctx = i915_gem_context_lookup(file_priv, args->ctx_id);
if (IS_ERR(ctx)) {
mutex_unlock(&dev->struct_mutex);
return PTR_ERR(ctx);
@@ -977,7 +963,7 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
if (ret)
return ret;
- ctx = i915_gem_context_get(file_priv, args->ctx_id);
+ ctx = i915_gem_context_lookup(file_priv, args->ctx_id);
if (IS_ERR(ctx)) {
mutex_unlock(&dev->struct_mutex);
return PTR_ERR(ctx);
@@ -1020,7 +1006,7 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
if (ret)
return ret;
- ctx = i915_gem_context_get(file_priv, args->ctx_id);
+ ctx = i915_gem_context_lookup(file_priv, args->ctx_id);
if (IS_ERR(ctx)) {
mutex_unlock(&dev->struct_mutex);
return PTR_ERR(ctx);
@@ -1072,7 +1058,7 @@ int i915_gem_context_reset_stats_ioctl(struct drm_device *dev,
if (ret)
return ret;
- ctx = i915_gem_context_get(file->driver_priv, args->ctx_id);
+ ctx = i915_gem_context_lookup(file->driver_priv, args->ctx_id);
if (IS_ERR(ctx)) {
mutex_unlock(&dev->struct_mutex);
return PTR_ERR(ctx);
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index e61b92d7b0bc..84d990331abd 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1075,7 +1075,7 @@ i915_gem_validate_context(struct drm_device *dev, struct drm_file *file,
if (engine->id != RCS && ctx_id != DEFAULT_CONTEXT_HANDLE)
return ERR_PTR(-EINVAL);
- ctx = i915_gem_context_get(file->driver_priv, ctx_id);
+ ctx = i915_gem_context_lookup(file->driver_priv, ctx_id);
if (IS_ERR(ctx))
return ctx;
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 04/11] drm/i915: Rename i915_gem_context_reference/unreference()
2016-05-23 11:34 Context churn Chris Wilson
` (2 preceding siblings ...)
2016-05-23 11:34 ` [PATCH v2 03/11] drm/i915: Rename and inline i915_gem_context_get() Chris Wilson
@ 2016-05-23 11:34 ` Chris Wilson
2016-05-24 11:21 ` Chris Wilson
2016-05-23 11:34 ` [PATCH v2 05/11] drm/i915: Name the inner most per-engine intel_context struct Chris Wilson
` (10 subsequent siblings)
14 siblings, 1 reply; 42+ messages in thread
From: Chris Wilson @ 2016-05-23 11:34 UTC (permalink / raw)
To: intel-gfx
As these are wrappers around kref_get/kref_put() it is preferable to
follow the naming convention and use the same verb get/put in our
wrapper names for manipulating a reference to the context.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 6 ++++--
drivers/gpu/drm/i915/i915_gem.c | 7 +++----
drivers/gpu/drm/i915/i915_gem_context.c | 22 ++++++++++------------
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 +++---
drivers/gpu/drm/i915/intel_lrc.c | 4 ++--
5 files changed, 22 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d162bf86db7e..ce5e92ce9652 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3435,12 +3435,14 @@ i915_gem_context_lookup(struct drm_i915_file_private *file_priv, u32 id)
return ctx;
}
-static inline void i915_gem_context_reference(struct i915_gem_context *ctx)
+static inline struct i915_gem_context *
+i915_gem_context_get(struct i915_gem_context *ctx)
{
kref_get(&ctx->ref);
+ return ctx;
}
-static inline void i915_gem_context_unreference(struct i915_gem_context *ctx)
+static inline void i915_gem_context_put(struct i915_gem_context *ctx)
{
lockdep_assert_held(&ctx->i915->dev->struct_mutex);
kref_put(&ctx->ref, i915_gem_context_free);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 728b66911840..d7ae030e5a0b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1418,7 +1418,7 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
request->engine);
}
- i915_gem_context_unreference(request->ctx);
+ i915_gem_context_put(request->ctx);
i915_gem_request_unreference(request);
}
@@ -2775,8 +2775,7 @@ __i915_gem_request_alloc(struct intel_engine_cs *engine,
req->i915 = dev_priv;
req->engine = engine;
req->reset_counter = reset_counter;
- req->ctx = ctx;
- i915_gem_context_reference(req->ctx);
+ req->ctx = i915_gem_context_get(ctx);
/*
* Reserve space in the ring buffer for all the commands required to
@@ -2798,7 +2797,7 @@ __i915_gem_request_alloc(struct intel_engine_cs *engine,
return 0;
err_ctx:
- i915_gem_context_unreference(ctx);
+ i915_gem_context_put(ctx);
err:
kmem_cache_free(dev_priv->requests, req);
return ret;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index aa329175f73a..a4c6377eea32 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -290,7 +290,7 @@ __create_hw_context(struct drm_device *dev,
return ctx;
err_out:
- i915_gem_context_unreference(ctx);
+ i915_gem_context_put(ctx);
return ERR_PTR(ret);
}
@@ -351,7 +351,7 @@ err_unpin:
i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
err_destroy:
idr_remove(&file_priv->context_idr, ctx->user_handle);
- i915_gem_context_unreference(ctx);
+ i915_gem_context_put(ctx);
return ERR_PTR(ret);
}
@@ -363,7 +363,7 @@ static void i915_gem_context_unpin(struct i915_gem_context *ctx,
} else {
if (engine->id == RCS && ctx->legacy_hw_ctx.rcs_state)
i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
- i915_gem_context_unreference(ctx);
+ i915_gem_context_put(ctx);
}
}
@@ -463,7 +463,7 @@ void i915_gem_context_fini(struct drm_device *dev)
if (dctx->legacy_hw_ctx.rcs_state)
i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
- i915_gem_context_unreference(dctx);
+ i915_gem_context_put(dctx);
dev_priv->kernel_context = NULL;
ida_destroy(&dev_priv->context_hw_ida);
@@ -473,7 +473,7 @@ static int context_idr_cleanup(int id, void *p, void *data)
{
struct i915_gem_context *ctx = p;
- i915_gem_context_unreference(ctx);
+ i915_gem_context_put(ctx);
return 0;
}
@@ -785,10 +785,9 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
/* obj is kept alive until the next request by its active ref */
i915_gem_object_ggtt_unpin(from->legacy_hw_ctx.rcs_state);
- i915_gem_context_unreference(from);
+ i915_gem_context_put(from);
}
- i915_gem_context_reference(to);
- engine->last_context = to;
+ engine->last_context = i915_gem_context_get(to);
/* GEN8 does *not* require an explicit reload if the PDPs have been
* setup, and we do not wish to move them.
@@ -873,10 +872,9 @@ int i915_switch_context(struct drm_i915_gem_request *req)
}
if (to != engine->last_context) {
- i915_gem_context_reference(to);
if (engine->last_context)
- i915_gem_context_unreference(engine->last_context);
- engine->last_context = to;
+ i915_gem_context_put(engine->last_context);
+ engine->last_context = i915_gem_context_get(to);
}
return 0;
@@ -944,7 +942,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
}
idr_remove(&ctx->file_priv->context_idr, ctx->user_handle);
- i915_gem_context_unreference(ctx);
+ i915_gem_context_put(ctx);
mutex_unlock(&dev->struct_mutex);
DRM_DEBUG_DRIVER("HW context %d destroyed\n", args->ctx_id);
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 84d990331abd..14628ce91273 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1492,7 +1492,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
goto pre_mutex_err;
}
- i915_gem_context_reference(ctx);
+ i915_gem_context_get(ctx);
if (ctx->ppgtt)
vm = &ctx->ppgtt->base;
@@ -1503,7 +1503,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
eb = eb_create(args);
if (eb == NULL) {
- i915_gem_context_unreference(ctx);
+ i915_gem_context_put(ctx);
mutex_unlock(&dev->struct_mutex);
ret = -ENOMEM;
goto pre_mutex_err;
@@ -1647,7 +1647,7 @@ err_batch_unpin:
err:
/* the request owns the ref now */
- i915_gem_context_unreference(ctx);
+ i915_gem_context_put(ctx);
eb_destroy(eb);
mutex_unlock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 426dc90aceed..9a55478e23ac 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -966,7 +966,6 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
if (ret)
goto unpin_map;
- i915_gem_context_reference(ctx);
ctx->engine[engine->id].lrc_vma = i915_gem_obj_to_ggtt(ctx_obj);
intel_lr_context_descriptor_update(ctx, engine);
lrc_reg_state[CTX_RING_BUFFER_START+1] = ringbuf->vma->node.start;
@@ -977,6 +976,7 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
if (i915.enable_guc_submission)
I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
+ i915_gem_context_get(ctx);
return 0;
unpin_map:
@@ -1009,7 +1009,7 @@ void intel_lr_context_unpin(struct i915_gem_context *ctx,
ctx->engine[engine->id].lrc_desc = 0;
ctx->engine[engine->id].lrc_reg_state = NULL;
- i915_gem_context_unreference(ctx);
+ i915_gem_context_put(ctx);
}
static int intel_logical_ring_workarounds_emit(struct drm_i915_gem_request *req)
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 05/11] drm/i915: Name the inner most per-engine intel_context struct
2016-05-23 11:34 Context churn Chris Wilson
` (3 preceding siblings ...)
2016-05-23 11:34 ` [PATCH v2 04/11] drm/i915: Rename i915_gem_context_reference/unreference() Chris Wilson
@ 2016-05-23 11:34 ` Chris Wilson
2016-05-23 11:34 ` [PATCH v2 06/11] drm/i915: Move pinning of dev_priv->kernel_context into its creator Chris Wilson
` (9 subsequent siblings)
14 siblings, 0 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-23 11:34 UTC (permalink / raw)
To: intel-gfx
We want to give a name to the currently anonymous per-engine struct
inside the context, so that we can assign it to a local variable and
save clumsy typing. The name we have chosen is intel_context as it
reflects the HW facing portion of the context state (the logical context
state, the registers, the ringbuffer etc).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Dave Gordon <david.s.gordon@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/i915_guc_submission.c | 15 ++---
drivers/gpu/drm/i915/intel_lrc.c | 90 +++++++++++++++---------------
3 files changed, 51 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ce5e92ce9652..bb62a02f933a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -869,7 +869,7 @@ struct i915_gem_context {
} legacy_hw_ctx;
/* Execlists */
- struct {
+ struct intel_context {
struct drm_i915_gem_object *state;
struct intel_ringbuffer *ringbuf;
int pin_count;
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index a292672f36d5..c29c1d19764f 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -363,7 +363,6 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
struct i915_gem_context *ctx = client->owner;
struct guc_context_desc desc;
struct sg_table *sg;
- enum intel_engine_id id;
u32 gfx_addr;
memset(&desc, 0, sizeof(desc));
@@ -373,10 +372,10 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
desc.priority = client->priority;
desc.db_id = client->doorbell_id;
- for_each_engine_id(engine, dev_priv, id) {
+ for_each_engine(engine, dev_priv) {
+ struct intel_context *ce = &ctx->engine[engine->id];
struct guc_execlist_context *lrc = &desc.lrc[engine->guc_id];
struct drm_i915_gem_object *obj;
- uint64_t ctx_desc;
/* TODO: We have a design issue to be solved here. Only when we
* receive the first batch, we know which engine is used by the
@@ -385,20 +384,18 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
* for now who owns a GuC client. But for future owner of GuC
* client, need to make sure lrc is pinned prior to enter here.
*/
- obj = ctx->engine[id].state;
- if (!obj)
+ if (!ce->state)
break; /* XXX: continue? */
- ctx_desc = intel_lr_context_descriptor(ctx, engine);
- lrc->context_desc = (u32)ctx_desc;
+ lrc->context_desc = lower_32_bits(ce->lrc_desc);
/* The state page is after PPHWSP */
- gfx_addr = i915_gem_obj_ggtt_offset(obj);
+ gfx_addr = i915_gem_obj_ggtt_offset(ce->state);
lrc->ring_lcra = gfx_addr + LRC_STATE_PN * PAGE_SIZE;
lrc->context_id = (client->ctx_index << GUC_ELC_CTXID_OFFSET) |
(engine->guc_id << GUC_ELC_ENGINE_OFFSET);
- obj = ctx->engine[id].ringbuf->obj;
+ obj = ce->ringbuf->obj;
gfx_addr = i915_gem_obj_ggtt_offset(obj);
lrc->ring_begin = gfx_addr;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 9a55478e23ac..4b7c9680b097 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -300,7 +300,7 @@ logical_ring_init_platform_invariants(struct intel_engine_cs *engine)
* descriptor for a pinned context
*
* @ctx: Context to work on
- * @ring: Engine the descriptor will be used with
+ * @engine: Engine the descriptor will be used with
*
* The context descriptor encodes various attributes of a context,
* including its GTT address and some flags. Because it's fairly
@@ -318,16 +318,17 @@ static void
intel_lr_context_descriptor_update(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
+ struct intel_context *ce = &ctx->engine[engine->id];
u64 desc;
BUILD_BUG_ON(MAX_CONTEXT_HW_ID > (1<<GEN8_CTX_ID_WIDTH));
desc = engine->ctx_desc_template; /* bits 0-11 */
- desc |= ctx->engine[engine->id].lrc_vma->node.start + /* bits 12-31 */
- LRC_PPHWSP_PN * PAGE_SIZE;
+ desc |= ce->lrc_vma->node.start + LRC_PPHWSP_PN * PAGE_SIZE;
+ /* bits 12-31 */
desc |= (u64)ctx->hw_id << GEN8_CTX_ID_SHIFT; /* bits 32-52 */
- ctx->engine[engine->id].lrc_desc = desc;
+ ce->lrc_desc = desc;
}
uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx,
@@ -674,6 +675,7 @@ static int execlists_move_to_gpu(struct drm_i915_gem_request *req,
int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request)
{
struct intel_engine_cs *engine = request->engine;
+ struct intel_context *ce = &request->ctx->engine[engine->id];
int ret;
/* Flush enough space to reduce the likelihood of waiting after
@@ -682,13 +684,13 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request
*/
request->reserved_space += EXECLISTS_REQUEST_SIZE;
- if (request->ctx->engine[engine->id].state == NULL) {
+ if (!ce->state) {
ret = execlists_context_deferred_alloc(request->ctx, engine);
if (ret)
return ret;
}
- request->ringbuf = request->ctx->engine[engine->id].ringbuf;
+ request->ringbuf = ce->ringbuf;
if (i915.enable_guc_submission) {
/*
@@ -711,12 +713,12 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request
if (ret)
goto err_unpin;
- if (!request->ctx->engine[engine->id].initialised) {
+ if (!ce->initialised) {
ret = engine->init_context(request);
if (ret)
goto err_unpin;
- request->ctx->engine[engine->id].initialised = true;
+ ce->initialised = true;
}
/* Note that after this point, we have committed to using
@@ -936,24 +938,22 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
struct drm_i915_private *dev_priv = ctx->i915;
- struct drm_i915_gem_object *ctx_obj;
- struct intel_ringbuffer *ringbuf;
+ struct intel_context *ce = &ctx->engine[engine->id];
void *vaddr;
u32 *lrc_reg_state;
int ret;
lockdep_assert_held(&ctx->i915->dev->struct_mutex);
- if (ctx->engine[engine->id].pin_count++)
+ if (ce->pin_count++)
return 0;
- ctx_obj = ctx->engine[engine->id].state;
- ret = i915_gem_obj_ggtt_pin(ctx_obj, GEN8_LR_CONTEXT_ALIGN,
- PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
+ ret = i915_gem_obj_ggtt_pin(ce->state, GEN8_LR_CONTEXT_ALIGN,
+ PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
if (ret)
goto err;
- vaddr = i915_gem_object_pin_map(ctx_obj);
+ vaddr = i915_gem_object_pin_map(ce->state);
if (IS_ERR(vaddr)) {
ret = PTR_ERR(vaddr);
goto unpin_ctx_obj;
@@ -961,16 +961,16 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
lrc_reg_state = vaddr + LRC_STATE_PN * PAGE_SIZE;
- ringbuf = ctx->engine[engine->id].ringbuf;
- ret = intel_pin_and_map_ringbuffer_obj(dev_priv, ringbuf);
+ ret = intel_pin_and_map_ringbuffer_obj(dev_priv, ce->ringbuf);
if (ret)
goto unpin_map;
- ctx->engine[engine->id].lrc_vma = i915_gem_obj_to_ggtt(ctx_obj);
+ ce->lrc_vma = i915_gem_obj_to_ggtt(ce->state);
intel_lr_context_descriptor_update(ctx, engine);
- lrc_reg_state[CTX_RING_BUFFER_START+1] = ringbuf->vma->node.start;
- ctx->engine[engine->id].lrc_reg_state = lrc_reg_state;
- ctx_obj->dirty = true;
+
+ lrc_reg_state[CTX_RING_BUFFER_START+1] = ce->ringbuf->vma->node.start;
+ ce->lrc_reg_state = lrc_reg_state;
+ ce->state->dirty = true;
/* Invalidate GuC TLB. */
if (i915.enable_guc_submission)
@@ -980,34 +980,33 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
return 0;
unpin_map:
- i915_gem_object_unpin_map(ctx_obj);
+ i915_gem_object_unpin_map(ce->state);
unpin_ctx_obj:
- i915_gem_object_ggtt_unpin(ctx_obj);
+ i915_gem_object_ggtt_unpin(ce->state);
err:
- ctx->engine[engine->id].pin_count = 0;
+ ce->pin_count = 0;
return ret;
}
void intel_lr_context_unpin(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
- struct drm_i915_gem_object *ctx_obj;
+ struct intel_context *ce = &ctx->engine[engine->id];
lockdep_assert_held(&ctx->i915->dev->struct_mutex);
- GEM_BUG_ON(ctx->engine[engine->id].pin_count == 0);
+ GEM_BUG_ON(ce->pin_count == 0);
- if (--ctx->engine[engine->id].pin_count)
+ if (--ce->pin_count)
return;
- intel_unpin_ringbuffer_obj(ctx->engine[engine->id].ringbuf);
+ intel_unpin_ringbuffer_obj(ce->ringbuf);
- ctx_obj = ctx->engine[engine->id].state;
- i915_gem_object_unpin_map(ctx_obj);
- i915_gem_object_ggtt_unpin(ctx_obj);
+ i915_gem_object_unpin_map(ce->state);
+ i915_gem_object_ggtt_unpin(ce->state);
- ctx->engine[engine->id].lrc_vma = NULL;
- ctx->engine[engine->id].lrc_desc = 0;
- ctx->engine[engine->id].lrc_reg_state = NULL;
+ ce->lrc_vma = NULL;
+ ce->lrc_desc = 0;
+ ce->lrc_reg_state = NULL;
i915_gem_context_put(ctx);
}
@@ -2493,12 +2492,13 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
struct drm_i915_gem_object *ctx_obj;
+ struct intel_context *ce = &ctx->engine[engine->id];
uint32_t context_size;
struct intel_ringbuffer *ringbuf;
int ret;
WARN_ON(ctx->legacy_hw_ctx.rcs_state != NULL);
- WARN_ON(ctx->engine[engine->id].state);
+ WARN_ON(ce->state);
context_size = round_up(intel_lr_context_size(engine), 4096);
@@ -2523,9 +2523,9 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
goto error_ringbuf;
}
- ctx->engine[engine->id].ringbuf = ringbuf;
- ctx->engine[engine->id].state = ctx_obj;
- ctx->engine[engine->id].initialised = engine->init_context == NULL;
+ ce->ringbuf = ringbuf;
+ ce->state = ctx_obj;
+ ce->initialised = engine->init_context == NULL;
return 0;
@@ -2533,8 +2533,8 @@ error_ringbuf:
intel_ringbuffer_free(ringbuf);
error_deref_obj:
drm_gem_object_unreference(&ctx_obj->base);
- ctx->engine[engine->id].ringbuf = NULL;
- ctx->engine[engine->id].state = NULL;
+ ce->ringbuf = NULL;
+ ce->state = NULL;
return ret;
}
@@ -2544,10 +2544,8 @@ void intel_lr_context_reset(struct drm_i915_private *dev_priv,
struct intel_engine_cs *engine;
for_each_engine(engine, dev_priv) {
- struct drm_i915_gem_object *ctx_obj =
- ctx->engine[engine->id].state;
- struct intel_ringbuffer *ringbuf =
- ctx->engine[engine->id].ringbuf;
+ struct intel_context *ce = &ctx->engine[engine->id];
+ struct drm_i915_gem_object *ctx_obj = ce->state;
void *vaddr;
uint32_t *reg_state;
@@ -2566,7 +2564,7 @@ void intel_lr_context_reset(struct drm_i915_private *dev_priv,
i915_gem_object_unpin_map(ctx_obj);
- ringbuf->head = 0;
- ringbuf->tail = 0;
+ ce->ringbuf->head = 0;
+ ce->ringbuf->tail = 0;
}
}
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 06/11] drm/i915: Move pinning of dev_priv->kernel_context into its creator
2016-05-23 11:34 Context churn Chris Wilson
` (4 preceding siblings ...)
2016-05-23 11:34 ` [PATCH v2 05/11] drm/i915: Name the inner most per-engine intel_context struct Chris Wilson
@ 2016-05-23 11:34 ` Chris Wilson
2016-05-23 12:09 ` Tvrtko Ursulin
2016-05-25 9:25 ` Joonas Lahtinen
2016-05-23 11:34 ` [PATCH v2 07/11] drm/i915: Show i915_gem_context owner in debugfs Chris Wilson
` (8 subsequent siblings)
14 siblings, 2 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-23 11:34 UTC (permalink / raw)
To: intel-gfx
Rather than have every context ask "am I owned by the kernel? pin!",
move that logic into the creator of the kernel context, in order to
improve code comprehension.
v2: Throw away the user_handle on failure to allocate the ppgtt.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/i915_gem_context.c | 53 +++++++++++++++------------------
1 file changed, 24 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index a4c6377eea32..474174c20be2 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -303,9 +303,7 @@ static struct i915_gem_context *
i915_gem_create_context(struct drm_device *dev,
struct drm_i915_file_private *file_priv)
{
- const bool is_global_default_ctx = file_priv == NULL;
struct i915_gem_context *ctx;
- int ret = 0;
BUG_ON(!mutex_is_locked(&dev->struct_mutex));
@@ -313,30 +311,15 @@ i915_gem_create_context(struct drm_device *dev,
if (IS_ERR(ctx))
return ctx;
- if (is_global_default_ctx && ctx->legacy_hw_ctx.rcs_state) {
- /* We may need to do things with the shrinker which
- * require us to immediately switch back to the default
- * context. This can cause a problem as pinning the
- * default context also requires GTT space which may not
- * be available. To avoid this we always pin the default
- * context.
- */
- ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
- get_context_alignment(to_i915(dev)), 0);
- if (ret) {
- DRM_DEBUG_DRIVER("Couldn't pin %d\n", ret);
- goto err_destroy;
- }
- }
-
if (USES_FULL_PPGTT(dev)) {
struct i915_hw_ppgtt *ppgtt = i915_ppgtt_create(dev, file_priv);
- if (IS_ERR_OR_NULL(ppgtt)) {
+ if (IS_ERR(ppgtt)) {
DRM_DEBUG_DRIVER("PPGTT setup failed (%ld)\n",
PTR_ERR(ppgtt));
- ret = PTR_ERR(ppgtt);
- goto err_unpin;
+ idr_remove(&file_priv->context_idr, ctx->user_handle);
+ i915_gem_context_put(ctx);
+ return ERR_CAST(ppgtt);
}
ctx->ppgtt = ppgtt;
@@ -345,14 +328,6 @@ i915_gem_create_context(struct drm_device *dev,
trace_i915_context_create(ctx);
return ctx;
-
-err_unpin:
- if (is_global_default_ctx && ctx->legacy_hw_ctx.rcs_state)
- i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
-err_destroy:
- idr_remove(&file_priv->context_idr, ctx->user_handle);
- i915_gem_context_put(ctx);
- return ERR_PTR(ret);
}
static void i915_gem_context_unpin(struct i915_gem_context *ctx,
@@ -426,6 +401,26 @@ int i915_gem_context_init(struct drm_device *dev)
return PTR_ERR(ctx);
}
+ if (ctx->legacy_hw_ctx.rcs_state) {
+ int ret;
+
+ /* We may need to do things with the shrinker which
+ * require us to immediately switch back to the default
+ * context. This can cause a problem as pinning the
+ * default context also requires GTT space which may not
+ * be available. To avoid this we always pin the default
+ * context.
+ */
+ ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
+ get_context_alignment(dev_priv), 0);
+ if (ret) {
+ DRM_ERROR("Failed to pinned default global context (error %d)\n",
+ ret);
+ i915_gem_context_put(ctx);
+ return ret;
+ }
+ }
+
dev_priv->kernel_context = ctx;
DRM_DEBUG_DRIVER("%s context support initialized\n",
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 07/11] drm/i915: Show i915_gem_context owner in debugfs
2016-05-23 11:34 Context churn Chris Wilson
` (5 preceding siblings ...)
2016-05-23 11:34 ` [PATCH v2 06/11] drm/i915: Move pinning of dev_priv->kernel_context into its creator Chris Wilson
@ 2016-05-23 11:34 ` Chris Wilson
2016-05-23 12:18 ` Tvrtko Ursulin
2016-05-25 9:18 ` Joonas Lahtinen
2016-05-23 11:34 ` [PATCH v2 08/11] drm/i915: Put the kernel_context in drm_i915_private next to its friends Chris Wilson
` (7 subsequent siblings)
14 siblings, 2 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-23 11:34 UTC (permalink / raw)
To: intel-gfx
Print the context's owner (via the pid under file_priv) under debugfs.
In doing so, we must be careful that the filp is not accessed after it
is freed (notified via i915_gem_context_close).
v2: Mark the file_priv as closed.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 17 +++++++++++++++--
drivers/gpu/drm/i915/i915_gem_context.c | 3 ++-
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index ae28e6e9d603..945fe4710b37 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2014,9 +2014,22 @@ static int i915_context_status(struct seq_file *m, void *unused)
continue;
seq_printf(m, "HW context %u ", ctx->hw_id);
+ if (IS_ERR(ctx->file_priv)) {
+ seq_puts(m, "(deleted) ");
+ } else if (ctx->file_priv) {
+ struct pid *pid = ctx->file_priv->file->pid;
+ struct task_struct *task;
+
+ task = get_pid_task(pid, PIDTYPE_PID);
+ if (task) {
+ seq_printf(m, "(%s [%d]) ",
+ task->comm, task->pid);
+ put_task_struct(task);
+ }
+ } else
+ seq_puts(m, "(kernel) ");
+
describe_ctx(m, ctx);
- if (ctx == dev_priv->kernel_context)
- seq_printf(m, "(kernel context) ");
if (i915.enable_execlists) {
seq_putc(m, '\n');
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 474174c20be2..e552c936d262 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -468,6 +468,7 @@ static int context_idr_cleanup(int id, void *p, void *data)
{
struct i915_gem_context *ctx = p;
+ ctx->file_priv = ERR_PTR(-EBADF);
i915_gem_context_put(ctx);
return 0;
}
@@ -936,7 +937,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
return PTR_ERR(ctx);
}
- idr_remove(&ctx->file_priv->context_idr, ctx->user_handle);
+ idr_remove(&file_priv->context_idr, ctx->user_handle);
i915_gem_context_put(ctx);
mutex_unlock(&dev->struct_mutex);
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 08/11] drm/i915: Put the kernel_context in drm_i915_private next to its friends
2016-05-23 11:34 Context churn Chris Wilson
` (6 preceding siblings ...)
2016-05-23 11:34 ` [PATCH v2 07/11] drm/i915: Show i915_gem_context owner in debugfs Chris Wilson
@ 2016-05-23 11:34 ` Chris Wilson
2016-05-23 11:34 ` [PATCH v2 09/11] drm/i915: Merge legacy+execlists context structs Chris Wilson
` (6 subsequent siblings)
14 siblings, 0 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-23 11:34 UTC (permalink / raw)
To: intel-gfx
Just move the kernel_context member of drm_i915_private next to the
engines it is associated with.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 3 +--
drivers/gpu/drm/i915/i915_guc_submission.c | 5 +++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bb62a02f933a..47b20069851d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1760,6 +1760,7 @@ struct drm_i915_private {
wait_queue_head_t gmbus_wait_queue;
struct pci_dev *bridge_dev;
+ struct i915_gem_context *kernel_context;
struct intel_engine_cs engine[I915_NUM_ENGINES];
struct drm_i915_gem_object *semaphore_obj;
uint32_t last_seqno, next_seqno;
@@ -2013,8 +2014,6 @@ struct drm_i915_private {
void (*stop_engine)(struct intel_engine_cs *engine);
} gt;
- struct i915_gem_context *kernel_context;
-
/* perform PHY state sanity checks? */
bool chv_phy_assert[2];
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index c29c1d19764f..78b70526c197 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -912,11 +912,12 @@ int i915_guc_submission_enable(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_guc *guc = &dev_priv->guc;
- struct i915_gem_context *ctx = dev_priv->kernel_context;
struct i915_guc_client *client;
/* client for execbuf submission */
- client = guc_client_alloc(dev, GUC_CTX_PRIORITY_KMD_NORMAL, ctx);
+ client = guc_client_alloc(dev,
+ GUC_CTX_PRIORITY_KMD_NORMAL,
+ dev_priv->kernel_context);
if (!client) {
DRM_ERROR("Failed to create execbuf guc_client\n");
return -ENOMEM;
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 09/11] drm/i915: Merge legacy+execlists context structs
2016-05-23 11:34 Context churn Chris Wilson
` (7 preceding siblings ...)
2016-05-23 11:34 ` [PATCH v2 08/11] drm/i915: Put the kernel_context in drm_i915_private next to its friends Chris Wilson
@ 2016-05-23 11:34 ` Chris Wilson
2016-05-23 11:34 ` [PATCH v2 10/11] drm/i915: Rearrange i915_gem_context Chris Wilson
` (5 subsequent siblings)
14 siblings, 0 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-23 11:34 UTC (permalink / raw)
To: intel-gfx
struct intel_context contains two substructs, one for the legacy RCS and
one for every execlists engine. Since legacy RCS is a subset of the
execlists engine support, just combine the two substructs.
v2: Only pin the default context for legacy mode (the object only exists
for legacy, but adding i915.enable_execlists provides symmetry with the
cleanup functions).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 41 +++++-------------
drivers/gpu/drm/i915/i915_drv.h | 7 ---
drivers/gpu/drm/i915/i915_gem_context.c | 75 +++++++++++++++++++--------------
drivers/gpu/drm/i915/intel_lrc.c | 26 ------------
drivers/gpu/drm/i915/intel_lrc.h | 1 -
5 files changed, 55 insertions(+), 95 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 945fe4710b37..30cb26fe2fa9 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -199,13 +199,6 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
seq_printf(m, " (frontbuffer: 0x%03x)", obj->frontbuffer_bits);
}
-static void describe_ctx(struct seq_file *m, struct i915_gem_context *ctx)
-{
- seq_putc(m, ctx->legacy_hw_ctx.initialized ? 'I' : 'i');
- seq_putc(m, ctx->remap_slice ? 'R' : 'r');
- seq_putc(m, ' ');
-}
-
static int i915_gem_object_list_info(struct seq_file *m, void *data)
{
struct drm_info_node *node = m->private;
@@ -2001,7 +1994,6 @@ static int i915_context_status(struct seq_file *m, void *unused)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_engine_cs *engine;
struct i915_gem_context *ctx;
- enum intel_engine_id id;
int ret;
ret = mutex_lock_interruptible(&dev->struct_mutex);
@@ -2009,10 +2001,6 @@ static int i915_context_status(struct seq_file *m, void *unused)
return ret;
list_for_each_entry(ctx, &dev_priv->context_list, link) {
- if (!i915.enable_execlists &&
- ctx->legacy_hw_ctx.rcs_state == NULL)
- continue;
-
seq_printf(m, "HW context %u ", ctx->hw_id);
if (IS_ERR(ctx->file_priv)) {
seq_puts(m, "(deleted) ");
@@ -2029,25 +2017,18 @@ static int i915_context_status(struct seq_file *m, void *unused)
} else
seq_puts(m, "(kernel) ");
- describe_ctx(m, ctx);
+ seq_putc(m, ctx->remap_slice ? 'R' : 'r');
+ seq_putc(m, '\n');
- if (i915.enable_execlists) {
+ for_each_engine(engine, dev_priv) {
+ struct intel_context *ce = &ctx->engine[engine->id];
+ seq_printf(m, "%s: ", engine->name);
+ seq_putc(m, ce->initialised ? 'I' : 'i');
+ if (ce->state)
+ describe_obj(m, ce->state);
+ if (ce->ringbuf)
+ describe_ctx_ringbuf(m, ce->ringbuf);
seq_putc(m, '\n');
- for_each_engine_id(engine, dev_priv, id) {
- struct drm_i915_gem_object *ctx_obj =
- ctx->engine[id].state;
- struct intel_ringbuffer *ringbuf =
- ctx->engine[id].ringbuf;
-
- seq_printf(m, "%s: ", engine->name);
- if (ctx_obj)
- describe_obj(m, ctx_obj);
- if (ringbuf)
- describe_ctx_ringbuf(m, ringbuf);
- seq_putc(m, '\n');
- }
- } else {
- describe_obj(m, ctx->legacy_hw_ctx.rcs_state);
}
seq_putc(m, '\n');
@@ -2062,10 +2043,10 @@ static void i915_dump_lrc_obj(struct seq_file *m,
struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
+ struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state;
struct page *page;
uint32_t *reg_state;
int j;
- struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state;
unsigned long ggtt_offset = 0;
seq_printf(m, "CONTEXT: %s %u\n", engine->name, ctx->hw_id);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 47b20069851d..9d7a088efc16 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -862,13 +862,6 @@ struct i915_gem_context {
/* Unique identifier for this context, used by the hw for tracking */
unsigned hw_id;
- /* Legacy ring buffer submission */
- struct {
- struct drm_i915_gem_object *rcs_state;
- bool initialized;
- } legacy_hw_ctx;
-
- /* Execlists */
struct intel_context {
struct drm_i915_gem_object *state;
struct intel_ringbuffer *ringbuf;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index e552c936d262..706f33f721aa 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -152,13 +152,11 @@ static void i915_gem_context_clean(struct i915_gem_context *ctx)
void i915_gem_context_free(struct kref *ctx_ref)
{
struct i915_gem_context *ctx = container_of(ctx_ref, typeof(*ctx), ref);
+ int i;
lockdep_assert_held(&ctx->i915->dev->struct_mutex);
trace_i915_context_free(ctx);
- if (i915.enable_execlists)
- intel_lr_context_free(ctx);
-
/*
* This context is going away and we need to remove all VMAs still
* around. This is to handle imported shared objects for which
@@ -168,8 +166,19 @@ void i915_gem_context_free(struct kref *ctx_ref)
i915_ppgtt_put(ctx->ppgtt);
- if (ctx->legacy_hw_ctx.rcs_state)
- drm_gem_object_unreference(&ctx->legacy_hw_ctx.rcs_state->base);
+ for (i = 0; i < I915_NUM_ENGINES; i++) {
+ struct intel_context *ce = &ctx->engine[i];
+
+ if (!ce->state)
+ continue;
+
+ WARN_ON(ce->pin_count);
+ if (ce->ringbuf)
+ intel_ringbuffer_free(ce->ringbuf);
+
+ drm_gem_object_unreference(&ce->state->base);
+ }
+
list_del(&ctx->link);
ida_simple_remove(&ctx->i915->context_hw_ida, ctx->hw_id);
@@ -266,7 +275,7 @@ __create_hw_context(struct drm_device *dev,
ret = PTR_ERR(obj);
goto err_out;
}
- ctx->legacy_hw_ctx.rcs_state = obj;
+ ctx->engine[RCS].state = obj;
}
/* Default context will never have a file_priv */
@@ -336,8 +345,11 @@ static void i915_gem_context_unpin(struct i915_gem_context *ctx,
if (i915.enable_execlists) {
intel_lr_context_unpin(ctx, engine);
} else {
- if (engine->id == RCS && ctx->legacy_hw_ctx.rcs_state)
- i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
+ struct intel_context *ce = &ctx->engine[engine->id];
+
+ if (ce->state)
+ i915_gem_object_ggtt_unpin(ce->state);
+
i915_gem_context_put(ctx);
}
}
@@ -401,7 +413,7 @@ int i915_gem_context_init(struct drm_device *dev)
return PTR_ERR(ctx);
}
- if (ctx->legacy_hw_ctx.rcs_state) {
+ if (!i915.enable_execlists && ctx->engine[RCS].state) {
int ret;
/* We may need to do things with the shrinker which
@@ -411,7 +423,7 @@ int i915_gem_context_init(struct drm_device *dev)
* be available. To avoid this we always pin the default
* context.
*/
- ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
+ ret = i915_gem_obj_ggtt_pin(ctx->engine[RCS].state,
get_context_alignment(dev_priv), 0);
if (ret) {
DRM_ERROR("Failed to pinned default global context (error %d)\n",
@@ -436,15 +448,17 @@ void i915_gem_context_lost(struct drm_i915_private *dev_priv)
lockdep_assert_held(&dev_priv->dev->struct_mutex);
for_each_engine(engine, dev_priv) {
- if (engine->last_context == NULL)
- continue;
+ if (engine->last_context) {
+ i915_gem_context_unpin(engine->last_context, engine);
+ engine->last_context = NULL;
+ }
- i915_gem_context_unpin(engine->last_context, engine);
- engine->last_context = NULL;
+ /* Force the GPU state to be reinitialised on enabling */
+ dev_priv->kernel_context->engine[engine->id].initialised =
+ engine->init_context == NULL;
}
/* Force the GPU state to be reinitialised on enabling */
- dev_priv->kernel_context->legacy_hw_ctx.initialized = false;
dev_priv->kernel_context->remap_slice = ALL_L3_SLICES(dev_priv);
}
@@ -455,8 +469,8 @@ void i915_gem_context_fini(struct drm_device *dev)
lockdep_assert_held(&dev->struct_mutex);
- if (dctx->legacy_hw_ctx.rcs_state)
- i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
+ if (!i915.enable_execlists && dctx->engine[RCS].state)
+ i915_gem_object_ggtt_unpin(dctx->engine[RCS].state);
i915_gem_context_put(dctx);
dev_priv->kernel_context = NULL;
@@ -564,7 +578,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags)
intel_ring_emit(engine, MI_NOOP);
intel_ring_emit(engine, MI_SET_CONTEXT);
intel_ring_emit(engine,
- i915_gem_obj_ggtt_offset(req->ctx->legacy_hw_ctx.rcs_state) |
+ i915_gem_obj_ggtt_offset(req->ctx->engine[RCS].state) |
flags);
/*
* w/a: MI_SET_CONTEXT must always be followed by MI_NOOP
@@ -641,7 +655,7 @@ static inline bool skip_rcs_switch(struct i915_hw_ppgtt *ppgtt,
if (to->remap_slice)
return false;
- if (!to->legacy_hw_ctx.initialized)
+ if (!to->engine[RCS].initialised)
return false;
if (ppgtt && (intel_engine_flag(engine) & ppgtt->pd_dirty_rings))
@@ -706,7 +720,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
return 0;
/* Trying to pin first makes error handling easier. */
- ret = i915_gem_obj_ggtt_pin(to->legacy_hw_ctx.rcs_state,
+ ret = i915_gem_obj_ggtt_pin(to->engine[RCS].state,
get_context_alignment(engine->i915),
0);
if (ret)
@@ -729,7 +743,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
*
* XXX: We need a real interface to do this instead of trickery.
*/
- ret = i915_gem_object_set_to_gtt_domain(to->legacy_hw_ctx.rcs_state, false);
+ ret = i915_gem_object_set_to_gtt_domain(to->engine[RCS].state, false);
if (ret)
goto unpin_out;
@@ -744,7 +758,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
goto unpin_out;
}
- if (!to->legacy_hw_ctx.initialized || i915_gem_context_is_default(to))
+ if (!to->engine[RCS].initialised || i915_gem_context_is_default(to))
/* NB: If we inhibit the restore, the context is not allowed to
* die because future work may end up depending on valid address
* space. This means we must enforce that a page table load
@@ -768,8 +782,8 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
* MI_SET_CONTEXT instead of when the next seqno has completed.
*/
if (from != NULL) {
- from->legacy_hw_ctx.rcs_state->base.read_domains = I915_GEM_DOMAIN_INSTRUCTION;
- i915_vma_move_to_active(i915_gem_obj_to_ggtt(from->legacy_hw_ctx.rcs_state), req);
+ from->engine[RCS].state->base.read_domains = I915_GEM_DOMAIN_INSTRUCTION;
+ i915_vma_move_to_active(i915_gem_obj_to_ggtt(from->engine[RCS].state), req);
/* As long as MI_SET_CONTEXT is serializing, ie. it flushes the
* whole damn pipeline, we don't need to explicitly mark the
* object dirty. The only exception is that the context must be
@@ -777,10 +791,10 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
* able to defer doing this until we know the object would be
* swapped, but there is no way to do that yet.
*/
- from->legacy_hw_ctx.rcs_state->dirty = 1;
+ from->engine[RCS].state->dirty = 1;
/* obj is kept alive until the next request by its active ref */
- i915_gem_object_ggtt_unpin(from->legacy_hw_ctx.rcs_state);
+ i915_gem_object_ggtt_unpin(from->engine[RCS].state);
i915_gem_context_put(from);
}
engine->last_context = i915_gem_context_get(to);
@@ -814,19 +828,19 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
to->remap_slice &= ~(1<<i);
}
- if (!to->legacy_hw_ctx.initialized) {
+ if (!to->engine[RCS].initialised) {
if (engine->init_context) {
ret = engine->init_context(req);
if (ret)
return ret;
}
- to->legacy_hw_ctx.initialized = true;
+ to->engine[RCS].initialised = true;
}
return 0;
unpin_out:
- i915_gem_object_ggtt_unpin(to->legacy_hw_ctx.rcs_state);
+ i915_gem_object_ggtt_unpin(to->engine[RCS].state);
return ret;
}
@@ -850,8 +864,7 @@ int i915_switch_context(struct drm_i915_gem_request *req)
WARN_ON(i915.enable_execlists);
lockdep_assert_held(&req->i915->dev->struct_mutex);
- if (engine->id != RCS ||
- req->ctx->legacy_hw_ctx.rcs_state == NULL) {
+ if (!req->ctx->engine[engine->id].state) {
struct i915_gem_context *to = req->ctx;
struct i915_hw_ppgtt *ppgtt =
to->ppgtt ?: req->i915->mm.aliasing_ppgtt;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 4b7c9680b097..558f069cd6d8 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2413,31 +2413,6 @@ populate_lr_context(struct i915_gem_context *ctx,
}
/**
- * intel_lr_context_free() - free the LRC specific bits of a context
- * @ctx: the LR context to free.
- *
- * The real context freeing is done in i915_gem_context_free: this only
- * takes care of the bits that are LRC related: the per-engine backing
- * objects and the logical ringbuffer.
- */
-void intel_lr_context_free(struct i915_gem_context *ctx)
-{
- int i;
-
- for (i = I915_NUM_ENGINES; --i >= 0; ) {
- struct intel_ringbuffer *ringbuf = ctx->engine[i].ringbuf;
- struct drm_i915_gem_object *ctx_obj = ctx->engine[i].state;
-
- if (!ctx_obj)
- continue;
-
- WARN_ON(ctx->engine[i].pin_count);
- intel_ringbuffer_free(ringbuf);
- drm_gem_object_unreference(&ctx_obj->base);
- }
-}
-
-/**
* intel_lr_context_size() - return the size of the context for an engine
* @ring: which engine to find the context size for
*
@@ -2497,7 +2472,6 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
struct intel_ringbuffer *ringbuf;
int ret;
- WARN_ON(ctx->legacy_hw_ctx.rcs_state != NULL);
WARN_ON(ce->state);
context_size = round_up(intel_lr_context_size(engine), 4096);
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index eb2e1d157fed..a8db42a9c50f 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -101,7 +101,6 @@ static inline void intel_logical_ring_emit_reg(struct intel_ringbuffer *ringbuf,
struct i915_gem_context;
-void intel_lr_context_free(struct i915_gem_context *ctx);
uint32_t intel_lr_context_size(struct intel_engine_cs *engine);
void intel_lr_context_unpin(struct i915_gem_context *ctx,
struct intel_engine_cs *engine);
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 10/11] drm/i915: Rearrange i915_gem_context
2016-05-23 11:34 Context churn Chris Wilson
` (8 preceding siblings ...)
2016-05-23 11:34 ` [PATCH v2 09/11] drm/i915: Merge legacy+execlists context structs Chris Wilson
@ 2016-05-23 11:34 ` Chris Wilson
2016-05-25 9:15 ` Joonas Lahtinen
2016-05-23 11:34 ` [PATCH v2 11/11] drm/i915: Show context objects in debugfs/i915_gem_objects Chris Wilson
` (4 subsequent siblings)
14 siblings, 1 reply; 42+ messages in thread
From: Chris Wilson @ 2016-05-23 11:34 UTC (permalink / raw)
To: intel-gfx
Pack the integers and related types together inside the struct.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9d7a088efc16..5dbce15e6e4c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -829,7 +829,6 @@ struct i915_ctx_hang_stats {
/* This must match up with the value previously used for execbuf2.rsvd1. */
#define DEFAULT_CONTEXT_HANDLE 0
-#define CONTEXT_NO_ZEROMAP (1<<0)
/**
* struct i915_gem_context - as the name implies, represents a context.
* @ref: reference count.
@@ -851,28 +850,31 @@ struct i915_ctx_hang_stats {
*/
struct i915_gem_context {
struct kref ref;
- int user_handle;
- uint8_t remap_slice;
struct drm_i915_private *i915;
- int flags;
struct drm_i915_file_private *file_priv;
- struct i915_ctx_hang_stats hang_stats;
struct i915_hw_ppgtt *ppgtt;
+ struct i915_ctx_hang_stats hang_stats;
+
/* Unique identifier for this context, used by the hw for tracking */
unsigned hw_id;
+ uint32_t user_handle;
+ unsigned long flags;
+#define CONTEXT_NO_ZEROMAP (1<<0)
struct intel_context {
struct drm_i915_gem_object *state;
struct intel_ringbuffer *ringbuf;
- int pin_count;
struct i915_vma *lrc_vma;
- u64 lrc_desc;
uint32_t *lrc_reg_state;
+ u64 lrc_desc;
+ int pin_count;
bool initialised;
} engine[I915_NUM_ENGINES];
struct list_head link;
+
+ uint8_t remap_slice;
};
enum fb_op_origin {
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 11/11] drm/i915: Show context objects in debugfs/i915_gem_objects
2016-05-23 11:34 Context churn Chris Wilson
` (9 preceding siblings ...)
2016-05-23 11:34 ` [PATCH v2 10/11] drm/i915: Rearrange i915_gem_context Chris Wilson
@ 2016-05-23 11:34 ` Chris Wilson
2016-05-23 12:31 ` Tvrtko Ursulin
2016-05-25 9:33 ` [PATCH v2 11/11] drm/i915: Show context objects in debugfs/i915_gem_objects Joonas Lahtinen
2016-05-23 12:01 ` ✓ Ro.CI.BAT: success for series starting with [v2,01/11] drm/i915: Rename struct intel_context Patchwork
` (3 subsequent siblings)
14 siblings, 2 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-23 11:34 UTC (permalink / raw)
To: intel-gfx
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 30cb26fe2fa9..3d14eb3215e1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -417,6 +417,40 @@ static void print_batch_pool_stats(struct seq_file *m,
print_file_stats(m, "[k]batch pool", stats);
}
+static int per_file_ctx_stats(int id, void *ptr, void *data)
+{
+ struct i915_gem_context *ctx = ptr;
+ int n;
+
+ for (n = 0; n < ARRAY_SIZE(ctx->engine); n++) {
+ if (ctx->engine[n].state)
+ per_file_stats(0, ctx->engine[n].state, data);
+ if (ctx->engine[n].ringbuf)
+ per_file_stats(0, ctx->engine[n].ringbuf->obj, data);
+ }
+
+ return 0;
+}
+
+static void print_context_stats(struct seq_file *m,
+ struct drm_i915_private *dev_priv)
+{
+ struct file_stats stats;
+ struct drm_file *file;
+
+ memset(&stats, 0, sizeof(stats));
+
+ if (dev_priv->kernel_context)
+ per_file_ctx_stats(0, dev_priv->kernel_context, &stats);
+
+ list_for_each_entry(file, &dev_priv->dev->filelist, lhead) {
+ struct drm_i915_file_private *fpriv = file->driver_priv;
+ idr_for_each(&fpriv->context_idr, per_file_ctx_stats, &stats);
+ }
+
+ print_file_stats(m, "[k]contexts", stats);
+}
+
#define count_vmas(list, member) do { \
list_for_each_entry(vma, list, member) { \
size += i915_gem_obj_total_ggtt_size(vma->obj); \
@@ -521,6 +555,7 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
seq_putc(m, '\n');
print_batch_pool_stats(m, dev_priv);
+ print_context_stats(m, dev_priv);
mutex_unlock(&dev->struct_mutex);
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 42+ messages in thread
* ✓ Ro.CI.BAT: success for series starting with [v2,01/11] drm/i915: Rename struct intel_context
2016-05-23 11:34 Context churn Chris Wilson
` (10 preceding siblings ...)
2016-05-23 11:34 ` [PATCH v2 11/11] drm/i915: Show context objects in debugfs/i915_gem_objects Chris Wilson
@ 2016-05-23 12:01 ` Patchwork
2016-05-23 14:30 ` ✗ Ro.CI.BAT: failure for series starting with [v2,01/11] drm/i915: Rename struct intel_context (rev2) Patchwork
` (2 subsequent siblings)
14 siblings, 0 replies; 42+ messages in thread
From: Patchwork @ 2016-05-23 12:01 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v2,01/11] drm/i915: Rename struct intel_context
URL : https://patchwork.freedesktop.org/series/7564/
State : success
== Summary ==
Series 7564v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/7564/revisions/1/mbox
Test kms_flip:
Subgroup basic-flip-vs-wf_vblank:
fail -> PASS (ro-bdw-i7-5600u)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
dmesg-warn -> PASS (ro-ivb-i7-3770)
Test kms_sink_crc_basic:
skip -> PASS (ro-skl-i7-6700hq)
fi-bdw-i7-5557u total:209 pass:197 dwarn:0 dfail:0 fail:0 skip:12
fi-bsw-n3050 total:209 pass:167 dwarn:0 dfail:0 fail:2 skip:40
fi-hsw-i7-4770k total:209 pass:190 dwarn:0 dfail:0 fail:0 skip:19
fi-hsw-i7-4770r total:209 pass:186 dwarn:0 dfail:0 fail:0 skip:23
fi-skl-i5-6260u total:209 pass:198 dwarn:0 dfail:0 fail:0 skip:11
fi-skl-i7-6700k total:209 pass:184 dwarn:0 dfail:0 fail:0 skip:25
fi-snb-i7-2600 total:209 pass:170 dwarn:0 dfail:0 fail:0 skip:39
ro-bdw-i5-5250u total:209 pass:172 dwarn:0 dfail:0 fail:0 skip:37
ro-bdw-i7-5557U total:209 pass:197 dwarn:0 dfail:0 fail:0 skip:12
ro-bdw-i7-5600u total:209 pass:180 dwarn:0 dfail:0 fail:1 skip:28
ro-bsw-n3050 total:209 pass:168 dwarn:0 dfail:0 fail:2 skip:39
ro-byt-n2820 total:209 pass:169 dwarn:0 dfail:0 fail:3 skip:37
ro-hsw-i3-4010u total:209 pass:186 dwarn:0 dfail:0 fail:0 skip:23
ro-hsw-i7-4770r total:209 pass:186 dwarn:0 dfail:0 fail:0 skip:23
ro-ilk-i7-620lm total:209 pass:146 dwarn:0 dfail:0 fail:1 skip:62
ro-ilk1-i5-650 total:204 pass:146 dwarn:0 dfail:0 fail:1 skip:57
ro-ivb-i7-3770 total:209 pass:177 dwarn:0 dfail:0 fail:0 skip:32
ro-ivb2-i7-3770 total:209 pass:181 dwarn:0 dfail:0 fail:0 skip:28
ro-skl-i7-6700hq total:204 pass:183 dwarn:0 dfail:0 fail:0 skip:21
ro-snb-i7-2620M total:209 pass:170 dwarn:0 dfail:0 fail:1 skip:38
fi-byt-n2820 failed to connect after reboot
Results at /archive/results/CI_IGT_test/RO_Patchwork_970/
eaa075a drm-intel-nightly: 2016y-05m-23d-08h-23m-34s UTC integration manifest
86197c5 drm/i915: Show context objects in debugfs/i915_gem_objects
0b5dacb drm/i915: Rearrange i915_gem_context
039cda6 drm/i915: Merge legacy+execlists context structs
1bc5739 drm/i915: Put the kernel_context in drm_i915_private next to its friends
0db38df drm/i915: Show i915_gem_context owner in debugfs
5646bb7 drm/i915: Move pinning of dev_priv->kernel_context into its creator
a7a13c2 drm/i915: Name the inner most per-engine intel_context struct
07d2480 drm/i915: Rename i915_gem_context_reference/unreference()
0c71a3e drm/i915: Rename and inline i915_gem_context_get()
b987d04 drm/i915: Apply lockdep annotations to i915_gem_context.c
9f66eaa drm/i915: Rename struct intel_context
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 06/11] drm/i915: Move pinning of dev_priv->kernel_context into its creator
2016-05-23 11:34 ` [PATCH v2 06/11] drm/i915: Move pinning of dev_priv->kernel_context into its creator Chris Wilson
@ 2016-05-23 12:09 ` Tvrtko Ursulin
2016-05-23 12:16 ` Chris Wilson
2016-05-25 9:25 ` Joonas Lahtinen
1 sibling, 1 reply; 42+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23 12:09 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 23/05/16 12:34, Chris Wilson wrote:
> Rather than have every context ask "am I owned by the kernel? pin!",
> move that logic into the creator of the kernel context, in order to
> improve code comprehension.
>
> v2: Throw away the user_handle on failure to allocate the ppgtt.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_context.c | 53 +++++++++++++++------------------
> 1 file changed, 24 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index a4c6377eea32..474174c20be2 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -303,9 +303,7 @@ static struct i915_gem_context *
> i915_gem_create_context(struct drm_device *dev,
> struct drm_i915_file_private *file_priv)
> {
> - const bool is_global_default_ctx = file_priv == NULL;
> struct i915_gem_context *ctx;
> - int ret = 0;
>
> BUG_ON(!mutex_is_locked(&dev->struct_mutex));
>
> @@ -313,30 +311,15 @@ i915_gem_create_context(struct drm_device *dev,
> if (IS_ERR(ctx))
> return ctx;
>
> - if (is_global_default_ctx && ctx->legacy_hw_ctx.rcs_state) {
> - /* We may need to do things with the shrinker which
> - * require us to immediately switch back to the default
> - * context. This can cause a problem as pinning the
> - * default context also requires GTT space which may not
> - * be available. To avoid this we always pin the default
> - * context.
> - */
> - ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
> - get_context_alignment(to_i915(dev)), 0);
> - if (ret) {
> - DRM_DEBUG_DRIVER("Couldn't pin %d\n", ret);
> - goto err_destroy;
> - }
> - }
> -
> if (USES_FULL_PPGTT(dev)) {
> struct i915_hw_ppgtt *ppgtt = i915_ppgtt_create(dev, file_priv);
>
> - if (IS_ERR_OR_NULL(ppgtt)) {
> + if (IS_ERR(ppgtt)) {
> DRM_DEBUG_DRIVER("PPGTT setup failed (%ld)\n",
> PTR_ERR(ppgtt));
> - ret = PTR_ERR(ppgtt);
> - goto err_unpin;
> + idr_remove(&file_priv->context_idr, ctx->user_handle);
> + i915_gem_context_put(ctx);
> + return ERR_CAST(ppgtt);
> }
>
> ctx->ppgtt = ppgtt;
> @@ -345,14 +328,6 @@ i915_gem_create_context(struct drm_device *dev,
> trace_i915_context_create(ctx);
>
> return ctx;
> -
> -err_unpin:
> - if (is_global_default_ctx && ctx->legacy_hw_ctx.rcs_state)
> - i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
> -err_destroy:
> - idr_remove(&file_priv->context_idr, ctx->user_handle);
> - i915_gem_context_put(ctx);
> - return ERR_PTR(ret);
> }
>
> static void i915_gem_context_unpin(struct i915_gem_context *ctx,
> @@ -426,6 +401,26 @@ int i915_gem_context_init(struct drm_device *dev)
> return PTR_ERR(ctx);
> }
>
> + if (ctx->legacy_hw_ctx.rcs_state) {
> + int ret;
> +
> + /* We may need to do things with the shrinker which
> + * require us to immediately switch back to the default
> + * context. This can cause a problem as pinning the
> + * default context also requires GTT space which may not
> + * be available. To avoid this we always pin the default
> + * context.
> + */
> + ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
> + get_context_alignment(dev_priv), 0);
> + if (ret) {
> + DRM_ERROR("Failed to pinned default global context (error %d)\n",
> + ret);
idr_remove here as well? Maybe we should split out the idr context API
from the GEM context to make it explicit - create context, export it to
userspace and back?
> + i915_gem_context_put(ctx);
> + return ret;
> + }
> + }
> +
> dev_priv->kernel_context = ctx;
>
> DRM_DEBUG_DRIVER("%s context support initialized\n",
>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 06/11] drm/i915: Move pinning of dev_priv->kernel_context into its creator
2016-05-23 12:09 ` Tvrtko Ursulin
@ 2016-05-23 12:16 ` Chris Wilson
2016-05-23 12:23 ` Tvrtko Ursulin
0 siblings, 1 reply; 42+ messages in thread
From: Chris Wilson @ 2016-05-23 12:16 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
On Mon, May 23, 2016 at 01:09:02PM +0100, Tvrtko Ursulin wrote:
> >@@ -426,6 +401,26 @@ int i915_gem_context_init(struct drm_device *dev)
> > return PTR_ERR(ctx);
> > }
> >
> >+ if (ctx->legacy_hw_ctx.rcs_state) {
> >+ int ret;
> >+
> >+ /* We may need to do things with the shrinker which
> >+ * require us to immediately switch back to the default
> >+ * context. This can cause a problem as pinning the
> >+ * default context also requires GTT space which may not
> >+ * be available. To avoid this we always pin the default
> >+ * context.
> >+ */
> >+ ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
> >+ get_context_alignment(dev_priv), 0);
> >+ if (ret) {
> >+ DRM_ERROR("Failed to pinned default global context (error %d)\n",
> >+ ret);
>
> idr_remove here as well? Maybe we should split out the idr context
> API from the GEM context to make it explicit - create context,
> export it to userspace and back?
No, this one is not in an idr as file_priv is NULL for the kernel
context.
-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] 42+ messages in thread
* Re: [PATCH v2 07/11] drm/i915: Show i915_gem_context owner in debugfs
2016-05-23 11:34 ` [PATCH v2 07/11] drm/i915: Show i915_gem_context owner in debugfs Chris Wilson
@ 2016-05-23 12:18 ` Tvrtko Ursulin
2016-05-25 9:18 ` Joonas Lahtinen
1 sibling, 0 replies; 42+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23 12:18 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 23/05/16 12:34, Chris Wilson wrote:
> Print the context's owner (via the pid under file_priv) under debugfs.
> In doing so, we must be careful that the filp is not accessed after it
> is freed (notified via i915_gem_context_close).
>
> v2: Mark the file_priv as closed.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 17 +++++++++++++++--
> drivers/gpu/drm/i915/i915_gem_context.c | 3 ++-
> 2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index ae28e6e9d603..945fe4710b37 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2014,9 +2014,22 @@ static int i915_context_status(struct seq_file *m, void *unused)
> continue;
>
> seq_printf(m, "HW context %u ", ctx->hw_id);
> + if (IS_ERR(ctx->file_priv)) {
> + seq_puts(m, "(deleted) ");
> + } else if (ctx->file_priv) {
> + struct pid *pid = ctx->file_priv->file->pid;
> + struct task_struct *task;
> +
> + task = get_pid_task(pid, PIDTYPE_PID);
> + if (task) {
> + seq_printf(m, "(%s [%d]) ",
> + task->comm, task->pid);
> + put_task_struct(task);
> + }
> + } else
> + seq_puts(m, "(kernel) ");
> +
> describe_ctx(m, ctx);
> - if (ctx == dev_priv->kernel_context)
> - seq_printf(m, "(kernel context) ");
>
> if (i915.enable_execlists) {
> seq_putc(m, '\n');
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 474174c20be2..e552c936d262 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -468,6 +468,7 @@ static int context_idr_cleanup(int id, void *p, void *data)
> {
> struct i915_gem_context *ctx = p;
>
> + ctx->file_priv = ERR_PTR(-EBADF);
> i915_gem_context_put(ctx);
> return 0;
> }
> @@ -936,7 +937,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
> return PTR_ERR(ctx);
> }
>
> - idr_remove(&ctx->file_priv->context_idr, ctx->user_handle);
> + idr_remove(&file_priv->context_idr, ctx->user_handle);
> i915_gem_context_put(ctx);
> mutex_unlock(&dev->struct_mutex);
>
>
Looks OK.
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 06/11] drm/i915: Move pinning of dev_priv->kernel_context into its creator
2016-05-23 12:16 ` Chris Wilson
@ 2016-05-23 12:23 ` Tvrtko Ursulin
0 siblings, 0 replies; 42+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23 12:23 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 23/05/16 13:16, Chris Wilson wrote:
> On Mon, May 23, 2016 at 01:09:02PM +0100, Tvrtko Ursulin wrote:
>>> @@ -426,6 +401,26 @@ int i915_gem_context_init(struct drm_device *dev)
>>> return PTR_ERR(ctx);
>>> }
>>>
>>> + if (ctx->legacy_hw_ctx.rcs_state) {
>>> + int ret;
>>> +
>>> + /* We may need to do things with the shrinker which
>>> + * require us to immediately switch back to the default
>>> + * context. This can cause a problem as pinning the
>>> + * default context also requires GTT space which may not
>>> + * be available. To avoid this we always pin the default
>>> + * context.
>>> + */
>>> + ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
>>> + get_context_alignment(dev_priv), 0);
>>> + if (ret) {
>>> + DRM_ERROR("Failed to pinned default global context (error %d)\n",
>>> + ret);
>>
>> idr_remove here as well? Maybe we should split out the idr context
>> API from the GEM context to make it explicit - create context,
>> export it to userspace and back?
>
> No, this one is not in an idr as file_priv is NULL for the kernel
> context.
Oh right, you can claim in the commit you fixed a bug as well then! :))
- const bool is_global_default_ctx = file_priv == NULL;
...
- if (is_global_default_ctx && ctx->legacy_hw_ctx.rcs_state) {
...
- goto err_destroy;
...
- idr_remove(&file_priv->context_idr, ctx->user_handle);
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 11/11] drm/i915: Show context objects in debugfs/i915_gem_objects
2016-05-23 11:34 ` [PATCH v2 11/11] drm/i915: Show context objects in debugfs/i915_gem_objects Chris Wilson
@ 2016-05-23 12:31 ` Tvrtko Ursulin
2016-05-23 12:40 ` Chris Wilson
2016-05-23 13:01 ` [PATCH v3] drm/i915/debugfs: Show context objects in i915_gem_objects Chris Wilson
2016-05-25 9:33 ` [PATCH v2 11/11] drm/i915: Show context objects in debugfs/i915_gem_objects Joonas Lahtinen
1 sibling, 2 replies; 42+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23 12:31 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 23/05/16 12:34, Chris Wilson wrote:
Blah blah blah, this, for that, etc... commit message ofc. :)
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 30cb26fe2fa9..3d14eb3215e1 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -417,6 +417,40 @@ static void print_batch_pool_stats(struct seq_file *m,
> print_file_stats(m, "[k]batch pool", stats);
> }
>
> +static int per_file_ctx_stats(int id, void *ptr, void *data)
> +{
> + struct i915_gem_context *ctx = ptr;
> + int n;
> +
> + for (n = 0; n < ARRAY_SIZE(ctx->engine); n++) {
> + if (ctx->engine[n].state)
> + per_file_stats(0, ctx->engine[n].state, data);
> + if (ctx->engine[n].ringbuf)
> + per_file_stats(0, ctx->engine[n].ringbuf->obj, data);
> + }
> +
> + return 0;
> +}
> +
> +static void print_context_stats(struct seq_file *m,
> + struct drm_i915_private *dev_priv)
> +{
> + struct file_stats stats;
> + struct drm_file *file;
> +
> + memset(&stats, 0, sizeof(stats));
> +
> + if (dev_priv->kernel_context)
> + per_file_ctx_stats(0, dev_priv->kernel_context, &stats);
> +
> + list_for_each_entry(file, &dev_priv->dev->filelist, lhead) {
Existing code which iterates this list grabs dev->filelist_mutex ?
> + struct drm_i915_file_private *fpriv = file->driver_priv;
> + idr_for_each(&fpriv->context_idr, per_file_ctx_stats, &stats);
> + }
> +
> + print_file_stats(m, "[k]contexts", stats);
> +}
> +
> #define count_vmas(list, member) do { \
> list_for_each_entry(vma, list, member) { \
> size += i915_gem_obj_total_ggtt_size(vma->obj); \
> @@ -521,6 +555,7 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
>
> seq_putc(m, '\n');
> print_batch_pool_stats(m, dev_priv);
> + print_context_stats(m, dev_priv);
>
> mutex_unlock(&dev->struct_mutex);
>
>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 11/11] drm/i915: Show context objects in debugfs/i915_gem_objects
2016-05-23 12:31 ` Tvrtko Ursulin
@ 2016-05-23 12:40 ` Chris Wilson
2016-05-23 13:01 ` [PATCH v3] drm/i915/debugfs: Show context objects in i915_gem_objects Chris Wilson
1 sibling, 0 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-23 12:40 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
On Mon, May 23, 2016 at 01:31:25PM +0100, Tvrtko Ursulin wrote:
>
> On 23/05/16 12:34, Chris Wilson wrote:
>
> Blah blah blah, this, for that, etc... commit message ofc. :)
>
> >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >---
> > drivers/gpu/drm/i915/i915_debugfs.c | 35 +++++++++++++++++++++++++++++++++++
> > 1 file changed, 35 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> >index 30cb26fe2fa9..3d14eb3215e1 100644
> >--- a/drivers/gpu/drm/i915/i915_debugfs.c
> >+++ b/drivers/gpu/drm/i915/i915_debugfs.c
> >@@ -417,6 +417,40 @@ static void print_batch_pool_stats(struct seq_file *m,
> > print_file_stats(m, "[k]batch pool", stats);
> > }
> >
> >+static int per_file_ctx_stats(int id, void *ptr, void *data)
> >+{
> >+ struct i915_gem_context *ctx = ptr;
> >+ int n;
> >+
> >+ for (n = 0; n < ARRAY_SIZE(ctx->engine); n++) {
> >+ if (ctx->engine[n].state)
> >+ per_file_stats(0, ctx->engine[n].state, data);
> >+ if (ctx->engine[n].ringbuf)
> >+ per_file_stats(0, ctx->engine[n].ringbuf->obj, data);
> >+ }
> >+
> >+ return 0;
> >+}
> >+
> >+static void print_context_stats(struct seq_file *m,
> >+ struct drm_i915_private *dev_priv)
> >+{
> >+ struct file_stats stats;
> >+ struct drm_file *file;
> >+
> >+ memset(&stats, 0, sizeof(stats));
> >+
> >+ if (dev_priv->kernel_context)
> >+ per_file_ctx_stats(0, dev_priv->kernel_context, &stats);
> >+
> >+ list_for_each_entry(file, &dev_priv->dev->filelist, lhead) {
>
> Existing code which iterates this list grabs dev->filelist_mutex ?
Recent invention, curses Daniel!
-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] 42+ messages in thread
* [PATCH v3] drm/i915/debugfs: Show context objects in i915_gem_objects
2016-05-23 12:31 ` Tvrtko Ursulin
2016-05-23 12:40 ` Chris Wilson
@ 2016-05-23 13:01 ` Chris Wilson
2016-05-23 13:42 ` kbuild test robot
` (3 more replies)
1 sibling, 4 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-23 13:01 UTC (permalink / raw)
To: intel-gfx
One of the uses for i915_gem_objects is pin-pointing leaks. For this, we
can compare the number of allocated objects and who owns them, a
discrepancy here often indicates a kernel bug. One allocator of unreported
objects is for backing context objects, so include those in the listing.
v2: Take filelist_mutex which requires a little dance with struct_mutex
to avoid nesting filelist_mutex inside struct_mutex.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 38 ++++++++++++++++++++++++++++++++++++-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 30cb26fe2fa9..1300bad67f5c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -417,6 +417,42 @@ static void print_batch_pool_stats(struct seq_file *m,
print_file_stats(m, "[k]batch pool", stats);
}
+static int per_file_ctx_stats(int id, void *ptr, void *data)
+{
+ struct i915_gem_context *ctx = ptr;
+ int n;
+
+ for (n = 0; n < ARRAY_SIZE(ctx->engine); n++) {
+ if (ctx->engine[n].state)
+ per_file_stats(0, ctx->engine[n].state, data);
+ if (ctx->engine[n].ringbuf)
+ per_file_stats(0, ctx->engine[n].ringbuf->obj, data);
+ }
+
+ return 0;
+}
+
+static void print_context_stats(struct seq_file *m,
+ struct drm_i915_private *dev_priv)
+{
+ struct file_stats stats;
+ struct drm_file *file;
+
+ memset(&stats, 0, sizeof(stats));
+
+ mutex_lock(&dev_priv->dev->struct_mutex);
+ if (dev_priv->kernel_context)
+ per_file_ctx_stats(0, dev_priv->kernel_context, &stats);
+
+ list_for_each_entry(file, &dev_priv->dev->filelist, lhead) {
+ struct drm_i915_file_private *fpriv = file->driver_priv;
+ idr_for_each(&fpriv->context_idr, per_file_ctx_stats, &stats);
+ }
+ mutex_unlock(&dev_priv->dev->struct_mutex);
+
+ print_file_stats(m, "[k]contexts", stats);
+}
+
#define count_vmas(list, member) do { \
list_for_each_entry(vma, list, member) { \
size += i915_gem_obj_total_ggtt_size(vma->obj); \
@@ -521,10 +557,10 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
seq_putc(m, '\n');
print_batch_pool_stats(m, dev_priv);
-
mutex_unlock(&dev->struct_mutex);
mutex_lock(&dev->filelist_mutex);
+ print_context_stats(m, dev_priv);
list_for_each_entry_reverse(file, &dev->filelist, lhead) {
struct file_stats stats;
struct task_struct *task;
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH v3] drm/i915/debugfs: Show context objects in i915_gem_objects
2016-05-23 13:01 ` [PATCH v3] drm/i915/debugfs: Show context objects in i915_gem_objects Chris Wilson
@ 2016-05-23 13:42 ` kbuild test robot
2016-05-23 13:50 ` kbuild test robot
` (2 subsequent siblings)
3 siblings, 0 replies; 42+ messages in thread
From: kbuild test robot @ 2016-05-23 13:42 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx, kbuild-all
[-- Attachment #1: Type: text/plain, Size: 5999 bytes --]
Hi,
[auto build test ERROR on next-20160523]
[cannot apply to drm-intel/for-linux-next v4.6-rc7 v4.6-rc6 v4.6-rc5 v4.6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Chris-Wilson/drm-i915-debugfs-Show-context-objects-in-i915_gem_objects/20160523-210704
config: x86_64-rhel (attached as .config)
compiler: gcc-4.9 (Debian 4.9.3-14) 4.9.3
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
Note: the linux-review/Chris-Wilson/drm-i915-debugfs-Show-context-objects-in-i915_gem_objects/20160523-210704 HEAD 3dfdb2b9e47b5ee4dfd9a21f80014bd9de87c4cb builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
In file included from include/asm-generic/bug.h:13:0,
from arch/x86/include/asm/bug.h:35,
from include/linux/bug.h:4,
from include/linux/seq_file.h:6,
from drivers/gpu/drm/i915/i915_debugfs.c:29:
drivers/gpu/drm/i915/i915_debugfs.c: In function 'per_file_ctx_stats':
>> drivers/gpu/drm/i915/i915_debugfs.c:432:32: error: dereferencing pointer to incomplete type
for (n = 0; n < ARRAY_SIZE(ctx->engine); n++) {
^
include/linux/kernel.h:54:33: note: in definition of macro 'ARRAY_SIZE'
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
^
>> drivers/gpu/drm/i915/i915_debugfs.c:432:32: error: dereferencing pointer to incomplete type
for (n = 0; n < ARRAY_SIZE(ctx->engine); n++) {
^
include/linux/kernel.h:54:48: note: in definition of macro 'ARRAY_SIZE'
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
^
In file included from include/linux/seq_file.h:6:0,
from drivers/gpu/drm/i915/i915_debugfs.c:29:
>> drivers/gpu/drm/i915/i915_debugfs.c:432:32: error: dereferencing pointer to incomplete type
for (n = 0; n < ARRAY_SIZE(ctx->engine); n++) {
^
include/linux/bug.h:34:55: note: in definition of macro 'BUILD_BUG_ON_ZERO'
#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
^
include/linux/compiler-gcc.h:64:46: note: in expansion of macro '__same_type'
#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
^
include/linux/kernel.h:54:59: note: in expansion of macro '__must_be_array'
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
^
drivers/gpu/drm/i915/i915_debugfs.c:432:18: note: in expansion of macro 'ARRAY_SIZE'
for (n = 0; n < ARRAY_SIZE(ctx->engine); n++) {
^
>> drivers/gpu/drm/i915/i915_debugfs.c:432:32: error: dereferencing pointer to incomplete type
for (n = 0; n < ARRAY_SIZE(ctx->engine); n++) {
^
include/linux/bug.h:34:55: note: in definition of macro 'BUILD_BUG_ON_ZERO'
#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
^
include/linux/compiler-gcc.h:64:46: note: in expansion of macro '__same_type'
#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
^
include/linux/kernel.h:54:59: note: in expansion of macro '__must_be_array'
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
^
drivers/gpu/drm/i915/i915_debugfs.c:432:18: note: in expansion of macro 'ARRAY_SIZE'
for (n = 0; n < ARRAY_SIZE(ctx->engine); n++) {
^
include/linux/bug.h:34:45: error: bit-field '<anonymous>' width not an integer constant
#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
^
include/linux/compiler-gcc.h:64:28: note: in expansion of macro 'BUILD_BUG_ON_ZERO'
#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
^
include/linux/kernel.h:54:59: note: in expansion of macro '__must_be_array'
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
^
drivers/gpu/drm/i915/i915_debugfs.c:432:18: note: in expansion of macro 'ARRAY_SIZE'
for (n = 0; n < ARRAY_SIZE(ctx->engine); n++) {
^
drivers/gpu/drm/i915/i915_debugfs.c:433:10: error: dereferencing pointer to incomplete type
if (ctx->engine[n].state)
^
drivers/gpu/drm/i915/i915_debugfs.c:434:25: error: dereferencing pointer to incomplete type
per_file_stats(0, ctx->engine[n].state, data);
^
drivers/gpu/drm/i915/i915_debugfs.c:435:10: error: dereferencing pointer to incomplete type
if (ctx->engine[n].ringbuf)
^
drivers/gpu/drm/i915/i915_debugfs.c:436:25: error: dereferencing pointer to incomplete type
per_file_stats(0, ctx->engine[n].ringbuf->obj, data);
^
vim +432 drivers/gpu/drm/i915/i915_debugfs.c
426
427 static int per_file_ctx_stats(int id, void *ptr, void *data)
428 {
429 struct i915_gem_context *ctx = ptr;
430 int n;
431
> 432 for (n = 0; n < ARRAY_SIZE(ctx->engine); n++) {
433 if (ctx->engine[n].state)
434 per_file_stats(0, ctx->engine[n].state, data);
435 if (ctx->engine[n].ringbuf)
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 36752 bytes --]
[-- Attachment #3: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3] drm/i915/debugfs: Show context objects in i915_gem_objects
2016-05-23 13:01 ` [PATCH v3] drm/i915/debugfs: Show context objects in i915_gem_objects Chris Wilson
2016-05-23 13:42 ` kbuild test robot
@ 2016-05-23 13:50 ` kbuild test robot
2016-05-24 11:15 ` Tvrtko Ursulin
2016-05-25 9:55 ` Joonas Lahtinen
3 siblings, 0 replies; 42+ messages in thread
From: kbuild test robot @ 2016-05-23 13:50 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx, kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3313 bytes --]
Hi,
[auto build test ERROR on next-20160523]
[cannot apply to drm-intel/for-linux-next v4.6-rc7 v4.6-rc6 v4.6-rc5 v4.6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Chris-Wilson/drm-i915-debugfs-Show-context-objects-in-i915_gem_objects/20160523-210704
config: i386-randconfig-x011-201621 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
Note: the linux-review/Chris-Wilson/drm-i915-debugfs-Show-context-objects-in-i915_gem_objects/20160523-210704 HEAD 3dfdb2b9e47b5ee4dfd9a21f80014bd9de87c4cb builds fine.
It only hurts bisectibility.
All error/warnings (new ones prefixed by >>):
In file included from include/asm-generic/bug.h:13:0,
from arch/x86/include/asm/bug.h:35,
from include/linux/bug.h:4,
from include/linux/seq_file.h:6,
from drivers/gpu/drm/i915/i915_debugfs.c:29:
drivers/gpu/drm/i915/i915_debugfs.c: In function 'per_file_ctx_stats':
>> drivers/gpu/drm/i915/i915_debugfs.c:432:32: error: dereferencing pointer to incomplete type 'struct i915_gem_context'
for (n = 0; n < ARRAY_SIZE(ctx->engine); n++) {
^
include/linux/kernel.h:54:33: note: in definition of macro 'ARRAY_SIZE'
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
^~~
In file included from include/linux/seq_file.h:6:0,
from drivers/gpu/drm/i915/i915_debugfs.c:29:
>> include/linux/bug.h:34:45: error: bit-field '<anonymous>' width not an integer constant
#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
^
include/linux/compiler-gcc.h:64:28: note: in expansion of macro 'BUILD_BUG_ON_ZERO'
#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
^~~~~~~~~~~~~~~~~
include/linux/kernel.h:54:59: note: in expansion of macro '__must_be_array'
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
^~~~~~~~~~~~~~~
>> drivers/gpu/drm/i915/i915_debugfs.c:432:18: note: in expansion of macro 'ARRAY_SIZE'
for (n = 0; n < ARRAY_SIZE(ctx->engine); n++) {
^~~~~~~~~~
drivers/gpu/drm/i915/i915_debugfs.c: In function 'i915_dump_lrc':
drivers/gpu/drm/i915/i915_debugfs.c:2139:6: warning: suggest explicit braces to avoid ambiguous 'else' [-Wparentheses]
if (ctx != dev_priv->kernel_context)
^
vim +432 drivers/gpu/drm/i915/i915_debugfs.c
426
427 static int per_file_ctx_stats(int id, void *ptr, void *data)
428 {
429 struct i915_gem_context *ctx = ptr;
430 int n;
431
> 432 for (n = 0; n < ARRAY_SIZE(ctx->engine); n++) {
433 if (ctx->engine[n].state)
434 per_file_stats(0, ctx->engine[n].state, data);
435 if (ctx->engine[n].ringbuf)
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 27555 bytes --]
[-- Attachment #3: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 42+ messages in thread
* ✗ Ro.CI.BAT: failure for series starting with [v2,01/11] drm/i915: Rename struct intel_context (rev2)
2016-05-23 11:34 Context churn Chris Wilson
` (11 preceding siblings ...)
2016-05-23 12:01 ` ✓ Ro.CI.BAT: success for series starting with [v2,01/11] drm/i915: Rename struct intel_context Patchwork
@ 2016-05-23 14:30 ` Patchwork
2016-05-24 13:53 ` [CI 01/10] drm/i915: Rename struct intel_context Chris Wilson
2016-05-24 14:19 ` ✗ Ro.CI.BAT: warning for series starting with [v2,01/11] drm/i915: Rename struct intel_context (rev10) Patchwork
14 siblings, 0 replies; 42+ messages in thread
From: Patchwork @ 2016-05-23 14:30 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v2,01/11] drm/i915: Rename struct intel_context (rev2)
URL : https://patchwork.freedesktop.org/series/7564/
State : failure
== Summary ==
Series 7564v2 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/7564/revisions/2/mbox
Test gem_exec_flush:
Subgroup basic-batch-kernel-default-cmd:
fail -> PASS (ro-byt-n2820)
fail -> PASS (fi-byt-n2820)
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-a-frame-sequence:
pass -> FAIL (ro-snb-i7-2620M)
fi-bdw-i7-5557u total:209 pass:197 dwarn:0 dfail:0 fail:0 skip:12
fi-bsw-n3050 total:209 pass:167 dwarn:0 dfail:0 fail:2 skip:40
fi-byt-n2820 total:209 pass:169 dwarn:0 dfail:0 fail:2 skip:38
fi-hsw-i7-4770k total:209 pass:190 dwarn:0 dfail:0 fail:0 skip:19
fi-hsw-i7-4770r total:209 pass:186 dwarn:0 dfail:0 fail:0 skip:23
fi-skl-i5-6260u total:209 pass:196 dwarn:2 dfail:0 fail:0 skip:11
fi-skl-i7-6700k total:209 pass:182 dwarn:2 dfail:0 fail:0 skip:25
fi-snb-i7-2600 total:209 pass:170 dwarn:0 dfail:0 fail:0 skip:39
ro-bdw-i5-5250u total:209 pass:172 dwarn:0 dfail:0 fail:0 skip:37
ro-bdw-i7-5557U total:209 pass:197 dwarn:0 dfail:0 fail:0 skip:12
ro-bdw-i7-5600u total:209 pass:179 dwarn:0 dfail:0 fail:2 skip:28
ro-bsw-n3050 total:209 pass:168 dwarn:0 dfail:0 fail:2 skip:39
ro-byt-n2820 total:209 pass:170 dwarn:0 dfail:0 fail:2 skip:37
ro-hsw-i3-4010u total:209 pass:186 dwarn:0 dfail:0 fail:0 skip:23
ro-hsw-i7-4770r total:209 pass:186 dwarn:0 dfail:0 fail:0 skip:23
ro-ilk-i7-620lm total:209 pass:146 dwarn:0 dfail:0 fail:1 skip:62
ro-ilk1-i5-650 total:204 pass:146 dwarn:0 dfail:0 fail:1 skip:57
ro-ivb-i7-3770 total:209 pass:177 dwarn:0 dfail:0 fail:0 skip:32
ro-ivb2-i7-3770 total:209 pass:181 dwarn:0 dfail:0 fail:0 skip:28
ro-skl-i7-6700hq total:204 pass:180 dwarn:2 dfail:0 fail:0 skip:22
ro-snb-i7-2620M total:209 pass:169 dwarn:0 dfail:0 fail:2 skip:38
Results at /archive/results/CI_IGT_test/RO_Patchwork_974/
54afd5a drm-intel-nightly: 2016y-05m-23d-13h-38m-05s UTC integration manifest
0d3420b drm/i915/debugfs: Show context objects in i915_gem_objects
0f5ae8c drm/i915: Rearrange i915_gem_context
82b468f drm/i915: Merge legacy+execlists context structs
1dcca1e drm/i915: Put the kernel_context in drm_i915_private next to its friends
f225058 drm/i915: Show i915_gem_context owner in debugfs
d5050ea drm/i915: Move pinning of dev_priv->kernel_context into its creator
3e18a92 drm/i915: Name the inner most per-engine intel_context struct
ab016a4 drm/i915: Rename i915_gem_context_reference/unreference()
2358d91 drm/i915: Rename and inline i915_gem_context_get()
409ed24 drm/i915: Apply lockdep annotations to i915_gem_context.c
0616d56 drm/i915: Rename struct intel_context
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3] drm/i915/debugfs: Show context objects in i915_gem_objects
2016-05-23 13:01 ` [PATCH v3] drm/i915/debugfs: Show context objects in i915_gem_objects Chris Wilson
2016-05-23 13:42 ` kbuild test robot
2016-05-23 13:50 ` kbuild test robot
@ 2016-05-24 11:15 ` Tvrtko Ursulin
2016-05-25 9:55 ` Joonas Lahtinen
3 siblings, 0 replies; 42+ messages in thread
From: Tvrtko Ursulin @ 2016-05-24 11:15 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 23/05/16 14:01, Chris Wilson wrote:
> One of the uses for i915_gem_objects is pin-pointing leaks. For this, we
> can compare the number of allocated objects and who owns them, a
> discrepancy here often indicates a kernel bug. One allocator of unreported
> objects is for backing context objects, so include those in the listing.
>
> v2: Take filelist_mutex which requires a little dance with struct_mutex
> to avoid nesting filelist_mutex inside struct_mutex.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 38 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 30cb26fe2fa9..1300bad67f5c 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -417,6 +417,42 @@ static void print_batch_pool_stats(struct seq_file *m,
> print_file_stats(m, "[k]batch pool", stats);
> }
>
> +static int per_file_ctx_stats(int id, void *ptr, void *data)
> +{
> + struct i915_gem_context *ctx = ptr;
> + int n;
> +
> + for (n = 0; n < ARRAY_SIZE(ctx->engine); n++) {
> + if (ctx->engine[n].state)
> + per_file_stats(0, ctx->engine[n].state, data);
> + if (ctx->engine[n].ringbuf)
> + per_file_stats(0, ctx->engine[n].ringbuf->obj, data);
> + }
> +
> + return 0;
> +}
> +
> +static void print_context_stats(struct seq_file *m,
> + struct drm_i915_private *dev_priv)
> +{
> + struct file_stats stats;
> + struct drm_file *file;
> +
> + memset(&stats, 0, sizeof(stats));
> +
> + mutex_lock(&dev_priv->dev->struct_mutex);
> + if (dev_priv->kernel_context)
> + per_file_ctx_stats(0, dev_priv->kernel_context, &stats);
> +
> + list_for_each_entry(file, &dev_priv->dev->filelist, lhead) {
> + struct drm_i915_file_private *fpriv = file->driver_priv;
> + idr_for_each(&fpriv->context_idr, per_file_ctx_stats, &stats);
> + }
> + mutex_unlock(&dev_priv->dev->struct_mutex);
> +
> + print_file_stats(m, "[k]contexts", stats);
> +}
> +
> #define count_vmas(list, member) do { \
> list_for_each_entry(vma, list, member) { \
> size += i915_gem_obj_total_ggtt_size(vma->obj); \
> @@ -521,10 +557,10 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
>
> seq_putc(m, '\n');
> print_batch_pool_stats(m, dev_priv);
> -
> mutex_unlock(&dev->struct_mutex);
>
> mutex_lock(&dev->filelist_mutex);
> + print_context_stats(m, dev_priv);
> list_for_each_entry_reverse(file, &dev->filelist, lhead) {
> struct file_stats stats;
> struct task_struct *task;
>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 04/11] drm/i915: Rename i915_gem_context_reference/unreference()
2016-05-23 11:34 ` [PATCH v2 04/11] drm/i915: Rename i915_gem_context_reference/unreference() Chris Wilson
@ 2016-05-24 11:21 ` Chris Wilson
0 siblings, 0 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-24 11:21 UTC (permalink / raw)
To: intel-gfx
On Mon, May 23, 2016 at 12:34:11PM +0100, Chris Wilson wrote:
> As these are wrappers around kref_get/kref_put() it is preferable to
> follow the naming convention and use the same verb get/put in our
> wrapper names for manipulating a reference to the context.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
The consensus is to punt this until we have more consistency (i.e.
converting a bunch of objects over).
-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] 42+ messages in thread
* [CI 01/10] drm/i915: Rename struct intel_context
2016-05-23 11:34 Context churn Chris Wilson
` (12 preceding siblings ...)
2016-05-23 14:30 ` ✗ Ro.CI.BAT: failure for series starting with [v2,01/11] drm/i915: Rename struct intel_context (rev2) Patchwork
@ 2016-05-24 13:53 ` Chris Wilson
2016-05-24 13:53 ` [CI 02/10] drm/i915: Apply lockdep annotations to i915_gem_context.c Chris Wilson
` (8 more replies)
2016-05-24 14:19 ` ✗ Ro.CI.BAT: warning for series starting with [v2,01/11] drm/i915: Rename struct intel_context (rev10) Patchwork
14 siblings, 9 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-24 13:53 UTC (permalink / raw)
To: intel-gfx
Our goal is to rename the anonymous per-engine struct beneath the
current intel_context. However, after a lively debate resolving around
the confusion between intel_context_engine and intel_engine_context, the
realisation is that the two structs target different users. The outer
struct is API / user facing, and so carries the higher level GEM
information. The inner struct is hw facing. Thus we want to name the
inner struct intel_context and the outer one i915_gem_context. As the
first step, we need to rename the current struct:
s/struct intel_context/struct i915_gem_context/
which fits much better with its constructors already conveying the
i915_gem_context prefix!
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Dave Gordon <david.s.gordon@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 10 +++---
drivers/gpu/drm/i915/i915_drv.h | 22 ++++++-------
drivers/gpu/drm/i915/i915_gem.c | 8 ++---
drivers/gpu/drm/i915/i915_gem_context.c | 52 +++++++++++++++---------------
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 10 +++---
drivers/gpu/drm/i915/i915_guc_submission.c | 12 +++----
drivers/gpu/drm/i915/i915_sysfs.c | 2 +-
drivers/gpu/drm/i915/i915_trace.h | 12 +++----
drivers/gpu/drm/i915/intel_guc.h | 2 +-
drivers/gpu/drm/i915/intel_lrc.c | 22 ++++++-------
drivers/gpu/drm/i915/intel_lrc.h | 10 +++---
drivers/gpu/drm/i915/intel_ringbuffer.h | 4 +--
12 files changed, 84 insertions(+), 82 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index e606c6acef0f..424e11efa3e1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -199,7 +199,7 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
seq_printf(m, " (frontbuffer: 0x%03x)", obj->frontbuffer_bits);
}
-static void describe_ctx(struct seq_file *m, struct intel_context *ctx)
+static void describe_ctx(struct seq_file *m, struct i915_gem_context *ctx)
{
seq_putc(m, ctx->legacy_hw_ctx.initialized ? 'I' : 'i');
seq_putc(m, ctx->remap_slice ? 'R' : 'r');
@@ -2000,7 +2000,7 @@ static int i915_context_status(struct seq_file *m, void *unused)
struct drm_device *dev = node->minor->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_engine_cs *engine;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
enum intel_engine_id id;
int ret;
@@ -2046,7 +2046,7 @@ static int i915_context_status(struct seq_file *m, void *unused)
}
static void i915_dump_lrc_obj(struct seq_file *m,
- struct intel_context *ctx,
+ struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
struct page *page;
@@ -2094,7 +2094,7 @@ static int i915_dump_lrc(struct seq_file *m, void *unused)
struct drm_device *dev = node->minor->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_engine_cs *engine;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
int ret;
if (!i915.enable_execlists) {
@@ -2274,7 +2274,7 @@ static int i915_swizzle_info(struct seq_file *m, void *data)
static int per_file_ctx(int id, void *ptr, void *data)
{
- struct intel_context *ctx = ptr;
+ struct i915_gem_context *ctx = ptr;
struct seq_file *m = data;
struct i915_hw_ppgtt *ppgtt = ctx->ppgtt;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 78d38c246491..961ef403154e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -831,7 +831,7 @@ struct i915_ctx_hang_stats {
#define CONTEXT_NO_ZEROMAP (1<<0)
/**
- * struct intel_context - as the name implies, represents a context.
+ * struct i915_gem_context - as the name implies, represents a context.
* @ref: reference count.
* @user_handle: userspace tracking identity for this context.
* @remap_slice: l3 row remapping information.
@@ -849,7 +849,7 @@ struct i915_ctx_hang_stats {
* Contexts are memory images used by the hardware to store copies of their
* internal state.
*/
-struct intel_context {
+struct i915_gem_context {
struct kref ref;
int user_handle;
uint8_t remap_slice;
@@ -1710,7 +1710,7 @@ struct i915_execbuffer_params {
uint64_t batch_obj_vm_offset;
struct intel_engine_cs *engine;
struct drm_i915_gem_object *batch_obj;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
struct drm_i915_gem_request *request;
};
@@ -2017,7 +2017,7 @@ struct drm_i915_private {
void (*stop_engine)(struct intel_engine_cs *engine);
} gt;
- struct intel_context *kernel_context;
+ struct i915_gem_context *kernel_context;
/* perform PHY state sanity checks? */
bool chv_phy_assert[2];
@@ -2385,7 +2385,7 @@ struct drm_i915_gem_request {
* i915_gem_request_free() will then decrement the refcount on the
* context.
*/
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
struct intel_ringbuffer *ringbuf;
/**
@@ -2397,7 +2397,7 @@ struct drm_i915_gem_request {
* we keep the previous context pinned until the following (this)
* request is retired.
*/
- struct intel_context *previous_context;
+ struct i915_gem_context *previous_context;
/** Batch buffer related to this request if any (used for
error state dump only) */
@@ -2441,7 +2441,7 @@ struct drm_i915_gem_request {
struct drm_i915_gem_request * __must_check
i915_gem_request_alloc(struct intel_engine_cs *engine,
- struct intel_context *ctx);
+ struct i915_gem_context *ctx);
void i915_gem_request_free(struct kref *req_ref);
int i915_gem_request_add_to_client(struct drm_i915_gem_request *req,
struct drm_file *file);
@@ -3427,22 +3427,22 @@ void i915_gem_context_reset(struct drm_device *dev);
int i915_gem_context_open(struct drm_device *dev, struct drm_file *file);
void i915_gem_context_close(struct drm_device *dev, struct drm_file *file);
int i915_switch_context(struct drm_i915_gem_request *req);
-struct intel_context *
+struct i915_gem_context *
i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id);
void i915_gem_context_free(struct kref *ctx_ref);
struct drm_i915_gem_object *
i915_gem_alloc_context_obj(struct drm_device *dev, size_t size);
-static inline void i915_gem_context_reference(struct intel_context *ctx)
+static inline void i915_gem_context_reference(struct i915_gem_context *ctx)
{
kref_get(&ctx->ref);
}
-static inline void i915_gem_context_unreference(struct intel_context *ctx)
+static inline void i915_gem_context_unreference(struct i915_gem_context *ctx)
{
kref_put(&ctx->ref, i915_gem_context_free);
}
-static inline bool i915_gem_context_is_default(const struct intel_context *c)
+static inline bool i915_gem_context_is_default(const struct i915_gem_context *c)
{
return c->user_handle == DEFAULT_CONTEXT_HANDLE;
}
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b3c15698d611..89967f97c097 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2689,7 +2689,7 @@ void __i915_add_request(struct drm_i915_gem_request *request,
}
static bool i915_context_is_banned(struct drm_i915_private *dev_priv,
- const struct intel_context *ctx)
+ const struct i915_gem_context *ctx)
{
unsigned long elapsed;
@@ -2714,7 +2714,7 @@ static bool i915_context_is_banned(struct drm_i915_private *dev_priv,
}
static void i915_set_reset_status(struct drm_i915_private *dev_priv,
- struct intel_context *ctx,
+ struct i915_gem_context *ctx,
const bool guilty)
{
struct i915_ctx_hang_stats *hs;
@@ -2742,7 +2742,7 @@ void i915_gem_request_free(struct kref *req_ref)
static inline int
__i915_gem_request_alloc(struct intel_engine_cs *engine,
- struct intel_context *ctx,
+ struct i915_gem_context *ctx,
struct drm_i915_gem_request **req_out)
{
struct drm_i915_private *dev_priv = engine->i915;
@@ -2818,7 +2818,7 @@ err:
*/
struct drm_i915_gem_request *
i915_gem_request_alloc(struct intel_engine_cs *engine,
- struct intel_context *ctx)
+ struct i915_gem_context *ctx)
{
struct drm_i915_gem_request *req;
int err;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 2aedd188473d..8484da26b5d4 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -134,7 +134,7 @@ static int get_context_size(struct drm_i915_private *dev_priv)
return ret;
}
-static void i915_gem_context_clean(struct intel_context *ctx)
+static void i915_gem_context_clean(struct i915_gem_context *ctx)
{
struct i915_hw_ppgtt *ppgtt = ctx->ppgtt;
struct i915_vma *vma, *next;
@@ -151,7 +151,7 @@ static void i915_gem_context_clean(struct intel_context *ctx)
void i915_gem_context_free(struct kref *ctx_ref)
{
- struct intel_context *ctx = container_of(ctx_ref, typeof(*ctx), ref);
+ struct i915_gem_context *ctx = container_of(ctx_ref, typeof(*ctx), ref);
trace_i915_context_free(ctx);
@@ -234,12 +234,12 @@ static int assign_hw_id(struct drm_i915_private *dev_priv, unsigned *out)
return 0;
}
-static struct intel_context *
+static struct i915_gem_context *
__create_hw_context(struct drm_device *dev,
struct drm_i915_file_private *file_priv)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
int ret;
ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
@@ -296,12 +296,12 @@ err_out:
* context state of the GPU for applications that don't utilize HW contexts, as
* well as an idle case.
*/
-static struct intel_context *
+static struct i915_gem_context *
i915_gem_create_context(struct drm_device *dev,
struct drm_i915_file_private *file_priv)
{
const bool is_global_default_ctx = file_priv == NULL;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
int ret = 0;
BUG_ON(!mutex_is_locked(&dev->struct_mutex));
@@ -352,7 +352,7 @@ err_destroy:
return ERR_PTR(ret);
}
-static void i915_gem_context_unpin(struct intel_context *ctx,
+static void i915_gem_context_unpin(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
if (i915.enable_execlists) {
@@ -369,7 +369,7 @@ void i915_gem_context_reset(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
if (i915.enable_execlists) {
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
list_for_each_entry(ctx, &dev_priv->context_list, link)
intel_lr_context_reset(dev_priv, ctx);
@@ -381,7 +381,7 @@ void i915_gem_context_reset(struct drm_device *dev)
int i915_gem_context_init(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
/* Init should only be called once per module load. Eventually the
* restriction on the context_disabled check can be loosened. */
@@ -449,7 +449,7 @@ void i915_gem_context_lost(struct drm_i915_private *dev_priv)
void i915_gem_context_fini(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_context *dctx = dev_priv->kernel_context;
+ struct i915_gem_context *dctx = dev_priv->kernel_context;
if (dctx->legacy_hw_ctx.rcs_state)
i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
@@ -462,7 +462,7 @@ void i915_gem_context_fini(struct drm_device *dev)
static int context_idr_cleanup(int id, void *p, void *data)
{
- struct intel_context *ctx = p;
+ struct i915_gem_context *ctx = p;
i915_gem_context_unreference(ctx);
return 0;
@@ -471,7 +471,7 @@ static int context_idr_cleanup(int id, void *p, void *data)
int i915_gem_context_open(struct drm_device *dev, struct drm_file *file)
{
struct drm_i915_file_private *file_priv = file->driver_priv;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
idr_init(&file_priv->context_idr);
@@ -495,12 +495,12 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file)
idr_destroy(&file_priv->context_idr);
}
-struct intel_context *
+struct i915_gem_context *
i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id)
{
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
- ctx = (struct intel_context *)idr_find(&file_priv->context_idr, id);
+ ctx = idr_find(&file_priv->context_idr, id);
if (!ctx)
return ERR_PTR(-ENOENT);
@@ -641,7 +641,7 @@ static int remap_l3(struct drm_i915_gem_request *req, int slice)
static inline bool skip_rcs_switch(struct i915_hw_ppgtt *ppgtt,
struct intel_engine_cs *engine,
- struct intel_context *to)
+ struct i915_gem_context *to)
{
if (to->remap_slice)
return false;
@@ -658,7 +658,7 @@ static inline bool skip_rcs_switch(struct i915_hw_ppgtt *ppgtt,
static bool
needs_pd_load_pre(struct i915_hw_ppgtt *ppgtt,
struct intel_engine_cs *engine,
- struct intel_context *to)
+ struct i915_gem_context *to)
{
if (!ppgtt)
return false;
@@ -683,7 +683,7 @@ needs_pd_load_pre(struct i915_hw_ppgtt *ppgtt,
static bool
needs_pd_load_post(struct i915_hw_ppgtt *ppgtt,
- struct intel_context *to,
+ struct i915_gem_context *to,
u32 hw_flags)
{
if (!ppgtt)
@@ -700,10 +700,10 @@ needs_pd_load_post(struct i915_hw_ppgtt *ppgtt,
static int do_rcs_switch(struct drm_i915_gem_request *req)
{
- struct intel_context *to = req->ctx;
+ struct i915_gem_context *to = req->ctx;
struct intel_engine_cs *engine = req->engine;
struct i915_hw_ppgtt *ppgtt = to->ppgtt ?: req->i915->mm.aliasing_ppgtt;
- struct intel_context *from;
+ struct i915_gem_context *from;
u32 hw_flags;
int ret, i;
@@ -859,7 +859,7 @@ int i915_switch_context(struct drm_i915_gem_request *req)
if (engine->id != RCS ||
req->ctx->legacy_hw_ctx.rcs_state == NULL) {
- struct intel_context *to = req->ctx;
+ struct i915_gem_context *to = req->ctx;
struct i915_hw_ppgtt *ppgtt =
to->ppgtt ?: req->i915->mm.aliasing_ppgtt;
@@ -897,7 +897,7 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
{
struct drm_i915_gem_context_create *args = data;
struct drm_i915_file_private *file_priv = file->driver_priv;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
int ret;
if (!contexts_enabled(dev))
@@ -926,7 +926,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
{
struct drm_i915_gem_context_destroy *args = data;
struct drm_i915_file_private *file_priv = file->driver_priv;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
int ret;
if (args->pad != 0)
@@ -958,7 +958,7 @@ 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 intel_context *ctx;
+ struct i915_gem_context *ctx;
int ret;
ret = i915_mutex_lock_interruptible(dev);
@@ -1001,7 +1001,7 @@ int i915_gem_context_setparam_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 intel_context *ctx;
+ struct i915_gem_context *ctx;
int ret;
ret = i915_mutex_lock_interruptible(dev);
@@ -1047,7 +1047,7 @@ int i915_gem_context_reset_stats_ioctl(struct drm_device *dev,
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_reset_stats *args = data;
struct i915_ctx_hang_stats *hs;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
int ret;
if (args->flags || args->pad)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index a54a243ccaac..e61b92d7b0bc 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -716,7 +716,7 @@ eb_vma_misplaced(struct i915_vma *vma)
static int
i915_gem_execbuffer_reserve(struct intel_engine_cs *engine,
struct list_head *vmas,
- struct intel_context *ctx,
+ struct i915_gem_context *ctx,
bool *need_relocs)
{
struct drm_i915_gem_object *obj;
@@ -828,7 +828,7 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev,
struct intel_engine_cs *engine,
struct eb_vmas *eb,
struct drm_i915_gem_exec_object2 *exec,
- struct intel_context *ctx)
+ struct i915_gem_context *ctx)
{
struct drm_i915_gem_relocation_entry *reloc;
struct i915_address_space *vm;
@@ -1065,11 +1065,11 @@ validate_exec_list(struct drm_device *dev,
return 0;
}
-static struct intel_context *
+static struct i915_gem_context *
i915_gem_validate_context(struct drm_device *dev, struct drm_file *file,
struct intel_engine_cs *engine, const u32 ctx_id)
{
- struct intel_context *ctx = NULL;
+ struct i915_gem_context *ctx = NULL;
struct i915_ctx_hang_stats *hs;
if (engine->id != RCS && ctx_id != DEFAULT_CONTEXT_HANDLE)
@@ -1430,7 +1430,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
struct drm_i915_gem_object *batch_obj;
struct drm_i915_gem_exec_object2 shadow_exec_entry;
struct intel_engine_cs *engine;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
struct i915_address_space *vm;
struct i915_execbuffer_params params_master; /* XXX: will be removed later */
struct i915_execbuffer_params *params = ¶ms_master;
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 42a85088657b..b42a3adc3fa4 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -360,7 +360,7 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
struct drm_i915_gem_object *client_obj = client->client_obj;
struct drm_i915_private *dev_priv = guc_to_i915(guc);
struct intel_engine_cs *engine;
- struct intel_context *ctx = client->owner;
+ struct i915_gem_context *ctx = client->owner;
struct guc_context_desc desc;
struct sg_table *sg;
enum intel_engine_id id;
@@ -426,7 +426,7 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
desc.wq_size = client->wq_size;
/*
- * XXX: Take LRCs from an existing intel_context if this is not an
+ * XXX: Take LRCs from an existing context if this is not an
* IsKMDCreatedContext client
*/
desc.desc_private = (uintptr_t)client;
@@ -700,7 +700,7 @@ static void guc_client_free(struct drm_device *dev,
*/
static struct i915_guc_client *guc_client_alloc(struct drm_device *dev,
uint32_t priority,
- struct intel_context *ctx)
+ struct i915_gem_context *ctx)
{
struct i915_guc_client *client;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -938,7 +938,7 @@ int i915_guc_submission_enable(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_guc *guc = &dev_priv->guc;
- struct intel_context *ctx = dev_priv->kernel_context;
+ struct i915_gem_context *ctx = dev_priv->kernel_context;
struct i915_guc_client *client;
/* client for execbuf submission */
@@ -989,7 +989,7 @@ int intel_guc_suspend(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_guc *guc = &dev_priv->guc;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
u32 data[3];
if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS)
@@ -1015,7 +1015,7 @@ int intel_guc_resume(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_guc *guc = &dev_priv->guc;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
u32 data[3];
if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS)
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index 37b6444b8e22..02507bfc8def 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -203,7 +203,7 @@ i915_l3_write(struct file *filp, struct kobject *kobj,
struct drm_minor *dminor = dev_to_drm_minor(dev);
struct drm_device *drm_dev = dminor->dev;
struct drm_i915_private *dev_priv = drm_dev->dev_private;
- struct intel_context *ctx;
+ struct i915_gem_context *ctx;
u32 *temp = NULL; /* Just here to make handling failures easy */
int slice = (int)(uintptr_t)attr->private;
int ret;
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index 20b2e4039792..6768db032f84 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -734,12 +734,12 @@ DEFINE_EVENT(i915_ppgtt, i915_ppgtt_release,
* the context.
*/
DECLARE_EVENT_CLASS(i915_context,
- TP_PROTO(struct intel_context *ctx),
+ TP_PROTO(struct i915_gem_context *ctx),
TP_ARGS(ctx),
TP_STRUCT__entry(
__field(u32, dev)
- __field(struct intel_context *, ctx)
+ __field(struct i915_gem_context *, ctx)
__field(struct i915_address_space *, vm)
),
@@ -754,12 +754,12 @@ DECLARE_EVENT_CLASS(i915_context,
)
DEFINE_EVENT(i915_context, i915_context_create,
- TP_PROTO(struct intel_context *ctx),
+ TP_PROTO(struct i915_gem_context *ctx),
TP_ARGS(ctx)
);
DEFINE_EVENT(i915_context, i915_context_free,
- TP_PROTO(struct intel_context *ctx),
+ TP_PROTO(struct i915_gem_context *ctx),
TP_ARGS(ctx)
);
@@ -771,13 +771,13 @@ DEFINE_EVENT(i915_context, i915_context_free,
* called only if full ppgtt is enabled.
*/
TRACE_EVENT(switch_mm,
- TP_PROTO(struct intel_engine_cs *engine, struct intel_context *to),
+ TP_PROTO(struct intel_engine_cs *engine, struct i915_gem_context *to),
TP_ARGS(engine, to),
TP_STRUCT__entry(
__field(u32, ring)
- __field(struct intel_context *, to)
+ __field(struct i915_gem_context *, to)
__field(struct i915_address_space *, vm)
__field(u32, dev)
),
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 7b1a5a3fb106..41601c71f529 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -64,7 +64,7 @@ struct drm_i915_gem_request;
struct i915_guc_client {
struct drm_i915_gem_object *client_obj;
void *client_base; /* first page (only) of above */
- struct intel_context *owner;
+ struct i915_gem_context *owner;
struct intel_guc *guc;
uint32_t priority;
uint32_t ctx_index;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 0d1772a122fe..3d95b26b32ef 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -231,9 +231,9 @@ enum {
/* Typical size of the average request (2 pipecontrols and a MI_BB) */
#define EXECLISTS_REQUEST_SIZE 64 /* bytes */
-static int execlists_context_deferred_alloc(struct intel_context *ctx,
+static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
struct intel_engine_cs *engine);
-static int intel_lr_context_pin(struct intel_context *ctx,
+static int intel_lr_context_pin(struct i915_gem_context *ctx,
struct intel_engine_cs *engine);
/**
@@ -315,7 +315,7 @@ logical_ring_init_platform_invariants(struct intel_engine_cs *engine)
* bits 55-63: group ID, currently unused and set to 0
*/
static void
-intel_lr_context_descriptor_update(struct intel_context *ctx,
+intel_lr_context_descriptor_update(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
u64 desc;
@@ -330,7 +330,7 @@ intel_lr_context_descriptor_update(struct intel_context *ctx,
ctx->engine[engine->id].lrc_desc = desc;
}
-uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
+uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
return ctx->engine[engine->id].lrc_desc;
@@ -929,7 +929,7 @@ int logical_ring_flush_all_caches(struct drm_i915_gem_request *req)
return 0;
}
-static int intel_lr_context_pin(struct intel_context *ctx,
+static int intel_lr_context_pin(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
struct drm_i915_private *dev_priv = ctx->i915;
@@ -985,7 +985,7 @@ err:
return ret;
}
-void intel_lr_context_unpin(struct intel_context *ctx,
+void intel_lr_context_unpin(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
struct drm_i915_gem_object *ctx_obj;
@@ -2046,7 +2046,7 @@ logical_ring_setup(struct drm_device *dev, enum intel_engine_id id)
static int
logical_ring_init(struct intel_engine_cs *engine)
{
- struct intel_context *dctx = engine->i915->kernel_context;
+ struct i915_gem_context *dctx = engine->i915->kernel_context;
int ret;
ret = i915_cmd_parser_init_ring(engine);
@@ -2270,7 +2270,7 @@ static u32 intel_lr_indirect_ctx_offset(struct intel_engine_cs *engine)
}
static int
-populate_lr_context(struct intel_context *ctx,
+populate_lr_context(struct i915_gem_context *ctx,
struct drm_i915_gem_object *ctx_obj,
struct intel_engine_cs *engine,
struct intel_ringbuffer *ringbuf)
@@ -2418,7 +2418,7 @@ populate_lr_context(struct intel_context *ctx,
* takes care of the bits that are LRC related: the per-engine backing
* objects and the logical ringbuffer.
*/
-void intel_lr_context_free(struct intel_context *ctx)
+void intel_lr_context_free(struct i915_gem_context *ctx)
{
int i;
@@ -2486,7 +2486,7 @@ uint32_t intel_lr_context_size(struct intel_engine_cs *engine)
*
* Return: non-zero on error.
*/
-static int execlists_context_deferred_alloc(struct intel_context *ctx,
+static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
struct drm_i915_gem_object *ctx_obj;
@@ -2536,7 +2536,7 @@ error_deref_obj:
}
void intel_lr_context_reset(struct drm_i915_private *dev_priv,
- struct intel_context *ctx)
+ struct i915_gem_context *ctx)
{
struct intel_engine_cs *engine;
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 1afba0331dc6..eb2e1d157fed 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -99,16 +99,18 @@ static inline void intel_logical_ring_emit_reg(struct intel_ringbuffer *ringbuf,
#define LRC_PPHWSP_PN (LRC_GUCSHR_PN + 1)
#define LRC_STATE_PN (LRC_PPHWSP_PN + 1)
-void intel_lr_context_free(struct intel_context *ctx);
+struct i915_gem_context;
+
+void intel_lr_context_free(struct i915_gem_context *ctx);
uint32_t intel_lr_context_size(struct intel_engine_cs *engine);
-void intel_lr_context_unpin(struct intel_context *ctx,
+void intel_lr_context_unpin(struct i915_gem_context *ctx,
struct intel_engine_cs *engine);
struct drm_i915_private;
void intel_lr_context_reset(struct drm_i915_private *dev_priv,
- struct intel_context *ctx);
-uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
+ struct i915_gem_context *ctx);
+uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx,
struct intel_engine_cs *engine);
/* Execlists */
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 929e7b4af2a4..b33c876fed20 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -119,7 +119,7 @@ struct intel_ringbuffer {
u32 last_retired_head;
};
-struct intel_context;
+struct i915_gem_context;
struct drm_i915_reg_table;
/*
@@ -310,7 +310,7 @@ struct intel_engine_cs {
wait_queue_head_t irq_queue;
- struct intel_context *last_context;
+ struct i915_gem_context *last_context;
struct intel_ring_hangcheck hangcheck;
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [CI 02/10] drm/i915: Apply lockdep annotations to i915_gem_context.c
2016-05-24 13:53 ` [CI 01/10] drm/i915: Rename struct intel_context Chris Wilson
@ 2016-05-24 13:53 ` Chris Wilson
2016-05-24 13:53 ` [CI 03/10] drm/i915: Rename and inline i915_gem_context_get() Chris Wilson
` (7 subsequent siblings)
8 siblings, 0 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-24 13:53 UTC (permalink / raw)
To: intel-gfx
Markup the functions that require the caller to hold struct_mutex with
lockdep_assert_held(). In the hopefully not-too-distant future we will
split the struct_mutex up, and in doing so we need to be sure that we
know what it protects - here the lockdep annotations are invaluable.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem_context.c | 18 +++++++++++++++---
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 961ef403154e..820748d65939 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3439,6 +3439,7 @@ static inline void i915_gem_context_reference(struct i915_gem_context *ctx)
static inline void i915_gem_context_unreference(struct i915_gem_context *ctx)
{
+ lockdep_assert_held(&ctx->i915->dev->struct_mutex);
kref_put(&ctx->ref, i915_gem_context_free);
}
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 8484da26b5d4..6b4c5adf4b89 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -153,6 +153,7 @@ void i915_gem_context_free(struct kref *ctx_ref)
{
struct i915_gem_context *ctx = container_of(ctx_ref, typeof(*ctx), ref);
+ lockdep_assert_held(&ctx->i915->dev->struct_mutex);
trace_i915_context_free(ctx);
if (i915.enable_execlists)
@@ -181,6 +182,8 @@ i915_gem_alloc_context_obj(struct drm_device *dev, size_t size)
struct drm_i915_gem_object *obj;
int ret;
+ lockdep_assert_held(&dev->struct_mutex);
+
obj = i915_gem_object_create(dev, size);
if (IS_ERR(obj))
return obj;
@@ -304,7 +307,7 @@ i915_gem_create_context(struct drm_device *dev,
struct i915_gem_context *ctx;
int ret = 0;
- BUG_ON(!mutex_is_locked(&dev->struct_mutex));
+ lockdep_assert_held(&dev->struct_mutex);
ctx = __create_hw_context(dev, file_priv);
if (IS_ERR(ctx))
@@ -368,6 +371,8 @@ void i915_gem_context_reset(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
+ lockdep_assert_held(&dev->struct_mutex);
+
if (i915.enable_execlists) {
struct i915_gem_context *ctx;
@@ -433,6 +438,8 @@ void i915_gem_context_lost(struct drm_i915_private *dev_priv)
{
struct intel_engine_cs *engine;
+ lockdep_assert_held(&dev_priv->dev->struct_mutex);
+
for_each_engine(engine, dev_priv) {
if (engine->last_context == NULL)
continue;
@@ -451,6 +458,8 @@ void i915_gem_context_fini(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
struct i915_gem_context *dctx = dev_priv->kernel_context;
+ lockdep_assert_held(&dev->struct_mutex);
+
if (dctx->legacy_hw_ctx.rcs_state)
i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
@@ -491,6 +500,8 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file)
{
struct drm_i915_file_private *file_priv = file->driver_priv;
+ lockdep_assert_held(&dev->struct_mutex);
+
idr_for_each(&file_priv->context_idr, context_idr_cleanup, NULL);
idr_destroy(&file_priv->context_idr);
}
@@ -500,6 +511,8 @@ i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id)
{
struct i915_gem_context *ctx;
+ lockdep_assert_held(&file_priv->dev_priv->dev->struct_mutex);
+
ctx = idr_find(&file_priv->context_idr, id);
if (!ctx)
return ERR_PTR(-ENOENT);
@@ -852,10 +865,9 @@ unpin_out:
int i915_switch_context(struct drm_i915_gem_request *req)
{
struct intel_engine_cs *engine = req->engine;
- struct drm_i915_private *dev_priv = req->i915;
WARN_ON(i915.enable_execlists);
- WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex));
+ lockdep_assert_held(&req->i915->dev->struct_mutex);
if (engine->id != RCS ||
req->ctx->legacy_hw_ctx.rcs_state == NULL) {
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [CI 03/10] drm/i915: Rename and inline i915_gem_context_get()
2016-05-24 13:53 ` [CI 01/10] drm/i915: Rename struct intel_context Chris Wilson
2016-05-24 13:53 ` [CI 02/10] drm/i915: Apply lockdep annotations to i915_gem_context.c Chris Wilson
@ 2016-05-24 13:53 ` Chris Wilson
2016-05-24 13:53 ` [CI 04/10] drm/i915: Name the inner most per-engine intel_context struct Chris Wilson
` (6 subsequent siblings)
8 siblings, 0 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-24 13:53 UTC (permalink / raw)
To: intel-gfx
i915_gem_context_get() is a very simple wrapper around idr_find(), so
simple that it would be smaller to do the lookup inline. Also we use the
verb 'lookup' to return a pointer from a handle, freeing 'get' to imply
obtaining a reference to the context.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 17 +++++++++++++++--
drivers/gpu/drm/i915/i915_gem_context.c | 22 ++++------------------
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
3 files changed, 20 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 820748d65939..e0c932054cd8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3427,11 +3427,24 @@ void i915_gem_context_reset(struct drm_device *dev);
int i915_gem_context_open(struct drm_device *dev, struct drm_file *file);
void i915_gem_context_close(struct drm_device *dev, struct drm_file *file);
int i915_switch_context(struct drm_i915_gem_request *req);
-struct i915_gem_context *
-i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id);
void i915_gem_context_free(struct kref *ctx_ref);
struct drm_i915_gem_object *
i915_gem_alloc_context_obj(struct drm_device *dev, size_t size);
+
+static inline struct i915_gem_context *
+i915_gem_context_lookup(struct drm_i915_file_private *file_priv, u32 id)
+{
+ struct i915_gem_context *ctx;
+
+ lockdep_assert_held(&file_priv->dev_priv->dev->struct_mutex);
+
+ ctx = idr_find(&file_priv->context_idr, id);
+ if (!ctx)
+ return ERR_PTR(-ENOENT);
+
+ return ctx;
+}
+
static inline void i915_gem_context_reference(struct i915_gem_context *ctx)
{
kref_get(&ctx->ref);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 6b4c5adf4b89..a3f9adb22a73 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -506,20 +506,6 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file)
idr_destroy(&file_priv->context_idr);
}
-struct i915_gem_context *
-i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id)
-{
- struct i915_gem_context *ctx;
-
- lockdep_assert_held(&file_priv->dev_priv->dev->struct_mutex);
-
- ctx = idr_find(&file_priv->context_idr, id);
- if (!ctx)
- return ERR_PTR(-ENOENT);
-
- return ctx;
-}
-
static inline int
mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags)
{
@@ -951,7 +937,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
if (ret)
return ret;
- ctx = i915_gem_context_get(file_priv, args->ctx_id);
+ ctx = i915_gem_context_lookup(file_priv, args->ctx_id);
if (IS_ERR(ctx)) {
mutex_unlock(&dev->struct_mutex);
return PTR_ERR(ctx);
@@ -977,7 +963,7 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
if (ret)
return ret;
- ctx = i915_gem_context_get(file_priv, args->ctx_id);
+ ctx = i915_gem_context_lookup(file_priv, args->ctx_id);
if (IS_ERR(ctx)) {
mutex_unlock(&dev->struct_mutex);
return PTR_ERR(ctx);
@@ -1020,7 +1006,7 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
if (ret)
return ret;
- ctx = i915_gem_context_get(file_priv, args->ctx_id);
+ ctx = i915_gem_context_lookup(file_priv, args->ctx_id);
if (IS_ERR(ctx)) {
mutex_unlock(&dev->struct_mutex);
return PTR_ERR(ctx);
@@ -1072,7 +1058,7 @@ int i915_gem_context_reset_stats_ioctl(struct drm_device *dev,
if (ret)
return ret;
- ctx = i915_gem_context_get(file->driver_priv, args->ctx_id);
+ ctx = i915_gem_context_lookup(file->driver_priv, args->ctx_id);
if (IS_ERR(ctx)) {
mutex_unlock(&dev->struct_mutex);
return PTR_ERR(ctx);
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index e61b92d7b0bc..84d990331abd 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1075,7 +1075,7 @@ i915_gem_validate_context(struct drm_device *dev, struct drm_file *file,
if (engine->id != RCS && ctx_id != DEFAULT_CONTEXT_HANDLE)
return ERR_PTR(-EINVAL);
- ctx = i915_gem_context_get(file->driver_priv, ctx_id);
+ ctx = i915_gem_context_lookup(file->driver_priv, ctx_id);
if (IS_ERR(ctx))
return ctx;
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [CI 04/10] drm/i915: Name the inner most per-engine intel_context struct
2016-05-24 13:53 ` [CI 01/10] drm/i915: Rename struct intel_context Chris Wilson
2016-05-24 13:53 ` [CI 02/10] drm/i915: Apply lockdep annotations to i915_gem_context.c Chris Wilson
2016-05-24 13:53 ` [CI 03/10] drm/i915: Rename and inline i915_gem_context_get() Chris Wilson
@ 2016-05-24 13:53 ` Chris Wilson
2016-05-24 13:53 ` [CI 05/10] drm/i915: Move pinning of dev_priv->kernel_context into its creator Chris Wilson
` (5 subsequent siblings)
8 siblings, 0 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-24 13:53 UTC (permalink / raw)
To: intel-gfx
We want to give a name to the currently anonymous per-engine struct
inside the context, so that we can assign it to a local variable and
save clumsy typing. The name we have chosen is intel_context as it
reflects the HW facing portion of the context state (the logical context
state, the registers, the ringbuffer etc).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Dave Gordon <david.s.gordon@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/i915_guc_submission.c | 15 ++---
drivers/gpu/drm/i915/intel_lrc.c | 90 +++++++++++++++---------------
3 files changed, 51 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e0c932054cd8..784978d33758 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -869,7 +869,7 @@ struct i915_gem_context {
} legacy_hw_ctx;
/* Execlists */
- struct {
+ struct intel_context {
struct drm_i915_gem_object *state;
struct intel_ringbuffer *ringbuf;
int pin_count;
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index b42a3adc3fa4..7b3c96e6ea37 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -363,7 +363,6 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
struct i915_gem_context *ctx = client->owner;
struct guc_context_desc desc;
struct sg_table *sg;
- enum intel_engine_id id;
u32 gfx_addr;
memset(&desc, 0, sizeof(desc));
@@ -373,10 +372,10 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
desc.priority = client->priority;
desc.db_id = client->doorbell_id;
- for_each_engine_id(engine, dev_priv, id) {
+ for_each_engine(engine, dev_priv) {
+ struct intel_context *ce = &ctx->engine[engine->id];
struct guc_execlist_context *lrc = &desc.lrc[engine->guc_id];
struct drm_i915_gem_object *obj;
- uint64_t ctx_desc;
/* TODO: We have a design issue to be solved here. Only when we
* receive the first batch, we know which engine is used by the
@@ -385,20 +384,18 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
* for now who owns a GuC client. But for future owner of GuC
* client, need to make sure lrc is pinned prior to enter here.
*/
- obj = ctx->engine[id].state;
- if (!obj)
+ if (!ce->state)
break; /* XXX: continue? */
- ctx_desc = intel_lr_context_descriptor(ctx, engine);
- lrc->context_desc = (u32)ctx_desc;
+ lrc->context_desc = lower_32_bits(ce->lrc_desc);
/* The state page is after PPHWSP */
- gfx_addr = i915_gem_obj_ggtt_offset(obj);
+ gfx_addr = i915_gem_obj_ggtt_offset(ce->state);
lrc->ring_lcra = gfx_addr + LRC_STATE_PN * PAGE_SIZE;
lrc->context_id = (client->ctx_index << GUC_ELC_CTXID_OFFSET) |
(engine->guc_id << GUC_ELC_ENGINE_OFFSET);
- obj = ctx->engine[id].ringbuf->obj;
+ obj = ce->ringbuf->obj;
gfx_addr = i915_gem_obj_ggtt_offset(obj);
lrc->ring_begin = gfx_addr;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 3d95b26b32ef..a1dba678942d 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -300,7 +300,7 @@ logical_ring_init_platform_invariants(struct intel_engine_cs *engine)
* descriptor for a pinned context
*
* @ctx: Context to work on
- * @ring: Engine the descriptor will be used with
+ * @engine: Engine the descriptor will be used with
*
* The context descriptor encodes various attributes of a context,
* including its GTT address and some flags. Because it's fairly
@@ -318,16 +318,17 @@ static void
intel_lr_context_descriptor_update(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
+ struct intel_context *ce = &ctx->engine[engine->id];
u64 desc;
BUILD_BUG_ON(MAX_CONTEXT_HW_ID > (1<<GEN8_CTX_ID_WIDTH));
desc = engine->ctx_desc_template; /* bits 0-11 */
- desc |= ctx->engine[engine->id].lrc_vma->node.start + /* bits 12-31 */
- LRC_PPHWSP_PN * PAGE_SIZE;
+ desc |= ce->lrc_vma->node.start + LRC_PPHWSP_PN * PAGE_SIZE;
+ /* bits 12-31 */
desc |= (u64)ctx->hw_id << GEN8_CTX_ID_SHIFT; /* bits 32-52 */
- ctx->engine[engine->id].lrc_desc = desc;
+ ce->lrc_desc = desc;
}
uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx,
@@ -674,6 +675,7 @@ static int execlists_move_to_gpu(struct drm_i915_gem_request *req,
int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request)
{
struct intel_engine_cs *engine = request->engine;
+ struct intel_context *ce = &request->ctx->engine[engine->id];
int ret;
/* Flush enough space to reduce the likelihood of waiting after
@@ -682,13 +684,13 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request
*/
request->reserved_space += EXECLISTS_REQUEST_SIZE;
- if (request->ctx->engine[engine->id].state == NULL) {
+ if (!ce->state) {
ret = execlists_context_deferred_alloc(request->ctx, engine);
if (ret)
return ret;
}
- request->ringbuf = request->ctx->engine[engine->id].ringbuf;
+ request->ringbuf = ce->ringbuf;
if (i915.enable_guc_submission) {
/*
@@ -709,12 +711,12 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request
if (ret)
goto err_unpin;
- if (!request->ctx->engine[engine->id].initialised) {
+ if (!ce->initialised) {
ret = engine->init_context(request);
if (ret)
goto err_unpin;
- request->ctx->engine[engine->id].initialised = true;
+ ce->initialised = true;
}
/* Note that after this point, we have committed to using
@@ -933,24 +935,22 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
struct drm_i915_private *dev_priv = ctx->i915;
- struct drm_i915_gem_object *ctx_obj;
- struct intel_ringbuffer *ringbuf;
+ struct intel_context *ce = &ctx->engine[engine->id];
void *vaddr;
u32 *lrc_reg_state;
int ret;
lockdep_assert_held(&ctx->i915->dev->struct_mutex);
- if (ctx->engine[engine->id].pin_count++)
+ if (ce->pin_count++)
return 0;
- ctx_obj = ctx->engine[engine->id].state;
- ret = i915_gem_obj_ggtt_pin(ctx_obj, GEN8_LR_CONTEXT_ALIGN,
- PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
+ ret = i915_gem_obj_ggtt_pin(ce->state, GEN8_LR_CONTEXT_ALIGN,
+ PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
if (ret)
goto err;
- vaddr = i915_gem_object_pin_map(ctx_obj);
+ vaddr = i915_gem_object_pin_map(ce->state);
if (IS_ERR(vaddr)) {
ret = PTR_ERR(vaddr);
goto unpin_ctx_obj;
@@ -958,17 +958,17 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
lrc_reg_state = vaddr + LRC_STATE_PN * PAGE_SIZE;
- ringbuf = ctx->engine[engine->id].ringbuf;
- ret = intel_pin_and_map_ringbuffer_obj(dev_priv, ringbuf);
+ ret = intel_pin_and_map_ringbuffer_obj(dev_priv, ce->ringbuf);
if (ret)
goto unpin_map;
i915_gem_context_reference(ctx);
- ctx->engine[engine->id].lrc_vma = i915_gem_obj_to_ggtt(ctx_obj);
+ ce->lrc_vma = i915_gem_obj_to_ggtt(ce->state);
intel_lr_context_descriptor_update(ctx, engine);
- lrc_reg_state[CTX_RING_BUFFER_START+1] = ringbuf->vma->node.start;
- ctx->engine[engine->id].lrc_reg_state = lrc_reg_state;
- ctx_obj->dirty = true;
+
+ lrc_reg_state[CTX_RING_BUFFER_START+1] = ce->ringbuf->vma->node.start;
+ ce->lrc_reg_state = lrc_reg_state;
+ ce->state->dirty = true;
/* Invalidate GuC TLB. */
if (i915.enable_guc_submission)
@@ -977,34 +977,33 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
return 0;
unpin_map:
- i915_gem_object_unpin_map(ctx_obj);
+ i915_gem_object_unpin_map(ce->state);
unpin_ctx_obj:
- i915_gem_object_ggtt_unpin(ctx_obj);
+ i915_gem_object_ggtt_unpin(ce->state);
err:
- ctx->engine[engine->id].pin_count = 0;
+ ce->pin_count = 0;
return ret;
}
void intel_lr_context_unpin(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
- struct drm_i915_gem_object *ctx_obj;
+ struct intel_context *ce = &ctx->engine[engine->id];
lockdep_assert_held(&ctx->i915->dev->struct_mutex);
- GEM_BUG_ON(ctx->engine[engine->id].pin_count == 0);
+ GEM_BUG_ON(ce->pin_count == 0);
- if (--ctx->engine[engine->id].pin_count)
+ if (--ce->pin_count)
return;
- intel_unpin_ringbuffer_obj(ctx->engine[engine->id].ringbuf);
+ intel_unpin_ringbuffer_obj(ce->ringbuf);
- ctx_obj = ctx->engine[engine->id].state;
- i915_gem_object_unpin_map(ctx_obj);
- i915_gem_object_ggtt_unpin(ctx_obj);
+ i915_gem_object_unpin_map(ce->state);
+ i915_gem_object_ggtt_unpin(ce->state);
- ctx->engine[engine->id].lrc_vma = NULL;
- ctx->engine[engine->id].lrc_desc = 0;
- ctx->engine[engine->id].lrc_reg_state = NULL;
+ ce->lrc_vma = NULL;
+ ce->lrc_desc = 0;
+ ce->lrc_reg_state = NULL;
i915_gem_context_unreference(ctx);
}
@@ -2490,12 +2489,13 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
struct drm_i915_gem_object *ctx_obj;
+ struct intel_context *ce = &ctx->engine[engine->id];
uint32_t context_size;
struct intel_ringbuffer *ringbuf;
int ret;
WARN_ON(ctx->legacy_hw_ctx.rcs_state != NULL);
- WARN_ON(ctx->engine[engine->id].state);
+ WARN_ON(ce->state);
context_size = round_up(intel_lr_context_size(engine), 4096);
@@ -2520,9 +2520,9 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
goto error_ringbuf;
}
- ctx->engine[engine->id].ringbuf = ringbuf;
- ctx->engine[engine->id].state = ctx_obj;
- ctx->engine[engine->id].initialised = engine->init_context == NULL;
+ ce->ringbuf = ringbuf;
+ ce->state = ctx_obj;
+ ce->initialised = engine->init_context == NULL;
return 0;
@@ -2530,8 +2530,8 @@ error_ringbuf:
intel_ringbuffer_free(ringbuf);
error_deref_obj:
drm_gem_object_unreference(&ctx_obj->base);
- ctx->engine[engine->id].ringbuf = NULL;
- ctx->engine[engine->id].state = NULL;
+ ce->ringbuf = NULL;
+ ce->state = NULL;
return ret;
}
@@ -2541,10 +2541,8 @@ void intel_lr_context_reset(struct drm_i915_private *dev_priv,
struct intel_engine_cs *engine;
for_each_engine(engine, dev_priv) {
- struct drm_i915_gem_object *ctx_obj =
- ctx->engine[engine->id].state;
- struct intel_ringbuffer *ringbuf =
- ctx->engine[engine->id].ringbuf;
+ struct intel_context *ce = &ctx->engine[engine->id];
+ struct drm_i915_gem_object *ctx_obj = ce->state;
void *vaddr;
uint32_t *reg_state;
@@ -2563,7 +2561,7 @@ void intel_lr_context_reset(struct drm_i915_private *dev_priv,
i915_gem_object_unpin_map(ctx_obj);
- ringbuf->head = 0;
- ringbuf->tail = 0;
+ ce->ringbuf->head = 0;
+ ce->ringbuf->tail = 0;
}
}
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [CI 05/10] drm/i915: Move pinning of dev_priv->kernel_context into its creator
2016-05-24 13:53 ` [CI 01/10] drm/i915: Rename struct intel_context Chris Wilson
` (2 preceding siblings ...)
2016-05-24 13:53 ` [CI 04/10] drm/i915: Name the inner most per-engine intel_context struct Chris Wilson
@ 2016-05-24 13:53 ` Chris Wilson
2016-05-24 13:53 ` [CI 06/10] drm/i915: Show i915_gem_context owner in debugfs Chris Wilson
` (4 subsequent siblings)
8 siblings, 0 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-24 13:53 UTC (permalink / raw)
To: intel-gfx
Rather than have every context ask "am I owned by the kernel? pin!",
move that logic into the creator of the kernel context, in order to
improve code comprehension.
v2: Throw away the user_handle on failure to allocate the ppgtt.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_gem_context.c | 53 +++++++++++++++------------------
1 file changed, 24 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index a3f9adb22a73..104d4819aca8 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -303,9 +303,7 @@ static struct i915_gem_context *
i915_gem_create_context(struct drm_device *dev,
struct drm_i915_file_private *file_priv)
{
- const bool is_global_default_ctx = file_priv == NULL;
struct i915_gem_context *ctx;
- int ret = 0;
lockdep_assert_held(&dev->struct_mutex);
@@ -313,30 +311,15 @@ i915_gem_create_context(struct drm_device *dev,
if (IS_ERR(ctx))
return ctx;
- if (is_global_default_ctx && ctx->legacy_hw_ctx.rcs_state) {
- /* We may need to do things with the shrinker which
- * require us to immediately switch back to the default
- * context. This can cause a problem as pinning the
- * default context also requires GTT space which may not
- * be available. To avoid this we always pin the default
- * context.
- */
- ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
- get_context_alignment(to_i915(dev)), 0);
- if (ret) {
- DRM_DEBUG_DRIVER("Couldn't pin %d\n", ret);
- goto err_destroy;
- }
- }
-
if (USES_FULL_PPGTT(dev)) {
struct i915_hw_ppgtt *ppgtt = i915_ppgtt_create(dev, file_priv);
- if (IS_ERR_OR_NULL(ppgtt)) {
+ if (IS_ERR(ppgtt)) {
DRM_DEBUG_DRIVER("PPGTT setup failed (%ld)\n",
PTR_ERR(ppgtt));
- ret = PTR_ERR(ppgtt);
- goto err_unpin;
+ idr_remove(&file_priv->context_idr, ctx->user_handle);
+ i915_gem_context_unreference(ctx);
+ return ERR_CAST(ppgtt);
}
ctx->ppgtt = ppgtt;
@@ -345,14 +328,6 @@ i915_gem_create_context(struct drm_device *dev,
trace_i915_context_create(ctx);
return ctx;
-
-err_unpin:
- if (is_global_default_ctx && ctx->legacy_hw_ctx.rcs_state)
- i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
-err_destroy:
- idr_remove(&file_priv->context_idr, ctx->user_handle);
- i915_gem_context_unreference(ctx);
- return ERR_PTR(ret);
}
static void i915_gem_context_unpin(struct i915_gem_context *ctx,
@@ -426,6 +401,26 @@ int i915_gem_context_init(struct drm_device *dev)
return PTR_ERR(ctx);
}
+ if (ctx->legacy_hw_ctx.rcs_state) {
+ int ret;
+
+ /* We may need to do things with the shrinker which
+ * require us to immediately switch back to the default
+ * context. This can cause a problem as pinning the
+ * default context also requires GTT space which may not
+ * be available. To avoid this we always pin the default
+ * context.
+ */
+ ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
+ get_context_alignment(dev_priv), 0);
+ if (ret) {
+ DRM_ERROR("Failed to pinned default global context (error %d)\n",
+ ret);
+ i915_gem_context_unreference(ctx);
+ return ret;
+ }
+ }
+
dev_priv->kernel_context = ctx;
DRM_DEBUG_DRIVER("%s context support initialized\n",
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [CI 06/10] drm/i915: Show i915_gem_context owner in debugfs
2016-05-24 13:53 ` [CI 01/10] drm/i915: Rename struct intel_context Chris Wilson
` (3 preceding siblings ...)
2016-05-24 13:53 ` [CI 05/10] drm/i915: Move pinning of dev_priv->kernel_context into its creator Chris Wilson
@ 2016-05-24 13:53 ` Chris Wilson
2016-05-24 13:53 ` [CI 07/10] drm/i915: Put the kernel_context in drm_i915_private next to its friends Chris Wilson
` (3 subsequent siblings)
8 siblings, 0 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-24 13:53 UTC (permalink / raw)
To: intel-gfx
Print the context's owner (via the pid under file_priv) under debugfs.
In doing so, we must be careful that the filp is not accessed after it
is freed (notified via i915_gem_context_close).
v2: Mark the file_priv as closed.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 17 +++++++++++++++--
drivers/gpu/drm/i915/i915_gem_context.c | 3 ++-
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 424e11efa3e1..e81b795005b9 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2014,9 +2014,22 @@ static int i915_context_status(struct seq_file *m, void *unused)
continue;
seq_printf(m, "HW context %u ", ctx->hw_id);
+ if (IS_ERR(ctx->file_priv)) {
+ seq_puts(m, "(deleted) ");
+ } else if (ctx->file_priv) {
+ struct pid *pid = ctx->file_priv->file->pid;
+ struct task_struct *task;
+
+ task = get_pid_task(pid, PIDTYPE_PID);
+ if (task) {
+ seq_printf(m, "(%s [%d]) ",
+ task->comm, task->pid);
+ put_task_struct(task);
+ }
+ } else
+ seq_puts(m, "(kernel) ");
+
describe_ctx(m, ctx);
- if (ctx == dev_priv->kernel_context)
- seq_printf(m, "(kernel context) ");
if (i915.enable_execlists) {
seq_putc(m, '\n');
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 104d4819aca8..8d8c79b88816 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -468,6 +468,7 @@ static int context_idr_cleanup(int id, void *p, void *data)
{
struct i915_gem_context *ctx = p;
+ ctx->file_priv = ERR_PTR(-EBADF);
i915_gem_context_unreference(ctx);
return 0;
}
@@ -938,7 +939,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
return PTR_ERR(ctx);
}
- idr_remove(&ctx->file_priv->context_idr, ctx->user_handle);
+ idr_remove(&file_priv->context_idr, ctx->user_handle);
i915_gem_context_unreference(ctx);
mutex_unlock(&dev->struct_mutex);
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [CI 07/10] drm/i915: Put the kernel_context in drm_i915_private next to its friends
2016-05-24 13:53 ` [CI 01/10] drm/i915: Rename struct intel_context Chris Wilson
` (4 preceding siblings ...)
2016-05-24 13:53 ` [CI 06/10] drm/i915: Show i915_gem_context owner in debugfs Chris Wilson
@ 2016-05-24 13:53 ` Chris Wilson
2016-05-24 13:53 ` [CI 08/10] drm/i915: Merge legacy+execlists context structs Chris Wilson
` (2 subsequent siblings)
8 siblings, 0 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-24 13:53 UTC (permalink / raw)
To: intel-gfx
Just move the kernel_context member of drm_i915_private next to the
engines it is associated with.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 3 +--
drivers/gpu/drm/i915/i915_guc_submission.c | 5 +++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 784978d33758..b2428bceadd9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1760,6 +1760,7 @@ struct drm_i915_private {
wait_queue_head_t gmbus_wait_queue;
struct pci_dev *bridge_dev;
+ struct i915_gem_context *kernel_context;
struct intel_engine_cs engine[I915_NUM_ENGINES];
struct drm_i915_gem_object *semaphore_obj;
uint32_t last_seqno, next_seqno;
@@ -2017,8 +2018,6 @@ struct drm_i915_private {
void (*stop_engine)(struct intel_engine_cs *engine);
} gt;
- struct i915_gem_context *kernel_context;
-
/* perform PHY state sanity checks? */
bool chv_phy_assert[2];
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 7b3c96e6ea37..ac72451c571c 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -935,11 +935,12 @@ int i915_guc_submission_enable(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_guc *guc = &dev_priv->guc;
- struct i915_gem_context *ctx = dev_priv->kernel_context;
struct i915_guc_client *client;
/* client for execbuf submission */
- client = guc_client_alloc(dev, GUC_CTX_PRIORITY_KMD_NORMAL, ctx);
+ client = guc_client_alloc(dev,
+ GUC_CTX_PRIORITY_KMD_NORMAL,
+ dev_priv->kernel_context);
if (!client) {
DRM_ERROR("Failed to create execbuf guc_client\n");
return -ENOMEM;
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [CI 08/10] drm/i915: Merge legacy+execlists context structs
2016-05-24 13:53 ` [CI 01/10] drm/i915: Rename struct intel_context Chris Wilson
` (5 preceding siblings ...)
2016-05-24 13:53 ` [CI 07/10] drm/i915: Put the kernel_context in drm_i915_private next to its friends Chris Wilson
@ 2016-05-24 13:53 ` Chris Wilson
2016-05-24 13:53 ` [CI 09/10] drm/i915: Rearrange i915_gem_context Chris Wilson
2016-05-24 13:53 ` [CI 10/10] drm/i915/debugfs: Show context objects in i915_gem_objects Chris Wilson
8 siblings, 0 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-24 13:53 UTC (permalink / raw)
To: intel-gfx
struct intel_context contains two substructs, one for the legacy RCS and
one for every execlists engine. Since legacy RCS is a subset of the
execlists engine support, just combine the two substructs.
v2: Only pin the default context for legacy mode (the object only exists
for legacy, but adding i915.enable_execlists provides symmetry with the
cleanup functions).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 41 +++++-------------
drivers/gpu/drm/i915/i915_drv.h | 7 ---
drivers/gpu/drm/i915/i915_gem_context.c | 75 +++++++++++++++++++--------------
drivers/gpu/drm/i915/intel_lrc.c | 26 ------------
drivers/gpu/drm/i915/intel_lrc.h | 1 -
5 files changed, 55 insertions(+), 95 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index e81b795005b9..95a871b8f379 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -199,13 +199,6 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
seq_printf(m, " (frontbuffer: 0x%03x)", obj->frontbuffer_bits);
}
-static void describe_ctx(struct seq_file *m, struct i915_gem_context *ctx)
-{
- seq_putc(m, ctx->legacy_hw_ctx.initialized ? 'I' : 'i');
- seq_putc(m, ctx->remap_slice ? 'R' : 'r');
- seq_putc(m, ' ');
-}
-
static int i915_gem_object_list_info(struct seq_file *m, void *data)
{
struct drm_info_node *node = m->private;
@@ -2001,7 +1994,6 @@ static int i915_context_status(struct seq_file *m, void *unused)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_engine_cs *engine;
struct i915_gem_context *ctx;
- enum intel_engine_id id;
int ret;
ret = mutex_lock_interruptible(&dev->struct_mutex);
@@ -2009,10 +2001,6 @@ static int i915_context_status(struct seq_file *m, void *unused)
return ret;
list_for_each_entry(ctx, &dev_priv->context_list, link) {
- if (!i915.enable_execlists &&
- ctx->legacy_hw_ctx.rcs_state == NULL)
- continue;
-
seq_printf(m, "HW context %u ", ctx->hw_id);
if (IS_ERR(ctx->file_priv)) {
seq_puts(m, "(deleted) ");
@@ -2029,25 +2017,18 @@ static int i915_context_status(struct seq_file *m, void *unused)
} else
seq_puts(m, "(kernel) ");
- describe_ctx(m, ctx);
+ seq_putc(m, ctx->remap_slice ? 'R' : 'r');
+ seq_putc(m, '\n');
- if (i915.enable_execlists) {
+ for_each_engine(engine, dev_priv) {
+ struct intel_context *ce = &ctx->engine[engine->id];
+ seq_printf(m, "%s: ", engine->name);
+ seq_putc(m, ce->initialised ? 'I' : 'i');
+ if (ce->state)
+ describe_obj(m, ce->state);
+ if (ce->ringbuf)
+ describe_ctx_ringbuf(m, ce->ringbuf);
seq_putc(m, '\n');
- for_each_engine_id(engine, dev_priv, id) {
- struct drm_i915_gem_object *ctx_obj =
- ctx->engine[id].state;
- struct intel_ringbuffer *ringbuf =
- ctx->engine[id].ringbuf;
-
- seq_printf(m, "%s: ", engine->name);
- if (ctx_obj)
- describe_obj(m, ctx_obj);
- if (ringbuf)
- describe_ctx_ringbuf(m, ringbuf);
- seq_putc(m, '\n');
- }
- } else {
- describe_obj(m, ctx->legacy_hw_ctx.rcs_state);
}
seq_putc(m, '\n');
@@ -2062,10 +2043,10 @@ static void i915_dump_lrc_obj(struct seq_file *m,
struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
+ struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state;
struct page *page;
uint32_t *reg_state;
int j;
- struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state;
unsigned long ggtt_offset = 0;
seq_printf(m, "CONTEXT: %s %u\n", engine->name, ctx->hw_id);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b2428bceadd9..5832082d6e8f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -862,13 +862,6 @@ struct i915_gem_context {
/* Unique identifier for this context, used by the hw for tracking */
unsigned hw_id;
- /* Legacy ring buffer submission */
- struct {
- struct drm_i915_gem_object *rcs_state;
- bool initialized;
- } legacy_hw_ctx;
-
- /* Execlists */
struct intel_context {
struct drm_i915_gem_object *state;
struct intel_ringbuffer *ringbuf;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 8d8c79b88816..a3b11aac23a4 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -152,13 +152,11 @@ static void i915_gem_context_clean(struct i915_gem_context *ctx)
void i915_gem_context_free(struct kref *ctx_ref)
{
struct i915_gem_context *ctx = container_of(ctx_ref, typeof(*ctx), ref);
+ int i;
lockdep_assert_held(&ctx->i915->dev->struct_mutex);
trace_i915_context_free(ctx);
- if (i915.enable_execlists)
- intel_lr_context_free(ctx);
-
/*
* This context is going away and we need to remove all VMAs still
* around. This is to handle imported shared objects for which
@@ -168,8 +166,19 @@ void i915_gem_context_free(struct kref *ctx_ref)
i915_ppgtt_put(ctx->ppgtt);
- if (ctx->legacy_hw_ctx.rcs_state)
- drm_gem_object_unreference(&ctx->legacy_hw_ctx.rcs_state->base);
+ for (i = 0; i < I915_NUM_ENGINES; i++) {
+ struct intel_context *ce = &ctx->engine[i];
+
+ if (!ce->state)
+ continue;
+
+ WARN_ON(ce->pin_count);
+ if (ce->ringbuf)
+ intel_ringbuffer_free(ce->ringbuf);
+
+ drm_gem_object_unreference(&ce->state->base);
+ }
+
list_del(&ctx->link);
ida_simple_remove(&ctx->i915->context_hw_ida, ctx->hw_id);
@@ -266,7 +275,7 @@ __create_hw_context(struct drm_device *dev,
ret = PTR_ERR(obj);
goto err_out;
}
- ctx->legacy_hw_ctx.rcs_state = obj;
+ ctx->engine[RCS].state = obj;
}
/* Default context will never have a file_priv */
@@ -336,8 +345,11 @@ static void i915_gem_context_unpin(struct i915_gem_context *ctx,
if (i915.enable_execlists) {
intel_lr_context_unpin(ctx, engine);
} else {
- if (engine->id == RCS && ctx->legacy_hw_ctx.rcs_state)
- i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
+ struct intel_context *ce = &ctx->engine[engine->id];
+
+ if (ce->state)
+ i915_gem_object_ggtt_unpin(ce->state);
+
i915_gem_context_unreference(ctx);
}
}
@@ -401,7 +413,7 @@ int i915_gem_context_init(struct drm_device *dev)
return PTR_ERR(ctx);
}
- if (ctx->legacy_hw_ctx.rcs_state) {
+ if (!i915.enable_execlists && ctx->engine[RCS].state) {
int ret;
/* We may need to do things with the shrinker which
@@ -411,7 +423,7 @@ int i915_gem_context_init(struct drm_device *dev)
* be available. To avoid this we always pin the default
* context.
*/
- ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
+ ret = i915_gem_obj_ggtt_pin(ctx->engine[RCS].state,
get_context_alignment(dev_priv), 0);
if (ret) {
DRM_ERROR("Failed to pinned default global context (error %d)\n",
@@ -436,15 +448,17 @@ void i915_gem_context_lost(struct drm_i915_private *dev_priv)
lockdep_assert_held(&dev_priv->dev->struct_mutex);
for_each_engine(engine, dev_priv) {
- if (engine->last_context == NULL)
- continue;
+ if (engine->last_context) {
+ i915_gem_context_unpin(engine->last_context, engine);
+ engine->last_context = NULL;
+ }
- i915_gem_context_unpin(engine->last_context, engine);
- engine->last_context = NULL;
+ /* Force the GPU state to be reinitialised on enabling */
+ dev_priv->kernel_context->engine[engine->id].initialised =
+ engine->init_context == NULL;
}
/* Force the GPU state to be reinitialised on enabling */
- dev_priv->kernel_context->legacy_hw_ctx.initialized = false;
dev_priv->kernel_context->remap_slice = ALL_L3_SLICES(dev_priv);
}
@@ -455,8 +469,8 @@ void i915_gem_context_fini(struct drm_device *dev)
lockdep_assert_held(&dev->struct_mutex);
- if (dctx->legacy_hw_ctx.rcs_state)
- i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
+ if (!i915.enable_execlists && dctx->engine[RCS].state)
+ i915_gem_object_ggtt_unpin(dctx->engine[RCS].state);
i915_gem_context_unreference(dctx);
dev_priv->kernel_context = NULL;
@@ -564,7 +578,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags)
intel_ring_emit(engine, MI_NOOP);
intel_ring_emit(engine, MI_SET_CONTEXT);
intel_ring_emit(engine,
- i915_gem_obj_ggtt_offset(req->ctx->legacy_hw_ctx.rcs_state) |
+ i915_gem_obj_ggtt_offset(req->ctx->engine[RCS].state) |
flags);
/*
* w/a: MI_SET_CONTEXT must always be followed by MI_NOOP
@@ -641,7 +655,7 @@ static inline bool skip_rcs_switch(struct i915_hw_ppgtt *ppgtt,
if (to->remap_slice)
return false;
- if (!to->legacy_hw_ctx.initialized)
+ if (!to->engine[RCS].initialised)
return false;
if (ppgtt && (intel_engine_flag(engine) & ppgtt->pd_dirty_rings))
@@ -706,7 +720,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
return 0;
/* Trying to pin first makes error handling easier. */
- ret = i915_gem_obj_ggtt_pin(to->legacy_hw_ctx.rcs_state,
+ ret = i915_gem_obj_ggtt_pin(to->engine[RCS].state,
get_context_alignment(engine->i915),
0);
if (ret)
@@ -729,7 +743,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
*
* XXX: We need a real interface to do this instead of trickery.
*/
- ret = i915_gem_object_set_to_gtt_domain(to->legacy_hw_ctx.rcs_state, false);
+ ret = i915_gem_object_set_to_gtt_domain(to->engine[RCS].state, false);
if (ret)
goto unpin_out;
@@ -744,7 +758,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
goto unpin_out;
}
- if (!to->legacy_hw_ctx.initialized || i915_gem_context_is_default(to))
+ if (!to->engine[RCS].initialised || i915_gem_context_is_default(to))
/* NB: If we inhibit the restore, the context is not allowed to
* die because future work may end up depending on valid address
* space. This means we must enforce that a page table load
@@ -768,8 +782,8 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
* MI_SET_CONTEXT instead of when the next seqno has completed.
*/
if (from != NULL) {
- from->legacy_hw_ctx.rcs_state->base.read_domains = I915_GEM_DOMAIN_INSTRUCTION;
- i915_vma_move_to_active(i915_gem_obj_to_ggtt(from->legacy_hw_ctx.rcs_state), req);
+ from->engine[RCS].state->base.read_domains = I915_GEM_DOMAIN_INSTRUCTION;
+ i915_vma_move_to_active(i915_gem_obj_to_ggtt(from->engine[RCS].state), req);
/* As long as MI_SET_CONTEXT is serializing, ie. it flushes the
* whole damn pipeline, we don't need to explicitly mark the
* object dirty. The only exception is that the context must be
@@ -777,10 +791,10 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
* able to defer doing this until we know the object would be
* swapped, but there is no way to do that yet.
*/
- from->legacy_hw_ctx.rcs_state->dirty = 1;
+ from->engine[RCS].state->dirty = 1;
/* obj is kept alive until the next request by its active ref */
- i915_gem_object_ggtt_unpin(from->legacy_hw_ctx.rcs_state);
+ i915_gem_object_ggtt_unpin(from->engine[RCS].state);
i915_gem_context_unreference(from);
}
i915_gem_context_reference(to);
@@ -815,19 +829,19 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
to->remap_slice &= ~(1<<i);
}
- if (!to->legacy_hw_ctx.initialized) {
+ if (!to->engine[RCS].initialised) {
if (engine->init_context) {
ret = engine->init_context(req);
if (ret)
return ret;
}
- to->legacy_hw_ctx.initialized = true;
+ to->engine[RCS].initialised = true;
}
return 0;
unpin_out:
- i915_gem_object_ggtt_unpin(to->legacy_hw_ctx.rcs_state);
+ i915_gem_object_ggtt_unpin(to->engine[RCS].state);
return ret;
}
@@ -851,8 +865,7 @@ int i915_switch_context(struct drm_i915_gem_request *req)
WARN_ON(i915.enable_execlists);
lockdep_assert_held(&req->i915->dev->struct_mutex);
- if (engine->id != RCS ||
- req->ctx->legacy_hw_ctx.rcs_state == NULL) {
+ if (!req->ctx->engine[engine->id].state) {
struct i915_gem_context *to = req->ctx;
struct i915_hw_ppgtt *ppgtt =
to->ppgtt ?: req->i915->mm.aliasing_ppgtt;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index a1dba678942d..53715037ab54 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2410,31 +2410,6 @@ populate_lr_context(struct i915_gem_context *ctx,
}
/**
- * intel_lr_context_free() - free the LRC specific bits of a context
- * @ctx: the LR context to free.
- *
- * The real context freeing is done in i915_gem_context_free: this only
- * takes care of the bits that are LRC related: the per-engine backing
- * objects and the logical ringbuffer.
- */
-void intel_lr_context_free(struct i915_gem_context *ctx)
-{
- int i;
-
- for (i = I915_NUM_ENGINES; --i >= 0; ) {
- struct intel_ringbuffer *ringbuf = ctx->engine[i].ringbuf;
- struct drm_i915_gem_object *ctx_obj = ctx->engine[i].state;
-
- if (!ctx_obj)
- continue;
-
- WARN_ON(ctx->engine[i].pin_count);
- intel_ringbuffer_free(ringbuf);
- drm_gem_object_unreference(&ctx_obj->base);
- }
-}
-
-/**
* intel_lr_context_size() - return the size of the context for an engine
* @ring: which engine to find the context size for
*
@@ -2494,7 +2469,6 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
struct intel_ringbuffer *ringbuf;
int ret;
- WARN_ON(ctx->legacy_hw_ctx.rcs_state != NULL);
WARN_ON(ce->state);
context_size = round_up(intel_lr_context_size(engine), 4096);
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index eb2e1d157fed..a8db42a9c50f 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -101,7 +101,6 @@ static inline void intel_logical_ring_emit_reg(struct intel_ringbuffer *ringbuf,
struct i915_gem_context;
-void intel_lr_context_free(struct i915_gem_context *ctx);
uint32_t intel_lr_context_size(struct intel_engine_cs *engine);
void intel_lr_context_unpin(struct i915_gem_context *ctx,
struct intel_engine_cs *engine);
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [CI 09/10] drm/i915: Rearrange i915_gem_context
2016-05-24 13:53 ` [CI 01/10] drm/i915: Rename struct intel_context Chris Wilson
` (6 preceding siblings ...)
2016-05-24 13:53 ` [CI 08/10] drm/i915: Merge legacy+execlists context structs Chris Wilson
@ 2016-05-24 13:53 ` Chris Wilson
2016-05-24 13:53 ` [CI 10/10] drm/i915/debugfs: Show context objects in i915_gem_objects Chris Wilson
8 siblings, 0 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-24 13:53 UTC (permalink / raw)
To: intel-gfx
Pack the integers and related types together inside the struct.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5832082d6e8f..490f0d821d2b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -829,7 +829,6 @@ struct i915_ctx_hang_stats {
/* This must match up with the value previously used for execbuf2.rsvd1. */
#define DEFAULT_CONTEXT_HANDLE 0
-#define CONTEXT_NO_ZEROMAP (1<<0)
/**
* struct i915_gem_context - as the name implies, represents a context.
* @ref: reference count.
@@ -851,28 +850,31 @@ struct i915_ctx_hang_stats {
*/
struct i915_gem_context {
struct kref ref;
- int user_handle;
- uint8_t remap_slice;
struct drm_i915_private *i915;
- int flags;
struct drm_i915_file_private *file_priv;
- struct i915_ctx_hang_stats hang_stats;
struct i915_hw_ppgtt *ppgtt;
+ struct i915_ctx_hang_stats hang_stats;
+
/* Unique identifier for this context, used by the hw for tracking */
unsigned hw_id;
+ uint32_t user_handle;
+ unsigned long flags;
+#define CONTEXT_NO_ZEROMAP (1<<0)
struct intel_context {
struct drm_i915_gem_object *state;
struct intel_ringbuffer *ringbuf;
- int pin_count;
struct i915_vma *lrc_vma;
- u64 lrc_desc;
uint32_t *lrc_reg_state;
+ u64 lrc_desc;
+ int pin_count;
bool initialised;
} engine[I915_NUM_ENGINES];
struct list_head link;
+
+ uint8_t remap_slice;
};
enum fb_op_origin {
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [CI 10/10] drm/i915/debugfs: Show context objects in i915_gem_objects
2016-05-24 13:53 ` [CI 01/10] drm/i915: Rename struct intel_context Chris Wilson
` (7 preceding siblings ...)
2016-05-24 13:53 ` [CI 09/10] drm/i915: Rearrange i915_gem_context Chris Wilson
@ 2016-05-24 13:53 ` Chris Wilson
8 siblings, 0 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-24 13:53 UTC (permalink / raw)
To: intel-gfx
One of the uses for i915_gem_objects is pin-pointing leaks. For this, we
can compare the number of allocated objects and who owns them, a
discrepancy here often indicates a kernel bug. One allocator of unreported
objects is for backing context objects, so include those in the listing.
v2: Take filelist_mutex which requires a little dance with struct_mutex
to avoid nesting filelist_mutex inside struct_mutex.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 38 ++++++++++++++++++++++++++++++++++++-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 95a871b8f379..17e2b1a1c4ad 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -417,6 +417,42 @@ static void print_batch_pool_stats(struct seq_file *m,
print_file_stats(m, "[k]batch pool", stats);
}
+static int per_file_ctx_stats(int id, void *ptr, void *data)
+{
+ struct i915_gem_context *ctx = ptr;
+ int n;
+
+ for (n = 0; n < ARRAY_SIZE(ctx->engine); n++) {
+ if (ctx->engine[n].state)
+ per_file_stats(0, ctx->engine[n].state, data);
+ if (ctx->engine[n].ringbuf)
+ per_file_stats(0, ctx->engine[n].ringbuf->obj, data);
+ }
+
+ return 0;
+}
+
+static void print_context_stats(struct seq_file *m,
+ struct drm_i915_private *dev_priv)
+{
+ struct file_stats stats;
+ struct drm_file *file;
+
+ memset(&stats, 0, sizeof(stats));
+
+ mutex_lock(&dev_priv->dev->struct_mutex);
+ if (dev_priv->kernel_context)
+ per_file_ctx_stats(0, dev_priv->kernel_context, &stats);
+
+ list_for_each_entry(file, &dev_priv->dev->filelist, lhead) {
+ struct drm_i915_file_private *fpriv = file->driver_priv;
+ idr_for_each(&fpriv->context_idr, per_file_ctx_stats, &stats);
+ }
+ mutex_unlock(&dev_priv->dev->struct_mutex);
+
+ print_file_stats(m, "[k]contexts", stats);
+}
+
#define count_vmas(list, member) do { \
list_for_each_entry(vma, list, member) { \
size += i915_gem_obj_total_ggtt_size(vma->obj); \
@@ -521,10 +557,10 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
seq_putc(m, '\n');
print_batch_pool_stats(m, dev_priv);
-
mutex_unlock(&dev->struct_mutex);
mutex_lock(&dev->filelist_mutex);
+ print_context_stats(m, dev_priv);
list_for_each_entry_reverse(file, &dev->filelist, lhead) {
struct file_stats stats;
struct task_struct *task;
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 42+ messages in thread
* ✗ Ro.CI.BAT: warning for series starting with [v2,01/11] drm/i915: Rename struct intel_context (rev10)
2016-05-23 11:34 Context churn Chris Wilson
` (13 preceding siblings ...)
2016-05-24 13:53 ` [CI 01/10] drm/i915: Rename struct intel_context Chris Wilson
@ 2016-05-24 14:19 ` Patchwork
14 siblings, 0 replies; 42+ messages in thread
From: Patchwork @ 2016-05-24 14:19 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v2,01/11] drm/i915: Rename struct intel_context (rev10)
URL : https://patchwork.freedesktop.org/series/7564/
State : warning
== Summary ==
Series 7564v10 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/7564/revisions/10/mbox
Test drv_module_reload_basic:
skip -> PASS (ro-ivb-i7-3770)
Test gem_busy:
Subgroup basic-parallel-blt:
pass -> DMESG-WARN (ro-skl-i7-6700hq)
Test gem_exec_basic:
Subgroup readonly-render:
pass -> DMESG-WARN (ro-skl-i7-6700hq)
Test kms_flip:
Subgroup basic-flip-vs-wf_vblank:
dmesg-warn -> PASS (ro-skl-i7-6700hq)
Test kms_frontbuffer_tracking:
Subgroup basic:
pass -> DMESG-WARN (ro-skl-i7-6700hq)
Test kms_setmode:
Subgroup basic-clone-single-crtc:
dmesg-warn -> PASS (ro-skl-i7-6700hq)
Test kms_sink_crc_basic:
pass -> SKIP (ro-skl-i7-6700hq)
Test pm_rps:
Subgroup basic-api:
pass -> DMESG-WARN (ro-skl-i7-6700hq)
ro-bdw-i5-5250u total:209 pass:172 dwarn:0 dfail:0 fail:0 skip:37
ro-bdw-i7-5557U total:209 pass:197 dwarn:0 dfail:0 fail:0 skip:12
ro-bdw-i7-5600u total:209 pass:180 dwarn:0 dfail:0 fail:1 skip:28
ro-bsw-n3050 total:209 pass:168 dwarn:0 dfail:0 fail:2 skip:39
ro-byt-n2820 total:209 pass:169 dwarn:0 dfail:0 fail:3 skip:37
ro-hsw-i3-4010u total:209 pass:186 dwarn:0 dfail:0 fail:0 skip:23
ro-hsw-i7-4770r total:209 pass:186 dwarn:0 dfail:0 fail:0 skip:23
ro-ilk-i7-620lm total:209 pass:146 dwarn:0 dfail:0 fail:1 skip:62
ro-ilk1-i5-650 total:204 pass:146 dwarn:0 dfail:0 fail:1 skip:57
ro-ivb-i7-3770 total:209 pass:177 dwarn:0 dfail:0 fail:0 skip:32
ro-ivb2-i7-3770 total:209 pass:181 dwarn:0 dfail:0 fail:0 skip:28
ro-skl-i7-6700hq total:204 pass:178 dwarn:4 dfail:0 fail:0 skip:22
ro-snb-i7-2620M total:209 pass:170 dwarn:0 dfail:0 fail:1 skip:38
Results at /archive/results/CI_IGT_test/RO_Patchwork_995/
3a7ef25 drm-intel-nightly: 2016y-05m-24d-12h-43m-51s UTC integration manifest
ec465c9 drm/i915/debugfs: Show context objects in i915_gem_objects
d9891a5 drm/i915: Rearrange i915_gem_context
1acdd6e drm/i915: Merge legacy+execlists context structs
23c3b42 drm/i915: Put the kernel_context in drm_i915_private next to its friends
49c2c78 drm/i915: Show i915_gem_context owner in debugfs
3f52ed9 drm/i915: Move pinning of dev_priv->kernel_context into its creator
685ca45 drm/i915: Name the inner most per-engine intel_context struct
746bc44 drm/i915: Rename and inline i915_gem_context_get()
5224ddc drm/i915: Apply lockdep annotations to i915_gem_context.c
3da12fa drm/i915: Rename struct intel_context
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 10/11] drm/i915: Rearrange i915_gem_context
2016-05-23 11:34 ` [PATCH v2 10/11] drm/i915: Rearrange i915_gem_context Chris Wilson
@ 2016-05-25 9:15 ` Joonas Lahtinen
0 siblings, 0 replies; 42+ messages in thread
From: Joonas Lahtinen @ 2016-05-25 9:15 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On ma, 2016-05-23 at 12:34 +0100, Chris Wilson wrote:
> Pack the integers and related types together inside the struct.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9d7a088efc16..5dbce15e6e4c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -829,7 +829,6 @@ struct i915_ctx_hang_stats {
> /* This must match up with the value previously used for execbuf2.rsvd1. */
> #define DEFAULT_CONTEXT_HANDLE 0
>
> -#define CONTEXT_NO_ZEROMAP (1<<0)
> /**
> * struct i915_gem_context - as the name implies, represents a context.
> * @ref: reference count.
> @@ -851,28 +850,31 @@ struct i915_ctx_hang_stats {
> */
> struct i915_gem_context {
> struct kref ref;
> - int user_handle;
> - uint8_t remap_slice;
> struct drm_i915_private *i915;
> - int flags;
> struct drm_i915_file_private *file_priv;
> - struct i915_ctx_hang_stats hang_stats;
> struct i915_hw_ppgtt *ppgtt;
>
> + struct i915_ctx_hang_stats hang_stats;
> +
> /* Unique identifier for this context, used by the hw for tracking */
> unsigned hw_id;
> + uint32_t user_handle;
> + unsigned long flags;
> +#define CONTEXT_NO_ZEROMAP (1<<0)
Add spaces (1 << 0), otherwise DIM will complain.
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>
> struct intel_context {
> struct drm_i915_gem_object *state;
> struct intel_ringbuffer *ringbuf;
> - int pin_count;
> struct i915_vma *lrc_vma;
> - u64 lrc_desc;
> uint32_t *lrc_reg_state;
> + u64 lrc_desc;
> + int pin_count;
> bool initialised;
> } engine[I915_NUM_ENGINES];
>
> struct list_head link;
> +
> + uint8_t remap_slice;
> };
>
> enum fb_op_origin {
--
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] 42+ messages in thread
* Re: [PATCH v2 07/11] drm/i915: Show i915_gem_context owner in debugfs
2016-05-23 11:34 ` [PATCH v2 07/11] drm/i915: Show i915_gem_context owner in debugfs Chris Wilson
2016-05-23 12:18 ` Tvrtko Ursulin
@ 2016-05-25 9:18 ` Joonas Lahtinen
1 sibling, 0 replies; 42+ messages in thread
From: Joonas Lahtinen @ 2016-05-25 9:18 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On ma, 2016-05-23 at 12:34 +0100, Chris Wilson wrote:
> Print the context's owner (via the pid under file_priv) under debugfs.
> In doing so, we must be careful that the filp is not accessed after it
> is freed (notified via i915_gem_context_close).
>
> v2: Mark the file_priv as closed.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 17 +++++++++++++++--
> drivers/gpu/drm/i915/i915_gem_context.c | 3 ++-
> 2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index ae28e6e9d603..945fe4710b37 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2014,9 +2014,22 @@ static int i915_context_status(struct seq_file *m, void *unused)
> continue;
>
> seq_printf(m, "HW context %u ", ctx->hw_id);
> + if (IS_ERR(ctx->file_priv)) {
> + seq_puts(m, "(deleted) ");
> + } else if (ctx->file_priv) {
> + struct pid *pid = ctx->file_priv->file->pid;
> + struct task_struct *task;
> +
> + task = get_pid_task(pid, PIDTYPE_PID);
> + if (task) {
> + seq_printf(m, "(%s [%d]) ",
> + task->comm, task->pid);
> + put_task_struct(task);
> + }
What if no task, should we display something still?
> + } else
> + seq_puts(m, "(kernel) ");
> +
> describe_ctx(m, ctx);
> - if (ctx == dev_priv->kernel_context)
> - seq_printf(m, "(kernel context) ");
>
> if (i915.enable_execlists) {
> seq_putc(m, '\n');
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 474174c20be2..e552c936d262 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -468,6 +468,7 @@ static int context_idr_cleanup(int id, void *p, void *data)
> {
> struct i915_gem_context *ctx = p;
>
> + ctx->file_priv = ERR_PTR(-EBADF);
> i915_gem_context_put(ctx);
> return 0;
> }
> @@ -936,7 +937,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
> return PTR_ERR(ctx);
> }
>
> - idr_remove(&ctx->file_priv->context_idr, ctx->user_handle);
> + idr_remove(&file_priv->context_idr, ctx->user_handle);
> i915_gem_context_put(ctx);
> mutex_unlock(&dev->struct_mutex);
>
--
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] 42+ messages in thread
* Re: [PATCH v2 06/11] drm/i915: Move pinning of dev_priv->kernel_context into its creator
2016-05-23 11:34 ` [PATCH v2 06/11] drm/i915: Move pinning of dev_priv->kernel_context into its creator Chris Wilson
2016-05-23 12:09 ` Tvrtko Ursulin
@ 2016-05-25 9:25 ` Joonas Lahtinen
1 sibling, 0 replies; 42+ messages in thread
From: Joonas Lahtinen @ 2016-05-25 9:25 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On ma, 2016-05-23 at 12:34 +0100, Chris Wilson wrote:
> Rather than have every context ask "am I owned by the kernel? pin!",
> move that logic into the creator of the kernel context, in order to
> improve code comprehension.
>
> v2: Throw away the user_handle on failure to allocate the ppgtt.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_context.c | 53 +++++++++++++++------------------
> 1 file changed, 24 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index a4c6377eea32..474174c20be2 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -303,9 +303,7 @@ static struct i915_gem_context *
> i915_gem_create_context(struct drm_device *dev,
> struct drm_i915_file_private *file_priv)
> {
> - const bool is_global_default_ctx = file_priv == NULL;
> struct i915_gem_context *ctx;
> - int ret = 0;
>
> BUG_ON(!mutex_is_locked(&dev->struct_mutex));
>
> @@ -313,30 +311,15 @@ i915_gem_create_context(struct drm_device *dev,
> if (IS_ERR(ctx))
> return ctx;
>
> - if (is_global_default_ctx && ctx->legacy_hw_ctx.rcs_state) {
> - /* We may need to do things with the shrinker which
> - * require us to immediately switch back to the default
> - * context. This can cause a problem as pinning the
> - * default context also requires GTT space which may not
> - * be available. To avoid this we always pin the default
> - * context.
> - */
> - ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
> - get_context_alignment(to_i915(dev)), 0);
> - if (ret) {
> - DRM_DEBUG_DRIVER("Couldn't pin %d\n", ret);
> - goto err_destroy;
> - }
> - }
> -
> if (USES_FULL_PPGTT(dev)) {
> struct i915_hw_ppgtt *ppgtt = i915_ppgtt_create(dev, file_priv);
>
> - if (IS_ERR_OR_NULL(ppgtt)) {
> + if (IS_ERR(ppgtt)) {
> DRM_DEBUG_DRIVER("PPGTT setup failed (%ld)\n",
> PTR_ERR(ppgtt));
> - ret = PTR_ERR(ppgtt);
> - goto err_unpin;
> + idr_remove(&file_priv->context_idr, ctx->user_handle);
> + i915_gem_context_put(ctx);
> + return ERR_CAST(ppgtt);
> }
>
> ctx->ppgtt = ppgtt;
> @@ -345,14 +328,6 @@ i915_gem_create_context(struct drm_device *dev,
> trace_i915_context_create(ctx);
>
> return ctx;
> -
> -err_unpin:
> - if (is_global_default_ctx && ctx->legacy_hw_ctx.rcs_state)
> - i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
> -err_destroy:
> - idr_remove(&file_priv->context_idr, ctx->user_handle);
> - i915_gem_context_put(ctx);
> - return ERR_PTR(ret);
Don't remove a perfectly good error path.
> }
>
> static void i915_gem_context_unpin(struct i915_gem_context *ctx,
> @@ -426,6 +401,26 @@ int i915_gem_context_init(struct drm_device *dev)
> return PTR_ERR(ctx);
> }
>
> + if (ctx->legacy_hw_ctx.rcs_state) {
> + int ret;
> +
> + /* We may need to do things with the shrinker which
> + * require us to immediately switch back to the default
> + * context. This can cause a problem as pinning the
> + * default context also requires GTT space which may not
> + * be available. To avoid this we always pin the default
> + * context.
> + */
> + ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
> + get_context_alignment(dev_priv), 0);
> + if (ret) {
> + DRM_ERROR("Failed to pinned default global context (error %d)\n",
> + ret);
> + i915_gem_context_put(ctx);
And please make this into teardown path.
> + return ret;
> + }
> + }
> +
> dev_priv->kernel_context = ctx;
>
> DRM_DEBUG_DRIVER("%s context support initialized\n",
--
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] 42+ messages in thread
* Re: [PATCH v2 11/11] drm/i915: Show context objects in debugfs/i915_gem_objects
2016-05-23 11:34 ` [PATCH v2 11/11] drm/i915: Show context objects in debugfs/i915_gem_objects Chris Wilson
2016-05-23 12:31 ` Tvrtko Ursulin
@ 2016-05-25 9:33 ` Joonas Lahtinen
1 sibling, 0 replies; 42+ messages in thread
From: Joonas Lahtinen @ 2016-05-25 9:33 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On ma, 2016-05-23 at 12:34 +0100, Chris Wilson wrote:
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Some commit message would be nice.
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 30cb26fe2fa9..3d14eb3215e1 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -417,6 +417,40 @@ static void print_batch_pool_stats(struct seq_file *m,
> print_file_stats(m, "[k]batch pool", stats);
> }
>
> +static int per_file_ctx_stats(int id, void *ptr, void *data)
> +{
> + struct i915_gem_context *ctx = ptr;
> + int n;
> +
> + for (n = 0; n < ARRAY_SIZE(ctx->engine); n++) {
I would have suggested assigining engine = ctx->engine + n, but it's unnamed :(
With commit message;
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Regards, Joonas
> + if (ctx->engine[n].state)
> + per_file_stats(0, ctx->engine[n].state, data);
> + if (ctx->engine[n].ringbuf)
> + per_file_stats(0, ctx->engine[n].ringbuf->obj, data);
> + }
> +
> + return 0;
> +}
> +
> +static void print_context_stats(struct seq_file *m,
> + struct drm_i915_private *dev_priv)
> +{
> + struct file_stats stats;
> + struct drm_file *file;
> +
> + memset(&stats, 0, sizeof(stats));
> +
> + if (dev_priv->kernel_context)
> + per_file_ctx_stats(0, dev_priv->kernel_context, &stats);
> +
> + list_for_each_entry(file, &dev_priv->dev->filelist, lhead) {
> + struct drm_i915_file_private *fpriv = file->driver_priv;
> + idr_for_each(&fpriv->context_idr, per_file_ctx_stats, &stats);
> + }
> +
> + print_file_stats(m, "[k]contexts", stats);
> +}
> +
> #define count_vmas(list, member) do { \
> list_for_each_entry(vma, list, member) { \
> size += i915_gem_obj_total_ggtt_size(vma->obj); \
> @@ -521,6 +555,7 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
>
> seq_putc(m, '\n');
> print_batch_pool_stats(m, dev_priv);
> + print_context_stats(m, dev_priv);
>
> mutex_unlock(&dev->struct_mutex);
>
--
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] 42+ messages in thread
* Re: [PATCH v3] drm/i915/debugfs: Show context objects in i915_gem_objects
2016-05-23 13:01 ` [PATCH v3] drm/i915/debugfs: Show context objects in i915_gem_objects Chris Wilson
` (2 preceding siblings ...)
2016-05-24 11:15 ` Tvrtko Ursulin
@ 2016-05-25 9:55 ` Joonas Lahtinen
2016-05-25 10:00 ` Chris Wilson
3 siblings, 1 reply; 42+ messages in thread
From: Joonas Lahtinen @ 2016-05-25 9:55 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On ma, 2016-05-23 at 14:01 +0100, Chris Wilson wrote:
> One of the uses for i915_gem_objects is pin-pointing leaks. For this, we
> can compare the number of allocated objects and who owns them, a
> discrepancy here often indicates a kernel bug. One allocator of unreported
> objects is for backing context objects, so include those in the listing.
>
> v2: Take filelist_mutex which requires a little dance with struct_mutex
> to avoid nesting filelist_mutex inside struct_mutex.
>
What's new in v3?
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 38 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 30cb26fe2fa9..1300bad67f5c 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -417,6 +417,42 @@ static void print_batch_pool_stats(struct seq_file *m,
> print_file_stats(m, "[k]batch pool", stats);
> }
>
> +static int per_file_ctx_stats(int id, void *ptr, void *data)
> +{
> + struct i915_gem_context *ctx = ptr;
> + int n;
> +
> + for (n = 0; n < ARRAY_SIZE(ctx->engine); n++) {
> + if (ctx->engine[n].state)
> + per_file_stats(0, ctx->engine[n].state, data);
> + if (ctx->engine[n].ringbuf)
> + per_file_stats(0, ctx->engine[n].ringbuf->obj, data);
> + }
> +
> + return 0;
> +}
> +
> +static void print_context_stats(struct seq_file *m,
> + struct drm_i915_private *dev_priv)
> +{
> + struct file_stats stats;
> + struct drm_file *file;
> +
> + memset(&stats, 0, sizeof(stats));
> +
> + mutex_lock(&dev_priv->dev->struct_mutex);
> + if (dev_priv->kernel_context)
> + per_file_ctx_stats(0, dev_priv->kernel_context, &stats);
> +
> + list_for_each_entry(file, &dev_priv->dev->filelist, lhead) {
> + struct drm_i915_file_private *fpriv = file->driver_priv;
> + idr_for_each(&fpriv->context_idr, per_file_ctx_stats, &stats);
> + }
> + mutex_unlock(&dev_priv->dev->struct_mutex);
> +
> + print_file_stats(m, "[k]contexts", stats);
> +}
> +
> #define count_vmas(list, member) do { \
> list_for_each_entry(vma, list, member) { \
> size += i915_gem_obj_total_ggtt_size(vma->obj); \
> @@ -521,10 +557,10 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
>
> seq_putc(m, '\n');
> print_batch_pool_stats(m, dev_priv);
> -
> mutex_unlock(&dev->struct_mutex);
>
> mutex_lock(&dev->filelist_mutex);
> + print_context_stats(m, dev_priv);
> list_for_each_entry_reverse(file, &dev->filelist, lhead) {
> struct file_stats stats;
> struct task_struct *task;
--
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] 42+ messages in thread
* Re: [PATCH v3] drm/i915/debugfs: Show context objects in i915_gem_objects
2016-05-25 9:55 ` Joonas Lahtinen
@ 2016-05-25 10:00 ` Chris Wilson
0 siblings, 0 replies; 42+ messages in thread
From: Chris Wilson @ 2016-05-25 10:00 UTC (permalink / raw)
To: Joonas Lahtinen; +Cc: intel-gfx
On Wed, May 25, 2016 at 12:55:21PM +0300, Joonas Lahtinen wrote:
> On ma, 2016-05-23 at 14:01 +0100, Chris Wilson wrote:
> > One of the uses for i915_gem_objects is pin-pointing leaks. For this, we
> > can compare the number of allocated objects and who owns them, a
> > discrepancy here often indicates a kernel bug. One allocator of unreported
> > objects is for backing context objects, so include those in the listing.
> >
> > v2: Take filelist_mutex which requires a little dance with struct_mutex
> > to avoid nesting filelist_mutex inside struct_mutex.
> >
>
> What's new in v3?
v1 was posted in two series, v3 here was to distinguish the update
within the thread.
-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] 42+ messages in thread
end of thread, other threads:[~2016-05-25 10:00 UTC | newest]
Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-23 11:34 Context churn Chris Wilson
2016-05-23 11:34 ` [PATCH v2 01/11] drm/i915: Rename struct intel_context Chris Wilson
2016-05-23 11:34 ` [PATCH v2 02/11] drm/i915: Apply lockdep annotations to i915_gem_context.c Chris Wilson
2016-05-23 11:34 ` [PATCH v2 03/11] drm/i915: Rename and inline i915_gem_context_get() Chris Wilson
2016-05-23 11:34 ` [PATCH v2 04/11] drm/i915: Rename i915_gem_context_reference/unreference() Chris Wilson
2016-05-24 11:21 ` Chris Wilson
2016-05-23 11:34 ` [PATCH v2 05/11] drm/i915: Name the inner most per-engine intel_context struct Chris Wilson
2016-05-23 11:34 ` [PATCH v2 06/11] drm/i915: Move pinning of dev_priv->kernel_context into its creator Chris Wilson
2016-05-23 12:09 ` Tvrtko Ursulin
2016-05-23 12:16 ` Chris Wilson
2016-05-23 12:23 ` Tvrtko Ursulin
2016-05-25 9:25 ` Joonas Lahtinen
2016-05-23 11:34 ` [PATCH v2 07/11] drm/i915: Show i915_gem_context owner in debugfs Chris Wilson
2016-05-23 12:18 ` Tvrtko Ursulin
2016-05-25 9:18 ` Joonas Lahtinen
2016-05-23 11:34 ` [PATCH v2 08/11] drm/i915: Put the kernel_context in drm_i915_private next to its friends Chris Wilson
2016-05-23 11:34 ` [PATCH v2 09/11] drm/i915: Merge legacy+execlists context structs Chris Wilson
2016-05-23 11:34 ` [PATCH v2 10/11] drm/i915: Rearrange i915_gem_context Chris Wilson
2016-05-25 9:15 ` Joonas Lahtinen
2016-05-23 11:34 ` [PATCH v2 11/11] drm/i915: Show context objects in debugfs/i915_gem_objects Chris Wilson
2016-05-23 12:31 ` Tvrtko Ursulin
2016-05-23 12:40 ` Chris Wilson
2016-05-23 13:01 ` [PATCH v3] drm/i915/debugfs: Show context objects in i915_gem_objects Chris Wilson
2016-05-23 13:42 ` kbuild test robot
2016-05-23 13:50 ` kbuild test robot
2016-05-24 11:15 ` Tvrtko Ursulin
2016-05-25 9:55 ` Joonas Lahtinen
2016-05-25 10:00 ` Chris Wilson
2016-05-25 9:33 ` [PATCH v2 11/11] drm/i915: Show context objects in debugfs/i915_gem_objects Joonas Lahtinen
2016-05-23 12:01 ` ✓ Ro.CI.BAT: success for series starting with [v2,01/11] drm/i915: Rename struct intel_context Patchwork
2016-05-23 14:30 ` ✗ Ro.CI.BAT: failure for series starting with [v2,01/11] drm/i915: Rename struct intel_context (rev2) Patchwork
2016-05-24 13:53 ` [CI 01/10] drm/i915: Rename struct intel_context Chris Wilson
2016-05-24 13:53 ` [CI 02/10] drm/i915: Apply lockdep annotations to i915_gem_context.c Chris Wilson
2016-05-24 13:53 ` [CI 03/10] drm/i915: Rename and inline i915_gem_context_get() Chris Wilson
2016-05-24 13:53 ` [CI 04/10] drm/i915: Name the inner most per-engine intel_context struct Chris Wilson
2016-05-24 13:53 ` [CI 05/10] drm/i915: Move pinning of dev_priv->kernel_context into its creator Chris Wilson
2016-05-24 13:53 ` [CI 06/10] drm/i915: Show i915_gem_context owner in debugfs Chris Wilson
2016-05-24 13:53 ` [CI 07/10] drm/i915: Put the kernel_context in drm_i915_private next to its friends Chris Wilson
2016-05-24 13:53 ` [CI 08/10] drm/i915: Merge legacy+execlists context structs Chris Wilson
2016-05-24 13:53 ` [CI 09/10] drm/i915: Rearrange i915_gem_context Chris Wilson
2016-05-24 13:53 ` [CI 10/10] drm/i915/debugfs: Show context objects in i915_gem_objects Chris Wilson
2016-05-24 14:19 ` ✗ Ro.CI.BAT: warning for series starting with [v2,01/11] drm/i915: Rename struct intel_context (rev10) 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.