2002-01-23 20:45:47

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH] 2.4.18-pre6 AGP enable fixes

In 2.4.17-pre5, agp_generic_agp_enable() and serverworks_agp_enable()
were changed to look for AGP devices as coprocessors on non-VGA
devices as well as VGA devices.

But those functions contain TWO loops that scan for AGP devices (one
to collect the settings they support, and a second to update the
command registers), and 2.4.17-pre5 only changed the first loop in
each case. Shouldn't both loops be changed, as in the "agp_dev" patch
attached?

While looking at this code, I noticed a possible further improvement.
Currently it looks like this:

pci_for_each_dev(device)
{
if((((device->class >> 16) & 0xFF) != PCI_BASE_CLASS_DISPLAY) &&
(device->class != (PCI_CLASS_PROCESSOR_CO << 8)))
continue;

pci_read_config_dword(device, 0x04, &scratch);

if (!(scratch & 0x00100000))
continue;

pci_read_config_byte(device, 0x34, &cap_ptr);

if (cap_ptr != 0x00) {
do {
pci_read_config_dword(device,
cap_ptr, &cap_id);

if ((cap_id & 0xff) != 0x02)
cap_ptr = (cap_id >> 8) & 0xff;
}
while (((cap_id & 0xff) != 0x02) && (cap_ptr != 0x00));
}
if (cap_ptr != 0x00) {
...

The bulk of the above is just a reimplementation of pci_find_capability(),
so what about changing the whole shooting match to the following:

pci_for_each_dev(device) {
cap_ptr = pci_find_capability(device, PCI_CAP_ID_AGP);
if (cap_ptr != 0x00) {
...

Is there anything gained by checking the device class before looking
for the AGP capability? The "agp_find_cap" patch attached implements
this idea. I've compiled and booted kernels with both patches, but
they're otherwise untested.

--
Bjorn Helgaas - [email protected]
Linux Systems Operation R&D
Hewlett-Packard


Attachments:
agp_dev-2.4.18-pre6.patch (1.32 kB)
agp_find_cap-2.4.18-pre6.patch (3.78 kB)
Download all attachments

2002-01-27 19:28:32

by Alan

[permalink] [raw]
Subject: Re: [PATCH] 2.4.18-pre6 AGP enable fixes

> Is there anything gained by checking the device class before looking
> for the AGP capability? The "agp_find_cap" patch attached implements
> this idea. I've compiled and booted kernels with both patches, but
> they're otherwise untested.

Just caution about not twiddling stuff we shouldnt touch.

Alan