[Mesa-dev] Batchbuffer question

[Kenneth Graunke]

On 11/11/2013 07:31 AM, Paul Berry wrote:
> On 8 November 2013 13:38, Kenneth Graunke <kenneth at whitecape.org
> <mailto:kenneth at whitecape.org>> wrote:
> 
>     On 11/08/2013 04:49 AM, Rogovin, Kevin wrote:
>     > Hi all,
>     >
>     > As I was poking into the magicks for the batchbuffer, I saw the
>     > following logical bits of code, that make sense by themselves but get
>     > me paranoid together. Firstly in intel_batchbuffer_begin() [
>     > intel_batchbuffer.h, and this is what BEGIN_BATCH maps to] there is a
>     > intel_batchbuffer_require_space() call that if too much room is
>     > needed then calls intel_batchbuffer_begin():
>     >
>     > from intel_batchbuffer_require_space():
>     >
>     >   115    if (intel_batchbuffer_space(brw) < sz)
>     >   116       intel_batchbuffer_flush(brw);
>     >
>     > and from intel_batchbuffer_space():
>     >
>     >    80 intel_batchbuffer_space(struct brw_context *brw)
>     >    81 {
>     >    82    return (brw->batch.state_batch_offset -
>     brw->batch.reserved_space)
>     >    83       - brw->batch.used*4;
>     >    84 }
>     >
>     >
>     > Now, for allocating space for state, there is brw_state_batch():
>     >
>     >
>     >   128    offset = ROUND_DOWN_TO(batch->state_batch_offset - size,
>     alignment);
>     >   129
>     >   130    /* If allocating from the top would wrap below the
>     batchbuffer, or
>     >   131     * if the batch's used space (plus the reserved pad)
>     collides with our
>     >   132     * space, then flush and try again.
>     >   133     */
>     >   134    if (batch->state_batch_offset < size ||
>     >   135        offset < 4*batch->used + batch->reserved_space) {
>     >   136       intel_batchbuffer_flush(brw);
>     >   137       offset = ROUND_DOWN_TO(batch->state_batch_offset -
>     size, alignment);
>     >   138    }
>     >
>     >
>     > These taken together, I interpret as meaning that state and commands
>     > try to be separated by atleast batch->reserved_space bytes. I guess
>     > state could take up more than (batch->bo->size -
>     > batch->reserved_space) from that second ROUND_DOWN_TO, but that would
>     > only happen right after a flush and any state or command afterwards
>     > would flush too.
>     >
>     > Now my questions: 1) it looks like the reserved space is not to be
>     > used for either state or commands. Is that correct? What is it used
>     > for?
> 
>     To explain a bit more...we store commands (like 3DSTATE_VS) and indirect
>     state (like BLEND_STATE) in the same buffer (the batchbuffer).
> 
>     We emit commands starting at the top of the buffer, since they need to
>     be in order.  Indirect state is always pointed to by a command (i.e.
>     3DSTATE_BLEND_STATE_POINTERS), so it can be in any order.  We fill
>     indirect state backwards, starting from the end of the buffer.
> 
>     Should they meet in the middle, we flush the batch and start a new one.
> 
>     It's sort of like how the stack and heap start at opposite sides and
>     grow towards each other.
> 
>     When we finish a batch, we need to append a few final commands.  These
>     use the reserved space.  From intel_batchbuffer.c:
> 
>        brw->batch.reserved_space = 0;
> 
>        brw_finish_batch(brw);
> 
>        /* Mark the end of the buffer. */
>        intel_batchbuffer_emit_dword(brw, MI_BATCH_BUFFER_END);
>        if (brw->batch.used & 1) {
>           /* Round batchbuffer usage to 2 DWORDs. */
>           intel_batchbuffer_emit_dword(brw, MI_NOOP);
>        }
> 
>     MI_BATCH_BUFFER_END is fairly self-explanatory, but there are others.
>     On Gen4-5, we need to take a snapshot of the PS_DEPTH_COUNT register in
>     order to make occlusion queries, so there's a PIPE_CONTROL.  With my
>     upcoming performance monitor changes, we also need to take snapshots of
>     the observability architecture counters as well, so there will be an
>     MI_REPORT_PERF_COUNT.
> 
>     See the comments above the BATCH_RESERVED #define, which list all the
>     things we might emit.
> 
>     Note that, prior to emitting this state, we set
>     brw->batch.reserved_space to 0, which makes that space available for
>     these final commands.  So we'll always have space and never try to flush
>     in the middle of flushing.
> 
>     > 2) if a function first calls brw_state_batch() to place state and it
>     > barely fits and then calls BEGIN_BATCH that does not fit, then the
>     > command will refer to an offset on the previous batch buffer, that
>     > cannot be good. Or does this never happen for other reasons? If so
>     > what are those reasons?
> 
>     Leaving additional indirect state in the buffer is harmless.  We do need
>     to remove commands.
> 
>     OpenGL draw calls involve emitting a bunch of commands, finishing with a
>     3DPRIMITIVE command.  Prior to emitting anything, we call:
> 
>           intel_batchbuffer_save_state(brw);
> 
>     which saves batch->used and the current number of relocations.  If we
>     run out of space before emitting the 3DPRIMITIVE, we call:
> 
>                 intel_batchbuffer_reset_to_saved(brw);
> 
>     which resets the batch->used to the saved value, effectively throwing
>     away those commands.  It also calls into libdrm to throw away the extra
>     relocations.
> 
> 
> Ken,
> 
> When you say "If we run out of space before emitting the 3DPRIMITIVE",
> it sounds from context like you are saying "if we run out of batch
> buffer space".  I used to think the same thing, however before answering
> Kevin's email I looked at the code, and found that this isn't the case. 
> In fact, we only use intel_batchbuffer_reset_to_saved() if we run out of
> space in the *aperture*.  There is no mechanism for retrying in the
> event that we run out of batch buffer space.  The way we avoid running
> out of batchbuffer space is that before starting the draw call, we make
> an estimate of how much batch buffer space is needed, and we flush if
> that much space isn't available.  If our estimate was wrong and we wind
> up running out of batch space anyhow, then we assertion fail (see the
> "assert(!brw->no_batch_wrap);" in _intel_batchbuffer_flush()).
> 
> Just trying to make sure we keep the record straight,
> 
> Paul

Good grief...you're right.  That's garbage.

We should really fix that...

--Ken