Commit a89d1f92 authored by Chris Wilson's avatar Chris Wilson
Browse files

drm/i915: Split i915_gem_timeline into individual timelines



We need to move to a more flexible timeline that doesn't assume one
fence context per engine, and so allow for a single timeline to be used
across a combination of engines. This means that preallocating a fence
context per engine is now a hindrance, and so we want to introduce the
singular timeline. From the code perspective, this has the notable
advantage of clearing up a lot of mirky semantics and some clumsy
pointer chasing.

By splitting the timeline up into a single entity rather than an array
of per-engine timelines, we can realise the goal of the previous patch
of tracking the timeline alongside the ring.

v2: Tweak wait_for_idle to stop the compiling thinking that ret may be
uninitialised.

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/20180502163839.3248-2-chris@chris-wilson.co.uk
parent 65fcb806
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
@@ -71,11 +71,11 @@ i915-y += i915_cmd_parser.o \
	  i915_gem_shrinker.o \
	  i915_gem_stolen.o \
	  i915_gem_tiling.o \
	  i915_gem_timeline.o \
	  i915_gem_userptr.o \
	  i915_gemfs.o \
	  i915_query.o \
	  i915_request.o \
	  i915_timeline.o \
	  i915_trace_points.o \
	  i915_vma.o \
	  intel_breadcrumbs.o \
+1 −3
Original line number Diff line number Diff line
@@ -72,10 +72,10 @@
#include "i915_gem_fence_reg.h"
#include "i915_gem_object.h"
#include "i915_gem_gtt.h"
#include "i915_gem_timeline.h"
#include "i915_gpu_error.h"
#include "i915_request.h"
#include "i915_scheduler.h"
#include "i915_timeline.h"
#include "i915_vma.h"

#include "intel_gvt.h"
@@ -2059,8 +2059,6 @@ struct drm_i915_private {
		void (*resume)(struct drm_i915_private *);
		void (*cleanup_engine)(struct intel_engine_cs *engine);

		struct i915_gem_timeline execution_timeline;
		struct i915_gem_timeline legacy_timeline;
		struct list_head timelines;

		struct list_head active_rings;
+56 −73
Original line number Diff line number Diff line
@@ -162,7 +162,7 @@ static u32 __i915_gem_park(struct drm_i915_private *i915)
	synchronize_irq(i915->drm.irq);

	intel_engines_park(i915);
	i915_gem_timelines_park(i915);
	i915_timelines_park(i915);

	i915_pmu_gt_parked(i915);

@@ -2977,8 +2977,8 @@ i915_gem_find_active_request(struct intel_engine_cs *engine)
	 * extra delay for a recent interrupt is pointless. Hence, we do
	 * not need an engine->irq_seqno_barrier() before the seqno reads.
	 */
	spin_lock_irqsave(&engine->timeline->lock, flags);
	list_for_each_entry(request, &engine->timeline->requests, link) {
	spin_lock_irqsave(&engine->timeline.lock, flags);
	list_for_each_entry(request, &engine->timeline.requests, link) {
		if (__i915_request_completed(request, request->global_seqno))
			continue;

@@ -2989,7 +2989,7 @@ i915_gem_find_active_request(struct intel_engine_cs *engine)
		active = request;
		break;
	}
	spin_unlock_irqrestore(&engine->timeline->lock, flags);
	spin_unlock_irqrestore(&engine->timeline.lock, flags);

	return active;
}
@@ -3110,15 +3110,15 @@ static void engine_skip_context(struct i915_request *request)
{
	struct intel_engine_cs *engine = request->engine;
	struct i915_gem_context *hung_ctx = request->ctx;
	struct intel_timeline *timeline = request->timeline;
	struct i915_timeline *timeline = request->timeline;
	unsigned long flags;

	GEM_BUG_ON(timeline == engine->timeline);
	GEM_BUG_ON(timeline == &engine->timeline);

	spin_lock_irqsave(&engine->timeline->lock, flags);
	spin_lock_irqsave(&engine->timeline.lock, flags);
	spin_lock(&timeline->lock);

	list_for_each_entry_continue(request, &engine->timeline->requests, link)
	list_for_each_entry_continue(request, &engine->timeline.requests, link)
		if (request->ctx == hung_ctx)
			skip_request(request);

@@ -3126,7 +3126,7 @@ static void engine_skip_context(struct i915_request *request)
		skip_request(request);

	spin_unlock(&timeline->lock);
	spin_unlock_irqrestore(&engine->timeline->lock, flags);
	spin_unlock_irqrestore(&engine->timeline.lock, flags);
}

/* Returns the request if it was guilty of the hang */
@@ -3183,11 +3183,11 @@ i915_gem_reset_request(struct intel_engine_cs *engine,
			dma_fence_set_error(&request->fence, -EAGAIN);

			/* Rewind the engine to replay the incomplete rq */
			spin_lock_irq(&engine->timeline->lock);
			spin_lock_irq(&engine->timeline.lock);
			request = list_prev_entry(request, link);
			if (&request->link == &engine->timeline->requests)
			if (&request->link == &engine->timeline.requests)
				request = NULL;
			spin_unlock_irq(&engine->timeline->lock);
			spin_unlock_irq(&engine->timeline.lock);
		}
	}

@@ -3300,10 +3300,10 @@ static void nop_complete_submit_request(struct i915_request *request)
		  request->fence.context, request->fence.seqno);
	dma_fence_set_error(&request->fence, -EIO);

	spin_lock_irqsave(&request->engine->timeline->lock, flags);
	spin_lock_irqsave(&request->engine->timeline.lock, flags);
	__i915_request_submit(request);
	intel_engine_init_global_seqno(request->engine, request->global_seqno);
	spin_unlock_irqrestore(&request->engine->timeline->lock, flags);
	spin_unlock_irqrestore(&request->engine->timeline.lock, flags);
}

void i915_gem_set_wedged(struct drm_i915_private *i915)
@@ -3372,10 +3372,10 @@ void i915_gem_set_wedged(struct drm_i915_private *i915)
		 * (lockless) lookup doesn't try and wait upon the request as we
		 * reset it.
		 */
		spin_lock_irqsave(&engine->timeline->lock, flags);
		spin_lock_irqsave(&engine->timeline.lock, flags);
		intel_engine_init_global_seqno(engine,
					       intel_engine_last_submit(engine));
		spin_unlock_irqrestore(&engine->timeline->lock, flags);
		spin_unlock_irqrestore(&engine->timeline.lock, flags);

		i915_gem_reset_finish_engine(engine);
	}
@@ -3387,8 +3387,7 @@ void i915_gem_set_wedged(struct drm_i915_private *i915)

bool i915_gem_unset_wedged(struct drm_i915_private *i915)
{
	struct i915_gem_timeline *tl;
	int i;
	struct i915_timeline *tl;

	lockdep_assert_held(&i915->drm.struct_mutex);
	if (!test_bit(I915_WEDGED, &i915->gpu_error.flags))
@@ -3407,10 +3406,9 @@ bool i915_gem_unset_wedged(struct drm_i915_private *i915)
	 * No more can be submitted until we reset the wedged bit.
	 */
	list_for_each_entry(tl, &i915->gt.timelines, link) {
		for (i = 0; i < ARRAY_SIZE(tl->engine); i++) {
		struct i915_request *rq;

			rq = i915_gem_active_peek(&tl->engine[i].last_request,
		rq = i915_gem_active_peek(&tl->last_request,
					  &i915->drm.struct_mutex);
		if (!rq)
			continue;
@@ -3430,7 +3428,6 @@ bool i915_gem_unset_wedged(struct drm_i915_private *i915)
					   MAX_SCHEDULE_TIMEOUT) < 0)
			return false;
	}
	}
	i915_retire_requests(i915);
	GEM_BUG_ON(i915->gt.active_requests);

@@ -3734,17 +3731,9 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
	return ret;
}

static int wait_for_timeline(struct i915_gem_timeline *tl, unsigned int flags)
static int wait_for_timeline(struct i915_timeline *tl, unsigned int flags)
{
	int ret, i;

	for (i = 0; i < ARRAY_SIZE(tl->engine); i++) {
		ret = i915_gem_active_wait(&tl->engine[i].last_request, flags);
		if (ret)
			return ret;
	}

	return 0;
	return i915_gem_active_wait(&tl->last_request, flags);
}

static int wait_for_engines(struct drm_i915_private *i915)
@@ -3762,30 +3751,37 @@ static int wait_for_engines(struct drm_i915_private *i915)

int i915_gem_wait_for_idle(struct drm_i915_private *i915, unsigned int flags)
{
	int ret;

	/* If the device is asleep, we have no requests outstanding */
	if (!READ_ONCE(i915->gt.awake))
		return 0;

	if (flags & I915_WAIT_LOCKED) {
		struct i915_gem_timeline *tl;
		struct i915_timeline *tl;
		int err;

		lockdep_assert_held(&i915->drm.struct_mutex);

		list_for_each_entry(tl, &i915->gt.timelines, link) {
			ret = wait_for_timeline(tl, flags);
			if (ret)
				return ret;
			err = wait_for_timeline(tl, flags);
			if (err)
				return err;
		}
		i915_retire_requests(i915);

		ret = wait_for_engines(i915);
		return wait_for_engines(i915);
	} else {
		ret = wait_for_timeline(&i915->gt.execution_timeline, flags);
		struct intel_engine_cs *engine;
		enum intel_engine_id id;
		int err;

		for_each_engine(engine, i915, id) {
			err = wait_for_timeline(&engine->timeline, flags);
			if (err)
				return err;
		}

	return ret;
		return 0;
	}
}

static void __i915_gem_object_flush_for_display(struct drm_i915_gem_object *obj)
@@ -4954,7 +4950,7 @@ static void assert_kernel_context_is_current(struct drm_i915_private *i915)
	enum intel_engine_id id;

	for_each_engine(engine, i915, id) {
		GEM_BUG_ON(__i915_gem_active_peek(&engine->timeline->last_request));
		GEM_BUG_ON(__i915_gem_active_peek(&engine->timeline.last_request));
		GEM_BUG_ON(engine->last_retired_context != kernel_context);
	}
}
@@ -5603,12 +5599,6 @@ int i915_gem_init_early(struct drm_i915_private *dev_priv)
	INIT_LIST_HEAD(&dev_priv->gt.timelines);
	INIT_LIST_HEAD(&dev_priv->gt.active_rings);

	mutex_lock(&dev_priv->drm.struct_mutex);
	err = i915_gem_timeline_init__global(dev_priv);
	mutex_unlock(&dev_priv->drm.struct_mutex);
	if (err)
		goto err_priorities;

	i915_gem_init__mm(dev_priv);

	INIT_DELAYED_WORK(&dev_priv->gt.retire_work,
@@ -5628,8 +5618,6 @@ int i915_gem_init_early(struct drm_i915_private *dev_priv)

	return 0;

err_priorities:
	kmem_cache_destroy(dev_priv->priorities);
err_dependencies:
	kmem_cache_destroy(dev_priv->dependencies);
err_requests:
@@ -5650,12 +5638,7 @@ void i915_gem_cleanup_early(struct drm_i915_private *dev_priv)
	GEM_BUG_ON(!llist_empty(&dev_priv->mm.free_list));
	GEM_BUG_ON(atomic_read(&dev_priv->mm.free_count));
	WARN_ON(dev_priv->mm.object_count);

	mutex_lock(&dev_priv->drm.struct_mutex);
	i915_gem_timeline_fini(&dev_priv->gt.legacy_timeline);
	i915_gem_timeline_fini(&dev_priv->gt.execution_timeline);
	WARN_ON(!list_empty(&dev_priv->gt.timelines));
	mutex_unlock(&dev_priv->drm.struct_mutex);

	kmem_cache_destroy(dev_priv->priorities);
	kmem_cache_destroy(dev_priv->dependencies);
+21 −27
Original line number Diff line number Diff line
@@ -122,7 +122,6 @@ static void i915_gem_context_free(struct i915_gem_context *ctx)
	lockdep_assert_held(&ctx->i915->drm.struct_mutex);
	GEM_BUG_ON(!i915_gem_context_is_closed(ctx));

	i915_gem_timeline_free(ctx->timeline);
	i915_ppgtt_put(ctx->ppgtt);

	for (n = 0; n < ARRAY_SIZE(ctx->__engine); n++) {
@@ -377,18 +376,6 @@ i915_gem_create_context(struct drm_i915_private *dev_priv,
		ctx->desc_template = default_desc_template(dev_priv, ppgtt);
	}

	if (HAS_EXECLISTS(dev_priv)) {
		struct i915_gem_timeline *timeline;

		timeline = i915_gem_timeline_create(dev_priv, ctx->name);
		if (IS_ERR(timeline)) {
			__destroy_hw_context(ctx, file_priv);
			return ERR_CAST(timeline);
		}

		ctx->timeline = timeline;
	}

	trace_i915_context_create(ctx);

	return ctx;
@@ -590,19 +577,29 @@ void i915_gem_context_close(struct drm_file *file)
	idr_destroy(&file_priv->context_idr);
}

static bool engine_has_idle_kernel_context(struct intel_engine_cs *engine)
static struct i915_request *
last_request_on_engine(struct i915_timeline *timeline,
		       struct intel_engine_cs *engine)
{
	struct i915_gem_timeline *timeline;
	struct i915_request *rq;

	list_for_each_entry(timeline, &engine->i915->gt.timelines, link) {
		struct intel_timeline *tl;
	if (timeline == &engine->timeline)
		return NULL;

		if (timeline == &engine->i915->gt.execution_timeline)
			continue;
	rq = i915_gem_active_raw(&timeline->last_request,
				 &engine->i915->drm.struct_mutex);
	if (rq && rq->engine == engine)
		return rq;

	return NULL;
}

		tl = &timeline->engine[engine->id];
		if (i915_gem_active_peek(&tl->last_request,
					 &engine->i915->drm.struct_mutex))
static bool engine_has_idle_kernel_context(struct intel_engine_cs *engine)
{
	struct i915_timeline *timeline;

	list_for_each_entry(timeline, &engine->i915->gt.timelines, link) {
		if (last_request_on_engine(timeline, engine))
			return false;
	}

@@ -612,7 +609,7 @@ static bool engine_has_idle_kernel_context(struct intel_engine_cs *engine)
int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv)
{
	struct intel_engine_cs *engine;
	struct i915_gem_timeline *timeline;
	struct i915_timeline *timeline;
	enum intel_engine_id id;

	lockdep_assert_held(&dev_priv->drm.struct_mutex);
@@ -632,11 +629,8 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv)
		/* Queue this switch after all other activity */
		list_for_each_entry(timeline, &dev_priv->gt.timelines, link) {
			struct i915_request *prev;
			struct intel_timeline *tl;

			tl = &timeline->engine[engine->id];
			prev = i915_gem_active_raw(&tl->last_request,
						   &dev_priv->drm.struct_mutex);
			prev = last_request_on_engine(timeline, engine);
			if (prev)
				i915_sw_fence_await_sw_fence_gfp(&rq->submit,
								 &prev->submit,
+0 −2
Original line number Diff line number Diff line
@@ -58,8 +58,6 @@ struct i915_gem_context {
	/** file_priv: owning file descriptor */
	struct drm_i915_file_private *file_priv;

	struct i915_gem_timeline *timeline;

	/**
	 * @ppgtt: unique address space (GTT)
	 *
Loading