Skip to content

Commit

Permalink
drm/i915: Invert the GEM wakeref hierarchy
Browse files Browse the repository at this point in the history 
In the current scheme, on submitting a request we take a single global
GEM wakeref, which trickles down to wake up all GT power domains. This
is undesirable as we would like to be able to localise our power
management to the available power domains and to remove the global GEM
operations from the heart of the driver. (The intent there is to push
global GEM decisions to the boundary as used by the GEM user interface.)

Now during request construction, each request is responsible via its
logical context to acquire a wakeref on each power domain it intends to
utilize. Currently, each request takes a wakeref on the engine(s) and
the engines themselves take a chipset wakeref. This gives us a
transition on each engine which we can extend if we want to insert more
powermangement control (such as soft rc6). The global GEM operations
that currently require a struct_mutex are reduced to listening to pm
events from the chipset GT wakeref. As we reduce the struct_mutex
requirement, these listeners should evaporate.

Perhaps the biggest immediate change is that this removes the
struct_mutex requirement around GT power management, allowing us greater
flexibility in request construction. Another important knock-on effect,
is that by tracking engine usage, we can insert a switch back to the
kernel context on that engine immediately, avoiding any extra delay or
inserting global synchronisation barriers. This makes tracking when an
engine and its associated contexts are idle much easier -- important for
when we forgo our assumed execution ordering and need idle barriers to
unpin used contexts. In the process, it means we remove a large chunk of
code whose only purpose was to switch back to the kernel context.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190424200717.1686-5-chris@chris-wilson.co.uk
  • Loading branch information
Chris Wilson committed Apr 24, 2019
1 parent 2ccdf6a commit 79ffac8
Show file tree
Hide file tree
Showing 37 changed files with 592 additions and 830 deletions.
2 changes: 2 additions & 0 deletions drivers/gpu/drm/i915/Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ gt-y += \
gt/intel_breadcrumbs.o \
gt/intel_context.o \
gt/intel_engine_cs.o \
gt/intel_engine_pm.o \
gt/intel_gt_pm.o \
gt/intel_hangcheck.o \
gt/intel_lrc.o \
gt/intel_reset.o \
Expand Down
18 changes: 8 additions & 10 deletions drivers/gpu/drm/i915/gt/intel_context.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "intel_context.h"
#include "intel_engine.h"
#include "intel_engine_pm.h"

static struct i915_global_context {
struct i915_global base;
Expand Down Expand Up @@ -162,7 +163,11 @@ intel_context_pin(struct i915_gem_context *ctx,
return ERR_PTR(-EINTR);

if (likely(!atomic_read(&ce->pin_count))) {
err = ce->ops->pin(ce);
intel_wakeref_t wakeref;

err = 0;
with_intel_runtime_pm(ce->engine->i915, wakeref)
err = ce->ops->pin(ce);
if (err)
goto err;

Expand Down Expand Up @@ -269,17 +274,10 @@ int __init i915_global_context_init(void)

void intel_context_enter_engine(struct intel_context *ce)
{
struct drm_i915_private *i915 = ce->gem_context->i915;

if (!i915->gt.active_requests++)
i915_gem_unpark(i915);
intel_engine_pm_get(ce->engine);
}

void intel_context_exit_engine(struct intel_context *ce)
{
struct drm_i915_private *i915 = ce->gem_context->i915;

GEM_BUG_ON(!i915->gt.active_requests);
if (!--i915->gt.active_requests)
i915_gem_park(i915);
intel_engine_pm_put(ce->engine);
}
9 changes: 3 additions & 6 deletions drivers/gpu/drm/i915/gt/intel_engine.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,8 @@ u64 intel_engine_get_last_batch_head(const struct intel_engine_cs *engine);
void intel_engine_get_instdone(struct intel_engine_cs *engine,
struct intel_instdone *instdone);

void intel_engine_init_execlists(struct intel_engine_cs *engine);

void intel_engine_init_breadcrumbs(struct intel_engine_cs *engine);
void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine);

Expand Down Expand Up @@ -458,19 +460,14 @@ static inline void intel_engine_reset(struct intel_engine_cs *engine,
{
if (engine->reset.reset)
engine->reset.reset(engine, stalled);
engine->serial++; /* contexts lost */
}

void intel_engines_sanitize(struct drm_i915_private *i915, bool force);
void intel_gt_resume(struct drm_i915_private *i915);

bool intel_engine_is_idle(struct intel_engine_cs *engine);
bool intel_engines_are_idle(struct drm_i915_private *dev_priv);

void intel_engine_lost_context(struct intel_engine_cs *engine);

void intel_engines_park(struct drm_i915_private *i915);
void intel_engines_unpark(struct drm_i915_private *i915);

void intel_engines_reset_default_submission(struct drm_i915_private *i915);
unsigned int intel_engines_has_context_isolation(struct drm_i915_private *i915);

Expand Down
142 changes: 5 additions & 137 deletions drivers/gpu/drm/i915/gt/intel_engine_cs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "i915_drv.h"

#include "intel_engine.h"
#include "intel_engine_pm.h"
#include "intel_lrc.h"
#include "intel_reset.h"

Expand Down Expand Up @@ -451,7 +452,7 @@ static void intel_engine_init_batch_pool(struct intel_engine_cs *engine)
i915_gem_batch_pool_init(&engine->batch_pool, engine);
}

static void intel_engine_init_execlist(struct intel_engine_cs *engine)
void intel_engine_init_execlists(struct intel_engine_cs *engine)
{
struct intel_engine_execlists * const execlists = &engine->execlists;

Expand Down Expand Up @@ -584,10 +585,11 @@ int intel_engine_setup_common(struct intel_engine_cs *engine)
i915_timeline_set_subclass(&engine->timeline, TIMELINE_ENGINE);

intel_engine_init_breadcrumbs(engine);
intel_engine_init_execlist(engine);
intel_engine_init_execlists(engine);
intel_engine_init_hangcheck(engine);
intel_engine_init_batch_pool(engine);
intel_engine_init_cmd_parser(engine);
intel_engine_init__pm(engine);

/* Use the whole device by default */
engine->sseu =
Expand Down Expand Up @@ -758,30 +760,6 @@ int intel_engine_init_common(struct intel_engine_cs *engine)
return ret;
}

void intel_gt_resume(struct drm_i915_private *i915)
{
struct intel_engine_cs *engine;
enum intel_engine_id id;

/*
* After resume, we may need to poke into the pinned kernel
* contexts to paper over any damage caused by the sudden suspend.
* Only the kernel contexts should remain pinned over suspend,
* allowing us to fixup the user contexts on their first pin.
*/
for_each_engine(engine, i915, id) {
struct intel_context *ce;

ce = engine->kernel_context;
if (ce)
ce->ops->reset(ce);

ce = engine->preempt_context;
if (ce)
ce->ops->reset(ce);
}
}

/**
* intel_engines_cleanup_common - cleans up the engine state created by
* the common initiailizers.
Expand Down Expand Up @@ -1128,117 +1106,6 @@ void intel_engines_reset_default_submission(struct drm_i915_private *i915)
engine->set_default_submission(engine);
}

static bool reset_engines(struct drm_i915_private *i915)
{
if (INTEL_INFO(i915)->gpu_reset_clobbers_display)
return false;

return intel_gpu_reset(i915, ALL_ENGINES) == 0;
}

/**
* intel_engines_sanitize: called after the GPU has lost power
* @i915: the i915 device
* @force: ignore a failed reset and sanitize engine state anyway
*
* Anytime we reset the GPU, either with an explicit GPU reset or through a
* PCI power cycle, the GPU loses state and we must reset our state tracking
* to match. Note that calling intel_engines_sanitize() if the GPU has not
* been reset results in much confusion!
*/
void intel_engines_sanitize(struct drm_i915_private *i915, bool force)
{
struct intel_engine_cs *engine;
enum intel_engine_id id;

GEM_TRACE("\n");

if (!reset_engines(i915) && !force)
return;

for_each_engine(engine, i915, id)
intel_engine_reset(engine, false);
}

/**
* intel_engines_park: called when the GT is transitioning from busy->idle
* @i915: the i915 device
*
* The GT is now idle and about to go to sleep (maybe never to wake again?).
* Time for us to tidy and put away our toys (release resources back to the
* system).
*/
void intel_engines_park(struct drm_i915_private *i915)
{
struct intel_engine_cs *engine;
enum intel_engine_id id;

for_each_engine(engine, i915, id) {
/* Flush the residual irq tasklets first. */
intel_engine_disarm_breadcrumbs(engine);
tasklet_kill(&engine->execlists.tasklet);

/*
* We are committed now to parking the engines, make sure there
* will be no more interrupts arriving later and the engines
* are truly idle.
*/
if (wait_for(intel_engine_is_idle(engine), 10)) {
struct drm_printer p = drm_debug_printer(__func__);

dev_err(i915->drm.dev,
"%s is not idle before parking\n",
engine->name);
intel_engine_dump(engine, &p, NULL);
}

/* Must be reset upon idling, or we may miss the busy wakeup. */
GEM_BUG_ON(engine->execlists.queue_priority_hint != INT_MIN);

if (engine->park)
engine->park(engine);

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

i915_gem_batch_pool_fini(&engine->batch_pool);
engine->execlists.no_priolist = false;
}

i915->gt.active_engines = 0;
}

/**
* intel_engines_unpark: called when the GT is transitioning from idle->busy
* @i915: the i915 device
*
* The GT was idle and now about to fire up with some new user requests.
*/
void intel_engines_unpark(struct drm_i915_private *i915)
{
struct intel_engine_cs *engine;
enum intel_engine_id id;

for_each_engine(engine, i915, id) {
void *map;

/* 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;

if (engine->unpark)
engine->unpark(engine);

intel_engine_init_hangcheck(engine);
}
}

/**
* intel_engine_lost_context: called when the GPU is reset into unknown state
* @engine: the engine
Expand Down Expand Up @@ -1523,6 +1390,7 @@ void intel_engine_dump(struct intel_engine_cs *engine,
if (i915_reset_failed(engine->i915))
drm_printf(m, "*** WEDGED ***\n");

drm_printf(m, "\tAwake? %d\n", atomic_read(&engine->wakeref.count));
drm_printf(m, "\tHangcheck %x:%x [%d ms]\n",
engine->hangcheck.last_seqno,
engine->hangcheck.next_seqno,
Expand Down
Loading

0 comments on commit 79ffac8

Please sign in to comment.