Re: [PATCH 10/43] drm/i915/bdw: Deferred creation of user-created LRCs

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Jul 24, 2014 at 05:04:18PM +0100, Thomas Daniel wrote:
> From: Oscar Mateo <oscar.mateo@intel.com>
> 
> The backing objects and ringbuffers for contexts created via open
> fd are actually empty until the user starts sending execbuffers to
> them. At that point, we allocate & populate them. We do this because,
> at create time, we really don't know which engine is going to be used
> with the context later on (and we don't want to waste memory on
> objects that we might never use).
> 
> v2: As contexts created via ioctl can only be used with the render
> ring, we have enough information to allocate & populate them right
> away.
> 
> v3: Defer the creation always, even with ioctl-created contexts, as
> requested by Daniel Vetter.
> 
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>

This patch seems to miss the hunk to remove

		/* FIXME: we really only want to do this for initialized rings */
		if (i915.enable_execlists)
			intel_lr_context_deferred_create(ctx, ring);

Also I just realized that that code completely lacks error handling.

If it's not already there please quickly submit a fixup patch.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem_context.c    |    7 +++----
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |    8 ++++++++
>  2 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 48d7476..fbe7278 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -784,9 +784,9 @@ int i915_switch_context(struct intel_engine_cs *ring,
>  	return do_switch(ring, to);
>  }
>  
> -static bool hw_context_enabled(struct drm_device *dev)
> +static bool contexts_enabled(struct drm_device *dev)
>  {
> -	return to_i915(dev)->hw_context_size;
> +	return i915.enable_execlists || to_i915(dev)->hw_context_size;
>  }
>  
>  int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
> @@ -797,8 +797,7 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
>  	struct intel_context *ctx;
>  	int ret;
>  
> -	/* FIXME: allow user-created LR contexts as well */
> -	if (!hw_context_enabled(dev))
> +	if (!contexts_enabled(dev))
>  		return -ENODEV;
>  
>  	ret = i915_mutex_lock_interruptible(dev);
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index c5115957..4e9b387 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -951,6 +951,14 @@ i915_gem_validate_context(struct drm_device *dev, struct drm_file *file,
>  		return ERR_PTR(-EIO);
>  	}
>  
> +	if (i915.enable_execlists && !ctx->engine[ring->id].state) {
> +		int ret = intel_lr_context_deferred_create(ctx, ring);
> +		if (ret) {
> +			DRM_DEBUG("Could not create LRC %u: %d\n", ctx_id, ret);
> +			return ERR_PTR(ret);
> +		}
> +	}
> +
>  	return ctx;
>  }
>  
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch