Hi Aubrey,
Sorry it's taken me so long to take a look at this. I think your changes
look very good. A couple of very minor comments:
- In cpuvar.h, I'm not sure if cpu_max_cstates should be added
directly to the cpu structure. Is this field going to be used
by SPARC? If so, then it's fine. Otherwise, I think it might
need to move to the i86pc version of machcpu.
- cpu_idle.c:
Really just a nit. Comments refer to SpeedStep?
- Have you given any thought on how the kernel is going to
access the ACPI C-state data?
Thanks for adding the OSPM_[CPT]_STATES and pm_cap field to cpudrv_pm_t.
I had planned on doing that myself and hadn't gotten to it.
Mark
Li, Aubrey wrote:
> <I should post the draft one week ago.
> I fighted with my box these days, and finally it refused to work for me.
> What a pity, currently it's the only machine supporting deeper C-state,
> ;( >
>
> The proposed driver was roughly tested on both machines supporting and
> not supporting deeper c-state.
> Before I checked into repo, I'd like to see your comments and
> suggestions. No wonder you see something ugly,
> It's really a draft for now.
> See below and attached.
>
> Thanks,
> -Aubrey
>
> =========================
> diff -r 779c86b8bbe0 usr/src/uts/common/os/cpu.c
> --- a/usr/src/uts/common/os/cpu.c Fri Dec 07 23:46:46 2007 -0500
> +++ b/usr/src/uts/common/os/cpu.c Thu Dec 13 15:17:35 2007 +0800
> @@ -2110,6 +2110,7 @@ static struct {
> kstat_named_t ci_clogid;
> kstat_named_t ci_ncpuperchip;
> kstat_named_t ci_ncoreperchip;
> + kstat_named_t ci_max_cstates;
> #endif
> } cpu_info_template = {
> { "state", KSTAT_DATA_CHAR },
> @@ -2135,6 +2136,7 @@ static struct {
> { "clog_id", KSTAT_DATA_INT32 },
> { "ncpu_per_chip", KSTAT_DATA_INT32 },
> { "ncore_per_chip", KSTAT_DATA_INT32 },
> + { "supported_max_cstates", KSTAT_DATA_INT32 },
> #endif
> };
>
> @@ -2203,6 +2205,7 @@ cpu_info_kstat_update(kstat_t *ksp, int
> cpu_info_template.ci_ncpuperchip.value.l =
> cpuid_get_ncpu_per_chip(cp);
> cpu_info_template.ci_ncoreperchip.value.l =
> cpuid_get_ncore_per_chip(cp);
> + cpu_info_template.ci_ncoreperchip.value.l = cp->cpu_max_cstates;
> #endif
>
> return (0);
> diff -r 779c86b8bbe0 usr/src/uts/common/sys/cpudrv.h
> --- a/usr/src/uts/common/sys/cpudrv.h Fri Dec 07 23:46:46 2007 -0500
> +++ b/usr/src/uts/common/sys/cpudrv.h Thu Dec 13 15:17:35 2007 +0800
> @@ -89,9 +89,15 @@ typedef struct cpudrv_pm {
> kcondvar_t timeout_cv; /* wait on timeout_count change
> */
> #if defined(__x86)
> kthread_t *pm_throttle_thread; /* throttling thread */
> + uint_t max_cstates; /* supported max_cstates */
> #endif
> boolean_t pm_started; /* PM really started */
> + uint_t pm_cap; /* PM capabilities */
> } cpudrv_pm_t;
> +
> +#define OSPM_C_STATES 0x01
> +#define OSPM_P_STATES 0x02
> +#define OSPM_T_STATES 0x04
>
> /*
> * Idle & user threads water marks in percentage
> diff -r 779c86b8bbe0 usr/src/uts/common/sys/cpuvar.h
> --- a/usr/src/uts/common/sys/cpuvar.h Fri Dec 07 23:46:46 2007 -0500
> +++ b/usr/src/uts/common/sys/cpuvar.h Thu Dec 13 15:17:35 2007 +0800
> @@ -212,6 +212,7 @@ typedef struct cpu {
>
> uint64_t cpu_curr_clock; /* current clock freq in
> Hz */
> char *cpu_supp_freqs; /* supported freqs in Hz
> */
> + int32_t cpu_max_cstates; /* supported max cstates
> */
>
> /*
> * Interrupt load factor used by dispatcher & softcall
> diff -r 779c86b8bbe0 usr/src/uts/i86pc/Makefile.files
> --- a/usr/src/uts/i86pc/Makefile.files Fri Dec 07 23:46:46 2007 -0500
> +++ b/usr/src/uts/i86pc/Makefile.files Thu Dec 13 15:17:35 2007 +0800
> @@ -177,7 +177,7 @@ MCAMD_OBJS += \
> mcamd_subr.o \
> mcamd_pcicfg.o
>
> -CPUDRV_OBJS += cpudrv.o cpudrv_plat.o cpu_acpi.o speedstep.o
> cpudrv_intel.o
> +CPUDRV_OBJS += cpudrv.o cpudrv_plat.o cpu_acpi.o cpu_idle.o
> speedstep.o cpudrv_intel.o
> PPM_OBJS += ppm_subr.o ppm.o ppm_plat.o
>
> ACPIPPM_OBJS += acpippm.o acpisleep.o
> diff -r 779c86b8bbe0 usr/src/uts/i86pc/io/cpu_acpi.c
> --- a/usr/src/uts/i86pc/io/cpu_acpi.c Fri Dec 07 23:46:46 2007 -0500
> +++ b/usr/src/uts/i86pc/io/cpu_acpi.c Thu Dec 13 15:17:35 2007 +0800
> @@ -368,6 +368,63 @@ cpu_acpi_cache_ppc(cpu_acpi_handle_t han
> AcpiOsFree(abuf.Pointer);
> }
>
> +int
> +cpu_acpi_cache_cst(cpu_acpi_handle_t handle)
> +{
> + ACPI_BUFFER abuf;
> + ACPI_OBJECT *obj;
> + ACPI_INTEGER count;
> + cpu_acpi_cstate_t *cstate;
> + int i;
> +
> + abuf.Length = ACPI_ALLOCATE_BUFFER;
> + abuf.Pointer = NULL;
> +
> + if (ACPI_FAILURE(AcpiEvaluateObject(handle->cs_handle, "_CST",
> + NULL, &abuf))) {
> + cmn_err(CE_NOTE, "!cpu_acpi: _CST evaluate
> failure");
> + return (-1);
> + }
> + obj = (ACPI_OBJECT *)abuf.Pointer;
> + if (obj->Package.Count < 2) {
> + cmn_err(CE_NOTE, "!cpu_acpi: _CST package bad count
> %d.",
> + obj->Package.Count);
> + goto out;
> + }
> +
> + count = obj->Package.Elements[0].Integer.Value;
> + if (count < 1 || count != obj->Package.Count - 1) {
> + cmn_err(CE_NOTE, "!cpu_acpi: _CST is not valid\n");
> + goto out;
> + }
> + handle->cs_cstates = kmem_zalloc(sizeof (cpu_acpi_cstates_t),
> KM_SLEEP);
> + handle->cs_cstates->cst_count = count;
> + handle->cs_cstates->cst_cstates = kmem_zalloc(sizeof
> (cpu_acpi_cstate_t)
> + * count, KM_SLEEP);
> + cstate = handle->cs_cstates->cst_cstates;
> + for (i = 1; i <=count; i++) {
> + ACPI_OBJECT *pkg;
> + AML_RESOURCE_GENERIC_REGISTER *reg;
> + ACPI_OBJECT *element;
> + pkg = &(obj->Package.Elements[i]);
> + reg = (AML_RESOURCE_GENERIC_REGISTER *)
> + pkg->Package.Elements[0].Buffer.Pointer;
> + cstate->cs_addrspace_id = reg->AddressSpaceId;
> + cstate->cs_address = reg->Address;
> + element = &(pkg->Package.Elements[1]);
> + cstate->cs_type = element->Integer.Value;
> + element = &(pkg->Package.Elements[2]);
> + cstate->cs_latency = element->Integer.Value;
> + element = &(pkg->Package.Elements[3]);
> + cstate->cs_power = element->Integer.Value;
> + cstate++;
> + }
> +
> +out:
> + AcpiOsFree(abuf.Pointer);
> + return (0);
> +}
> +
> /*
> * Cache the _PCT, _PSS, _PSD and _PPC data.
> */
> @@ -508,3 +565,13 @@ cpu_acpi_free_speeds(int *speeds, uint_t
> {
> kmem_free(speeds, nspeeds * sizeof (int));
> }
> +
> +uint_t
> +cpu_acpi_get_max_cstates(cpu_acpi_handle_t handle)
> +{
> + cpu_acpi_cstates_t *cstates = handle->cs_cstates;
> + if (cstates)
> + return (cstates->cst_count);
> + else
> + return (1);
> +}
> diff -r 779c86b8bbe0 usr/src/uts/i86pc/io/cpu_idle.c
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/usr/src/uts/i86pc/io/cpu_idle.c Thu Dec 13 15:18:16 2007 +0800
> @@ -0,0 +1,68 @@
> +/*
> + * CDDL HEADER START
> + *
> + * The contents of this file are subject to the terms of the
> + * Common Development and Distribution License (the "License").
> + * You may not use this file except in compliance with the License.
> + *
> + * You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
> + * or http://www.opensolaris.org/os/licensing.
> + * See the License for the specific language governing permissions
> + * and limitations under the License.
> + *
> + * When distributing Covered Code, include this CDDL HEADER in each
> + * file and include the License file at usr/src/OPENSOLARIS.LICENSE.
> + * If applicable, add the following below this CDDL HEADER, with the
> + * fields enclosed by brackets "[]" replaced with your own identifying
> + * information: Portions Copyright [yyyy] [name of copyright owner]
> + *
> + * CDDL HEADER END
> + */
> +/*
> + * Copyright 2007 Sun Microsystems, Inc. All rights reserved.
> + * Use is subject to license terms.
> + */
> +
> +#pragma ident "%Z%%M% %I% %E% SMI"
> +
> +#include <sys/x86_archext.h>
> +#include <sys/machsystm.h>
> +#include <sys/x_call.h>
> +#include <sys/acpi/acpi.h>
> +#include <sys/acpica.h>
> +#include <sys/cpudrv.h>
> +#include <sys/cpu_idle.h>
> +#include <sys/cpu_acpi.h>
> +#include <sys/cpupm.h>
> +#include <sys/dtrace.h>
> +#include <sys/sdt.h>
> +
> +static int cpu_idle_init(cpudrv_devstate_t *);
> +
> +/*
> + * Interfaces for modules implementing Intel's Enhanced SpeedStep.
> + */
> +struct cpudrv_cstate_ops cpu_idle_ops = {
> + cpu_idle_init,
> +};
> +
> +/*
> + * Validate that this processor supports Speedstep and if so,
> + * get the P-state data from ACPI and cache it.
> + */
> +static int
> +cpu_idle_init(cpudrv_devstate_t *cpudsp)
> +{
> + cpu_acpi_handle_t handle = cpudsp->acpi_handle;
> +
> + /*
> + * Cache the C-state specific ACPI data.
> + */
> + if (cpu_acpi_cache_cst(handle) != 0) {
> + cmn_err(CE_WARN, "Failed to cache ACPI data\n");
> + cpu_acpi_fini(handle);
> + return (-1);
> + }
> +
> + return (0);
> +}
> diff -r 779c86b8bbe0 usr/src/uts/i86pc/io/cpudrv_intel.c
> --- a/usr/src/uts/i86pc/io/cpudrv_intel.c Fri Dec 07 23:46:46 2007
> -0500
> +++ b/usr/src/uts/i86pc/io/cpudrv_intel.c Thu Dec 13 15:17:35 2007
> +0800
> @@ -33,6 +33,7 @@
> #include <sys/cpudrv_vendor.h>
> #include <sys/cpu_acpi.h>
> #include <sys/speedstep.h>
> +#include <sys/cpu_idle.h>
>
> /*
> * The Intel Processor Driver Capabilities (_PDC).
> @@ -42,8 +43,16 @@
> */
> #define CPUDRV_INTEL_PDC_REVISION 0x1
> #define CPUDRV_INTEL_PDC_PS_MSR (1<<0)
> +#define CPUDRV_INTEL_PDC_C1_HALT (1<<1)
> +#define CPUDRV_INTEL_PDC_TS_MSR (1<<2)
> #define CPUDRV_INTEL_PDC_MP (1<<3)
> +#define CPUDRV_INTEL_PDC_C2C3 (1<<4)
> #define CPUDRV_INTEL_PDC_PSD (1<<5)
> +#define CPUDRV_INTEL_PDC_CSD (1<<6)
> +#define CPUDRV_INTEL_PDC_TSD (1<<7)
> +#define CPUDRV_INTEL_PDC_NATIVE_C1 (1<<8)
> +#define CPUDRV_INTEL_PDC_NATIVE_C2C3 (1<<9)
> +#define CPUDRV_INTEL_PDC_HW_PS (1<<11)
>
> static uint32_t cpudrv_intel_pdccap = 0;
>
> @@ -84,8 +93,9 @@ intel_cpupm_init(cpudrv_devstate_t *cpud
> * Set the correct cstate_ops for the processor and
> * enable appropriate _PDC bits.
> */
> - cpuvops->cpupm_cstate_ops = NULL; /* XXX until we support
> */
> - cpudrv_intel_pdccap |= 0; /* XXX until we support
> */
> + cpuvops->cpupm_cstate_ops = &cpu_idle_ops;
> + cpudrv_intel_pdccap |= CPUDRV_INTEL_PDC_C1_HALT |
> + CPUDRV_INTEL_PDC_C2C3;
>
> handle = cpudsp->acpi_handle = cpu_acpi_init(cpudsp->dip);
> if (handle == NULL) {
> diff -r 779c86b8bbe0 usr/src/uts/i86pc/io/cpudrv_plat.c
> --- a/usr/src/uts/i86pc/io/cpudrv_plat.c Fri Dec 07 23:46:46 2007
> -0500
> +++ b/usr/src/uts/i86pc/io/cpudrv_plat.c Thu Dec 13 15:17:35 2007
> +0800
> @@ -35,6 +35,7 @@
> #include <sys/cpudrv_plat.h>
> #include <sys/cpudrv.h>
> #include <sys/speedstep.h>
> +#include <sys/cpu_idle.h>
> #include <sys/machsystm.h>
>
> /*
> @@ -118,6 +119,9 @@ cpudrv_pm_init_module(cpudrv_devstate_t
> cpudrv_pm_init_module(cpudrv_devstate_t *cpudsp)
> {
> cpudrv_vendor_t *vendor;
> + cpudrv_pm_t *cpupm = &(cpudsp->cpudrv_pm);
> + uint_t cpu_max_cstates;
> + cpu_t *cp;
> int ret;
>
> /*
> @@ -148,15 +152,33 @@ cpudrv_pm_init_module(cpudrv_devstate_t
> " unable to initialize P-state support",
> ddi_get_instance(cpudsp->dip));
> cpuvops->cpupm_pstate_ops = NULL;
> - return (B_FALSE);
> }
> + cpupm->pm_cap |= OSPM_P_STATES;
> + }
> +
> + /*
> + * Until C-state and T-state are added, we'll return false here.
> + */
> + if (cpuvops->cpupm_cstate_ops != NULL) {
> + ret = cpuvops->cpupm_cstate_ops->cpucs_init(cpudsp);
> + if (ret !=0) {
> + cmn_err(CE_WARN, "!cpudrv_pm_init_module:
> instance %d:"
> + " unable to initialize C-state support",
> + ddi_get_instance(cpudsp->dip));
> + cpuvops->cpupm_cstate_ops = NULL;
> + }
> + cpupm->pm_cap |= OSPM_C_STATES;
> + }
> +
> + CPUDRV_PM_GET_MAX_CSTATES(cpudsp, cpu_max_cstates);
> + cpupm->max_cstates = cpu_max_cstates;
> +
> + if (cpupm->pm_cap & (OSPM_P_STATES | OSPM_C_STATES)) {
> return (B_TRUE);
> - }
> -
> - /*
> - * Until C-state and T-state are added, we'll return false here.
> - */
> - return (B_FALSE);
> + } else {
> + cpu_acpi_fini(cpudsp->acpi_handle);
> + return (B_FALSE);
> + }
> }
>
> /*
> diff -r 779c86b8bbe0 usr/src/uts/i86pc/sys/cpu_acpi.h
> --- a/usr/src/uts/i86pc/sys/cpu_acpi.h Fri Dec 07 23:46:46 2007 -0500
> +++ b/usr/src/uts/i86pc/sys/cpu_acpi.h Thu Dec 13 15:17:35 2007 +0800
> @@ -105,6 +105,23 @@ typedef int cpu_acpi_ppc_t;
> typedef int cpu_acpi_ppc_t;
>
> /*
> + * Container for _CST information
> + */
> +typedef struct cpu_acpi_cstate
> +{
> + uint32_t cs_addrspace_id;
> + uint32_t cs_address;
> + uint32_t cs_type;
> + uint32_t cs_latency;
> + uint32_t cs_power;
> +} cpu_acpi_cstate_t;
> +
> +typedef struct cpu_acpi_cstates {
> + cpu_acpi_cstate_t *cst_cstates;
> + uint32_t cst_count;
> +} cpu_acpi_cstates_t;
> +
> +/*
> * Container for cached ACPI data.
> */
> typedef struct cpu_acpi_state {
> @@ -112,6 +129,7 @@ typedef struct cpu_acpi_state {
> dev_info_t *cs_dip;
> uint_t cpu_acpi_cached;
> cpu_acpi_pstates_t *cs_pstates;
> + cpu_acpi_cstates_t *cs_cstates;
> cpu_acpi_pct_t cs_pct[2];
> cpu_acpi_psd_t cs_psd;
> cpu_acpi_ppc_t cs_ppc;
> @@ -125,6 +143,7 @@ extern int cpu_acpi_cache_pct(cpu_acpi_h
> extern int cpu_acpi_cache_pct(cpu_acpi_handle_t);
> extern int cpu_acpi_cache_psd(cpu_acpi_handle_t);
> extern void cpu_acpi_cache_ppc(cpu_acpi_handle_t);
> +extern int cpu_acpi_cache_cst(cpu_acpi_handle_t);
> extern int cpu_acpi_cache_data(cpu_acpi_handle_t);
> extern void cpu_acpi_install_ppc_handler(cpu_acpi_handle_t,
> ACPI_NOTIFY_HANDLER, dev_info_t *);
> @@ -133,6 +152,7 @@ extern int cpu_acpi_write_port(ACPI_IO_A
> extern int cpu_acpi_write_port(ACPI_IO_ADDRESS, uint32_t, uint32_t);
> extern int cpu_acpi_read_port(ACPI_IO_ADDRESS, uint32_t *, uint32_t);
> extern uint_t cpu_acpi_get_speeds(cpu_acpi_handle_t, int **);
> +extern uint_t cpu_acpi_get_max_cstates(cpu_acpi_handle_t);
> extern void cpu_acpi_free_speeds(int *, uint_t);
>
> #ifdef __cplusplus
> diff -r 779c86b8bbe0 usr/src/uts/i86pc/sys/cpu_idle.h
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/usr/src/uts/i86pc/sys/cpu_idle.h Thu Dec 13 15:18:00 2007 +0800
> @@ -0,0 +1,43 @@
> +/*
> + * CDDL HEADER START
> + *
> + * The contents of this file are subject to the terms of the
> + * Common Development and Distribution License (the "License").
> + * You may not use this file except in compliance with the License.
> + *
> + * You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
> + * or http://www.opensolaris.org/os/licensing.
> + * See the License for the specific language governing permissions
> + * and limitations under the License.
> + *
> + * When distributing Covered Code, include this CDDL HEADER in each
> + * file and include the License file at usr/src/OPENSOLARIS.LICENSE.
> + * If applicable, add the following below this CDDL HEADER, with the
> + * fields enclosed by brackets "[]" replaced with your own identifying
> + * information: Portions Copyright [yyyy] [name of copyright owner]
> + *
> + * CDDL HEADER END
> + */
> +/*
> + * Copyright 2007 Sun Microsystems, Inc. All rights reserved.
> + * Use is subject to license terms.
> + */
> +
> +#ifndef _CPUIDLE_H
> +#define _CPUIDLE_H
> +
> +#pragma ident "%Z%%M% %I% %E% SMI"
> +
> +#include <sys/cpudrv_vendor.h>
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +struct cpudrv_cstate_ops cpu_idle_ops;
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* _CPUIDLE_H */
> diff -r 779c86b8bbe0 usr/src/uts/i86pc/sys/cpudrv_plat.h
> --- a/usr/src/uts/i86pc/sys/cpudrv_plat.h Fri Dec 07 23:46:46 2007
> -0500
> +++ b/usr/src/uts/i86pc/sys/cpudrv_plat.h Thu Dec 13 15:17:35 2007
> +0800
> @@ -86,6 +86,12 @@ extern ulong_t cpu_ready_set;
> cpu_acpi_free_speeds(speeds, nspeeds);
>
> /*
> + * ACPI provides the supported C-states.
> + */
> +#define CPUDRV_PM_GET_MAX_CSTATES(cpudsp, cpu_max_cstates) \
> + cpu_max_cstates = cpu_acpi_get_max_cstates(cpudsp->acpi_handle);
> +
> +/*
> * Convert speed to Hz.
> */
> #define CPUDRV_PM_SPEED_HZ(unused, mhz) ((uint64_t)mhz *
> 1000000)
> ============================================================
>
>