2005-10-03 17:30:46

by Jordan Crouse

[permalink] [raw]
Subject: [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])

Index: linux-2.6.14-rc2-mm2/MAINTAINERS
===================================================================
--- linux-2.6.14-rc2-mm2.orig/MAINTAINERS
+++ 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]
Index: linux-2.6.14-rc2-mm2/arch/i386/Kconfig
===================================================================
--- linux-2.6.14-rc2-mm2.orig/arch/i386/Kconfig
+++ 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.
Index: linux-2.6.14-rc2-mm2/arch/i386/kernel/cpu/cyrix.c
===================================================================
--- linux-2.6.14-rc2-mm2.orig/arch/i386/kernel/cpu/cyrix.c
+++ linux-2.6.14-rc2-mm2/arch/i386/kernel/cpu/cyrix.c
@@ -342,6 +342,50 @@ static void __init init_cyrix(struct cpu
return;
}

+
+static void __init init_nsc(struct cpuinfo_x86 *c)
+{
+
+
+ /* Handle the National Semiconductor models with non-Cyrix init */
+ if ( (c->x86 == 5) && (c->x86_model >= 4 && c->x86_model <= 5)) {
+ /* Bit 31 in normal CPUID used for nonstandard 3DNow ID;
+ 3DNow is IDd by bit 31 in extended CPUID (1*32+31) anyway */
+ clear_bit(0*32+31, c->x86_capability);
+
+ get_model_name(c);
+
+ switch ( c->x86_model ) {
+ case 4: /* GX1/SCxx00 */
+
+ /* TODO Finish up the GX1/SCxx00 detection */
+ /* GX1 uses bits 16 and 24 differently -
+ you could probably just do
+
+ 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.
+
+ Also checking GX1 cache here needs to be done -
+ display_cacheinfo() won't work according to
+ AMD specs.
+ */
+
+ break;
+
+ case 5: /* GX */
+ display_cacheinfo(c);
+ break;
+ }
+ } else {
+ /* invoke the 'base class' */
+ init_cyrix(c);
+ }
+}
+
+
/*
* Cyrix CPUs without cpuid or with cpuid not yet enabled can be detected
* by the fact that they preserve the flags across the division of 5/2.
@@ -422,7 +466,7 @@ int __init cyrix_init_cpu(void)
static struct cpu_dev nsc_cpu_dev __initdata = {
.c_vendor = "NSC",
.c_ident = { "Geode by NSC" },
- .c_init = init_cyrix,
+ .c_init = init_nsc,
.c_identify = generic_identify,
};

Index: linux-2.6.14-rc2-mm2/include/asm-i386/module.h
===================================================================
--- linux-2.6.14-rc2-mm2.orig/include/asm-i386/module.h
+++ 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
Index: linux-2.6.14-rc2-mm2/include/linux/pci_ids.h
===================================================================
--- linux-2.6.14-rc2-mm2.orig/include/linux/pci_ids.h
+++ 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


2005-10-03 18:02:38

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: [PATCH 1/7] AMD Geode GX/LX support

Hello Jordan,

On Mon, 3 Oct 2005, Jordan Crouse wrote:

> @@ -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)

Can't the Geode boot with a local APIC enabled kernel, albeit without
using it? If it doesn't halt during boot then you don't need this change.

> @@ -749,6 +756,7 @@ config HIGHMEM4G
>
> config HIGHMEM64G
> bool "64GB"
> + depends on !MGEODE_GX

As above.

> +static void __init init_nsc(struct cpuinfo_x86 *c)
> +{
> +
> +
> + /* Handle the National Semiconductor models with non-Cyrix init */
> + if ( (c->x86 == 5) && (c->x86_model >= 4 && c->x86_model <= 5)) {
> + /* Bit 31 in normal CPUID used for nonstandard 3DNow ID;
> + 3DNow is IDd by bit 31 in extended CPUID (1*32+31) anyway */
> + clear_bit(0*32+31, c->x86_capability);

Please create a define.

2005-10-03 18:05:35

by Adrian Bunk

[permalink] [raw]
Subject: Re: [PATCH 1/7] AMD Geode GX/LX support

On Mon, Oct 03, 2005 at 11:47:38AM -0600, Jordan Crouse wrote:
> 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
> ===================================================================
> --- linux-2.6.14-rc2-mm2.orig/MAINTAINERS
> +++ 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
>...

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

> +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

--

"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

2005-10-03 18:34:04

by Alan

[permalink] [raw]
Subject: Re: [PATCH 1/7] AMD Geode GX/LX support

On Llu, 2005-10-03 at 11:47 -0600, Jordan Crouse wrote:
> + - "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)



> config X86_USE_3DNOW
> 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 ?

>
> 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

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"

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

Ditto

> + 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.



2005-10-03 19:29:56

by Jordan Crouse

[permalink] [raw]
Subject: Re: AMD Geode GX/LX support

> > +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.

> 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

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

2005-10-03 19:38:40

by Jordan Crouse

[permalink] [raw]
Subject: Re: AMD Geode GX/LX support

On 03/10/05 20:01 +0100, Alan Cox wrote:
> On Llu, 2005-10-03 at 11:47 -0600, Jordan Crouse wrote:
> > + - "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.

> > config X86_USE_3DNOW
> > 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

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

2005-10-03 20:18:56

by Alan

[permalink] [raw]
Subject: Re: AMD Geode GX/LX support

On Llu, 2005-10-03 at 13:55 -0600, Jordan Crouse wrote:
> 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.

> 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.


2005-10-03 22:05:54

by Jordan Crouse

[permalink] [raw]
Subject: Re: AMD Geode GX/LX support

On 03/10/05 21:46 +0100, Alan Cox wrote:
> On Llu, 2005-10-03 at 13:55 -0600, Jordan Crouse wrote:
> > 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.

> > 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

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

2005-10-05 19:26:11

by Adrian Bunk

[permalink] [raw]
Subject: Re: AMD Geode GX/LX support

On Mon, Oct 03, 2005 at 01:46:55PM -0600, Jordan Crouse wrote:
> > > +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.
>...

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.

> Thanks for your comments,
> Jordan

cu
Adrian

--

"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