[virglrenderer-devel] [PATCH 1/2] virgl/egl: Add option to use the surfaceless platform

[emil.velikov at collabora.com]

Hi Tomeu,

Sharing a few fly-by comments:

On Fri, Apr 20, 2018 at 04:16:48PM +0200, Tomeu Vizoso wrote:
> To make it easier to run the test suite on environments without graphics
> hardware, add an environment variable VIRGL_EGL_SURFACELESS that will
> force the use of the surfaceless platform.
> 
> Combined with the following flags, we can run virglrenderer and its
> tests without any graphics hardware or windowing system present:
> 
> LIBGL_ALWAYS_SOFTWARE=true GALLIUM_DRIVER=llvmpipe VIRGL_EGL_SURFACELESS=yes
> 

1)
FWIW there should be _no_ need to force either SW rendering or LLVMpipe
in particular. They will be selected as:
 a) HW rendering fails - missing driver/hardware, etc.
 b) LLVMpipe will be attempted before softpipe, assuming it's built

2)
I think that Elie was looking for a way to plumb the GL (desktop vs GLES)
type down to virgl_egl_init. Might be worth expanding that to cover the
EGL platform type. Just an idea for the long run.

>     struct virgl_egl *d;
> +   bool surfaceless = getenv("VIRGL_EGL_SURFACELESS");
>  
>     d = malloc(sizeof(struct virgl_egl));
>     if (!d)
>        return NULL;
>  
> -   d->fd = egl_rendernode_open();
> -   if (d->fd == -1)
> -      goto fail;
> -   d->gbm_dev = gbm_create_device(d->fd);
> -   if (!d->gbm_dev)
> -      goto fail;
> +   if (surfaceless) {
> +      conf_att[1] = EGL_PBUFFER_BIT;
> +      d->fd = -1;
> +      d->gbm_dev = NULL;
> +   } else {
> +      d->fd = egl_rendernode_open();
> +      if (d->fd == -1)
> +         goto fail;
> +      d->gbm_dev = gbm_create_device(d->fd);
> +      if (!d->gbm_dev)
> +         goto fail;
> +   }
>  
>     const char *client_extensions = eglQueryString (NULL, EGL_EXTENSIONS);
>  
> @@ -161,14 +168,26 @@ struct virgl_egl *virgl_egl_init(void)
>        PFNEGLGETPLATFORMDISPLAYEXTPROC get_platform_display =
>           (PFNEGLGETPLATFORMDISPLAYEXTPROC) eglGetProcAddress ("eglGetPlatformDisplay");
>  
> -      if (get_platform_display)
> +      if (!get_platform_display)
> +        goto fail;
> +
There is no reason why we cannot fallback to using the EXT extension or
the old-school eglGetDisplay. Regardless - it's something the existing
code does, not something that should be changed here.

Think I'll follow-up with a patch addressing that plus couple related
fixes.

HTH
Emil