[Mesa-stable] [PATCH 6/8] egl/dri: flesh out and use dri2_create_drawable()

Marek Olšák maraeo at gmail.com
Mon Jun 3 22:51:29 UTC 2019


Would you please review this fixed version:
https://cgit.freedesktop.org/~mareko/mesa/commit/?h=master&id=40e4702ef815410f74130f349e2b40cc0524e422

It trivially solves the GBM crash by checking that gbm_surf != NULL before
using it.

Marek

On Mon, May 20, 2019 at 3:03 PM Mathias Fröhlich <Mathias.Froehlich at gmx.net>
wrote:

> Hi,
>
> I did not have time to really look into the series, but a quick retest:
>
> wflinfo --platform=gbm -a gl
>
> still crashes on my site.
> Can you recheck that?
>
> best
>
> Mathias
>
> On Thursday, 16 May 2019 19:01:38 CEST Emil Velikov wrote:
> > From: Emil Velikov <emil.velikov at collabora.com>
> >
> > Wrap the loader->createNewDrawable() dance into a helper and use it
> > throughout the codebase.
> >
> > This addresses a cases like surfaceless (SL) on swrast (SL on kms_swrast
> > is fine) where we'd attempt using the wrong driver and crash out.
> >
> > v2: fixup quirky GBM (Mathias)
> >
> > Cc: mesa-stable at lists.freedesktop.org
> > Cc: Mathias Fröhlich <Mathias.Froehlich at web.de>
> > Reviewed-by: Mathias Fröhlich <Mathias.Froehlich at web.de> (v1)
> > Reviewed-by: Marek Olšák <marek.olsak at amd.com> (v1)
> > Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> > ---
> >  src/egl/drivers/dri2/egl_dri2.c             | 39 +++++++++++++++++++++
> >  src/egl/drivers/dri2/egl_dri2.h             |  5 +++
> >  src/egl/drivers/dri2/platform_android.c     | 12 +------
> >  src/egl/drivers/dri2/platform_drm.c         | 17 +--------
> >  src/egl/drivers/dri2/platform_surfaceless.c |  7 +---
> >  src/egl/drivers/dri2/platform_wayland.c     | 14 +-------
> >  src/egl/drivers/dri2/platform_x11.c         | 15 +-------
> >  7 files changed, 49 insertions(+), 60 deletions(-)
> >
> > diff --git a/src/egl/drivers/dri2/egl_dri2.c
> b/src/egl/drivers/dri2/egl_dri2.c
> > index 09d6315b19e..17a0176b646 100644
> > --- a/src/egl/drivers/dri2/egl_dri2.c
> > +++ b/src/egl/drivers/dri2/egl_dri2.c
> > @@ -1425,6 +1425,45 @@ dri2_surf_update_fence_fd(_EGLContext *ctx,
> >     dri2_surface_set_out_fence_fd(surf, fence_fd);
> >  }
> >
> > +static inline _EGLDisplay *
> > +dri2_display_to_egl_display(struct dri2_egl_display *dri2_dpy)
> > +{
> > +   const size_t offset = offsetof(_EGLDisplay, DriverData);
> > +   return (_EGLDisplay *)(((void*) dri2_dpy) - offset);
> > +}
> > +
> > +EGLBoolean
> > +dri2_create_drawable(struct dri2_egl_display *dri2_dpy,
> > +                     const __DRIconfig *config,
> > +                     struct dri2_egl_surface *dri2_surf)
> > +{
> > +   __DRIcreateNewDrawableFunc createNewDrawable;
> > +   _EGLDisplay *disp = dri2_display_to_egl_display(dri2_dpy);
> > +   void *loaderPrivate = dri2_surf;
> > +
> > +   if (dri2_dpy->image_driver)
> > +      createNewDrawable = dri2_dpy->image_driver->createNewDrawable;
> > +   else if (dri2_dpy->dri2)
> > +      createNewDrawable = dri2_dpy->dri2->createNewDrawable;
> > +   else if (dri2_dpy->swrast)
> > +      createNewDrawable = dri2_dpy->swrast->createNewDrawable;
> > +   else
> > +      return _eglError(EGL_BAD_ALLOC, "no createNewDrawable");
> > +
> > +   /* As always gbm is a bit special.. */
> > +#ifdef HAVE_DRM_PLATFORM
> > +   if (disp->Platform == _EGL_PLATFORM_DRM)
> > +      loaderPrivate = dri2_surf->gbm_surf;
> > +#endif
> > +
> > +   dri2_surf->dri_drawable = (*createNewDrawable)(dri2_dpy->dri_screen,
> > +                                                  config,
> loaderPrivate);
> > +   if (dri2_surf->dri_drawable == NULL)
> > +      return _eglError(EGL_BAD_ALLOC, "createNewDrawable");
> > +
> > +   return EGL_TRUE;
> > +}
> > +
> >  /**
> >   * Called via eglMakeCurrent(), drv->API.MakeCurrent().
> >   */
> > diff --git a/src/egl/drivers/dri2/egl_dri2.h
> b/src/egl/drivers/dri2/egl_dri2.h
> > index 4918517b572..5555bcea3ab 100644
> > --- a/src/egl/drivers/dri2/egl_dri2.h
> > +++ b/src/egl/drivers/dri2/egl_dri2.h
> > @@ -541,6 +541,11 @@ dri2_init_surface(_EGLSurface *surf, _EGLDisplay
> *disp, EGLint type,
> >  void
> >  dri2_fini_surface(_EGLSurface *surf);
> >
> > +EGLBoolean
> > +dri2_create_drawable(struct dri2_egl_display *dri2_dpy,
> > +                     const __DRIconfig *config,
> > +                     struct dri2_egl_surface *dri2_surf);
> > +
> >  static inline uint64_t
> >  combine_u32_into_u64(uint32_t hi, uint32_t lo)
> >  {
> > diff --git a/src/egl/drivers/dri2/platform_android.c
> b/src/egl/drivers/dri2/platform_android.c
> > index fece214d939..e44447c7df7 100644
> > --- a/src/egl/drivers/dri2/platform_android.c
> > +++ b/src/egl/drivers/dri2/platform_android.c
> > @@ -341,7 +341,6 @@ droid_create_surface(_EGLDriver *drv, _EGLDisplay
> *disp, EGLint type,
> >                   _EGLConfig *conf, void *native_window,
> >                   const EGLint *attrib_list)
> >  {
> > -   __DRIcreateNewDrawableFunc createNewDrawable;
> >     struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> >     struct dri2_egl_config *dri2_conf = dri2_egl_config(conf);
> >     struct dri2_egl_surface *dri2_surf;
> > @@ -386,17 +385,8 @@ droid_create_surface(_EGLDriver *drv, _EGLDisplay
> *disp, EGLint type,
> >        goto cleanup_surface;
> >     }
> >
> > -   if (dri2_dpy->image_driver)
> > -      createNewDrawable = dri2_dpy->image_driver->createNewDrawable;
> > -   else
> > -      createNewDrawable = dri2_dpy->dri2->createNewDrawable;
> > -
> > -   dri2_surf->dri_drawable = (*createNewDrawable)(dri2_dpy->dri_screen,
> config,
> > -                                                  dri2_surf);
> > -   if (dri2_surf->dri_drawable == NULL) {
> > -      _eglError(EGL_BAD_ALLOC, "createNewDrawable");
> > +   if (!dri2_create_drawable(dri2_dpy, config, dri2_surf))
> >        goto cleanup_surface;
> > -   }
> >
> >     if (window) {
> >        window->common.incRef(&window->common);
> > diff --git a/src/egl/drivers/dri2/platform_drm.c
> b/src/egl/drivers/dri2/platform_drm.c
> > index 7d916fe92b7..c59b9341d87 100644
> > --- a/src/egl/drivers/dri2/platform_drm.c
> > +++ b/src/egl/drivers/dri2/platform_drm.c
> > @@ -171,23 +171,8 @@ dri2_drm_create_window_surface(_EGLDriver *drv,
> _EGLDisplay *disp,
> >     dri2_surf->base.Height = surf->base.height;
> >     surf->dri_private = dri2_surf;
> >
> > -   if (dri2_dpy->dri2) {
> > -      dri2_surf->dri_drawable =
> > -         dri2_dpy->dri2->createNewDrawable(dri2_dpy->dri_screen, config,
> > -                                           dri2_surf->gbm_surf);
> > -
> > -   } else {
> > -      assert(dri2_dpy->swrast != NULL);
> > -
> > -      dri2_surf->dri_drawable =
> > -         dri2_dpy->swrast->createNewDrawable(dri2_dpy->dri_screen,
> config,
> > -                                             dri2_surf->gbm_surf);
> > -
> > -   }
> > -   if (dri2_surf->dri_drawable == NULL) {
> > -      _eglError(EGL_BAD_ALLOC, "createNewDrawable()");
> > +   if (!dri2_create_drawable(dri2_dpy, config, dri2_surf))
> >        goto cleanup_surf;
> > -   }
> >
> >     return &dri2_surf->base;
> >
> > diff --git a/src/egl/drivers/dri2/platform_surfaceless.c
> b/src/egl/drivers/dri2/platform_surfaceless.c
> > index 27b1d44ebec..ce9f7d0f980 100644
> > --- a/src/egl/drivers/dri2/platform_surfaceless.c
> > +++ b/src/egl/drivers/dri2/platform_surfaceless.c
> > @@ -136,13 +136,8 @@ dri2_surfaceless_create_surface(_EGLDriver *drv,
> _EGLDisplay *disp, EGLint type,
> >        goto cleanup_surface;
> >     }
> >
> > -   dri2_surf->dri_drawable =
> > -      dri2_dpy->image_driver->createNewDrawable(dri2_dpy->dri_screen,
> config,
> > -                                                dri2_surf);
> > -   if (dri2_surf->dri_drawable == NULL) {
> > -      _eglError(EGL_BAD_ALLOC, "image->createNewDrawable");
> > +   if (!dri2_create_drawable(dri2_dpy, config, dri2_surf))
> >        goto cleanup_surface;
> > -    }
> >
> >     if (conf->RedSize == 5)
> >        dri2_surf->visual = __DRI_IMAGE_FORMAT_RGB565;
> > diff --git a/src/egl/drivers/dri2/platform_wayland.c
> b/src/egl/drivers/dri2/platform_wayland.c
> > index 0f5d85be326..fb5ecdab2c5 100644
> > --- a/src/egl/drivers/dri2/platform_wayland.c
> > +++ b/src/egl/drivers/dri2/platform_wayland.c
> > @@ -272,7 +272,6 @@ dri2_wl_create_window_surface(_EGLDriver *drv,
> _EGLDisplay *disp,
> >                                _EGLConfig *conf, void *native_window,
> >                                const EGLint *attrib_list)
> >  {
> > -   __DRIcreateNewDrawableFunc createNewDrawable;
> >     struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> >     struct dri2_egl_config *dri2_conf = dri2_egl_config(conf);
> >     struct wl_egl_window *window = native_window;
> > @@ -349,19 +348,8 @@ dri2_wl_create_window_surface(_EGLDriver *drv,
> _EGLDisplay *disp,
> >     if (dri2_dpy->flush)
> >        dri2_surf->wl_win->resize_callback = resize_callback;
> >
> > -   if (dri2_dpy->image_driver)
> > -      createNewDrawable = dri2_dpy->image_driver->createNewDrawable;
> > -   else if (dri2_dpy->dri2)
> > -      createNewDrawable = dri2_dpy->dri2->createNewDrawable;
> > -   else
> > -      createNewDrawable = dri2_dpy->swrast->createNewDrawable;
> > -
> > -   dri2_surf->dri_drawable = (*createNewDrawable)(dri2_dpy->dri_screen,
> config,
> > -                                                  dri2_surf);
> > -    if (dri2_surf->dri_drawable == NULL) {
> > -      _eglError(EGL_BAD_ALLOC, "createNewDrawable");
> > +   if (!dri2_create_drawable(dri2_dpy, config, dri2_surf))
> >         goto cleanup_surf_wrapper;
> > -    }
> >
> >     dri2_surf->base.SwapInterval = dri2_dpy->default_swap_interval;
> >
> > diff --git a/src/egl/drivers/dri2/platform_x11.c
> b/src/egl/drivers/dri2/platform_x11.c
> > index 538cffd6a76..be9eb513243 100644
> > --- a/src/egl/drivers/dri2/platform_x11.c
> > +++ b/src/egl/drivers/dri2/platform_x11.c
> > @@ -290,21 +290,8 @@ dri2_x11_create_surface(_EGLDriver *drv,
> _EGLDisplay *disp, EGLint type,
> >        goto cleanup_pixmap;
> >     }
> >
> > -   if (dri2_dpy->dri2) {
> > -      dri2_surf->dri_drawable =
> > -         dri2_dpy->dri2->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, "dri2->createNewDrawable");
> > +   if (!dri2_create_drawable(dri2_dpy, config, dri2_surf))
> >        goto cleanup_pixmap;
> > -   }
> >
> >     if (type != EGL_PBUFFER_BIT) {
> >        cookie = xcb_get_geometry (dri2_dpy->conn, dri2_surf->drawable);
> >
>
>
>
>
> _______________________________________________
> mesa-stable mailing list
> mesa-stable at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-stable/attachments/20190603/f41bc485/attachment-0001.html>


More information about the mesa-stable mailing list