Re: [PATCH] drm/i915: Reserve powerctx for chv from the stolen allocator

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Sat, Nov 04, 2017 at 09:43:38PM +0000, Chris Wilson wrote:
> Ensure that we do not overwrite the cherryview power context by
> reserving its range in the stolen allocator; exactly like how we handle
> the same reservation for valleyview.

IIRC CHV pctx must live inside the "reserved" region. So this
should never happen.

> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  3 +-
>  drivers/gpu/drm/i915/intel_pm.c | 97 +++++++++++++++--------------------------
>  2 files changed, 37 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 72bb5b51035a..d6462388c3a6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1377,6 +1377,7 @@ struct intel_rps {
>  };
>  
>  struct intel_rc6 {
> +	struct drm_i915_gem_object *pctx;
>  	bool enabled;
>  };
>  
> @@ -2464,8 +2465,6 @@ struct drm_i915_private {
>  
>  	struct i915_gpu_error gpu_error;
>  
> -	struct drm_i915_gem_object *vlv_pctx;
> -
>  	/* list of fbdev register on this device */
>  	struct intel_fbdev *fbdev;
>  	struct work_struct fbdev_suspend_work;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 07118c0b69d3..1a0cf53e0638 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -7034,7 +7034,7 @@ static void valleyview_check_pctx(struct drm_i915_private *dev_priv)
>  	unsigned long pctx_addr = I915_READ(VLV_PCBR) & ~4095;
>  
>  	WARN_ON(pctx_addr != dev_priv->mm.stolen_base +
> -			     dev_priv->vlv_pctx->stolen->start);
> +			     dev_priv->gt_pm.rc6.pctx->stolen->start);
>  }
>  
>  
> @@ -7046,77 +7046,54 @@ static void cherryview_check_pctx(struct drm_i915_private *dev_priv)
>  	WARN_ON((pctx_addr >> VLV_PCBR_ADDR_SHIFT) == 0);
>  }
>  
> -static void cherryview_setup_pctx(struct drm_i915_private *dev_priv)
> -{
> -	struct i915_ggtt *ggtt = &dev_priv->ggtt;
> -	unsigned long pctx_paddr, paddr;
> -	u32 pcbr;
> -	int pctx_size = 32*1024;
> -
> -	pcbr = I915_READ(VLV_PCBR);
> -	if ((pcbr >> VLV_PCBR_ADDR_SHIFT) == 0) {
> -		DRM_DEBUG_DRIVER("BIOS didn't set up PCBR, fixing up\n");
> -		paddr = (dev_priv->mm.stolen_base +
> -			 (ggtt->stolen_size - pctx_size));
> -
> -		pctx_paddr = (paddr & (~4095));
> -		I915_WRITE(VLV_PCBR, pctx_paddr);
> -	}
> -
> -	DRM_DEBUG_DRIVER("PCBR: 0x%08x\n", I915_READ(VLV_PCBR));
> -}
> -
> -static void valleyview_setup_pctx(struct drm_i915_private *dev_priv)
> +static void setup_pctx(struct drm_i915_private *dev_priv, int pctx_size)
>  {
>  	struct drm_i915_gem_object *pctx;
> -	unsigned long pctx_paddr;
>  	u32 pcbr;
> -	int pctx_size = 24*1024;
>  
>  	pcbr = I915_READ(VLV_PCBR);
>  	if (pcbr) {
>  		/* BIOS set it up already, grab the pre-alloc'd space */
> -		int pcbr_offset;
> +		u32 start = round_down(pcbr, 4096);
> +		u32 end = round_up(pcbr + pctx_size, 4096);
>  
> -		pcbr_offset = (pcbr & (~4095)) - dev_priv->mm.stolen_base;
>  		pctx = i915_gem_object_create_stolen_for_preallocated(dev_priv,
> -								      pcbr_offset,
> +								      start - dev_priv->mm.stolen_base,
>  								      I915_GTT_OFFSET_NONE,
> -								      pctx_size);
> -		goto out;
> -	}
> +								      end - start);
> +	} else {
> +		DRM_DEBUG_DRIVER("BIOS didn't set up PCBR, fixing up\n");
>  
> -	DRM_DEBUG_DRIVER("BIOS didn't set up PCBR, fixing up\n");
> +		/*
> +		 * From the Gunit register HAS:
> +		 * The Gfx driver is expected to program this register and
> +		 * ensure proper allocation within Gfx stolen memory.  For
> +		 * example, this register should be programmed such than the
> +		 * PCBR range does not overlap with other ranges, such as the
> +		 * frame buffer, protected memory, or any other relevant ranges.
> +		 */
> +		pctx = i915_gem_object_create_stolen(dev_priv, pctx_size);
> +		if (!pctx) {
> +			DRM_DEBUG("not enough stolen space for PCTX, disabling\n");
> +			return;
> +		}
>  
> -	/*
> -	 * From the Gunit register HAS:
> -	 * The Gfx driver is expected to program this register and ensure
> -	 * proper allocation within Gfx stolen memory.  For example, this
> -	 * register should be programmed such than the PCBR range does not
> -	 * overlap with other ranges, such as the frame buffer, protected
> -	 * memory, or any other relevant ranges.
> -	 */
> -	pctx = i915_gem_object_create_stolen(dev_priv, pctx_size);
> -	if (!pctx) {
> -		DRM_DEBUG("not enough stolen space for PCTX, disabling\n");
> -		goto out;
> +		I915_WRITE(VLV_PCBR,
> +			   dev_priv->mm.stolen_base + pctx->stolen->start);
>  	}
> +	DRM_DEBUG_DRIVER("PCBR: 0x%08x [0x%08x]\n", I915_READ(VLV_PCBR), pcbr);
>  
> -	pctx_paddr = dev_priv->mm.stolen_base + pctx->stolen->start;
> -	I915_WRITE(VLV_PCBR, pctx_paddr);
> -
> -out:
> -	DRM_DEBUG_DRIVER("PCBR: 0x%08x\n", I915_READ(VLV_PCBR));
> -	dev_priv->vlv_pctx = pctx;
> +	dev_priv->gt_pm.rc6.pctx = pctx;
>  }
>  
> -static void valleyview_cleanup_pctx(struct drm_i915_private *dev_priv)
> +static void cherryview_setup_pctx(struct drm_i915_private *dev_priv)
>  {
> -	if (WARN_ON(!dev_priv->vlv_pctx))
> -		return;
> +	setup_pctx(dev_priv, 32 << 10);
> +}
>  
> -	i915_gem_object_put(dev_priv->vlv_pctx);
> -	dev_priv->vlv_pctx = NULL;
> +static void valleyview_setup_pctx(struct drm_i915_private *dev_priv)
> +{
> +	setup_pctx(dev_priv, 24 << 10);
>  }
>  
>  static void vlv_init_gpll_ref_freq(struct drm_i915_private *dev_priv)
> @@ -7225,11 +7202,6 @@ static void cherryview_init_gt_powersave(struct drm_i915_private *dev_priv)
>  		  "Odd GPU freq values\n");
>  }
>  
> -static void valleyview_cleanup_gt_powersave(struct drm_i915_private *dev_priv)
> -{
> -	valleyview_cleanup_pctx(dev_priv);
> -}
> -
>  static void cherryview_enable_rc6(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_engine_cs *engine;
> @@ -7949,8 +7921,11 @@ void intel_init_gt_powersave(struct drm_i915_private *dev_priv)
>  
>  void intel_cleanup_gt_powersave(struct drm_i915_private *dev_priv)
>  {
> -	if (IS_VALLEYVIEW(dev_priv))
> -		valleyview_cleanup_gt_powersave(dev_priv);
> +	struct drm_i915_gem_object *pctx;
> +
> +	pctx = fetch_and_zero(&dev_priv->gt_pm.rc6.pctx);
> +	if (pctx)
> +		i915_gem_object_put(pctx);
>  
>  	if (!i915_modparams.enable_rc6)
>  		intel_runtime_pm_put(dev_priv);
> -- 
> 2.15.0
> 
> _______________________________________________
> 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