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

From: Daniel Vetter <daniel@ffwll.ch>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Arun Siluvery <arun.siluvery@linux.intel.com>,
	intel-gfx@lists.freedesktop.org,
	Namrta <namrta.salonie@intel.com>
Subject: Re: [PATCH v2 1/7] drm/i915/gen8: Add infrastructure to initialize WA batch buffers
Date: Mon, 15 Jun 2015 12:40:26 +0200	[thread overview]
Message-ID: <20150615104026.GY8341@phenom.ffwll.local> (raw)
In-Reply-To: <20150529181638.GO19710@nuc-i3427.alporthouse.com>

On Fri, May 29, 2015 at 07:16:38PM +0100, 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@intel.com>
> > Signed-off-by: Arun Siluvery <arun.siluvery@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....
> 
> And these should be pinned alongside the context *not permanently*.
> 
> I want a debug mode that limits us to say 16M of GGTT space so that
> these address space leaks are easier to demonstrate in practice.

Yeah the "fix up execlist context to use active list tracking" series
is still pending too ... And artificially limiting ggtt space would indeed
be a neat debug trick. Maybe we'd need to have different limits for
display and other buffers just to avoid making 4k completely unuseable
with this debug knob enabled.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx