[Mesa-dev] [PATCH] i915: BINDING_TABLE_POINTER_* after CONSTANT_* for SKL & BXT

[Ben Widawsky]

On Wed, Aug 12, 2015 at 03:09:44PM +0300, Joonas Lahtinen wrote:
> Add a comment about reinforcing command order so that
> 3DSTATE_BINDING_TABLE_POINTER_* commands are after
> 3DSTATE_CONSTANT_* commands for SKL & BXT, otherwise the
> GPU might hang.
> 
> Changing the BLORP code is not relevant (where the order
> is "wrong"), as it is not used for GEN8 or up.
> 
> Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> Cc: Arun Siluvery <arun.siluvery at linux.intel.com>
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_state_upload.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c b/src/mesa/drivers/dri/i965/brw_state_upload.c
> index 9de42ce..9078e11 100644
> --- a/src/mesa/drivers/dri/i965/brw_state_upload.c
> +++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
> @@ -299,9 +299,9 @@ static const struct brw_tracked_state *gen8_render_atoms[] =
>     &brw_wm_abo_surfaces,
>     &gen6_renderbuffer_surfaces,
>     &brw_texture_surfaces,
> -   &brw_vs_binding_table,
> -   &brw_gs_binding_table,
> -   &brw_wm_binding_table,
> +   &brw_vs_binding_table, /* Must come after vs_push_constants for Skylake and Broxton. */
> +   &brw_gs_binding_table, /* Must come after gs_push_constants for Skylake and Broxton. */
> +   &brw_wm_binding_table, /* Must come after wm_push_constants for Skylake and Broxton. */
>  
>     &brw_fs_samplers,
>     &brw_vs_samplers,

Does anyone understand why this actually causes a hang on the IGT test? I
certainly don't. The docs are pretty clear that the constant command is not
committed until the BTP command, but I can't make any sense of how it related to
a GPU hang.

In any event, I don't think the comments are super useful, but they're not
harmful either. I'd suggest one line instead:
"NOTE: push_constant_ff must precede binding table pointer upload"

Reviewed-by: Ben Widawsky <ben at bwidawsk.net>