Re: [PATCH 43/89] drm/i915/skl: Read the Memory Latency Values for WM computation

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

On Thu, Sep 04, 2014 at 12:27:09PM +0100, Damien Lespiau wrote:
> From: Pradeep Bhat <pradeep.bhat@intel.com>
> 
> This patch reads the memory latency values for all the 8 levels for
> SKL. These values are needed for the Watermark computation.
> 
> v2: Incorporated the review comments from Damien on register
>     indentation.
> 
> v3: Updated the code to use the sandybridge_pcode_read for reading
>     memory latencies for GEN9.
> 
> v4: Don't put gen 9 in the middle of an ordered list of ifs
>     (Damien)
> 
> v5 take the rps.hw_lock around sandybridge_pcode_read() (Damien)
> 
> Signed-off-by: Pradeep Bhat <pradeep.bhat@intel.com>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  6 ++++
>  drivers/gpu/drm/i915/i915_reg.h |  9 +++++
>  drivers/gpu/drm/i915/intel_pm.c | 73 +++++++++++++++++++++++++++++++++++++----
>  3 files changed, 82 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index dcd1c72..32be299 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1665,6 +1665,12 @@ struct drm_i915_private {
>  		uint16_t spr_latency[5];
>  		/* cursor */
>  		uint16_t cur_latency[5];
> +		/*
> +		 * Raw watermark memory latency values
> +		 * for SKL for all 8 levels
> +		 * in 1us units.
> +		 */
> +		uint16_t skl_latency[8];

Not sure if we could unify these somehow to avoid wasted space. But that
can be left as a future exercise.

>  
>  		/* current hardware state */
>  		struct ilk_wm_values hw;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0159f2d..bc55990 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2018,6 +2018,15 @@ enum punit_power_well {
>  #define   MAD_DIMM_A_SIZE_SHIFT		0
>  #define   MAD_DIMM_A_SIZE_MASK		(0xff << MAD_DIMM_A_SIZE_SHIFT)
>  
> +/* SKL GT Driver Mailbox registers for reading memory latencies */
> +#define GEN9_MAILBOX_DATA1		0x13812C
> +#define   GEN9_MAILBOX_READ_MEM_LAT	(0x6)
> +#define   GEN9_MAILBOX_READ_TIMEOUT	150

Timeout not used anywhere. Also spec says 100us.

> +#define   GEN9_MEM_LAT_LEVEL_MASK	0xFF
> +#define   GEN9_MEM_LAT_LEVEL_1_5_SHIFT	8
> +#define   GEN9_MEM_LAT_LEVEL_2_6_SHIFT	16
> +#define   GEN9_MEM_LAT_LEVEL_3_7_SHIFT	24

This stuff should be grouped along the other pcode register defines.

Also according to Bspec the mailbox data1 register already existed since
snb. The hsw cdclk change sequence also mentions that it should be set
to 0, but eg. the bdw IPS sequence doesn't mention it. I guess in theory
some pcode command might cause it to be clobbered, so I'm thinking we
should just explicitly set it to 0 for all platforms in the pcode
read/write functions. That should be a separate patch though.

> +
>  /* snb MCH registers for priority tuning */
>  #define MCH_SSKPD			(MCHBAR_MIRROR_BASE_SNB + 0x5d10)
>  #define   MCH_SSKPD_WM0_MASK		0x3f
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a236e77..d8c8531 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2239,11 +2239,53 @@ hsw_compute_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
>  	       PIPE_WM_LINETIME_TIME(linetime);
>  }
>  
> -static void intel_read_wm_latency(struct drm_device *dev, uint16_t wm[5])
> +static void intel_read_wm_latency(struct drm_device *dev, uint16_t wm[8])
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> +	if (IS_GEN9(dev)) {
> +		uint32_t val;
> +		int ret;
> +
> +		/* read the first set of memory latencies[0:3] */
> +		val = 0; /* data0 to be programmed to 0 for first set */
> +		mutex_lock(&dev_priv->rps.hw_lock);
> +		ret = sandybridge_pcode_read(dev_priv,
> +					     GEN9_MAILBOX_READ_MEM_LAT,
> +					     &val);
> +		mutex_unlock(&dev_priv->rps.hw_lock);
> +
> +		if (ret) {
> +			DRM_ERROR("SKL Mailbox read error = %d\n", ret);
> +			return;
> +		}
> +		wm[0] = val & GEN9_MEM_LAT_LEVEL_MASK;
> +		wm[1] = (val >> GEN9_MEM_LAT_LEVEL_1_5_SHIFT) &
> +				GEN9_MEM_LAT_LEVEL_MASK;
> +		wm[2] = (val >> GEN9_MEM_LAT_LEVEL_2_6_SHIFT) &
> +				GEN9_MEM_LAT_LEVEL_MASK;
> +		wm[3] = (val >> GEN9_MEM_LAT_LEVEL_3_7_SHIFT) &
> +				GEN9_MEM_LAT_LEVEL_MASK;
> +
> +		/* read the second set of memory latencies[4:7] */
> +		val = 1; /* data0 to be programmed to 1 for second set */
> +		mutex_lock(&dev_priv->rps.hw_lock);
> +		ret = sandybridge_pcode_read(dev_priv,
> +					     GEN9_MAILBOX_READ_MEM_LAT,
> +					     &val);
> +		mutex_unlock(&dev_priv->rps.hw_lock);
> +		if (ret) {
> +			DRM_ERROR("SKL Mailbox read error = %d\n", ret);
> +			return;
> +		}
> +		wm[4] = val & GEN9_MEM_LAT_LEVEL_MASK;
> +		wm[5] = (val >> GEN9_MEM_LAT_LEVEL_1_5_SHIFT) &
> +				GEN9_MEM_LAT_LEVEL_MASK;
> +		wm[6] = (val >> GEN9_MEM_LAT_LEVEL_2_6_SHIFT) &
> +				GEN9_MEM_LAT_LEVEL_MASK;
> +		wm[7] = (val >> GEN9_MEM_LAT_LEVEL_3_7_SHIFT) &
> +				GEN9_MEM_LAT_LEVEL_MASK;
> +	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
>  		uint64_t sskpd = I915_READ64(MCH_SSKPD);
>  
>  		wm[0] = (sskpd >> 56) & 0xFF;
> @@ -2291,7 +2333,9 @@ static void intel_fixup_cur_wm_latency(struct drm_device *dev, uint16_t wm[5])
>  int ilk_wm_max_level(const struct drm_device *dev)
>  {
>  	/* how many WM levels are we expecting */
> -	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> +	if (IS_GEN9(dev))
> +		return 7;
> +	else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
>  		return 4;
>  	else if (INTEL_INFO(dev)->gen >= 6)
>  		return 3;
> @@ -2300,7 +2344,7 @@ int ilk_wm_max_level(const struct drm_device *dev)
>  }
>  static void intel_print_wm_latency(struct drm_device *dev,
>  				   const char *name,
> -				   const uint16_t wm[5])
> +				   const uint16_t wm[8])
>  {
>  	int level, max_level = ilk_wm_max_level(dev);
>  
> @@ -2313,8 +2357,13 @@ static void intel_print_wm_latency(struct drm_device *dev,
>  			continue;
>  		}
>  
> -		/* WM1+ latency values in 0.5us units */
> -		if (level > 0)
> +		/*
> +		 * - latencies are in us on gen9.
> +		 * - before then, WM1+ latency values are in 0.5us units
> +		 */
> +		if (IS_GEN9(dev))
> +			latency *= 10;
> +		else if (level > 0)
>  			latency *= 5;
>  
>  		DRM_DEBUG_KMS("%s WM%d latency %u (%u.%u usec)\n",
> @@ -2382,6 +2431,14 @@ static void ilk_setup_wm_latency(struct drm_device *dev)
>  		snb_wm_latency_quirk(dev);
>  }
>  
> +static void skl_setup_wm_latency(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	intel_read_wm_latency(dev, dev_priv->wm.skl_latency);
> +	intel_print_wm_latency(dev, "Gen9 Plane", dev_priv->wm.skl_latency);
> +}
> +
>  static void ilk_compute_wm_parameters(struct drm_crtc *crtc,
>  				      struct ilk_pipe_wm_parameters *p)
>  {
> @@ -7396,6 +7453,8 @@ void intel_init_pm(struct drm_device *dev)
>  
>  	/* For FIFO watermark updates */
>  	if (IS_GEN9(dev)) {
> +		skl_setup_wm_latency(dev);
> +
>  		dev_priv->display.init_clock_gating = gen9_init_clock_gating;
>  	} else if (HAS_PCH_SPLIT(dev)) {
>  		ilk_setup_wm_latency(dev);
> @@ -7488,6 +7547,8 @@ int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u8 mbox, u32 *val)
>  	}
>  
>  	I915_WRITE(GEN6_PCODE_DATA, *val);
> +	if (IS_GEN9(dev_priv->dev))
> +		I915_WRITE(GEN9_MAILBOX_DATA1, 0);
>  	I915_WRITE(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY | mbox);
>  
>  	if (wait_for((I915_READ(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) == 0,
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC