Re: [Intel-gfx] [IGT 1/2] tools/intel_gpu_top: Add support for stdout logging

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 11/02/2019 12:21, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-02-11 11:45:22)
>> +static enum {
>> +       INTERACTIVE,
>> +       STDOUT,
>> +       JSON
>> +} output_mode;
>> +
>> +struct cnt_item {
>> +       struct pmu_counter *pmu;
> 
> /* "%*.*f", fmt_d, fmt_dd, X */
> 
> I tried fmt_d == fmt_width and fmt_dd == fmt_decimals
> 
> It's called field width and precision in the manpage, so
> maybe fmt_width and fmt_precision.

Makes sense indeed.

> Having figured that out, the use makes sense.
> 
>> +       unsigned int fmt_d;
>> +       unsigned int fmt_dd;
>> +       double d;
>> +       double t;
>> +       double s;
>> +       const char *name;
>> +       const char *unit;
>> +
>> +       /* Internal fields. */
>> +       char buf[16];
>> +};
>> +
>> +struct cnt_group {
>> +       const char *name;
>> +       const char *display_name;
>> +       struct cnt_item *items;
>> +};
>> +
>> +static unsigned int json_indent_level;
>> +
>> +static const char *json_indent[] = {
>> +       "",
>> +       "\t",
>> +       "\t\t",
>> +       "\t\t\t",
>> +       "\t\t\t\t",
>> +       "\t\t\t\t\t",
>> +};
>> +
>> +#define ARRAY_SIZE(arr) (sizeof(arr)/sizeof(arr[0]))
>> +
>> +static unsigned int json_prev_struct_members;
>> +static unsigned int json_struct_members;
>> +
>> +static void
>> +json_open_struct(const char *name)
>> +{
>> +       assert(json_indent_level < ARRAY_SIZE(json_indent));
>> +
>> +       json_prev_struct_members = json_struct_members;
>> +       json_struct_members = 0;
>> +
>> +       if (name)
>> +               printf("%s%s\"%s\": {\n",
>> +                      json_prev_struct_members ? ",\n" : "",
>> +                      json_indent[json_indent_level],
> 
> "%*s", json_indent_level, "\t\t\t\t\t"
> didn't stick?

No, I lost patience trying to make it do what I want. AFAIR it insisted 
on right justifying and the negative count also did not work.

> I could follow the flow, dug into a few of the details, and only have
> some nits. One thing, could we feed in a perf record? I was thinking for
> testing the various output modes with known data, but may also be useful
> for replay.

Could do, should be too hard. Writing tests for tool output though 
sounds like something I won't have time to do in the near future.

> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Thanks, I'll keep it when I fix the asprintf return value inspection 
which I butchered in a hasty copy and paste work.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx