Browse Source

Change AMD overclocking to simply be setting the power_dpm_force_performance_level file for now

	This covers the MVP for now, and simply allows pinning the power level to "high"

	Full overclocking set up is somewhat more complicated, and it'll be better to implement that at the same time as the same for Nvidia, where we're currently only really setting the top end power level
Marc Di Luzio 6 years ago
parent
commit
9ade4481c3

+ 11 - 8
daemon/daemon_config.c

@@ -92,8 +92,7 @@ struct GameModeConfig {
 		long nv_core_clock_mhz_offset;
 		long nv_mem_clock_mhz_offset;
 		long nv_perf_level;
-		long amd_core_clock_percentage;
-		long amd_mem_clock_percentage;
+		char amd_performance_level[CONFIG_VALUE_MAX];
 
 		long require_supervisor;
 		char supervisor_whitelist[CONFIG_LIST_MAX][CONFIG_VALUE_MAX];
@@ -259,10 +258,8 @@ static int inih_handler(void *user, const char *section, const char *name, const
 			valid = get_long_value(name, value, &self->values.nv_mem_clock_mhz_offset);
 		} else if (strcmp(name, "nv_perf_level") == 0) {
 			valid = get_long_value(name, value, &self->values.nv_perf_level);
-		} else if (strcmp(name, "amd_core_clock_percentage") == 0) {
-			valid = get_long_value(name, value, &self->values.amd_core_clock_percentage);
-		} else if (strcmp(name, "amd_mem_clock_percentage") == 0) {
-			valid = get_long_value(name, value, &self->values.amd_mem_clock_percentage);
+		} else if (strcmp(name, "amd_performance_level") == 0) {
+			valid = get_string_value(value, self->values.amd_performance_level);
 		}
 	} else if (strcmp(section, "supervisor") == 0) {
 		/* Supervisor subsection */
@@ -585,8 +582,14 @@ DEFINE_CONFIG_GET(gpu_device)
 DEFINE_CONFIG_GET(nv_core_clock_mhz_offset)
 DEFINE_CONFIG_GET(nv_mem_clock_mhz_offset)
 DEFINE_CONFIG_GET(nv_perf_level)
-DEFINE_CONFIG_GET(amd_core_clock_percentage)
-DEFINE_CONFIG_GET(amd_mem_clock_percentage)
+
+void config_get_amd_performance_level(GameModeConfig *self, char value[CONFIG_VALUE_MAX])
+{
+	memcpy_locked_config(self,
+	                     value,
+	                     &self->values.amd_performance_level,
+	                     sizeof(self->values.amd_performance_level));
+}
 
 /*
         char supervisor_whitelist[CONFIG_LIST_MAX][CONFIG_VALUE_MAX];

+ 1 - 2
daemon/daemon_config.h

@@ -143,8 +143,7 @@ long config_get_gpu_device(GameModeConfig *self);
 long config_get_nv_core_clock_mhz_offset(GameModeConfig *self);
 long config_get_nv_mem_clock_mhz_offset(GameModeConfig *self);
 long config_get_nv_perf_level(GameModeConfig *self);
-long config_get_amd_core_clock_percentage(GameModeConfig *self);
-long config_get_amd_mem_clock_percentage(GameModeConfig *self);
+void config_get_amd_performance_level(GameModeConfig *self, char value[CONFIG_VALUE_MAX]);
 
 /**
  * Functions to get supervisor config permissions

+ 21 - 29
daemon/gamemode-gpu.c

@@ -150,20 +150,11 @@ int game_mode_initialise_gpu(GameModeConfig *config, GameModeGPUInfo **info)
 
 		break;
 	case Vendor_AMD:
-		new_info->nv_core = config_get_amd_core_clock_percentage(config);
-		new_info->nv_mem = config_get_amd_mem_clock_percentage(config);
+		config_get_amd_performance_level(config, new_info->amd_performance_level);
 
-		/* Reject values over 20%
-		 * If a user wants to go into very unsafe levels they can recompile
-		 * As far as I can tell the driver doesn't allow values over 20 anyway
-		 */
-		const int amd_hard_limit = 20;
-		if (new_info->nv_core > amd_hard_limit || new_info->nv_mem > amd_hard_limit) {
-			LOG_ERROR("AMD Overclock value above safety level of %d%%, will not overclock!\n",
-			          amd_hard_limit);
-			LOG_ERROR("amd_core_clock_percentage:%ld amd_mem_clock_percentage:%ld\n",
-			          new_info->nv_core,
-			          new_info->nv_mem);
+		/* Error about unsupported "manual" option, for now */
+		if (strcmp(new_info->amd_performance_level, "manual") == 0) {
+			LOG_ERROR("AMD Performance level set to \"manual\", this is currently unsupported");
 			free(new_info);
 			return -1;
 		}
@@ -193,22 +184,20 @@ void game_mode_free_gpu(GameModeGPUInfo **info)
 /**
  * Applies GPU optimisations when gamemode is active and removes them after
  */
-int game_mode_apply_gpu(const GameModeGPUInfo *info, bool apply)
+int game_mode_apply_gpu(const GameModeGPUInfo *info)
 {
 	// Null info means don't apply anything
 	if (!info)
 		return 0;
 
-	LOG_MSG("Requesting GPU optimisations on device:%ld with settings nv_core:%ld clock:%ld\n",
-	        info->device,
-	        info->nv_core,
-	        info->nv_mem);
+	LOG_MSG("Requesting GPU optimisations on device:%ld\n", info->device);
 
 	/* Generate the input strings */
 	char vendor[7];
 	snprintf(vendor, 7, "0x%04x", (short)info->vendor);
 	char device[4];
 	snprintf(device, 4, "%ld", info->device);
+
 	char nv_core[8];
 	snprintf(nv_core, 8, "%ld", info->nv_core);
 	char nv_mem[8];
@@ -223,8 +212,8 @@ int game_mode_apply_gpu(const GameModeGPUInfo *info, bool apply)
 		vendor,
 		device,
 		"set",
-		apply ? nv_core : "0",
-		apply ? nv_mem : "0",
+		info->vendor == Vendor_NVIDIA ? nv_core : info->amd_performance_level,
+		info->vendor == Vendor_NVIDIA ? nv_mem : NULL,        /* Only use this if Nvidia */
 		info->vendor == Vendor_NVIDIA ? nv_perf_level : NULL, /* Only use this if Nvidia */
 		NULL,
 	};
@@ -263,19 +252,22 @@ int game_mode_get_gpu(GameModeGPUInfo *info)
 
 	char buffer[EXTERNAL_BUFFER_MAX] = { 0 };
 	if (run_external_process(exec_args, buffer, -1) != 0) {
-		LOG_ERROR("Failed to call gpuclockctl, could get values!\n");
+		LOG_ERROR("Failed to call gpuclockctl, could not get values!\n");
 		return -1;
 	}
 
-	int nv_core = 0;
-	int nv_mem = 0;
-	if (sscanf(buffer, "%i %i", &nv_core, &nv_mem) != 2) {
-		LOG_ERROR("Failed to parse gpuclockctl output: %s\n", buffer);
-		return -1;
+	switch (info->vendor) {
+	case Vendor_NVIDIA:
+		if (sscanf(buffer, "%ld %ld", &info->nv_core, &info->nv_mem) != 2) {
+			LOG_ERROR("Failed to parse gpuclockctl output: %s\n", buffer);
+			return -1;
+		}
+		break;
+	case Vendor_AMD:
+		strncpy(info->amd_performance_level, buffer, CONFIG_VALUE_MAX);
+		strtok(info->amd_performance_level, "\n");
+		break;
 	}
 
-	info->nv_core = nv_core;
-	info->nv_mem = nv_mem;
-
 	return 0;
 }

+ 32 - 17
daemon/gamemode-tests.c

@@ -338,7 +338,7 @@ static int run_cpu_governor_tests(struct GameModeConfig *config)
 	/* Verify the governor is the desired one */
 	const char *currentgov = get_gov_state();
 	if (strncmp(currentgov, desiredgov, CONFIG_VALUE_MAX) != 0) {
-		LOG_ERROR("Govenor was not set to %s (was actually %s)!", desiredgov, currentgov);
+		LOG_ERROR("Governor was not set to %s (was actually %s)!\n", desiredgov, currentgov);
 		gamemode_request_end();
 		return -1;
 	}
@@ -349,7 +349,7 @@ static int run_cpu_governor_tests(struct GameModeConfig *config)
 	/* Verify the governor has been set back */
 	currentgov = get_gov_state();
 	if (strncmp(currentgov, defaultgov, CONFIG_VALUE_MAX) != 0) {
-		LOG_ERROR("Govenor was not set back to %s (was actually %s)!", defaultgov, currentgov);
+		LOG_ERROR("Governor was not set back to %s (was actually %s)!\n", defaultgov, currentgov);
 		return -1;
 	}
 
@@ -444,18 +444,15 @@ int run_gpu_optimisation_tests(struct GameModeConfig *config)
 	/* Grab the expected values */
 	long expected_core = gpuinfo->nv_core;
 	long expected_mem = gpuinfo->nv_mem;
+	char expected_amd_performance_level[CONFIG_VALUE_MAX];
+	strncpy(expected_amd_performance_level, gpuinfo->amd_performance_level, CONFIG_VALUE_MAX);
 
 	/* Get current stats */
 	game_mode_get_gpu(gpuinfo);
-	long original_core = gpuinfo->nv_core;
-	long original_mem = gpuinfo->nv_mem;
-
-	LOG_MSG("Configured with vendor:0x%04x device:%ld nv_core:%ld nv_mem:%ld (nv_perf_level:%ld)\n",
-	        (unsigned int)gpuinfo->vendor,
-	        gpuinfo->device,
-	        expected_core,
-	        expected_mem,
-	        gpuinfo->nv_perf_level);
+	long original_nv_core = gpuinfo->nv_core;
+	long original_nv_mem = gpuinfo->nv_mem;
+	char original_amd_performance_level[CONFIG_VALUE_MAX];
+	strncpy(original_amd_performance_level, gpuinfo->amd_performance_level, CONFIG_VALUE_MAX);
 
 	/* Start gamemode and check the new values */
 	gamemode_request_start();
@@ -467,15 +464,24 @@ int run_gpu_optimisation_tests(struct GameModeConfig *config)
 		return -1;
 	}
 
-	if (gpuinfo->nv_core != expected_core || gpuinfo->nv_mem != expected_mem) {
+	if (gpuinfo->vendor == Vendor_NVIDIA &&
+	    (gpuinfo->nv_core != expected_core || gpuinfo->nv_mem != expected_mem)) {
 		LOG_ERROR(
-		    "Current GPU clocks during gamemode do not match requested values!\n"
+		    "Current Nvidia GPU clocks during gamemode do not match requested values!\n"
 		    "\tnv_core - expected:%ld was:%ld | nv_mem - expected:%ld was:%ld\n",
 		    expected_core,
 		    gpuinfo->nv_core,
 		    expected_mem,
 		    gpuinfo->nv_mem);
 		gpustatus = -1;
+	} else if (gpuinfo->vendor == Vendor_AMD &&
+	           strcmp(expected_amd_performance_level, gpuinfo->amd_performance_level) != 0) {
+		LOG_ERROR(
+		    "Current AMD GPU performance level during gamemode does not match requested value!\n"
+		    "\texpected:%s was:%s\n",
+		    expected_amd_performance_level,
+		    gpuinfo->amd_performance_level);
+		gpustatus = -1;
 	}
 
 	/* End gamemode and check the values have returned */
@@ -487,15 +493,24 @@ int run_gpu_optimisation_tests(struct GameModeConfig *config)
 		return -1;
 	}
 
-	if (gpuinfo->nv_core != original_core || gpuinfo->nv_mem != original_mem) {
+	if (gpuinfo->vendor == Vendor_NVIDIA &&
+	    (gpuinfo->nv_core != original_nv_core || gpuinfo->nv_mem != original_nv_mem)) {
 		LOG_ERROR(
-		    "Current GPU clocks after gamemode do not matcch original values!\n"
+		    "Current Nvidia GPU clocks after gamemode do not matcch original values!\n"
 		    "\tcore - original:%ld was:%ld | nv_mem - original:%ld was:%ld\n",
-		    original_core,
+		    original_nv_core,
 		    gpuinfo->nv_core,
-		    original_mem,
+		    original_nv_mem,
 		    gpuinfo->nv_mem);
 		gpustatus = -1;
+	} else if (gpuinfo->vendor == Vendor_AMD &&
+	           strcmp(original_amd_performance_level, gpuinfo->amd_performance_level) != 0) {
+		LOG_ERROR(
+		    "Current AMD GPU performance level after gamemode does not match requested value!\n"
+		    "\texpected:%s was:%s\n",
+		    original_amd_performance_level,
+		    gpuinfo->amd_performance_level);
+		gpustatus = -1;
 	}
 
 	return gpustatus;

+ 10 - 6
daemon/gamemode.c

@@ -64,7 +64,8 @@ struct GameModeContext {
 
 	char initial_cpu_mode[64]; /**<Only updates when we can */
 
-	struct GameModeGPUInfo *gpu_info; /**<Stored GPU info for the current GPU */
+	struct GameModeGPUInfo *stored_gpu; /**<Stored GPU info for the current GPU */
+	struct GameModeGPUInfo *target_gpu; /**<Target GPU info for the current GPU */
 
 	/* Reaper control */
 	struct {
@@ -112,7 +113,8 @@ void game_mode_context_init(GameModeContext *self)
 	config_init(self->config);
 
 	/* Initialise the current GPU info */
-	game_mode_initialise_gpu(self->config, &self->gpu_info);
+	game_mode_initialise_gpu(self->config, &self->stored_gpu);
+	game_mode_initialise_gpu(self->config, &self->target_gpu);
 
 	pthread_rwlock_init(&self->rwlock, NULL);
 	pthread_mutex_init(&self->reaper.mutex, NULL);
@@ -152,7 +154,8 @@ void game_mode_context_destroy(GameModeContext *self)
 	pthread_mutex_destroy(&self->reaper.mutex);
 
 	/* Destroy the gpu object */
-	game_mode_free_gpu(&self->gpu_info);
+	game_mode_free_gpu(&self->stored_gpu);
+	game_mode_free_gpu(&self->target_gpu);
 
 	/* Destroy the config object */
 	config_destroy(self->config);
@@ -201,8 +204,9 @@ static void game_mode_context_enter(GameModeContext *self)
 	if (config_get_inhibit_screensaver(self->config))
 		game_mode_inhibit_screensaver(true);
 
-	/* Apply GPU optimisations */
-	game_mode_apply_gpu(self->gpu_info, true);
+	/* Apply GPU optimisations by first getting the current values, and then setting the target */
+	game_mode_get_gpu(self->stored_gpu);
+	game_mode_apply_gpu(self->target_gpu);
 
 	/* Run custom scripts last - ensures the above are applied first and these scripts can react to
 	 * them if needed */
@@ -225,7 +229,7 @@ static void game_mode_context_leave(GameModeContext *self)
 	sd_notifyf(0, "STATUS=%sGameMode is currently deactivated.%s\n", "\x1B[1;36m", "\x1B[0m");
 
 	/* Remove GPU optimisations */
-	game_mode_apply_gpu(self->gpu_info, false);
+	game_mode_apply_gpu(self->stored_gpu);
 
 	/* UnInhibit the screensaver */
 	if (config_get_inhibit_screensaver(self->config))

+ 1 - 1
daemon/gamemode.h

@@ -150,5 +150,5 @@ int game_mode_run_client_tests(void);
 typedef struct GameModeGPUInfo GameModeGPUInfo;
 int game_mode_initialise_gpu(GameModeConfig *config, GameModeGPUInfo **info);
 void game_mode_free_gpu(GameModeGPUInfo **info);
-int game_mode_apply_gpu(const GameModeGPUInfo *info, bool apply);
+int game_mode_apply_gpu(const GameModeGPUInfo *info);
 int game_mode_get_gpu(GameModeGPUInfo *info);

+ 6 - 4
daemon/gpu-control.h

@@ -30,6 +30,7 @@ POSSIBILITY OF SUCH DAMAGE.
  */
 
 #pragma once
+#include "daemon_config.h"
 
 /* Enums for GPU vendors */
 enum GPUVendor {
@@ -45,10 +46,11 @@ enum GPUVendor {
 /* Storage for GPU info*/
 struct GameModeGPUInfo {
 	long vendor;
-	long device; /* path to device, ie. /sys/class/drm/card#/ */
+	long device;        /* path to device, ie. /sys/class/drm/card#/ */
+	long nv_perf_level; /* The Nvidia Performance Level to adjust */
 
-	long nv_core; /* Core clock to apply */
-	long nv_mem;  /* Mem clock to apply */
+	long nv_core; /* Nvidia core clock */
+	long nv_mem;  /* Nvidia mem clock */
 
-	long nv_perf_level; /* The Nvidia Performance Level to adjust */
+	char amd_performance_level[CONFIG_VALUE_MAX]; /* The AMD performance level set to */
 };

+ 66 - 32
daemon/gpuclockctl.c

@@ -112,16 +112,6 @@ static int get_gpu_state_nv(struct GameModeGPUInfo *info)
 	return 0;
 }
 
-/**
- * Get the gpu state
- * Populates the struct with the GPU info on the system
- */
-static int get_gpu_state_amd(struct GameModeGPUInfo *info)
-{
-	fprintf(stderr, "Fetching GPU state on AMD is currently unimplemented!\n");
-	return info != NULL;
-}
-
 /**
  * Set the gpu state based on input parameters on Nvidia
  */
@@ -168,13 +158,46 @@ static int set_gpu_state_nv(struct GameModeGPUInfo *info)
 	return 0;
 }
 
+/**
+ * Get the gpu state
+ * Populates the struct with the GPU info on the system
+ */
+static int get_gpu_state_amd(struct GameModeGPUInfo *info)
+{
+	if (info->vendor != Vendor_AMD)
+		return -1;
+
+	/* Get the contents of power_dpm_force_performance_level */
+	char path[64];
+	snprintf(path, 64, AMD_DRM_PATH, info->device, "power_dpm_force_performance_level");
+
+	FILE *file = fopen(path, "r");
+	if (!file) {
+		LOG_ERROR("Could not open %s for read (%s)!\n", path, strerror(errno));
+		return -1;
+	}
+
+	char buff[CONFIG_VALUE_MAX];
+	if (!fgets(buff, CONFIG_VALUE_MAX, file)) {
+		LOG_ERROR("Could not read file %s (%s)!\n", path, strerror(errno));
+		return -1;
+	}
+
+	if (fclose(file) != 0) {
+		LOG_ERROR("Could not close %s after reading (%s)!\n", path, strerror(errno));
+		return -1;
+	}
+
+	/* Copy in the value from the file */
+	strncpy(info->amd_performance_level, buff, CONFIG_VALUE_MAX);
+
+	return info == NULL;
+}
+
 /*
- * Sets the value in a file in the AMDGPU driver config
- * Files are:
- * /sys/class/drm/card0/device/pp_sclk_od
- * /sys/class/drm/card0/device/pp_mclk_od
+ * Simply set an amd drm file to a value
  */
-static int set_gpu_state_amd_file(const char *filename, long device, long value)
+static int set_gpu_state_amd_file(const char *filename, long device, const char *value)
 {
 	char path[64];
 	snprintf(path, 64, AMD_DRM_PATH, device, filename);
@@ -185,7 +208,7 @@ static int set_gpu_state_amd_file(const char *filename, long device, long value)
 		return -1;
 	}
 
-	if (fprintf(file, "%ld", value) < 0) {
+	if (fprintf(file, "%s", value) < 0) {
 		LOG_ERROR("Could not write to %s (%s)!\n", path, strerror(errno));
 		return -1;
 	}
@@ -212,12 +235,17 @@ static int set_gpu_state_amd(struct GameModeGPUInfo *info)
 		print_usage_and_exit();
 	}
 
-	// Set the the nv_core and nv_mem clock speeds using the OverDrive files
-	if (set_gpu_state_amd_file("pp_sclk_od", info->device, info->nv_core) != 0)
-		return -1;
-	if (set_gpu_state_amd_file("pp_mclk_od", info->device, info->nv_mem) != 0)
+	/* First set the amd_performance_level to the chosen setting */
+	if (set_gpu_state_amd_file("power_dpm_force_performance_level",
+	                           info->device,
+	                           info->amd_performance_level) != 0)
 		return -1;
 
+	/* TODO: If amd_performance_level is set to "manual" we need to adjust pp_table and/or
+	   pp_od_clk_voltage see
+	   https://dri.freedesktop.org/docs/drm/gpu/amdgpu.html#gpu-power-thermal-controls-and-monitoring
+	*/
+
 	return 0;
 }
 
@@ -278,38 +306,44 @@ int main(int argc, char *argv[])
 			/* Get nvidia power level */
 			if (get_gpu_state_nv(&info) != 0)
 				exit(EXIT_FAILURE);
+			printf("%ld %ld\n", info.nv_core, info.nv_mem);
 			break;
 		case Vendor_AMD:
 			if (get_gpu_state_amd(&info) != 0)
 				exit(EXIT_FAILURE);
+			printf("%s\n", info.amd_performance_level);
 			break;
 		default:
 			printf("Currently unsupported GPU vendor 0x%04x, doing nothing!\n", (short)info.vendor);
 			break;
 		}
-		printf("%ld %ld\n", info.nv_core, info.nv_mem);
 
-	} else if (argc >= 6 && argc <= 7 && strncmp(argv[3], "set", 3) == 0) {
+	} else if (argc >= 5 && argc <= 7 && strncmp(argv[3], "set", 3) == 0) {
 		/* Get and verify the vendor and device */
 		struct GameModeGPUInfo info;
 		memset(&info, 0, sizeof(info));
 		info.vendor = get_vendor(argv[1]);
 		info.device = get_device(argv[2]);
-		info.nv_core = get_generic_value(argv[4]);
-		info.nv_mem = get_generic_value(argv[5]);
 
-		if (info.vendor == Vendor_NVIDIA && argc > 6)
-			info.nv_perf_level = get_generic_value(argv[6]);
+		switch (info.vendor) {
+		case Vendor_NVIDIA:
+			info.nv_core = get_generic_value(argv[4]);
+			info.nv_mem = get_generic_value(argv[5]);
+			if (argc > 6)
+				info.nv_perf_level = get_generic_value(argv[6]);
+			break;
+		case Vendor_AMD:
+			strncpy(info.amd_performance_level, argv[4], CONFIG_VALUE_MAX);
+			break;
+		default:
+			printf("Currently unsupported GPU vendor 0x%04x, doing nothing!\n", (short)info.vendor);
+			break;
+		}
 
-		printf("gpuclockctl setting nv_core:%ld nv_mem:%ld on device:%ld with vendor 0x%04x\n",
-		       info.nv_core,
-		       info.nv_mem,
+		printf("gpuclockctl setting values on device:%ld with vendor 0x%04x",
 		       info.device,
 		       (unsigned short)info.vendor);
 
-		if (info.vendor == Vendor_NVIDIA)
-			printf("on Performance Level %ld\n", info.nv_perf_level);
-
 		switch (info.vendor) {
 		case Vendor_NVIDIA:
 			return set_gpu_state_nv(&info);

+ 6 - 5
example/gamemode.ini

@@ -50,14 +50,15 @@ inhibit_screensaver=1
 ; Requires the coolbits extension activated in nvidia-xconfig
 ;nv_core_clock_mhz_offset=0
 ;nv_mem_clock_mhz_offset=0
-; This corresponds to the performance level to edit in nvidia-xconfig
+; This corresponds to the performance level to edit in nvidia-xconfig (usually the highest level - on older cards 1, newer cards can be 4 or higher)
 ;nv_perf_level=1
 
-; AMD specific settings (these are percentages applied on top of the baseline, ie. 0 applies no change)
-; Requires the the AMDGPU kernel module
+; AMD specific settings
+; Requires a relatively up to date AMDGPU kernel module
+; See: https://dri.freedesktop.org/docs/drm/gpu/amdgpu.html#gpu-power-thermal-controls-and-monitoring
 ; It is also highly recommended you use lm-sensors (or other available tools) to verify card temperatures
-;amd_core_clock_percentage=0
-;amd_mem_clock_percentage=0
+; This corresponds to power_dpm_force_performance_level, "manual" is not supported for now
+;amd_performance_level=high
 
 [supervisor]
 ; This section controls the new gamemode functions gamemode_request_start_for and gamemode_request_end_for