On Fri, Jun 13, 2014 at 04:37:27PM +0100, oscar.mateo@xxxxxxxxx wrote:
> From: Oscar Mateo <oscar.mateo@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
