From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH v2 3/3] drm/i915: Prevent MI_DISPLAY_FLIP straddling two cachelines on IVB Date: Tue, 11 Feb 2014 17:50:05 +0200 Message-ID: <20140211155005.GV3891@intel.com> References: <1392126950-13225-1-git-send-email-ville.syrjala@linux.intel.com> <1392126950-13225-4-git-send-email-ville.syrjala@linux.intel.com> <20140211141428.GC11275@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id BDA55FA9AD for ; Tue, 11 Feb 2014 07:50:30 -0800 (PST) Content-Disposition: inline In-Reply-To: <20140211141428.GC11275@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 , intel-gfx@lists.freedesktop.org, Enrico Tagliavini , Bjoern C , Alexandru DAMIAN List-Id: intel-gfx@lists.freedesktop.org On Tue, Feb 11, 2014 at 02:14:28PM +0000, Chris Wilson wrote: > On Tue, Feb 11, 2014 at 03:55:50PM +0200, ville.syrjala@linux.intel.com w= rote: > > From: Ville Syrj=E4l=E4 > > = > > According to BSpec the entire MI_DISPLAY_FLIP packet must be contained > > in a single cacheline. Make sure that happens. > > = > > v2: Use intel_ring_begin_cacheline_safe() > = > Ugh, no. Let's not make intel_ring_begin() any more complicated and just > introduce a function to align the current head in the ringbuffer to a > cacheline. Especially with such an interface that is hard to get right. This doesn't make intel_ring_begin() itself more complicated, but I guess you meant that the new special version is too complicated for your taste? So I guess you want somehting like this: int ring_align() { nops =3D (64 - (tail & 63)) / 4; ret =3D ring_begin(nops); if (ret) return ret; while (nops--) ring_emit(MI_NOOP); ring_advance() return 0; } int queue_flip() { ret =3D ring_align(); if (ret) return ret; ret =3D ring_begin(len); if (ret) return ret; if (RCS) emit LRI DERRMR; emit MI_DISPLAY_FLIP; ring_advance() return 0; } So we end up relying on the fact that the entire LRI+MI_DISPLAY_FLIP sequence will fit within one cacheline. Although if that would be problem I suppose we could always emit the LRI before aligning the tail. If no one is concerned about the useless MI_NOOPs we'll be emitting for most flips, I guess it's a good enough solution. -- = Ville Syrj=E4l=E4 Intel OTC