<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Oct 20, 2016 at 11:10 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 Thu, Oct 20, 2016 at 10:19:05PM +0100, Robert Bragg wrote:<br>
> +int i915_gem_context_pin_legacy_rc<wbr>s_state(struct drm_i915_private *dev_priv,<br>
> +                                       struct i915_gem_context *ctx,<br>
> +                                       u64 flags)<br>
<br>
</span>This is still no.<br>
<div><div class="m_1424486935637572171m_-6368040556181966247h5"><br>
> +static int alloc_oa_buffer(struct drm_i915_private *dev_priv)<br>
> +{<br>
> +     struct drm_i915_gem_object *bo;<br>
> +     enum i915_map_type map;<br>
> +     struct i915_vma *vma;<br>
> +     int ret;<br>
> +<br>
> +     BUG_ON(dev_priv->perf.oa.oa_b<wbr>uffer.obj);<br>
> +<br>
> +     ret = i915_mutex_lock_interruptible(<wbr>&dev_priv->drm);<br>
> +     if (ret)<br>
> +             return ret;<br>
> +<br>
> +     BUILD_BUG_ON_NOT_POWER_OF_2(O<wbr>A_BUFFER_SIZE);<br>
> +     BUILD_BUG_ON(OA_BUFFER_SIZE < SZ_128K || OA_BUFFER_SIZE > SZ_16M);<br>
> +<br>
> +     bo = i915_gem_object_create(&dev_pr<wbr>iv->drm, OA_BUFFER_SIZE);<br>
> +     if (IS_ERR(bo)) {<br>
> +             DRM_ERROR("Failed to allocate OA buffer\n");<br>
> +             ret = PTR_ERR(bo);<br>
> +             goto unlock;<br>
> +     }<br>
> +     dev_priv->perf.oa.oa_buffer.o<wbr>bj = bo;<br>
> +<br>
> +     ret = i915_gem_object_set_cache_leve<wbr>l(bo, I915_CACHE_LLC);<br>
> +     if (ret)<br>
> +             goto err_unref;<br>
> +<br>
> +     /* PreHSW required 512K alignment, HSW requires 16M */<br>
> +     vma = i915_gem_object_ggtt_pin(bo, NULL, 0, SZ_16M, PIN_MAPPABLE);<br>
> +     if (IS_ERR(vma)) {<br>
> +             ret = PTR_ERR(vma);<br>
> +             goto err_unref;<br>
> +     }<br>
> +     dev_priv->perf.oa.oa_buffer.v<wbr>ma = vma;<br>
> +<br>
> +     map = HAS_LLC(dev_priv) ? I915_MAP_WB : I915_MAP_WC;<br>
<br>
</div></div>You set the hw up to do coherent writes into the CPU cache, and then you<br>
request WC access to the pages? With set_cache_level(LLC) you can use<br>
MAP_WB on both llc and snoop based architectures. Fortunately this is<br>
only HSW!<br></blockquote><div><br></div><div>hmm, yeah it looks like I unwittingly added this recently as part of a rebase, I think from lazily copying some similar code from intel_ringbuffer.c when I hit a conflict, without thinking more carefully, sorry.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span><br>
> +     dev_priv->perf.oa.oa_buffer.g<wbr>tt_offset = i915_ggtt_offset(vma);<br>
<br>
</span>I haven't spotted the advantage of storing both the ggtt_offset in<br>
addition to the vma (or the bo as well as the vma).<br></blockquote><div><br></div><div>right, it looks like this can be cleaned up.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="m_1424486935637572171m_-6368040556181966247HOEnZb"><div class="m_1424486935637572171m_-6368040556181966247h5"><br>
> +     dev_priv->perf.oa.oa_buffer.a<wbr>ddr = i915_gem_object_pin_map(bo, map);<br>
> +     if (IS_ERR(dev_priv->perf.oa.oa_b<wbr>uffer.addr)) {<br>
> +             ret = PTR_ERR(dev_priv->perf.oa.oa_b<wbr>uffer.addr);<br>
> +             goto err_unpin;<br>
> +     }<br>
<br>
</div></div><span class="m_1424486935637572171m_-6368040556181966247HOEnZb"><font color="#888888">--<br>
Chris Wilson, Intel Open Source Technology Centre<br></font></span></blockquote><div><br></div><div>Thanks,<br></div><div>- Robert <br></div></div><br></div></div>