Re: [Intel-gfx] [PATCH v2 3/3] drm/i915/stolen: Deduce base of reserved portion as top-size on vlv
Quoting Ville Syrjälä (2018-03-16 12:00:45) > On Mon, Mar 12, 2018 at 04:52:06PM +, 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 > > Cc: Ville Syrjälä > > Cc: Imre Deak > > Didn't spot anything wrong. Series is > Reviewed-by: Ville Syrjälä Thank you very much, pushed. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 3/3] drm/i915/stolen: Deduce base of reserved portion as top-size on vlv
On Mon, Mar 12, 2018 at 04:52:06PM +, 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
> Cc: Ville Syrjälä
> Cc: Imre Deak
Didn't spot anything wrong. Series is
Reviewed-by: Ville Syrjälä
> ---
> 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))
> ret
[Intel-gfx] [PATCH v2 3/3] drm/i915/stolen: Deduce base of reserved portion as top-size on vlv
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
Cc: Ville Syrjälä
Cc: Imre Deak
---
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 = I9
