Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Ben Widawsky <ben@bwidawsk.net>
To: Intel GFX <intel-gfx@lists.freedesktop.org>
Cc: Ben Widawsky <ben@bwidawsk.net>
Subject: [PATCH 09/34] drm/i915: Use PDEs as the guard page
Date: Sat, 25 May 2013 12:26:43 -0700	[thread overview]
Message-ID: <1369510028-3343-10-git-send-email-ben@bwidawsk.net> (raw)
In-Reply-To: <1369510028-3343-1-git-send-email-ben@bwidawsk.net>

Scary alert.

AFAICT, we simply do not need the guard page if we have the PDEs at the
top since all prefetching is CS related, and it should always be safe to
prefetch into a PDE (provided the PDE is valid). The PDE fetching itself
should not be subject to the prefetching problem, though without full
PPGTT support, we may never know.

Potentially this is achievable even without using the PPGTT reworks I've
done prior to this, but I figure there is no point in rocking the boat
without the PPGTT generalizations I've submitted.

There is a very simple bool left to help test if things break and we
fear guard page related stuff. Of course the commit is easily
revertable as well.

One reason why it's desirable to do this is with the drm_mm_node
allocation I've done with the PDEs, we fragment a bit of space finding the
the first properly aligned address (if my math is right, we get 11 spare
pages fragmented at the top of our address space). This problem didn't
exist with the original implementation that stole the PDEs out from
under the drm_mm.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_drv.h     |  3 +-
 drivers/gpu/drm/i915/i915_gem.c     |  2 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c | 80 +++++++++++++++++++++++--------------
 3 files changed, 54 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 59f98ec..d6dd26a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1750,7 +1750,8 @@ void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj);
 void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj);
 void i915_gem_init_global_gtt(struct drm_device *dev);
 void i915_gem_setup_global_gtt(struct drm_device *dev, unsigned long start,
-			       unsigned long mappable_end, unsigned long end);
+			       unsigned long mappable_end, unsigned long end,
+			       bool guard_page);
 int i915_gem_gtt_init(struct drm_device *dev);
 static inline void i915_gem_chipset_flush(struct drm_device *dev)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2fc9b93..f85fb0e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -161,7 +161,7 @@ i915_gem_init_ioctl(struct drm_device *dev, void *data,
 
 	mutex_lock(&dev->struct_mutex);
 	i915_gem_setup_global_gtt(dev, args->gtt_start, args->gtt_end,
-				  args->gtt_end);
+				  args->gtt_end, true);
 	dev_priv->gtt.mappable_end = args->gtt_end;
 	mutex_unlock(&dev->struct_mutex);
 
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 9ff8769..49467c1 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -282,7 +282,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 						  &ppgtt->node, GEN6_PD_SIZE,
 						  GEN6_PD_ALIGN, 0,
 						  dev_priv->gtt.mappable_end,
-						  dev_priv->gtt.total-PAGE_SIZE,
+						  dev_priv->gtt.total,
 						  DRM_MM_TOPDOWN);
 	if (ret)
 		return ret;
@@ -360,10 +360,9 @@ err_pt_alloc:
 	return ret;
 }
 
-static int i915_gem_init_aliasing_ppgtt(struct drm_device *dev)
+int i915_gem_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct i915_hw_ppgtt *ppgtt;
 	int ret;
 
 	ppgtt = kzalloc(sizeof(*ppgtt), GFP_KERNEL);
@@ -630,10 +629,26 @@ static void i915_gtt_color_adjust(struct drm_mm_node *node,
 			*end -= 4096;
 	}
 }
+
+static bool intel_enable_ppgtt(struct drm_device *dev)
+{
+	if (i915_enable_ppgtt >= 0)
+		return i915_enable_ppgtt;
+
+#ifdef CONFIG_INTEL_IOMMU
+	/* Disable ppgtt on SNB if VT-d is on. */
+	if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped)
+		return false;
+#endif
+
+	return true;
+}
+
 void i915_gem_setup_global_gtt(struct drm_device *dev,
 			       unsigned long start,
 			       unsigned long mappable_end,
-			       unsigned long end)
+			       unsigned long end,
+			       bool guard_page)
 {
 	/* Let GEM Manage all of the aperture.
 	 *
@@ -651,8 +666,12 @@ void i915_gem_setup_global_gtt(struct drm_device *dev,
 
 	BUG_ON(mappable_end > end);
 
-	/* Subtract the guard page ... */
-	drm_mm_init(&dev_priv->mm.gtt_space, start, end - start - PAGE_SIZE);
+	if (!guard_page)
+		drm_mm_init(&dev_priv->mm.gtt_space, start, end - start);
+	else
+		drm_mm_init(&dev_priv->mm.gtt_space, start,
+			    end - start - PAGE_SIZE); /* Guard page */
+
 	if (!HAS_LLC(dev))
 		dev_priv->mm.gtt_space.color_adjust = i915_gtt_color_adjust;
 
@@ -681,46 +700,49 @@ void i915_gem_setup_global_gtt(struct drm_device *dev,
 					      (hole_end-hole_start) / PAGE_SIZE);
 	}
 
-	/* And finally clear the reserved guard page */
-	dev_priv->gtt.gtt_clear_range(dev, end / PAGE_SIZE - 1, 1);
-}
-
-static bool
-intel_enable_ppgtt(struct drm_device *dev)
-{
-	if (i915_enable_ppgtt >= 0)
-		return i915_enable_ppgtt;
-
-#ifdef CONFIG_INTEL_IOMMU
-	/* Disable ppgtt on SNB if VT-d is on. */
-	if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped)
-		return false;
-#endif
-
-	return true;
+	/* And finally clear the reserved guard page (if exists) */
+	if (guard_page)
+		dev_priv->gtt.gtt_clear_range(dev, end / PAGE_SIZE - 1, 1);
 }
 
 void i915_gem_init_global_gtt(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	unsigned long gtt_size, mappable_size;
+	int ret = 0;
 
 	gtt_size = dev_priv->gtt.total;
 	mappable_size = dev_priv->gtt.mappable_end;
 
 	if (intel_enable_ppgtt(dev) && HAS_ALIASING_PPGTT(dev)) {
-		int ret;
+		struct i915_hw_ppgtt *ppgtt;
+
+		i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size,
+					  false);
+
+		ppgtt = kzalloc(sizeof(*ppgtt), GFP_KERNEL);
+		if (!ppgtt) {
+			ret = -ENOMEM;
+			goto ggtt_only;
+		}
 
-		i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
+		ret = i915_gem_ppgtt_init(dev, ppgtt);
+		if (ret)
+			goto ggtt_only;
 
-		ret = i915_gem_init_aliasing_ppgtt(dev);
-		if (!ret)
-			return;
+		return;
+	}
 
+/* XXX: We need to takedown the drm_mm and have this fall back because of the
+ * conditional use of the guard page.
+ * TODO: It's a bit hackish and could use cleanup.
+ */
+ggtt_only:
+	if (ret) {
 		DRM_ERROR("Aliased PPGTT setup failed %d\n", ret);
 		drm_mm_takedown(&dev_priv->mm.gtt_space);
 	}
-	i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
+	i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size, true);
 }
 
 static int setup_scratch_page(struct drm_device *dev)
-- 
1.8.2.3

  parent reply	other threads:[~2013-05-25 19:24 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 ` Ben Widawsky [this message]
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   ` [PATCH v3 " Ben Widawsky
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=1369510028-3343-10-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