From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by gabe.freedesktop.org (Postfix) with ESMTPS id 464FE10E012 for ; Mon, 6 Mar 2023 23:13:01 +0000 (UTC) Date: Mon, 06 Mar 2023 15:13:00 -0800 Message-ID: <87o7p5mq2r.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Umesh Nerlige Ramappa In-Reply-To: <20230215004648.2100655-9-umesh.nerlige.ramappa@intel.com> References: <20230215004648.2100655-1-umesh.nerlige.ramappa@intel.com> <20230215004648.2100655-9-umesh.nerlige.ramappa@intel.com> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Subject: Re: [igt-dev] [PATCH i-g-t 08/31] i915/perf: Treat ticks as 64 bit List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: igt-dev@lists.freedesktop.org, Lionel G Landwerlin Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: On Tue, 14 Feb 2023 16:46:25 -0800, Umesh Nerlige Ramappa wrote: > Hi Umesh, > To support 64 bit OA report headers, treat all ticks as 64 bits. > > Signed-off-by: Umesh Nerlige Ramappa > --- > tests/i915/perf.c | 79 ++++++++++++++++++++++++++++++++--------------- > 1 file changed, 54 insertions(+), 25 deletions(-) > > diff --git a/tests/i915/perf.c b/tests/i915/perf.c > index bd3cdb6a..4c7b4e49 100644 > --- a/tests/i915/perf.c > +++ b/tests/i915/perf.c > @@ -125,6 +125,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] = { > @@ -244,7 +245,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, > @@ -438,7 +439,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); > @@ -448,10 +449,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; > + > + return t1 - t0; OK, this looks fine due to 2's complement arithmentic. > +} > + > +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); Why are ticks always considered 32 bits? For 64 bit report headers aren't ticks 64 bits? Also, sorry my ignorance, what is the difference between timestamps and ticks (both are present in the report headers)? Thanks. -- Ashutosh