On Thu, Jul 24, 2014 at 05:04:12PM +0100, Thomas Daniel wrote:
> From: Oscar Mateo <oscar.ma...@intel.com>
>
> For the moment this is just a placeholder, but it shows one of the
> main differences between the good ol' HW contexts and the shiny
> new Logical Ring Contexts: LR contexts allocate and free their
> own backing objects. Another difference is that the allocation is
> deferred (as the create function name suggests), but that does not
> happen in this patch yet, because for the moment we are only dealing
> with the default context.
>
> Early in the series we had our own gen8_gem_context_init/fini
> functions, but the truth is they now look almost the same as the
> legacy hw context init/fini functions. We can always split them
> later if this ceases to be the case.
>
> Also, we do not fall back to legacy ringbuffers when logical ring
> context initialization fails (not very likely to happen and, even
> if it does, hw contexts would probably fail as well).
>
> v2: Daniel says "explain, do not showcase".
>
> Signed-off-by: Oscar Mateo <oscar.ma...@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_context.c | 29 +++++++++++++++++++++++------
> drivers/gpu/drm/i915/intel_lrc.c | 15 +++++++++++++++
> drivers/gpu/drm/i915/intel_lrc.h | 5 +++++
> 3 files changed, 43 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
> b/drivers/gpu/drm/i915/i915_gem_context.c
> index de72a28..718150e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -182,7 +182,10 @@ void i915_gem_context_free(struct kref *ctx_ref)
> typeof(*ctx), ref);
> struct i915_hw_ppgtt *ppgtt = NULL;
>
> - if (ctx->legacy_hw_ctx.rcs_state) {
> + if (i915.enable_execlists) {
> + ppgtt = ctx_to_ppgtt(ctx);
> + intel_lr_context_free(ctx);
> + } else if (ctx->legacy_hw_ctx.rcs_state) {
> /* We refcount even the aliasing PPGTT to keep the code
> symmetric */
> if (USES_PPGTT(ctx->legacy_hw_ctx.rcs_state->base.dev))
> ppgtt = ctx_to_ppgtt(ctx);
> @@ -419,7 +422,11 @@ int i915_gem_context_init(struct drm_device *dev)
> if (WARN_ON(dev_priv->ring[RCS].default_context))
> return 0;
>
> - if (HAS_HW_CONTEXTS(dev)) {
> + if (i915.enable_execlists) {
> + /* NB: intentionally left blank. We will allocate our own
> + * backing objects as we need them, thank you very much */
> + dev_priv->hw_context_size = 0;
> + } else if (HAS_HW_CONTEXTS(dev)) {
> dev_priv->hw_context_size = round_up(get_context_size(dev),
> 4096);
> if (dev_priv->hw_context_size > (1<<20)) {
> DRM_DEBUG_DRIVER("Disabling HW Contexts; invalid size
> %d\n",
> @@ -435,11 +442,20 @@ int i915_gem_context_init(struct drm_device *dev)
> return PTR_ERR(ctx);
> }
>
> - /* NB: RCS will hold a ref for all rings */
> - for (i = 0; i < I915_NUM_RINGS; i++)
> - dev_priv->ring[i].default_context = ctx;
> + for (i = 0; i < I915_NUM_RINGS; i++) {
> + struct intel_engine_cs *ring = &dev_priv->ring[i];
> +
> + /* NB: RCS will hold a ref for all rings */
> + ring->default_context = ctx;
> +
> + /* FIXME: we really only want to do this for initialized rings
> */
> + if (i915.enable_execlists)
> + intel_lr_context_deferred_create(ctx, ring);
> + }
>
> - DRM_DEBUG_DRIVER("%s context support initialized\n",
> dev_priv->hw_context_size ? "HW" : "fake");
> + DRM_DEBUG_DRIVER("%s context support initialized\n",
> + i915.enable_execlists ? "LR" :
> + dev_priv->hw_context_size ? "HW" : "fake");
> return 0;
> }
>
> @@ -781,6 +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))
> return -ENODEV;
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> b/drivers/gpu/drm/i915/intel_lrc.c
> index 21f7f1c..8cc6b55 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -51,3 +51,18 @@ int intel_sanitize_enable_execlists(struct drm_device
> *dev, int enable_execlists
>
> return 0;
> }
> +
> +void intel_lr_context_free(struct intel_context *ctx)
> +{
> + /* TODO */
> +}
> +
> +int intel_lr_context_deferred_create(struct intel_context *ctx,
> + struct intel_engine_cs *ring)
> +{
> + BUG_ON(ctx->legacy_hw_ctx.rcs_state != NULL);
I command thy:
Thou shalt not use BUG_ON except in especially dire circumstances!
Fixed while applying.
-Daniel
> +
> + /* TODO */
> +
> + return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h
> b/drivers/gpu/drm/i915/intel_lrc.h
> index 75ee9c3..3b93572 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -24,6 +24,11 @@
> #ifndef _INTEL_LRC_H_
> #define _INTEL_LRC_H_
>
> +/* Logical Ring Contexts */
> +void intel_lr_context_free(struct intel_context *ctx);
> +int intel_lr_context_deferred_create(struct intel_context *ctx,
> + struct intel_engine_cs *ring);
> +
> /* Execlists */
> int intel_sanitize_enable_execlists(struct drm_device *dev, int
> enable_execlists);
>
> --
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
