Op 13-05-16 om 11:16 schreef John Harrison:
> On 13/05/2016 08:39, Chris Wilson wrote:
>> On Thu, May 12, 2016 at 10:06:31PM +0100, john.c.harri...@intel.com wrote:
>>> From: John Harrison <john.c.harri...@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.
>>>
>>> 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.
>>>
>>> v2: New patch in series.
>>>
>>> v3: Renamed/retyped timeline structure fields after review comments by
>>> Tvrtko Ursulin.
>>>
>>> Added context information to the timeline's name string for better
>>> identification in debugfs output.
>>>
>>> v5: Line wrapping and other white space fixes to keep style checker
>>> happy.
>>>
>>> v7: Updated to newer nightly (lots of ring -> engine renaming).
>>>
>>> v8: Moved to earlier in patch series so no longer needs to remove the
>>> quick hack timeline that was being added before.
>>>
>>> For: VIZ-5190
>>> Signed-off-by: John Harrison <john.c.harri...@intel.com>
>>> Cc: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
>>> Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
>>> ---
>>> drivers/gpu/drm/i915/i915_drv.h | 14 ++++++++++++
>>> drivers/gpu/drm/i915/i915_gem.c | 40
>>> +++++++++++++++++++++++++++++++++
>>> drivers/gpu/drm/i915/i915_gem_context.c | 14 ++++++++++++
>>> drivers/gpu/drm/i915/intel_lrc.c | 8 +++++++
>>> 4 files changed, 76 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>> b/drivers/gpu/drm/i915/i915_drv.h
>>> index d5496ab..520480b 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -818,6 +818,19 @@ struct i915_ctx_hang_stats {
>>> bool banned;
>>> };
>>> +struct i915_fence_timeline {
>>> + char name[32];
>>> + unsigned fence_context;
>>> + unsigned next;
>>> +
>>> + struct intel_context *ctx;
>>> + struct intel_engine_cs *engine;
>>> +};
>>> +
>>> +int i915_create_fence_timeline(struct drm_device *dev,
>>> + struct intel_context *ctx,
>>> + struct intel_engine_cs *ring);
>>> +
>>> /* This must match up with the value previously used for execbuf2.rsvd1.
>>> */
>>> #define DEFAULT_CONTEXT_HANDLE 0
>>> @@ -869,6 +882,7 @@ struct intel_context {
>>> u64 lrc_desc;
>>> uint32_t *lrc_reg_state;
>>> bool initialised;
>>> + struct i915_fence_timeline fence_timeline;
>> This is still fundamentally wrong. This i915_fence_timeline is both the
>> fence context and timeline. Our timelines are singular per vm, with a
>> fence context under each timeline per engine.
> As I said last time you mentioned 'per vm', please explain. Where does a
> virtual machine come in? Or is that not what you mean? What is the purpose of
> having multiple fence contexts within a single timeline? It will stop
> external uses attempting to combine fences but it won't stop them from
> actually being out of order. The timeline needs to be per hardware context
> not simply per engine because the whole point is that requests should not be
> re-ordered once they have been allocated a point on a timeline. However, the
> purpose of the scheduler (which is what this series is preparation for) is to
> re-order requests between contexts. Thus the timeline must be per context to
> prevent requests ending up with out of order completions.
Virtual memory, not machine. Guessing i915_address_space :-)
Fence contexts may not have to be bound to a single timeline, lets say you have
2 engines, running commands for Com1 and Com2.
ENG1: Com1.1
ENG2: Com2.1 Com1.2 Com1.3
then there's no harm if you end up with multiple fence contexts since they're
independently executing.
>
>> Please complete the legacy/execlists unification first so that we can
>> have timelines work correctly for the whole driver.
> No. That is a much larger task that people are working towards. However, we
> urgently need a scheduler for specific customers to use right now. That means
> we need to get something working right now not in some random number of years
> time. If an intermediate step is less than perfect but still functional that
> is better than not having anything at all.
>
>> -Chris
>>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
