Comment # 33 on bug 835975 from Egbert Eich

(In reply to Gary Greene from comment #31)
> After doing an hour or so of tracing the code, it appears that the issue is
> in the for loop at line 125 in xf86Bus.c. In the loop, it calls
> xf86CallDriverProbe with args xf86DriverList[i] and FALSE.
> 
> This is _supposed_ to append a new entry to the xf86Screens[] array
> according to the comments in the code. However, from reading through the
> code, this comment seems to be stale, since it returns an int, not an array,
> and the call itself does not push anything onto that array either.
> 
No, it's more complicated than this:
xf86CallDriverProbe() calls xf86platformProbeDev() and if this doesn't succeed,
xf86PciProbeDev().
* xf86platformProbeDev() calls probeSingleDevice() which in turn calls 
  doPlatformProbe() which itself calls into the driver ie. 
  drvp->platformProbe(). The platformProbe() function provided by the driver
  then calls the xf86AllocateScreen() helper which adds the screen to the 
  xf86Screens[] array and increments the xf86NumScreens counter.
* xf86PciProbeDev() likewise calls into the driver (directly): it calls 
  drvp->PciProbe() which does similar things to drvp->platformProbe() ie. it
  calls xf86AllocateScreen().

> On top of that, the logic for this section of code seems a bit screwy, since
> normally, we'd want that returned INTO something, but it just sits there in
> void return context.
> 
> Walking through xf86CallDriverProbe, the call returns foundScreen as TRUE,
> but if you look, there is no assignment to store how many screens were
> detected. This causes the if block right after that to fail.

When you step thru xf86CallDriverProbe() you will see that it calls
xf86platformProbeDev() and if this returns FALSE, it will call
xf86PciProbeDev(). One of those functions should have returned TRUE, otherwise
you would not see xf86CallDriverProbe() return TRUE.
Once you see one of these functions (xf86platformProbeDev() or
xf86PciProbeDev()) return TRUE you should check if xf86NumScreens was
incremented. If it wasn't, it is in contradiction to the TRUE result and it
should be checked why.
If you have the debuginfo and debugsource packages for xf86-video-ati
installed, you can breakpoint on radeon_platform_probe() and radeon_pci_probe()
to avoid the majority of the comparison loops and see if you will end up in one
of these.

> 
> Honestly, I'm pretty unsure how this works even for other cards in the
> series, since it seems that this would occur more often.

If it returns TRUE but xf86NumScreens has not incremented, something is
definitely wrong.

> I _think_ the right change for this would be to change the code like so:
> 
> @@ -125,3 +125,3 @@
>      for (i = 0; i < xf86NumDrivers; i++) {
> -         xf86CallDriverProbe(xf86DriverList[i], FALSE);
> +         xf86NumScreens = xf86CallDriverProbe(xf86DriverList[i], FALSE);
>      }

No, xf86CallDriverProbe() returns a Bool.

(In reply to Gary Greene from comment #32)
> (In reply to Gary Greene from comment #31)
> > @@ -125,3 +125,3 @@
> >      for (i = 0; i < xf86NumDrivers; i++) {
> > -         xf86CallDriverProbe(xf86DriverList[i], FALSE);
> > +         xf86NumScreens = xf86CallDriverProbe(xf86DriverList[i], FALSE);
> >      }
> 
> Hmmm, after thinking about it, the code needs to do something like this
> instead:
> 
> @@ -125,3 +125,4 @@
>      for (i = 0; i < xf86NumDrivers; i++) {
> -         xf86CallDriverProbe(xf86DriverList[i], FALSE);
> +         xf8NumScreens = 0;

No, that's even worse. This is a global variable calling all screens (even if
you have different drivers).