<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Oct 28, 2013 at 6:48 PM, Paul Berry <span dir="ltr"><<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div><div class="h5">On 25 October 2013 16:45, Anuj Phogat <span dir="ltr"><<a href="mailto:anuj.phogat@gmail.com" target="_blank">anuj.phogat@gmail.com</a>></span> wrote:<br>
</div></div><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">- Enable GEN7_WM_MSDISPMODE_PERSAMPLE, GEN7_WM_POSOFFSET_SAMPLE,<br>
GEN7_WM_OMASK_TO_RENDER_TARGET as per extension's specification.<br>
- Only enable one of GEN7_WM_8_DISPATCH_ENABLE or GEN7_WM_16_DISPATCH_ENABLE<br>
when GEN7_WM_MSDISPMODE_PERSAMPLE is enabled. Refer IVB PRM Vol. 2, Part 1,<br>
Page 288 for details.<br>
<br>
V2: - Add a comment explaining why only SIMD8 mode is enabled with<br>
MSDISPMODE_PERSAMPLE.<br>
- Use shared function _mesa_get_min_invocations_per_fragment().<br>
- Use brw_wm_prog_data variables: uses_pos_offset, uses_omask.<br>
Signed-off-by: Anuj Phogat <<a href="mailto:anuj.phogat@gmail.com" target="_blank">anuj.phogat@gmail.com</a>><br>
---<br>
src/mesa/drivers/dri/i965/gen7_wm_state.c | 53 +++++++++++++++++++++++++++++--<br>
1 file changed, 50 insertions(+), 3 deletions(-)<br>
<br>
diff --git a/src/mesa/drivers/dri/i965/gen7_wm_state.c b/src/mesa/drivers/dri/i965/gen7_wm_state.c<br>
index a2046c3..493b107 100644<br>
--- a/src/mesa/drivers/dri/i965/gen7_wm_state.c<br>
+++ b/src/mesa/drivers/dri/i965/gen7_wm_state.c<br>
@@ -27,6 +27,7 @@<br>
#include "brw_defines.h"<br>
#include "brw_util.h"<br>
#include "brw_wm.h"<br>
+#include "program/program.h"<br>
#include "program/prog_parameter.h"<br>
#include "program/prog_statevars.h"<br>
#include "intel_batchbuffer.h"<br>
@@ -82,9 +83,13 @@ upload_wm_state(struct brw_context *brw)<br>
GEN7_WM_BARYCENTRIC_INTERPOLATION_MODE_SHIFT;<br>
<br>
/* _NEW_COLOR, _NEW_MULTISAMPLE */<br>
+ /* Enable if the pixel shader kernel generates and outputs oMask.<br>
+ */<br>
if (fp->program.UsesKill || ctx->Color.AlphaEnabled ||<br>
- ctx->Multisample.SampleAlphaToCoverage)<br>
+ ctx->Multisample.SampleAlphaToCoverage ||<br>
+ brw->wm.prog_data->uses_omask) {<br>
dw1 |= GEN7_WM_KILL_ENABLE;<br>
+ }<br>
<br>
/* _NEW_BUFFERS */<br>
if (brw_color_buffer_write_enabled(brw) || writes_depth ||<br>
@@ -97,7 +102,11 @@ upload_wm_state(struct brw_context *brw)<br>
dw1 |= GEN7_WM_MSRAST_ON_PATTERN;<br>
else<br>
dw1 |= GEN7_WM_MSRAST_OFF_PIXEL;<br>
- dw2 |= GEN7_WM_MSDISPMODE_PERPIXEL;<br>
+<br>
+ if (_mesa_get_min_invocations_per_fragment(ctx, brw->fragment_program) > 1)<br>
+ dw2 |= GEN7_WM_MSDISPMODE_PERSAMPLE;<br>
+ else<br>
+ dw2 |= GEN7_WM_MSDISPMODE_PERPIXEL;<br>
} else {<br>
dw1 |= GEN7_WM_MSRAST_OFF_PIXEL;<br>
dw2 |= GEN7_WM_MSDISPMODE_PERSAMPLE;<br>
@@ -169,6 +178,32 @@ upload_ps_state(struct brw_context *brw)<br>
if (brw->wm.prog_data->nr_params > 0)<br>
dw4 |= GEN7_PS_PUSH_CONSTANT_ENABLE;<br>
<br>
+ /* From the IVB PRM, volume 2 part 1, page 287:<br>
+ * "This bit is inserted in the PS payload header and made available to<br>
+ * the DataPort (either via the message header or via header bypass) to<br>
+ * indicate that oMask data (one or two phases) is included in Render<br>
+ * Target Write messages. If present, the oMask data is used to mask off<br>
+ * samples."<br>
+ */<br>
+ if (brw->wm.prog_data->uses_omask)<br>
+ dw4 |= GEN7_PS_OMASK_TO_RENDER_TARGET;<br>
+<br>
+ /* From the IVB PRM, volume 2 part 1, page 287:<br>
+ * "If the PS kernel does not need the Position XY Offsets to<br>
+ * compute a Position Value, then this field should be programmed<br>
+ * to POSOFFSET_NONE."<br>
+ * "SW Recommendation: If the PS kernel needs the Position Offsets<br>
+ * to compute a Position XY value, this field should match Position<br>
+ * ZW Interpolation Mode to ensure a consistent position.xyzw<br>
+ * computation."<br>
+ * We only require XY sample offsets. So, this recommendation doesn't<br>
+ * look useful at the moment. We might need this in future.<br>
+ */<br>
+ if (brw->wm.prog_data->uses_pos_offset)<br>
+ dw4 |= GEN7_PS_POSOFFSET_SAMPLE;<br>
+ else<br>
+ dw4 |= GEN7_PS_POSOFFSET_NONE;<br>
+<br>
/* CACHE_NEW_WM_PROG | _NEW_COLOR<br>
*<br>
* The hardware wedges if you have this bit set but don't turn on any dual<br>
@@ -184,8 +219,20 @@ upload_ps_state(struct brw_context *brw)<br>
if (brw->wm.prog_data->num_varying_inputs != 0)<br>
dw4 |= GEN7_PS_ATTRIBUTE_ENABLE;<br>
<br>
+ /* In case of non 1x (i.e 4x, 8x) multisampling with MSDISPMODE_PERSAMPLE,<br>
+ * only one of SIMD8 and SIMD16 should be enabled. So, we have two options<br>
+ * in that case:<br>
+ * 'SIMD8 only' dispatch: allowed on gen7.<br>
+ * 'SIMD16 only' dispatch: allowed on gen7 except when in PERSAMPLE mode<br>
+ * with number of multisamples >= 8.<br>
+ * TODO: Currently we enable 'SIMD8 only' dispatch in above mentioned case.<br>
+ * Make changes to allow 'SIMD16 only' dispatch for multisamples < 8.<br></blockquote><div><br></div></div></div><div>I believe this is incorrect (see my comment on patch 10/12). On Gen7, we can enable SIMD8 or SIMD16 or both. (I'm fairly certain about this because blorp enables SIMD16 mode with 8x PERSAMPLE dispatch, and it's been working fine since May 2012). </div>
</div></div></div></blockquote><div style>We had this discussion on my earlier patch for Gen6. We reached a conclusion that on Gen7 with non 1x persample, we can enable either SIMD8 or SIMD16.</div><div style><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> The only reason we are not enabling SIMD16 yet is because you had some concerns that the code generation for SIMD16 might not be correct.<br></div></div>
</div></div></blockquote><div style>Right. I am also getting GPU hangs with SIMD16 only dispatch. I'll look in to it after pushing this series.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>
<br>I think the right way to deal with this is to drop this coment (and the remainder of the patch), and instead, disable SIMD16 in the same way we do for the other features that we don't have SIMD16 support for yet: by calling fail() in brw_fs_visitor.cpp.<br>
<br></div><div>That way (a) we won't generate a 16-wide shader that will never get used, and (b) later, when we go looking for reasons why SIMD16 shaders aren't getting generated, we will only have to consider one reason (shaders failing SIMD16 compilation) instead of two.<br>
</div></div></div></div></blockquote><div style>sounds better. I'll call fail() if (dispatch_width == 16 && _mesa_get_min_invocations_per_fragment(ctx, fp) > 1)</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>
<br></div><div>With that change, this patch is:<br><br>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a>><br></div><div class="im"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+ */<br>
dw4 |= GEN7_PS_8_DISPATCH_ENABLE;<br>
- if (brw->wm.prog_data->prog_offset_16)<br>
+<br>
+ if (brw->wm.prog_data->prog_offset_16 &&<br>
+ !(_mesa_get_min_invocations_per_fragment(ctx,<br>
+ brw->fragment_program ) > 1))<br>
dw4 |= GEN7_PS_16_DISPATCH_ENABLE;<br>
<br>
dw5 |= (brw->wm.prog_data->first_curbe_grf <<<br>
<span><font color="#888888">--<br>
1.8.1.4<br>
<br>
</font></span></blockquote></div></div><br></div></div>
</blockquote></div><br></div></div>