From: Oscar Mateo <oscar.mateo@xxxxxxxxx> On execbuffer, either...: A) there is not standalone context (and we error: user provided ctx id is invalid). B) the standalone context is the one we are looking for (and we return it). C) the standalone context is blank (and we populate and return it). D) one of the dependent contexts is the one we want (and we return it). E) none of the above (and we create and populate a new dependent context). Note that, historically, HW contexts other than the default one have only worked for the render ring (in Full PPGTT, contexts were used for other engines in a tricky way: the render ring would really receive MI_SET_CONTEXTs while the other engines would simply use their contexts to know which PPGTT to switch to). Now we are going to really save and restore contexts for other engines, but we still don't allow the context create ioctl to work them (since it changes the ABI). Signed-off-by: Oscar Mateo <oscar.mateo@xxxxxxxxx> --- drivers/gpu/drm/i915/i915_drv.h | 5 +++ drivers/gpu/drm/i915/i915_gem_context.c | 8 ++-- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 ++- drivers/gpu/drm/i915/i915_lrc.c | 59 ++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index d9470a4..a9f807b 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -605,6 +605,8 @@ struct i915_hw_context { /* Advanced contexts only */ struct list_head dependent_contexts; + bool is_blank; + enum intel_ring_id ring_id; }; struct i915_fbc { @@ -2322,6 +2324,9 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data, /* i915_lrc.c */ int gen8_gem_context_init(struct drm_device *dev); void gen8_gem_context_fini(struct drm_device *dev); +struct i915_hw_context *gen8_gem_validate_context(struct drm_device *dev, + struct drm_file *file, + struct intel_engine *ring, const u32 ctx_id); struct i915_hw_context *gen8_gem_create_context(struct drm_device *dev, struct intel_engine *ring, struct drm_i915_file_private *file_priv, diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 17015b2..a4e878e 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -550,8 +550,8 @@ int i915_gem_context_open(struct drm_device *dev, struct drm_file *file) mutex_lock(&dev->struct_mutex); if (dev_priv->lrc_enabled) file_priv->private_default_ctx = gen8_gem_create_context(dev, - &dev_priv->ring[RCS], file_priv, - NULL, USES_FULL_PPGTT(dev)); + NULL, file_priv, NULL, + USES_FULL_PPGTT(dev)); else file_priv->private_default_ctx = i915_gem_create_context(dev, file_priv, USES_FULL_PPGTT(dev)); @@ -812,8 +812,8 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data, return ret; if (dev_priv->lrc_enabled) - ctx = gen8_gem_create_context(dev, &dev_priv->ring[RCS], - file_priv, NULL, USES_FULL_PPGTT(dev)); + ctx = gen8_gem_create_context(dev, NULL, file_priv, NULL, + USES_FULL_PPGTT(dev)); else ctx = i915_gem_create_context(dev, file_priv, USES_FULL_PPGTT(dev)); diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index fa5a439..72bda74 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1131,7 +1131,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, goto pre_mutex_err; } - ctx = i915_gem_validate_context(dev, file, ring, ctx_id); + if (dev_priv->lrc_enabled) + ctx = gen8_gem_validate_context(dev, file, ring, ctx_id); + else + ctx = i915_gem_validate_context(dev, file, ring, ctx_id); if (IS_ERR(ctx)) { mutex_unlock(&dev->struct_mutex); ret = PTR_ERR(ctx); diff --git a/drivers/gpu/drm/i915/i915_lrc.c b/drivers/gpu/drm/i915/i915_lrc.c index 99011cc..3065bca 100644 --- a/drivers/gpu/drm/i915/i915_lrc.c +++ b/drivers/gpu/drm/i915/i915_lrc.c @@ -192,6 +192,9 @@ intel_populate_lrc(struct i915_hw_context *ctx, set_page_dirty(page); i915_gem_object_unpin_pages(ctx->obj); + ctx->ring_id = ring->id; + ctx->is_blank = false; + return 0; } @@ -309,6 +312,9 @@ gen8_gem_create_context(struct drm_device *dev, ret = intel_populate_lrc(ctx, ring); if (ret) goto unmap_ringbuf; + } else { + WARN(is_dependent, "Dependent but blank context!\n"); + ctx->is_blank = true; } return ctx; @@ -334,6 +340,59 @@ destroy_ring_obj: return ERR_PTR(ret); } +struct i915_hw_context * +gen8_gem_validate_context(struct drm_device *dev, struct drm_file *file, + struct intel_engine *ring, const u32 ctx_id) +{ + struct drm_i915_file_private *file_priv = file->driver_priv; + struct i915_hw_context *ctx = NULL; + struct i915_hw_context *cursor = NULL; + struct i915_ctx_hang_stats *hs; + bool found = false; + int ret; + + /* There is no reason why we cannot accept non-default, non-render contexts, + * other than it changes the ABI (these kind of custom contexts have not been + * allowed before) */ + if (ring->id != RCS && ctx_id != DEFAULT_CONTEXT_ID) + return ERR_PTR(-EINVAL); + + ctx = (struct i915_hw_context *)idr_find(&file_priv->context_idr, ctx_id); + if (!ctx) + return ERR_PTR(-ENOENT); + + if (ctx->is_blank) { + ret = intel_populate_lrc(ctx, ring); + if (ret) + return ERR_PTR(ret); + } + else if (ctx->ring_id != ring->id) { + list_for_each_entry(cursor, &ctx->dependent_contexts, dependent_contexts) { + if (cursor->ring_id == ring->id) { + found = true; + break; + } + } + + if (found) + ctx = cursor; + else { + ctx = gen8_gem_create_context(dev, ring, file_priv, + ctx, USES_FULL_PPGTT(dev)); + if (!ctx) + return ERR_PTR(-ENOENT); + } + } + + hs = &ctx->hang_stats; + if (hs->banned) { + DRM_DEBUG("Context %u tried to submit while banned\n", ctx_id); + return ERR_PTR(-EIO); + } + + return ctx; +} + void gen8_gem_context_fini(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; -- 1.9.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx