Screen flickering

Hi,

I noticed that on my machine the screen starts to flicker after I suspend and resume my machine, on the main laptop display if an external display is attached with kernel v4.1-rc1. I tracked the regression down to commit c9f038a1a592 ("drm/i915: Don't assume primary & cursor are always on for wm calculation (v4)"), and sent a patch that fixes that behavior at [1] about two weeks ago, although I'm not sure it's the right thing to do, as I'm not very familiar with the code. The same bug still exists in vv4.1-rc3.

Jan Niehusmann confirmed the behavior, but there has been no further discussion on the topic. I also forgot to cc the people that were involved in the patch that caused the regression (sorry about that).

Is there anything else that I can do to help fixing this issue?

Thanks,
Thomas

[1] http://lists.freedesktop.org/archives/intel-gfx/2015-April/065494.html

Is this the same as https://bugzilla.kernel.org/show_bug.cgi?id=98141 ?

Hi,

Quoted 1 line · Jani Nikula

Is this the same as https://bugzilla.kernel.org/show_bug.cgi?id=98141 ?

The visible effect in the video is similar to what I see on the LVDS display. I also see some influence of the mouse pointer position on the blanked areas.

The effect on the external screen is more like single scan lines jumping to the right and back, at a high (but still visible) frequency.

What I'm missing in the report, are some log entries I'm seeing on my notebook:

Apr 30 08:50:23 localhost kernel: [drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun
Apr 30 08:50:23 localhost kernel: [drm:intel_pch_fifo_underrun_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun
Apr 30 08:50:29 localhost kernel: [drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun
Apr 30 08:50:29 localhost kernel: [drm:intel_pch_fifo_underrun_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun

Jan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Quoted 7 lines

What I'm missing in the report, are some log entries I'm seeing on my notebook:

Apr 30 08:50:23 localhost kernel: [drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun
Apr 30 08:50:23 localhost kernel: [drm:intel_pch_fifo_underrun_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun
Apr 30 08:50:29 localhost kernel: [drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun
Apr 30 08:50:29 localhost kernel: [drm:intel_pch_fifo_underrun_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun

hi,
I also have the similar entries in my dmesg, though my problem is a little different from yours and my bisect lead to another commit. And that also is still not fixed (last tested with 4.0).

Quoted 11 lines · Sudip Mukherjee

Quoted 7 lines

What I'm missing in the report, are some log entries I'm seeing on my notebook:

Apr 30 08:50:23 localhost kernel: [drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun
Apr 30 08:50:23 localhost kernel: [drm:intel_pch_fifo_underrun_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun
Apr 30 08:50:29 localhost kernel: [drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun
Apr 30 08:50:29 localhost kernel: [drm:intel_pch_fifo_underrun_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun

hi,
I also have the similar entries in my dmesg, though my problem is a little different from yours and my bisect lead to another commit. And that also is still not fixed (last tested with 4.0).

(Please note that I didn't do a bisect - that was Thomas. I only noted that I can confirm his observations and that his patch helps to prevent or hide the issue.)

Perhaps these are two completely unrelated issues?

Jan

Quoted 4 lines · Jan Niehusmann

Apr 30 08:50:23 localhost kernel: [drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun
Apr 30 08:50:23 localhost kernel: [drm:intel_pch_fifo_underrun_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun
Apr 30 08:50:29 localhost kernel: [drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun
Apr 30 08:50:29 localhost kernel: [drm:intel_pch_fifo_underrun_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun

hi,
I also have the similar entries in my dmesg, though my problem is a little different from yours and my bisect lead to another commit. And that also is still not fixed (last tested with 4.0).

http://patchwork.freedesktop.org/patch/46071/

regards
sudip

Quoted 2 lines · Sudip Mukherjee

On Wed, May 13, 2015 at 11:53:19AM +0200, Jan Niehusmann wrote:
http://patchwork.freedesktop.org/patch/46071/

Thank you for the pointer, but this seems to be an unrelated issue. I tried applying the patch, but it doesn't fix the issues that I'm experiencing.

Quoted 3 lines

regards
sudip

Hi,

Quoted 1 line · Jani Nikula

Is this the same as https://bugzilla.kernel.org/show_bug.cgi?id=98141 ?

The visible effect in the video is similar to what I see on the LVDS display. I also see some influence of the mouse pointer position on the blanked areas.

The effect on the external screen is more like single scan lines jumping to the right and back, at a high (but still visible) frequency.

What I'm missing in the report, are some log entries I'm seeing on my notebook:

Apr 30 08:50:23 localhost kernel: [drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun
Apr 30 08:50:23 localhost kernel: [drm:intel_pch_fifo_underrun_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun
Apr 30 08:50:29 localhost kernel: [drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun
Apr 30 08:50:29 localhost kernel: [drm:intel_pch_fifo_underrun_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun

Jan

Hi,

I noticed that on my machine the screen starts to flicker after I suspend and resume my machine, on the main laptop display if an external display is attached with kernel v4.1-rc1. I tracked the regression down to commit c9f038a1a592 ("drm/i915: Don't assume primary & cursor are always on for wm calculation (v4)"), and sent a patch that fixes that behavior at [1] about two weeks ago, although I'm not sure it's the right thing to do, as I'm not very familiar with the code. The same bug still exists in vv4.1-rc3.

Jan Niehusmann confirmed the behavior, but there has been no further discussion on the topic. I also forgot to cc the people that were involved in the patch that caused the regression (sorry about that).

Is there anything else that I can do to help fixing this issue?

Thanks,
Thomas

[1] http://lists.freedesktop.org/archives/intel-gfx/2015-April/065494.html

Sorry, I missed your patch when you first sent it. That type of fix looks like an okay workaround while we do a more in-depth rework of the watermark system. However I think your patch could cause a crash if we disable the primary plane via the universal plane interface; if we do that, p->pri.bytes_per_pixel is set to 0, but since we're now pretending the primary plane is always enabled, ilk_wm_fbc() can eventually get called and use that 0 in the denominator of a division operation.

If you just squash the following change into your patch, I think it should be safe:
[...]

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.

Helped-by Matt Roper <[email protected]>

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

---

Quoted 11 lines

Sorry, I missed your patch when you first sent it. That type of fix looks like an okay workaround while we do a more in-depth rework of the watermark system. However I think your patch could cause a crash if we disable the primary plane via the universal plane interface; if we do that, p->pri.bytes_per_pixel is set to 0, but since we're now pretending the primary plane is always enabled, ilk_wm_fbc() can eventually get called and use that 0 in the denominator of a division operation.

If you just squash the following change into your patch, I think it should be safe:
[...]

Thank you very much for the suggestion, here is an updated version of the patch.

 drivers/gpu/drm/i915/intel_pm.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

drivers/gpu/drm/i915/intel_pm.c +11 -13

index fa4ccb3..555b896 100644
@@ -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;
-		p->pri.bytes_per_pixel = 0;
-	}
+	else
+		p->pri.bytes_per_pixel = 4;
+
+	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;
 
-	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->pri.horiz_pixels = intel_crtc->config->pipe_src_w;
 	p->cur.horiz_pixels = intel_crtc->base.cursor->state->crtc_w;
 

Signature · 1 line

2.4.0.184.g8e1974e

Quoted 13 lines · Thomas Gummerer

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.

Quoted 2 lines

Helped-by Matt Roper <[email protected]>

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

Seems like a reasonable short-term workaround and returns us to how the code used to behaves.

Reviewed-by Matt Roper <[email protected]>

Quoted 62 lines

---

Quoted 11 lines

Sorry, I missed your patch when you first sent it. That type of fix looks like an okay workaround while we do a more in-depth rework of the watermark system. However I think your patch could cause a crash if we disable the primary plane via the universal plane interface; if we do that, p->pri.bytes_per_pixel is set to 0, but since we're now pretending the primary plane is always enabled, ilk_wm_fbc() can eventually get called and use that 0 in the denominator of a division operation.

If you just squash the following change into your patch, I think it should be safe:
[...]

Thank you very much for the suggestion, here is an updated version of the patch.

 drivers/gpu/drm/i915/intel_pm.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

drivers/gpu/drm/i915/intel_pm.c +11 -13

index fa4ccb3..555b896 100644
@@ -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;
-		p->pri.bytes_per_pixel = 0;
-	}
+	else
+		p->pri.bytes_per_pixel = 4;
+
+	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;
 
-	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->pri.horiz_pixels = intel_crtc->config->pipe_src_w;
 	p->cur.horiz_pixels = intel_crtc->base.cursor->state->crtc_w;
 

Signature · 1 line

2.4.0.184.g8e1974e

Signature · 5 lines

Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795