diff mbox

drm/i915: Assert that the request->tail is always qword aligned

Message ID 20170217163833.731-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson Feb. 17, 2017, 4:38 p.m. UTC
The hardware requires that the tail pointer only advance in qword units,
so assert that the value we write is aligned to qwords, and similarly
enforce this restriction onto the request->tail.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c        | 4 ++++
 drivers/gpu/drm/i915/intel_ringbuffer.c | 3 +++
 2 files changed, 7 insertions(+)

Comments

Michał Winiarski Feb. 20, 2017, 1:45 p.m. UTC | #1
On Fri, Feb 17, 2017 at 04:38:33PM +0000, Chris Wilson wrote:
> The hardware requires that the tail pointer only advance in qword units,
> so assert that the value we write is aligned to qwords, and similarly
> enforce this restriction onto the request->tail.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>

LGTM.
Quick grep confirms that those are indeed all the relevant spots where we're
updating req->tail.

Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_lrc.c        | 4 ++++
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 3 +++
>  2 files changed, 7 insertions(+)
Chris Wilson Feb. 20, 2017, 2:54 p.m. UTC | #2
On Mon, Feb 20, 2017 at 02:45:30PM +0100, Michał Winiarski wrote:
> On Fri, Feb 17, 2017 at 04:38:33PM +0000, Chris Wilson wrote:
> > The hardware requires that the tail pointer only advance in qword units,
> > so assert that the value we write is aligned to qwords, and similarly
> > enforce this restriction onto the request->tail.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> 
> LGTM.
> Quick grep confirms that those are indeed all the relevant spots where we're
> updating req->tail.
> 
> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>

Ta, pushed just to exercise igt.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index afcb1f189162..d6b67be4715d 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -325,6 +325,7 @@  static u64 execlists_update_context(struct drm_i915_gem_request *rq)
 		rq->ctx->ppgtt ?: rq->i915->mm.aliasing_ppgtt;
 	u32 *reg_state = ce->lrc_reg_state;
 
+	GEM_BUG_ON(!IS_ALIGNED(rq->tail, 8));
 	reg_state[CTX_RING_TAIL+1] = rq->tail;
 
 	/* True 32b PPGTT with dynamic page allocation: update PDP
@@ -1283,6 +1284,7 @@  static void reset_common_ring(struct intel_engine_cs *engine,
 
 	/* Reset WaIdleLiteRestore:bdw,skl as well */
 	request->tail = request->wa_tail - WA_TAIL_DWORDS * sizeof(u32);
+	GEM_BUG_ON(!IS_ALIGNED(request->tail, 8));
 }
 
 static int intel_logical_ring_emit_pdps(struct drm_i915_gem_request *req)
@@ -1494,6 +1496,7 @@  static void gen8_emit_breadcrumb(struct drm_i915_gem_request *request, u32 *cs)
 	*cs++ = MI_USER_INTERRUPT;
 	*cs++ = MI_NOOP;
 	request->tail = intel_ring_offset(request, cs);
+	GEM_BUG_ON(!IS_ALIGNED(request->tail, 8));
 
 	gen8_emit_wa_tail(request, cs);
 }
@@ -1521,6 +1524,7 @@  static void gen8_emit_breadcrumb_render(struct drm_i915_gem_request *request,
 	*cs++ = MI_USER_INTERRUPT;
 	*cs++ = MI_NOOP;
 	request->tail = intel_ring_offset(request, cs);
+	GEM_BUG_ON(!IS_ALIGNED(request->tail, 8));
 
 	gen8_emit_wa_tail(request, cs);
 }
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index d56f384938f7..f62afffef682 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -784,6 +784,7 @@  static void i9xx_submit_request(struct drm_i915_gem_request *request)
 
 	i915_gem_request_submit(request);
 
+	GEM_BUG_ON(!IS_ALIGNED(request->tail, 8));
 	I915_WRITE_TAIL(request->engine, request->tail);
 }
 
@@ -795,6 +796,7 @@  static void i9xx_emit_breadcrumb(struct drm_i915_gem_request *req, u32 *cs)
 	*cs++ = MI_USER_INTERRUPT;
 
 	req->tail = intel_ring_offset(req, cs);
+	GEM_BUG_ON(!IS_ALIGNED(req->tail, 8));
 }
 
 static const int i9xx_emit_breadcrumb_sz = 4;
@@ -833,6 +835,7 @@  static void gen8_render_emit_breadcrumb(struct drm_i915_gem_request *req,
 	*cs++ = MI_NOOP;
 
 	req->tail = intel_ring_offset(req, cs);
+	GEM_BUG_ON(!IS_ALIGNED(req->tail, 8));
 }
 
 static const int gen8_render_emit_breadcrumb_sz = 8;