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?