On Wed, Sep 18, 2013 at 12:33:32AM +0100, Chris Wilson wrote: > On Tue, Sep 17, 2013 at 04:14:43PM -0700, Ben Widawsky wrote: > > On Tue, Sep 17, 2013 at 09:55:35PM +0100, Chris Wilson wrote: > > > On Tue, Sep 17, 2013 at 10:01:33AM -0700, Ben Widawsky wrote: > > > > @@ -1117,8 +1109,13 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, > > > > * batch" bit. Hence we need to pin secure batches into the global gtt. > > > > * hsw should have this fixed, but let's be paranoid and do it > > > > * unconditionally for now. */ > > > > - if (flags & I915_DISPATCH_SECURE && !batch_obj->has_global_gtt_mapping) > > > > - i915_gem_gtt_bind_object(batch_obj, batch_obj->cache_level); > > > > + if (flags & I915_DISPATCH_SECURE && > > > > + !batch_obj->has_global_gtt_mapping) { > > > > + const struct i915_address_space *ggtt = obj_to_ggtt(batch_obj); > > > > + struct i915_vma *vma = i915_gem_obj_to_ggtt(batch_obj); > > > > + BUG_ON(!vma); > > > > + ggtt->bind_vma(vma, batch_obj->cache_level, GLOBAL_BIND); > > > > + } > > > > > > The issue here is that if we don't set the USE_PPGTT/USE_SECURE flag in > > > the dispatch, the CS will use the GGTT (hence our binding) but so we > > > then need to use the GGTT offset for the dispatch as well. > > > > > > Is that as concisely as we can write bind_to_ggtt? :( > > > -Chris > > > > > > > Resuming the conversation started on irc... what do you want from me? > > I think we need to pass the ggtt offset to dispatch for > I915_DISPATCH_SECURE -- which offset to use might even depend upon the > implementation and hw generation in intel_ringbuffer.c. But at the very > least, I think SNB/IVB will be executing the wrong address come full > ppgtt. > > dev_priv->ggtt.base.bind_vma(i915_gem_obj_to_ggtt(batch_obj), > batch_obj->cache_level, > GLOBAL_BIND); > > #define i915_vm_bind(vm__, vma__, cache_level__, flags__) \ > (vm__)->bind_vma((vma__), (cache_level__), (flags__)) > > i915_vm_bind(&dev_priv->ggtt.base, > i915_gem_obj_to_ggtt(batch_obj), > batch_obj->cache_level, > GLOBAL_BIND); > -Chris > I915_DISPATCH_SECURE is a special case. If we see the flag, we look up the GGTT offset as opposed to the offset in the VM being used at execbuf. We can either bind the batchbuffer into both the PPGTT, and GGTT, or it's only even in the GGTT - in either case, we'll have to have done the bind (and found space in the drm_mm). It just seems like this is duplicating the already existing i915_gem_object_bind_to_vm code that's in place. Sorry if I am not following what you're asking. I'm just failing to see a problem, or maybe you're just trying to solve problems that I haven't yet conceived; or solved in a different way. It's pretty darn hard to discuss this given the piecemeal nature of the thing. -- Ben Widawsky, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx