Remove passing the vendor to gpuclockctl

daemon/gamemode-gpu.c

@@ -47,8 +47,6 @@ POSSIBILITY OF SUCH DAMAGE.
  */
 int game_mode_initialise_gpu(GameModeConfig *config, GameModeGPUInfo **info)
 {
-	int status = 0;
-
 	/* Verify input, this is programmer error */
 	if (!info || *info)
 		FATAL_ERROR("Invalid GameModeGPUInfo passed to %s", __func__);
@@ -83,33 +81,9 @@ int game_mode_initialise_gpu(GameModeConfig *config, GameModeGPUInfo **info)
 	}
 
 	/* Fill in GPU vendor */
-	char path[64] = { 0 };
+	new_info->vendor = gamemode_get_gpu_vendor(new_info->device);
-	if (snprintf(path, 64, "/sys/class/drm/card%ld/device/vendor", new_info->device) < 0) {
-		LOG_ERROR("snprintf failed, will not apply gpu optimisations!\n");
-		return -1;
-	}
-	FILE *vendor = fopen(path, "r");
-	if (!vendor) {
-		LOG_ERROR("Couldn't open vendor file at %s, will not apply gpu optimisations!\n", path);
-		return -1;
-	}
-	char buff[64];
-	if (fgets(buff, 64, vendor) != NULL) {
-		new_info->vendor = strtol(buff, NULL, 0);
-	} else {
-		LOG_ERROR("Coudn't read contents of file %s, will not apply optimisations!\n", path);
-		return -1;
-	}
-
-	/* verify GPU vendor */
 	if (!GPUVendorValid(new_info->vendor)) {
-		LOG_ERROR("Unknown vendor value (0x%04x) found, cannot apply optimisations!\n",
+		LOG_ERROR("Found invalid vendor, will not apply optimisations!\n");
-		          (unsigned int)new_info->vendor);
-		LOG_ERROR("Known values are: 0x%04x (NVIDIA) 0x%04x (AMD) 0x%04x (Intel)\n",
-		          Vendor_NVIDIA,
-		          Vendor_AMD,
-		          Vendor_Intel);
-		free(new_info);
 		return -1;
 	}
 
@@ -168,7 +142,7 @@ int game_mode_initialise_gpu(GameModeConfig *config, GameModeGPUInfo **info)
 
 	/* Give back the new gpu info */
 	*info = new_info;
-	return status;
+	return 0;
 }
 
 /* Simply used to free the GPU info object */
@@ -196,8 +170,6 @@ int game_mode_apply_gpu(const GameModeGPUInfo *info)
 	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);
 
@@ -214,7 +186,6 @@ int game_mode_apply_gpu(const GameModeGPUInfo *info)
 	const char *const exec_args[] = {
 		"/usr/bin/pkexec",
 		LIBEXECDIR "/gpuclockctl",
-		vendor,
 		device,
 		"set",
 		info->vendor == Vendor_NVIDIA ? nv_core : info->amd_performance_level,
@@ -238,8 +209,6 @@ int game_mode_get_gpu(GameModeGPUInfo *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];
@@ -249,7 +218,6 @@ int game_mode_get_gpu(GameModeGPUInfo *info)
 	// 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 */

daemon/gpu-control.c

@@ -0,0 +1,72 @@
+/*
+
+Copyright (c) 2017-2018, Feral Interactive
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions are met:
+
+ * Redistributions of source code must retain the above copyright notice,
+   this list of conditions and the following disclaimer.
+ * Redistributions in binary form must reproduce the above copyright
+   notice, this list of conditions and the following disclaimer in the
+   documentation and/or other materials provided with the distribution.
+ * Neither the name of Feral Interactive nor the names of its contributors
+   may be used to endorse or promote products derived from this software
+   without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+POSSIBILITY OF SUCH DAMAGE.
+
+ */
+#include "gpu-control.h"
+#include "logging.h"
+
+#include <stdio.h>
+
+/* Get the vendor for a device */
+enum GPUVendor gamemode_get_gpu_vendor(long device)
+{
+	enum GPUVendor vendor = Vendor_Invalid;
+
+	/* Fill in GPU vendor */
+	char path[64] = { 0 };
+	if (snprintf(path, 64, "/sys/class/drm/card%ld/device/vendor", device) < 0) {
+		LOG_ERROR("snprintf failed, will not apply gpu optimisations!\n");
+		return Vendor_Invalid;
+	}
+	FILE *file = fopen(path, "r");
+	if (!file) {
+		LOG_ERROR("Couldn't open vendor file at %s, will not apply gpu optimisations!\n", path);
+		return Vendor_Invalid;
+	}
+	char buff[64];
+	if (fgets(buff, 64, file) != NULL) {
+		vendor = strtol(buff, NULL, 0);
+	} else {
+		LOG_ERROR("Coudn't read contents of file %s, will not apply optimisations!\n", path);
+		return Vendor_Invalid;
+	}
+
+	/* verify GPU vendor */
+	if (!GPUVendorValid(vendor)) {
+		LOG_ERROR("Unknown vendor value (0x%04x) found, cannot apply optimisations!\n",
+		          (unsigned int)vendor);
+		LOG_ERROR("Known values are: 0x%04x (NVIDIA) 0x%04x (AMD) 0x%04x (Intel)\n",
+		          Vendor_NVIDIA,
+		          Vendor_AMD,
+		          Vendor_Intel);
+		return Vendor_Invalid;
+	}
+
+	return vendor;
+}

daemon/gpu-control.h

@@ -55,3 +55,6 @@ struct GameModeGPUInfo {
 
 	char amd_performance_level[CONFIG_VALUE_MAX]; /* The AMD performance level set to */
 };
+
+/* Get the vendor for a device */
+enum GPUVendor gamemode_get_gpu_vendor(long device);

daemon/gpuclockctl.c

@@ -284,18 +284,6 @@ static int set_gpu_state_amd(struct GameModeGPUInfo *info)
 	return 0;
 }
 
-/* Helper to get and verify vendor value */
-static long get_vendor(const char *val)
-{
-	char *end;
-	long ret = strtol(val, &end, 0);
-	if (!GPUVendorValid(ret) || end == val) {
-		LOG_ERROR("Invalid GPU Vendor passed (0x%04x)!\n", (unsigned short)ret);
-		print_usage_and_exit();
-	}
-	return ret;
-}
-
 /* Helper to get and verify device value */
 static long get_device(const char *val)
 {
@@ -325,20 +313,23 @@ 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 >= 3 && strncmp(argv[2], "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[1]);
-		info.device = get_device(argv[2]);
+		info.vendor = gamemode_get_gpu_vendor(info.device);
 
 		/* Fetch the state and print it out */
 		switch (info.vendor) {
 		case Vendor_NVIDIA:
-			info.nv_perf_level = -1;
+			if (argc < 3) {
-			if (argc > 4)
+				LOG_ERROR("Must pass perf_level for nvidia gpu!\n");
-				info.nv_perf_level = get_generic_value(argv[4]);
+				print_usage_and_exit();
-			/* Get nvidia power level */
+			}
+			info.nv_perf_level = get_generic_value(argv[3]);
+
+			/* Get nvidia info */
 			if (get_gpu_state_nv(&info) != 0)
 				exit(EXIT_FAILURE);
 			printf("%ld %ld %ld\n", info.nv_core, info.nv_mem, info.nv_powermizer_mode);
@@ -349,33 +340,44 @@ int main(int argc, char *argv[])
 			printf("%s\n", info.amd_performance_level);
 			break;
 		default:
-			printf("Currently unsupported GPU vendor 0x%04x, doing nothing!\n", (short)info.vendor);
+			LOG_ERROR("Currently unsupported GPU vendor 0x%04x, doing nothing!\n",
+			          (short)info.vendor);
 			break;
 		}
 
-	} else if (argc >= 5 && argc <= 8 && strncmp(argv[3], "set", 3) == 0) {
+	} else if (argc >= 4 && argc <= 7 && strncmp(argv[2], "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[1]);
-		info.device = get_device(argv[2]);
+		info.vendor = gamemode_get_gpu_vendor(info.device);
 
 		switch (info.vendor) {
 		case Vendor_NVIDIA:
-			info.nv_core = get_generic_value(argv[4]);
+			if (argc < 5) {
-			info.nv_mem = get_generic_value(argv[5]);
+				LOG_ERROR("Must pass at least 5 arguments for nvidia gpu!\n");
-			info.nv_perf_level = -1;
+				print_usage_and_exit();
-			if (argc > 6)
+			}
-				info.nv_perf_level = get_generic_value(argv[6]);
+			info.nv_core = get_generic_value(argv[3]);
+			info.nv_mem = get_generic_value(argv[4]);
+			info.nv_perf_level = get_generic_value(argv[5]);
+
+			/* Optional */
 			info.nv_powermizer_mode = -1;
-			if (argc > 7)
+			if (argc >= 5)
-				info.nv_powermizer_mode = get_generic_value(argv[7]);
+				info.nv_powermizer_mode = get_generic_value(argv[6]);
 			break;
 		case Vendor_AMD:
-			strncpy(info.amd_performance_level, argv[4], CONFIG_VALUE_MAX);
+			if (argc < 3) {
+				LOG_ERROR("Must pass performance level for AMD gpu!\n");
+				print_usage_and_exit();
+			}
+			strncpy(info.amd_performance_level, argv[3], CONFIG_VALUE_MAX);
 			break;
 		default:
-			printf("Currently unsupported GPU vendor 0x%04x, doing nothing!\n", (short)info.vendor);
+			LOG_ERROR("Currently unsupported GPU vendor 0x%04x, doing nothing!\n",
+			          (short)info.vendor);
+			print_usage_and_exit();
 			break;
 		}
 
@@ -389,7 +391,6 @@ int main(int argc, char *argv[])
 		case Vendor_AMD:
 			return set_gpu_state_amd(&info);
 		default:
-			printf("Currently unsupported GPU vendor 0x%04x, doing nothing!\n", (short)info.vendor);
 			break;
 		}
 	} else {

daemon/meson.build

@@ -3,6 +3,7 @@ common_sources = [
     'logging.c',
     'governors-query.c',
     'external-helper.c',
+    'gpu-control.c',
 ]
 
 daemon_common = static_library(