Maarten Lankhorst wrote: > Op 20-11-17 om 09:51 schreef Rainer Fiebig: >> Jani Nikula wrote: >>> On Sun, 19 Nov 2017, Greg KH <gre...@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 >>>>> sta...@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? > > https://cgit.freedesktop.org/~mlankhorst/linux/log/?h=v4.9 >
On top of 4.14? _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx