Sorry about how long this took! Anyway, finally got to testing it Reviewed-by: Lyude <cpaul@xxxxxxxxxx> On Tue, 2016-10-04 at 14:37 -0300, Paulo Zanoni wrote: > With the previous code we were only recomputing the DDB partitioning > for the CRTCs included in the atomic commit, so any other active > CRTCs > would end up having their DDB registers zeroed. In this patch we make > sure that the computed state starts as a copy of the current > partitioning, and then we only zero the DDBs that we're actually > going to recompute. > > How to reproduce the bug: > 1 - Enable the primary plane on pipe A > 2 - Enable the primary plane on pipe B > 3 - Enable the sprite plane on pipe A > > Step 3 will zero the DDB partitioning for pipe B since it's not > included in the commit that enabled the sprite for pipe A. > > I expect this to fix many FIFO underrun problems on gen9+. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96226 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96828 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97450 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97596 > Bugzilla: https://www.phoronix.com/scan.php?page=news_item&px=Intel-S > kylake-Multi-Screen-Woes > Cc: stable@xxxxxxxxxxxxxxx > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_pm.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > I still have to confirm whether this closes the above bugs, but it > certainly fixes the problem I described. > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > b/drivers/gpu/drm/i915/intel_pm.c > index 425544b..0c2e252 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3362,13 +3362,15 @@ skl_allocate_pipe_ddb(struct intel_crtc_state > *cstate, > int num_active; > int id, i; > > + /* Clear the partitioning for disabled planes. */ > + memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe])); > + memset(ddb->y_plane[pipe], 0, sizeof(ddb->y_plane[pipe])); > + > if (WARN_ON(!state)) > return 0; > > if (!cstate->base.active) { > ddb->pipe[pipe].start = ddb->pipe[pipe].end = 0; > - memset(ddb->plane[pipe], 0, sizeof(ddb- > >plane[pipe])); > - memset(ddb->y_plane[pipe], 0, sizeof(ddb- > >y_plane[pipe])); > return 0; > } > > @@ -4054,6 +4056,12 @@ skl_compute_ddb(struct drm_atomic_state > *state) > intel_state->wm_results.dirty_pipes = ~0; > } > > + /* > + * We're not recomputing for the pipes not included in the > commit, so > + * make sure we start with the current state. > + */ > + memcpy(ddb, &dev_priv->wm.skl_hw.ddb, sizeof(*ddb)); > + > for_each_intel_crtc_mask(dev, intel_crtc, realloc_pipes) { > struct intel_crtc_state *cstate; > -- Cheers, Lyude -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html