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.27223.1658151068461536531 for ; Mon, 18 Jul 2022 06:31:08 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: pierre.gondois@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 40FA51042; Mon, 18 Jul 2022 06:31:08 -0700 (PDT) Received: from [10.34.100.102] (pierre123.nice.arm.com [10.34.100.102]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id EB5A13F766; Mon, 18 Jul 2022 06:31:06 -0700 (PDT) Message-ID: <38013f32-0646-6405-9dfd-6e3fb4508803@arm.com> Date: Mon, 18 Jul 2022 15:30:43 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [PATCH edk2-platforms v1 1/3] Platform/ARM: Juno: Generate ASL description of CPU topology To: Sami Mujawar , devel@edk2.groups.io Cc: Ard Biesheuvel , nd@arm.com References: <20220427145943.402487-1-Pierre.Gondois@arm.com> <20220427145943.402487-2-Pierre.Gondois@arm.com> <3c56d998-8b61-a0e2-a32a-f8bcdd01efe8@arm.com> From: "PierreGondois" In-Reply-To: <3c56d998-8b61-a0e2-a32a-f8bcdd01efe8@arm.com> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi Sami, On 7/18/22 15:25, Sami Mujawar wrote: > Hi Pierre, > > Thank you for this patch. > > Please find my response inline marked [SAMI]. > > Other than the comment below, these changes looks good to me. > > Reviewed-by: Sami Mujawar > > Regards, > > Sami Mujawar > > On 27/04/2022 03:59 pm, Pierre.Gondois@arm.com wrote: >> From: Pierre Gondois >> >> The Dynamic Tables Framework has been updated to add support for >> generating an ASL description of a CPU topology and its _LPI states. >> This patch: >> - Describes the _LPI states in the Configuration Manager of the Juno >> - Add the generation of a new SSDT table describing the CPU topology >> and its _LPI states >> - Removes the CPU topology description of the DSDT table >> >> Signed-off-by: Pierre Gondois >> --- >> .../AslTables/Dsdt.asl | 198 ------------------ >> .../ConfigurationManager.c | 196 ++++++++++++++++- >> .../ConfigurationManager.h | 20 +- >> 3 files changed, 206 insertions(+), 208 deletions(-) >> [snip] >> >> @@ -549,6 +575,84 @@ EDKII_PLATFORM_REPOSITORY_INFO ArmJunoPlatformRepositoryInfo = { >> { >> { REFERENCE_TOKEN (CacheInfo[4]) }, // -> 'LITTLE' core's L1 I-cache >> { REFERENCE_TOKEN (CacheInfo[5]) } // -> 'LITTLE' core's L1 D-cache >> + }, >> + >> + // Low Power Idle state information (LPI) for all cores/clusters >> + { >> + { // LpiInfo[0] -> Clusters CluPwrDn >> + 2500, // MinResidency >> + 1150, // WorstCaseWakeLatency >> + 1, // Flags >> + 1, // ArchFlags >> + 100, // ResCntFreq >> + 0, // EnableParentState >> + 1, // IsInteger > > [SAMI] IsInteger field is of type BOOLEAN. Therefore, the value here > should be TRUE. Similar changes are requires at other places in this file. > > If you agree, I will make the nessary changes locally before pushing the > patch. > > [/SAMI] > Thanks for catching this. Yes if you can make this change it would be awesome, Regards, Pierre