| Message ID | 1461586822-1003-2-git-send-email-bernd.kuhls@t-online.de |
|---|---|
| State | Changes Requested |
| Headers | show |
On 25/04/16 09:20, Bernd Kuhls wrote: > https://git.busybox.net/buildroot/commit/package/mesa3d?id=f1894ec95728806e09405d26663e0ea371afaeab > removed the dependency on X.org for DRI drivers, this patch uses the > new mesa3d option BR2_PACKAGE_MESA3D_OPENGL_GL to determine OpenGL > support. > > Fixes > http://autobuild.buildroot.net/results/461/46146a63f83e318f1213ec1d558558e3404d8ff8/ > http://autobuild.buildroot.net/results/c91/c91f6bce6f8a0691467a5ca16d5fe15687ee945c/ > http://autobuild.buildroot.net/results/ac5/ac560e36977f90cf93098c987d401d64edce24cb/ > http://autobuild.buildroot.net/results/c71/c712af468012b1e23e3048207611f0da78fb1ae0/ > http://autobuild.buildroot.net/results/b82/b8251a40f5eb5e1124e1a6e1abe407db83c75371/ > http://autobuild.buildroot.net/results/667/667b7847e47b1d272d028da4a18d77e70ab01875/ > http://autobuild.buildroot.net/results/6a4/6a45e32c0f894508d05c964a9a0fd645b6f3d6e4/ > http://autobuild.buildroot.net/results/d96/d960bcfff386310e265d0d40533ef2ef7f74d5fe/ > http://autobuild.buildroot.net/results/7bd/7bd41a86d60543d3de4483f86f0b5a719cf480b9/ > and others > > Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de> Acked-by: Gustavo Zacarias <gustavo@zacarias.com.ar> (pending patch 1 from the series getting applied, and making my glmark2 autobuild fix completely irrelevant)
Hello, On Mon, 25 Apr 2016 14:20:16 +0200, Bernd Kuhls wrote: > diff --git a/package/glmark2/Config.in b/package/glmark2/Config.in > index 9167fc5..10c88ee 100644 > --- a/package/glmark2/Config.in > +++ b/package/glmark2/Config.in > @@ -7,7 +7,7 @@ config BR2_PACKAGE_GLMARK2_EGL_GLES > config BR2_PACKAGE_GLMARK2_GL > bool > default y > - depends on BR2_PACKAGE_MESA3D_DRI_DRIVER > + depends on BR2_PACKAGE_MESA3D_OPENGL_GL I know that may sound like a nitpicking debate, but I'm wondering if we shouldn't keep BR2_PACKAGE_MESA3D_OPENGL_GL as an internal mesa3d symbol, and instead use: depends on BR2_PACKAGE_HAS_LIBGL && BR2_PACKAGE_MESA3D which really expresses what we want: we want an OpenGL implementation, and this OpenGL implementation has to be mesa3d. Thomas
On 27/04/16 16:32, Thomas Petazzoni wrote: > I know that may sound like a nitpicking debate, but I'm wondering if we > shouldn't keep BR2_PACKAGE_MESA3D_OPENGL_GL as an internal mesa3d > symbol, and instead use: > > depends on BR2_PACKAGE_HAS_LIBGL && BR2_PACKAGE_MESA3D > > which really expresses what we want: we want an OpenGL implementation, > and this OpenGL implementation has to be mesa3d. > > Thomas I'm fine with nitpicking, however we already use BR2_PACKAGE_MESA3D_OPENGL_EGL and BR2_PACKAGE_MESA3D_OPENGL_ES in glmark2 and weston, which could also ostensibly be replaced by the more verbose conditional. Granted, for weston what it really wants is wayland-egl, which AFAIK with our current packages is only provided by mesa3d. Also while nitpicking the xorg drivers only want DRI so we can keep that as is since the xorg conditional is in there already (packages in x11r7/ already depend on xorg, plus mesa3d_dri gives opengl). Kind of a maybe take back my ACKs for the drivers. Regards.
Hello, On Wed, 27 Apr 2016 16:38:38 -0300, Gustavo Zacarias wrote: > I'm fine with nitpicking, however we already use > BR2_PACKAGE_MESA3D_OPENGL_EGL and BR2_PACKAGE_MESA3D_OPENGL_ES in > glmark2 and weston, which could also ostensibly be replaced by the more > verbose conditional. I don't feel super strongly about using BR2_PACKAGE_MESA3D_OPENGL_GL vs. BR2_PACKAGE_HAS_LIBGL && BR2_PACKAGE_MESA3D. The former is admittedly shorter. > Granted, for weston what it really wants is wayland-egl, which AFAIK > with our current packages is only provided by mesa3d. Not sure what you mean here. Do you mean we should have a BR2_PACKAGE_MESA3D_WAYLAND_EGL blind option? > Also while nitpicking the xorg drivers only want DRI so we can keep that > as is since the xorg conditional is in there already (packages in x11r7/ > already depend on xorg, plus mesa3d_dri gives opengl). Kind of a maybe > take back my ACKs for the drivers. Hum, yes for the X.org drivers, I don't see why the patches are needed indeed. Unless Bernd disagrees, I will mark patches 5, 6, 7 and 8 as Rejected. Bernd, can you respin patches 1, 2, 3 and 4 to take into account the comments Gustavo and I made (as well as include the Acked-by from Gustavo on patches 2, 3 and 4) ? Thanks a lot! Thomas
On 27/04/16 16:49, Thomas Petazzoni wrote: > Not sure what you mean here. Do you mean we should have a > BR2_PACKAGE_MESA3D_WAYLAND_EGL blind option? Hi. wayland-egl is basically (IIRC, i may miss a bit) a small piece of code of egl surface copy accelerator for wayland, which only mesa3d provides for now (like always when wayland is enabled). So it should really be BR2_PACKAGE_PROVIDES_WAYLAND_EGL since i expect some other EGL stack will provide it eventually to play nice - essentially required for pure libgtk3 wayland support (example: rpi-userland can't work with libgtk3 that way, though i think that piece of code from mesa3d could be stripped out and work, maybe not with the greatest level of performance and probably somewhat grey regarding licensing). Regards.
Am Wed, 27 Apr 2016 21:49:46 +0200 schrieb Thomas Petazzoni: > Bernd, can you respin patches 1, 2, 3 and 4 to take into account the > comments Gustavo and I made (as well as include the Acked-by from > Gustavo on patches 2, 3 and 4) ? Hi, I marked my series as "Changes requested" because I will not have time in the next days to send a new version. Regards, Bernd
diff --git a/package/glmark2/Config.in b/package/glmark2/Config.in index 9167fc5..10c88ee 100644 --- a/package/glmark2/Config.in +++ b/package/glmark2/Config.in @@ -7,7 +7,7 @@ config BR2_PACKAGE_GLMARK2_EGL_GLES config BR2_PACKAGE_GLMARK2_GL bool default y - depends on BR2_PACKAGE_MESA3D_DRI_DRIVER + depends on BR2_PACKAGE_MESA3D_OPENGL_GL comment "glmark2 needs an OpenGL or an openGL ES and EGL backend provided by mesa3d" depends on !BR2_PACKAGE_GLMARK2_GL && !BR2_PACKAGE_GLMARK2_EGL_GLES
https://git.busybox.net/buildroot/commit/package/mesa3d?id=f1894ec95728806e09405d26663e0ea371afaeab removed the dependency on X.org for DRI drivers, this patch uses the new mesa3d option BR2_PACKAGE_MESA3D_OPENGL_GL to determine OpenGL support. Fixes http://autobuild.buildroot.net/results/461/46146a63f83e318f1213ec1d558558e3404d8ff8/ http://autobuild.buildroot.net/results/c91/c91f6bce6f8a0691467a5ca16d5fe15687ee945c/ http://autobuild.buildroot.net/results/ac5/ac560e36977f90cf93098c987d401d64edce24cb/ http://autobuild.buildroot.net/results/c71/c712af468012b1e23e3048207611f0da78fb1ae0/ http://autobuild.buildroot.net/results/b82/b8251a40f5eb5e1124e1a6e1abe407db83c75371/ http://autobuild.buildroot.net/results/667/667b7847e47b1d272d028da4a18d77e70ab01875/ http://autobuild.buildroot.net/results/6a4/6a45e32c0f894508d05c964a9a0fd645b6f3d6e4/ http://autobuild.buildroot.net/results/d96/d960bcfff386310e265d0d40533ef2ef7f74d5fe/ http://autobuild.buildroot.net/results/7bd/7bd41a86d60543d3de4483f86f0b5a719cf480b9/ and others Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de> --- package/glmark2/Config.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)