Commit 08819549 authored by Chris Wilson's avatar Chris Wilson

drm/i915: Introduce intel_context.pin_mutex for pin management

Introduce a mutex to start locking the HW contexts independently of
struct_mutex, with a view to reducing the coarse struct_mutex. The
intel_context.pin_mutex is used to guard the transition to and from being
pinned on the gpu, and so is required before starting to build any
request. The intel_context will then remain pinned until the request
completes, but the mutex can be released immediately unpin completion of
pinning the context.

A slight variant of the above is used by per-context sseu that wants to
inspect the pinned status of the context, and requires that it remains
stable (either !pinned or pinned) across its operation. By using the
pin_mutex to serialise operations while pin_count==0, we can take that
pin_mutex for stabilise the boolean pin status.

v2: for Tvrtko!
* Improved commit message.
* Dropped _gpu suffix from gen8_modify_rpcs_gpu.
v3: Repair the locking for sseu selftests
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190308132522.21573-7-chris@chris-wilson.co.uk
parent 9dbfea98
...@@ -4667,7 +4667,7 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915) ...@@ -4667,7 +4667,7 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915)
if (!state) if (!state)
continue; continue;
GEM_BUG_ON(ce->pin_count); GEM_BUG_ON(intel_context_is_pinned(ce));
/* /*
* As we will hold a reference to the logical state, it will * As we will hold a reference to the logical state, it will
......
...@@ -810,7 +810,6 @@ static int get_sseu(struct i915_gem_context *ctx, ...@@ -810,7 +810,6 @@ static int get_sseu(struct i915_gem_context *ctx,
struct drm_i915_gem_context_param_sseu user_sseu; struct drm_i915_gem_context_param_sseu user_sseu;
struct intel_engine_cs *engine; struct intel_engine_cs *engine;
struct intel_context *ce; struct intel_context *ce;
int ret;
if (args->size == 0) if (args->size == 0)
goto out; goto out;
...@@ -830,21 +829,16 @@ static int get_sseu(struct i915_gem_context *ctx, ...@@ -830,21 +829,16 @@ static int get_sseu(struct i915_gem_context *ctx,
if (!engine) if (!engine)
return -EINVAL; return -EINVAL;
ce = intel_context_instance(ctx, engine); ce = intel_context_pin_lock(ctx, engine); /* serialises with set_sseu */
if (IS_ERR(ce)) if (IS_ERR(ce))
return PTR_ERR(ce); return PTR_ERR(ce);
/* Only use for mutex here is to serialize get_param and set_param. */
ret = mutex_lock_interruptible(&ctx->i915->drm.struct_mutex);
if (ret)
return ret;
user_sseu.slice_mask = ce->sseu.slice_mask; user_sseu.slice_mask = ce->sseu.slice_mask;
user_sseu.subslice_mask = ce->sseu.subslice_mask; user_sseu.subslice_mask = ce->sseu.subslice_mask;
user_sseu.min_eus_per_subslice = ce->sseu.min_eus_per_subslice; user_sseu.min_eus_per_subslice = ce->sseu.min_eus_per_subslice;
user_sseu.max_eus_per_subslice = ce->sseu.max_eus_per_subslice; user_sseu.max_eus_per_subslice = ce->sseu.max_eus_per_subslice;
mutex_unlock(&ctx->i915->drm.struct_mutex); intel_context_pin_unlock(ce);
if (copy_to_user(u64_to_user_ptr(args->value), &user_sseu, if (copy_to_user(u64_to_user_ptr(args->value), &user_sseu,
sizeof(user_sseu))) sizeof(user_sseu)))
...@@ -940,23 +934,28 @@ static int gen8_emit_rpcs_config(struct i915_request *rq, ...@@ -940,23 +934,28 @@ static int gen8_emit_rpcs_config(struct i915_request *rq,
} }
static int static int
gen8_modify_rpcs_gpu(struct intel_context *ce, gen8_modify_rpcs(struct intel_context *ce, struct intel_sseu sseu)
struct intel_engine_cs *engine,
struct intel_sseu sseu)
{ {
struct drm_i915_private *i915 = engine->i915; struct drm_i915_private *i915 = ce->engine->i915;
struct i915_request *rq, *prev; struct i915_request *rq, *prev;
intel_wakeref_t wakeref; intel_wakeref_t wakeref;
int ret; int ret;
GEM_BUG_ON(!ce->pin_count); lockdep_assert_held(&ce->pin_mutex);
lockdep_assert_held(&i915->drm.struct_mutex); /*
* If the context is not idle, we have to submit an ordered request to
* modify its context image via the kernel context (writing to our own
* image, or into the registers directory, does not stick). Pristine
* and idle contexts will be configured on pinning.
*/
if (!intel_context_is_pinned(ce))
return 0;
/* Submitting requests etc needs the hw awake. */ /* Submitting requests etc needs the hw awake. */
wakeref = intel_runtime_pm_get(i915); wakeref = intel_runtime_pm_get(i915);
rq = i915_request_alloc(engine, i915->kernel_context); rq = i915_request_alloc(ce->engine, i915->kernel_context);
if (IS_ERR(rq)) { if (IS_ERR(rq)) {
ret = PTR_ERR(rq); ret = PTR_ERR(rq);
goto out_put; goto out_put;
...@@ -1010,25 +1009,20 @@ __i915_gem_context_reconfigure_sseu(struct i915_gem_context *ctx, ...@@ -1010,25 +1009,20 @@ __i915_gem_context_reconfigure_sseu(struct i915_gem_context *ctx,
GEM_BUG_ON(INTEL_GEN(ctx->i915) < 8); GEM_BUG_ON(INTEL_GEN(ctx->i915) < 8);
GEM_BUG_ON(engine->id != RCS0); GEM_BUG_ON(engine->id != RCS0);
ce = intel_context_instance(ctx, engine); ce = intel_context_pin_lock(ctx, engine);
if (IS_ERR(ce)) if (IS_ERR(ce))
return PTR_ERR(ce); return PTR_ERR(ce);
/* Nothing to do if unmodified. */ /* Nothing to do if unmodified. */
if (!memcmp(&ce->sseu, &sseu, sizeof(sseu))) if (!memcmp(&ce->sseu, &sseu, sizeof(sseu)))
return 0; goto unlock;
/*
* If context is not idle we have to submit an ordered request to modify
* its context image via the kernel context. Pristine and idle contexts
* will be configured on pinning.
*/
if (ce->pin_count)
ret = gen8_modify_rpcs_gpu(ce, engine, sseu);
ret = gen8_modify_rpcs(ce, sseu);
if (!ret) if (!ret)
ce->sseu = sseu; ce->sseu = sseu;
unlock:
intel_context_pin_unlock(ce);
return ret; return ret;
} }
......
...@@ -102,7 +102,7 @@ void __intel_context_remove(struct intel_context *ce) ...@@ -102,7 +102,7 @@ void __intel_context_remove(struct intel_context *ce)
spin_unlock(&ctx->hw_contexts_lock); spin_unlock(&ctx->hw_contexts_lock);
} }
struct intel_context * static struct intel_context *
intel_context_instance(struct i915_gem_context *ctx, intel_context_instance(struct i915_gem_context *ctx,
struct intel_engine_cs *engine) struct intel_engine_cs *engine)
{ {
...@@ -126,6 +126,23 @@ intel_context_instance(struct i915_gem_context *ctx, ...@@ -126,6 +126,23 @@ intel_context_instance(struct i915_gem_context *ctx,
return pos; return pos;
} }
struct intel_context *
intel_context_pin_lock(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
__acquires(ce->pin_mutex)
{
struct intel_context *ce;
ce = intel_context_instance(ctx, engine);
if (IS_ERR(ce))
return ce;
if (mutex_lock_interruptible(&ce->pin_mutex))
return ERR_PTR(-EINTR);
return ce;
}
struct intel_context * struct intel_context *
intel_context_pin(struct i915_gem_context *ctx, intel_context_pin(struct i915_gem_context *ctx,
struct intel_engine_cs *engine) struct intel_engine_cs *engine)
...@@ -133,16 +150,20 @@ intel_context_pin(struct i915_gem_context *ctx, ...@@ -133,16 +150,20 @@ intel_context_pin(struct i915_gem_context *ctx,
struct intel_context *ce; struct intel_context *ce;
int err; int err;
lockdep_assert_held(&ctx->i915->drm.struct_mutex);
ce = intel_context_instance(ctx, engine); ce = intel_context_instance(ctx, engine);
if (IS_ERR(ce)) if (IS_ERR(ce))
return ce; return ce;
if (unlikely(!ce->pin_count++)) { if (likely(atomic_inc_not_zero(&ce->pin_count)))
return ce;
if (mutex_lock_interruptible(&ce->pin_mutex))
return ERR_PTR(-EINTR);
if (likely(!atomic_read(&ce->pin_count))) {
err = ce->ops->pin(ce); err = ce->ops->pin(ce);
if (err) if (err)
goto err_unpin; goto err;
mutex_lock(&ctx->mutex); mutex_lock(&ctx->mutex);
list_add(&ce->active_link, &ctx->active_engines); list_add(&ce->active_link, &ctx->active_engines);
...@@ -150,16 +171,35 @@ intel_context_pin(struct i915_gem_context *ctx, ...@@ -150,16 +171,35 @@ intel_context_pin(struct i915_gem_context *ctx,
i915_gem_context_get(ctx); i915_gem_context_get(ctx);
GEM_BUG_ON(ce->gem_context != ctx); GEM_BUG_ON(ce->gem_context != ctx);
smp_mb__before_atomic(); /* flush pin before it is visible */
} }
GEM_BUG_ON(!ce->pin_count); /* no overflow! */
atomic_inc(&ce->pin_count);
GEM_BUG_ON(!intel_context_is_pinned(ce)); /* no overflow! */
mutex_unlock(&ce->pin_mutex);
return ce; return ce;
err_unpin: err:
ce->pin_count = 0; mutex_unlock(&ce->pin_mutex);
return ERR_PTR(err); return ERR_PTR(err);
} }
void intel_context_unpin(struct intel_context *ce)
{
if (likely(atomic_add_unless(&ce->pin_count, -1, 1)))
return;
/* We may be called from inside intel_context_pin() to evict another */
mutex_lock_nested(&ce->pin_mutex, SINGLE_DEPTH_NESTING);
if (likely(atomic_dec_and_test(&ce->pin_count)))
ce->ops->unpin(ce);
mutex_unlock(&ce->pin_mutex);
}
static void intel_context_retire(struct i915_active_request *active, static void intel_context_retire(struct i915_active_request *active,
struct i915_request *rq) struct i915_request *rq)
{ {
...@@ -181,6 +221,8 @@ intel_context_init(struct intel_context *ce, ...@@ -181,6 +221,8 @@ intel_context_init(struct intel_context *ce,
INIT_LIST_HEAD(&ce->signal_link); INIT_LIST_HEAD(&ce->signal_link);
INIT_LIST_HEAD(&ce->signals); INIT_LIST_HEAD(&ce->signals);
mutex_init(&ce->pin_mutex);
/* Use the whole device by default */ /* Use the whole device by default */
ce->sseu = intel_device_default_sseu(ctx->i915); ce->sseu = intel_device_default_sseu(ctx->i915);
......
...@@ -7,6 +7,8 @@ ...@@ -7,6 +7,8 @@
#ifndef __INTEL_CONTEXT_H__ #ifndef __INTEL_CONTEXT_H__
#define __INTEL_CONTEXT_H__ #define __INTEL_CONTEXT_H__
#include <linux/lockdep.h>
#include "intel_context_types.h" #include "intel_context_types.h"
#include "intel_engine_types.h" #include "intel_engine_types.h"
...@@ -29,18 +31,30 @@ intel_context_lookup(struct i915_gem_context *ctx, ...@@ -29,18 +31,30 @@ intel_context_lookup(struct i915_gem_context *ctx,
struct intel_engine_cs *engine); struct intel_engine_cs *engine);
/** /**
* intel_context_instance - Lookup or allocate the HW context for (ctx, engine) * intel_context_pin_lock - Stablises the 'pinned' status of the HW context
* @ctx - the parent GEM context * @ctx - the parent GEM context
* @engine - the target HW engine * @engine - the target HW engine
* *
* Returns the existing HW context for this pair of (GEM context, engine), or * Acquire a lock on the pinned status of the HW context, such that the context
* allocates and initialises a fresh context. Once allocated, the HW context * can neither be bound to the GPU or unbound whilst the lock is held, i.e.
* remains resident until the GEM context is destroyed. * intel_context_is_pinned() remains stable.
*/ */
struct intel_context * struct intel_context *
intel_context_instance(struct i915_gem_context *ctx, intel_context_pin_lock(struct i915_gem_context *ctx,
struct intel_engine_cs *engine); struct intel_engine_cs *engine);
static inline bool
intel_context_is_pinned(struct intel_context *ce)
{
return atomic_read(&ce->pin_count);
}
static inline void intel_context_pin_unlock(struct intel_context *ce)
__releases(ce->pin_mutex)
{
mutex_unlock(&ce->pin_mutex);
}
struct intel_context * struct intel_context *
__intel_context_insert(struct i915_gem_context *ctx, __intel_context_insert(struct i915_gem_context *ctx,
struct intel_engine_cs *engine, struct intel_engine_cs *engine,
...@@ -53,18 +67,10 @@ intel_context_pin(struct i915_gem_context *ctx, struct intel_engine_cs *engine); ...@@ -53,18 +67,10 @@ intel_context_pin(struct i915_gem_context *ctx, struct intel_engine_cs *engine);
static inline void __intel_context_pin(struct intel_context *ce) static inline void __intel_context_pin(struct intel_context *ce)
{ {
GEM_BUG_ON(!ce->pin_count); GEM_BUG_ON(!intel_context_is_pinned(ce));
ce->pin_count++; atomic_inc(&ce->pin_count);
} }
static inline void intel_context_unpin(struct intel_context *ce) void intel_context_unpin(struct intel_context *ce);
{
GEM_BUG_ON(!ce->pin_count);
if (--ce->pin_count)
return;
GEM_BUG_ON(!ce->ops);
ce->ops->unpin(ce);
}
#endif /* __INTEL_CONTEXT_H__ */ #endif /* __INTEL_CONTEXT_H__ */
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#define __INTEL_CONTEXT_TYPES__ #define __INTEL_CONTEXT_TYPES__
#include <linux/list.h> #include <linux/list.h>
#include <linux/mutex.h>
#include <linux/rbtree.h> #include <linux/rbtree.h>
#include <linux/types.h> #include <linux/types.h>
...@@ -38,14 +39,19 @@ struct intel_context { ...@@ -38,14 +39,19 @@ struct intel_context {
struct i915_gem_context *gem_context; struct i915_gem_context *gem_context;
struct intel_engine_cs *engine; struct intel_engine_cs *engine;
struct intel_engine_cs *active; struct intel_engine_cs *active;
struct list_head active_link; struct list_head active_link;
struct list_head signal_link; struct list_head signal_link;
struct list_head signals; struct list_head signals;
struct i915_vma *state; struct i915_vma *state;
struct intel_ring *ring; struct intel_ring *ring;
u32 *lrc_reg_state; u32 *lrc_reg_state;
u64 lrc_desc; u64 lrc_desc;
int pin_count;
atomic_t pin_count;
struct mutex pin_mutex; /* guards pinning and associated on-gpuing */
/** /**
* active_tracker: Active tracker for the external rq activity * active_tracker: Active tracker for the external rq activity
......
...@@ -1244,7 +1244,7 @@ static void __execlists_context_fini(struct intel_context *ce) ...@@ -1244,7 +1244,7 @@ static void __execlists_context_fini(struct intel_context *ce)
static void execlists_context_destroy(struct intel_context *ce) static void execlists_context_destroy(struct intel_context *ce)
{ {
GEM_BUG_ON(ce->pin_count); GEM_BUG_ON(intel_context_is_pinned(ce));
if (ce->state) if (ce->state)
__execlists_context_fini(ce); __execlists_context_fini(ce);
...@@ -1481,7 +1481,7 @@ static int execlists_request_alloc(struct i915_request *request) ...@@ -1481,7 +1481,7 @@ static int execlists_request_alloc(struct i915_request *request)
{ {
int ret; int ret;
GEM_BUG_ON(!request->hw_context->pin_count); GEM_BUG_ON(!intel_context_is_pinned(request->hw_context));
/* /*
* Flush enough space to reduce the likelihood of waiting after * Flush enough space to reduce the likelihood of waiting after
......
...@@ -1357,7 +1357,7 @@ static void __ring_context_fini(struct intel_context *ce) ...@@ -1357,7 +1357,7 @@ static void __ring_context_fini(struct intel_context *ce)
static void ring_context_destroy(struct intel_context *ce) static void ring_context_destroy(struct intel_context *ce)
{ {
GEM_BUG_ON(ce->pin_count); GEM_BUG_ON(intel_context_is_pinned(ce));
if (ce->state) if (ce->state)
__ring_context_fini(ce); __ring_context_fini(ce);
...@@ -1918,7 +1918,7 @@ static int ring_request_alloc(struct i915_request *request) ...@@ -1918,7 +1918,7 @@ static int ring_request_alloc(struct i915_request *request)
{ {
int ret; int ret;
GEM_BUG_ON(!request->hw_context->pin_count); GEM_BUG_ON(!intel_context_is_pinned(request->hw_context));
GEM_BUG_ON(request->timeline->has_initial_breadcrumb); GEM_BUG_ON(request->timeline->has_initial_breadcrumb);
/* /*
......
...@@ -131,7 +131,7 @@ static void mock_context_unpin(struct intel_context *ce) ...@@ -131,7 +131,7 @@ static void mock_context_unpin(struct intel_context *ce)
static void mock_context_destroy(struct intel_context *ce) static void mock_context_destroy(struct intel_context *ce)
{ {
GEM_BUG_ON(ce->pin_count); GEM_BUG_ON(intel_context_is_pinned(ce));
if (ce->ring) if (ce->ring)
mock_ring_free(ce->ring); mock_ring_free(ce->ring);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment