From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kenneth Graunke Subject: Re: [Mesa-dev] [PATCH] drm/i915: Enable the HiZ RAW Stall Optimization on Gen8. Date: Sun, 11 Jan 2015 18:53:32 -0800 Message-ID: <2487831.m4DGgTKFrR@vakarian> References: <1420944289-832-1-git-send-email-kenneth@whitecape.org> <3086167.N74lbW7sKX@vakarian> <20150112014609.GA1677@bwidawsk.net> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0710208481==" Return-path: In-Reply-To: <20150112014609.GA1677@bwidawsk.net> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Ben Widawsky Cc: mesa-dev@lists.freedesktop.org, intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============0710208481== Content-Type: multipart/signed; boundary="nextPart9812382.fT62jnzS7u"; micalg="pgp-sha256"; protocol="application/pgp-signature" --nextPart9812382.fT62jnzS7u Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="us-ascii" On Sunday, January 11, 2015 05:46:09 PM Ben Widawsky wrote: > On Sun, Jan 11, 2015 at 04:05:25PM -0800, Kenneth Graunke wrote: > > On Sunday, January 11, 2015 01:49:41 PM Ben Widawsky wrote: > > > On Sat, Jan 10, 2015 at 06:44:49PM -0800, Kenneth Graunke wrote: > > > > This is an important optimization for avoiding read-after-write= (RAW) > > > > stalls in the HiZ buffer. Certain workloads would run very slo= wly with > > > > HiZ enabled, but run much faster with the "hiz=3Dfalse" driconf= option. > > > > With this patch, they run at full speed even with HiZ. > > > >=20 > > > > Improves performance in OglVSInstancing by 3.2x on Broadwell GT= 3e > > > > (Iris Pro 6200). > > > >=20 > > > > Thanks to Jesse Barnes for finding this missing bit! > > > > Thanks to Chris Wilson for helping me find where to set it. > > > >=20 > > > > Signed-off-by: Kenneth Graunke > > > > Cc: Jesse Barnes > > > > --- > > > > drivers/gpu/drm/i915/intel_ringbuffer.c | 15 +++++++++++++++ > > > > 1 file changed, 15 insertions(+) > > > >=20 > > > > Here's an alternate patch which implements the workaround in th= e kernel > > > > instead of Mesa. It's probably better to do it there, since th= e kernel > > > > does it on Haswell already. > > > >=20 > > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/= gpu/drm/i915/intel_ringbuffer.c > > > > index dabc1d8..23020d6 100644 > > > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > > > @@ -796,6 +796,16 @@ static int bdw_init_workarounds(struct int= el_engine_cs *ring) > > > > =09=09=09 HDC_DONOT_FETCH_MEM_WHEN_MASKED | > > > > =09=09=09 (IS_BDW_GT3(dev) ? HDC_FENCE_DEST_SLM_DISABLE : 0))= ; > > > > =20 > > > > +=09/* From the Haswell PRM, Command Reference: Registers, CACH= E_MODE_0: > > > > +=09 * "The Hierarchical Z RAW Stall Optimization allows non-ov= erlapping > > > > +=09 * polygons in the same 8x4 pixel/sample area to be proces= sed without > > > > +=09 * stalling waiting for the earlier ones to write to Hiera= rchical Z > > > > +=09 * buffer." > > > > +=09 * > > > > +=09 * This optimization is off by default for Broadwell; turn = it on. > > > > +=09 */ > > > > +=09WA_CLR_BIT_MASKED(CACHE_MODE_0_GEN7, HIZ_RAW_STALL_OPT_DISA= BLE); > > > > + > > > > =09/* Wa4x4STCOptimizationDisable:bdw */ > > > > =09WA_SET_BIT_MASKED(CACHE_MODE_1, > > > > =09=09=09 GEN8_4x4_STC_OPTIMIZATION_DISABLE); > > > > @@ -836,6 +846,11 @@ static int chv_init_workarounds(struct int= el_engine_cs *ring) > > > > =09=09=09 HDC_FORCE_NON_COHERENT | > > > > =09=09=09 HDC_DONOT_FETCH_MEM_WHEN_MASKED); > > > > =20 > > > > +=09/* According to the CACHE_MODE_0 default value documentatio= n, some > > > > +=09 * CHV platforms disable this optimization by default. Tur= n it on. > > > > +=09 */ > > > > +=09WA_CLR_BIT_MASKED(CACHE_MODE_0_GEN7, HIZ_RAW_STALL_OPT_DISA= BLE); > > > > + > > > > =09/* Improve HiZ throughput on CHV. */ > > > > =09WA_SET_BIT_MASKED(HIZ_CHICKEN, CHV_HZ_8X8_MODE_IN_1X); > > > > =20 > > >=20 > > > I think you should do this as two separate patches, 1 per platfor= m. For the BSW > > > patch (given that I had the same functionality in the kernel patc= h I asked you > > > to look at ;-) and FWIW, Jordan has numbers on BSW B-step with my= kernel patch > > > which we can use for the commit): > > > Signed-off-by: Ben Widawsky > >=20 > > Huh, I don't recall seeing that kernel patch. Sorry. I guess I'll= split it > > and resubmit... > >=20 >=20 > It's not my call, it's just nice to have platform specific bisection.= And the > patch wasn't on the list, it was the one I kept asking you to look at= in my > branch :-) > > > > I haven't looked at Broadwell docs, so I'll let someone else take= care of that. > > >=20 > > > I don't know if I agree with Chris that we should call these in t= he workaround > > > section, but whatever. init_clock_gating is equally sucky. > >=20 > > init_clock_gating doesn't work. The register writes don't stick an= d they have > > no effect at all. Setting them here makes them actually take effec= t in the > > context. > >=20 > > --Ken >=20 > Separate thread now, but are you sure? We're setting at least two con= text > specific registers in there today, among them: GEN7_FF_THREAD_MODE (w= hich is > important to performance). >=20 > AFAIK it should stick, and if it doesn't it's not expected behavior. = Unless you > know something I do not? Jesse had suggested setting it in broadwell_init_clock_gating on Januar= y 5th, and Valtteri tried it on January 7th. He found "no noticeable differen= ce". I tried it again, and confirmed his result: there was zero performance = impact. Setting it via an LRI in Mesa did have a performance impact. I reverte= d my Mesa patch, and tried setting it here, and it had the same performance = impact. I rebooted between kernels several times to confirm. It works here, bu= t it doesn't there. I'm pretty sure I confirmed the same result with this bit. Feel free t= o try. Perhaps we should move the rest of the per-context bits here instead of= *_init_clock_gating. We should also confirm that the other bits are ac= tually having an effect. I don't know why it works on Haswell, but it does there - the HiZ RAW s= tall bit is set via haswell_init_clock_gating, and it's clearly having an im= pact. Maybe it has something to do with the golden context, which is new on B= DW. But I'm probably wrong about that. Setting it when a context is active= does seem more reliable... =2D-Ken --nextPart9812382.fT62jnzS7u Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part. Content-Transfer-Encoding: 7Bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJUszcsAAoJEFtb2gcdScw4MioQAI9FO+evSGStei8L84cJp1S2 KKCzF7tBFhfpU9Ep+Gf+76N2ylSIeFzESHlDNd5xlECCkG9H3njXKNZvwrJoSB3D KExtfOKhqRedNE5nxgj37ftA3LBZL0V8tFq2sB0s5fmtGT7dNv2y/+ICMrEHLLLw z3vY1taH8sVIYaGWqNYZ6qQVOHkI+3ntOjJSXRkPMNXB0fCSDU5DRfsd645lBRoX sFaYFIr+4d/+caUQ7j8X3mxx3xv71+T1Gp+/yl4aCNtGuGq2vX0QM7FcvrDrHKNx /Y5DSgwnuwIesnhENuBq/GAA12Lrkobfft5oO3t6x3zUlIf5ncHp1P+YlvMjEjbh D2mn5kF4XReroOPGrOaoYCKzciYEOvWofBF5WhHsONnTXi1jbyrQNbFz0DDgaqIh rXsa4PbAgxXPEq0V1Xs+ZI+P1qMdoI7DSSIMQA3tfhDyGsco3gk8IEMWmQ6Z1b4l NB0T2a81y+YG6OA58iub1uW7ABn5beywoMikEZAj1yTA5FVQ9tjRQpnzkpJT2jzo jcLCJlBS2CHqbi03aCdVK+KOHlrdO+EtgfNrkBZPa+CN7Ad8Cv7MtUP5TfuV/vXc WfZeQYB+QTuHmIHgiJBOeer6dNcO7YsApK4fCWDDE5P98YKkFUpCz+sVcBLlfPmG rEhnMXcPFYhzoQtDm0n3 =PNGH -----END PGP SIGNATURE----- --nextPart9812382.fT62jnzS7u-- --===============0710208481== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK --===============0710208481==--