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

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Jun 13, 2014 at 04:37:31PM +0100, oscar.mateo@intel.com wrote:
> From: Oscar Mateo <oscar.mateo@intel.com>
> 
> The backing objects for contexts created via open fd are actually
> empty until the user starts sending execbuffers to 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.
> 
> v2: As context created via ioctl can only be used with the render ring,
> we have enough information to allocate & populate them right away.

Not sure this is a good choice to special-case the ioctl. We might want
to allow contexts also on non-render rings, and it introduces a special
case for not much gain. Since we must have the deferred alloca anyway lets
use that for all cases. And if there's a (performance) issue with it we
need to address that. One true (code) path to rule them all.
-Daniel

> 
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 1fb4592..70bf6d0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -795,6 +795,7 @@ static bool contexts_enabled(struct drm_device *dev)
>  int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
>  				  struct drm_file *file)
>  {
> +	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_i915_gem_context_create *args = data;
>  	struct drm_i915_file_private *file_priv = file->driver_priv;
>  	struct intel_context *ctx;
> @@ -808,9 +809,23 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
>  		return ret;
>  
>  	ctx = i915_gem_create_context(dev, file_priv, USES_FULL_PPGTT(dev));
> -	mutex_unlock(&dev->struct_mutex);
> -	if (IS_ERR(ctx))
> +	if (IS_ERR(ctx)) {
> +		mutex_unlock(&dev->struct_mutex);
>  		return PTR_ERR(ctx);
> +	}
> +
> +	if (dev_priv->lrc_enabled) {
> +		/* NB: We know this context will only be used with the render ring
> +		 * (as we enforce it) so we can allocate & populate it already */
> +		int ret = intel_lr_context_deferred_create(ctx, &dev_priv->ring[RCS]);
> +		if (ret) {
> +			mutex_unlock(&dev->struct_mutex);
> +			DRM_DEBUG_DRIVER("Could not create LRC: %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	mutex_unlock(&dev->struct_mutex);
>  
>  	args->ctx_id = ctx->id;
>  	DRM_DEBUG_DRIVER("HW context %d created\n", args->ctx_id);
> @@ -851,6 +866,7 @@ struct intel_context *
>  i915_gem_context_validate(struct drm_device *dev, struct drm_file *file,
>  			  struct intel_engine_cs *ring, const u32 ctx_id)
>  {
> +	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_context *ctx = NULL;
>  	struct i915_ctx_hang_stats *hs;
>  
> @@ -867,5 +883,13 @@ i915_gem_context_validate(struct drm_device *dev, struct drm_file *file,
>  		return ERR_PTR(-EIO);
>  	}
>  
> +	if (dev_priv->lrc_enabled && !ctx->engine[ring->id].obj) {
> +		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.9.0
> 
> _______________________________________________
> 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