<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 03/04/17 21:04, Robert Bragg wrote:<br>
</div>
<blockquote
cite="mid:CAMou1-3u8FmLjYZHkVr1h11tjEPwZ00Uo-oCmyt=-KSKarcpiw@mail.gmail.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<div dir="auto">
<div><br>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Mar 30, 2017 16:16, "Lionel
Landwerlin" <<a moz-do-not-send="true"
href="mailto:lionel.g.landwerlin@intel.com">lionel.g.landwerlin@intel.com</a>>
wrote:<br type="attribution">
<blockquote class="quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">While
exercising reading report with moderate load, we might
have to<br>
wait for all the reports to land in the OA buffer,
otherwise we might<br>
miss some reports. That means we need to keep on reading
the OA stream<br>
until the last report we read has a timestamp older that
the timestamp<br>
recorded by the MI_REPORT_PERF_COUNT at the end of the
performance<br>
query.<br>
</blockquote>
</div>
</div>
</div>
<div dir="auto"><br>
</div>
<div dir="auto">The expectation is that since we have received
the second MI_REPORT_PERF_COUNT report that implies any
periodic/automatic reports that the could possibly relate to a
query must have been written to the OABUFFER. Since we read()
as much as is available from i915 perf when we come to finish
processing a query then we shouldn't miss anything.</div>
<div dir="auto"><br>
</div>
<div dir="auto">We shouldn't synchronously wait in a busy loop
until we've found a report that has a timestamp >= our end
MI_RPC because that could be a really significant amount of
time (e.g. 50+ milliseconds on HSW). NB. the periodic samples
are just to account for counter overflow. On Gen8+ the loop
would likely block until the next context switch happens.</div>
</div>
</blockquote>
<br>
Right, so if we don't get all the reports until the second
MI_REPORT_PERF_COUNT, we might be dealing with a kernel issue.<br>
<br>
I'm still suspicious of the code in mesa. I think in some cases (in
particular with many applications running) it might loop forever.<br>
<br>
Also the igt tests using roughly the same loop while (read()
&& errno != EINTR) are pretty reliable at not finding all
the reports we need from to the stream.<br>
So I would expect the same thing from mesa.<br>
<br>
<blockquote
cite="mid:CAMou1-3u8FmLjYZHkVr1h11tjEPwZ00Uo-oCmyt=-KSKarcpiw@mail.gmail.com"
type="cite">
<div dir="auto">
<div dir="auto"><br>
</div>
<div dir="auto">Was this proposal based on a code inspection or
did you maybe hit a problem correctly measuring something?</div>
<div dir="auto"><br>
</div>
<div dir="auto">Maybe if based on inspection we can find
somewhere to put a comment to clarify the assumptions above?</div>
<div dir="auto"><br>
</div>
<div dir="auto">Br,</div>
<div dir="auto">- Robert</div>
<div dir="auto"><br>
</div>
<div dir="auto">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Signed-off-by: Lionel Landwerlin <<a
moz-do-not-send="true"
href="mailto:lionel.g.landwerlin@intel.com">lionel.g.landwerlin@intel.com</a><wbr>><br>
Cc: Robert Bragg <<a moz-do-not-send="true"
href="mailto:robert@sixbynine.org">robert@sixbynine.org</a>><br>
---<br>
src/mesa/drivers/dri/i965/brw_<wbr>performance_query.c
| 51 ++++++++++++++---------<br>
1 file changed, 32 insertions(+), 19 deletions(-)<br>
<br>
diff --git a/src/mesa/drivers/dri/i965/<wbr>brw_performance_query.c
b/src/mesa/drivers/dri/i965/<wbr>brw_performance_query.c<br>
index 4a94e4b3cc..076c59e633 100644<br>
--- a/src/mesa/drivers/dri/i965/<wbr>brw_performance_query.c<br>
+++ b/src/mesa/drivers/dri/i965/<wbr>brw_performance_query.c<br>
@@ -647,38 +647,50 @@ discard_all_queries(struct
brw_context *brw)<br>
}<br>
<br>
static bool<br>
-read_oa_samples(struct brw_context *brw)<br>
+read_oa_samples_until(struct brw_context *brw,<br>
+ uint32_t start_timestamp,<br>
+ uint32_t end_timestamp)<br>
{<br>
- while (1) {<br>
+ uint32_t last_timestamp = start_timestamp;<br>
+<br>
+ while (last_timestamp < end_timestamp) {<br>
struct brw_oa_sample_buf *buf =
get_free_sample_buf(brw);<br>
+ uint32_t offset;<br>
int len;<br>
<br>
while ((len = read(brw->perfquery.oa_stream_<wbr>fd,
buf->buf,<br>
- sizeof(buf->buf))) < 0
&& errno == EINTR)<br>
+ sizeof(buf->buf))) < 0
&&<br>
+ (errno == EINTR || errno == EAGAIN))<br>
;<br>
<br>
if (len <= 0) {<br>
exec_list_push_tail(&brw-><wbr>perfquery.free_sample_buffers,
&buf->link);<br>
<br>
- if (len < 0) {<br>
- if (errno == EAGAIN)<br>
- return true;<br>
- else {<br>
- DBG("Error reading i915 perf samples:
%m\n");<br>
- return false;<br>
- }<br>
- } else {<br>
+ if (len < 0)<br>
+ DBG("Error reading i915 perf samples:
%m\n");<br>
+ else<br>
DBG("Spurious EOF reading i915 perf
samples\n");<br>
- return false;<br>
- }<br>
+<br>
+ return false;<br>
}<br>
<br>
buf->len = len;<br>
exec_list_push_tail(&brw-><wbr>perfquery.sample_buffers,
&buf->link);<br>
+<br>
+ /* Go through the reports and update the last
timestamp. */<br>
+ while (offset < buf->len) {<br>
+ const struct drm_i915_perf_record_header
*header =<br>
+ (const struct drm_i915_perf_record_header
*) &buf->buf[offset];<br>
+ uint32_t *report = (uint32_t *) (header + 1);<br>
+<br>
+ if (header->type ==
DRM_I915_PERF_RECORD_SAMPLE)<br>
+ last_timestamp = report[1];<br>
+<br>
+ offset += header->size;<br>
+ }<br>
}<br>
<br>
- unreachable("not reached");<br>
- return false;<br>
+ return true;<br>
}<br>
<br>
/**<br>
@@ -709,10 +721,6 @@ accumulate_oa_reports(struct
brw_context *brw,<br>
<br>
assert(o->Ready);<br>
<br>
- /* Collect the latest periodic OA reports from i915
perf */<br>
- if (!read_oa_samples(brw))<br>
- goto error;<br>
-<br>
drm_intel_bo_map(obj-><a moz-do-not-send="true"
href="http://oa.bo" rel="noreferrer" target="_blank">oa.bo</a>,
false);<br>
query_buffer = obj->oa.bo->virtual;<br>
<br>
@@ -728,6 +736,11 @@ accumulate_oa_reports(struct
brw_context *brw,<br>
goto error;<br>
}<br>
<br>
+ /* Collect the latest periodic OA reports from i915
perf */<br>
+ if (!read_oa_samples_until(brw, start[1], end[1]))<br>
+ goto error;<br>
+<br>
+<br>
/* See if we have any periodic reports to accumulate
too... */<br>
<br>
/* N.B. The oa.samples_head was set when the query
began and<br>
<font color="#888888">--<br>
2.11.0<br>
<br>
</font></blockquote>
</div>
<br>
</div>
</div>
</div>
</blockquote>
<p><br>
</p>
</body>
</html>