All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH umr] read all sensors in a thread
@ 2017-03-27 13:41 Tom St Denis
       [not found] ` <20170327134104.21708-1-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 2+ messages in thread
From: Tom St Denis @ 2017-03-27 13:41 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Tom St Denis

Eventually the GPU_POWER sensor will be rated at around
50Hz but that's still too slow to read from the main loop of
umr's --top so we move all sensor operations to a thread.

Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
---
 src/app/top.c | 110 +++++++++++++++++++++++++++++-----------------------------
 1 file changed, 55 insertions(+), 55 deletions(-)

diff --git a/src/app/top.c b/src/app/top.c
index 74ae5dfbb98a..dedad93f029c 100644
--- a/src/app/top.c
+++ b/src/app/top.c
@@ -279,17 +279,35 @@ static struct umr_bitfield stat_drm_bits[] = {
 static FILE *logfile = NULL;
 
 static volatile int sensor_thread_quit = 0;
-static uint32_t gpu_power_data[4];
-static void *vi_sensor_thread(void *data)
+static volatile uint32_t gpu_power_data[32];
+static volatile struct umr_bitfield *sensor_bits = NULL;
+static void *gpu_sensor_thread(void *data)
 {
 	struct umr_asic asic = *((struct umr_asic*)data);
-	int size = sizeof(gpu_power_data);
+	int size, rem, off, x;
 	char fname[128];
 
 	snprintf(fname, sizeof(fname)-1, "/sys/kernel/debug/dri/%d/amdgpu_sensors", asic.instance);
 	asic.fd.sensors = open(fname, O_RDWR);
-	while (!sensor_thread_quit)
-		umr_read_sensor(&asic, AMDGPU_PP_SENSOR_GPU_POWER, gpu_power_data, &size);
+	while (!sensor_thread_quit) {
+		rem = sizeof gpu_power_data;
+		off = 0;
+		for (x = 0; sensor_bits[x].regname; ) {
+			switch (sensor_bits[x].start) {
+				case AMDGPU_PP_SENSOR_GPU_POWER:
+					size = 16;
+					break;
+				default:
+					size = 4;
+					break;
+			}
+			if (size < rem)
+				umr_read_sensor(&asic, sensor_bits[x].start, (uint32_t*)&gpu_power_data[off], &size);
+			off += size / 4;
+			rem -= size;
+			x   += size / 4;
+		}
+	}
 	close(asic.fd.sensors);
 	return NULL;
 }
@@ -536,8 +554,8 @@ static void parse_bits(struct umr_asic *asic, uint32_t addr, struct umr_bitfield
 
 static void parse_sensors(struct umr_asic *asic, uint32_t addr, struct umr_bitfield *bits, uint64_t *counts, uint32_t *mask, uint32_t *cmp, uint64_t addr_mask)
 {
-	int j, size, x;
-	uint32_t value[16];
+	int j, x;
+	uint32_t value;
 
 	(void)addr;
 	(void)mask;
@@ -547,42 +565,29 @@ static void parse_sensors(struct umr_asic *asic, uint32_t addr, struct umr_bitfi
 	if (asic->fd.sensors < 0)
 		return;
 
-	for (j = 0; bits[j].regname; ) {
-		size = 4;
-		if (bits[j].start == AMDGPU_PP_SENSOR_GPU_POWER || !umr_read_sensor(asic, bits[j].start, &value[0], &size)) {
-			x = 0;
-			if (bits[j].start == AMDGPU_PP_SENSOR_GPU_POWER) {
-				size = 4 * sizeof(uint32_t);
-				memcpy(value, gpu_power_data, size);
-			}
-
-			while (size) {
-				switch (bits[j].stop & 0x0F) {
-					case SENSOR_IDENTITY:
-						counts[j] = value[x];
-						break;
-					case SENSOR_D1000:
-						counts[j] = value[x] / 1000;
-						break;
-					case SENSOR_D100:
-						counts[j] = value[x] / 100;
-						break;
-					case SENSOR_WATT:
-						counts[j] = ((value[x] >> 8) * 1000);
-						if ((value[x] & 0xFF) < 100)
-							counts[j] += (value[x] & 0xFF) * 10;
-						else
-							counts[j] += value[x];
-						counts[j] /= 10; // convert to centiwatts since we don't need 3 digits of excess precision
-						break;
-				}
-				size -= 4;
-				++j;
-				++x;
-			}
-		} else {
-			++j;
+	for (x = j = 0; bits[j].regname; ) {
+		value = gpu_power_data[x];
+		switch (bits[j].stop & 0x0F) {
+			case SENSOR_IDENTITY:
+				counts[j] = value;
+				break;
+			case SENSOR_D1000:
+				counts[j] = value / 1000;
+				break;
+			case SENSOR_D100:
+				counts[j] = value / 100;
+				break;
+			case SENSOR_WATT:
+				counts[j] = ((value >> 8) * 1000);
+				if ((value & 0xFF) < 100)
+					counts[j] += (value & 0xFF) * 10;
+				else
+					counts[j] += value;
+				counts[j] /= 10; // convert to centiwatts since we don't need 3 digits of excess precision
+				break;
 		}
+		++j;
+		++x;
 	}
 }
 
@@ -805,6 +810,7 @@ static void top_build_vi_program(struct umr_asic *asic)
 		// SI
 		ENTRY_SENSOR(i++, "GFX_SCLK", &stat_si_sensor_bits[0], &top_options.vi.sensors, "Sensors");
 	}
+	sensor_bits = stat_counters[i-1].bits;
 
 	// More GFX bits
 	ENTRY(i++, "mmTA_STATUS", &stat_ta_bits[0], &top_options.vi.ta, "TA");
@@ -899,7 +905,7 @@ static void toggle_logger(void)
 
 void umr_top(struct umr_asic *asic)
 {
-	int i, j, k, use_thread;
+	int i, j, k;
 	struct timespec req;
 	uint32_t rep;
 	time_t tt;
@@ -927,15 +933,11 @@ void umr_top(struct umr_asic *asic)
 			grab_bits(stat_counters[i].name, asic, stat_counters[i].bits, &stat_counters[i].addr);
 
 	sensor_thread_quit = 0;
-	use_thread = 0;
 
-	// start thread to grab sensor data for VI
-	if (asic->family == FAMILY_VI) {
-		if (pthread_create(&sensor_thread, NULL, vi_sensor_thread, asic)) {
-			fprintf(stderr, "[ERROR] Cannot create vi_sensor_thread\n");
-			return;
-		}
-		use_thread = 1;
+	// start thread to grab sensor data
+	if (pthread_create(&sensor_thread, NULL, gpu_sensor_thread, asic)) {
+		fprintf(stderr, "[ERROR] Cannot create gpu_sensor_thread\n");
+		return;
 	}
 
 	initscr();
@@ -1068,8 +1070,6 @@ void umr_top(struct umr_asic *asic)
 	}
 	endwin();
 
-	if (use_thread) {
-		sensor_thread_quit = 1;
-		pthread_join(sensor_thread, NULL);
-	}
+	sensor_thread_quit = 1;
+	pthread_join(sensor_thread, NULL);
 }
-- 
2.12.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH umr] read all sensors in a thread
       [not found] ` <20170327134104.21708-1-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
@ 2017-03-27 13:47   ` Tom St Denis
  0 siblings, 0 replies; 2+ messages in thread
From: Tom St Denis @ 2017-03-27 13:47 UTC (permalink / raw)
  To: Tom St Denis, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

On 27/03/17 09:41 AM, Tom St Denis wrote:
> Eventually the GPU_POWER sensor will be rated at around
> 50Hz but that's still too slow to read from the main loop of
> umr's --top so we move all sensor operations to a thread.
>
> Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
> ---
>  src/app/top.c | 110 +++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 55 insertions(+), 55 deletions(-)
>
> diff --git a/src/app/top.c b/src/app/top.c
> index 74ae5dfbb98a..dedad93f029c 100644
> --- a/src/app/top.c
> +++ b/src/app/top.c
> @@ -279,17 +279,35 @@ static struct umr_bitfield stat_drm_bits[] = {
>  static FILE *logfile = NULL;
>
>  static volatile int sensor_thread_quit = 0;
> -static uint32_t gpu_power_data[4];
> -static void *vi_sensor_thread(void *data)
> +static volatile uint32_t gpu_power_data[32];
> +static volatile struct umr_bitfield *sensor_bits = NULL;
> +static void *gpu_sensor_thread(void *data)
>  {
>  	struct umr_asic asic = *((struct umr_asic*)data);
> -	int size = sizeof(gpu_power_data);
> +	int size, rem, off, x;
>  	char fname[128];
>
>  	snprintf(fname, sizeof(fname)-1, "/sys/kernel/debug/dri/%d/amdgpu_sensors", asic.instance);
>  	asic.fd.sensors = open(fname, O_RDWR);
> -	while (!sensor_thread_quit)
> -		umr_read_sensor(&asic, AMDGPU_PP_SENSOR_GPU_POWER, gpu_power_data, &size);
> +	while (!sensor_thread_quit) {
> +		rem = sizeof gpu_power_data;
> +		off = 0;
> +		for (x = 0; sensor_bits[x].regname; ) {
> +			switch (sensor_bits[x].start) {
> +				case AMDGPU_PP_SENSOR_GPU_POWER:
> +					size = 16;
> +					break;
> +				default:
> +					size = 4;
> +					break;
> +			}
> +			if (size < rem)

Not that this will manifest yet but that should be size <= rem.  Assume 
this is fixed locally when reviewing please.

Tom


> +				umr_read_sensor(&asic, sensor_bits[x].start, (uint32_t*)&gpu_power_data[off], &size);
> +			off += size / 4;
> +			rem -= size;
> +			x   += size / 4;
> +		}
> +	}
>  	close(asic.fd.sensors);
>  	return NULL;
>  }
> @@ -536,8 +554,8 @@ static void parse_bits(struct umr_asic *asic, uint32_t addr, struct umr_bitfield
>
>  static void parse_sensors(struct umr_asic *asic, uint32_t addr, struct umr_bitfield *bits, uint64_t *counts, uint32_t *mask, uint32_t *cmp, uint64_t addr_mask)
>  {
> -	int j, size, x;
> -	uint32_t value[16];
> +	int j, x;
> +	uint32_t value;
>
>  	(void)addr;
>  	(void)mask;
> @@ -547,42 +565,29 @@ static void parse_sensors(struct umr_asic *asic, uint32_t addr, struct umr_bitfi
>  	if (asic->fd.sensors < 0)
>  		return;
>
> -	for (j = 0; bits[j].regname; ) {
> -		size = 4;
> -		if (bits[j].start == AMDGPU_PP_SENSOR_GPU_POWER || !umr_read_sensor(asic, bits[j].start, &value[0], &size)) {
> -			x = 0;
> -			if (bits[j].start == AMDGPU_PP_SENSOR_GPU_POWER) {
> -				size = 4 * sizeof(uint32_t);
> -				memcpy(value, gpu_power_data, size);
> -			}
> -
> -			while (size) {
> -				switch (bits[j].stop & 0x0F) {
> -					case SENSOR_IDENTITY:
> -						counts[j] = value[x];
> -						break;
> -					case SENSOR_D1000:
> -						counts[j] = value[x] / 1000;
> -						break;
> -					case SENSOR_D100:
> -						counts[j] = value[x] / 100;
> -						break;
> -					case SENSOR_WATT:
> -						counts[j] = ((value[x] >> 8) * 1000);
> -						if ((value[x] & 0xFF) < 100)
> -							counts[j] += (value[x] & 0xFF) * 10;
> -						else
> -							counts[j] += value[x];
> -						counts[j] /= 10; // convert to centiwatts since we don't need 3 digits of excess precision
> -						break;
> -				}
> -				size -= 4;
> -				++j;
> -				++x;
> -			}
> -		} else {
> -			++j;
> +	for (x = j = 0; bits[j].regname; ) {
> +		value = gpu_power_data[x];
> +		switch (bits[j].stop & 0x0F) {
> +			case SENSOR_IDENTITY:
> +				counts[j] = value;
> +				break;
> +			case SENSOR_D1000:
> +				counts[j] = value / 1000;
> +				break;
> +			case SENSOR_D100:
> +				counts[j] = value / 100;
> +				break;
> +			case SENSOR_WATT:
> +				counts[j] = ((value >> 8) * 1000);
> +				if ((value & 0xFF) < 100)
> +					counts[j] += (value & 0xFF) * 10;
> +				else
> +					counts[j] += value;
> +				counts[j] /= 10; // convert to centiwatts since we don't need 3 digits of excess precision
> +				break;
>  		}
> +		++j;
> +		++x;
>  	}
>  }
>
> @@ -805,6 +810,7 @@ static void top_build_vi_program(struct umr_asic *asic)
>  		// SI
>  		ENTRY_SENSOR(i++, "GFX_SCLK", &stat_si_sensor_bits[0], &top_options.vi.sensors, "Sensors");
>  	}
> +	sensor_bits = stat_counters[i-1].bits;
>
>  	// More GFX bits
>  	ENTRY(i++, "mmTA_STATUS", &stat_ta_bits[0], &top_options.vi.ta, "TA");
> @@ -899,7 +905,7 @@ static void toggle_logger(void)
>
>  void umr_top(struct umr_asic *asic)
>  {
> -	int i, j, k, use_thread;
> +	int i, j, k;
>  	struct timespec req;
>  	uint32_t rep;
>  	time_t tt;
> @@ -927,15 +933,11 @@ void umr_top(struct umr_asic *asic)
>  			grab_bits(stat_counters[i].name, asic, stat_counters[i].bits, &stat_counters[i].addr);
>
>  	sensor_thread_quit = 0;
> -	use_thread = 0;
>
> -	// start thread to grab sensor data for VI
> -	if (asic->family == FAMILY_VI) {
> -		if (pthread_create(&sensor_thread, NULL, vi_sensor_thread, asic)) {
> -			fprintf(stderr, "[ERROR] Cannot create vi_sensor_thread\n");
> -			return;
> -		}
> -		use_thread = 1;
> +	// start thread to grab sensor data
> +	if (pthread_create(&sensor_thread, NULL, gpu_sensor_thread, asic)) {
> +		fprintf(stderr, "[ERROR] Cannot create gpu_sensor_thread\n");
> +		return;
>  	}
>
>  	initscr();
> @@ -1068,8 +1070,6 @@ void umr_top(struct umr_asic *asic)
>  	}
>  	endwin();
>
> -	if (use_thread) {
> -		sensor_thread_quit = 1;
> -		pthread_join(sensor_thread, NULL);
> -	}
> +	sensor_thread_quit = 1;
> +	pthread_join(sensor_thread, NULL);
>  }
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-03-27 13:47 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-27 13:41 [PATCH umr] read all sensors in a thread Tom St Denis
     [not found] ` <20170327134104.21708-1-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
2017-03-27 13:47   ` Tom St Denis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.