Date: Mon, 12 Feb 2007 22:53:24 -0500 From: "Stephane E. Potvin" <sepotvin@videotron.ca> To: Adam McDougall <mcdouga9@egr.msu.edu> Cc: freebsd-current@freebsd.org Subject: Re: cpufreq est and Enhanced Sleep (Cx) States for Intel Core and above Message-ID: <45D13634.2020206@videotron.ca> In-Reply-To: <20070212203524.GX88326@egr.msu.edu> References: <20061210002923.GO81923@egr.msu.edu> <20061213003744.GP18799@egr.msu.edu> <4580350F.8080904@videotron.ca> <20070212203524.GX88326@egr.msu.edu>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format. --Boundary_(ID_wuBfOAKY8MGU3NqFx2PydQ) Content-type: text/plain; charset=ISO-8859-1 Content-transfer-encoding: 7BIT Adam McDougall wrote: > On Wed, Dec 13, 2006 at 12:14:55PM -0500, Stephane E. Potvin wrote: > > Adam McDougall wrote: > >On Sat, Dec 09, 2006 at 07:29:23PM -0500, Adam McDougall wrote: > > > > Another thing I wish could work is the Enhanced cpu Sleep States; > > this Dell Latitude D820 laptop only sees C1 although the document > > above indicates it should probably support 4 unique states. Is > > there a way I can debug and/or fix this? I can post dumps of the > > acpi stuff and/or verbose boot logs if it would be helpful. > > > > Thanks > > _______________________________________________ > > > >I am attaching my asl and dsdt acpi dumps incase someone knows for > >something to look for as for why it thinks I only have C1, unless > >its related to the speed control problem above. > > > Hi Adam, > > It's only finding the C1 state for various reasons that you'll find > described in some details in the following email that I send to the acpi > mailing list in June this year. The major reason being that the acpi cpu > driver does not support well multiprocessor systems. > > http://docs.freebsd.org/cgi/getmsg.cgi?fetch=116103+0+archive/2006/freebsd-acpi/20060611.freebsd-acpi > > The email also included a patch to add support for multiprocessor > systems to the acpi cpu driver. I've not updated the patch since then so > it might or might not apply cleanly to a recent current. > > Steph > > I didn't get around to trying that patch, but I have tried -current > after code was committed, including as late as Feb 11. It seems to work, > but if both of my cpus are allowed to enter C3 state, my laptop > clock stops advancing (unless you are causing activity) and the performance > gets very choppy, including mouse cursor halting for a second or two, etc. > Typing or moving the mouse seems to nudge the system into crawling along, > but if I leave it alone, timing loops stall. I could do a while loop with > a sleep on the command line and time just doesn't advance on its own, and > the clock in Gnome would halt. My laptop is using the Hpet timer, but it > doesn't seem to make a difference if I use sysctl to try ACPI-fast or i8254. > Could you try the attached patch on your system? It adds a crude check to make sure that the system is not sleeping for more than 1/hz secs. The acpi_cpu driver should back off to a lower Cx state if it does. Steph --Boundary_(ID_wuBfOAKY8MGU3NqFx2PydQ) Content-type: text/x-patch; CHARSET=US-ASCII; name=longsleep.diff Content-transfer-encoding: 7BIT Content-disposition: inline; filename=longsleep.diff Index: acpi_cpu.c =================================================================== RCS file: /home/FreeBSD/ncvs/src/sys/dev/acpica/acpi_cpu.c,v retrieving revision 1.63 diff -u -r1.63 acpi_cpu.c --- acpi_cpu.c 23 Jan 2007 07:20:44 -0000 1.63 +++ acpi_cpu.c 13 Feb 2007 03:28:32 -0000 @@ -80,6 +80,7 @@ /* Runtime state. */ int cpu_non_c3; /* Index of lowest non-C3 state. */ int cpu_short_slp; /* Count of < 1us sleeps. */ + int cpu_long_slp; /* Count of > hz sleeps. */ u_int cpu_cx_stats[MAX_CX_STATES];/* Cx usage history. */ /* Values for sysctl. */ struct sysctl_ctx_list cpu_sysctl_ctx; @@ -123,6 +124,9 @@ static int cpu_disable_idle; /* Disable entry to idle function */ static int cpu_cx_count; /* Number of valid Cx states */ +/* Longest allowable sleep */ +static int cpu_max_sleep = 0; + /* Values for sysctl. */ static struct sysctl_ctx_list cpu_sysctl_ctx; static struct sysctl_oid *cpu_sysctl_tree; @@ -278,6 +282,9 @@ cpu_smi_cmd = AcpiGbl_FADT->SmiCmd; cpu_cst_cnt = AcpiGbl_FADT->CstCnt; + sc->cpu_short_slp = 0; + sc->cpu_long_slp = 0; + buf.Pointer = NULL; buf.Length = ACPI_ALLOCATE_BUFFER; status = AcpiEvaluateObject(sc->cpu_handle, NULL, NULL, &buf); @@ -311,6 +318,10 @@ /* Queue post cpu-probing task handler */ AcpiOsQueueForExecution(OSD_PRIORITY_LO, acpi_cpu_startup, NULL); + + /* Setup the longest allowable sleep */ + cpu_max_sleep = 1000000 / hz; + cpu_max_sleep *= 10; } /* @@ -832,10 +843,17 @@ * that both cores will get out of C3 state as soon as one of them * requires it. This breaks the sleep detection logic as the sleep * counter is local to each cpu. Disable the sleep logic for now as a - * workaround if there's more than one CPU. The right fix would probably - * be to add quirks for system that don't really support C3 state. + * workaround if we have more than one cpus and some of them are. + * logical (more than one core per package). The right fix would + * probably be to add quirks for system that don't really support + * C3 state. */ - if (mp_ncpus < 2 && sc->cpu_prev_sleep <= 1) { +#ifdef SMP + if (((mp_ncpus >= 2 && logical_cpus_mask == 0) || mp_ncpus < 2) + && sc->cpu_prev_sleep <= 1) { +#else + if (sc->cpu_prev_sleep <= 1) { +#endif sc->cpu_short_slp++; if (sc->cpu_short_slp == 1000 && sc->cpu_cx_lowest != 0) { if (sc->cpu_non_c3 == sc->cpu_cx_lowest && sc->cpu_non_c3 != 0) @@ -849,6 +867,26 @@ } else sc->cpu_short_slp = 0; + /* + * If we keep sleeping for more than 1 hz it may indicate a broken + * system where the clock will not fire anymore in low Cx states. + * Keep backing off in this case until we start getting back clock + * interrupts. + */ + if (sc->cpu_prev_sleep > cpu_max_sleep) { + sc->cpu_long_slp++; + if (sc->cpu_long_slp == 10 && sc->cpu_cx_lowest != 0) { + if (sc->cpu_non_c3 == sc->cpu_cx_lowest && sc->cpu_non_c3 != 0) + sc->cpu_non_c3--; + sc->cpu_cx_lowest--; + sc->cpu_long_slp = 0; + device_printf(sc->cpu_dev, + "too many long sleeps, backing off to C%d\n", + sc->cpu_cx_lowest + 1); + } + } else + sc->cpu_long_slp = 0; + for (i = sc->cpu_cx_lowest; i >= 0; i--) if (sc->cpu_cx_states[i].trans_lat <= sc->cpu_prev_sleep) { cx_next_idx = i; --Boundary_(ID_wuBfOAKY8MGU3NqFx2PydQ)--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?45D13634.2020206>