[Mesa-dev] [PATCH] st/dri: Remove useless flush front.

[Stéphane Marchesin]

On Wed, Jan 11, 2012 at 10:13, Jose Fonseca <jfonseca at vmware.com> wrote:
> ----- Original Message -----
>> Where else would the flush go?
>
> Maybe one of the callers already invoked glFlush, or something like that.
>

Not that I could see.

>> I'll look into fixing it differently
>> but if the flush is anywhere in the makecurrent path, the same issue
>> will happen. Again, none of the classic dri drivers do it.
>
> If you wanna a short term fix, it might be OK with just commenting out the flush call, but please leave a note there saying that a flush is needed for strict conformance.

Maybe I'll do that, what about those lines in g/st/dri/common/dri_drawable.c:
/* XXX remove this and just set the correct one on the framebuffer */
drawable->flush_frontbuffer(drawable, statt);

It's on the same path that leads to that crash and seems to suggest
that the flush might not be required?

Stéphane


>
> Jose
>
>> Stéphane
>>
>>
>> 2012/1/11 Jose Fonseca <jfonseca at vmware.com>:
>> > Stephane,
>> >
>> > Although flushing the front buffer may be unnecessary, flushing
>> > itself is not, as glXMakeCurrent mandates that the current must be
>> > flushed:
>> >
>> >   "Pending commands to the previous context, if any, are flushed
>> >   before it is released."
>> >
>> > So this flush call can only be removed, if a flush is done
>> > elsewhere in DRI shared code, otherwise rendering queued in that
>> > context may not happen.
>> >
>> > Jose
>> >
>> > ----- Original Message -----
>> >> In the following scenario:
>> >> - CreateContext C1
>> >> - MakeCurrent C1
>> >> - DestroyContext C1 (does not actually destroy the first context,
>> >> postponed
>> >>   until the next MakeCurrent)
>> >> - CreateContext C2
>> >> - MakeCurrent C2
>> >>
>> >> MakeCurrent will call flush on a context which might not even have
>> >> had a front
>> >> buffer, leading to crashes. Since none of the dri drivers use the
>> >> flush in
>> >> their unbind_context implementation either, we remove this useless
>> >> flush front
>> >> call.
>> >>
>> >> This fixes GPU crashes with Chrome and gallium drivers.
>> >> ---
>> >>  .../state_trackers/dri/common/dri_context.c        |    1 -
>> >>  1 files changed, 0 insertions(+), 1 deletions(-)
>> >>
>> >> diff --git a/src/gallium/state_trackers/dri/common/dri_context.c
>> >> b/src/gallium/state_trackers/dri/common/dri_context.c
>> >> index b47d8d9..226c630 100644
>> >> --- a/src/gallium/state_trackers/dri/common/dri_context.c
>> >> +++ b/src/gallium/state_trackers/dri/common/dri_context.c
>> >> @@ -188,7 +188,6 @@ dri_unbind_context(__DRIcontext * cPriv)
>> >>
>> >>     if (--ctx->bind_count == 0) {
>> >>        if (ctx->st == ctx->stapi->get_current(ctx->stapi)) {
>> >> -         ctx->st->flush(ctx->st, ST_FLUSH_FRONT, NULL);
>> >>           stapi->make_current(stapi, NULL, NULL, NULL);
>> >>        }
>> >>     }
>> >> --
>> >> 1.7.5.3.367.ga9930
>> >>
>> >> _______________________________________________
>> >> mesa-dev mailing list
>> >> mesa-dev at lists.freedesktop.org
>> >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> >>
>> > _______________________________________________
>> > mesa-dev mailing list
>> > mesa-dev at lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>