Skip to content

Commit

Permalink
drm/i915/gt: Keep a no-frills swappable copy of the default context s…
Browse files Browse the repository at this point in the history
…tate

We need to keep the default context state around to instantiate new
contexts (aka golden rendercontext), and we also keep it pinned while
the engine is active so that we can quickly reset a hanging context.
However, the default contexts are large enough to merit keeping in
swappable memory as opposed to kernel memory, so we store them inside
shmemfs. Currently, we use the normal GEM objects to create the default
context image, but we can throw away all but the shmemfs file.

This greatly simplifies the tricky power management code which wants to
run underneath the normal GT locking, and we definitely do not want to
use any high level objects that may appear to recurse back into the GT.
Though perhaps the primary advantage of the complex GEM object is that
we aggressively cache the mapping, but here we are recreating the
vm_area everytime time we unpark. At the worst, we add a lightweight
cache, but first find a microbenchmark that is impacted.

Having started to create some utility functions to make working with
shmemfs objects easier, we can start putting them to wider use, where
GEM objects are overkill, such as storing persistent error state.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Ramalingam C <ramalingam.c@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200429172429.6054-1-chris@chris-wilson.co.uk
  • Loading branch information
Chris Wilson committed Apr 29, 2020
1 parent 8c35a19 commit be1cb55
Show file tree
Hide file tree
Showing 14 changed files with 291 additions and 123 deletions.
1 change: 1 addition & 0 deletions drivers/gpu/drm/i915/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ gt-y += \
gt/intel_sseu.o \
gt/intel_timeline.o \
gt/intel_workarounds.o \
gt/shmem_utils.o \
gt/sysfs_engines.o
# autogenerated null render state
gt-y += \
Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/i915/gt/intel_engine_cs.c
Original file line number Diff line number Diff line change
Expand Up @@ -834,7 +834,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
intel_engine_cleanup_cmd_parser(engine);

if (engine->default_state)
i915_gem_object_put(engine->default_state);
fput(engine->default_state);

if (engine->kernel_context) {
intel_context_unpin(engine->kernel_context);
Expand Down
10 changes: 5 additions & 5 deletions drivers/gpu/drm/i915/gt/intel_engine_pm.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "intel_gt_pm.h"
#include "intel_rc6.h"
#include "intel_ring.h"
#include "shmem_utils.h"

static int __engine_unpark(struct intel_wakeref *wf)
{
Expand All @@ -30,10 +31,8 @@ static int __engine_unpark(struct intel_wakeref *wf)
/* Pin the default state for fast resets from atomic context. */
map = NULL;
if (engine->default_state)
map = i915_gem_object_pin_map(engine->default_state,
I915_MAP_WB);
if (!IS_ERR_OR_NULL(map))
engine->pinned_default_state = map;
map = shmem_pin_map(engine->default_state);
engine->pinned_default_state = map;

/* Discard stale context state from across idling */
ce = engine->kernel_context;
Expand Down Expand Up @@ -264,7 +263,8 @@ static int __engine_park(struct intel_wakeref *wf)
engine->park(engine);

if (engine->pinned_default_state) {
i915_gem_object_unpin_map(engine->default_state);
shmem_unpin_map(engine->default_state,
engine->pinned_default_state);
engine->pinned_default_state = NULL;
}

Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/i915/gt/intel_engine_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ struct intel_engine_cs {

unsigned long wakeref_serial;
struct intel_wakeref wakeref;
struct drm_i915_gem_object *default_state;
struct file *default_state;
void *pinned_default_state;

struct {
Expand Down
60 changes: 8 additions & 52 deletions drivers/gpu/drm/i915/gt/intel_gt.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "intel_rps.h"
#include "intel_uncore.h"
#include "intel_pm.h"
#include "shmem_utils.h"

void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915)
{
Expand Down Expand Up @@ -371,18 +372,6 @@ static struct i915_address_space *kernel_vm(struct intel_gt *gt)
return i915_vm_get(&gt->ggtt->vm);
}

static int __intel_context_flush_retire(struct intel_context *ce)
{
struct intel_timeline *tl;

tl = intel_context_timeline_lock(ce);
if (IS_ERR(tl))
return PTR_ERR(tl);

intel_context_timeline_unlock(tl);
return 0;
}

static int __engines_record_defaults(struct intel_gt *gt)
{
struct i915_request *requests[I915_NUM_ENGINES] = {};
Expand Down Expand Up @@ -448,8 +437,7 @@ static int __engines_record_defaults(struct intel_gt *gt)

for (id = 0; id < ARRAY_SIZE(requests); id++) {
struct i915_request *rq;
struct i915_vma *state;
void *vaddr;
struct file *state;

rq = requests[id];
if (!rq)
Expand All @@ -461,48 +449,16 @@ static int __engines_record_defaults(struct intel_gt *gt)
}

GEM_BUG_ON(!test_bit(CONTEXT_ALLOC_BIT, &rq->context->flags));
state = rq->context->state;
if (!state)
if (!rq->context->state)
continue;

/* Serialise with retirement on another CPU */
GEM_BUG_ON(!i915_request_completed(rq));
err = __intel_context_flush_retire(rq->context);
if (err)
goto out;

/* We want to be able to unbind the state from the GGTT */
GEM_BUG_ON(intel_context_is_pinned(rq->context));

/*
* As we will hold a reference to the logical state, it will
* not be torn down with the context, and importantly the
* object will hold onto its vma (making it possible for a
* stray GTT write to corrupt our defaults). Unmap the vma
* from the GTT to prevent such accidents and reclaim the
* space.
*/
err = i915_vma_unbind(state);
if (err)
goto out;

i915_gem_object_lock(state->obj);
err = i915_gem_object_set_to_cpu_domain(state->obj, false);
i915_gem_object_unlock(state->obj);
if (err)
goto out;

i915_gem_object_set_cache_coherency(state->obj, I915_CACHE_LLC);

/* Check we can acquire the image of the context state */
vaddr = i915_gem_object_pin_map(state->obj, I915_MAP_FORCE_WB);
if (IS_ERR(vaddr)) {
err = PTR_ERR(vaddr);
/* Keep a copy of the state's backing pages; free the obj */
state = shmem_create_from_object(rq->context->state->obj);
if (IS_ERR(state)) {
err = PTR_ERR(state);
goto out;
}

rq->engine->default_state = i915_gem_object_get(state->obj);
i915_gem_object_unpin_map(state->obj);
rq->engine->default_state = state;
}

out:
Expand Down
25 changes: 6 additions & 19 deletions drivers/gpu/drm/i915/gt/intel_lrc.c
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@
#include "intel_reset.h"
#include "intel_ring.h"
#include "intel_workarounds.h"
#include "shmem_utils.h"

#define RING_EXECLIST_QFULL (1 << 0x2)
#define RING_EXECLIST1_VALID (1 << 0x3)
Expand Down Expand Up @@ -5083,30 +5084,18 @@ populate_lr_context(struct intel_context *ce,
{
bool inhibit = true;
void *vaddr;
int ret;

vaddr = i915_gem_object_pin_map(ctx_obj, I915_MAP_WB);
if (IS_ERR(vaddr)) {
ret = PTR_ERR(vaddr);
drm_dbg(&engine->i915->drm,
"Could not map object pages! (%d)\n", ret);
return ret;
drm_dbg(&engine->i915->drm, "Could not map object pages!\n");
return PTR_ERR(vaddr);
}

set_redzone(vaddr, engine);

if (engine->default_state) {
void *defaults;

defaults = i915_gem_object_pin_map(engine->default_state,
I915_MAP_WB);
if (IS_ERR(defaults)) {
ret = PTR_ERR(defaults);
goto err_unpin_ctx;
}

memcpy(vaddr, defaults, engine->context_size);
i915_gem_object_unpin_map(engine->default_state);
shmem_read(engine->default_state, 0,
vaddr, engine->context_size);
__set_bit(CONTEXT_VALID_BIT, &ce->flags);
inhibit = false;
}
Expand All @@ -5121,11 +5110,9 @@ populate_lr_context(struct intel_context *ce,
execlists_init_reg_state(vaddr + LRC_STATE_OFFSET,
ce, engine, ring, inhibit);

ret = 0;
err_unpin_ctx:
__i915_gem_object_flush_map(ctx_obj, 0, engine->context_size);
i915_gem_object_unpin_map(ctx_obj);
return ret;
return 0;
}

static int __execlists_context_alloc(struct intel_context *ce,
Expand Down
16 changes: 4 additions & 12 deletions drivers/gpu/drm/i915/gt/intel_ring_submission.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
#include "intel_reset.h"
#include "intel_ring.h"
#include "intel_workarounds.h"
#include "shmem_utils.h"

/* Rough estimate of the typical request size, performing a flush,
* set-context and then emitting the batch.
Expand Down Expand Up @@ -1241,23 +1242,16 @@ alloc_context_vma(struct intel_engine_cs *engine)
i915_gem_object_set_cache_coherency(obj, I915_CACHE_L3_LLC);

if (engine->default_state) {
void *defaults, *vaddr;
void *vaddr;

vaddr = i915_gem_object_pin_map(obj, I915_MAP_WB);
if (IS_ERR(vaddr)) {
err = PTR_ERR(vaddr);
goto err_obj;
}

defaults = i915_gem_object_pin_map(engine->default_state,
I915_MAP_WB);
if (IS_ERR(defaults)) {
err = PTR_ERR(defaults);
goto err_map;
}

memcpy(vaddr, defaults, engine->context_size);
i915_gem_object_unpin_map(engine->default_state);
shmem_read(engine->default_state, 0,
vaddr, engine->context_size);

i915_gem_object_flush_map(obj);
i915_gem_object_unpin_map(obj);
Expand All @@ -1271,8 +1265,6 @@ alloc_context_vma(struct intel_engine_cs *engine)

return vma;

err_map:
i915_gem_object_unpin_map(obj);
err_obj:
i915_gem_object_put(obj);
return ERR_PTR(err);
Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/i915/gt/selftest_context.c
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ static int live_context_size(void *arg)

for_each_engine(engine, gt, id) {
struct {
struct drm_i915_gem_object *state;
struct file *state;
void *pinned;
} saved;

Expand Down
10 changes: 4 additions & 6 deletions drivers/gpu/drm/i915/gt/selftest_lrc.c
Original file line number Diff line number Diff line change
Expand Up @@ -4452,8 +4452,7 @@ static int live_lrc_layout(void *arg)
if (!engine->default_state)
continue;

hw = i915_gem_object_pin_map(engine->default_state,
I915_MAP_WB);
hw = shmem_pin_map(engine->default_state);
if (IS_ERR(hw)) {
err = PTR_ERR(hw);
break;
Expand Down Expand Up @@ -4525,7 +4524,7 @@ static int live_lrc_layout(void *arg)
hexdump(lrc, PAGE_SIZE);
}

i915_gem_object_unpin_map(engine->default_state);
shmem_unpin_map(engine->default_state, hw);
if (err)
break;
}
Expand Down Expand Up @@ -4630,8 +4629,7 @@ static int live_lrc_fixed(void *arg)
if (!engine->default_state)
continue;

hw = i915_gem_object_pin_map(engine->default_state,
I915_MAP_WB);
hw = shmem_pin_map(engine->default_state);
if (IS_ERR(hw)) {
err = PTR_ERR(hw);
break;
Expand All @@ -4652,7 +4650,7 @@ static int live_lrc_fixed(void *arg)
}
}

i915_gem_object_unpin_map(engine->default_state);
shmem_unpin_map(engine->default_state, hw);
}

return err;
Expand Down
Loading

0 comments on commit be1cb55

Please sign in to comment.