Re: [PATCH 2/2] xfree86: dri2: free variable properly if code fails
On Tue, Jun 29, 2010 at 06:55:02PM +0200, ext Michel Dänzer wrote: On Die, 2010-06-29 at 19:00 +0300, Tiago Vignatti wrote: Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com --- Same as the previous patch: I'm not exactly seeing any problem or segfault with this code. I just got this issue with the static analyzer. hw/xfree86/dri2/dri2.c |8 ++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index d4181c9..26f5ac4 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -207,10 +207,10 @@ DRI2AddDrawableRef(DRI2DrawablePtr pPriv, XID id, XID dri2_id, return BadAlloc; if (!AddResource(dri2_id, dri2DrawableRes, pPriv)) - return BadAlloc; + goto err_out; if (!DRI2LookupDrawableRef(pPriv, id)) if (!AddResource(id, dri2DrawableRes, pPriv)) - return BadAlloc; + goto err_out; The second error path should also free the first resource created, shouldn't it? yeah, I think it makes sense. But I don't understand how exactly the resource code works. Do you think adding FreeResource(dri2_id, RT_NONE) is enough there? Thanks, Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 1/2] xfree86: dri2: check for drawable value
On Tue, Jun 29, 2010 at 06:52:18PM +0200, ext Michel Dänzer wrote:
On Die, 2010-06-29 at 19:00 +0300, Tiago Vignatti wrote:
Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com
---
I'm not exactly seeing any problem or segfault with this code. I just got
this
issue with the static analyzer.
hw/xfree86/dri2/dri2.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index f9ba8e7..d4181c9 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -734,6 +734,11 @@ Bool
DRI2WaitSwap(ClientPtr client, DrawablePtr pDrawable)
{
DRI2DrawablePtr pPriv = DRI2GetDrawable(pDrawable);
+if (pPriv == NULL) {
+xf86DrvMsg(pScreen-myNum, X_ERROR,
+ [DRI2] %s: bad drawable\n, __func__);
This allows clients to spam the log file.
The same happens with DRI2SwapBuffers and other functions then. Do we really
care about clients spamming here?
Tiago
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH v4 3/3] xfree86: remove board and vendor identifier strings from the configuration path
On Wed, Jun 30, 2010 at 03:30:03PM +0200, ext Dan Nicholson wrote: On Wed, Jun 30, 2010 at 6:07 AM, Vignatti Tiago (Nokia-D/Helsinki) I'd say to push the first and second of this set to 1.9, given their importance. This third one concerns more clean up, so I'll work a bit more, squash to my tree and eventually we can push later when the merge window is opened. I don't see any reason to introduce an ABI break for 1.9. We can still get the benefit of not calling the expensive pciaccess functions without causing any superfluous rebuilds. that's what I meant, isn't? The first two patches don't introduce ABI breakage. Cheers, Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH v3 3/3] xfree86: remove board and vendor identifier strings from the configuration path
On Thu, Jun 24, 2010 at 08:07:07AM +0200, ext Keith Packard wrote: On Wed, 23 Jun 2010 16:24:29 +0300, Tiago Vignatti tiago.vigna...@nokia.com wrote: ABI break. Doesn't this also break anhy config files that include vendorname or boardname elements? yes, it does. Do you think is it a problem? Anyway, I'll have to amend in this patch the removal of VendorName entry in xorg man page. Tiago signature.asc Description: Digital signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] XWin: Fix for static libXfont use
On Mon, Jun 28, 2010 at 01:36:31PM +0200, ext Julien Cristau wrote: On Mon, Jun 28, 2010 at 14:20:51 +0300, Tiago Vignatti wrote: On Mon, Jun 28, 2010 at 12:48:53PM +0200, ext Colin Harrison wrote: Hi, All XWin DDX builds use libXfont built static. The following libXfont patch is needed following recent master git changes (use of register_fpe_functions from libXfont in the server)... --- ./src/util/save_miscutil.c 2010-06-21 16:47:00.0 +0100 +++ ./src/util/miscutil.c 2010-06-28 11:25:19.0 +0100 @@ -48,8 +48,10 @@ extern void BuiltinRegisterFpeFunctions(void); +#ifndef WIN32 /* make sure everything initializes themselves at least once */ weak long serverGeneration = 1; +#endif can't we just live without serverGeneration then? My quickly compile test approach here worked well when doing it. What did you test exactly? The X server has a non-weak serverGeneration so removing the weak one from libXfont doesn't make a difference there, but it might for other libXfont users. oh, that's true. I forgot that the server and libXfont is not 1:1 mapping. Tiago signature.asc Description: Digital signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH v3 1/2] dix: use one single function to register fpe fonts
On Tue, Jun 22, 2010 at 08:30:41PM +0200, ext Keith Packard wrote: On Sat, 19 Jun 2010 14:02:02 +0300, Vignatti Tiago (Nokia-D/Helsinki) tiago.vigna...@nokia.com wrote: ping? Yeah, this all seems reasonable. But, as I'm trying to get release candidates made frequently, and those can't depend on unreleased modules. It doesn't look like libXfont 1.4.2 has been released yet; there's no 1.4.2 tag, and the tarball hasn't been uploaded yet. should be done now. I sent version 4 of the server side patches, including some Peter comment. Thanks, Tiago signature.asc Description: Digital signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH v2] xfree86: configure: remove vendor and card name matching rules
On Sat, Jun 19, 2010 at 12:05:22PM +0200, Vignatti Tiago (Nokia-D/Helsinki) wrote: Although vendor and board naming are used to create the configure file, the server doesn't actually use it when fetching such file and probing devices. Reported-by: Richard Barnette jrbarne...@chromium.org Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com --- from v1: - remove two more superfluous assignments (kudos Keith). Richard and James put the review stamp before, but I preferred to remove given there was two bugs spotted by Keith. Anyway, your review again would be much appreciated. ping. I'm cc-ing now the ones reviewed before :) Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: Post-RC1 merging criteria (was Re: [PULL] cleanups)
Hi, On Tue, Jun 22, 2010 at 04:21:42AM +0200, ext Keith Packard wrote: On Mon, 21 Jun 2010 16:04:51 +0300, Tiago Vignatti tiago.vigna...@nokia.com wrote: The problem is that people are creating patches but they got lost somewhere and/or take so many time to go upstream. Am I still dropping patches on the floor somehow? I don't think so. You've been doing the work pretty good in my opinion. But I'm mostly worried with patches not related with bugfixing, that will appear while we're in the huge bugfixing time-frame. I'll discuss bellow... In my opinion, reducing the release schedule now would improve the throughput of patches going in. Probably three months for the whole release would be better with the majority of time being gates opened for merge window - two months maybe? Daniel's point here is that a shorter release cycle will increase the number of releases that we as a community need to maintain as upstream and in distributions. Having to do security updates for one release is hard enough, having to do them simultaneously for many releases would be even harder. Right, seems developers are opposing to shorten the release schedule time and I perfectly understand the reasons. Mostly surrounds the goal of regular vendors that needs to keep a big range of devices working (almost all Linux distributions), which means big time to stabilize all drivers and, mainly, support them after X is released. On the other hand, MeeGo-like distributions supports just a few devices and can quite easily follow the shiny, paraphrasing Dave. Different goals within the same product :/ Even so, our problem remains: while we'll be living inside these three months of bugfixing phase, patches will come to the list, reviewing will be made and very few patches are going to be queued on developers' trees. Mostly will be floating in the list for _three_ _huge_ months. I can consider two options that fits for me while we are in this bugfixing mode: either a xserver-meego or a xserver-staging. - xserver-meego would be what I particularly care for MeeGo devices in a near future. Could be my work or any other patches floating in the mailing list that I care. When the upstream merge window opens, I will definitely want to merge this tree there (by any chance we want a fork!). I'm the only one committing there and I'd be selfish because other parallel works that I don't care won't be applied there. - xserver-staging would be a tree for everyone interesting in development while upstream are in bugfixing. This tree (well, -meego also) stays opened only when the merge window is closed. The administrators of -staging would pull basically *everything* reviewed in the mailing list there. Besides pulling minor patches, any developer with a given work that is ready to be merged upstream (Keith's tree) and it doesn't fit as fix, is encouraged to push there. When upstream open the gates again, we sync both trees and close -staging. This tree should aid Keith only, so the sync part should be done very carefully. For instance, I'm not sure that a single pull req on the beginning of the merge window is enough because Keith would be overloaded. One single guy running it won't make sense. For instance Jamey, Mikhail and other guys running fast development could use / administrate the same tree. It resembles just in a bit the linux-staging. It resembles a bit also the idea of swap RMs in each versioning, which now would be sort-of 3 months. There would be more people working, thus more confusing. In general is more complex also. xserver-meego will not solve the issue of floating patches at mailing list, while xserver-staging solves. xserver-meego requires less work from me. xserver-staging requires more work from me and will be successful only if there would be a real collaboration between developers. Real consumers, i.e. people testing, is by far not the goal of both trees. The goal is to fasten development while upstream stays in freeze mode. What you think? Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH v3 1/2] dix: use one single function to register fpe fonts
On Wed, Jun 16, 2010 at 05:52:23PM +0200, Vignatti Tiago (Nokia-D/Helsinki) wrote: X server doesn't need to understand fpe internals, so use register_fpe_functions from libXfont. It's required to get new version of libXfont, therefore adjust it to be passed to autoconf. Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com Reviewed-by: Mikhail Gusarov dotted...@dottedmag.net Reviewed-by: Alex Deucher alexdeuc...@gmail.com ping? We got also one more review in this series (lib and server side): Reviewed-by: Daniel Stone dan...@fooishbar.org Thanks, Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH v3 1/2] dix: use one single function to register fpe fonts
On Sat, Jun 19, 2010 at 05:34:52PM +0200, ext Dan Nicholson wrote: On Wed, Jun 16, 2010 at 8:52 AM, Tiago Vignatti tiago.vigna...@nokia.com wrote: X server doesn't need to understand fpe internals, so use register_fpe_functions from libXfont. It's required to get new version of libXfont, therefore adjust it to be passed to autoconf. Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com Reviewed-by: Mikhail Gusarov dotted...@dottedmag.net Reviewed-by: Alex Deucher alexdeuc...@gmail.com --- changes from v2: - modified with Julien's suggestion to not check libXfont again due it's being already being done later when REQUIRED_LIBS is checked. configure.ac | 9 + dix/dixfonts.c | 4 +--- include/dixfont.h | 5 + 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/configure.ac b/configure.ac index d41191f..bb4f445 100644 --- a/configure.ac +++ b/configure.ac @@ -794,7 +794,7 @@ APPLEWMPROTO=applewmproto = 1.4 dnl Core modules for most extensions, et al. REQUIRED_MODULES=[randrproto = 1.2.99.3] [renderproto = 0.11] [fixesproto = 4.1] [damageproto = 1.1] [xcmiscproto = 1.2.0] [xextproto = 7.0.99.3] [xproto = 7.0.17] [xtrans = 1.2.2] [bigreqsproto = 1.1.0] fontsproto [inputproto = 1.9.99.902] [kbproto = 1.0.3] -REQUIRED_LIBS=xfont xau +REQUIRED_LIBS=xau dnl List of libraries that require a specific version LIBAPPLEWM=applewm = 1.4 @@ -803,6 +803,7 @@ LIBDRI=dri = 7.8.0 LIBDRM=libdrm = 2.3.0 LIBGL=gl = 7.1.0 LIBXEXT=xext = 1.0.99.4 +LIBXFONT=xfont = 1.4.2 LIBXI=xi = 1.2.99.1 LIBXTST=xtst = 1.0.99.2 LIBPCIACCESS=pciaccess = 0.8.0 @@ -812,10 +813,10 @@ LIBSELINUX=libselinux = 2.0.86 LIBDBUS=dbus-1 = 1.0 LIBPIXMAN=pixman-1 = 0.15.20 -dnl Pixman is always required, but we separate it out so we can link -dnl specific modules against it +dnl Pixman and Xfont are always required. For pixman we separate it out so we +dnl can link specific modules against it PKG_CHECK_MODULES(PIXMAN, $LIBPIXMAN) -REQUIRED_LIBS=$REQUIRED_LIBS $LIBPIXMAN +REQUIRED_LIBS=$REQUIRED_LIBS $LIBPIXMAN $LIBXFONT I don't understand this diff at all. Why not just do -REQUIRED_LIBS=xfont xau +REQUIRED_LIBS=xfont = 1.4.2 xau since it ends up in exactly the same place (REQUIRED_LIBS). It's being used in exactly the same way it always was, but we just require a newer version. I've been cheated by the commentary List of libraries that require a specific version, given the server now would require the given version of xfont. Even so I do agree with you that is much more cleaner simply put it REQUIRED_LIBS. Do you think I should move there and change the commentary a bit? Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 2/2] xfixes: Use list.h macros in tracking hide count
On Mon, Jun 14, 2010 at 05:48:54PM +0200, ext Kristian Høgsberg wrote: 2010/6/14 Vignatti Tiago (Nokia-D/Helsinki) tiago.vigna...@nokia.com: I think open coded list implementation everywhere is more error prone than just have one single set of basic macros. Isn't it? All the open coded lists in ther server are there now, and they're working. If it's not broken, don't mess with it. If you come across an error in an open coded list implementation, that would be a good reason to port it to list.h. But otherwise, you're just almost certainly going to break working code. I disagree with you Kristian. Pick any code related with clean-up we had done recently; for instance the replacement of the memory allocation function wrappers by the C89 ones. We can say that they were working, but this wouldn't be an argument to not remove. If X would have a shiny code base and a driver API stable enough then I'd agree with you to not touch. But you know we still have a way long to go to achieve it. Seems that we're living in a era that Xorg project has more developers than never [0], so the moment of clean up (including low-hanging fruits like this list stuff) is right now. Cheers, [0] the traffic on xorg-devel on last months are _at_ _least_ 4 times higher than one year ago. Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: Performance improvement to vga arbitration
Hi Henry, On Sat, Jun 12, 2010 at 04:38:14AM +0200, ext Henry Zhao wrote: On 06/11/10 04:56, Tiago Vignatti wrote: http://people.freedesktop.org/~vignatti/tmp/0001-xfree86-vgaarb-disable-VGA-decoding-after-POST.patch At this point on the X server, we already POSTed all cards and we could be optimistic, letting the drivers say when we actually need VGA legacy. Right now, as you said, we're assuming legacy access as default whenever the system has more than one card, not driven by DRI (DRI and VGA legacy conflict with the current design. Dave preferred to let this away). The patch didn't go to git master yet ? not yet. Do you mind to do a proper review and insert your review tag there? For the X, we have a strict and rigorous development process now, trying to minimize the amount of errors being pushed upstream trees. For this we needed to set up a way get patches reviewed by stamping a review tag. In general it's not so different from what linux kernel is being doing. You may want to take a look on these wiki pages: http://www.x.org/wiki/XServer http://wiki.x.org/wiki/Development/Documentation/SubmittingPatches We still need arbitration once a device gets graphics mode. Say devices a and b are working in graphics mode, then (1) A new device c may start, which does legacy accesses during initialization; (2) While b is still running, device a may exit, which also involves legacy accesses. Therefore we need to use another lock (locks2), in addition to the current locks (for legacy accesses), for non-legacy accesses such that locks2 and locks are in conflict, but locks2s themselves are not. Besides, we cannot rule out possibilities that some drivers may use legacy access in some operations in graphics mode in certain cases, therefore we need to give drivers a final say on kind of accesses they use for the operations, although the default is non-legacy access. right, I was missing the hotplug case. It makes perfect sense for me. And yes, I'd prefer to keep VGA decoding disabled by default after the session is up, exactly as my patch is doing. Also, I posted two patches to fix the userspace decoding interface on the mailing list some days ago. If you're carrying about remove cards from arbitration and stop decoding then definitely you'll want to take a look on those: http://lists.x.org/archives/xorg-devel/2010-June/009559.html I had this fixed in my patch. can you please put your reviewing tags please? 3 patches are posted. Your explanation was good enough and clear for me. However, in all of the three patches, you're inserting more modifications than you described here. It makes the review difficult and tough. A good habit is to split one in a series in which each patch has a different semantical meaning - one for cleaning up only, another introducing a hook function, another for the actual changes and so on. If possible, if each patch is independent from the other (able to compile) then it would also ease the search for some possible errors (using git-bisect) I'd please ask you to take a look on this page and re-send the patches, using a proper git-format-patch style: http://wiki.x.org/wiki/Development/Documentation/SubmittingPatches I have to admit that there are some parts of your kernel patch that simply goes being the scope of my knowledge. I'm pretty sure if you follow this tips on the wiki we're gonna be able to bring other guys from the community to do the review. Thank you, Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] dix: fix NULL pointer dereferenced in memset
Hey Jamey, On Sat, Jun 12, 2010 at 05:23:09PM +0200, ext Jamey Sharp wrote: On Sat, Jun 12, 2010 at 7:39 AM, Tiago Vignatti tiago.vigna...@nokia.com wrote: Apparently memset doesn't complain if the memory area is null (addr) and something is being written there. Even though, this patch guarantees that nothing is written at 0x0 memory address. I'm confused by this comment. Did you get a segfault, or what? What do you mean by memset doesn't complain, and why is the patch necessary? I've just checked POSIX and C99, and neither one specifies anything about memset's behavior when length is 0 and address is null. Seems like no correct implementation could possibly dereference the null pointer though... No, I didn't get a segfault. So this patch is not exactly necessarily. I'm playing a bit with a static analysis tool, which complained about this NULL pointer dereference. So maybe this patch stills valid as a matter to fix only a bad habit of programming, right? Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 1/2 v2] Use one single function to register fpe functions
On Fri, Jun 11, 2010 at 02:35:23PM +0200, Vignatti Tiago (Nokia-D/Helsinki) wrote: X server doesn't need to understand fpe internals, so let it transparent turning all registration functions in a single one. For that, fill the already existent register_fpe_functions(). Some X servers don't want font server support, so this patch also sets font server support to be configured in build time. In my machine, I see 20kB of RSS being saved in libXfont mapped in Xorg process when I disabled font server support and other kind of fonts in the library (--disable-pcfformat --disable-bdfformat --disable-snfformat --disable-freetype --disable-fc). The default library built was taking: textdata bss dec hex filename 26184744841536 267867 4165b ./lib/libXfont.so and with these flags, it jumps to: textdata bss dec hex filename 15776424281188 161380 27664 ./lib/libXfont.so Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com argh. I forgot to stamp here the reviews: Reviewed-by: Mikhail Gusarov dotted...@dottedmag.net Reviewed-by: Alex Deucher alexdeuc...@gmail.com Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: Input thread on EVoC
Oi, On Wed, Jun 09, 2010 at 02:37:22AM +0200, ext Fernando Carrijo wrote: Tiago Vignatti tiago.vigna...@nokia.com wrote: it's not that straightforward given, as the guys said already, X event dequeuing is very tied with clients and the server may spend considerable amount of time doing the locking/unlocking dance only. But it is worth trying, right? I guess no one is sure whether lock the client output buffer is a good idea or not. It will depend exactly the amount of code we'll be locking there, and in the end if we are able to have always a low lock contention between the threads. To have a fine granularity within code here is the hint. Honestly, I may be telling you a dumb approach here, but I think the best way to see if it's worth or not is just going and implement. Hard to say. Agreed. I have rebased your input thread code upon a local branch based on xserver master and as soon as I figure out how to solve some issues with the s3virge driver which serves me at home, I will start benchmarking the X input subsystem. Some featureful tools come to my mind, like x11perf and perf itself, but if you know about anything more appropriate, please enlighten me. be sure you can have both sw and hw cursor available with this s3virge driver. Also a SMP machine is desirable to see significant improvements probably. For the tools, I can think in a simple program that keeps moving the cursor while at the same moment tries to draw things on screen. x11perf + xtest probably do the trick for you. But I'm not that sure xtest will follow exactly the same input paths... I fear I couldn't parse what you said above. When you talk about the lack of predictability, isnt it a natural consequence of us relinquishing the burden of process scheduling and caring only about client scheduling? Maybe you implied that it is important for us to offer correcteness of execution by having some control on thread scheduling? when myself and Daniel designed the input thread for the event generation, we thought that such thread would have a small footprint which would be always be kept in the memory when the device is moving. And whenever needed the CPU would schedule such thread, which in turn would do the work instantly. However that wasn't the case in practice. We couldn't see any apparent improvement and sometimes I had the impression that was performing worst. Sigh. The reason wasn't so clear on that time whether the problem was in fact with the kernel scheduler (taking it off and not prioritizing its execution on a needed time) or the thread going swap to disk. I'd guess the later. I didn't even try anything like this before, but if I lived in the desert whith no one else to ask, mlocking would be my first try. Why do people refrain from using things like __atribute__(__section__(input_thread_related)) and some linker trickery, à la ld scripts, to put ELF sections into well known virtual memory addresses? Lack of portability is the cause, isn't it? Actually I tried exactly this. But I'm not sure if this is the best and more beautiful way though. As I said on the paragraph before, first we have to be sure memory (the thread swap back do disk) is the problem here. If this is the case then we can start to work in ways to solve it. Hard to say. But definitely start to chop of parts and thread them is one way to figure it out :) Yes. Peter said the same. To be honest, right now I'm prone to doing this out of EVoC, since it seems that the board expects some guarantees, specially related to timeline, which I cannot afford. The reason being that, as I said privately before, I have all the time in the world, but unfortunatelly not all expertise. Either way, I'm really really really keen to start exploring and coding, in or out of EVoC. :) Okay, I'd say to just send the proposal to the board and let's see the feedback there now. G'luck! :) Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCHv2 0/6] fbdevhw cleanup and pciaccess free
Hi, On Tue, Jun 01, 2010 at 02:22:29PM +0200, Vignatti Tiago (Nokia-D/Helsinki) wrote: Here's some trivial cleanup of the fbdevhw module. Just like my last patch series I sent to the list, this one also removes PCI related symbols from a given module - fbdevhw in this case - and puts in a meaningful place [0]. From patch set version 1, only patch #5 and #6 changed: myself and Michel exchanged some messages discussing about the removal or not of the secondary way to match fb - PCI. Given stills obscure its real need, I just left the code there, changing the location only. Mark Kettenis and Alan pointed about the file location of the new xf86PciOpenFbdev function, which went to OS specific place. Besides, Mark Kettenis complained about patch #2. I still don't believe anyone uses this kind of debugging so I left intact the patch there. But I can be wrong though. If someone wants to give a try on this patchset, it was just rebased on the current master and lives here: http://cgit.freedesktop.org/~vignatti/xserver/log/?h=fbdevhw-cleanup git://anongit.freedesktop.org/~vignatti/xserver fbdevhw-cleanup Thanks, Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH resent 0/5] xfree86: vgaarb: clean up and device hidden
Hi, On Thu, Jun 03, 2010 at 11:29:46AM +0200, ext Mark Kettenis wrote: From: Tiago Vignatti tiago.vigna...@nokia.com Is it just me or are others also utterly confused by Tiago's diffs? Let me explain what I mean: Tiago Vignatti (5): xfree86: vgaarb: change macros by inline functions to ease debug This diff makes VGAGet_GC and VGAPut_GC inline functions... xfree86: vgaarb: remove superfluous and confusing VGAGet_GC and VGAPut_GC ...and then this diff removes them again. It's this sort of thing that keeps confusing me (the fact that the diffs end up arriving out of order in my mailbox doesn't make it easier either). If it is just me, I'll just keep my mouth shut after this. But otherwise I'd like to ask Tiago to spend a little bit more effort not to create patch series that touch the same bit of code multiple times. My main concern when creating a series is to keep each commit able to compile after it's introduced. This ease future bisects when searching for problems. And what happened in the commits you pointed above was that exactly. I was just trying to make sure every patch is compilable and the drawbacks are a bit of confusing like there. Anyway, thanks for pointing out. Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 4/5] xfree86: vgaarb: fix device decoding interface to send resources type properly
On Thu, Jun 03, 2010 at 11:42:18AM +0200, ext Mark Kettenis wrote: From: Tiago Vignatti tiago.vigna...@nokia.com Date: Thu, 3 Jun 2010 11:34:13 +0300 Right now, when there is more than one vide card on the machine, we're adopting a pessimistic approach and setting all cards to decode VGA legacy address. Some cards may want to skip the arbitration and the only way to do so is through pci_device_vgaarb_decodes. Therefore, send the desired kind of resource instead force the worst case. Note that xf86VGAarbiterDeviceDecodes is not being used so far by any open-source driver. Even so, API break. Ah, I've wondered about this quite a bit while writing the VGA arbitration code for the OpenBSD kernel. The old interface didn't make much sense to me. I'm a little bit worried though that none of the open source drivers uses this yet, since that means it is hard to test this bit of code. So you might be interested in this patch also: http://lists.x.org/archives/xorg-devel/2010-May/009087.html Reviewed-by: Mark Kettenis kette...@openbsd.org Thanks for reviewing! Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: more MAXSCREENS patches
Hi, On Wed, Jun 02, 2010 at 08:15:30PM +0200, ext Jamey Sharp wrote: So, yes. I think I'd like to see the xfree86 DDX made usable on all platforms that have active maintainers; in parallel, factor out common code between Xvfb/Xfake/dummy and between Xnest/Xephyr/Xdmx; and finally, unify those servers as drivers for the xfree86 DDX. That's not going to happen this week though. ;-) exactly! I've been thinking about this for months already. All DDX unified would be very beneficial in terms of development. Think for instance about devPrivates rework / MAXSCREENS removal. It would be much more easier and less painful if we had less lines of code. Another example is what Alan mentioned, where Sun simply decided to fork the server to contour the problem of Xorg being glued with buses hardware routines. Right now it is (well, was) so hard to separate all this hw infrastructure from the server that they preferred to go for another implementation. Same happened some months ago when I started to investigate the memory usage of Xorg. I decided to stop to work on the middle of the way because the server code stills huge and difficult to add any new idea. My main effort lately is only related with the organization of the code. I'm not caring about memory usage or other features in which the device I'm working would benefit immediately - it will eventually come after; and for free if the code is clear, small and easy to understand. So I really appreciate any move like you said above. Merging the video-dummy and input-void drivers into the xserver tree is an independent question, and I'd still like to see that happen in 1.9. I don't think any of the arguments for keeping drivers out-of-tree apply to these two. as Peter and Alan pointed out, the implementation is pretty much done already if we want to start the server without input drivers. Still missing the video side though. Before start to work on that I still would like to clean up a bit more bus code. My last patches to optionalize PCI code stills not enough. Better than that, we would want an xorg module called libbuses.so to dynamically load it when needed. It would be loaded if any access for buses would be needed and instead to hardcode the type of bus as we have now, it would have a callback choosing it. There's a quite big amount of implementation to make this possible. On the top of my head I can think in the bus entity code, to be removed (which is very tough cause all the drivers use some piece of it) and instead export xf86Pci.h for drivers, the API would be xf86Bus.h to have the proper callbacks, as I said. This would be _much_ more easier if the drivers would be together glued with the server (!!!). Bus related stuff inside ScrnInfoRec could be moved also to a new structure inside this new libbus.so. Cheers, Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] xfree86: Unbreak autoconfig following 0abf065e38c4
On Thu, May 27, 2010 at 02:26:23PM +0200, ext Chris Wilson wrote: The move of the PCI device id probing into a separate file neglected to return the number of found devices, and so the PCI devices were being overwritten by the default entries for vesa and fbdev. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Tiago Vignatti tiago.vigna...@nokia.com Cc: Alex Deucher alexdeuc...@gmail.com Reviewed-by: Julien Cristau jcris...@debian.org Reviewed-by: Tiago Vignatti tiago.vigna...@nokia.com Keith, can you please merge this asap to unbreak master? Thanks, Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] Install fbdevhw.so in the correct os-specific path
On Tue, Jun 01, 2010 at 03:17:26AM +0200, ext Alan Coopersmith wrote: Only if you're ready to say f*** you to people with out of tree drivers, whether closed source like nvidia ati, open source like openchrome Virtual Box, or just plain confused like Poulsbo. I'm not and since my employer would require maintaining a fork containing the loader code, I certainly hope most other people aren't either. you're correct, I was missing the closed-source ones. Besides, after your crusade to reduce memory usage are you really suggesting that users waste disk space and memory on the drivers for the two dozen video card models they aren't using? Memory-wise, if there's a dozen drivers linked and only one being used is the same as having only one linked. Memory counts only when the driver bits are touched. And regarding the space occupied, distributions that use Xorg for small set of devices (like MeeGo) would not be shipping with a bunch of video drivers linked with the server. Even though, disk is free and embedded people don't care that much. Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCHv2 6/6] xfree86: fbdevhw: remove all pci symbols from the module
On Tue, Jun 01, 2010 at 04:54:26PM +0200, ext Alan Coopersmith wrote: Tiago Vignatti wrote: xf86PciOpenFbDev was created and put inside its proper file. Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com --- Mark and Alan: I hope this is okay for you now? I can live with that, though I wonder if you're missing an _X_EXPORT to allow this function to be seen from the fbdevhw module. oh, nice catch Alan! The header is at xf86Pci.h now. I amend this locally. All this patch set is here also: http://cgit.freedesktop.org/~vignatti/xserver/log/?h=fbdevhw-cleanup Thank you Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 08/14] xfree86: delete useless Primary device is not PCI message
On Tue, Jun 01, 2010 at 05:30:14PM +0200, ext Daniel Stone wrote:
On Tue, Jun 01, 2010 at 05:59:04PM +0300, Tiago Vignatti wrote:
It's not PCI? So what?!
Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com
---
hw/xfree86/common/xf86pciBus.c |6 --
1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/hw/xfree86/common/xf86pciBus.c b/hw/xfree86/common/xf86pciBus.c
index a751427..b64bae3 100644
--- a/hw/xfree86/common/xf86pciBus.c
+++ b/hw/xfree86/common/xf86pciBus.c
@@ -1318,14 +1318,8 @@ xf86PciMatchDriver(char* matches[], int nmatches) {
}
pci_iterator_destroy(iter);
-
-if (!info) {
- ErrorF(Primary device is not PCI\n);
-}
#ifdef __linux__
-else {
matchDriverFromFiles(matches, info-vendor_id, info-device_id);
-}
#endif /* __linux__ */
for (i = 0; (i nmatches) (matches[i]); i++) {
Er, this changes the behaviour for non-PCI primary devices from printing
an arguably useless error, to segfaulting because info is NULL and is
now dereferenced unconditionally.
D'oh. Of course. Fixed locally.
Thanks Daniel!
Tiago
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 08/14] xfree86: delete useless Primary device is not PCI message
On Tue, Jun 01, 2010 at 06:29:28PM +0200, ext Tomas Carnecky wrote:
On 6/1/10 4:59 PM, Tiago Vignatti wrote:
It's not PCI? So what?!
Signed-off-by: Tiago Vignattitiago.vigna...@nokia.com
---
hw/xfree86/common/xf86pciBus.c |6 --
1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/hw/xfree86/common/xf86pciBus.c b/hw/xfree86/common/xf86pciBus.c
index a751427..b64bae3 100644
--- a/hw/xfree86/common/xf86pciBus.c
+++ b/hw/xfree86/common/xf86pciBus.c
@@ -1318,14 +1318,8 @@ xf86PciMatchDriver(char* matches[], int nmatches) {
}
pci_iterator_destroy(iter);
-
-if (!info) {
- ErrorF(Primary device is not PCI\n);
-}
#ifdef __linux__
-else {
matchDriverFromFiles(matches, info-vendor_id, info-device_id);
How much do Xorg developers care about indentation? This should be at
the same level as pci_iterator_destroy() a couple lines above.
How much do Xorg reviewers care about the logic and the correct behaviour of
the code without caring about tiny details like indentation?
Yeah, besides the indentation, the hunk itself was wrong.
Tiago
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 06/14] xfree86: bus: move macros from common PCI header to private file
Hi, On Tue, Jun 01, 2010 at 05:43:10PM +0200, ext Mark Kettenis wrote: I really, really don't see the point if this diff, and a lot of others you're sending. Yes, this is only used in the int10 module at this moment. But obviously Pci.h is the right place for these macros. It seems to me that you're just randomly shifting code around. I don't get the big picture, and your terse explanations don't give me any insight. So all I'm left with is the fear that this will break the platform and/or drivers I care about. If you pay attention in the X server code base, it still contains very old and complex PCI handling code that does things that only the kernel should do: moreover has some left over of ISA code, has int10's ugly and mostly not functional, the 16-bit x86 emulator enormous code, remnants from the dead Resource Access mechanism, it's mixed with the pciaccess library and so on. You will see that all PCI code is a recipe for disaster! Pciaccess library, which came for the good, was introduced in a very dubious way because implemented half of the functionally we needed on the server. The most apparent effect was the multi-card support, which is broken until today! And this is part of the practical situation we live nowadays. Now, in one perspective your reviews are very good, because you scrutinize very well each modification is done. I do really appreciate. On the other hand, in each patch I sent you criticize aspects of compatibility, that you're trying to keep, dating before 2006 when libpciaccess was introduced. I understand your conservative philosophy, but this really slows the overall process of development. Now the proper answer: OF COURSE I'M SHIFTING CODE AROUND, HONEY! int10's the only component needs that right now. AND IT'S NOT RANDOMLY! Pci.h file has the private definitions and won't break your jurrasic drivers. Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 12/14] xfree86: IOADDRESS is PCIism and shouldn't be used by common header files
On Tue, Jun 01, 2010 at 05:50:17PM +0200, ext Mark Kettenis wrote: From: Tiago Vignatti tiago.vigna...@nokia.com Date: Tue, 1 Jun 2010 17:59:08 +0300 This change does not make it easier to understand the code. Imagine someone looking at it after you apply this diff. What is the meaning of 'vertsyncreg'? Really, if the type is a PCIism, so are the variables that are declared using that type. That may be undesirable, but at least the use of IOADDRESS as a type makes that clear. If I move the IOADDRESS definition to xf86str.h will help then? Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] Install fbdevhw.so in the correct os-specific path
On Sun, May 30, 2010 at 11:57:04PM +0200, ext Alan Coopersmith wrote: I also don't know any reason not to move it out of the OS-specific libdir and up to the generic modules directory, now that all modules there use the OS'es actual libc system calls instead of the libcwrappers. exactly. And, at least using this approach, we'd get rid of all OS directories crap used by the loader. On the other hand, fbdevhw will keep being in a portable fashion due its stub version - while in theory shouldn't. You got my Acked here anyway. ajax had suggested this solution on IRC as it would be a no-op for Linux builders and just fix other OS'es, but we didn't discuss just moving up a level instead - which then raises the question of if there's any point having the module loader waste time doing all those extra stat() calls during module loading to find modules in OS-specific subdirs in a post-libcwrap Xorg. going more in depth: why we need a loader? Seriously, distros could have all drivers library linked with the server anyway, which generate trash in the memory only when using them. So not big deal. Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 5/6] xfree86: fbdevhw: remove secondary way to find fb PCI device
On Mon, May 31, 2010 at 03:58:46PM +0200, ext Michel Dänzer wrote: On Mon, 2010-05-31 at 16:16 +0300, Tiago Vignatti wrote: fbdevhw already relies in libpciaccess, which in turn relies in sysfs to probe devices. If sysfs is reporting wrong values then we're doomed anyway. So we totally rely on sys and this second suspicious way to match fb - PCI becomes superfluous. So the /sys/bus/pci/devices/%04x:%02x:%02x.%d/graphics/fb%d / /sys/bus/pci/devices/%04x:%02x:%02x.%d/graphics:fb%d files were already there in the minimum Linux kernel version required by libpciaccess? I don't know the answer, but why you care? Sorry, I think I didn't get what you're trying to say with this question. Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 0/6] fbdevhw cleanup and pciaccess free
On Mon, May 31, 2010 at 04:29:11PM +0200, ext Mark Kettenis wrote: From: Tiago Vignatti tiago.vigna...@nokia.com [0] I'm wondering if it doesn't make sense for someone to free modules from PCI code? For me it's pretty clear but I'll explain the machiavellian plan anyway in the next series that I'll send today. So instead you decide to pollute the generic PCI code with Linux-specific cra^H^H^Hcode? can you please point exactly where I'm polluting generic PCI code with Linux specific? I found your commentary pretty interesting because this is one of the problems I'm trying to address with the cleaning up patches I've been sending. Anyway, thanks for checking the series Mark! Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 5/6] xfree86: fbdevhw: remove secondary way to find fb PCI device
On Mon, May 31, 2010 at 04:36:51PM +0200, ext Michel Dänzer wrote: On Mon, 2010-05-31 at 17:10 +0300, Vignatti Tiago (Nokia-D/Helsinki) wrote: On Mon, May 31, 2010 at 03:58:46PM +0200, ext Michel Dänzer wrote: On Mon, 2010-05-31 at 16:16 +0300, Tiago Vignatti wrote: fbdevhw already relies in libpciaccess, which in turn relies in sysfs to probe devices. If sysfs is reporting wrong values then we're doomed anyway. So we totally rely on sys and this second suspicious way to match fb - PCI becomes superfluous. So the /sys/bus/pci/devices/%04x:%02x:%02x.%d/graphics/fb%d / /sys/bus/pci/devices/%04x:%02x:%02x.%d/graphics:fb%d files were already there in the minimum Linux kernel version required by libpciaccess? I don't know the answer, but why you care? Because otherwise this change will break at least the fbdev driver on some systems. How it will break? I though I was pretty clear regarding the commit message saying that we have to trust on sysfs otherwise we're doomed. When I mentioned libpciaccess there was just to mention that we are already relying on sysfs for other purposes. I'm afraid I'm still not getting your point Michel :/ Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 2/6] xfree86: fbdevhw: remove superfluous debug code
On Mon, May 31, 2010 at 04:24:27PM +0200, ext Mark Kettenis wrote: From: Tiago Vignatti tiago.vigna...@nokia.com Date: Mon, 31 May 2010 16:16:52 +0300 Rarely one would want to print all functions of a given file when debugging. If this is the case, then a mix of ctags + cpp + gdb probably can do the same (useless?) job. That's why that #define DEBUG 0 is there! sorry, but this doesn't convince me to keep applying the changes this patch do. Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 0/6] fbdevhw cleanup and pciaccess free
On Mon, May 31, 2010 at 05:04:37PM +0200, ext Mark Kettenis wrote:
Date: Mon, 31 May 2010 17:42:51 +0300
From: Vignatti Tiago (Nokia-D/Helsinki) tiago.vigna...@nokia.com
On Mon, May 31, 2010 at 04:29:11PM +0200, ext Mark Kettenis wrote:
From: Tiago Vignatti tiago.vigna...@nokia.com
[0] I'm wondering if it doesn't make sense for someone to free modules
from
PCI code? For me it's pretty clear but I'll explain the machiavellian
plan
anyway in the next series that I'll send today.
So instead you decide to pollute the generic PCI code with
Linux-specific cra^H^H^Hcode?
can you please point exactly where I'm polluting generic PCI code with Linux
specific? I found your commentary pretty interesting because this is one of
the problems I'm trying to address with the cleaning up patches I've been
sending.
The bit below.
the reason for that is on the commit message. Or maybe I should be more clear?
--- a/hw/xfree86/os-support/bus/Pci.c
+++ b/hw/xfree86/os-support/bus/Pci.c
@@ -134,6 +134,7 @@
#include Pci.h
#include pciaccess.h
+#include fcntl.h
/* Global data */
@@ -155,3 +156,24 @@ xf86scanpci(void)
return success;
}
+
+int
+xf86PciOpenFbDev(struct pci_device *pPci, int num)
+{
+char filename[256];
+int fd;
+
+sprintf(filename,
+/sys/bus/pci/devices/%04x:%02x:%02x.%d/graphics/fb%d,
+pPci-domain, pPci-bus, pPci-dev, pPci-func, num);
+
+fd = open(filename, O_RDONLY, 0);
+if (fd 0) {
+sprintf(filename,
+/sys/bus/pci/devices/%04x:%02x:%02x.%d/graphics:fb%d,
+pPci-domain, pPci-bus, pPci-dev, pPci-func, num);
+fd = open(filename, O_RDONLY, 0);
+}
+
+return fd;
+}
Tiago
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 5/6] xfree86: fbdevhw: remove secondary way to find fb PCI device
On Mon, May 31, 2010 at 05:03:56PM +0200, ext Michel Dänzer wrote: sysfs didn't appear in its current form instantly but in evolutionary steps. AFAIR the files above (which with your change are required for the matching to work) were added relatively recently, and it might be possible for libpciaccess to otherwise work fine with kernels which didn't have them. Now I may well be wrong about this, but it would be nice if you could check rather than asserting the code is 'superfluous'. Ah, okay. Now I understood what you're saying. X server's commit 46f55f5d, dating Jun 2006, introduced the code to use the interface /sys/bus/pci/devices/*/graphics:fb*. So I'd presume sysfs was working since that time and that we're safe enough to remove this secondary way to match PCI devices (the actual changes introduced by my patch). Therefore, if no one loudly scream now and if it's fine for you I'll keep my patch as it is (in the worst case should be pretty easy to create a hook for this on Pci.c though). Okay? Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] xfree86: Unbreak autoconfig following 0abf065e38c4
Hi Chris,
On Wed, May 26, 2010 at 09:44:51PM +0200, ext Chris Wilson wrote:
The move of the PCI device id probing into a separate file neglected to
return the number of found devices, and so the PCI devices were being
overwritten by the default entries for vesa and fbdev.
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Reviewed-by: Tiago Vignatti tiago.vigna...@nokia.com
We also can add a short description here. Something like:
/*
* @return The numbers of found devices that match with the current system
* drivers.
*/
Can you amend this, please?
-void
+int
xf86PciMatchDriver(char* matches[], int nmatches) {
int i;
struct pci_device * info = NULL;
@@ -1326,4 +1326,6 @@ xf86PciMatchDriver(char* matches[], int nmatches) {
if ((info != NULL) (i nmatches)) {
i += videoPtrToDriverList(info, (matches[i]), nmatches - i);
}
+
+return i;
}
Thanks,
Tiago
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: State of the 1.9 release
Hi, On Thu, May 20, 2010 at 05:02:15PM +0200, ext Keith Packard wrote: git://people.freedesktop.org/~keithp/xserver fix-private-usage I see that you're registering some structures that are used only in xfree86 ddx using dix devPrivates. But for this ddx seems we have a similar mechanism to of privates inside ScrnInfoRec (xf86AllocateScrnInfoPrivateIndex, or just everything that uses DevUnion). So makes sense to deprecate the ddx one and use only dix? Even so: Tested-by: Tiago Vignatti tiago.vigna...@nokia.com Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 04/11] xfree86: bus: remove superfluous and confused structures in BusRec
On Fri, May 21, 2010 at 04:40:59PM +0200, ext Keith Packard wrote:
On Fri, 21 May 2010 14:43:17 +0300, Tiago Vignatti tiago.vigna...@nokia.com
wrote:
Although API is break, luckily any drivers right now is using such monster.
diff --git a/hw/xfree86/common/xf86str.h b/hw/xfree86/common/xf86str.h
index 485c15a..e5713db 100644
--- a/hw/xfree86/common/xf86str.h
+++ b/hw/xfree86/common/xf86str.h
@@ -355,16 +355,10 @@ typedef enum {
struct pci_device;
-typedef struct {
-intfbNum;
-} SbusBusId;
-
typedef struct _bus {
BusType type;
-union {
- struct pci_device *pci;
- SbusBusId sbus;
-} id;
+struct pci_device *pci;
+int sbus;
} BusRec, *BusPtr;
This doeesn't make sense to me -- a bus is either an SBus or PCI, and so
sticking those alternatives in a union (and nicely tagged at that with
the 'type') is logical, even if it does add another level of naming to
accessing the elements.
I'd say it does make sense but depends on the style of the programmer. But
c'mon, look again at this naming level:
- return (pEnt-bus.id.sbus.fbNum == primaryBus.id.sbus.fbNum);
+ return (pEnt-bus.sbus == primaryBus.sbus);
the previous code seems pretty messy!
Did I change your opinion? :)
Tiago
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 03/11] xfree86: bus: move IOADDRESS definition to a common header file
On Fri, May 21, 2010 at 04:21:26PM +0200, ext Alan Coopersmith wrote: Tiago Vignatti wrote: IOADDRESS type definition encompass more than just pci code, so move it to common. Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com --- hw/xfree86/common/xf86str.h |1 + hw/xfree86/os-support/bus/xf86Pci.h |3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/xfree86/common/xf86str.h b/hw/xfree86/common/xf86str.h index de1f1b6..485c15a 100644 --- a/hw/xfree86/common/xf86str.h +++ b/hw/xfree86/common/xf86str.h @@ -58,6 +58,7 @@ typedef uint64_t memType; typedef uintptr_t memType; #endif +typedef unsigned long IOADDRESS;/* Must be large enough for a pointer */ If it must be large enough for a pointer, why not use uintptr_t like the code two lines before it? I don't know. Seems uintptr_t is unsigned int in Linux, does it seems enough to carry IO address there? Ajax? Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 03/11] xfree86: bus: move IOADDRESS definition to a common header file
On Fri, May 21, 2010 at 04:55:23PM +0200, ext Mark Kettenis wrote: From: Tiago Vignatti tiago.vigna...@nokia.com Date: Fri, 21 May 2010 14:43:16 +0300 IOADDRESS type definition encompass more than just pci code, so move it to common. diff --git a/hw/xfree86/os-support/bus/xf86Pci.h b/hw/xfree86/os-support/bus/xf86Pci.h index ce1336b..a9a2779 100644 --- a/hw/xfree86/os-support/bus/xf86Pci.h +++ b/hw/xfree86/os-support/bus/xf86Pci.h @@ -235,7 +235,6 @@ /* Primitive Types */ typedef unsigned long ADDRESS; /* Memory/PCI address */ -typedef unsigned long IOADDRESS; /* Must be large enough for a pointer */ typedef unsigned long PCITAG; Moving IOADDRESS over, but leaving the other ibviously related ones behind, seems a bad idea to me. All right, I'll do it. But honestly I've done this initial patch a bit against my will because all this typedefs should be only constrained inside PCI code, but with the current code base seems really tough :( Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH resent] configure: introduce --{enable, disable}-fontserver
On Fri, May 21, 2010 at 04:54:01PM +0200, ext Keith Packard wrote: On Wed, 5 May 2010 17:14:41 +0300, Tiago Vignatti tiago.vigna...@nokia.com wrote: Keith, no one took this one. Care to pull it? I though I had replied suggesting that this should depend on what support was provided in the system libXfont instead of having the server be configured separately. yeah, adjust .pc from libXfont seems to solve the issue. I'm just lazily hanging this work for some days because I don't know exactly the trick to do so. Maybe some autoconf ninja on the list knows? Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 1/2] xfree86: vgaarb: simplify the arguments passed to lock/unlock
On Fri, May 21, 2010 at 06:43:11PM +0200, ext Keith Packard wrote: On Fri, 21 May 2010 18:48:14 +0300, Tiago Vignatti tiago.vigna...@nokia.com wrote: Send only screen index instead the whole rec for lock and remove the argument of unlock. nak -- the arbiter presumably needs to run during server initialization before the screen is allocated, which means that any per-GPU data will end up hanging from the scrninfo in any case. You gave the nak for the wrong patch from the series, right? First patch is only a cleanup. Anyway, as I said in the second patch, I'm pretty sure we can allocate ScreenRec before to make this idea succeeds. I'll think about it and come back with something. Tiago ~ ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: multi-card breakage
Hi Pierre,
On Tue, May 04, 2010 at 10:21:55PM +0200, ext Pierre-Loup A. Griffais wrote:
I just reproduced something that sounds like what you're describing with two
R520 cards (one X screen per card) and the 'radeon' driver. However, it seems
unrelated to my change; that's what the hang looks like:
575 VGAGet();
(gdb) bt
#0 VGAarbiterCreateGC (pGC=0x83ebab0)
at ../../../../hw/xfree86/common/xf86VGAarbiter.c:575
#1 0x080777ba in CreateGC (pDrawable=0x82d8d78, mask=value optimized out,
pval=0xb534, pStatus=0xb53c, gcid=0, client=0x81ffca8)
at ../../dix/gc.c:647
#2 0x0819e612 in miDCMakeGC (pWin=0x82b5530) at ../../mi/midispcur.c:422
#3 0x0819e7c4 in miDCDeviceInitialize (pDev=0x83ebdf0, pScreen=0x8263688)
at ../../mi/midispcur.c:790
#4 0x081c48cf in miSpriteDeviceCursorInitialize (pDev=0x83ebdf0,
pScreen=0x8263688) at ../../mi/misprite.c:949
#5 0x08186364 in xf86DeviceCursorInitialize (pDev=0x83ebdf0,
pScreen=0x8263688) at ../../../../hw/xfree86/ramdac/xf86Cursor.c:453
#6 0x081672ba in VGAarbiterDeviceCursorInitialize (pDev=0x83ebdf0,
pScreen=0x8263688) at ../../../../hw/xfree86/common/xf86VGAarbiter.c:1035
#7 0x080a1e0c in miPointerDeviceInitialize (pDev=0x83ebdf0,
pScreen=0x8263688)
at ../../mi/mipointer.c:283
#8 0x08087ed5 in ActivateDevice (dev=0x83ebdf0, sendevent=1 '\001')
at ../../dix/devices.c:477
#9 0x08088f08 in InitCoreDevices () at ../../dix/devices.c:610
#10 0x08066d18 in main (argc=1, argv=0xb8a4, envp=0xb8ac)
at ../../dix/main.c:255
The reason my change exposes this bug is that it creates a GC attached to the
second screen upfront. If I roll it back, I still get the same hang after
trying
to move a SW cursor to the second screen of connecting an X client to the
second
screen.
I'm still getting a very weird lock-up with your patch. I can get it even with
hw cursor. Seems not related at all with the log bellow when radeon POST bios,
so I guess your commit added a regression. Just reverting it solves the
problem - and sorry, I don't know this code in depth to start dig the reason.
BTW, Peter did you test this patch there with multiple cards? I'd revert this
patch meanwhile (attached).
Looking at the X log, I see:
(II) RADEON(1): PCIE card detected
(II) Loading sub module int10
(II) LoadModule: int10
(II) Reloading /usr/lib/xorg/modules/libint10.so
(II) RADEON(1): initializing int10
(EE) RADEON(1): Cannot read V_BIOS (3) Input/output error
(WW) RADEON(1): Failed to read PCI ROM!
(II) RADEON(1): Attempting to read un-POSTed bios
and in the kernel log:
[ 1240.582149] pci :05:00.0: Invalid ROM contents
That means the VGA arbiter tried to switch VGA access to an un-posted device,
which is presumably the cause of the hang. It seems like the X screen should
fail ScreenInit() and get discarded after initializing int10 fails. Whatever
the
reason behind that is, the driver ought to fail more gracefully.
In any case, I'm guessing you have similar spew in your logs?
no. My logs are normal, without any apparent errors.
Thank you,
Tiago
From efe63a191e72eef40454a2bd7cf3a8f0d0d41389 Mon Sep 17 00:00:00 2001
From: Tiago Vignatti tiago.vigna...@nokia.com
Date: Wed, 19 May 2010 17:29:25 +0300
Subject: [PATCH] Revert mi: don't thrash resources when displaying the software cursor across screens
This reverts commit 518f3b189b6c8aa28b62837d14309fd06163ccbb.
Conflicts:
mi/midispcur.c
---
mi/midispcur.c | 269 +--
1 files changed, 161 insertions(+), 108 deletions(-)
diff --git a/mi/midispcur.c b/mi/midispcur.c
index 1acc469..ccc8ffa 100644
--- a/mi/midispcur.c
+++ b/mi/midispcur.c
@@ -59,9 +59,9 @@ static DevPrivateKey miDCScreenKey = miDCScreenKeyIndex;
static Bool miDCCloseScreen(int index, ScreenPtr pScreen);
-/* per device per-screen private data */
-static int miDCSpriteKeyIndex[MAXSCREENS];
-static DevPrivateKey miDCSpriteKey = miDCSpriteKeyIndex;
+/* per device private data */
+static int miDCSpriteKeyIndex;
+static DevPrivateKey miDCSpriteKey = miDCSpriteKeyIndex;
typedef struct {
GCPtr pSourceGC, pMaskGC;
@@ -75,10 +75,10 @@ typedef struct {
#endif
} miDCBufferRec, *miDCBufferPtr;
-#define MIDCBUFFER(dev, screen) \
+#define MIDCBUFFER(dev) \
((DevHasCursor(dev)) ? \
- (miDCBufferPtr)dixLookupPrivate(dev-devPrivates, miDCSpriteKey + (screen)-myNum) : \
- (miDCBufferPtr)dixLookupPrivate(dev-u.master-devPrivates, miDCSpriteKey + (screen)-myNum))
+ (miDCBufferPtr)dixLookupPrivate(dev-devPrivates, miDCSpriteKey) : \
+ (miDCBufferPtr)dixLookupPrivate(dev-u.master-devPrivates, miDCSpriteKey))
/*
* The core pointer buffer will point to the index of the virtual core pointer
@@ -158,6 +158,10 @@ miDCInitialize (ScreenPtr pScreen, miPointerScreenFuncPtr screenFuncs)
return TRUE;
}
+#define tossGC(gc) (gc ? FreeGC (gc, (GContext) 0) : 0)
Re: [PATCH 00/10 v2] Clean up and reorganize BUS configuration code
On Fri, May 07, 2010 at 02:43:55PM +0200, Vignatti Tiago (Nokia-D/Helsinki) wrote: Tiago Vignatti (10): xfree86: remove unused xf86AccessInit() xfree86: bus: fix Enter/Leave accesses behaviour xfree86: bus: reuse already assigned variable when fb driver claimed xfree86: bus: rework xf86PostProbe logic and remove useless log info xfree86: bus: enable declaration of sparc function as its code usage xfree86: bus: simplify entity related hooks xfree86: bus: fb drivers might want to use vga arbitration either xfree86: bus: remove SetSIGIOForState and simplify the code xfree86: remove xf86EnableAccess xfree86: remove PCI dependency from InitOutput Clarifying a bit the changes from v1: only patch #5 was changed based on Mark's comment. The rest of commentaries were about the API break when xf86EnableAccess is removed (#9). For this one, the work was done, and applied already, in driver side. Thanks, Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 10/10] xfree86: remove PCI dependency from InitOutput
On Fri, May 07, 2010 at 04:39:40PM +0200, ext Adam Jackson wrote:
On Fri, 2010-05-07 at 15:44 +0300, Tiago Vignatti wrote:
+Bool
+xf86CallDriverProbe( DriverPtr drv, Bool detect_only )
+{
+Bool foundScreen = FALSE;
+
+if ( drv-PciProbe != NULL ) {
+if ( xf86DoConfigure xf86DoConfigurePass1 ) {
+assert( detect_only );
+foundScreen = xf86PciAddMatchingDev(drv);
+}
+else {
+assert( ! detect_only );
+foundScreen = xf86PciProbeDev(drv);
+}
+}
+
+if ( ! foundScreen (drv-Probe != NULL) ) {
+xf86Msg( X_WARNING, Falling back to old probe method for %s\n,
+ drv-driverName );
+foundScreen = (*drv-Probe)( drv, (detect_only) ? PROBE_DETECT
+ : PROBE_DEFAULT );
+}
+
+return foundScreen;
+}
You're missing some indent levels here. In fact most of this patch
seems bizarrely outdented in places. I think your editor is doing
something strange.
All right. Fixed locally.
Thanks,
Tiago
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 08/10] xfree86: bus: remove SetSIGIOForState and simplify the code
On Fri, May 07, 2010 at 04:32:32PM +0200, ext Adam Jackson wrote: On Fri, 2010-05-07 at 15:44 +0300, Tiago Vignatti wrote: + /* + * This is a good place to block SIGIO during SETUP state. SIGIO should be +* blocked in SETUP state otherwise (u)sleep() might get interrupted +* early. We take care not to call xf86BlockSIGIO() twice. + */ Curious indentation. Fixed locally. Thanks, Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 09/10] xfree86: remove xf86EnableAccess
On Thu, May 06, 2010 at 09:58:25AM +0200, ext Mark Kettenis wrote: From: Keith Packard kei...@keithp.com Date: Wed, 05 May 2010 20:21:29 -0700 On Wed, 5 May 2010 23:57:00 +0200 (CEST), Mark Kettenis mark.kette...@xs4all.nl wrote: You can probabaly use a version smaller than 9 as there have been a few ABI bumps since xf86EnableAccess() became a no-op. But you'll have to bump the ABI anyway when you remove the interface from the server. I think we only need to bump the ABI once per server release, and it's already been bumped for 1.9. Ok, I wasn't sure what the policiy there was. I don't really see a downside in bumping the ABI multiple times in a release cycle, but let's not turn this into an ABI versioning discussion. In that case, Tiago should definitely use: #if GET_ABI_MAJOR(ABI_VIDEODRV_VERSION) 8 in his diff. Okay. I've done this and applied for chips, tseng and cyrix. Thanks, Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PULL] MAXSCREENS removal preparation
On Mon, Apr 26, 2010 at 05:48:54PM +0200, ext Keith Packard wrote: One thing I'm working on in parallel is to fix the devPrivates implementation in the server so that adding a bunch of screen privates won't impact memory usage. Right now, every screen private we add increases the size of every pixmap, window, gc, colormap and cursor by 8 bytes (on a 32-bit system). I've got a patch for that almost ready, which is independent of your changes but which should be included in the 1.9 release with your changes. I think you'll like them in any case; we'll shrink all of these data structures by a significant amount. sweet. One thing I find pretty bad with devPrivates logic is the readability of the code. To debug it's even worst because we inherited from somewhere this crazy idea to use macros in mostly dixLookupPrivate calls. I don't think there're some apparent issues that don't let us to organize better this all over the server. For instance, just use inline functions instead macros would help a lot when debugging. Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 3/2] Track screens' installed colormaps as screen privates.
Hey Jamey, On Fri, Apr 23, 2010 at 07:02:54AM +0200, ext Jamey Sharp wrote: On Thu, Apr 22, 2010 at 10:42 AM, Vignatti Tiago (Nokia-D/Helsinki) If we get cooked this and target to apply this before 1.9 merge window be closed, so why bother? Even though, I removed this commentary from this patch which seems unrelated. I'm sorry, I didn't understand that. Do you mean it's OK to make ABI changes for 1.9 so long as we get a bunch of them done at once and remember to bump the ABI version? Yeah, that what I meant... (see bellow) Excellent. The bulk of the patch makes perfect sense to me. There are a couple things I'd like to understand still though: Since this adds and removes globals marked _X_EXPORT, it is an ABI change, right? I'm still trying to work out what the rules are. probably yeah. Please disregard any comment regarding ABI/API being break here. I didn't know what I was talking previously, but I got already a feedback from some guys in IRC. Does micmapScrPrivateKeyIndex need to be exported? AFAICT nothing should ever refer to it except the initializer for micmapScrPrivateKey, so it can be declared static, right? you are right. I just amend it. Thanks, Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 3/2] Track screens' installed colormaps as screen privates.
On Thu, Apr 22, 2010 at 07:07:58PM +0200, Vignatti Tiago (Nokia-D/Helsinki)
wrote:
On Thu, Apr 22, 2010 at 07:34:04AM +0200, ext Jamey Sharp wrote:
mi/micmap.c has the same pattern, except there the miInstalledMaps array
is used in the xfree86 DDX, and I guess it's part of the server's ABI.
So for now I left it alone.
If we get cooked this and target to apply this before 1.9 merge window be
closed, so why bother? Even though, I removed this commentary from this patch
which seems unrelated.
Well I did the same for micmap as well - look the attached.
PS: there's a bug in your xv patch that I'm trying to track down now.
Tiago
From 4b233315a73a1e2bc35b9d1eceb5f164ae119a73 Mon Sep 17 00:00:00 2001
From: Tiago Vignatti tiago.vigna...@nokia.com
Date: Thu, 22 Apr 2010 20:16:58 +0300
Subject: [PATCH] mi: track screens' installed colormaps as screen privates
Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com
---
hw/xfree86/common/xf86DGA.c |2 +-
hw/xfree86/common/xf86cmap.c | 49 -
hw/xfree86/vgahw/vgaCmap.c | 14 ++--
mi/micmap.c | 15 ++--
mi/micmap.h |9 ++-
5 files changed, 47 insertions(+), 42 deletions(-)
diff --git a/hw/xfree86/common/xf86DGA.c b/hw/xfree86/common/xf86DGA.c
index 804fd37..1a10327 100644
--- a/hw/xfree86/common/xf86DGA.c
+++ b/hw/xfree86/common/xf86DGA.c
@@ -731,7 +731,7 @@ DGAInstallCmap(ColormapPtr cmap)
/* We rely on the extension to check that DGA is active */
if(!pScreenPriv-dgaColormap)
- pScreenPriv-savedColormap = miInstalledMaps[pScreen-myNum];
+ pScreenPriv-savedColormap = GetInstalledmiColormap(pScreen);
pScreenPriv-dgaColormap = cmap;
diff --git a/hw/xfree86/common/xf86cmap.c b/hw/xfree86/common/xf86cmap.c
index f60d96e..e266ffb 100644
--- a/hw/xfree86/common/xf86cmap.c
+++ b/hw/xfree86/common/xf86cmap.c
@@ -63,10 +63,10 @@
#define SCREEN_EPILOGUE(pScreen, field, wrapper)\
((pScreen)-field = wrapper)
-#define LOAD_PALETTE(pmap, index) \
-((pmap == miInstalledMaps[index]) \
+#define LOAD_PALETTE(pmap) \
+((pmap == GetInstalledmiColormap(pmap-pScreen)) \
((pScreenPriv-flags CMAP_LOAD_EVEN_IF_OFFSCREEN) || \
- xf86Screens[index]-vtSema || pScreenPriv-isDGAmode))
+ xf86Screens[pmap-pScreen-myNum]-vtSema || pScreenPriv-isDGAmode))
typedef struct _CMapLink {
@@ -221,7 +221,7 @@ Bool xf86HandleColormaps(
}
/* Force the initial map to be loaded */
-miInstalledMaps[pScreen-myNum] = NULL;
+SetInstalledmiColormap(pScreen, NULL);
CMapInstallColormap(pDefMap);
return TRUE;
}
@@ -425,11 +425,10 @@ static void
CMapInstallColormap(ColormapPtr pmap)
{
ScreenPtr pScreen = pmap-pScreen;
-int index = pScreen-myNum;
CMapScreenPtr pScreenPriv = (CMapScreenPtr)dixLookupPrivate(
pScreen-devPrivates, CMapScreenKey);
-if (pmap == miInstalledMaps[index])
+if (pmap == GetInstalledmiColormap(pmap-pScreen))
return;
pScreen-InstallColormap = pScreenPriv-InstallColormap;
@@ -438,15 +437,15 @@ CMapInstallColormap(ColormapPtr pmap)
/* Important. We let the lower layers, namely DGA,
overwrite the choice of Colormap to install */
-if (miInstalledMaps[index])
- pmap = miInstalledMaps[index];
+if (GetInstalledmiColormap(pmap-pScreen))
+ pmap = GetInstalledmiColormap(pmap-pScreen);
if (!(pScreenPriv-flags CMAP_PALETTED_TRUECOLOR)
(pmap-pVisual-class == TrueColor)
CMapColormapUseMax(pmap-pVisual, pScreenPriv))
return;
-if(LOAD_PALETTE(pmap, index))
+if(LOAD_PALETTE(pmap))
CMapReinstallMap(pmap);
}
@@ -461,8 +460,8 @@ CMapEnterVT(int index, int flags)
pScreen-devPrivates, CMapScreenKey);
if((*pScreenPriv-EnterVT)(index, flags)) {
- if(miInstalledMaps[index])
- CMapReinstallMap(miInstalledMaps[index]);
+ if(GetInstalledmiColormap(pScreen))
+ CMapReinstallMap(GetInstalledmiColormap(pScreen));
return TRUE;
}
return FALSE;
@@ -477,8 +476,8 @@ CMapSwitchMode(int index, DisplayModePtr mode, int flags)
pScreen-devPrivates, CMapScreenKey);
if((*pScreenPriv-SwitchMode)(index, mode, flags)) {
- if(miInstalledMaps[index])
- CMapReinstallMap(miInstalledMaps[index]);
+ if(GetInstalledmiColormap(pScreen))
+ CMapReinstallMap(GetInstalledmiColormap(pScreen));
return TRUE;
}
return FALSE;
@@ -497,9 +496,9 @@ CMapSetDGAMode(int index, int num, DGADevicePtr dev)
pScreenPriv-isDGAmode = DGAActive(index);
-if(!pScreenPriv-isDGAmode miInstalledMaps[index]
+if(!pScreenPriv-isDGAmode GetInstalledmiColormap(pScreen)
xf86Screens[pScreen-myNum]-vtSema)
- CMapReinstallMap(miInstalledMaps[index]);
+ CMapReinstallMap(GetInstalledmiColormap(pScreen));
return ret;
}
@@ -649,7 +648,7 @@ CMapRefreshColors(ColormapPtr pmap, int defs, int* indices)
}
-if(LOAD_PALETTE(pmap, pmap-pScreen
Re: [PATCH v3 3/4] xfree86/dri2: wrap drm bits with macros and change drm_magic type
On Wed, Apr 21, 2010 at 03:00:52PM +0200, ext Mark Kettenis wrote: So instead you decide to increase the maintenance burden on everybody else by making the code harder to read and requiring them to remember to wrap new code that depends on libdrm in more #ifdefs. As I said, it's a trade-off. Customize an Xorg for a given system is only being discussed recently. I'd like to see an implementation very small and with few code, so I can maintain and enhance easily. Besides, Xorg is open-source and we should try discuss ways to embrace everyone's willings. Anyway, I'm pretty sure you understand my point of view. I'm totally opened to hear best approaches how to circumvent this. Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] xfree86: fix not reached code in tty code
On Wed, Apr 21, 2010 at 12:19:07AM +0200, ext Peter Hutterer wrote: stack the reviewed patches into a for-keith branch, push them to your $HOME on people.freedesktop.org and then send Keith a pull request. what I usually do is 'git request-pull master git://people.freedesktop.org/~whot/xserver.git for-keith pull-req' and then 'mutt -H pull-req' note that this requires master to be something close-ish to upstream, if you've diverged in other ways just insert the last sha1 instead of master. in my case, master is usually origin/master from where I started development, so origin/master may have moved on since. given that you already have the patches in a tree, it's faster to do this than to wait for Keith to apply and push them, he has pull requests on precedence AFAICT. very useful information, Peter. Thanks. I was thinking maybe to drop this in our wiki. Do you think there's a better place than here: http://wiki.x.org/wiki/Development/Documentation/SubmittingPatches Cheers, Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [RFC] Make MAXSCREENS run-time configurable
On Thu, Apr 01, 2010 at 09:47:35PM +0200, ext Kevin E Martin wrote: On Thu, Apr 01, 2010 at 07:19:11PM +0300, Tiago Vignatti wrote: Allows MAXSCREENS to be determined at run time instead of compile time, adding a new -maxscreens command line flag. *** Attention ABI broken everywhere! *** There's also a tiny work needed on driver side, to change some DGA structure. And if you don't, some really weird pointers will get lost everywhere. The original authors of this patch are Kevin Martin and Rik Faith, which created it against XFree86, back in 2005: https://bugs.freedesktop.org/show_bug.cgi?id=3876 https://bugs.freedesktop.org/attachment.cgi?id=5708 Small correction, the original authors were Rik Faith and James Antill. Tiago, thanks for bringing the patch up-to-date. All right, Kevin. I just amended it. Thanks, Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xextproto] Death to Multibuffer extension
On Tue, Mar 30, 2010 at 06:58:28PM +0200, ext Alan Coopersmith wrote: Isn't removing the headers from xextproto going to break the build of libXext older Xservers? (No, you can't delete functions from libXext - we don't break client library ABI like that. This is why we also no longer add new extensions to libXext, so we're not left with unused appendixes forever.) right. So lets just ignore this patch (but not the xserver's). Thanks, Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] configure: set X render animated cursors optional
Hi, On Tue, Sep 01, 2009 at 05:50:36AM +0200, ext Peter Hutterer wrote: IMO, this is the wrong approach and I was a bit surprised to see this after our latest IRC discussion. There are two parts to animated cursors that affect you. 1. The rendering of a cursor takes time and effort. 2. Animated cursors schedule wakeups for the server every so-often. Even if you disable 1, you still get the wakeups and the many calls to the block handler. Once you stop those you should be done. Now, last time you needed to disable the block handler when the cursor is invisible. we found a way how to do that. has the problem scope changed since? It turns out that pScreen == animCurState[dev-id].pScreen (line 175) already does the trick that we need to skip the loop and not schedule a wakeup. My bad not seen this before :( Makes sense for you? My only doubt is regarding the return type on ProcRenderCreateAnimCursor when the server does not support animated cursor. I put as BadImplementation. Doing so means that the server intentionally has an incomplete/wrong implementation of render. Not sure if that's a good idea. This seems not to be a big deal. We already have something like this on the server. See for instance ProcXFixesCreateRegionFromPicture(). This seems to be niche feature and I wonder if it wouldn't be better to adjust the environment accordingly (i.e. no animated cursor themes). But my concern is another one now - we can save 16KB just removing this animated cursor code. Yeah, I've been non-verbose about this and you thought we're treating another issue. Sorry. Thank you, Tiago ___ xorg-devel mailing list xorg-devel@lists.x.org http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 1/2] mi: fix cursor warping screens
On Fri, Aug 07, 2009 at 01:58:41AM +0200, Peter Hutterer wrote: How about the patch attached. It ends up being the same logic but the code looks nicer (even though the patch doesn't). Instead of the fallthrough into the default statement, the default block is moved out of the switch. coolio! Worked fine here. I'll apply. This leaves one remaining issue though. Previously, the code would not process motion events if a screen switch takes place. I've already explained why we can't drop button and key events but - should we still drop motion events? i.e. do we need a if (type == motion) return; after the NewCurrentScreen? For me it just make sense to let the server continue the normal processing and deliver the motion event to some eventual client. So I would not put this return there. Cheers, Tiago ___ xorg-devel mailing list xorg-devel@lists.x.org http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 1/2] mi: fix cursor warping screens
Ooops, I messed the script. Well, the email is mine :)
On Wed, Aug 05, 2009 at 08:18:55PM +0200, y...@mail.nokia.com wrote:
From: Tiago Vignatti tiago.vigna...@nokia.com
The server was processing ET_RawMotion type when the cursor was wrapping to
another screen and getting wrong valuator values. This fix such issue
considering only ET_Motion, ET_KeyPress, ET_KeyRelease, ET_ButtonPress and
ET_ButtonRelease types when the cursor detects a new screen.
Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com
---
mi/mieq.c | 24
1 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/mi/mieq.c b/mi/mieq.c
index 6ec2dba..b52ed84 100644
--- a/mi/mieq.c
+++ b/mi/mieq.c
@@ -367,14 +367,22 @@ mieqProcessDeviceEvent(DeviceIntPtr dev,
/* Custom event handler */
handler = miEventQueue.handlers[event-any.type];
-if (dev screen screen != DequeueScreen(dev) !handler) {
-/* Assumption - screen switching can only occur on motion events. */
-DequeueScreen(dev) = screen;
-x = event-device.root_x;
-y = event-device.root_y;
-NewCurrentScreen (dev, DequeueScreen(dev), x, y);
-}
-else {
+switch (event-any.type) {
+/* Catch events that include valuator information and check if they
+ * are changing the screen */
+case ET_Motion:
+case ET_KeyPress:
+case ET_KeyRelease:
+case ET_ButtonPress:
+case ET_ButtonRelease:
+if (dev screen screen != DequeueScreen(dev) !handler) {
+DequeueScreen(dev) = screen;
+x = event-device.root_x;
+y = event-device.root_y;
+NewCurrentScreen (dev, DequeueScreen(dev), x, y);
+break;
+}
+default:
master = CopyGetMasterEvent(dev, event, mevent);
if (master)
--
1.5.6.3
Tiago
___
xorg-devel mailing list
xorg-devel@lists.x.org
http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] pciaccess: remove xorg macros dependency
On Thu, Jul 16, 2009 at 02:03:11PM +0200, ext Julien Cristau wrote: On Thu, Jul 16, 2009 at 14:53:47 +0300, Vignatti Tiago (Nokia-D/Helsinki) wrote: On Thu, Jul 16, 2009 at 01:30:22PM +0200, ext Julien Cristau wrote: why? Why pci access library must rely on X server? Who said anything about the X server? This is a just bunch of m4 macros whatever, the library does not need it. Ack? Tiago ___ xorg-devel mailing list xorg-devel@lists.x.org http://lists.x.org/mailman/listinfo/xorg-devel
Re: [RFC][PATCH 3/6] xfree86: delete devices probe code (-probe and -probeonly options)
On Tue, Jul 14, 2009 at 07:54:19PM +0200, ext Mark Kettenis wrote: From: Tiago Vignatti tiago.vigna...@nokia.com Date: Tue, 14 Jul 2009 20:06:40 +0300 In a near future KMS drivers will do all device's configuration inside the kernel. For those that don't want/need/can't do the mode setting inside the kernel, then we need to design a tool _external_ from the server to do such work. But here, inside a windowing system, it's not the place to probe for devices. Goodbye -probe and -probeonly. Isn't this a little bit premature? It's a bit like removing support for multi-card because it will be done by KMS before any KMS drivers actually exist ;). Err. FYI multi-card support is broken since 7.4 release. Tiago ___ xorg-devel mailing list xorg-devel@lists.x.org http://lists.x.org/mailman/listinfo/xorg-devel
Re: [RFC][PATCH 0/6] towards the dietary: removing libpciaccess dependency from Xorg
Hi, On Tue, Jul 14, 2009 at 08:03:35PM +0200, ext Matthias Hopf wrote: On Jul 14, 09 20:06:37 +0300, Tiago Vignatti wrote: (take a breath; this set comes with radical changes) I'm not completely familiar with the code changed, and looked at it only briefly - and it looked like this removes static configuration completely. Nop. It removes only the code that generates a config file. It's the ugliest thing ever: it starts the X server, probe some devices and then exists. Sigh. So even with the patch applied, the static way, using xorg.conf, continues there working. (yeah, the description on the patch is not clear enough. Sorry.) If that is the case, that's a no-go ATM, because we don't have any other way e.g. to configure the framebuffer size for now (for drivers that are not capable of resizing on the fly, i.e. all but intel (maybe radeon?)). And we probably need static config for nontrivial multiscreen setups, until all login managers know how to do that dynamically. OTOH if this doesn't touch static config, please ignore :-) ignored :) idiot and not the role of the X server, so lets do it right (BTW, I'm curious to know which applications are using this probe methods and eventually help fix those applications properly). I'm unsure ATM, but I think that sax2 still uses 'X -probe'. OTOH we will deprecate sax2 for openSUSE 11.2 anyway... In any case, this is something not to be applied unless we want to bump at least the minor version number. nods. As always, these are my 2 cents. Mine! Okay, thanks Matthias. Tiago ___ xorg-devel mailing list xorg-devel@lists.x.org http://lists.x.org/mailman/listinfo/xorg-devel
