On Fri, Jun 13, 2014 at 04:37:24PM +0100, oscar.mateo@xxxxxxxxx wrote:
> From: Oscar Mateo <oscar.mateo@xxxxxxxxx>
>
> A context backing object only makes sense for a given engine (because
> it holds state data specific to that engine).
>
> In legacy ringbuffer sumission mode, the only MI_SET_CONTEXT we really
> perform is for the render engine, so one backing object is all we needed.
>
> With Execlists, however, we need backing objects for every engine, as
> contexts become the only way to submit workloads to the GPU. To tackle
> this problem, we multiplex the context struct to contain <no-of-engines>
> objects.
>
> Originally, I colored this code by instantiating one new context for
> every engine I wanted to use, but this change suggested by Brad Volkin
> makes it more elegant.
>
> v2: Leave the old backing object pointer behind. Daniel Vetter suggested
> using a union, but it makes more sense to keep render_obj as a NULL
> pointer behind, to make sure no one uses it incorrectly when Execlists
> are enabled, similar to what we are doing with ring->buffer (Rusty's API
> level 5).
>
> Signed-off-by: Oscar Mateo <oscar.mateo@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a15370c..ccc1ba6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -593,7 +593,14 @@ struct intel_context {
> uint8_t remap_slice;
> struct drm_i915_file_private *file_priv;
> struct intel_engine_cs *last_ring;
> +
> + /* Legacy ring buffer submission */
> struct drm_i915_gem_object *render_obj;
Per my previous request, is_initialized should also be nearby, maybe
wrapped in a struct. So
union {
struct {
struct gem_bo *obj;
bool is_iniatlized;
} render_ctx;
struct {
...
} lrc[I915_NUM_RINGS];
}
Or some other means to make it clearer which fields are for legacy render
ctx objects and which for lrc contexts. I also wonder whether we should
shovel all the hw specific stuff at the end to have a clearer separation
between the sw-side field members associated with the software context
object and the stuff for the hw thing.
Just ideas to pick&choose really, we can cocci-polish this once it's all
settled easily (i.e. afterwards).
-Daniel
> + /* Execlists */
> + struct {
> + struct drm_i915_gem_object *obj;
> + } engine[I915_NUM_RINGS];
> +
> struct i915_ctx_hang_stats hang_stats;
> struct i915_address_space *vm;
>
> --
> 1.9.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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
