[Intel-gfx] [PATCH v4 7/7] drm/i915: Tidy up execbuffer command parsing code

[Chris Wilson]

On Fri, Nov 07, 2014 at 02:22:07PM -0800, bradley.d.volkin at intel.com wrote:
> From: Brad Volkin <bradley.d.volkin at intel.com>
> 
> Move it to a separate function since the main do_execbuffer function
> already has so much going on.
> 
> Signed-off-by: Brad Volkin <bradley.d.volkin at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 136 +++++++++++++++++------------
>  1 file changed, 79 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index a271bc0..58f0a6c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1026,6 +1026,75 @@ i915_reset_gen7_sol_offsets(struct drm_device *dev,
>  	return 0;
>  }
>  
> +static struct drm_i915_gem_object*
> +i915_gem_execbuffer_parse(struct intel_engine_cs *ring,
> +			  struct drm_i915_gem_exec_object2 *shadow_exec_entry,
> +			  struct eb_vmas *eb,
> +			  struct drm_i915_gem_object *batch_obj,
> +			  u32 batch_start_offset,
> +			  u32 batch_len,
> +			  bool is_master,
> +			  u32 *flags)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(batch_obj->base.dev);
> +	struct drm_i915_gem_object *shadow_batch_obj;
> +	int ret;
> +
> +	shadow_batch_obj = i915_gem_batch_pool_get(&dev_priv->mm.batch_pool,
> +						   batch_obj->base.size);
> +	if (IS_ERR(shadow_batch_obj))
> +		return shadow_batch_obj;
> +
> +	shadow_batch_obj->madv = I915_MADV_WILLNEED;
> +
> +	ret = i915_gem_obj_ggtt_pin(shadow_batch_obj, 4096, 0);
> +	if (ret)
> +		goto err;

Pardon? This feels an implementation issue of i915_parse_cmds() and should
be resolved there. Presumably you are not actually reading back through
the GTT? That would be insane...

> +	ret = i915_parse_cmds(ring,
> +			      batch_obj,
> +			      shadow_batch_obj,
> +			      batch_start_offset,
> +			      batch_len,
> +			      is_master);
> +	i915_gem_object_ggtt_unpin(shadow_batch_obj);

Yet pin+unpin around the parser seems to serve no other purpose.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre