On ke, 2016-08-10 at 10:02 +0100, Chris Wilson wrote:
> On Wed, Aug 10, 2016 at 11:41:39AM +0300, Joonas Lahtinen wrote:
> >
> > On su, 2016-08-07 at 15:45 +0100, Chris Wilson wrote:
> > >
> > > @@ -771,6 +771,13 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
> > > if (skip_rcs_switch(ppgtt, engine, to))
> > > return 0;
> > >
> > > + if (!(to->engine[RCS].state->flags & I915_VMA_GLOBAL_BIND)) {
> > > + ret = i915_gem_object_set_to_gtt_domain(to->engine[RCS].state->obj,
> > > + false);
> > > + if (ret)
> > > + return ret;
> > > + }
> > > +
> > > /* Trying to pin first makes error handling easier. */
> > > ret = i915_vma_pin(to->engine[RCS].state,
> > > 0, to->ggtt_alignment,
> > This could be lifted inside the if?
> I'd rather not as that would be more unusual and we would then need a
> complicated error path.
>
> If the compiler is feeling intelligent it could combine the two blocks
> into one, i.e.
>
> if (unlikely((++vma->flags ^ flags) & I915_VMA_BIND_MASK)) {
> ret = i915_gem_object_set_to_gtt_domain(vma->obj, false);
> if (ret)
> return ret;
>
> ret = __i915_vma_do_pin(vma, size, alignment, flags);
> if (ret)
> return ret;
> }
>
> I don't suggest we do the expansion ourselves, and I don't have a bit to
> spare in the PIN_FLAGS.
With the comment moved/updated (mentioned in previous patch);
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> -Chris
>
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
