On Sun, 2012-10-14 at 13:20 +0200, Daniel Vetter wrote:
> On Sun, Oct 14, 2012 at 12:14 PM, Ben Hutchings <[email protected]> wrote:
> > Please don't drop the context of which commit we are talking about.
> >
> > For further reference, it is 7dd490658627 ('drm/i915: Mark untiled BLT
> > commands as fenced on gen2/3')
> >
> > On Mon, 2012-10-08 at 10:49 +0100, Luís Picciochi Oliveira wrote:
> >> Hi,
> >>
> >> Here is the actual diff to the 3.2 kernel:
> >>
> >> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c      2012-07-12
> >> 04:32:21.000000000 +0100
> >> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c      2012-10-02
> >> 22:17:56.079302807 +0100
> >> @@ -573,8 +573,8 @@ i915_gem_execbuffer_reserve(struct intel
> >>                                       ret = i915_gem_object_put_fence(obj);
> >>                                       if (ret)
> >>                                               break;
> >> +                             obj->pending_fenced_gpu_access = true;
> >>                               }
> >> -                             obj->pending_fenced_gpu_access = need_fence;
> >>                       }
> >>
> >>                       entry->offset = obj->gtt_offset;
[...]
> > Maybe it should be:
> >
> > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > @@ -574,7 +574,7 @@
> >                                         if (ret)
> >                                                 break;
> >                                 }
> > -                               obj->pending_fenced_gpu_access = need_fence;
> > +                               obj->pending_fenced_gpu_access = true;
> >                         }
> >
> >                         entry->offset = obj->gtt_offset;
> >                 }
> 
> This should work, although for safety I think I'd be better to keep
> the 2nd conditional, i.e.
> 
>  --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>  +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>  @@ -574,7 +574,7 @@
>                                          if (ret)
>                                                  break;
>                                  }
>  -                               obj->pending_fenced_gpu_access = need_fence;
>  +                               obj->pending_fenced_gpu_access
> =entry->flags & EXEC_OBJECT_NEEDS_FENCE ;
>                          }
> 
>                          entry->offset = obj->gtt_offset;
>                  }
> 
> In any case, intel-gpu-tools
> (http://cgit.freedesktop.org/xorg/app/intel-gpu-tools/) has a neat
> testcase to exercise this corner case. If
> i-g-t/tests/gem_set_tiling_vs_blt is still happy, the fix should be
> correct.

I don't have anything with gen2/gen3 chips in (I think that's i830 up to
i945) so I'll put your version in the queue now and let Luís or others
test that.

Ben.

-- 
Ben Hutchings
Always try to do things in chronological order;
it's less confusing that way.

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to