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 <t...@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 <t...@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
