From: "Andrei Warkentin" <awarkentin@vmware.com>
To: Jeremy Linton <jeremy.linton@arm.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: Leif Lindholm <leif@nuviainc.com>, Pete Batard <pete@akeo.ie>,
Ard Biesheuvel <ard.biesheuvel@arm.com>,
Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Subject: Re: [PATCH v2 3/4] Platform/RaspberryPi: Add entry for user fan control
Date: Thu, 20 Aug 2020 16:15:43 +0000 [thread overview]
Message-ID: <BN6PR05MB341184F799120CC1E644F0A1B95A0@BN6PR05MB3411.namprd05.prod.outlook.com> (raw)
In-Reply-To: <20200820044217.39839-4-jeremy.linton@arm.com>
[-- Attachment #1: Type: text/plain, Size: 13064 bytes --]
Hi Jeremy,
Overall, great idea. An improvement on your implementation would be to use LocateAndInstallAcpiFromFvConditional (which will call your call back on every table, so you can allow/disallow/patch) instead of calling LocateAndInstallAcpiFromFv and the locating the SSDT manually.
A
________________________________
From: Jeremy Linton <jeremy.linton@arm.com>
Sent: Wednesday, August 19, 2020 11:42 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Jeremy Linton <jeremy.linton@arm.com>; Leif Lindholm <leif@nuviainc.com>; Pete Batard <pete@akeo.ie>; Andrei Warkentin <awarkentin@vmware.com>; Ard Biesheuvel <ard.biesheuvel@arm.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Subject: [PATCH v2 3/4] Platform/RaspberryPi: Add entry for user fan control
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>
Reviewed-by: Pete Batard <@pbatard>
---
Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 78 ++++++++++++++++++++++
.../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 3 +
.../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni | 6 ++
.../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 18 +++++
Platform/RaspberryPi/Include/ConfigVars.h | 4 ++
Platform/RaspberryPi/RPi3/RPi3.dsc | 5 ++
Platform/RaspberryPi/RPi4/RPi4.dsc | 8 +++
Platform/RaspberryPi/RaspberryPi.dec | 1 +
8 files changed, 123 insertions(+)
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index af54136ade..24e65eeb5e 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,71 @@ 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);
+ }
}
+#define SSDT_PATTERN_LEN 6
+
+EFI_STATUS
+FindInstallSsdt (UINT64 OemTableId, CHAR8 *Name, UINT8 Value)
+{
+ EFI_ACPI_TABLE_PROTOCOL *AcpiTable;
+ UINTN Index;
+ EFI_ACPI_DESCRIPTION_HEADER *Ssdt;
+ UINTN SsdtSize;
+ EFI_STATUS Status;
+ UINTN TableKey;
+ CHAR8 Pattern[SSDT_PATTERN_LEN];
+
+
+ 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) {
+ /*
+ * Do a single 8 bit NameOp variable replacement these are of the
+ * form 08 XXXX SIZE VAL, where SIZE is 0A=byte, 0B=word, 0C=dword
+ * and XXXX is the name and VAL is the value
+ */
+ Pattern[0] = 0x08; //NameOp
+ Pattern[1] = Name[0]; //Name
+ Pattern[2] = Name[1];
+ Pattern[3] = Name[2];
+ Pattern[4] = Name[3];
+ Pattern[5] = 0x0A; //Size
+
+ for (Index = 0; Index < (SsdtSize - SSDT_PATTERN_LEN); Index++) {
+ if (CompareMem ((UINT8*)Ssdt + Index, Pattern, SSDT_PATTERN_LEN) == 0) {
+ *((UINT8 *)Ssdt + Index + SSDT_PATTERN_LEN) = Value;
+ break;
+ }
+ }
+
+ 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 +694,10 @@ 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'),
+ "GIOP", (UINT8)PcdGet32 (PcdFanOnGpio));
+ }
}
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..e2d1bb4b39 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
@@ -48,6 +48,12 @@
#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 if temp exceeds 60C"
+#string STR_ADVANCED_FANONGPIO_OFF #language en-US "Disabled"
+#string STR_ADVANCED_FANONGPIO_18 #language en-US "Fan Shim/GPIO-18"
+#string STR_ADVANCED_FANONGPIO_19 #language en-US "GPIO-19"
+
#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..94332caab3 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,18 @@ 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_18), value = 18, flags = 0;
+ option text = STRING_TOKEN(STR_ADVANCED_FANONGPIO_19), value = 19, 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
--
2.13.7
[-- Attachment #2: Type: text/html, Size: 21590 bytes --]
next prev parent reply other threads:[~2020-08-20 16:15 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-20 4:42 [PATCH v2 0/4] Platform/RasberryPi: Thermal zone Jeremy Linton
2020-08-20 4:42 ` [PATCH v2 1/4] Platform/RaspberryPi4: Add a basic thermal zone Jeremy Linton
2020-08-20 7:59 ` Ard Biesheuvel
2020-08-20 14:41 ` Jeremy Linton
2020-08-27 8:06 ` Ard Biesheuvel
2020-08-27 15:21 ` Jeremy Linton
2020-08-20 4:42 ` [PATCH v2 2/4] Platform/RaspberryPi4: Create ACPI fan object Jeremy Linton
2020-08-20 4:42 ` [PATCH v2 3/4] Platform/RaspberryPi: Add entry for user fan control Jeremy Linton
2020-08-20 16:15 ` Andrei Warkentin [this message]
2020-08-21 4:15 ` Jeremy Linton
2020-08-20 4:42 ` [PATCH v2 4/4] Platform/RaspberryPi: Trivial whitespace cleanup Jeremy Linton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BN6PR05MB341184F799120CC1E644F0A1B95A0@BN6PR05MB3411.namprd05.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox