Bug 130888

Summary: X hangs at startup.
Product: [Fedora] Fedora Reporter: David Woodhouse <dwmw2>
Component: xorg-x11Assignee: X/OpenGL Maintenance List <xgl-maint>
Status: CLOSED RAWHIDE QA Contact: David Lawrence <dkl>
Severity: medium Docs Contact:
Priority: medium    
Version: 3   
Target Milestone: ---   
Target Release: ---   
Hardware: powerpc   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2005-02-17 15:36:55 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 121179, 123268    
Attachments:
Description Flags
Failing log file with -logverbose 255
none
Verbose output from working 6.7.0-7.2
none
Patch to fix segv
none
Workaround for initializing panel timing params none

Description David Woodhouse 2004-08-25 16:07:01 UTC
Description of problem:

X hangs on Powerbook with Radeon 9600 M10, at startup after printing
'(WW) RADEON(0): No valid timing info from BIOS.'. That write() call
is also the last thing that strace sees.

Version-Release number of selected component (if applicable):
xorg-x11-6.7.99.902-6

How reproducible:
100%

Comment 1 David Woodhouse 2004-08-25 16:07:58 UTC
Created attachment 103076 [details]
Failing log file with -logverbose 255

Comment 2 Mike A. Harris 2004-08-25 16:31:44 UTC
Please report in Xorg bugzilla, so hopefully the issue will
be addressed for 6.8.0, which is very near.  Paste the upstream
URL here, and the X Devel team will track the issue in Xorg
bugzilla.

http://bugs.freedesktop.org  "xorg" component.

Thanks in advance.

Comment 3 David Woodhouse 2004-08-25 16:41:01 UTC
The requested URL /bugzilla/index.cgi was not found on this server.

Comment 4 David Woodhouse 2004-08-25 16:42:16 UTC
Created attachment 103081 [details]
Verbose output from working 6.7.0-7.2

Comment 5 Mike A. Harris 2004-08-25 17:49:49 UTC
Freedesktop bugzilla seemed to be down briefly, but it is
back again now.

HTH

Comment 6 David Woodhouse 2004-08-25 17:51:00 UTC
Now I'm just waiting for it to actually send me the password it
promised to send... :)

Comment 7 David Woodhouse 2004-08-25 19:09:20 UTC
I rebuilt X from CVS with the dlopen loader instead of the old hacks.
Why aren't we shipping it that way?

From CVS, the behaviour is different:

(WW) RADEON(0): No valid timing info from BIOS.
(EE) RADEON(0): MergedFB does not work with Option UseFBDev, MergedFB
mode is disabled
(==) RADEON(0): Using gamma correction (1.0, 1.0, 1.0)
(II) RADEON(0): Validating modes on Primary head ---------
(II) RADEON(0): Panel size found from DDC: 1440x900
(II) RADEON(0): Valid Mode from Detailed timing table: 1440x900
(II) RADEON(0): Total of 1 mode(s) found.
(II) RADEON(0): Total number of valid DDC mode(s) found: 1
 
Program received signal SIGSEGV, Segmentation fault.
0x0f26eb60 in RADEONValidateFPModes (pScrn=0x10324a08,
ppModeName=0x102fa660)
    at radeon_driver.c:3160
3160                    last->next = new;
(gdb) p last
$1 = 0x0
(gdb) bt
#0  0x0f26eb60 in RADEONValidateFPModes (pScrn=0x10324a08,
    ppModeName=0x102fa660) at radeon_driver.c:3160
#1  0x0f2703d8 in RADEONPreInitModes (pScrn=0x10324a08, pInt10=0x0)
    at radeon_driver.c:3673
#2  0x0f271a78 in RADEONPreInit (pScrn=0x10324a08, flags=0)
    at radeon_driver.c:4227
#3  0x10028568 in InitOutput (pScreenInfo=0x102ef464, argc=9,
argv=0x7ffff8b4)
    at xf86Init.c:587
#4  0x100d7980 in main (argc=9, argv=0x7ffff8b4, envp=0x7ffff8dc) at
main.c:365
(gdb)


Comment 8 David Woodhouse 2004-08-25 20:08:07 UTC
The original bug was
http://freedesktop.org/bugzilla/show_bug.cgi?id=1007 and the SEGV
appears to be a different linked list handling bug.

Comment 9 David Woodhouse 2004-08-26 01:12:47 UTC
I've worked around the problem by adding a modeline. The problem is
that we're not correctly setting the DotClock from the EDID info. Then
the list of valid modes is empty and the list handling is broken too
-- but even when you apply the obvious fix to stop the SEGV, there's
still no valid modes and it doesn't actually _work_.


Comment 10 David Woodhouse 2004-08-27 12:43:08 UTC
This patch fixes the SEGV. It'll probably get mangled, but it was
probably wrong anyway -- it shows precisely where the problem is, at
least. The lack of DotClock seems to be because we never suck in the
EDID info in RADEONGetLVDSInfo(), because this is never true:

            if ((tmp_mode->HDisplay == info->PanelXRes) &&
                (tmp_mode->VDisplay == info->PanelYRes)) {

That check isn't true because we already got the size from the
registers in RADEONGetPanelInfoFromReg(), I think.

--- radeon_driver.c     25 Aug 2004 00:30:41 -0000      1.19
+++ radeon_driver.c     27 Aug 2004 12:42:01 -0000
@@ -3157,7 +3157,8 @@
                new->next       = NULL;
                new->prev       = last;
  
-               last->next = new;
+               if (last)
+                       last->next = new;
                last = new;
            }
        }
@@ -3166,8 +3167,9 @@
     /* Close the doubly-linked mode list, if we found any usable modes */
     if (last) {
        last->next   = first;
-       first->prev  = last;
-       pScrn->modes = first;
+       if (first)
+               first->prev  = last;
+       pScrn->modes = first?:last;
        RADEONSetPitch(pScrn);
     }
  


Comment 11 David Woodhouse 2004-08-27 12:45:08 UTC
Bollocks. It's not RADEONGetLVDSInfo() I was looking at, it was
RADEONUpdatePanelSize(), and the check which is never true is:
            if (info->PanelXRes < d_timings->h_active &&
                info->PanelYRes < d_timings->v_active) {


Comment 12 David Woodhouse 2004-09-08 11:59:12 UTC
Still present in CVS build of Xorg 6.8.0

Comment 13 David Woodhouse 2004-09-08 12:34:39 UTC
freedesktop.org outgoing mail from bugzilla is still broken so it
still hasn't managed to send me a password, but I got someone else to
report it there -- http://freedesktop.org/bugzilla/show_bug.cgi?id=1306


Comment 14 Mike A. Harris 2004-10-13 18:42:04 UTC
The monitor probing code and mode validation code seems to currently
be in a very sad and sorry state.  There are many cards which
just dont work at all, and other obscure problems being found
currently.  ;o/

We should have other radeon driver developers review the patch
as well in upstream bugzilla.

Comment 15 Kevin E. Martin 2004-10-15 19:42:07 UTC
Created attachment 105297 [details]
Patch to fix segv

The attached patch should work around the SEGV; however, there is a larger
problem with initializing the panel params, which comes from Hui's updated
probe code.  See the next comment.

Comment 16 Kevin E. Martin 2004-10-15 19:52:12 UTC
Created attachment 105298 [details]
Workaround for initializing panel timing params

This patch should work around the panel timing params not being initialized. 
Here's the problem:

Previously, the PanelXRes and PanelYRes were either read from the BIOS or were
left as 0 (if no BIOS was detected).  Then, in RADEONUpdatePanelSize(), the max
panel size was found and the timing parameters were initialized, which worked
fine for this ppc system.

Now, the PanelXRes and PanelYRes are either read from the BIOS or are read from
the registers.	Note that the other timing parameters (in particular the
DotClock) are not initialized when reading from the registers.	Then, when
RADEONUpdatePanelSize() is called, the max panel size is already set, so none
of the other timing parameters are initialized here either (or anywhere else
for that matter), which appears to be why the new code fails for this ppc
system.

The patch changes the test from < to <= in RADEONUpdatePanelSize() and then
tests to make sure that only the first set of timings for the panel size read
from the registers will be used -- this mimics the way the previous code
worked.  The only problem with this code occurs when the registers hold invalid
panel size params, which do not match any of the monitor's DDC info.  This
should never happen; however, if it does, then the only solution in this case
is to explicitly set the panel size in the config file.

Comment 17 Kevin E. Martin 2004-10-15 19:55:28 UTC
Could you please test to see if the two patches together work around
the problem for you?


Comment 18 David Woodhouse 2004-10-15 22:04:38 UTC
As expected, the first patch along makes it not segfault, although it
obviously still doesn't actually _work_, but exits relatively
gracefully. The second patch does make it work. Thanks.

Comment 19 Mike A. Harris 2004-10-18 22:44:31 UTC
I've added both of Kevin's above patches to xorg-x11-6.8.1-8 and
it's building in beehive.  It should be ready for testing within
the next hour.  Please test this as much as possible once it's
built, and provide a status update.  If the new package fixes the
problems, which I assume it will based on comment #18, please close
as "RAWHIDE", otherwise set status to "ASSIGNED"

Setting status to "MODIFIED" pending testing results.

Thanks in advance.