<div dir="ltr">On 9 September 2013 17:56, Kenneth Graunke <span dir="ltr"><<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">

<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div>On 09/09/2013 08:20 AM, Paul Berry wrote:<br>
> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h<br>
> index 0406c4d..6db2570 100644<br>
> --- a/src/mesa/drivers/dri/i965/brw_defines.h<br>
> +++ b/src/mesa/drivers/dri/i965/brw_defines.h<br>
> @@ -1337,6 +1337,10 @@ enum brw_message_target {<br>
>  /* DW5 */<br>
>  # define GEN6_GS_MAX_THREADS_SHIFT                   25<br>
>  # define HSW_GS_MAX_THREADS_SHIFT                    24<br>
> +# define GEN7_GS_CONTROL_DATA_FORMAT_SHIFT           24<br>
> +# define GEN7_GS_CONTROL_DATA_FORMAT_GSCTL_CUT               0<br>
> +# define GEN7_GS_CONTROL_DATA_FORMAT_GSCTL_SID               1<br>
> +# define GEN7_GS_CONTROL_DATA_HEADER_SIZE_SHIFT              20<br>
<br>
</div></div>This won't work for Haswell (note the overlap GSCTL and MAX_THREADS).<br>
<br>
Apparently GSCTL is stored in DW6 at bit 31 on Haswell.<br>
<br>
I think it probably makes sense to address that in this patch.<br>
<br>
Otherwise this looks fine.<br></blockquote><div><br></div><div>Oh, wow, I completely screwed this up.  Not only did I fail to notice that there was an IVB/HSW difference, but when I used the #define over in gen7_gs_state.c, I applied the shift to dw6 instead of dw5.  It's only by dumb luck that it worked at all even on Ivy Bridge.<br>

<br></div><div>I've changed the #defines to <br><br>/* DW5 */<br># define GEN6_GS_MAX_THREADS_SHIFT            25<br># define HSW_GS_MAX_THREADS_SHIFT            24<br># define IVB_GS_CONTROL_DATA_FORMAT_SHIFT        24<br>

# define GEN7_GS_CONTROL_DATA_FORMAT_GSCTL_CUT        0<br># define GEN7_GS_CONTROL_DATA_FORMAT_GSCTL_SID        1<br># define GEN7_GS_CONTROL_DATA_HEADER_SIZE_SHIFT        20<br>...<br>/* DW6 */<br># define HSW_GS_CONTROL_DATA_FORMAT_SHIFT        31<br>

<br></div><div>and the code in gen7_gs_state.c to use temporaries, like we do in a lot of other state commands:<br><br>      uint32_t dw5 =<br>         ((brw->gs.prog_data->output_vertex_size_hwords * 2 - 1) <<<br>

          GEN7_GS_OUTPUT_VERTEX_SIZE_SHIFT) |<br>         (brw->gs.prog_data->output_topology <<<br>          GEN7_GS_OUTPUT_TOPOLOGY_SHIFT) |<br>         (prog_data->urb_read_length <<<br>          GEN6_GS_URB_READ_LENGTH_SHIFT) |<br>

         (0 << GEN6_GS_URB_ENTRY_READ_OFFSET_SHIFT) |<br>         (prog_data->dispatch_grf_start_reg <<<br>          GEN6_GS_DISPATCH_START_GRF_SHIFT);<br>      uint32_t dw6 =<br>         ((brw->max_gs_threads - 1) << max_threads_shift) |<br>

         (brw->gs.prog_data->control_data_header_size_hwords <<<br>          GEN7_GS_CONTROL_DATA_HEADER_SIZE_SHIFT) |<br>         GEN7_GS_DISPATCH_MODE_DUAL_OBJECT |<br>         GEN6_GS_STATISTICS_ENABLE |<br>

         GEN7_GS_ENABLE;<br><br>      if (brw->is_haswell) {<br>         dw6 |= brw->gs.prog_data->control_data_format <<<br>            HSW_GS_CONTROL_DATA_FORMAT_SHIFT;<br>      } else {<br>         dw5 |= brw->gs.prog_data->control_data_format <<<br>

            IVB_GS_CONTROL_DATA_FORMAT_SHIFT;<br>      }<br><br>      OUT_BATCH(dw5);<br>      OUT_BATCH(dw6);<br><br></div><div>Thanks for catching this.<br></div></div></div></div>