[ANNOUNCE] xorg-server 1.7.99.902

Ruediger Oertel ro at suse.de
Thu Mar 25 10:56:36 PDT 2010


On Wednesday 24 March 2010 17:20:24 Dan Nicholson wrote:
> On Wed, Mar 24, 2010 at 06:05:27AM +0100, Stefan Dirsch wrote:
> > On Mon, Mar 22, 2010 at 07:59:18AM -0700, Dan Nicholson wrote:
> > > 2. Make the handling of missing sections from an existing
> > > configuration behave more like the full autoconfig. In other words, if
> > > there's a missing Screen section, generate multiple Screen/Device
> > > pairs with fallbacks. I think this is the correct fix, but I'm not
> > > that familiar with the autoconfig code so I don't think I could whip
> > > up a patch that fast.
> > 
> > I've now attached a patch to
> > 
> >   https://bugs.freedesktop.org/show_bug.cgi?id=27229
> > 
> > which does that. It works fine for us.
> 
> Looks pretty good from my understanding, but I'd like to polish it a bit more
> before applying it. I'm gonna do the review here since I'm not that familiar
> with that part of the code.
> 
> The first thing is that it would be great if this was a git formatted patch
> with a commit message. But if it's easier to keep it as a separate patch, can
> you add some information to the patch header? It would also be great if there
> could be some comments in the code. It looks like multiple Screen sections are
> being handled, but not really.
> 
> Now to the patch.

okay, I've reworked the patch again, thanks for all your comments.
I hope the patch looks better now and is easier to read with the
inlined comments.

however there is an issue with the patch: if you really have more than one
possible driver, the Xserver will segfault on exit/cleanup in randr/rroutput.c:RROutputDestroyResource
when cleaning up the resources.
I've tried several approaches to get the copied screen sections initialized better, but end
up in nothing but trouble. Any help would be appreciated to clean this last glitch.

Here it is:
--------------------------------------------------------------------------------------------------------------------------------------------------
commit 4da6cffa8b6169595ea447cc53dfab857c04db04
Author: h_root <root at hilbert.suse.de>
Date:   Thu Mar 25 18:32:04 2010 +0100

    when doing driver autoconfiguration with some parts of the config
    file present but no driver set (e.g. only input configuration)
    fix the case that we may have multiple drivers to try.
    
    create a screen section for each driver and let them be tried
    in a row

diff --git a/hw/xfree86/common/xf86AutoConfig.c b/hw/xfree86/common/xf86AutoConfig.c
index 7f4ada8..56f7deb 100644
--- a/hw/xfree86/common/xf86AutoConfig.c
+++ b/hw/xfree86/common/xf86AutoConfig.c
@@ -546,10 +546,41 @@ chooseVideoDriver(void)
     return chosen_driver;
 }
 
+
+/* copy a screen section and enter the desired driver
+ * and insert it at i in the list of screens */
+static Bool
+copyScreen(confScreenPtr oscreen, GDevPtr odev, int i, char *driver)
+{
+    GDevPtr cptr = NULL;
+
+    xf86ConfigLayout.screens[i].screen = xnfcalloc(1, sizeof(confScreenRec));
+    if(!xf86ConfigLayout.screens[i].screen)
+        return FALSE;
+    memcpy(xf86ConfigLayout.screens[i].screen, oscreen, sizeof(confScreenRec));
+
+    cptr = xcalloc(1, sizeof(GDevRec));
+    if (!cptr)
+        return FALSE;
+    memcpy(cptr, odev, sizeof(GDevRec));
+
+    cptr->identifier = Xprintf("Autoconfigured Video Device %s", driver);
+    cptr->driver = driver;
+
+    /* now associate the new driver entry with the new screen entry */
+    xf86ConfigLayout.screens[i].screen->device = cptr;
+    cptr->myScreenSection = xf86ConfigLayout.screens[i].screen;
+
+    return TRUE;
+}
+
 GDevPtr
 autoConfigDevice(GDevPtr preconf_device)
 {
     GDevPtr ptr = NULL;
+    char *matches[20]; /* If we have more than 20 drivers we're in trouble */
+    int num_matches = 0, num_screens = 0, i;
+    screenLayoutPtr slp;
 
     if (!xf86configptr) {
         return NULL;
@@ -573,14 +604,59 @@ autoConfigDevice(GDevPtr preconf_device)
         ptr->driver = NULL;
     }
     if (!ptr->driver) {
-        ptr->driver = chooseVideoDriver();
-    }
+        /* get all possible video drivers and count them */
+        listPossibleVideoDrivers(matches, 20);
+        for (; matches[num_matches]; num_matches++) {
+            xf86Msg(X_DEFAULT, "Matched %s as autoconfigured driver %d\n",
+                    matches[num_matches], num_matches);
+        }
+
+        slp = xf86ConfigLayout.screens;
+        if (slp) {
+            /* count the number of screens and make space for
+             * a new screen for each additional possible driver
+             * minus one for the already existing first one
+             * plus one for the terminating NULL */
+            for (; slp[num_screens].screen; num_screens++);
+            xf86ConfigLayout.screens = xnfcalloc(num_screens + num_matches,
+                                                sizeof(screenLayoutRec));
+            xf86ConfigLayout.screens[0] = slp[0];
+
+            /* do the first match and set that for the original first screen */
+            ptr->driver = matches[0];
+            if (!xf86ConfigLayout.screens[0].screen->device) {
+                xf86ConfigLayout.screens[0].screen->device = ptr;
+                ptr->myScreenSection = xf86ConfigLayout.screens[0].screen;
+            }
+
+            /* for each other driver found, copy the first screen, insert it
+             * into the list of screens and set the driver */
+            i = 0;
+            while (i++ < num_matches) {
+                if (!copyScreen(slp[0].screen, ptr, i, matches[i]))
+                    return NULL;
+            }
 
-    /* TODO Handle multiple screen sections */
-    if (xf86ConfigLayout.screens && !xf86ConfigLayout.screens->screen->device) {
-        xf86ConfigLayout.screens->screen->device = ptr;
-        ptr->myScreenSection = xf86ConfigLayout.screens->screen;
+            /* shift the rest of the original screen list
+             * to the end of the current screen list
+             *
+             * TODO Handle rest of multiple screen sections */
+            for (i = 1; i < num_screens; i++) {
+                xf86ConfigLayout.screens[i+num_matches] = slp[i];
+            }
+            xf86ConfigLayout.screens[num_screens+num_matches-1].screen = NULL;
+            xfree(slp);
+        } else {
+            /* layout does not have any screens, not much to do */
+            ptr->driver = matches[0];
+            for (i = 1; matches[i] ; i++) {
+                if (matches[i] != matches[0]) {
+                    xfree(matches[i]);
+                }
+            }
+        }
     }
+
     xf86Msg(X_DEFAULT, "Assigned the driver to the xf86ConfigLayout\n");
 
     return ptr;


-- 
with kind regards (mit freundlichem Grinsen),
   Ruediger Oertel (ro at novell.com,ro at suse.de,bugfinder at t-online.de)
----------------------------------------------------------------------
Linux Fatou 2.6.33-6-desktop #1 SMP PREEMPT 2010-02-25 20:06:12 +0100 x86_64
Key fingerprint = 17DC 6553 86A7 384B 53C5  CA5C 3CE4 F2E7 23F2 B417
SUSE LINUX Products GmbH,  GF: Markus Rex,   HRB 16746 (AG Nürnberg)


More information about the xorg-devel mailing list