On to, 2016-10-20 at 13:49 +0100, Chris Wilson wrote:
> On Mon, Sep 19, 2016 at 06:52:13PM +0300, Joonas Lahtinen wrote:
> >
> > On ke, 2016-09-14 at 07:52 +0100, Chris Wilson wrote:
> > >
> > > @@ -315,17 +304,42 @@ submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
> > > {
> > > struct drm_i915_gem_request *request =
> > > container_of(fence, typeof(*request), submit);
> > > + struct intel_timeline *timeline;
> > > + struct intel_engine_cs *engine = request->engine;
> > > + unsigned long flags;
> > > + u32 seqno;
> > >
> > > /* Will be called from irq-context when using foreign DMA fences */
> > >
> > > - switch (state) {
> > > - case FENCE_COMPLETE:
> > > - request->engine->submit_request(request);
> > > - break;
> > > + if (state != FENCE_COMPLETE)
> > > + return NOTIFY_DONE;
> > >
> > > - case FENCE_FREE:
> > > - break;
> > > - }
> > > + timeline = engine->timeline;
> > > + GEM_BUG_ON(timeline == request->timeline);
> >
> > Umm, why this BUG_ON?
>
> To document that the intent here is to move from the per-context
> timeline onto the global per-engine timeline. If the request was already
> on the engine->timeline bad things would happen, at the very simplest a
> deadlock here.
I see. I would have lifted the assignment and the BUG_ON before actual
code, like elsewhere. But I can live with this too.
The new patch seems to have got rid of the emit_request weirdness too,
so I'll add by R-b there.
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
