[Mesa-dev] egl: Add swrast support to surfaceless platform

Submitted by Akihiko Odaki on Aug. 1, 2017, 5:49 a.m.

Details

Message ID 20170801054932.4408-1-nekomanma@pixiv.co.jp
State New
Headers show
Series "egl: Add swrast support to surfaceless platform" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Akihiko Odaki Aug. 1, 2017, 5:49 a.m.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101397
---
 src/egl/drivers/dri2/platform_surfaceless.c | 114 ++++++++++++++++++++++++++--
 src/gallium/state_trackers/dri/drisw.c      |  45 ++++++++++-
 2 files changed, 148 insertions(+), 11 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/egl/drivers/dri2/platform_surfaceless.c b/src/egl/drivers/dri2/platform_surfaceless.c
index 1091b4febd..5487c89816 100644
--- a/src/egl/drivers/dri2/platform_surfaceless.c
+++ b/src/egl/drivers/dri2/platform_surfaceless.c
@@ -133,9 +133,16 @@  dri2_surfaceless_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type,
    if (!config)
       goto cleanup_surface;
 
-   dri2_surf->dri_drawable =
-      dri2_dpy->image_driver->createNewDrawable(dri2_dpy->dri_screen, config,
-                                                dri2_surf);
+   if (dri2_dpy->image_driver) {
+      dri2_surf->dri_drawable =
+         dri2_dpy->image_driver->createNewDrawable(dri2_dpy->dri_screen, config,
+                                                   dri2_surf);
+   } else {
+      assert(dri2_dpy->swrast);
+      dri2_surf->dri_drawable =
+         dri2_dpy->swrast->createNewDrawable(dri2_dpy->dri_screen, config,
+                                             dri2_surf);
+   }
    if (dri2_surf->dri_drawable == NULL) {
       _eglError(EGL_BAD_ALLOC, "image->createNewDrawable");
       goto cleanup_surface;
@@ -229,7 +236,24 @@  surfaceless_add_configs_for_visuals(_EGLDriver *drv, _EGLDisplay *dpy)
    return (config_count != 0);
 }
 
-static const struct dri2_egl_display_vtbl dri2_surfaceless_display_vtbl = {
+static const struct dri2_egl_display_vtbl dri2_surfaceless_swrast_display_vtbl = {
+   .create_pixmap_surface = dri2_fallback_create_pixmap_surface,
+   .create_pbuffer_surface = dri2_surfaceless_create_pbuffer_surface,
+   .destroy_surface = surfaceless_destroy_surface,
+   .create_image = dri2_create_image_khr,
+   .swap_interval = dri2_fallback_swap_interval,
+   .swap_buffers = surfaceless_swap_buffers,
+   .swap_buffers_region = dri2_fallback_swap_buffers_region,
+   .set_damage_region = dri2_fallback_set_damage_region,
+   .post_sub_buffer = dri2_fallback_post_sub_buffer,
+   .copy_buffers = dri2_fallback_copy_buffers,
+   .query_buffer_age = dri2_fallback_query_buffer_age,
+   .create_wayland_buffer_from_image = dri2_fallback_create_wayland_buffer_from_image,
+   .get_sync_values = dri2_fallback_get_sync_values,
+   .get_dri_drawable = dri2_surface_get_dri_drawable,
+};
+
+static const struct dri2_egl_display_vtbl dri2_surfaceless_dri3_display_vtbl = {
    .create_pixmap_surface = dri2_fallback_create_pixmap_surface,
    .create_pbuffer_surface = dri2_surfaceless_create_pbuffer_surface,
    .destroy_surface = surfaceless_destroy_surface,
@@ -252,6 +276,66 @@  surfaceless_flush_front_buffer(__DRIdrawable *driDrawable, void *loaderPrivate)
 {
 }
 
+static const __DRIextension *swrast_loader_extensions[] = {
+   NULL,
+};
+
+static EGLBoolean
+dri2_initialize_surfaceless_swrast(_EGLDriver *drv, _EGLDisplay *disp)
+{
+   struct dri2_egl_display *dri2_dpy;
+   const char* err;
+
+   loader_set_logger(_eglLog);
+
+   dri2_dpy = calloc(1, sizeof *dri2_dpy);
+   if (!dri2_dpy)
+      return _eglError(EGL_BAD_ALLOC, "eglInitialize");
+
+   dri2_dpy->fd = -1;
+   disp->DriverData = (void *) dri2_dpy;
+
+   /*
+    * Every hardware driver_name is set using strdup. Doing the same in
+    * here will allow is to simply free the memory at dri2_terminate().
+    */
+   dri2_dpy->driver_name = strdup("swrast");
+   if (!dri2_load_driver_swrast(disp)) {
+      err = "DRI2: failed to load driver";
+      goto cleanup;
+   }
+
+   dri2_dpy->loader_extensions = swrast_loader_extensions;
+
+   if (!dri2_create_screen(disp)) {
+      err = "DRI2: failed to create screen";
+      goto cleanup;
+   }
+
+   if (!dri2_setup_extensions(disp)) {
+      err = "DRI2: failed to find required DRI extensions";
+      goto cleanup;
+   }
+
+   dri2_setup_screen(disp);
+
+   if (!surfaceless_add_configs_for_visuals(drv, disp)) {
+      err = "DRI2: failed to add configs";
+      goto cleanup;
+   }
+
+   /* Fill vtbl last to prevent accidentally calling virtual function during
+    * initialization.
+    */
+   dri2_dpy->vtbl = &dri2_surfaceless_swrast_display_vtbl;
+
+   return EGL_TRUE;
+
+ cleanup:
+   dri2_display_destroy(disp);
+   return _eglError(EGL_NOT_INITIALIZED, err);
+}
+
 static const __DRIimageLoaderExtension image_loader_extension = {
    .base             = { __DRI_IMAGE_LOADER, 1 },
    .getBuffers       = surfaceless_image_get_buffers,
@@ -267,8 +351,8 @@  static const __DRIextension *image_loader_extensions[] = {
    NULL,
 };
 
-EGLBoolean
-dri2_initialize_surfaceless(_EGLDriver *drv, _EGLDisplay *disp)
+static EGLBoolean
+dri2_initialize_surfaceless_dri3(_EGLDriver *drv, _EGLDisplay *disp)
 {
    struct dri2_egl_display *dri2_dpy;
    const char* err;
@@ -336,7 +420,7 @@  dri2_initialize_surfaceless(_EGLDriver *drv, _EGLDisplay *disp)
    /* Fill vtbl last to prevent accidentally calling virtual function during
     * initialization.
     */
-   dri2_dpy->vtbl = &dri2_surfaceless_display_vtbl;
+   dri2_dpy->vtbl = &dri2_surfaceless_dri3_display_vtbl;
 
    return EGL_TRUE;
 
@@ -344,3 +428,19 @@  cleanup:
    dri2_display_destroy(disp);
    return _eglError(EGL_NOT_INITIALIZED, err);
 }
+
+EGLBoolean
+dri2_initialize_surfaceless(_EGLDriver *drv, _EGLDisplay *disp)
+{
+   EGLBoolean initialized = EGL_FALSE;
+
+   if (!getenv("LIBGL_ALWAYS_SOFTWARE")) {
+      initialized = dri2_initialize_surfaceless_dri3(drv, disp);
+   }
+
+   if (!initialized) {
+      initialized = dri2_initialize_surfaceless_swrast(drv, disp);
+   }
+
+   return initialized;
+}
diff --git a/src/gallium/state_trackers/dri/drisw.c b/src/gallium/state_trackers/dri/drisw.c
index ac4095618c..0d1825defa 100644
--- a/src/gallium/state_trackers/dri/drisw.c
+++ b/src/gallium/state_trackers/dri/drisw.c
@@ -52,6 +52,10 @@ 
 DEBUG_GET_ONCE_BOOL_OPTION(swrast_no_present, "SWRAST_NO_PRESENT", FALSE);
 static boolean swrast_no_present = FALSE;
 
+/* Check dPriv->driScreenPriv->swrast_loader before calling this function
+ * since this function refers to the pointer and it would be NULL for offscreen
+ * rendering.
+ */
 static inline void
 get_drawable_info(__DRIdrawable *dPriv, int *x, int *y, int *w, int *h)
 {
@@ -63,6 +67,9 @@  get_drawable_info(__DRIdrawable *dPriv, int *x, int *y, int *w, int *h)
                            dPriv->loaderPrivate);
 }
 
+/* Check dPriv->driScreenPriv->swrast_loader before calling this function
+ * (c.f. get_drawable_info)
+ */
 static inline void
 put_image(__DRIdrawable *dPriv, void *data, unsigned width, unsigned height)
 {
@@ -74,6 +81,9 @@  put_image(__DRIdrawable *dPriv, void *data, unsigned width, unsigned height)
                     data, dPriv->loaderPrivate);
 }
 
+/* Check dPriv->driScreenPriv->swrast_loader before calling this function
+ * (c.f. get_drawable_info)
+ */
 static inline void
 put_image2(__DRIdrawable *dPriv, void *data, int x, int y,
            unsigned width, unsigned height, unsigned stride)
@@ -86,6 +96,9 @@  put_image2(__DRIdrawable *dPriv, void *data, int x, int y,
                      data, dPriv->loaderPrivate);
 }
 
+/* Check dPriv->driScreenPriv->swrast_loader before calling this function
+ * (c.f. get_drawable_info)
+ */
 static inline void
 get_image(__DRIdrawable *dPriv, int x, int y, int width, int height, void *data)
 {
@@ -97,6 +110,9 @@  get_image(__DRIdrawable *dPriv, int x, int y, int width, int height, void *data)
                     data, dPriv->loaderPrivate);
 }
 
+/* Check dPriv->driScreenPriv->swrast_loader before calling this function
+ * (c.f. get_drawable_info)
+ */
 static inline void
 get_image2(__DRIdrawable *dPriv, int x, int y, int width, int height, int stride, void *data)
 {
@@ -112,6 +128,9 @@  get_image2(__DRIdrawable *dPriv, int x, int y, int width, int height, int stride
                      data, dPriv->loaderPrivate);
 }
 
+/* Check drawable->dPriv->driScreenPriv->swrast_loader before calling this
+ * function (c.f. get_drawable_info)
+ */
 static void
 drisw_update_drawable_info(struct dri_drawable *drawable)
 {
@@ -121,6 +140,9 @@  drisw_update_drawable_info(struct dri_drawable *drawable)
    get_drawable_info(dPriv, &x, &y, &dPriv->w, &dPriv->h);
 }
 
+/* Check drawable->dPriv->driScreenPriv->swrast_loader before calling this
+ * function (c.f. get_drawable_info and get_image2)
+ */
 static void
 drisw_get_image(struct dri_drawable *drawable,
                 int x, int y, unsigned width, unsigned height, unsigned stride,
@@ -133,6 +155,9 @@  drisw_get_image(struct dri_drawable *drawable,
    get_image2(dPriv, x, y, draw_w, draw_h, stride, data);
 }
 
+/* Check drawable->dPriv->driScreenPriv->swrast_loader before calling this function
+ * (c.f. put_image)
+ */
 static void
 drisw_put_image(struct dri_drawable *drawable,
                 void *data, unsigned width, unsigned height)
@@ -142,6 +167,9 @@  drisw_put_image(struct dri_drawable *drawable,
    put_image(dPriv, data, width, height);
 }
 
+/* Check drawable->dPriv->driScreenPriv->swrast_loader before calling this function
+ * (c.f. put_image2)
+ */
 static void
 drisw_put_image2(struct dri_drawable *drawable,
                  void *data, int x, int y, unsigned width, unsigned height,
@@ -313,7 +341,7 @@  drisw_allocate_textures(struct dri_context *stctx,
 
       if (statts[i] == ST_ATTACHMENT_FRONT_LEFT &&
           screen->base.screen->resource_create_front &&
-          loader->base.version >= 3) {
+          loader && loader->base.version >= 3) {
          drawable->textures[statts[i]] =
             screen->base.screen->resource_create_front(screen->base.screen, &templ, (const void *)drawable);
       } else
@@ -325,6 +353,9 @@  drisw_allocate_textures(struct dri_context *stctx,
    drawable->old_h = height;
 }
 
+/* Check drawable->dPriv->driScreenPriv->swrast_loader before calling this
+ * function (c.f. get_drawable_info and get_image)
+ */
 static void
 drisw_update_tex_buffer(struct dri_drawable *drawable,
                         struct dri_context *ctx,
@@ -375,6 +406,9 @@  static const __DRIextension *drisw_screen_extensions[] = {
    NULL
 };
 
+/* Check drawable->dPriv->driScreenPriv->swrast_loader before referring to
+ * functions referred by this structure.
+ */
 static struct drisw_loader_funcs drisw_lf = {
    .get_image = drisw_get_image,
    .put_image = drisw_put_image,
@@ -402,7 +436,7 @@  drisw_init_screen(__DRIscreen * sPriv)
 
    unsigned flags = dri_init_options_get_screen_flags(screen, "swrast");
 
-   if (pipe_loader_sw_probe_dri(&screen->dev, &drisw_lf))
+   if (pipe_loader_sw_probe_dri(&screen->dev, sPriv->swrast_loader ? &drisw_lf : NULL))
       pscreen = pipe_loader_create_screen(screen->dev, flags);
 
    if (!pscreen)
@@ -434,9 +468,12 @@  drisw_create_buffer(__DRIscreen * sPriv,
    drawable = dPriv->driverPrivate;
 
    drawable->allocate_textures = drisw_allocate_textures;
-   drawable->update_drawable_info = drisw_update_drawable_info;
    drawable->flush_frontbuffer = drisw_flush_frontbuffer;
-   drawable->update_tex_buffer = drisw_update_tex_buffer;
+
+   if (sPriv->swrast_loader) {
+      drawable->update_drawable_info = drisw_update_drawable_info;
+      drawable->update_tex_buffer = drisw_update_tex_buffer;
+   }
 
    return TRUE;
 }

Comments

[+Chad]

Hi Akihiko Odaki,

Thank you for the patch and welcome to Mesa!

How you tested this patch? Did you run a test suite like piglit, dEQP, etc?
Can you give it a spin with either one of these, comparing HW vs
swrast surfacess and share the results.

On 1 August 2017 at 06:49, Akihiko Odaki <nekomanma@pixiv.co.jp> wrote:
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101397
> ---
>  src/egl/drivers/dri2/platform_surfaceless.c | 114 ++++++++++++++++++++++++++--
>  src/gallium/state_trackers/dri/drisw.c      |  45 ++++++++++-
A general rule of thumb - the difference in paths indicate these two
are distinct components.
As such they ought to be separate patches.

But before that, please look at my other comment below.

>  2 files changed, 148 insertions(+), 11 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/platform_surfaceless.c b/src/egl/drivers/dri2/platform_surfaceless.c
> index 1091b4febd..5487c89816 100644
> --- a/src/egl/drivers/dri2/platform_surfaceless.c
> +++ b/src/egl/drivers/dri2/platform_surfaceless.c
> @@ -133,9 +133,16 @@ dri2_surfaceless_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type,
>     if (!config)
>        goto cleanup_surface;
>
> -   dri2_surf->dri_drawable =
> -      dri2_dpy->image_driver->createNewDrawable(dri2_dpy->dri_screen, config,
> -                                                dri2_surf);
> +   if (dri2_dpy->image_driver) {
> +      dri2_surf->dri_drawable =
> +         dri2_dpy->image_driver->createNewDrawable(dri2_dpy->dri_screen, config,
> +                                                   dri2_surf);
> +   } else {
> +      assert(dri2_dpy->swrast);
> +      dri2_surf->dri_drawable =
> +         dri2_dpy->swrast->createNewDrawable(dri2_dpy->dri_screen, config,
> +                                             dri2_surf);
> +   }
>     if (dri2_surf->dri_drawable == NULL) {
>        _eglError(EGL_BAD_ALLOC, "image->createNewDrawable");
>        goto cleanup_surface;
> @@ -229,7 +236,24 @@ surfaceless_add_configs_for_visuals(_EGLDriver *drv, _EGLDisplay *dpy)
>     return (config_count != 0);
>  }
>
> -static const struct dri2_egl_display_vtbl dri2_surfaceless_display_vtbl = {
> +static const struct dri2_egl_display_vtbl dri2_surfaceless_swrast_display_vtbl = {
> +   .create_pixmap_surface = dri2_fallback_create_pixmap_surface,
> +   .create_pbuffer_surface = dri2_surfaceless_create_pbuffer_surface,
> +   .destroy_surface = surfaceless_destroy_surface,
> +   .create_image = dri2_create_image_khr,
> +   .swap_interval = dri2_fallback_swap_interval,
> +   .swap_buffers = surfaceless_swap_buffers,
> +   .swap_buffers_region = dri2_fallback_swap_buffers_region,
> +   .set_damage_region = dri2_fallback_set_damage_region,
> +   .post_sub_buffer = dri2_fallback_post_sub_buffer,
> +   .copy_buffers = dri2_fallback_copy_buffers,
> +   .query_buffer_age = dri2_fallback_query_buffer_age,
> +   .create_wayland_buffer_from_image = dri2_fallback_create_wayland_buffer_from_image,
> +   .get_sync_values = dri2_fallback_get_sync_values,
> +   .get_dri_drawable = dri2_surface_get_dri_drawable,
> +};
> +
> +static const struct dri2_egl_display_vtbl dri2_surfaceless_dri3_display_vtbl = {
>     .create_pixmap_surface = dri2_fallback_create_pixmap_surface,
>     .create_pbuffer_surface = dri2_surfaceless_create_pbuffer_surface,
>     .destroy_surface = surfaceless_destroy_surface,
> @@ -252,6 +276,66 @@ surfaceless_flush_front_buffer(__DRIdrawable *driDrawable, void *loaderPrivate)
>  {
>  }
>
> +static const __DRIextension *swrast_loader_extensions[] = {
> +   NULL,
I'm not 100% sure if it makes sense, for swrast at least, to have no
loader extensions.

Need to think about this a bit more. Although a related series comes
to mind [1].
It adds partial DRI_IMAGE support to state_trackers/dri/drisw.

I'm wondering if we cannot reuse/share some goals across the board.

Thanks
Emil

[1] https://patchwork.freedesktop.org/project/mesa/patches/?submitter=16100&state=&q=&archive=&delegate=
Would kms_swrast + vgem for surfaceless also work for the use case raised
in the bug, or is that a no-go because it assumes the presence of a
driver?  I've been testing it[1], it works pretty well, except for one
issue during ChromeOS startup.

[1] https://chromium-review.googlesource.com/c/558218

On Wed, Aug 2, 2017 at 4:05 AM, Emil Velikov <emil.l.velikov@gmail.com>
wrote:

> [+Chad]
>
> Hi Akihiko Odaki,
>
> Thank you for the patch and welcome to Mesa!
>
> How you tested this patch? Did you run a test suite like piglit, dEQP, etc?
> Can you give it a spin with either one of these, comparing HW vs
> swrast surfacess and share the results.
>
> On 1 August 2017 at 06:49, Akihiko Odaki <nekomanma@pixiv.co.jp> wrote:
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101397
> > ---
> >  src/egl/drivers/dri2/platform_surfaceless.c | 114
> ++++++++++++++++++++++++++--
> >  src/gallium/state_trackers/dri/drisw.c      |  45 ++++++++++-
> A general rule of thumb - the difference in paths indicate these two
> are distinct components.
> As such they ought to be separate patches.
>
> But before that, please look at my other comment below.
>
> >  2 files changed, 148 insertions(+), 11 deletions(-)
> >
> > diff --git a/src/egl/drivers/dri2/platform_surfaceless.c
> b/src/egl/drivers/dri2/platform_surfaceless.c
> > index 1091b4febd..5487c89816 100644
> > --- a/src/egl/drivers/dri2/platform_surfaceless.c
> > +++ b/src/egl/drivers/dri2/platform_surfaceless.c
> > @@ -133,9 +133,16 @@ dri2_surfaceless_create_surface(_EGLDriver *drv,
> _EGLDisplay *disp, EGLint type,
> >     if (!config)
> >        goto cleanup_surface;
> >
> > -   dri2_surf->dri_drawable =
> > -      dri2_dpy->image_driver->createNewDrawable(dri2_dpy->dri_screen,
> config,
> > -                                                dri2_surf);
> > +   if (dri2_dpy->image_driver) {
> > +      dri2_surf->dri_drawable =
> > +         dri2_dpy->image_driver->createNewDrawable(dri2_dpy->dri_screen,
> config,
> > +                                                   dri2_surf);
> > +   } else {
> > +      assert(dri2_dpy->swrast);
> > +      dri2_surf->dri_drawable =
> > +         dri2_dpy->swrast->createNewDrawable(dri2_dpy->dri_screen,
> config,
> > +                                             dri2_surf);
> > +   }
> >     if (dri2_surf->dri_drawable == NULL) {
> >        _eglError(EGL_BAD_ALLOC, "image->createNewDrawable");
> >        goto cleanup_surface;
> > @@ -229,7 +236,24 @@ surfaceless_add_configs_for_visuals(_EGLDriver
> *drv, _EGLDisplay *dpy)
> >     return (config_count != 0);
> >  }
> >
> > -static const struct dri2_egl_display_vtbl dri2_surfaceless_display_vtbl
> = {
> > +static const struct dri2_egl_display_vtbl dri2_surfaceless_swrast_display_vtbl
> = {
> > +   .create_pixmap_surface = dri2_fallback_create_pixmap_surface,
> > +   .create_pbuffer_surface = dri2_surfaceless_create_pbuffer_surface,
> > +   .destroy_surface = surfaceless_destroy_surface,
> > +   .create_image = dri2_create_image_khr,
> > +   .swap_interval = dri2_fallback_swap_interval,
> > +   .swap_buffers = surfaceless_swap_buffers,
> > +   .swap_buffers_region = dri2_fallback_swap_buffers_region,
> > +   .set_damage_region = dri2_fallback_set_damage_region,
> > +   .post_sub_buffer = dri2_fallback_post_sub_buffer,
> > +   .copy_buffers = dri2_fallback_copy_buffers,
> > +   .query_buffer_age = dri2_fallback_query_buffer_age,
> > +   .create_wayland_buffer_from_image = dri2_fallback_create_wayland_b
> uffer_from_image,
> > +   .get_sync_values = dri2_fallback_get_sync_values,
> > +   .get_dri_drawable = dri2_surface_get_dri_drawable,
> > +};
> > +
> > +static const struct dri2_egl_display_vtbl dri2_surfaceless_dri3_display_vtbl
> = {
> >     .create_pixmap_surface = dri2_fallback_create_pixmap_surface,
> >     .create_pbuffer_surface = dri2_surfaceless_create_pbuffer_surface,
> >     .destroy_surface = surfaceless_destroy_surface,
> > @@ -252,6 +276,66 @@ surfaceless_flush_front_buffer(__DRIdrawable
> *driDrawable, void *loaderPrivate)
> >  {
> >  }
> >
> > +static const __DRIextension *swrast_loader_extensions[] = {
> > +   NULL,
> I'm not 100% sure if it makes sense, for swrast at least, to have no
> loader extensions.
>
> Need to think about this a bit more. Although a related series comes
> to mind [1].
> It adds partial DRI_IMAGE support to state_trackers/dri/drisw.
>
> I'm wondering if we cannot reuse/share some goals across the board.
>
> Thanks
> Emil
>
> [1] https://patchwork.freedesktop.org/project/mesa/patches/?subm
> itter=16100&state=&q=&archive=&delegate=
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>