From: Daniel Vetter <daniel@ffwll.ch>
To: Thomas Richter <thor@math.tu-berlin.de>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] Added a lower limit for the watermark setting
Date: Tue, 19 Nov 2013 17:41:39 +0100 [thread overview]
Message-ID: <20131119164139.GE5794@phenom.ffwll.local> (raw)
In-Reply-To: <528B8E8D.6090203@math.tu-berlin.de>
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@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@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
next prev parent reply other threads:[~2013-11-19 16:41 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-18 6:38 [PATCH] drm/i915: Replicate BIOS eDP bpp clamping hack for hsw Daniel Vetter
2013-11-19 15:51 ` [Intel-gfx] " Ville Syrjälä
2013-11-19 16:01 ` Daniel Vetter
[not found] ` <24336_1384876863_528B8B3F_24336_14011_1_20131119160127.GC5794@phenom.ffwll.local>
2013-11-19 16:15 ` [PATCH] Added a lower limit for the watermark setting Thomas Richter
2013-11-19 16:41 ` Daniel Vetter [this message]
[not found] ` <24336_1384879265_528B94A1_24336_14365_1_20131119164139.GE5794@phenom.ffwll.local>
2013-11-20 9:18 ` Thomas Richter
2013-11-20 9:27 ` Daniel Vetter
[not found] ` <24336_1384939634_528C8071_24336_18909_1_CAKMK7uH2VXy05gW7yCpkzUCe9nO+e9XU+jYEq8RaNKOXj4G2iw@mail.gmail.com>
2013-11-20 10:50 ` Thomas Richter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20131119164139.GE5794@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=intel-gfx@lists.freedesktop.org \
--cc=thor@math.tu-berlin.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox