On Thu, Oct 20, 2016 at 11:10 PM, Chris Wilson chris@chris-wilson.co.uk wrote:
On Thu, Oct 20, 2016 at 10:19:05PM +0100, Robert Bragg wrote:
+int i915_gem_context_pin_legacy_rcs_state(struct drm_i915_private
*dev_priv,
struct i915_gem_context *ctx,u64 flags)This is still no.
+static int alloc_oa_buffer(struct drm_i915_private *dev_priv) +{
struct drm_i915_gem_object *bo;enum i915_map_type map;struct i915_vma *vma;int ret;BUG_ON(dev_priv->perf.oa.oa_buffer.obj);ret = i915_mutex_lock_interruptible(&dev_priv->drm);if (ret)return ret;BUILD_BUG_ON_NOT_POWER_OF_2(OA_BUFFER_SIZE);BUILD_BUG_ON(OA_BUFFER_SIZE < SZ_128K || OA_BUFFER_SIZE > SZ_16M);bo = i915_gem_object_create(&dev_priv->drm, OA_BUFFER_SIZE);if (IS_ERR(bo)) {DRM_ERROR("Failed to allocate OA buffer\n");ret = PTR_ERR(bo);goto unlock;}dev_priv->perf.oa.oa_buffer.obj = bo;ret = i915_gem_object_set_cache_level(bo, I915_CACHE_LLC);if (ret)goto err_unref;/* PreHSW required 512K alignment, HSW requires 16M */vma = i915_gem_object_ggtt_pin(bo, NULL, 0, SZ_16M, PIN_MAPPABLE);if (IS_ERR(vma)) {ret = PTR_ERR(vma);goto err_unref;}dev_priv->perf.oa.oa_buffer.vma = vma;map = HAS_LLC(dev_priv) ? I915_MAP_WB : I915_MAP_WC;You set the hw up to do coherent writes into the CPU cache, and then you request WC access to the pages? With set_cache_level(LLC) you can use MAP_WB on both llc and snoop based architectures. Fortunately this is only HSW!
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.
dev_priv->perf.oa.oa_buffer.gtt_offset = i915_ggtt_offset(vma);I haven't spotted the advantage of storing both the ggtt_offset in addition to the vma (or the bo as well as the vma).
right, it looks like this can be cleaned up.
dev_priv->perf.oa.oa_buffer.addr = i915_gem_object_pin_map(bo,map);
if (IS_ERR(dev_priv->perf.oa.oa_buffer.addr)) {ret = PTR_ERR(dev_priv->perf.oa.oa_buffer.addr);goto err_unpin;}-- Chris Wilson, Intel Open Source Technology Centre
Thanks, - Robert