From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 03/13] drm/i915: Split watermark level computation from the code Date: Wed, 7 Aug 2013 12:56:48 +0300 Message-ID: <20130807095648.GP5004@intel.com> References: <1375817052-32310-1-git-send-email-ville.syrjala@linux.intel.com> <1375817052-32310-4-git-send-email-ville.syrjala@linux.intel.com> <20130806205631.GO8181@cantiga.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id EE79EE5CAB for ; Wed, 7 Aug 2013 02:57:09 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20130806205631.GO8181@cantiga.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Chris Wilson , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Tue, Aug 06, 2013 at 09:56:31PM +0100, Chris Wilson wrote: > On Tue, Aug 06, 2013 at 10:24:02PM +0300, ville.syrjala@linux.intel.com w= rote: > > From: Ville Syrj=E4l=E4 > > = > > Refactor the watermarks computation for one level to a separate > > function. This function will now set the ->enable flat to true, > = > s/flat/flag/ > = > Though I did think you meant a flatten value at one point. > = > > even if the watermark level wasn't actually checked yet. In the > > future we will delay the checking so we must consider all unchecked > > watermarks as possibly valid. > > = > > v2: Preserve comment about latency units > > = > > Signed-off-by: Ville Syrj=E4l=E4 > = > I don't have the context here to confirm passing level to > ilk_compute_pri_wm is exactly what you want, and it seems that level is > implicitly greater than 0 in the original code. > = > The transformation looks fine by itself. If you can calm my quirms over > the value of level (in the changelog is fine), Looks like Daniel already took this one. But yeah, I guess we should really pass 'level > 0' to make it clearer that it's a boolean. If you recall I had a patch in the original series to pass level all the way down, but I dropped it since it didn't actually do any good. As far as level=3D0, I didn't convert the hsw_compute_wm_pipe() to use the new function. I could do that as a followup patch. > Reviewed-by: Chris Wilson > -Chris > = > -- = > Chris Wilson, Intel Open Source Technology Centre -- = Ville Syrj=E4l=E4 Intel OTC