<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Apr 20, 2016 at 11:46 PM, Chris Wilson <span dir="ltr"><<a href="mailto:chris@chris-wilson.co.uk" target="_blank">chris@chris-wilson.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>On Wed, Apr 20, 2016 at 03:23:10PM +0100, Robert Bragg wrote:<br>
</span><span>> +static void gen7_init_oa_buffer(struct drm_i915_private *dev_priv)<br>
> +{<br>
> +     /* Pre-DevBDW: OABUFFER must be set with counters off,<br>
> +      * before OASTATUS1, but after OASTATUS2<br>
> +      */<br>
> +     I915_WRITE(GEN7_OASTATUS2, dev_priv->perf.oa.oa_buffer.gtt_offset |<br>
> +                OA_MEM_SELECT_GGTT); /* head */<br>
> +     I915_WRITE(GEN7_OABUFFER, dev_priv->perf.oa.oa_buffer.gtt_offset);<br>
> +     I915_WRITE(GEN7_OASTATUS1, dev_priv->perf.oa.oa_buffer.gtt_offset |<br>
> +                OABUFFER_SIZE_16M); /* tail */<br>
> +<br>
> +     /* On Haswell we have to track which OASTATUS1 flags we've<br>
> +      * already seen since they can't be cleared while periodic<br>
> +      * sampling is enabled.<br>
> +      */<br>
> +     dev_priv->perf.oa.gen7_latched_oastatus1 = 0;<br>
> +<br>
> +     /* We have a sanity check in gen7_append_oa_reports() that<br>
> +      * looks at the report-id field to make sure it's non-zero<br>
> +      * which relies on the assumption that new reports are<br>
> +      * being written to zeroed memory...<br>
> +      */<br>
> +     memset(dev_priv->perf.oa.oa_buffer.addr, 0, SZ_16M);<br>
<br>
</span>You allocated zeroed memory.<br></blockquote><div><br></div><div>yup. currently I have this memset here because we may re-init the buffer if the stream is disabled then re-enabled (via I915_PERF_IOCTL_ENABLE) or if we have to reset the unit on error. In these cases there may be some number of reports in the buffer with non-zero report-id fields while we still want to be sure new reports are being written to zereod memory so that the sanity check that report-id != 0 will continue to be valid.<br><br></div><div>I've had it in mind to consider optimizing this at some point to minimize how much of the buffer is cleared, maybe just  for the _DISABLE/_ENABLE case where I'd expect the buffer will mostly be empty before disabling the stream.<br><br></div><div>- Robert <br></div><div><br><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div>-Chris<br>
<br>
--<br>
Chris Wilson, Intel Open Source Technology Centre<br>
</div></div></blockquote></div><br></div></div>