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