From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web12.55360.1598886805128688248 for ; Mon, 31 Aug 2020 08:13:25 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: jeremy.linton@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id AD44E1FB; Mon, 31 Aug 2020 08:13:24 -0700 (PDT) Received: from [192.168.122.166] (unknown [10.119.48.15]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 6021E3F68F; Mon, 31 Aug 2020 08:13:24 -0700 (PDT) Subject: Re: [PATCH v3 1/5] Platform/RaspberryPi4: Add a basic thermal zone To: Pete Batard , devel@edk2.groups.io Cc: Leif Lindholm , Andrei Warkentin , Ard Biesheuvel , Samer El-Haj-Mahmoud References: <20200828220215.101919-1-jeremy.linton@arm.com> <20200828220215.101919-2-jeremy.linton@arm.com> From: "Jeremy Linton" Message-ID: <326f6e0a-85a0-2a9e-9c4b-2afd4541018b@arm.com> Date: Mon, 31 Aug 2020 10:13:23 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable Hi, On 8/31/20 8:15 AM, Pete Batard wrote: > One general, non-blocking comment below: >=20 > On 2020.08.28 23:02, 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. >> >> As a first pass add 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 >> Cc: Pete Batard >> Cc: Andrei Warkentin >> Cc: Ard Biesheuvel >> Cc: Samer El-Haj-Mahmoud >> Signed-off-by: Jeremy Linton >> Reviewed-by: Pete Batard <@pbatard> >> --- >> =C2=A0 Platform/RaspberryPi/AcpiTables/Dsdt.asl=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 | 31=20 >> ++++++++++++++++++++++ >> =C2=A0 .../Bcm27xx/Include/IndustryStandard/Bcm2711.h=C2=A0=C2=A0=C2=A0= =C2=A0 |=C2=A0 2 ++ >> =C2=A0 2 files changed, 33 insertions(+) >> >> diff --git a/Platform/RaspberryPi/AcpiTables/Dsdt.asl=20 >> b/Platform/RaspberryPi/AcpiTables/Dsdt.asl >> index 353af2d876..73067aefd2 100644 >> --- a/Platform/RaspberryPi/AcpiTables/Dsdt.asl >> +++ b/Platform/RaspberryPi/AcpiTables/Dsdt.asl >> @@ -252,6 +252,37 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN",= =20 >> "RPI", 2) >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } >> >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 }) >> >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } >> >> + >> >> +=C2=A0=C2=A0=C2=A0 // Define a simple thermal zone. The idea here is = we compute the=20 >> SOC temp >> >> +=C2=A0=C2=A0=C2=A0 // via a register we can read, and give it to the = OS. This=20 >> enables basic >> >> +=C2=A0=C2=A0=C2=A0 // reports from the "sensors" utility, and the OS = can then poll=20 >> and take >> >> +=C2=A0=C2=A0=C2=A0 // actions if that temp exceeds any of the given t= hresholds. >> >> +=C2=A0=C2=A0=C2=A0 Device (EC0) >=20 > Just going to point out that all the other ACPI devices we seem to=20 > define have 4 characters, so I'm not sure if we're breaking a conventio= n=20 > by introducing a 3 character one here... Well not an ACPI spec convention, because it seems the spec examples are=20 mostly 3 characters. EDK2 OTOH, seems to be largely 4 but there are a=20 fair number of 3 character device/etc methods around I'm not sure it matters, unless there is a edk2 convention I'm unaware of= . >=20 >> >> +=C2=A0=C2=A0=C2=A0 { >> >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Name (_HID, EISAID ("PNP0C06")) >> >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Name (_CCA, 0x0) >> >> + >> >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 // all temps in are tenths of K (aka 2= 732 is the min temps in=20 >> Linux (aka 0C)) >> >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ThermalZone (TZ0) { >=20 > Likewise, what I'm seeing by googling around for ThermalZone () names=20 > would be 4 character ones such as "TZ00", "TZ01" or "TZS0" (For 'Therma= l=20 > Zone Sensor 0') or "TMZN" for single entries. >=20 >> >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Method (_TMP, 0, Serialize= d) { >> >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 OperationRegio= n (TEMS, SystemMemory, THERM_SENSOR, 0x8) >> >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Field (TEMS, D= WordAcc, NoLock, Preserve) { >> >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 TM= PS, 32 >> >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } >> >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return (((4199= 49 - ((TMPS & 0x3ff) * 487)) / 100) + 2732); >> >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } >> >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Method (_SCP, 3) { }=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0 // receive cooling policy=20 >> from OS >> >> + >> >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Method (_CRT) { Return (36= 32) }=C2=A0=C2=A0=C2=A0 // (90C) Critical temp=20 >> point (immediate power-off) >> >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Method (_HOT) { Return (35= 82) }=C2=A0=C2=A0=C2=A0 // (85C) HOT state where=20 >> OS should hibernate >> >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Method (_PSV) { Return (35= 32) }=C2=A0=C2=A0=C2=A0 // (80C) Passive cooling=20 >> (CPU throttling) trip point >> >> + >> >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 // SSDT inserts _AC0/_AL0 = @60C here, if a FAN is configured >> >> + >> >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Name (_TZP, 10)=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0 //The OSPM must poll this=20 >> device every 1 seconds >> >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Name (_PSL, Package () { \= _SB_.CPU0, \_SB_.CPU1, \_SB_.CPU2,=20 >> \_SB_.CPU3 }) >> >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } >> >> +=C2=A0=C2=A0=C2=A0 } >> >> =C2=A0 #endif >> >> >> =C2=A0=C2=A0=C2=A0 } >> >> diff --git=20 >> a/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h=20 >> b/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h >> index e9c81cafa1..86906b2438 100644 >> --- a/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h >> +++ b/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h >> @@ -86,4 +86,6 @@ >> =C2=A0 #define GENET_BASE_ADDRESS=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0 FixedPcdGet64=20 >> (PcdBcmGenetRegistersAddress) >> >> =C2=A0 #define GENET_LENGTH=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 0x00010000 >> >> >> +#define THERM_SENSOR=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 0xfd5d2200 >> >> + >> >> =C2=A0 #endif /* BCM2711_H__ */ >> >=20 > Reviewed-by: Pete Batard