2001-10-02 01:32:08

by Christof Efkemann

[permalink] [raw]
Subject: [PATCH] Intel 830 support for agpgart

Hi,

this patch for 2.4.10 adds support for the Intel i830 chipset to the agpgart
module, thus eliminating the need for "agp_try_unsupported" with this chip.
It seems to work fine on my Siemens Fujitsu notebook which has an ATI Radeon
Mobility M6 graphics chip.
I guess it should work for others too, maybe someone could try.

--
Regards,
Christof Efkemann


Attachments:
i830-agp.patch (5.02 kB)

2001-10-02 02:02:17

by Robert Love

[permalink] [raw]
Subject: Re: [PATCH] Intel 830 support for agpgart

On Mon, 2001-10-01 at 21:32, Christof Efkemann wrote:

> this patch for 2.4.10 adds support for the Intel i830 chipset to the agpgart
> module, thus eliminating the need for "agp_try_unsupported" with this chip.
> It seems to work fine on my Siemens Fujitsu notebook which has an ATI Radeon
> Mobility M6 graphics chip.
> I guess it should work for others too, maybe someone could try.

You don't need all that code. If agp_try_unsupported works, then
intel_generic_setup works, so you don't need an intel_830_setup.

In other words, if agp_try_unsupported=1 makes everything OK, then you
just need to add the detection code.

Thus, the patch becomes the following. Give that a try.


diff -ur linux-2.4.10.orig/drivers/char/Config.in linux/drivers/char/Config.in
--- linux-2.4.10.orig/drivers/char/Config.in Sun Sep 23 18:52:38 2001
+++ linux/drivers/char/Config.in Tue Oct 2 01:00:37 2001
@@ -205,7 +205,7 @@

dep_tristate '/dev/agpgart (AGP Support)' CONFIG_AGP $CONFIG_DRM_AGP
if [ "$CONFIG_AGP" != "n" ]; then
- bool ' Intel 440LX/BX/GX and I815/I840/I850 support' CONFIG_AGP_INTEL
+ bool ' Intel 440LX/BX/GX and I815/I830/I840/I850 support' CONFIG_AGP_INTEL
bool ' Intel I810/I815 (on-board) support' CONFIG_AGP_I810
bool ' VIA chipset support' CONFIG_AGP_VIA
bool ' AMD Irongate, 761, and 762 support' CONFIG_AGP_AMD
diff -ur linux-2.4.10.orig/drivers/char/agp/agp.h linux/drivers/char/agp/agp.h
--- linux-2.4.10.orig/drivers/char/agp/agp.h Sun Sep 23 18:52:38 2001
+++ linux/drivers/char/agp/agp.h Tue Oct 2 01:01:31 2001
@@ -166,6 +166,9 @@
#ifndef PCI_DEVICE_ID_INTEL_810_0
#define PCI_DEVICE_ID_INTEL_810_0 0x7120
#endif
+#ifndef PCI_DEVICE_ID_INTEL_830_0
+#define PCI_DEVICE_ID_INTEL_830_0 0x3575
+#endif
#ifndef PCI_DEVICE_ID_INTEL_840_0
#define PCI_DEVICE_ID_INTEL_840_0 0x1a21
#endif
diff -ur linux-2.4.10.orig/drivers/char/agp/agpgart_be.c linux/drivers/char/agp/agpgart_be.c
--- linux-2.4.10.orig/drivers/char/agp/agpgart_be.c Sun Sep 23 18:52:38 2001
+++ linux/drivers/char/agp/agpgart_be.c Tue Oct 2 01:05:01 2001
@@ -387,7 +387,7 @@
/*
* Driver routines - start
* Currently this module supports the following chipsets:
- * i810, i815, 440lx, 440bx, 440gx, i840, i850, via vp3, via mvp3,
+ * i810, i815, 440lx, 440bx, 440gx, i830, i840, i850, via vp3, via mvp3,
* via kx133, via kt133, amd irongate, amd 761, amd 762, ALi M1541,
* and generic support for the SiS chipsets.
*/
@@ -2976,6 +3041,12 @@
"Intel",
"i815",
intel_generic_setup },
+ { PCI_DEVICE_ID_INTEL_830_0,
+ PCI_VENDOR_ID_INTEL,
+ INTEL_I830,
+ "Intel",
+ "i830",
+ intel_generic_setup },
{ PCI_DEVICE_ID_INTEL_840_0,
PCI_VENDOR_ID_INTEL,
INTEL_I840,
diff -ur linux-2.4.10.orig/include/linux/agp_backend.h linux/include/linux/agp_backend.h
--- linux-2.4.10.orig/include/linux/agp_backend.h Sun Sep 23 18:52:38 2001
+++ linux/include/linux/agp_backend.h Tue Oct 2 01:00:37 2001
@@ -46,6 +46,7 @@
INTEL_GX,
INTEL_I810,
INTEL_I815,
+ INTEL_I830,
INTEL_I840,
INTEL_I850,
VIA_GENERIC,




--
Robert M. Love
rml at ufl.edu
rml at tech9.net

2001-10-02 13:10:53

by Christof Efkemann

[permalink] [raw]
Subject: Re: [PATCH] Intel 830 support for agpgart

On 01 Oct 2001 22:02:08 -0400
Robert Love <[email protected]> wrote:

> You don't need all that code. If agp_try_unsupported works, then
> intel_generic_setup works, so you don't need an intel_830_setup.
>
> In other words, if agp_try_unsupported=1 makes everything OK, then you
> just need to add the detection code.
>
> Thus, the patch becomes the following. Give that a try.

Yes, that seems to work as well. Although there are two minor things I
noticed:
- First, intel_generic_setup sets num_aperture_sizes to 7, while the i830
has only 4 valid values (32 to 256 MB).
- Second, when intel_generic_configure clears the error status register, it
resets bits 8, 9 and 10. With an i830 it should clear bits 2, 3 and 4.

So I'm not sure if this works in general, or could it cause errors on other
systems?

--
Regards,
Christof Efkemann

2001-10-02 23:45:29

by Robert Love

[permalink] [raw]
Subject: Re: [PATCH] Intel 830 support for agpgart

On Tue, 2001-10-02 at 09:10, Christof Efkemann wrote:
> Yes, that seems to work as well. Although there are two minor things I
> noticed:
> - First, intel_generic_setup sets num_aperture_sizes to 7, while the i830
> has only 4 valid values (32 to 256 MB).
> - Second, when intel_generic_configure clears the error status register, it
> resets bits 8, 9 and 10. With an i830 it should clear bits 2, 3 and 4.
>
> So I'm not sure if this works in general, or could it cause errors on other
> systems?

It will probably work fine on all systems, but its not the right way to
go IMO. Your original implementation was better.

However, I am still disliking the multiple function idea. Same thing
with the i840. The only real difference is those defines.

A _very_ simple solution, IF we had separate CONFIG statements for each
i8xx (or at least one for i830, one for i840, and one for the rest)
would be:

/* all the normal defines here */
#ifdef CONFIG_AGP_I830
#undef whatever_define_i830_is_different_on
#define whatever xxx
/* etc */
#endif
#ifdef CONFIG_AGP_I840
#undef whatever
#define whatever xxx
/* etc */
#endif

and then, voila, we have but one setup function! we can remove all the
unique i830 and i840 muck...

Is seperate config statements a problem? We already have multiple ones
for the i810/i815 on/off-board versions...Hmm.

--
Robert M. Love
rml at ufl.edu
rml at tech9.net

2001-10-03 00:17:15

by David Weinehall

[permalink] [raw]
Subject: Re: [PATCH] Intel 830 support for agpgart

On Tue, Oct 02, 2001 at 07:45:42PM -0400, Robert Love wrote:
> On Tue, 2001-10-02 at 09:10, Christof Efkemann wrote:
> > Yes, that seems to work as well. Although there are two minor things I
> > noticed:
> > - First, intel_generic_setup sets num_aperture_sizes to 7, while the i830
> > has only 4 valid values (32 to 256 MB).
> > - Second, when intel_generic_configure clears the error status register, it
> > resets bits 8, 9 and 10. With an i830 it should clear bits 2, 3 and 4.
> >
> > So I'm not sure if this works in general, or could it cause errors on other
> > systems?
>
> It will probably work fine on all systems, but its not the right way to
> go IMO. Your original implementation was better.
>
> However, I am still disliking the multiple function idea. Same thing
> with the i840. The only real difference is those defines.
>
> A _very_ simple solution, IF we had separate CONFIG statements for each
> i8xx (or at least one for i830, one for i840, and one for the rest)
> would be:
>
> /* all the normal defines here */
> #ifdef CONFIG_AGP_I830
> #undef whatever_define_i830_is_different_on
> #define whatever xxx
> /* etc */
> #endif
> #ifdef CONFIG_AGP_I840
> #undef whatever
> #define whatever xxx
> /* etc */
> #endif
>
> and then, voila, we have but one setup function! we can remove all the
> unique i830 and i840 muck...
>
> Is seperate config statements a problem? We already have multiple ones
> for the i810/i815 on/off-board versions...Hmm.

If the only differences between the different cards are the nr of
aperture-sizes and the status-register settings, why not have a struct
which contains all the valid cards, and use a scan-routine?!


/David Weinehall
_ _
// David Weinehall <[email protected]> /> Northern lights wander \\
// Project MCA Linux hacker // Dance across the winter sky //
\> http://www.acc.umu.se/~tao/ </ Full colour fire </

2001-10-03 02:20:59

by Robert Love

[permalink] [raw]
Subject: Re: [PATCH] Intel 830 support for agpgart

On Tue, 2001-10-02 at 20:16, David Weinehall wrote:
> If the only differences between the different cards are the nr of
> aperture-sizes and the status-register settings, why not have a struct
> which contains all the valid cards, and use a scan-routine?!

I suppose we could, and this is a good idea -- although we'd be
reapproaching the size of the current implementation which would be
theoretically faster, too.

There are only 3 possibilities right now (i830, i840, and everything
else).

Hmm, maybe I will code this nonetheless...

--
Robert M. Love
rml at ufl.edu
rml at tech9.net

2001-10-03 02:52:54

by David Weinehall

[permalink] [raw]
Subject: Re: [PATCH] Intel 830 support for agpgart

On Tue, Oct 02, 2001 at 10:20:43PM -0400, Robert Love wrote:
> On Tue, 2001-10-02 at 20:16, David Weinehall wrote:
> > If the only differences between the different cards are the nr of
> > aperture-sizes and the status-register settings, why not have a struct
> > which contains all the valid cards, and use a scan-routine?!
>
> I suppose we could, and this is a good idea -- although we'd be
> reapproaching the size of the current implementation which would be
> theoretically faster, too.

Afaik, speed is not really an issue (it's not like you're going to
notice a difference anyway, even if you had a struct with 100 different
adapters in it.) As for reapproaching the size of the current
implementation, the difference is that you get one single function that
you don't have to change. You just add one single line to the struct
for each adapter.

Clean design is a virtue ;-)

> There are only 3 possibilities right now (i830, i840, and everything
> else).
>
> Hmm, maybe I will code this nonetheless...

It's your call.


/David
_ _
// David Weinehall <[email protected]> /> Northern lights wander \\
// Project MCA Linux hacker // Dance across the winter sky //
\> http://www.acc.umu.se/~tao/ </ Full colour fire </

2001-10-05 18:51:21

by Christof Efkemann

[permalink] [raw]
Subject: Re: [PATCH] Intel 830 support for agpgart

On Wed, 3 Oct 2001 04:52:57 +0200
David Weinehall <[email protected]> wrote:

> On Tue, Oct 02, 2001 at 10:20:43PM -0400, Robert Love wrote:
> > On Tue, 2001-10-02 at 20:16, David Weinehall wrote:
> > > If the only differences between the different cards are the nr of
> > > aperture-sizes and the status-register settings, why not have a struct
> > > which contains all the valid cards, and use a scan-routine?!

There is already such a struct, agp_bridge_info, and a scan-routine,
agp_lookup_host_bridge. These values could probably be added easily.
Although it would then be necessary for the other chipsets, too.

> Afaik, speed is not really an issue (it's not like you're going to
> notice a difference anyway, even if you had a struct with 100 different
> adapters in it.) As for reapproaching the size of the current
> implementation, the difference is that you get one single function that
> you don't have to change. You just add one single line to the struct
> for each adapter.

Even if it was slower it wouldn't really matter, as this is executed only
once during initialization.

> > There are only 3 possibilities right now (i830, i840, and everything
> > else).

And don't forget the i850 ;-)

--
Regards,
Christof Efkemann

2001-10-06 23:48:04

by Paul Mundt

[permalink] [raw]
Subject: Re: [PATCH] Intel 830 support for agpgart

On Tue, Oct 02, 2001 at 10:20:43PM -0400, Robert Love wrote:
> I suppose we could, and this is a good idea -- although we'd be
> reapproaching the size of the current implementation which would be
> theoretically faster, too.
>
> There are only 3 possibilities right now (i830, i840, and everything
> else).
>
Make that i830, i840, i850, i860 and everything else. A better way for
dealing with specialized configure routines for these various chipsets
would definately seem to be in order. Between i840/i850/i860, there's
very little difference except for what bits to clear in the ERRSTS register.

It would be nice to be able to get the 840/850/860 down to just a few lines
of code a peice instead of this huge overlap of generic routines thats
happening currently.

Here's a patch for i860..

Regards,

--
Paul Mundt <[email protected]>


Attachments:
(No filename) (851.00 B)
2.4.10-i860-agpgart.diff (4.96 kB)
Download all attachments

2001-10-07 01:30:49

by Wenzhuo Zhang

[permalink] [raw]
Subject: Re: [PATCH] Intel 830 support for agpgart

On Sat, Oct 06, 2001 at 04:47:34PM -0700, Paul Mundt wrote:
> On Tue, Oct 02, 2001 at 10:20:43PM -0400, Robert Love wrote:
> > I suppose we could, and this is a good idea -- although we'd be
> > reapproaching the size of the current implementation which would be
> > theoretically faster, too.
> >
> > There are only 3 possibilities right now (i830, i840, and everything
> > else).
> >
> Make that i830, i840, i850, i860 and everything else. A better way for
> dealing with specialized configure routines for these various chipsets
> would definately seem to be in order. Between i840/i850/i860, there's
> very little difference except for what bits to clear in the ERRSTS register.
>
> It would be nice to be able to get the 840/850/860 down to just a few lines
> of code a peice instead of this huge overlap of generic routines thats
> happening currently.
>
> Here's a patch for i860..
>

May I ask how to implement agp_bridge.configure for i845? Dell Dimension
4300 uses this chipset.

# lspci
00:00.0 Host bridge: Intel Corporation: Unknown device 1a30 (rev 03)
00:01.0 PCI bridge: Intel Corporation: Unknown device 1a31 (rev 03)
00:1e.0 PCI bridge: Intel Corporation 82820 820 (Camino 2) Chipset PCI (rev 12)
00:1f.0 ISA bridge: Intel Corporation 82820 820 (Camino 2) Chipset ISA Bridge (ICH2) (rev 12)
00:1f.1 IDE interface: Intel Corporation 82820 820 (Camino 2) Chipset IDE U100 (rev 12)
00:1f.2 USB Controller: Intel Corporation 82820 820 (Camino 2) Chipset USB (Hub A) (rev 12)
00:1f.3 SMBus: Intel Corporation 82820 820 (Camino 2) Chipset SMBus (rev 12)
00:1f.4 USB Controller: Intel Corporation 82820 820 (Camino 2) Chipset USB (Hub B) (rev 12)
00:1f.5 Multimedia audio controller: Intel Corporation: Unknown device 2445 (rev 12)
01:00.0 VGA compatible controller: ATI Technologies Inc: Unknown device 5446
02:09.0 Ethernet controller: Davicom Semiconductor, Inc. Ethernet 100/10 MBit (rev 31)

# lspci -n
00:00.0 Class 0600: 8086:1a30 (rev 03)
00:01.0 Class 0604: 8086:1a31 (rev 03)
00:1e.0 Class 0604: 8086:244e (rev 12)
00:1f.0 Class 0601: 8086:2440 (rev 12)
00:1f.1 Class 0101: 8086:244b (rev 12)
00:1f.2 Class 0c03: 8086:2442 (rev 12)
00:1f.3 Class 0c05: 8086:2443 (rev 12)
00:1f.4 Class 0c03: 8086:2444 (rev 12)
00:1f.5 Class 0401: 8086:2445 (rev 12)
01:00.0 Class 0300: 1002:5446
02:09.0 Class 0200: 1282:9102 (rev 31)

--
Wenzhuo
GnuPG Key ID 0xBA586A68
Key fingerprint = 89C7 C6DE D956 F978 3F12 A8AF 5847 F840 BA58 6A68