From 97cfc3bb171f401256c464c42ceb79381cb94ba1 Mon Sep 17 00:00:00 2001
From: Marc Di Luzio <marc.diluzio@gmail.com>
Date: Wed, 6 Feb 2019 18:59:38 +0000
Subject: [PATCH 01/18] Add game_mode_run_feature_tests to start putting
 feature tests

---
 daemon/gamemode-tests.c | 42 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/daemon/gamemode-tests.c b/daemon/gamemode-tests.c
index 0959aea..4c7181d 100644
--- a/daemon/gamemode-tests.c
+++ b/daemon/gamemode-tests.c
@@ -239,6 +239,44 @@ static int run_dual_client_tests(void)
 	return status;
 }
 
+/**
+ * game_mode_run_feature_tests runs a set of tests for each current feature (based on the current
+ * config) returns 0 for success, -1 for failure
+ */
+static int game_mode_run_feature_tests(void)
+{
+	int status = 0;
+	fprintf(stdout, "   *feature tests*\n");
+
+	/* If we reach here, we should assume the basic requests and register functions are working */
+
+	/* Does the CPU governor get set properly? */
+	/* TODO */
+
+	/* Do custom scripts run? */
+	/* TODO */
+
+	/* Does the screensaver get inhibited? */
+	/* TODO */
+
+	/* Do GPU optimisations get applied? */
+	/* TODO */
+
+	/* Was the process reniced? */
+	/* TODO */
+
+	/* Was the scheduling applied? */
+	/* TODO */
+
+	/* Were io priorities changed? */
+	/* TODO */
+
+	if (status != -1)
+		fprintf(stdout, "       *passed*%s\n", status > 0 ? " (with optional failures)" : "");
+
+	return status;
+}
+
 /**
  * game_mode_run_client_tests runs a set of tests of the client code
  * we simply verify that the client can request the status and recieves the correct results
@@ -258,5 +296,9 @@ int game_mode_run_client_tests()
 	if (run_dual_client_tests() != 0)
 		return -1;
 
+	/* Run the feature tests */
+	if (game_mode_run_feature_tests() != 0)
+		return -1;
+
 	return status;
 }

From 7773e5d8b7f68d1865eb10886a96b248076a1a24 Mon Sep 17 00:00:00 2001
From: Marc Di Luzio <marc.diluzio@gmail.com>
Date: Wed, 6 Feb 2019 19:02:37 +0000
Subject: [PATCH 02/18] Use the logging macros for tests

---
 daemon/gamemode-tests.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/daemon/gamemode-tests.c b/daemon/gamemode-tests.c
index 4c7181d..074a14e 100644
--- a/daemon/gamemode-tests.c
+++ b/daemon/gamemode-tests.c
@@ -246,7 +246,7 @@ static int run_dual_client_tests(void)
 static int game_mode_run_feature_tests(void)
 {
 	int status = 0;
-	fprintf(stdout, "   *feature tests*\n");
+	LOG_MSG("   *feature tests*\n");
 
 	/* If we reach here, we should assume the basic requests and register functions are working */
 
@@ -272,7 +272,7 @@ static int game_mode_run_feature_tests(void)
 	/* TODO */
 
 	if (status != -1)
-		fprintf(stdout, "       *passed*%s\n", status > 0 ? " (with optional failures)" : "");
+		LOG_MSG("       *passed*%s\n", status > 0 ? " (with optional failures)" : "");
 
 	return status;
 }

From 6ba74284b9a41cdc28be97eefcd5d5a76d2ebd76 Mon Sep 17 00:00:00 2001
From: Marc Di Luzio <marc.diluzio@gmail.com>
Date: Wed, 6 Feb 2019 19:50:10 +0000
Subject: [PATCH 03/18] Add test to verify that governor setting works

---
 daemon/gamemode-tests.c | 58 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 57 insertions(+), 1 deletion(-)

diff --git a/daemon/gamemode-tests.c b/daemon/gamemode-tests.c
index 074a14e..995921e 100644
--- a/daemon/gamemode-tests.c
+++ b/daemon/gamemode-tests.c
@@ -40,7 +40,9 @@ POSSIBILITY OF SUCH DAMAGE.
 #include <sys/wait.h>
 #include <unistd.h>
 
+#include "daemon_config.h"
 #include "gamemode_client.h"
+#include "governors-query.h"
 
 /* Initial verify step to ensure gamemode isn't already active */
 static int verify_gamemode_initial(void)
@@ -250,8 +252,62 @@ static int game_mode_run_feature_tests(void)
 
 	/* If we reach here, we should assume the basic requests and register functions are working */
 
+	/* Grab the config */
+	/* Note: this config may pick up a local gamemode.ini, or the daemon may have one, we may need
+	 * to cope with that */
+	GameModeConfig *config = config_create();
+	config_init(config);
+
 	/* Does the CPU governor get set properly? */
-	/* TODO */
+	int cpustatus = 0;
+	{
+		/* get the two config parameters we care about */
+		char desiredgov[CONFIG_VALUE_MAX] = { 0 };
+		config_get_desired_governor(config, desiredgov);
+
+		if (desiredgov[0] == '\0')
+			strcpy(desiredgov, "performance");
+
+		char defaultgov[CONFIG_VALUE_MAX] = { 0 };
+		config_get_default_governor(config, defaultgov);
+
+		if (desiredgov[0] == '\0') {
+			const char *currentgov = get_gov_state();
+			if (currentgov) {
+				strncpy(desiredgov, currentgov, CONFIG_VALUE_MAX);
+			} else {
+				LOG_ERROR(
+				    "Could not get current CPU governor state, this indicates an error! See rest "
+				    "of log.\n");
+				cpustatus = -1;
+			}
+		}
+
+		/* Only continue if we had no error before */
+		if (cpustatus == 0) {
+			/* Start gamemode */
+			gamemode_request_start();
+
+			/* 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);
+				cpustatus = -1;
+			}
+
+			/* End gamemode */
+			gamemode_request_end();
+
+			/* 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);
+				cpustatus = -1;
+			}
+		}
+	}
 
 	/* Do custom scripts run? */
 	/* TODO */

From cbf7f975d320f4e7c8ce5b833d62ad9221f9a555 Mon Sep 17 00:00:00 2001
From: Marc Di Luzio <marc.diluzio@gmail.com>
Date: Thu, 7 Feb 2019 17:04:24 +0000
Subject: [PATCH 04/18] Add basic framework to do some script tests

---
 daemon/gamemode-tests.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/daemon/gamemode-tests.c b/daemon/gamemode-tests.c
index 995921e..5622020 100644
--- a/daemon/gamemode-tests.c
+++ b/daemon/gamemode-tests.c
@@ -310,7 +310,26 @@ static int game_mode_run_feature_tests(void)
 	}
 
 	/* Do custom scripts run? */
-	/* TODO */
+	int scriptstatus = 0;
+	{
+		/* Grab the scripts */
+		char scripts[CONFIG_LIST_MAX][CONFIG_VALUE_MAX];
+		memset(scripts, 0, sizeof(scripts));
+		config_get_gamemode_start_scripts(config, scripts);
+
+		if (scripts[0][0] != '\0') {
+			/* Print out the scripts to run */
+			LOG_MSG("Custom scripts:\n");
+
+			int i = 0;
+			while (*scripts[i] != '\0' && i < CONFIG_LIST_MAX)
+				LOG_MSG("%s\n", scripts[i]);
+		}
+
+		/* TODO: Somehow verify these get run
+		 * Possibly by watching if the the binary part gets run?
+		 */
+	}
 
 	/* Does the screensaver get inhibited? */
 	/* TODO */

From f7dce41d8a37321b0e54355cc56d46543bf5a38f Mon Sep 17 00:00:00 2001
From: Marc Di Luzio <marc.diluzio@gmail.com>
Date: Thu, 7 Feb 2019 19:20:41 +0000
Subject: [PATCH 05/18] Improve test logging output

---
 daemon/gamemode-tests.c | 40 +++++++++++++++++++++++++++-------------
 daemon/main.c           | 12 ++----------
 2 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/daemon/gamemode-tests.c b/daemon/gamemode-tests.c
index 5622020..8ebf3e4 100644
--- a/daemon/gamemode-tests.c
+++ b/daemon/gamemode-tests.c
@@ -124,7 +124,7 @@ static int verify_other_client_connected(void)
  */
 static int run_basic_client_tests(void)
 {
-	LOG_MSG("   *basic client tests*\n");
+	LOG_MSG(":: Basic client tests\n");
 
 	/* First verify that gamemode is not currently active on the system
 	 * As well as it being currently installed and queryable
@@ -152,7 +152,7 @@ static int run_basic_client_tests(void)
 	if (verify_deactivated() != 0)
 		return -1;
 
-	LOG_MSG("       *passed*\n");
+	LOG_MSG(":: Passed\n\n");
 
 	return 0;
 }
@@ -165,7 +165,7 @@ static int run_dual_client_tests(void)
 	int status = 0;
 
 	/* Try running some process interop tests */
-	LOG_MSG("   *dual clients tests*\n");
+	LOG_MSG(":: Dual client tests\n");
 
 	/* Get the current path to this binary */
 	char mypath[PATH_MAX];
@@ -227,7 +227,7 @@ static int run_dual_client_tests(void)
 	// Wait for the child to finish up
 	int wstatus;
 	while (waitpid(child, &wstatus, WNOHANG) == 0) {
-		LOG_MSG("   Waiting for child to quit...\n");
+		LOG_MSG("...Waiting for child to quit...\n");
 		usleep(10000);
 	}
 
@@ -236,7 +236,7 @@ static int run_dual_client_tests(void)
 		return -1;
 
 	if (status == 0)
-		LOG_MSG("       *passed*\n");
+		LOG_MSG(":: Passed\n\n");
 
 	return status;
 }
@@ -248,7 +248,7 @@ static int run_dual_client_tests(void)
 static int game_mode_run_feature_tests(void)
 {
 	int status = 0;
-	LOG_MSG("   *feature tests*\n");
+	LOG_MSG(":: Feature tests\n");
 
 	/* If we reach here, we should assume the basic requests and register functions are working */
 
@@ -261,6 +261,8 @@ static int game_mode_run_feature_tests(void)
 	/* Does the CPU governor get set properly? */
 	int cpustatus = 0;
 	{
+		LOG_MSG("::: Verifying CPU governor setting\n");
+
 		/* get the two config parameters we care about */
 		char desiredgov[CONFIG_VALUE_MAX] = { 0 };
 		config_get_desired_governor(config, desiredgov);
@@ -312,6 +314,8 @@ static int game_mode_run_feature_tests(void)
 	/* Do custom scripts run? */
 	int scriptstatus = 0;
 	{
+		LOG_MSG("::: Verifying Scripts\n");
+
 		/* Grab the scripts */
 		char scripts[CONFIG_LIST_MAX][CONFIG_VALUE_MAX];
 		memset(scripts, 0, sizeof(scripts));
@@ -347,7 +351,9 @@ static int game_mode_run_feature_tests(void)
 	/* TODO */
 
 	if (status != -1)
-		LOG_MSG("       *passed*%s\n", status > 0 ? " (with optional failures)" : "");
+		LOG_MSG(":: Passed%s\n\n", status > 0 ? " (with optional failures)" : "");
+	else
+		LOG_ERROR(":: Failed!\n");
 
 	return status;
 }
@@ -361,19 +367,27 @@ static int game_mode_run_feature_tests(void)
 int game_mode_run_client_tests()
 {
 	int status = 0;
-	LOG_MSG("Running tests...\n");
+	LOG_MSG(": Running tests\n\n");
 
 	/* Run the basic tests */
 	if (run_basic_client_tests() != 0)
-		return -1;
+		status = -1;
 
 	/* Run the dual client tests */
 	if (run_dual_client_tests() != 0)
-		return -1;
+		status = -1;
 
-	/* Run the feature tests */
-	if (game_mode_run_feature_tests() != 0)
-		return -1;
+	if (status != 0) {
+		LOG_MSG(": Client tests failed, skipping feature tests\n");
+	} else {
+		/* Run the feature tests */
+		status = game_mode_run_feature_tests();
+	}
+
+	if (status >= 0)
+		LOG_MSG(": All Tests Passed%s!\n", status > 0 ? " (with optional failures)" : "");
+	else
+		LOG_MSG(": Tests Failed!\n");
 
 	return status;
 }
diff --git a/daemon/main.c b/daemon/main.c
index 71b4a9e..5c00e1a 100644
--- a/daemon/main.c
+++ b/daemon/main.c
@@ -155,16 +155,8 @@ int main(int argc, char *argv[])
 			exit(EXIT_SUCCESS);
 			break;
 		case 't':
-			if ((status = game_mode_run_client_tests()) == 0) {
-				LOG_MSG("gamemode tests succeeded\n");
-				exit(EXIT_SUCCESS);
-			} else if (status == -1) {
-				LOG_ERROR("gamemode tests failed\n");
-				exit(EXIT_FAILURE);
-			} else {
-				LOG_ERROR("gamemode test results unknown: %d\n", status);
-				exit(EXIT_FAILURE);
-			}
+			status = game_mode_run_client_tests();
+			exit(status);
 			break;
 		case 'v':
 			LOG_MSG(VERSION_TEXT);

From 6bfedc96921e0cb08dee9bf5f24fc78470ff64e1 Mon Sep 17 00:00:00 2001
From: Marc Di Luzio <marc.diluzio@gmail.com>
Date: Thu, 7 Feb 2019 19:34:44 +0000
Subject: [PATCH 06/18] Call the configured scripts to test them

---
 daemon/gamemode-tests.c | 63 ++++++++++++++++++++++++++++++++---------
 1 file changed, 49 insertions(+), 14 deletions(-)

diff --git a/daemon/gamemode-tests.c b/daemon/gamemode-tests.c
index 8ebf3e4..29cf1ba 100644
--- a/daemon/gamemode-tests.c
+++ b/daemon/gamemode-tests.c
@@ -312,27 +312,62 @@ static int game_mode_run_feature_tests(void)
 	}
 
 	/* Do custom scripts run? */
-	int scriptstatus = 0;
 	{
+		int scriptstatus = 0;
 		LOG_MSG("::: Verifying Scripts\n");
 
-		/* Grab the scripts */
-		char scripts[CONFIG_LIST_MAX][CONFIG_VALUE_MAX];
-		memset(scripts, 0, sizeof(scripts));
-		config_get_gamemode_start_scripts(config, scripts);
-
-		if (scripts[0][0] != '\0') {
-			/* Print out the scripts to run */
-			LOG_MSG("Custom scripts:\n");
+		/* Grab and test the start scripts */
+		char startscripts[CONFIG_LIST_MAX][CONFIG_VALUE_MAX];
+		memset(startscripts, 0, sizeof(startscripts));
+		config_get_gamemode_start_scripts(config, startscripts);
 
+		if (startscripts[0][0] != '\0') {
 			int i = 0;
-			while (*scripts[i] != '\0' && i < CONFIG_LIST_MAX)
-				LOG_MSG("%s\n", scripts[i]);
+			while (*startscripts[i] != '\0' && i < CONFIG_LIST_MAX) {
+				LOG_MSG(":::: Running start script [%s]\n", startscripts[i]);
+
+				int ret = system(startscripts[i]);
+
+				if (ret == 0)
+					LOG_MSG(":::: Passed\n");
+				else {
+					LOG_MSG(":::: Failed!\n");
+					scriptstatus = -1;
+				}
+				i++;
+			}
 		}
 
-		/* TODO: Somehow verify these get run
-		 * Possibly by watching if the the binary part gets run?
-		 */
+		/* Grab and test the end scripts */
+		char endscripts[CONFIG_LIST_MAX][CONFIG_VALUE_MAX];
+		memset(endscripts, 0, sizeof(endscripts));
+		config_get_gamemode_end_scripts(config, endscripts);
+
+		if (endscripts[0][0] != '\0') {
+			int i = 0;
+			while (*endscripts[i] != '\0' && i < CONFIG_LIST_MAX) {
+				LOG_MSG(":::: Running end script [%s]\n", endscripts[i]);
+
+				int ret = system(endscripts[i]);
+
+				if (ret == 0)
+					LOG_MSG(":::: Passed\n");
+				else {
+					LOG_MSG(":::: Failed!\n");
+					scriptstatus = -1;
+				}
+				i++;
+			}
+		}
+
+		if (endscripts[0][0] == '\0' && startscripts[0][0] == '\0')
+			LOG_MSG("::: Passed (no scripts configured to run)\n");
+		else if (scriptstatus == 0)
+			LOG_MSG("::: Passed\n");
+		else {
+			LOG_MSG("::: Failed!\n");
+			status = 1;
+		}
 	}
 
 	/* Does the screensaver get inhibited? */

From 7f5e59b7c4f0c04f094e2ff941de9a1c86986b51 Mon Sep 17 00:00:00 2001
From: Marc Di Luzio <marc.diluzio@gmail.com>
Date: Thu, 7 Feb 2019 19:37:32 +0000
Subject: [PATCH 07/18] Add pass/fail for the CPU tests

---
 daemon/gamemode-tests.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/daemon/gamemode-tests.c b/daemon/gamemode-tests.c
index 29cf1ba..1e088b4 100644
--- a/daemon/gamemode-tests.c
+++ b/daemon/gamemode-tests.c
@@ -259,8 +259,8 @@ static int game_mode_run_feature_tests(void)
 	config_init(config);
 
 	/* Does the CPU governor get set properly? */
-	int cpustatus = 0;
 	{
+		int cpustatus = 0;
 		LOG_MSG("::: Verifying CPU governor setting\n");
 
 		/* get the two config parameters we care about */
@@ -309,6 +309,13 @@ static int game_mode_run_feature_tests(void)
 				cpustatus = -1;
 			}
 		}
+
+		if (cpustatus == 0)
+			LOG_MSG("::: Passed\n");
+		else {
+			LOG_MSG("::: Failed!\n");
+			status = 1;
+		}
 	}
 
 	/* Do custom scripts run? */

From 1bc4ac626aeb2ec29bcdd5b85e0e1ee23cb56826 Mon Sep 17 00:00:00 2001
From: Marc Di Luzio <marc.diluzio@gmail.com>
Date: Fri, 8 Feb 2019 16:36:27 +0000
Subject: [PATCH 08/18] Comment about register features

	Also add comment about org.freedesktop.ScreenSaver not being fully testable
---
 daemon/gamemode-tests.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/daemon/gamemode-tests.c b/daemon/gamemode-tests.c
index 1e088b4..31101c1 100644
--- a/daemon/gamemode-tests.c
+++ b/daemon/gamemode-tests.c
@@ -378,18 +378,15 @@ static int game_mode_run_feature_tests(void)
 	}
 
 	/* Does the screensaver get inhibited? */
-	/* TODO */
+	/* TODO: Unknown if this is testable, org.freedesktop.ScreenSaver has no query method */
 
 	/* Do GPU optimisations get applied? */
 	/* TODO */
 
 	/* Was the process reniced? */
-	/* TODO */
-
 	/* Was the scheduling applied? */
-	/* TODO */
-
 	/* Were io priorities changed? */
+	/* Note: These don't get cleared up on un-register, so will have already been applied */
 	/* TODO */
 
 	if (status != -1)

From 9cd32c63eb33067b52c5a8d856e7ed9454addde7 Mon Sep 17 00:00:00 2001
From: Marc Di Luzio <marc.diluzio@gmail.com>
Date: Sat, 9 Feb 2019 19:07:20 +0000
Subject: [PATCH 09/18] Implement getting GPU clocks for NV

---
 daemon/gpuclockctl.c | 119 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 109 insertions(+), 10 deletions(-)

diff --git a/daemon/gpuclockctl.c b/daemon/gpuclockctl.c
index 51971f0..3e7dc3c 100644
--- a/daemon/gpuclockctl.c
+++ b/daemon/gpuclockctl.c
@@ -36,6 +36,14 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "external-helper.h"
 #include "gpu-control.h"
 
+/* NV constants */
+#define NV_CORE_OFFSET_ATTRIBUTE "GPUGraphicsClockOffset"
+#define NV_MEM_OFFSET_ATTRIBUTE "GPUMemoryTransferRateOffset"
+#define NV_ATTRIBUTE_FORMAT "[gpu:%ld]/%s[%ld]"
+
+/* AMD constants */
+#define AMD_DRM_PATH "/sys/class/drm/card%ld/device/%s"
+
 /* Plausible extras to add:
  * Intel support - https://blog.ffwll.ch/2013/03/overclocking-your-intel-gpu-on-linux.html
  * AMD - Allow setting fan speed as well
@@ -51,20 +59,96 @@ static void print_usage_and_exit(void)
 	exit(EXIT_FAILURE);
 }
 
+static int get_gpu_state_nv(struct GameModeGPUInfo *info)
+{
+	if (info->vendor != Vendor_NVIDIA)
+		return -1;
+
+	char cmd[128] = { 0 };
+	char arg[128] = { 0 };
+	char buf[128] = { 0 };
+
+	/* Set the GPUGraphicsClockOffset parameter */
+	snprintf(arg,
+	         128,
+	         NV_ATTRIBUTE_FORMAT,
+	         info->device,
+	         NV_CORE_OFFSET_ATTRIBUTE,
+	         info->nv_perf_level);
+	snprintf(cmd, 128, "/usr/bin/nvidia-settings -q %s -t", arg );
+
+	FILE* run = popen(cmd, "r");
+	if( run == NULL)
+	{
+		LOG_ERROR("Command [%s] failed to run!\n", cmd);
+		return -1;
+	}
+
+	if( fgets( buf, sizeof(buf)-1, run) == NULL && buf[0] != '\0' )
+	{
+		LOG_ERROR("Failed to get output of [%s]!\n",cmd);
+		return -1;
+	}
+
+	char* end = NULL;
+	info->core = strtol(buf, &end, 10);
+	pclose(run);
+
+	if( end == buf )
+	{
+		LOG_ERROR("Failed to parse output of [%s] output [%s]!\n",cmd, buf);
+		return -1;
+	}
+
+	/* Set the GPUMemoryTransferRateOffset parameter */
+	snprintf(arg,
+	         128,
+	         NV_ATTRIBUTE_FORMAT,
+	         info->device,
+	         NV_MEM_OFFSET_ATTRIBUTE,
+	         info->nv_perf_level);
+	snprintf(cmd, 128, "/usr/bin/nvidia-settings -q %s -t", arg );
+
+	run = popen(cmd, "r");
+	if( run == NULL)
+	{
+		LOG_ERROR("Command [%s] failed to run!\n", cmd);
+		return -1;
+	}
+
+	if( fgets( buf, sizeof(buf)-1, run) == NULL && buf[0] != '\0' )
+	{
+		LOG_ERROR("Failed to get output of [%s]!\n",cmd);
+		return -1;
+	}
+
+	end = NULL;
+	info->mem = strtol(buf, &end, 10);
+	pclose(run);
+
+	if( end == buf )
+	{
+		LOG_ERROR("Failed to parse output of [%s] output [%s]!\n",cmd, buf);
+		return -1;
+	}
+
+	return 0;
+}
+
 /**
  * Get the gpu state
  * Populates the struct with the GPU info on the system
  */
-int get_gpu_state(struct GameModeGPUInfo *info)
+static int get_gpu_state_amd(struct GameModeGPUInfo *info)
 {
-	fprintf(stderr, "Fetching GPU state is currently unimplemented!\n");
+	fprintf(stderr, "Fetching GPU state on AMD is currently unimplemented!\n");
 	return info != NULL;
 }
 
 /**
  * Set the gpu state based on input parameters on Nvidia
  */
-int set_gpu_state_nv(struct GameModeGPUInfo *info)
+static int set_gpu_state_nv(struct GameModeGPUInfo *info)
 {
 	if (info->vendor != Vendor_NVIDIA)
 		return -1;
@@ -75,8 +159,9 @@ int set_gpu_state_nv(struct GameModeGPUInfo *info)
 	char core_arg[128];
 	snprintf(core_arg,
 	         128,
-	         "[gpu:%ld]/GPUGraphicsClockOffset[%ld]=%ld",
+	         NV_ATTRIBUTE_FORMAT "=%ld",
 	         info->device,
+	         NV_CORE_OFFSET_ATTRIBUTE,
 	         info->nv_perf_level,
 	         info->core);
 	const char *exec_args_core[] = { "/usr/bin/nvidia-settings", "-a", core_arg, NULL };
@@ -89,8 +174,9 @@ int set_gpu_state_nv(struct GameModeGPUInfo *info)
 	char mem_arg[128];
 	snprintf(mem_arg,
 	         128,
-	         "[gpu:%ld]/GPUMemoryTransferRateOffset[%ld]=%ld",
+	         NV_ATTRIBUTE_FORMAT "=%ld",
 	         info->device,
+	         NV_MEM_OFFSET_ATTRIBUTE,
 	         info->nv_perf_level,
 	         info->mem);
 	const char *exec_args_mem[] = { "/usr/bin/nvidia-settings", "-a", mem_arg, NULL };
@@ -110,9 +196,8 @@ int set_gpu_state_nv(struct GameModeGPUInfo *info)
  */
 static int set_gpu_state_amd_file(const char *filename, long device, long value)
 {
-	const char *drm_path = "/sys/class/drm/card%ld/device/%s";
 	char path[64];
-	snprintf(path, 64, drm_path, device, filename);
+	snprintf(path, 64, AMD_DRM_PATH, device, filename);
 
 	FILE *file = fopen(path, "w");
 	if (!file) {
@@ -136,7 +221,7 @@ static int set_gpu_state_amd_file(const char *filename, long device, long value)
 /**
  * Set the gpu state based on input parameters on amd
  */
-int set_gpu_state_amd(struct GameModeGPUInfo *info)
+static int set_gpu_state_amd(struct GameModeGPUInfo *info)
 {
 	if (info->vendor != Vendor_AMD)
 		return -1;
@@ -191,15 +276,29 @@ static long get_generic_value(const char *val)
  */
 int main(int argc, char *argv[])
 {
-	if (argc == 4 && strncmp(argv[3], "get", 3) == 0) {
+	if (argc >= 4 && strncmp(argv[3], "get", 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]);
 
+		if (info.vendor == Vendor_NVIDIA)
+			info.nv_perf_level = get_generic_value(argv[4]);
+
 		/* Fetch the state and print it out */
-		get_gpu_state(&info);
+		switch (info.vendor) {
+		case Vendor_NVIDIA:
+			/* Get nvidia power level */
+			get_gpu_state_nv(&info);
+			break;
+		case Vendor_AMD:
+			get_gpu_state_amd(&info);
+			break;
+		default:
+			printf("Currently unsupported GPU vendor 0x%04x, doing nothing!\n", (short)info.vendor);
+			break;
+		}
 		printf("%ld %ld\n", info.core, info.mem);
 
 	} else if (argc >= 6 && argc <= 7 && strncmp(argv[3], "set", 3) == 0) {

From 392fb221dc71c05a268ba75c14f6154103066d7c Mon Sep 17 00:00:00 2001
From: Marc Di Luzio <marc.diluzio@gmail.com>
Date: Sat, 9 Feb 2019 19:45:44 +0000
Subject: [PATCH 10/18] Return failure on failure, and don't try and read past
 the end of argv

---
 daemon/gpuclockctl.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/daemon/gpuclockctl.c b/daemon/gpuclockctl.c
index 3e7dc3c..78501b5 100644
--- a/daemon/gpuclockctl.c
+++ b/daemon/gpuclockctl.c
@@ -283,17 +283,19 @@ int main(int argc, char *argv[])
 		info.vendor = get_vendor(argv[1]);
 		info.device = get_device(argv[2]);
 
-		if (info.vendor == Vendor_NVIDIA)
+		if (info.vendor == Vendor_NVIDIA && argc > 4)
 			info.nv_perf_level = get_generic_value(argv[4]);
 
 		/* Fetch the state and print it out */
 		switch (info.vendor) {
 		case Vendor_NVIDIA:
 			/* Get nvidia power level */
-			get_gpu_state_nv(&info);
+			if( get_gpu_state_nv(&info) != 0 )
+				exit(EXIT_FAILURE);
 			break;
 		case Vendor_AMD:
-			get_gpu_state_amd(&info);
+			if( get_gpu_state_amd(&info) != 0)
+				exit(EXIT_FAILURE);
 			break;
 		default:
 			printf("Currently unsupported GPU vendor 0x%04x, doing nothing!\n", (short)info.vendor);
@@ -316,7 +318,7 @@ int main(int argc, char *argv[])
 		info.core = get_generic_value(argv[4]);
 		info.mem = get_generic_value(argv[5]);
 
-		if (info.vendor == Vendor_NVIDIA)
+		if (info.vendor == Vendor_NVIDIA && argc > 6)
 			info.nv_perf_level = get_generic_value(argv[6]);
 
 		printf("gpuclockctl setting core:%ld mem:%ld on device:%ld with vendor 0x%04x\n",

From e31a811946ca26e86e41d3837ab67f49ef417244 Mon Sep 17 00:00:00 2001
From: Marc Di Luzio <marc.diluzio@gmail.com>
Date: Sat, 9 Feb 2019 20:11:07 +0000
Subject: [PATCH 11/18] Add run_external_process_get_output function to get
 output as well

---
 daemon/external-helper.c | 76 +++++++++++++++++++++++++++++++++-------
 daemon/external-helper.h |  5 +++
 daemon/gpuclockctl.c     | 54 ++++++++--------------------
 3 files changed, 82 insertions(+), 53 deletions(-)

diff --git a/daemon/external-helper.c b/daemon/external-helper.c
index da3db90..ca19e96 100644
--- a/daemon/external-helper.c
+++ b/daemon/external-helper.c
@@ -31,6 +31,7 @@ POSSIBILITY OF SUCH DAMAGE.
 
 #define _GNU_SOURCE
 
+#include "external-helper.h"
 #include "logging.h"
 
 #include <linux/limits.h>
@@ -45,8 +46,6 @@ int run_external_process(const char *const *exec_args)
 {
 	pid_t p;
 	int status = 0;
-	int ret = 0;
-	int r = -1;
 
 	if ((p = fork()) < 0) {
 		LOG_ERROR("Failed to fork(): %s\n", strerror(errno));
@@ -59,23 +58,74 @@ int run_external_process(const char *const *exec_args)
 		 *   bindings that these objects are completely constant.
 		 * http://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html
 		 */
-		if ((r = execv(exec_args[0], (char *const *)exec_args)) != 0) {
+		if (execv(exec_args[0], (char *const *)exec_args) != 0) {
 			LOG_ERROR("Failed to execute external process: %s %s\n", exec_args[0], strerror(errno));
 			exit(EXIT_FAILURE);
 		}
 		_exit(EXIT_SUCCESS);
-	} else {
-		if (waitpid(p, &status, 0) < 0) {
-			LOG_ERROR("Failed to waitpid(%d): %s\n", (int)p, strerror(errno));
-			return -1;
-		}
-		/* i.e. sigsev */
-		if (!WIFEXITED(status)) {
-			LOG_ERROR("Child process '%s' exited abnormally\n", exec_args[0]);
-		}
 	}
 
-	if ((ret = WEXITSTATUS(status)) != 0) {
+	if (waitpid(p, &status, 0) < 0) {
+		LOG_ERROR("Failed to waitpid(%d): %s\n", (int)p, strerror(errno));
+		return -1;
+	}
+
+	/* i.e. sigsev */
+	if (!WIFEXITED(status)) {
+		LOG_ERROR("Child process '%s' exited abnormally\n", exec_args[0]);
+	} else if (WEXITSTATUS(status) != 0) {
+		LOG_ERROR("External process failed\n");
+		return -1;
+	}
+
+	return 0;
+}
+
+/**
+ * Call an external process and get output
+ */
+int run_external_process_get_output(const char *const *exec_args, char buffer[EXTERNAL_BUFFER_MAX])
+{
+	pid_t p;
+	int status = 0;
+	int pipes[2];
+
+	if (pipe(pipes) == -1) {
+		LOG_ERROR("Could not create pipe: %s!\n", strerror(errno));
+		return -1;
+	}
+
+	if ((p = fork()) < 0) {
+		LOG_ERROR("Failed to fork(): %s\n", strerror(errno));
+		return false;
+	} else if (p == 0) {
+		/* Send STDOUT to the pipe */
+		dup2(pipes[1], STDOUT_FILENO);
+		close(pipes[0]);
+		close(pipes[1]);
+		/* Launch the process */
+		if (execv(exec_args[0], (char *const *)exec_args) != 0) {
+			LOG_ERROR("Failed to execute external process %s: %s\n", exec_args[0], strerror(errno));
+			exit(EXIT_FAILURE);
+		}
+		_exit(EXIT_SUCCESS);
+	}
+
+	close(pipes[1]);
+	if (read(pipes[0], buffer, EXTERNAL_BUFFER_MAX) < 0) {
+		LOG_ERROR("Failed to read from process %s: %s\n", exec_args[0], strerror(errno));
+		return -1;
+	}
+
+	if (waitpid(p, &status, 0) < 0) {
+		LOG_ERROR("Failed to waitpid(%d): %s\n", (int)p, strerror(errno));
+		return -1;
+	}
+
+	/* i.e. sigsev */
+	if (!WIFEXITED(status)) {
+		LOG_ERROR("Child process '%s' exited abnormally\n", exec_args[0]);
+	} else if (WEXITSTATUS(status) != 0) {
 		LOG_ERROR("External process failed\n");
 		return -1;
 	}
diff --git a/daemon/external-helper.h b/daemon/external-helper.h
index 9d51db8..0febe20 100644
--- a/daemon/external-helper.h
+++ b/daemon/external-helper.h
@@ -31,5 +31,10 @@ POSSIBILITY OF SUCH DAMAGE.
 
 #pragma once
 
+#define EXTERNAL_BUFFER_MAX 1024
+
 /* Run an external process and capture the return value */
 int run_external_process(const char *const *exec_args);
+
+/* Run an external process and capture the return value as well as output */
+int run_external_process_get_output(const char *const *exec_args, char buffer[EXTERNAL_BUFFER_MAX]);
diff --git a/daemon/gpuclockctl.c b/daemon/gpuclockctl.c
index 78501b5..e1ac086 100644
--- a/daemon/gpuclockctl.c
+++ b/daemon/gpuclockctl.c
@@ -64,9 +64,9 @@ static int get_gpu_state_nv(struct GameModeGPUInfo *info)
 	if (info->vendor != Vendor_NVIDIA)
 		return -1;
 
-	char cmd[128] = { 0 };
 	char arg[128] = { 0 };
-	char buf[128] = { 0 };
+	char buf[EXTERNAL_BUFFER_MAX] = { 0 };
+	char *end;
 
 	/* Set the GPUGraphicsClockOffset parameter */
 	snprintf(arg,
@@ -75,28 +75,15 @@ static int get_gpu_state_nv(struct GameModeGPUInfo *info)
 	         info->device,
 	         NV_CORE_OFFSET_ATTRIBUTE,
 	         info->nv_perf_level);
-	snprintf(cmd, 128, "/usr/bin/nvidia-settings -q %s -t", arg );
-
-	FILE* run = popen(cmd, "r");
-	if( run == NULL)
-	{
-		LOG_ERROR("Command [%s] failed to run!\n", cmd);
+	const char *exec_args_core[] = { "/usr/bin/nvidia-settings", "-q", arg, "-t", NULL };
+	if (run_external_process_get_output(exec_args_core, buf) != 0) {
+		LOG_ERROR("Failed to set %s!\n", arg);
 		return -1;
 	}
 
-	if( fgets( buf, sizeof(buf)-1, run) == NULL && buf[0] != '\0' )
-	{
-		LOG_ERROR("Failed to get output of [%s]!\n",cmd);
-		return -1;
-	}
-
-	char* end = NULL;
 	info->core = strtol(buf, &end, 10);
-	pclose(run);
-
-	if( end == buf )
-	{
-		LOG_ERROR("Failed to parse output of [%s] output [%s]!\n",cmd, buf);
+	if (end == buf) {
+		LOG_ERROR("Failed to parse output for \"%s\" output was \"%s\"!\n", arg, buf);
 		return -1;
 	}
 
@@ -107,28 +94,15 @@ static int get_gpu_state_nv(struct GameModeGPUInfo *info)
 	         info->device,
 	         NV_MEM_OFFSET_ATTRIBUTE,
 	         info->nv_perf_level);
-	snprintf(cmd, 128, "/usr/bin/nvidia-settings -q %s -t", arg );
-
-	run = popen(cmd, "r");
-	if( run == NULL)
-	{
-		LOG_ERROR("Command [%s] failed to run!\n", cmd);
+	const char *exec_args_mem[] = { "/usr/bin/nvidia-settings", "-q", arg, "-t", NULL };
+	if (run_external_process_get_output(exec_args_mem, buf) != 0) {
+		LOG_ERROR("Failed to set %s!\n", arg);
 		return -1;
 	}
 
-	if( fgets( buf, sizeof(buf)-1, run) == NULL && buf[0] != '\0' )
-	{
-		LOG_ERROR("Failed to get output of [%s]!\n",cmd);
-		return -1;
-	}
-
-	end = NULL;
 	info->mem = strtol(buf, &end, 10);
-	pclose(run);
-
-	if( end == buf )
-	{
-		LOG_ERROR("Failed to parse output of [%s] output [%s]!\n",cmd, buf);
+	if (end == buf) {
+		LOG_ERROR("Failed to parse output for \"%s\" output was \"%s\"!\n", arg, buf);
 		return -1;
 	}
 
@@ -290,11 +264,11 @@ int main(int argc, char *argv[])
 		switch (info.vendor) {
 		case Vendor_NVIDIA:
 			/* Get nvidia power level */
-			if( get_gpu_state_nv(&info) != 0 )
+			if (get_gpu_state_nv(&info) != 0)
 				exit(EXIT_FAILURE);
 			break;
 		case Vendor_AMD:
-			if( get_gpu_state_amd(&info) != 0)
+			if (get_gpu_state_amd(&info) != 0)
 				exit(EXIT_FAILURE);
 			break;
 		default:

From 20a4862888aa48aefb7aa605d00a35f25d3ee517 Mon Sep 17 00:00:00 2001
From: Marc Di Luzio <marc.diluzio@gmail.com>
Date: Sat, 9 Feb 2019 20:29:06 +0000
Subject: [PATCH 12/18] Add functionality to get the current GPU information in
 the daemon

---
 daemon/gamemode-gpu.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 daemon/gamemode.h     |  1 +
 2 files changed, 44 insertions(+)

diff --git a/daemon/gamemode-gpu.c b/daemon/gamemode-gpu.c
index adc9f37..ecc01a2 100644
--- a/daemon/gamemode-gpu.c
+++ b/daemon/gamemode-gpu.c
@@ -220,3 +220,46 @@ int game_mode_apply_gpu(const GameModeGPUInfo *info, bool apply)
 
 	return 0;
 }
+
+int game_mode_get_gpu(GameModeGPUInfo *info)
+{
+	if (!info)
+		return 0;
+
+	/* 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_perf_level[4];
+	snprintf(nv_perf_level, 4, "%ld", info->nv_perf_level);
+
+	// Set up our command line to pass to gpuclockctl
+	// This doesn't need pkexec as get does not need elevated perms
+	const char *const exec_args[] = {
+		LIBEXECDIR "/gpuclockctl",
+		vendor,
+		device,
+		"get",
+		info->vendor == Vendor_NVIDIA ? nv_perf_level : NULL, /* Only use this if Nvidia */
+		NULL,
+	};
+
+	char buffer[EXTERNAL_BUFFER_MAX] = { 0 };
+	if (run_external_process_get_output(exec_args, buffer) != 0) {
+		LOG_ERROR("Failed to call gpuclockctl, could get values!\n");
+		return -1;
+	}
+
+	int core = 0;
+	int mem = 0;
+	if (sscanf(buffer, "%i %i", &core, &mem) != 2) {
+		LOG_ERROR("Failed to parse gpuclockctl output: %s\n", buffer);
+		return -1;
+	}
+
+	info->core = core;
+	info->mem = mem;
+
+	return 0;
+}
diff --git a/daemon/gamemode.h b/daemon/gamemode.h
index c70c048..6e9124f 100644
--- a/daemon/gamemode.h
+++ b/daemon/gamemode.h
@@ -144,3 +144,4 @@ 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_get_gpu(GameModeGPUInfo *info);

From a9572e689698fe90731fcb3aafce8aceb7c9e26a Mon Sep 17 00:00:00 2001
From: Marc Di Luzio <marc.diluzio@gmail.com>
Date: Sat, 9 Feb 2019 20:29:27 +0000
Subject: [PATCH 13/18] Begin GPU test implementation

---
 daemon/gamemode-tests.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/daemon/gamemode-tests.c b/daemon/gamemode-tests.c
index 31101c1..86226bc 100644
--- a/daemon/gamemode-tests.c
+++ b/daemon/gamemode-tests.c
@@ -43,6 +43,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "daemon_config.h"
 #include "gamemode_client.h"
 #include "governors-query.h"
+#include "gpu-control.h"
 
 /* Initial verify step to ensure gamemode isn't already active */
 static int verify_gamemode_initial(void)
@@ -381,7 +382,23 @@ static int game_mode_run_feature_tests(void)
 	/* TODO: Unknown if this is testable, org.freedesktop.ScreenSaver has no query method */
 
 	/* Do GPU optimisations get applied? */
-	/* TODO */
+	{
+		/* First get current GPU values */
+		GameModeGPUInfo gpuinfo;
+		gpuinfo.device = config_get_gpu_device(config);
+		gpuinfo.vendor = config_get_gpu_vendor(config);
+
+		if( gpuinfo.vendor == Vendor_NVIDIA )
+			gpuinfo.nv_perf_level = config_get_nv_perf_level(config);
+
+		if( game_mode_get_gpu(&gpuinfo) != 0 )
+		{
+			LOG_ERROR("Could not get current GPU info, see above!\n");
+			status = 1;
+		}
+
+		/* TODO continue to run gamemode and check GPU stats */
+	}
 
 	/* Was the process reniced? */
 	/* Was the scheduling applied? */

From 784cb0053d958da9053a558ea3251377b0df414e Mon Sep 17 00:00:00 2001
From: Marc Di Luzio <marc.diluzio@gmail.com>
Date: Sat, 9 Feb 2019 21:04:09 +0000
Subject: [PATCH 14/18] Extract testing to individual functions

---
 daemon/gamemode-tests.c | 268 +++++++++++++++++++++++-----------------
 1 file changed, 156 insertions(+), 112 deletions(-)

diff --git a/daemon/gamemode-tests.c b/daemon/gamemode-tests.c
index 86226bc..ba76d14 100644
--- a/daemon/gamemode-tests.c
+++ b/daemon/gamemode-tests.c
@@ -242,6 +242,142 @@ static int run_dual_client_tests(void)
 	return status;
 }
 
+static int run_cpu_governor_tests(struct GameModeConfig *config)
+{
+	/* get the two config parameters we care about */
+	char desiredgov[CONFIG_VALUE_MAX] = { 0 };
+	config_get_desired_governor(config, desiredgov);
+
+	if (desiredgov[0] == '\0')
+		strcpy(desiredgov, "performance");
+
+	char defaultgov[CONFIG_VALUE_MAX] = { 0 };
+	config_get_default_governor(config, defaultgov);
+
+	if (desiredgov[0] == '\0') {
+		const char *currentgov = get_gov_state();
+		if (currentgov) {
+			strncpy(desiredgov, currentgov, CONFIG_VALUE_MAX);
+		} else {
+			LOG_ERROR(
+			    "Could not get current CPU governor state, this indicates an error! See rest "
+			    "of log.\n");
+			return -1;
+		}
+	}
+
+	/* Start gamemode */
+	gamemode_request_start();
+
+	/* 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);
+		gamemode_request_end();
+		return -1;
+	}
+
+	/* End gamemode */
+	gamemode_request_end();
+
+	/* 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);
+		return -1;
+	}
+
+	return 0;
+}
+
+static int run_custom_scripts_tests(struct GameModeConfig *config)
+{
+	int scriptstatus = 0;
+
+	/* Grab and test the start scripts */
+	char startscripts[CONFIG_LIST_MAX][CONFIG_VALUE_MAX];
+	memset(startscripts, 0, sizeof(startscripts));
+	config_get_gamemode_start_scripts(config, startscripts);
+
+	if (startscripts[0][0] != '\0') {
+		int i = 0;
+		while (*startscripts[i] != '\0' && i < CONFIG_LIST_MAX) {
+			LOG_MSG(":::: Running start script [%s]\n", startscripts[i]);
+
+			int ret = system(startscripts[i]);
+
+			if (ret == 0)
+				LOG_MSG(":::: Passed\n");
+			else {
+				LOG_MSG(":::: Failed!\n");
+				scriptstatus = -1;
+			}
+			i++;
+		}
+	}
+
+	/* Grab and test the end scripts */
+	char endscripts[CONFIG_LIST_MAX][CONFIG_VALUE_MAX];
+	memset(endscripts, 0, sizeof(endscripts));
+	config_get_gamemode_end_scripts(config, endscripts);
+
+	if (endscripts[0][0] != '\0') {
+		int i = 0;
+		while (*endscripts[i] != '\0' && i < CONFIG_LIST_MAX) {
+			LOG_MSG(":::: Running end script [%s]\n", endscripts[i]);
+
+			int ret = system(endscripts[i]);
+
+			if (ret == 0)
+				LOG_MSG(":::: Passed\n");
+			else {
+				LOG_MSG(":::: Failed!\n");
+				scriptstatus = -1;
+			}
+			i++;
+		}
+	}
+
+	/* Specal value for no scripts */
+	if (endscripts[0][0] == '\0' && startscripts[0][0] == '\0')
+		return 1;
+
+	return scriptstatus;
+}
+
+int run_gpu_optimisation_tests(struct GameModeConfig *config)
+{
+	/* First check if these are turned on */
+	char apply[CONFIG_VALUE_MAX];
+	config_get_apply_gpu_optimisations(config, apply);
+	if (strlen(apply) == 0) {
+		/* Special value for disabled */
+		return 1;
+	} else if (strncmp(apply, "accept-responsibility", CONFIG_VALUE_MAX) != 0) {
+		LOG_ERROR(
+		    "apply_gpu_optimisations set to value other than \"accept-responsibility\" (%s), will "
+		    "not apply GPU optimisations!\n",
+		    apply);
+		return -1;
+	}
+
+	/* Get current GPU values */
+	GameModeGPUInfo gpuinfo;
+	gpuinfo.device = config_get_gpu_device(config);
+	gpuinfo.vendor = config_get_gpu_vendor(config);
+
+	if (gpuinfo.vendor == Vendor_NVIDIA)
+		gpuinfo.nv_perf_level = config_get_nv_perf_level(config);
+
+	if (game_mode_get_gpu(&gpuinfo) != 0) {
+		LOG_ERROR("Could not get current GPU info, see above!\n");
+		return -1;
+	}
+
+	/* TODO continue to run gamemode and check GPU stats */
+	return 0;
+}
+
 /**
  * game_mode_run_feature_tests runs a set of tests for each current feature (based on the current
  * config) returns 0 for success, -1 for failure
@@ -261,119 +397,46 @@ static int game_mode_run_feature_tests(void)
 
 	/* Does the CPU governor get set properly? */
 	{
-		int cpustatus = 0;
 		LOG_MSG("::: Verifying CPU governor setting\n");
 
-		/* get the two config parameters we care about */
-		char desiredgov[CONFIG_VALUE_MAX] = { 0 };
-		config_get_desired_governor(config, desiredgov);
-
-		if (desiredgov[0] == '\0')
-			strcpy(desiredgov, "performance");
-
-		char defaultgov[CONFIG_VALUE_MAX] = { 0 };
-		config_get_default_governor(config, defaultgov);
-
-		if (desiredgov[0] == '\0') {
-			const char *currentgov = get_gov_state();
-			if (currentgov) {
-				strncpy(desiredgov, currentgov, CONFIG_VALUE_MAX);
-			} else {
-				LOG_ERROR(
-				    "Could not get current CPU governor state, this indicates an error! See rest "
-				    "of log.\n");
-				cpustatus = -1;
-			}
-		}
-
-		/* Only continue if we had no error before */
-		if (cpustatus == 0) {
-			/* Start gamemode */
-			gamemode_request_start();
-
-			/* 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);
-				cpustatus = -1;
-			}
-
-			/* End gamemode */
-			gamemode_request_end();
-
-			/* 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);
-				cpustatus = -1;
-			}
-		}
+		int cpustatus = run_cpu_governor_tests(config);
 
 		if (cpustatus == 0)
 			LOG_MSG("::: Passed\n");
 		else {
 			LOG_MSG("::: Failed!\n");
+			// Consider the CPU governor feature required
 			status = 1;
 		}
 	}
 
 	/* Do custom scripts run? */
 	{
-		int scriptstatus = 0;
 		LOG_MSG("::: Verifying Scripts\n");
+		int scriptstatus = run_custom_scripts_tests(config);
 
-		/* Grab and test the start scripts */
-		char startscripts[CONFIG_LIST_MAX][CONFIG_VALUE_MAX];
-		memset(startscripts, 0, sizeof(startscripts));
-		config_get_gamemode_start_scripts(config, startscripts);
-
-		if (startscripts[0][0] != '\0') {
-			int i = 0;
-			while (*startscripts[i] != '\0' && i < CONFIG_LIST_MAX) {
-				LOG_MSG(":::: Running start script [%s]\n", startscripts[i]);
-
-				int ret = system(startscripts[i]);
-
-				if (ret == 0)
-					LOG_MSG(":::: Passed\n");
-				else {
-					LOG_MSG(":::: Failed!\n");
-					scriptstatus = -1;
-				}
-				i++;
-			}
-		}
-
-		/* Grab and test the end scripts */
-		char endscripts[CONFIG_LIST_MAX][CONFIG_VALUE_MAX];
-		memset(endscripts, 0, sizeof(endscripts));
-		config_get_gamemode_end_scripts(config, endscripts);
-
-		if (endscripts[0][0] != '\0') {
-			int i = 0;
-			while (*endscripts[i] != '\0' && i < CONFIG_LIST_MAX) {
-				LOG_MSG(":::: Running end script [%s]\n", endscripts[i]);
-
-				int ret = system(endscripts[i]);
-
-				if (ret == 0)
-					LOG_MSG(":::: Passed\n");
-				else {
-					LOG_MSG(":::: Failed!\n");
-					scriptstatus = -1;
-				}
-				i++;
-			}
-		}
-
-		if (endscripts[0][0] == '\0' && startscripts[0][0] == '\0')
+		if (scriptstatus == 1)
 			LOG_MSG("::: Passed (no scripts configured to run)\n");
 		else if (scriptstatus == 0)
 			LOG_MSG("::: Passed\n");
 		else {
 			LOG_MSG("::: Failed!\n");
+			// Any custom scripts should be expected to work
+			status = 1;
+		}
+	}
+
+	/* Do GPU optimisations get applied? */
+	{
+		int gpustatus = run_gpu_optimisation_tests(config);
+
+		if (gpustatus == 1)
+			LOG_MSG("::: Passed (gpu optimisations not configured to run)\n");
+		else if (gpustatus == 0)
+			LOG_MSG("::: Passed\n");
+		else {
+			LOG_MSG("::: Failed!\n");
+			// Any custom scripts should be expected to work
 			status = 1;
 		}
 	}
@@ -381,25 +444,6 @@ static int game_mode_run_feature_tests(void)
 	/* Does the screensaver get inhibited? */
 	/* TODO: Unknown if this is testable, org.freedesktop.ScreenSaver has no query method */
 
-	/* Do GPU optimisations get applied? */
-	{
-		/* First get current GPU values */
-		GameModeGPUInfo gpuinfo;
-		gpuinfo.device = config_get_gpu_device(config);
-		gpuinfo.vendor = config_get_gpu_vendor(config);
-
-		if( gpuinfo.vendor == Vendor_NVIDIA )
-			gpuinfo.nv_perf_level = config_get_nv_perf_level(config);
-
-		if( game_mode_get_gpu(&gpuinfo) != 0 )
-		{
-			LOG_ERROR("Could not get current GPU info, see above!\n");
-			status = 1;
-		}
-
-		/* TODO continue to run gamemode and check GPU stats */
-	}
-
 	/* Was the process reniced? */
 	/* Was the scheduling applied? */
 	/* Were io priorities changed? */

From e36a172144e1ef2d6d7eb785bd6328bdaf1ede21 Mon Sep 17 00:00:00 2001
From: Marc Di Luzio <marc.diluzio@gmail.com>
Date: Sat, 9 Feb 2019 21:24:26 +0000
Subject: [PATCH 15/18] Add full test for GPU settings

---
 daemon/gamemode-tests.c | 73 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 71 insertions(+), 2 deletions(-)

diff --git a/daemon/gamemode-tests.c b/daemon/gamemode-tests.c
index ba76d14..e28adcc 100644
--- a/daemon/gamemode-tests.c
+++ b/daemon/gamemode-tests.c
@@ -347,6 +347,8 @@ static int run_custom_scripts_tests(struct GameModeConfig *config)
 
 int run_gpu_optimisation_tests(struct GameModeConfig *config)
 {
+	int gpustatus = 0;
+
 	/* First check if these are turned on */
 	char apply[CONFIG_VALUE_MAX];
 	config_get_apply_gpu_optimisations(config, apply);
@@ -374,8 +376,74 @@ int run_gpu_optimisation_tests(struct GameModeConfig *config)
 		return -1;
 	}
 
-	/* TODO continue to run gamemode and check GPU stats */
-	return 0;
+	/* Store the original values */
+	long original_core = gpuinfo.core;
+	long original_mem = gpuinfo.mem;
+
+	/* Grab the expected values */
+	long expected_core = 0;
+	long expected_mem = 0;
+	switch (gpuinfo.vendor) {
+	case Vendor_NVIDIA:
+		expected_core = config_get_nv_core_clock_mhz_offset(config);
+		expected_mem = config_get_nv_mem_clock_mhz_offset(config);
+		break;
+	case Vendor_AMD:
+		expected_core = config_get_amd_core_clock_percentage(config);
+		expected_mem = config_get_amd_mem_clock_percentage(config);
+		break;
+	default:
+		LOG_ERROR("Configured for unsupported GPU vendor 0x%04x!\n", (unsigned int)gpuinfo.vendor);
+		return -1;
+	}
+
+	LOG_MSG("Configured with vendor:0x%04x device:%ld core:%ld mem:%ld (nv_perf_level:%ld)\n",
+	        (unsigned int)gpuinfo.vendor,
+	        gpuinfo.device,
+	        expected_core,
+	        expected_mem,
+	        gpuinfo.nv_perf_level);
+
+	/* Start gamemode and check the new values */
+	gamemode_request_start();
+
+	if (game_mode_get_gpu(&gpuinfo) != 0) {
+		LOG_ERROR("Could not get current GPU info, see above!\n");
+		gamemode_request_end();
+		return -1;
+	}
+
+	if (gpuinfo.core != expected_core || gpuinfo.mem != expected_mem) {
+		LOG_ERROR(
+		    "Current GPU clocks during gamemode do not match requested values!\n"
+		    "\tcore - expected:%ld was:%ld | mem - expected:%ld was:%ld\n",
+		    expected_core,
+		    gpuinfo.core,
+		    expected_mem,
+		    gpuinfo.mem);
+		gpustatus = -1;
+	}
+
+	/* End gamemode and check the values have returned */
+	gamemode_request_end();
+
+	if (game_mode_get_gpu(&gpuinfo) != 0) {
+		LOG_ERROR("Could not get current GPU info, see above!\n");
+		return -1;
+	}
+
+	if (gpuinfo.core != original_core || gpuinfo.mem != original_mem) {
+		LOG_ERROR(
+		    "Current GPU clocks after gamemode do not matcch original values!\n"
+		    "\tcore - original:%ld was:%ld | mem - original:%ld was:%ld\n",
+		    original_core,
+		    gpuinfo.core,
+		    original_mem,
+		    gpuinfo.mem);
+		gpustatus = -1;
+	}
+
+	return gpustatus;
 }
 
 /**
@@ -428,6 +496,7 @@ static int game_mode_run_feature_tests(void)
 
 	/* Do GPU optimisations get applied? */
 	{
+		LOG_MSG("::: Verifying GPU Optimisations\n");
 		int gpustatus = run_gpu_optimisation_tests(config);
 
 		if (gpustatus == 1)

From ab5fdad3cb7234741dbac60531cdaa36256b41bc Mon Sep 17 00:00:00 2001
From: Marc Di Luzio <marc.diluzio@gmail.com>
Date: Sun, 10 Feb 2019 12:30:29 +0000
Subject: [PATCH 16/18] Give the child more time to quit

---
 daemon/gamemode-tests.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/daemon/gamemode-tests.c b/daemon/gamemode-tests.c
index e28adcc..0b97d4c 100644
--- a/daemon/gamemode-tests.c
+++ b/daemon/gamemode-tests.c
@@ -223,13 +223,13 @@ static int run_dual_client_tests(void)
 	}
 
 	/* Give the child a chance to finish */
-	usleep(10000);
+	usleep(100000);
 
 	// Wait for the child to finish up
 	int wstatus;
 	while (waitpid(child, &wstatus, WNOHANG) == 0) {
 		LOG_MSG("...Waiting for child to quit...\n");
-		usleep(10000);
+		usleep(100000);
 	}
 
 	/* Verify that gamemode is now innactive */

From 42d75034825b2e9ad089df42c547fd5f40ae3404 Mon Sep 17 00:00:00 2001
From: Marc Di Luzio <marc.diluzio@gmail.com>
Date: Sat, 16 Feb 2019 12:16:06 +0000
Subject: [PATCH 17/18] Adjust checking in gpuclockctl to assist when errors
 happen

---
 daemon/gpuclockctl.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/daemon/gpuclockctl.c b/daemon/gpuclockctl.c
index e1ac086..28963f4 100644
--- a/daemon/gpuclockctl.c
+++ b/daemon/gpuclockctl.c
@@ -64,6 +64,9 @@ static int get_gpu_state_nv(struct GameModeGPUInfo *info)
 	if (info->vendor != Vendor_NVIDIA)
 		return -1;
 
+	if (!getenv("DISPLAY"))
+		LOG_ERROR("Getting Nvidia parameters requires DISPLAY to be set - will likely fail!\n");
+
 	char arg[128] = { 0 };
 	char buf[EXTERNAL_BUFFER_MAX] = { 0 };
 	char *end;
@@ -127,7 +130,10 @@ static int set_gpu_state_nv(struct GameModeGPUInfo *info)
 	if (info->vendor != Vendor_NVIDIA)
 		return -1;
 
-	// These commands don't technically even need root
+	if (!getenv("DISPLAY") || !getenv("XAUTHORITY"))
+		LOG_ERROR(
+		    "Setting Nvidia parameters requires DISPLAY and XAUTHORITY to be set - will likely "
+		    "fail!\n");
 
 	/* Set the GPUGraphicsClockOffset parameter */
 	char core_arg[128];
@@ -200,6 +206,12 @@ static int set_gpu_state_amd(struct GameModeGPUInfo *info)
 	if (info->vendor != Vendor_AMD)
 		return -1;
 
+	/* Must be root to set the state */
+	if (geteuid() != 0) {
+		fprintf(stderr, "gpuclockctl must be run as root to set AMD values\n");
+		print_usage_and_exit();
+	}
+
 	// Set the the core and mem clock speeds using the OverDrive files
 	if (set_gpu_state_amd_file("pp_sclk_od", info->device, info->core) != 0)
 		return -1;
@@ -278,12 +290,6 @@ int main(int argc, char *argv[])
 		printf("%ld %ld\n", info.core, info.mem);
 
 	} else if (argc >= 6 && argc <= 7 && strncmp(argv[3], "set", 3) == 0) {
-		/* Must be root to set the state */
-		if (geteuid() != 0) {
-			fprintf(stderr, "gpuclockctl must be run as root to set values\n");
-			print_usage_and_exit();
-		}
-
 		/* Get and verify the vendor and device */
 		struct GameModeGPUInfo info;
 		memset(&info, 0, sizeof(info));

From 94b6a34f65c6a80b7768591eba625723218e49f2 Mon Sep 17 00:00:00 2001
From: Marc Di Luzio <marc.diluzio@gmail.com>
Date: Tue, 19 Feb 2019 18:33:06 +0000
Subject: [PATCH 18/18] Add test for gamemoderun and the reaper thread

	Using a simple test allows us to check both at once
---
 daemon/gamemode-tests.c | 77 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 69 insertions(+), 8 deletions(-)

diff --git a/daemon/gamemode-tests.c b/daemon/gamemode-tests.c
index 0b97d4c..a5d36bc 100644
--- a/daemon/gamemode-tests.c
+++ b/daemon/gamemode-tests.c
@@ -242,6 +242,61 @@ static int run_dual_client_tests(void)
 	return status;
 }
 
+/* Check gamemoderun works */
+static int run_gamemoderun_and_reaper_tests(struct GameModeConfig *config)
+{
+	int status = 0;
+
+	LOG_MSG(":: Gamemoderun and reaper thread tests\n");
+
+	/* Fork so that the child can request gamemode */
+	int child = fork();
+	if (child == 0) {
+		/* Close stdout, we don't care if sh prints anything */
+		fclose(stdout);
+		/* Preload into sh and then kill it */
+		if (execl("/usr/bin/gamemoderun", "/usr/bin/gamemoderun", "sh", (char *)NULL) == -1) {
+			LOG_ERROR("failed to launch gamemoderun with execl: %s\n", strerror(errno));
+			return -1;
+		}
+	}
+
+	/* Give the child a chance to reqeust gamemode */
+	usleep(10000);
+
+	/* Check that when we request gamemode, it replies that the other client is connected */
+	if (verify_other_client_connected() != 0)
+		status = -1;
+
+	/* Send SIGTERM to the child to stop it*/
+	if (kill(child, SIGTERM) == -1) {
+		LOG_ERROR("failed to send continue signal to other client: %s\n", strerror(errno));
+		status = -1;
+	}
+
+	/* Wait for the child to clean up */
+	int wstatus;
+	while (waitpid(child, &wstatus, WNOHANG) == 0) {
+		LOG_MSG("...Waiting for child to quit...\n");
+		usleep(100000);
+	}
+
+	/* And give gamemode a chance to reap the process */
+	long freq = config_get_reaper_frequency(config);
+	LOG_MSG("...Waiting for reaper thread (reaper_frequency set to %ld seconds)...\n", freq);
+	sleep((unsigned int)freq);
+
+	/* Verify that gamemode is now innactive */
+	if (verify_deactivated() != 0)
+		return -1;
+
+	if (status == 0)
+		LOG_MSG(":: Passed\n\n");
+
+	return status;
+}
+
+/* Check the cpu governor setting works */
 static int run_cpu_governor_tests(struct GameModeConfig *config)
 {
 	/* get the two config parameters we care about */
@@ -450,19 +505,13 @@ int run_gpu_optimisation_tests(struct GameModeConfig *config)
  * game_mode_run_feature_tests runs a set of tests for each current feature (based on the current
  * config) returns 0 for success, -1 for failure
  */
-static int game_mode_run_feature_tests(void)
+static int game_mode_run_feature_tests(struct GameModeConfig *config)
 {
 	int status = 0;
 	LOG_MSG(":: Feature tests\n");
 
 	/* If we reach here, we should assume the basic requests and register functions are working */
 
-	/* Grab the config */
-	/* Note: this config may pick up a local gamemode.ini, or the daemon may have one, we may need
-	 * to cope with that */
-	GameModeConfig *config = config_create();
-	config_init(config);
-
 	/* Does the CPU governor get set properly? */
 	{
 		LOG_MSG("::: Verifying CPU governor setting\n");
@@ -536,6 +585,14 @@ static int game_mode_run_feature_tests(void)
 int game_mode_run_client_tests()
 {
 	int status = 0;
+
+	LOG_MSG(": Loading config\n");
+	/* Grab the config */
+	/* Note: this config may pick up a local gamemode.ini, or the daemon may have one, we may need
+	 * to cope with that */
+	GameModeConfig *config = config_create();
+	config_init(config);
+
 	LOG_MSG(": Running tests\n\n");
 
 	/* Run the basic tests */
@@ -546,11 +603,15 @@ int game_mode_run_client_tests()
 	if (run_dual_client_tests() != 0)
 		status = -1;
 
+	/* Check gamemoderun and the reaper thread work */
+	if (run_gamemoderun_and_reaper_tests(config) != 0)
+		status = -1;
+
 	if (status != 0) {
 		LOG_MSG(": Client tests failed, skipping feature tests\n");
 	} else {
 		/* Run the feature tests */
-		status = game_mode_run_feature_tests();
+		status = game_mode_run_feature_tests(config);
 	}
 
 	if (status >= 0)