Re: [igt-dev] [PATCH i-g-t] tools/intel_gpu_top: Include total package power

From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, igt-dev@lists.freedesktop.org
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Subject: Re: [igt-dev] [PATCH i-g-t] tools/intel_gpu_top: Include total package power
Date: Tue, 24 Nov 2020 16:01:03 +0000	[thread overview]
Message-ID: <95e7c59a-4b2c-6406-f92d-fed75edea006@linux.intel.com> (raw)
In-Reply-To: <160623299271.28476.1503734847384709621@build.alporthouse.com>

On 24/11/2020 15:49, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2020-11-24 15:30:53)
>>
>> On 24/11/2020 14:19, Chris Wilson wrote:
>>> -static const char *rapl_gpu_power_unit(void)
>>> -{
>>> -     char buf[32];
>>> -
>>> -     if (filename_to_buf(RAPL_EVENT "energy-gpu.unit",
>>> -                         buf, sizeof(buf)) == 0)
>>> -             if (!strcmp(buf, "Joules"))
>>> -                     return strdup("Watts");
>>> -             else
>>> -                     return strdup(buf);
>>
>> We lose this handling of unexpected changes. Hard to decide if that is
>> very important. Should be possible to keep by a simple addition to
>> rapl_parse I think.
> 
> Do you think we need more than a fprintf(stderr) ?
> 
> 	snprintf(buf, sizeof(buf), "events/energy-%s.unit", str);
>          units = igt_sysfs_get(dir, buf);
>          if (units && strcmp(units, "Joules"))
>                  fprintf(stderr, "Unexpected units for RAPL %s: found %s\n",
>                          str, units);
> 	free(units);

That sounds fine to me. "if (!units || strcmp)" probably.

Regards,

Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev