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

[Emil Velikov]

On Tue, 19 Feb 2019 at 16:00, Eric Engestrom <eric.engestrom at intel.com> wrote:
>
> 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 :)
>
Sure, done.

> > +      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>
>
Thanks, pushed with Gurchetan's off-list rb.

-Emil