Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Ben Widawsky <ben@bwidawsk.net>
To: intel-gfx@lists.freedesktop.org
Cc: Ben Widawsky <ben@bwidawsk.net>
Subject: [PATCH v3 12/34] drm/i915: Split context enabling from init
Date: Sat, 25 May 2013 18:41:08 -0700	[thread overview]
Message-ID: <1369532468-18326-1-git-send-email-ben@bwidawsk.net> (raw)
In-Reply-To: <1369510028-3343-13-git-send-email-ben@bwidawsk.net>

We **need** to do this for exactly 1 reason, because we want to embed a
PPGTT into the context, but we don't want to special case the default
context.

To achieve that, we must be able to initialize contexts after the GTT is
setup (so we can allocate and pin the default context's BO), but before
the PPGTT and rings are initialized. This is because, currently, context
initialization requires ring usage. We don't have rings until after the
GTT is setup. If we split the enabling part of context initialization,
the part requiring the ringbuffer, we can untangle this, and then later
embed the PPGTT

Incidentally this allows us to also adhere to the original design of
context init/fini in future patches: they were only ever meant to be
called at driver load and unload.

v2: Move hw_contexts_disabled test in i915_gem_context_enable() (Chris)

v3: BUG_ON after checking for disabled contexts. Or else it blows up pre
gen6 (Ben)

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_drv.h         |  1 +
 drivers/gpu/drm/i915/i915_gem.c         | 15 ++++++++++++---
 drivers/gpu/drm/i915/i915_gem_context.c | 23 ++++++++++++-----------
 3 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d6dd26a..f1b59b1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1715,6 +1715,7 @@ struct dma_buf *i915_gem_prime_export(struct drm_device *dev,
 /* i915_gem_context.c */
 void i915_gem_context_init(struct drm_device *dev);
 void i915_gem_context_fini(struct drm_device *dev);
+int i915_gem_context_enable(struct drm_i915_private *dev_priv);
 void i915_gem_context_close(struct drm_device *dev, struct drm_file *file);
 int i915_switch_context(struct intel_ring_buffer *ring,
 			struct drm_file *file, int to_id);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6d0d302..674154f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4036,10 +4036,17 @@ i915_gem_init_hw(struct drm_device *dev)
 		return ret;
 
 	/*
-	 * XXX: There was some w/a described somewhere suggesting loading
-	 * contexts before PPGTT.
+	 * XXX: Contexts should only be initialized once. Doing a switch to the
+	 * default context switch however is something we'd like to do after
+	 * reset or thaw (the latter may not actually be necessary for HW, but
+	 * goes with our code better). Context switching requires rings (for
+	 * the do_switch), but before enabling PPGTT. So don't move this.
 	 */
-	i915_gem_context_init(dev);
+	if (i915_gem_context_enable(dev_priv)) {
+		i915_gem_context_fini(dev);
+		dev_priv->hw_contexts_disabled = true;
+	}
+
 	if (dev_priv->mm.aliasing_ppgtt) {
 		ret = dev_priv->mm.aliasing_ppgtt->enable(dev);
 		if (ret) {
@@ -4067,6 +4074,8 @@ int i915_gem_init(struct drm_device *dev)
 
 	i915_gem_init_global_gtt(dev);
 
+	i915_gem_context_init(dev);
+
 	ret = i915_gem_init_hw(dev);
 	mutex_unlock(&dev->struct_mutex);
 	if (ret) {
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 9ce0acd..22f90c7 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -219,19 +219,11 @@ static int create_default_context(struct drm_i915_private *dev_priv)
 		goto err_destroy;
 	}
 
-	ret = do_switch(ctx);
-	if (ret) {
-		DRM_DEBUG_DRIVER("Switch failed %d\n", ret);
-		goto err_unpin;
-	}
-
 	dev_priv->ring[RCS].default_context = ctx;
 
 	DRM_DEBUG_DRIVER("Default HW context loaded\n");
 	return 0;
 
-err_unpin:
-	i915_gem_object_unpin(ctx->obj);
 err_destroy:
 	i915_gem_context_unreference(ctx);
 	return ret;
@@ -247,9 +239,10 @@ void i915_gem_context_init(struct drm_device *dev)
 		return;
 	}
 
-	/* If called from reset, or thaw... we've been here already */
-	if (dev_priv->hw_contexts_disabled ||
-	    dev_priv->ring[RCS].default_context)
+	/* Init should only be called once per module load. Eventually the
+	 * restriction on the context_disabled check can be loosened. */
+	if (WARN_ON(dev_priv->hw_contexts_disabled ||
+	    WARN_ON(dev_priv->ring[RCS].default_context)))
 		return;
 
 	dev_priv->hw_context_size = round_up(get_context_size(dev), 4096);
@@ -302,6 +295,14 @@ void i915_gem_context_fini(struct drm_device *dev)
 	dev_priv->ring[RCS].last_context = NULL;
 }
 
+int i915_gem_context_enable(struct drm_i915_private *dev_priv)
+{
+	if (dev_priv->hw_contexts_disabled)
+		return 0;
+	BUG_ON(!dev_priv->ring[RCS].default_context);
+	return do_switch(dev_priv->ring[RCS].default_context);
+}
+
 static int context_idr_cleanup(int id, void *p, void *data)
 {
 	struct i915_hw_context *ctx = p;
-- 
1.8.2.3

  reply	other threads:[~2013-05-26  1:38 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-25 19:26 [PATCH 00/34] PPGTT prep part 2 (and unmerged part 1) Ben Widawsky
2013-05-25 19:26 ` [PATCH 01/34] drm/i915: pre-fixes for checkpatch Ben Widawsky
2013-05-25 19:26 ` [PATCH 02/34] drm/i915: use mappable size for fb kickout Ben Widawsky
2013-05-25 19:26 ` [PATCH 03/34] drm/i915: use drm_mm_takedown Ben Widawsky
2013-05-25 19:26 ` [PATCH 04/34] drm/i915: context debug messages Ben Widawsky
2013-05-25 19:26 ` [PATCH 05/34] drm/i915: Call context fini at cleanup Ben Widawsky
2013-05-26 13:27   ` Daniel Vetter
2013-05-25 19:26 ` [PATCH 06/34] drm/i915: make PDE|PTE platform specific Ben Widawsky
2013-05-25 19:26 ` [PATCH 07/34] drm: Optionally create mm blocks from top-to-bottom Ben Widawsky
2013-05-25 19:26 ` [PATCH 08/34] drm/i915: Use drm_mm for PPGTT PDEs Ben Widawsky
2013-05-25 19:26 ` [PATCH 09/34] drm/i915: Use PDEs as the guard page Ben Widawsky
2013-05-25 19:26 ` [PATCH 10/34] drm/i915: cleanup context fini Ben Widawsky
2013-05-25 19:26 ` [PATCH 11/34] drm/i915: Do a fuller init after reset Ben Widawsky
2013-05-25 19:26 ` [PATCH 12/34] drm/i915: Split context enabling from init Ben Widawsky
2013-05-26  1:41   ` Ben Widawsky [this message]
2013-05-25 19:26 ` [PATCH 13/34] drm/i915: destroy i915_gem_init_global_gtt Ben Widawsky
2013-05-25 19:26 ` [PATCH 14/34] drm/i915: Embed PPGTT into the context Ben Widawsky
2013-05-25 19:26 ` [PATCH 15/34] drm/i915: Tie context to PPGTT Ben Widawsky
2013-05-29  5:32   ` [PATCH 14.5/34] drm/i915: Unify PPGTT codepaths on gen6+ Ben Widawsky
2013-05-25 19:26 ` [PATCH 16/34] drm/i915: Really share scratch page Ben Widawsky
2013-05-25 19:26 ` [PATCH 17/34] drm/i915: Combine scratch members into a struct Ben Widawsky
2013-05-25 19:26 ` [PATCH 18/34] drm/i915: Drop dev from pte_encode Ben Widawsky
2013-05-25 20:14   ` Kenneth Graunke
2013-05-25 19:26 ` [PATCH 19/34] drm/i915: Use gtt shortform where possible Ben Widawsky
2013-05-25 19:26 ` [PATCH 20/34] drm/i915: Move fbc members out of line Ben Widawsky
2013-05-25 19:26 ` [PATCH 21/34] drm/i915: Move gtt and ppgtt under address space umbrella Ben Widawsky
2013-05-25 19:26 ` [PATCH 22/34] drm/i915: Move gtt_mtrr to i915_gtt Ben Widawsky
2013-05-25 19:26 ` [PATCH 23/34] drm/i915: Move stolen stuff " Ben Widawsky
2013-05-25 19:26 ` [PATCH 24/34] drm/i915: Move aliasing_ppgtt Ben Widawsky
2013-05-25 19:26 ` [PATCH 25/34] drm/i915: Put the mm in the parent address space Ben Widawsky
2013-05-25 19:27 ` [PATCH 26/34] drm/i915: Move object tracking lists to new mm Ben Widawsky
2013-05-25 19:27 ` [PATCH 27/34] drm/i915: Create a global list of vms Ben Widawsky
2013-05-26 17:19   ` Ben Widawsky
2013-05-25 19:27 ` [PATCH 28/34] drm/i915: Start using vm lists Ben Widawsky
2013-05-25 19:27 ` [PATCH 29/34] drm/i915: Remove object's gtt_offset Ben Widawsky
2013-05-25 19:27 ` [PATCH 30/34] drm: pre allocate node for create_block Ben Widawsky
2013-05-25 19:27 ` [PATCH 31/34] drm/i915: Getter/setter for object attributes Ben Widawsky
2013-05-25 19:27 ` [PATCH 32/34] drm/i915: Create VMAs (part 1) Ben Widawsky
2013-05-25 19:27 ` [PATCH 33/34] drm/i915: Create VMAs (part 2) Ben Widawsky
2013-05-25 19:27 ` [PATCH 34/34] drm/i915: Create VMAs (part 3) Ben Widawsky
2013-05-27  7:31   ` Chris Wilson
2013-05-27  8:59     ` Daniel Vetter
2013-06-04 21:49 ` [PATCH 00/34] PPGTT prep part 2 (and unmerged part 1) Ben Widawsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1369532468-18326-1-git-send-email-ben@bwidawsk.net \
    --to=ben@bwidawsk.net \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox