On Fri, Jun 13, 2014 at 04:37:54PM +0100, oscar.mateo@xxxxxxxxx wrote:
> From: Oscar Mateo <oscar.mateo@xxxxxxxxx>
>
> As suggested by Daniel.
>
> Signed-off-by: Oscar Mateo <oscar.mateo@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 26 ++++++++++++++++
> drivers/gpu/drm/i915/i915_gem.c | 48 +++++++++++++++++++-----------
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 25 +++++++---------
> 3 files changed, 67 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 3e9983c..89b6d5c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1554,6 +1554,23 @@ struct drm_i915_private {
> /* Old ums support infrastructure, same warning applies. */
> struct i915_ums_state ums;
>
> + /* Abstract the submission mechanism (legacy ringbuffer or execlists) away */
> + struct {
> + int (*do_execbuf) (struct drm_device *dev, struct drm_file *file,
> + struct intel_engine_cs *ring,
> + struct intel_context *ctx,
> + struct drm_i915_gem_execbuffer2 *args,
> + struct list_head *vmas,
> + struct drm_i915_gem_object *batch_obj,
> + u64 exec_start, u32 flags);
> + int (*add_request) (struct intel_engine_cs *ring,
> + struct drm_file *file,
> + struct drm_i915_gem_object *obj,
> + u32 *out_seqno);
Hm, what do we need add_request for? With the clean split in command
submission I'd expect every function to know whether it'll submit to an
lrc (everything in intel_lrc.c) or whether it'll submit to a legacy ring
(existing code), so I don't see a need for an add_request vfunc.
Au contraire that looks a bit dangerous since code might get run with
execlist that assumes we have a legcy ring ...
A bit confused, but let's hope this clears up in further patches.
-Daniel
> + int (*init_rings) (struct drm_device *dev);
> + void (*cleanup_ring) (struct intel_engine_cs *ring);
> + } gt;
> +
> /*
> * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
> * will be rejected. Instead look for a better place.
> @@ -2131,6 +2148,14 @@ void i915_gem_execbuffer_retire_commands(struct drm_device *dev,
> struct drm_file *file,
> struct intel_engine_cs *ring,
> struct drm_i915_gem_object *obj);
> +int i915_gem_ringbuffer_submission(struct drm_device *dev,
> + struct drm_file *file,
> + struct intel_engine_cs *ring,
> + struct intel_context *ctx,
> + struct drm_i915_gem_execbuffer2 *args,
> + struct list_head *vmas,
> + struct drm_i915_gem_object *batch_obj,
> + u64 exec_start, u32 flags);
> int i915_gem_execbuffer(struct drm_device *dev, void *data,
> struct drm_file *file_priv);
> int i915_gem_execbuffer2(struct drm_device *dev, void *data,
> @@ -2281,6 +2306,7 @@ void i915_gem_reset(struct drm_device *dev);
> bool i915_gem_clflush_object(struct drm_i915_gem_object *obj, bool force);
> int __must_check i915_gem_object_finish_gpu(struct drm_i915_gem_object *obj);
> int __must_check i915_gem_init(struct drm_device *dev);
> +int i915_gem_init_rings(struct drm_device *dev);
> int __must_check i915_gem_init_hw(struct drm_device *dev);
> int i915_gem_l3_remap(struct intel_engine_cs *ring, int slice);
> void i915_gem_init_swizzling(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 69db71a..7c10540 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2310,19 +2310,16 @@ i915_gem_get_seqno(struct drm_device *dev, u32 *seqno)
> return 0;
> }
>
> -int __i915_add_request(struct intel_engine_cs *ring,
> - struct drm_file *file,
> - struct drm_i915_gem_object *obj,
> - u32 *out_seqno)
> +static int i915_gem_add_request(struct intel_engine_cs *ring,
> + struct drm_file *file,
> + struct drm_i915_gem_object *obj,
> + u32 *out_seqno)
> {
> struct drm_i915_private *dev_priv = ring->dev->dev_private;
> struct drm_i915_gem_request *request;
> u32 request_ring_position, request_start;
> int ret;
>
> - if (intel_enable_execlists(ring->dev))
> - return intel_logical_ring_add_request(ring, file, obj, out_seqno);
> -
> request_start = intel_ring_get_tail(ring->buffer);
> /*
> * Emit any outstanding flushes - execbuf can fail to emit the flush
> @@ -2403,6 +2400,16 @@ int __i915_add_request(struct intel_engine_cs *ring,
> return 0;
> }
>
> +int __i915_add_request(struct intel_engine_cs *ring,
> + struct drm_file *file,
> + struct drm_i915_gem_object *obj,
> + u32 *out_seqno)
> +{
> + struct drm_i915_private *dev_priv = ring->dev->dev_private;
> +
> + return dev_priv->gt.add_request(ring, file, obj, out_seqno);
> +}
> +
> static inline void
> i915_gem_request_remove_from_client(struct drm_i915_gem_request *request)
> {
> @@ -4627,7 +4634,7 @@ intel_enable_blt(struct drm_device *dev)
> return true;
> }
>
> -static int i915_gem_init_rings(struct drm_device *dev)
> +int i915_gem_init_rings(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> int ret;
> @@ -4710,10 +4717,7 @@ i915_gem_init_hw(struct drm_device *dev)
>
> i915_gem_init_swizzling(dev);
>
> - if (intel_enable_execlists(dev))
> - ret = intel_logical_rings_init(dev);
> - else
> - ret = i915_gem_init_rings(dev);
> + ret = dev_priv->gt.init_rings(dev);
> if (ret)
> return ret;
>
> @@ -4751,6 +4755,18 @@ int i915_gem_init(struct drm_device *dev)
> DRM_DEBUG_DRIVER("allow wake ack timed out\n");
> }
>
> + if (intel_enable_execlists(dev)) {
> + dev_priv->gt.do_execbuf = intel_execlists_submission;
> + dev_priv->gt.add_request = intel_logical_ring_add_request;
> + dev_priv->gt.init_rings = intel_logical_rings_init;
> + dev_priv->gt.cleanup_ring = intel_logical_ring_cleanup;
> + } else {
> + dev_priv->gt.do_execbuf = i915_gem_ringbuffer_submission;
> + dev_priv->gt.add_request = i915_gem_add_request;
> + dev_priv->gt.init_rings = i915_gem_init_rings;
> + dev_priv->gt.cleanup_ring = intel_cleanup_ring_buffer;
> + }
> +
> i915_gem_init_userptr(dev);
> i915_gem_init_global_gtt(dev);
>
> @@ -4785,12 +4801,8 @@ i915_gem_cleanup_ringbuffer(struct drm_device *dev)
> struct intel_engine_cs *ring;
> int i;
>
> - for_each_ring(ring, dev_priv, i) {
> - if (intel_enable_execlists(dev))
> - intel_logical_ring_cleanup(ring);
> - else
> - intel_cleanup_ring_buffer(ring);
> - }
> + for_each_ring(ring, dev_priv, i)
> + dev_priv->gt.cleanup_ring(ring);
> }
>
> int
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 36c7f0c..f0dd31f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1005,14 +1005,15 @@ i915_reset_gen7_sol_offsets(struct drm_device *dev,
> return 0;
> }
>
> -static int
> -legacy_ringbuffer_submission(struct drm_device *dev, struct drm_file *file,
> - struct intel_engine_cs *ring,
> - struct intel_context *ctx,
> - struct drm_i915_gem_execbuffer2 *args,
> - struct list_head *vmas,
> - struct drm_i915_gem_object *batch_obj,
> - u64 exec_start, u32 flags)
> +int
> +i915_gem_ringbuffer_submission(struct drm_device *dev,
> + struct drm_file *file,
> + struct intel_engine_cs *ring,
> + struct intel_context *ctx,
> + struct drm_i915_gem_execbuffer2 *args,
> + struct list_head *vmas,
> + struct drm_i915_gem_object *batch_obj,
> + u64 exec_start, u32 flags)
> {
> struct drm_clip_rect *cliprects = NULL;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -1379,12 +1380,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
> else
> exec_start += i915_gem_obj_offset(batch_obj, vm);
>
> - if (intel_enable_execlists(dev))
> - ret = intel_execlists_submission(dev, file, ring, ctx,
> - args, &eb->vmas, batch_obj, exec_start, flags);
> - else
> - ret = legacy_ringbuffer_submission(dev, file, ring, ctx,
> - args, &eb->vmas, batch_obj, exec_start, flags);
> + ret = dev_priv->gt.do_execbuf(dev, file, ring, ctx, args,
> + &eb->vmas, batch_obj, exec_start, flags);
> if (ret)
> goto err;
>
> --
> 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
