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.

Sign up for GitHub

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

update-amd-tdp #13157

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

update-amd-tdp #13157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 29 additions & 32 deletions board/batocera/x86/fsoverlay/etc/pm/sleep.d/99resume_tdp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,48 +1,45 @@
#!/bin/bash

# check we have a max system TDP value
CPU_TDP=$(/usr/bin/batocera-settings-get system.cpu.tdp)
# if not, we exit as the CPU is not supported by the TDP values
if [ -z "$CPU_TDP" ]; then
echo "No CPU TDP value found."
exit 0
fi

TDP_SAVED="/var/run/amd_tdp_saved"

# set the final TDP value
set_tdp() {
echo "Setting AMD Processor TDP to ${1}W"
/usr/bin/batocera-amd-tdp $1
local TDP_VALUE=$1

echo "Setting AMD Processor TDP to ${TDP_VALUE}W"
/usr/bin/batocera-amd-tdp "${TDP_VALUE}"
}

# determine the new TDP value based on max TDP
handle_tdp() {
TDP_PERCENTAGE=$1
MAX_TDP=$(/usr/bin/batocera-settings-get system.cpu.tdp)
# Check if MAX_TDP is defined and non-empty
if [ -n "$MAX_TDP" ]; then
# round the value up or down to make bash happy
TDP_VALUE=$(awk -v max_tdp="$MAX_TDP" -v tdp_percentage="$TDP_PERCENTAGE" 'BEGIN { printf("%.0f\n", max_tdp * tdp_percentage / 100) }')
set_tdp "${TDP_VALUE}"
else
echo "Max TDP is not defined, cannot set TDP."
exit 1
save_tdp() {
local CURRENT_TDP=$(ryzenadj -i | grep 'PPT LIMIT FAST' | awk '{printf "%.0f\n", $6}')

if [[ "$CURRENT_TDP" =~ ^[0-9]+$ && "$CPU_TDP" =~ ^[0-9]+$ ]] && [ "$CURRENT_TDP" -ne "$CPU_TDP" ]; then
echo "$CURRENT_TDP" > "$TDP_SAVED"
fi
}

case "$1" in
suspend|hibernate)
# We want to save the current TDP to restore after resuming from suspend
save_tdp
;;
resume|thaw)
# check we have a max system TDP value
CPU_TDP=$(/usr/bin/batocera-settings-get system.cpu.tdp)
# if not, we exit as the CPU is not supported by the TDP values
if [ -z "$CPU_TDP" ]; then
echo "No CPU TDP value found."
sleep 1 # Some BIOS may revert to defaults when resuming from suspend. Ensures that saved tdp is set after this.
if [ ! -s "$TDP_SAVED" ]; then
echo "No saved TDP value before suspending found."
exit 0
else
TDP_SETTING=$(printf "%.0f" "$(/usr/bin/batocera-settings-get global.tdp)")
if [ -z "${TDP_SETTING}" ]; then
TDP_SETTING="$(/usr/bin/batocera-settings-get-master system.cpu.tdp)"

if [ -n "${TDP_SETTING}" ]; then
set_tdp "${TDP_SETTING}"
else
echo "TDP setting is not defined, cannot set TDP."
exit 1
fi
else
handle_tdp "${TDP_SETTING}"
fi
SAVED_VALUE=$(cat "$TDP_SAVED")
set_tdp "${SAVED_VALUE}"
rm -f "$TDP_SAVED" # Clean up after restoring
fi
;;
esac
Expand Down
122 changes: 63 additions & 59 deletions package/batocera/core/batocera-configgen/scripts/tdp_hooks.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,89 +20,93 @@ fi

# set the final tdp value
set_tdp() {
echo "Game ${2} requested setting AMD Mobile Processor TDP to ${1} Watts" >> $log
/usr/bin/batocera-amd-tdp $1
local TDP_VALUE=$1
local ROM_NAME=$2

if [ -z "${ROM_NAME}" ]; then
echo "Setting AMD Mobile Processor TDP to ${TDP_VALUE} Watts" >> $log
else
echo "Game ${ROM_NAME} requested setting AMD Mobile Processor TDP to ${TDP_VALUE} Watts" >> $log
fi

/usr/bin/batocera-amd-tdp "$TDP_VALUE"
}

# determine the new TDP value based on max TDP
handle_tdp() {
TDP_PERCENTAGE=$1
ROM_NAME=$2
MAX_TDP=$(/usr/bin/batocera-settings-get system.cpu.tdp)
# Check if MAX_TDP is defined and non-empty
if [ -n "$MAX_TDP" ]; then
local TDP_PERCENTAGE=$1
local ROM_NAME=$2

# Check if TDP is defined and non-empty
if [ -n "$CPU_TDP" ]; then
# round the value up or down to make bash happy
TDP_VALUE=$(awk -v max_tdp="$MAX_TDP" -v tdp_percentage="$TDP_PERCENTAGE" 'BEGIN { printf("%.0f\n", max_tdp * tdp_percentage / 100) }')
TDP_VALUE=$(awk -v max_tdp="$CPU_TDP" -v tdp_percentage="$TDP_PERCENTAGE" 'BEGIN { printf("%.0f\n", max_tdp * tdp_percentage / 100) }')
set_tdp "${TDP_VALUE}" "${ROM_NAME}"
else
echo "A maximum TDP is not defined, cannot set TDP." >> $log
exit 1
fi
}

# Check for events
EVENT=$1
SYSTEM_NAME=$2
ROM_PATH=$5
handle_game_stop() {
TDP_SETTING=$(printf "%.0f" "$(/usr/bin/batocera-settings-get global.tdp)")
if [ -z "${TDP_SETTING}" ]; then
local CURRENT_TDP=$(ryzenadj -i | grep 'PPT LIMIT FAST' | awk '{printf "%.0f\n", $6}')

# Get the rom name from ROM_PATH
ROM_NAME=$(basename "$ROM_PATH")
if [[ "$CURRENT_TDP" =~ ^[0-9]+$ && "$CPU_TDP" =~ ^[0-9]+$ ]] && [ "$CURRENT_TDP" -ne "$CPU_TDP" ]; then
set_tdp "${CPU_TDP}"
fi
else
handle_tdp "${TDP_SETTING}"
fi

# exit accordingly if the event is neither gameStart nor gameStop
if [ "$EVENT" != "gameStart" ] && [ "$EVENT" != "gameStop" ]; then
exit 0
fi
}

# handle gameStop event
if [ "$EVENT" == "gameStop" ]; then
# set either user global setting or default tdp
TDP_SETTING=$(printf "%.0f" "$(/usr/bin/batocera-settings-get global.tdp)")
if [ -z "${TDP_SETTING}" ]; then
TDP_SETTING="$(/usr/bin/batocera-settings-get system.cpu.tdp)"

if [ -n "$TDP_SETTING" ]; then
set_tdp "${TDP_SETTING}" "STOP"
else
echo "No TDP setting defined, cannot set TDP." >> $log
exit 1
handle_game_start() {
local SYSTEM_NAME="$1"
local ROM_PATH="$2"

# Extract the base game name
ROM_NAME="${ROM_PATH##*/}"

# check for user set system specific setting
if [ -n "${SYSTEM_NAME}" ]; then
# check for rom specific config
TDP_SETTING=$(/usr/bin/batocera-settings-get "${SYSTEM_NAME}[\"${ROM_NAME}\"].tdp")
if [ -z "${TDP_SETTING}" ]; then
TDP_SETTING="$(/usr/bin/batocera-settings-get ${SYSTEM_NAME}.tdp)"
fi
exit 0
fi
handle_tdp "${TDP_SETTING}" "STOP"
exit 0
fi

# run through determining the desired TDP setting
# check for user set system specific setting
if [ -n "${SYSTEM_NAME}" ]; then
# check for rom specific config
TDP_SETTING=$(/usr/bin/batocera-settings-get "${SYSTEM_NAME}[\"${ROM_NAME}\"].tdp")
# If no user set system specific setting check for user set global setting
if [ -z "${TDP_SETTING}" ]; then
TDP_SETTING="$(/usr/bin/batocera-settings-get ${SYSTEM_NAME}.tdp)"
TDP_SETTING=$(printf "%.0f" "$(/usr/bin/batocera-settings-get global.tdp)")
fi
fi

# If no user set system specific setting check for user set global setting
if [ -z "${TDP_SETTING}" ]; then
TDP_SETTING=$(printf "%.0f" "$(/usr/bin/batocera-settings-get global.tdp)")
fi

# If no value is found ensure tdp is default before exiting
if [ -z "${TDP_SETTING}" ]; then
TDP_SETTING="$(/usr/bin/batocera-settings-get-master system.cpu.tdp)"
# now apply TDP percentage accordingly
if [ -n "${TDP_SETTING}" ]; then
set_tdp "${TDP_SETTING}" "${ROM_NAME}"
handle_tdp "${TDP_SETTING}" "${ROM_NAME}"
else
echo "No TDP setting defined, cannot set TDP." >> $log
exit 1
fi
exit 0
fi
fi
}

# now apply TDP percentage accordingly
if [ -n "${TDP_SETTING}" ]; then
handle_tdp "${TDP_SETTING}" "${ROM_NAME}"
else
echo "No TDP setting defined, cannot set TDP." >> $log
exit 1
fi
# Check for events
SYSTEM_NAME="$2"
ROM_PATH="$5"

case "$1" in
gameStart)
handle_game_start "$SYSTEM_NAME" "$ROM_PATH"
;;
gameStop)
handle_game_stop
;;
*)
exit 0
;;
esac

exit 0
2 changes: 1 addition & 1 deletion package/batocera/core/batocera-scripts/scripts/batocera-amd-tdp
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

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

why did you remove the thermal control - --tctl-temp=95 ?

Copy link
Contributor Author

@Mikhailzrick Mikhailzrick Apr 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

To allow for bios control. Or rather default to bios controlled. Hard coding it in batocera makes it override any bios setting whereas allowing bios control will still default to 95c on most chips without overriding any user bios settings. There is the possibility of PR’ing a user set temp slider in the future. But I am not sure that needs to be in es_features at least not for per system/per game. Maybe either in just the global game settings or maybe even dev menu.

Copy link
Collaborator

Choose a reason for hiding this comment

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

No we want to set a higher threshold and not another slider either. add it back in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A higher threshold? I don’t understand. But if there’s no chance of having a user defined slider for temp limit in the future I definitely do not want it hard coded and would prefer to have the bios control temp limits. Many mini pc’s can get noisy at max fan speeds and the easiest solution is to limit the temp via the bios.

One of the main reasons for this PR is to default to bios settings while still allowing a user to set tdp parameters, hard coding something like the temp limit just seems unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the bios controlled temp limit lower than 95c for some handhelds? I’m approaching this from a general APU pov where the default bios temp limit is already 95c. If this is something specific to certain handhelds maybe it should just be applied for the specified devices in the handheld dictionary list otherwise if it’s an unknown/generic device the temp limit is not set?

Copy link
Collaborator

Choose a reason for hiding this comment

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

What it does is allow the higher TDP up to 95 degrees. This is still within a safety margin rather than a system setting. Most BIOS' don't set a threshold it's handled by normal CPU scaling. We don't need another vague slider.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have several amd apu systems that allow bios control of cpu thermal limits. It’s a common amd bios setting like PBO, setting PPT, EDC, TDC, and curve optimizer.

I fundamentally disagree with taking away users ability to control their own hardware by forcing a hard coded software implementation, even going so far as overwriting a users desired bios settings.

If there’s a specific use case for setting a higher than default bios determined temp limit for certain hardware I think that’s fair and we can work to accommodate that, otherwise I’m still seeing this as a situation where something that was previously under user control is being taken away with no benefit in exchange.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The benefit is that most users won't set it in the bios or even know about it. It also allows for their intended TDP to last longer.
You had no issues using this default before this minor refactor. There have been no reported problems either. Other distributions also set this variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I never used it, I disabled the feature when it was implemented because it was overwriting my bios settings. I’m admittedly a power user, and I know my hardware better than anyone else, but I don’t think it’s right to completely exclude power users like myself just because the average user won’t ever change it, especially if there’s no beneficial gain from the exclusion.

Let me try to understand this from a different angle. What evidence is there that a hard coded temp limit is needed? I need to see numbers to understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A potential compromise might be to guarantee that ryzenadj is only applied for handhelds and excludes any generic devices with AMD APU’s like mini pc’s, desktops, laptops, etc. I don’t immediately know how I’d do that but it’s a thought.

Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ fi
WATTS=$((TDP * 1000))

# Set TDP using ryzenadj
if /usr/bin/ryzenadj --stapm-limit="${WATTS}" --fast-limit="${WATTS}" --slow-limit="$((WATTS * 80 / 100))" --tctl-temp=95; then
if /usr/bin/ryzenadj --stapm-limit="${WATTS}" --fast-limit="${WATTS}" --slow-limit="${WATTS}"; then
echo "TDP of $TDP Watts has been set" >> "$log"
else
echo "Error setting TDP. Please check the logs for more information." >> "$log"
Expand Down
4 changes: 2 additions & 2 deletions package/batocera/emulationstation/batocera-es-system/es_features.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ shared:
group: POWER OPTIONS
prompt: RYZEN THERMAL DESIGN POWER
description: Adjust the percentage of power (watts) provided to a Ryzen Mobile Series CPU based on it's default max value. Caution, setting wattage too high could damage your device.
preset: slider
preset_parameters: 10 130 5 100 %
preset: sliderauto
preset_parameters: 10 130 5 %
videomode:
prompt: VIDEO MODE
description: Set the display's resolution. Does not affect the rendering resolution.
Expand Down