On Fri, Feb 13, 2015 at 11:48:10AM +0000, [email protected] wrote: > From: John Harrison <[email protected]> > > There is a flags word that is passed through the execbuffer code path all the > way from initial decoding of the user parameters down to the very final > dispatch > buffer call. It is simply called 'flags'. Unfortuantely, there are many other > flags words floating around in the same blocks of code. Even more once the GPU > scheduler arrives. > > This patch makes it more obvious exactly which flags word is which by renaming > 'flags' to 'dispatch_flags'. Note that the bit definitions for this flags word > already have an 'I915_DISPATCH_' prefix on them and so are not quite so > ambiguous. > > For: VIZ-1587
I've thought we've decided that the tag is OTC-Jira or similar. For: certainly looks a bit too generic, and a prefix helps in namespacing if you want to scan with automated tools for this. > Signed-off-by: John Harrison <[email protected]> Anyway looks like a good idea, queued for -next, thanks for the patch. -Daniel > --- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 25 ++++++++++---------- > drivers/gpu/drm/i915/intel_lrc.c | 10 ++++---- > drivers/gpu/drm/i915/intel_lrc.h | 2 +- > drivers/gpu/drm/i915/intel_ringbuffer.c | 35 > ++++++++++++++++------------ > drivers/gpu/drm/i915/intel_ringbuffer.h | 4 ++-- > 5 files changed, 41 insertions(+), 35 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index b773368..ec9ea45 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -1138,7 +1138,7 @@ i915_gem_ringbuffer_submission(struct drm_device *dev, > struct drm_file *file, > struct drm_i915_gem_execbuffer2 *args, > struct list_head *vmas, > struct drm_i915_gem_object *batch_obj, > - u64 exec_start, u32 flags) > + u64 exec_start, u32 dispatch_flags) > { > struct drm_clip_rect *cliprects = NULL; > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -1266,19 +1266,19 @@ i915_gem_ringbuffer_submission(struct drm_device > *dev, struct drm_file *file, > > ret = ring->dispatch_execbuffer(ring, > exec_start, exec_len, > - flags); > + dispatch_flags); > if (ret) > goto error; > } > } else { > ret = ring->dispatch_execbuffer(ring, > exec_start, exec_len, > - flags); > + dispatch_flags); > if (ret) > return ret; > } > > - trace_i915_gem_ring_dispatch(intel_ring_get_request(ring), flags); > + trace_i915_gem_ring_dispatch(intel_ring_get_request(ring), > dispatch_flags); > > i915_gem_execbuffer_move_to_active(vmas, ring); > i915_gem_execbuffer_retire_commands(dev, file, ring, batch_obj); > @@ -1353,7 +1353,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void > *data, > struct i915_address_space *vm; > const u32 ctx_id = i915_execbuffer2_get_context_id(*args); > u64 exec_start = args->batch_start_offset; > - u32 flags; > + u32 dispatch_flags; > int ret; > bool need_relocs; > > @@ -1364,15 +1364,15 @@ i915_gem_do_execbuffer(struct drm_device *dev, void > *data, > if (ret) > return ret; > > - flags = 0; > + dispatch_flags = 0; > if (args->flags & I915_EXEC_SECURE) { > if (!file->is_master || !capable(CAP_SYS_ADMIN)) > return -EPERM; > > - flags |= I915_DISPATCH_SECURE; > + dispatch_flags |= I915_DISPATCH_SECURE; > } > if (args->flags & I915_EXEC_IS_PINNED) > - flags |= I915_DISPATCH_PINNED; > + dispatch_flags |= I915_DISPATCH_PINNED; > > if ((args->flags & I915_EXEC_RING_MASK) > LAST_USER_RING) { > DRM_DEBUG("execbuf with unknown ring: %d\n", > @@ -1495,7 +1495,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void > *data, > args->batch_start_offset, > args->batch_len, > file->is_master, > - &flags); > + &dispatch_flags); > if (IS_ERR(batch_obj)) { > ret = PTR_ERR(batch_obj); > goto err; > @@ -1507,7 +1507,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void > *data, > /* snb/ivb/vlv conflate the "batch in ppgtt" bit with the "non-secure > * batch" bit. Hence we need to pin secure batches into the global gtt. > * hsw should have this fixed, but bdw mucks it up again. */ > - if (flags & I915_DISPATCH_SECURE) { > + if (dispatch_flags & I915_DISPATCH_SECURE) { > /* > * So on first glance it looks freaky that we pin the batch here > * outside of the reservation loop. But: > @@ -1527,7 +1527,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void > *data, > exec_start += i915_gem_obj_offset(batch_obj, vm); > > ret = dev_priv->gt.do_execbuf(dev, file, ring, ctx, args, > - &eb->vmas, batch_obj, exec_start, flags); > + &eb->vmas, batch_obj, exec_start, > + dispatch_flags); > > /* > * FIXME: We crucially rely upon the active tracking for the (ppgtt) > @@ -1535,7 +1536,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void > *data, > * needs to be adjusted to also track the ggtt batch vma properly as > * active. > */ > - if (flags & I915_DISPATCH_SECURE) > + if (dispatch_flags & I915_DISPATCH_SECURE) > i915_gem_object_ggtt_unpin(batch_obj); > err: > /* the request owns the ref now */ > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > b/drivers/gpu/drm/i915/intel_lrc.c > index a94346f..0376285 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -611,7 +611,7 @@ static int execlists_move_to_gpu(struct intel_ringbuffer > *ringbuf, > * @vmas: list of vmas. > * @batch_obj: the batchbuffer to submit. > * @exec_start: batchbuffer start virtual address pointer. > - * @flags: translated execbuffer call flags. > + * @dispatch_flags: translated execbuffer call flags. > * > * This is the evil twin version of i915_gem_ringbuffer_submission. It > abstracts > * away the submission details of the execbuffer ioctl call. > @@ -624,7 +624,7 @@ int intel_execlists_submission(struct drm_device *dev, > struct drm_file *file, > struct drm_i915_gem_execbuffer2 *args, > struct list_head *vmas, > struct drm_i915_gem_object *batch_obj, > - u64 exec_start, u32 flags) > + u64 exec_start, u32 dispatch_flags) > { > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf; > @@ -697,7 +697,7 @@ int intel_execlists_submission(struct drm_device *dev, > struct drm_file *file, > dev_priv->relative_constants_mode = instp_mode; > } > > - ret = ring->emit_bb_start(ringbuf, ctx, exec_start, flags); > + ret = ring->emit_bb_start(ringbuf, ctx, exec_start, dispatch_flags); > if (ret) > return ret; > > @@ -1142,9 +1142,9 @@ static int gen8_init_render_ring(struct intel_engine_cs > *ring) > > static int gen8_emit_bb_start(struct intel_ringbuffer *ringbuf, > struct intel_context *ctx, > - u64 offset, unsigned flags) > + u64 offset, unsigned dispatch_flags) > { > - bool ppgtt = !(flags & I915_DISPATCH_SECURE); > + bool ppgtt = !(dispatch_flags & I915_DISPATCH_SECURE); > int ret; > > ret = intel_logical_ring_begin(ringbuf, ctx, 4); > diff --git a/drivers/gpu/drm/i915/intel_lrc.h > b/drivers/gpu/drm/i915/intel_lrc.h > index 6f2d7da..3093836 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.h > +++ b/drivers/gpu/drm/i915/intel_lrc.h > @@ -86,7 +86,7 @@ int intel_execlists_submission(struct drm_device *dev, > struct drm_file *file, > struct drm_i915_gem_execbuffer2 *args, > struct list_head *vmas, > struct drm_i915_gem_object *batch_obj, > - u64 exec_start, u32 flags); > + u64 exec_start, u32 dispatch_flags); > u32 intel_execlists_ctx_id(struct drm_i915_gem_object *ctx_obj); > > void intel_lrc_irq_handler(struct intel_engine_cs *ring); > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 0bd3976..d611608 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -1611,7 +1611,7 @@ gen8_ring_put_irq(struct intel_engine_cs *ring) > static int > i965_dispatch_execbuffer(struct intel_engine_cs *ring, > u64 offset, u32 length, > - unsigned flags) > + unsigned dispatch_flags) > { > int ret; > > @@ -1622,7 +1622,8 @@ i965_dispatch_execbuffer(struct intel_engine_cs *ring, > intel_ring_emit(ring, > MI_BATCH_BUFFER_START | > MI_BATCH_GTT | > - (flags & I915_DISPATCH_SECURE ? 0 : > MI_BATCH_NON_SECURE_I965)); > + (dispatch_flags & I915_DISPATCH_SECURE ? > + 0 : MI_BATCH_NON_SECURE_I965)); > intel_ring_emit(ring, offset); > intel_ring_advance(ring); > > @@ -1635,8 +1636,8 @@ i965_dispatch_execbuffer(struct intel_engine_cs *ring, > #define I830_WA_SIZE max(I830_TLB_ENTRIES*4096, I830_BATCH_LIMIT) > static int > i830_dispatch_execbuffer(struct intel_engine_cs *ring, > - u64 offset, u32 len, > - unsigned flags) > + u64 offset, u32 len, > + unsigned dispatch_flags) > { > u32 cs_offset = ring->scratch.gtt_offset; > int ret; > @@ -1654,7 +1655,7 @@ i830_dispatch_execbuffer(struct intel_engine_cs *ring, > intel_ring_emit(ring, MI_NOOP); > intel_ring_advance(ring); > > - if ((flags & I915_DISPATCH_PINNED) == 0) { > + if ((dispatch_flags & I915_DISPATCH_PINNED) == 0) { > if (len > I830_BATCH_LIMIT) > return -ENOSPC; > > @@ -1686,7 +1687,8 @@ i830_dispatch_execbuffer(struct intel_engine_cs *ring, > return ret; > > intel_ring_emit(ring, MI_BATCH_BUFFER); > - intel_ring_emit(ring, offset | (flags & I915_DISPATCH_SECURE ? 0 : > MI_BATCH_NON_SECURE)); > + intel_ring_emit(ring, offset | (dispatch_flags & I915_DISPATCH_SECURE ? > + 0 : MI_BATCH_NON_SECURE)); > intel_ring_emit(ring, offset + len - 8); > intel_ring_emit(ring, MI_NOOP); > intel_ring_advance(ring); > @@ -1697,7 +1699,7 @@ i830_dispatch_execbuffer(struct intel_engine_cs *ring, > static int > i915_dispatch_execbuffer(struct intel_engine_cs *ring, > u64 offset, u32 len, > - unsigned flags) > + unsigned dispatch_flags) > { > int ret; > > @@ -1706,7 +1708,8 @@ i915_dispatch_execbuffer(struct intel_engine_cs *ring, > return ret; > > intel_ring_emit(ring, MI_BATCH_BUFFER_START | MI_BATCH_GTT); > - intel_ring_emit(ring, offset | (flags & I915_DISPATCH_SECURE ? 0 : > MI_BATCH_NON_SECURE)); > + intel_ring_emit(ring, offset | (dispatch_flags & I915_DISPATCH_SECURE ? > + 0 : MI_BATCH_NON_SECURE)); > intel_ring_advance(ring); > > return 0; > @@ -2265,9 +2268,10 @@ static int gen6_bsd_ring_flush(struct intel_engine_cs > *ring, > static int > gen8_ring_dispatch_execbuffer(struct intel_engine_cs *ring, > u64 offset, u32 len, > - unsigned flags) > + unsigned dispatch_flags) > { > - bool ppgtt = USES_PPGTT(ring->dev) && !(flags & I915_DISPATCH_SECURE); > + bool ppgtt = USES_PPGTT(ring->dev) && > + !(dispatch_flags & I915_DISPATCH_SECURE); > int ret; > > ret = intel_ring_begin(ring, 4); > @@ -2286,8 +2290,8 @@ gen8_ring_dispatch_execbuffer(struct intel_engine_cs > *ring, > > static int > hsw_ring_dispatch_execbuffer(struct intel_engine_cs *ring, > - u64 offset, u32 len, > - unsigned flags) > + u64 offset, u32 len, > + unsigned dispatch_flags) > { > int ret; > > @@ -2297,7 +2301,7 @@ hsw_ring_dispatch_execbuffer(struct intel_engine_cs > *ring, > > intel_ring_emit(ring, > MI_BATCH_BUFFER_START | > - (flags & I915_DISPATCH_SECURE ? > + (dispatch_flags & I915_DISPATCH_SECURE ? > 0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW)); > /* bit0-7 is the length on GEN6+ */ > intel_ring_emit(ring, offset); > @@ -2309,7 +2313,7 @@ hsw_ring_dispatch_execbuffer(struct intel_engine_cs > *ring, > static int > gen6_ring_dispatch_execbuffer(struct intel_engine_cs *ring, > u64 offset, u32 len, > - unsigned flags) > + unsigned dispatch_flags) > { > int ret; > > @@ -2319,7 +2323,8 @@ gen6_ring_dispatch_execbuffer(struct intel_engine_cs > *ring, > > intel_ring_emit(ring, > MI_BATCH_BUFFER_START | > - (flags & I915_DISPATCH_SECURE ? 0 : > MI_BATCH_NON_SECURE_I965)); > + (dispatch_flags & I915_DISPATCH_SECURE ? > + 0 : MI_BATCH_NON_SECURE_I965)); > /* bit0-7 is the length on GEN6+ */ > intel_ring_emit(ring, offset); > intel_ring_advance(ring); > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h > b/drivers/gpu/drm/i915/intel_ringbuffer.h > index 714f3fd..26e5774 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -164,7 +164,7 @@ struct intel_engine_cs { > u32 seqno); > int (*dispatch_execbuffer)(struct intel_engine_cs *ring, > u64 offset, u32 length, > - unsigned flags); > + unsigned dispatch_flags); > #define I915_DISPATCH_SECURE 0x1 > #define I915_DISPATCH_PINNED 0x2 > void (*cleanup)(struct intel_engine_cs *ring); > @@ -242,7 +242,7 @@ struct intel_engine_cs { > u32 flush_domains); > int (*emit_bb_start)(struct intel_ringbuffer *ringbuf, > struct intel_context *ctx, > - u64 offset, unsigned flags); > + u64 offset, unsigned dispatch_flags); > > /** > * List of objects currently involved in rendering from the > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > [email protected] > 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 [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
