Re: [PATCH] drm/i915: Reduce engine->emit_flush() to a single mode parameter

From: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Reduce engine->emit_flush() to a single mode parameter
Date: Thu, 28 Jul 2016 13:03:02 +0300	[thread overview]
Message-ID: <1469700182.3897.50.camel@linux.intel.com> (raw)
In-Reply-To: <20160728083756.GA28802@nuc-i3427.alporthouse.com>

On to, 2016-07-28 at 09:37 +0100, Chris Wilson wrote:
> On Thu, Jul 28, 2016 at 10:11:43AM +0300, Joonas Lahtinen wrote:
> > 
> > On ke, 2016-07-27 at 11:53 +0100, Chris Wilson wrote:
> > > 
> > > @@ -1713,9 +1711,7 @@ static int gen7_mm_switch(struct i915_hw_ppgtt *ppgtt,
> > >  
> > >  	/* XXX: RCS is the only one to auto invalidate the TLBs? */
> > >  	if (engine->id != RCS) {
> > > -		ret = engine->emit_flush(req,
> > > -					 I915_GEM_GPU_DOMAINS,
> > > -					 I915_GEM_GPU_DOMAINS);
> > > +		ret = engine->emit_flush(req, EMIT_INVALIDATE | EMIT_FLUSH);
> > Not quite sure why you want to use a combination of flags vs. _BARRIER
> > at different places?
> Just trying to express the intent of the code. Since this code is
> concerned about TLB loading, I though keeping the INVALIDATE explicit
> was best. Around the MRI we strictly only need a command stream barrier
> and so used BARRIER for documentation (in fact both of these instances
> of barriers before the operation can be proven to be not required, as
> the required barrier is already provided). After the MRI, we can use a
> slightly different barrier than a pure flush/invalidate which might show
> a miniscule difference in a purpose built microbenchmark.

I do not have a strong opinion, guess it is fine either way.

Regards, Joonas

> -Chris
> 
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx