[Intel-gfx] [PATCH v2 1/7] drm/i915/gen8: Add infrastructure to initialize WA batch buffers

[Chris Wilson]

On Mon, Jun 01, 2015 at 11:01:08AM +0100, Siluvery, Arun wrote:
> On 29/05/2015 19:16, Chris Wilson wrote:
> >On Fri, May 29, 2015 at 07:03:19PM +0100, Arun Siluvery wrote:
> >>This patch adds functions to setup WA batch buffers but they are not yet
> >>enabled in this patch. Some of the WA are to be applied during context save
> >>but before restore and some at the end of context save/restore but before
> >>executing the instructions in the ring, WA batch buffers are created for
> >>this purpose and these WA cannot be applied using normal means.
> >>
> >>Signed-off-by: Namrta <namrta.salonie at intel.com>
> >>Signed-off-by: Arun Siluvery <arun.siluvery at linux.intel.com>
> >>---
> >>  drivers/gpu/drm/i915/i915_drv.h  |   3 ++
> >>  drivers/gpu/drm/i915/intel_lrc.c | 101 +++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 104 insertions(+)
> >>
> >>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >>index 731b5ce..dd4b31d 100644
> >>--- a/drivers/gpu/drm/i915/i915_drv.h
> >>+++ b/drivers/gpu/drm/i915/i915_drv.h
> >>@@ -814,6 +814,9 @@ struct intel_context {
> >>
> >>  	/* Execlists */
> >>  	bool rcs_initialized;
> >>+	struct intel_ringbuffer *indirect_ctx_wa_bb;
> >>+	struct intel_ringbuffer *per_ctx_wa_bb;
> >
> >Eh? They are only command sequences whose starting addresses you encode
> >into the execlists context. Why have you allocated a ringbuf not an
> >object? Why have you allocated 2 pages when you only need one, and could
> >even find space elsewhere in the context....
> 
> ringbuf is only used so that I can use logical_ring_*(), object can
> also be used.
> Single page is enough but since we have two batch buffers and need
> to provide offsets in two different registers, two pages are used
> for simplifying things, I guess we can manage with single page, I
> will try this.
> 
> Your idea of using space in context itself simplifies many things
> but the context size varies across Gens, is it safe to pick last
> page or increase the size by one more page and use that to load
> these instructions? I think using an additional page is safe to
> avoid the risk of HW overwriting that page or do you have any other
> recommendation? I will first try and see if it works.
> 
> >
> >And these should be pinned alongside the context *not permanently*.
> right, I will correct this but this won't be required if we use the
> space in context.

Indeed, allocating an extra scratch page in the context would simplify
vma/mm management. A trick might be to allocate the scratch page at the
start, then offset the lrc regs etc - that would then be consistent
amongst gen and be easy enough to extend if we need more per-context
scratch space in future.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre