Patchwork [1/1] xserver-xorg: do not disable-dga as VESA driver requires it

login
register
mail settings
Submitter Otavio Salvador
Date Nov. 22, 2011, 1:47 p.m.
Message ID <65caf0c80f03e4fd3710b708541b0a7dd8f310ba.1321969615.git.otavio@ossystems.com.br>
Download mbox | patch
Permalink /patch/15241/
State New
Headers show

Comments

Otavio Salvador - Nov. 22, 2011, 1:47 p.m.
Xorg VESA driver calls DGAInit enforcing this symbol to be available
to the driver to work.

Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
---
 .../xorg-xserver/xserver-xorg-1.11.1.inc           |    2 +-
 .../xorg-xserver/xserver-xorg-common.inc           |    1 -
 2 files changed, 1 insertions(+), 2 deletions(-)
Phil Blundell - Nov. 22, 2011, 5:14 p.m.
On Tue, 2011-11-22 at 13:47 +0000, Otavio Salvador wrote:
> Xorg VESA driver calls DGAInit enforcing this symbol to be available
> to the driver to work.

Maybe it would be better to patch vesa.c to not require this symbol
rather than forcing it on in the server for everyone.  Or alternatively,
if you genuinely want DGA, maybe make it a PACKAGECONFIG option.

p.
Otavio Salvador - Nov. 22, 2011, 5:24 p.m.
On Tue, Nov 22, 2011 at 15:14, Phil Blundell <philb@gnu.org> wrote:

> On Tue, 2011-11-22 at 13:47 +0000, Otavio Salvador wrote:
> > Xorg VESA driver calls DGAInit enforcing this symbol to be available
> > to the driver to work.
>
> Maybe it would be better to patch vesa.c to not require this symbol
> rather than forcing it on in the server for everyone.  Or alternatively,
> if you genuinely want DGA, maybe make it a PACKAGECONFIG option.
>

What worries me is that this changed lately as it were working fine.

When xserver-xorg were update to 1.11 this has changed behind the scenes.
This used to be disabled on lite flavour only.
Richard Purdie - Nov. 25, 2011, midnight
On Tue, 2011-11-22 at 15:24 -0200, Otavio Salvador wrote:
> On Tue, Nov 22, 2011 at 15:14, Phil Blundell <philb@gnu.org> wrote:
>         On Tue, 2011-11-22 at 13:47 +0000, Otavio Salvador wrote:
>         > Xorg VESA driver calls DGAInit enforcing this symbol to be
>         available
>         > to the driver to work.
>
>         Maybe it would be better to patch vesa.c to not require this
>         symbol
>         rather than forcing it on in the server for everyone.  Or
>         alternatively,
>         if you genuinely want DGA, maybe make it a PACKAGECONFIG
>         option.
>
> What worries me is that this changed lately as it were working fine.
>
> When xserver-xorg were update to 1.11 this has changed behind the
> scenes. This used to be disabled on lite flavour only.

Looking at vesa.c, there is quite a chunk of code in there depending on
DGA.

Is there any harm to building DGA apart from an extra package? Its a
self contained module isn't it?

If so, we should just be able to turn it on, package it separately and
forget about it?

Of course the ideal solution would be for someone to convert the vesa
driver to use the xrandr API...

Cheers,

Richard
Otavio Salvador - Nov. 25, 2011, 10:41 a.m.
On Thu, Nov 24, 2011 at 22:00, Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> ...
>
Looking at vesa.c, there is quite a chunk of code in there depending on
> DGA.
>

Yes.


> Is there any harm to building DGA apart from an extra package? Its a
> self contained module isn't it?
>

AFAIK no. It seems to be a blob static linked onto the Xorg.


> ...

Of course the ideal solution would be for someone to convert the vesa
> driver to use the xrandr API...
>

Well, yes. But I cannot work on that now.

In meanwhile I sent another patch proposing to disable it for lite flavor
only, as it was before the update to 1.11 Xorg Xserver.
Phil Blundell - Nov. 25, 2011, 11:03 a.m.
On Fri, 2011-11-25 at 00:00 +0000, Richard Purdie wrote:
> Looking at vesa.c, there is quite a chunk of code in there depending on
> DGA.

I guess the question is, is this chunk of code actually doing anything
useful/important, or is it just there to support DGA on VESA?  If the
latter then people who don't need/want DGA (which is probably everybody,
nowadays) can just take it out.

All that said, I would have thought that the VESA driver itself was
pretty much a fringe interest in this day and age.  Is anybody really
using that on a shipping system?  If it's just for qemux86 test harness
purposes then maybe we could turn on the vesafb driver in the kernel and
use the straight framebuffer driver in Xorg rather than the VESA one.

> Is there any harm to building DGA apart from an extra package? Its a
> self contained module isn't it?

I don't think it's that self-contained.  Admittedly it isn't all that
big either, but still.

> If so, we should just be able to turn it on, package it separately and
> forget about it?
> 
> Of course the ideal solution would be for someone to convert the vesa
> driver to use the xrandr API...

I don't quite understand what xrandr has to do with that.  How does this
relate to DGA?

p.
Richard Purdie - Nov. 25, 2011, 11:08 a.m.
On Fri, 2011-11-25 at 11:03 +0000, Phil Blundell wrote:
> On Fri, 2011-11-25 at 00:00 +0000, Richard Purdie wrote:
> > Looking at vesa.c, there is quite a chunk of code in there depending on
> > DGA.
> 
> I guess the question is, is this chunk of code actually doing anything
> useful/important, or is it just there to support DGA on VESA?  If the
> latter then people who don't need/want DGA (which is probably everybody,
> nowadays) can just take it out.
> 
> All that said, I would have thought that the VESA driver itself was
> pretty much a fringe interest in this day and age.  Is anybody really
> using that on a shipping system?  If it's just for qemux86 test harness
> purposes then maybe we could turn on the vesafb driver in the kernel and
> use the straight framebuffer driver in Xorg rather than the VESA one.
> 
> > Is there any harm to building DGA apart from an extra package? Its a
> > self contained module isn't it?
> 
> I don't think it's that self-contained.  Admittedly it isn't all that
> big either, but still.
> 
> > If so, we should just be able to turn it on, package it separately and
> > forget about it?
> > 
> > Of course the ideal solution would be for someone to convert the vesa
> > driver to use the xrandr API...
> 
> I don't quite understand what xrandr has to do with that.  How does this
> relate to DGA?

The vesa driver looks like its using DGA APIs to expose mode setting. A
comment to that end is also in a TODO list at the top of the file.

Cheers,

Richard
Otavio Salvador - Nov. 25, 2011, 11:08 a.m.
On Fri, Nov 25, 2011 at 09:03, Phil Blundell <philb@gnu.org> wrote:

> On Fri, 2011-11-25 at 00:00 +0000, Richard Purdie wrote:
> > Looking at vesa.c, there is quite a chunk of code in there depending on
> > DGA.
>
> I guess the question is, is this chunk of code actually doing anything
> useful/important, or is it just there to support DGA on VESA?  If the
> latter then people who don't need/want DGA (which is probably everybody,
> nowadays) can just take it out.
>
> All that said, I would have thought that the VESA driver itself was
> pretty much a fringe interest in this day and age.  Is anybody really
> using that on a shipping system?  If it's just for qemux86 test harness
> purposes then maybe we could turn on the vesafb driver in the kernel and
> use the straight framebuffer driver in Xorg rather than the VESA one.


I do. I use it a lot and that's why I found it was broken.

For me, having it on xserver-xorg and it disabled for xserver-xorg-lite
works for me for now.
Phil Blundell - Nov. 25, 2011, 11:57 a.m.
On Fri, 2011-11-25 at 11:08 +0000, Richard Purdie wrote:
> The vesa driver looks like its using DGA APIs to expose mode setting. A
> comment to that end is also in a TODO list at the top of the file.

Ah, I see.  I guess that can just be turned off for non-DGA builds then.
As you say, making it use Xrandr would be a better solution in the long
term but it doesn't seem all that likely that anybody is going to have
much enthusiasm for enhancing the VESA driver at this point.

p.

Patch

diff --git a/meta/recipes-graphics/xorg-xserver/xserver-xorg-1.11.1.inc b/meta/recipes-graphics/xorg-xserver/xserver-xorg-1.11.1.inc
index 2c5bb0a..8b0808d 100644
--- a/meta/recipes-graphics/xorg-xserver/xserver-xorg-1.11.1.inc
+++ b/meta/recipes-graphics/xorg-xserver/xserver-xorg-1.11.1.inc
@@ -3,7 +3,7 @@  SRC_URI += "file://crosscompile.patch"
 # Misc build failure for master HEAD
 SRC_URI += "file://fix_open_max_preprocessor_error.patch"
 
-PR = "r1"
+PR = "r2"
 
 SRC_URI[md5sum] = "4cdf54dc569b6a5548257c93a45dcd3d"
 SRC_URI[sha256sum] = "c069c177d9a80bca2ee1d49aa1af6d9f996c83e4ff12393cab2ca3f8817276f6"
diff --git a/meta/recipes-graphics/xorg-xserver/xserver-xorg-common.inc b/meta/recipes-graphics/xorg-xserver/xserver-xorg-common.inc
index 2559397..c1a552e 100644
--- a/meta/recipes-graphics/xorg-xserver/xserver-xorg-common.inc
+++ b/meta/recipes-graphics/xorg-xserver/xserver-xorg-common.inc
@@ -100,7 +100,6 @@  EXTRA_OECONF += "--with-fop=no \
                  --disable-acfb \
                  --disable-ccfb \
                  --disable-mcfb \
-                 --disable-dga \
                  --disable-xinerama \
                  --disable-xf86misc \
                  --disable-xorgcfg \