Hi!
Hopefully the right addressee.
Encountered two bad backports which cause screen-flicker. dmesg shows:
... [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun ...
CPU: Intel Core i3 (Clarkdale/Ironlake)
The backports are:
- diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 49de476..277a802 100644 - diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index a19ec06..3ce9ba3 100644
After reversing them the flicker is gone, no more messages in dmesg. All else OK so far.
I have also reported this here:
https://bugs.freedesktop.org/show_bug.cgi?id=103796
So long!
Rainer Fiebig
On Sat, Nov 18, 2017 at 01:47:32PM +0100, Rainer Fiebig wrote:
Hi!
Hopefully the right addressee.
Encountered two bad backports which cause screen-flicker. dmesg shows:
... [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun ...
CPU: Intel Core i3 (Clarkdale/Ironlake)
The backports are:
- diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c index 49de476..277a802 100644
- diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h index a19ec06..3ce9ba3 100644
After reversing them the flicker is gone, no more messages in dmesg. All else OK so far.
So which commit was the one that caused the problem? I will be glad to revert it.
thanks,
greg k-h
Greg KH wrote:
On Sat, Nov 18, 2017 at 01:47:32PM +0100, Rainer Fiebig wrote:
Hi!
Hopefully the right addressee.
Encountered two bad backports which cause screen-flicker. dmesg shows:
... [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun ...
CPU: Intel Core i3 (Clarkdale/Ironlake)
The backports are:
- diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c index 49de476..277a802 100644
- diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h index a19ec06..3ce9ba3 100644
After reversing them the flicker is gone, no more messages in dmesg. All else OK so far.
So which commit was the one that caused the problem? I will be glad to revert it.
thanks,
greg k-h
I started by reverting the more complex one first ("index 49de476..277a802100644"). But the kernel wouldn't compile then.
So I also reverted "index a19ec06..3ce9ba3 100644". After that the kernel compiled just fine and the problems were gone (still are).
I haven't yet tried it vice versa - it was a bit late yesterday. But I'll give it a try somewhat later today and let you know.
Personally, I could live without *both* of them.
So long! Rainer Fiebig
On Sat, Nov 18, 2017 at 05:08:20PM +0100, Rainer Fiebig wrote:
Greg KH wrote:
On Sat, Nov 18, 2017 at 01:47:32PM +0100, Rainer Fiebig wrote:
Hi!
Hopefully the right addressee.
Encountered two bad backports which cause screen-flicker. dmesg shows:
... [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun ...
CPU: Intel Core i3 (Clarkdale/Ironlake)
The backports are:
- diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c index 49de476..277a802 100644
- diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h index a19ec06..3ce9ba3 100644
After reversing them the flicker is gone, no more messages in dmesg. All else OK so far.
So which commit was the one that caused the problem? I will be glad to revert it.
thanks,
greg k-h
I started by reverting the more complex one first ("index 49de476..277a802100644"). But the kernel wouldn't compile then.
What git commit id is that? I don't see those ids in the 4.9-stable tree.
So I also reverted "index a19ec06..3ce9ba3 100644". After that the kernel compiled just fine and the problems were gone (still are).
Same here, what git commit id was this?
thanks,
greg k-h
Greg KH wrote:
On Sat, Nov 18, 2017 at 05:08:20PM +0100, Rainer Fiebig wrote:
Greg KH wrote:
On Sat, Nov 18, 2017 at 01:47:32PM +0100, Rainer Fiebig wrote:
Hi!
Hopefully the right addressee.
Encountered two bad backports which cause screen-flicker. dmesg shows:
... [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun ...
CPU: Intel Core i3 (Clarkdale/Ironlake)
The backports are:
- diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c index 49de476..277a802 100644
- diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h index a19ec06..3ce9ba3 100644
After reversing them the flicker is gone, no more messages in dmesg. All else OK so far.
So which commit was the one that caused the problem? I will be glad to revert it.
thanks,
greg k-h
I started by reverting the more complex one first ("index 49de476..277a802100644"). But the kernel wouldn't compile then.
What git commit id is that? I don't see those ids in the 4.9-stable tree.
So I also reverted "index a19ec06..3ce9ba3 100644". After that the kernel compiled just fine and the problems were gone (still are).
Same here, what git commit id was this?
thanks,
greg k-h
Please wait a moment. I'm checking whether I've made a mistake here.
Greg KH wrote:
On Sat, Nov 18, 2017 at 05:08:20PM +0100, Rainer Fiebig wrote:
Greg KH wrote:
On Sat, Nov 18, 2017 at 01:47:32PM +0100, Rainer Fiebig wrote:
Hi!
Hopefully the right addressee.
Encountered two bad backports which cause screen-flicker. dmesg shows:
... [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun ...
CPU: Intel Core i3 (Clarkdale/Ironlake)
The backports are:
- diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c index 49de476..277a802 100644
- diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h index a19ec06..3ce9ba3 100644
After reversing them the flicker is gone, no more messages in dmesg. All else OK so far.
So which commit was the one that caused the problem? I will be glad to revert it.
thanks,
greg k-h
I started by reverting the more complex one first ("index 49de476..277a802100644"). But the kernel wouldn't compile then.
What git commit id is that? I don't see those ids in the 4.9-stable tree.
So I also reverted "index a19ec06..3ce9ba3 100644". After that the kernel compiled just fine and the problems were gone (still are).
Same here, what git commit id was this?
thanks,
greg k-h
OK, no mistake. IIRC, I took the patches (and the IDs) from the changelog for patch-4.9.62. I've attached both, so you can check yourself.
I've also applied a freshly downloaded patch-4.9.62 to a freshly expanded 4.9 and re-compiled. The flicker is there. I haven't yet reverted the two patches but I'm confident that after having done so the flicker will be gone. If not I'll let you know.
As a good news: 4.14 is *not* affected. So to me it seems those two patches are part of sort of a package and can not be backported alone.
So long! Rainer Fiebig
On Sun, Nov 19, 2017 at 12:56:26PM +0100, Rainer Fiebig wrote:
Greg KH wrote:
On Sat, Nov 18, 2017 at 05:08:20PM +0100, Rainer Fiebig wrote:
Greg KH wrote:
On Sat, Nov 18, 2017 at 01:47:32PM +0100, Rainer Fiebig wrote:
Hi!
Hopefully the right addressee.
Encountered two bad backports which cause screen-flicker. dmesg shows:
... [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun ...
CPU: Intel Core i3 (Clarkdale/Ironlake)
The backports are:
- diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c index 49de476..277a802 100644
- diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h index a19ec06..3ce9ba3 100644
After reversing them the flicker is gone, no more messages in dmesg. All else OK so far.
So which commit was the one that caused the problem? I will be glad to revert it.
thanks,
greg k-h
I started by reverting the more complex one first ("index 49de476..277a802100644"). But the kernel wouldn't compile then.
What git commit id is that? I don't see those ids in the 4.9-stable tree.
So I also reverted "index a19ec06..3ce9ba3 100644". After that the kernel compiled just fine and the problems were gone (still are).
Same here, what git commit id was this?
thanks,
greg k-h
OK, no mistake. IIRC, I took the patches (and the IDs) from the changelog for patch-4.9.62. I've attached both, so you can check yourself.
I've also applied a freshly downloaded patch-4.9.62 to a freshly expanded 4.9 and re-compiled. The flicker is there. I haven't yet reverted the two patches but I'm confident that after having done so the flicker will be gone. If not I'll let you know.
As a good news: 4.14 is *not* affected. So to me it seems those two patches are part of sort of a package and can not be backported alone.
So long! Rainer Fiebig
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 49de476..277a802 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -27,6 +27,7 @@ #include <linux/cpufreq.h> #include <drm/drm_plane_helper.h> +#include <drm/drm_atomic_helper.h> #include "i915_drv.h" #include "intel_drv.h" #include "../../../platform/x86/intel_ips.h" @@ -2017,9 +2018,9 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv, const struct intel_crtc *intel_crtc, int level, struct intel_crtc_state *cstate,
struct intel_plane_state *pristate,struct intel_plane_state *sprstate,struct intel_plane_state *curstate,
const struct intel_plane_state *pristate,const struct intel_plane_state *sprstate,const struct intel_plane_state *curstate, struct intel_wm_level *result){ uint16_t pri_latency = dev_priv->wm.pri_latency[level]; @@ -2341,28 +2342,24 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) struct intel_pipe_wm *pipe_wm; struct drm_device *dev = state->dev; const struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_plane *intel_plane;
- struct intel_plane_state *pristate = NULL;
- struct intel_plane_state *sprstate = NULL;
- struct intel_plane_state *curstate = NULL;
- struct drm_plane *plane;
- const struct drm_plane_state *plane_state;
- const struct intel_plane_state *pristate = NULL;
- const struct intel_plane_state *sprstate = NULL;
- const struct intel_plane_state *curstate = NULL; int level, max_level = ilk_wm_max_level(dev), usable_level; struct ilk_wm_maximums max;
pipe_wm = &cstate->wm.ilk.optimal;
- for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
struct intel_plane_state *ps;
- drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, &cstate->base) {
const struct intel_plane_state *ps = to_intel_plane_state(plane_state);
ps = intel_atomic_get_existing_plane_state(state,intel_plane);if (!ps)continue;if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY)
if (plane->type == DRM_PLANE_TYPE_PRIMARY) pristate = ps;
else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY)
else if (plane->type == DRM_PLANE_TYPE_OVERLAY) sprstate = ps;
else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR)
else if (plane->type == DRM_PLANE_TYPE_CURSOR) curstate = ps;@@ -2384,11 +2381,9 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) if (pipe_wm->sprites_scaled) usable_level = 0;
- ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate,
pristate, sprstate, curstate, &pipe_wm->raw_wm[0]);- memset(&pipe_wm->wm, 0, sizeof(pipe_wm->wm));
- pipe_wm->wm[0] = pipe_wm->raw_wm[0];
- ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate,
pristate, sprstate, curstate, &pipe_wm->wm[0]);if (IS_HASWELL(dev) || IS_BROADWELL(dev)) pipe_wm->linetime = hsw_compute_linetime_wm(cstate); @@ -2398,8 +2393,8 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) ilk_compute_wm_reg_maximums(dev, 1, &max);
- for (level = 1; level <= max_level; level++) {
struct intel_wm_level *wm = &pipe_wm->raw_wm[level];
- for (level = 1; level <= usable_level; level++) {
struct intel_wm_level *wm = &pipe_wm->wm[level];ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, pristate, sprstate, curstate, wm); @@ -2409,13 +2404,10 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) * register maximums since such watermarks are * always invalid. */
if (level > usable_level)continue;if (ilk_validate_wm_level(level, &max, wm))pipe_wm->wm[level] = *wm;elseusable_level = level;
if (!ilk_validate_wm_level(level, &max, wm)) {memset(wm, 0, sizeof(*wm));break;}return 0;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index a19ec06..3ce9ba3 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -457,7 +457,6 @@ struct intel_crtc_scaler_state { struct intel_pipe_wm { struct intel_wm_level wm[5];
- struct intel_wm_level raw_wm[5]; uint32_t linetime; bool fbc_wm_enabled; bool pipe_enabled;
Ok, so this looks like commit 8777b927b92cf5b6c29f9f9d3c737addea9ac8a7 upstream which is commit 7de694782cbe7840f2c0de6f1e70f41fc1b8b6e8 in 4.9.62.
I've cc:ed the authors of that patch now.
Maarten, any hints? Should I revert this from 4.9-stable, or was there a follow-on patch that resolved this issue in mainline?
thanks,
greg k-h
Greg KH wrote:
On Sun, Nov 19, 2017 at 12:56:26PM +0100, Rainer Fiebig wrote:
Greg KH wrote:
On Sat, Nov 18, 2017 at 05:08:20PM +0100, Rainer Fiebig wrote:
Greg KH wrote:
On Sat, Nov 18, 2017 at 01:47:32PM +0100, Rainer Fiebig wrote:
Hi!
Hopefully the right addressee.
Encountered two bad backports which cause screen-flicker. dmesg shows:
... [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun ...
CPU: Intel Core i3 (Clarkdale/Ironlake)
The backports are:
- diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c index 49de476..277a802 100644
- diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h index a19ec06..3ce9ba3 100644
After reversing them the flicker is gone, no more messages in dmesg. All else OK so far.
So which commit was the one that caused the problem? I will be glad to revert it.
thanks,
greg k-h
I started by reverting the more complex one first ("index 49de476..277a802100644"). But the kernel wouldn't compile then.
What git commit id is that? I don't see those ids in the 4.9-stable tree.
So I also reverted "index a19ec06..3ce9ba3 100644". After that the kernel compiled just fine and the problems were gone (still are).
Same here, what git commit id was this?
thanks,
greg k-h
OK, no mistake. IIRC, I took the patches (and the IDs) from the changelog for patch-4.9.62. I've attached both, so you can check yourself.
I've also applied a freshly downloaded patch-4.9.62 to a freshly expanded 4.9 and re-compiled. The flicker is there. I haven't yet reverted the two patches but I'm confident that after having done so the flicker will be gone. If not I'll let you know.
As a good news: 4.14 is *not* affected. So to me it seems those two patches are part of sort of a package and can not be backported alone.
So long! Rainer Fiebig
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 49de476..277a802 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -27,6 +27,7 @@ #include <linux/cpufreq.h> #include <drm/drm_plane_helper.h> +#include <drm/drm_atomic_helper.h> #include "i915_drv.h" #include "intel_drv.h" #include "../../../platform/x86/intel_ips.h" @@ -2017,9 +2018,9 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv, const struct intel_crtc *intel_crtc, int level, struct intel_crtc_state *cstate,
struct intel_plane_state *pristate,struct intel_plane_state *sprstate,struct intel_plane_state *curstate,
const struct intel_plane_state *pristate,const struct intel_plane_state *sprstate,const struct intel_plane_state *curstate, struct intel_wm_level *result){ uint16_t pri_latency = dev_priv->wm.pri_latency[level]; @@ -2341,28 +2342,24 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) struct intel_pipe_wm *pipe_wm; struct drm_device *dev = state->dev; const struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_plane *intel_plane;
- struct intel_plane_state *pristate = NULL;
- struct intel_plane_state *sprstate = NULL;
- struct intel_plane_state *curstate = NULL;
- struct drm_plane *plane;
- const struct drm_plane_state *plane_state;
- const struct intel_plane_state *pristate = NULL;
- const struct intel_plane_state *sprstate = NULL;
- const struct intel_plane_state *curstate = NULL; int level, max_level = ilk_wm_max_level(dev), usable_level; struct ilk_wm_maximums max;
pipe_wm = &cstate->wm.ilk.optimal;
- for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
struct intel_plane_state *ps;
- drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, &cstate->base) {
const struct intel_plane_state *ps = to_intel_plane_state(plane_state);
ps = intel_atomic_get_existing_plane_state(state,intel_plane);if (!ps)continue;if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY)
if (plane->type == DRM_PLANE_TYPE_PRIMARY) pristate = ps;
else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY)
else if (plane->type == DRM_PLANE_TYPE_OVERLAY) sprstate = ps;
else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR)
else if (plane->type == DRM_PLANE_TYPE_CURSOR) curstate = ps;@@ -2384,11 +2381,9 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) if (pipe_wm->sprites_scaled) usable_level = 0;
- ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate,
pristate, sprstate, curstate, &pipe_wm->raw_wm[0]);- memset(&pipe_wm->wm, 0, sizeof(pipe_wm->wm));
- pipe_wm->wm[0] = pipe_wm->raw_wm[0];
- ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate,
pristate, sprstate, curstate, &pipe_wm->wm[0]);if (IS_HASWELL(dev) || IS_BROADWELL(dev)) pipe_wm->linetime = hsw_compute_linetime_wm(cstate); @@ -2398,8 +2393,8 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) ilk_compute_wm_reg_maximums(dev, 1, &max);
- for (level = 1; level <= max_level; level++) {
struct intel_wm_level *wm = &pipe_wm->raw_wm[level];
- for (level = 1; level <= usable_level; level++) {
struct intel_wm_level *wm = &pipe_wm->wm[level];ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, pristate, sprstate, curstate, wm); @@ -2409,13 +2404,10 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) * register maximums since such watermarks are * always invalid. */
if (level > usable_level)continue;if (ilk_validate_wm_level(level, &max, wm))pipe_wm->wm[level] = *wm;elseusable_level = level;
if (!ilk_validate_wm_level(level, &max, wm)) {memset(wm, 0, sizeof(*wm));break;}return 0;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index a19ec06..3ce9ba3 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -457,7 +457,6 @@ struct intel_crtc_scaler_state { struct intel_pipe_wm { struct intel_wm_level wm[5];
- struct intel_wm_level raw_wm[5]; uint32_t linetime; bool fbc_wm_enabled; bool pipe_enabled;
Ok, so this looks like commit 8777b927b92cf5b6c29f9f9d3c737addea9ac8a7 upstream which is commit 7de694782cbe7840f2c0de6f1e70f41fc1b8b6e8 in 4.9.62.
I've cc:ed the authors of that patch now.
Maarten, any hints? Should I revert this from 4.9-stable, or was there a follow-on patch that resolved this issue in mainline?
thanks,
greg k-h
OK, after reverting the patches, the flicker *is* gone.
BTW (for the future): Was it the right way to address stable@vger.kernel.org in this matter or would the bugreport at freedesktop.org have been enough? I'm a bit unsure about that.
Thanks. Rainer Fiebig
On Sun, Nov 19, 2017 at 01:44:06PM +0100, Rainer Fiebig wrote:
Greg KH wrote:
On Sun, Nov 19, 2017 at 12:56:26PM +0100, Rainer Fiebig wrote:
Greg KH wrote:
On Sat, Nov 18, 2017 at 05:08:20PM +0100, Rainer Fiebig wrote:
Greg KH wrote:
On Sat, Nov 18, 2017 at 01:47:32PM +0100, Rainer Fiebig wrote: > Hi! > > Hopefully the right addressee. > > Encountered two bad backports which cause screen-flicker. > dmesg shows: > > ... > [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun > [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun > [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun > [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun > ... > > CPU: Intel Core i3 (Clarkdale/Ironlake) > > The backports are: > > - diff --git a/drivers/gpu/drm/i915/intel_pm.c > b/drivers/gpu/drm/i915/intel_pm.c > index 49de476..277a802 100644 > - diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index a19ec06..3ce9ba3 100644 > > After reversing them the flicker is gone, no more messages in dmesg. All > else OK so far.
So which commit was the one that caused the problem? I will be glad to revert it.
thanks,
greg k-h
I started by reverting the more complex one first ("index 49de476..277a802100644"). But the kernel wouldn't compile then.
What git commit id is that? I don't see those ids in the 4.9-stable tree.
So I also reverted "index a19ec06..3ce9ba3 100644". After that the kernel compiled just fine and the problems were gone (still are).
Same here, what git commit id was this?
thanks,
greg k-h
OK, no mistake. IIRC, I took the patches (and the IDs) from the changelog for patch-4.9.62. I've attached both, so you can check yourself.
I've also applied a freshly downloaded patch-4.9.62 to a freshly expanded 4.9 and re-compiled. The flicker is there. I haven't yet reverted the two patches but I'm confident that after having done so the flicker will be gone. If not I'll let you know.
As a good news: 4.14 is *not* affected. So to me it seems those two patches are part of sort of a package and can not be backported alone.
So long! Rainer Fiebig
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 49de476..277a802 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -27,6 +27,7 @@ #include <linux/cpufreq.h> #include <drm/drm_plane_helper.h> +#include <drm/drm_atomic_helper.h> #include "i915_drv.h" #include "intel_drv.h" #include "../../../platform/x86/intel_ips.h" @@ -2017,9 +2018,9 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv, const struct intel_crtc *intel_crtc, int level, struct intel_crtc_state *cstate,
struct intel_plane_state *pristate,struct intel_plane_state *sprstate,struct intel_plane_state *curstate,
const struct intel_plane_state *pristate,const struct intel_plane_state *sprstate,const struct intel_plane_state *curstate, struct intel_wm_level *result){ uint16_t pri_latency = dev_priv->wm.pri_latency[level]; @@ -2341,28 +2342,24 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) struct intel_pipe_wm *pipe_wm; struct drm_device *dev = state->dev; const struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_plane *intel_plane;
- struct intel_plane_state *pristate = NULL;
- struct intel_plane_state *sprstate = NULL;
- struct intel_plane_state *curstate = NULL;
- struct drm_plane *plane;
- const struct drm_plane_state *plane_state;
- const struct intel_plane_state *pristate = NULL;
- const struct intel_plane_state *sprstate = NULL;
- const struct intel_plane_state *curstate = NULL; int level, max_level = ilk_wm_max_level(dev), usable_level; struct ilk_wm_maximums max;
pipe_wm = &cstate->wm.ilk.optimal;
- for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
struct intel_plane_state *ps;
- drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, &cstate->base) {
const struct intel_plane_state *ps = to_intel_plane_state(plane_state);
ps = intel_atomic_get_existing_plane_state(state,intel_plane);if (!ps)continue;if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY)
if (plane->type == DRM_PLANE_TYPE_PRIMARY) pristate = ps;
else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY)
else if (plane->type == DRM_PLANE_TYPE_OVERLAY) sprstate = ps;
else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR)
else if (plane->type == DRM_PLANE_TYPE_CURSOR) curstate = ps;@@ -2384,11 +2381,9 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) if (pipe_wm->sprites_scaled) usable_level = 0;
- ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate,
pristate, sprstate, curstate, &pipe_wm->raw_wm[0]);- memset(&pipe_wm->wm, 0, sizeof(pipe_wm->wm));
- pipe_wm->wm[0] = pipe_wm->raw_wm[0];
- ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate,
pristate, sprstate, curstate, &pipe_wm->wm[0]);if (IS_HASWELL(dev) || IS_BROADWELL(dev)) pipe_wm->linetime = hsw_compute_linetime_wm(cstate); @@ -2398,8 +2393,8 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) ilk_compute_wm_reg_maximums(dev, 1, &max);
- for (level = 1; level <= max_level; level++) {
struct intel_wm_level *wm = &pipe_wm->raw_wm[level];
- for (level = 1; level <= usable_level; level++) {
struct intel_wm_level *wm = &pipe_wm->wm[level];ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, pristate, sprstate, curstate, wm); @@ -2409,13 +2404,10 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) * register maximums since such watermarks are * always invalid. */
if (level > usable_level)continue;if (ilk_validate_wm_level(level, &max, wm))pipe_wm->wm[level] = *wm;elseusable_level = level;
if (!ilk_validate_wm_level(level, &max, wm)) {memset(wm, 0, sizeof(*wm));break;}return 0;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index a19ec06..3ce9ba3 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -457,7 +457,6 @@ struct intel_crtc_scaler_state { struct intel_pipe_wm { struct intel_wm_level wm[5];
- struct intel_wm_level raw_wm[5]; uint32_t linetime; bool fbc_wm_enabled; bool pipe_enabled;
Ok, so this looks like commit 8777b927b92cf5b6c29f9f9d3c737addea9ac8a7 upstream which is commit 7de694782cbe7840f2c0de6f1e70f41fc1b8b6e8 in 4.9.62.
I've cc:ed the authors of that patch now.
Maarten, any hints? Should I revert this from 4.9-stable, or was there a follow-on patch that resolved this issue in mainline?
thanks,
greg k-h
OK, after reverting the patches, the flicker *is* gone.
Thanks for confirming this.
BTW (for the future): Was it the right way to address stable@vger.kernel.org in this matter or would the bugreport at freedesktop.org have been enough? I'm a bit unsure about that.
I have no idea what the i915 developers want, but as far as I'm concerned, sending this to stable@vger was fine with me, I have no problem doing a bit of work in tracking down the specific patch before bugging the developers involved.
thanks,
greg k-h
On Sun, 19 Nov 2017, Greg KH gregkh@linuxfoundation.org wrote:
On Sun, Nov 19, 2017 at 01:44:06PM +0100, Rainer Fiebig wrote:
Greg KH wrote:
On Sun, Nov 19, 2017 at 12:56:26PM +0100, Rainer Fiebig wrote:
Greg KH wrote:
On Sat, Nov 18, 2017 at 05:08:20PM +0100, Rainer Fiebig wrote:
Greg KH wrote: > On Sat, Nov 18, 2017 at 01:47:32PM +0100, Rainer Fiebig wrote: >> Hi! >> >> Hopefully the right addressee. >> >> Encountered two bad backports which cause screen-flicker. >> dmesg shows: >> >> ... >> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun >> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun >> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun >> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun >> ... >> >> CPU: Intel Core i3 (Clarkdale/Ironlake) >> >> The backports are: >> >> - diff --git a/drivers/gpu/drm/i915/intel_pm.c >> b/drivers/gpu/drm/i915/intel_pm.c >> index 49de476..277a802 100644 >> - diff --git a/drivers/gpu/drm/i915/intel_drv.h >> b/drivers/gpu/drm/i915/intel_drv.h >> index a19ec06..3ce9ba3 100644 >> >> After reversing them the flicker is gone, no more messages in dmesg. All >> else OK so far. > > So which commit was the one that caused the problem? I will be glad to > revert it. > > thanks, > > greg k-h > >
I started by reverting the more complex one first ("index 49de476..277a802100644"). But the kernel wouldn't compile then.
What git commit id is that? I don't see those ids in the 4.9-stable tree.
So I also reverted "index a19ec06..3ce9ba3 100644". After that the kernel compiled just fine and the problems were gone (still are).
Same here, what git commit id was this?
thanks,
greg k-h
OK, no mistake. IIRC, I took the patches (and the IDs) from the changelog for patch-4.9.62. I've attached both, so you can check yourself.
I've also applied a freshly downloaded patch-4.9.62 to a freshly expanded 4.9 and re-compiled. The flicker is there. I haven't yet reverted the two patches but I'm confident that after having done so the flicker will be gone. If not I'll let you know.
As a good news: 4.14 is *not* affected. So to me it seems those two patches are part of sort of a package and can not be backported alone.
So long! Rainer Fiebig
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 49de476..277a802 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -27,6 +27,7 @@ #include <linux/cpufreq.h> #include <drm/drm_plane_helper.h> +#include <drm/drm_atomic_helper.h> #include "i915_drv.h" #include "intel_drv.h" #include "../../../platform/x86/intel_ips.h" @@ -2017,9 +2018,9 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv, const struct intel_crtc *intel_crtc, int level, struct intel_crtc_state *cstate,
struct intel_plane_state *pristate,struct intel_plane_state *sprstate,struct intel_plane_state *curstate,
const struct intel_plane_state *pristate,const struct intel_plane_state *sprstate,const struct intel_plane_state *curstate, struct intel_wm_level *result){ uint16_t pri_latency = dev_priv->wm.pri_latency[level]; @@ -2341,28 +2342,24 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) struct intel_pipe_wm *pipe_wm; struct drm_device *dev = state->dev; const struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_plane *intel_plane;
- struct intel_plane_state *pristate = NULL;
- struct intel_plane_state *sprstate = NULL;
- struct intel_plane_state *curstate = NULL;
- struct drm_plane *plane;
- const struct drm_plane_state *plane_state;
- const struct intel_plane_state *pristate = NULL;
- const struct intel_plane_state *sprstate = NULL;
- const struct intel_plane_state *curstate = NULL; int level, max_level = ilk_wm_max_level(dev), usable_level; struct ilk_wm_maximums max;
pipe_wm = &cstate->wm.ilk.optimal;
- for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
struct intel_plane_state *ps;
- drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, &cstate->base) {
const struct intel_plane_state *ps = to_intel_plane_state(plane_state);
ps = intel_atomic_get_existing_plane_state(state,intel_plane);if (!ps)continue;if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY)
if (plane->type == DRM_PLANE_TYPE_PRIMARY) pristate = ps;
else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY)
else if (plane->type == DRM_PLANE_TYPE_OVERLAY) sprstate = ps;
else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR)
else if (plane->type == DRM_PLANE_TYPE_CURSOR) curstate = ps;@@ -2384,11 +2381,9 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) if (pipe_wm->sprites_scaled) usable_level = 0;
- ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate,
pristate, sprstate, curstate, &pipe_wm->raw_wm[0]);- memset(&pipe_wm->wm, 0, sizeof(pipe_wm->wm));
- pipe_wm->wm[0] = pipe_wm->raw_wm[0];
- ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate,
pristate, sprstate, curstate, &pipe_wm->wm[0]);if (IS_HASWELL(dev) || IS_BROADWELL(dev)) pipe_wm->linetime = hsw_compute_linetime_wm(cstate); @@ -2398,8 +2393,8 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) ilk_compute_wm_reg_maximums(dev, 1, &max);
- for (level = 1; level <= max_level; level++) {
struct intel_wm_level *wm = &pipe_wm->raw_wm[level];
- for (level = 1; level <= usable_level; level++) {
struct intel_wm_level *wm = &pipe_wm->wm[level];ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, pristate, sprstate, curstate, wm); @@ -2409,13 +2404,10 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) * register maximums since such watermarks are * always invalid. */
if (level > usable_level)continue;if (ilk_validate_wm_level(level, &max, wm))pipe_wm->wm[level] = *wm;elseusable_level = level;
if (!ilk_validate_wm_level(level, &max, wm)) {memset(wm, 0, sizeof(*wm));break;}return 0;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index a19ec06..3ce9ba3 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -457,7 +457,6 @@ struct intel_crtc_scaler_state { struct intel_pipe_wm { struct intel_wm_level wm[5];
- struct intel_wm_level raw_wm[5]; uint32_t linetime; bool fbc_wm_enabled; bool pipe_enabled;
Ok, so this looks like commit 8777b927b92cf5b6c29f9f9d3c737addea9ac8a7 upstream which is commit 7de694782cbe7840f2c0de6f1e70f41fc1b8b6e8 in 4.9.62.
I've cc:ed the authors of that patch now.
Maarten, any hints? Should I revert this from 4.9-stable, or was there a follow-on patch that resolved this issue in mainline?
thanks,
greg k-h
OK, after reverting the patches, the flicker *is* gone.
Thanks for confirming this.
BTW (for the future): Was it the right way to address stable@vger.kernel.org in this matter or would the bugreport at freedesktop.org have been enough? I'm a bit unsure about that.
I have no idea what the i915 developers want, but as far as I'm concerned, sending this to stable@vger was fine with me, I have no problem doing a bit of work in tracking down the specific patch before bugging the developers involved.
Well, this one we wanted to be backported, and so indicated with cc: stable, but apparently it went south anyway. :(
Rainer, does v4.14 work for you? I.e. is the commit okay or not before the backport?
Maarten?
BR, Jani.
Jani Nikula wrote:
On Sun, 19 Nov 2017, Greg KH gregkh@linuxfoundation.org wrote:
On Sun, Nov 19, 2017 at 01:44:06PM +0100, Rainer Fiebig wrote:
Greg KH wrote:
On Sun, Nov 19, 2017 at 12:56:26PM +0100, Rainer Fiebig wrote:
Greg KH wrote:
On Sat, Nov 18, 2017 at 05:08:20PM +0100, Rainer Fiebig wrote: > Greg KH wrote: >> On Sat, Nov 18, 2017 at 01:47:32PM +0100, Rainer Fiebig wrote: >>> Hi! >>> >>> Hopefully the right addressee. >>> >>> Encountered two bad backports which cause screen-flicker. >>> dmesg shows: >>> >>> ... >>> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun >>> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun >>> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun >>> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun >>> ... >>> >>> CPU: Intel Core i3 (Clarkdale/Ironlake) >>> >>> The backports are: >>> >>> - diff --git a/drivers/gpu/drm/i915/intel_pm.c >>> b/drivers/gpu/drm/i915/intel_pm.c >>> index 49de476..277a802 100644 >>> - diff --git a/drivers/gpu/drm/i915/intel_drv.h >>> b/drivers/gpu/drm/i915/intel_drv.h >>> index a19ec06..3ce9ba3 100644 >>> >>> After reversing them the flicker is gone, no more messages in dmesg. All >>> else OK so far. >> >> So which commit was the one that caused the problem? I will be glad to >> revert it. >> >> thanks, >> >> greg k-h >> >> > > I started by reverting the more complex one first ("index > 49de476..277a802100644"). But the kernel wouldn't compile then.
What git commit id is that? I don't see those ids in the 4.9-stable tree.
> So I also reverted "index a19ec06..3ce9ba3 100644". After that the > kernel compiled just fine and the problems were gone (still are).
Same here, what git commit id was this?
thanks,
greg k-h
OK, no mistake. IIRC, I took the patches (and the IDs) from the changelog for patch-4.9.62. I've attached both, so you can check yourself.
I've also applied a freshly downloaded patch-4.9.62 to a freshly expanded 4.9 and re-compiled. The flicker is there. I haven't yet reverted the two patches but I'm confident that after having done so the flicker will be gone. If not I'll let you know.
As a good news: 4.14 is *not* affected. So to me it seems those two patches are part of sort of a package and can not be backported alone.
So long! Rainer Fiebig
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 49de476..277a802 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -27,6 +27,7 @@ #include <linux/cpufreq.h> #include <drm/drm_plane_helper.h> +#include <drm/drm_atomic_helper.h> #include "i915_drv.h" #include "intel_drv.h" #include "../../../platform/x86/intel_ips.h" @@ -2017,9 +2018,9 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv, const struct intel_crtc *intel_crtc, int level, struct intel_crtc_state *cstate,
struct intel_plane_state *pristate,struct intel_plane_state *sprstate,struct intel_plane_state *curstate,
const struct intel_plane_state *pristate,const struct intel_plane_state *sprstate,const struct intel_plane_state *curstate, struct intel_wm_level *result){ uint16_t pri_latency = dev_priv->wm.pri_latency[level]; @@ -2341,28 +2342,24 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) struct intel_pipe_wm *pipe_wm; struct drm_device *dev = state->dev; const struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_plane *intel_plane;
- struct intel_plane_state *pristate = NULL;
- struct intel_plane_state *sprstate = NULL;
- struct intel_plane_state *curstate = NULL;
- struct drm_plane *plane;
- const struct drm_plane_state *plane_state;
- const struct intel_plane_state *pristate = NULL;
- const struct intel_plane_state *sprstate = NULL;
- const struct intel_plane_state *curstate = NULL; int level, max_level = ilk_wm_max_level(dev), usable_level; struct ilk_wm_maximums max;
pipe_wm = &cstate->wm.ilk.optimal;
- for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
struct intel_plane_state *ps;
- drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, &cstate->base) {
const struct intel_plane_state *ps = to_intel_plane_state(plane_state);
ps = intel_atomic_get_existing_plane_state(state,intel_plane);if (!ps)continue;if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY)
if (plane->type == DRM_PLANE_TYPE_PRIMARY) pristate = ps;
else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY)
else if (plane->type == DRM_PLANE_TYPE_OVERLAY) sprstate = ps;
else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR)
else if (plane->type == DRM_PLANE_TYPE_CURSOR) curstate = ps;@@ -2384,11 +2381,9 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) if (pipe_wm->sprites_scaled) usable_level = 0;
- ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate,
pristate, sprstate, curstate, &pipe_wm->raw_wm[0]);- memset(&pipe_wm->wm, 0, sizeof(pipe_wm->wm));
- pipe_wm->wm[0] = pipe_wm->raw_wm[0];
- ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate,
pristate, sprstate, curstate, &pipe_wm->wm[0]);if (IS_HASWELL(dev) || IS_BROADWELL(dev)) pipe_wm->linetime = hsw_compute_linetime_wm(cstate); @@ -2398,8 +2393,8 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) ilk_compute_wm_reg_maximums(dev, 1, &max);
- for (level = 1; level <= max_level; level++) {
struct intel_wm_level *wm = &pipe_wm->raw_wm[level];
- for (level = 1; level <= usable_level; level++) {
struct intel_wm_level *wm = &pipe_wm->wm[level];ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, pristate, sprstate, curstate, wm); @@ -2409,13 +2404,10 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) * register maximums since such watermarks are * always invalid. */
if (level > usable_level)continue;if (ilk_validate_wm_level(level, &max, wm))pipe_wm->wm[level] = *wm;elseusable_level = level;
if (!ilk_validate_wm_level(level, &max, wm)) {memset(wm, 0, sizeof(*wm));break;}return 0;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index a19ec06..3ce9ba3 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -457,7 +457,6 @@ struct intel_crtc_scaler_state { struct intel_pipe_wm { struct intel_wm_level wm[5];
- struct intel_wm_level raw_wm[5]; uint32_t linetime; bool fbc_wm_enabled; bool pipe_enabled;
Ok, so this looks like commit 8777b927b92cf5b6c29f9f9d3c737addea9ac8a7 upstream which is commit 7de694782cbe7840f2c0de6f1e70f41fc1b8b6e8 in 4.9.62.
I've cc:ed the authors of that patch now.
Maarten, any hints? Should I revert this from 4.9-stable, or was there a follow-on patch that resolved this issue in mainline?
thanks,
greg k-h
OK, after reverting the patches, the flicker *is* gone.
Thanks for confirming this.
BTW (for the future): Was it the right way to address stable@vger.kernel.org in this matter or would the bugreport at freedesktop.org have been enough? I'm a bit unsure about that.
I have no idea what the i915 developers want, but as far as I'm concerned, sending this to stable@vger was fine with me, I have no problem doing a bit of work in tracking down the specific patch before bugging the developers involved.
Well, this one we wanted to be backported, and so indicated with cc: stable, but apparently it went south anyway. :(
Rainer, does v4.14 work for you? I.e. is the commit okay or not before the backport?
Maarten?
BR, Jani.
4.14 is OK, no problems.
So long! Rainer Fiebig
Op 20-11-17 om 09:51 schreef Rainer Fiebig:
Jani Nikula wrote:
On Sun, 19 Nov 2017, Greg KH gregkh@linuxfoundation.org wrote:
On Sun, Nov 19, 2017 at 01:44:06PM +0100, Rainer Fiebig wrote:
Greg KH wrote:
On Sun, Nov 19, 2017 at 12:56:26PM +0100, Rainer Fiebig wrote:
Greg KH wrote: > On Sat, Nov 18, 2017 at 05:08:20PM +0100, Rainer Fiebig wrote: >> Greg KH wrote: >>> On Sat, Nov 18, 2017 at 01:47:32PM +0100, Rainer Fiebig wrote: >>>> Hi! >>>> >>>> Hopefully the right addressee. >>>> >>>> Encountered two bad backports which cause screen-flicker. >>>> dmesg shows: >>>> >>>> ... >>>> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun >>>> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun >>>> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun >>>> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun >>>> ... >>>> >>>> CPU: Intel Core i3 (Clarkdale/Ironlake) >>>> >>>> The backports are: >>>> >>>> - diff --git a/drivers/gpu/drm/i915/intel_pm.c >>>> b/drivers/gpu/drm/i915/intel_pm.c >>>> index 49de476..277a802 100644 >>>> - diff --git a/drivers/gpu/drm/i915/intel_drv.h >>>> b/drivers/gpu/drm/i915/intel_drv.h >>>> index a19ec06..3ce9ba3 100644 >>>> >>>> After reversing them the flicker is gone, no more messages in dmesg. All >>>> else OK so far. >>> So which commit was the one that caused the problem? I will be glad to >>> revert it. >>> >>> thanks, >>> >>> greg k-h >>> >>> >> I started by reverting the more complex one first ("index >> 49de476..277a802100644"). But the kernel wouldn't compile then. > What git commit id is that? I don't see those ids in the 4.9-stable > tree. > >> So I also reverted "index a19ec06..3ce9ba3 100644". After that the >> kernel compiled just fine and the problems were gone (still are). > Same here, what git commit id was this? > > thanks, > > greg k-h > OK, no mistake. IIRC, I took the patches (and the IDs) from the changelog for patch-4.9.62. I've attached both, so you can check yourself.
I've also applied a freshly downloaded patch-4.9.62 to a freshly expanded 4.9 and re-compiled. The flicker is there. I haven't yet reverted the two patches but I'm confident that after having done so the flicker will be gone. If not I'll let you know.
As a good news: 4.14 is *not* affected. So to me it seems those two patches are part of sort of a package and can not be backported alone.
So long! Rainer Fiebig diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 49de476..277a802 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -27,6 +27,7 @@ #include <linux/cpufreq.h> #include <drm/drm_plane_helper.h> +#include <drm/drm_atomic_helper.h> #include "i915_drv.h" #include "intel_drv.h" #include "../../../platform/x86/intel_ips.h" @@ -2017,9 +2018,9 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv, const struct intel_crtc *intel_crtc, int level, struct intel_crtc_state *cstate,
struct intel_plane_state *pristate,struct intel_plane_state *sprstate,struct intel_plane_state *curstate,
const struct intel_plane_state *pristate,const struct intel_plane_state *sprstate,const struct intel_plane_state *curstate, struct intel_wm_level *result){ uint16_t pri_latency = dev_priv->wm.pri_latency[level]; @@ -2341,28 +2342,24 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) struct intel_pipe_wm *pipe_wm; struct drm_device *dev = state->dev; const struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_plane *intel_plane;
- struct intel_plane_state *pristate = NULL;
- struct intel_plane_state *sprstate = NULL;
- struct intel_plane_state *curstate = NULL;
- struct drm_plane *plane;
- const struct drm_plane_state *plane_state;
- const struct intel_plane_state *pristate = NULL;
- const struct intel_plane_state *sprstate = NULL;
- const struct intel_plane_state *curstate = NULL; int level, max_level = ilk_wm_max_level(dev), usable_level; struct ilk_wm_maximums max;
pipe_wm = &cstate->wm.ilk.optimal;
- for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
struct intel_plane_state *ps;
- drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, &cstate->base) {
const struct intel_plane_state *ps = to_intel_plane_state(plane_state);
ps = intel_atomic_get_existing_plane_state(state,intel_plane);if (!ps)continue;if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY)
if (plane->type == DRM_PLANE_TYPE_PRIMARY) pristate = ps;
else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY)
else if (plane->type == DRM_PLANE_TYPE_OVERLAY) sprstate = ps;
else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR)
else if (plane->type == DRM_PLANE_TYPE_CURSOR) curstate = ps;@@ -2384,11 +2381,9 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) if (pipe_wm->sprites_scaled) usable_level = 0;
- ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate,
pristate, sprstate, curstate, &pipe_wm->raw_wm[0]);- memset(&pipe_wm->wm, 0, sizeof(pipe_wm->wm));
- pipe_wm->wm[0] = pipe_wm->raw_wm[0];
- ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate,
pristate, sprstate, curstate, &pipe_wm->wm[0]);if (IS_HASWELL(dev) || IS_BROADWELL(dev)) pipe_wm->linetime = hsw_compute_linetime_wm(cstate); @@ -2398,8 +2393,8 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) ilk_compute_wm_reg_maximums(dev, 1, &max);
- for (level = 1; level <= max_level; level++) {
struct intel_wm_level *wm = &pipe_wm->raw_wm[level];
- for (level = 1; level <= usable_level; level++) {
struct intel_wm_level *wm = &pipe_wm->wm[level];ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, pristate, sprstate, curstate, wm); @@ -2409,13 +2404,10 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) * register maximums since such watermarks are * always invalid. */
if (level > usable_level)continue;if (ilk_validate_wm_level(level, &max, wm))pipe_wm->wm[level] = *wm;elseusable_level = level;
if (!ilk_validate_wm_level(level, &max, wm)) {memset(wm, 0, sizeof(*wm));break;}return 0; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index a19ec06..3ce9ba3 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -457,7 +457,6 @@ struct intel_crtc_scaler_state { struct intel_pipe_wm { struct intel_wm_level wm[5];
- struct intel_wm_level raw_wm[5]; uint32_t linetime; bool fbc_wm_enabled; bool pipe_enabled;
Ok, so this looks like commit 8777b927b92cf5b6c29f9f9d3c737addea9ac8a7 upstream which is commit 7de694782cbe7840f2c0de6f1e70f41fc1b8b6e8 in 4.9.62.
I've cc:ed the authors of that patch now.
Maarten, any hints? Should I revert this from 4.9-stable, or was there a follow-on patch that resolved this issue in mainline?
thanks,
greg k-h
OK, after reverting the patches, the flicker *is* gone.
Thanks for confirming this.
BTW (for the future): Was it the right way to address stable@vger.kernel.org in this matter or would the bugreport at freedesktop.org have been enough? I'm a bit unsure about that.
I have no idea what the i915 developers want, but as far as I'm concerned, sending this to stable@vger was fine with me, I have no problem doing a bit of work in tracking down the specific patch before bugging the developers involved.
Well, this one we wanted to be backported, and so indicated with cc: stable, but apparently it went south anyway. :(
Rainer, does v4.14 work for you? I.e. is the commit okay or not before the backport?
Maarten?
BR, Jani.
4.14 is OK, no problems.
So long! Rainer Fiebig
What happens when you apply both other backported patches on top?
Maarten Lankhorst wrote:
Op 20-11-17 om 09:51 schreef Rainer Fiebig:
Jani Nikula wrote:
On Sun, 19 Nov 2017, Greg KH gregkh@linuxfoundation.org wrote:
On Sun, Nov 19, 2017 at 01:44:06PM +0100, Rainer Fiebig wrote:
Greg KH wrote:
On Sun, Nov 19, 2017 at 12:56:26PM +0100, Rainer Fiebig wrote: > Greg KH wrote: >> On Sat, Nov 18, 2017 at 05:08:20PM +0100, Rainer Fiebig wrote: >>> Greg KH wrote: >>>> On Sat, Nov 18, 2017 at 01:47:32PM +0100, Rainer Fiebig wrote: >>>>> Hi! >>>>> >>>>> Hopefully the right addressee. >>>>> >>>>> Encountered two bad backports which cause screen-flicker. >>>>> dmesg shows: >>>>> >>>>> ... >>>>> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun >>>>> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun >>>>> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun >>>>> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun >>>>> ... >>>>> >>>>> CPU: Intel Core i3 (Clarkdale/Ironlake) >>>>> >>>>> The backports are: >>>>> >>>>> - diff --git a/drivers/gpu/drm/i915/intel_pm.c >>>>> b/drivers/gpu/drm/i915/intel_pm.c >>>>> index 49de476..277a802 100644 >>>>> - diff --git a/drivers/gpu/drm/i915/intel_drv.h >>>>> b/drivers/gpu/drm/i915/intel_drv.h >>>>> index a19ec06..3ce9ba3 100644 >>>>> >>>>> After reversing them the flicker is gone, no more messages in dmesg. All >>>>> else OK so far. >>>> So which commit was the one that caused the problem? I will be glad to >>>> revert it. >>>> >>>> thanks, >>>> >>>> greg k-h >>>> >>>> >>> I started by reverting the more complex one first ("index >>> 49de476..277a802100644"). But the kernel wouldn't compile then. >> What git commit id is that? I don't see those ids in the 4.9-stable >> tree. >> >>> So I also reverted "index a19ec06..3ce9ba3 100644". After that the >>> kernel compiled just fine and the problems were gone (still are). >> Same here, what git commit id was this? >> >> thanks, >> >> greg k-h >> > OK, no mistake. IIRC, I took the patches (and the IDs) from the > changelog for patch-4.9.62. I've attached both, so you can check yourself. > > I've also applied a freshly downloaded patch-4.9.62 to a freshly > expanded 4.9 and re-compiled. The flicker is there. I haven't yet > reverted the two patches but I'm confident that after having done so the > flicker will be gone. If not I'll let you know. > > As a good news: 4.14 is *not* affected. So to me it seems those two > patches are part of sort of a package and can not be backported alone. > > So long! > Rainer Fiebig > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 49de476..277a802 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -27,6 +27,7 @@ > > #include <linux/cpufreq.h> > #include <drm/drm_plane_helper.h> > +#include <drm/drm_atomic_helper.h> > #include "i915_drv.h" > #include "intel_drv.h" > #include "../../../platform/x86/intel_ips.h" > @@ -2017,9 +2018,9 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv, > const struct intel_crtc *intel_crtc, > int level, > struct intel_crtc_state *cstate, > - struct intel_plane_state *pristate, > - struct intel_plane_state *sprstate, > - struct intel_plane_state *curstate, > + const struct intel_plane_state *pristate, > + const struct intel_plane_state *sprstate, > + const struct intel_plane_state *curstate, > struct intel_wm_level *result) > { > uint16_t pri_latency = dev_priv->wm.pri_latency[level]; > @@ -2341,28 +2342,24 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) > struct intel_pipe_wm *pipe_wm; > struct drm_device *dev = state->dev; > const struct drm_i915_private *dev_priv = to_i915(dev); > - struct intel_plane *intel_plane; > - struct intel_plane_state *pristate = NULL; > - struct intel_plane_state *sprstate = NULL; > - struct intel_plane_state *curstate = NULL; > + struct drm_plane *plane; > + const struct drm_plane_state *plane_state; > + const struct intel_plane_state *pristate = NULL; > + const struct intel_plane_state *sprstate = NULL; > + const struct intel_plane_state *curstate = NULL; > int level, max_level = ilk_wm_max_level(dev), usable_level; > struct ilk_wm_maximums max; > > pipe_wm = &cstate->wm.ilk.optimal; > > - for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) { > - struct intel_plane_state *ps; > + drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, &cstate->base) { > + const struct intel_plane_state *ps = to_intel_plane_state(plane_state); > > - ps = intel_atomic_get_existing_plane_state(state, > - intel_plane); > - if (!ps) > - continue; > - > - if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY) > + if (plane->type == DRM_PLANE_TYPE_PRIMARY) > pristate = ps; > - else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY) > + else if (plane->type == DRM_PLANE_TYPE_OVERLAY) > sprstate = ps; > - else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR) > + else if (plane->type == DRM_PLANE_TYPE_CURSOR) > curstate = ps; > } > > @@ -2384,11 +2381,9 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) > if (pipe_wm->sprites_scaled) > usable_level = 0; > > - ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, > - pristate, sprstate, curstate, &pipe_wm->raw_wm[0]); > - > memset(&pipe_wm->wm, 0, sizeof(pipe_wm->wm)); > - pipe_wm->wm[0] = pipe_wm->raw_wm[0]; > + ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, > + pristate, sprstate, curstate, &pipe_wm->wm[0]); > > if (IS_HASWELL(dev) || IS_BROADWELL(dev)) > pipe_wm->linetime = hsw_compute_linetime_wm(cstate); > @@ -2398,8 +2393,8 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) > > ilk_compute_wm_reg_maximums(dev, 1, &max); > > - for (level = 1; level <= max_level; level++) { > - struct intel_wm_level *wm = &pipe_wm->raw_wm[level]; > + for (level = 1; level <= usable_level; level++) { > + struct intel_wm_level *wm = &pipe_wm->wm[level]; > > ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, > pristate, sprstate, curstate, wm); > @@ -2409,13 +2404,10 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) > * register maximums since such watermarks are > * always invalid. > */ > - if (level > usable_level) > - continue; > - > - if (ilk_validate_wm_level(level, &max, wm)) > - pipe_wm->wm[level] = *wm; > - else > - usable_level = level; > + if (!ilk_validate_wm_level(level, &max, wm)) { > + memset(wm, 0, sizeof(*wm)); > + break; > + } > } > > return 0; > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index a19ec06..3ce9ba3 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -457,7 +457,6 @@ struct intel_crtc_scaler_state { > > struct intel_pipe_wm { > struct intel_wm_level wm[5]; > - struct intel_wm_level raw_wm[5]; > uint32_t linetime; > bool fbc_wm_enabled; > bool pipe_enabled; Ok, so this looks like commit 8777b927b92cf5b6c29f9f9d3c737addea9ac8a7 upstream which is commit 7de694782cbe7840f2c0de6f1e70f41fc1b8b6e8 in 4.9.62.
I've cc:ed the authors of that patch now.
Maarten, any hints? Should I revert this from 4.9-stable, or was there a follow-on patch that resolved this issue in mainline?
thanks,
greg k-h
OK, after reverting the patches, the flicker *is* gone.
Thanks for confirming this.
BTW (for the future): Was it the right way to address stable@vger.kernel.org in this matter or would the bugreport at freedesktop.org have been enough? I'm a bit unsure about that.
I have no idea what the i915 developers want, but as far as I'm concerned, sending this to stable@vger was fine with me, I have no problem doing a bit of work in tracking down the specific patch before bugging the developers involved.
Well, this one we wanted to be backported, and so indicated with cc: stable, but apparently it went south anyway. :(
Rainer, does v4.14 work for you? I.e. is the commit okay or not before the backport?
Maarten?
BR, Jani.
4.14 is OK, no problems.
So long! Rainer Fiebig
What happens when you apply both other backported patches on top?
On top of 4.14?
Op 20-11-17 om 12:38 schreef Rainer Fiebig:
Maarten Lankhorst wrote:
Op 20-11-17 om 09:51 schreef Rainer Fiebig:
Jani Nikula wrote:
On Sun, 19 Nov 2017, Greg KH gregkh@linuxfoundation.org wrote:
On Sun, Nov 19, 2017 at 01:44:06PM +0100, Rainer Fiebig wrote:
Greg KH wrote: > On Sun, Nov 19, 2017 at 12:56:26PM +0100, Rainer Fiebig wrote: >> Greg KH wrote: >>> On Sat, Nov 18, 2017 at 05:08:20PM +0100, Rainer Fiebig wrote: >>>> Greg KH wrote: >>>>> On Sat, Nov 18, 2017 at 01:47:32PM +0100, Rainer Fiebig wrote: >>>>>> Hi! >>>>>> >>>>>> Hopefully the right addressee. >>>>>> >>>>>> Encountered two bad backports which cause screen-flicker. >>>>>> dmesg shows: >>>>>> >>>>>> ... >>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun >>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun >>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun >>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun >>>>>> ... >>>>>> >>>>>> CPU: Intel Core i3 (Clarkdale/Ironlake) >>>>>> >>>>>> The backports are: >>>>>> >>>>>> - diff --git a/drivers/gpu/drm/i915/intel_pm.c >>>>>> b/drivers/gpu/drm/i915/intel_pm.c >>>>>> index 49de476..277a802 100644 >>>>>> - diff --git a/drivers/gpu/drm/i915/intel_drv.h >>>>>> b/drivers/gpu/drm/i915/intel_drv.h >>>>>> index a19ec06..3ce9ba3 100644 >>>>>> >>>>>> After reversing them the flicker is gone, no more messages in dmesg. All >>>>>> else OK so far. >>>>> So which commit was the one that caused the problem? I will be glad to >>>>> revert it. >>>>> >>>>> thanks, >>>>> >>>>> greg k-h >>>>> >>>>> >>>> I started by reverting the more complex one first ("index >>>> 49de476..277a802100644"). But the kernel wouldn't compile then. >>> What git commit id is that? I don't see those ids in the 4.9-stable >>> tree. >>> >>>> So I also reverted "index a19ec06..3ce9ba3 100644". After that the >>>> kernel compiled just fine and the problems were gone (still are). >>> Same here, what git commit id was this? >>> >>> thanks, >>> >>> greg k-h >>> >> OK, no mistake. IIRC, I took the patches (and the IDs) from the >> changelog for patch-4.9.62. I've attached both, so you can check yourself. >> >> I've also applied a freshly downloaded patch-4.9.62 to a freshly >> expanded 4.9 and re-compiled. The flicker is there. I haven't yet >> reverted the two patches but I'm confident that after having done so the >> flicker will be gone. If not I'll let you know. >> >> As a good news: 4.14 is *not* affected. So to me it seems those two >> patches are part of sort of a package and can not be backported alone. >> >> So long! >> Rainer Fiebig >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >> index 49de476..277a802 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -27,6 +27,7 @@ >> >> #include <linux/cpufreq.h> >> #include <drm/drm_plane_helper.h> >> +#include <drm/drm_atomic_helper.h> >> #include "i915_drv.h" >> #include "intel_drv.h" >> #include "../../../platform/x86/intel_ips.h" >> @@ -2017,9 +2018,9 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv, >> const struct intel_crtc *intel_crtc, >> int level, >> struct intel_crtc_state *cstate, >> - struct intel_plane_state *pristate, >> - struct intel_plane_state *sprstate, >> - struct intel_plane_state *curstate, >> + const struct intel_plane_state *pristate, >> + const struct intel_plane_state *sprstate, >> + const struct intel_plane_state *curstate, >> struct intel_wm_level *result) >> { >> uint16_t pri_latency = dev_priv->wm.pri_latency[level]; >> @@ -2341,28 +2342,24 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >> struct intel_pipe_wm *pipe_wm; >> struct drm_device *dev = state->dev; >> const struct drm_i915_private *dev_priv = to_i915(dev); >> - struct intel_plane *intel_plane; >> - struct intel_plane_state *pristate = NULL; >> - struct intel_plane_state *sprstate = NULL; >> - struct intel_plane_state *curstate = NULL; >> + struct drm_plane *plane; >> + const struct drm_plane_state *plane_state; >> + const struct intel_plane_state *pristate = NULL; >> + const struct intel_plane_state *sprstate = NULL; >> + const struct intel_plane_state *curstate = NULL; >> int level, max_level = ilk_wm_max_level(dev), usable_level; >> struct ilk_wm_maximums max; >> >> pipe_wm = &cstate->wm.ilk.optimal; >> >> - for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) { >> - struct intel_plane_state *ps; >> + drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, &cstate->base) { >> + const struct intel_plane_state *ps = to_intel_plane_state(plane_state); >> >> - ps = intel_atomic_get_existing_plane_state(state, >> - intel_plane); >> - if (!ps) >> - continue; >> - >> - if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY) >> + if (plane->type == DRM_PLANE_TYPE_PRIMARY) >> pristate = ps; >> - else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY) >> + else if (plane->type == DRM_PLANE_TYPE_OVERLAY) >> sprstate = ps; >> - else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR) >> + else if (plane->type == DRM_PLANE_TYPE_CURSOR) >> curstate = ps; >> } >> >> @@ -2384,11 +2381,9 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >> if (pipe_wm->sprites_scaled) >> usable_level = 0; >> >> - ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, >> - pristate, sprstate, curstate, &pipe_wm->raw_wm[0]); >> - >> memset(&pipe_wm->wm, 0, sizeof(pipe_wm->wm)); >> - pipe_wm->wm[0] = pipe_wm->raw_wm[0]; >> + ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, >> + pristate, sprstate, curstate, &pipe_wm->wm[0]); >> >> if (IS_HASWELL(dev) || IS_BROADWELL(dev)) >> pipe_wm->linetime = hsw_compute_linetime_wm(cstate); >> @@ -2398,8 +2393,8 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >> >> ilk_compute_wm_reg_maximums(dev, 1, &max); >> >> - for (level = 1; level <= max_level; level++) { >> - struct intel_wm_level *wm = &pipe_wm->raw_wm[level]; >> + for (level = 1; level <= usable_level; level++) { >> + struct intel_wm_level *wm = &pipe_wm->wm[level]; >> >> ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, >> pristate, sprstate, curstate, wm); >> @@ -2409,13 +2404,10 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >> * register maximums since such watermarks are >> * always invalid. >> */ >> - if (level > usable_level) >> - continue; >> - >> - if (ilk_validate_wm_level(level, &max, wm)) >> - pipe_wm->wm[level] = *wm; >> - else >> - usable_level = level; >> + if (!ilk_validate_wm_level(level, &max, wm)) { >> + memset(wm, 0, sizeof(*wm)); >> + break; >> + } >> } >> >> return 0; >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >> index a19ec06..3ce9ba3 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -457,7 +457,6 @@ struct intel_crtc_scaler_state { >> >> struct intel_pipe_wm { >> struct intel_wm_level wm[5]; >> - struct intel_wm_level raw_wm[5]; >> uint32_t linetime; >> bool fbc_wm_enabled; >> bool pipe_enabled; > Ok, so this looks like commit 8777b927b92cf5b6c29f9f9d3c737addea9ac8a7 > upstream which is commit 7de694782cbe7840f2c0de6f1e70f41fc1b8b6e8 in > 4.9.62. > > I've cc:ed the authors of that patch now. > > Maarten, any hints? Should I revert this from 4.9-stable, or was there > a follow-on patch that resolved this issue in mainline? > > thanks, > > greg k-h > OK, after reverting the patches, the flicker *is* gone.
Thanks for confirming this.
BTW (for the future): Was it the right way to address stable@vger.kernel.org in this matter or would the bugreport at freedesktop.org have been enough? I'm a bit unsure about that.
I have no idea what the i915 developers want, but as far as I'm concerned, sending this to stable@vger was fine with me, I have no problem doing a bit of work in tracking down the specific patch before bugging the developers involved.
Well, this one we wanted to be backported, and so indicated with cc: stable, but apparently it went south anyway. :(
Rainer, does v4.14 work for you? I.e. is the commit okay or not before the backport?
Maarten?
BR, Jani.
4.14 is OK, no problems.
So long! Rainer Fiebig
What happens when you apply both other backported patches on top?
On top of 4.14?
No, on top of v4.9 :-)
Maarten Lankhorst wrote:
Op 20-11-17 om 09:51 schreef Rainer Fiebig:
Jani Nikula wrote:
On Sun, 19 Nov 2017, Greg KH gregkh@linuxfoundation.org wrote:
On Sun, Nov 19, 2017 at 01:44:06PM +0100, Rainer Fiebig wrote:
Greg KH wrote:
On Sun, Nov 19, 2017 at 12:56:26PM +0100, Rainer Fiebig wrote: > Greg KH wrote: >> On Sat, Nov 18, 2017 at 05:08:20PM +0100, Rainer Fiebig wrote: >>> Greg KH wrote: >>>> On Sat, Nov 18, 2017 at 01:47:32PM +0100, Rainer Fiebig wrote: >>>>> Hi! >>>>> >>>>> Hopefully the right addressee. >>>>> >>>>> Encountered two bad backports which cause screen-flicker. >>>>> dmesg shows: >>>>> >>>>> ... >>>>> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun >>>>> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun >>>>> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun >>>>> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun >>>>> ... >>>>> >>>>> CPU: Intel Core i3 (Clarkdale/Ironlake) >>>>> >>>>> The backports are: >>>>> >>>>> - diff --git a/drivers/gpu/drm/i915/intel_pm.c >>>>> b/drivers/gpu/drm/i915/intel_pm.c >>>>> index 49de476..277a802 100644 >>>>> - diff --git a/drivers/gpu/drm/i915/intel_drv.h >>>>> b/drivers/gpu/drm/i915/intel_drv.h >>>>> index a19ec06..3ce9ba3 100644 >>>>> >>>>> After reversing them the flicker is gone, no more messages in dmesg. All >>>>> else OK so far. >>>> So which commit was the one that caused the problem? I will be glad to >>>> revert it. >>>> >>>> thanks, >>>> >>>> greg k-h >>>> >>>> >>> I started by reverting the more complex one first ("index >>> 49de476..277a802100644"). But the kernel wouldn't compile then. >> What git commit id is that? I don't see those ids in the 4.9-stable >> tree. >> >>> So I also reverted "index a19ec06..3ce9ba3 100644". After that the >>> kernel compiled just fine and the problems were gone (still are). >> Same here, what git commit id was this? >> >> thanks, >> >> greg k-h >> > OK, no mistake. IIRC, I took the patches (and the IDs) from the > changelog for patch-4.9.62. I've attached both, so you can check yourself. > > I've also applied a freshly downloaded patch-4.9.62 to a freshly > expanded 4.9 and re-compiled. The flicker is there. I haven't yet > reverted the two patches but I'm confident that after having done so the > flicker will be gone. If not I'll let you know. > > As a good news: 4.14 is *not* affected. So to me it seems those two > patches are part of sort of a package and can not be backported alone. > > So long! > Rainer Fiebig > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 49de476..277a802 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -27,6 +27,7 @@ > > #include <linux/cpufreq.h> > #include <drm/drm_plane_helper.h> > +#include <drm/drm_atomic_helper.h> > #include "i915_drv.h" > #include "intel_drv.h" > #include "../../../platform/x86/intel_ips.h" > @@ -2017,9 +2018,9 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv, > const struct intel_crtc *intel_crtc, > int level, > struct intel_crtc_state *cstate, > - struct intel_plane_state *pristate, > - struct intel_plane_state *sprstate, > - struct intel_plane_state *curstate, > + const struct intel_plane_state *pristate, > + const struct intel_plane_state *sprstate, > + const struct intel_plane_state *curstate, > struct intel_wm_level *result) > { > uint16_t pri_latency = dev_priv->wm.pri_latency[level]; > @@ -2341,28 +2342,24 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) > struct intel_pipe_wm *pipe_wm; > struct drm_device *dev = state->dev; > const struct drm_i915_private *dev_priv = to_i915(dev); > - struct intel_plane *intel_plane; > - struct intel_plane_state *pristate = NULL; > - struct intel_plane_state *sprstate = NULL; > - struct intel_plane_state *curstate = NULL; > + struct drm_plane *plane; > + const struct drm_plane_state *plane_state; > + const struct intel_plane_state *pristate = NULL; > + const struct intel_plane_state *sprstate = NULL; > + const struct intel_plane_state *curstate = NULL; > int level, max_level = ilk_wm_max_level(dev), usable_level; > struct ilk_wm_maximums max; > > pipe_wm = &cstate->wm.ilk.optimal; > > - for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) { > - struct intel_plane_state *ps; > + drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, &cstate->base) { > + const struct intel_plane_state *ps = to_intel_plane_state(plane_state); > > - ps = intel_atomic_get_existing_plane_state(state, > - intel_plane); > - if (!ps) > - continue; > - > - if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY) > + if (plane->type == DRM_PLANE_TYPE_PRIMARY) > pristate = ps; > - else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY) > + else if (plane->type == DRM_PLANE_TYPE_OVERLAY) > sprstate = ps; > - else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR) > + else if (plane->type == DRM_PLANE_TYPE_CURSOR) > curstate = ps; > } > > @@ -2384,11 +2381,9 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) > if (pipe_wm->sprites_scaled) > usable_level = 0; > > - ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, > - pristate, sprstate, curstate, &pipe_wm->raw_wm[0]); > - > memset(&pipe_wm->wm, 0, sizeof(pipe_wm->wm)); > - pipe_wm->wm[0] = pipe_wm->raw_wm[0]; > + ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, > + pristate, sprstate, curstate, &pipe_wm->wm[0]); > > if (IS_HASWELL(dev) || IS_BROADWELL(dev)) > pipe_wm->linetime = hsw_compute_linetime_wm(cstate); > @@ -2398,8 +2393,8 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) > > ilk_compute_wm_reg_maximums(dev, 1, &max); > > - for (level = 1; level <= max_level; level++) { > - struct intel_wm_level *wm = &pipe_wm->raw_wm[level]; > + for (level = 1; level <= usable_level; level++) { > + struct intel_wm_level *wm = &pipe_wm->wm[level]; > > ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, > pristate, sprstate, curstate, wm); > @@ -2409,13 +2404,10 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) > * register maximums since such watermarks are > * always invalid. > */ > - if (level > usable_level) > - continue; > - > - if (ilk_validate_wm_level(level, &max, wm)) > - pipe_wm->wm[level] = *wm; > - else > - usable_level = level; > + if (!ilk_validate_wm_level(level, &max, wm)) { > + memset(wm, 0, sizeof(*wm)); > + break; > + } > } > > return 0; > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index a19ec06..3ce9ba3 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -457,7 +457,6 @@ struct intel_crtc_scaler_state { > > struct intel_pipe_wm { > struct intel_wm_level wm[5]; > - struct intel_wm_level raw_wm[5]; > uint32_t linetime; > bool fbc_wm_enabled; > bool pipe_enabled; Ok, so this looks like commit 8777b927b92cf5b6c29f9f9d3c737addea9ac8a7 upstream which is commit 7de694782cbe7840f2c0de6f1e70f41fc1b8b6e8 in 4.9.62.
I've cc:ed the authors of that patch now.
Maarten, any hints? Should I revert this from 4.9-stable, or was there a follow-on patch that resolved this issue in mainline?
thanks,
greg k-h
OK, after reverting the patches, the flicker *is* gone.
Thanks for confirming this.
BTW (for the future): Was it the right way to address stable@vger.kernel.org in this matter or would the bugreport at freedesktop.org have been enough? I'm a bit unsure about that.
I have no idea what the i915 developers want, but as far as I'm concerned, sending this to stable@vger was fine with me, I have no problem doing a bit of work in tracking down the specific patch before bugging the developers involved.
Well, this one we wanted to be backported, and so indicated with cc: stable, but apparently it went south anyway. :(
Rainer, does v4.14 work for you? I.e. is the commit okay or not before the backport?
Maarten?
BR, Jani.
4.14 is OK, no problems.
So long! Rainer Fiebig
What happens when you apply both other backported patches on top?
Ah, you probably mean applying the latest two patches in your repo on top of the two orig-backports for 4.9.62?
I'll try.
Maarten Lankhorst wrote:
Op 20-11-17 om 09:51 schreef Rainer Fiebig:
Jani Nikula wrote:
On Sun, 19 Nov 2017, Greg KH gregkh@linuxfoundation.org wrote:
On Sun, Nov 19, 2017 at 01:44:06PM +0100, Rainer Fiebig wrote:
Greg KH wrote:
On Sun, Nov 19, 2017 at 12:56:26PM +0100, Rainer Fiebig wrote: > Greg KH wrote: >> On Sat, Nov 18, 2017 at 05:08:20PM +0100, Rainer Fiebig wrote: >>> Greg KH wrote: >>>> On Sat, Nov 18, 2017 at 01:47:32PM +0100, Rainer Fiebig wrote: >>>>> Hi! >>>>> >>>>> Hopefully the right addressee. >>>>> >>>>> Encountered two bad backports which cause screen-flicker. >>>>> dmesg shows: >>>>> >>>>> ... >>>>> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun >>>>> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun >>>>> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun >>>>> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun >>>>> ... >>>>> >>>>> CPU: Intel Core i3 (Clarkdale/Ironlake) >>>>> >>>>> The backports are: >>>>> >>>>> - diff --git a/drivers/gpu/drm/i915/intel_pm.c >>>>> b/drivers/gpu/drm/i915/intel_pm.c >>>>> index 49de476..277a802 100644 >>>>> - diff --git a/drivers/gpu/drm/i915/intel_drv.h >>>>> b/drivers/gpu/drm/i915/intel_drv.h >>>>> index a19ec06..3ce9ba3 100644 >>>>> >>>>> After reversing them the flicker is gone, no more messages in dmesg. All >>>>> else OK so far. >>>> So which commit was the one that caused the problem? I will be glad to >>>> revert it. >>>> >>>> thanks, >>>> >>>> greg k-h >>>> >>>> >>> I started by reverting the more complex one first ("index >>> 49de476..277a802100644"). But the kernel wouldn't compile then. >> What git commit id is that? I don't see those ids in the 4.9-stable >> tree. >> >>> So I also reverted "index a19ec06..3ce9ba3 100644". After that the >>> kernel compiled just fine and the problems were gone (still are). >> Same here, what git commit id was this? >> >> thanks, >> >> greg k-h >> > OK, no mistake. IIRC, I took the patches (and the IDs) from the > changelog for patch-4.9.62. I've attached both, so you can check yourself. > > I've also applied a freshly downloaded patch-4.9.62 to a freshly > expanded 4.9 and re-compiled. The flicker is there. I haven't yet > reverted the two patches but I'm confident that after having done so the > flicker will be gone. If not I'll let you know. > > As a good news: 4.14 is *not* affected. So to me it seems those two > patches are part of sort of a package and can not be backported alone. > > So long! > Rainer Fiebig > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 49de476..277a802 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -27,6 +27,7 @@ > > #include <linux/cpufreq.h> > #include <drm/drm_plane_helper.h> > +#include <drm/drm_atomic_helper.h> > #include "i915_drv.h" > #include "intel_drv.h" > #include "../../../platform/x86/intel_ips.h" > @@ -2017,9 +2018,9 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv, > const struct intel_crtc *intel_crtc, > int level, > struct intel_crtc_state *cstate, > - struct intel_plane_state *pristate, > - struct intel_plane_state *sprstate, > - struct intel_plane_state *curstate, > + const struct intel_plane_state *pristate, > + const struct intel_plane_state *sprstate, > + const struct intel_plane_state *curstate, > struct intel_wm_level *result) > { > uint16_t pri_latency = dev_priv->wm.pri_latency[level]; > @@ -2341,28 +2342,24 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) > struct intel_pipe_wm *pipe_wm; > struct drm_device *dev = state->dev; > const struct drm_i915_private *dev_priv = to_i915(dev); > - struct intel_plane *intel_plane; > - struct intel_plane_state *pristate = NULL; > - struct intel_plane_state *sprstate = NULL; > - struct intel_plane_state *curstate = NULL; > + struct drm_plane *plane; > + const struct drm_plane_state *plane_state; > + const struct intel_plane_state *pristate = NULL; > + const struct intel_plane_state *sprstate = NULL; > + const struct intel_plane_state *curstate = NULL; > int level, max_level = ilk_wm_max_level(dev), usable_level; > struct ilk_wm_maximums max; > > pipe_wm = &cstate->wm.ilk.optimal; > > - for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) { > - struct intel_plane_state *ps; > + drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, &cstate->base) { > + const struct intel_plane_state *ps = to_intel_plane_state(plane_state); > > - ps = intel_atomic_get_existing_plane_state(state, > - intel_plane); > - if (!ps) > - continue; > - > - if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY) > + if (plane->type == DRM_PLANE_TYPE_PRIMARY) > pristate = ps; > - else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY) > + else if (plane->type == DRM_PLANE_TYPE_OVERLAY) > sprstate = ps; > - else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR) > + else if (plane->type == DRM_PLANE_TYPE_CURSOR) > curstate = ps; > } > > @@ -2384,11 +2381,9 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) > if (pipe_wm->sprites_scaled) > usable_level = 0; > > - ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, > - pristate, sprstate, curstate, &pipe_wm->raw_wm[0]); > - > memset(&pipe_wm->wm, 0, sizeof(pipe_wm->wm)); > - pipe_wm->wm[0] = pipe_wm->raw_wm[0]; > + ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, > + pristate, sprstate, curstate, &pipe_wm->wm[0]); > > if (IS_HASWELL(dev) || IS_BROADWELL(dev)) > pipe_wm->linetime = hsw_compute_linetime_wm(cstate); > @@ -2398,8 +2393,8 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) > > ilk_compute_wm_reg_maximums(dev, 1, &max); > > - for (level = 1; level <= max_level; level++) { > - struct intel_wm_level *wm = &pipe_wm->raw_wm[level]; > + for (level = 1; level <= usable_level; level++) { > + struct intel_wm_level *wm = &pipe_wm->wm[level]; > > ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, > pristate, sprstate, curstate, wm); > @@ -2409,13 +2404,10 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) > * register maximums since such watermarks are > * always invalid. > */ > - if (level > usable_level) > - continue; > - > - if (ilk_validate_wm_level(level, &max, wm)) > - pipe_wm->wm[level] = *wm; > - else > - usable_level = level; > + if (!ilk_validate_wm_level(level, &max, wm)) { > + memset(wm, 0, sizeof(*wm)); > + break; > + } > } > > return 0; > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index a19ec06..3ce9ba3 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -457,7 +457,6 @@ struct intel_crtc_scaler_state { > > struct intel_pipe_wm { > struct intel_wm_level wm[5]; > - struct intel_wm_level raw_wm[5]; > uint32_t linetime; > bool fbc_wm_enabled; > bool pipe_enabled; Ok, so this looks like commit 8777b927b92cf5b6c29f9f9d3c737addea9ac8a7 upstream which is commit 7de694782cbe7840f2c0de6f1e70f41fc1b8b6e8 in 4.9.62.
I've cc:ed the authors of that patch now.
Maarten, any hints? Should I revert this from 4.9-stable, or was there a follow-on patch that resolved this issue in mainline?
thanks,
greg k-h
OK, after reverting the patches, the flicker *is* gone.
Thanks for confirming this.
BTW (for the future): Was it the right way to address stable@vger.kernel.org in this matter or would the bugreport at freedesktop.org have been enough? I'm a bit unsure about that.
I have no idea what the i915 developers want, but as far as I'm concerned, sending this to stable@vger was fine with me, I have no problem doing a bit of work in tracking down the specific patch before bugging the developers involved.
Well, this one we wanted to be backported, and so indicated with cc: stable, but apparently it went south anyway. :(
Rainer, does v4.14 work for you? I.e. is the commit okay or not before the backport?
Maarten?
BR, Jani.
4.14 is OK, no problems.
So long! Rainer Fiebig
What happens when you apply both other backported patches on top?
So I applied the 2 new patches on top of the orig. 4.9.62.
Better now but not yet completely OK:
... [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun ...
No random flicker so far but flicker when I enter a character in an x-terminal.
Seems your patches fixed only half of the pipes. ;)
So long! Rainer Fiebig
Rainer Fiebig wrote:
Maarten Lankhorst wrote:
Op 20-11-17 om 09:51 schreef Rainer Fiebig:
Jani Nikula wrote:
On Sun, 19 Nov 2017, Greg KH gregkh@linuxfoundation.org wrote:
On Sun, Nov 19, 2017 at 01:44:06PM +0100, Rainer Fiebig wrote:
Greg KH wrote: > On Sun, Nov 19, 2017 at 12:56:26PM +0100, Rainer Fiebig wrote: >> Greg KH wrote: >>> On Sat, Nov 18, 2017 at 05:08:20PM +0100, Rainer Fiebig wrote: >>>> Greg KH wrote: >>>>> On Sat, Nov 18, 2017 at 01:47:32PM +0100, Rainer Fiebig wrote: >>>>>> Hi! >>>>>> >>>>>> Hopefully the right addressee. >>>>>> >>>>>> Encountered two bad backports which cause screen-flicker. >>>>>> dmesg shows: >>>>>> >>>>>> ... >>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun >>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun >>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun >>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun >>>>>> ... >>>>>> >>>>>> CPU: Intel Core i3 (Clarkdale/Ironlake) >>>>>> >>>>>> The backports are: >>>>>> >>>>>> - diff --git a/drivers/gpu/drm/i915/intel_pm.c >>>>>> b/drivers/gpu/drm/i915/intel_pm.c >>>>>> index 49de476..277a802 100644 >>>>>> - diff --git a/drivers/gpu/drm/i915/intel_drv.h >>>>>> b/drivers/gpu/drm/i915/intel_drv.h >>>>>> index a19ec06..3ce9ba3 100644 >>>>>> >>>>>> After reversing them the flicker is gone, no more messages in dmesg. All >>>>>> else OK so far. >>>>> So which commit was the one that caused the problem? I will be glad to >>>>> revert it. >>>>> >>>>> thanks, >>>>> >>>>> greg k-h >>>>> >>>>> >>>> I started by reverting the more complex one first ("index >>>> 49de476..277a802100644"). But the kernel wouldn't compile then. >>> What git commit id is that? I don't see those ids in the 4.9-stable >>> tree. >>> >>>> So I also reverted "index a19ec06..3ce9ba3 100644". After that the >>>> kernel compiled just fine and the problems were gone (still are). >>> Same here, what git commit id was this? >>> >>> thanks, >>> >>> greg k-h >>> >> OK, no mistake. IIRC, I took the patches (and the IDs) from the >> changelog for patch-4.9.62. I've attached both, so you can check yourself. >> >> I've also applied a freshly downloaded patch-4.9.62 to a freshly >> expanded 4.9 and re-compiled. The flicker is there. I haven't yet >> reverted the two patches but I'm confident that after having done so the >> flicker will be gone. If not I'll let you know. >> >> As a good news: 4.14 is *not* affected. So to me it seems those two >> patches are part of sort of a package and can not be backported alone. >> >> So long! >> Rainer Fiebig >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >> index 49de476..277a802 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -27,6 +27,7 @@ >> >> #include <linux/cpufreq.h> >> #include <drm/drm_plane_helper.h> >> +#include <drm/drm_atomic_helper.h> >> #include "i915_drv.h" >> #include "intel_drv.h" >> #include "../../../platform/x86/intel_ips.h" >> @@ -2017,9 +2018,9 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv, >> const struct intel_crtc *intel_crtc, >> int level, >> struct intel_crtc_state *cstate, >> - struct intel_plane_state *pristate, >> - struct intel_plane_state *sprstate, >> - struct intel_plane_state *curstate, >> + const struct intel_plane_state *pristate, >> + const struct intel_plane_state *sprstate, >> + const struct intel_plane_state *curstate, >> struct intel_wm_level *result) >> { >> uint16_t pri_latency = dev_priv->wm.pri_latency[level]; >> @@ -2341,28 +2342,24 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >> struct intel_pipe_wm *pipe_wm; >> struct drm_device *dev = state->dev; >> const struct drm_i915_private *dev_priv = to_i915(dev); >> - struct intel_plane *intel_plane; >> - struct intel_plane_state *pristate = NULL; >> - struct intel_plane_state *sprstate = NULL; >> - struct intel_plane_state *curstate = NULL; >> + struct drm_plane *plane; >> + const struct drm_plane_state *plane_state; >> + const struct intel_plane_state *pristate = NULL; >> + const struct intel_plane_state *sprstate = NULL; >> + const struct intel_plane_state *curstate = NULL; >> int level, max_level = ilk_wm_max_level(dev), usable_level; >> struct ilk_wm_maximums max; >> >> pipe_wm = &cstate->wm.ilk.optimal; >> >> - for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) { >> - struct intel_plane_state *ps; >> + drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, &cstate->base) { >> + const struct intel_plane_state *ps = to_intel_plane_state(plane_state); >> >> - ps = intel_atomic_get_existing_plane_state(state, >> - intel_plane); >> - if (!ps) >> - continue; >> - >> - if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY) >> + if (plane->type == DRM_PLANE_TYPE_PRIMARY) >> pristate = ps; >> - else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY) >> + else if (plane->type == DRM_PLANE_TYPE_OVERLAY) >> sprstate = ps; >> - else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR) >> + else if (plane->type == DRM_PLANE_TYPE_CURSOR) >> curstate = ps; >> } >> >> @@ -2384,11 +2381,9 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >> if (pipe_wm->sprites_scaled) >> usable_level = 0; >> >> - ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, >> - pristate, sprstate, curstate, &pipe_wm->raw_wm[0]); >> - >> memset(&pipe_wm->wm, 0, sizeof(pipe_wm->wm)); >> - pipe_wm->wm[0] = pipe_wm->raw_wm[0]; >> + ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, >> + pristate, sprstate, curstate, &pipe_wm->wm[0]); >> >> if (IS_HASWELL(dev) || IS_BROADWELL(dev)) >> pipe_wm->linetime = hsw_compute_linetime_wm(cstate); >> @@ -2398,8 +2393,8 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >> >> ilk_compute_wm_reg_maximums(dev, 1, &max); >> >> - for (level = 1; level <= max_level; level++) { >> - struct intel_wm_level *wm = &pipe_wm->raw_wm[level]; >> + for (level = 1; level <= usable_level; level++) { >> + struct intel_wm_level *wm = &pipe_wm->wm[level]; >> >> ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, >> pristate, sprstate, curstate, wm); >> @@ -2409,13 +2404,10 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >> * register maximums since such watermarks are >> * always invalid. >> */ >> - if (level > usable_level) >> - continue; >> - >> - if (ilk_validate_wm_level(level, &max, wm)) >> - pipe_wm->wm[level] = *wm; >> - else >> - usable_level = level; >> + if (!ilk_validate_wm_level(level, &max, wm)) { >> + memset(wm, 0, sizeof(*wm)); >> + break; >> + } >> } >> >> return 0; >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >> index a19ec06..3ce9ba3 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -457,7 +457,6 @@ struct intel_crtc_scaler_state { >> >> struct intel_pipe_wm { >> struct intel_wm_level wm[5]; >> - struct intel_wm_level raw_wm[5]; >> uint32_t linetime; >> bool fbc_wm_enabled; >> bool pipe_enabled; > Ok, so this looks like commit 8777b927b92cf5b6c29f9f9d3c737addea9ac8a7 > upstream which is commit 7de694782cbe7840f2c0de6f1e70f41fc1b8b6e8 in > 4.9.62. > > I've cc:ed the authors of that patch now. > > Maarten, any hints? Should I revert this from 4.9-stable, or was there > a follow-on patch that resolved this issue in mainline? > > thanks, > > greg k-h > OK, after reverting the patches, the flicker *is* gone.
Thanks for confirming this.
BTW (for the future): Was it the right way to address stable@vger.kernel.org in this matter or would the bugreport at freedesktop.org have been enough? I'm a bit unsure about that.
I have no idea what the i915 developers want, but as far as I'm concerned, sending this to stable@vger was fine with me, I have no problem doing a bit of work in tracking down the specific patch before bugging the developers involved.
Well, this one we wanted to be backported, and so indicated with cc: stable, but apparently it went south anyway. :(
Rainer, does v4.14 work for you? I.e. is the commit okay or not before the backport?
Maarten?
BR, Jani.
4.14 is OK, no problems.
So long! Rainer Fiebig
What happens when you apply both other backported patches on top?
So I applied the 2 new patches on top of the orig. 4.9.62.
Better now but not yet completely OK:
... [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun ...
No random flicker so far but flicker when I enter a character in an x-terminal.
Seems your patches fixed only half of the pipes. ;)
So long! Rainer Fiebig
There's also random flicker after a while.
On Thu, Nov 23, 2017 at 10:09:25PM +0100, Rainer Fiebig wrote:
Rainer Fiebig wrote:
Maarten Lankhorst wrote:
Op 20-11-17 om 09:51 schreef Rainer Fiebig:
Jani Nikula wrote:
On Sun, 19 Nov 2017, Greg KH gregkh@linuxfoundation.org wrote:
On Sun, Nov 19, 2017 at 01:44:06PM +0100, Rainer Fiebig wrote: > Greg KH wrote: >> On Sun, Nov 19, 2017 at 12:56:26PM +0100, Rainer Fiebig wrote: >>> Greg KH wrote: >>>> On Sat, Nov 18, 2017 at 05:08:20PM +0100, Rainer Fiebig wrote: >>>>> Greg KH wrote: >>>>>> On Sat, Nov 18, 2017 at 01:47:32PM +0100, Rainer Fiebig wrote: >>>>>>> Hi! >>>>>>> >>>>>>> Hopefully the right addressee. >>>>>>> >>>>>>> Encountered two bad backports which cause screen-flicker. >>>>>>> dmesg shows: >>>>>>> >>>>>>> ... >>>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun >>>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun >>>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun >>>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun >>>>>>> ... >>>>>>> >>>>>>> CPU: Intel Core i3 (Clarkdale/Ironlake) >>>>>>> >>>>>>> The backports are: >>>>>>> >>>>>>> - diff --git a/drivers/gpu/drm/i915/intel_pm.c >>>>>>> b/drivers/gpu/drm/i915/intel_pm.c >>>>>>> index 49de476..277a802 100644 >>>>>>> - diff --git a/drivers/gpu/drm/i915/intel_drv.h >>>>>>> b/drivers/gpu/drm/i915/intel_drv.h >>>>>>> index a19ec06..3ce9ba3 100644 >>>>>>> >>>>>>> After reversing them the flicker is gone, no more messages in dmesg. All >>>>>>> else OK so far. >>>>>> So which commit was the one that caused the problem? I will be glad to >>>>>> revert it. >>>>>> >>>>>> thanks, >>>>>> >>>>>> greg k-h >>>>>> >>>>>> >>>>> I started by reverting the more complex one first ("index >>>>> 49de476..277a802100644"). But the kernel wouldn't compile then. >>>> What git commit id is that? I don't see those ids in the 4.9-stable >>>> tree. >>>> >>>>> So I also reverted "index a19ec06..3ce9ba3 100644". After that the >>>>> kernel compiled just fine and the problems were gone (still are). >>>> Same here, what git commit id was this? >>>> >>>> thanks, >>>> >>>> greg k-h >>>> >>> OK, no mistake. IIRC, I took the patches (and the IDs) from the >>> changelog for patch-4.9.62. I've attached both, so you can check yourself. >>> >>> I've also applied a freshly downloaded patch-4.9.62 to a freshly >>> expanded 4.9 and re-compiled. The flicker is there. I haven't yet >>> reverted the two patches but I'm confident that after having done so the >>> flicker will be gone. If not I'll let you know. >>> >>> As a good news: 4.14 is *not* affected. So to me it seems those two >>> patches are part of sort of a package and can not be backported alone. >>> >>> So long! >>> Rainer Fiebig >>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >>> index 49de476..277a802 100644 >>> --- a/drivers/gpu/drm/i915/intel_pm.c >>> +++ b/drivers/gpu/drm/i915/intel_pm.c >>> @@ -27,6 +27,7 @@ >>> >>> #include <linux/cpufreq.h> >>> #include <drm/drm_plane_helper.h> >>> +#include <drm/drm_atomic_helper.h> >>> #include "i915_drv.h" >>> #include "intel_drv.h" >>> #include "../../../platform/x86/intel_ips.h" >>> @@ -2017,9 +2018,9 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv, >>> const struct intel_crtc *intel_crtc, >>> int level, >>> struct intel_crtc_state *cstate, >>> - struct intel_plane_state *pristate, >>> - struct intel_plane_state *sprstate, >>> - struct intel_plane_state *curstate, >>> + const struct intel_plane_state *pristate, >>> + const struct intel_plane_state *sprstate, >>> + const struct intel_plane_state *curstate, >>> struct intel_wm_level *result) >>> { >>> uint16_t pri_latency = dev_priv->wm.pri_latency[level]; >>> @@ -2341,28 +2342,24 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >>> struct intel_pipe_wm *pipe_wm; >>> struct drm_device *dev = state->dev; >>> const struct drm_i915_private *dev_priv = to_i915(dev); >>> - struct intel_plane *intel_plane; >>> - struct intel_plane_state *pristate = NULL; >>> - struct intel_plane_state *sprstate = NULL; >>> - struct intel_plane_state *curstate = NULL; >>> + struct drm_plane *plane; >>> + const struct drm_plane_state *plane_state; >>> + const struct intel_plane_state *pristate = NULL; >>> + const struct intel_plane_state *sprstate = NULL; >>> + const struct intel_plane_state *curstate = NULL; >>> int level, max_level = ilk_wm_max_level(dev), usable_level; >>> struct ilk_wm_maximums max; >>> >>> pipe_wm = &cstate->wm.ilk.optimal; >>> >>> - for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) { >>> - struct intel_plane_state *ps; >>> + drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, &cstate->base) { >>> + const struct intel_plane_state *ps = to_intel_plane_state(plane_state); >>> >>> - ps = intel_atomic_get_existing_plane_state(state, >>> - intel_plane); >>> - if (!ps) >>> - continue; >>> - >>> - if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY) >>> + if (plane->type == DRM_PLANE_TYPE_PRIMARY) >>> pristate = ps; >>> - else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY) >>> + else if (plane->type == DRM_PLANE_TYPE_OVERLAY) >>> sprstate = ps; >>> - else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR) >>> + else if (plane->type == DRM_PLANE_TYPE_CURSOR) >>> curstate = ps; >>> } >>> >>> @@ -2384,11 +2381,9 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >>> if (pipe_wm->sprites_scaled) >>> usable_level = 0; >>> >>> - ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, >>> - pristate, sprstate, curstate, &pipe_wm->raw_wm[0]); >>> - >>> memset(&pipe_wm->wm, 0, sizeof(pipe_wm->wm)); >>> - pipe_wm->wm[0] = pipe_wm->raw_wm[0]; >>> + ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, >>> + pristate, sprstate, curstate, &pipe_wm->wm[0]); >>> >>> if (IS_HASWELL(dev) || IS_BROADWELL(dev)) >>> pipe_wm->linetime = hsw_compute_linetime_wm(cstate); >>> @@ -2398,8 +2393,8 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >>> >>> ilk_compute_wm_reg_maximums(dev, 1, &max); >>> >>> - for (level = 1; level <= max_level; level++) { >>> - struct intel_wm_level *wm = &pipe_wm->raw_wm[level]; >>> + for (level = 1; level <= usable_level; level++) { >>> + struct intel_wm_level *wm = &pipe_wm->wm[level]; >>> >>> ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, >>> pristate, sprstate, curstate, wm); >>> @@ -2409,13 +2404,10 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >>> * register maximums since such watermarks are >>> * always invalid. >>> */ >>> - if (level > usable_level) >>> - continue; >>> - >>> - if (ilk_validate_wm_level(level, &max, wm)) >>> - pipe_wm->wm[level] = *wm; >>> - else >>> - usable_level = level; >>> + if (!ilk_validate_wm_level(level, &max, wm)) { >>> + memset(wm, 0, sizeof(*wm)); >>> + break; >>> + } >>> } >>> >>> return 0; >>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >>> index a19ec06..3ce9ba3 100644 >>> --- a/drivers/gpu/drm/i915/intel_drv.h >>> +++ b/drivers/gpu/drm/i915/intel_drv.h >>> @@ -457,7 +457,6 @@ struct intel_crtc_scaler_state { >>> >>> struct intel_pipe_wm { >>> struct intel_wm_level wm[5]; >>> - struct intel_wm_level raw_wm[5]; >>> uint32_t linetime; >>> bool fbc_wm_enabled; >>> bool pipe_enabled; >> Ok, so this looks like commit 8777b927b92cf5b6c29f9f9d3c737addea9ac8a7 >> upstream which is commit 7de694782cbe7840f2c0de6f1e70f41fc1b8b6e8 in >> 4.9.62. >> >> I've cc:ed the authors of that patch now. >> >> Maarten, any hints? Should I revert this from 4.9-stable, or was there >> a follow-on patch that resolved this issue in mainline? >> >> thanks, >> >> greg k-h >> > OK, after reverting the patches, the flicker *is* gone. Thanks for confirming this.
> BTW (for the future): Was it the right way to address > stable@vger.kernel.org in this matter or would the bugreport at > freedesktop.org have been enough? I'm a bit unsure about that. I have no idea what the i915 developers want, but as far as I'm concerned, sending this to stable@vger was fine with me, I have no problem doing a bit of work in tracking down the specific patch before bugging the developers involved.
Well, this one we wanted to be backported, and so indicated with cc: stable, but apparently it went south anyway. :(
Rainer, does v4.14 work for you? I.e. is the commit okay or not before the backport?
Maarten?
BR, Jani.
4.14 is OK, no problems.
So long! Rainer Fiebig
What happens when you apply both other backported patches on top?
So I applied the 2 new patches on top of the orig. 4.9.62.
Better now but not yet completely OK:
... [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun ...
No random flicker so far but flicker when I enter a character in an x-terminal.
Seems your patches fixed only half of the pipes. ;)
So long! Rainer Fiebig
There's also random flicker after a while.
Ok, I'm just going to revert this patch for the next release unless one of the i915 developers has a better idea...
thanks,
greg k-h
Greg KH wrote:
On Thu, Nov 23, 2017 at 10:09:25PM +0100, Rainer Fiebig wrote:
Rainer Fiebig wrote:
Maarten Lankhorst wrote:
Op 20-11-17 om 09:51 schreef Rainer Fiebig:
Jani Nikula wrote:
On Sun, 19 Nov 2017, Greg KH gregkh@linuxfoundation.org wrote: > On Sun, Nov 19, 2017 at 01:44:06PM +0100, Rainer Fiebig wrote: >> Greg KH wrote: >>> On Sun, Nov 19, 2017 at 12:56:26PM +0100, Rainer Fiebig wrote: >>>> Greg KH wrote: >>>>> On Sat, Nov 18, 2017 at 05:08:20PM +0100, Rainer Fiebig wrote: >>>>>> Greg KH wrote: >>>>>>> On Sat, Nov 18, 2017 at 01:47:32PM +0100, Rainer Fiebig wrote: >>>>>>>> Hi! >>>>>>>> >>>>>>>> Hopefully the right addressee. >>>>>>>> >>>>>>>> Encountered two bad backports which cause screen-flicker. >>>>>>>> dmesg shows: >>>>>>>> >>>>>>>> ... >>>>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun >>>>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun >>>>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun >>>>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun >>>>>>>> ... >>>>>>>> >>>>>>>> CPU: Intel Core i3 (Clarkdale/Ironlake) >>>>>>>> >>>>>>>> The backports are: >>>>>>>> >>>>>>>> - diff --git a/drivers/gpu/drm/i915/intel_pm.c >>>>>>>> b/drivers/gpu/drm/i915/intel_pm.c >>>>>>>> index 49de476..277a802 100644 >>>>>>>> - diff --git a/drivers/gpu/drm/i915/intel_drv.h >>>>>>>> b/drivers/gpu/drm/i915/intel_drv.h >>>>>>>> index a19ec06..3ce9ba3 100644 >>>>>>>> >>>>>>>> After reversing them the flicker is gone, no more messages in dmesg. All >>>>>>>> else OK so far. >>>>>>> So which commit was the one that caused the problem? I will be glad to >>>>>>> revert it. >>>>>>> >>>>>>> thanks, >>>>>>> >>>>>>> greg k-h >>>>>>> >>>>>>> >>>>>> I started by reverting the more complex one first ("index >>>>>> 49de476..277a802100644"). But the kernel wouldn't compile then. >>>>> What git commit id is that? I don't see those ids in the 4.9-stable >>>>> tree. >>>>> >>>>>> So I also reverted "index a19ec06..3ce9ba3 100644". After that the >>>>>> kernel compiled just fine and the problems were gone (still are). >>>>> Same here, what git commit id was this? >>>>> >>>>> thanks, >>>>> >>>>> greg k-h >>>>> >>>> OK, no mistake. IIRC, I took the patches (and the IDs) from the >>>> changelog for patch-4.9.62. I've attached both, so you can check yourself. >>>> >>>> I've also applied a freshly downloaded patch-4.9.62 to a freshly >>>> expanded 4.9 and re-compiled. The flicker is there. I haven't yet >>>> reverted the two patches but I'm confident that after having done so the >>>> flicker will be gone. If not I'll let you know. >>>> >>>> As a good news: 4.14 is *not* affected. So to me it seems those two >>>> patches are part of sort of a package and can not be backported alone. >>>> >>>> So long! >>>> Rainer Fiebig >>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >>>> index 49de476..277a802 100644 >>>> --- a/drivers/gpu/drm/i915/intel_pm.c >>>> +++ b/drivers/gpu/drm/i915/intel_pm.c >>>> @@ -27,6 +27,7 @@ >>>> >>>> #include <linux/cpufreq.h> >>>> #include <drm/drm_plane_helper.h> >>>> +#include <drm/drm_atomic_helper.h> >>>> #include "i915_drv.h" >>>> #include "intel_drv.h" >>>> #include "../../../platform/x86/intel_ips.h" >>>> @@ -2017,9 +2018,9 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv, >>>> const struct intel_crtc *intel_crtc, >>>> int level, >>>> struct intel_crtc_state *cstate, >>>> - struct intel_plane_state *pristate, >>>> - struct intel_plane_state *sprstate, >>>> - struct intel_plane_state *curstate, >>>> + const struct intel_plane_state *pristate, >>>> + const struct intel_plane_state *sprstate, >>>> + const struct intel_plane_state *curstate, >>>> struct intel_wm_level *result) >>>> { >>>> uint16_t pri_latency = dev_priv->wm.pri_latency[level]; >>>> @@ -2341,28 +2342,24 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >>>> struct intel_pipe_wm *pipe_wm; >>>> struct drm_device *dev = state->dev; >>>> const struct drm_i915_private *dev_priv = to_i915(dev); >>>> - struct intel_plane *intel_plane; >>>> - struct intel_plane_state *pristate = NULL; >>>> - struct intel_plane_state *sprstate = NULL; >>>> - struct intel_plane_state *curstate = NULL; >>>> + struct drm_plane *plane; >>>> + const struct drm_plane_state *plane_state; >>>> + const struct intel_plane_state *pristate = NULL; >>>> + const struct intel_plane_state *sprstate = NULL; >>>> + const struct intel_plane_state *curstate = NULL; >>>> int level, max_level = ilk_wm_max_level(dev), usable_level; >>>> struct ilk_wm_maximums max; >>>> >>>> pipe_wm = &cstate->wm.ilk.optimal; >>>> >>>> - for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) { >>>> - struct intel_plane_state *ps; >>>> + drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, &cstate->base) { >>>> + const struct intel_plane_state *ps = to_intel_plane_state(plane_state); >>>> >>>> - ps = intel_atomic_get_existing_plane_state(state, >>>> - intel_plane); >>>> - if (!ps) >>>> - continue; >>>> - >>>> - if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY) >>>> + if (plane->type == DRM_PLANE_TYPE_PRIMARY) >>>> pristate = ps; >>>> - else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY) >>>> + else if (plane->type == DRM_PLANE_TYPE_OVERLAY) >>>> sprstate = ps; >>>> - else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR) >>>> + else if (plane->type == DRM_PLANE_TYPE_CURSOR) >>>> curstate = ps; >>>> } >>>> >>>> @@ -2384,11 +2381,9 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >>>> if (pipe_wm->sprites_scaled) >>>> usable_level = 0; >>>> >>>> - ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, >>>> - pristate, sprstate, curstate, &pipe_wm->raw_wm[0]); >>>> - >>>> memset(&pipe_wm->wm, 0, sizeof(pipe_wm->wm)); >>>> - pipe_wm->wm[0] = pipe_wm->raw_wm[0]; >>>> + ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, >>>> + pristate, sprstate, curstate, &pipe_wm->wm[0]); >>>> >>>> if (IS_HASWELL(dev) || IS_BROADWELL(dev)) >>>> pipe_wm->linetime = hsw_compute_linetime_wm(cstate); >>>> @@ -2398,8 +2393,8 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >>>> >>>> ilk_compute_wm_reg_maximums(dev, 1, &max); >>>> >>>> - for (level = 1; level <= max_level; level++) { >>>> - struct intel_wm_level *wm = &pipe_wm->raw_wm[level]; >>>> + for (level = 1; level <= usable_level; level++) { >>>> + struct intel_wm_level *wm = &pipe_wm->wm[level]; >>>> >>>> ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, >>>> pristate, sprstate, curstate, wm); >>>> @@ -2409,13 +2404,10 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >>>> * register maximums since such watermarks are >>>> * always invalid. >>>> */ >>>> - if (level > usable_level) >>>> - continue; >>>> - >>>> - if (ilk_validate_wm_level(level, &max, wm)) >>>> - pipe_wm->wm[level] = *wm; >>>> - else >>>> - usable_level = level; >>>> + if (!ilk_validate_wm_level(level, &max, wm)) { >>>> + memset(wm, 0, sizeof(*wm)); >>>> + break; >>>> + } >>>> } >>>> >>>> return 0; >>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >>>> index a19ec06..3ce9ba3 100644 >>>> --- a/drivers/gpu/drm/i915/intel_drv.h >>>> +++ b/drivers/gpu/drm/i915/intel_drv.h >>>> @@ -457,7 +457,6 @@ struct intel_crtc_scaler_state { >>>> >>>> struct intel_pipe_wm { >>>> struct intel_wm_level wm[5]; >>>> - struct intel_wm_level raw_wm[5]; >>>> uint32_t linetime; >>>> bool fbc_wm_enabled; >>>> bool pipe_enabled; >>> Ok, so this looks like commit 8777b927b92cf5b6c29f9f9d3c737addea9ac8a7 >>> upstream which is commit 7de694782cbe7840f2c0de6f1e70f41fc1b8b6e8 in >>> 4.9.62. >>> >>> I've cc:ed the authors of that patch now. >>> >>> Maarten, any hints? Should I revert this from 4.9-stable, or was there >>> a follow-on patch that resolved this issue in mainline? >>> >>> thanks, >>> >>> greg k-h >>> >> OK, after reverting the patches, the flicker *is* gone. > Thanks for confirming this. > >> BTW (for the future): Was it the right way to address >> stable@vger.kernel.org in this matter or would the bugreport at >> freedesktop.org have been enough? I'm a bit unsure about that. > I have no idea what the i915 developers want, but as far as I'm > concerned, sending this to stable@vger was fine with me, I have no > problem doing a bit of work in tracking down the specific patch before > bugging the developers involved. Well, this one we wanted to be backported, and so indicated with cc: stable, but apparently it went south anyway. :(
Rainer, does v4.14 work for you? I.e. is the commit okay or not before the backport?
Maarten?
BR, Jani.
4.14 is OK, no problems.
So long! Rainer Fiebig
What happens when you apply both other backported patches on top?
So I applied the 2 new patches on top of the orig. 4.9.62.
Better now but not yet completely OK:
... [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun ...
No random flicker so far but flicker when I enter a character in an x-terminal.
Seems your patches fixed only half of the pipes. ;)
So long! Rainer Fiebig
There's also random flicker after a while.
Ok, I'm just going to revert this patch for the next release unless one of the i915 developers has a better idea...
thanks,
greg k-h
Right decision, thanks! I'm still offering to test patches in this matter once the devs think they've figured it out.
So long! Rainer Fiebig
Greg KH wrote:
On Thu, Nov 23, 2017 at 10:09:25PM +0100, Rainer Fiebig wrote:
Rainer Fiebig wrote:
Maarten Lankhorst wrote:
Op 20-11-17 om 09:51 schreef Rainer Fiebig:
Jani Nikula wrote:
On Sun, 19 Nov 2017, Greg KH gregkh@linuxfoundation.org wrote: > On Sun, Nov 19, 2017 at 01:44:06PM +0100, Rainer Fiebig wrote: >> Greg KH wrote: >>> On Sun, Nov 19, 2017 at 12:56:26PM +0100, Rainer Fiebig wrote: >>>> Greg KH wrote: >>>>> On Sat, Nov 18, 2017 at 05:08:20PM +0100, Rainer Fiebig wrote: >>>>>> Greg KH wrote: >>>>>>> On Sat, Nov 18, 2017 at 01:47:32PM +0100, Rainer Fiebig wrote: >>>>>>>> Hi! >>>>>>>> >>>>>>>> Hopefully the right addressee. >>>>>>>> >>>>>>>> Encountered two bad backports which cause screen-flicker. >>>>>>>> dmesg shows: >>>>>>>> >>>>>>>> ... >>>>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun >>>>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun >>>>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun >>>>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun >>>>>>>> ... >>>>>>>> >>>>>>>> CPU: Intel Core i3 (Clarkdale/Ironlake) >>>>>>>> >>>>>>>> The backports are: >>>>>>>> >>>>>>>> - diff --git a/drivers/gpu/drm/i915/intel_pm.c >>>>>>>> b/drivers/gpu/drm/i915/intel_pm.c >>>>>>>> index 49de476..277a802 100644 >>>>>>>> - diff --git a/drivers/gpu/drm/i915/intel_drv.h >>>>>>>> b/drivers/gpu/drm/i915/intel_drv.h >>>>>>>> index a19ec06..3ce9ba3 100644 >>>>>>>> >>>>>>>> After reversing them the flicker is gone, no more messages in dmesg. All >>>>>>>> else OK so far. >>>>>>> So which commit was the one that caused the problem? I will be glad to >>>>>>> revert it. >>>>>>> >>>>>>> thanks, >>>>>>> >>>>>>> greg k-h >>>>>>> >>>>>>> >>>>>> I started by reverting the more complex one first ("index >>>>>> 49de476..277a802100644"). But the kernel wouldn't compile then. >>>>> What git commit id is that? I don't see those ids in the 4.9-stable >>>>> tree. >>>>> >>>>>> So I also reverted "index a19ec06..3ce9ba3 100644". After that the >>>>>> kernel compiled just fine and the problems were gone (still are). >>>>> Same here, what git commit id was this? >>>>> >>>>> thanks, >>>>> >>>>> greg k-h >>>>> >>>> OK, no mistake. IIRC, I took the patches (and the IDs) from the >>>> changelog for patch-4.9.62. I've attached both, so you can check yourself. >>>> >>>> I've also applied a freshly downloaded patch-4.9.62 to a freshly >>>> expanded 4.9 and re-compiled. The flicker is there. I haven't yet >>>> reverted the two patches but I'm confident that after having done so the >>>> flicker will be gone. If not I'll let you know. >>>> >>>> As a good news: 4.14 is *not* affected. So to me it seems those two >>>> patches are part of sort of a package and can not be backported alone. >>>> >>>> So long! >>>> Rainer Fiebig >>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >>>> index 49de476..277a802 100644 >>>> --- a/drivers/gpu/drm/i915/intel_pm.c >>>> +++ b/drivers/gpu/drm/i915/intel_pm.c >>>> @@ -27,6 +27,7 @@ >>>> >>>> #include <linux/cpufreq.h> >>>> #include <drm/drm_plane_helper.h> >>>> +#include <drm/drm_atomic_helper.h> >>>> #include "i915_drv.h" >>>> #include "intel_drv.h" >>>> #include "../../../platform/x86/intel_ips.h" >>>> @@ -2017,9 +2018,9 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv, >>>> const struct intel_crtc *intel_crtc, >>>> int level, >>>> struct intel_crtc_state *cstate, >>>> - struct intel_plane_state *pristate, >>>> - struct intel_plane_state *sprstate, >>>> - struct intel_plane_state *curstate, >>>> + const struct intel_plane_state *pristate, >>>> + const struct intel_plane_state *sprstate, >>>> + const struct intel_plane_state *curstate, >>>> struct intel_wm_level *result) >>>> { >>>> uint16_t pri_latency = dev_priv->wm.pri_latency[level]; >>>> @@ -2341,28 +2342,24 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >>>> struct intel_pipe_wm *pipe_wm; >>>> struct drm_device *dev = state->dev; >>>> const struct drm_i915_private *dev_priv = to_i915(dev); >>>> - struct intel_plane *intel_plane; >>>> - struct intel_plane_state *pristate = NULL; >>>> - struct intel_plane_state *sprstate = NULL; >>>> - struct intel_plane_state *curstate = NULL; >>>> + struct drm_plane *plane; >>>> + const struct drm_plane_state *plane_state; >>>> + const struct intel_plane_state *pristate = NULL; >>>> + const struct intel_plane_state *sprstate = NULL; >>>> + const struct intel_plane_state *curstate = NULL; >>>> int level, max_level = ilk_wm_max_level(dev), usable_level; >>>> struct ilk_wm_maximums max; >>>> >>>> pipe_wm = &cstate->wm.ilk.optimal; >>>> >>>> - for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) { >>>> - struct intel_plane_state *ps; >>>> + drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, &cstate->base) { >>>> + const struct intel_plane_state *ps = to_intel_plane_state(plane_state); >>>> >>>> - ps = intel_atomic_get_existing_plane_state(state, >>>> - intel_plane); >>>> - if (!ps) >>>> - continue; >>>> - >>>> - if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY) >>>> + if (plane->type == DRM_PLANE_TYPE_PRIMARY) >>>> pristate = ps; >>>> - else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY) >>>> + else if (plane->type == DRM_PLANE_TYPE_OVERLAY) >>>> sprstate = ps; >>>> - else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR) >>>> + else if (plane->type == DRM_PLANE_TYPE_CURSOR) >>>> curstate = ps; >>>> } >>>> >>>> @@ -2384,11 +2381,9 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >>>> if (pipe_wm->sprites_scaled) >>>> usable_level = 0; >>>> >>>> - ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, >>>> - pristate, sprstate, curstate, &pipe_wm->raw_wm[0]); >>>> - >>>> memset(&pipe_wm->wm, 0, sizeof(pipe_wm->wm)); >>>> - pipe_wm->wm[0] = pipe_wm->raw_wm[0]; >>>> + ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, >>>> + pristate, sprstate, curstate, &pipe_wm->wm[0]); >>>> >>>> if (IS_HASWELL(dev) || IS_BROADWELL(dev)) >>>> pipe_wm->linetime = hsw_compute_linetime_wm(cstate); >>>> @@ -2398,8 +2393,8 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >>>> >>>> ilk_compute_wm_reg_maximums(dev, 1, &max); >>>> >>>> - for (level = 1; level <= max_level; level++) { >>>> - struct intel_wm_level *wm = &pipe_wm->raw_wm[level]; >>>> + for (level = 1; level <= usable_level; level++) { >>>> + struct intel_wm_level *wm = &pipe_wm->wm[level]; >>>> >>>> ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, >>>> pristate, sprstate, curstate, wm); >>>> @@ -2409,13 +2404,10 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >>>> * register maximums since such watermarks are >>>> * always invalid. >>>> */ >>>> - if (level > usable_level) >>>> - continue; >>>> - >>>> - if (ilk_validate_wm_level(level, &max, wm)) >>>> - pipe_wm->wm[level] = *wm; >>>> - else >>>> - usable_level = level; >>>> + if (!ilk_validate_wm_level(level, &max, wm)) { >>>> + memset(wm, 0, sizeof(*wm)); >>>> + break; >>>> + } >>>> } >>>> >>>> return 0; >>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >>>> index a19ec06..3ce9ba3 100644 >>>> --- a/drivers/gpu/drm/i915/intel_drv.h >>>> +++ b/drivers/gpu/drm/i915/intel_drv.h >>>> @@ -457,7 +457,6 @@ struct intel_crtc_scaler_state { >>>> >>>> struct intel_pipe_wm { >>>> struct intel_wm_level wm[5]; >>>> - struct intel_wm_level raw_wm[5]; >>>> uint32_t linetime; >>>> bool fbc_wm_enabled; >>>> bool pipe_enabled; >>> Ok, so this looks like commit 8777b927b92cf5b6c29f9f9d3c737addea9ac8a7 >>> upstream which is commit 7de694782cbe7840f2c0de6f1e70f41fc1b8b6e8 in >>> 4.9.62. >>> >>> I've cc:ed the authors of that patch now. >>> >>> Maarten, any hints? Should I revert this from 4.9-stable, or was there >>> a follow-on patch that resolved this issue in mainline? >>> >>> thanks, >>> >>> greg k-h >>> >> OK, after reverting the patches, the flicker *is* gone. > Thanks for confirming this. > >> BTW (for the future): Was it the right way to address >> stable@vger.kernel.org in this matter or would the bugreport at >> freedesktop.org have been enough? I'm a bit unsure about that. > I have no idea what the i915 developers want, but as far as I'm > concerned, sending this to stable@vger was fine with me, I have no > problem doing a bit of work in tracking down the specific patch before > bugging the developers involved. Well, this one we wanted to be backported, and so indicated with cc: stable, but apparently it went south anyway. :(
Rainer, does v4.14 work for you? I.e. is the commit okay or not before the backport?
Maarten?
BR, Jani.
4.14 is OK, no problems.
So long! Rainer Fiebig
What happens when you apply both other backported patches on top?
So I applied the 2 new patches on top of the orig. 4.9.62.
Better now but not yet completely OK:
... [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun ...
No random flicker so far but flicker when I enter a character in an x-terminal.
Seems your patches fixed only half of the pipes. ;)
So long! Rainer Fiebig
There's also random flicker after a while.
Ok, I'm just going to revert this patch for the next release unless one of the i915 developers has a better idea...
thanks,
greg k-h
I assume this implies that you also revert the two backports for 4.9.62 that startet the problem?
So long! Rainer Fiebig
On Fri, Nov 24, 2017 at 07:48:22AM +0100, Greg KH wrote:
On Thu, Nov 23, 2017 at 10:09:25PM +0100, Rainer Fiebig wrote:
Rainer Fiebig wrote:
Maarten Lankhorst wrote:
Op 20-11-17 om 09:51 schreef Rainer Fiebig:
Jani Nikula wrote:
On Sun, 19 Nov 2017, Greg KH gregkh@linuxfoundation.org wrote: > On Sun, Nov 19, 2017 at 01:44:06PM +0100, Rainer Fiebig wrote: >> Greg KH wrote: >>> On Sun, Nov 19, 2017 at 12:56:26PM +0100, Rainer Fiebig wrote: >>>> Greg KH wrote: >>>>> On Sat, Nov 18, 2017 at 05:08:20PM +0100, Rainer Fiebig wrote: >>>>>> Greg KH wrote: >>>>>>> On Sat, Nov 18, 2017 at 01:47:32PM +0100, Rainer Fiebig wrote: >>>>>>>> Hi! >>>>>>>> >>>>>>>> Hopefully the right addressee. >>>>>>>> >>>>>>>> Encountered two bad backports which cause screen-flicker. >>>>>>>> dmesg shows: >>>>>>>> >>>>>>>> ... >>>>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun >>>>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun >>>>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun >>>>>>>> [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun >>>>>>>> ... >>>>>>>> >>>>>>>> CPU: Intel Core i3 (Clarkdale/Ironlake) >>>>>>>> >>>>>>>> The backports are: >>>>>>>> >>>>>>>> - diff --git a/drivers/gpu/drm/i915/intel_pm.c >>>>>>>> b/drivers/gpu/drm/i915/intel_pm.c >>>>>>>> index 49de476..277a802 100644 >>>>>>>> - diff --git a/drivers/gpu/drm/i915/intel_drv.h >>>>>>>> b/drivers/gpu/drm/i915/intel_drv.h >>>>>>>> index a19ec06..3ce9ba3 100644 >>>>>>>> >>>>>>>> After reversing them the flicker is gone, no more messages in dmesg. All >>>>>>>> else OK so far. >>>>>>> So which commit was the one that caused the problem? I will be glad to >>>>>>> revert it. >>>>>>> >>>>>>> thanks, >>>>>>> >>>>>>> greg k-h >>>>>>> >>>>>>> >>>>>> I started by reverting the more complex one first ("index >>>>>> 49de476..277a802100644"). But the kernel wouldn't compile then. >>>>> What git commit id is that? I don't see those ids in the 4.9-stable >>>>> tree. >>>>> >>>>>> So I also reverted "index a19ec06..3ce9ba3 100644". After that the >>>>>> kernel compiled just fine and the problems were gone (still are). >>>>> Same here, what git commit id was this? >>>>> >>>>> thanks, >>>>> >>>>> greg k-h >>>>> >>>> OK, no mistake. IIRC, I took the patches (and the IDs) from the >>>> changelog for patch-4.9.62. I've attached both, so you can check yourself. >>>> >>>> I've also applied a freshly downloaded patch-4.9.62 to a freshly >>>> expanded 4.9 and re-compiled. The flicker is there. I haven't yet >>>> reverted the two patches but I'm confident that after having done so the >>>> flicker will be gone. If not I'll let you know. >>>> >>>> As a good news: 4.14 is *not* affected. So to me it seems those two >>>> patches are part of sort of a package and can not be backported alone. >>>> >>>> So long! >>>> Rainer Fiebig >>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >>>> index 49de476..277a802 100644 >>>> --- a/drivers/gpu/drm/i915/intel_pm.c >>>> +++ b/drivers/gpu/drm/i915/intel_pm.c >>>> @@ -27,6 +27,7 @@ >>>> >>>> #include <linux/cpufreq.h> >>>> #include <drm/drm_plane_helper.h> >>>> +#include <drm/drm_atomic_helper.h> >>>> #include "i915_drv.h" >>>> #include "intel_drv.h" >>>> #include "../../../platform/x86/intel_ips.h" >>>> @@ -2017,9 +2018,9 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv, >>>> const struct intel_crtc *intel_crtc, >>>> int level, >>>> struct intel_crtc_state *cstate, >>>> - struct intel_plane_state *pristate, >>>> - struct intel_plane_state *sprstate, >>>> - struct intel_plane_state *curstate, >>>> + const struct intel_plane_state *pristate, >>>> + const struct intel_plane_state *sprstate, >>>> + const struct intel_plane_state *curstate, >>>> struct intel_wm_level *result) >>>> { >>>> uint16_t pri_latency = dev_priv->wm.pri_latency[level]; >>>> @@ -2341,28 +2342,24 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >>>> struct intel_pipe_wm *pipe_wm; >>>> struct drm_device *dev = state->dev; >>>> const struct drm_i915_private *dev_priv = to_i915(dev); >>>> - struct intel_plane *intel_plane; >>>> - struct intel_plane_state *pristate = NULL; >>>> - struct intel_plane_state *sprstate = NULL; >>>> - struct intel_plane_state *curstate = NULL; >>>> + struct drm_plane *plane; >>>> + const struct drm_plane_state *plane_state; >>>> + const struct intel_plane_state *pristate = NULL; >>>> + const struct intel_plane_state *sprstate = NULL; >>>> + const struct intel_plane_state *curstate = NULL; >>>> int level, max_level = ilk_wm_max_level(dev), usable_level; >>>> struct ilk_wm_maximums max; >>>> >>>> pipe_wm = &cstate->wm.ilk.optimal; >>>> >>>> - for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) { >>>> - struct intel_plane_state *ps; >>>> + drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, &cstate->base) { >>>> + const struct intel_plane_state *ps = to_intel_plane_state(plane_state); >>>> >>>> - ps = intel_atomic_get_existing_plane_state(state, >>>> - intel_plane); >>>> - if (!ps) >>>> - continue; >>>> - >>>> - if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY) >>>> + if (plane->type == DRM_PLANE_TYPE_PRIMARY) >>>> pristate = ps; >>>> - else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY) >>>> + else if (plane->type == DRM_PLANE_TYPE_OVERLAY) >>>> sprstate = ps; >>>> - else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR) >>>> + else if (plane->type == DRM_PLANE_TYPE_CURSOR) >>>> curstate = ps; >>>> } >>>> >>>> @@ -2384,11 +2381,9 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >>>> if (pipe_wm->sprites_scaled) >>>> usable_level = 0; >>>> >>>> - ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, >>>> - pristate, sprstate, curstate, &pipe_wm->raw_wm[0]); >>>> - >>>> memset(&pipe_wm->wm, 0, sizeof(pipe_wm->wm)); >>>> - pipe_wm->wm[0] = pipe_wm->raw_wm[0]; >>>> + ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, >>>> + pristate, sprstate, curstate, &pipe_wm->wm[0]); >>>> >>>> if (IS_HASWELL(dev) || IS_BROADWELL(dev)) >>>> pipe_wm->linetime = hsw_compute_linetime_wm(cstate); >>>> @@ -2398,8 +2393,8 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >>>> >>>> ilk_compute_wm_reg_maximums(dev, 1, &max); >>>> >>>> - for (level = 1; level <= max_level; level++) { >>>> - struct intel_wm_level *wm = &pipe_wm->raw_wm[level]; >>>> + for (level = 1; level <= usable_level; level++) { >>>> + struct intel_wm_level *wm = &pipe_wm->wm[level]; >>>> >>>> ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, >>>> pristate, sprstate, curstate, wm); >>>> @@ -2409,13 +2404,10 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate) >>>> * register maximums since such watermarks are >>>> * always invalid. >>>> */ >>>> - if (level > usable_level) >>>> - continue; >>>> - >>>> - if (ilk_validate_wm_level(level, &max, wm)) >>>> - pipe_wm->wm[level] = *wm; >>>> - else >>>> - usable_level = level; >>>> + if (!ilk_validate_wm_level(level, &max, wm)) { >>>> + memset(wm, 0, sizeof(*wm)); >>>> + break; >>>> + } >>>> } >>>> >>>> return 0; >>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >>>> index a19ec06..3ce9ba3 100644 >>>> --- a/drivers/gpu/drm/i915/intel_drv.h >>>> +++ b/drivers/gpu/drm/i915/intel_drv.h >>>> @@ -457,7 +457,6 @@ struct intel_crtc_scaler_state { >>>> >>>> struct intel_pipe_wm { >>>> struct intel_wm_level wm[5]; >>>> - struct intel_wm_level raw_wm[5]; >>>> uint32_t linetime; >>>> bool fbc_wm_enabled; >>>> bool pipe_enabled; >>> Ok, so this looks like commit 8777b927b92cf5b6c29f9f9d3c737addea9ac8a7 >>> upstream which is commit 7de694782cbe7840f2c0de6f1e70f41fc1b8b6e8 in >>> 4.9.62. >>> >>> I've cc:ed the authors of that patch now. >>> >>> Maarten, any hints? Should I revert this from 4.9-stable, or was there >>> a follow-on patch that resolved this issue in mainline? >>> >>> thanks, >>> >>> greg k-h >>> >> OK, after reverting the patches, the flicker *is* gone. > Thanks for confirming this. > >> BTW (for the future): Was it the right way to address >> stable@vger.kernel.org in this matter or would the bugreport at >> freedesktop.org have been enough? I'm a bit unsure about that. > I have no idea what the i915 developers want, but as far as I'm > concerned, sending this to stable@vger was fine with me, I have no > problem doing a bit of work in tracking down the specific patch before > bugging the developers involved. Well, this one we wanted to be backported, and so indicated with cc: stable, but apparently it went south anyway. :(
Rainer, does v4.14 work for you? I.e. is the commit okay or not before the backport?
Maarten?
BR, Jani.
4.14 is OK, no problems.
So long! Rainer Fiebig
What happens when you apply both other backported patches on top?
So I applied the 2 new patches on top of the orig. 4.9.62.
Better now but not yet completely OK:
... [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun ...
No random flicker so far but flicker when I enter a character in an x-terminal.
Seems your patches fixed only half of the pipes. ;)
So long! Rainer Fiebig
There's also random flicker after a while.
Ok, I'm just going to revert this patch for the next release unless one of the i915 developers has a better idea...
Now reverted.
thanks,
greg k-h
Greg KH wrote:
On Sat, Nov 18, 2017 at 01:47:32PM +0100, Rainer Fiebig wrote:
Hi!
Hopefully the right addressee.
Encountered two bad backports which cause screen-flicker. dmesg shows:
... [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder A FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun [drm:ironlake_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun ...
CPU: Intel Core i3 (Clarkdale/Ironlake)
The backports are:
- diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c index 49de476..277a802 100644
- diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h index a19ec06..3ce9ba3 100644
After reversing them the flicker is gone, no more messages in dmesg. All else OK so far.
So which commit was the one that caused the problem? I will be glad to revert it.
thanks,
greg k-h
OK, when reversing only "index 49de476..277a802 100644" the kernel doesn't compile.
When reversing only "index a19ec06..3ce9ba3 100644" the kernel compiles but the flicker is there.
Seems those patches are a nasty couple that needs to be removed as such. ;)
So long! Rainer Fiebig
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Jani Nikula -
Maarten Lankhorst -
Rainer Fiebig