2015-04-28 18:14:16

by Thomas Gummerer

[permalink] [raw]
Subject: [PATCH/RFC] i915: fix screen flickering

Commit c9f038a1a592 ("drm/i915: Don't assume primary & cursor are
always on for wm calculation (v4)") fixes a null pointer dereference.
Setting the primary and cursor panes to false in
ilk_compute_wm_parameters to false does however give the following
errors in the kernel log and causes the screen to flicker.

[ 101.133716] [drm:intel_set_cpu_fifo_underrun_reporting [i915]]
*ERROR* uncleared fifo underrun on pipe A
[ 101.133725] [drm:intel_cpu_fifo_underrun_irq_handler [i915]]
*ERROR* CPU pipe A FIFO underrun

Always setting the panes to enabled fixes this error.

Signed-off-by: Thomas Gummerer <[email protected]>
---
Hi,

I've noticed the regression in v4.1-rc1. I'm not sure this is the best
solution, but it's the best I could come up with in the time I had
available.

drivers/gpu/drm/i915/intel_pm.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index fa4ccb3..377df60 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2045,22 +2045,20 @@ static void ilk_compute_wm_parameters(struct drm_crtc *crtc,
p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
p->pixel_rate = ilk_pipe_pixel_rate(dev, crtc);

- if (crtc->primary->state->fb) {
- p->pri.enabled = true;
+ if (crtc->primary->state->fb)
p->pri.bytes_per_pixel =
crtc->primary->state->fb->bits_per_pixel / 8;
- } else {
- p->pri.enabled = false;
+ else
p->pri.bytes_per_pixel = 0;
- }

- if (crtc->cursor->state->fb) {
- p->cur.enabled = true;
- p->cur.bytes_per_pixel = 4;
- } else {
- p->cur.enabled = false;
- p->cur.bytes_per_pixel = 0;
- }
+ p->cur.bytes_per_pixel = 4;
+ /*
+ * TODO: for now, assume primary and cursor planes are always enabled.
+ * Setting them to false makes the screen flicker.
+ */
+ p->pri.enabled = true;
+ p->cur.enabled = true;
+
p->pri.horiz_pixels = intel_crtc->config->pipe_src_w;
p->cur.horiz_pixels = intel_crtc->base.cursor->state->crtc_w;

--
2.1.0.264.g0463184.dirty


2015-04-28 22:23:58

by Thomas Gummerer

[permalink] [raw]
Subject: Re: [PATCH/RFC] i915: fix screen flickering

Thomas Gummerer <[email protected]> writes:

> Commit c9f038a1a592 ("drm/i915: Don't assume primary & cursor are
> always on for wm calculation (v4)") fixes a null pointer dereference.
> Setting the primary and cursor panes to false in
> ilk_compute_wm_parameters to false does however give the following
> errors in the kernel log and causes the screen to flicker.
>
> [ 101.133716] [drm:intel_set_cpu_fifo_underrun_reporting [i915]]
> *ERROR* uncleared fifo underrun on pipe A
> [ 101.133725] [drm:intel_cpu_fifo_underrun_irq_handler [i915]]
> *ERROR* CPU pipe A FIFO underrun
>
> Always setting the panes to enabled fixes this error.
>
> Signed-off-by: Thomas Gummerer <[email protected]>
> ---
> Hi,
>
> I've noticed the regression in v4.1-rc1. I'm not sure this is the best
> solution, but it's the best I could come up with in the time I had
> available.

I forgot to mention, the bug only occurs after suspending the system and
waking it up again. In that case, moving the mouse pointer around on
the build in laptop screen triggers the flickering, while the external
monitor works just fine.

Might be unrelated, but I only managed to trigger the bug when the
system is suspended after the synergy server has started.

> drivers/gpu/drm/i915/intel_pm.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index fa4ccb3..377df60 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2045,22 +2045,20 @@ static void ilk_compute_wm_parameters(struct drm_crtc *crtc,
> p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
> p->pixel_rate = ilk_pipe_pixel_rate(dev, crtc);
>
> - if (crtc->primary->state->fb) {
> - p->pri.enabled = true;
> + if (crtc->primary->state->fb)
> p->pri.bytes_per_pixel =
> crtc->primary->state->fb->bits_per_pixel / 8;
> - } else {
> - p->pri.enabled = false;
> + else
> p->pri.bytes_per_pixel = 0;
> - }
>
> - if (crtc->cursor->state->fb) {
> - p->cur.enabled = true;
> - p->cur.bytes_per_pixel = 4;
> - } else {
> - p->cur.enabled = false;
> - p->cur.bytes_per_pixel = 0;
> - }
> + p->cur.bytes_per_pixel = 4;
> + /*
> + * TODO: for now, assume primary and cursor planes are always enabled.
> + * Setting them to false makes the screen flicker.
> + */
> + p->pri.enabled = true;
> + p->cur.enabled = true;
> +
> p->pri.horiz_pixels = intel_crtc->config->pipe_src_w;
> p->cur.horiz_pixels = intel_crtc->base.cursor->state->crtc_w;
>
> --
> 2.1.0.264.g0463184.dirty

2015-05-02 12:24:31

by Jan Niehusmann

[permalink] [raw]
Subject: Re: [PATCH/RFC] i915: fix screen flickering

On Wed, Apr 29, 2015 at 12:23:43AM +0200, Thomas Gummerer wrote:
> I forgot to mention, the bug only occurs after suspending the system and
> waking it up again. In that case, moving the mouse pointer around on
> the build in laptop screen triggers the flickering, while the external
> monitor works just fine.

My Lenovo X201s shows the same symptoms with v4.1.0-rc1, and your patch
seems to help.

Another detail: The flickering only happens when both outputs are
active, using xrandr to disable one output makes the other one working
fine, again.

> Might be unrelated, but I only managed to trigger the bug when the
> system is suspended after the synergy server has started.

I don't have synergy installed, so this can't be a strict requirement
for triggering the bug.

Jan