I wrote: > I guess I should try to explore intelMakeCurrent in a bit more > detail to try to determine whether it's ok not to flush.
It looks like all the code between the flush and the call to _mesa_make_current it just operating on the new context not the old context and it looks fairly trivial so it's hard to see how it could make much difference. Seeing as the flush is almost never hit anyway I'm fairly confident it should be safe to remove. Here is a v2 of the patch with a better commit message. - Neil ------- >8 --------------- (use git am --scissors to automatically chop here) It shouldn't be necessary to flush the context within the driver implementation because the old context is explicitly flushed in _mesa_make_current which is called a little further on. It is useful to only have a single place that flushes when switching contexts to make it easier to later implement the GL_KHR_context_flush_control extension. The flush in intelMakeCurrent was added in commit 5505865 to implement the GLX semantics that the context should be flushed when it is released. When the commit was made there was no flush in _mesa_make_current because it was only added later in 93102b4c. I think that later commit effectively makes the first commit redundant. The later commit was added because it looks like the flush in intelMakeCurrent is actually not even hit anymore. This is because when a new context is bound the old context is usually unbound first in MakeContextCurrent and dri2_make_current. Since commit b4bb668020 the unbind implementations call _mesa_make_current(NULL) which effectively means that there should almost never be a current context when intelMakeCurrent is called. However one caveat with that is that MakeContextCurrent doesn't call the unbind virtual if the context is still bound in another thread. It seems to me like this is a bug anyway because the call to _mesa_make_current(NULL) was added to fix a problem where the bind implementation can incorrectly think the context is already bound when switching to an indirect context and back. Presumably this bug will still happen if you do that while the context is bound in another thread. I don't think it should make any difference anyway if the flush is moved lower in intelMakeCurrent because all of the code between the original flush and the call to _mesa_make_current looks quite trivial and it only operates on the new context not the old one. --- src/mesa/drivers/dri/i915/intel_context.c | 9 --------- src/mesa/drivers/dri/i965/brw_context.c | 9 --------- 2 files changed, 18 deletions(-) diff --git a/src/mesa/drivers/dri/i915/intel_context.c b/src/mesa/drivers/dri/i915/intel_context.c index 3104776..c70bd25 100644 --- a/src/mesa/drivers/dri/i915/intel_context.c +++ b/src/mesa/drivers/dri/i915/intel_context.c @@ -622,21 +622,12 @@ intelMakeCurrent(__DRIcontext * driContextPriv, __DRIdrawable * driReadPriv) { struct intel_context *intel; - GET_CURRENT_CONTEXT(curCtx); if (driContextPriv) intel = (struct intel_context *) driContextPriv->driverPrivate; else intel = NULL; - /* According to the glXMakeCurrent() man page: "Pending commands to - * the previous context, if any, are flushed before it is released." - * But only flush if we're actually changing contexts. - */ - if (intel_context(curCtx) && intel_context(curCtx) != intel) { - _mesa_flush(curCtx); - } - if (driContextPriv) { struct gl_context *ctx = &intel->ctx; struct gl_framebuffer *fb, *readFb; diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c index c1520bd..6d54a2b 100644 --- a/src/mesa/drivers/dri/i965/brw_context.c +++ b/src/mesa/drivers/dri/i965/brw_context.c @@ -980,21 +980,12 @@ intelMakeCurrent(__DRIcontext * driContextPriv, __DRIdrawable * driReadPriv) { struct brw_context *brw; - GET_CURRENT_CONTEXT(curCtx); if (driContextPriv) brw = (struct brw_context *) driContextPriv->driverPrivate; else brw = NULL; - /* According to the glXMakeCurrent() man page: "Pending commands to - * the previous context, if any, are flushed before it is released." - * But only flush if we're actually changing contexts. - */ - if (brw_context(curCtx) && brw_context(curCtx) != brw) { - _mesa_flush(curCtx); - } - if (driContextPriv) { struct gl_context *ctx = &brw->ctx; struct gl_framebuffer *fb, *readFb; -- 1.9.3 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev