[Mesa-dev] [PATCH v2] intel: Don't flush the old context in intelMakeCurrent

[Neil Roberts]

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