Re: [edk2-devel] [PATCH v2 02/11] ArmPkg/ArmScmiDxe: Add PERFORMANCE_DESCRIBE_FASTCHANNEL support

2023-11-02 Thread Leif Lindholm

On 2023-11-02 10:19, PierreGondois wrote:


+/// SCMI Message Ids for the Performance Protocol.
+typedef enum {
+  ScmiMessageIdPerformanceDomainAttributes    = 0x3,
+  ScmiMessageIdPerformanceDescribeLevels  = 0x4,
+  ScmiMessageIdPerformanceLimitsSet   = 0x5,
+  ScmiMessageIdPerformanceLimitsGet   = 0x6,
+  ScmiMessageIdPerformanceLevelSet    = 0x7,
+  ScmiMessageIdPerformanceLevelGet    = 0x8,
+  ScmiMessageIdPerformanceDescribeFastchannel = 0xB,
+} SCMI_MESSAGE_ID_PERFORMANCE;


This struct appears to move in the code at the same time as having an
entry added to it. This seems superficially unmotivated.
However it is also moved to inside the pack(1) block, which makes no
sense for an enum.


Yes right, I will move it out of the pack(1) section.


Thx, with that change:
Reviewed-by: Leif Lindholm 

The reason to move the SCMI_MESSAGE_ID_PERFORMANCE definition up in the 
file

was that the SCMI_PERFORMANCE_DESCRIBE_FASTCHANNEL interface added in this
patch takes a SCMI_MESSAGE_ID_PERFORMANCE parameter as an argument, so the
enum needs to be defined before.
I can add a comment about this in the commit message.


It doesn't really need to be in the commit message, but it's the kind of 
thing that can be helpful to point out below ---.


Regards,

Leif



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110527): https://edk2.groups.io/g/devel/message/110527
Mute This Topic: https://groups.io/mt/102175812/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v2 02/11] ArmPkg/ArmScmiDxe: Add PERFORMANCE_DESCRIBE_FASTCHANNEL support

2023-11-02 Thread PierreGondois




On 10/26/23 12:37, Leif Lindholm wrote:

On Wed, Oct 25, 2023 at 13:25:31 +0200, PierreGondois wrote:

From: Pierre Gondois 


Sidebar: I think if you correct your name in your git config, the
above tag will drop out.


Ok I will check.




The PERFORMANCE_DESCRIBE_FASTCHANNEL Scmi command is available
since SCMI v2.0 and allows to query information about the supported
fast-channels of the Scmi performance protocol.
Add support for this command.

Signed-off-by: Pierre Gondois 
---
  .../ArmScmiDxe/ScmiPerformanceProtocol.c  | 80 +++--
  .../Protocol/ArmScmiPerformanceProtocol.h | 90 ---
  2 files changed, 155 insertions(+), 15 deletions(-)

diff --git a/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c 
b/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
index 0f89808fbdf9..1d87339209fd 100644
--- a/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
+++ b/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
@@ -1,12 +1,12 @@
  /** @file
  
-  Copyright (c) 2017-2021, Arm Limited. All rights reserved.

+  Copyright (c) 2017-2023, Arm Limited. All rights reserved.
  
SPDX-License-Identifier: BSD-2-Clause-Patent
  
-  System Control and Management Interface V1.0

-http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/
-DEN0056A_System_Control_and_Management_Interface.pdf
+  System Control and Management Interface, latest version:
+  - https://developer.arm.com/documentation/den0056/latest/
+
  **/
  
  #include 

@@ -416,6 +416,75 @@ PerformanceLevelGet (
return EFI_SUCCESS;
  }
  
+/** Discover the attributes of the FastChannel for the specified

+performance domain and the specified message.
+
+  @param[in]  ThisA Pointer to SCMI_PERFORMANCE_PROTOCOL Instance.
+  @param[in]  DomainIdIdentifier for the performance domain.
+  @param[in]  MessageId   Message Id of the FastChannel to discover.
+  Must be one of:
+   - PERFORMANCE_LIMITS_SET
+   - PERFORMANCE_LIMITS_GET
+   - PERFORMANCE_LEVEL_SET
+   - PERFORMANCE_LEVEL_GET
+  @param[out] FastChannel If success, contains the FastChannel description.
+
+  @retval EFI_SUCCESS Performance level got successfully.
+  @retval EFI_DEVICE_ERRORSCP returns an SCMI error.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_TIMEOUT Time out.
+  @retval EFI_UNSUPPORTED Unsupported.
+**/
+EFI_STATUS
+DescribeFastchannel (
+  IN  SCMI_PERFORMANCE_PROTOCOL *This,
+  IN  UINT32DomainId,
+  IN  SCMI_MESSAGE_ID_PERFORMANCE   MessageId,
+  OUT SCMI_PERFORMANCE_FASTCHANNEL  *FastChannel
+  )
+{
+  EFI_STATUSStatus;
+  SCMI_COMMAND  Cmd;
+  UINT32PayloadLength;
+  UINT32*ReturnValues;
+  UINT32*MessageParams;
+
+  if ((This == NULL)  ||
+  (FastChannel == NULL))
+  {
+return EFI_INVALID_PARAMETER;
+  }
+
+  Status = ScmiCommandGetPayload ();
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  *MessageParams++ = DomainId;
+  *MessageParams   = MessageId;
+
+  Cmd.ProtocolId = ScmiProtocolIdPerformance;
+  Cmd.MessageId  = ScmiMessageIdPerformanceDescribeFastchannel;
+  PayloadLength  = sizeof (DomainId) + sizeof (MessageId);
+
+  Status = ScmiCommandExecute (
+ ,
+ ,
+ 
+ );
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  CopyMem (
+FastChannel,
+ReturnValues,
+sizeof (SCMI_PERFORMANCE_FASTCHANNEL)
+);
+
+  return Status;
+}
+
  // Instance of the SCMI performance management protocol.
  STATIC CONST SCMI_PERFORMANCE_PROTOCOL  PerformanceProtocol = {
PerformanceGetVersion,
@@ -425,7 +494,8 @@ STATIC CONST SCMI_PERFORMANCE_PROTOCOL  PerformanceProtocol 
= {
PerformanceLimitsSet,
PerformanceLimitsGet,
PerformanceLevelSet,
-  PerformanceLevelGet
+  PerformanceLevelGet,
+  DescribeFastchannel,
  };
  
  /** Initialize performance management protocol and install on a given Handle.

diff --git a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h 
b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
index 8e8e05d5a5f6..088182945a06 100644
--- a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
+++ b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
@@ -14,7 +14,7 @@
  
  #include 
  
-/// Arm Scmi performance protocol versions.

+/// Arm SCMI performance protocol versions.
  #define PERFORMANCE_PROTOCOL_VERSION_V1  0x1
  #define PERFORMANCE_PROTOCOL_VERSION_V2  0x2
  #define PERFORMANCE_PROTOCOL_VERSION_V3  0x3
@@ -79,6 +79,56 @@ typedef struct {
UINT32RangeMin;
  } SCMI_PERFORMANCE_LIMITS;
  
+/// Doorbell Support bit.

+#define SCMI_PERF_FC_ATTRIB_HAS_DOORBELL  BIT0
+
+/// Performance protocol describe fastchannel
+typedef struct {
+  /// Attributes.
+  UINT32Attributes;
+
+  /// Rate limit.
+  UINT32RateLimit;
+
+  /// Lower 32 bits of 

Re: [edk2-devel] [PATCH v2 02/11] ArmPkg/ArmScmiDxe: Add PERFORMANCE_DESCRIBE_FASTCHANNEL support

2023-10-26 Thread Leif Lindholm
On Wed, Oct 25, 2023 at 13:25:31 +0200, PierreGondois wrote:
> From: Pierre Gondois 

Sidebar: I think if you correct your name in your git config, the
above tag will drop out.

> The PERFORMANCE_DESCRIBE_FASTCHANNEL Scmi command is available
> since SCMI v2.0 and allows to query information about the supported
> fast-channels of the Scmi performance protocol.
> Add support for this command.
> 
> Signed-off-by: Pierre Gondois 
> ---
>  .../ArmScmiDxe/ScmiPerformanceProtocol.c  | 80 +++--
>  .../Protocol/ArmScmiPerformanceProtocol.h | 90 ---
>  2 files changed, 155 insertions(+), 15 deletions(-)
> 
> diff --git a/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c 
> b/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
> index 0f89808fbdf9..1d87339209fd 100644
> --- a/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
> +++ b/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
> @@ -1,12 +1,12 @@
>  /** @file
>  
> -  Copyright (c) 2017-2021, Arm Limited. All rights reserved.
> +  Copyright (c) 2017-2023, Arm Limited. All rights reserved.
>  
>SPDX-License-Identifier: BSD-2-Clause-Patent
>  
> -  System Control and Management Interface V1.0
> -http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/
> -DEN0056A_System_Control_and_Management_Interface.pdf
> +  System Control and Management Interface, latest version:
> +  - https://developer.arm.com/documentation/den0056/latest/
> +
>  **/
>  
>  #include 
> @@ -416,6 +416,75 @@ PerformanceLevelGet (
>return EFI_SUCCESS;
>  }
>  
> +/** Discover the attributes of the FastChannel for the specified
> +performance domain and the specified message.
> +
> +  @param[in]  ThisA Pointer to SCMI_PERFORMANCE_PROTOCOL Instance.
> +  @param[in]  DomainIdIdentifier for the performance domain.
> +  @param[in]  MessageId   Message Id of the FastChannel to discover.
> +  Must be one of:
> +   - PERFORMANCE_LIMITS_SET
> +   - PERFORMANCE_LIMITS_GET
> +   - PERFORMANCE_LEVEL_SET
> +   - PERFORMANCE_LEVEL_GET
> +  @param[out] FastChannel If success, contains the FastChannel description.
> +
> +  @retval EFI_SUCCESS Performance level got successfully.
> +  @retval EFI_DEVICE_ERRORSCP returns an SCMI error.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +  @retval EFI_TIMEOUT Time out.
> +  @retval EFI_UNSUPPORTED Unsupported.
> +**/
> +EFI_STATUS
> +DescribeFastchannel (
> +  IN  SCMI_PERFORMANCE_PROTOCOL *This,
> +  IN  UINT32DomainId,
> +  IN  SCMI_MESSAGE_ID_PERFORMANCE   MessageId,
> +  OUT SCMI_PERFORMANCE_FASTCHANNEL  *FastChannel
> +  )
> +{
> +  EFI_STATUSStatus;
> +  SCMI_COMMAND  Cmd;
> +  UINT32PayloadLength;
> +  UINT32*ReturnValues;
> +  UINT32*MessageParams;
> +
> +  if ((This == NULL)  ||
> +  (FastChannel == NULL))
> +  {
> +return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Status = ScmiCommandGetPayload ();
> +  if (EFI_ERROR (Status)) {
> +return Status;
> +  }
> +
> +  *MessageParams++ = DomainId;
> +  *MessageParams   = MessageId;
> +
> +  Cmd.ProtocolId = ScmiProtocolIdPerformance;
> +  Cmd.MessageId  = ScmiMessageIdPerformanceDescribeFastchannel;
> +  PayloadLength  = sizeof (DomainId) + sizeof (MessageId);
> +
> +  Status = ScmiCommandExecute (
> + ,
> + ,
> + 
> + );
> +  if (EFI_ERROR (Status)) {
> +return Status;
> +  }
> +
> +  CopyMem (
> +FastChannel,
> +ReturnValues,
> +sizeof (SCMI_PERFORMANCE_FASTCHANNEL)
> +);
> +
> +  return Status;
> +}
> +
>  // Instance of the SCMI performance management protocol.
>  STATIC CONST SCMI_PERFORMANCE_PROTOCOL  PerformanceProtocol = {
>PerformanceGetVersion,
> @@ -425,7 +494,8 @@ STATIC CONST SCMI_PERFORMANCE_PROTOCOL  
> PerformanceProtocol = {
>PerformanceLimitsSet,
>PerformanceLimitsGet,
>PerformanceLevelSet,
> -  PerformanceLevelGet
> +  PerformanceLevelGet,
> +  DescribeFastchannel,
>  };
>  
>  /** Initialize performance management protocol and install on a given Handle.
> diff --git a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h 
> b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
> index 8e8e05d5a5f6..088182945a06 100644
> --- a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
> +++ b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
> @@ -14,7 +14,7 @@
>  
>  #include 
>  
> -/// Arm Scmi performance protocol versions.
> +/// Arm SCMI performance protocol versions.
>  #define PERFORMANCE_PROTOCOL_VERSION_V1  0x1
>  #define PERFORMANCE_PROTOCOL_VERSION_V2  0x2
>  #define PERFORMANCE_PROTOCOL_VERSION_V3  0x3
> @@ -79,6 +79,56 @@ typedef struct {
>UINT32RangeMin;
>  } SCMI_PERFORMANCE_LIMITS;
>  
> +/// Doorbell Support bit.
> +#define SCMI_PERF_FC_ATTRIB_HAS_DOORBELL  BIT0
> +
> +///