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

drm/i915: Remove i915_request.global_seqno 



Having weaned the interrupt handling off using a single global execution
queue, we no longer need to emit a global_seqno. Note that we still have
a few assumptions about execution order along engine timelines, but this
removes the most obvious artefact!

Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190226094922.31617-3-chris@chris-wilson.co.uk
parent 8892f477

drivers/gpu/drm/i915/i915_gpu_error.c

+4 −30
Original line number Diff line number Diff line
@@ -380,19 +380,16 @@ static void print_error_buffers(struct drm_i915_error_state_buf *m, 
	err_printf(m, "%s [%d]:\n", name, count);



	while (count--) {

		err_printf(m, "    %08x_%08x %8u %02x %02x %02x",

		err_printf(m, "    %08x_%08x %8u %02x %02x",

			   upper_32_bits(err->gtt_offset),

			   lower_32_bits(err->gtt_offset),

			   err->size,

			   err->read_domains,

			   err->write_domain,

			   err->wseqno);

			   err->write_domain);

		err_puts(m, tiling_flag(err->tiling));

		err_puts(m, dirty_flag(err->dirty));

		err_puts(m, purgeable_flag(err->purgeable));

		err_puts(m, err->userptr ? " userptr" : "");

		err_puts(m, err->engine != -1 ? " " : "");

		err_puts(m, engine_name(m->i915, err->engine));

		err_puts(m, i915_cache_level_str(m->i915, err->cache_level));



		if (err->name)
@@ -1048,27 +1045,6 @@ i915_error_object_create(struct drm_i915_private *i915, 
	return dst;

}



/* The error capture is special as tries to run underneath the normal

 * locking rules - so we use the raw version of the i915_active_request lookup.

 */

static inline u32

__active_get_seqno(struct i915_active_request *active)

{

	struct i915_request *request;



	request = __i915_active_request_peek(active);

	return request ? request->global_seqno : 0;

}



static inline int

__active_get_engine_id(struct i915_active_request *active)

{

	struct i915_request *request;



	request = __i915_active_request_peek(active);

	return request ? request->engine->id : -1;

}



static void capture_bo(struct drm_i915_error_buffer *err,

		       struct i915_vma *vma)

{
@@ -1077,9 +1053,6 @@ static void capture_bo(struct drm_i915_error_buffer *err, 
	err->size = obj->base.size;

	err->name = obj->base.name;



	err->wseqno = __active_get_seqno(&obj->frontbuffer_write);

	err->engine = __active_get_engine_id(&obj->frontbuffer_write);



	err->gtt_offset = vma->node.start;

	err->read_domains = obj->read_domains;

	err->write_domain = obj->write_domain;
@@ -1284,7 +1257,8 @@ static void record_request(struct i915_request *request, 
	struct i915_gem_context *ctx = request->gem_context;



	erq->flags = request->fence.flags;

	erq->context = ctx->hw_id;

	erq->context = request->fence.context;

	erq->seqno = request->fence.seqno;

	erq->sched_attr = request->sched.attr;

	erq->jiffies = request->emitted_jiffies;

	erq->start = i915_ggtt_offset(request->ring->vma);

drivers/gpu/drm/i915/i915_gpu_error.h

+0 −2
Original line number Diff line number Diff line
@@ -164,7 +164,6 @@ struct i915_gpu_state { 
	struct drm_i915_error_buffer {

		u32 size;

		u32 name;

		u32 wseqno;

		u64 gtt_offset;

		u32 read_domains;

		u32 write_domain;
@@ -173,7 +172,6 @@ struct i915_gpu_state { 
		u32 dirty:1;

		u32 purgeable:1;

		u32 userptr:1;

		s32 engine:4;

		u32 cache_level:3;

	} *active_bo[I915_NUM_ENGINES], *pinned_bo;

	u32 active_bo_count[I915_NUM_ENGINES], pinned_bo_count;

drivers/gpu/drm/i915/i915_request.c

+5 −29
Original line number Diff line number Diff line
@@ -179,10 +179,9 @@ static void free_capture_list(struct i915_request *request) 
static void __retire_engine_request(struct intel_engine_cs *engine,

				    struct i915_request *rq)

{

	GEM_TRACE("%s(%s) fence %llx:%lld, global=%d, current %d\n",

	GEM_TRACE("%s(%s) fence %llx:%lld, current %d\n",

		  __func__, engine->name,

		  rq->fence.context, rq->fence.seqno,

		  rq->global_seqno,

		  hwsp_seqno(rq));



	GEM_BUG_ON(!i915_request_completed(rq));
@@ -242,10 +241,9 @@ static void i915_request_retire(struct i915_request *request) 
{

	struct i915_active_request *active, *next;



	GEM_TRACE("%s fence %llx:%lld, global=%d, current %d\n",

	GEM_TRACE("%s fence %llx:%lld, current %d\n",

		  request->engine->name,

		  request->fence.context, request->fence.seqno,

		  request->global_seqno,

		  hwsp_seqno(request));



	lockdep_assert_held(&request->i915->drm.struct_mutex);
@@ -303,10 +301,9 @@ void i915_request_retire_upto(struct i915_request *rq) 
	struct intel_ring *ring = rq->ring;

	struct i915_request *tmp;



	GEM_TRACE("%s fence %llx:%lld, global=%d, current %d\n",

	GEM_TRACE("%s fence %llx:%lld, current %d\n",

		  rq->engine->name,

		  rq->fence.context, rq->fence.seqno,

		  rq->global_seqno,

		  hwsp_seqno(rq));



	lockdep_assert_held(&rq->i915->drm.struct_mutex);
@@ -339,22 +336,13 @@ static void move_to_timeline(struct i915_request *request, 
	spin_unlock(&request->timeline->lock);

}



static u32 next_global_seqno(struct i915_timeline *tl)

{

	if (!++tl->seqno)

		++tl->seqno;

	return tl->seqno;

}



void __i915_request_submit(struct i915_request *request)

{

	struct intel_engine_cs *engine = request->engine;

	u32 seqno;



	GEM_TRACE("%s fence %llx:%lld -> global=%d, current %d\n",

	GEM_TRACE("%s fence %llx:%lld -> current %d\n",

		  engine->name,

		  request->fence.context, request->fence.seqno,

		  engine->timeline.seqno + 1,

		  hwsp_seqno(request));



	GEM_BUG_ON(!irqs_disabled());
@@ -363,16 +351,10 @@ void __i915_request_submit(struct i915_request *request) 
	if (i915_gem_context_is_banned(request->gem_context))

		i915_request_skip(request, -EIO);



	GEM_BUG_ON(request->global_seqno);



	seqno = next_global_seqno(&engine->timeline);

	GEM_BUG_ON(!seqno);



	/* We may be recursing from the signal callback of another i915 fence */

	spin_lock_nested(&request->lock, SINGLE_DEPTH_NESTING);

	GEM_BUG_ON(test_bit(I915_FENCE_FLAG_ACTIVE, &request->fence.flags));

	set_bit(I915_FENCE_FLAG_ACTIVE, &request->fence.flags);

	request->global_seqno = seqno;

	if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &request->fence.flags) &&

	    !i915_request_enable_breadcrumb(request))

		intel_engine_queue_breadcrumbs(engine);
@@ -404,10 +386,9 @@ void __i915_request_unsubmit(struct i915_request *request) 
{

	struct intel_engine_cs *engine = request->engine;



	GEM_TRACE("%s fence %llx:%lld <- global=%d, current %d\n",

	GEM_TRACE("%s fence %llx:%lld, current %d\n",

		  engine->name,

		  request->fence.context, request->fence.seqno,

		  request->global_seqno,

		  hwsp_seqno(request));



	GEM_BUG_ON(!irqs_disabled());
@@ -417,13 +398,9 @@ void __i915_request_unsubmit(struct i915_request *request) 
	 * Only unwind in reverse order, required so that the per-context list

	 * is kept in seqno/ring order.

	 */

	GEM_BUG_ON(!request->global_seqno);

	GEM_BUG_ON(request->global_seqno != engine->timeline.seqno);

	engine->timeline.seqno--;



	/* We may be recursing from the signal callback of another i915 fence */

	spin_lock_nested(&request->lock, SINGLE_DEPTH_NESTING);

	request->global_seqno = 0;

	if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &request->fence.flags))

		i915_request_cancel_breadcrumb(request);

	GEM_BUG_ON(!test_bit(I915_FENCE_FLAG_ACTIVE, &request->fence.flags));
@@ -637,7 +614,6 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx) 
	i915_sched_node_init(&rq->sched);



	/* No zalloc, must clear what we need by hand */

	rq->global_seqno = 0;

	rq->file_priv = NULL;

	rq->batch = NULL;

	rq->capture_list = NULL;

drivers/gpu/drm/i915/i915_request.h

+0 −32
Original line number Diff line number Diff line
@@ -147,14 +147,6 @@ struct i915_request { 
	 */

	const u32 *hwsp_seqno;



	/**

	 * GEM sequence number associated with this request on the

	 * global execution timeline. It is zero when the request is not

	 * on the HW queue (i.e. not on the engine timeline list).

	 * Its value is guarded by the timeline spinlock.

	 */

	u32 global_seqno;



	/** Position in the ring of the start of the request */

	u32 head;
@@ -247,30 +239,6 @@ i915_request_put(struct i915_request *rq) 
	dma_fence_put(&rq->fence);

}



/**

 * i915_request_global_seqno - report the current global seqno

 * @request - the request

 *

 * A request is assigned a global seqno only when it is on the hardware

 * execution queue. The global seqno can be used to maintain a list of

 * requests on the same engine in retirement order, for example for

 * constructing a priority queue for waiting. Prior to its execution, or

 * if it is subsequently removed in the event of preemption, its global

 * seqno is zero. As both insertion and removal from the execution queue

 * may operate in IRQ context, it is not guarded by the usual struct_mutex

 * BKL. Instead those relying on the global seqno must be prepared for its

 * value to change between reads. Only when the request is complete can

 * the global seqno be stable (due to the memory barriers on submitting

 * the commands to the hardware to write the breadcrumb, if the HWS shows

 * that it has passed the global seqno and the global seqno is unchanged

 * after the read, it is indeed complete).

 */

static inline u32

i915_request_global_seqno(const struct i915_request *request)

{

	return READ_ONCE(request->global_seqno);

}



int i915_request_await_object(struct i915_request *to,

			      struct drm_i915_gem_object *obj,

			      bool write);

drivers/gpu/drm/i915/i915_trace.h

+8 −17
Original line number Diff line number Diff line
@@ -708,7 +708,6 @@ DECLARE_EVENT_CLASS(i915_request, 
			     __field(u16, class)

			     __field(u16, instance)

			     __field(u32, seqno)

			     __field(u32, global)

			     ),



	    TP_fast_assign(
@@ -718,13 +717,11 @@ DECLARE_EVENT_CLASS(i915_request, 
			   __entry->instance = rq->engine->instance;

			   __entry->ctx = rq->fence.context;

			   __entry->seqno = rq->fence.seqno;

			   __entry->global = rq->global_seqno;

			   ),



	    TP_printk("dev=%u, engine=%u:%u, hw_id=%u, ctx=%llu, seqno=%u, global=%u",

	    TP_printk("dev=%u, engine=%u:%u, hw_id=%u, ctx=%llu, seqno=%u",

		      __entry->dev, __entry->class, __entry->instance,

		      __entry->hw_id, __entry->ctx, __entry->seqno,

		      __entry->global)

		      __entry->hw_id, __entry->ctx, __entry->seqno)

);



DEFINE_EVENT(i915_request, i915_request_add,
@@ -754,7 +751,6 @@ TRACE_EVENT(i915_request_in, 
			     __field(u16, class)

			     __field(u16, instance)

			     __field(u32, seqno)

			     __field(u32, global_seqno)

			     __field(u32, port)

			     __field(u32, prio)

			    ),
@@ -766,15 +762,14 @@ TRACE_EVENT(i915_request_in, 
			   __entry->instance = rq->engine->instance;

			   __entry->ctx = rq->fence.context;

			   __entry->seqno = rq->fence.seqno;

			   __entry->global_seqno = rq->global_seqno;

			   __entry->prio = rq->sched.attr.priority;

			   __entry->port = port;

			   ),



	    TP_printk("dev=%u, engine=%u:%u, hw_id=%u, ctx=%llu, seqno=%u, prio=%u, global=%u, port=%u",

	    TP_printk("dev=%u, engine=%u:%u, hw_id=%u, ctx=%llu, seqno=%u, prio=%u, port=%u",

		      __entry->dev, __entry->class, __entry->instance,

		      __entry->hw_id, __entry->ctx, __entry->seqno,

		      __entry->prio, __entry->global_seqno, __entry->port)

		      __entry->prio, __entry->port)

);



TRACE_EVENT(i915_request_out,
@@ -788,7 +783,6 @@ TRACE_EVENT(i915_request_out, 
			     __field(u16, class)

			     __field(u16, instance)

			     __field(u32, seqno)

			     __field(u32, global_seqno)

			     __field(u32, completed)

			    ),
@@ -799,14 +793,13 @@ TRACE_EVENT(i915_request_out, 
			   __entry->instance = rq->engine->instance;

			   __entry->ctx = rq->fence.context;

			   __entry->seqno = rq->fence.seqno;

			   __entry->global_seqno = rq->global_seqno;

			   __entry->completed = i915_request_completed(rq);

			   ),



		    TP_printk("dev=%u, engine=%u:%u, hw_id=%u, ctx=%llu, seqno=%u, global=%u, completed?=%u",

		    TP_printk("dev=%u, engine=%u:%u, hw_id=%u, ctx=%llu, seqno=%u, completed?=%u",

			      __entry->dev, __entry->class, __entry->instance,

			      __entry->hw_id, __entry->ctx, __entry->seqno,

			      __entry->global_seqno, __entry->completed)

			      __entry->completed)

);



#else
@@ -849,7 +842,6 @@ TRACE_EVENT(i915_request_wait_begin, 
			     __field(u16, class)

			     __field(u16, instance)

			     __field(u32, seqno)

			     __field(u32, global)

			     __field(unsigned int, flags)

			     ),
@@ -866,14 +858,13 @@ TRACE_EVENT(i915_request_wait_begin, 
			   __entry->instance = rq->engine->instance;

			   __entry->ctx = rq->fence.context;

			   __entry->seqno = rq->fence.seqno;

			   __entry->global = rq->global_seqno;

			   __entry->flags = flags;

			   ),



	    TP_printk("dev=%u, engine=%u:%u, hw_id=%u, ctx=%llu, seqno=%u, global=%u, blocking=%u, flags=0x%x",

	    TP_printk("dev=%u, engine=%u:%u, hw_id=%u, ctx=%llu, seqno=%u, blocking=%u, flags=0x%x",

		      __entry->dev, __entry->class, __entry->instance,

		      __entry->hw_id, __entry->ctx, __entry->seqno,

		      __entry->global, !!(__entry->flags & I915_WAIT_LOCKED),

		      !!(__entry->flags & I915_WAIT_LOCKED),

		      __entry->flags)

);

CRA Git | Maintained and supported by SUSTech CRA and CCSE