Re: [PATCH 1/6] drm/i915/execlists: Move RPCS setup to context pin

From: Chris Wilson <chris@chris-wilson.co.uk>
To: Intel-gfx@lists.freedesktop.org, Tvrtko Ursulin <tursulin@ursulin.net>
Subject: Re: [PATCH 1/6] drm/i915/execlists: Move RPCS setup to context pin
Date: Mon, 17 Sep 2018 12:43:58 +0100	[thread overview]
Message-ID: <153718463879.19296.14603779986765190145@skylake-alporthouse-com> (raw)
In-Reply-To: <20180917113058.28994-2-tvrtko.ursulin@linux.intel.com>

Quoting Tvrtko Ursulin (2018-09-17 12:30:53)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Configuring RPCS in context image just before pin is sufficient and will
> come extra handy in one of the following patches.
> 
> v2:
>  * Split image setup a bit differently. (Chris Wilson)
> 
> v3:
>  * Update context image after reset as well - otherwise the application
>    of pinned default state clears the RPCS.
> 
> v4:
>  * Use local variable throughout the function. (Chris Wilson)
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> # v2

> @@ -1955,14 +1971,14 @@ static void execlists_reset(struct intel_engine_cs *engine,
>                        engine->pinned_default_state + LRC_STATE_PN * PAGE_SIZE,
>                        engine->context_size - PAGE_SIZE);
>         }
> -       execlists_init_reg_state(regs,
> -                                request->gem_context, engine, request->ring);
>  
>         /* Move the RING_HEAD onto the breadcrumb, past the hanging batch */
> -       regs[CTX_RING_BUFFER_START + 1] = i915_ggtt_offset(request->ring->vma);
> -
>         request->ring->head = intel_ring_wrap(request->ring, request->postfix);
> -       regs[CTX_RING_HEAD + 1] = request->ring->head;
> +
> +       execlists_init_reg_state(regs, request->gem_context, engine,
> +                                request->ring);
> +
> +       __execlists_update_reg_state(engine, request->hw_context);

Probably need to reinforce the comments that the context state after the
guilty reset is our arbitrary defaults and not the state userspace
expects.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx