Attachment 125045 Details for Bug 96878 – Proposed fix

[patch] Proposed fix

0001-i965-blorp-gen7-Bring-back-push-constant-setup.patch (text/plain), 4.83 KB, created by Topi Pohjolainen on 2016-07-13 07:12:49 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Topi Pohjolainen

Created: 2016-07-13 07:12:49 UTC

Size: 4.83 KB

patch

obsolete

>From 13a977ee577c516082bc0cd85a48345ac2157bdc Mon Sep 17 00:00:00 2001
>From: Topi Pohjolainen <topi.pohjolainen@intel.com>
>Date: Tue, 12 Jul 2016 22:09:42 +0300
>Subject: [PATCH] i965/blorp/gen7+: Bring back push constant setup
>
>This is partial revert of commit cc2d0e64.
>
>It looks that even though blorp disables a stage the corresponding
>3DSTATE_CONSTANT_XS packet is needed to be programmed. Hardware
>seems to try to fetch the constants even for disabled stages.
>Therefore care needs to be taken that the constant buffer is
>set up properly. Blorp will continue to trash it into non-existing
>such as before.
>It is possible that this could be omitted on SKL where the
>constant buffer is considered when the corresponding binding table
>settings are changed. Bspec:
>
>  "The 3DSTATE_CONSTANT_* command is not committed to the shader
>   unit until the corresponding (same shader)
>   3DSTATE_BINDING_TABLE_POINTER_* command is parsed."
>
>However, as CONSTANT_XS packet itself does not seem to stall on its
>own, it is safer to emit the packets for SKL also.
>
>Possible alternative to blorp trashing could have been to setup
>defaults in the beginning of each batch buffer. However, hardware
>doesn't seem to tolerate these packets being programmed multiple
>times per primitive. Bspec for IVB:
>
>  "It is invalid to execute this command more than once between
>   3D_PRIMITIVE commands."
>
>Signed-off-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96878
>CC: Jason Ekstrand <jason@jlekstrand.net>
>CC: Kenneth Graunke <kenneth@whitecape.org>
>CC: arek.rusi@gmail.com
>CC: awatry@gmail.com
>---
> src/mesa/drivers/dri/i965/gen7_blorp.c | 24 ++++++++++++++++++++++++
> src/mesa/drivers/dri/i965/gen8_blorp.c | 28 ++++++++++++++++++++++++++++
> 2 files changed, 52 insertions(+)
>
>diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.c b/src/mesa/drivers/dri/i965/gen7_blorp.c
>index 0afd76b..dc62a93 100644
>--- a/src/mesa/drivers/dri/i965/gen7_blorp.c
>+++ b/src/mesa/drivers/dri/i965/gen7_blorp.c
>@@ -228,6 +228,24 @@ gen7_blorp_emit_surface_state(struct brw_context *brw,
>    return wm_surf_offset;
> }
> 
>+/* Hardware seems to try to fetch the constants even though the corresponding
>+ * stage gets disables. Therefore make sure the settings for the constant
>+ * buffer are valid.
>+ */
>+static void
>+gen7_blorp_emit_disable_constant_state(struct brw_context *brw,
>+                                       unsigned opcode)
>+{
>+   BEGIN_BATCH(7);
>+   OUT_BATCH(opcode << 16 | (7 - 2));
>+   OUT_BATCH(0);
>+   OUT_BATCH(0);
>+   OUT_BATCH(0);
>+   OUT_BATCH(0);
>+   OUT_BATCH(0);
>+   OUT_BATCH(0);
>+   ADVANCE_BATCH();
>+}
> 
> /* 3DSTATE_VS
>  *
>@@ -760,6 +778,12 @@ gen7_blorp_exec(struct brw_context *brw,
>       gen7_blorp_emit_blend_state_pointer(brw, cc_blend_state_offset);
>       gen7_blorp_emit_cc_state_pointer(brw, cc_state_offset);
>    }
>+
>+   gen7_blorp_emit_disable_constant_state(brw, _3DSTATE_CONSTANT_VS);
>+   gen7_blorp_emit_disable_constant_state(brw, _3DSTATE_CONSTANT_HS);
>+   gen7_blorp_emit_disable_constant_state(brw, _3DSTATE_CONSTANT_DS);
>+   gen7_blorp_emit_disable_constant_state(brw, _3DSTATE_CONSTANT_GS);
>+
>    depthstencil_offset = gen6_blorp_emit_depth_stencil_state(brw, params);
>    gen7_blorp_emit_depth_stencil_state_pointers(brw, depthstencil_offset);
>    if (brw->use_resource_streamer)
>diff --git a/src/mesa/drivers/dri/i965/gen8_blorp.c b/src/mesa/drivers/dri/i965/gen8_blorp.c
>index 7ca24a8..bc113ec 100644
>--- a/src/mesa/drivers/dri/i965/gen8_blorp.c
>+++ b/src/mesa/drivers/dri/i965/gen8_blorp.c
>@@ -156,6 +156,29 @@ gen8_blorp_emit_blend_state(struct brw_context *brw,
>    return blend_state_offset;
> }
> 
>+/* Hardware seems to try to fetch the constants even though the corresponding
>+ * stage gets disables. Therefore make sure the settings for the constant
>+ * buffer are valid.
>+ */
>+static void
>+gen8_blorp_emit_disable_constant_state(struct brw_context *brw,
>+                                       unsigned opcode)
>+{
>+   BEGIN_BATCH(11);
>+   OUT_BATCH(opcode << 16 | (11 - 2));
>+   OUT_BATCH(0);
>+   OUT_BATCH(0);
>+   OUT_BATCH(0);
>+   OUT_BATCH(0);
>+   OUT_BATCH(0);
>+   OUT_BATCH(0);
>+   OUT_BATCH(0);
>+   OUT_BATCH(0);
>+   OUT_BATCH(0);
>+   OUT_BATCH(0);
>+   ADVANCE_BATCH();
>+}
>+
> /* 3DSTATE_VS
>  *
>  * Disable vertex shader.
>@@ -657,6 +680,11 @@ gen8_blorp_exec(struct brw_context *brw, const struct brw_blorp_params *params)
>    const uint32_t cc_state_offset = gen6_blorp_emit_cc_state(brw);
>    gen7_blorp_emit_cc_state_pointer(brw, cc_state_offset);
> 
>+   gen8_blorp_emit_disable_constant_state(brw, _3DSTATE_CONSTANT_VS);
>+   gen8_blorp_emit_disable_constant_state(brw, _3DSTATE_CONSTANT_HS);
>+   gen8_blorp_emit_disable_constant_state(brw, _3DSTATE_CONSTANT_DS);
>+   gen8_blorp_emit_disable_constant_state(brw, _3DSTATE_CONSTANT_GS);
>+
>    gen8_blorp_emit_disable_constant_ps(brw);
>    wm_bind_bo_offset = gen8_blorp_emit_surface_states(brw, params);
> 
>-- 
>2.5.5
>

Actions: View | Splinter Review

Attachments on bug 96878: 124984 | 124985 | 124988 | 125021 | 125022 | 125045

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.