On Mon, May 19, 2014 at 03:26:09PM +0000, Mateo Lozano, Oscar wrote: > > I think special-casing the i915_gem_context_get function for the default > > context and using private_default_ctx a bit more sounds good. We need to > > adjust the idr allocator a bit though to reserve 0, and a bit of frobbing in the > > context create code. > > Ok, no problem. I´ll send an early patch that uses private_default_ctx > to do the special casing (no functionality change) together with the > other refactoring patches. > > > Wrt ctx abstraction I think separate functions for execlist/legacy contexts > > should be good enough. The lookup/create/destroy logic should carry over. > > Including the creation of I915_NUM_RINGS contexts per-filp? do you want > that to happen for the legacy case as well? This implies a number of > other changes, like struct intel_engine *last_ring not making sense > anymore and frobbing i915_gem_create_context so that we can create more > than one context with the same ppgtt... Damn, I didn't think this through. ctx->hang_stats already tracks random stuff for logical contexts and we only have one of those for the implicit per-filp default context. Still, allocating backing storage for the big execlist contexts looks really wasteful since a lot of userspace will just use 1 ring (e.g. mesa or opencl). Also we've decided to at least for now support vcs2 implicitly, so I915_NUM_RINGS is kinda already the wrong thing to use (presuming we can indeed switch hw contexts between vcs and vcs2). Otoh adding fake contexts for the legacy case also feels ugly. So maybe we should rename i915_hw_context to i915_context and figure out a sane design for the differences later on. Same option would be to embedded struct i915_context into a i915_legacy_rcs_context and i915_execlist_context structures, but meh. So now I think we should: a) s/i915_hw_context/i915_context/ since its long past a 1:1 relationship with hw b) create a union in i915_context for legacy and execlists contexts. Legacy contexts would just have the single gem bo backing storage needed for rcs, execlists contexts would have an array of rings plus the backing storage (since the ring is just embedded in there). How does this sound? If we decide later on that the union is too ugly and we want a cleaner split, we can do the usual subclassing later on. -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
