[PATCH 3/3] drm/i915: Remove ppgtt enum from device info

Bob Paauwe bob.j.paauwe at intel.com
Thu Oct 11 22:57:22 UTC 2018


Having both an enum that specifies the range in a generic maner and
explicitly setting the size is redundent. In most cases we only care
if ppgtt is going to be used or not and that can be determined by the
number of bits.

The only exception is GEN6 which requires requires special handling
so lets make that obvious in the code.

Signed-off-by: Bob Paauwe <bob.j.paauwe at intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c             | 23 ++++++++++++++++++++-
 drivers/gpu/drm/i915/i915_drv.h             |  8 +++++++
 drivers/gpu/drm/i915/i915_gem_context.c     |  5 +++++
 drivers/gpu/drm/i915/i915_gem_gtt.c         |  9 +++++++-
 drivers/gpu/drm/i915/i915_pci.c             |  6 ------
 drivers/gpu/drm/i915/intel_device_info.c    |  2 +-
 drivers/gpu/drm/i915/intel_device_info.h    | 14 ++++++++-----
 drivers/gpu/drm/i915/selftests/huge_pages.c |  4 ++--
 8 files changed, 55 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 8fcdde9a12e0..482471f171a4 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -345,7 +345,28 @@ static int i915_getparam_ioctl(struct drm_device *dev, void *data,
 		value = HAS_WT(dev_priv);
 		break;
 	case I915_PARAM_HAS_ALIASING_PPGTT:
-		value = min_t(int, INTEL_PPGTT(dev_priv), I915_GEM_PPGTT_FULL);
+		/*
+		 * This is the one area that relies on the enum values for
+		 * PPGTT.  We need to maintain a maping for userspace.
+		 *
+		 * Maybe this should be based on platform?
+		 */
+		if (INTEL_GEN(dev_priv) < 6)
+			value = I915_GEM_PPGTT_NONE;
+		else if (IS_GEN6(dev_priv))
+			value = I915_GEM_PPGTT_ALIASING;
+		else
+			value = I915_GEM_PPGTT_FULL;
+#if 0
+		if (INTEL_PPGTT_BITS(dev_priv) == 0)
+			value = I915_GEM_PPGTT_NONE;
+		/* NOPE, this won't work aliasing is 31 bits too */
+		else if (INTEL_PPGTT_BITS(dev_priv) < 31)
+			value = I915_GEM_PPGTT_ALIASING;
+		else if (INTEL_PPGTT_BITS(dev_priv) >= 31)
+			value = I915_GEM_PPGTT_FULL;
+		//value = min_t(int, INTEL_PPGTT(dev_priv), I915_GEM_PPGTT_FULL);
+#endif
 		break;
 	case I915_PARAM_HAS_SEMAPHORES:
 		value = HAS_LEGACY_SEMAPHORES(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 709c5ce2caa9..2331e3c575d6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2600,11 +2600,19 @@ intel_info(const struct drm_i915_private *dev_priv)
 
 #define HAS_EXECLISTS(dev_priv) HAS_LOGICAL_RING_CONTEXTS(dev_priv)
 
+#define INTEL_PPGTT_BITS(dev_priv) (INTEL_INFO(dev_priv)->ppgtt_bits)
+#define HAS_PPGTT(dev_priv) \
+	(INTEL_PPGTT_BITS(dev_priv) != 0)
+#define HAS_FULL_PPGTT(dev_priv) \
+	(INTEL_PPGTT_BITS(dev_priv) >= 31)
+
+/*
 #define INTEL_PPGTT(dev_priv) (INTEL_INFO(dev_priv)->ppgtt)
 #define HAS_PPGTT(dev_priv) \
 	(INTEL_PPGTT(dev_priv) != INTEL_PPGTT_NONE)
 #define HAS_FULL_PPGTT(dev_priv) \
 	(INTEL_PPGTT(dev_priv) >= INTEL_PPGTT_FULL)
+*/
 
 #define HAS_PAGE_SIZES(dev_priv, sizes) ({ \
 	GEM_BUG_ON((sizes) == 0); \
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 2fb88f717136..609e1e72be13 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -414,6 +414,11 @@ i915_gem_create_context(struct drm_i915_private *dev_priv,
 	if (IS_ERR(ctx))
 		return ctx;
 
+	/* BOB:
+	 * This is maybe also a problem in that GEN6, which is aliased
+	 * ppgtt is also 31 bits which now matches here. Maybe this
+	 * changes to > GEN6?
+	 */
 	if (HAS_FULL_PPGTT(dev_priv)) {
 		struct i915_hw_ppgtt *ppgtt;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 2f603ce94ad4..4b8d9314e46f 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2861,7 +2861,14 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
 	/* And finally clear the reserved guard page */
 	ggtt->vm.clear_range(&ggtt->vm, ggtt->vm.total - PAGE_SIZE, PAGE_SIZE);
 
-	if (INTEL_PPGTT(dev_priv) == INTEL_PPGTT_ALIASING) {
+	/*
+	 * BOB:
+	 * PROBLEM: it looks like the bit size is 31, just like FULL PPGTT
+	 * but this is only called for GEN6.  Could this be replaced
+	 * with a IS_GEN6() check?
+	 */
+	if (IS_GEN6(dev_priv)) {
+	//if (INTEL_PPGTT(dev_priv) == INTEL_PPGTT_ALIASING) {
 		ret = i915_gem_init_aliasing_ppgtt(dev_priv);
 		if (ret)
 			goto err;
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 4ac4d23fbd5f..7fef9a2193fe 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -252,7 +252,6 @@ static const struct intel_device_info intel_ironlake_m_info = {
 	.has_llc = 1, \
 	.has_rc6 = 1, \
 	.has_rc6p = 1, \
-	.ppgtt = INTEL_PPGTT_ALIASING, \
 	.ppgtt_bits = 31, \
 	GEN_DEFAULT_PIPEOFFSETS, \
 	GEN_DEFAULT_PAGE_SIZES, \
@@ -298,7 +297,6 @@ static const struct intel_device_info intel_sandybridge_m_gt2_info = {
 	.has_llc = 1, \
 	.has_rc6 = 1, \
 	.has_rc6p = 1, \
-	.ppgtt = INTEL_PPGTT_FULL, \
 	.ppgtt_bits = 31, \
 	GEN_DEFAULT_PIPEOFFSETS, \
 	GEN_DEFAULT_PAGE_SIZES, \
@@ -352,7 +350,6 @@ static const struct intel_device_info intel_valleyview_info = {
 	.has_rc6 = 1,
 	.has_gmch_display = 1,
 	.has_hotplug = 1,
-	.ppgtt = INTEL_PPGTT_FULL,
 	.ppgtt_bits = 31,
 	.has_snoop = true,
 	.has_coherent_ggtt = false,
@@ -400,7 +397,6 @@ static const struct intel_device_info intel_haswell_gt3_info = {
 	.page_sizes = I915_GTT_PAGE_SIZE_4K | \
 		      I915_GTT_PAGE_SIZE_2M, \
 	.has_logical_ring_contexts = 1, \
-	.ppgtt = INTEL_PPGTT_FULL, \
 	.ppgtt_bits = 48, \
 	.has_64bit_reloc = 1, \
 	.has_reset_engine = 1
@@ -445,7 +441,6 @@ static const struct intel_device_info intel_cherryview_info = {
 	.has_rc6 = 1,
 	.has_logical_ring_contexts = 1,
 	.has_gmch_display = 1,
-	.ppgtt = INTEL_PPGTT_FULL,
 	.ppgtt_bits = 32,
 	.has_reset_engine = 1,
 	.has_snoop = true,
@@ -522,7 +517,6 @@ static const struct intel_device_info intel_skylake_gt4_info = {
 	.has_logical_ring_contexts = 1, \
 	.has_logical_ring_preemption = 1, \
 	.has_guc = 1, \
-	.ppgtt = INTEL_PPGTT_FULL, \
 	.ppgtt_bits = 48, \
 	.has_reset_engine = 1, \
 	.has_snoop = true, \
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index d5f52793d4f1..4e696e87da6f 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -853,7 +853,7 @@ void intel_device_info_runtime_init(struct intel_device_info *info)
 
 	if (IS_GEN6(dev_priv) && intel_vtd_active()) {
 		DRM_INFO("Disabling ppGTT for VT-d support\n");
-		info->ppgtt = INTEL_PPGTT_NONE;
+		info->ppgtt_bits = 0;
 	}
 
 	/* Initialize command stream timestamp frequency */
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index dc60be4b1435..1c391f6712e9 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -76,11 +76,15 @@ enum intel_platform {
 	INTEL_MAX_PLATFORMS
 };
 
+/*
+ * BOB: Remove this?
 enum intel_ppgtt {
-	INTEL_PPGTT_NONE = I915_GEM_PPGTT_NONE,
-	INTEL_PPGTT_ALIASING = I915_GEM_PPGTT_ALIASING,
-	INTEL_PPGTT_FULL = I915_GEM_PPGTT_FULL,
+	INTEL_PPGTT_NONE = 0,
+	INTEL_PPGTT_ALIASING = 31,
+	INTEL_PPGTT_FULL = 31,
+	INTEL_PPGTT_4LVL = 48,
 };
+*/
 
 #define DEV_INFO_FOR_EACH_FLAG(func) \
 	func(is_mobile); \
@@ -159,7 +163,7 @@ struct intel_device_info {
 	enum intel_platform platform;
 	u32 platform_mask;
 
-	enum intel_ppgtt ppgtt;
+	//enum intel_ppgtt ppgtt;
 	unsigned int page_sizes; /* page sizes supported by the HW */
 
 	u32 display_mmio_offset;
@@ -189,7 +193,7 @@ struct intel_device_info {
 		u16 gamma_lut_size;
 	} color;
 
-	/* Full PPGTT address range size */
+	/* PPGTT address range size in number of bits */
 	int ppgtt_bits;
 };
 
diff --git a/drivers/gpu/drm/i915/selftests/huge_pages.c b/drivers/gpu/drm/i915/selftests/huge_pages.c
index 95abf8475464..11973452fed6 100644
--- a/drivers/gpu/drm/i915/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/selftests/huge_pages.c
@@ -1696,8 +1696,8 @@ int i915_gem_huge_page_mock_selftests(void)
 	if (!dev_priv)
 		return -ENOMEM;
 
-	/* Pretend to be a device which supports the 48b PPGTT */
-	mkwrite_device_info(dev_priv)->ppgtt = INTEL_PPGTT_FULL;
+	/* Pretend to be a device which supports the 63b PPGTT */
+	mkwrite_device_info(dev_priv)->ppgtt_bits = 63;
 
 	pdev = dev_priv->drm.pdev;
 	dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(39));
-- 
2.17.1



More information about the Intel-gfx-trybot mailing list