Hi, On ke, 2015-08-12 at 18:34 -0700, Ben Widawsky wrote: > 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 <[email protected]> > > Cc: Arun Siluvery <[email protected]> > > Signed-off-by: Joonas Lahtinen <[email protected]> > > --- > > 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. >
Discussion about this continued in the driver list. > 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" > The table previously seemed to contain per-line comments for other ordering restrictions, so I just went with style that looks consitent with the rest. Also it makes some sense, as it's only the respective 3DSTATE_CONSTANT_* whose parsing is triggered by matching a 3DSTATE_BINDING_TABLE_POINTER_* command. They could be interleaved too. I'll correct the s/i915/i965/ as noted by Matt. How about the comments? Regards, Joonas > Reviewed-by: Ben Widawsky <[email protected]> _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
