[igt-dev] [PATCH i-g-t 16/23] i915/perf: Treat ticks as 64 bit
Lionel Landwerlin
lionel.g.landwerlin at intel.com
Tue Sep 6 19:06:23 UTC 2022
On 06/09/2022 21:56, Umesh Nerlige Ramappa wrote:
> On Tue, Sep 06, 2022 at 05:08:50PM +0300, Lionel Landwerlin wrote:
>> On 23/08/2022 21:30, Umesh Nerlige Ramappa wrote:
>>> DG2 A0 has a buggy header in the 64 bit OAG format. Except the ticks,
>>> all other fields are 32 bit. To resolve this, we will store ticks in 64
>>> bit type.
>>>
>>> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>
>>> ---
>>> tests/i915/perf.c | 82 ++++++++++++++++++++++++++++++++---------------
>>> 1 file changed, 56 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/tests/i915/perf.c b/tests/i915/perf.c
>>> index 3eb31f3b..8836da66 100644
>>> --- a/tests/i915/perf.c
>>> +++ b/tests/i915/perf.c
>>> @@ -117,6 +117,7 @@ struct oa_format {
>>> int c_off;
>>> int n_c;
>>> int oa_type;
>>> + bool report_hdr_64bit;
>>> };
>>> static struct oa_format hsw_oa_formats[I915_OA_FORMAT_MAX] = {
>>> @@ -203,7 +204,8 @@ static struct oa_format
>>> dg2_oa_formats[I915_OA_FORMAT_MAX] = {
>>> "A38u64_R2u64_B8_C8", .size = 448,
>>> .a64_off = 32, .n_a64 = 38,
>>> .b_off = 352, .n_b = 8,
>>> - .c_off = 384, .n_c = 8, .oa_type = OAG, },
>>> + .c_off = 384, .n_c = 8, .oa_type = OAG,
>>> + .report_hdr_64bit = true, },
>>> [I915_OAR_FORMAT_A36u64_B8_C8] = {
>>> "A36u64_B8_C8", .size = 384,
>>> .a64_off = 32, .n_a64 = 36,
>>> @@ -245,7 +247,7 @@ static bool *undefined_a_counters;
>>> static uint64_t oa_exp_1_millisec;
>>> static igt_render_copyfunc_t render_copy = NULL;
>>> -static uint32_t (*read_report_ticks)(const uint32_t *report,
>>> +static uint64_t (*read_report_ticks)(const uint32_t *report,
>>> enum drm_i915_oa_format format);
>>> static void (*sanity_check_reports)(const uint32_t *oa_report0,
>>> const uint32_t *oa_report1,
>>> @@ -395,7 +397,7 @@ sysfs_read(enum i915_attr_id id)
>>> * C2 corresponds to a clock counter for the Haswell render basic
>>> metric set
>>> * but it's not included in all of the formats.
>>> */
>>> -static uint32_t
>>> +static uint64_t
>>> hsw_read_report_ticks(const uint32_t *report, enum
>>> drm_i915_oa_format format)
>>> {
>>> uint32_t *c = (uint32_t *)(((uint8_t *)report) +
>>> get_oa_format(format).c_off);
>>> @@ -405,10 +407,41 @@ hsw_read_report_ticks(const uint32_t *report,
>>> enum drm_i915_oa_format format)
>>> return c[2];
>>> }
>>> -static uint32_t
>>> +static uint64_t
>>> gen8_read_report_ticks(const uint32_t *report, enum
>>> drm_i915_oa_format format)
>>> {
>>> - return report[3];
>>> +
>>> + struct oa_format fmt = get_oa_format(format);
>>> +
>>> + return fmt.report_hdr_64bit ? *(uint64_t *)&report[6] : report[3];
>>> +}
>>> +
>>> +/*
>>> + * t0 is a value sampled before t1. width is number of bits used to
>>> represent
>>> + * t0/t1. Normally t1 is greater than t0. In cases where t1 < t0
>>> use this
>>> + * helper. Since the size of t1/t0 is already 64 bits, no special
>>> handling is
>>> + * needed for width = 64.
>>> + */
>>> +static uint64_t
>>> +__elapsed_delta(uint64_t t1, uint64_t t0, uint32_t width)
>>> +{
>>> + uint32_t max_bits = sizeof(t1) * 8;
>>> +
>>> + igt_assert(width <= max_bits);
>>> +
>>> + if (t1 < t0 && width != max_bits)
>>> + return ((1ULL << width) - t0) + t1;
>>
>>
>> Should this be : ((1ULL << width) + t1) - t0 ?
>>
>
> Both ways should yield the same result.
My bad :
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>
>>
>> Since we know the t1 wrapped around the counter's max bit size, the
>> actual difference is the max value + whatever is in the counter minus
>> t0.
>>
>>
>>> +
>>> + return t1 - t0;
>>> +}
>>> +
>>> +static uint64_t
>>> +oa_tick_delta(const uint32_t *report1,
>>> + const uint32_t *report0,
>>> + enum drm_i915_oa_format format)
>>> +{
>>> + return __elapsed_delta(read_report_ticks(report1, format),
>>> + read_report_ticks(report0, format), 32);
>>> }
>>> static void
>>> @@ -638,8 +671,8 @@ hsw_sanity_check_render_basic_reports(const
>>> uint32_t *oa_report0,
>>> enum drm_i915_oa_format fmt)
>>> {
>>> uint32_t time_delta = timebase_scale(oa_report1[1] -
>>> oa_report0[1]);
>>> - uint32_t clock_delta;
>>> - uint32_t max_delta;
>>> + uint64_t clock_delta;
>>> + uint64_t max_delta;
>>> struct oa_format format = get_oa_format(fmt);
>>> igt_assert_neq(time_delta, 0);
>>> @@ -654,21 +687,19 @@ hsw_sanity_check_render_basic_reports(const
>>> uint32_t *oa_report0,
>>> clock_delta = (gt_max_freq_mhz *
>>> (uint64_t)time_delta) / 1000;
>>> } else {
>>> - uint32_t ticks0 = read_report_ticks(oa_report0, fmt);
>>> - uint32_t ticks1 = read_report_ticks(oa_report1, fmt);
>>> uint64_t freq;
>>> - clock_delta = ticks1 - ticks0;
>>> + clock_delta = oa_tick_delta(oa_report1, oa_report0, fmt);
>>> - igt_assert_neq(clock_delta, 0);
>>> + igt_assert_neq_u64(clock_delta, 0);
>>> - freq = ((uint64_t)clock_delta * 1000) / time_delta;
>>> + freq = (clock_delta * 1000) / time_delta;
>>> igt_debug("freq = %"PRIu64"\n", freq);
>>> igt_assert(freq <= gt_max_freq_mhz);
>>> }
>>> - igt_debug("clock delta = %"PRIu32"\n", clock_delta);
>>> + igt_debug("clock delta = %"PRIu64"\n", clock_delta);
>>> /* The maximum rate for any HSW counter =
>>> * clock_delta * N EUs
>>> @@ -801,7 +832,8 @@ accumulate_reports(struct accumulator *accumulator,
>>> accumulate_uint32(4, start, end, deltas + idx++);
>>> /* clock cycles */
>>> - accumulate_uint32(12, start, end, deltas + idx++);
>>> + deltas[idx] += oa_tick_delta(end, start, accumulator->format);
>>> + idx++;
>>> } else {
>>> /* timestamp */
>>> accumulate_uint32(4, start, end, deltas + idx++);
>>> @@ -874,10 +906,8 @@ gen8_sanity_check_test_oa_reports(const
>>> uint32_t *oa_report0,
>>> {
>>> struct oa_format format = get_oa_format(fmt);
>>> uint32_t time_delta = timebase_scale(oa_report1[1] -
>>> oa_report0[1]);
>>> - uint32_t ticks0 = read_report_ticks(oa_report0, fmt);
>>> - uint32_t ticks1 = read_report_ticks(oa_report1, fmt);
>>> - uint32_t clock_delta = ticks1 - ticks0;
>>> - uint32_t max_delta;
>>> + uint64_t clock_delta = oa_tick_delta(oa_report1, oa_report0, fmt);
>>> + uint64_t max_delta;
>>> uint64_t freq;
>>> uint32_t *rpt0_b = (uint32_t *)(((uint8_t *)oa_report0) +
>>> format.b_off);
>>> @@ -890,10 +920,10 @@ gen8_sanity_check_test_oa_reports(const
>>> uint32_t *oa_report0,
>>> gen8_read_report_reason(oa_report0),
>>> gen8_read_report_reason(oa_report1));
>>> - freq = time_delta ? ((uint64_t)clock_delta * 1000) / time_delta
>>> : 0;
>>> + freq = time_delta ? (clock_delta * 1000) / time_delta : 0;
>>> igt_debug("freq = %"PRIu64"\n", freq);
>>> - igt_debug("clock delta = %"PRIu32"\n", clock_delta);
>>> + igt_debug("clock delta = %"PRIu64"\n", clock_delta);
>>> max_delta = clock_delta * intel_perf->devinfo.n_eus;
>>> @@ -994,7 +1024,7 @@ gen8_sanity_check_test_oa_reports(const
>>> uint32_t *oa_report0,
>>> format.c_off);
>>> uint32_t delta = c1[j] - c0[j];
>>> - igt_debug("C%d: delta = %"PRIu32", max_delta=%"PRIu32"\n",
>>> + igt_debug("C%d: delta = %"PRIu32", max_delta=%"PRIu64"\n",
>>> j, delta, max_delta);
>>> igt_assert(delta <= max_delta);
>>> }
>>> @@ -1440,10 +1470,10 @@ print_reports(uint32_t *oa_report0, uint32_t
>>> *oa_report1, int fmt)
>>> if (IS_HASWELL(devid) && format.n_c == 0) {
>>> igt_debug("CLOCK = N/A\n");
>>> } else {
>>> - uint32_t clock0 = read_report_ticks(oa_report0, fmt);
>>> - uint32_t clock1 = read_report_ticks(oa_report1, fmt);
>>> + uint64_t clock0 = read_report_ticks(oa_report0, fmt);
>>> + uint64_t clock1 = read_report_ticks(oa_report1, fmt);
>>> - igt_debug("CLOCK: 1st = %"PRIu32", 2nd = %"PRIu32", delta =
>>> %"PRIu32"\n",
>>> + igt_debug("CLOCK: 1st = %"PRIu64", 2nd = %"PRIu64", delta =
>>> %"PRIu64"\n",
>>> clock0, clock1, clock1 - clock0);
>>> }
>>> @@ -1545,9 +1575,9 @@ print_report(uint32_t *report, int fmt)
>>> if (IS_HASWELL(devid) && format.n_c == 0) {
>>> igt_debug("CLOCK = N/A\n");
>>> } else {
>>> - uint32_t clock = read_report_ticks(report, fmt);
>>> + uint64_t clock = read_report_ticks(report, fmt);
>>> - igt_debug("CLOCK: %"PRIu32"\n", clock);
>>> + igt_debug("CLOCK: %"PRIu64"\n", clock);
>>> }
>>> if (intel_gen(devid) >= 8) {
>>
>>
More information about the igt-dev
mailing list