On 12 November 2017 at 07:15, sguttula <[email protected]> wrote: > This patch will fix build error when we build with_platforms=drm > and without enabling gbm. > > Chromiumos uses surfaceless platform and minigbm[not mesa-gbm] > and drm platform needed for va.When we enable drm platform , > it is failing for dependency on gbm[as we disable mesa-gbm] > at compile time.To fix this moved link dependency to runtime. > > Cc: [email protected] > Cc: Emil Velikov <[email protected]> > Cc: Deepak Sharma <[email protected]> > > Signed-off-by: sguttula <[email protected]> > Reviewed-by: Deepak Sharma <[email protected]> > --- > src/egl/Makefile.am | 2 +- > src/egl/drivers/dri2/platform_drm.c | 103 > +++++++++++++++++++++++++++++++----- > src/egl/main/egldisplay.c | 5 +- > src/gbm/main/gbm.c | 11 ---- > 4 files changed, 95 insertions(+), 26 deletions(-) > mode change 100644 => 100755 src/egl/Makefile.am > mode change 100644 => 100755 src/egl/drivers/dri2/platform_drm.c > mode change 100644 => 100755 src/egl/main/egldisplay.c > mode change 100644 => 100755 src/gbm/main/gbm.c > Hmm are you developing a Linux driver on Windows? These mode changes should not be here.
In a TL;DR: I'd suggest: - picking something like my series [1] as a base - doing the dladdr trick as 1/X, updating gbm_device::dummy and related documentation (suggest making it an ABI check) - 2/X - add separate file which provides wrappers - add small prefix say "foo_gbm...", call "dlsym_all" from dri2_initialize_drm, "close_the_handle at teardown - 3/X update existing codebase to use ^^, drop linking - ... - profit There are a few small suggestions below. [1] https://patchwork.freedesktop.org/series/33716/ Reviews will be appreciated > diff --git a/src/egl/Makefile.am b/src/egl/Makefile.am > old mode 100644 > new mode 100755 > index eeb745f..3bad977 > --- a/src/egl/Makefile.am > +++ b/src/egl/Makefile.am > @@ -94,7 +94,7 @@ dri2_backend_FILES += \ > endif > > if HAVE_PLATFORM_DRM > -libEGL_common_la_LIBADD += $(top_builddir)/src/gbm/libgbm.la > + There is a similar hunk in src/egl/meson.build. You want to drop that one as well. Namely: link_for_egl += libgbm > dri2_backend_FILES += drivers/dri2/platform_drm.c > endif > > diff --git a/src/egl/drivers/dri2/platform_drm.c > b/src/egl/drivers/dri2/platform_drm.c > old mode 100644 > new mode 100755 > index 9005f5d..f489881 > --- a/src/egl/drivers/dri2/platform_drm.c > +++ b/src/egl/drivers/dri2/platform_drm.c > @@ -39,7 +39,19 @@ > #include "egl_dri2.h" > #include "egl_dri2_fallbacks.h" > #include "loader.h" > +static void* gbm_handle =NULL; > Nit: Please follow surrounding coding style - space on each side of = > +/** Destroy the gbm device and free all resources associated with it. > + * > + * \param gbm The device created using gbm_create_device() > + */ > +GBM_EXPORT void > +gbm_device_destroy(struct gbm_device *gbm) > +{ > + gbm->refcount--; > + if (gbm->refcount == 0) > + gbm->destroy(gbm); > +} This is a _very_ bad idea. You're breaking GBM instead of moving the call from egl_dri2.c here. See my series for an example. > static struct gbm_bo * > lock_front_buffer(struct gbm_surface *_surf) > { > @@ -169,11 +181,18 @@ dri2_drm_destroy_surface(_EGLDriver *drv, _EGLDisplay > *disp, _EGLSurface *surf) > struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp); > struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf); > > + void (* gbm_fp)(); > dri2_dpy->core->destroyDrawable(dri2_surf->dri_drawable); > > for (unsigned i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) { > - if (dri2_surf->color_buffers[i].bo) > - gbm_bo_destroy(dri2_surf->color_buffers[i].bo); > + if (dri2_surf->color_buffers[i].bo){ > + gbm_fp = dlsym(gbm_handle,"gbm_bo_destroy"); This will (and below) lead to a lot of unnecessary dlsym calls. You can do it once, as mentioned above. > @@ -660,6 +713,15 @@ dri2_initialize_drm(_EGLDriver *drv, _EGLDisplay *disp) > > dri2_dpy->fd = -1; > disp->DriverData = (void *) dri2_dpy; > +#if defined(__x86_64__) > + gbm_handle = dlopen("/usr/lib64/libgbm.so.1", RTLD_LAZY); > +#else > + gbm_handle = dlopen("/usr/lib/libgbm.so.1", RTLD_LAZY); Why are you hard coding the path? Please don't use LAZY -> LOCAL | NOW is a wiser move. > @@ -124,7 +126,8 @@ _eglNativePlatformDetectNativeDisplay(void *nativeDisplay) > > #ifdef HAVE_DRM_PLATFORM > /* gbm has a pointer to its constructor as first element. */ Nit: Comment should be fixed up. Thanks Emil _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
