[Mesa-dev] [PATCH v1 1/2] egl/android: #ifdef out flink name support

[Robert Foss]

Hey,

On 2018-05-01 08:29, Tomasz Figa wrote:
> On Tue, May 1, 2018 at 11:20 AM Rob Herring <robh at kernel.org> wrote:
> 
>> On Fri, Apr 27, 2018 at 6:57 AM, Robert Foss <robert.foss at collabora.com>
> wrote:
>>> From: Rob Herring <robh at kernel.org>
> [snip]
>>> @@ -1228,20 +1254,31 @@ dri2_initialize_android(_EGLDriver *drv,
> _EGLDisplay *disp)
>>>
>>>      dri2_dpy->is_render_node = drmGetNodeTypeFromFd(dri2_dpy->fd) ==
> DRM_NODE_RENDER;
>>>
>>> -   /* render nodes cannot use Gem names, and thus do not support
>>> -    * the __DRI_DRI2_LOADER extension */
>>>      if (!dri2_dpy->is_render_node) {
>>> +#ifdef HAVE_DRM_GRALLOC
>>>         dri2_dpy->loader_extensions = droid_dri2_loader_extensions;
>>>         if (!dri2_load_driver(disp)) {
>>>            err = "DRI2: failed to load driver";
>>>            goto cleanup;
>>>         }
>>>      } else {
>>> +      /* render nodes cannot use Gem names, and thus do not support
>>> +       * the __DRI_DRI2_LOADER extension */
> 
> I think we can replace this comment with something properly reflecting the
> bad nature of this code. See below.
> 
>>>         dri2_dpy->loader_extensions = droid_image_loader_extensions;
>>>         if (!dri2_load_driver_dri3(disp)) {
>>>            err = "DRI3: failed to load driver";
>>>            goto cleanup;
>>>         }
>>> +#else
>>> +      err = "DRI2: handle is not for a render node";
>>> +      goto cleanup;
>>> +   }
>>> +
>>> +   dri2_dpy->loader_extensions = droid_image_loader_extensions;
>>> +   if (!dri2_load_driver_dri3(disp)) {
> 
>> Do we really need this twice?
> 
> Yeah, I think we could do with something like below:
> 
> if (!dri2_dpy->is_render_node) {
> #ifdef HAVE_DRM_GRALLOC
>      /* Handle control nodes using __DRI_DRI2_LOADER extension and GEM names
>       * for backwards compatibility with drm_gralloc. (Do not use on new
> systems.) */
>      dri2_dpy->loader_extensions = droid_dri2_loader_extensions;
>      if (!dri2_load_driver(disp)) {
>         err = "DRI2: failed to load driver";
>         goto cleanup;
>      }
> #else
>      err = "DRI2: handle is not for a render node";
>      goto cleanup;
> #endif
> } else {
>      dri2_dpy->loader_extensions = droid_image_loader_extensions;
>      if (!dri2_load_driver_dri3(disp)) {
>         err = "DRI3: failed to load driver";
>         goto cleanup;
>      }
> }

This does look better to me too, I'll use it for v2.


Rob.

> 
> Best regards,
> Tomasz
>