On Sun, 28 Jun 2015, Mark Kettenis wrote:
> I have an x41 that prints the following in dmesg:
>
> acpicpu0 at acpi0
> C1: unknown FFH vendor 8: !C3(250@85 io@0x1015), C2(500@1 io@0x1014), PSS
>
> The relevant AML indead has that strange value in the "Bit Width" field:
...
> Obviously this ACPI BIOS is buggy and perhaps we should indeed
> complain. But we also should include a C1 HALT state in the list
> regardless, and I don't think we currently do that.
Give the diff below a try. It inserts a fallback "C1-via-halt" state
before parsing the _CST objects, overwriting it if a valid _CST C1 state
is found. If no valid _CST C1 was found, the fallback should show in
dmesg as "C1(@1 halt!)".
As a side-benefit, this renders a couple paranoia checks obsolete in
acpicpu_idle().
Index: dev/acpi/acpicpu.c
===================================================================
RCS file: /data/src/openbsd/src/sys/dev/acpi/acpicpu.c,v
retrieving revision 1.64
diff -u -p -r1.64 acpicpu.c
--- dev/acpi/acpicpu.c 13 Jun 2015 21:41:42 -0000 1.64
+++ dev/acpi/acpicpu.c 28 Jun 2015 21:55:51 -0000
@@ -77,7 +77,7 @@ void acpicpu_setperf_ppc_change(struct a
/* flags on Intel's FFH mwait method */
#define CST_FLAG_MWAIT_HW_COORD 0x1
#define CST_FLAG_MWAIT_BM_AVOIDANCE 0x2
-#define CST_FLAG_UP_ONLY 0x4000 /* ignore if MP */
+#define CST_FLAG_FALLBACK 0x4000 /* fallback for broken _CST */
#define CST_FLAG_SKIP 0x8000 /* state is worse choice */
#define FLAGS_MWAIT_ONLY 0x02
@@ -339,7 +339,13 @@ acpicpu_add_cstate(struct acpicpu_softc
dnprintf(10," C%d: latency:.%4x power:%.4x addr:%.16llx\n",
state, latency, power, address);
- cx = malloc(sizeof(*cx), M_DEVBUF, M_WAITOK);
+ /* add a new state, or overwrite the fallback C1 state? */
+ if (state != ACPI_STATE_C1 ||
+ (cx = SLIST_FIRST(&sc->sc_cstates)) == NULL ||
+ (cx->flags & CST_FLAG_FALLBACK) == 0) {
+ cx = malloc(sizeof(*cx), M_DEVBUF, M_WAITOK);
+ SLIST_INSERT_HEAD(&sc->sc_cstates, cx, link);
+ }
cx->state = state;
cx->method = method;
@@ -347,8 +353,6 @@ acpicpu_add_cstate(struct acpicpu_softc
cx->latency = latency;
cx->power = power;
cx->address = address;
-
- SLIST_INSERT_HEAD(&sc->sc_cstates, cx, link);
}
/* Found a _CST object, add new cstate for each entry */
@@ -489,9 +493,18 @@ acpicpu_getcst(struct acpicpu_softc *sc)
if (aml_evalname(sc->sc_acpi, sc->sc_devnode, "_CST", 0, NULL, &res))
return (1);
+ /* provide a fallback C1-via-halt in case _CST's C1 is bogus */
+ acpicpu_add_cstate(sc, ACPI_STATE_C1, CST_METH_HALT,
+ CST_FLAG_FALLBACK, 1, -1, 0);
+
aml_foreachpkg(&res, 1, acpicpu_add_cstatepkg, sc);
aml_freevalue(&res);
+ /* only have fallback state? then no _CST objects were understood */
+ cx = SLIST_FIRST(&sc->sc_cstates);
+ if (cx->flags & CST_FLAG_FALLBACK)
+ return (1);
+
/*
* Scan the list for states that are neither lower power nor
* lower latency than the next state; mark them to be skipped.
@@ -500,19 +513,15 @@ acpicpu_getcst(struct acpicpu_softc *sc)
* Also keep track if all the states we'll use use mwait.
*/
use_nonmwait = 0;
- if ((cx = SLIST_FIRST(&sc->sc_cstates)) == NULL)
- use_nonmwait = 1;
- else {
- while ((next_cx = SLIST_NEXT(cx, link)) != NULL) {
- if ((cx->power >= next_cx->power &&
- cx->latency >= next_cx->latency) ||
- (cx->state > 1 &&
- (sc->sc_ci->ci_feature_tpmflags & TPM_ARAT) == 0))
- cx->flags |= CST_FLAG_SKIP;
- else if (cx->method != CST_METH_MWAIT)
- use_nonmwait = 1;
- cx = next_cx;
- }
+ while ((next_cx = SLIST_NEXT(cx, link)) != NULL) {
+ if ((cx->power >= next_cx->power &&
+ cx->latency >= next_cx->latency) ||
+ (cx->state > 1 &&
+ (sc->sc_ci->ci_feature_tpmflags & TPM_ARAT) == 0))
+ cx->flags |= CST_FLAG_SKIP;
+ else if (cx->method != CST_METH_MWAIT)
+ use_nonmwait = 1;
+ cx = next_cx;
}
if (use_nonmwait)
sc->sc_flags &= ~FLAGS_MWAIT_ONLY;
@@ -581,8 +590,12 @@ acpicpu_print_one_cst(struct acpi_cstate
if (cx->power != -1)
printf("%d", cx->power);
printf("@%d%s", cx->latency, meth);
- if (cx->flags & ~CST_FLAG_SKIP)
- printf(".%x", (cx->flags & ~CST_FLAG_SKIP));
+ if (cx->flags & ~CST_FLAG_SKIP) {
+ if (cx->flags & CST_FLAG_FALLBACK)
+ printf("!");
+ else
+ printf(".%x", (cx->flags & ~CST_FLAG_SKIP));
+ }
if (show_addr)
printf("@0x%llx", cx->address);
printf(")");
@@ -1114,12 +1127,6 @@ acpicpu_idle(void)
* states marked skippable
*/
best = cx = SLIST_FIRST(&sc->sc_cstates);
- if (cx == NULL) {
-halt:
- __asm volatile("sti; hlt");
- return;
- }
-
while ((cx->flags & CST_FLAG_SKIP) ||
cx->latency * 3 > sc->sc_prev_sleep) {
if ((cx = SLIST_NEXT(cx, link)) == NULL)
@@ -1140,8 +1147,6 @@ halt:
(cx->flags & CST_FLAG_MWAIT_BM_AVOIDANCE) == 0)
break;
}
- if (cx == NULL)
- goto halt;
best = cx;
}
