Re: [PATCH 03/11] drm/i915/execlists: Pull submit after dequeue under timeline lock

From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 03/11] drm/i915/execlists: Pull submit after dequeue under timeline lock
Date: Mon, 4 Jun 2018 10:25:46 +0100	[thread overview]
Message-ID: <9acbd3f1-f19f-20c9-24ed-4dfc9c3b4b21@linux.intel.com> (raw)
In-Reply-To: <152786207790.8943.10821645677650099400@mail.alporthouse.com>

On 01/06/2018 15:07, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-06-01 15:02:38)
>>
>> On 31/05/2018 19:51, Chris Wilson wrote:
>>> In the next patch, we will begin processing the CSB from inside the
>>> interrupt handler. This means that updating the execlists->port[] will
>>
>> The message that we will be processing CSB from irq handler, here and in
>> following patch 5/11, doesn't seem to be true. So why is this needed?
>> Why not just drop some patches from the series?
> 
> It will still be called from irq context; as submit_request is called
> from irq context. Hence we still require irqsafe variants.

submit_request is already called from irqoff contexts so I don't get it.

> So yes, while the overall direction of the patchset changed slightly to
> be less enthusiastic about calling from irq context, such calls were not
> eliminated.

I have a problem with commit messages where one says we'll be processing 
CSB directly from hard irq, while another says we'll always use the tasklet.

It is very hard to review it like this since I cannot rely on the high 
level narrative for a little bit of help. To help me figure out what is 
needed in this latest incarnation, and what perhaps isn't. And we 
shouldn't really do it like this in general (have inconsistent commit 
messages).

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx