All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER
@ 2018-03-12 12:55 Chris Wilson
  2018-03-12 12:55 ` [PATCH 2/2] drm/i915/stolen: Intepret reserved_base=0 as deduce from top Chris Wilson
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Chris Wilson @ 2018-03-12 12:55 UTC (permalink / raw)
  To: intel-gfx

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>
---
 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] drm/i915/stolen: Intepret reserved_base=0 as deduce from top
  2018-03-12 12:55 [PATCH 1/2] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER Chris Wilson
@ 2018-03-12 12:55 ` Chris Wilson
  2018-03-12 14:55   ` Ville Syrjälä
  2018-03-12 14:25 ` [PATCH 1/2] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER Ville Syrjälä
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2018-03-12 12:55 UTC (permalink / raw)
  To: intel-gfx

On my j1900 Valleyview, and on both of CI's, if I allocate above the power
context (as allotted by PCBR) then the objects are not accessed. For
example, if it is a ringbuffer, then the commands are not executed,
causing a GPU hang with no breadcrumb advancement. On inspection,
GEN6_STOLEN_RESERVED is enabled, but has base==0 which we interpret a
being invalid. However, it fits neatly in after the powercontext if we
interpret base==0 as meaning allocate from top instead, the GPU doesn't
hang on boot!

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

diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index b04e2551bae6..1bd542977e4c 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -386,12 +386,13 @@ 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;
-		reserved_base = stolen_top;
-	}
+	/*
+	 * It is possible for the reserved base to be zero, but the register
+	 * field for size doesn't have a zero option. Experience says they
+	 * mean allocate from top.
+	 */
+	if (reserved_base == 0)
+		reserved_base = stolen_top - reserved_size;
 
 	dev_priv->dsm_reserved =
 		(struct resource) DEFINE_RES_MEM(reserved_base, reserved_size);
-- 
2.16.2

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER
  2018-03-12 12:55 [PATCH 1/2] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER Chris Wilson
  2018-03-12 12:55 ` [PATCH 2/2] drm/i915/stolen: Intepret reserved_base=0 as deduce from top Chris Wilson
@ 2018-03-12 14:25 ` Ville Syrjälä
  2018-03-12 14:30 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Ville Syrjälä @ 2018-03-12 14:25 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Mon, Mar 12, 2018 at 12:55:41PM +0000, Chris Wilson wrote:
> 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

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER
  2018-03-12 12:55 [PATCH 1/2] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER Chris Wilson
  2018-03-12 12:55 ` [PATCH 2/2] drm/i915/stolen: Intepret reserved_base=0 as deduce from top Chris Wilson
  2018-03-12 14:25 ` [PATCH 1/2] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER Ville Syrjälä
@ 2018-03-12 14:30 ` Patchwork
  2018-03-12 16:13 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER (rev2) Patchwork
  2018-03-12 17:18 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER (rev3) Patchwork
  4 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2018-03-12 14:30 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

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

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:430s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:437s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:384s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:532s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:297s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:508s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:508s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:509s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:495s
fi-cfl-8700k     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:411s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:587s
fi-cnl-y3        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:591s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:420s
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:535s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:405s
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:474s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:428s
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:470s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:514s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:644s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:437s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:528s
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:507s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:493s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:429s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:434s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:540s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:399s
Blacklisted hosts:
fi-cnl-drrs      total:288  pass:0    dwarn:0   dfail:0   fail:0   skip:0   time:3s

1bf8f00fed0b78f5d286304deb1f1526b10aeaca drm-tip: 2018y-03m-12d-11h-28m-33s UTC integration manifest
a2bbfffb687e drm/i915/stolen: Intepret reserved_base=0 as deduce from top
1bc2ff6650bd 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_8303/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] drm/i915/stolen: Intepret reserved_base=0 as deduce from top
  2018-03-12 12:55 ` [PATCH 2/2] drm/i915/stolen: Intepret reserved_base=0 as deduce from top Chris Wilson
@ 2018-03-12 14:55   ` Ville Syrjälä
  2018-03-12 15:29     ` [PATCH] drm/i915/stolen: Deduce base of reserved portion as top-size on vlv Chris Wilson
  0 siblings, 1 reply; 10+ messages in thread
From: Ville Syrjälä @ 2018-03-12 14:55 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Mon, Mar 12, 2018 at 12:55:42PM +0000, Chris Wilson wrote:
> On my j1900 Valleyview, and on both of CI's, if I allocate above the power
> context (as allotted by PCBR) then the objects are not accessed. For
> example, if it is a ringbuffer, then the commands are not executed,
> causing a GPU hang with no breadcrumb advancement. On inspection,
> GEN6_STOLEN_RESERVED is enabled, but has base==0 which we interpret a
> being invalid. However, it fits neatly in after the powercontext if we
> interpret base==0 as meaning allocate from top instead, the GPU doesn't
> hang on boot!
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index b04e2551bae6..1bd542977e4c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -386,12 +386,13 @@ 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;
> -		reserved_base = stolen_top;
> -	}
> +	/*
> +	 * It is possible for the reserved base to be zero, but the register
> +	 * field for size doesn't have a zero option. Experience says they
> +	 * mean allocate from top.
> +	 */
> +	if (reserved_base == 0)
> +		reserved_base = stolen_top - reserved_size;

I think on VLV and CHV the base is not used by the hardware. The docs
list the field as reserved, and there's a note that the hw just derives
the value as (top_of_stolen - size).

Although it does look like some BIOSen do still program the base.
My BSW has it set, but neither of my two VLVs have it.

So maybe we want to put this logic into the vlv/chv specific functions,
and maybe have the common code WARN if we get inconsistent values out?

>  
>  	dev_priv->dsm_reserved =
>  		(struct resource) DEFINE_RES_MEM(reserved_base, reserved_size);
> -- 
> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] drm/i915/stolen: Deduce base of reserved portion as top-size on vlv
  2018-03-12 14:55   ` Ville Syrjälä
@ 2018-03-12 15:29     ` Chris Wilson
  2018-03-12 15:46       ` Ville Syrjälä
  2018-03-12 16:34       ` [PATCH v2] " Chris Wilson
  0 siblings, 2 replies; 10+ messages in thread
From: Chris Wilson @ 2018-03-12 15:29 UTC (permalink / raw)
  To: intel-gfx

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.

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>
---
I left the suggestion to modify chv as we already have a custom function
for Braswell and so far it appears to be happy. If it ain't broke...
-Chris
---
 drivers/gpu/drm/i915/i915_gem_stolen.c | 91 +++++++++++++++++++++++++++-------
 1 file changed, 74 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index b04e2551bae6..5c361a7c3b83 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 = %x\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 = %x\n", reg_val);
 
 	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
 		*base = 0;
@@ -239,10 +246,46 @@ 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);
+
+	DRM_DEBUG_DRIVER("GEN6_STOLEN_RESERVED = %x\n", reg_val);
+
+	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
+		*base = 0;
+		*size = 0;
+		return;
+	}
+
+	switch (reg_val & GEN7_STOLEN_RESERVED_SIZE_MASK) {
+	case GEN7_STOLEN_RESERVED_1M:
+		*size = 1024 * 1024;
+		break;
+	case GEN7_STOLEN_RESERVED_256K:
+		*size = 256 * 1024;
+		break;
+	default:
+		*size = 1024 * 1024;
+		MISSING_CASE(reg_val & GEN7_STOLEN_RESERVED_SIZE_MASK);
+	}
+
+	/*
+	 * On vlv, the ADDR_MASK portion is left as 0 and HW deduces the
+	 * reserved location as (top - size).
+	 */
+	*base = dev_priv->dsm.end + 1 - *size;
+}
+
 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 = %x\n", reg_val);
 
 	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
 		*base = 0;
@@ -266,9 +309,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 = %x\n", reg_val);
 
 	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
 		*base = 0;
@@ -298,11 +344,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 = %x\n", reg_val);
+
 	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
 		*base = 0;
 		*size = 0;
@@ -373,8 +422,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))
@@ -386,9 +439,13 @@ 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) {
+	/*
+	 * Our expectation is that the reserved space is at the top of the
+	 * stolen region and *never* at the bottom.
+	 */
+	if (reserved_size && !reserved_base) {
+		DRM_ERROR("inconsistent reservation %pa + %pa; ignoring\n",
+			  &reserved_base, &reserved_size);
 		reserved_size = 0;
 		reserved_base = stolen_top;
 	}
-- 
2.16.2

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915/stolen: Deduce base of reserved portion as top-size on vlv
  2018-03-12 15:29     ` [PATCH] drm/i915/stolen: Deduce base of reserved portion as top-size on vlv Chris Wilson
@ 2018-03-12 15:46       ` Ville Syrjälä
  2018-03-12 16:34       ` [PATCH v2] " Chris Wilson
  1 sibling, 0 replies; 10+ messages in thread
From: Ville Syrjälä @ 2018-03-12 15:46 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Mon, Mar 12, 2018 at 03:29:13PM +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.
> 
> 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>
> ---
> I left the suggestion to modify chv as we already have a custom function
> for Braswell and so far it appears to be happy. If it ain't broke...
> -Chris
> ---
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 91 +++++++++++++++++++++++++++-------
>  1 file changed, 74 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index b04e2551bae6..5c361a7c3b83 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 = %x\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 = %x\n", reg_val);
>  
>  	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
>  		*base = 0;
> @@ -239,10 +246,46 @@ 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);
> +
> +	DRM_DEBUG_DRIVER("GEN6_STOLEN_RESERVED = %x\n", reg_val);
> +
> +	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
> +		*base = 0;
> +		*size = 0;
> +		return;
> +	}
> +
> +	switch (reg_val & GEN7_STOLEN_RESERVED_SIZE_MASK) {
> +	case GEN7_STOLEN_RESERVED_1M:
> +		*size = 1024 * 1024;
> +		break;
> +	case GEN7_STOLEN_RESERVED_256K:
> +		*size = 256 * 1024;
> +		break;

I believe 1MiB is the only legal size on VLV. So we might want to leave
out the 256KiB case here.

Either way
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +	default:
> +		*size = 1024 * 1024;
> +		MISSING_CASE(reg_val & GEN7_STOLEN_RESERVED_SIZE_MASK);
> +	}
> +
> +	/*
> +	 * On vlv, the ADDR_MASK portion is left as 0 and HW deduces the
> +	 * reserved location as (top - size).
> +	 */
> +	*base = dev_priv->dsm.end + 1 - *size;
> +}
> +
>  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 = %x\n", reg_val);
>  
>  	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
>  		*base = 0;
> @@ -266,9 +309,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 = %x\n", reg_val);
>  
>  	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
>  		*base = 0;
> @@ -298,11 +344,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 = %x\n", reg_val);
> +
>  	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
>  		*base = 0;
>  		*size = 0;
> @@ -373,8 +422,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))
> @@ -386,9 +439,13 @@ 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) {
> +	/*
> +	 * Our expectation is that the reserved space is at the top of the
> +	 * stolen region and *never* at the bottom.
> +	 */
> +	if (reserved_size && !reserved_base) {
> +		DRM_ERROR("inconsistent reservation %pa + %pa; ignoring\n",
> +			  &reserved_base, &reserved_size);
>  		reserved_size = 0;
>  		reserved_base = stolen_top;
>  	}
> -- 
> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER (rev2)
  2018-03-12 12:55 [PATCH 1/2] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER Chris Wilson
                   ` (2 preceding siblings ...)
  2018-03-12 14:30 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
@ 2018-03-12 16:13 ` Patchwork
  2018-03-12 17:18 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER (rev3) Patchwork
  4 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2018-03-12 16:13 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER (rev2)
URL   : https://patchwork.freedesktop.org/series/39787/
State : warning

== Summary ==

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

---- Possible new issues:

Test drv_module_reload:
        Subgroup basic-no-display:
                pass       -> DMESG-WARN (fi-blb-e6850)
                pass       -> DMESG-WARN (fi-gdg-551)
                pass       -> DMESG-WARN (fi-ilk-650)
                pass       -> DMESG-WARN (fi-pnv-d510)
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup basic-reload:
                pass       -> DMESG-WARN (fi-blb-e6850)
                pass       -> DMESG-WARN (fi-bwr-2160)
                pass       -> DMESG-WARN (fi-elk-e7500)
                pass       -> DMESG-WARN (fi-gdg-551)
                pass       -> DMESG-WARN (fi-ilk-650)
                pass       -> DMESG-WARN (fi-pnv-d510)
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup basic-reload-inject:
                pass       -> DMESG-WARN (fi-blb-e6850)
                pass       -> DMESG-WARN (fi-elk-e7500)
                pass       -> DMESG-WARN (fi-gdg-551)
                pass       -> DMESG-WARN (fi-ilk-650)
                pass       -> DMESG-WARN (fi-pnv-d510)
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)

---- Known issues:

Test drv_module_reload:
        Subgroup basic-no-display:
                pass       -> DMESG-WARN (fi-bwr-2160) fdo#105268 +1
                pass       -> DMESG-WARN (fi-elk-e7500) fdo#105074
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> INCOMPLETE (fi-cnl-y3) fdo#104951
        Subgroup suspend-read-crc-pipe-c:
                pass       -> INCOMPLETE (fi-skl-6700k2) fdo#104108

fdo#105268 https://bugs.freedesktop.org/show_bug.cgi?id=105268
fdo#105074 https://bugs.freedesktop.org/show_bug.cgi?id=105074
fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951
fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:431s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:433s
fi-blb-e6850     total:288  pass:220  dwarn:4   dfail:0   fail:0   skip:64  time:382s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:534s
fi-bwr-2160      total:288  pass:180  dwarn:3   dfail:0   fail:0   skip:105 time:298s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:506s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:508s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:506s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:495s
fi-cfl-8700k     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:411s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:582s
fi-cnl-y3        total:245  pass:220  dwarn:0   dfail:0   fail:0   skip:24 
fi-elk-e7500     total:288  pass:226  dwarn:3   dfail:0   fail:0   skip:59  time:427s
fi-gdg-551       total:288  pass:176  dwarn:3   dfail:0   fail:1   skip:108 time:312s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:531s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:402s
fi-ilk-650       total:288  pass:225  dwarn:3   dfail:0   fail:0   skip:60  time:418s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:466s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:429s
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:470s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:515s
fi-pnv-d510      total:288  pass:219  dwarn:4   dfail:0   fail:0   skip:65  time:643s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:452s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:529s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:542s
fi-skl-6700k2    total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:490s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:429s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:437s
fi-snb-2520m     total:288  pass:245  dwarn:3   dfail:0   fail:0   skip:40  time:539s
fi-snb-2600      total:288  pass:245  dwarn:3   dfail:0   fail:0   skip:40  time:399s
Blacklisted hosts:
fi-cnl-drrs      total:288  pass:257  dwarn:3   dfail:0   fail:0   skip:28  time:527s
fi-glk-j5005 failed to collect. IGT log at Patchwork_8308/fi-glk-j5005/run0.log

1bf8f00fed0b78f5d286304deb1f1526b10aeaca drm-tip: 2018y-03m-12d-11h-28m-33s UTC integration manifest
6c8d4e28691f drm/i915/stolen: Deduce base of reserved portion as top-size on vlv
f55101aa7527 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_8308/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2] drm/i915/stolen: Deduce base of reserved portion as top-size on vlv
  2018-03-12 15:29     ` [PATCH] drm/i915/stolen: Deduce base of reserved portion as top-size on vlv Chris Wilson
  2018-03-12 15:46       ` Ville Syrjälä
@ 2018-03-12 16:34       ` Chris Wilson
  1 sibling, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2018-03-12 16:34 UTC (permalink / raw)
  To: intel-gfx

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 | 143 +++++++++++++++++++--------------
 1 file changed, 84 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index b04e2551bae6..f9972d11a66c 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -174,18 +174,19 @@ 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;
 
-	if ((reg_val & G4X_STOLEN_RESERVED_ENABLE) == 0) {
-		*base = 0;
-		*size = 0;
+	DRM_DEBUG_DRIVER("%s_STOLEN_RESERVED = %x\n",
+			 IS_GM45(dev_priv) ? "CTG" : "ELK", reg_val);
+
+	if ((reg_val & G4X_STOLEN_RESERVED_ENABLE) == 0)
 		return;
-	}
 
 	/*
 	 * Whether ILK really reuses the ELK register for this is unclear.
@@ -193,30 +194,25 @@ 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,
-				     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 = %x\n", reg_val);
 
-	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
-		*base = 0;
-		*size = 0;
+	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0)
 		return;
-	}
 
 	*base = reg_val & GEN6_STOLEN_RESERVED_ADDR_MASK;
 
@@ -239,17 +235,44 @@ 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)
+static void vlv_get_stolen_reserved(struct drm_i915_private *dev_priv,
+				    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 = dev_priv->dsm.end + 1;
 
-	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
-		*base = 0;
-		*size = 0;
+	DRM_DEBUG_DRIVER("GEN6_STOLEN_RESERVED = %x\n", reg_val);
+
+	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0)
 		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)
+{
+	u32 reg_val = I915_READ(GEN6_STOLEN_RESERVED);
+
+	DRM_DEBUG_DRIVER("GEN6_STOLEN_RESERVED = %x\n", reg_val);
+
+	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0)
+		return;
+
 	*base = reg_val & GEN7_STOLEN_RESERVED_ADDR_MASK;
 
 	switch (reg_val & GEN7_STOLEN_RESERVED_SIZE_MASK) {
@@ -266,15 +289,15 @@ 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);
 
-	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
-		*base = 0;
-		*size = 0;
+	DRM_DEBUG_DRIVER("GEN6_STOLEN_RESERVED = %x\n", reg_val);
+
+	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0)
 		return;
-	}
 
 	*base = reg_val & GEN6_STOLEN_RESERVED_ADDR_MASK;
 
@@ -298,29 +321,22 @@ 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);
-	resource_size_t stolen_top;
+	u32 reg_val = I915_READ(GEN6_STOLEN_RESERVED);
+	resource_size_t stolen_top = dev_priv->dsm.end + 1;
 
-	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0) {
-		*base = 0;
-		*size = 0;
+	DRM_DEBUG_DRIVER("GEN6_STOLEN_RESERVED = %x\n", reg_val);
+
+	if ((reg_val & GEN6_STOLEN_RESERVED_ENABLE) == 0)
 		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)
@@ -353,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)) {
@@ -373,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))
@@ -386,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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER (rev3)
  2018-03-12 12:55 [PATCH 1/2] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER Chris Wilson
                   ` (3 preceding siblings ...)
  2018-03-12 16:13 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER (rev2) Patchwork
@ 2018-03-12 17:18 ` Patchwork
  4 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2018-03-12 17:18 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER (rev3)
URL   : https://patchwork.freedesktop.org/series/39787/
State : success

== Summary ==

Series 39787v3 series starting with [1/2] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER
https://patchwork.freedesktop.org/api/1.0/series/39787/revisions/3/mbox/

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:433s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:437s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:384s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:538s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:297s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:513s
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:510s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:500s
fi-cfl-8700k     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:413s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:581s
fi-cnl-y3        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:598s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:427s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:318s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:534s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:402s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:420s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:460s
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:477s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:472s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:515s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:644s
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:529s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:549s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:500s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:501s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:428s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:437s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:547s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:401s
Blacklisted hosts:
fi-cnl-drrs      total:288  pass:257  dwarn:3   dfail:0   fail:0   skip:28  time:523s
fi-glk-j5005 failed to collect. IGT log at Patchwork_8310/fi-glk-j5005/run0.log

1bf8f00fed0b78f5d286304deb1f1526b10aeaca drm-tip: 2018y-03m-12d-11h-28m-33s UTC integration manifest
8b699d05a83c drm/i915/stolen: Deduce base of reserved portion as top-size on vlv
5cc226327690 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_8310/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-03-12 17:18 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-12 12:55 [PATCH 1/2] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER Chris Wilson
2018-03-12 12:55 ` [PATCH 2/2] drm/i915/stolen: Intepret reserved_base=0 as deduce from top Chris Wilson
2018-03-12 14:55   ` Ville Syrjälä
2018-03-12 15:29     ` [PATCH] drm/i915/stolen: Deduce base of reserved portion as top-size on vlv Chris Wilson
2018-03-12 15:46       ` Ville Syrjälä
2018-03-12 16:34       ` [PATCH v2] " Chris Wilson
2018-03-12 14:25 ` [PATCH 1/2] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER Ville Syrjälä
2018-03-12 14:30 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
2018-03-12 16:13 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER (rev2) Patchwork
2018-03-12 17:18 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/stolen: Switch from DEBUG_KMS to DEBUG_DRIVER (rev3) Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.