[Intel-gfx] [PATCH 09/53] drm/i915/bdw: Initialization for Logical Ring Contexts

[Daniel Vetter]

On Fri, Jun 13, 2014 at 04:37:27PM +0100, oscar.mateo at intel.com wrote:
> From: Oscar Mateo <oscar.mateo at intel.com>
> 
> 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).
> 
> Signed-off-by: Oscar Mateo <oscar.mateo at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 801b891..3f3fb36 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -416,7 +416,13 @@ 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)) {
> +	dev_priv->lrc_enabled = intel_enable_execlists(dev);
> +
> +	if (dev_priv->lrc_enabled) {
> +		/* 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",
> @@ -436,7 +442,9 @@ int i915_gem_context_init(struct drm_device *dev)
>  	for (i = 0; i < I915_NUM_RINGS; i++)
>  		dev_priv->ring[i].default_context = ctx;
>  
> -	DRM_DEBUG_DRIVER("%s context support initialized\n", dev_priv->hw_context_size ? "HW" : "fake");
> +	DRM_DEBUG_DRIVER("%s context support initialized\n",
> +			dev_priv->lrc_enabled ? "LR" :
> +			dev_priv->hw_context_size ? "HW" : "fake");
>  	return 0;
>  }
>  
> @@ -765,9 +773,12 @@ 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;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +
> +	/* FIXME: this would be cleaner with a "context type" enum */
> +	return dev_priv->lrc_enabled || dev_priv->hw_context_size;

Since you have a bunch of if ladders the usual approach isn't an enum but
a vfunc table to abstract behaviour. Think object types instead of switch
statements. Style bikeshed though (presume code later on doesn't have
excesses here).
-Daniel

>  }
>  
>  int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
> @@ -778,7 +789,7 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
>  	struct intel_context *ctx;
>  	int ret;
>  
> -	if (!hw_context_enabled(dev))
> +	if (!contexts_enabled(dev))
>  		return -ENODEV;
>  
>  	ret = i915_mutex_lock_interruptible(dev);
> -- 
> 1.9.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at 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