[Mesa-dev] [PATCH v2 3/3] egl/sl: use kms_swrast with vgem instead of a random GPU

[Eric Engestrom]

On Tuesday, 2019-02-19 14:08:08 +0000, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov at collabora.com>
> 
> VGEM and kms_swrast were introduced to work with one another.
> 
> All we do is CPU rendering to dumb buffers. There is no reason to carve
> out GPU memory, increasing the memory pressure on a device that could
> make a better use of it.
> 
> Note:
>  - The original code did not work out of the box, since the dumb buffer
> ioctls are not exposed to render nodes.
>  - This requires libdrm commit 3df8a7f0 ("xf86drm: fallback to MODALIAS
> for OF less platform devices")
>  - The non-kms, swrast is unaffected by this change.
> 
> v2:
>  - elaborate what and how is/isn't working (Eric)
>  - simplify driver_name handling (Eric)
> 
> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> ---
>  src/egl/drivers/dri2/platform_surfaceless.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/src/egl/drivers/dri2/platform_surfaceless.c b/src/egl/drivers/dri2/platform_surfaceless.c
> index ccdc370d059..0917c15e16d 100644
> --- a/src/egl/drivers/dri2/platform_surfaceless.c
> +++ b/src/egl/drivers/dri2/platform_surfaceless.c
> @@ -286,10 +286,11 @@ surfaceless_probe_device(_EGLDisplay *disp, bool swrast)
>     for (i = 0; i < num_devices; ++i) {
>        device = devices[i];
>  
> -      if (!(device->available_nodes & (1 << DRM_NODE_RENDER)))
> +      const unsigned node_type = swrast ? DRM_NODE_PRIMARY : DRM_NODE_RENDER;

Nittiest of nits: this could be outside the loop :)

> +      if (!(device->available_nodes & (1 << node_type)))
>           continue;
>  
> -      dri2_dpy->fd = loader_open_device(device->nodes[DRM_NODE_RENDER]);
> +      dri2_dpy->fd = loader_open_device(device->nodes[node_type]);
>        if (dri2_dpy->fd < 0)
>           continue;
>  
> @@ -300,10 +301,16 @@ surfaceless_probe_device(_EGLDisplay *disp, bool swrast)
>           continue;
>        }
>  
> -      if (swrast)
> -         dri2_dpy->driver_name = strdup("kms_swrast");
> -      else
> -         dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd);
> +      char *driver_name = loader_get_driver_for_fd(dri2_dpy->fd);
> +      if (swrast) {
> +         /* Use kms swrast only with vgem */
> +         if (strcmp(driver_name, "vgem") == 0)
> +            dri2_dpy->driver_name = strdup("kms_swrast");
> +         free(driver_name);
> +      } else {
> +         /* Use the given hardware driver */
> +         dri2_dpy->driver_name = driver_name;
> +      }

That's easier to follow, thanks!

Please wait for the chromium guys to weigh in, but:
Reviewed-by: Eric Engestrom <eric.engestrom at intel.com>

>  
>        if (dri2_dpy->driver_name && dri2_load_driver_dri3(disp))
>           break;
> -- 
> 2.20.1
>