2004-06-01 16:11:36

by Matt Domsch

[permalink] [raw]
Subject: intel-agp: skip non-AGP devices

Dave,

On our PowerEdge 2600 system, which has an Intel E7501 Memroy
Controller Hub, the intel-agp probe code is reporting, at KERN_ERR no less:

agpgart: Unsupported Intel chipset (device id 254c)

Now, of course it says this, as this device does not present itself as
AGP-capable:

00:00.0 Host bridge: Intel Corp. E7501 Memory Controller Hub (rev 01)
Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr+ Stepping- SERR- FastB2B-
Status: Cap+ 66Mhz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR-
Latency: 0
Capabilities: [40] #09 [1105]


agp_intel_probe() calls pci_find_capability(PCI_CAP_ID_AGP)
but doesn't check the return value (should be zero in this case) prior
to moving into the switch.


The patch below checks for a valid cap_ptr prior to printing the
message, now at KERN_WARNING level (it's not really an error, is it?)

Thoughts?

Thanks,
Matt

--
Matt Domsch
Sr. Software Engineer, Lead Engineer
Dell Linux Solutions linux.dell.com & http://www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

--- intel-agp.c.orig Tue Jun 1 10:45:59 2004
+++ intel-agp.c Tue Jun 1 11:02:56 2004
@@ -1382,8 +1382,9 @@ static int __devinit agp_intel_probe(str
name = "E7205";
break;
default:
- printk(KERN_ERR PFX "Unsupported Intel chipset (device id: %04x)\n",
- pdev->device);
+ if (cap_ptr)
+ printk(KERN_WARNING PFX "Unsupported Intel chipset (device id: %04x)\n",
+ pdev->device);
return -ENODEV;
};


Attachments:
(No filename) (1.47 kB)
(No filename) (189.00 B)
Download all attachments

2004-06-01 16:26:23

by Dave Jones

[permalink] [raw]
Subject: Re: intel-agp: skip non-AGP devices

On Tue, Jun 01, 2004 at 11:04:57AM -0500, Matt Domsch wrote:

> On our PowerEdge 2600 system, which has an Intel E7501 Memroy
> Controller Hub, the intel-agp probe code is reporting, at KERN_ERR no less:
>
> agpgart: Unsupported Intel chipset (device id 254c)
>
> Now, of course it says this, as this device does not present itself as
> AGP-capable:
>
> The patch below checks for a valid cap_ptr prior to printing the
> message, now at KERN_WARNING level (it's not really an error, is it?)

indeed, patch applied.

Thanks,

Dave

2004-06-01 16:33:00

by Dave Jones

[permalink] [raw]
Subject: Re: intel-agp: skip non-AGP devices

On Tue, Jun 01, 2004 at 05:20:58PM +0100, Christoph Hellwig wrote:

> > The patch below checks for a valid cap_ptr prior to printing the
> > message, now at KERN_WARNING level (it's not really an error, is it?)
>
> The real problem is that agpgart doesn't properly fill in the pci_id
> table but claims all devices and then does it's own probing internally.
> This also breaks hotplug in a funny way.

This is fixed in agpgart-bk / -mm. Andi went through all the drivers
adding their id's. Should be going to Linus soon.

Dave

2004-06-01 16:37:00

by Christoph Hellwig

[permalink] [raw]
Subject: Re: intel-agp: skip non-AGP devices

On Tue, Jun 01, 2004 at 11:04:57AM -0500, Matt Domsch wrote:
> Dave,
>
> On our PowerEdge 2600 system, which has an Intel E7501 Memroy
> Controller Hub, the intel-agp probe code is reporting, at KERN_ERR no less:
>
> agpgart: Unsupported Intel chipset (device id 254c)
>

...

> agp_intel_probe() calls pci_find_capability(PCI_CAP_ID_AGP)
> but doesn't check the return value (should be zero in this case) prior
> to moving into the switch.
>
>
> The patch below checks for a valid cap_ptr prior to printing the
> message, now at KERN_WARNING level (it's not really an error, is it?)

The real problem is that agpgart doesn't properly fill in the pci_id
table but claims all devices and then does it's own probing internally.

This also breaks hotplug in a funny way.

2004-06-08 16:00:49

by Matt Domsch

[permalink] [raw]
Subject: Re: intel-agp: skip non-AGP devices

On Tue, Jun 01, 2004 at 05:31:00PM +0100, Dave Jones wrote:
> On Tue, Jun 01, 2004 at 05:20:58PM +0100, Christoph Hellwig wrote:
>
> > > The patch below checks for a valid cap_ptr prior to printing the
> > > message, now at KERN_WARNING level (it's not really an error, is it?)
> >
> > The real problem is that agpgart doesn't properly fill in the pci_id
> > table but claims all devices and then does it's own probing internally.
> > This also breaks hotplug in a funny way.
>
> This is fixed in agpgart-bk / -mm. Andi went through all the drivers
> adding their id's. Should be going to Linus soon.
>
> Dave

FWIW, sworks-agp.c has the same issue in mainline yet today.
agpgart: Unsupported Serverworks chipset (device id: 0011)
agpgart: Unsupported Serverworks chipset (device id: 0201)

I'll look at -mm to verify it's fixed there.

Thanks,
Matt

--
Matt Domsch
Sr. Software Engineer, Lead Engineer
Dell Linux Solutions linux.dell.com & http://www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com


Attachments:
(No filename) (1.01 kB)
(No filename) (189.00 B)
Download all attachments

2004-06-08 16:12:10

by Dave Jones

[permalink] [raw]
Subject: Re: intel-agp: skip non-AGP devices

On Tue, Jun 08, 2004 at 11:00:27AM -0500, Matt Domsch wrote:

> > This is fixed in agpgart-bk / -mm. Andi went through all the drivers
> > adding their id's. Should be going to Linus soon.
>
> FWIW, sworks-agp.c has the same issue in mainline yet today.
> agpgart: Unsupported Serverworks chipset (device id: 0011)
> agpgart: Unsupported Serverworks chipset (device id: 0201)
>
> I'll look at -mm to verify it's fixed there.

Andi did all of the drivers iirc, and these changes are now
in mainline btw.

Dave

2004-06-08 16:18:00

by Matt Domsch

[permalink] [raw]
Subject: Re: intel-agp: skip non-AGP devices

On Tue, Jun 08, 2004 at 11:00:27AM -0500, Matt Domsch wrote:
> On Tue, Jun 01, 2004 at 05:31:00PM +0100, Dave Jones wrote:
> > On Tue, Jun 01, 2004 at 05:20:58PM +0100, Christoph Hellwig wrote:
> >
> > > > The patch below checks for a valid cap_ptr prior to printing the
> > > > message, now at KERN_WARNING level (it's not really an error, is it?)
> > >
> > > The real problem is that agpgart doesn't properly fill in the pci_id
> > > table but claims all devices and then does it's own probing internally.
> > > This also breaks hotplug in a funny way.
> >
> > This is fixed in agpgart-bk / -mm. Andi went through all the drivers
> > adding their id's. Should be going to Linus soon.
> >
> > Dave
>
> FWIW, sworks-agp.c has the same issue in mainline yet today.
> agpgart: Unsupported Serverworks chipset (device id: 0011)
> agpgart: Unsupported Serverworks chipset (device id: 0201)
>
> I'll look at -mm to verify it's fixed there.

agpgart-bk and -mm didn't add proper PCI ID lists to sworks-agp.c (yet
I assume). Patch below does the same for this as I submitted for
Intel previously. It only prints a warning now if the device is AGP
capable but unrecognized.

Thanks,
Matt

--
Matt Domsch
Sr. Software Engineer, Lead Engineer
Dell Linux Solutions linux.dell.com & http://www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

===== drivers/char/agp/sworks-agp.c 1.41 vs edited =====
--- 1.41/drivers/char/agp/sworks-agp.c Tue Jun 1 02:50:11 2004
+++ edited/drivers/char/agp/sworks-agp.c Tue Jun 8 11:13:38 2004
@@ -447,6 +447,7 @@
struct agp_bridge_data *bridge;
struct pci_dev *bridge_dev;
u32 temp, temp2;
+ u8 cap_ptr = 0;

/* Everything is on func 1 here so we are hardcoding function one */
bridge_dev = pci_find_slot((unsigned int)pdev->bus->number,
@@ -457,6 +458,8 @@
return -ENODEV;
}

+ cap_ptr = pci_find_capability(pdev, PCI_CAP_ID_AGP);
+
switch (pdev->device) {
case 0x0006:
/* ServerWorks CNB20HE
@@ -470,8 +473,9 @@
break;

default:
- printk(KERN_ERR PFX "Unsupported Serverworks chipset "
- "(device id: %04x)\n", pdev->device);
+ if (cap_ptr)
+ printk(KERN_WARNING PFX "Unsupported Serverworks chipset "
+ "(device id: %04x)\n", pdev->device);
return -ENODEV;
}


Attachments:
(No filename) (2.22 kB)
(No filename) (189.00 B)
Download all attachments

2004-06-08 16:26:13

by Dave Jones

[permalink] [raw]
Subject: Re: intel-agp: skip non-AGP devices

On Tue, Jun 08, 2004 at 11:17:45AM -0500, Matt Domsch wrote:

> agpgart-bk and -mm didn't add proper PCI ID lists to sworks-agp.c (yet
> I assume). Patch below does the same for this as I submitted for
> Intel previously. It only prints a warning now if the device is AGP
> capable but unrecognized.

Cool, thanks Matt. I'll look over the other drivers to double check.

Dave


2004-06-08 16:46:10

by Dave Jones

[permalink] [raw]
Subject: Re: intel-agp: skip non-AGP devices

On Tue, Jun 08, 2004 at 11:17:45AM -0500, Matt Domsch wrote:

> agpgart-bk and -mm didn't add proper PCI ID lists to sworks-agp.c (yet
> I assume). Patch below does the same for this as I submitted for
> Intel previously. It only prints a warning now if the device is AGP
> capable but unrecognized.

Ok, the other drivers do sensible things like checking for cap_ptr,
and -ENODEV'ing if we don't find it. Too bad there are some wacky
Intel/Sworks chipsets out there that don't have the AGP header,
yet have AGP.

Dave