AMD Geode GX/LX support

PATCH In review Jordan Crouse 4 participants 9 messages Oct 3, 2005 – Oct 5, 2005 PCI

Share & tools

Jordan Crouse

author

2005-10-03 17:30 UTC #0 lore raw

PATCH 1/7 AMD Geode GX/LX support

This patch adds support for the GX processor including processor identification, and enabling / disabling the appropriate configuration options. Please apply against linux-2.4.14-rc2-mm2.

Signed off by: Jordan Crouse ([email protected])

MAINTAINERS +7 -0

Index: linux-2.6.14-rc2-mm2/MAINTAINERS
===================================================================
@@ -259,6 +259,13 @@ P:	Ivan Kokshaysky
 M:	[email protected]
 S:	Maintained for 2.4; PCI support for 2.6.
 
+AMD GEODE PROCESSOR/CHIPSET SUPPORT
+P:
+M:
+L:	[email protected]
+W:	http://www.amd.com/us-en/ConnectivitySolutions/TechnicalResources/0,,50_2334_2452_11363,00.html
+S:	Supported
+
 APM DRIVER
 P:	Stephen Rothwell
 M:	[email protected]

arch/i386/Kconfig +12 -4

Index: linux-2.6.14-rc2-mm2/arch/i386/Kconfig
===================================================================
@@ -189,6 +189,7 @@ config M386
 	  - "Pentium-4" for the Intel Pentium 4 or P4-based Celeron.
 	  - "K6" for the AMD K6, K6-II and K6-III (aka K6-3D).
 	  - "Athlon" for the AMD K7 family (Athlon/Duron/Thunderbird).
+	  - "Geode GX" for AMD Geode GX processors
 	  - "Crusoe" for the Transmeta Crusoe series.
 	  - "Efficeon" for the Transmeta Efficeon series.
 	  - "Winchip-C6" for original IDT Winchip.

@@ -287,6 +288,12 @@ config MK8
 	  use of some extended instructions, and passes appropriate optimization
 	  flags to GCC.
 
+config MGEODE_GX
+	bool "Geode GX"
+	help
+	  Select this for AMD Geode GX processors.  Enables use of some extended
+	  instructions, and passes appropriate optimization flags to GCC.
+
 config MCRUSOE
 	bool "Crusoe"
 	help

@@ -377,7 +384,7 @@ config X86_L1_CACHE_SHIFT
 	int
 	default "7" if MPENTIUM4 || X86_GENERIC
 	default "4" if X86_ELAN || M486 || M386
-	default "5" if MWINCHIP3D || MWINCHIP2 || MWINCHIPC6 || MCRUSOE || MEFFICEON || MCYRIXIII || MK6 || MPENTIUMIII || MPENTIUMII || M686 || M586MMX || M586TSC || M586 || MVIAC3_2 || MGEODEGX1
+	default "5" if MWINCHIP3D || MWINCHIP2 || MWINCHIPC6 || MCRUSOE || MEFFICEON || MCYRIXIII || MK6 || MPENTIUMIII || MPENTIUMII || M686 || M586MMX || M586TSC || M586 || MVIAC3_2 || MGEODEGX1 || MGEODE_GX
 	default "6" if MK7 || MK8 || MPENTIUMM
 
 config RWSEM_GENERIC_SPINLOCK

@@ -446,12 +453,12 @@ config X86_INTEL_USERCOPY
 
 config X86_USE_PPRO_CHECKSUM
 	bool
-	depends on MWINCHIP3D || MWINCHIP2 || MWINCHIPC6 || MCYRIXIII || MK7 || MK6 || MPENTIUM4 || MPENTIUMM || MPENTIUMIII || MPENTIUMII || M686 || MK8 || MVIAC3_2 || MEFFICEON
+	depends on MWINCHIP3D || MWINCHIP2 || MWINCHIPC6 || MCYRIXIII || MK7 || MK6 || MPENTIUM4 || MPENTIUMM || MPENTIUMIII || MPENTIUMII || M686 || MK8 || MVIAC3_2 || MEFFICEON || MGEODE_GX
 	default y
 
 config X86_USE_3DNOW
 	bool
-	depends on MCYRIXIII || MK7
+	depends on MCYRIXIII || MK7 || MGEODE_GX
 	default y
 
 config X86_OOSTORE

@@ -532,7 +539,7 @@ source "kernel/Kconfig.preempt"
 
 config X86_UP_APIC
 	bool "Local APIC support on uniprocessors"
-	depends on !SMP && !(X86_VISWS || X86_VOYAGER)
+	depends on !SMP && !(X86_VISWS || X86_VOYAGER || MGEODE_GX)
 	help
 	  A local APIC (Advanced Programmable Interrupt Controller) is an
 	  integrated interrupt controller in the CPU. If you have a single-CPU

@@ -749,6 +756,7 @@ config HIGHMEM4G
 
 config HIGHMEM64G
 	bool "64GB"
+	depends on !MGEODE_GX
 	help
 	  Select this if you have a 32-bit processor and more than 4
 	  gigabytes of physical RAM.

arch/i386/kernel/cpu/cyrix.c +45 -1

58 lines not on this page · raw message

include/asm-i386/module.h +2 -0

Index: linux-2.6.14-rc2-mm2/include/asm-i386/module.h
===================================================================
@@ -36,6 +36,8 @@ struct mod_arch_specific
 #define MODULE_PROC_FAMILY "K7 "
 #elif defined CONFIG_MK8
 #define MODULE_PROC_FAMILY "K8 "
+#elif defined CONFIG_MGEODE_GX
+#define MODULE_PROC_FAMILY "GEODE_GX "
 #elif defined CONFIG_X86_ELAN
 #define MODULE_PROC_FAMILY "ELAN "
 #elif defined CONFIG_MCRUSOE

include/linux/pci_ids.h +7 -0

Index: linux-2.6.14-rc2-mm2/include/linux/pci_ids.h
===================================================================
@@ -408,6 +408,13 @@
 #define PCI_DEVICE_ID_NS_SC1100_XBUS	0x0515
 #define PCI_DEVICE_ID_NS_87410		0xd001
 
+#define PCI_DEVICE_ID_NS_CS5535_HOST_BRIDGE  0x0028
+#define PCI_DEVICE_ID_NS_CS5535_ISA_BRIDGE   0x002b
+#define PCI_DEVICE_ID_NS_CS5535_IDE          0x002d
+#define PCI_DEVICE_ID_NS_CS5535_AUDIO        0x002e
+#define PCI_DEVICE_ID_NS_CS5535_USB          0x002f
+#define PCI_DEVICE_ID_NS_CS5535_VIDEO        0x0030
+
 #define PCI_VENDOR_ID_TSENG		0x100c
 #define PCI_DEVICE_ID_TSENG_W32P_2	0x3202
 #define PCI_DEVICE_ID_TSENG_W32P_b	0x3205

Adrian Bunk

2005-10-03 18:05 UTC #2 lore raw

In reply to Jordan Crouse

Quoted 11 lines · Jordan Crouse

This patch adds support for the GX processor including processor identification, and enabling / disabling the appropriate configuration options. Please apply against linux-2.4.14-rc2-mm2.
Signed off by: Jordan Crouse ([email protected])
Index: linux-2.6.14-rc2-mm2/MAINTAINERS
===================================================================

MAINTAINERS P: Ivan Kokshaysky · line 267

Quoted 10 lines

MAINTAINERS +6 -0

@@ -259,6 +259,13 @@ P:	Ivan Kokshaysky
 M:	[email protected]
 S:	Maintained for 2.4; PCI support for 2.6.
 
+AMD GEODE PROCESSOR/CHIPSET SUPPORT
+P:
+M:
+L:	[email protected]
+W:	http://www.amd.com/us-en/ConnectivitySolutions/TechnicalResources/0,,50_2334_2452_11363,00.html
+S:	Supported
...

Could you add yourself in the P: and M: lines?

MAINTAINERS

Quoted 6 lines

MAINTAINERS +5 -0

+config MGEODE_GX
+	bool "Geode GX"
+	help
+	  Select this for AMD Geode GX processors.  Enables use of some extended
+	  instructions, and passes appropriate optimization flags to GCC.
...

The "and passes appropriate optimization flags to GCC" part seems to be missing in your patches.

And it's not clear to me how your new MGEODE_GX option relates to the already existing MGEODEGX1 option.

cu
Adrian

Signature · 4 lines

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Alan Cox

2005-10-03 18:34 UTC #5 lore raw

In reply to Jordan Crouse

arch/i386/Kconfig config M386 · line 195

Quoted 4 lines · Jordan Crouse

arch/i386/Kconfig +1 -0

+	  - "Geode GX" for AMD Geode GX processors
 	  - "Crusoe" for the Transmeta Crusoe series.
 	  - "Efficeon" for the Transmeta Efficeon series.
 	  - "Winchip-C6" for original IDT Winchip.

Whats wrong with the existing MGEODEGX1 define (other than it doesn't say AMD)

arch/i386/Kconfig config X86_INTEL_USERCOPY · line 462

Quoted 5 lines

config X86_USE_3DNOW

arch/i386/Kconfig +1 -1

 	bool
-	depends on MCYRIXIII || MK7
+	depends on MCYRIXIII || MK7 || MGEODE_GX
 	default y

Is this correct - last time I benchmarked it the older GEODE was better off using non MMX copies ?

arch/i386/Kconfig source "kernel/Kconfig.preempt" · line 543

Quoted 9 lines

config X86_OOSTORE

arch/i386/Kconfig +1 -1

@@ -532,7 +539,7 @@ source "kernel/Kconfig.preempt"
 
 config X86_UP_APIC
 	bool "Local APIC support on uniprocessors"
-	depends on !SMP && !(X86_VISWS || X86_VOYAGER)
+	depends on !SMP && !(X86_VISWS || X86_VOYAGER || MGEODE_GX)
 	help

This is wrong - a GEODE kernel can support APIC even if the Geode itself doesn't. Remeber it is *optimised for at least... * not "xyz only"

arch/i386/Kconfig config HIGHMEM4G · line 762

Quoted 4 lines

arch/i386/Kconfig +1 -0

+	depends on !MGEODE_GX
 	help
 	  Select this if you have a 32-bit processor and more than 4
 	  gigabytes of physical RAM.

Ditto

arch/i386/kernel/cpu/cyrix.c static void __init init_cyrix(struct cpu · line 369

Quoted 5 lines

arch/i386/kernel/cpu/cyrix.c +5 -0

+                           clear_bit(0*32+16, &c->x86_capability);
+                           clear_bit(0*32+24, &c->x86_capability);
+
+                           since I don't think the kernel supports
+                           FPU-CMOV or Cyrix MMX.  Unsure tho.

Kernel space enables Cyrix MMX on the processors it knows about that it can. Some userspace will use the extensions if present. The Cyrix init code you unplugged it from knows how to do this which your unfinished patch doesn't.

Jordan Crouse

author

2005-10-03 19:29 UTC #3 lore raw

In reply to Adrian Bunk

Quoted 9 lines

arch/i386/Kconfig config MK8 · line 295
Quoted 6 lines
arch/i386/Kconfig +5 -0
+config MGEODE_GX
+	bool "Geode GX"
+	help
+	  Select this for AMD Geode GX processors.  Enables use of some extended
+	  instructions, and passes appropriate optimization flags to GCC.
...
The "and passes appropriate optimization flags to GCC" part seems to be missing in your patches.

Yes - that is incorrect. As you can no doubt see, I cut-n-pasted from another processor.

Quoted 2 lines

And it's not clear to me how your new MGEODE_GX option relates to the already existing MGEODEGX1 option.

The already existing GEODEGX1 option as it stands has an invalid cache line size, it should be 4 not 5. I'll fix that up with the next revision of the patch.

Thanks for your comments,
Jordan

Signature · 4 lines

Jordan Crouse
Senior Linux Engineer
AMD - Personal Connectivity Solutions Group
<www.amd.com/embeddedprocessors>

Jordan Crouse

author

2005-10-03 19:38 UTC #6 lore raw

In reply to Alan Cox

Quoted 8 lines · Alan Cox

arch/i386/Kconfig config M386 · line 195
Quoted 4 lines · Jordan Crouse
arch/i386/Kconfig +1 -0
+	  - "Geode GX" for AMD Geode GX processors
 	  - "Crusoe" for the Transmeta Crusoe series.
 	  - "Efficeon" for the Transmeta Efficeon series.
 	  - "Winchip-C6" for original IDT Winchip.
Whats wrong with the existing MGEODEGX1 define (other than it doesn't say AMD)

As I mentioned in the previous e-mail, the GEODEGX1 define as it stands is incorrect - the cache line size should be 16 bytes for the GX1. The GX and LX share a newer core, so it stands, I think that they should have a different define.

Quoted 8 lines

arch/i386/Kconfig config X86_INTEL_USERCOPY · line 462
Quoted 5 lines
config X86_USE_3DNOW
arch/i386/Kconfig +1 -1
 	bool
-	depends on MCYRIXIII || MK7
+	depends on MCYRIXIII || MK7 || MGEODE_GX
 	default y
Is this correct - last time I benchmarked it the older GEODE was better off using non MMX copies ?

The jury is still out on that. Certainly, optimizing for i586 architectures is incorrect, because the pipelining is different then on a Pentium machine (i586 optimized code is much slower, especially in userland). I don't have any solid numbers on the performance of just -mmx or -3dnow, but my gut feeling from months of use is that the performance isn't any worse, at least. I suppose that I should come with something more solid then a gut feeling, though, substantial as my gut may be.

Jordan

Signature · 4 lines

Jordan Crouse
Senior Linux Engineer
AMD - Personal Connectivity Solutions Group
<www.amd.com/embeddedprocessors>

Alan Cox

2005-10-03 20:18 UTC #7 lore raw

Quoted 4 lines · Jordan Crouse

As I mentioned in the previous e-mail, the GEODEGX1 define as it stands is incorrect - the cache line size should be 16 bytes for the GX1. The GX and LX share a newer core, so it stands, I think that they should have a different define.

What makes the cores different ? Cache line size ? If they share the same kernel options and build then they don't need a new define, the existing one just need generalising.

Quoted 2 lines

I suppose that I should come with something more solid then a gut feeling, though, substantial as my gut may be.

Indeed. With gcc 3.x I ended up with -m486 -falign-functions=0 and that used to be the settings. I don't know who changed it to pentium-mmx in the end but I objected to about four different patches that did this over time and people still kept submitting them.

Jordan Crouse

author

2005-10-03 22:05 UTC #8 lore raw

Quoted 9 lines · Alan Cox

Quoted 4 lines · Jordan Crouse
As I mentioned in the previous e-mail, the GEODEGX1 define as it stands is incorrect - the cache line size should be 16 bytes for the GX1. The GX and LX share a newer core, so it stands, I think that they should have a different define.
What makes the cores different ? Cache line size ? If they share the same kernel options and build then they don't need a new define, the existing one just need generalising.

After thinking about this a bit more, I think we should keep the GEODEGX1 define because it was for a different core, but consolidate the GX and LX defines into a single define. Originally, we added the defines for ease-of-use in the configuration realm, for example, not allowing the user to build the LX TRNG for a GX kernel. Based on the other comments regarding similar things, I now think that we should avoid it all together.

I'm going to keep the GEODE_LX define, and send GEODE_GX to the briny deep. I guess technically, we could use the codename for the core, but I think that would be more confusing. Best to stick with the marketing names, I think.

Quoted 7 lines

Quoted 2 lines
I suppose that I should come with something more solid then a gut feeling, though, substantial as my gut may be.
Indeed. With gcc 3.x I ended up with -m486 -falign-functions=0 and that used to be the settings. I don't know who changed it to pentium-mmx in the end but I objected to about four different patches that did this over time and people still kept submitting them.

I'll remove the defines for now until we can prove that its better to have them turned on.

Thanks for your comments,
Jordan

Signature · 4 lines

Jordan Crouse
Senior Linux Engineer
AMD - Personal Connectivity Solutions Group
<www.amd.com/embeddedprocessors>

Adrian Bunk

2005-10-05 19:26 UTC #4 lore raw

In reply to Jordan Crouse

Quoted 10 lines · Jordan Crouse

Quoted 9 lines
arch/i386/Kconfig config MK8 · line 295
Quoted 6 lines
arch/i386/Kconfig +5 -0
+config MGEODE_GX
+	bool "Geode GX"
+	help
+	  Select this for AMD Geode GX processors.  Enables use of some extended
+	  instructions, and passes appropriate optimization flags to GCC.
...
The "and passes appropriate optimization flags to GCC" part seems to be missing in your patches.

Quoted 3 lines

Yes - that is incorrect. As you can no doubt see, I cut-n-pasted from another processor.
...

Why don't you set any compiler flags?

I'd assume that there is one compiler flag usually producing the best code for the CPU.

Quoted 2 lines

Thanks for your comments,
Jordan

cu
Adrian

Signature · 4 lines

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed