[coreboot-gerrit] Change in coreboot[master]: [WIP]mb/lenovo/x200: Use lower C-states on battery

[Arthur Heymans (Code Review)]

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/22932


Change subject: [WIP]mb/lenovo/x200: Use lower C-states on battery
......................................................................

[WIP]mb/lenovo/x200: Use lower C-states on battery

The PNOT method in cpu.asl is very broken as it depends on inexisting
PDCx variables.

This adds an array with C6 as lowest CPU cstate on battery for
thinkpad x200.

TODO: fix cpu.asl (newer gen seems to do it better)
      Fix all other boards

Change-Id: Idf32ee2ac3272fee74ce3b92b2c537cb722991d7
Signed-off-by: Arthur Heymans <arthur at aheymans.xyz>
---
M src/arch/x86/include/arch/acpigen.h
M src/cpu/intel/common/acpi/cpu.asl
M src/cpu/intel/speedstep/acpi.c
M src/mainboard/lenovo/x200/cstates.c
4 files changed, 61 insertions(+), 21 deletions(-)



  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/22932/1

diff --git a/src/arch/x86/include/arch/acpigen.h b/src/arch/x86/include/arch/acpigen.h
index 9ec6ee1..c415da1 100644
--- a/src/arch/x86/include/arch/acpigen.h
+++ b/src/arch/x86/include/arch/acpigen.h
@@ -277,6 +277,7 @@
 			 uint8_t flags);
 
 int get_cst_entries(acpi_cstate_t **);
+int get_bat_cst_entries(acpi_cstate_t **);
 
 /*
  * Soc-implemented functions for generating ACPI AML code for GPIO handling. All
diff --git a/src/cpu/intel/common/acpi/cpu.asl b/src/cpu/intel/common/acpi/cpu.asl
index 117ef46..456b9f7 100644
--- a/src/cpu/intel/common/acpi/cpu.asl
+++ b/src/cpu/intel/common/acpi/cpu.asl
@@ -25,22 +25,13 @@
 Method (PNOT)
 {
 	If (MPEN) {
-		If(And(PDC0, 0x08)) {
-			Notify (\_PR_.CP00, 0x80)	 // _PPC
+	  	Notify (\_PR_.CP00, 0x80)	 // _PPC
+		Sleep(100)
+		Notify(\_PR_.CP00, 0x81) // _CST
 
-			If (And(PDC0, 0x10)) {
-				Sleep(100)
-				Notify(\_PR_.CP00, 0x81) // _CST
-			}
-		}
-
-		If(And(PDC1, 0x08)) {
-			Notify (\_PR_.CP01, 0x80)	 // _PPC
-			If (And(PDC1, 0x10)) {
-				Sleep(100)
-				Notify(\_PR_.CP01, 0x81) // _CST
-			}
-		}
+		Notify (\_PR_.CP01, 0x80)	 // _PPC
+		Sleep(100)
+		Notify(\_PR_.CP01, 0x81) // _CST
 
 	} Else { // UP
 		Notify (\_PR_.CP00, 0x80)
diff --git a/src/cpu/intel/speedstep/acpi.c b/src/cpu/intel/speedstep/acpi.c
index c154da0..969fc9b 100644
--- a/src/cpu/intel/speedstep/acpi.c
+++ b/src/cpu/intel/speedstep/acpi.c
@@ -109,6 +109,19 @@
 	acpigen_write_PPC(0);
 }
 
+static void generate_C_state_entries(acpi_cstate_t *cstate, int nentries)
+{
+	int i;
+	acpigen_write_package(nentries+1);
+	acpigen_write_dword(nentries);
+
+	for (i = 0; i < nentries; i++)
+		acpigen_write_CST_package_entry(cstate + i);
+
+	acpigen_pop_len();
+
+}
+
 /**
  * @brief Generate ACPI entries for Speedstep for each cpu
  */
@@ -120,8 +133,8 @@
 	int numcpus = totalcores/cores_per_package; /* This assumes that all
 						       CPUs share the same
 						       layout. */
-	int num_cstates;
-	acpi_cstate_t *cstates;
+	int num_cstates, num_bat_cstates;
+	acpi_cstate_t *cstates, *bat_cstates;
 	sst_table_t pstates;
 	uint8_t coordination;
 
@@ -129,6 +142,7 @@
 	       numcpus, cores_per_package);
 
 	num_cstates = get_cst_entries(&cstates);
+	num_bat_cstates = get_bat_cst_entries(&bat_cstates);
 	speedstep_gen_pstates(&pstates);
 	if (((cpuid_eax(1) >> 4) & 0xffff) == 0x1067)
 		/* For Penryn use HW_ALL. */
@@ -154,10 +168,20 @@
 					cores_per_package, coordination);
 
 			/* Generate c-state entries. */
-			if (num_cstates > 0)
-				acpigen_write_CST_package(
-							cstates, num_cstates);
-
+			if (num_cstates > 0) {
+				acpigen_write_method("_CST", 0);
+				acpigen_write_if();
+				acpigen_emit_namestring("PWRS");
+				acpigen_emit_byte(RETURN_OP);
+			        generate_C_state_entries(cstates, num_cstates);
+				acpigen_pop_len();
+				if (num_bat_cstates > 0) {
+					acpigen_emit_byte(RETURN_OP);
+					generate_C_state_entries(bat_cstates,
+								num_bat_cstates);
+				}
+				acpigen_pop_len();
+			}
 			acpigen_pop_len();
 		}
 	}
diff --git a/src/mainboard/lenovo/x200/cstates.c b/src/mainboard/lenovo/x200/cstates.c
index 175153e..66d50ad 100644
--- a/src/mainboard/lenovo/x200/cstates.c
+++ b/src/mainboard/lenovo/x200/cstates.c
@@ -35,8 +35,32 @@
 	},
 };
 
+static acpi_cstate_t bat_cst_entries[] = {
+	{
+		/* ACPI C1 / CPU C1 */
+		1, 0x01, 1000,
+		{ ACPI_ADDRESS_SPACE_FIXED, 1, 2, { 1 }, 0, 0 }
+	},
+	{
+		/* ACPI C2 / CPU C2 */
+		2, 0x01,  500,
+		{ ACPI_ADDRESS_SPACE_FIXED, 1, 2, { 1 }, 0x10, 0 }
+	},
+	{
+		/* acpi C3 / cpu C6 */
+		3, 0x37,  250,
+		{ ACPI_ADDRESS_SPACE_FIXED, 1, 2, { 1 }, 0x50, 0 }
+	},
+};
+
 int get_cst_entries(acpi_cstate_t **entries)
 {
 	*entries = cst_entries;
 	return ARRAY_SIZE(cst_entries);
 }
+
+int get_bat_cst_entries(acpi_cstate_t **entries)
+{
+	*entries = bat_cst_entries;
+	return ARRAY_SIZE(bat_cst_entries);
+}

-- 
To view, visit https://review.coreboot.org/22932
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idf32ee2ac3272fee74ce3b92b2c537cb722991d7
Gerrit-Change-Number: 22932
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur at aheymans.xyz>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20171218/405fc0ed/attachment-0001.html>