[Intel-gfx] [PATCH 48/62] drm/i915/bdw: Add Broadwell display FIFO limits

[Jani Nikula]

On Sun, 03 Nov 2013, Ben Widawsky <benjamin.widawsky at intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> Broadwell has bigger display FIFOs than Haswell. Otherwise the
> two are very similar.
>
> v2: Fix FBC WM_LP shift for BDW
>
> v3: Rebase on top of the big Haswell wm rework.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com> (v2)
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_reg.h |  1 +
>  drivers/gpu/drm/i915/intel_pm.c | 33 ++++++++++++++++++++++++---------
>  2 files changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 6f834b3..2a65f92 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3366,6 +3366,7 @@
>  #define  WM1_LP_LATENCY_MASK	(0x7f<<24)
>  #define  WM1_LP_FBC_MASK	(0xf<<20)
>  #define  WM1_LP_FBC_SHIFT	20
> +#define  WM1_LP_FBC_SHIFT_BDW	19
>  #define  WM1_LP_SR_MASK		(0x7ff<<8)

Meh, the above _MASKs would need some love too. WM1_LP_SR_MASK is wrong
for HSW already I think. But nobody's using them, so not a blocker for
this patch.

>  #define  WM1_LP_SR_SHIFT	8
>  #define  WM1_LP_CURSOR_MASK	(0xff)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 68dc363..6d14182 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2291,7 +2291,9 @@ static uint32_t ilk_compute_fbc_wm(const struct hsw_pipe_wm_parameters *params,
>  
>  static unsigned int ilk_display_fifo_size(const struct drm_device *dev)
>  {
> -	if (INTEL_INFO(dev)->gen >= 7)
> +	if (INTEL_INFO(dev)->gen >= 8)
> +		return 3072;

It's probably just me, but I couldn't find this in the spec, so can't
verify. Apart from that,

Reviewed-by: Jani Nikula <jani.nikula at intel.com>

...but read on, some non-blocking bikeshedding below.

> +	else if (INTEL_INFO(dev)->gen >= 7)
>  		return 768;
>  	else
>  		return 512;
> @@ -2336,7 +2338,9 @@ static unsigned int ilk_plane_wm_max(const struct drm_device *dev,
>  	}
>  
>  	/* clamp to max that the registers can hold */
> -	if (INTEL_INFO(dev)->gen >= 7)
> +	if (INTEL_INFO(dev)->gen >= 8)
> +		max = level == 0 ? 255 : 2047;

Not having looked at the WM stuff before, it took me a while to find the
registers and check the maximums. Which made me wonder why we don't fix
the masks and use them here, so it would be bloody obvious what we're
referring to?

if (level)
	max = WM1_LP_SR_MASK_BDW >> WM1_LP_SR_SHIFT_BDW;
else
	max = WM0_PIPE_PLANE_MASK_BDW >> WM0_PIPE_PLANE_SHIFT;

> +	else if (INTEL_INFO(dev)->gen >= 7)
>  		/* IVB/HSW primary/sprite plane watermarks */
>  		max = level == 0 ? 127 : 1023;
>  	else if (!is_sprite)
> @@ -2366,10 +2370,13 @@ static unsigned int ilk_cursor_wm_max(const struct drm_device *dev,
>  }
>  
>  /* Calculate the maximum FBC watermark */
> -static unsigned int ilk_fbc_wm_max(void)
> +static unsigned int ilk_fbc_wm_max(struct drm_device *dev)
>  {
>  	/* max that registers can hold */
> -	return 15;
> +	if (INTEL_INFO(dev)->gen >= 8)
> +		return 31;
> +	else
> +		return 15;
>  }
>  
>  static void ilk_compute_wm_maximums(struct drm_device *dev,
> @@ -2381,7 +2388,7 @@ static void ilk_compute_wm_maximums(struct drm_device *dev,
>  	max->pri = ilk_plane_wm_max(dev, level, config, ddb_partitioning, false);
>  	max->spr = ilk_plane_wm_max(dev, level, config, ddb_partitioning, true);
>  	max->cur = ilk_cursor_wm_max(dev, level, config);
> -	max->fbc = ilk_fbc_wm_max();
> +	max->fbc = ilk_fbc_wm_max(dev);
>  }
>  
>  static bool ilk_validate_wm_level(int level,
> @@ -2722,10 +2729,18 @@ static void hsw_compute_wm_results(struct drm_device *dev,
>  		if (!r->enable)
>  			break;
>  
> -		results->wm_lp[wm_lp - 1] = HSW_WM_LP_VAL(level * 2,
> -							  r->fbc_val,
> -							  r->pri_val,
> -							  r->cur_val);

This leaves HSW_WM_LP_VAL() macro unused.

> +		results->wm_lp[wm_lp - 1] = WM3_LP_EN |
> +			((level * 2) << WM1_LP_LATENCY_SHIFT) |
> +			(r->pri_val << WM1_LP_SR_SHIFT) |
> +			r->cur_val;
> +
> +		if (INTEL_INFO(dev)->gen >= 8)
> +			results->wm_lp[wm_lp - 1] |=
> +				r->fbc_val << WM1_LP_FBC_SHIFT_BDW;
> +		else
> +			results->wm_lp[wm_lp - 1] |=
> +				r->fbc_val << WM1_LP_FBC_SHIFT;
> +
>  		results->wm_lp_spr[wm_lp - 1] = r->spr_val;
>  	}
>  
> -- 
> 1.8.4.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center