Re: [PATCH 3/3] configure.ac: remove --enable-aiglx option

2016-10-12 Thread Emil Velikov
On 11 October 2016 at 14:34, Jon Turney  wrote:
> On 10/10/2016 19:31, Mihail Konev wrote:
>>
>> Hello.
>>
>> On Mon Oct 10 14:59:52 UTC 2016, Jon Turney wrote:
>>>
>>> @@ -16,11 +20,10 @@ AM_CPPFLAGS = \
>>> -I$(top_srcdir)/hw/xfree86/os-support/bus \
>>> -I$(top_srcdir)/hw/xfree86/common \
>>> -I$(top_srcdir)/hw/xfree86/dri \
>>> +   -I$(top_srcdir)/hw/xfree86/dri2
>>> -I$(top_srcdir)/mi \
>>> -I$(top_srcdir)/present
>>
>>
>> Shouldn't there be a backslash?
>
>
> Absolutely!  Revised patch attached.
>
One could have kept the include path as-is or even wrap it in if DRI2
(like the original code). Either way, the updated patch looks correct
and is
Reviewed-by: Emil Velikov 

Emil
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 3/3] configure.ac: remove --enable-aiglx option

2016-10-11 Thread Jon Turney

On 10/10/2016 19:31, Mihail Konev wrote:

Hello.

On Mon Oct 10 14:59:52 UTC 2016, Jon Turney wrote:

@@ -16,11 +20,10 @@ AM_CPPFLAGS = \
-I$(top_srcdir)/hw/xfree86/os-support/bus \
-I$(top_srcdir)/hw/xfree86/common \
-I$(top_srcdir)/hw/xfree86/dri \
+   -I$(top_srcdir)/hw/xfree86/dri2
-I$(top_srcdir)/mi \
-I$(top_srcdir)/present


Shouldn't there be a backslash?


Absolutely!  Revised patch attached.

I build-tested this on linux with --enable-dri2 and --disabl-dri2

From 549d3d4b82009319563c42b3d0113361ded257aa Mon Sep 17 00:00:00 2001
From: Jon Turney 
Date: Thu, 6 Oct 2016 22:13:07 +0100
Subject: [PATCH xserver] glx/dri2: Don't build DRI loader if DRI2 isn't
 enabled

This partially reverts 501d8e2b.

Signed-off-by: Jon Turney 
---
 glx/Makefile.am| 11 ---
 hw/xfree86/dixmods/Makefile.am |  2 ++
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/glx/Makefile.am b/glx/Makefile.am
index fc0b76a..699de63 100644
--- a/glx/Makefile.am
+++ b/glx/Makefile.am
@@ -1,4 +1,8 @@
-noinst_LTLIBRARIES = libglx.la libglxdri.la
+if DRI2
+GLXDRI_LIBRARY = libglxdri.la
+endif
+
+noinst_LTLIBRARIES = libglx.la $(GLXDRI_LIBRARY)
 
 AM_CFLAGS = \
@DIX_CFLAGS@ \
@@ -16,11 +20,10 @@ AM_CPPFLAGS = \
-I$(top_srcdir)/hw/xfree86/os-support/bus \
-I$(top_srcdir)/hw/xfree86/common \
-I$(top_srcdir)/hw/xfree86/dri \
+   -I$(top_srcdir)/hw/xfree86/dri2 \
-I$(top_srcdir)/mi \
-I$(top_srcdir)/present
 
-AM_CPPFLAGS += -I$(top_srcdir)/hw/xfree86/dri2
-
 indirect_sources = \
indirect_dispatch.c \
indirect_dispatch.h \
@@ -33,7 +36,9 @@ indirect_sources =\
indirect_table.c
 
 libglxdri_la_SOURCES =
+if DRI2
 libglxdri_la_SOURCES += glxdri2.c
+endif
 
 libglxdri_la_LIBADD = $(DLOPEN_LIBS)
 
diff --git a/hw/xfree86/dixmods/Makefile.am b/hw/xfree86/dixmods/Makefile.am
index be43e8f..d534c78 100644
--- a/hw/xfree86/dixmods/Makefile.am
+++ b/hw/xfree86/dixmods/Makefile.am
@@ -29,10 +29,12 @@ libwfb_la_CFLAGS = $(AM_CFLAGS) -DFB_ACCESS_WRAPPER
 
 libglx_la_LDFLAGS = -module -avoid-version $(LD_NO_UNDEFINED_FLAG)
 libglx_la_LIBADD = $(top_builddir)/glx/libglx.la $(GLX_SYS_LIBS)
+if DRI2
 libglx_la_LIBADD += $(top_builddir)/glx/libglxdri.la
 if NO_UNDEFINED
 libglx_la_LIBADD += $(LIBDRM_LIBS) $(PIXMAN_LIBS)
 endif
+endif
 libglx_la_SOURCES = glxmodule.c
 
 libshadow_la_LDFLAGS = -module -avoid-version $(LD_NO_UNDEFINED_FLAG)
-- 
2.8.3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 3/3] configure.ac: remove --enable-aiglx option

2016-10-10 Thread Jon Turney

On 09/10/2016 15:42, Emil Velikov wrote:

On Friday, 7 October 2016, Jon Turney wrote:


That's not quite enough, as building glxdri2.c also requires dri2proto
headers.

At the moment, configure.ac only requires dri2proto when --enable-dri2
turns on.

So either that needs to be made unconditional, or building glxdri2.c made
conditional on DRI2 (untested patch attached)

You're correct. Wrapping it in DRI2 conditional is a good idea.


Note creating an empty (no sources or static libs) library is likely to
cause problems. Just use the form prior to my patch ?


Yes, that seems possible.  Updated patch attached.


From 42f74bb44190be06b9630dfcaae48b27533a28cd Mon Sep 17 00:00:00 2001
From: Jon Turney 
Date: Thu, 6 Oct 2016 22:13:07 +0100
Subject: [PATCH xserver] glx/dri2: Don't build DRI loader if DRI2 isn't
 enabled

This partially reverts 501d8e2b.
---
 glx/Makefile.am| 11 ---
 hw/xfree86/dixmods/Makefile.am |  2 ++
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/glx/Makefile.am b/glx/Makefile.am
index fc0b76a..60bd84c 100644
--- a/glx/Makefile.am
+++ b/glx/Makefile.am
@@ -1,4 +1,8 @@
-noinst_LTLIBRARIES = libglx.la libglxdri.la
+if DRI2
+GLXDRI_LIBRARY = libglxdri.la
+endif
+
+noinst_LTLIBRARIES = libglx.la $(GLXDRI_LIBRARY)
 
 AM_CFLAGS = \
@DIX_CFLAGS@ \
@@ -16,11 +20,10 @@ AM_CPPFLAGS = \
-I$(top_srcdir)/hw/xfree86/os-support/bus \
-I$(top_srcdir)/hw/xfree86/common \
-I$(top_srcdir)/hw/xfree86/dri \
+   -I$(top_srcdir)/hw/xfree86/dri2
-I$(top_srcdir)/mi \
-I$(top_srcdir)/present
 
-AM_CPPFLAGS += -I$(top_srcdir)/hw/xfree86/dri2
-
 indirect_sources = \
indirect_dispatch.c \
indirect_dispatch.h \
@@ -33,7 +36,9 @@ indirect_sources =\
indirect_table.c
 
 libglxdri_la_SOURCES =
+if DRI2
 libglxdri_la_SOURCES += glxdri2.c
+endif
 
 libglxdri_la_LIBADD = $(DLOPEN_LIBS)
 
diff --git a/hw/xfree86/dixmods/Makefile.am b/hw/xfree86/dixmods/Makefile.am
index be43e8f..d534c78 100644
--- a/hw/xfree86/dixmods/Makefile.am
+++ b/hw/xfree86/dixmods/Makefile.am
@@ -29,10 +29,12 @@ libwfb_la_CFLAGS = $(AM_CFLAGS) -DFB_ACCESS_WRAPPER
 
 libglx_la_LDFLAGS = -module -avoid-version $(LD_NO_UNDEFINED_FLAG)
 libglx_la_LIBADD = $(top_builddir)/glx/libglx.la $(GLX_SYS_LIBS)
+if DRI2
 libglx_la_LIBADD += $(top_builddir)/glx/libglxdri.la
 if NO_UNDEFINED
 libglx_la_LIBADD += $(LIBDRM_LIBS) $(PIXMAN_LIBS)
 endif
+endif
 libglx_la_SOURCES = glxmodule.c
 
 libshadow_la_LDFLAGS = -module -avoid-version $(LD_NO_UNDEFINED_FLAG)
-- 
2.8.3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 3/3] configure.ac: remove --enable-aiglx option

2016-10-09 Thread Emil Velikov
On Friday, 7 October 2016, Jon Turney  wrote:

> On 06/10/2016 19:15, Emil Velikov wrote:
>
>> On 6 October 2016 at 19:02, Jon Turney 
>> wrote:
>>
>>> On 29/09/2016 18:41, Emil Velikov wrote:
>>>

 Presently the option guards both direct and accelerated indirect GLX. As
 such when one toggles it off they end up without any acceleration.

 Remove the option all together until we have the time to split/rework
 things.

 Cc: Jon Turney 
 Cc: Adam Jackson 
 Signed-off-by: Emil Velikov 
 ---
 Jon, I've not checked the Xwin side of things but considering that the
 option is enabled by default and having it for Xwin only will be
 confusing I've nuked the guards throughout the tree.

>>>
>>> Sorry I didn't get around to testing this before it was committed.
>>>
>>> This breaks my build (See [1]), as the DRI2 loader is now built
>>> unconditionally, which fails without drm.h
>>>
>>> I'm not sure exactly what problem this change is fixing, so I'm not sure
>>> how
>>> to address that.
>>>
>>> Is it ok to restore the part which makes building the DRI2 loader
>>> conditional?
>>>
>>> I had a bad feeling about this, fortunately it seems pretty easy to
>> handle.
>>
>> From a quick look nothing in glx/glxdri2.c should require libdrm so we
>> can nuke the drm.h and xf86drm.h includes, which will get you back up
>> and going. Alternatively we can add those in a ifdef WITH_LIBDRM/endif
>> block.
>>
>
> That's not quite enough, as building glxdri2.c also requires dri2proto
> headers.
>
> At the moment, configure.ac only requires dri2proto when --enable-dri2
> turns on.
>
> So either that needs to be made unconditional, or building glxdri2.c made
> conditional on DRI2 (untested patch attached)
>
> You're correct. Wrapping it in DRI2 conditional is a good idea.

Note creating an empty (no sources or static libs) library is likely to
cause problems. Just use the form prior to my patch ?

Emil
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 3/3] configure.ac: remove --enable-aiglx option

2016-10-07 Thread Jon Turney

On 06/10/2016 19:15, Emil Velikov wrote:

On 6 October 2016 at 19:02, Jon Turney  wrote:

On 29/09/2016 18:41, Emil Velikov wrote:


Presently the option guards both direct and accelerated indirect GLX. As
such when one toggles it off they end up without any acceleration.

Remove the option all together until we have the time to split/rework
things.

Cc: Jon Turney 
Cc: Adam Jackson 
Signed-off-by: Emil Velikov 
---
Jon, I've not checked the Xwin side of things but considering that the
option is enabled by default and having it for Xwin only will be
confusing I've nuked the guards throughout the tree.


Sorry I didn't get around to testing this before it was committed.

This breaks my build (See [1]), as the DRI2 loader is now built
unconditionally, which fails without drm.h

I'm not sure exactly what problem this change is fixing, so I'm not sure how
to address that.

Is it ok to restore the part which makes building the DRI2 loader
conditional?


I had a bad feeling about this, fortunately it seems pretty easy to handle.

From a quick look nothing in glx/glxdri2.c should require libdrm so we
can nuke the drm.h and xf86drm.h includes, which will get you back up
and going. Alternatively we can add those in a ifdef WITH_LIBDRM/endif
block.


That's not quite enough, as building glxdri2.c also requires dri2proto 
headers.


At the moment, configure.ac only requires dri2proto when --enable-dri2 
turns on.


So either that needs to be made unconditional, or building glxdri2.c 
made conditional on DRI2 (untested patch attached)



Even then we can make the compilation of the DRI2 loader but let's not
call it AIGLX ;-)


From db2fcb2636a659abd0998d82204813fd5dcd57ff Mon Sep 17 00:00:00 2001
From: Jon Turney 
Date: Thu, 6 Oct 2016 22:13:07 +0100
Subject: [PATCH xserver] Don't build DRI loader if DRI2 isn't enabled

---
 glx/Makefile.am | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/glx/Makefile.am b/glx/Makefile.am
index fc0b76a..9af7080 100644
--- a/glx/Makefile.am
+++ b/glx/Makefile.am
@@ -16,11 +16,10 @@ AM_CPPFLAGS = \
-I$(top_srcdir)/hw/xfree86/os-support/bus \
-I$(top_srcdir)/hw/xfree86/common \
-I$(top_srcdir)/hw/xfree86/dri \
+   -I$(top_srcdir)/hw/xfree86/dri2
-I$(top_srcdir)/mi \
-I$(top_srcdir)/present
 
-AM_CPPFLAGS += -I$(top_srcdir)/hw/xfree86/dri2
-
 indirect_sources = \
indirect_dispatch.c \
indirect_dispatch.h \
@@ -33,7 +32,9 @@ indirect_sources =\
indirect_table.c
 
 libglxdri_la_SOURCES =
+if DRI2
 libglxdri_la_SOURCES += glxdri2.c
+endif
 
 libglxdri_la_LIBADD = $(DLOPEN_LIBS)
 
-- 
2.8.3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 3/3] configure.ac: remove --enable-aiglx option

2016-10-06 Thread Emil Velikov
On 6 October 2016 at 19:02, Jon Turney  wrote:
> On 29/09/2016 18:41, Emil Velikov wrote:
>>
>> Presently the option guards both direct and accelerated indirect GLX. As
>> such when one toggles it off they end up without any acceleration.
>>
>> Remove the option all together until we have the time to split/rework
>> things.
>>
>> Cc: Jon Turney 
>> Cc: Adam Jackson 
>> Signed-off-by: Emil Velikov 
>> ---
>> Jon, I've not checked the Xwin side of things but considering that the
>> option is enabled by default and having it for Xwin only will be
>> confusing I've nuked the guards throughout the tree.
>
>
> Sorry I didn't get around to testing this before it was committed.
>
> This breaks my build (See [1]), as the DRI2 loader is now built
> unconditionally, which fails without drm.h
>
> I'm not sure exactly what problem this change is fixing, so I'm not sure how
> to address that.
>
> Is it ok to restore the part which makes building the DRI2 loader
> conditional?
>
I had a bad feeling about this, fortunately it seems pretty easy to handle.

From a quick look nothing in glx/glxdri2.c should require libdrm so we
can nuke the drm.h and xf86drm.h includes, which will get you back up
and going. Alternatively we can add those in a ifdef WITH_LIBDRM/endif
block.

Even then we can make the compilation of the DRI2 loader but let's not
call it AIGLX ;-)

Thanks
Emil
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 3/3] configure.ac: remove --enable-aiglx option

2016-10-06 Thread Jon Turney

On 29/09/2016 18:41, Emil Velikov wrote:

Presently the option guards both direct and accelerated indirect GLX. As
such when one toggles it off they end up without any acceleration.

Remove the option all together until we have the time to split/rework
things.

Cc: Jon Turney 
Cc: Adam Jackson 
Signed-off-by: Emil Velikov 
---
Jon, I've not checked the Xwin side of things but considering that the
option is enabled by default and having it for Xwin only will be
confusing I've nuked the guards throughout the tree.


Sorry I didn't get around to testing this before it was committed.

This breaks my build (See [1]), as the DRI2 loader is now built 
unconditionally, which fails without drm.h


I'm not sure exactly what problem this change is fixing, so I'm not sure 
how to address that.


Is it ok to restore the part which makes building the DRI2 loader 
conditional?


[1] http://dronecode.duckdns.org:8010/builders/xserver/builds/821

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel