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

Daniel Vetter
2014-08-11T14:25:24Z

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