From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH 06/13] drm/i915/bdw: implement semaphore signal Date: Tue, 11 Feb 2014 14:25:43 -0800 Message-ID: <20140211222543.GE2219@intel.com> References: <1391025333-31587-1-git-send-email-benjamin.widawsky@intel.com> <1391025333-31587-7-git-send-email-benjamin.widawsky@intel.com> <20140130123817.GJ9454@intel.com> <20140130124658.GG29091@nuc-i3427.alporthouse.com> <20140130133541.GK29091@nuc-i3427.alporthouse.com> <20140211214821.GA2219@bwidawsk.net> <20140211222338.GA5298@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-pd0-f175.google.com (mail-pd0-f175.google.com [209.85.192.175]) by gabe.freedesktop.org (Postfix) with ESMTP id E28AEFAB62 for ; Tue, 11 Feb 2014 14:25:46 -0800 (PST) Received: by mail-pd0-f175.google.com with SMTP id w10so8058032pde.6 for ; Tue, 11 Feb 2014 14:25:46 -0800 (PST) Content-Disposition: inline In-Reply-To: <20140211222338.GA5298@nuc-i3427.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Chris Wilson , Ben Widawsky , Daniel Vetter , Ville =?utf-8?B?U3lyasOkbMOk?= , Intel GFX List-Id: intel-gfx@lists.freedesktop.org On Tue, Feb 11, 2014 at 10:23:38PM +0000, Chris Wilson wrote: > On Tue, Feb 11, 2014 at 01:48:22PM -0800, Ben Widawsky wrote: > > On Thu, Jan 30, 2014 at 01:35:41PM +0000, Chris Wilson wrote: > > > On Thu, Jan 30, 2014 at 02:18:32PM +0100, Daniel Vetter wrote: > > > > On Thu, Jan 30, 2014 at 1:46 PM, Chris Wilson wrote: > > > > > Oh. So they changed how post-sync writes operated - this should be a > > > > > separate fix for stable I believe (so that batches are not run before we > > > > > have finished invalidating the TLBs required). > > > > > > > > We have an igt to exercise tlb invalidation stuff, which runs on all > > > > rings. But it only runs a batch, so only uses the CS tlb. Do we need > > > > to extend this? > > > > > > So the spec says: > > > > > > Pipe Control Flush Enable (IVB+) > > > If ENABLED, the PIPE_CONTROL command will wait until all previous writes > > > of immediate data from post sync circles are complete before executing > > > the next command. > > > > > > Post Sync Operation > > > This field specifies an optional action to be taken upon completion of > > > the synchronization operation. > > > > > > TLB Invalidate > > > If ENABLED, all TLBs belonging to Render Engine will be invalidated once > > > the flush operation is complete. > > > > > > Command Streamer Stall Enable > > > If ENABLED, the sync operation will not occur until all previous flush > > > operations pending a completion of those previous flushes will complete, > > > including the flush produced from this command. This enables the command > > > to act similar to the legacy MI_FLUSH command. > > > > > > Going by that, the order is > > > > > > flush, stall, TLB invalidate / post-sync op, [pipe control flush] > > > > > > Based on my reading of the above (which unless someone has a more > > > definitive source) says that without the CONTROL_FLUSH_ENABLE, the CS > > > can continue operations as soon as the flush is complete - in parallel > > > to the TLB invalidate. Adding CONTROL_FLUSH_ENABLE would then stall the > > > CS until the post-sync operation completes. That still leaves the > > > possibility that the TLB invalidate is being performed in parallel and > > > is itself provides no CS sync. > > > -Chris > > > > > > -- > > > Chris Wilson, Intel Open Source Technology Centre > > > > so.... what the verdict? > > Gut feeling is that it fixes an issue with IVB TLB invalidate. > (Not yet sure if the bug I was looking at was accidentally fixed at the > same time as testing this.) > So cc stable@ > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre You still want a separate patch? -- Ben Widawsky, Intel Open Source Technology Center