Kenneth Graunke <kenn...@whitecape.org> writes: > When uploading state for the compute pipeline, we don't want to > look at VS/TCS/TES/GS/FS programs, as they might be stale, and > aren't relevant anyway. Likewise, the render pipeline shouldn't > look at CS.
The intended behaviour of this function is to look at the whole pipeline state. The reason is that reprogramming the L3 is expensive and selecting an L3 configuration that works for the whole pipeline avoids an unnecessary L3 reprogramming on pipeline select if the bound shader programs didn't change (or changed to a different program with the similar L3 requirements). Regardless of its performance implications, it doesn't seem correct to do this unless you can guarantee that the pipeline state is evaluated again after pipeline select, which I don't think is the case right now. Anyway it seems like this is hiding a bug elsewhere. It smells rather fishy that the brw_context structure can suddenly lose its consistency anytime. brw_state_cache_check_size() frees the stage prog data structures pointed to by the context and then leaves dangling pointers pointing at the deallocated storage -- that seems like the real bug to me. You need to set the prog data pointers to NULL when they become invalid. If you do that get_pipeline_state_l3_weights() won't look at them until the program switches back to the pipeline they belong to, at which point the invalidated prog data structures will be regenerated and BRW_NEW_XS_PROG_DATA will be signaled causing the L3 configuration to be reevaluated. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93790 > Signed-off-by: Kenneth Graunke <kenn...@whitecape.org> > Reviewed-by: Jordan Justen <jordan.l.jus...@intel.com> > --- > src/mesa/drivers/dri/i965/gen7_l3_state.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/src/mesa/drivers/dri/i965/gen7_l3_state.c > b/src/mesa/drivers/dri/i965/gen7_l3_state.c > index c4babc2..b2e9306 100644 > --- a/src/mesa/drivers/dri/i965/gen7_l3_state.c > +++ b/src/mesa/drivers/dri/i965/gen7_l3_state.c > @@ -307,7 +307,12 @@ get_pipeline_state_l3_weights(const struct brw_context > *brw) > }; > bool needs_dc = false, needs_slm = false; > > - for (unsigned i = 0; i < ARRAY_SIZE(stage_states); i++) { > + unsigned first_stage = MESA_SHADER_VERTEX; > + unsigned last_stage = MESA_SHADER_FRAGMENT; > + if (brw->last_pipeline == BRW_COMPUTE_PIPELINE) > + first_stage = last_stage = MESA_SHADER_COMPUTE; > + > + for (unsigned i = first_stage; i <= last_stage; i++) { > const struct gl_shader_program *prog = > brw->ctx._Shader->CurrentProgram[stage_states[i]->stage]; > const struct brw_stage_prog_data *prog_data = > stage_states[i]->prog_data; > -- > 2.7.0 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev