Bug 1306 - [ATI/radeon] segfault
Summary: [ATI/radeon] segfault
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Driver/Radeon (show other bugs)
Version: git
Hardware: x86 (IA32) Linux (All)
: high normal
Assignee: Xorg Project Team
QA Contact:
URL:
Whiteboard:
Keywords: regression
Depends on:
Blocks: 1690
  Show dependency treegraph
 
Reported: 2004-09-08 05:35 UTC by Matthew Allum
Modified: 2005-07-27 02:01 UTC (History)
9 users (show)

See Also:
i915 platform:
i915 features:


Attachments
patch for panel info problem (1.26 KB, patch)
2004-10-19 21:07 UTC, HUI YU
no flags Details | Splinter Review
[FIXED_X11R68x] SEGV Fix (701 bytes, patch)
2004-11-11 10:32 UTC, Kristian Høgsberg
roland.mainz: 6.8-branch+
Details | Splinter Review
[FIXED_X11R68x] Panel Timing FIx (2.23 KB, patch)
2004-11-11 10:34 UTC, Kristian Høgsberg
roland.mainz: 6.8-branch+
Details | Splinter Review

Description Matthew Allum 2004-09-08 05:35:44 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.

See https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=130888

Reporting on behalf of David Woodhouse.
Comment 1 David Woodhouse 2004-10-18 06:27:18 UTC
Note the two patches attached to the Red Hat bug which fix both the SEGV and the
underlying problem.

https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=105297&action=view
https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=105298&action=view
Comment 2 Mike A. Harris 2004-10-18 14:42:40 UTC
For anyone experiencing this bug, I've added these 2 patches into
our Red Hat build "xorg-x11-6.8.1-8" which should be in Fedora devel
soon.
Comment 3 Kevin E. Martin 2004-10-18 20:53:05 UTC
Copied from my comments in the Red Hat bug for those interested in hearing what
the patches in comment #1 does.  Note this is a workaround, and when a proper
fix is available, it should be applied.

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 4 HUI YU 2004-10-19 21:07:47 UTC
Created attachment 1138 [details] [review]
patch for panel info problem 

The patches post before should work fine with this problem, I don't see any
side effect. A more complete fix for this problem would be bypassing
GetPanelInfoFromReg function when DDC is available (see attached patch).
GetPanelInfoFromReg is really meant for the case when both DDC and BIOS info
are not available. Note that this patch hasn't been tested at all (none of my
LVDS panels has DDC).
Comment 5 Kristian Høgsberg 2004-11-11 10:32:21 UTC
Created attachment 1291 [details] [review]
[FIXED_X11R68x] SEGV Fix

Patch for radeon SEGV problem, nominating for 6.8.2.
Comment 6 Kristian Høgsberg 2004-11-11 10:34:01 UTC
Created attachment 1292 [details] [review]
[FIXED_X11R68x] Panel Timing FIx

Patch for radeon panel timing problem, nominating for 6.8.2.
Comment 7 Roland Mainz 2004-11-12 09:03:51 UTC
Comment on attachment 1291 [details] [review]
[FIXED_X11R68x] SEGV Fix

Approved for checkin into X11R6.8.x in the 2004-11-12 release-wranglers call.
Comment 8 Roland Mainz 2004-11-12 09:03:55 UTC
Comment on attachment 1292 [details] [review]
[FIXED_X11R68x] Panel Timing FIx

Approved for checkin into X11R6.8.x in the 2004-11-12 release-wranglers call.
Comment 9 Alex Deucher 2004-11-12 09:13:54 UTC
when this is applied to 6.8.2, can someone please update HEAD as well?  If not,
I'll do it if everyone is ok with that.
Comment 10 Roland Mainz 2004-12-15 17:54:40 UTC
Comment on attachment 1291 [details] [review]
[FIXED_X11R68x] SEGV Fix

Patch checked-in into X11R6.8.x stable branch:

/cvs/xorg/xc/ChangeLog,v  <--  ChangeLog
new revision: 1.365.2.95; previous revision: 1.365.2.94
cvs commit: Using deprecated info format strings.  Convert your scripts to use
the new argument format and remove '1's from your info file format strings.
/cvs/xorg/xc/programs/Xserver/hw/xfree86/drivers/ati/radeon_driver.c,v	<-- 
radeon_driver.c
new revision: 1.19.2.2; previous revision: 1.19.2.1
cvs commit: Using deprecated info format strings.  Convert your scripts to use
the new argument format and remove '1's from your info file format strings.
Mailing the commit message to xorg-commit@lists.freedesktop.org...
Comment 11 Roland Mainz 2004-12-15 17:59:26 UTC
Comment on attachment 1292 [details] [review]
[FIXED_X11R68x] Panel Timing FIx

Patch checked-in into X11R6.8.x stable branch:

/cvs/xorg/xc/ChangeLog,v  <--  ChangeLog
new revision: 1.365.2.96; previous revision: 1.365.2.95
cvs commit: Using deprecated info format strings.  Convert your scripts to use
the new argument format and remove '1's from your info file format strings.
/cvs/xorg/xc/programs/Xserver/hw/xfree86/drivers/ati/radeon_driver.c,v	<-- 
radeon_driver.c
new revision: 1.19.2.3; previous revision: 1.19.2.2
cvs commit: Using deprecated info format strings.  Convert your scripts to use
the new argument format and remove '1's from your info file format strings.
Mailing the commit message to xorg-commit@lists.freedesktop.org...
Comment 12 Adam Jackson 2005-04-03 14:46:47 UTC
mass update: this bug was applied to the stable 6.8 branch but NOT to HEAD!
Comment 13 Alan Coopersmith 2005-06-04 13:14:45 UTC
Patch 1291 is now applied to HEAD as well:

CVSROOT:	/cvs/xorg
Module name:	xc
Changes by:	alanc@gabe.freedesktop.org	05/06/04 13:04:36

Log message:
  2005-06-04  Alan Coopersmith  <alan.coopersmith@sun.com>
  
  	* programs/Xserver/hw/xfree86/drivers/ati/radeon_driver.c
(RADEONValidateFPModes):
  	Sync with 6.8.2 branch:
  	Bugzilla #1306 (https://bugs.freedesktop.org/show_bug.cgi?id=1306)
  	attachment #1291 [details] [review] (https://bugs.freedesktop.org/attachment.cgi?id=1291):
  	Fix SEGV in "radeon" driver.
  	Patch by Kevin E. Martin <kem@freedesktop.org>

Modified files:
      ./:
        ChangeLog 
      xc/programs/Xserver/hw/xfree86/drivers/ati/:
        radeon_driver.c 
  
  Revision      Changes    Path
  1.973         +8 -0      xc/ChangeLog
  1.53          +2 -1     
xc/programs/Xserver/hw/xfree86/drivers/ati/radeon_driver.c

The code around the panel timing fix has changed, so I'll let someone more
familiar with that code figure out what to do about it and if the first patch
(which wasn't applied to 6.8.2) should still be applied.
Comment 14 Chris Lee 2005-07-28 12:01:36 UTC
Looks like the bug's status wasn't updated when it was fixed. 


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.