From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:46762 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757261AbcJGPbE (ORCPT ); Fri, 7 Oct 2016 11:31:04 -0400 Message-ID: <1475854262.30987.0.camel@redhat.com> Subject: Re: [Intel-gfx] [PATCH] drm/i915/gen9: fix DDB partitioning for multi-screen cases From: Lyude To: Paulo Zanoni , intel-gfx@lists.freedesktop.org Cc: stable@vger.kernel.org Date: Fri, 07 Oct 2016 11:31:02 -0400 In-Reply-To: <1475602652-17326-1-git-send-email-paulo.r.zanoni@intel.com> References: <1475602652-17326-1-git-send-email-paulo.r.zanoni@intel.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: stable-owner@vger.kernel.org List-ID: Sorry about how long this took! Anyway, finally got to testing it Reviewed-by: Lyude 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@vger.kernel.org > Signed-off-by: Paulo Zanoni > --- >  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