Bug 255

Summary: [PATCH] Radeon driver - AGP/PCI autodetection is totally broken
Product: xorg Reporter: Mike A. Harris <mharris>
Component: Server/GeneralAssignee: Xorg Project Team <xorg-team>
Status: RESOLVED FIXED QA Contact:
Severity: critical    
Priority: highest CC: dberkholz, eich, kem
Version: unspecified   
Hardware: x86 (IA32)   
OS: All   
Whiteboard:
i915 platform: i915 features:
Bug Depends on:    
Bug Blocks: 351    
Attachments:
Description Flags
Patch to implement AGP/PCI detection in the Radeon driver properly by walking the PCI capabilities list in PCI config space. none

Description Mike A. Harris 2004-03-01 22:52:13 UTC
The AGP/PCI detection in the radeon driver is totally broken, both in
XFree86 4.3.0, 4.4.0, and current xorg trees, although there were various
attempts to fix it in 4.3.0 and later, none of them succeed.

I got tonnes of bug reports about this in Red Hat bugzilla, and bit my
tongue and fixed the problem for once and for all properly, however the
code is Radeon specific currently as that was easiest at the time.  My
plan is to replace this radeon specific code with generic code in the
future which all drivers can use.

My patch should be included in the upcoming release, or else many Radeon AGP
users, which make up the overwhelming majority of Radeon users, will end up
with their cards being treated like PCI cards.  In the lesser case, many
users just end up with a large performance loss.  In the worst case however,
users who had a previously working AGP card in a prior XFree86 release, may
end up upgrading and getting their card being treated as PCI all of a sudden,
and using pcigart for DRI.  On many systems pcigart does not work at all, and
so they upgrade and end up with a totally broken nonfunctional system.

I will submit a patch to fix this problem in the near future.  This is
currently a placeholder.
Comment 1 Mike A. Harris 2004-03-08 14:19:45 UTC
Created attachment 134 [details] [review]
Patch to implement AGP/PCI detection in the Radeon driver properly by walking the PCI capabilities list in PCI config space.

Patch by Mike A. Harris <mharris@redhat.com> to implement proper AGP/PCI
autodetection in Radeon driver by walking the PCI capabilities list.  This
patch is ported forward from the patch I wrote for XFree86 4.3.0, and I
consider it to be a temporary radeon driver specific fix.  In the future,
this should be reimplemented in os-support somewhere, and hooks put in all
drivers to detect AGP/PCI using this method.
Comment 2 Mike A. Harris 2004-03-09 18:22:20 UTC
I'm going to commit this to the xorg source tree unless there are any
objections.  It can always be changed after the fact if someone thinks
there is anything wrong with it.
Comment 3 Mike A. Harris 2004-03-11 08:54:13 UTC
Checked into XORG-RELEASE-1:


Log message:
  Fixed AGP/PCI card detection in Radeon driver, by walking the PCI capabilities
list in PCI config space (Bugzilla #255) (Mike A. Harris).

Modified files:
      ./:       Tag: XORG-RELEASE-1
        CHANGELOG-RELEASE-1
      xc/programs/Xserver/hw/xfree86/drivers/ati/:      Tag: XORG-RELEASE-1
        radeon_driver.c radeon_reg.h

  Revision      Changes    Path
  1.1.2.15      +2 -0      xc/Attic/CHANGELOG-RELEASE-1
  1.1.4.1.6.2   +56 -18   
xc/programs/Xserver/hw/xfree86/drivers/ati/radeon_driver.c
  1.1.4.1.6.2   +7 -0      xc/programs/Xserver/hw/xfree86/drivers/ati/radeon_reg.h
Comment 4 Mike A. Harris 2004-08-06 23:21:09 UTC
Someone reverted my patch during 6.7.0->6.8.0 development.  I haven't yet
tracked down the culprit or the reasoning, but it needs to go back in.  The
consequence of it not going back in are tonnes of Radeon user bug reports
hitting bugzilla with oddball bug reports that many people won't be able
to reproduce due to the way the problem manifests itself.
Comment 5 Kevin E. Martin 2004-08-09 21:26:54 UTC
This code is basically still in the current CVS head -- see lines 2404-2464 in
radeon_driver.c -- but it is slightly different from what you checked in.  This
change was something that Egbert made in revision 1.1.4.1.6.3.  If that change
was incorrect, please submit another patch that fixes the problem.
Comment 6 Mike A. Harris 2004-08-11 20:53:46 UTC
It appears this was an error on my part.  I've examined the current sources,
and the sources from the timeframe that I was looking at when I reopened this,
and the code is indeed there.  I believe I may have been in the wrong source
tree of an older set of X sources.

Closing as 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.