Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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 <mika.kuoppala@xxxxxxxxxxxxxxx>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> ---
> 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 <mika.kuoppala@xxxxxxxxxxxxxxx>
> 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
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
