[Intel-gfx] [PATCH 3/7] drm/i915: Add per context timelines to fence object

[John Harrison]

On 11/01/2016 22:58, Chris Wilson wrote:
> On Mon, Jan 11, 2016 at 02:47:33PM -0800, Jesse Barnes wrote:
>> On 01/11/2016 11:03 AM, John Harrison wrote:
>>> On 08/01/2016 22:05, Chris Wilson wrote:
>>>> On Fri, Jan 08, 2016 at 06:47:24PM +0000, John.C.Harrison at Intel.com wrote:
>>>>> From: John Harrison <John.C.Harrison at Intel.com>
>>>>>
>>>>> The fence object used inside the request structure requires a sequence
>>>>> number. Although this is not used by the i915 driver itself, it could
>>>>> potentially be used by non-i915 code if the fence is passed outside of
>>>>> the driver. This is the intention as it allows external kernel drivers
>>>>> and user applications to wait on batch buffer completion
>>>>> asynchronously via the dma-buff fence API.
>>>> That doesn't make any sense as they are not limited by a single
>>>> timeline.
>>> I don't understand what you mean. Who is not limited by a single timeline?  The point is that the current seqno values cannot be used as there is no guarantee that they will increment globally once things like a scheduler and pre-emption arrive. Whereas, the fence internal implementation makes various assumptions about the linearity of the timeline. External users do not want to care about timelines or seqnos at all, they just want the fence API to work as documented.
>>>
>>>>> To ensure that such external users are not confused by strange things
>>>>> happening with the seqno, this patch adds in a per context timeline
>>>>> that can provide a guaranteed in-order seqno value for the fence. This
>>>>> is safe because the scheduler will not re-order batch buffers within a
>>>>> context - they are considered to be mutually dependent.
>>>> You haven't added per-context breadcrumbs. What we need for being able
>>>> to execute requests from parallel timelines, but with requests within a
>>>> timeline being ordered, is a per-context page where we can emit the
>>>> per-context issued breadcrumb. Then instead of looking up the current
>>>> HW seqno in a global page, the request just looks at the current context
>>>> HW seqno in the context seq, just
>>>> i915_seqno_passed(*req->p_context_seqno, req->seqno).
>>> This patch is not attempting to implement per context seqno values. That can be done as future work. This patch is doing the simplest, least invasive implementation in order to make external fences work.
>> Right.  I think we want to move to per-context seqnos, but we don't have to do it before this work lands.  It should be easier to do it after the rest of these bits land in fact, since seqno handling will be well encapsulated aiui.
> This patch is irrelevent then. I think it is actually worse because it
> is encapsulating a design detail that is fundamentally wrong.
> -Chris
>

Some kind of per-context timeline is required for the external use of 
i915 fences. Seqnos cannot be used without a lot of rework because they 
dance around with scheduler re-ordering and pre-emption - a low priority 
request could go through many different seqnos if it keeps getting 
pre-empted. We need to be able to use fences externally on Android at 
least, and with SVM it becomes vital for linux too. Therefore we need 
some solution. And this is much simpler and than re-writing the whole of 
the driver's seqno management.