Kenneth Graunke <kenn...@whitecape.org> writes: > The L3 partitioning code tries to look at all programs - both render > programs (VS/TCS/TES/GS/FS) and compute (CS). > > After calling brw_clear_cache, all prog_data pointers are invalid and > point to freed data. The intention was that flagging the dirty bits for > all programs would cause the next draw call to re-run the atoms for each > program stage, uploading new programs and installing new, valid pointers. > > However, this doesn't quite work in our new multi-pipeline world. When > drawing or dispatching a compute workload, we only consider the programs > for the appropriate pipeline: drawing sets up VS/TCS/TES/GS/FS, but not > CS, and vice versa. This leaves pointers dangling a bit longer than > intended. > > The L3 configuration code tries to inspect the prog_data for all shader > stages, so that we avoid having to reconfigure it when swapping back and > forth between render and compute workloads. So we can't have dangling > pointers. > > The fix is simple: have brw_clear_cache NULL out stale prog_data > pointers, making it safe to inspect. The next L3 configuration pass > will see either the render shaders or compute shader as missing for > one go around, but will pick them up when both pipelines have run. > > In other words, we'll simply reconfigure L3 twice, which is safe, > if a tiny bit wasteful - but then again, we just threw every compiled > shader we had on the floor and started recompiling the from scratch, > which is massively more wasteful, so it's not much of a concern. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93790 > Signed-off-by: Kenneth Graunke <kenn...@whitecape.org> > Cc: Francisco Jerez <curroje...@riseup.net> > Cc: Jordan Justen <jljus...@gmail.com> > --- > src/mesa/drivers/dri/i965/brw_state_cache.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/src/mesa/drivers/dri/i965/brw_state_cache.c > b/src/mesa/drivers/dri/i965/brw_state_cache.c > index ce178aa..c6aa134 100644 > --- a/src/mesa/drivers/dri/i965/brw_state_cache.c > +++ b/src/mesa/drivers/dri/i965/brw_state_cache.c > @@ -393,6 +393,21 @@ brw_clear_cache(struct brw_context *brw, struct > brw_cache *cache) > brw->state.pipelines[BRW_RENDER_PIPELINE].brw = ~0ull; > brw->state.pipelines[BRW_COMPUTE_PIPELINE].mesa = ~0; > brw->state.pipelines[BRW_COMPUTE_PIPELINE].brw = ~0ull; > + > + /* Also, NULL out any stale program pointers. */ > + brw->vs.prog_data = NULL; > + brw->vs.base.prog_data = NULL; > + brw->tcs.prog_data = NULL; > + brw->tcs.base.prog_data = NULL; > + brw->tes.prog_data = NULL; > + brw->tes.base.prog_data = NULL; > + brw->gs.prog_data = NULL; > + brw->gs.base.prog_data = NULL; > + brw->wm.prog_data = NULL; > + brw->wm.base.prog_data = NULL; > + brw->cs.prog_data = NULL; > + brw->cs.base.prog_data = NULL; > +
Thanks, this seems far more convincing, Reviewed-by: Francisco Jerez <curroje...@riseup.net> > intel_batchbuffer_flush(brw); > } > > -- > 2.7.0
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
