All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915/stolen: Deduce base of reserved portion as top-size on vlv
Date: Mon, 12 Mar 2018 17:46:35 +0200	[thread overview]
Message-ID: <20180312154635.GM5453@intel.com> (raw)
In-Reply-To: <20180312152913.6821-1-chris@chris-wilson.co.uk>

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

  reply	other threads:[~2018-03-12 15:46 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  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 ` [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ä [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180312154635.GM5453@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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.