| Message ID | 528B8E8D.6090203@math.tu-berlin.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
Am 19.11.2013 17:41, schrieb Daniel Vetter: > 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 ;-) Actually, the fun part is that it does not because all the lower limits are zero for all other architectures. > > 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 Could you please help me here how to apply it? I'm not very experienced with git, and it does not seem to fit to the sources of intel_pm.c I have. Do I first need to instruct git to download another branch? I'm currently at drm-intel-nightly. Thanks, Thomas
On Wed, Nov 20, 2013 at 10:18 AM, Thomas Richter <thor@math.tu-berlin.de> wrote: >> 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 ;-) > > > Actually, the fun part is that it does not because all the lower limits are > zero for all other architectures. What I've meant to say is that I want to split up the watermark code more anyway, so that there's no need to fill in the 0 all over the place where we don't care one bit. I.e. all the gen4+ platforms. >> 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 > > > Could you please help me here how to apply it? I'm not very experienced with > git, and it does not seem to fit to the sources of > intel_pm.c I have. Do I first need to instruct git to download another > branch? I'm currently at drm-intel-nightly. It's more just to read through the patches for ideas. Atm the branch doesn't even compile - I'll try to clean it up and beat it into shape in the next few days. The core idea is to have a minimal wm of 4 (lowest burst setting) and then set the actual burst setting to the watermark rounded down to the next multiple of 4, up to the recommended value in Bspec (which is 16 fifo cachelines). But I've fumbled the job a bit and broke a few things ;-) -Daniel
Am 20.11.2013 10:27, schrieb Daniel Vetter: > What I've meant to say is that I want to split up the watermark code > more anyway, so that there's no need to fill in the 0 all over the > place where we don't care one bit. I.e. all the gen4+ platforms. Ok - well, I guess I cannot judge whether that's necessary or required. Given that the i830 is the only chipset that comes with a unified FIFO, it might be actually necessary. > >>> 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 >> >> Could you please help me here how to apply it? I'm not very experienced with >> git, and it does not seem to fit to the sources of >> intel_pm.c I have. Do I first need to instruct git to download another >> branch? I'm currently at drm-intel-nightly. > It's more just to read through the patches for ideas. Atm the branch > doesn't even compile - I'll try to clean it up and beat it into shape > in the next few days. The core idea is to have a minimal wm of 4 > (lowest burst setting) and then set the actual burst setting to the > watermark rounded down to the next multiple of 4, up to the > recommended value in Bspec (which is 16 fifo cachelines). But I've > fumbled the job a bit and broke a few things ;-) Ah, that explains something, thank you. Probably two things: The way how the current code is organized, the wm handling of the i830 is handled in two functions, not one. In i830_update_wm and i9xx_update_wm. The former seems to be used during bootstrap when only one pipe is active, the latter as soon as I activate the second pipe. This is probably needlessly complex, and it would be better to have this just in one place, not two (found that when manually adding a lower limit as first attempt). The second is concerned the lower limit: Four is not enough, six gives a "reasonably stable image" most the time, but if scrolling wildly, still some hickups. Eight is better, but still not perfect. If you try really, really hard, one can still provoke a crash if a video overlay is active. Unclear why this is - if it happens, the screen seems to go blank for just one frame, then recovers. Given that the plane pointers are only shadow registers that are automatically updated by the chip on a vblank, I have no good idea why that happens. I also see that your code seems to use a modified formula for the "latency". Given that I cannot compile the code, it's at this time hard to say whether that's better or worse. The current one is "too pessimistic". I tried to find the minimum permissible latency and find that for a pixel clock of 135Mhz, the watermark should be at most 30, and for a pixel clock of 108 Mhz, the watermark should be at most 32. From this one can compute the latencies in ns. If I did everything right, I get values of approximately 1185 ns and 1066 ns, thus approximately in the same ball park. A value of 1500 should likely be fine, but requires testing on additional hardware (I've currently no access to the Fujistu, the other laptop with a i830 chipset and the strange DVO). BTW, even with the maximal watermarks (minimum latency), I do get "hickups" if I try really really hard. So they are likely provoked by something else, not the wrong watermark. Thanks! 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
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. Greetings, Thomas