Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> Quoting Mika Kuoppala (2019-08-06 11:40:48)
>> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
>> > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
>> > index 8ac7d14ec8c9..81094f250bdb 100644
>> > --- a/drivers/gpu/drm/i915/i915_request.c
>> > +++ b/drivers/gpu/drm/i915/i915_request.c
>> > @@ -1198,7 +1198,6 @@ struct i915_request *__i915_request_commit(struct i915_request *rq)
>> > */
>> > local_bh_disable();
>> > i915_sw_fence_commit(&rq->semaphore);
>> > - rcu_read_lock(); /* RCU serialisation for set-wedged protection */
>>
>> We don't need to protect against attr changes anymore so yes...
>>
>> > if (engine->schedule) {
>> > struct i915_sched_attr attr = rq->gem_context->sched;
>> >
>> > @@ -1228,7 +1227,6 @@ struct i915_request *__i915_request_commit(struct i915_request *rq)
>> >
>> > engine->schedule(rq, &attr);
>>
>> but will now schedule during wedged. Didn't notice anything that
>> would blowup on reordering but is this intentional?
>
> How do you think it will blow up? engine->schedule remains untouched
> over wedged, and all it is doing is traversing the dependency lists
> (which remain intact) and the scheduler list (which is controlled by
> locking and cleared inside engine->cancel_requests, after which point it
> will remain clear as nop_submit_request should not care).
>
> I don't think there's a bad interaction there between i915_schedule()
> and nop_submit_request...
Didn't find anything that would blow up. Just a notable change in
behaviour so tried to poke around a bit. The less we special case
the better so nothing against the idea.
Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
