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

2018-03-12 Thread Ville Syrjälä 
On Mon, Mar 12, 2018 at 03:29:13PM +, 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 
> Cc: Ville Syrjälä 
> Cc: Imre Deak 
> ---
> 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ä 

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

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

2018-03-12 Thread 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.

Signed-off-by: Chris Wilson 
Cc: Ville Syrjälä 
Cc: Imre Deak 
---
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