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

John Harrison
2016-08-23T13:28:16Z

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

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>

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

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