Re: [PATCH 03/17] drm/i915: Allow the user to pass a context to any ring

John Harrison
2016-08-23T13:33:23Z

[-- Attachment #1.1: Type: text/plain, Size: 2744 bytes --]

On 23/08/2016 14:28, John Harrison wrote:
> On 22/08/2016 13:23, Chris Wilson wrote:
>> On Mon, Aug 22, 2016 at 02:23:28PM +0300, Joonas Lahtinen wrote:
>>> On ma, 2016-08-22 at 09:03 +0100, Chris Wilson wrote:
>>>> With full-ppgtt, we want the user to have full control over their memory
>>>> layout, with a separate instance per context. Forcing them to use a
>>>> shared memory layout for !RCS not only duplicates the amount of work we
>>>> have to do, but also defeats the memory segregation on offer.
>>>>
>>>> Signed-off-by: Chris Wilson<chris@chris-wilson.co.uk>
>>>> ---
>>>> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 +----
>>>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>>>> index 8f9d5ad0cfd8..fb1a64738fb8 100644
>>>> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>>>> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>>>> @@ -1250,12 +1250,9 @@ static struct i915_gem_context *
>>>> i915_gem_validate_context(struct drm_device *dev, struct drm_file *file,
>>>> struct intel_engine_cs *engine, const u32 ctx_id)
>>>> {
>>>> - struct i915_gem_context *ctx = NULL;
>>>> + struct i915_gem_context *ctx;
>>>> struct i915_ctx_hang_stats *hs;
>>>>
>>>> - if (engine->id != RCS && ctx_id != DEFAULT_CONTEXT_HANDLE)
>>>> - return ERR_PTR(-EINVAL);
>>>> -
>>> One would think this existed due to lack of testing or bugs in early
>>> hardware. Do we need to use IS_GEN or some other means of validation?
>> No.
>>
>> This has nothing to do with the hardware logical state (that is found
>> within intel_context and only enabled where appropriate). The
>> i915_gem_context is the driver's segregation between clients. Not only
>> is t required for tracking clients independently (currently hangstats,
>> but the context would be the first place we start enforcing cgroups like
>> controls), but it is vital for clients who want to control their memory
>> layout without conflicts (with themselves and others).
>> -Chris
>>
> It is also important for clients that want to submit lots of work in
> parallel from a single application by using multiple contexts. Other
> internal teams have been running with this patch for quite some time.
> I believe the only reason it has not been merged upstream before (it
> has been on the mailing list at least twice before that I know of) was
> the argument of no open source user.
>
> Reviewed-by: John Harrison <john.c.harrison@intel.com>
>

Actually, just found a previous instance. It had an r-b from Daniel
Thomas but Tvrtko vetoed it on the grounds of needing IGT coverage first
- message id '<565EF558.5050705@linux.intel.com>' on Dec 2nd 2015.

[-- Attachment #1.2: Type: text/html, Size: 3741 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]