Re: [Mesa3d-dev] dri glx fixes

[Thomas H. <the...@vm...>]

Kristian Høgsberg wrote:
> On Mon, Mar 30, 2009 at 4:46 AM, Thomas Hellstrom <the...@vm...> wrote:
>   
>> Hi!
>>
>> Thomas Hellstrom wrote:
>>     
>>> Hi!
>>>
>>> I've sent two glx dri patches for review. The first one does some minor
>>> optimizations and propagates driver makeCurrent back.
>>> The second one is intended to fix the makeContextCurrent(NULL, None,
>>> None) call and to do some additional error checking. This was prompted
>>> by the newly added sharedtex_mt demo.
>>>
>>> Most if not all mesa dri drivers are still broken w r t to the above
>>> call, as it still does not get through to the driver makeCurrent hook.
>>> Instead the correct functionality needs to be implemented in the driver
>>> unbindContext hook, similar to the following:
>>>
>>> GLboolean
>>> viaUnbindContext(__DRIcontextPrivate * driContextPriv)
>>> {
>>>     if (driContextPriv) {
>>>     struct via_context *vmesa =
>>>         (struct via_context *)driContextPriv->driverPrivate;
>>>     GLcontext *ctx = vmesa->glCtx;
>>>
>>>     GET_CURRENT_CONTEXT(oldctx);
>>>     if (ctx == oldctx)
>>>         VIA_FLUSH_DMA(vmesa);
>>>
>>>     _mesa_make_current(NULL, NULL, NULL);
>>>     }
>>>
>>>     return GL_TRUE;
>>> }
>>>
>>>
>>>       
>> Actually, the above code doesn't work, as unbindContext is always called
>> even if the new context and the old context are the same. Instead,
>> contexts need to be refcounted in the driver.
>>
>> /Thomas
>>
>>
>>     
>>> In addition, if the context is changed in the driver makeCurrent hook,
>>> the driver needs to flush the old context first. That could
>>> theoretically also be done unbindContext, but since unbindContext is
>>> called on the old context _after_ makeCurrent, that may not be a good idea.
>>>       
>
> Do you have a new patch on the way or do you want me to look at the
> ones you sent earlier?
>
> cheers,
> Kristian
>   

Hi, Kristian!

The patches should be OK. It's the above sample code that should go in 
each dri driver that doesn't work.
I have a code snippet instead that looks like this:

GLboolean
viaUnbindContext(__DRIcontextPrivate * driContextPriv)
{
    if (driContextPriv) {
    struct via_context *vmesa =
        (struct via_context *)driContextPriv->driverPrivate;
    GLcontext *ctx = vmesa->glCtx;

    if (--vmesa->bindCount == 0) {
        GET_CURRENT_CONTEXT(curctx);
       
        if (ctx == curctx) {
        VIA_FLUSH_DMA(vmesa);   
        _mesa_make_current(NULL, NULL, NULL);
        }
    }
    }
   
    return GL_TRUE;
}

and

GLboolean
viaMakeCurrent(__DRIcontextPrivate * driContextPriv,
           __DRIdrawablePrivate * driDrawPriv,
           __DRIdrawablePrivate * driReadPriv)
{
...
  
    if (driContextPriv) {
    struct via_context *vmesa =
        (struct via_context *)driContextPriv->driverPrivate;
    GLcontext *ctx = vmesa->glCtx;
    GET_CURRENT_CONTEXT(oldctx);

    if (ctx != oldctx && oldctx != NULL) {
        VIA_FLUSH_DMA(VIA_CONTEXT(oldctx));
    }
   
    ++vmesa->bindCount;
...

This combination should makeCurrent work as specified, namely

1) Always flush the old context before binding the new one.
2) Always flush the old context on makeCurrent(NULL, None, None);

While never attempting to flush a context that is not current.

Similar code needs to be added to each DRI driver, but that isn't a 
requirement for pulling in the patches.

Thanks,
/Thomas