Chris Wilson <[email protected]> writes: > On retiring the request, we should not re-use these elements in the ring > (at least not until we fill the ringbuffer and knowingly reuse the space). > Leave behind some poison to (hopefully) trap ourselves if we make a > mistake. > > Suggested-by: Mika Kuoppala <[email protected]> > Signed-off-by: Chris Wilson <[email protected]> > Cc: Mika Kuoppala <[email protected]> > --- > drivers/gpu/drm/i915/i915_request.c | 26 +++++++++++++++++--------- > 1 file changed, 17 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_request.c > b/drivers/gpu/drm/i915/i915_request.c > index 0ecc2cf64216..9ee7bf0200b0 100644 > --- a/drivers/gpu/drm/i915/i915_request.c > +++ b/drivers/gpu/drm/i915/i915_request.c > @@ -203,6 +203,19 @@ static void free_capture_list(struct i915_request > *request) > } > } > > +static void __i915_request_fill(struct i915_request *rq, u8 val) > +{ > + void *vaddr = rq->ring->vaddr; > + u32 head; > + > + head = rq->infix; > + if (rq->postfix < head) { > + memset(vaddr + head, val, rq->ring->size - head); > + head = 0; > + } > + memset(vaddr + head, val, rq->postfix - head); > +} > + > static void remove_from_engine(struct i915_request *rq) > { > struct intel_engine_cs *engine, *locked; > @@ -247,6 +260,9 @@ bool i915_request_retire(struct i915_request *rq) > */ > GEM_BUG_ON(!list_is_first(&rq->link, > &i915_request_timeline(rq)->requests)); > + if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)) > + /* Poison before we release our space in the ring */ > + __i915_request_fill(rq, POISON_FREE);
Would it be too detrimental for perf to check for POISON_FREE when we emit the requests? I think it is a positive problem that we are in brink of a DEBUG_GEM_LEVEL split :O Regardless, with this we get gpu helping on revealing our bookkeepping failures. Reviewed-by: Mika Kuoppala <[email protected]> > rq->ring->head = rq->postfix; > > /* > @@ -1175,9 +1191,6 @@ i915_request_await_object(struct i915_request *to, > > void i915_request_skip(struct i915_request *rq, int error) > { > - void *vaddr = rq->ring->vaddr; > - u32 head; > - > GEM_BUG_ON(!IS_ERR_VALUE((long)error)); > dma_fence_set_error(&rq->fence, error); > > @@ -1189,12 +1202,7 @@ void i915_request_skip(struct i915_request *rq, int > error) > * context, clear out all the user operations leaving the > * breadcrumb at the end (so we get the fence notifications). > */ > - head = rq->infix; > - if (rq->postfix < head) { > - memset(vaddr + head, 0, rq->ring->size - head); > - head = 0; > - } > - memset(vaddr + head, 0, rq->postfix - head); > + __i915_request_fill(rq, 0); > rq->infix = rq->postfix; > } > > -- > 2.25.0 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
