[Intel-gfx] [PATCH 06/50] drm/i915: s/intel_ring_buffer/intel_engine

Daniel Vetter daniel at ffwll.ch
Mon May 19 15:52:30 CEST 2014


On Mon, May 19, 2014 at 3:41 PM, Mateo Lozano, Oscar
<oscar.mateo at intel.com> wrote:
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h index 108e1ec2fa4b..e34db43dead3
>> 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1825,7 +1825,9 @@ struct drm_i915_file_private {
>>       } mm;
>>       struct idr context_idr;
>>
>> -     struct i915_hw_context *private_default_ctx;
>> +     /* default context for each ring, NULL if hw doesn't support hw
>> contexts
>> +      * (or fancy new lrcs) on that ring. */
>> +     struct i915_hw_context *private_default_ctx[I915_NUM_RINGS];
>>       atomic_t rps_wait_boost;
>>  };
>>
>> Of course we need to add an i915_hw_context->engine_cs pointer and we need
>> to check that at execbuf to make sure we don't run contexts on the wrong
>> engine.
>> If we later on decide that we want to expose multiple hw contexts for !RCS to
>> userspace we can easily add a bunch of ring flags to the context create ioctl. So
>> this doesn't restrict us at all in the features we can support without jumping
>> through hoops.
>>
>> Also if we'd shovel all per-ring lrcs into the same i915_hw_context structure
>> then we'd need to rename that and drop the _hw part - it's no longer a 1:1
>> correspondence to an actual hw ring/context/lrc/whatever wizzbang thingy.
>
> Ok, so we create I915_NUM_RINGS contexts for the global default contexts, plus I915_NUM_RINGS contexts for every filp and 1 render context for every create ioctl.
> But the magic stuff is going to pop out in many more places: I cannot idr_alloc/idr_find for the per-filp contexts, because all of them cannot have  ctx->id = DEFAULT_CONTEXT_ID at the same time (I´ll have to special-case them by using dev_priv->private_default_ctx[RING] to find them). Of course, if you prefer, I can abstract away most of the functionality in i915_gem_context.c and make sure this kind magic is only done for the LRC path (similar to what you propose to do with intel_ringbuffer.c).

Argh, forgotten about the pageflips again. But for those we already
need some other context pointer, and thus far we've only supported
ring-switching on one ring (well, almost everywhere at least). Since
the mmio base pageflip patch seems mostly ready I think we could just
merge that one first and then forget about ring-based pageflips for
execlists. Way too much pain to be worth it really ;-)

For the default context special-casing I've somehow though we
special-case that in the lookup code. But the code in there is a bit
convoluted, so a bit of tidying up (and shoveling more of the checking
and lookup logic into i915_gem_context.c) can't hurt really. Also we
seem to lack error checking for the creation of the default context.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch



More information about the Intel-gfx mailing list