* [PATCH] drm/i915: Refactor tests for validity of RING_TAIL
@ 2017-03-27 9:50 Chris Wilson
2017-03-27 10:17 ` Tvrtko Ursulin
0 siblings, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2017-03-27 9:50 UTC (permalink / raw)
To: intel-gfx
Whilst I like having the assertions clearly visible in the code, they
are quite repetitious! As we find new limits we want to incorporate into
the set of assertions, it make sense to refactor them to a common
routine.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_lrc.c | 12 ++++--------
drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +++------
drivers/gpu/drm/i915/intel_ringbuffer.h | 7 +++++++
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 3b84fbb7da5d..ccbe1a16f3c3 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -326,8 +326,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));
- GEM_BUG_ON(rq->tail >= rq->ring->size);
+ assert_ring_tail_valid(rq->ring, rq->tail);
reg_state[CTX_RING_TAIL+1] = rq->tail;
/* True 32b PPGTT with dynamic page allocation: update PDP
@@ -1304,8 +1303,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);
request->tail &= request->ring->size - 1;
- GEM_BUG_ON(!IS_ALIGNED(request->tail, 8));
- GEM_BUG_ON(request->tail >= request->ring->size);
+ assert_ring_tail_valid(request->ring, request->tail);
}
static int intel_logical_ring_emit_pdps(struct drm_i915_gem_request *req)
@@ -1516,8 +1514,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));
- GEM_BUG_ON(request->tail >= request->ring->size);
+ assert_ring_tail_valid(request->ring, request->tail);
gen8_emit_wa_tail(request, cs);
}
@@ -1545,8 +1542,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));
- GEM_BUG_ON(request->tail >= request->ring->size);
+ assert_ring_tail_valid(request->ring, request->tail);
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 47921dcbedb3..66a2b8b83972 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -774,8 +774,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));
- GEM_BUG_ON(request->tail >= request->ring->size);
+ assert_ring_tail_valid(request->ring, request->tail);
I915_WRITE_TAIL(request->engine, request->tail);
}
@@ -787,8 +786,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));
- GEM_BUG_ON(req->tail >= req->ring->size);
+ assert_ring_tail_valid(req->ring, req->tail);
}
static const int i9xx_emit_breadcrumb_sz = 4;
@@ -827,8 +825,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));
- GEM_BUG_ON(req->tail >= req->ring->size);
+ assert_ring_tail_valid(req->ring, req->tail);
}
static const int gen8_render_emit_breadcrumb_sz = 8;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 2ecb41788fb6..6bc89719250a 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -523,6 +523,13 @@ intel_ring_offset(struct drm_i915_gem_request *req, void *addr)
return offset & (req->ring->size - 1);
}
+static inline void
+assert_ring_tail_valid(const struct intel_ring *ring, unsigned int tail)
+{
+ GEM_BUG_ON(!IS_ALIGNED(tail, 8));
+ GEM_BUG_ON(tail >= ring->size);
+}
+
void intel_ring_update_space(struct intel_ring *ring);
void intel_engine_init_global_seqno(struct intel_engine_cs *engine, u32 seqno);
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Refactor tests for validity of RING_TAIL
2017-03-27 9:50 [PATCH] drm/i915: Refactor tests for validity of RING_TAIL Chris Wilson
@ 2017-03-27 10:17 ` Tvrtko Ursulin
2017-03-27 10:41 ` Chris Wilson
0 siblings, 1 reply; 4+ messages in thread
From: Tvrtko Ursulin @ 2017-03-27 10:17 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 27/03/2017 10:50, Chris Wilson wrote:
> Whilst I like having the assertions clearly visible in the code, they
> are quite repetitious! As we find new limits we want to incorporate into
> the set of assertions, it make sense to refactor them to a common
> routine.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/intel_lrc.c | 12 ++++--------
> drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +++------
> drivers/gpu/drm/i915/intel_ringbuffer.h | 7 +++++++
> 3 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 3b84fbb7da5d..ccbe1a16f3c3 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -326,8 +326,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));
> - GEM_BUG_ON(rq->tail >= rq->ring->size);
> + assert_ring_tail_valid(rq->ring, rq->tail);
> reg_state[CTX_RING_TAIL+1] = rq->tail;
>
> /* True 32b PPGTT with dynamic page allocation: update PDP
> @@ -1304,8 +1303,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);
> request->tail &= request->ring->size - 1;
> - GEM_BUG_ON(!IS_ALIGNED(request->tail, 8));
> - GEM_BUG_ON(request->tail >= request->ring->size);
> + assert_ring_tail_valid(request->ring, request->tail);
> }
>
> static int intel_logical_ring_emit_pdps(struct drm_i915_gem_request *req)
> @@ -1516,8 +1514,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));
> - GEM_BUG_ON(request->tail >= request->ring->size);
> + assert_ring_tail_valid(request->ring, request->tail);
>
> gen8_emit_wa_tail(request, cs);
> }
> @@ -1545,8 +1542,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));
> - GEM_BUG_ON(request->tail >= request->ring->size);
> + assert_ring_tail_valid(request->ring, request->tail);
>
> 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 47921dcbedb3..66a2b8b83972 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -774,8 +774,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));
> - GEM_BUG_ON(request->tail >= request->ring->size);
> + assert_ring_tail_valid(request->ring, request->tail);
> I915_WRITE_TAIL(request->engine, request->tail);
> }
>
> @@ -787,8 +786,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));
> - GEM_BUG_ON(req->tail >= req->ring->size);
> + assert_ring_tail_valid(req->ring, req->tail);
> }
>
> static const int i9xx_emit_breadcrumb_sz = 4;
> @@ -827,8 +825,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));
> - GEM_BUG_ON(req->tail >= req->ring->size);
> + assert_ring_tail_valid(req->ring, req->tail);
> }
>
> static const int gen8_render_emit_breadcrumb_sz = 8;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 2ecb41788fb6..6bc89719250a 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -523,6 +523,13 @@ intel_ring_offset(struct drm_i915_gem_request *req, void *addr)
> return offset & (req->ring->size - 1);
> }
>
> +static inline void
> +assert_ring_tail_valid(const struct intel_ring *ring, unsigned int tail)
> +{
> + GEM_BUG_ON(!IS_ALIGNED(tail, 8));
> + GEM_BUG_ON(tail >= ring->size);
> +}
> +
> void intel_ring_update_space(struct intel_ring *ring);
>
> void intel_engine_init_global_seqno(struct intel_engine_cs *engine, u32 seqno);
>
Looking at the call sites, how about assert_request_tail_valid(req)? And
squash with the one where you are adding the second check.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Refactor tests for validity of RING_TAIL
2017-03-27 10:17 ` Tvrtko Ursulin
@ 2017-03-27 10:41 ` Chris Wilson
2017-03-27 11:40 ` Tvrtko Ursulin
0 siblings, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2017-03-27 10:41 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
On Mon, Mar 27, 2017 at 11:17:20AM +0100, Tvrtko Ursulin wrote:
>
> On 27/03/2017 10:50, Chris Wilson wrote:
> >Whilst I like having the assertions clearly visible in the code, they
> >are quite repetitious! As we find new limits we want to incorporate into
> >the set of assertions, it make sense to refactor them to a common
> >routine.
> >
> >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >---
> > drivers/gpu/drm/i915/intel_lrc.c | 12 ++++--------
> > drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +++------
> > drivers/gpu/drm/i915/intel_ringbuffer.h | 7 +++++++
> > 3 files changed, 14 insertions(+), 14 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> >index 3b84fbb7da5d..ccbe1a16f3c3 100644
> >--- a/drivers/gpu/drm/i915/intel_lrc.c
> >+++ b/drivers/gpu/drm/i915/intel_lrc.c
> >@@ -326,8 +326,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));
> >- GEM_BUG_ON(rq->tail >= rq->ring->size);
> >+ assert_ring_tail_valid(rq->ring, rq->tail);
> > reg_state[CTX_RING_TAIL+1] = rq->tail;
> >
> > /* True 32b PPGTT with dynamic page allocation: update PDP
> >@@ -1304,8 +1303,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);
> > request->tail &= request->ring->size - 1;
> >- GEM_BUG_ON(!IS_ALIGNED(request->tail, 8));
> >- GEM_BUG_ON(request->tail >= request->ring->size);
> >+ assert_ring_tail_valid(request->ring, request->tail);
> > }
> >
> > static int intel_logical_ring_emit_pdps(struct drm_i915_gem_request *req)
> >@@ -1516,8 +1514,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));
> >- GEM_BUG_ON(request->tail >= request->ring->size);
> >+ assert_ring_tail_valid(request->ring, request->tail);
> >
> > gen8_emit_wa_tail(request, cs);
> > }
> >@@ -1545,8 +1542,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));
> >- GEM_BUG_ON(request->tail >= request->ring->size);
> >+ assert_ring_tail_valid(request->ring, request->tail);
> >
> > 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 47921dcbedb3..66a2b8b83972 100644
> >--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> >+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >@@ -774,8 +774,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));
> >- GEM_BUG_ON(request->tail >= request->ring->size);
> >+ assert_ring_tail_valid(request->ring, request->tail);
> > I915_WRITE_TAIL(request->engine, request->tail);
> > }
> >
> >@@ -787,8 +786,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));
> >- GEM_BUG_ON(req->tail >= req->ring->size);
> >+ assert_ring_tail_valid(req->ring, req->tail);
> > }
> >
> > static const int i9xx_emit_breadcrumb_sz = 4;
> >@@ -827,8 +825,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));
> >- GEM_BUG_ON(req->tail >= req->ring->size);
> >+ assert_ring_tail_valid(req->ring, req->tail);
> > }
> >
> > static const int gen8_render_emit_breadcrumb_sz = 8;
> >diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> >index 2ecb41788fb6..6bc89719250a 100644
> >--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> >+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> >@@ -523,6 +523,13 @@ intel_ring_offset(struct drm_i915_gem_request *req, void *addr)
> > return offset & (req->ring->size - 1);
> > }
> >
> >+static inline void
> >+assert_ring_tail_valid(const struct intel_ring *ring, unsigned int tail)
> >+{
> >+ GEM_BUG_ON(!IS_ALIGNED(tail, 8));
> >+ GEM_BUG_ON(tail >= ring->size);
> >+}
> >+
> > void intel_ring_update_space(struct intel_ring *ring);
> >
> > void intel_engine_init_global_seqno(struct intel_engine_cs *engine, u32 seqno);
> >
>
> Looking at the call sites, how about assert_request_tail_valid(req)?
> And squash with the one where you are adding the second check.
I considered it, but I thought using request distracted from the
intention of checking against the current hardware limits (hence
intel_ringbuffer.h for lack of a better home rather than
i915_gem_request.h)
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Refactor tests for validity of RING_TAIL
2017-03-27 10:41 ` Chris Wilson
@ 2017-03-27 11:40 ` Tvrtko Ursulin
0 siblings, 0 replies; 4+ messages in thread
From: Tvrtko Ursulin @ 2017-03-27 11:40 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 27/03/2017 11:41, Chris Wilson wrote:
> On Mon, Mar 27, 2017 at 11:17:20AM +0100, Tvrtko Ursulin wrote:
>>
>> On 27/03/2017 10:50, Chris Wilson wrote:
>>> Whilst I like having the assertions clearly visible in the code, they
>>> are quite repetitious! As we find new limits we want to incorporate into
>>> the set of assertions, it make sense to refactor them to a common
>>> routine.
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> ---
>>> drivers/gpu/drm/i915/intel_lrc.c | 12 ++++--------
>>> drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +++------
>>> drivers/gpu/drm/i915/intel_ringbuffer.h | 7 +++++++
>>> 3 files changed, 14 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>>> index 3b84fbb7da5d..ccbe1a16f3c3 100644
>>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>>> @@ -326,8 +326,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));
>>> - GEM_BUG_ON(rq->tail >= rq->ring->size);
>>> + assert_ring_tail_valid(rq->ring, rq->tail);
>>> reg_state[CTX_RING_TAIL+1] = rq->tail;
>>>
>>> /* True 32b PPGTT with dynamic page allocation: update PDP
>>> @@ -1304,8 +1303,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);
>>> request->tail &= request->ring->size - 1;
>>> - GEM_BUG_ON(!IS_ALIGNED(request->tail, 8));
>>> - GEM_BUG_ON(request->tail >= request->ring->size);
>>> + assert_ring_tail_valid(request->ring, request->tail);
>>> }
>>>
>>> static int intel_logical_ring_emit_pdps(struct drm_i915_gem_request *req)
>>> @@ -1516,8 +1514,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));
>>> - GEM_BUG_ON(request->tail >= request->ring->size);
>>> + assert_ring_tail_valid(request->ring, request->tail);
>>>
>>> gen8_emit_wa_tail(request, cs);
>>> }
>>> @@ -1545,8 +1542,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));
>>> - GEM_BUG_ON(request->tail >= request->ring->size);
>>> + assert_ring_tail_valid(request->ring, request->tail);
>>>
>>> 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 47921dcbedb3..66a2b8b83972 100644
>>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>>> @@ -774,8 +774,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));
>>> - GEM_BUG_ON(request->tail >= request->ring->size);
>>> + assert_ring_tail_valid(request->ring, request->tail);
>>> I915_WRITE_TAIL(request->engine, request->tail);
>>> }
>>>
>>> @@ -787,8 +786,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));
>>> - GEM_BUG_ON(req->tail >= req->ring->size);
>>> + assert_ring_tail_valid(req->ring, req->tail);
>>> }
>>>
>>> static const int i9xx_emit_breadcrumb_sz = 4;
>>> @@ -827,8 +825,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));
>>> - GEM_BUG_ON(req->tail >= req->ring->size);
>>> + assert_ring_tail_valid(req->ring, req->tail);
>>> }
>>>
>>> static const int gen8_render_emit_breadcrumb_sz = 8;
>>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
>>> index 2ecb41788fb6..6bc89719250a 100644
>>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
>>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
>>> @@ -523,6 +523,13 @@ intel_ring_offset(struct drm_i915_gem_request *req, void *addr)
>>> return offset & (req->ring->size - 1);
>>> }
>>>
>>> +static inline void
>>> +assert_ring_tail_valid(const struct intel_ring *ring, unsigned int tail)
>>> +{
>>> + GEM_BUG_ON(!IS_ALIGNED(tail, 8));
>>> + GEM_BUG_ON(tail >= ring->size);
>>> +}
>>> +
>>> void intel_ring_update_space(struct intel_ring *ring);
>>>
>>> void intel_engine_init_global_seqno(struct intel_engine_cs *engine, u32 seqno);
>>>
>>
>> Looking at the call sites, how about assert_request_tail_valid(req)?
>> And squash with the one where you are adding the second check.
>
> I considered it, but I thought using request distracted from the
> intention of checking against the current hardware limits (hence
> intel_ringbuffer.h for lack of a better home rather than
> i915_gem_request.h)
Yes makes sense.
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-03-27 11:40 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-27 9:50 [PATCH] drm/i915: Refactor tests for validity of RING_TAIL Chris Wilson
2017-03-27 10:17 ` Tvrtko Ursulin
2017-03-27 10:41 ` Chris Wilson
2017-03-27 11:40 ` Tvrtko Ursulin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.