[Mesa-dev] [PATCH 3/4] i965: Make L3 partitioning code only use the current pipeline's shaders.

[Kenneth Graunke]

On Wednesday, February 10, 2016 12:10:41 PM PST Francisco Jerez wrote:
> Kenneth Graunke <kenneth at 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.

That makes a lot of sense.  I've written a patch to do that instead, and
it too solves the problem.  I'll send it out shortly, assuming it passes
the usual regression testing.

--Ken
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160211/7581b008/attachment.sig>