On Mon, May 19, 2014 at 3:41 PM, Mateo Lozano, Oscar <oscar.mateo@xxxxxxxxx> 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx