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!
> + 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).
> + 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
