* [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization
@ 2016-05-22 13:02 Chris Wilson
2016-05-22 13:02 ` [PATCH 02/12] drm/i915: Rename struct intel_context Chris Wilson
` (11 more replies)
0 siblings, 12 replies; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 UTC (permalink / raw)
To: intel-gfx
During cleanup we have to synchronise with the async task we are using
to initialise and register our fbdev. Currently, we are using a full
synchronisation on the global domain, but we can restrict this to just
synchronising up to our task if we remember our cookie.
v2: async_synchronize_cookie() takes an exclusive upper bound, to
synchronize with our task we have to pass in the next cookie.
v3: Drop premature disregarding of the active cookie (we need to wait
until the task is complete before continuing in the teardown).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Lukas Wunner <lukas@wunner.de>
---
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_fbdev.c | 29 ++++++++++++++++-------------
2 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0741b2d3aa65..68751aee52cc 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -159,6 +159,7 @@ struct intel_framebuffer {
struct intel_fbdev {
struct drm_fb_helper helper;
struct intel_framebuffer *fb;
+ async_cookie_t cookie;
int preferred_bpp;
};
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 99e27530e264..bfb2fd291e4e 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -538,8 +538,7 @@ static const struct drm_fb_helper_funcs intel_fb_helper_funcs = {
.fb_probe = intelfb_create,
};
-static void intel_fbdev_destroy(struct drm_device *dev,
- struct intel_fbdev *ifbdev)
+static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)
{
/* We rely on the object-free to release the VMA pinning for
* the info->screen_base mmaping. Leaking the VMA is simpler than
@@ -554,12 +553,14 @@ static void intel_fbdev_destroy(struct drm_device *dev,
if (ifbdev->fb) {
drm_framebuffer_unregister_private(&ifbdev->fb->base);
- mutex_lock(&dev->struct_mutex);
+ mutex_lock(&ifbdev->helper.dev->struct_mutex);
intel_unpin_fb_obj(&ifbdev->fb->base, BIT(DRM_ROTATE_0));
- mutex_unlock(&dev->struct_mutex);
+ mutex_unlock(&ifbdev->helper.dev->struct_mutex);
drm_framebuffer_remove(&ifbdev->fb->base);
}
+
+ kfree(ifbdev);
}
/*
@@ -736,32 +737,34 @@ int intel_fbdev_init(struct drm_device *dev)
static void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
{
- struct drm_i915_private *dev_priv = data;
- struct intel_fbdev *ifbdev = dev_priv->fbdev;
+ struct intel_fbdev *ifbdev = data;
/* Due to peculiar init order wrt to hpd handling this is separate. */
if (drm_fb_helper_initial_config(&ifbdev->helper,
ifbdev->preferred_bpp))
- intel_fbdev_fini(dev_priv->dev);
+ intel_fbdev_fini(ifbdev->helper.dev);
}
void intel_fbdev_initial_config_async(struct drm_device *dev)
{
- async_schedule(intel_fbdev_initial_config, to_i915(dev));
+ struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
+
+ ifbdev->cookie = async_schedule(intel_fbdev_initial_config, ifbdev);
}
void intel_fbdev_fini(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- if (!dev_priv->fbdev)
+ struct intel_fbdev *ifbdev = dev_priv->fbdev;
+
+ if (!ifbdev)
return;
flush_work(&dev_priv->fbdev_suspend_work);
+ if (ifbdev->cookie && !current_is_async())
+ async_synchronize_cookie(ifbdev->cookie + 1);
- if (!current_is_async())
- async_synchronize_full();
- intel_fbdev_destroy(dev, dev_priv->fbdev);
- kfree(dev_priv->fbdev);
+ intel_fbdev_destroy(ifbdev);
dev_priv->fbdev = 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] 33+ messages in thread
* [PATCH 02/12] drm/i915: Rename struct intel_context
2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
@ 2016-05-22 13:02 ` Chris Wilson
2016-05-23 9:09 ` Tvrtko Ursulin
2016-05-22 13:02 ` [PATCH 03/12] drm/i915: Apply lockdep annotations to i915_gem_context.c Chris Wilson
` (10 subsequent siblings)
11 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 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>
---
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 e2abd9eb352b..8380102bbdd8 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] 33+ messages in thread
* [PATCH 03/12] drm/i915: Apply lockdep annotations to i915_gem_context.c
2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
2016-05-22 13:02 ` [PATCH 02/12] drm/i915: Rename struct intel_context Chris Wilson
@ 2016-05-22 13:02 ` Chris Wilson
2016-05-23 9:11 ` Tvrtko Ursulin
2016-05-22 13:02 ` [PATCH 04/12] drm/i915: Rename and inline i915_gem_context_get() Chris Wilson
` (9 subsequent siblings)
11 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 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>
---
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 8380102bbdd8..48a222ddd8b3 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] 33+ messages in thread
* [PATCH 04/12] drm/i915: Rename and inline i915_gem_context_get()
2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
2016-05-22 13:02 ` [PATCH 02/12] drm/i915: Rename struct intel_context Chris Wilson
2016-05-22 13:02 ` [PATCH 03/12] drm/i915: Apply lockdep annotations to i915_gem_context.c Chris Wilson
@ 2016-05-22 13:02 ` Chris Wilson
2016-05-23 9:15 ` Tvrtko Ursulin
2016-05-22 13:02 ` [PATCH 05/12] drm/i915: Rename i915_gem_context_reference/unreference() Chris Wilson
` (8 subsequent siblings)
11 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 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>
---
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 48a222ddd8b3..e4e3614335ec 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] 33+ messages in thread
* [PATCH 05/12] drm/i915: Rename i915_gem_context_reference/unreference()
2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
` (2 preceding siblings ...)
2016-05-22 13:02 ` [PATCH 04/12] drm/i915: Rename and inline i915_gem_context_get() Chris Wilson
@ 2016-05-22 13:02 ` Chris Wilson
2016-05-23 9:17 ` Tvrtko Ursulin
2016-05-22 13:02 ` [PATCH 06/12] drm/i915: Name the inner most per-engine intel_context struct Chris Wilson
` (7 subsequent siblings)
11 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 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 e4e3614335ec..0a3442fcd93e 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] 33+ messages in thread
* [PATCH 06/12] drm/i915: Name the inner most per-engine intel_context struct
2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
` (3 preceding siblings ...)
2016-05-22 13:02 ` [PATCH 05/12] drm/i915: Rename i915_gem_context_reference/unreference() Chris Wilson
@ 2016-05-22 13:02 ` Chris Wilson
2016-05-23 9:26 ` Tvrtko Ursulin
2016-05-22 13:02 ` [PATCH 07/12] drm/i915: Move pinning of dev_priv->kernel_context into its creator Chris Wilson
` (6 subsequent siblings)
11 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 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>
---
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 0a3442fcd93e..e6735dc9eeb2 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] 33+ messages in thread
* [PATCH 07/12] drm/i915: Move pinning of dev_priv->kernel_context into its creator
2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
` (4 preceding siblings ...)
2016-05-22 13:02 ` [PATCH 06/12] drm/i915: Name the inner most per-engine intel_context struct Chris Wilson
@ 2016-05-22 13:02 ` Chris Wilson
2016-05-23 9:33 ` Tvrtko Ursulin
2016-05-22 13:02 ` [PATCH 08/12] drm/i915: Show i915_gem_context owner in debugfs Chris Wilson
` (5 subsequent siblings)
11 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 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.
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 | 52 +++++++++++++++------------------
1 file changed, 23 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..51e83a79ab36 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,14 @@ 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;
+ i915_gem_context_put(ctx);
+ return ERR_CAST(ppgtt);
}
ctx->ppgtt = ppgtt;
@@ -345,14 +327,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 +400,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] 33+ messages in thread
* [PATCH 08/12] drm/i915: Show i915_gem_context owner in debugfs
2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
` (5 preceding siblings ...)
2016-05-22 13:02 ` [PATCH 07/12] drm/i915: Move pinning of dev_priv->kernel_context into its creator Chris Wilson
@ 2016-05-22 13:02 ` Chris Wilson
2016-05-23 9:42 ` Tvrtko Ursulin
2016-05-22 13:02 ` [PATCH 09/12] drm/i915: Put the kernel_context in drm_i915_private next to its friends Chris Wilson
` (4 subsequent siblings)
11 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 UTC (permalink / raw)
To: intel-gfx
Print the context's owner (via the pid under file_priv) under debugfs.
Note that since this was originally introducing
dev_priv->kernel_context, there are a couple of leftover minor chunks.
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 +++++++++++++++--
1 file changed, 15 insertions(+), 2 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');
--
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] 33+ messages in thread
* [PATCH 09/12] drm/i915: Put the kernel_context in drm_i915_private next to its friends
2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
` (6 preceding siblings ...)
2016-05-22 13:02 ` [PATCH 08/12] drm/i915: Show i915_gem_context owner in debugfs Chris Wilson
@ 2016-05-22 13:02 ` Chris Wilson
2016-05-23 9:45 ` Tvrtko Ursulin
2016-05-22 13:02 ` [PATCH 10/12] drm/i915: Merge legacy+execlists context structs Chris Wilson
` (3 subsequent siblings)
11 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 UTC (permalink / raw)
To: intel-gfx
Just move the kernel_context memboer 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>
---
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 e6735dc9eeb2..d3d2538d17e8 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] 33+ messages in thread
* [PATCH 10/12] drm/i915: Merge legacy+execlists context structs
2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
` (7 preceding siblings ...)
2016-05-22 13:02 ` [PATCH 09/12] drm/i915: Put the kernel_context in drm_i915_private next to its friends Chris Wilson
@ 2016-05-22 13:02 ` Chris Wilson
2016-05-23 10:26 ` Tvrtko Ursulin
2016-05-22 13:02 ` [PATCH 11/12] drm/i915: Rearrange i915_gem_context Chris Wilson
` (2 subsequent siblings)
11 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 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.
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 | 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 d3d2538d17e8..259a0ee7a601 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 51e83a79ab36..9847235997b9 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 */
@@ -335,8 +344,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);
}
}
@@ -400,7 +412,7 @@ int i915_gem_context_init(struct drm_device *dev)
return PTR_ERR(ctx);
}
- if (ctx->legacy_hw_ctx.rcs_state) {
+ if (ctx->engine[RCS].state) {
int ret;
/* We may need to do things with the shrinker which
@@ -410,7 +422,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",
@@ -435,15 +447,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);
}
@@ -454,8 +468,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;
@@ -562,7 +576,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
@@ -639,7 +653,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))
@@ -704,7 +718,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)
@@ -727,7 +741,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;
@@ -742,7 +756,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
@@ -766,8 +780,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
@@ -775,10 +789,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);
@@ -812,19 +826,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;
}
@@ -848,8 +862,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] 33+ messages in thread
* [PATCH 11/12] drm/i915: Rearrange i915_gem_context
2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
` (8 preceding siblings ...)
2016-05-22 13:02 ` [PATCH 10/12] drm/i915: Merge legacy+execlists context structs Chris Wilson
@ 2016-05-22 13:02 ` Chris Wilson
2016-05-23 10:27 ` Tvrtko Ursulin
2016-05-22 13:02 ` [PATCH 12/12] drm/i915: Show context objects in debugfs/i915_gem_objects Chris Wilson
2016-05-22 13:33 ` ✗ Ro.CI.BAT: failure for series starting with [01/12] drm/i915/fbdev: Limit the global async-domain synchronization Patchwork
11 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 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 259a0ee7a601..3b5cf15d85ab 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] 33+ messages in thread
* [PATCH 12/12] drm/i915: Show context objects in debugfs/i915_gem_objects
2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
` (9 preceding siblings ...)
2016-05-22 13:02 ` [PATCH 11/12] drm/i915: Rearrange i915_gem_context Chris Wilson
@ 2016-05-22 13:02 ` Chris Wilson
2016-05-24 8:13 ` Daniel Vetter
2016-05-22 13:33 ` ✗ Ro.CI.BAT: failure for series starting with [01/12] drm/i915/fbdev: Limit the global async-domain synchronization Patchwork
11 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-22 13:02 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] 33+ messages in thread
* ✗ Ro.CI.BAT: failure for series starting with [01/12] drm/i915/fbdev: Limit the global async-domain synchronization
2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
` (10 preceding siblings ...)
2016-05-22 13:02 ` [PATCH 12/12] drm/i915: Show context objects in debugfs/i915_gem_objects Chris Wilson
@ 2016-05-22 13:33 ` Patchwork
11 siblings, 0 replies; 33+ messages in thread
From: Patchwork @ 2016-05-22 13:33 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [01/12] drm/i915/fbdev: Limit the global async-domain synchronization
URL : https://patchwork.freedesktop.org/series/7525/
State : failure
== Summary ==
Series 7525v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/7525/revisions/1/mbox
Test gem_exec_gttfill:
Subgroup basic:
skip -> FAIL (ro-bdw-i7-5557U)
skip -> PASS (fi-bdw-i7-5557u)
skip -> FAIL (ro-bdw-i7-5600u)
skip -> PASS (fi-hsw-i7-4770r)
skip -> FAIL (ro-bdw-i5-5250u)
skip -> PASS (fi-skl-i7-6700k)
skip -> PASS (ro-hsw-i7-4770r)
skip -> PASS (fi-hsw-i7-4770k)
skip -> FAIL (ro-skl-i7-6700hq)
skip -> PASS (ro-ivb2-i7-3770)
skip -> PASS (ro-byt-n2820)
fi-bdw-i7-5557u total:209 pass:197 dwarn:0 dfail:0 fail:0 skip:12
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-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:171 dwarn:0 dfail:0 fail:1 skip:37
ro-bdw-i7-5557U total:209 pass:196 dwarn:0 dfail:0 fail:1 skip:12
ro-bdw-i7-5600u total:209 pass:179 dwarn:0 dfail:0 fail:2 skip:28
ro-bsw-n3050 total:209 pass:167 dwarn:0 dfail:0 fail:3 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:182 dwarn:0 dfail:0 fail:1 skip:21
ro-snb-i7-2620M total:209 pass:170 dwarn:0 dfail:0 fail:1 skip:38
fi-bsw-n3050 failed to connect after reboot
Results at /archive/results/CI_IGT_test/RO_Patchwork_960/
f1eaed1 drm-intel-nightly: 2016y-05m-20d-14h-35m-29s UTC integration manifest
83aedbd drm/i915: Show context objects in debugfs/i915_gem_objects
40af450 drm/i915: Rearrange i915_gem_context
3cb2b9f drm/i915: Merge legacy+execlists context structs
efc48e4 drm/i915: Put the kernel_context in drm_i915_private next to its friends
21b66dc drm/i915: Show i915_gem_context owner in debugfs
74e3400 drm/i915: Move pinning of dev_priv->kernel_context into its creator
3a68060 drm/i915: Name the inner most per-engine intel_context struct
59da8ff drm/i915: Rename i915_gem_context_reference/unreference()
185cd9c drm/i915: Rename and inline i915_gem_context_get()
1ce9e14 drm/i915: Apply lockdep annotations to i915_gem_context.c
35e3a4b drm/i915: Rename struct intel_context
2522d87 drm/i915/fbdev: Limit the global async-domain synchronization
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 02/12] drm/i915: Rename struct intel_context
2016-05-22 13:02 ` [PATCH 02/12] drm/i915: Rename struct intel_context Chris Wilson
@ 2016-05-23 9:09 ` Tvrtko Ursulin
0 siblings, 0 replies; 33+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23 9:09 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 22/05/16 14:02, Chris Wilson wrote:
> 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!
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
> 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>
> ---
> 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 e2abd9eb352b..8380102bbdd8 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;
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 03/12] drm/i915: Apply lockdep annotations to i915_gem_context.c
2016-05-22 13:02 ` [PATCH 03/12] drm/i915: Apply lockdep annotations to i915_gem_context.c Chris Wilson
@ 2016-05-23 9:11 ` Tvrtko Ursulin
0 siblings, 0 replies; 33+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23 9:11 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 22/05/16 14:02, Chris Wilson wrote:
> 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>
> ---
> 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 8380102bbdd8..48a222ddd8b3 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) {
>
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] 33+ messages in thread
* Re: [PATCH 04/12] drm/i915: Rename and inline i915_gem_context_get()
2016-05-22 13:02 ` [PATCH 04/12] drm/i915: Rename and inline i915_gem_context_get() Chris Wilson
@ 2016-05-23 9:15 ` Tvrtko Ursulin
0 siblings, 0 replies; 33+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23 9:15 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 22/05/16 14:02, Chris Wilson wrote:
> 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>
> ---
> 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 48a222ddd8b3..e4e3614335ec 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;
>
>
Makes sense.
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] 33+ messages in thread
* Re: [PATCH 05/12] drm/i915: Rename i915_gem_context_reference/unreference()
2016-05-22 13:02 ` [PATCH 05/12] drm/i915: Rename i915_gem_context_reference/unreference() Chris Wilson
@ 2016-05-23 9:17 ` Tvrtko Ursulin
2016-05-23 9:25 ` Chris Wilson
2016-05-24 8:10 ` Daniel Vetter
0 siblings, 2 replies; 33+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23 9:17 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 22/05/16 14:02, 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.
Not so sure about this one. We got objects, framebuffers and requests
using (un)reference naming so don't see that making contexts different
is a good idea.
Regards,
Tvrtko
> 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 e4e3614335ec..0a3442fcd93e 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)
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 05/12] drm/i915: Rename i915_gem_context_reference/unreference()
2016-05-23 9:17 ` Tvrtko Ursulin
@ 2016-05-23 9:25 ` Chris Wilson
2016-05-24 8:10 ` Daniel Vetter
1 sibling, 0 replies; 33+ messages in thread
From: Chris Wilson @ 2016-05-23 9:25 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
On Mon, May 23, 2016 at 10:17:01AM +0100, Tvrtko Ursulin wrote:
>
> On 22/05/16 14:02, 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.
>
> Not so sure about this one. We got objects, framebuffers and
> requests using (un)reference naming so don't see that making
> contexts different is a good idea.
I've converted objects and requests in the lockless patches. So contexts
were a bit of an eyesore in amongst the *_get()/*_put().
-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] 33+ messages in thread
* Re: [PATCH 06/12] drm/i915: Name the inner most per-engine intel_context struct
2016-05-22 13:02 ` [PATCH 06/12] drm/i915: Name the inner most per-engine intel_context struct Chris Wilson
@ 2016-05-23 9:26 ` Tvrtko Ursulin
2016-05-23 10:17 ` Chris Wilson
0 siblings, 1 reply; 33+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23 9:26 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 22/05/16 14:02, Chris Wilson wrote:
> 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>
> ---
> 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 0a3442fcd93e..e6735dc9eeb2 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);
Could have kept use of intel_lr_context_descriptor for better separation.
>
> /* 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;
> }
> }
>
Looks OK anyway.
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] 33+ messages in thread
* Re: [PATCH 07/12] drm/i915: Move pinning of dev_priv->kernel_context into its creator
2016-05-22 13:02 ` [PATCH 07/12] drm/i915: Move pinning of dev_priv->kernel_context into its creator Chris Wilson
@ 2016-05-23 9:33 ` Tvrtko Ursulin
2016-05-23 9:45 ` Chris Wilson
0 siblings, 1 reply; 33+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23 9:33 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 22/05/16 14:02, 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.
Makes sense.
> 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 | 52 +++++++++++++++------------------
> 1 file changed, 23 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..51e83a79ab36 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,14 @@ 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;
> + i915_gem_context_put(ctx);
> + return ERR_CAST(ppgtt);
> }
>
> ctx->ppgtt = ppgtt;
> @@ -345,14 +327,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);
Isn't idr_remove still required in the error path above?
> - i915_gem_context_put(ctx);
> - return ERR_PTR(ret);
> }
>
> static void i915_gem_context_unpin(struct i915_gem_context *ctx,
> @@ -426,6 +400,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",
>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 08/12] drm/i915: Show i915_gem_context owner in debugfs
2016-05-22 13:02 ` [PATCH 08/12] drm/i915: Show i915_gem_context owner in debugfs Chris Wilson
@ 2016-05-23 9:42 ` Tvrtko Ursulin
2016-05-23 9:52 ` Chris Wilson
0 siblings, 1 reply; 33+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23 9:42 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 22/05/16 14:02, Chris Wilson wrote:
> Print the context's owner (via the pid under file_priv) under debugfs.
>
> Note that since this was originally introducing
> dev_priv->kernel_context, there are a couple of leftover minor chunks.
>
> 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 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 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;
Hm, can you deref file_priv after the file has been closed / client
exited? We could still have the context on the list after that point..
> + 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');
>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 09/12] drm/i915: Put the kernel_context in drm_i915_private next to its friends
2016-05-22 13:02 ` [PATCH 09/12] drm/i915: Put the kernel_context in drm_i915_private next to its friends Chris Wilson
@ 2016-05-23 9:45 ` Tvrtko Ursulin
0 siblings, 0 replies; 33+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23 9:45 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 22/05/16 14:02, Chris Wilson wrote:
> Just move the kernel_context memboer 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>
> ---
> 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 e6735dc9eeb2..d3d2538d17e8 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];
OK bit.
>
> 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;
>
Churn bit. :)
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] 33+ messages in thread
* Re: [PATCH 07/12] drm/i915: Move pinning of dev_priv->kernel_context into its creator
2016-05-23 9:33 ` Tvrtko Ursulin
@ 2016-05-23 9:45 ` Chris Wilson
0 siblings, 0 replies; 33+ messages in thread
From: Chris Wilson @ 2016-05-23 9:45 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
On Mon, May 23, 2016 at 10:33:24AM +0100, Tvrtko Ursulin wrote:
> >@@ -313,30 +311,14 @@ 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;
> >+ i915_gem_context_put(ctx);
> >+ return ERR_CAST(ppgtt);
> > }
> >
> > ctx->ppgtt = ppgtt;
> >@@ -345,14 +327,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);
>
> Isn't idr_remove still required in the error path above?
Yes. I can blame a rebase error here since in the kernel it was
extracted upon we call context_close() here instead which does the idr
removal as well.
-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] 33+ messages in thread
* Re: [PATCH 08/12] drm/i915: Show i915_gem_context owner in debugfs
2016-05-23 9:42 ` Tvrtko Ursulin
@ 2016-05-23 9:52 ` Chris Wilson
0 siblings, 0 replies; 33+ messages in thread
From: Chris Wilson @ 2016-05-23 9:52 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
On Mon, May 23, 2016 at 10:42:42AM +0100, Tvrtko Ursulin wrote:
>
>
> On 22/05/16 14:02, Chris Wilson wrote:
> >Print the context's owner (via the pid under file_priv) under debugfs.
> >
> >Note that since this was originally introducing
> >dev_priv->kernel_context, there are a couple of leftover minor chunks.
> >
> >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 +++++++++++++++--
> > 1 file changed, 15 insertions(+), 2 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;
>
> Hm, can you deref file_priv after the file has been closed / client
> exited? We could still have the context on the list after that
> point..
Hmm, this was extracted from after the context_close patch because I
didn't think we had that bug... We do.
That's easy enough to mark as closed when the file is, which is what we
need later anyway.
-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] 33+ messages in thread
* Re: [PATCH 06/12] drm/i915: Name the inner most per-engine intel_context struct
2016-05-23 9:26 ` Tvrtko Ursulin
@ 2016-05-23 10:17 ` Chris Wilson
2016-05-23 10:55 ` Tvrtko Ursulin
0 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2016-05-23 10:17 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
On Mon, May 23, 2016 at 10:26:39AM +0100, Tvrtko Ursulin wrote:
> >@@ -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);
>
> Could have kept use of intel_lr_context_descriptor for better separation.
I was leaning the other way, since the code doesn't want
intel_lr_context_descriptor() just happens to want to reuse some of the
bits e.g. engine->ctx_desc_template | lrca
i.e. lrc->context_desc != ce->lrc_desc and I would prefer it to be
clarified as to exactly what the GuC expects.
-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] 33+ messages in thread
* Re: [PATCH 10/12] drm/i915: Merge legacy+execlists context structs
2016-05-22 13:02 ` [PATCH 10/12] drm/i915: Merge legacy+execlists context structs Chris Wilson
@ 2016-05-23 10:26 ` Tvrtko Ursulin
2016-05-23 10:40 ` Chris Wilson
0 siblings, 1 reply; 33+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23 10:26 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 22/05/16 14:02, Chris Wilson wrote:
> 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.
>
> 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 | 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 d3d2538d17e8..259a0ee7a601 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 51e83a79ab36..9847235997b9 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 */
> @@ -335,8 +344,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);
> }
> }
> @@ -400,7 +412,7 @@ int i915_gem_context_init(struct drm_device *dev)
> return PTR_ERR(ctx);
> }
>
> - if (ctx->legacy_hw_ctx.rcs_state) {
> + if (ctx->engine[RCS].state) {
Maybe now put a comment saying this is for legacy now, to make the
asymmetry in ctx pinning paths between the two stick out more.
> int ret;
>
> /* We may need to do things with the shrinker which
> @@ -410,7 +422,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",
> @@ -435,15 +447,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);
> }
>
> @@ -454,8 +468,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;
> @@ -562,7 +576,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
> @@ -639,7 +653,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))
> @@ -704,7 +718,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)
> @@ -727,7 +741,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;
>
> @@ -742,7 +756,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
> @@ -766,8 +780,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
> @@ -775,10 +789,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);
> @@ -812,19 +826,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;
> }
>
> @@ -848,8 +862,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);
>
Looks good to me.
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] 33+ messages in thread
* Re: [PATCH 11/12] drm/i915: Rearrange i915_gem_context
2016-05-22 13:02 ` [PATCH 11/12] drm/i915: Rearrange i915_gem_context Chris Wilson
@ 2016-05-23 10:27 ` Tvrtko Ursulin
0 siblings, 0 replies; 33+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23 10:27 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 22/05/16 14:02, 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 259a0ee7a601..3b5cf15d85ab 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 {
>
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] 33+ messages in thread
* Re: [PATCH 10/12] drm/i915: Merge legacy+execlists context structs
2016-05-23 10:26 ` Tvrtko Ursulin
@ 2016-05-23 10:40 ` Chris Wilson
0 siblings, 0 replies; 33+ messages in thread
From: Chris Wilson @ 2016-05-23 10:40 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
On Mon, May 23, 2016 at 11:26:13AM +0100, Tvrtko Ursulin wrote:
> >@@ -400,7 +412,7 @@ int i915_gem_context_init(struct drm_device *dev)
> > return PTR_ERR(ctx);
> > }
> >
> >- if (ctx->legacy_hw_ctx.rcs_state) {
> >+ if (ctx->engine[RCS].state) {
>
> Maybe now put a comment saying this is for legacy now, to make the
> asymmetry in ctx pinning paths between the two stick out more.
Good point. This needs the i915.enable_execlists markup. Ultimately, I
think we should move the pin/unpin into the legacy engine setup for
symmetry with execlists (and killing off one more pair of
i915.enable_execlists).
-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] 33+ messages in thread
* Re: [PATCH 06/12] drm/i915: Name the inner most per-engine intel_context struct
2016-05-23 10:17 ` Chris Wilson
@ 2016-05-23 10:55 ` Tvrtko Ursulin
2016-05-23 11:08 ` Chris Wilson
0 siblings, 1 reply; 33+ messages in thread
From: Tvrtko Ursulin @ 2016-05-23 10:55 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 23/05/16 11:17, Chris Wilson wrote:
> On Mon, May 23, 2016 at 10:26:39AM +0100, Tvrtko Ursulin wrote:
>>> @@ -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);
>>
>> Could have kept use of intel_lr_context_descriptor for better separation.
>
> I was leaning the other way, since the code doesn't want
> intel_lr_context_descriptor() just happens to want to reuse some of the
> bits e.g. engine->ctx_desc_template | lrca
Thats true, but it was at least using an exported function with
documented content, rather than directly fishing out stuff from
essentially private data elsewhere.
> i.e. lrc->context_desc != ce->lrc_desc and I would prefer it to be
> clarified as to exactly what the GuC expects.
Yes that would be best. Cc-ed Dave for when he is back to comment what
does GuC really wants in there.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 06/12] drm/i915: Name the inner most per-engine intel_context struct
2016-05-23 10:55 ` Tvrtko Ursulin
@ 2016-05-23 11:08 ` Chris Wilson
0 siblings, 0 replies; 33+ messages in thread
From: Chris Wilson @ 2016-05-23 11:08 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
On Mon, May 23, 2016 at 11:55:16AM +0100, Tvrtko Ursulin wrote:
>
> On 23/05/16 11:17, Chris Wilson wrote:
> >On Mon, May 23, 2016 at 10:26:39AM +0100, Tvrtko Ursulin wrote:
> >>>@@ -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);
> >>
> >>Could have kept use of intel_lr_context_descriptor for better separation.
> >
> >I was leaning the other way, since the code doesn't want
> >intel_lr_context_descriptor() just happens to want to reuse some of the
> >bits e.g. engine->ctx_desc_template | lrca
>
> Thats true, but it was at least using an exported function with
> documented content, rather than directly fishing out stuff from
> essentially private data elsewhere.
It's still fishing out essentially private data though :)
If engine->ctx_desc_template considers GuC for its set of flags, it is
purely by happenstance.
-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] 33+ messages in thread
* Re: [PATCH 05/12] drm/i915: Rename i915_gem_context_reference/unreference()
2016-05-23 9:17 ` Tvrtko Ursulin
2016-05-23 9:25 ` Chris Wilson
@ 2016-05-24 8:10 ` Daniel Vetter
1 sibling, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2016-05-24 8:10 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
On Mon, May 23, 2016 at 10:17:01AM +0100, Tvrtko Ursulin wrote:
>
> On 22/05/16 14:02, 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.
>
> Not so sure about this one. We got objects, framebuffers and requests using
> (un)reference naming so don't see that making contexts different is a good
> idea.
And drm_blob too. For core modeset stuff I think converting to _get/put
might be worth it, too, but needs 3-stage approach: Add new _get/put
funcs. 2) cocci-generated patches for each driver/file to convert over. 3)
drop _reference/unreference variants once no longer used.
-Daniel
>
> Regards,
>
> Tvrtko
>
>
> >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 e4e3614335ec..0a3442fcd93e 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)
> >
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 12/12] drm/i915: Show context objects in debugfs/i915_gem_objects
2016-05-22 13:02 ` [PATCH 12/12] drm/i915: Show context objects in debugfs/i915_gem_objects Chris Wilson
@ 2016-05-24 8:13 ` Daniel Vetter
2016-05-24 8:21 ` Chris Wilson
0 siblings, 1 reply; 33+ messages in thread
From: Daniel Vetter @ 2016-05-24 8:13 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Sun, May 22, 2016 at 02:02:32PM +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>
> ---
> 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) {
dev->filelist has grown it's own lock now, dev->filelist_mutex. Should we
have a drm_for_each_file macro which includes a helpful
lockdep_assert_held?
-Daniel
> + 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
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 12/12] drm/i915: Show context objects in debugfs/i915_gem_objects
2016-05-24 8:13 ` Daniel Vetter
@ 2016-05-24 8:21 ` Chris Wilson
0 siblings, 0 replies; 33+ messages in thread
From: Chris Wilson @ 2016-05-24 8:21 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On Tue, May 24, 2016 at 10:13:24AM +0200, Daniel Vetter wrote:
> On Sun, May 22, 2016 at 02:02:32PM +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>
> > ---
> > 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) {
>
> dev->filelist has grown it's own lock now, dev->filelist_mutex. Should we
> have a drm_for_each_file macro which includes a helpful
> lockdep_assert_held?
I have a list_for_each_entry_check() in my tree, which allows you to
annotate which lock you are expected to be holding. (Because I wanted to
uplift the drm utilities.)
But iterating over filelist is not something I'd like to encourage.
drm_debug_for_each_file()
-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] 33+ messages in thread
end of thread, other threads:[~2016-05-24 8:21 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-22 13:02 [PATCH 01/12] drm/i915/fbdev: Limit the global async-domain synchronization Chris Wilson
2016-05-22 13:02 ` [PATCH 02/12] drm/i915: Rename struct intel_context Chris Wilson
2016-05-23 9:09 ` Tvrtko Ursulin
2016-05-22 13:02 ` [PATCH 03/12] drm/i915: Apply lockdep annotations to i915_gem_context.c Chris Wilson
2016-05-23 9:11 ` Tvrtko Ursulin
2016-05-22 13:02 ` [PATCH 04/12] drm/i915: Rename and inline i915_gem_context_get() Chris Wilson
2016-05-23 9:15 ` Tvrtko Ursulin
2016-05-22 13:02 ` [PATCH 05/12] drm/i915: Rename i915_gem_context_reference/unreference() Chris Wilson
2016-05-23 9:17 ` Tvrtko Ursulin
2016-05-23 9:25 ` Chris Wilson
2016-05-24 8:10 ` Daniel Vetter
2016-05-22 13:02 ` [PATCH 06/12] drm/i915: Name the inner most per-engine intel_context struct Chris Wilson
2016-05-23 9:26 ` Tvrtko Ursulin
2016-05-23 10:17 ` Chris Wilson
2016-05-23 10:55 ` Tvrtko Ursulin
2016-05-23 11:08 ` Chris Wilson
2016-05-22 13:02 ` [PATCH 07/12] drm/i915: Move pinning of dev_priv->kernel_context into its creator Chris Wilson
2016-05-23 9:33 ` Tvrtko Ursulin
2016-05-23 9:45 ` Chris Wilson
2016-05-22 13:02 ` [PATCH 08/12] drm/i915: Show i915_gem_context owner in debugfs Chris Wilson
2016-05-23 9:42 ` Tvrtko Ursulin
2016-05-23 9:52 ` Chris Wilson
2016-05-22 13:02 ` [PATCH 09/12] drm/i915: Put the kernel_context in drm_i915_private next to its friends Chris Wilson
2016-05-23 9:45 ` Tvrtko Ursulin
2016-05-22 13:02 ` [PATCH 10/12] drm/i915: Merge legacy+execlists context structs Chris Wilson
2016-05-23 10:26 ` Tvrtko Ursulin
2016-05-23 10:40 ` Chris Wilson
2016-05-22 13:02 ` [PATCH 11/12] drm/i915: Rearrange i915_gem_context Chris Wilson
2016-05-23 10:27 ` Tvrtko Ursulin
2016-05-22 13:02 ` [PATCH 12/12] drm/i915: Show context objects in debugfs/i915_gem_objects Chris Wilson
2016-05-24 8:13 ` Daniel Vetter
2016-05-24 8:21 ` Chris Wilson
2016-05-22 13:33 ` ✗ Ro.CI.BAT: failure for series starting with [01/12] drm/i915/fbdev: Limit the global async-domain synchronization 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.