[Intel-gfx] [PATCH] Added a lower limit for the watermark setting

Daniel Vetter daniel at ffwll.ch
Tue Nov 19 17:41:39 CET 2013


On Tue, Nov 19, 2013 at 05:15:09PM +0100, Thomas Richter wrote:
> Hi Daniel, dear intel experts,
> 
> please find a patch attached that finally addresses the display
> flicker on i830 chipsets. This
> patch adds a lower watermark setting in intel_watermark_params{},
> but keeps it zero for
> all but the i830 chipsets. The necessary new defines are in i915_reg.h.

I think we ned to split out the gen2/3 single/dual pipe watermark code a
bit better from everything else. A bugfix for i830M shouldn't really
affect snb ;-)

I've pushed out my current (and rather broken) wip branch with my idea on
the take to

http://cgit.freedesktop.org/~danvet/drm/log/?h=for-thomas

Cheers, Daniel
> 
> Greetings,
>     Thomas
> 

> From 916d8354b2134d587b946fa88c66c3d098d38df4 Mon Sep 17 00:00:00 2001
> From: Thomas Richter <thor at math.tu-berlin.de>
> Date: Tue, 19 Nov 2013 17:09:45 +0100
> Subject: [PATCH 3/3] Added a lower limit for the watermark setting.
> 
> This patch adds a minimum value for the watermark setting, in addition
> to the already existing upper limit. The lower limit is necessary on
> i830 chipsets as otherwise display flickering may result, especially
> on panning. While the flicker itself is caused by the FIFO running
> dry. Interestingly, an overrunning FIFO may also cause a hickup in
> the chipset which results in a pipeline stall with the very same
> effects.
> 
> Lower limits for all other chipsets are zero to remain consistent
> with the currently active code.
> 
> Signed-off-by: Thomas Richter <thor at math.tu-berlin.de>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |   16 ++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |    1 +
>  drivers/gpu/drm/i915/intel_pm.c  |   24 ++++++++++++++++++++++++
>  3 files changed, 41 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 849e595..c144957 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3361,24 +3361,32 @@
>  #define I915_FIFO_SIZE		95
>  #define I855GM_FIFO_SIZE	127 /* In cachelines */
>  #define I830_FIFO_SIZE		95
> +#define I830_MIN_WM             8 /* instabilities if below this point */
>  
> +#define VALLEYVIEW_MIN_WM	0
>  #define VALLEYVIEW_MAX_WM	0xff
> +#define G4X_MIN_WM		0
>  #define G4X_MAX_WM		0x3f
> +#define I915_MIN_WM		0
>  #define I915_MAX_WM		0x3f
>  
>  #define PINEVIEW_DISPLAY_FIFO	512 /* in 64byte unit */
>  #define PINEVIEW_FIFO_LINE_SIZE	64
> +#define PINEVIEW_MIN_WM                0
>  #define PINEVIEW_MAX_WM		0x1ff
>  #define PINEVIEW_DFT_WM		0x3f
>  #define PINEVIEW_DFT_HPLLOFF_WM	0
>  #define PINEVIEW_GUARD_WM		10
>  #define PINEVIEW_CURSOR_FIFO		64
> +#define PINEVIEW_CURSOR_MIN_WM	0
>  #define PINEVIEW_CURSOR_MAX_WM	0x3f
>  #define PINEVIEW_CURSOR_DFT_WM	0
>  #define PINEVIEW_CURSOR_GUARD_WM	5
>  
>  #define VALLEYVIEW_CURSOR_MAX_WM 64
> +#define VALLEYVIEW_CURSOR_MIN_WM 0
>  #define I965_CURSOR_FIFO	64
> +#define I965_CURSOR_MIN_WM	0
>  #define I965_CURSOR_MAX_WM	32
>  #define I965_CURSOR_DFT_WM	8
>  
> @@ -3424,16 +3432,20 @@
>  
>  /* define the fifo size on Ironlake */
>  #define ILK_DISPLAY_FIFO	128
> +#define ILK_DISPLAY_MINWM	0
>  #define ILK_DISPLAY_MAXWM	64
>  #define ILK_DISPLAY_DFTWM	8
>  #define ILK_CURSOR_FIFO		32
> +#define ILK_CURSOR_MINWM        0
>  #define ILK_CURSOR_MAXWM	16
>  #define ILK_CURSOR_DFTWM	8
>  
>  #define ILK_DISPLAY_SR_FIFO	512
> +#define ILK_DISPLAY_MIN_SRWM	0
>  #define ILK_DISPLAY_MAX_SRWM	0x1ff
>  #define ILK_DISPLAY_DFT_SRWM	0x3f
>  #define ILK_CURSOR_SR_FIFO	64
> +#define ILK_CURSOR_MIN_SRWM	0
>  #define ILK_CURSOR_MAX_SRWM	0x3f
>  #define ILK_CURSOR_DFT_SRWM	8
>  
> @@ -3441,16 +3453,20 @@
>  
>  /* define the WM info on Sandybridge */
>  #define SNB_DISPLAY_FIFO	128
> +#define SNB_DISPLAY_MINWM	0
>  #define SNB_DISPLAY_MAXWM	0x7f	/* bit 16:22 */
>  #define SNB_DISPLAY_DFTWM	8
>  #define SNB_CURSOR_FIFO		32
> +#define SNB_CURSOR_MINWM	0
>  #define SNB_CURSOR_MAXWM	0x1f	/* bit 4:0 */
>  #define SNB_CURSOR_DFTWM	8
>  
>  #define SNB_DISPLAY_SR_FIFO	512
> +#define SNB_DISPLAY_MIN_SRWM	0
>  #define SNB_DISPLAY_MAX_SRWM	0x1ff	/* bit 16:8 */
>  #define SNB_DISPLAY_DFT_SRWM	0x3f
>  #define SNB_CURSOR_SR_FIFO	64
> +#define SNB_CURSOR_MIN_SRWM	0
>  #define SNB_CURSOR_MAX_SRWM	0x3f	/* bit 5:0 */
>  #define SNB_CURSOR_DFT_SRWM	8
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 8e133de..9400b1a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -418,6 +418,7 @@ struct intel_plane {
>  
>  struct intel_watermark_params {
>  	unsigned long fifo_size;
> +	unsigned long min_wm;
>  	unsigned long max_wm;
>  	unsigned long default_wm;
>  	unsigned long guard_size;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 9c8b374..47f6fc8 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -868,6 +868,7 @@ static int i830_get_fifo_size(struct drm_device *dev, int plane)
>  /* Pineview has different values for various configs */
>  static const struct intel_watermark_params pineview_display_wm = {
>  	PINEVIEW_DISPLAY_FIFO,
> +	PINEVIEW_MIN_WM,
>  	PINEVIEW_MAX_WM,
>  	PINEVIEW_DFT_WM,
>  	PINEVIEW_GUARD_WM,
> @@ -875,6 +876,7 @@ static const struct intel_watermark_params pineview_display_wm = {
>  };
>  static const struct intel_watermark_params pineview_display_hplloff_wm = {
>  	PINEVIEW_DISPLAY_FIFO,
> +	PINEVIEW_MIN_WM,
>  	PINEVIEW_MAX_WM,
>  	PINEVIEW_DFT_HPLLOFF_WM,
>  	PINEVIEW_GUARD_WM,
> @@ -882,6 +884,7 @@ static const struct intel_watermark_params pineview_display_hplloff_wm = {
>  };
>  static const struct intel_watermark_params pineview_cursor_wm = {
>  	PINEVIEW_CURSOR_FIFO,
> +	PINEVIEW_CURSOR_MIN_WM,
>  	PINEVIEW_CURSOR_MAX_WM,
>  	PINEVIEW_CURSOR_DFT_WM,
>  	PINEVIEW_CURSOR_GUARD_WM,
> @@ -889,6 +892,7 @@ static const struct intel_watermark_params pineview_cursor_wm = {
>  };
>  static const struct intel_watermark_params pineview_cursor_hplloff_wm = {
>  	PINEVIEW_CURSOR_FIFO,
> +	PINEVIEW_CURSOR_MIN_WM,
>  	PINEVIEW_CURSOR_MAX_WM,
>  	PINEVIEW_CURSOR_DFT_WM,
>  	PINEVIEW_CURSOR_GUARD_WM,
> @@ -896,6 +900,7 @@ static const struct intel_watermark_params pineview_cursor_hplloff_wm = {
>  };
>  static const struct intel_watermark_params g4x_wm_info = {
>  	G4X_FIFO_SIZE,
> +	G4X_MIN_WM,
>  	G4X_MAX_WM,
>  	G4X_MAX_WM,
>  	2,
> @@ -903,6 +908,7 @@ static const struct intel_watermark_params g4x_wm_info = {
>  };
>  static const struct intel_watermark_params g4x_cursor_wm_info = {
>  	I965_CURSOR_FIFO,
> +	I965_CURSOR_MIN_WM,
>  	I965_CURSOR_MAX_WM,
>  	I965_CURSOR_DFT_WM,
>  	2,
> @@ -910,6 +916,7 @@ static const struct intel_watermark_params g4x_cursor_wm_info = {
>  };
>  static const struct intel_watermark_params valleyview_wm_info = {
>  	VALLEYVIEW_FIFO_SIZE,
> +	VALLEYVIEW_MIN_WM,
>  	VALLEYVIEW_MAX_WM,
>  	VALLEYVIEW_MAX_WM,
>  	2,
> @@ -917,6 +924,7 @@ static const struct intel_watermark_params valleyview_wm_info = {
>  };
>  static const struct intel_watermark_params valleyview_cursor_wm_info = {
>  	I965_CURSOR_FIFO,
> +	VALLEYVIEW_CURSOR_MIN_WM,
>  	VALLEYVIEW_CURSOR_MAX_WM,
>  	I965_CURSOR_DFT_WM,
>  	2,
> @@ -924,6 +932,7 @@ static const struct intel_watermark_params valleyview_cursor_wm_info = {
>  };
>  static const struct intel_watermark_params i965_cursor_wm_info = {
>  	I965_CURSOR_FIFO,
> +	I965_CURSOR_MIN_WM,
>  	I965_CURSOR_MAX_WM,
>  	I965_CURSOR_DFT_WM,
>  	2,
> @@ -931,6 +940,7 @@ static const struct intel_watermark_params i965_cursor_wm_info = {
>  };
>  static const struct intel_watermark_params i945_wm_info = {
>  	I945_FIFO_SIZE,
> +	I915_MIN_WM,
>  	I915_MAX_WM,
>  	1,
>  	2,
> @@ -938,6 +948,7 @@ static const struct intel_watermark_params i945_wm_info = {
>  };
>  static const struct intel_watermark_params i915_wm_info = {
>  	I915_FIFO_SIZE,
> +	I915_MIN_WM,
>  	I915_MAX_WM,
>  	1,
>  	2,
> @@ -945,6 +956,7 @@ static const struct intel_watermark_params i915_wm_info = {
>  };
>  static const struct intel_watermark_params i855_wm_info = {
>  	I855GM_FIFO_SIZE,
> +	I830_MIN_WM,
>  	I915_MAX_WM,
>  	1,
>  	2,
> @@ -952,6 +964,7 @@ static const struct intel_watermark_params i855_wm_info = {
>  };
>  static const struct intel_watermark_params i830_wm_info = {
>  	I830_FIFO_SIZE,
> +	I830_MIN_WM,
>  	I915_MAX_WM,
>  	1,
>  	2,
> @@ -960,6 +973,7 @@ static const struct intel_watermark_params i830_wm_info = {
>  
>  static const struct intel_watermark_params ironlake_display_wm_info = {
>  	ILK_DISPLAY_FIFO,
> +	ILK_DISPLAY_MINWM,
>  	ILK_DISPLAY_MAXWM,
>  	ILK_DISPLAY_DFTWM,
>  	2,
> @@ -967,6 +981,7 @@ static const struct intel_watermark_params ironlake_display_wm_info = {
>  };
>  static const struct intel_watermark_params ironlake_cursor_wm_info = {
>  	ILK_CURSOR_FIFO,
> +	ILK_CURSOR_MINWM,
>  	ILK_CURSOR_MAXWM,
>  	ILK_CURSOR_DFTWM,
>  	2,
> @@ -974,6 +989,7 @@ static const struct intel_watermark_params ironlake_cursor_wm_info = {
>  };
>  static const struct intel_watermark_params ironlake_display_srwm_info = {
>  	ILK_DISPLAY_SR_FIFO,
> +	ILK_DISPLAY_MIN_SRWM,
>  	ILK_DISPLAY_MAX_SRWM,
>  	ILK_DISPLAY_DFT_SRWM,
>  	2,
> @@ -981,6 +997,7 @@ static const struct intel_watermark_params ironlake_display_srwm_info = {
>  };
>  static const struct intel_watermark_params ironlake_cursor_srwm_info = {
>  	ILK_CURSOR_SR_FIFO,
> +	ILK_CURSOR_MIN_SRWM,
>  	ILK_CURSOR_MAX_SRWM,
>  	ILK_CURSOR_DFT_SRWM,
>  	2,
> @@ -989,6 +1006,7 @@ static const struct intel_watermark_params ironlake_cursor_srwm_info = {
>  
>  static const struct intel_watermark_params sandybridge_display_wm_info = {
>  	SNB_DISPLAY_FIFO,
> +	SNB_DISPLAY_MINWM,
>  	SNB_DISPLAY_MAXWM,
>  	SNB_DISPLAY_DFTWM,
>  	2,
> @@ -996,6 +1014,7 @@ static const struct intel_watermark_params sandybridge_display_wm_info = {
>  };
>  static const struct intel_watermark_params sandybridge_cursor_wm_info = {
>  	SNB_CURSOR_FIFO,
> +	SNB_CURSOR_MINWM,
>  	SNB_CURSOR_MAXWM,
>  	SNB_CURSOR_DFTWM,
>  	2,
> @@ -1003,6 +1022,7 @@ static const struct intel_watermark_params sandybridge_cursor_wm_info = {
>  };
>  static const struct intel_watermark_params sandybridge_display_srwm_info = {
>  	SNB_DISPLAY_SR_FIFO,
> +	SNB_DISPLAY_MIN_SRWM,
>  	SNB_DISPLAY_MAX_SRWM,
>  	SNB_DISPLAY_DFT_SRWM,
>  	2,
> @@ -1010,6 +1030,7 @@ static const struct intel_watermark_params sandybridge_display_srwm_info = {
>  };
>  static const struct intel_watermark_params sandybridge_cursor_srwm_info = {
>  	SNB_CURSOR_SR_FIFO,
> +	SNB_CURSOR_MIN_SRWM,
>  	SNB_CURSOR_MAX_SRWM,
>  	SNB_CURSOR_DFT_SRWM,
>  	2,
> @@ -1064,6 +1085,9 @@ static unsigned long intel_calculate_wm(unsigned long clock_in_khz,
>  		wm_size = wm->max_wm;
>  	if (wm_size <= 0)
>  		wm_size = wm->default_wm;
> +	if (wm_size < (long)wm->min_wm)
> +		wm_size = wm->min_wm;
> +
>  	return wm_size;
>  }
>  
> -- 
> 1.7.10.4
> 


-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch



More information about the Intel-gfx mailing list