[Mesa-dev] [PATCH v2 3/5] i965: perf: read slice/unslice frequencies from OA reports

[Kenneth Graunke]

On Tuesday, April 17, 2018 4:45:13 PM PDT Lionel Landwerlin wrote:
> On 16/04/18 23:16, Kenneth Graunke wrote:
> > On Tuesday, April 3, 2018 7:48:11 AM PDT Lionel Landwerlin wrote:
> >> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> >> ---
> >>   src/mesa/drivers/dri/i965/brw_performance_query.c | 37 +++++++++++++++++++++++
> >>   src/mesa/drivers/dri/i965/brw_performance_query.h | 12 ++++++++
> >>   2 files changed, 49 insertions(+)
> >>
> >> diff --git a/src/mesa/drivers/dri/i965/brw_performance_query.c b/src/mesa/drivers/dri/i965/brw_performance_query.c
> >> index 32cf96a333d..d20bc58ee33 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_performance_query.c
> >> +++ b/src/mesa/drivers/dri/i965/brw_performance_query.c
> >> @@ -1341,6 +1341,42 @@ brw_is_perf_query_ready(struct gl_context *ctx,
> >>      return false;
> >>   }
> >>   
> >> +static void
> >> +gen8_read_report_clock_ratios(const uint32_t *report,
> >> +                              uint64_t *slice_freq_hz,
> >> +                              uint64_t *unslice_freq_hz)
> >> +{
> >> +   uint32_t unslice_freq = report[0] & 0x1ff;
> >> +   uint32_t slice_freq_low = (report[0] >> 25) & 0x7f;
> >> +   uint32_t slice_freq_high = (report[0] >> 9) & 0x3;
> >> +   uint32_t slice_freq = slice_freq_low | (slice_freq_high << 7);
> >> +
> >> +   *slice_freq_hz = slice_freq * 16666000ULL;
> >> +   *unslice_freq_hz = unslice_freq * 16666000ULL;
> > What is this reading?  Could we add a comment to the function?
> > Otherwise, I'm not sure where to even begin to look for this in docs.
> 
> Apologies, I've just added a fairly descriptive comment. I should have 
> put that in the first version.
> 
> >
> >> +}
> >> +
> >> +static void
> >> +read_slice_unslice_frequencies(struct brw_context *brw,
> >> +                               struct brw_perf_query_object *obj)
> >> +{
> >> +   const struct gen_device_info *devinfo = &brw->screen->devinfo;
> >> +   uint32_t *begin_report, *end_report;
> >> +
> >> +   /* No data available before gen8. */
> >> +   if (devinfo->gen < 8)
> >> +      return;
> >> +
> >> +   begin_report = obj->oa.map;
> >> +   end_report = obj->oa.map + MI_RPC_BO_END_OFFSET_BYTES;
> > I'm probably missing something obvious here, but...this looks fishy.
> >
> > What actually writes these values to the OA buffer?  And here, you're
> > reading the starting value from DWord 0 of the buffer...is that really
> > where those values live?
> >
> > I guess I would have expected to see a capture_uncore_frequencies()
> > and MI_SLICE_FREQ_START_OFFSET_BYTES or something, like patch 2/5 does.
> 
> We read these values from the OA reports. It's bits mashed into the 
> RPT_ID field.
> RPT_ID is the first dword indeed.

Oh, wow, so it is.  I hadn't realized until talking to you today and
reading this that RPT_ID has all kinds of crazy information jammed into
it.  I thought it was just an integer identifier.

This is what I get for not reading those docs in a few generations :)

Looking at the docs for RPT_ID, I'm still not fully tracking, so I
guess I'll look forward to reading your comment...

Thanks for helping me understand all this.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180417/edc9fd2c/attachment.sig>