Commit 58ea7d62 authored by John Harrison's avatar John Harrison

drm/i915/guc: Split guc_lrc_desc_pin apart

The LRC descriptor pool is going away. Further, the function that was
populating it was also doing a bunch of logic about the context
registration sequence. So, split that code apart into separate state
setup and try to register functions. Note that some of those 'try to
register' code paths actually undo the state setup and leave it to be
redone again later (with potentially different values). This is
inefficient. The next patch will correct this.

Also, move a comment about ignoring return values to the place where
the return values are actually ignored.

v2: Move some more splitting from a later patch (and do it correctly).
Signed-off-by: default avatarJohn Harrison <John.C.Harrison@Intel.com>
Reviewed-by: default avatarDaniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220302003357.4188363-5-John.C.Harrison@Intel.com
parent d1249022
...@@ -631,7 +631,7 @@ int intel_guc_wait_for_idle(struct intel_guc *guc, long timeout) ...@@ -631,7 +631,7 @@ int intel_guc_wait_for_idle(struct intel_guc *guc, long timeout)
true, timeout); true, timeout);
} }
static int guc_lrc_desc_pin(struct intel_context *ce, bool loop); static int try_context_registration(struct intel_context *ce, bool loop);
static int __guc_add_request(struct intel_guc *guc, struct i915_request *rq) static int __guc_add_request(struct intel_guc *guc, struct i915_request *rq)
{ {
...@@ -929,7 +929,7 @@ static int guc_dequeue_one_context(struct intel_guc *guc) ...@@ -929,7 +929,7 @@ static int guc_dequeue_one_context(struct intel_guc *guc)
if (unlikely(!ctx_id_mapped(guc, ce->guc_id.id) && if (unlikely(!ctx_id_mapped(guc, ce->guc_id.id) &&
!intel_context_is_banned(ce))) { !intel_context_is_banned(ce))) {
ret = guc_lrc_desc_pin(ce, false); ret = try_context_registration(ce, false);
if (unlikely(ret == -EPIPE)) { if (unlikely(ret == -EPIPE)) {
goto deadlk; goto deadlk;
} else if (ret == -EBUSY) { } else if (ret == -EBUSY) {
...@@ -2224,20 +2224,15 @@ static void guc_context_policy_init(struct intel_engine_cs *engine, ...@@ -2224,20 +2224,15 @@ static void guc_context_policy_init(struct intel_engine_cs *engine,
desc->preemption_timeout = engine->props.preempt_timeout_ms * 1000; desc->preemption_timeout = engine->props.preempt_timeout_ms * 1000;
} }
static int guc_lrc_desc_pin(struct intel_context *ce, bool loop) static void prepare_context_registration_info(struct intel_context *ce)
{ {
struct intel_engine_cs *engine = ce->engine; struct intel_engine_cs *engine = ce->engine;
struct intel_runtime_pm *runtime_pm = engine->uncore->rpm;
struct intel_guc *guc = &engine->gt->uc.guc; struct intel_guc *guc = &engine->gt->uc.guc;
u32 desc_idx = ce->guc_id.id; u32 desc_idx = ce->guc_id.id;
struct guc_lrc_desc *desc; struct guc_lrc_desc *desc;
bool context_registered;
intel_wakeref_t wakeref;
struct intel_context *child; struct intel_context *child;
int ret = 0;
GEM_BUG_ON(!engine->mask); GEM_BUG_ON(!engine->mask);
GEM_BUG_ON(!sched_state_is_init(ce));
/* /*
* Ensure LRC + CT vmas are is same region as write barrier is done * Ensure LRC + CT vmas are is same region as write barrier is done
...@@ -2246,11 +2241,6 @@ static int guc_lrc_desc_pin(struct intel_context *ce, bool loop) ...@@ -2246,11 +2241,6 @@ static int guc_lrc_desc_pin(struct intel_context *ce, bool loop)
GEM_BUG_ON(i915_gem_object_is_lmem(guc->ct.vma->obj) != GEM_BUG_ON(i915_gem_object_is_lmem(guc->ct.vma->obj) !=
i915_gem_object_is_lmem(ce->ring->vma->obj)); i915_gem_object_is_lmem(ce->ring->vma->obj));
context_registered = ctx_id_mapped(guc, desc_idx);
clr_ctx_id_mapping(guc, desc_idx);
set_ctx_id_mapping(guc, desc_idx, ce);
desc = __get_lrc_desc(guc, desc_idx); desc = __get_lrc_desc(guc, desc_idx);
desc->engine_class = engine_class_to_guc_class(engine->class); desc->engine_class = engine_class_to_guc_class(engine->class);
desc->engine_submit_mask = engine->logical_mask; desc->engine_submit_mask = engine->logical_mask;
...@@ -2295,6 +2285,26 @@ static int guc_lrc_desc_pin(struct intel_context *ce, bool loop) ...@@ -2295,6 +2285,26 @@ static int guc_lrc_desc_pin(struct intel_context *ce, bool loop)
clear_children_join_go_memory(ce); clear_children_join_go_memory(ce);
} }
}
static int try_context_registration(struct intel_context *ce, bool loop)
{
struct intel_engine_cs *engine = ce->engine;
struct intel_runtime_pm *runtime_pm = engine->uncore->rpm;
struct intel_guc *guc = &engine->gt->uc.guc;
intel_wakeref_t wakeref;
u32 desc_idx = ce->guc_id.id;
bool context_registered;
int ret = 0;
GEM_BUG_ON(!sched_state_is_init(ce));
context_registered = ctx_id_mapped(guc, desc_idx);
clr_ctx_id_mapping(guc, desc_idx);
set_ctx_id_mapping(guc, desc_idx, ce);
prepare_context_registration_info(ce);
/* /*
* The context_lookup xarray is used to determine if the hardware * The context_lookup xarray is used to determine if the hardware
...@@ -3132,7 +3142,7 @@ static int guc_request_alloc(struct i915_request *rq) ...@@ -3132,7 +3142,7 @@ static int guc_request_alloc(struct i915_request *rq)
if (unlikely(ret < 0)) if (unlikely(ret < 0))
return ret; return ret;
if (context_needs_register(ce, !!ret)) { if (context_needs_register(ce, !!ret)) {
ret = guc_lrc_desc_pin(ce, true); ret = try_context_registration(ce, true);
if (unlikely(ret)) { /* unwind */ if (unlikely(ret)) { /* unwind */
if (ret == -EPIPE) { if (ret == -EPIPE) {
disable_submission(guc); disable_submission(guc);
...@@ -3623,9 +3633,17 @@ static void guc_set_default_submission(struct intel_engine_cs *engine) ...@@ -3623,9 +3633,17 @@ static void guc_set_default_submission(struct intel_engine_cs *engine)
static inline void guc_kernel_context_pin(struct intel_guc *guc, static inline void guc_kernel_context_pin(struct intel_guc *guc,
struct intel_context *ce) struct intel_context *ce)
{ {
/*
* Note: we purposefully do not check the returns below because
* the registration can only fail if a reset is just starting.
* This is called at the end of reset so presumably another reset
* isn't happening and even it did this code would be run again.
*/
if (context_guc_id_invalid(ce)) if (context_guc_id_invalid(ce))
pin_guc_id(guc, ce); pin_guc_id(guc, ce);
guc_lrc_desc_pin(ce, true);
try_context_registration(ce, true);
} }
static inline void guc_init_lrc_mapping(struct intel_guc *guc) static inline void guc_init_lrc_mapping(struct intel_guc *guc)
...@@ -3643,13 +3661,7 @@ static inline void guc_init_lrc_mapping(struct intel_guc *guc) ...@@ -3643,13 +3661,7 @@ static inline void guc_init_lrc_mapping(struct intel_guc *guc)
* Also, after a reset the of the GuC we want to make sure that the * Also, after a reset the of the GuC we want to make sure that the
* information shared with GuC is properly reset. The kernel LRCs are * information shared with GuC is properly reset. The kernel LRCs are
* not attached to the gem_context, so they need to be added separately. * not attached to the gem_context, so they need to be added separately.
*
* Note: we purposefully do not check the return of guc_lrc_desc_pin,
* because that function can only fail if a reset is just starting. This
* is at the end of reset so presumably another reset isn't happening
* and even it did this code would be run again.
*/ */
for_each_engine(engine, gt, id) { for_each_engine(engine, gt, id) {
struct intel_context *ce; struct intel_context *ce;
......
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