* [PATCH 0/3] Platform/RasberryPi: Thermal zone
@ 2020-08-13 23:00 Jeremy Linton
2020-08-13 23:00 ` [PATCH 1/3] Platform/RaspberryPi4: Add a basic thermal zone Jeremy Linton
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Jeremy Linton @ 2020-08-13 23:00 UTC (permalink / raw)
To: devel
Cc: Jeremy Linton, Leif Lindholm, Pete Batard, Andrei Warkentin,
Ard Biesheuvel, Samer El-Haj-Mahmoud
This set creates a basic thermal zone, which reads the
SOC temp via a direct register read in AML. It also
adds an active cooling policy using a GPIO pin for fan
control that can optionally be enabled/disabled by the
user from the BDS.
With the fan enabled it should be possible to see the
soc temp like:
# sensors
acpitz-acpi-0
Adapter: ACPI interface
temp1: +57.6C (crit = +90.0C)
and the fan state may be read/cycled with:
/sys/bus/acpi/devices/PNP0C06:00/PNP0C0B:00/physical_node/thermal_cooling/cur_state
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Pete Batard <pete@akeo.ie>
Cc: Andrei Warkentin <awarkentin@vmware.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Jeremy Linton (3):
Platform/RaspberryPi4: Add a basic thermal zone
Platform/RaspberryPi4: Create ACPI fan object
Platform/RaspberryPi: Add entry for user fan control
Platform/RaspberryPi/AcpiTables/Dsdt.asl | 31 ++++++++
Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 55 ++++++++++++++
.../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 3 +
.../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni | 5 ++
.../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 17 +++++
.../RaspberryPi/Drivers/ConfigDxe/SsdtThermal.asl | 83 ++++++++++++++++++++++
Platform/RaspberryPi/Include/ConfigVars.h | 4 ++
Platform/RaspberryPi/RPi3/RPi3.dsc | 5 ++
Platform/RaspberryPi/RPi4/RPi4.dsc | 8 +++
Platform/RaspberryPi/RaspberryPi.dec | 1 +
.../Bcm27xx/Include/IndustryStandard/Bcm2711.h | 2 +
11 files changed, 214 insertions(+)
create mode 100644 Platform/RaspberryPi/Drivers/ConfigDxe/SsdtThermal.asl
--
2.13.7
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/3] Platform/RaspberryPi4: Add a basic thermal zone
2020-08-13 23:00 [PATCH 0/3] Platform/RasberryPi: Thermal zone Jeremy Linton
@ 2020-08-13 23:00 ` Jeremy Linton
2020-08-17 11:10 ` Pete Batard
2020-08-13 23:00 ` [PATCH 2/3] Platform/RaspberryPi4: Create ACPI fan object Jeremy Linton
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Jeremy Linton @ 2020-08-13 23:00 UTC (permalink / raw)
To: devel
Cc: Jeremy Linton, Leif Lindholm, Pete Batard, Andrei Warkentin,
Ard Biesheuvel, Samer El-Haj-Mahmoud
Rather than exporting the temp sensor or mailbox
in ACPI land we can wrap them in AML and use the default
ACPI drivers provided by the OS. This enables the use of
"sensors" in linux to report the SOC temp.
This commit also adds a basic passive cooling ACPI thermalzone
with trip points for passive cooling (throttling) handled
by the vc firmware, hibernate and critical shutdown. The
vc apparently kicks in at ~80C, so the hibernate and critical
set points are set at +5 and +10 of that. In the future
CPPC should be able to monitor the thermal throttling.
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Pete Batard <pete@akeo.ie>
Cc: Andrei Warkentin <awarkentin@vmware.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
Platform/RaspberryPi/AcpiTables/Dsdt.asl | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/Platform/RaspberryPi/AcpiTables/Dsdt.asl b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
index 353af2d876..a5c9567cdf 100644
--- a/Platform/RaspberryPi/AcpiTables/Dsdt.asl
+++ b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
@@ -252,6 +252,37 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN", "RPI", 2)
}
})
}
+
+ // Define a simple thermal zone. The idea here is we compute the SOC temp
+ // via a register we can read, and give it to the OS. This enables basic
+ // reports from the "sensors" utility, and the OS can then poll and take
+ // actions if that temp exceeds any of the given thresholds.
+ Device(EC0)
+ {
+ Name(_HID, EISAID("PNP0C06"))
+ Name (_CCA, 0x0)
+
+ // all temps in are tenths of K (aka 2732 is the min temps in linux (aka 0C))
+ ThermalZone(TZ0) {
+ Method(_TMP, 0, Serialized) {
+ OperationRegion (TEMS, SystemMemory, 0xfd5d2200, 0x8)
+ Field (TEMS, DWordAcc, NoLock, Preserve) {
+ TMPS, 32
+ }
+ return (((419949 - ((TMPS & 0x3ff) * 487)) / 100) + 2732);
+ }
+ Method(_SCP, 3) { } // receive cooling policy from OS
+
+ Method(_CRT) { return(3632) } // (90K) Critical temp point (immediate power-off)
+ Method(_HOT) { return(3582) } // (85K) HOT state where OS should hibernate
+ Method(_PSV) { return(3532) } // (80K) Passive cooling (CPU throttling) trip point
+
+ // SSDT inserts _AC0/_AL0 @60C here, if a FAN is configured
+
+ Name(_TZP, 10) //The OSPM must poll this device every 1 seconds
+ Name(_PSL, Package(){ \_SB_.CPU0, \_SB_.CPU1, \_SB_.CPU2, \_SB_.CPU3})
+ }
+ }
#endif
}
--
2.13.7
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] Platform/RaspberryPi4: Create ACPI fan object
2020-08-13 23:00 [PATCH 0/3] Platform/RasberryPi: Thermal zone Jeremy Linton
2020-08-13 23:00 ` [PATCH 1/3] Platform/RaspberryPi4: Add a basic thermal zone Jeremy Linton
@ 2020-08-13 23:00 ` Jeremy Linton
2020-08-17 11:10 ` [edk2-devel] " Pete Batard
2020-08-13 23:00 ` [PATCH 3/3] Platform/RaspberryPi: Add entry for user fan control Jeremy Linton
2020-08-17 14:31 ` [PATCH 0/3] Platform/RasberryPi: Thermal zone Ard Biesheuvel
3 siblings, 1 reply; 10+ messages in thread
From: Jeremy Linton @ 2020-08-13 23:00 UTC (permalink / raw)
To: devel
Cc: Jeremy Linton, Leif Lindholm, Pete Batard, Andrei Warkentin,
Ard Biesheuvel, Samer El-Haj-Mahmoud
Now that we have a thermal zone we can add active cooling
by specifying active cooling points (_ACx) which can
be tied to fan objects that turn fans on/off using GPIO
pins.
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Pete Batard <pete@akeo.ie>
Cc: Andrei Warkentin <awarkentin@vmware.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
.../RaspberryPi/Drivers/ConfigDxe/SsdtThermal.asl | 83 ++++++++++++++++++++++
1 file changed, 83 insertions(+)
create mode 100644 Platform/RaspberryPi/Drivers/ConfigDxe/SsdtThermal.asl
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/SsdtThermal.asl b/Platform/RaspberryPi/Drivers/ConfigDxe/SsdtThermal.asl
new file mode 100644
index 0000000000..c87bda6dbc
--- /dev/null
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/SsdtThermal.asl
@@ -0,0 +1,83 @@
+/** @file
+ *
+ * Secondary System Description Table (SSDT) for active (fan) cooling
+ *
+ * Copyright (c) 2020, Arm Ltd. All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-2-Clause-Patent
+ *
+ **/
+
+#include <IndustryStandard/Bcm2711.h>
+#include <IndustryStandard/Bcm2836.h>
+#include <IndustryStandard/Bcm2836Gpio.h>
+
+#include <IndustryStandard/Acpi.h>
+
+DefinitionBlock(__FILE__, "SSDT", 5, "RPIFDN", "RPITHFAN", 2)
+{
+#if (GPIO_FAN_PIN != 0)
+ External(\_SB_.EC0, DeviceObj)
+ External(\_SB_.EC0.TZ0, DeviceObj)
+
+ Scope (\_SB_.EC0)
+ {
+ // Describe a fan
+ PowerResource(PFAN, 0, 0) {
+ OperationRegion (GPIO, SystemMemory, GPIO_BASE_ADDRESS, 0x1000)
+ Field (GPIO, DWordAcc, NoLock, Preserve) {
+ Offset(0x1C),
+ GPS0, 32,
+ GPS1, 32,
+ RES1, 32,
+ GPC0, 32,
+ GPC1, 32,
+ RES2, 32,
+ GPL1, 32,
+ GPL2, 32
+ }
+ // We are hitting a GPIO pin to on/off the fan
+ // this assumes that UEFI has programmed the
+ // direction as OUT.
+ // (search "rpi gpio fan controller" for how to
+ // wire this up if your not electrically inclined
+ // the basic idea is to use a BJT/etc to switch a
+ // larger voltage through a fan where the GPIO pin
+ // feeds a NPN/PNP base. Thats because its unlikly
+ // that the fan can be driven directly from the GPIO
+ // pin due to hitting the current limit on the pins.
+ // Matching a resistor between the GPIO->Base can
+ // allow pretty much any random NPN with a reasonable
+ // EC current to work (to limit the GPIO current).)
+ Method (_STA) {
+ if ( GPL1 & (1 << GPIO_FAN_PIN) ) {
+ Return ( 1 ) // present and enabled
+ }
+ Return ( 0 )
+ }
+ Method (_ON) { //turn fan on
+ Store((1 << GPIO_FAN_PIN), GPS0)
+ }
+ Method (_OFF) { //turn fan off
+ Store((1 << GPIO_FAN_PIN), GPC0)
+ }
+ }
+ Device(FAN) {
+ // Note, not currently an ACPIv4 fan
+ // the latter adds speed control/detection
+ // but in the case of linux needs FIF, FPS, FSL, and FST
+ Name(_HID, EISAID("PNP0C0B"))
+ Name(_PR0, Package() {PFAN})
+ }
+ }
+
+ // merge in an active cooling point.
+ Scope (\_SB_.EC0.TZ0)
+ {
+ Method(_AC0) { return(3332) } // (60K) active cooling trip point,
+ // if this is lower than PSV then we
+ // prefer active cooling
+ Name(_AL0, Package(){\_SB_.EC0.FAN}) // the fan used for AC0 above
+ }
+#endif
+}
--
2.13.7
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] Platform/RaspberryPi: Add entry for user fan control
2020-08-13 23:00 [PATCH 0/3] Platform/RasberryPi: Thermal zone Jeremy Linton
2020-08-13 23:00 ` [PATCH 1/3] Platform/RaspberryPi4: Add a basic thermal zone Jeremy Linton
2020-08-13 23:00 ` [PATCH 2/3] Platform/RaspberryPi4: Create ACPI fan object Jeremy Linton
@ 2020-08-13 23:00 ` Jeremy Linton
2020-08-17 11:10 ` Pete Batard
2020-08-17 14:31 ` [PATCH 0/3] Platform/RasberryPi: Thermal zone Ard Biesheuvel
3 siblings, 1 reply; 10+ messages in thread
From: Jeremy Linton @ 2020-08-13 23:00 UTC (permalink / raw)
To: devel
Cc: Jeremy Linton, Leif Lindholm, Pete Batard, Andrei Warkentin,
Ard Biesheuvel, Samer El-Haj-Mahmoud
Add a menu item that allows the user to enable GPIO based
fan control via SSDT. This should only be seen/enabled on RPI4
because that is what its been tested with. As of this commit
its currently limited to only operating on a single GPIO pin (19).
Given GPIO pin current limitations its likely that a bit of
additional circuitry is required to drive a fan, and the GPIO
high/low signal can only be used as a enable/disable signal. A
search for "rpi npn gpio fan" or similar should turn up some
hits for how to do this simply.
It appears there are a couple boards (fan SHIM) which operate this
way, and probably should have custom menu items/SSDT edits as
people acquire the boards and test them.
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Pete Batard <pete@akeo.ie>
Cc: Andrei Warkentin <awarkentin@vmware.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 55 ++++++++++++++++++++++
.../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 3 ++
.../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni | 5 ++
.../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 17 +++++++
Platform/RaspberryPi/Include/ConfigVars.h | 4 ++
Platform/RaspberryPi/RPi3/RPi3.dsc | 5 ++
Platform/RaspberryPi/RPi4/RPi4.dsc | 8 ++++
Platform/RaspberryPi/RaspberryPi.dec | 1 +
.../Bcm27xx/Include/IndustryStandard/Bcm2711.h | 2 +
9 files changed, 100 insertions(+)
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index af54136ade..f10347be64 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -15,6 +15,7 @@
#include <Library/AcpiLib.h>
#include <Library/DebugLib.h>
#include <Library/DevicePathLib.h>
+#include <Library/DxeServicesLib.h>
#include <Library/DxeServicesTableLib.h>
#include <Library/GpioLib.h>
#include <Library/HiiLib.h>
@@ -22,6 +23,7 @@
#include <Library/NetLib.h>
#include <Library/UefiBootServicesTableLib.h>
#include <Library/UefiRuntimeServicesTableLib.h>
+#include <Protocol/AcpiTable.h>
#include <Protocol/BcmGenetPlatformDevice.h>
#include <Protocol/RpiFirmware.h>
#include <ConfigVars.h>
@@ -246,6 +248,14 @@ SetupVariables (
ASSERT_EFI_ERROR (Status);
}
+ Size = sizeof (UINT32);
+ Status = gRT->GetVariable (L"FanOnGpio",
+ &gConfigDxeFormSetGuid,
+ NULL, &Size, &Var32);
+ if (EFI_ERROR (Status)) {
+ PcdSet32 (PcdFanOnGpio, PcdGet32 (PcdFanOnGpio));
+ }
+
Size = sizeof(AssetTagVar);
Status = gRT->GetVariable(L"AssetTag",
@@ -368,6 +378,7 @@ ApplyVariables (
UINT32 CpuClock = PcdGet32 (PcdCpuClock);
UINT32 CustomCpuClock = PcdGet32 (PcdCustomCpuClock);
UINT32 Rate = 0;
+ UINT32 FanOnGpio = PcdGet32 (PcdFanOnGpio);
switch (CpuClock) {
case CHIPSET_CPU_CLOCK_LOW:
@@ -565,8 +576,49 @@ ApplyVariables (
GpioPinFuncSet (23, GPIO_FSEL_INPUT);
GpioPinFuncSet (24, GPIO_FSEL_INPUT);
}
+
+ if (FanOnGpio) {
+ DEBUG ((DEBUG_INFO, "Fan enabled on GPIO %d\n", FanOnGpio));
+ GpioPinFuncSet(FanOnGpio, GPIO_FSEL_OUTPUT);
+ }
}
+EFI_STATUS
+FindInstallSsdt(UINT64 OemTableId)
+{
+ EFI_ACPI_TABLE_PROTOCOL *AcpiTable;
+ UINTN Index;
+ EFI_ACPI_DESCRIPTION_HEADER *Ssdt;
+ UINTN SsdtSize;
+ EFI_STATUS Status;
+ UINTN TableKey;
+
+
+ Status = gBS->LocateProtocol (&gEfiAcpiTableProtocolGuid, NULL,
+ (VOID **)&AcpiTable);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ for (Index = 0; !EFI_ERROR(Status); Index++) {
+ Status = GetSectionFromFv (&gEfiCallerIdGuid, EFI_SECTION_RAW, Index,
+ (VOID **)&Ssdt, &SsdtSize);
+ if (Ssdt->OemTableId == OemTableId)
+ break;
+ SsdtSize = 0;
+ }
+
+ if (SsdtSize > 0) {
+ Status = AcpiTable->InstallAcpiTable (AcpiTable, Ssdt, SsdtSize,
+ &TableKey);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_WARN, "%a: failed to install SSDT table %r\n",
+ __FUNCTION__, Status));
+ }
+ }
+
+ return Status;
+}
EFI_STATUS
EFIAPI
@@ -620,6 +672,9 @@ ConfigInitialize (
PcdGet32 (PcdSystemTableMode) == SYSTEM_TABLE_MODE_BOTH) {
Status = LocateAndInstallAcpiFromFv (&mAcpiTableFile);
ASSERT_EFI_ERROR (Status);
+ if (PcdGet32 (PcdFanOnGpio)) {
+ FindInstallSsdt(SIGNATURE_64 ('R', 'P', 'I', 'T', 'H', 'F', 'A', 'N'));
+ }
}
Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_NOTIFY, RegisterDevices,
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
index cdce35bc74..fe3a01a570 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
@@ -28,6 +28,7 @@
ConfigDxeFormSetGuid.h
ConfigDxeHii.vfr
ConfigDxeHii.uni
+ SsdtThermal.asl
XhciQuirk.c
[Packages]
@@ -46,6 +47,7 @@
AcpiLib
BaseLib
DebugLib
+ DxeServicesLib
DxeServicesTableLib
GpioLib
HiiLib
@@ -89,6 +91,7 @@
gRaspberryPiTokenSpaceGuid.PcdSystemTableMode
gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB
gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB
+ gRaspberryPiTokenSpaceGuid.PcdFanOnGpio
[Depex]
gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
index 03763710a1..491d022fff 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
@@ -48,6 +48,11 @@
#string STR_ADVANCED_SYSTAB_BOTH #language en-US "ACPI + Devicetree"
#string STR_ADVANCED_SYSTAB_DT #language en-US "Devicetree"
+#string STR_ADVANCED_FANONGPIO_PROMPT #language en-US "ACPI fan control"
+#string STR_ADVANCED_FANONGPIO_HELP #language en-US "Cycle a fan via GPIO-19 if temp exceeds 60C"
+#string STR_ADVANCED_FANONGPIO_OFF #language en-US "Disabled"
+#string STR_ADVANCED_FANONGPIO_ON #language en-US "Enabled"
+
#string STR_ADVANCED_ASSET_TAG_PROMPT #language en-US "Asset Tag"
#string STR_ADVANCED_ASSET_TAG_HELP #language en-US "Set the system Asset Tag"
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
index d5615d7af0..0a5e4163e8 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -10,6 +10,7 @@
#include <Guid/HiiPlatformSetupFormset.h>
#include "ConfigDxeFormSetGuid.h"
#include <ConfigVars.h>
+#include <IndustryStandard/Bcm2711.h>
//
// EFI Variable attributes
@@ -45,6 +46,11 @@ formset
name = RamLimitTo3GB,
guid = CONFIGDXE_FORM_SET_GUID;
+ efivarstore ADVANCED_FAN_ON_GPIO_VARSTORE_DATA,
+ attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
+ name = FanOnGpio,
+ guid = CONFIGDXE_FORM_SET_GUID;
+
efivarstore SYSTEM_TABLE_MODE_VARSTORE_DATA,
attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
name = SystemTableMode,
@@ -174,6 +180,17 @@ formset
option text = STRING_TOKEN(STR_ADVANCED_SYSTAB_DT), value = SYSTEM_TABLE_MODE_DT, flags = DEFAULT;
endoneof;
+#if (RPI_MODEL == 4)
+ grayoutif NOT ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_ACPI;
+ oneof varid = FanOnGpio.Enabled,
+ prompt = STRING_TOKEN(STR_ADVANCED_FANONGPIO_PROMPT),
+ help = STRING_TOKEN(STR_ADVANCED_FANONGPIO_HELP),
+ flags = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
+ option text = STRING_TOKEN(STR_ADVANCED_FANONGPIO_OFF), value = 0, flags = DEFAULT;
+ option text = STRING_TOKEN(STR_ADVANCED_FANONGPIO_ON), value = GPIO_FAN_PIN, flags = 0;
+ endoneof;
+ endif;
+#endif
string varid = AssetTag.AssetTag,
prompt = STRING_TOKEN(STR_ADVANCED_ASSET_TAG_PROMPT),
help = STRING_TOKEN(STR_ADVANCED_ASSET_TAG_HELP),
diff --git a/Platform/RaspberryPi/Include/ConfigVars.h b/Platform/RaspberryPi/Include/ConfigVars.h
index b1689b004d..1a40469bfa 100644
--- a/Platform/RaspberryPi/Include/ConfigVars.h
+++ b/Platform/RaspberryPi/Include/ConfigVars.h
@@ -69,6 +69,10 @@ typedef struct {
} ADVANCED_RAM_LIMIT_TO_3GB_VARSTORE_DATA;
typedef struct {
+ UINT32 Enabled;
+} ADVANCED_FAN_ON_GPIO_VARSTORE_DATA;
+
+typedef struct {
#define SYSTEM_TABLE_MODE_ACPI 0
#define SYSTEM_TABLE_MODE_BOTH 1
#define SYSTEM_TABLE_MODE_DT 2
diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
index 0998d8366c..cef8932ca2 100644
--- a/Platform/RaspberryPi/RPi3/RPi3.dsc
+++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
@@ -499,6 +499,11 @@
gRaspberryPiTokenSpaceGuid.PcdSystemTableMode|L"SystemTableMode"|gConfigDxeFormSetGuid|0x0|1
#
+ # Enable a fan in the ACPI thermal zone on GPIO pin #
+ #
+ gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFormSetGuid|0x0|0
+
+ #
# Common UEFI ones.
#
diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index baa7e63483..9d0eaf10a1 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -510,6 +510,14 @@
gRaspberryPiTokenSpaceGuid.PcdSystemTableMode|L"SystemTableMode"|gConfigDxeFormSetGuid|0x0|0
#
+ # Enable a fan in the ACPI thermal zone on GPIO pin #
+ #
+ # 0 - DISABLED
+ # 19 - Enabled on pin 19
+ #
+ gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFormSetGuid|0x0|0
+
+ #
# Common UEFI ones.
#
diff --git a/Platform/RaspberryPi/RaspberryPi.dec b/Platform/RaspberryPi/RaspberryPi.dec
index c71177a2f7..a73650f2c3 100644
--- a/Platform/RaspberryPi/RaspberryPi.dec
+++ b/Platform/RaspberryPi/RaspberryPi.dec
@@ -66,3 +66,4 @@
gRaspberryPiTokenSpaceGuid.PcdSystemTableMode|1|UINT32|0x0000001B
gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB|0|UINT32|0x00000019
gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|0|UINT32|0x0000001A
+ gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|0|UINT32|0x0000001C
diff --git a/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h b/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
index e9c81cafa1..7d9ea5d35c 100644
--- a/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
+++ b/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
@@ -86,4 +86,6 @@
#define GENET_BASE_ADDRESS FixedPcdGet64 (PcdBcmGenetRegistersAddress)
#define GENET_LENGTH 0x00010000
+#define GPIO_FAN_PIN 19 // fan shim uses GPIO 18
+
#endif /* BCM2711_H__ */
--
2.13.7
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] Platform/RaspberryPi4: Add a basic thermal zone
2020-08-13 23:00 ` [PATCH 1/3] Platform/RaspberryPi4: Add a basic thermal zone Jeremy Linton
@ 2020-08-17 11:10 ` Pete Batard
0 siblings, 0 replies; 10+ messages in thread
From: Pete Batard @ 2020-08-17 11:10 UTC (permalink / raw)
To: Jeremy Linton, devel
Cc: Leif Lindholm, Andrei Warkentin, Ard Biesheuvel,
Samer El-Haj-Mahmoud
Nothing major, just whitespace/style and one minor capitalization issue:
On 2020.08.14 00:00, Jeremy Linton wrote:
> Rather than exporting the temp sensor or mailbox
> in ACPI land we can wrap them in AML and use the default
> ACPI drivers provided by the OS. This enables the use of
> "sensors" in linux to report the SOC temp.
>
> This commit also adds a basic passive cooling ACPI thermalzone
> with trip points for passive cooling (throttling) handled
> by the vc firmware, hibernate and critical shutdown. The
> vc apparently kicks in at ~80C, so the hibernate and critical
> set points are set at +5 and +10 of that. In the future
> CPPC should be able to monitor the thermal throttling.
>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Andrei Warkentin <awarkentin@vmware.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
> Platform/RaspberryPi/AcpiTables/Dsdt.asl | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/Platform/RaspberryPi/AcpiTables/Dsdt.asl b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
> index 353af2d876..a5c9567cdf 100644
> --- a/Platform/RaspberryPi/AcpiTables/Dsdt.asl
> +++ b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
> @@ -252,6 +252,37 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN", "RPI", 2)
> }
>
> })
>
> }
>
> +
>
> + // Define a simple thermal zone. The idea here is we compute the SOC temp
>
> + // via a register we can read, and give it to the OS. This enables basic
>
> + // reports from the "sensors" utility, and the OS can then poll and take
>
> + // actions if that temp exceeds any of the given thresholds.
>
> + Device(EC0)
>
> + {
>
> + Name(_HID, EISAID("PNP0C06"))
Space between EISAID and ("PNP0C06")
>
> + Name (_CCA, 0x0)
>
> +
>
> + // all temps in are tenths of K (aka 2732 is the min temps in linux (aka 0C))
I'd use a capital 'L' in "Linux"
>
> + ThermalZone(TZ0) {
Space before '('
>
> + Method(_TMP, 0, Serialized) {
Space before '('
>
> + OperationRegion (TEMS, SystemMemory, 0xfd5d2200, 0x8)
>
> + Field (TEMS, DWordAcc, NoLock, Preserve) {
>
> + TMPS, 32
>
> + }
>
> + return (((419949 - ((TMPS & 0x3ff) * 487)) / 100) + 2732);
>
> + }
>
> + Method(_SCP, 3) { } // receive cooling policy from OS
>
> +
>
> + Method(_CRT) { return(3632) } // (90K) Critical temp point (immediate power-off)
>
> + Method(_HOT) { return(3582) } // (85K) HOT state where OS should hibernate
>
> + Method(_PSV) { return(3532) } // (80K) Passive cooling (CPU throttling) trip point
>
> +
>
> + // SSDT inserts _AC0/_AL0 @60C here, if a FAN is configured
>
> +
>
> + Name(_TZP, 10) //The OSPM must poll this device every 1 seconds
>
> + Name(_PSL, Package(){ \_SB_.CPU0, \_SB_.CPU1, \_SB_.CPU2, \_SB_.CPU3})
6 x Space before '('
>
> + }
>
> + }
>
> #endif
>
>
>
> }
>
Reviewed by: Pete Batard <pete@akeo.ie>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH 2/3] Platform/RaspberryPi4: Create ACPI fan object
2020-08-13 23:00 ` [PATCH 2/3] Platform/RaspberryPi4: Create ACPI fan object Jeremy Linton
@ 2020-08-17 11:10 ` Pete Batard
0 siblings, 0 replies; 10+ messages in thread
From: Pete Batard @ 2020-08-17 11:10 UTC (permalink / raw)
To: devel, jeremy.linton
Cc: Leif Lindholm, Andrei Warkentin, Ard Biesheuvel,
Samer El-Haj-Mahmoud
Whitespace/style and typos:
On 2020.08.14 00:00, Jeremy Linton wrote:
> Now that we have a thermal zone we can add active cooling
> by specifying active cooling points (_ACx) which can
> be tied to fan objects that turn fans on/off using GPIO
> pins.
>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Andrei Warkentin <awarkentin@vmware.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
> .../RaspberryPi/Drivers/ConfigDxe/SsdtThermal.asl | 83 ++++++++++++++++++++++
> 1 file changed, 83 insertions(+)
> create mode 100644 Platform/RaspberryPi/Drivers/ConfigDxe/SsdtThermal.asl
>
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/SsdtThermal.asl b/Platform/RaspberryPi/Drivers/ConfigDxe/SsdtThermal.asl
> new file mode 100644
> index 0000000000..c87bda6dbc
> --- /dev/null
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/SsdtThermal.asl
> @@ -0,0 +1,83 @@
> +/** @file
> + *
> + * Secondary System Description Table (SSDT) for active (fan) cooling
> + *
> + * Copyright (c) 2020, Arm Ltd. All rights reserved.
> + *
> + * SPDX-License-Identifier: BSD-2-Clause-Patent
> + *
> + **/
> +
> +#include <IndustryStandard/Bcm2711.h>
> +#include <IndustryStandard/Bcm2836.h>
> +#include <IndustryStandard/Bcm2836Gpio.h>
> +
> +#include <IndustryStandard/Acpi.h>
> +
> +DefinitionBlock(__FILE__, "SSDT", 5, "RPIFDN", "RPITHFAN", 2)
Space before '('
> +{
> +#if (GPIO_FAN_PIN != 0)
> + External(\_SB_.EC0, DeviceObj)
> + External(\_SB_.EC0.TZ0, DeviceObj)
> +
> + Scope (\_SB_.EC0)
3 x Space before '('
> + {
> + // Describe a fan
> + PowerResource(PFAN, 0, 0) {
Space before '('
> + OperationRegion (GPIO, SystemMemory, GPIO_BASE_ADDRESS, 0x1000)
> + Field (GPIO, DWordAcc, NoLock, Preserve) {
> + Offset(0x1C),
Space before '('
> + GPS0, 32,
> + GPS1, 32,
> + RES1, 32,
> + GPC0, 32,
> + GPC1, 32,
> + RES2, 32,
> + GPL1, 32,
> + GPL2, 32
> + }
> + // We are hitting a GPIO pin to on/off the fan
> + // this assumes that UEFI has programmed the
> + // direction as OUT.
> + // (search "rpi gpio fan controller" for how to
> + // wire this up if your not electrically inclined
"your" -> "you're"
> + // the basic idea is to use a BJT/etc to switch a
> + // larger voltage through a fan where the GPIO pin
> + // feeds a NPN/PNP base. Thats because its unlikly
"Thats" -> "That's", "unlikly" -> "unlikely"
> + // that the fan can be driven directly from the GPIO
> + // pin due to hitting the current limit on the pins.
> + // Matching a resistor between the GPIO->Base can
> + // allow pretty much any random NPN with a reasonable
> + // EC current to work (to limit the GPIO current).)
> + Method (_STA) {
> + if ( GPL1 & (1 << GPIO_FAN_PIN) ) {
No space between '(' and 'GPL1', as well as between the last 2 ')'
> + Return ( 1 ) // present and enabled
No spaces inside the parenthesis
> + }
> + Return ( 0 )
No spaces inside the parenthesis
> + }
> + Method (_ON) { //turn fan on
> + Store((1 << GPIO_FAN_PIN), GPS0)
Space before '('
> + }
> + Method (_OFF) { //turn fan off
> + Store((1 << GPIO_FAN_PIN), GPC0)
Space before '('
> + }
> + }
> + Device(FAN) {
Space before '('
> + // Note, not currently an ACPIv4 fan
> + // the latter adds speed control/detection
> + // but in the case of linux needs FIF, FPS, FSL, and FST
> + Name(_HID, EISAID("PNP0C0B"))
> + Name(_PR0, Package() {PFAN})
2 x Space before '(', space after '{' and before '}'
> + }
> + }
> +
> + // merge in an active cooling point.
> + Scope (\_SB_.EC0.TZ0)
> + {
> + Method(_AC0) { return(3332) } // (60K) active cooling trip point,
Space before '('
> + // if this is lower than PSV then we
> + // prefer active cooling
> + Name(_AL0, Package(){\_SB_.EC0.FAN}) // the fan used for AC0 above
Space before '(' for 'Name' and 'Package', space after () of 'Package',
space after '{' and before '}'
> + }
> +#endif
> +}
>
Reviewed-by: Pete Batard <pete@akeo.ie>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] Platform/RaspberryPi: Add entry for user fan control
2020-08-13 23:00 ` [PATCH 3/3] Platform/RaspberryPi: Add entry for user fan control Jeremy Linton
@ 2020-08-17 11:10 ` Pete Batard
2020-08-17 18:10 ` Jeremy Linton
0 siblings, 1 reply; 10+ messages in thread
From: Pete Batard @ 2020-08-17 11:10 UTC (permalink / raw)
To: Jeremy Linton, devel
Cc: Leif Lindholm, Andrei Warkentin, Ard Biesheuvel,
Samer El-Haj-Mahmoud
More minor style issues:
On 2020.08.14 00:00, Jeremy Linton wrote:
> Add a menu item that allows the user to enable GPIO based
> fan control via SSDT. This should only be seen/enabled on RPI4
> because that is what its been tested with. As of this commit
> its currently limited to only operating on a single GPIO pin (19).
>
> Given GPIO pin current limitations its likely that a bit of
> additional circuitry is required to drive a fan, and the GPIO
> high/low signal can only be used as a enable/disable signal. A
> search for "rpi npn gpio fan" or similar should turn up some
> hits for how to do this simply.
>
> It appears there are a couple boards (fan SHIM) which operate this
> way, and probably should have custom menu items/SSDT edits as
> people acquire the boards and test them.
>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Andrei Warkentin <awarkentin@vmware.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 55 ++++++++++++++++++++++
> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 3 ++
> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni | 5 ++
> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 17 +++++++
> Platform/RaspberryPi/Include/ConfigVars.h | 4 ++
> Platform/RaspberryPi/RPi3/RPi3.dsc | 5 ++
> Platform/RaspberryPi/RPi4/RPi4.dsc | 8 ++++
> Platform/RaspberryPi/RaspberryPi.dec | 1 +
> .../Bcm27xx/Include/IndustryStandard/Bcm2711.h | 2 +
> 9 files changed, 100 insertions(+)
>
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> index af54136ade..f10347be64 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> @@ -15,6 +15,7 @@
> #include <Library/AcpiLib.h>
>
> #include <Library/DebugLib.h>
>
> #include <Library/DevicePathLib.h>
>
> +#include <Library/DxeServicesLib.h>
>
> #include <Library/DxeServicesTableLib.h>
>
> #include <Library/GpioLib.h>
>
> #include <Library/HiiLib.h>
>
> @@ -22,6 +23,7 @@
> #include <Library/NetLib.h>
>
> #include <Library/UefiBootServicesTableLib.h>
>
> #include <Library/UefiRuntimeServicesTableLib.h>
>
> +#include <Protocol/AcpiTable.h>
>
> #include <Protocol/BcmGenetPlatformDevice.h>
>
> #include <Protocol/RpiFirmware.h>
>
> #include <ConfigVars.h>
>
> @@ -246,6 +248,14 @@ SetupVariables (
> ASSERT_EFI_ERROR (Status);
>
> }
>
>
>
> + Size = sizeof (UINT32);
>
> + Status = gRT->GetVariable (L"FanOnGpio",
>
> + &gConfigDxeFormSetGuid,
>
> + NULL, &Size, &Var32);
>
> + if (EFI_ERROR (Status)) {
>
> + PcdSet32 (PcdFanOnGpio, PcdGet32 (PcdFanOnGpio));
>
> + }
>
> +
>
> Size = sizeof(AssetTagVar);
Space before '('
>
>
>
> Status = gRT->GetVariable(L"AssetTag",
Space before '('
>
> @@ -368,6 +378,7 @@ ApplyVariables (
> UINT32 CpuClock = PcdGet32 (PcdCpuClock);
>
> UINT32 CustomCpuClock = PcdGet32 (PcdCustomCpuClock);
>
> UINT32 Rate = 0;
>
> + UINT32 FanOnGpio = PcdGet32 (PcdFanOnGpio);
>
>
>
> switch (CpuClock) {
>
> case CHIPSET_CPU_CLOCK_LOW:
>
> @@ -565,8 +576,49 @@ ApplyVariables (
> GpioPinFuncSet (23, GPIO_FSEL_INPUT);
>
> GpioPinFuncSet (24, GPIO_FSEL_INPUT);
>
> }
>
> +
>
> + if (FanOnGpio) {
>
> + DEBUG ((DEBUG_INFO, "Fan enabled on GPIO %d\n", FanOnGpio));
>
> + GpioPinFuncSet(FanOnGpio, GPIO_FSEL_OUTPUT);
Space before '('
>
> + }
>
> }
>
>
>
> +EFI_STATUS
>
> +FindInstallSsdt(UINT64 OemTableId)
Space before '('
>
> +{
>
> + EFI_ACPI_TABLE_PROTOCOL *AcpiTable;
>
> + UINTN Index;
>
> + EFI_ACPI_DESCRIPTION_HEADER *Ssdt;
>
> + UINTN SsdtSize;
>
> + EFI_STATUS Status;
>
> + UINTN TableKey;
>
> +
>
> +
>
> + Status = gBS->LocateProtocol (&gEfiAcpiTableProtocolGuid, NULL,
>
> + (VOID **)&AcpiTable);
>
> + if (EFI_ERROR (Status)) {
>
> + return Status;
>
> + }
>
> +
>
> + for (Index = 0; !EFI_ERROR(Status); Index++) {
Space before '('
>
> + Status = GetSectionFromFv (&gEfiCallerIdGuid, EFI_SECTION_RAW, Index,
>
> + (VOID **)&Ssdt, &SsdtSize);
>
> + if (Ssdt->OemTableId == OemTableId)
>
> + break;
Indentation should be 2 spaces after 'if'
>
> + SsdtSize = 0;
>
> + }
>
> +
>
> + if (SsdtSize > 0) {
>
> + Status = AcpiTable->InstallAcpiTable (AcpiTable, Ssdt, SsdtSize,
>
> + &TableKey);
>
> + if (EFI_ERROR (Status)) {
>
> + DEBUG ((DEBUG_WARN, "%a: failed to install SSDT table %r\n",
>
> + __FUNCTION__, Status));
>
> + }
>
> + }
>
> +
>
> + return Status;
>
> +}
>
>
>
> EFI_STATUS
>
> EFIAPI
>
> @@ -620,6 +672,9 @@ ConfigInitialize (
> PcdGet32 (PcdSystemTableMode) == SYSTEM_TABLE_MODE_BOTH) {
>
> Status = LocateAndInstallAcpiFromFv (&mAcpiTableFile);
>
> ASSERT_EFI_ERROR (Status);
>
> + if (PcdGet32 (PcdFanOnGpio)) {
>
> + FindInstallSsdt(SIGNATURE_64 ('R', 'P', 'I', 'T', 'H', 'F', 'A', 'N'));
Space before '(' and indentation should be 2 spaces after 'if'
>
> + }
>
> }
>
>
>
> Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_NOTIFY, RegisterDevices,
>
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> index cdce35bc74..fe3a01a570 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> @@ -28,6 +28,7 @@
> ConfigDxeFormSetGuid.h
>
> ConfigDxeHii.vfr
>
> ConfigDxeHii.uni
>
> + SsdtThermal.asl
>
> XhciQuirk.c
>
>
>
> [Packages]
>
> @@ -46,6 +47,7 @@
> AcpiLib
>
> BaseLib
>
> DebugLib
>
> + DxeServicesLib
>
> DxeServicesTableLib
>
> GpioLib
>
> HiiLib
>
> @@ -89,6 +91,7 @@
> gRaspberryPiTokenSpaceGuid.PcdSystemTableMode
>
> gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB
>
> gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB
>
> + gRaspberryPiTokenSpaceGuid.PcdFanOnGpio
>
>
>
> [Depex]
>
> gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
>
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> index 03763710a1..491d022fff 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> @@ -48,6 +48,11 @@
> #string STR_ADVANCED_SYSTAB_BOTH #language en-US "ACPI + Devicetree"
>
> #string STR_ADVANCED_SYSTAB_DT #language en-US "Devicetree"
>
>
>
> +#string STR_ADVANCED_FANONGPIO_PROMPT #language en-US "ACPI fan control"
>
> +#string STR_ADVANCED_FANONGPIO_HELP #language en-US "Cycle a fan via GPIO-19 if temp exceeds 60C"
>
> +#string STR_ADVANCED_FANONGPIO_OFF #language en-US "Disabled"
>
> +#string STR_ADVANCED_FANONGPIO_ON #language en-US "Enabled"
>
> +
>
> #string STR_ADVANCED_ASSET_TAG_PROMPT #language en-US "Asset Tag"
>
> #string STR_ADVANCED_ASSET_TAG_HELP #language en-US "Set the system Asset Tag"
>
>
>
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> index d5615d7af0..0a5e4163e8 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> @@ -10,6 +10,7 @@
> #include <Guid/HiiPlatformSetupFormset.h>
>
> #include "ConfigDxeFormSetGuid.h"
>
> #include <ConfigVars.h>
>
> +#include <IndustryStandard/Bcm2711.h>
>
>
>
> //
>
> // EFI Variable attributes
>
> @@ -45,6 +46,11 @@ formset
> name = RamLimitTo3GB,
>
> guid = CONFIGDXE_FORM_SET_GUID;
>
>
>
> + efivarstore ADVANCED_FAN_ON_GPIO_VARSTORE_DATA,
>
> + attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
>
> + name = FanOnGpio,
>
> + guid = CONFIGDXE_FORM_SET_GUID;
>
> +
>
> efivarstore SYSTEM_TABLE_MODE_VARSTORE_DATA,
>
> attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
>
> name = SystemTableMode,
>
> @@ -174,6 +180,17 @@ formset
> option text = STRING_TOKEN(STR_ADVANCED_SYSTAB_DT), value = SYSTEM_TABLE_MODE_DT, flags = DEFAULT;
>
> endoneof;
>
>
>
> +#if (RPI_MODEL == 4)
>
> + grayoutif NOT ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_ACPI;
>
> + oneof varid = FanOnGpio.Enabled,
>
> + prompt = STRING_TOKEN(STR_ADVANCED_FANONGPIO_PROMPT),
>
> + help = STRING_TOKEN(STR_ADVANCED_FANONGPIO_HELP),
>
> + flags = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
>
> + option text = STRING_TOKEN(STR_ADVANCED_FANONGPIO_OFF), value = 0, flags = DEFAULT;
>
> + option text = STRING_TOKEN(STR_ADVANCED_FANONGPIO_ON), value = GPIO_FAN_PIN, flags = 0;
>
> + endoneof;
>
> + endif;
>
> +#endif
>
> string varid = AssetTag.AssetTag,
>
> prompt = STRING_TOKEN(STR_ADVANCED_ASSET_TAG_PROMPT),
>
> help = STRING_TOKEN(STR_ADVANCED_ASSET_TAG_HELP),
>
> diff --git a/Platform/RaspberryPi/Include/ConfigVars.h b/Platform/RaspberryPi/Include/ConfigVars.h
> index b1689b004d..1a40469bfa 100644
> --- a/Platform/RaspberryPi/Include/ConfigVars.h
> +++ b/Platform/RaspberryPi/Include/ConfigVars.h
> @@ -69,6 +69,10 @@ typedef struct {
> } ADVANCED_RAM_LIMIT_TO_3GB_VARSTORE_DATA;
>
>
>
> typedef struct {
>
> + UINT32 Enabled;
>
> +} ADVANCED_FAN_ON_GPIO_VARSTORE_DATA;
>
> +
>
> +typedef struct {
>
> #define SYSTEM_TABLE_MODE_ACPI 0
>
> #define SYSTEM_TABLE_MODE_BOTH 1
>
> #define SYSTEM_TABLE_MODE_DT 2
>
> diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
> index 0998d8366c..cef8932ca2 100644
> --- a/Platform/RaspberryPi/RPi3/RPi3.dsc
> +++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
> @@ -499,6 +499,11 @@
> gRaspberryPiTokenSpaceGuid.PcdSystemTableMode|L"SystemTableMode"|gConfigDxeFormSetGuid|0x0|1
>
>
>
> #
>
> + # Enable a fan in the ACPI thermal zone on GPIO pin #
>
> + #
>
> + gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFormSetGuid|0x0|0
>
> +
>
> + #
>
> # Common UEFI ones.
>
> #
>
>
>
> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
> index baa7e63483..9d0eaf10a1 100644
> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc
> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
> @@ -510,6 +510,14 @@
> gRaspberryPiTokenSpaceGuid.PcdSystemTableMode|L"SystemTableMode"|gConfigDxeFormSetGuid|0x0|0
>
>
>
> #
>
> + # Enable a fan in the ACPI thermal zone on GPIO pin #
>
> + #
>
> + # 0 - DISABLED
>
> + # 19 - Enabled on pin 19
>
> + #
>
> + gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFormSetGuid|0x0|0
>
> +
>
> + #
>
> # Common UEFI ones.
>
> #
>
>
>
> diff --git a/Platform/RaspberryPi/RaspberryPi.dec b/Platform/RaspberryPi/RaspberryPi.dec
> index c71177a2f7..a73650f2c3 100644
> --- a/Platform/RaspberryPi/RaspberryPi.dec
> +++ b/Platform/RaspberryPi/RaspberryPi.dec
> @@ -66,3 +66,4 @@
> gRaspberryPiTokenSpaceGuid.PcdSystemTableMode|1|UINT32|0x0000001B
>
> gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB|0|UINT32|0x00000019
>
> gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|0|UINT32|0x0000001A
>
> + gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|0|UINT32|0x0000001C
>
> diff --git a/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h b/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
> index e9c81cafa1..7d9ea5d35c 100644
> --- a/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
> +++ b/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
> @@ -86,4 +86,6 @@
> #define GENET_BASE_ADDRESS FixedPcdGet64 (PcdBcmGenetRegistersAddress)
>
> #define GENET_LENGTH 0x00010000
>
>
>
> +#define GPIO_FAN_PIN 19 // fan shim uses GPIO 18
>
> +
>
> #endif /* BCM2711_H__ */
>
Reviewed-by: Pete Batard <pete@akeo.ie>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] Platform/RasberryPi: Thermal zone
2020-08-13 23:00 [PATCH 0/3] Platform/RasberryPi: Thermal zone Jeremy Linton
` (2 preceding siblings ...)
2020-08-13 23:00 ` [PATCH 3/3] Platform/RaspberryPi: Add entry for user fan control Jeremy Linton
@ 2020-08-17 14:31 ` Ard Biesheuvel
2020-08-17 19:03 ` [edk2-devel] " Jeremy Linton
3 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2020-08-17 14:31 UTC (permalink / raw)
To: Jeremy Linton, devel
Cc: Leif Lindholm, Pete Batard, Andrei Warkentin,
Samer El-Haj-Mahmoud
On 8/14/20 1:00 AM, Jeremy Linton wrote:
> This set creates a basic thermal zone, which reads the
> SOC temp via a direct register read in AML. It also
> adds an active cooling policy using a GPIO pin for fan
> control that can optionally be enabled/disabled by the
> user from the BDS.
>
> With the fan enabled it should be possible to see the
> soc temp like:
>
> # sensors
> acpitz-acpi-0
> Adapter: ACPI interface
> temp1: +57.6C (crit = +90.0C)
>
> and the fan state may be read/cycled with:
>
> /sys/bus/acpi/devices/PNP0C06:00/PNP0C0B:00/physical_node/thermal_cooling/cur_state
>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Andrei Warkentin <awarkentin@vmware.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
>
> Jeremy Linton (3):
> Platform/RaspberryPi4: Add a basic thermal zone
> Platform/RaspberryPi4: Create ACPI fan object
> Platform/RaspberryPi: Add entry for user fan control
>
I like this code a lot. It is very helpful to have working sample AML
code that implements a thermal zone. Could you elaborate on the
additional components that are needed for this? Is this a standard cape
(or whatever rpi calls it)? I assume the fan just switches between 0 and
max rpm depending on the actual temp wrt the trip point?
> Platform/RaspberryPi/AcpiTables/Dsdt.asl | 31 ++++++++
> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 55 ++++++++++++++
> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 3 +
> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni | 5 ++
> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 17 +++++
> .../RaspberryPi/Drivers/ConfigDxe/SsdtThermal.asl | 83 ++++++++++++++++++++++
> Platform/RaspberryPi/Include/ConfigVars.h | 4 ++
> Platform/RaspberryPi/RPi3/RPi3.dsc | 5 ++
> Platform/RaspberryPi/RPi4/RPi4.dsc | 8 +++
> Platform/RaspberryPi/RaspberryPi.dec | 1 +
> .../Bcm27xx/Include/IndustryStandard/Bcm2711.h | 2 +
> 11 files changed, 214 insertions(+)
> create mode 100644 Platform/RaspberryPi/Drivers/ConfigDxe/SsdtThermal.asl
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] Platform/RaspberryPi: Add entry for user fan control
2020-08-17 11:10 ` Pete Batard
@ 2020-08-17 18:10 ` Jeremy Linton
0 siblings, 0 replies; 10+ messages in thread
From: Jeremy Linton @ 2020-08-17 18:10 UTC (permalink / raw)
To: Pete Batard, devel
Cc: Leif Lindholm, Andrei Warkentin, Ard Biesheuvel,
Samer El-Haj-Mahmoud
Hi,
Thanks for taking a look at this, I will roll another version with the
suggested changes. It seems my space '(' rules are broken :)
On 8/17/20 6:10 AM, Pete Batard wrote:
> More minor style issues:
>
> On 2020.08.14 00:00, Jeremy Linton wrote:
>> Add a menu item that allows the user to enable GPIO based
>> fan control via SSDT. This should only be seen/enabled on RPI4
>> because that is what its been tested with. As of this commit
>> its currently limited to only operating on a single GPIO pin (19).
>>
>> Given GPIO pin current limitations its likely that a bit of
>> additional circuitry is required to drive a fan, and the GPIO
>> high/low signal can only be used as a enable/disable signal. A
>> search for "rpi npn gpio fan" or similar should turn up some
>> hits for how to do this simply.
>>
>> It appears there are a couple boards (fan SHIM) which operate this
>> way, and probably should have custom menu items/SSDT edits as
>> people acquire the boards and test them.
>>
>> Cc: Leif Lindholm <leif@nuviainc.com>
>> Cc: Pete Batard <pete@akeo.ie>
>> Cc: Andrei Warkentin <awarkentin@vmware.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>> ---
>> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 55
>> ++++++++++++++++++++++
>> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 3 ++
>> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni | 5 ++
>> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 17 +++++++
>> Platform/RaspberryPi/Include/ConfigVars.h | 4 ++
>> Platform/RaspberryPi/RPi3/RPi3.dsc | 5 ++
>> Platform/RaspberryPi/RPi4/RPi4.dsc | 8 ++++
>> Platform/RaspberryPi/RaspberryPi.dec | 1 +
>> .../Bcm27xx/Include/IndustryStandard/Bcm2711.h | 2 +
>> 9 files changed, 100 insertions(+)
>>
>> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
>> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
>> index af54136ade..f10347be64 100644
>> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
>> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
>> @@ -15,6 +15,7 @@
>> #include <Library/AcpiLib.h>
>>
>> #include <Library/DebugLib.h>
>>
>> #include <Library/DevicePathLib.h>
>>
>> +#include <Library/DxeServicesLib.h>
>>
>> #include <Library/DxeServicesTableLib.h>
>>
>> #include <Library/GpioLib.h>
>>
>> #include <Library/HiiLib.h>
>>
>> @@ -22,6 +23,7 @@
>> #include <Library/NetLib.h>
>>
>> #include <Library/UefiBootServicesTableLib.h>
>>
>> #include <Library/UefiRuntimeServicesTableLib.h>
>>
>> +#include <Protocol/AcpiTable.h>
>>
>> #include <Protocol/BcmGenetPlatformDevice.h>
>>
>> #include <Protocol/RpiFirmware.h>
>>
>> #include <ConfigVars.h>
>>
>> @@ -246,6 +248,14 @@ SetupVariables (
>> ASSERT_EFI_ERROR (Status);
>>
>> }
>>
>>
>> + Size = sizeof (UINT32);
>>
>> + Status = gRT->GetVariable (L"FanOnGpio",
>>
>> + &gConfigDxeFormSetGuid,
>>
>> + NULL, &Size, &Var32);
>>
>> + if (EFI_ERROR (Status)) {
>>
>> + PcdSet32 (PcdFanOnGpio, PcdGet32 (PcdFanOnGpio));
>>
>> + }
>>
>> +
>>
>> Size = sizeof(AssetTagVar);
>
> Space before '('
>
>>
>>
>> Status = gRT->GetVariable(L"AssetTag",
>
> Space before '('
>
>>
>> @@ -368,6 +378,7 @@ ApplyVariables (
>> UINT32 CpuClock = PcdGet32 (PcdCpuClock);
>>
>> UINT32 CustomCpuClock = PcdGet32 (PcdCustomCpuClock);
>>
>> UINT32 Rate = 0;
>>
>> + UINT32 FanOnGpio = PcdGet32 (PcdFanOnGpio);
>>
>>
>> switch (CpuClock) {
>>
>> case CHIPSET_CPU_CLOCK_LOW:
>>
>> @@ -565,8 +576,49 @@ ApplyVariables (
>> GpioPinFuncSet (23, GPIO_FSEL_INPUT);
>>
>> GpioPinFuncSet (24, GPIO_FSEL_INPUT);
>>
>> }
>>
>> +
>>
>> + if (FanOnGpio) {
>>
>> + DEBUG ((DEBUG_INFO, "Fan enabled on GPIO %d\n", FanOnGpio));
>>
>> + GpioPinFuncSet(FanOnGpio, GPIO_FSEL_OUTPUT);
>
> Space before '('
>>
>> + }
>>
>> }
>>
>>
>> +EFI_STATUS
>>
>> +FindInstallSsdt(UINT64 OemTableId)
>
> Space before '('
>
>>
>> +{
>>
>> + EFI_ACPI_TABLE_PROTOCOL *AcpiTable;
>>
>> + UINTN Index;
>>
>> + EFI_ACPI_DESCRIPTION_HEADER *Ssdt;
>>
>> + UINTN SsdtSize;
>>
>> + EFI_STATUS Status;
>>
>> + UINTN TableKey;
>>
>> +
>>
>> +
>>
>> + Status = gBS->LocateProtocol (&gEfiAcpiTableProtocolGuid, NULL,
>>
>> + (VOID **)&AcpiTable);
>>
>> + if (EFI_ERROR (Status)) {
>>
>> + return Status;
>>
>> + }
>>
>> +
>>
>> + for (Index = 0; !EFI_ERROR(Status); Index++) {
>
> Space before '('
>
>>
>> + Status = GetSectionFromFv (&gEfiCallerIdGuid, EFI_SECTION_RAW,
>> Index,
>>
>> + (VOID **)&Ssdt, &SsdtSize);
>>
>> + if (Ssdt->OemTableId == OemTableId)
>>
>> + break;
>
> Indentation should be 2 spaces after 'if'
>
>>
>> + SsdtSize = 0;
>>
>> + }
>>
>> +
>>
>> + if (SsdtSize > 0) {
>>
>> + Status = AcpiTable->InstallAcpiTable (AcpiTable, Ssdt, SsdtSize,
>>
>> + &TableKey);
>>
>> + if (EFI_ERROR (Status)) {
>>
>> + DEBUG ((DEBUG_WARN, "%a: failed to install SSDT table %r\n",
>>
>> + __FUNCTION__, Status));
>>
>> + }
>>
>> + }
>>
>> +
>>
>> + return Status;
>>
>> +}
>>
>>
>> EFI_STATUS
>>
>> EFIAPI
>>
>> @@ -620,6 +672,9 @@ ConfigInitialize (
>> PcdGet32 (PcdSystemTableMode) == SYSTEM_TABLE_MODE_BOTH) {
>>
>> Status = LocateAndInstallAcpiFromFv (&mAcpiTableFile);
>>
>> ASSERT_EFI_ERROR (Status);
>>
>> + if (PcdGet32 (PcdFanOnGpio)) {
>>
>> + FindInstallSsdt(SIGNATURE_64 ('R', 'P', 'I', 'T', 'H', 'F',
>> 'A', 'N'));
>
> Space before '(' and indentation should be 2 spaces after 'if'
>
>>
>> + }
>>
>> }
>>
>>
>> Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_NOTIFY,
>> RegisterDevices,
>>
>> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
>> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
>> index cdce35bc74..fe3a01a570 100644
>> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
>> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
>> @@ -28,6 +28,7 @@
>> ConfigDxeFormSetGuid.h
>>
>> ConfigDxeHii.vfr
>>
>> ConfigDxeHii.uni
>>
>> + SsdtThermal.asl
>>
>> XhciQuirk.c
>>
>>
>> [Packages]
>>
>> @@ -46,6 +47,7 @@
>> AcpiLib
>>
>> BaseLib
>>
>> DebugLib
>>
>> + DxeServicesLib
>>
>> DxeServicesTableLib
>>
>> GpioLib
>>
>> HiiLib
>>
>> @@ -89,6 +91,7 @@
>> gRaspberryPiTokenSpaceGuid.PcdSystemTableMode
>>
>> gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB
>>
>> gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB
>>
>> + gRaspberryPiTokenSpaceGuid.PcdFanOnGpio
>>
>>
>> [Depex]
>>
>> gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
>>
>> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
>> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
>> index 03763710a1..491d022fff 100644
>> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
>> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
>> @@ -48,6 +48,11 @@
>> #string STR_ADVANCED_SYSTAB_BOTH #language en-US "ACPI +
>> Devicetree"
>>
>> #string STR_ADVANCED_SYSTAB_DT #language en-US "Devicetree"
>>
>>
>> +#string STR_ADVANCED_FANONGPIO_PROMPT #language en-US "ACPI fan control"
>>
>> +#string STR_ADVANCED_FANONGPIO_HELP #language en-US "Cycle a fan
>> via GPIO-19 if temp exceeds 60C"
>>
>> +#string STR_ADVANCED_FANONGPIO_OFF #language en-US "Disabled"
>>
>> +#string STR_ADVANCED_FANONGPIO_ON #language en-US "Enabled"
>>
>> +
>>
>> #string STR_ADVANCED_ASSET_TAG_PROMPT #language en-US "Asset Tag"
>>
>> #string STR_ADVANCED_ASSET_TAG_HELP #language en-US "Set the
>> system Asset Tag"
>>
>>
>> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
>> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
>> index d5615d7af0..0a5e4163e8 100644
>> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
>> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
>> @@ -10,6 +10,7 @@
>> #include <Guid/HiiPlatformSetupFormset.h>
>>
>> #include "ConfigDxeFormSetGuid.h"
>>
>> #include <ConfigVars.h>
>>
>> +#include <IndustryStandard/Bcm2711.h>
>>
>>
>> //
>>
>> // EFI Variable attributes
>>
>> @@ -45,6 +46,11 @@ formset
>> name = RamLimitTo3GB,
>>
>> guid = CONFIGDXE_FORM_SET_GUID;
>>
>>
>> + efivarstore ADVANCED_FAN_ON_GPIO_VARSTORE_DATA,
>>
>> + attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS |
>> EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
>>
>> + name = FanOnGpio,
>>
>> + guid = CONFIGDXE_FORM_SET_GUID;
>>
>> +
>>
>> efivarstore SYSTEM_TABLE_MODE_VARSTORE_DATA,
>>
>> attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS |
>> EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
>>
>> name = SystemTableMode,
>>
>> @@ -174,6 +180,17 @@ formset
>> option text = STRING_TOKEN(STR_ADVANCED_SYSTAB_DT),
>> value = SYSTEM_TABLE_MODE_DT, flags = DEFAULT;
>>
>> endoneof;
>>
>>
>> +#if (RPI_MODEL == 4)
>>
>> + grayoutif NOT ideqval SystemTableMode.Mode ==
>> SYSTEM_TABLE_MODE_ACPI;
>>
>> + oneof varid = FanOnGpio.Enabled,
>>
>> + prompt = STRING_TOKEN(STR_ADVANCED_FANONGPIO_PROMPT),
>>
>> + help = STRING_TOKEN(STR_ADVANCED_FANONGPIO_HELP),
>>
>> + flags = NUMERIC_SIZE_4 | INTERACTIVE |
>> RESET_REQUIRED,
>>
>> + option text = STRING_TOKEN(STR_ADVANCED_FANONGPIO_OFF),
>> value = 0, flags = DEFAULT;
>>
>> + option text = STRING_TOKEN(STR_ADVANCED_FANONGPIO_ON),
>> value = GPIO_FAN_PIN, flags = 0;
>>
>> + endoneof;
>>
>> + endif;
>>
>> +#endif
>>
>> string varid = AssetTag.AssetTag,
>>
>> prompt = STRING_TOKEN(STR_ADVANCED_ASSET_TAG_PROMPT),
>>
>> help = STRING_TOKEN(STR_ADVANCED_ASSET_TAG_HELP),
>>
>> diff --git a/Platform/RaspberryPi/Include/ConfigVars.h
>> b/Platform/RaspberryPi/Include/ConfigVars.h
>> index b1689b004d..1a40469bfa 100644
>> --- a/Platform/RaspberryPi/Include/ConfigVars.h
>> +++ b/Platform/RaspberryPi/Include/ConfigVars.h
>> @@ -69,6 +69,10 @@ typedef struct {
>> } ADVANCED_RAM_LIMIT_TO_3GB_VARSTORE_DATA;
>>
>>
>> typedef struct {
>>
>> + UINT32 Enabled;
>>
>> +} ADVANCED_FAN_ON_GPIO_VARSTORE_DATA;
>>
>> +
>>
>> +typedef struct {
>>
>> #define SYSTEM_TABLE_MODE_ACPI 0
>>
>> #define SYSTEM_TABLE_MODE_BOTH 1
>>
>> #define SYSTEM_TABLE_MODE_DT 2
>>
>> diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc
>> b/Platform/RaspberryPi/RPi3/RPi3.dsc
>> index 0998d8366c..cef8932ca2 100644
>> --- a/Platform/RaspberryPi/RPi3/RPi3.dsc
>> +++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
>> @@ -499,6 +499,11 @@
>>
>> gRaspberryPiTokenSpaceGuid.PcdSystemTableMode|L"SystemTableMode"|gConfigDxeFormSetGuid|0x0|1
>>
>>
>>
>> #
>>
>> + # Enable a fan in the ACPI thermal zone on GPIO pin #
>>
>> + #
>>
>> +
>> gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFormSetGuid|0x0|0
>>
>>
>> +
>>
>> + #
>>
>> # Common UEFI ones.
>>
>> #
>>
>>
>> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc
>> b/Platform/RaspberryPi/RPi4/RPi4.dsc
>> index baa7e63483..9d0eaf10a1 100644
>> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc
>> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
>> @@ -510,6 +510,14 @@
>>
>> gRaspberryPiTokenSpaceGuid.PcdSystemTableMode|L"SystemTableMode"|gConfigDxeFormSetGuid|0x0|0
>>
>>
>>
>> #
>>
>> + # Enable a fan in the ACPI thermal zone on GPIO pin #
>>
>> + #
>>
>> + # 0 - DISABLED
>>
>> + # 19 - Enabled on pin 19
>>
>> + #
>>
>> +
>> gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFormSetGuid|0x0|0
>>
>>
>> +
>>
>> + #
>>
>> # Common UEFI ones.
>>
>> #
>>
>>
>> diff --git a/Platform/RaspberryPi/RaspberryPi.dec
>> b/Platform/RaspberryPi/RaspberryPi.dec
>> index c71177a2f7..a73650f2c3 100644
>> --- a/Platform/RaspberryPi/RaspberryPi.dec
>> +++ b/Platform/RaspberryPi/RaspberryPi.dec
>> @@ -66,3 +66,4 @@
>> gRaspberryPiTokenSpaceGuid.PcdSystemTableMode|1|UINT32|0x0000001B
>>
>> gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB|0|UINT32|0x00000019
>>
>> gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|0|UINT32|0x0000001A
>>
>> + gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|0|UINT32|0x0000001C
>>
>> diff --git
>> a/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
>> b/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
>> index e9c81cafa1..7d9ea5d35c 100644
>> --- a/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
>> +++ b/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
>> @@ -86,4 +86,6 @@
>> #define GENET_BASE_ADDRESS FixedPcdGet64
>> (PcdBcmGenetRegistersAddress)
>>
>> #define GENET_LENGTH 0x00010000
>>
>>
>> +#define GPIO_FAN_PIN 19 // fan shim uses GPIO 18
>>
>> +
>>
>> #endif /* BCM2711_H__ */
>>
>
> Reviewed-by: Pete Batard <pete@akeo.ie>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH 0/3] Platform/RasberryPi: Thermal zone
2020-08-17 14:31 ` [PATCH 0/3] Platform/RasberryPi: Thermal zone Ard Biesheuvel
@ 2020-08-17 19:03 ` Jeremy Linton
0 siblings, 0 replies; 10+ messages in thread
From: Jeremy Linton @ 2020-08-17 19:03 UTC (permalink / raw)
To: devel, ard.biesheuvel
Cc: Leif Lindholm, Pete Batard, Andrei Warkentin,
Samer El-Haj-Mahmoud
Hi,
On 8/17/20 9:31 AM, Ard Biesheuvel via groups.io wrote:
> On 8/14/20 1:00 AM, Jeremy Linton wrote:
>> This set creates a basic thermal zone, which reads the
>> SOC temp via a direct register read in AML. It also
>> adds an active cooling policy using a GPIO pin for fan
>> control that can optionally be enabled/disabled by the
>> user from the BDS.
>>
>> With the fan enabled it should be possible to see the
>> soc temp like:
^ That should have read something like:
"Even without the fan enabled it is possible to see the SOC temp like:"
>>
>> # sensors
>> acpitz-acpi-0
>> Adapter: ACPI interface
>> temp1: +57.6C (crit = +90.0C)
>>
>> and the fan state may be read/cycled with:
>>
>> /sys/bus/acpi/devices/PNP0C06:00/PNP0C0B:00/physical_node/thermal_cooling/cur_state
>>
>>
>> Cc: Leif Lindholm <leif@nuviainc.com>
>> Cc: Pete Batard <pete@akeo.ie>
>> Cc: Andrei Warkentin <awarkentin@vmware.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
>>
>> Jeremy Linton (3):
>> Platform/RaspberryPi4: Add a basic thermal zone
>> Platform/RaspberryPi4: Create ACPI fan object
>> Platform/RaspberryPi: Add entry for user fan control
>>
>
> I like this code a lot. It is very helpful to have working sample AML
> code that implements a thermal zone. Could you elaborate on the
> additional components that are needed for this? Is this a standard cape
> (or whatever rpi calls it)? I assume the fan just switches between 0 and
> max rpm depending on the actual temp wrt the trip point?
I've got something similar to this circuit:
https://www.raspberrypi.org/forums/viewtopic.php?t=194621#p1220502 wired
up. There are a number of variations on the web
(https://www.instructables.com/id/PWM-Regulated-Fan-Based-on-CPU-Temperature-for-Ras/),
frequently including python control scripts, which are unnecessary given
this AML/patch. Most of the variation is simply matching an appropriate
resistor between the GPIO and transistor base for the given transistor's
gain.
A board which implements a similar circuit can be purchased here:
https://shop.pimoroni.com/products/fan-shim
Most of these circuits are designed for simple On/Off control. So as it
stands, the kernel calls the ON() method, when it polls the zone temp,
and discovers that it exceeds the active cooling threshold. Or the OFF()
if the temp falls below. The slow polling and large heatsink on my rpi
tends to keep it from excessive hunting/cycling since the kernel doesn't
implement much in the way of hysteresis control.
The GPIO pin I selected also (AFAIK) has a PWM function that could be
leveraged in the future for variable speed control. That said, most of
these little 5V fans seem to be basically silent (well the ones I have,
or they are 12V already running at low voltage), so there is little
advantage to slowing them down.
The big variation is which GPIO controls the fan. This patch at the
moment has a #define setting the pin, but the general plan was to add
some additional user controllable options for board/GPIO pin selection.
Originally I was planning on just compiling the ASL multiple times for
each GPIO and then picking one at runtime, but now I'm thinking
dynamically editing the binary AML before calling InstallTable() on it
might be a better choice to reduce bloat.
A few of the fan boards/cases implement their own fan controllers with
programmable thermal profiles over I2C. This patch won't support those
boards. (yet! :)
>
>
>
>> Platform/RaspberryPi/AcpiTables/Dsdt.asl | 31 ++++++++
>> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 55 ++++++++++++++
>> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 3 +
>> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni | 5 ++
>> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 17 +++++
>> .../RaspberryPi/Drivers/ConfigDxe/SsdtThermal.asl | 83
>> ++++++++++++++++++++++
>> Platform/RaspberryPi/Include/ConfigVars.h | 4 ++
>> Platform/RaspberryPi/RPi3/RPi3.dsc | 5 ++
>> Platform/RaspberryPi/RPi4/RPi4.dsc | 8 +++
>> Platform/RaspberryPi/RaspberryPi.dec | 1 +
>> .../Bcm27xx/Include/IndustryStandard/Bcm2711.h | 2 +
>> 11 files changed, 214 insertions(+)
>> create mode 100644
>> Platform/RaspberryPi/Drivers/ConfigDxe/SsdtThermal.asl
>>
>
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-08-17 19:03 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-13 23:00 [PATCH 0/3] Platform/RasberryPi: Thermal zone Jeremy Linton
2020-08-13 23:00 ` [PATCH 1/3] Platform/RaspberryPi4: Add a basic thermal zone Jeremy Linton
2020-08-17 11:10 ` Pete Batard
2020-08-13 23:00 ` [PATCH 2/3] Platform/RaspberryPi4: Create ACPI fan object Jeremy Linton
2020-08-17 11:10 ` [edk2-devel] " Pete Batard
2020-08-13 23:00 ` [PATCH 3/3] Platform/RaspberryPi: Add entry for user fan control Jeremy Linton
2020-08-17 11:10 ` Pete Batard
2020-08-17 18:10 ` Jeremy Linton
2020-08-17 14:31 ` [PATCH 0/3] Platform/RasberryPi: Thermal zone Ard Biesheuvel
2020-08-17 19:03 ` [edk2-devel] " Jeremy Linton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox