Re: [Intel-gfx] [PATCH] drm/i915: Unconditionally flush writes before execbuffer

From: Daniel Vetter <daniel@ffwll.ch>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Daniel Vetter <daniel@ffwll.ch>,
	intel-gfx <intel-gfx@lists.freedesktop.org>,
	Akash Goel <akash.goel@intel.com>,
	stable <stable@vger.kernel.org>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Unconditionally flush writes before execbuffer
Date: Tue, 26 May 2015 10:00:41 +0200	[thread overview]
Message-ID: <CAKMK7uE42P4BgVywBKwwLBg8GmMHtfMgwPpQCmDhpgVGTarvfA@mail.gmail.com> (raw)
In-Reply-To: <20150521153032.GI15256@phenom.ffwll.local>

On Thu, May 21, 2015 at 5:30 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, May 21, 2015 at 04:22:55PM +0100, Chris Wilson wrote:
>> On Thu, May 21, 2015 at 04:21:46PM +0200, Daniel Vetter wrote:
>> > Hm right. What about emphasising this a bit more in the comment:
>> >
>> >     /*
>> >      * Empirical evidence indicates that we need a write barrier to
>> >      * make sure write-combined writes (both to the gtt, but also to
>> >      * the cpu mmaps). But userspace also uses wc mmaps as
>> >      * unsynchronized upload paths where it inform the kernel about
>> >      * domain changes (to avoid the stalls). Hence we must do this
>> >      * barrier unconditinally.
>> >      */
>>
>> For reference the wording in the commit is:
>>
>> /* Unconditionally flush out writes to memory as the user may be
>>  * doing asynchronous streaming writes to active buffers (i.e.
>>  * lazy domain management to avoid serialisation) directly into
>>  * the physical pages and so not naturally serialised by the GTT.
>>  */
>>
>> > Mostly just rewording, unsing unsynchronized as used by gl/libdrm and
>> > clarification why we need to have the barrier unconditionally. With that
>>
>> Hmm, glMapBufferRange does use unsynchronized, but async is almost
>> universally preferred when talking about io and runqueues.
>>
>> /* Unconditionally flush out writes to memory as the user may be
>>  * doing asynchronous streaming writes to active buffers in this
>>  * batch (i.e. lazy domain management to avoid serialisation, for
>>  * example with glMapBufferRange(GL_MAP_UNSYNCHRONIZED_BIT)) directly
>>  * into the physical pages and so not naturally serialised by the GTT.
>>  */
>>
>> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Yeah, r-b: me also with this one. Jani, can you please exchange the
> comment and apply to -fixes?

I retract my r-b for now, this seems to be a much bigger can of worms
- it seems like we need to make all the mb()/wmb() we have all over
the place unconditional.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch