On Wed, Oct 22, 2014 at 04:28:51PM +0100, Robert Bragg wrote: > + /* XXX: Not sure that this is really acceptable... > + * > + * i915_gem_context.c currently owns pinning/unpinning legacy > + * context buffers and although that code has a > + * get_context_alignment() func to handle a different > + * constraint for gen6 we are assuming it's fixed for gen7 > + * here. Another option besides pinning here would be to > + * instead hook into context switching and update the > + * OACONTROL configuration on the fly. > + */ > + if (dev_priv->oa_pmu.specific_ctx) { > + struct intel_context *ctx = dev_priv->oa_pmu.specific_ctx; > + int ret; > + > + ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state, > + 4096, 0);
Right if you pin it here with a different alignment, when we try to pin it with the required hw ctx alignment it will fail. Easiest way is to record the ctx->legacy_hw_ctx.alignment and reuse that here. > + if (ret) { > + DRM_DEBUG_DRIVER("Couldn't pin %d\n", ret); > + ret = -EBUSY; As an exercise, think of all the possible error values from pin() and tell me why overriding that here is a bad, bad idea. -Chris -- Chris Wilson, Intel Open Source Technology Centre -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/