2001-11-05 15:49:55

by Nicolas ASPERT

[permalink] [raw]
Subject: [PATCH]agp for i820 chipset

diff -Nru linux-2.4.13-ac7.clean/Documentation/Configure.help linux-2.4.13-ac7_i820/Documentation/Configure.help
--- linux-2.4.13-ac7.clean/Documentation/Configure.help Mon Nov 5 16:26:56 2001
+++ linux-2.4.13-ac7_i820/Documentation/Configure.help Mon Nov 5 16:13:09 2001
@@ -3098,10 +3098,10 @@
a module, say M here and read <file:Documentation/modules.txt>. The
module will be called agpgart.o.

-Intel 440LX/BX/GX/815/830/840/845/850/860 support
+Intel 440LX/BX/GX/815/820/830/840/845/850/860 support
CONFIG_AGP_INTEL
This option gives you AGP support for the GLX component of the
- XFree86 4.x on Intel 440LX/BX/GX, 815, 830, 840, 845, 850 and 860 chipsets.
+ XFree86 4.x on Intel 440LX/BX/GX, 815, 820, 830, 840, 845, 850 and 860 chipsets.

You should say Y here if you use XFree86 3.3.6 or 4.x and want to
use GLX or DRI. If unsure, say N.
diff -Nru linux-2.4.13-ac7.clean/drivers/char/Config.in linux-2.4.13-ac7_i820/drivers/char/Config.in
--- linux-2.4.13-ac7.clean/drivers/char/Config.in Mon Nov 5 16:26:57 2001
+++ linux-2.4.13-ac7_i820/drivers/char/Config.in Mon Nov 5 16:13:34 2001
@@ -213,7 +213,7 @@
bool ' ALI chipset support' CONFIG_AGP_ALI
bool ' AMD Irongate, 761, and 762 support' CONFIG_AGP_AMD
bool ' Generic SiS support' CONFIG_AGP_SIS
- bool ' Intel 440LX/BX/GX and I815/I830/I840/I845/I850/I860 support' CONFIG_AGP_INTEL
+ bool ' Intel 440LX/BX/GX and I815/I820/I830/I840/I845/I850/I860 support' CONFIG_AGP_INTEL
bool ' Intel I810/I815/I830M (on-board) support' CONFIG_AGP_I810
bool ' Serverworks LE/HE support' CONFIG_AGP_SWORKS
bool ' VIA chipset support' CONFIG_AGP_VIA
diff -Nru linux-2.4.13-ac7.clean/drivers/char/agp/agp.h linux-2.4.13-ac7_i820/drivers/char/agp/agp.h
--- linux-2.4.13-ac7.clean/drivers/char/agp/agp.h Mon Nov 5 16:26:57 2001
+++ linux-2.4.13-ac7_i820/drivers/char/agp/agp.h Mon Nov 5 16:32:44 2001
@@ -176,6 +176,9 @@
#ifndef PCI_DEVICE_ID_INTEL_830_M_1
#define PCI_DEVICE_ID_INTEL_830_M_1 0x3577
#endif
+#ifndef PCI_DEVICE_ID_INTEL_820_0
+#define PCI_DEVICE_ID_INTEL_820_0 0x2500
+#endif
#ifndef PCI_DEVICE_ID_INTEL_840_0
#define PCI_DEVICE_ID_INTEL_840_0 0x1a21
#endif
@@ -200,6 +203,9 @@
#ifndef PCI_DEVICE_ID_INTEL_810_1
#define PCI_DEVICE_ID_INTEL_810_1 0x7121
#endif
+#ifndef PCI_DEVICE_ID_INTEL_820_1
+#define PCI_DEVICE_ID_INTEL_820_1 0x250f
+#endif
#ifndef PCI_DEVICE_ID_INTEL_810_DC100_1
#define PCI_DEVICE_ID_INTEL_810_DC100_1 0x7123
#endif
@@ -273,6 +279,10 @@
#define I830_RDRAM_ND(x) (((x) & 0x20) >> 5)
#define I830_RDRAM_DDT(x) (((x) & 0x18) >> 3)

+/* intel i820 registers */
+#define INTEL_I820_RDCR 0x51
+#define INTEL_I820_ERRSTS 0xc8
+
/* intel i840 registers */
#define INTEL_I840_MCHCFG 0x50
#define INTEL_I840_ERRSTS 0xc8
@@ -307,6 +317,8 @@
#define I810_DRAM_ROW_0 0x00000001
#define I810_DRAM_ROW_0_SDRAM 0x00000001

+
+
/* VIA register */
#define VIA_APBASE 0x10
#define VIA_GARTCTRL 0x80
diff -Nru linux-2.4.13-ac7.clean/drivers/char/agp/agpgart_be.c linux-2.4.13-ac7_i820/drivers/char/agp/agpgart_be.c
--- linux-2.4.13-ac7.clean/drivers/char/agp/agpgart_be.c Mon Nov 5 16:26:57 2001
+++ linux-2.4.13-ac7_i820/drivers/char/agp/agpgart_be.c Mon Nov 5 16:18:04 2001
@@ -1477,6 +1477,79 @@
return 0;
}

+static void intel_820_tlbflush(agp_memory * mem)
+{
+ return;
+}
+
+static void intel_820_cleanup(void)
+{
+ u8 temp;
+ aper_size_info_16 *previous_size;
+
+ previous_size = A_SIZE_16(agp_bridge.previous_size);
+ pci_read_config_byte(agp_bridge.dev, INTEL_I820_RDCR, &temp);
+ pci_write_config_byte(agp_bridge.dev, INTEL_I820_RDCR,
+ temp & ~(1 << 1));
+ pci_write_config_byte(agp_bridge.dev, INTEL_APSIZE,
+ (u8)(previous_size->size_value));
+}
+
+static int intel_820_fetch_size(void)
+{
+ int i;
+ u8 temp;
+ aper_size_info_16 *values;
+
+ pci_read_config_byte(agp_bridge.dev, INTEL_APSIZE, &temp);
+ values = A_SIZE_16(agp_bridge.aperture_sizes);
+
+ for (i = 0; i < agp_bridge.num_aperture_sizes; i++) {
+ if (temp == (u8)(values[i].size_value)) {
+ agp_bridge.previous_size =
+ agp_bridge.current_size = (void *) (values + i);
+ agp_bridge.aperture_size_idx = i;
+ return values[i].size;
+ }
+ }
+
+ return 0;
+}
+
+static int intel_820_configure(void)
+{
+ u32 temp;
+ u8 temp2;
+ aper_size_info_16 *current_size;
+
+ current_size = A_SIZE_16(agp_bridge.current_size);
+
+ /* aperture size */
+ pci_write_config_byte(agp_bridge.dev, INTEL_APSIZE,
+ (char)current_size->size_value);
+
+ /* address to map to */
+ pci_read_config_dword(agp_bridge.dev, INTEL_APBASE, &temp);
+ agp_bridge.gart_bus_addr = (temp & PCI_BASE_ADDRESS_MEM_MASK);
+
+ /* attbase - aperture base */
+ pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE,
+ agp_bridge.gatt_bus_addr);
+
+ /* agpctrl */
+ pci_write_config_dword(agp_bridge.dev, INTEL_AGPCTRL, 0x0000);
+
+ /* global enable aperture access */
+ /* This flag is not accessed through MCHCFG register as in */
+ /* i850 chipset. */
+ pci_read_config_byte(agp_bridge.dev, INTEL_I820_RDCR, &temp2);
+ pci_write_config_byte(agp_bridge.dev, INTEL_I820_RDCR,
+ temp2 | (1 << 1));
+ /* clear any possible AGP-related error conditions */
+ pci_write_config_word(agp_bridge.dev, INTEL_I820_ERRSTS, 0x001c);
+ return 0;
+}
+
static int intel_840_configure(void)
{
u32 temp;
@@ -1667,6 +1740,38 @@
(void) pdev; /* unused */
}

+
+static int __init intel_820_setup (struct pci_dev *pdev)
+{
+ agp_bridge.masks = intel_generic_masks;
+ agp_bridge.num_of_masks = 1;
+ agp_bridge.aperture_sizes = (void *) intel_generic_sizes;
+ agp_bridge.size_type = U16_APER_SIZE;
+ agp_bridge.num_aperture_sizes = 7;
+ agp_bridge.dev_private_data = NULL;
+ agp_bridge.needs_scratch_page = FALSE;
+ agp_bridge.configure = intel_820_configure;
+ agp_bridge.fetch_size = intel_820_fetch_size;
+ agp_bridge.cleanup = intel_cleanup;
+ agp_bridge.tlb_flush = intel_820_tlbflush;
+ agp_bridge.mask_memory = intel_mask_memory;
+ agp_bridge.agp_enable = agp_generic_agp_enable;
+ agp_bridge.cache_flush = global_cache_flush;
+ agp_bridge.create_gatt_table = agp_generic_create_gatt_table;
+ agp_bridge.free_gatt_table = agp_generic_free_gatt_table;
+ agp_bridge.insert_memory = agp_generic_insert_memory;
+ agp_bridge.remove_memory = agp_generic_remove_memory;
+ agp_bridge.alloc_by_type = agp_generic_alloc_by_type;
+ agp_bridge.free_by_type = agp_generic_free_by_type;
+ agp_bridge.agp_alloc_page = agp_generic_alloc_page;
+ agp_bridge.agp_destroy_page = agp_generic_destroy_page;
+
+ return 0;
+
+ (void) pdev; /* unused */
+}
+
+
static int __init intel_840_setup (struct pci_dev *pdev)
{
agp_bridge.masks = intel_generic_masks;
@@ -3403,6 +3508,12 @@
"Intel",
"i815",
intel_generic_setup },
+ { PCI_DEVICE_ID_INTEL_820_0,
+ PCI_VENDOR_ID_INTEL,
+ INTEL_I820,
+ "Intel",
+ "i820",
+ intel_820_setup },
{ PCI_DEVICE_ID_INTEL_830_M_0,
PCI_VENDOR_ID_INTEL,
INTEL_I830_M,
diff -Nru linux-2.4.13-ac7.clean/drivers/char/drm/drm_agpsupport.h linux-2.4.13-ac7_i820/drivers/char/drm/drm_agpsupport.h
--- linux-2.4.13-ac7.clean/drivers/char/drm/drm_agpsupport.h Mon Nov 5 16:26:57 2001
+++ linux-2.4.13-ac7_i820/drivers/char/drm/drm_agpsupport.h Mon Nov 5 16:19:17 2001
@@ -273,6 +273,7 @@

#if LINUX_VERSION_CODE >= 0x020400
case INTEL_I815: head->chipset = "Intel i815"; break;
+ case INTEL_I820: head->chipset = "Intel i820"; break;
case INTEL_I840: head->chipset = "Intel i840"; break;
case INTEL_I845: head->chipset = "Intel i845"; break;
case INTEL_I850: head->chipset = "Intel i850"; break;
diff -Nru linux-2.4.13-ac7.clean/include/linux/agp_backend.h linux-2.4.13-ac7_i820/include/linux/agp_backend.h
--- linux-2.4.13-ac7.clean/include/linux/agp_backend.h Mon Nov 5 16:27:04 2001
+++ linux-2.4.13-ac7_i820/include/linux/agp_backend.h Mon Nov 5 16:19:58 2001
@@ -46,6 +46,7 @@
INTEL_GX,
INTEL_I810,
INTEL_I815,
+ INTEL_I820,
INTEL_I830_M,
INTEL_I840,
INTEL_I845,


Attachments:
patch-agp_i820-2.4.13-ac7 (8.00 kB)

2001-11-05 16:01:55

by Robert Love

[permalink] [raw]
Subject: Re: [PATCH]agp for i820 chipset

On Mon, 2001-11-05 at 10:49, Nicolas Aspert wrote:
> Here is an unpdated version of the patch I posted last week that enables
> AGP support for i820 chipset. This one has been done against 2.4.13-ac7,
> _but_ not tested under this kernel. However, it works fine under 2.4.9
> Redhat's kernel (i.e. close to a 2.4.9-ac kernel I think). Xfree-4.1,
> openGL apps and Quake3 are running smoothly.
> [...]

Hey, very good. I don't have an i820 but I am working with the AGPGART
driver so I can say everything looks good. I am actually working on a
rewrite; I find it ridiculous we need all these specific 820 functions.
I am have a design where we load a lookup table, index by the enum, with
the register information and then a generic function can load in the
right value. I really working on cleaning the cruft up ...

I do have two comments, though. I would suggest if you don't hear
anything negative and the patch works for you to go ahead and send it to
Alan and Linus, although you should make sure it is diffed against their
newest trees.


> @@ -200,6 +203,9 @@
> #ifndef PCI_DEVICE_ID_INTEL_810_1
> #define PCI_DEVICE_ID_INTEL_810_1 0x7121
> #endif
> +#ifndef PCI_DEVICE_ID_INTEL_820_1
> +#define PCI_DEVICE_ID_INTEL_820_1 0x250f
> +#endif

I'm not too sure why you need this. I see other chipsets have their
device 0:01 defined but I can't reason why. When I add AGP drivers I
never add it. If you remove it, I think you will find everything still
works.

> +static int intel_820_fetch_size(void)
> +{
> + int i;
> + u8 temp;
> + aper_size_info_16 *values;
> +
> + pci_read_config_byte(agp_bridge.dev, INTEL_APSIZE, &temp);
> + values = A_SIZE_16(agp_bridge.aperture_sizes);
> +
> + for (i = 0; i < agp_bridge.num_aperture_sizes; i++) {
> + if (temp == (u8)(values[i].size_value)) {
> + agp_bridge.previous_size =
> + agp_bridge.current_size = (void *) (values + i);
> + agp_bridge.aperture_size_idx = i;
> + return values[i].size;
> + }
> + }
> +
> + return 0;
> +}

You can just use intel_generic_fetch_size or even one of the
i840-specific or whatever versions, here. Note you don't use anything
specific to the i820, so reduce the footprint and ditch it.

Robert Love

2001-11-05 18:03:55

by Nicolas ASPERT

[permalink] [raw]
Subject: Re: [PATCH]agp for i820 chipset


> Hey, very good. I don't have an i820 but I am working with the AGPGART
> driver so I can say everything looks good. I am actually working on a
> rewrite; I find it ridiculous we need all these specific 820 functions.
> I am have a design where we load a lookup table, index by the enum, with
> the register information and then a generic function can load in the
> right value. I really working on cleaning the cruft up ...


This would be great. This source _does_ need cleanup. Since I saw taht
everyone did copy all the functions I just did the same :-)


> I do have two comments, though. I would suggest if you don't hear
> anything negative and the patch works for you to go ahead and send it to
> Alan and Linus, although you should make sure it is diffed against their
> newest trees.
>


I sent the patch to linux-kernel, but do you think I have to send it
personnally to Alan & Linus ?


>
>>@@ -200,6 +203,9 @@
>> #ifndef PCI_DEVICE_ID_INTEL_810_1
>> #define PCI_DEVICE_ID_INTEL_810_1 0x7121
>> #endif
>>+#ifndef PCI_DEVICE_ID_INTEL_820_1
>>+#define PCI_DEVICE_ID_INTEL_820_1 0x250f
>>+#endif
>>
>
> I'm not too sure why you need this. I see other chipsets have their
> device 0:01 defined but I can't reason why. When I add AGP drivers I
> never add it. If you remove it, I think you will find everything still
> works.
>


Damn ! You're right :-). The first entry is needed, because the i810
chipset uses the secondary device (at least this is what is written in
the code. see the 'agp_find_supported_device' routine. I remember this
is needed for on-board chips. Does that make any sense :-) ? ), but the
i820 related one is useless (afaik).



>
> You can just use intel_generic_fetch_size or even one of the
> i840-specific or whatever versions, here. Note you don't use anything
> specific to the i820, so reduce the footprint and ditch it.
>


The reason for rewriting this function is that, according to the Intel specs,

the APSIZE register is 8bits long (at least for the i820)! The generic function reads

16 bits, so who knows what will be in the neighbouting register ? I guess it was

working by accident, but if you have an argument for sticking to the generic 'fetch_size'

I am all ready to replace my part by the generic one :-)

I have to leave by now, I'll check my e-mail tonight, but it is

likely that I won't make any patch before tomorrow :-(

Best regards.

--
Nicolas Aspert Signal Processing Laboratory (LTS)
Swiss Federal Institute of Technology (EPFL)
Office: ELE 237
Phone: +41 - 21 - 693 36 32 (Office) or 46 21 (LTS lab)
Fax: +41 - 21 - 693 76 00

2001-11-05 20:19:30

by Robert Love

[permalink] [raw]
Subject: Re: [PATCH]agp for i820 chipset

On Mon, 2001-11-05 at 13:03, Nicolas Aspert wrote:
> This would be great. This source _does_ need cleanup. Since I saw taht
> everyone did copy all the functions I just did the same :-)

Yah, it is currently a mess. I hope to have it done soon.

> I sent the patch to linux-kernel, but do you think I have to send it
> personnally to Alan & Linus ?

You will certainly need to send it to Linus, perhaps multiple times :)

Alan may pick it up, but if he doesn't it wouldn't hurt to cut a diff
against his most recent kernel and send it to him and the list.

> > I'm not too sure why you need this. I see other chipsets have their
> > device 0:01 defined but I can't reason why. When I add AGP drivers I
> > never add it. If you remove it, I think you will find everything still
> > works.
>
> Damn ! You're right :-). The first entry is needed, because the i810
> chipset uses the secondary device (at least this is what is written in
> the code. see the 'agp_find_supported_device' routine. I remember this
> is needed for on-board chips. Does that make any sense :-) ? ), but the
> i820 related one is useless (afaik).

Yah, you should be able to remove this no problem.

> > You can just use intel_generic_fetch_size or even one of the
> > i840-specific or whatever versions, here. Note you don't use anything
> > specific to the i820, so reduce the footprint and ditch it.
>
> The reason for rewriting this function is that, according to the Intel specs,
> the APSIZE register is 8bits long (at least for the i820)! The generic function reads
> 16 bits, so who knows what will be in the neighbouting register ? I guess it was
> working by accident, but if you have an argument for sticking to the generic 'fetch_size'
> I am all ready to replace my part by the generic one :-)

Ohh, I didn't notice this. It is odd the two sizes don't match. If
they really are different then I agree; don't use the generic one.

> I have to leave by now, I'll check my e-mail tonight, but it is
> likely that I won't make any patch before tomorrow :-(

No problem ... send it along when you do.

Robert Love

2001-11-05 22:14:57

by Robert Love

[permalink] [raw]
Subject: Re: [PATCH]agp for i820 chipset

On Mon, 2001-11-05 at 15:19, Robert Love wrote:

> Ohh, I didn't notice this. It is odd the two sizes don't match. If
> they really are different then I agree; don't use the generic one.

Hm, one thing to add is if other Intel chipsets have varying APSIZE
values, and this is all they differ by, and one has an APSIZE of 8bits,
then you can cheat and just point to that function.

Robert Love

2001-11-06 07:20:47

by Nicolas ASPERT

[permalink] [raw]
Subject: Re: [PATCH]agp for i820 chipset

Robert Love wrote:

>
>
> Hm, one thing to add is if other Intel chipsets have varying APSIZE
> values, and this is all they differ by, and one has an APSIZE of 8bits,
> then you can cheat and just point to that function.
>


Hello

Finally back to my computer that contains all the info.

In fact, I just gave a quick look to other Intel chipset specs, and I found out

that the 830, 840, 845, 850, and of course my beloved 820 chipset have a 8 bits

APSIZE register ! I think this may be the time to write two 'generic' fetch_size

functions : one for the 8 bits register and one for the 16 bits
register... unless no intel chipset using the generic 'fetch_size' have
16 bits APSIZE registers, in which case we could replace it...

I'll try to see what I can do, and I will send an update for my patch.

Nicolas.

--
Nicolas Aspert Signal Processing Laboratory (LTS)
Swiss Federal Institute of Technology (EPFL)