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 at 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);
> }
>
