From patchwork Tue Jul 19 15:50:49 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 9237759 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 10F51602F0 for ; Tue, 19 Jul 2016 15:51:03 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 01A6D2624C for ; Tue, 19 Jul 2016 15:51:03 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id EA20D2756B; Tue, 19 Jul 2016 15:51:02 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 74A012624C for ; Tue, 19 Jul 2016 15:51:02 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id EEBEF6E6AC; Tue, 19 Jul 2016 15:50:57 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from fireflyinternet.com (mail.fireflyinternet.com [109.228.58.192]) by gabe.freedesktop.org (Postfix) with ESMTP id A074F6E6AC for ; Tue, 19 Jul 2016 15:50:54 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from nuc-i3427.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 569002-1500050 for multiple; Tue, 19 Jul 2016 16:50:51 +0100 Received: by nuc-i3427.alporthouse.com (sSMTP sendmail emulation); Tue, 19 Jul 2016 16:50:49 +0100 Date: Tue, 19 Jul 2016 16:50:49 +0100 From: Chris Wilson To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Message-ID: <20160719155049.GL17604@nuc-i3427.alporthouse.com> Mail-Followup-To: Chris Wilson , Ville =?iso-8859-1?Q?Syrj=E4l=E4?= , Maarten Lankhorst , yetundex.adebisi@intel.com, isg-gms@eclists.intel.com, Chi Ding , intel-gfx@lists.freedesktop.org References: <1466685388-16504-1-git-send-email-chix.ding@intel.com> <20160719152542.GN4329@intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20160719152542.GN4329@intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: yetundex.adebisi@intel.com, isg-gms@eclists.intel.com, Chi Ding , intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: Fix off-by-1 error in calculating num_levels. X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Jul 19, 2016 at 06:25:42PM +0300, Ville Syrjälä wrote: > On Tue, Jul 19, 2016 at 05:14:23PM +0200, Maarten Lankhorst wrote: > > num_levels should be level+1, not level, else num_levels - 1 becomes > > negative. This resulted in bogus watermarks being written to the first > > 255 levels like below: > > > > [drm] Setting FIFO watermarks - C: plane=0, cursor=0, sprite0=0, sprite1=0, SR: plane=0, cursor=0 level=255 cxsr=0 > > [drm:chv_set_memory_dvfs [i915]] *ERROR* timed out waiting for Punit DDR DVFS request > > [drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe C FIFO underrun > > [drm:chv_set_memory_dvfs [i915]] *ERROR* timed out waiting for Punit DDR DVFS request > > > > Testcase: kms_atomic_transition > > Fixes: 262cd2e154c2 ("drm/i915: CHV DDR DVFS support and another watermark rewrite") > > Cc: stable@vger.kernel.org > > Cc: Ville Syrjälä > > Signed-off-by: Maarten Lankhorst > > --- > > Urgent fix for watermark support. This is definitely a pre-requisite for this series. > > With this I've noticed that patch "[RFC 3/8] drm/i915/vlv: Move fifo_size from > > intel_plane_wm_parameters to vlv_wm_state" introduces a regression with invalid FIFO split. > > > > I need to find out what's going wrong in that patch before this series can be applied. > > > > drivers/gpu/drm/i915/intel_pm.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index 376c60b98515..8defdcc54529 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -1148,7 +1148,7 @@ static void vlv_compute_wm(struct intel_crtc *crtc) > > } > > } > > > > - wm_state->num_levels = level; > > + wm_state->num_levels = level + 1; > > Nope. The loop above breaks when the current level is bad, hence level-1 > is actually the higher usable level. Without knowing the limits of plane->wm.fifo_size, it looks like it can break on level == 0 though. Might as well set that hack to paranoid levels: diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 630b116988f6..e8c2874b8629 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1124,11 +1124,13 @@ static void vlv_compute_wm(struct intel_crtc *crtc) /* normal watermarks */ for (level = 0; level < wm_state->num_levels; level++) { int wm = vlv_compute_wm_level(plane, crtc, state, level); - int max_wm = plane->base.type == DRM_PLANE_TYPE_CURSOR ? 63 : 511; - /* hack */ - if (WARN_ON(level == 0 && wm > max_wm)) - wm = max_wm; + if (level == 0) { + int plane_wm = plane->base.type == DRM_PLANE_TYPE_CURSOR ? 63 : 511; + int max_wm = min(plane->wm.fifo_size, plane_wm); + if (WARN_ON(wm > max_wm)) + wm = max_wm; + } if (wm > plane->wm.fifo_size) break;