Re: [PATCH] drm/i915: Use true PPGTT in Gen8 if execlists are enabled

From: Daniel Vetter <daniel@ffwll.ch>
To: Michel Thierry <michel.thierry@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Use true PPGTT in Gen8 if execlists are enabled
Date: Mon, 15 Dec 2014 15:28:09 +0100	[thread overview]
Message-ID: <20141215142809.GC27182@phenom.ffwll.local> (raw)
In-Reply-To: <20141215142208.GA27182@phenom.ffwll.local>

On Mon, Dec 15, 2014 at 03:22:08PM +0100, Daniel Vetter wrote:
> On Mon, Dec 15, 2014 at 12:47:14PM +0000, Michel Thierry wrote:
> > On 12/15/2014 10:11 AM, Daniel Vetter wrote:
> > >On Thu, Dec 11, 2014 at 12:07:18PM +0000, Michel Thierry wrote:
> > >>When execlists submission is enabled, try full ppgtt by default.
> > >>
> > >>Note, this patch considers that execlist support has been enabled by
> > >>default on Gen8.
> > >>
> > >>Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> > >>---
> > >>  drivers/gpu/drm/i915/i915_gem_gtt.c | 9 ++++++---
> > >>  1 file changed, 6 insertions(+), 3 deletions(-)
> > >>
> > >>diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > >>index 171f6ea..4ed3904 100644
> > >>--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > >>+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > >>@@ -40,8 +40,8 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
> > >>  	has_aliasing_ppgtt = INTEL_INFO(dev)->gen >= 6;
> > >>  	has_full_ppgtt = INTEL_INFO(dev)->gen >= 7;
> > >>-	if (IS_GEN8(dev))
> > >>-		has_full_ppgtt = false; /* XXX why? */
> > >>+	if (IS_GEN8(dev) && !i915.enable_execlists)
> > >>+		has_full_ppgtt = false; /* Only enforce with execlists */
> > >Imo this has outlived it's usefulness - enable_ppgtt is an unsafe
> > >parameter so everyone setting it themselves gets what they need.
> > >Afair this was just because of the execlist depency on gen8 for ppgtt.
> > >-Daniel
> > Not sure if I'm following you up on this...
> > The aim was to change the default value to _full_ only when execlists are
> > also enable (after Thomas' patch).  In gen8, we don't want to have full
> > ppgtt with legacy ring submission.
> 
> Yeah I gotten confused. The comment is a bit misleading thought, maybe
> "Full ppgtt needs execlist since otherwise the ctx switch can hang"?

Actually I wasnt' confused: gen8+ has full ppgtt, like gen7. It's broken
without execlist though, so what we should do is just change the default
below. Otherwise if a user explicitly sets execlist=0 and ppgtt=2 we won't
obey that, which doesn't make sense - for isolating bugs this might be a
valid testcase (even when we know there's problems). So your patch should
do two things:
- Generally allow full ppgtt on gen8 - the current restriction doesn't
  make sense.
- Switch the default to full ppgtt when execlist is enabled on gen8+, leave
  the default to aliasing/disabled everywhere else as is.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx