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.56195.1598888949998021048 for ; Mon, 31 Aug 2020 08:49:10 -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 88C911FB; Mon, 31 Aug 2020 08:49:09 -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 08F763F66F; Mon, 31 Aug 2020 08:49:08 -0700 (PDT) Subject: Re: [PATCH v3 1/5] Platform/RaspberryPi4: Add a basic thermal zone To: Ard Biesheuvel , Pete Batard , devel@edk2.groups.io Cc: Leif Lindholm , Andrei Warkentin , Samer El-Haj-Mahmoud References: <20200828220215.101919-1-jeremy.linton@arm.com> <20200828220215.101919-2-jeremy.linton@arm.com> <326f6e0a-85a0-2a9e-9c4b-2afd4541018b@arm.com> From: "Jeremy Linton" Message-ID: Date: Mon, 31 Aug 2020 10:49:08 -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 On 8/31/20 10:33 AM, Ard Biesheuvel wrote: > On 8/31/20 5:13 PM, Jeremy Linton wrote: >> Hi, >> >> On 8/31/20 8:15 AM, Pete Batard wrote: >>> One general, non-blocking comment below: >>> >>> 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,=20 >>>> "RPIFDN", "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 i= s we compute=20 >>>> the SOC temp >>>> >>>> +=C2=A0=C2=A0=C2=A0 // via a register we can read, and give it to th= e OS. This=20 >>>> enables basic >>>> >>>> +=C2=A0=C2=A0=C2=A0 // reports from the "sensors" utility, and the O= S can then poll=20 >>>> and take >>>> >>>> +=C2=A0=C2=A0=C2=A0 // actions if that temp exceeds any of the given= thresholds. >>>> >>>> +=C2=A0=C2=A0=C2=A0 Device (EC0) >>> >>> 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=20 >>> convention by introducing a 3 character one here... >> >> Well not an ACPI spec convention, because it seems the spec examples=20 >> are mostly 3 characters. EDK2 OTOH, seems to be largely 4 but there=20 >> are a fair number of 3 character device/etc methods around >> >> I'm not sure it matters, unless there is a edk2 convention I'm unaware= =20 >> of. >> >=20 > I don't think such a convention exists, although I never tried the MS=20 > iasl compiler, and the RPi asl code (which was contributed by MS)=20 > suspiciously uses 4 letter identifiers everywhere. >=20 > Samer, any clue? >=20 I have another patch to allow the user to set the temp trip point which=20 IIRC Andrei had asked about months ago when I originally mentioned that=20 I had hacked up a thermal zone. I might as well roll a v4 with that, the whitespace, and "0"'s appended=20 to EC0 and TZ0 just in case... I'm sanity checking right now that it=20 still works.