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