[Mesa-dev] [PATCH] DRI2: don't advertise GLX_INTEL_swap_event if it can't

[Rob Clark]

(switching over mesa-dev.. sent to the wrong list initially)

On Wed, Mar 13, 2013 at 8:25 AM, Paul Menzel
<paulepanter at users.sourceforge.net> wrote:
> Dear Rob,
>
>
> Am Dienstag, den 12.03.2013, 19:44 -0400 schrieb Rob Clark:
>
> »it« sounds  strange in commit summary.
>
>> If ddx does not support swap, don't advertise it.

Hmm, yeah.. I somehow was having trouble coming up with something short enough


> So how is `dri2BindExtensions` changed. Some things passed beforehand
> are already available in `struct dri2_screen *psc`?

yeah, I suppose I didn't have to remove the extensions arg, but the
code seemed a bit cleaner this way and was trying to avoid
dri2BindExtensions() growing to a huge # of args

> Are bugs fixed by this or did you find this reading through the code?

yes, with "DRI2: Don't disable GLX_INTEL_swap_event unconditionally"
and without this patch, gnome-shell (and probably I guess anything
built on clutter) will be broken for ddx drivers which don't support
swap.  I noticed this when rebasing freedreno to latest mesa (since
currently I have no good kernel interface for page flipping, so I only
advertise DRI2 1.1 (DRI2InfoRec version==3).

>> We might also be able to get rid of the vmwgfx check (I'm not quite
>> sure the purpose of that check vs. just checking dri2Minor.
>
> Missing »)«.

oh, whoops.. well that is easy enough to fix at least

BR,
-R


>> Signed-off-by: Rob Clark <robdclark at gmail.com>
>> ---
>>  src/glx/dri2_glx.c | 12 ++++++++----
>>  1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
>> index c4f6996..b2d712c 100644
>> --- a/src/glx/dri2_glx.c
>> +++ b/src/glx/dri2_glx.c
>> @@ -1051,11 +1051,16 @@ static const struct glx_context_vtable dri2_context_vtable = {
>>  };
>>
>>  static void
>> -dri2BindExtensions(struct dri2_screen *psc, const __DRIextension **extensions,
>> +dri2BindExtensions(struct dri2_screen *psc, struct glx_display * priv,
>
> No space after the * in `* priv`?
>
>>                     const char *driverName)
>>  {
>> +   const struct dri2_display *const pdp = (struct dri2_display *)
>> +      priv->dri2Display;
>> +   const __DRIextension **extensions;
>>     int i;
>>
>> +   extensions = psc->core->getExtensions(psc->driScreen);
>> +
>>     __glXEnableDirectExtension(&psc->base, "GLX_SGI_video_sync");
>>     __glXEnableDirectExtension(&psc->base, "GLX_SGI_swap_control");
>>     __glXEnableDirectExtension(&psc->base, "GLX_MESA_swap_control");
>> @@ -1069,7 +1074,7 @@ dri2BindExtensions(struct dri2_screen *psc, const __DRIextension **extensions,
>>      * of disabling it uncondtionally, just disable it for drivers
>>      * which are known to not support it.
>>      */
>> -   if (strcmp(driverName, "vmwgfx") != 0) {
>> +   if (pdp->swapAvailable && strcmp(driverName, "vmwgfx") != 0) {
>>        __glXEnableDirectExtension(&psc->base, "GLX_INTEL_swap_event");
>>     }
>>
>> @@ -1212,8 +1217,7 @@ dri2CreateScreen(int screen, struct glx_display * priv)
>>        goto handle_error;
>>     }
>>
>> -   extensions = psc->core->getExtensions(psc->driScreen);
>> -   dri2BindExtensions(psc, extensions, driverName);
>> +   dri2BindExtensions(psc, priv, driverName);
>>
>>     configs = driConvertConfigs(psc->core, psc->base.configs, driver_configs);
>>     visuals = driConvertConfigs(psc->core, psc->base.visuals, driver_configs);
>
>
> Thanks,
>
> Paul