[Mesa-dev] [PATCH 2/4] loader: don't limit the non-udev path to only android
Emil Velikov
emil.l.velikov at gmail.com
Sat Mar 22 12:22:09 PDT 2014
On 22/03/14 11:54, Jonathan Gray wrote:
> On Sat, Mar 22, 2014 at 11:45:35AM +0000, Emil Velikov wrote:
>> On 22/03/14 04:05, Jonathan Gray wrote:
>>> On Sat, Mar 22, 2014 at 12:10:26AM +0000, Emil Velikov wrote:
>>>> On 19/03/14 01:06, Jonathan Gray wrote:
>>>>> On Tue, Mar 18, 2014 at 07:56:21PM +0000, Emil Velikov wrote:
>>>>>> On 18/03/14 14:59, Jonathan Gray wrote:
>>>>>>> Signed-off-by: Jonathan Gray <jsg at jsg.id.au>
>>>>>>> ---
>>>>>> Hi Jonathan
>>>>>>
>>>>>> While the summary covers what the patch does, the *ahem* commit message
>>>>>> fails to explain why it's needed. AFAICS this will cause some very nasty
>>>>>> breakage in some cases, which we want to avoid without a valid reason.
>>>>>>
>>>>>> -Emil
>>>>>
>>>>> The summary is the commit message though?
>>>>>
>>>>> Anyway without this I can't load dri drivers at all on OpenBSD
>>>>> with mesa 10.x. FreeBSD/NetBSD/Solaris/etc would also be broken
>>>>> which strikes me as rather serious breakage...
>>>>>
>>>> Kind of expecting to see a fraction of the above in the commit message,
>>>> maybe I was expecting too much.
>>>>
>>>> Wrt "will cause some very nasty breakage" I take that back, as I've
>>>> missed the commit that enforces libudev on linux, which handles those
>>>> lovely scenarios.
>>>>
>>>> FWIW For patches 1, 2 and 4.
>>>> Reviewed-by: Emil Velikov <emil.l.velikov at gmail.com>
>>>
>>> Thanks, I can send another patch with more explanation in the
>>> commit message if you like.
>>>
>> No problems, just as a future reference it would be nice :)
>>
>>>>
>>>> Btw, if you're looking for egl, wayland, opencl etc. similar fixes to
>>>> patch 3 may be needed.
>>>
>>> The sticking point for egl with non x11 platforms has been
>>> the udev dependency in gbm. Though it seems the loader changes have
>>> removed this, the configure script still wants libudev to configure gbm.
>>>
>>> The drm platform has the same issue issue with configure.
>>>
>>> libgbm and drm platform support in egl seem to build and install fine
>>> with the following patch for example:
>>>
>> Build is the smallest problem here. There is some deeper hacking
>> required, as the loader functions are executed before the
>> wayland/egl/gbm auth hooks are setup, let alone executed. I've never
>> been too deep into the respective codebases so I could be wrong.
>>
>> IMHO it would be better to hold off the following patch until we make
>> sure that the resulting binaries work.
>>
>> -Emil
>
> Well that and two other small patches result in being able
> to use glamoregl with the intel driver here
>
> [ 1003.926] (II) Loading sub module "glamoregl"
> [ 1003.926] (II) LoadModule: "glamoregl"
> [ 1003.928] (II) Loading /usr/X11R6/lib/modules/libglamoregl.so
> [ 1003.929] (II) Module glamoregl: vendor="X.Org Foundation"
> [ 1003.929] compiled for 1.14.5, module version = 0.6.0
> [ 1003.929] ABI class: X.Org ANSI C Emulation, version 0.4
> [ 1003.929] (II) glamor: OpenGL accelerated X.org driver based.
> [ 1003.960] (II) glamor: EGL version 1.4 (DRI2):
> [ 1003.974] (II) intel(0): glamor detected, initialising egl layer.
>
> commit 92681e94efe1fa3b85df431d6ddda7bbda2e5314
> Author: Jonathan Gray <jsg at jsg.id.au>
> Date: Sat Mar 22 18:44:38 2014 +1100
>
> configure: don't require libudev for gbm or egl drm/wayland
>
> After the loader changes libudev is no longer required for
> gbm or the egl drm/wayland platforms. Lets these build/run
> on OpenBSD.
>
IMHO we need to preserve the libudev requirement for linux while
allowing others to use the fallbacks.
One could add a hunk like
case $hostos
linux*)
need_libudev=yes ;;
*)
need_libudev=no ;;
esac
and update all the libudev usecases within configure.ac
> Signed-off-by: Jonathan Gray <jsg at jsg.id.au>
>
> diff --git a/configure.ac b/configure.ac
> index 9c67a9b..47c315b 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1133,10 +1133,6 @@ if test "x$enable_gbm" = xauto; then
> esac
> fi
> if test "x$enable_gbm" = xyes; then
> - if test x"$have_libudev" != xyes; then
if test "x$need_libudev$have_libudev" = xyesno; then
...
> - AC_MSG_ERROR([gbm requires udev >= $LIBUDEV_REQUIRED])
> - fi
> -
> if test "x$enable_dri" = xyes; then
> GBM_BACKEND_DIRS="$GBM_BACKEND_DIRS dri"
> if test "x$enable_shared_glapi" = xno; then
> @@ -1145,7 +1141,7 @@ if test "x$enable_gbm" = xyes; then
> fi
> fi
> AM_CONDITIONAL(HAVE_GBM, test "x$enable_gbm" = xyes)
> -GBM_PC_REQ_PRIV="libudev >= $LIBUDEV_REQUIRED"
> +GBM_PC_REQ_PRIV=""
> GBM_PC_LIB_PRIV="$DLOPEN_LIBS"
> AC_SUBST([GBM_PC_REQ_PRIV])
> AC_SUBST([GBM_PC_LIB_PRIV])
> @@ -1426,11 +1422,6 @@ for plat in $egl_platforms; do
> AC_MSG_ERROR([EGL platform '$plat' does not exist])
> ;;
> esac
> -
> - case "$plat$have_libudev" in
> - waylandno|drmno)
> - AC_MSG_ERROR([cannot build $plat platform without udev >= $LIBUDEV_REQUIRED]) ;;
> - esac
> done
>
> # libEGL wants to default to the first platform specified in
>
> commit b5a6354509d02459a2bc04433075268fa855583c
> Author: Jonathan Gray <jsg at jsg.id.au>
> Date: Sat Mar 22 18:35:37 2014 +1100
>
> egl/dri2: don't require libudev to build drm/wayland platforms
>
> After the loader changes libudev is no longer required to
> build gbm or the egl drm/wayland platforms.
>
I see things differently the "to libudev or not to libudev" mayhem is
consolidated into the loader and no-one else needs to know the gory
details :-P
> Remove a libudev ifdef which allows the the drm egl driver
> to be loaded on OpenBSD.
>
> Signed-off-by: Jonathan Gray <jsg at jsg.id.au>
Reviewed-by: Emil Velikov <emil.l.velikov at gmail.com>
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index 8abe8ac..dc541ad 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -630,7 +630,6 @@ dri2_initialize(_EGLDriver *drv, _EGLDisplay *disp)
> return dri2_initialize_x11(drv, disp);
> #endif
>
> -#ifdef HAVE_LIBUDEV
> #ifdef HAVE_DRM_PLATFORM
> case _EGL_PLATFORM_DRM:
> if (disp->Options.TestOnly)
> @@ -643,7 +642,6 @@ dri2_initialize(_EGLDriver *drv, _EGLDisplay *disp)
> return EGL_TRUE;
> return dri2_initialize_wayland(drv, disp);
> #endif
> -#endif
> #ifdef HAVE_ANDROID_PLATFORM
> case _EGL_PLATFORM_ANDROID:
> if (disp->Options.TestOnly)
>
> commit b4556b175a7c8a76f1efdf6f07cca2094f6b5859
> Author: Jonathan Gray <jsg at jsg.id.au>
> Date: Sat Mar 22 18:31:12 2014 +1100
>
> egl/dri2: use drm macros to construct device name
>
> Don't hardcode /dev/dri/card0 but instead use the drm
> macros which allows the correct /dev/drm0 device to be
> opened on OpenBSD.
>
> Signed-off-by: Jonathan Gray <jsg at jsg.id.au>
>
> diff --git a/src/egl/drivers/dri2/platform_drm.c b/src/egl/drivers/dri2/platform_drm.c
> index 2f7edb9..a866e9a 100644
> --- a/src/egl/drivers/dri2/platform_drm.c
> +++ b/src/egl/drivers/dri2/platform_drm.c
> @@ -481,6 +481,7 @@ dri2_initialize_drm(_EGLDriver *drv, _EGLDisplay *disp)
> struct gbm_device *gbm;
> int fd = -1;
> int i;
> + char buf[64];
>
> loader_set_logger(_eglLog);
>
> @@ -491,8 +492,9 @@ dri2_initialize_drm(_EGLDriver *drv, _EGLDisplay *disp)
> disp->DriverData = (void *) dri2_dpy;
>
> gbm = disp->PlatformDisplay;
> + sprintf(buf, DRM_DEV_NAME, DRM_DIR_NAME, 0);
Please use snprintf() and check the function return status. If it fails
one can fallback to the current /dev/dri/card0.
Please send the next revision with git send-email, it makes it easier to
apply :-)
Thanks
-Emil
> if (gbm == NULL) {
> - fd = open("/dev/dri/card0", O_RDWR);
> + fd = open(buf, O_RDWR);
> dri2_dpy->own_device = 1;
> gbm = gbm_create_device(fd);
> if (gbm == NULL)
>
More information about the mesa-dev
mailing list