[PATCH v2 1/3] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER

[PATCH v2 1/3] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER

[Chris Wilson]

i915_gem_stolen is an allocator for the reserved portion of memory
("stolen" from the system by the BIOS). It is not tied to KMS but
central to the driver, so prefer DRM_DEBUG_DRIVER.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_stolen.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 62aa67960bf4..b04e2551bae6 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -121,8 +121,8 @@ static int i915_adjust_stolen(struct drm_i915_private *dev_priv,
 
 		if (stolen[0].start != stolen[1].start ||
 		    stolen[0].end != stolen[1].end) {
-			DRM_DEBUG_KMS("GTT within stolen memory at %pR\n", &ggtt_res);
-			DRM_DEBUG_KMS("Stolen memory adjusted to %pR\n", dsm);
+			DRM_DEBUG_DRIVER("GTT within stolen memory at %pR\n", &ggtt_res);
+			DRM_DEBUG_DRIVER("Stolen memory adjusted to %pR\n", dsm);
 		}
 	}
 
@@ -406,9 +406,9 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
 	 * memory, so just consider the start. */
 	reserved_total = stolen_top - reserved_base;
 
-	DRM_DEBUG_KMS("Memory reserved for graphics device: %lluK, usable: %lluK\n",
-		      (u64)resource_size(&dev_priv->dsm) >> 10,
-		      ((u64)resource_size(&dev_priv->dsm) - reserved_total) >> 10);
+	DRM_DEBUG_DRIVER("Memory reserved for graphics device: %lluK, usable: %lluK\n",
+			(u64)resource_size(&dev_priv->dsm) >> 10,
+			((u64)resource_size(&dev_priv->dsm) - reserved_total) >> 10);
 
 	stolen_usable_start = 0;
 	/* WaSkipStolenMemoryFirstPage:bdw+ */
@@ -580,7 +580,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_i915_private *dev_priv
 
 	lockdep_assert_held(&dev_priv->drm.struct_mutex);
 
-	DRM_DEBUG_KMS("creating preallocated stolen object: stolen_offset=%pa, gtt_offset=%pa, size=%pa\n",
+	DRM_DEBUG_DRIVER("creating preallocated stolen object: stolen_offset=%pa, gtt_offset=%pa, size=%pa\n",
 			&stolen_offset, &gtt_offset, &size);
 
 	/* KISS and expect everything to be page-aligned */
@@ -599,14 +599,14 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_i915_private *dev_priv
 	ret = drm_mm_reserve_node(&dev_priv->mm.stolen, stolen);
 	mutex_unlock(&dev_priv->mm.stolen_lock);
 	if (ret) {
-		DRM_DEBUG_KMS("failed to allocate stolen space\n");
+		DRM_DEBUG_DRIVER("failed to allocate stolen space\n");
 		kfree(stolen);
 		return NULL;
 	}
 
 	obj = _i915_gem_object_create_stolen(dev_priv, stolen);
 	if (obj == NULL) {
-		DRM_DEBUG_KMS("failed to allocate stolen object\n");
+		DRM_DEBUG_DRIVER("failed to allocate stolen object\n");
 		i915_gem_stolen_remove_node(dev_priv, stolen);
 		kfree(stolen);
 		return NULL;
@@ -635,7 +635,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_i915_private *dev_priv
 				   size, gtt_offset, obj->cache_level,
 				   0);
 	if (ret) {
-		DRM_DEBUG_KMS("failed to allocate stolen GTT space\n");
+		DRM_DEBUG_DRIVER("failed to allocate stolen GTT space\n");
 		goto err_pages;
 	}
 
-- 
2.16.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 2/3] drm/i915/stolen: Checkpatch cleansing

[Chris Wilson]

In the next patch, we will introduce a new vlv_get_stolen_reserved, so
before we do, make sure checkpatch is happy with the surrounding code.
Sneak in some debug output while we are here.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_stolen.c | 40 ++++++++++++++++++++++++----------
 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index b04e2551bae6..7cc273e690d0 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -174,13 +174,17 @@ void i915_gem_cleanup_stolen(struct drm_device *dev)
 }
 
 static void g4x_get_stolen_reserved(struct drm_i915_private *dev_priv,
-				    resource_size_t *base, resource_size_t *size)
+				    resource_size_t *base,
+				    resource_size_t *size)
 {
-	uint32_t reg_val = I915_READ(IS_GM45(dev_priv) ?
-				     CTG_STOLEN_RESERVED :
-				     ELK_STOLEN_RESERVED);
+	u32 reg_val = I915_READ(IS_GM45(dev_priv) ?
+				CTG_STOLEN_RESERVED :
+				ELK_STOLEN_RESERVED);
 	resource_size_t stolen_top = dev_priv->dsm.end + 1;
 
+	DRM_DEBUG_DRIVER("%s_STOLEN_RESERVED = %08x\n",
+			 IS_GM45(dev_priv) ? "CTG" : "ELK", reg_val);
+
 	if ((reg_val & G4X_STOLEN_RESERVED_ENABLE) == 0) {
 		*base = 0;
 		*size = 0;
@@ -208,9 +212,12 @@ static void g4x_get_stolen_reserved(struct drm_i915_private *dev_priv,
 }
 
 static void gen6_get_stolen_reserved(struct drm_i915_private *dev_priv,
-				     resource_size_t *base, resource_size_t *size)
+				     resource_size_t *base,
+				     resource_size_t *size)
 {
-	uint32_t reg_val = I915_READ(GEN6_STOLEN_RESERVED);
+	u32 reg_val = I915_READ(GEN6_STOLEN_RESERVED);
+
+	DRM_DEBUG_DRIVER("GEN6_STOLEN_RESERVED = %08x\n", reg_val);
 
 	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
 		*base = 0;
@@ -240,9 +247,12 @@ static void gen6_get_stolen_reserved(struct drm_i915_private *dev_priv,
 }
 
 static void gen7_get_stolen_reserved(struct drm_i915_private *dev_priv,
-				     resource_size_t *base, resource_size_t *size)
+				     resource_size_t *base,
+				     resource_size_t *size)
 {
-	uint32_t reg_val = I915_READ(GEN6_STOLEN_RESERVED);
+	u32 reg_val = I915_READ(GEN6_STOLEN_RESERVED);
+
+	DRM_DEBUG_DRIVER("GEN6_STOLEN_RESERVED = %08x\n", reg_val);
 
 	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
 		*base = 0;
@@ -266,9 +276,12 @@ static void gen7_get_stolen_reserved(struct drm_i915_private *dev_priv,
 }
 
 static void chv_get_stolen_reserved(struct drm_i915_private *dev_priv,
-				    resource_size_t *base, resource_size_t *size)
+				    resource_size_t *base,
+				    resource_size_t *size)
 {
-	uint32_t reg_val = I915_READ(GEN6_STOLEN_RESERVED);
+	u32 reg_val = I915_READ(GEN6_STOLEN_RESERVED);
+
+	DRM_DEBUG_DRIVER("GEN6_STOLEN_RESERVED = %08x\n", reg_val);
 
 	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
 		*base = 0;
@@ -298,11 +311,14 @@ static void chv_get_stolen_reserved(struct drm_i915_private *dev_priv,
 }
 
 static void bdw_get_stolen_reserved(struct drm_i915_private *dev_priv,
-				    resource_size_t *base, resource_size_t *size)
+				    resource_size_t *base,
+				    resource_size_t *size)
 {
-	uint32_t reg_val = I915_READ(GEN6_STOLEN_RESERVED);
+	u32 reg_val = I915_READ(GEN6_STOLEN_RESERVED);
 	resource_size_t stolen_top;
 
+	DRM_DEBUG_DRIVER("GEN6_STOLEN_RESERVED = %08x\n", reg_val);
+
 	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
 		*base = 0;
 		*size = 0;
-- 
2.16.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 3/3] drm/i915/stolen: Deduce base of reserved portion as top-size on vlv

[Chris Wilson]

On Valleyview, the HW deduces the base of the reserved portion of stolen
memory as being (top - size) and the address field within
GEN6_STOLEN_RESERVED is set to 0. Add yet another GEN6_STOLEN_RESERVED
reader to cope with the subtly different path required for vlv.

v2: Avoid using reserved_base = reserved_size = 0 as the invalid
condition as that typically falls outside of the stolen region,
provoking a consistency error.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_stolen.c | 103 ++++++++++++++++++---------------
 1 file changed, 56 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 7cc273e690d0..664afcffc41d 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -185,11 +185,8 @@ static void g4x_get_stolen_reserved(struct drm_i915_private *dev_priv,
 	DRM_DEBUG_DRIVER("%s_STOLEN_RESERVED = %08x\n",
 			 IS_GM45(dev_priv) ? "CTG" : "ELK", reg_val);
 
-	if ((reg_val & G4X_STOLEN_RESERVED_ENABLE) == 0) {
-		*base = 0;
-		*size = 0;
+	if ((reg_val & G4X_STOLEN_RESERVED_ENABLE) == 0)
 		return;
-	}
 
 	/*
 	 * Whether ILK really reuses the ELK register for this is unclear.
@@ -197,18 +194,13 @@ static void g4x_get_stolen_reserved(struct drm_i915_private *dev_priv,
 	 */
 	WARN(IS_GEN5(dev_priv), "ILK stolen reserved found? 0x%08x\n", reg_val);
 
-	*base = (reg_val & G4X_STOLEN_RESERVED_ADDR2_MASK) << 16;
+	if (!(reg_val & G4X_STOLEN_RESERVED_ADDR2_MASK))
+		return;
 
+	*base = (reg_val & G4X_STOLEN_RESERVED_ADDR2_MASK) << 16;
 	WARN_ON((reg_val & G4X_STOLEN_RESERVED_ADDR1_MASK) < *base);
 
-	/* On these platforms, the register doesn't have a size field, so the
-	 * size is the distance between the base and the top of the stolen
-	 * memory. We also have the genuine case where base is zero and there's
-	 * nothing reserved. */
-	if (*base == 0)
-		*size = 0;
-	else
-		*size = stolen_top - *base;
+	*size = stolen_top - *base;
 }
 
 static void gen6_get_stolen_reserved(struct drm_i915_private *dev_priv,
@@ -219,11 +211,8 @@ static void gen6_get_stolen_reserved(struct drm_i915_private *dev_priv,
 
 	DRM_DEBUG_DRIVER("GEN6_STOLEN_RESERVED = %08x\n", reg_val);
 
-	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
-		*base = 0;
-		*size = 0;
+	if (!(reg_val & GEN6_STOLEN_RESERVED_ENABLE))
 		return;
-	}
 
 	*base = reg_val & GEN6_STOLEN_RESERVED_ADDR_MASK;
 
@@ -246,6 +235,33 @@ static void gen6_get_stolen_reserved(struct drm_i915_private *dev_priv,
 	}
 }
 
+static void vlv_get_stolen_reserved(struct drm_i915_private *dev_priv,
+				    resource_size_t *base,
+				    resource_size_t *size)
+{
+	u32 reg_val = I915_READ(GEN6_STOLEN_RESERVED);
+	resource_size_t stolen_top = dev_priv->dsm.end + 1;
+
+	DRM_DEBUG_DRIVER("GEN6_STOLEN_RESERVED = %08x\n", reg_val);
+
+	if (!(reg_val & GEN6_STOLEN_RESERVED_ENABLE))
+		return;
+
+	switch (reg_val & GEN7_STOLEN_RESERVED_SIZE_MASK) {
+	default:
+		MISSING_CASE(reg_val & GEN7_STOLEN_RESERVED_SIZE_MASK);
+	case GEN7_STOLEN_RESERVED_1M:
+		*size = 1024 * 1024;
+		break;
+	}
+
+	/*
+	 * On vlv, the ADDR_MASK portion is left as 0 and HW deduces the
+	 * reserved location as (top - size).
+	 */
+	*base = stolen_top - *size;
+}
+
 static void gen7_get_stolen_reserved(struct drm_i915_private *dev_priv,
 				     resource_size_t *base,
 				     resource_size_t *size)
@@ -254,11 +270,8 @@ static void gen7_get_stolen_reserved(struct drm_i915_private *dev_priv,
 
 	DRM_DEBUG_DRIVER("GEN6_STOLEN_RESERVED = %08x\n", reg_val);
 
-	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
-		*base = 0;
-		*size = 0;
+	if (!(reg_val & GEN6_STOLEN_RESERVED_ENABLE))
 		return;
-	}
 
 	*base = reg_val & GEN7_STOLEN_RESERVED_ADDR_MASK;
 
@@ -283,11 +296,8 @@ static void chv_get_stolen_reserved(struct drm_i915_private *dev_priv,
 
 	DRM_DEBUG_DRIVER("GEN6_STOLEN_RESERVED = %08x\n", reg_val);
 
-	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
-		*base = 0;
-		*size = 0;
+	if (!(reg_val & GEN6_STOLEN_RESERVED_ENABLE))
 		return;
-	}
 
 	*base = reg_val & GEN6_STOLEN_RESERVED_ADDR_MASK;
 
@@ -315,28 +325,18 @@ static void bdw_get_stolen_reserved(struct drm_i915_private *dev_priv,
 				    resource_size_t *size)
 {
 	u32 reg_val = I915_READ(GEN6_STOLEN_RESERVED);
-	resource_size_t stolen_top;
+	resource_size_t stolen_top = dev_priv->dsm.end + 1;
 
 	DRM_DEBUG_DRIVER("GEN6_STOLEN_RESERVED = %08x\n", reg_val);
 
-	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
-		*base = 0;
-		*size = 0;
+	if (!(reg_val & GEN6_STOLEN_RESERVED_ENABLE))
 		return;
-	}
 
-	stolen_top = dev_priv->dsm.end + 1;
+	if (!(reg_val & GEN6_STOLEN_RESERVED_ADDR_MASK))
+		return;
 
 	*base = reg_val & GEN6_STOLEN_RESERVED_ADDR_MASK;
-
-	/* On these platforms, the register doesn't have a size field, so the
-	 * size is the distance between the base and the top of the stolen
-	 * memory. We also have the genuine case where base is zero and there's
-	 * nothing reserved. */
-	if (*base == 0)
-		*size = 0;
-	else
-		*size = stolen_top - *base;
+	*size = stolen_top - *base;
 }
 
 int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
@@ -369,7 +369,7 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
 	GEM_BUG_ON(dev_priv->dsm.end <= dev_priv->dsm.start);
 
 	stolen_top = dev_priv->dsm.end + 1;
-	reserved_base = 0;
+	reserved_base = stolen_top;
 	reserved_size = 0;
 
 	switch (INTEL_GEN(dev_priv)) {
@@ -389,8 +389,12 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
 					 &reserved_base, &reserved_size);
 		break;
 	case 7:
-		gen7_get_stolen_reserved(dev_priv,
-					 &reserved_base, &reserved_size);
+		if (IS_VALLEYVIEW(dev_priv))
+			vlv_get_stolen_reserved(dev_priv,
+						&reserved_base, &reserved_size);
+		else
+			gen7_get_stolen_reserved(dev_priv,
+						 &reserved_base, &reserved_size);
 		break;
 	default:
 		if (IS_LP(dev_priv))
@@ -402,11 +406,16 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
 		break;
 	}
 
-	/* It is possible for the reserved base to be zero, but the register
-	 * field for size doesn't have a zero option. */
-	if (reserved_base == 0) {
-		reserved_size = 0;
+	/*
+	 * Our expectation is that the reserved space is at the top of the
+	 * stolen region and *never* at the bottom. If we see !reserved_base,
+	 * it likely means we failed to read the registers correctly.
+	 */
+	if (!reserved_base) {
+		DRM_ERROR("inconsistent reservation %pa + %pa; ignoring\n",
+			  &reserved_base, &reserved_size);
 		reserved_base = stolen_top;
+		reserved_size = 0;
 	}
 
 	dev_priv->dsm_reserved =
-- 
2.16.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/3] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER
URL   : https://patchwork.freedesktop.org/series/39801/
State : success

== Summary ==

Series 39801v1 series starting with [v2,1/3] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER
https://patchwork.freedesktop.org/api/1.0/series/39801/revisions/1/mbox/

---- Known issues:

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> INCOMPLETE (fi-bdw-5557u) fdo#104162

fdo#104162 https://bugs.freedesktop.org/show_bug.cgi?id=104162

fi-bdw-5557u     total:109  pass:105  dwarn:0   dfail:0   fail:0   skip:3  
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:431s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:383s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:544s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:301s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:510s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:513s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:498s
fi-cfl-8700k     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:412s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:577s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:432s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:315s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:540s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:403s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:422s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:471s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:427s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:476s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:465s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:517s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:639s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:441s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:531s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:543s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:497s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:488s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:431s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:435s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:541s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:402s
Blacklisted hosts:
fi-cnl-drrs      total:288  pass:257  dwarn:3   dfail:0   fail:0   skip:28  time:535s
fi-glk-j5005 failed to collect. IGT log at Patchwork_8311/fi-glk-j5005/run0.log

1bf8f00fed0b78f5d286304deb1f1526b10aeaca drm-tip: 2018y-03m-12d-11h-28m-33s UTC integration manifest
9c278882084c drm/i915/stolen: Deduce base of reserved portion as top-size on vlv
34d7320e633c drm/i915/stolen: Checkpatch cleansing
83dee962ddb3 drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8311/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: warning for series starting with [v2,1/3] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/3] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER
URL   : https://patchwork.freedesktop.org/series/39801/
State : warning

== Summary ==

---- Possible new issues:

Test kms_chv_cursor_fail:
        Subgroup pipe-a-128x128-top-edge:
                fail       -> PASS       (shard-apl)
Test kms_vblank:
        Subgroup pipe-a-ts-continuation-suspend:
                pass       -> SKIP       (shard-snb)

---- Known issues:

Test gem_eio:
        Subgroup in-flight-contexts:
                incomplete -> PASS       (shard-apl) fdo#105341
Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-hsw) fdo#99912
Test kms_vblank:
        Subgroup pipe-b-ts-continuation-dpms-suspend:
                pass       -> INCOMPLETE (shard-hsw) fdo#105054
Test pm_lpsp:
        Subgroup screens-disabled:
                fail       -> PASS       (shard-hsw) fdo#104941
Test testdisplay:
                pass       -> DMESG-WARN (shard-apl) fdo#104727

fdo#105341 https://bugs.freedesktop.org/show_bug.cgi?id=105341
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#105054 https://bugs.freedesktop.org/show_bug.cgi?id=105054
fdo#104941 https://bugs.freedesktop.org/show_bug.cgi?id=104941
fdo#104727 https://bugs.freedesktop.org/show_bug.cgi?id=104727

shard-apl        total:3467 pass:1825 dwarn:2   dfail:0   fail:7   skip:1632 time:13013s
shard-hsw        total:3444 pass:1761 dwarn:1   dfail:0   fail:0   skip:1680 time:11593s
shard-snb        total:3467 pass:1363 dwarn:1   dfail:0   fail:2   skip:2101 time:7295s
Blacklisted hosts:
shard-kbl        total:3350 pass:1875 dwarn:5   dfail:0   fail:7   skip:1460 time:8728s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8311/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 3/3] drm/i915/stolen: Deduce base of reserved portion as top-size on vlv

[Ville Syrjälä]

On Mon, Mar 12, 2018 at 04:52:06PM +0000, Chris Wilson wrote:
> On Valleyview, the HW deduces the base of the reserved portion of stolen
> memory as being (top - size) and the address field within
> GEN6_STOLEN_RESERVED is set to 0. Add yet another GEN6_STOLEN_RESERVED
> reader to cope with the subtly different path required for vlv.
> 
> v2: Avoid using reserved_base = reserved_size = 0 as the invalid
> condition as that typically falls outside of the stolen region,
> provoking a consistency error.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>

Didn't spot anything wrong. Series is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 103 ++++++++++++++++++---------------
>  1 file changed, 56 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 7cc273e690d0..664afcffc41d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -185,11 +185,8 @@ static void g4x_get_stolen_reserved(struct drm_i915_private *dev_priv,
>  	DRM_DEBUG_DRIVER("%s_STOLEN_RESERVED = %08x\n",
>  			 IS_GM45(dev_priv) ? "CTG" : "ELK", reg_val);
>  
> -	if ((reg_val & G4X_STOLEN_RESERVED_ENABLE) == 0) {
> -		*base = 0;
> -		*size = 0;
> +	if ((reg_val & G4X_STOLEN_RESERVED_ENABLE) == 0)
>  		return;
> -	}
>  
>  	/*
>  	 * Whether ILK really reuses the ELK register for this is unclear.
> @@ -197,18 +194,13 @@ static void g4x_get_stolen_reserved(struct drm_i915_private *dev_priv,
>  	 */
>  	WARN(IS_GEN5(dev_priv), "ILK stolen reserved found? 0x%08x\n", reg_val);
>  
> -	*base = (reg_val & G4X_STOLEN_RESERVED_ADDR2_MASK) << 16;
> +	if (!(reg_val & G4X_STOLEN_RESERVED_ADDR2_MASK))
> +		return;
>  
> +	*base = (reg_val & G4X_STOLEN_RESERVED_ADDR2_MASK) << 16;
>  	WARN_ON((reg_val & G4X_STOLEN_RESERVED_ADDR1_MASK) < *base);
>  
> -	/* On these platforms, the register doesn't have a size field, so the
> -	 * size is the distance between the base and the top of the stolen
> -	 * memory. We also have the genuine case where base is zero and there's
> -	 * nothing reserved. */
> -	if (*base == 0)
> -		*size = 0;
> -	else
> -		*size = stolen_top - *base;
> +	*size = stolen_top - *base;
>  }
>  
>  static void gen6_get_stolen_reserved(struct drm_i915_private *dev_priv,
> @@ -219,11 +211,8 @@ static void gen6_get_stolen_reserved(struct drm_i915_private *dev_priv,
>  
>  	DRM_DEBUG_DRIVER("GEN6_STOLEN_RESERVED = %08x\n", reg_val);
>  
> -	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
> -		*base = 0;
> -		*size = 0;
> +	if (!(reg_val & GEN6_STOLEN_RESERVED_ENABLE))
>  		return;
> -	}
>  
>  	*base = reg_val & GEN6_STOLEN_RESERVED_ADDR_MASK;
>  
> @@ -246,6 +235,33 @@ static void gen6_get_stolen_reserved(struct drm_i915_private *dev_priv,
>  	}
>  }
>  
> +static void vlv_get_stolen_reserved(struct drm_i915_private *dev_priv,
> +				    resource_size_t *base,
> +				    resource_size_t *size)
> +{
> +	u32 reg_val = I915_READ(GEN6_STOLEN_RESERVED);
> +	resource_size_t stolen_top = dev_priv->dsm.end + 1;
> +
> +	DRM_DEBUG_DRIVER("GEN6_STOLEN_RESERVED = %08x\n", reg_val);
> +
> +	if (!(reg_val & GEN6_STOLEN_RESERVED_ENABLE))
> +		return;
> +
> +	switch (reg_val & GEN7_STOLEN_RESERVED_SIZE_MASK) {
> +	default:
> +		MISSING_CASE(reg_val & GEN7_STOLEN_RESERVED_SIZE_MASK);
> +	case GEN7_STOLEN_RESERVED_1M:
> +		*size = 1024 * 1024;
> +		break;
> +	}
> +
> +	/*
> +	 * On vlv, the ADDR_MASK portion is left as 0 and HW deduces the
> +	 * reserved location as (top - size).
> +	 */
> +	*base = stolen_top - *size;
> +}
> +
>  static void gen7_get_stolen_reserved(struct drm_i915_private *dev_priv,
>  				     resource_size_t *base,
>  				     resource_size_t *size)
> @@ -254,11 +270,8 @@ static void gen7_get_stolen_reserved(struct drm_i915_private *dev_priv,
>  
>  	DRM_DEBUG_DRIVER("GEN6_STOLEN_RESERVED = %08x\n", reg_val);
>  
> -	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
> -		*base = 0;
> -		*size = 0;
> +	if (!(reg_val & GEN6_STOLEN_RESERVED_ENABLE))
>  		return;
> -	}
>  
>  	*base = reg_val & GEN7_STOLEN_RESERVED_ADDR_MASK;
>  
> @@ -283,11 +296,8 @@ static void chv_get_stolen_reserved(struct drm_i915_private *dev_priv,
>  
>  	DRM_DEBUG_DRIVER("GEN6_STOLEN_RESERVED = %08x\n", reg_val);
>  
> -	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
> -		*base = 0;
> -		*size = 0;
> +	if (!(reg_val & GEN6_STOLEN_RESERVED_ENABLE))
>  		return;
> -	}
>  
>  	*base = reg_val & GEN6_STOLEN_RESERVED_ADDR_MASK;
>  
> @@ -315,28 +325,18 @@ static void bdw_get_stolen_reserved(struct drm_i915_private *dev_priv,
>  				    resource_size_t *size)
>  {
>  	u32 reg_val = I915_READ(GEN6_STOLEN_RESERVED);
> -	resource_size_t stolen_top;
> +	resource_size_t stolen_top = dev_priv->dsm.end + 1;
>  
>  	DRM_DEBUG_DRIVER("GEN6_STOLEN_RESERVED = %08x\n", reg_val);
>  
> -	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
> -		*base = 0;
> -		*size = 0;
> +	if (!(reg_val & GEN6_STOLEN_RESERVED_ENABLE))
>  		return;
> -	}
>  
> -	stolen_top = dev_priv->dsm.end + 1;
> +	if (!(reg_val & GEN6_STOLEN_RESERVED_ADDR_MASK))
> +		return;
>  
>  	*base = reg_val & GEN6_STOLEN_RESERVED_ADDR_MASK;
> -
> -	/* On these platforms, the register doesn't have a size field, so the
> -	 * size is the distance between the base and the top of the stolen
> -	 * memory. We also have the genuine case where base is zero and there's
> -	 * nothing reserved. */
> -	if (*base == 0)
> -		*size = 0;
> -	else
> -		*size = stolen_top - *base;
> +	*size = stolen_top - *base;
>  }
>  
>  int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
> @@ -369,7 +369,7 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
>  	GEM_BUG_ON(dev_priv->dsm.end <= dev_priv->dsm.start);
>  
>  	stolen_top = dev_priv->dsm.end + 1;
> -	reserved_base = 0;
> +	reserved_base = stolen_top;
>  	reserved_size = 0;
>  
>  	switch (INTEL_GEN(dev_priv)) {
> @@ -389,8 +389,12 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
>  					 &reserved_base, &reserved_size);
>  		break;
>  	case 7:
> -		gen7_get_stolen_reserved(dev_priv,
> -					 &reserved_base, &reserved_size);
> +		if (IS_VALLEYVIEW(dev_priv))
> +			vlv_get_stolen_reserved(dev_priv,
> +						&reserved_base, &reserved_size);
> +		else
> +			gen7_get_stolen_reserved(dev_priv,
> +						 &reserved_base, &reserved_size);
>  		break;
>  	default:
>  		if (IS_LP(dev_priv))
> @@ -402,11 +406,16 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
>  		break;
>  	}
>  
> -	/* It is possible for the reserved base to be zero, but the register
> -	 * field for size doesn't have a zero option. */
> -	if (reserved_base == 0) {
> -		reserved_size = 0;
> +	/*
> +	 * Our expectation is that the reserved space is at the top of the
> +	 * stolen region and *never* at the bottom. If we see !reserved_base,
> +	 * it likely means we failed to read the registers correctly.
> +	 */
> +	if (!reserved_base) {
> +		DRM_ERROR("inconsistent reservation %pa + %pa; ignoring\n",
> +			  &reserved_base, &reserved_size);
>  		reserved_base = stolen_top;
> +		reserved_size = 0;
>  	}
>  
>  	dev_priv->dsm_reserved =
> -- 
> 2.16.2

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 3/3] drm/i915/stolen: Deduce base of reserved portion as top-size on vlv

[Chris Wilson]

Quoting Ville Syrjälä (2018-03-16 12:00:45)
> On Mon, Mar 12, 2018 at 04:52:06PM +0000, Chris Wilson wrote:
> > On Valleyview, the HW deduces the base of the reserved portion of stolen
> > memory as being (top - size) and the address field within
> > GEN6_STOLEN_RESERVED is set to 0. Add yet another GEN6_STOLEN_RESERVED
> > reader to cope with the subtly different path required for vlv.
> > 
> > v2: Avoid using reserved_base = reserved_size = 0 as the invalid
> > condition as that typically falls outside of the stolen region,
> > provoking a consistency error.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> 
> Didn't spot anything wrong. Series is
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thank you very much, pushed.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx