[Mesa-dev] [PATCH V6 10/27] i965: fix atomic buffer index for bindings other than 0

[Timothy Arceri]

On Tue, 2015-09-29 at 02:55 -0400, Ilia Mirkin wrote:
> On Tue, Sep 29, 2015 at 2:48 AM, Timothy Arceri <
> t_arceri at yahoo.com.au> wrote:
> > On Tue, 2015-09-29 at 02:33 -0400, Ilia Mirkin wrote:
> > > On Tue, Sep 29, 2015 at 2:26 AM, Timothy Arceri <
> > > t_arceri at yahoo.com.au> wrote:
> > > > On Tue, 2015-09-29 at 02:08 -0400, Ilia Mirkin wrote:
> > > > > On Tue, Sep 29, 2015 at 2:05 AM, Timothy Arceri <
> > > > > t_arceri at yahoo.com.au> wrote:
> > > > > > On Tue, 2015-09-29 at 01:05 -0400, Ilia Mirkin wrote:
> > > > > No. I mean the line:
> > > > > 
> > > > >       brw->vtbl.emit_buffer_surface_state(brw,
> > > > > &surf_offsets[i],
> > > > > bo,
> > > > > 
> > > > > Shouldn't that look up the atomic buffer index in prog
> > > > > ->Uniforms
> > > > > instead of using 'i'?
> > > > 
> > > > No that looks fine. The problem in the patch was we didn't have
> > > > the
> > > > atomic buffex index and we used the binding as the offset which
> > > > doesn't
> > > > have to be the same as the buffer index.
> > > > 
> > > > In brw_upload_abo_surfaces its the opposite we have the atomic
> > > > buffer
> > > > index which is i and we are looking up the binding to get the
> > > > buffer
> > > > object.
> > > > 
> > > > Does that make sense?
> > > 
> > > i is the binding index though...
> > 
> > i is the buffer index
> > 
> > See find_active_atomic_counters() for the code the counts the
> > active
> > buffers.
> > 
> > http://cgit.freedesktop.org/mesa/mesa/tree/src/glsl/link_atomics.cp
> > p#n9
> > 9
> > 
> > This count is then used to set NumAtomicBuffers and to create the
> > prog
> > ->AtomicBuffers array so the array is no bigger than it needs to
> > be.
> > This is the reason the binding and atomic buffer index can differ.
> > 
> > http://cgit.freedesktop.org/mesa/mesa/tree/src/glsl/link_atomics.cp
> > p#n1
> > 73
> 
> Do you agree that NumAtomicBuffers can go up to
> MaxCombinedAtomicBuffers? If so, that's problematic, when it's >
> MaxAtomicBuffers, right?

Ok, so now we are talking about a new problem from what this patch is
trying to solve.

I agree it can go up to MaxCombinedAtomicBuffers.

I'm not sure what the answer is, maybe a question for curro. I don't
know enough about this code to know if we want to upload everything
here or just do a stage at a time.


> 
>   -ilia