On Friday, September 12, 2014 08:35:03 AM Ian Romanick wrote:
> On 09/10/2014 03:41 PM, Kenneth Graunke wrote:
> > In the non-indirect draw case, we call intel_upload_data to upload
> > gl_BaseVertex. It makes brw->draw.draw_params_bo point to the upload
> > buffer, and increments the upload BO reference count.
> >
> > So, we need to unreference it when making brw->draw.draw_params_bo point
> > at something else, or else we'll retain a reference to stale upload
> > buffers and hold on to them forever.
> >
> > This also means that the indirect case should increment the reference
> > count on the indirect draw buffer when making brw->draw.draw_params_bo
> > point at it. That way, both paths increment the reference count, so
> > we can safely unreference it every time.
> >
> > Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
> > Cc: "10.3" <mesa-sta...@lists.freedesktop.org>
> > ---
> > src/mesa/drivers/dri/i965/brw_draw.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_draw.c
> > b/src/mesa/drivers/dri/i965/brw_draw.c
> > index efa85de..b28eaf2 100644
> > --- a/src/mesa/drivers/dri/i965/brw_draw.c
> > +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> > @@ -434,10 +434,13 @@ static bool brw_try_draw_prims( struct gl_context
> > *ctx,
> > brw->draw.start_vertex_location = prims[i].start;
> > brw->draw.base_vertex_location = prims[i].basevertex;
> >
> > + drm_intel_bo_unreference(brw->draw.draw_params_bo);
> > +
> > if (prims[i].is_indirect) {
> > /* Point draw_params_bo at the indirect buffer. */
> > brw->draw.draw_params_bo =
> > intel_buffer_object(ctx->DrawIndirectBuffer)->buffer;
> > + drm_intel_bo_reference(brw->draw.draw_params_bo);
>
> Is there are a race here, or do we know that the reference count is at
> least two when we unreference above?The idea is to unreference the buffer used for the previous draw call - which may be either a DrawIndirect buffer, or the upload buffer. Then, we'll decide what buffer we want for the current draw call. For background: Legitimate GL-facing buffers hold a reference count until DeleteBuffer. brw->upload.bo holds a reference count on the upload buffer until batch flush, at which point we toss it and get a new (non-busy) upload buffer. I think it works out in all cases. Here's my logic: Case: Non-indirect -> Non-indirect If we haven't flushed the batch buffer, then brw->upload.bo still holds a reference to the upload buffer. We drop our reference, then pick it back up. If we've flushed the batch buffer, then brw->upload.bo will have been released, and will now point to a fresh buffer. We may hold the last reference on the old upload buffer - but releasing it is okay, because we don't want it anymore. We drop our reference to the old buffer, and take a reference on the new one. Case: Indirect -> Indirect The application may have switched out the indirect buffer between calls. If not, it's still bound as ctx->DrawIndirectBuffer, which holds a gl_buffer_object::RefCount on the gl_buffer_object, keeping it alive. So we can drop our reference and re-take it. If they did, we drop our reference on the old one. This could allow it to be freed, if the API-facing buffer was deleted. But we don't care. We take a reference to the new ctx->DrawIndirectBuffer's BO, which definitely exists. Case: Non-indirect -> Indirect We stop referencing the upload BO, and instead reference DrawIndirectBuffer. The upload BO will live or die based on other people holding refcounts, which is the desired behavior. Case: Indirect -> Non-indirect We stop referencing the BO which was bound as ctx->DrawIndirectBuffer when we prepared the previous draw call. It will probably live, unless the API- facing buffer object was deleted and this was the last use...and that's okay. We take a reference on the upload buffer, which is referenced by brw->upload.bo.
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
