<div dir="ltr"><div><div><div>Sorry for the delay replying to this, I missed it.<br></div></div></div><div><div><div><div><div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Apr 23, 2016 at 11:34 AM, Martin Peres <span dir="ltr"><<a href="mailto:martin.peres@free.fr" target="_blank">martin.peres@free.fr</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="">On 20/04/16 17:23, Robert Bragg wrote:<br>
</span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="">
Gen graphics hardware can be set up to periodically write snapshots of<br>
performance counters into a circular buffer via its Observation<br>
Architecture and this patch exposes that capability to userspace via the<br>
i915 perf interface.<br>
<br>
Cc: Chris Wilson <<a href="mailto:chris@chris-wilson.co.uk" target="_blank">chris@chris-wilson.co.uk</a>><br>
Signed-off-by: Robert Bragg <<a href="mailto:robert@sixbynine.org" target="_blank">robert@sixbynine.org</a>><br>
Signed-off-by: Zhenyu Wang <<a href="mailto:zhenyuw@linux.intel.com" target="_blank">zhenyuw@linux.intel.com</a>><br>
---<br>
  drivers/gpu/drm/i915/i915_drv.h         |  56 +-<br>
  drivers/gpu/drm/i915/i915_gem_context.c |  24 +-<br>
  drivers/gpu/drm/i915/i915_perf.c        | 940 +++++++++++++++++++++++++++++++-<br>
  drivers/gpu/drm/i915/i915_reg.h         | 338 ++++++++++++<br>
  include/uapi/drm/i915_drm.h             |  70 ++-<br>
  5 files changed, 1408 insertions(+), 20 deletions(-)<br>
<br></span>
+<span class=""><br>
+<br>
+       /* It takes a fairly long time for a new MUX configuration to<br>
+        * be be applied after these register writes. This delay<br>
+        * duration was derived empirically based on the render_basic<br>
+        * config but hopefully it covers the maximum configuration<br>
+        * latency...<br>
+        */<br>
+       mdelay(100);<br>
</span></blockquote>
<br>
With such a HW and SW design, how can we ever expose hope to get any<br>
kind of performance when we are trying to monitor different metrics on each<br>
draw call? This may be acceptable for system monitoring, but it is problematic<br>
for the GL extensions :s<br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Since it seems like we are going for a perf API, it means that for every change<br>
of metrics, we need to flush the commands, wait for the GPU to be done, then<br>
program the new set of metrics via an IOCTL, wait 100 ms, and then we may<br>
resume rendering ... until the next change. We are talking about a latency of<br>
6-7 frames at 60 Hz here... this is non-negligeable...<br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
I understand that we have a ton of counters and we may hide latency by not<br>
allowing using more than half of the counters for every draw call or frame, but<br>
even then, this 100ms delay is killing this approach altogether.<br></blockquote><div><br><div><br></div><div>Although I'm also really unhappy about introducing 
this delay recently, the impact of the delay is typically amortized 
somewhat by keeping a configuration open as long as possible.<br><br>Even
 without this explicit delay here the OA unit isn't suited  to being 
reconfigured on a per draw call basis, though it is able to 
support per draw call queries with the same config.<br><br></div><div>The above assessment assumes wanting to change config between draw calls which is not something this driver aims to support - as the HW isn't really designed for that model.<br></div><div><br>E.g. in the 
case of INTEL_performance_query, the backend keeps the i915 perf stream 
open until all OA based  query objects are deleted - so you  have to be 
pretty explicit if you want to change config.<br><br></div><div>Considering the sets available on Haswell:<br>* Render Metrics Basic<br>* Compute Metrics Basic<br>* Compute Metrics Extended<br>* Memory Reads Distribution<br>* Memory Writes Distribution<br>* Metric set SamplerBalance<br><br></div><div>Each of  these configs can expose around 50 counters as a set.<br></div><div><br></div><div>A GL application is most likely just going to use the render basic set, and In the case of a tool like gputop/GPA then changing config would usually be driven by some user 
interaction to select a set of metrics, where even a 100ms delay will go unnoticed.<br><br></div><div>In case you aren't familiar with how the GL_INTEL_performance_query side of things works for OA counters; one thing to be aware of is that there's a separate MI_REPORT_PERF_COUNT command that Mesa writes either side of a query which writes all the counters for the current OA config (as configured via this i915 perf interface) to a buffer. In addition to collecting reports via MI_REPORT_PERF_COUNT Mesa also configures the unit for periodic sampling to be able to account for potential counter overflow.<br></div><div><br><br>It also might be worth keeping in mind that per draw queries will 
anyway trash the pipelining of work, since it's necessary to put stalls
 between the draw calls to avoid conflated metrics (not to do with 
the details of this driver) so use cases will probably be limited to 
those that just want the draw call numbers but don't mind ruining overall 
frame/application performance. Periodic sampling or 
swap-to-swap queries would be better suited to cases that should 
minimize their impact. <br> </div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
To be honest, if it indeed is an HW bug, then the approach that Samuel Pitoiset<br>
and I used for Nouveau involving pushing an handle representing a<br>
pre-computed configuration to the command buffer so as a software method<br>
can be ask the kernel to reprogram the counters with as little idle time as<br>
possible, would be useless as waiting for the GPU to be idle would usually not<br>
take more than a few ms... which is nothing compared to waiting 100ms.<br></blockquote><div><br></div><div>Yeah, I think this is a really quite different programming model to what the OA unit is geared for, even if we can somehow knock out this 100ms MUX config delay.<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
So, now, the elephant in the room, how can it take that long to apply the<br>
change? Are the OA registers double buffered (NVIDIA's are, so as we can<br>
reconfigure and start monitoring multiple counters at the same time)?<br></blockquote><br></div><div>Based on my understanding of how the HW works internally I can see how some delay would be expected, but can't currently  fathom why it would need to have this order of magnitude, and so the delay is currently simply based on experimentation where I was getting unit test failures at 10ms, for invalid looking reports, but the tests ran reliably at 100ms.<br></div><div><br></div><div>OA configuration state isn't  double buffered to allow configuration while in use.<br></div><div><br></div><div> </div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Maybe this 100ms is the polling period and the HW does not allow changing<br>
the configuration in the middle of a polling session. In this case, this delay<br>
should be dependent on the polling frequency. But even then, I would really<br>
hope that the HW would allow us to tear down everything, reconfigure and<br>
start polling again without waiting for the next tick. If not possible, maybe we<br>
can change the frequency for the polling clock to make the polling event happen<br>
sooner.<br></blockquote><div> <br></div><div>The tests currently test periods from 160ns to 168 milliseconds while the delay required falls somewhere between 10 and 100 milliseconds. I think I'd expect the delay to be > all periods tested if this was the link.<br><br></div><div>Generally this seems unlikely to me, in part considering how the MUX isn't really part of the OA unit that handles periodic sampling. I wouldn't rule out some interaction though so some experimenting along these lines could be interesting.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
HW delays are usually a few microseconds, not milliseconds, that really suggests<br>
that something funny is happening and the HW design is not understood properly.<br></blockquote><div><br></div><div>Yup.<br><br></div><div>Although I understand more about the HW than I can write up here, I can't currently see why the HW should ever really take this long to apply a MUX config - although I can see why some delay would be required.<br><br>It's on my list of things to try and get feedback/ideas on from the OA architect/HW engineers. I brought this up briefly some time ago but we didn't have time to go into details.<br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
If the documentation has nothing on this and the HW teams cannot help, then I<br>
suggest a little REing session </blockquote><div> <br></div><div>There's no  precisely documented delay requirement. Insofar as REing is the process of inferring how black box HW works through poking it with a stick and seeing how it reacts, then yep more of that may be necessary. At least in this case the HW isn't really a black box (maybe stain glass), where I hopefully have a fairly good sense of how the HW is designed and can prod folks closer to the HW for feedback/ideas.<br></div><div><br></div><div>So far I haven't spent too long investigating this besides recently homing in on needing a delay here when my unit tests were failing.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I really want to see this work land, but the way I see<br>
it right now is that we cannot rely on it because of this bug. Maybe fixing this bug<br>
would require changing the architecture, so better address it before landing the<br>
patches.<br></blockquote><div><br></div><div>I think it's unlikely to change the architecture; rather we might just find some other things to frob that make the MUX config apply faster (e.g. clock gating issue); we find a way to get explicit feedback of completion so we can minimize the delay or a better understanding that lets us choose a shorter delay in most cases.<br><br></div><div>The driver is already usable with gputop with this delay and considering how config changes are typically associated with user interaction I wouldn't see this as a show stopper - even though it's not ideal. I think the assertions about it being unusable with GL, were a little overstated based on making frequent OA config changes which is not really how the interface is intended to be used.<br></div><div><br><br></div><div>Thanks for starting to take a look through the code.<br></div><div><br></div><div>Kind Regards,<br></div><div>- Robert<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Worst case scenario, do not hesitate to contact me if non of the proposed<br>
explanation pans out, I will take the time to read through the OA material and try my<br>
REing skills on it. As I said, I really want to see this upstream! <br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Sorry...<span class=""><font color="#888888"><br>
<br>
Martin<br>
</font></span></blockquote></div><br></div></div></div></div></div></div></div></div></div>