On 11/11/2013 07:31 AM, Paul Berry wrote: > On 8 November 2013 13:38, Kenneth Graunke <kenn...@whitecape.org > <mailto:kenn...@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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev