Looks good to me Thomas. Jose
----- Original Message ----- > The api and the state tracker manager code as well as the state > tracker code > assumed that only a single context could be bound to a drawable. That > is not > a valid assumption, since multiple contexts can bind to the same > drawable. > > Fix this by making it the state tracker's responsibility to update > all > contexts binding to a drawable. > > Signed-off-by: Thomas Hellstrom <thellst...@vmware.com> > --- > src/gallium/include/state_tracker/st_api.h | 26 ++----- > .../state_trackers/dri/common/dri_drawable.c | 3 + > src/gallium/state_trackers/dri/drm/dri2.c | 4 +- > src/gallium/state_trackers/dri/sw/drisw.c | 4 +- > src/gallium/state_trackers/egl/common/egl_g3d.c | 12 +-- > .../state_trackers/egl/common/egl_g3d_api.c | 7 -- > src/gallium/state_trackers/egl/common/egl_g3d_st.c | 8 ++ > src/gallium/state_trackers/vega/vg_context.h | 4 +- > src/gallium/state_trackers/vega/vg_manager.c | 40 ++++------ > src/gallium/state_trackers/wgl/stw_context.c | 6 +- > src/gallium/state_trackers/wgl/stw_st.c | 3 + > src/mesa/state_tracker/st_cb_viewport.c | 15 +++- > src/mesa/state_tracker/st_context.h | 5 +- > src/mesa/state_tracker/st_manager.c | 85 > +++++++++++--------- > 14 files changed, 109 insertions(+), 113 deletions(-) > > diff --git a/src/gallium/include/state_tracker/st_api.h > b/src/gallium/include/state_tracker/st_api.h > index 04fc7c6..b08972f 100644 > --- a/src/gallium/include/state_tracker/st_api.h > +++ b/src/gallium/include/state_tracker/st_api.h > @@ -253,6 +253,13 @@ struct st_context_attribs > struct st_framebuffer_iface > { > /** > + * Atomic flag to indicate invalid status, which can only be > resolved > + * by a call to validate. > + */ > + > + int32_t invalid; > + > + /** > * Available for the state tracker manager to use. > */ > void *st_manager_private; > @@ -315,25 +322,6 @@ struct st_context_iface > void (*destroy)(struct st_context_iface *stctxi); > > /** > - * Invalidate the current textures that was taken from a > framebuffer. > - * > - * The state tracker manager calls this function to let the > rendering > - * context know that it should update the textures it got from > - * st_framebuffer_iface::validate. It should do so at the latest > time possible. > - * Possible right before sending triangles to the pipe context. > - * > - * For certain platforms this function might be called from a > thread other > - * than the thread that the context is currently bound in, and > must > - * therefore be thread safe. But it is the state tracker > manager's > - * responsibility to make sure that the framebuffer is bound to > the context > - * and the API context is current for the duration of this call. > - * > - * Thus reducing the sync primitive needed to a single atomic > flag. > - */ > - void (*notify_invalid_framebuffer)(struct st_context_iface > *stctxi, > - struct st_framebuffer_iface > *stfbi); > - > - /** > * Flush all drawing from context to the pipe also flushes the > pipe. > */ > void (*flush)(struct st_context_iface *stctxi, unsigned flags, > diff --git a/src/gallium/state_trackers/dri/common/dri_drawable.c > b/src/gallium/state_trackers/dri/common/dri_drawable.c > index 28a33ac..09d25bc 100644 > --- a/src/gallium/state_trackers/dri/common/dri_drawable.c > +++ b/src/gallium/state_trackers/dri/common/dri_drawable.c > @@ -82,6 +82,8 @@ dri_st_framebuffer_validate(struct > st_framebuffer_iface *stfbi, > drawable->texture_mask = statt_mask; > } > > + p_atomic_set(&drawable->base.invalid, FALSE); > + > if (!out) > return TRUE; > > @@ -136,6 +138,7 @@ dri_create_buffer(__DRIscreen * sPriv, > drawable->sPriv = sPriv; > drawable->dPriv = dPriv; > dPriv->driverPrivate = (void *)drawable; > + p_atomic_set(&drawable->base.invalid, TRUE); > > return GL_TRUE; > fail: > diff --git a/src/gallium/state_trackers/dri/drm/dri2.c > b/src/gallium/state_trackers/dri/drm/dri2.c > index e471e8e..9e61d7e 100644 > --- a/src/gallium/state_trackers/dri/drm/dri2.c > +++ b/src/gallium/state_trackers/dri/drm/dri2.c > @@ -52,13 +52,11 @@ static void > dri2_invalidate_drawable(__DRIdrawable *dPriv) > { > struct dri_drawable *drawable = dri_drawable(dPriv); > - struct dri_context *ctx = drawable->context; > > dri2InvalidateDrawable(dPriv); > drawable->dPriv->lastStamp = *drawable->dPriv->pStamp; > > - if (ctx) > - ctx->st->notify_invalid_framebuffer(ctx->st, &drawable->base); > + p_atomic_set(&drawable->base.invalid, TRUE); > } > > static const __DRI2flushExtension dri2FlushExtension = { > diff --git a/src/gallium/state_trackers/dri/sw/drisw.c > b/src/gallium/state_trackers/dri/sw/drisw.c > index ac11f7c..f9ba971 100644 > --- a/src/gallium/state_trackers/dri/sw/drisw.c > +++ b/src/gallium/state_trackers/dri/sw/drisw.c > @@ -108,9 +108,7 @@ drisw_invalidate_drawable(__DRIdrawable *dPriv) > > drawable->texture_stamp = dPriv->lastStamp - 1; > > - /* check if swapping currently bound buffer */ > - if (ctx && ctx->dPriv == dPriv) > - ctx->st->notify_invalid_framebuffer(ctx->st, &drawable->base); > + p_atomic_set(&drawable->base.invalid, TRUE); > } > > static INLINE void > diff --git a/src/gallium/state_trackers/egl/common/egl_g3d.c > b/src/gallium/state_trackers/egl/common/egl_g3d.c > index 29dbbef..45db90a 100644 > --- a/src/gallium/state_trackers/egl/common/egl_g3d.c > +++ b/src/gallium/state_trackers/egl/common/egl_g3d.c > @@ -45,15 +45,9 @@ egl_g3d_invalid_surface(struct native_display > *ndpy, > { > /* XXX not thread safe? */ > struct egl_g3d_surface *gsurf = > egl_g3d_surface(nsurf->user_data); > - struct egl_g3d_context *gctx; > - > - /* > - * Some functions such as egl_g3d_copy_buffers create a temporary > native > - * surface. There is no gsurf associated with it. > - */ > - gctx = (gsurf) ? egl_g3d_context(gsurf->base.CurrentContext) : > NULL; > - if (gctx) > - gctx->stctxi->notify_invalid_framebuffer(gctx->stctxi, > gsurf->stfbi); > + > + if (gsurf && gsurf->stfbi) > + p_atomic_set(&gsurf->stfbi->invalid, TRUE); > } > > static struct pipe_screen * > diff --git a/src/gallium/state_trackers/egl/common/egl_g3d_api.c > b/src/gallium/state_trackers/egl/common/egl_g3d_api.c > index 8b1821e..f0d6d71 100644 > --- a/src/gallium/state_trackers/egl/common/egl_g3d_api.c > +++ b/src/gallium/state_trackers/egl/common/egl_g3d_api.c > @@ -536,19 +536,12 @@ egl_g3d_make_current(_EGLDriver *drv, > _EGLDisplay *dpy, > (gdraw) ? gdraw->stfbi : NULL, (gread) ? gread->stfbi : > NULL); > if (ok) { > if (gdraw) { > - gctx->stctxi->notify_invalid_framebuffer(gctx->stctxi, > - gdraw->stfbi); > - > if (gdraw->base.Type == EGL_WINDOW_BIT) { > gctx->base.WindowRenderBuffer = > (gdraw->stvis.render_buffer == > ST_ATTACHMENT_FRONT_LEFT) ? > EGL_SINGLE_BUFFER : EGL_BACK_BUFFER; > } > } > - if (gread && gread != gdraw) { > - gctx->stctxi->notify_invalid_framebuffer(gctx->stctxi, > - gread->stfbi); > - } > } > } > else if (old_gctx) { > diff --git a/src/gallium/state_trackers/egl/common/egl_g3d_st.c > b/src/gallium/state_trackers/egl/common/egl_g3d_st.c > index 2b944b5..cb9c237 100644 > --- a/src/gallium/state_trackers/egl/common/egl_g3d_st.c > +++ b/src/gallium/state_trackers/egl/common/egl_g3d_st.c > @@ -182,6 +182,9 @@ egl_g3d_st_framebuffer_validate_pbuffer(struct > st_framebuffer_iface *stfbi, > pipe_resource_reference(&out[i], gsurf->render_texture); > } > > + if (gsurf->stfbi) > + p_atomic_set(&gsurf->stfbi->invalid, FALSE); > + > return TRUE; > } > > @@ -278,6 +281,9 @@ egl_g3d_st_framebuffer_validate(struct > st_framebuffer_iface *stfbi, > } > } > > + if (gsurf->stfbi) > + p_atomic_set(&gsurf->stfbi->invalid, FALSE); > + > return TRUE; > } > > @@ -292,6 +298,8 @@ egl_g3d_create_st_framebuffer(_EGLSurface *surf) > return NULL; > > stfbi->visual = &gsurf->stvis; > + p_atomic_set(&stfbi->invalid, TRUE); > + > if (gsurf->base.Type != EGL_PBUFFER_BIT) { > stfbi->flush_front = egl_g3d_st_framebuffer_flush_front; > stfbi->validate = egl_g3d_st_framebuffer_validate; > diff --git a/src/gallium/state_trackers/vega/vg_context.h > b/src/gallium/state_trackers/vega/vg_context.h > index 71491a5..0d91007 100644 > --- a/src/gallium/state_trackers/vega/vg_context.h > +++ b/src/gallium/state_trackers/vega/vg_context.h > @@ -65,6 +65,7 @@ struct st_framebuffer { > enum st_attachment_type strb_att; > > void *privateData; > + int32_t stamp; > }; > > enum vg_object_type { > @@ -105,7 +106,6 @@ struct vg_context > VGErrorCode _error; > > struct st_framebuffer *draw_buffer; > - int32_t draw_buffer_invalid; > > struct cso_hash *owned_objects[VG_OBJECT_LAST]; > > @@ -129,6 +129,8 @@ struct vg_context > struct vg_paint *default_paint; > > struct blit_state *blit; > + > + int32_t draw_stamp; > }; > > > diff --git a/src/gallium/state_trackers/vega/vg_manager.c > b/src/gallium/state_trackers/vega/vg_manager.c > index eeea686..c9149f8 100644 > --- a/src/gallium/state_trackers/vega/vg_manager.c > +++ b/src/gallium/state_trackers/vega/vg_manager.c > @@ -111,30 +111,25 @@ vg_manager_validate_framebuffer(struct > vg_context *ctx) > if (!stfb) > return; > > - if (!p_atomic_read(&ctx->draw_buffer_invalid)) > - return; > - > - /* validate the fb */ > - if (!stfb->iface->validate(stfb->iface, &stfb->strb_att, 1, &pt) > || !pt) > - return; > + if (p_atomic_read(&stfb->iface->invalid)) { > > - p_atomic_set(&ctx->draw_buffer_invalid, FALSE); > + /* validate the fb */ > + if (!stfb->iface->validate(stfb->iface, &stfb->strb_att, 1, > &pt) || !pt) > + return; > > - if (vg_context_update_color_rb(ctx, pt) || > - stfb->width != pt->width0 || > - stfb->height != pt->height0) > - ctx->state.dirty |= FRAMEBUFFER_DIRTY; > + if (vg_context_update_color_rb(ctx, pt) || > + stfb->width != pt->width0 || > + stfb->height != pt->height0) > + ++stfb->stamp; > > - stfb->width = pt->width0; > - stfb->height = pt->height0; > -} > + stfb->width = pt->width0; > + stfb->height = pt->height0; > + } > > -static void > -vg_context_notify_invalid_framebuffer(struct st_context_iface > *stctxi, > - struct st_framebuffer_iface > *stfbi) > -{ > - struct vg_context *ctx = (struct vg_context *) stctxi; > - p_atomic_set(&ctx->draw_buffer_invalid, TRUE); > + if (ctx->draw_stamp != stfb->stamp) { > + ctx->state.dirty |= FRAMEBUFFER_DIRTY; > + ctx->draw_stamp = stfb->stamp; > + } > } > > static void > @@ -187,8 +182,6 @@ vg_api_create_context(struct st_api *stapi, > struct st_manager *smapi, > > ctx->iface.destroy = vg_context_destroy; > > - ctx->iface.notify_invalid_framebuffer = > - vg_context_notify_invalid_framebuffer; > ctx->iface.flush = vg_context_flush; > > ctx->iface.teximage = NULL; > @@ -266,8 +259,6 @@ vg_context_bind_framebuffers(struct > st_context_iface *stctxi, > if (stdrawi != streadi) > return FALSE; > > - p_atomic_set(&ctx->draw_buffer_invalid, TRUE); > - > strb_att = (stdrawi) ? choose_attachment(stdrawi) : > ST_ATTACHMENT_INVALID; > > if (ctx->draw_buffer) { > @@ -318,6 +309,7 @@ vg_context_bind_framebuffers(struct > st_context_iface *stctxi, > } > > ctx->draw_buffer->iface = stdrawi; > + ctx->draw_stamp = ctx->draw_buffer->stamp - 1; > > return TRUE; > } > diff --git a/src/gallium/state_trackers/wgl/stw_context.c > b/src/gallium/state_trackers/wgl/stw_context.c > index 5608d4f..7e29960 100644 > --- a/src/gallium/state_trackers/wgl/stw_context.c > +++ b/src/gallium/state_trackers/wgl/stw_context.c > @@ -31,6 +31,7 @@ > #include "pipe/p_context.h" > #include "pipe/p_state.h" > #include "util/u_memory.h" > +#include "util/u_atomic.h" > #include "state_tracker/st_api.h" > > #include "stw_icd.h" > @@ -361,10 +362,7 @@ stw_flush_current_locked( struct stw_framebuffer > *fb ) > void > stw_notify_current_locked( struct stw_framebuffer *fb ) > { > - struct stw_context *ctx = stw_current_context(); > - > - if (ctx && ctx->current_framebuffer == fb) > - ctx->st->notify_invalid_framebuffer(ctx->st, fb->stfb); > + p_atomic_set(&fb->stfb->invalid, TRUE); > } > > /** > diff --git a/src/gallium/state_trackers/wgl/stw_st.c > b/src/gallium/state_trackers/wgl/stw_st.c > index 9174533..7299441 100644 > --- a/src/gallium/state_trackers/wgl/stw_st.c > +++ b/src/gallium/state_trackers/wgl/stw_st.c > @@ -27,6 +27,7 @@ > > #include "util/u_memory.h" > #include "util/u_inlines.h" > +#include "util/u_atomic.h" > #include "state_tracker/st_gl_api.h" /* for st_gl_api_create */ > > #include "stw_st.h" > @@ -117,6 +118,7 @@ stw_st_framebuffer_validate_locked(struct > st_framebuffer_iface *stfb, > stwfb->texture_width = width; > stwfb->texture_height = height; > stwfb->texture_mask = mask; > + p_atomic_set(&stfb->invalid, FALSE); > } > > static boolean > @@ -196,6 +198,7 @@ stw_st_create_framebuffer(struct stw_framebuffer > *fb) > stwfb->stvis = fb->pfi->stvis; > > stwfb->base.visual = &stwfb->stvis; > + p_atomic_set(&stwfb->base.invalid, TRUE); > stwfb->base.flush_front = stw_st_framebuffer_flush_front; > stwfb->base.validate = stw_st_framebuffer_validate; > > diff --git a/src/mesa/state_tracker/st_cb_viewport.c > b/src/mesa/state_tracker/st_cb_viewport.c > index 049755e..c74d013 100644 > --- a/src/mesa/state_tracker/st_cb_viewport.c > +++ b/src/mesa/state_tracker/st_cb_viewport.c > @@ -56,13 +56,20 @@ static void st_viewport(struct gl_context * ctx, > GLint x, GLint y, > if (!st->invalidate_on_gl_viewport) > return; > > + /* > + * Normally we'd want the state tracker manager to mark the > drawables > + * invalid only when needed. This will force the state tracker > manager > + * to revalidate the drawable, rather than just update the > context with > + * the latest cached drawable info. > + */ > + > stdraw = st_ws_framebuffer(st->ctx->DrawBuffer); > stread = st_ws_framebuffer(st->ctx->ReadBuffer); > > - if (stdraw) > - p_atomic_set(&stdraw->revalidate, TRUE); > - if (stread && stread != stdraw) > - p_atomic_set(&stread->revalidate, TRUE); > + if (stdraw && stdraw->iface) > + p_atomic_set(&stdraw->iface->invalid, TRUE); > + if (stread && stread != stdraw && stread->iface) > + p_atomic_set(&stread->iface->invalid, TRUE); > } > > void st_init_viewport_functions(struct dd_function_table *functions) > diff --git a/src/mesa/state_tracker/st_context.h > b/src/mesa/state_tracker/st_context.h > index ff20703..f76fae3 100644 > --- a/src/mesa/state_tracker/st_context.h > +++ b/src/mesa/state_tracker/st_context.h > @@ -204,6 +204,9 @@ struct st_context > /* Active render condition. */ > struct pipe_query *render_condition; > unsigned condition_mode; > + > + int32_t draw_stamp; > + int32_t read_stamp; > }; > > > @@ -227,7 +230,7 @@ struct st_framebuffer > struct st_framebuffer_iface *iface; > enum st_attachment_type statts[ST_ATTACHMENT_COUNT]; > unsigned num_statts; > - int32_t revalidate; > + int32_t stamp; > }; > > > diff --git a/src/mesa/state_tracker/st_manager.c > b/src/mesa/state_tracker/st_manager.c > index a68544d..cc688a3 100644 > --- a/src/mesa/state_tracker/st_manager.c > +++ b/src/mesa/state_tracker/st_manager.c > @@ -139,18 +139,52 @@ buffer_index_to_attachment(gl_buffer_index > index) > } > > /** > + * Make sure a context picks up the latest cached state of the > + * drawables it binds to. > + */ > +static void > +st_context_validate(struct st_context *st, > + struct st_framebuffer *stdraw, > + struct st_framebuffer *stread) > +{ > + if (stdraw && stdraw->stamp != st->draw_stamp) { > + st->dirty.st |= ST_NEW_FRAMEBUFFER; > + _mesa_resize_framebuffer(st->ctx, &stdraw->Base, > + stdraw->Base.Width, > + stdraw->Base.Height); > + st->draw_stamp = stdraw->stamp; > + } > + > + if (stread && stread->stamp != st->read_stamp) { > + if (stread != stdraw) { > + st->dirty.st |= ST_NEW_FRAMEBUFFER; > + _mesa_resize_framebuffer(st->ctx, &stread->Base, > + stread->Base.Width, > + stread->Base.Height); > + } > + st->read_stamp = stread->stamp; > + } > +} > + > +/** > * Validate a framebuffer to make sure up-to-date pipe_textures are > used. > + * The context we need to pass in is s dummy context needed only to > be > + * able to get a pipe context to create pipe surfaces, and to have a > + * context to call _mesa_resize_framebuffer(): > + * (That should probably be rethought, since those surfaces become > + * drawable state, not context state, and can be freed by another > pipe > + * context). > */ > static void > -st_framebuffer_validate(struct st_framebuffer *stfb, struct > st_context *st) > +st_framebuffer_validate(struct st_framebuffer *stfb, > + struct st_context *st) > { > - struct pipe_context *pipe = st->pipe; > struct pipe_resource *textures[ST_ATTACHMENT_COUNT]; > uint width, height; > unsigned i; > boolean changed = FALSE; > > - if (!p_atomic_read(&stfb->revalidate)) > + if (!p_atomic_read(&stfb->iface->invalid)) > return; > > /* validate the fb */ > @@ -184,7 +218,7 @@ st_framebuffer_validate(struct st_framebuffer > *stfb, struct st_context *st) > memset(&surf_tmpl, 0, sizeof(surf_tmpl)); > u_surface_default_template(&surf_tmpl, textures[i], > PIPE_BIND_RENDER_TARGET); > - ps = pipe->create_surface(pipe, textures[i], &surf_tmpl); > + ps = st->pipe->create_surface(st->pipe, textures[i], > &surf_tmpl); > if (ps) { > pipe_surface_reference(&strb->surface, ps); > pipe_resource_reference(&strb->texture, ps->texture); > @@ -204,14 +238,9 @@ st_framebuffer_validate(struct st_framebuffer > *stfb, struct st_context *st) > } > > if (changed) { > - st->dirty.st |= ST_NEW_FRAMEBUFFER; > + ++stfb->stamp; > _mesa_resize_framebuffer(st->ctx, &stfb->Base, width, height); > - > - assert(stfb->Base.Width == width); > - assert(stfb->Base.Height == height); > } > - > - p_atomic_set(&stfb->revalidate, FALSE); > } > > /** > @@ -237,7 +266,7 @@ st_framebuffer_update_attachments(struct > st_framebuffer *stfb) > stfb->statts[stfb->num_statts++] = statt; > } > > - p_atomic_set(&stfb->revalidate, TRUE); > + stfb->stamp = 0; > } > > /** > @@ -443,6 +472,7 @@ st_framebuffer_create(struct st_framebuffer_iface > *stfbi) > &stfb->Base._ColorReadBufferIndex); > > stfb->iface = stfbi; > + p_atomic_set(&stfbi->invalid, TRUE); > > /* add the color buffer */ > idx = stfb->Base._ColorDrawBufferIndexes[0]; > @@ -473,31 +503,6 @@ st_framebuffer_reference(struct st_framebuffer > **ptr, > } > > static void > -st_context_notify_invalid_framebuffer(struct st_context_iface > *stctxi, > - struct st_framebuffer_iface > *stfbi) > -{ > - struct st_context *st = (struct st_context *) stctxi; > - struct st_framebuffer *stfb; > - > - /* either draw or read winsys fb */ > - stfb = st_ws_framebuffer(st->ctx->WinSysDrawBuffer); > - if (!stfb || stfb->iface != stfbi) > - stfb = st_ws_framebuffer(st->ctx->WinSysReadBuffer); > - > - if (stfb && stfb->iface == stfbi) { > - p_atomic_set(&stfb->revalidate, TRUE); > - } > - else { > - /* This function is probably getting called when we've > detected a > - * change in a window's size but the currently bound context > is > - * not bound to that window. > - * If the st_framebuffer_iface structure had a pointer to the > - * corresponding st_framebuffer we'd be able to handle this. > - */ > - } > -} > - > -static void > st_context_flush(struct st_context_iface *stctxi, unsigned flags, > struct pipe_fence_handle **fence) > { > @@ -696,8 +701,6 @@ st_api_create_context(struct st_api *stapi, > struct st_manager *smapi, > smapi->get_param(smapi, ST_MANAGER_BROKEN_INVALIDATE); > > st->iface.destroy = st_context_destroy; > - st->iface.notify_invalid_framebuffer = > - st_context_notify_invalid_framebuffer; > st->iface.flush = st_context_flush; > st->iface.teximage = st_context_teximage; > st->iface.copy = st_context_copy; > @@ -757,6 +760,10 @@ st_api_make_current(struct st_api *stapi, struct > st_context_iface *stctxi, > } > > ret = _mesa_make_current(st->ctx, &stdraw->Base, > &stread->Base); > + > + st->draw_stamp = stdraw->stamp - 1; > + st->read_stamp = stread->stamp - 1; > + st_context_validate(st, stdraw, stread); > } > else { > struct gl_framebuffer *incomplete = > _mesa_get_incomplete_framebuffer(); > @@ -857,6 +864,8 @@ st_manager_validate_framebuffers(struct > st_context *st) > st_framebuffer_validate(stdraw, st); > if (stread && stread != stdraw) > st_framebuffer_validate(stread, st); > + > + st_context_validate(st, stdraw, stread); > } > > /** > -- > 1.7.4.4 > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev