Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

performance: Update based on ARC review #86

Merged
merged 1 commit into from
Jan 9, 2025

Conversation

lftan
Copy link
Collaborator

@lftan lftan commented Jan 7, 2025

  • Fix rate_limit to transition_latency
  • Add more descriptions for PERF_SET_LIMIT

#79 (comment)

@@ -734,7 +734,15 @@ performance domain in the system.

==== Service: PERF_SET_LIMIT (SERVICE_ID: 0x08)
This service is used to set the performance limit of a specific
performance domain in the system.
performance domain in the system. Any subsequent calls to `PERF_SET_LEVEL`
will ensure the requested performance level stays within the defined range.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The platform must ensure that any subsequent calls to the PERF_SET_LEVEL to set performance level is within the current set limit"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay

it to comply with the updated limits. If notifications are enabled, the
platform microcontroller firmware will send an appropriate notification
(e.g., `PERF_LEVEL_CHANGE`, `PERF_POWER_CHANGE`, etc.) to the
application processor.
Copy link
Collaborator

@pathakraul pathakraul Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This text will then require more details. If the the new limit is NEW_LEVEL_MIN, NEW_LEVEL_MAX. Then what will be the NEW_LEVEL if the CURRENT_LEVEL < NEW_LEVEL_MIN and what will the NEW_LEVEL if the CURRENT_LEVEL > NEW_LEVEL_MAX.

Ideally NEW_LEVEL == NEW_LEVEL_MIN if the CURRENT_LEVEL < NEW_LEVEL_MIN
and
NEW_LEVEL == NEW_LEVEL_MAX if the CURRENT_LEVEL > MEW_LEVEL_MAX.

and NEW_LEVEL == CURRENT_LEVEL if its already within the range.

In some form this behaviour can be mentioned here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay

- Fix rate_limit to transition_latency
- Add more descriptions for PERF_SET_LIMIT

riscv-non-isa#79 (comment)

Signed-off-by: Ley Foon Tan <[email protected]>
Examples:

* If the current performance level is below the new minimum limit, the platform
will set it to the new minimum limit.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NITs: just change will to may

will set it to the new minimum limit.

* If the current performance level exceeds the new maximum limit, the platform
will set it to the new maximum limit.
Copy link
Collaborator

@pathakraul pathakraul Jan 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NITs: just change will to may

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than that looks good and we can merge

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will update this in my PR

@pathakraul pathakraul merged commit 126f04b into riscv-non-isa:main Jan 9, 2025
1 check passed
@lftan lftan deleted the arc-perf2 branch January 9, 2025 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants