Date: Mon, 14 Sep 2009 16:08:51 -0400 From: Jung-uk Kim <jkim@FreeBSD.org> To: Kris Kennaway <kris@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r197102 - head/sys/dev/amdtemp Message-ID: <200909141608.53244.jkim@FreeBSD.org> In-Reply-To: <200909141238.23995.jkim@FreeBSD.org> References: <200909112147.n8BLlind064388@svn.freebsd.org> <200909141229.23229.jkim@FreeBSD.org> <200909141238.23995.jkim@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--Boundary-00=_VLqrKhZyAMGWxDj Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline On Monday 14 September 2009 12:38 pm, Jung-uk Kim wrote: > On Monday 14 September 2009 12:29 pm, Jung-uk Kim wrote: > > On Sunday 13 September 2009 12:12 pm, Kris Kennaway wrote: > > > Jung-uk Kim wrote: > > > > Author: jkim > > > > Date: Fri Sep 11 21:47:44 2009 > > > > New Revision: 197102 > > > > URL: http://svn.freebsd.org/changeset/base/197102 > > > > > > > > Log: > > > > Improve amdtemp(4) significantly: > > > > > > > > - Improve newer AMD processor support (Family 0Fh Revision > > > > F and later). - Adjust offset if DiodeOffet is set and valid. > > > > Note it is experimental but it seems to give us more > > > > realistic temperatures. Newer Linux driver blindly adds 21C > > > > for Family 0Fh desktop processors, however. - Always populate > > > > dev.cpu and dev.amdtemp sysctl trees regardless of probe > > > > order for consistency. Previously, dev.cpu.N.temperature was > > > > not populated if amdtemp was loaded later than ACPI CPU > > > > driver and temperatures were not accessible from > > > > dev.amdtemp.N.sensor0 tree for Family 10h/11h processors. - > > > > Read the CPUID from PCI register instead of CPUID instruction > > > > to prevent possible revision mismatches on multi-socket > > > > system. > > > > - Change macros and variables to make them closer to AMD > > > > documents. - Fix style(9) nits and improve comments. > > > > > > It no longer appears to work for me. The old version reported: > > > > > > dev.amdtemp.0.%desc: AMD K8 Thermal Sensors > > > dev.amdtemp.0.%driver: amdtemp > > > dev.amdtemp.0.%parent: hostb9 > > > dev.amdtemp.0.sensor0.core0: 38.0C > > > dev.amdtemp.0.sensor0.core1: 45.0C > > > dev.amdtemp.0.sensor1.core0: 38.0C > > > dev.amdtemp.0.sensor1.core1: 45.0C > > > dev.cpu.0.temperature: 38.0C > > > dev.cpu.1.temperature: 38.0C > > > > > > but none of those sysctl nodes are now present. > > > > > > CPU: AMD Athlon(tm) 64 X2 Dual Core Processor 4600+ > > > (2400.10-MHz K8-class CPU) > > > Origin = "AuthenticAMD" Id = 0x20f32 Stepping = 2 > > > > > > Features=0x178bfbff<FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP > > >,M TR R,PGE,MCA,CMOV,PAT,PSE36,CLFLUSH,MMX,FXSR,SSE,SSE2,HTT> > > > Features2=0x1<SSE3> > > > AMD > > > Features=0xe2500800<SYSCALL,NX,MMX+,FFXSR,LM,3DNow!+,3DNow!> > > > AMD Features2=0x3<LAHF,CMP> > > > ACPI APIC Table: <A M I OEMAPIC > > > > > Arg... This is a Socket 939, Revision E processor and Revision > > C/D/E seems to have different DiodeOffset encoding. Please try > > the attached patch until I fix it properly. > > Please ignore this patch. I need some time to think and fix it > properly. There are too many families and revisions. :-( Please try the attached patch. I tried to implement all the quirks in Revision Guide carefully but I must admit that I haven't tried it on anything earlier than Revision F. Thanks, Jung-uk Kim --Boundary-00=_VLqrKhZyAMGWxDj Content-Type: text/plain; charset="iso-8859-1"; name="amdtemp.c.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="amdtemp.c.diff" --- sys/dev/amdtemp/amdtemp.c +++ sys/dev/amdtemp/amdtemp.c @@ -42,6 +42,7 @@ #include <sys/sysctl.h> #include <sys/systm.h> +#include <machine/cpufunc.h> #include <machine/md_var.h> #include <machine/specialreg.h> @@ -58,10 +59,14 @@ struct amdtemp_softc { device_t sc_dev; - uint32_t sc_mask; + int sc_flags; +#define AMDTEMP_FLAG_DO_QUIRK 0x01 /* DiodeOffset is incorrect. */ +#define AMDTEMP_FLAG_DO_ZERO 0x02 /* DiodeOffset starts from 0C. */ +#define AMDTEMP_FLAG_DO_SIGN 0x04 /* DiodeOffsetSignBit is present. */ +#define AMDTEMP_FLAG_CSWAP 0x08 /* Core0/Core1 selector is swapped. */ +#define AMDTEMP_FLAG_10BIT 0x10 /* CurTmp is 10-bit wide. */ int sc_ncores; int sc_ntemps; - int sc_swap; int32_t (*sc_gettemp)(device_t, amdsensor_t); struct sysctl_oid *sc_sysctl_cpu[MAXCPU]; struct intr_config_hook sc_ich; @@ -168,30 +173,22 @@ static int amdtemp_probe(device_t dev) { - uint32_t cpuid, family, model, temp; + uint32_t family, model; if (resource_disabled("amdtemp", 0)) return (ENXIO); - cpuid = pci_read_config(dev, AMDTEMP_CPUID, 4); - family = CPUID_TO_FAMILY(cpuid); - model = CPUID_TO_MODEL(cpuid); + family = CPUID_TO_FAMILY(cpu_id); + model = CPUID_TO_MODEL(cpu_id); switch (family) { case 0x0f: - if ((model == 0x04 && (cpuid & CPUID_STEPPING) == 0) || - (model == 0x05 && (cpuid & CPUID_STEPPING) <= 1)) + if ((model == 0x04 && (cpu_id & CPUID_STEPPING) == 0) || + (model == 0x05 && (cpu_id & CPUID_STEPPING) <= 1)) return (ENXIO); break; case 0x10: case 0x11: - /* - * DiodeOffset must be non-zero if thermal diode is supported. - */ - temp = pci_read_config(dev, AMDTEMP_THERMTP_STAT, 4); - temp = (temp >> 8) & 0x7f; - if (temp == 0) - return (ENXIO); break; default: return (ENXIO); @@ -207,34 +204,59 @@ struct amdtemp_softc *sc = device_get_softc(dev); struct sysctl_ctx_list *sysctlctx; struct sysctl_oid *sysctlnode; + uint32_t regs[4]; uint32_t cpuid, family, model; - cpuid = pci_read_config(dev, AMDTEMP_CPUID, 4); - family = CPUID_TO_FAMILY(cpuid); - model = CPUID_TO_MODEL(cpuid); + /* + * Errata #154: Incorect Diode Offset + */ + if (cpu_id == 0x20f32) { + do_cpuid(0x80000001, regs); + if ((regs[1] && 0xfff) == 0x2c) + sc->sc_flags |= AMDTEMP_FLAG_DO_QUIRK; + } + /* + * CPUID register is available from Revision F. + */ + family = CPUID_TO_FAMILY(cpu_id); + model = CPUID_TO_MODEL(cpu_id); + if (family != 0x0f || model >= 0x40) { + cpuid = pci_read_config(dev, AMDTEMP_CPUID, 4); + family = CPUID_TO_FAMILY(cpuid); + model = CPUID_TO_MODEL(cpuid); + } + switch (family) { case 0x0f: /* - * Thermaltrip Status Register - CurTmp + * Thermaltrip Status Register * + * - DiodeOffsetSignBit + * + * Revision D & E: bit 24 + * Other: N/A + * + * - ThermSenseCoreSel + * + * Revision F & G: 0 - Core1, 1 - Core0 + * Other: 0 - Core0, 1 - Core1 + * + * - CurTmp + * * Revision G: bits 23-14 - * Earlier: bits 23-16 + * Other: bits 23-16 */ + if (model < 0x40) + sc->sc_flags |= AMDTEMP_FLAG_DO_ZERO; + if (model >= 0x08 && model < 0x40) + sc->sc_flags |= AMDTEMP_FLAG_DO_SIGN; + if (model >= 0x40) + sc->sc_flags |= AMDTEMP_FLAG_CSWAP; if (model >= 0x60 && model != 0xc1) - sc->sc_mask = 0x3ff << 14; - else - sc->sc_mask = 0xff << 16; + sc->sc_flags |= AMDTEMP_FLAG_10BIT; /* - * Thermaltrip Status Register - ThermSenseCoreSel - * - * Revision F: 0 - Core1, 1 - Core0 - * Earlier: 0 - Core0, 1 - Core1 - */ - sc->sc_swap = (model >= 0x40); - - /* * There are two sensors per core. */ sc->sc_ntemps = 2; @@ -244,11 +266,6 @@ case 0x10: case 0x11: /* - * Reported Temperature Control Register - Curtmp - */ - sc->sc_mask = 0x3ff << 21; - - /* * There is only one sensor per package. */ sc->sc_ntemps = 1; @@ -413,7 +430,7 @@ amdtemp_gettemp0f(device_t dev, amdsensor_t sensor) { struct amdtemp_softc *sc = device_get_softc(dev); - uint32_t temp; + uint32_t mask, temp; int32_t diode_offset, offset; uint8_t cfg, sel; @@ -425,7 +442,7 @@ /* FALLTHROUGH */ case SENSOR0_CORE0: case CORE0: - if (sc->sc_swap) + if ((sc->sc_flags & AMDTEMP_FLAG_CSWAP) != 0) sel |= AMDTEMP_TTSR_SELCORE; break; case SENSOR1_CORE1: @@ -433,7 +450,7 @@ /* FALLTHROUGH */ case SENSOR0_CORE1: case CORE1: - if (!sc->sc_swap) + if ((sc->sc_flags & AMDTEMP_FLAG_CSWAP) == 0) sel |= AMDTEMP_TTSR_SELCORE; break; } @@ -447,10 +464,19 @@ /* Adjust offset if DiodeOffset is set and valid. */ temp = pci_read_config(dev, AMDTEMP_THERMTP_STAT, 4); diode_offset = (temp >> 8) & 0x3f; - if (diode_offset != 0) + if ((sc->sc_flags & AMDTEMP_FLAG_DO_ZERO) != 0) { + if ((sc->sc_flags & AMDTEMP_FLAG_DO_SIGN) != 0 && + ((temp >> 24) & 0x1) != 0) + diode_offset *= -1; + if ((sc->sc_flags & AMDTEMP_FLAG_DO_QUIRK) != 0 && + ((temp >> 25) & 0xf) <= 2) + diode_offset += 10; + offset += diode_offset * 10; + } else if (diode_offset != 0) offset += (diode_offset - 11) * 10; - temp = ((temp & sc->sc_mask) >> 14) * 5 / 2 + offset; + mask = (sc->sc_flags & AMDTEMP_FLAG_10BIT) != 0 ? 0x3ff : 0x3fc; + temp = ((temp >> 14) & mask) * 5 / 2 + offset; return (temp); } @@ -458,7 +484,6 @@ static int32_t amdtemp_gettemp(device_t dev, amdsensor_t sensor) { - struct amdtemp_softc *sc = device_get_softc(dev); uint32_t temp; int32_t diode_offset, offset; @@ -472,7 +497,7 @@ offset += (diode_offset - 11) * 10; temp = pci_read_config(dev, AMDTEMP_REPTMP_CTRL, 4); - temp = ((temp & sc->sc_mask) >> 21) * 5 / 4 + offset; + temp = ((temp >> 21) & 0x7ff) * 5 / 4 + offset; return (temp); } --Boundary-00=_VLqrKhZyAMGWxDj--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200909141608.53244.jkim>