"Rebecca N. Palmer" <rebecca_palmer@xxxxxxxx> writes: Hi, >> where cmdparser is disabled, batch_obj is >> left dangling > Sorry! Fixed now. > There is absolutely nothing to be sorry about. > This version also brings exec_start = 0 inside this check, as it > appears to be there because the copying (i915_cmd_parser.c:1054) > removes any offset the original might have had. > > When tested on next-20150508 (675b3fb), it passed my checks > (libva tests, vlc video, glxgears, beignet tests), and didn't > show the "missing window title bar" problem [0-1] in 3 attempts, > but given the intermittent nature of that I can't be sure. > > I still can't give useful i-g-t results, as it works on 3.16 > but reports "GPU HANG" for most tests on 4.0 and (both patched and > unpatched) next (scripts/run-tests.sh at the recovery-mode > (single-user-mode) prompt, both i-g-t 1.10 and latest git). > > [0] http://lists.freedesktop.org/archives/intel-gfx/2015-May/065705.html > [1] http://lists.freedesktop.org/archives/intel-gfx/2015-May/065906.html > > --- > > i915_gem_execbuffer_parse returns the original batch_obj on batches > it can't check (currently, chained batches). Don't set the secure > bit in this case. > > v2 (thanks to Mika Kuoppala): > Don't leave batch_obj unset when the parser is not run. > Only do exec_start = 0 on parsed batches. > Add comments. > > Signed-off-by: Rebecca Palmer <rebecca_palmer@xxxxxxxx> > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index 7ab63d9..2fb6dc1 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -1540,28 +1540,38 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, > } > > if (i915_needs_cmd_parser(ring) && args->batch_len) { > - batch_obj = i915_gem_execbuffer_parse(ring, > + struct drm_i915_gem_object *parsed_batch_obj; > + > + parsed_batch_obj = i915_gem_execbuffer_parse(ring, > &shadow_exec_entry, > eb, > batch_obj, > args->batch_start_offset, > args->batch_len, > file->is_master); > - if (IS_ERR(batch_obj)) { > - ret = PTR_ERR(batch_obj); > + if (IS_ERR(parsed_batch_obj)) { > + /* Batch rejected by parser, or an error > occurred */ This comment should be omitted as the rejection part is not valid in here and the error part is redudant. Daniel can squash it while applying I think. For a first patch, stellar work! Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx> > + ret = PTR_ERR(parsed_batch_obj); > goto err; > } > > - /* > - * Set the DISPATCH_SECURE bit to remove the NON_SECURE > - * bit from MI_BATCH_BUFFER_START commands issued in the > - * dispatch_execbuffer implementations. We specifically > - * don't want that set when the command parser is > - * enabled. > - */ > - dispatch_flags |= I915_DISPATCH_SECURE; > - > - exec_start = 0; > + /* parsed_batch_obj == batch_obj means batch not fully parsed: > + * accept, but don't promote to secure */ > + > + if (parsed_batch_obj != batch_obj) { > + /* > + * Batch parsed and accepted: > + * > + * Set the DISPATCH_SECURE bit to remove the NON_SECURE > + * bit from MI_BATCH_BUFFER_START commands issued in > + * the dispatch_execbuffer implementations. We > + * specifically don't want that set on batches the > + * command parser has accepted. > + */ > + dispatch_flags |= I915_DISPATCH_SECURE; > + exec_start = 0; > + batch_obj = parsed_batch_obj; > + } > } > > batch_obj->base.pending_read_domains |= I915_GEM_DOMAIN_COMMAND; > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
