Re: [edk2-platforms][PATCH V1 10/17] Platform/Sgi: Macro definitions for ACPI CPPC

["Sami Mujawar" <sami.mujawar@arm.com>]

Hi Pranav,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar


On 28/04/2021 01:35 PM, Pranav Madhu wrote:
> Add helper macros required for use with ACPI collaborative processor
> performance control (CPPC). This patch adds macros for initializing ACPI
> _CPC and _PSD control method. The CPC initializer macro initializes _CPC
> control method with revision 3 as specified in Arm FFH specification
> 1.1. The CPC initilizer exposes the reference performance counter and
> delivered perfrmance counter (AMU registers) as FFixedHW registers. The
> initilizer also expose the fastchannel memories for performance level
> set performance limit set protocols as desired performance register and
> performance limited register respectively.
>
> Signed-off-by: Pranav Madhu <pranav.madhu@arm.com>
> ---
>   Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h | 53 ++++++++++++++++++++
>   1 file changed, 53 insertions(+)
>
> diff --git a/Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h b/Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h
> index 4977d4d898aa..1b5305f15fb8 100644
> --- a/Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h
> +++ b/Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h
> @@ -423,4 +423,57 @@ typedef struct {
>       LineSize                                          /* Line size in bytes */ \
>     }
>   
> +// CPPC _CPC object initialization
> +#define CPPC_PACKAGE_INIT(DesiredPerfReg, PerfLimitedReg, GranularityMHz,      \
> +  HighestPerf, NominalPerf, LowestNonlinearPerf, LowestPerf, RefPerf)          \
[SAMI] I think it would be good to add a coment in the macro header 
clarifying that this macro is intended to be used in ASL definitition.
The same would apply for the following macro.
Alternatively, you could introduce a new header file for the ASL macro 
definitions SgiAslHeader.h. What do you think ?
[/SAMI]
> +  {                                                                            \
> +    23,                                 /* NumEntries */                       \
> +    3,                                  /* Revision */                         \
> +    HighestPerf,                        /* Highest Performance */              \
> +    NominalPerf,                        /* Nominal Performance */              \
> +    LowestNonlinearPerf,                /* Lowest Nonlinear Performance */     \
> +    LowestPerf,                         /* Lowest Performance */               \
> +    /* Guaranteed Performance Register */                                      \
> +    ResourceTemplate () { Register (SystemMemory, 0, 0, 0, 0) },               \
> +    /* Desired Performance Register */                                         \
> +    ResourceTemplate () { Register (SystemMemory, 32, 0, DesiredPerfReg, 3) }, \
> +    /* Minimum Performance Register */                                         \
> +    ResourceTemplate () { Register (SystemMemory, 0, 0, 0, 0) },               \
> +    /* Maximum Performance Register */                                         \
> +    ResourceTemplate () { Register (SystemMemory, 0, 0, 0, 0) },               \
> +    /* Performance Reduction Tolerance Register */                             \
> +    ResourceTemplate () { Register (SystemMemory, 0, 0, 0, 0) },               \
> +    /* Time Window Register */                                                 \
> +    ResourceTemplate () { Register (SystemMemory, 0, 0, 0, 0) },               \
> +    /* Counter Wraparound Time */                                              \
> +    ResourceTemplate () { Register (SystemMemory, 0, 0, 0, 0) },               \
> +    /* Reference Performance Counter Register */                               \
> +    ResourceTemplate () { Register (FFixedHW, 64, 0, 1, 4) },                  \
> +    /* Delivered Performance Counter Register */                               \
> +    ResourceTemplate () { Register (FFixedHW, 64, 0, 0, 4) },                  \
> +    /* Performance Limited Register */                                         \
> +    ResourceTemplate () { Register (SystemMemory, 32, 0, PerfLimitedReg, 3) }, \
> +    /* CPPC Enable Register */                                                 \
> +    ResourceTemplate () { Register (SystemMemory, 0, 0, 0, 0) },               \
> +    /* Autonomous Selection Enable Register */                                 \
> +    ResourceTemplate () { Register (SystemMemory, 0, 0, 0, 0) },               \
> +    /* Autonomous Activity Window Register */                                  \
> +    ResourceTemplate () { Register (SystemMemory, 0, 0, 0, 0) },               \
> +    /* Energy Performance Preference Register */                               \
> +    ResourceTemplate () { Register (SystemMemory, 0, 0, 0, 0) },               \
> +    RefPerf,                            /* Reference Performance */            \
> +    (LowestPerf * GranularityMHz),      /* Lowest Frequency */                 \
> +    (NominalPerf * GranularityMHz),     /* Nominal Frequency */                \
> +  }
> +
> +// Power state dependancy (_PSD) for CPPC
> +#define PSD_INIT(Domain)                                                       \
> +  {                                                                            \
> +    5,              /* Entries */                                              \
> +    0,              /* Revision */                                             \
> +    Domain,         /* Domain */                                               \
> +    0xFD,           /* Coord Type- SW_ANY */                                   \
> +    1               /* Processors */                                           \
> +  }
> +
>   #endif /* __SGI_ACPI_HEADER__ */