From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 06/35] drm/i915: Pass the watermark level to primary WM compute functions Date: Thu, 1 Aug 2013 11:01:58 +0300 Message-ID: <20130801080158.GH5004@intel.com> References: <1373014667-19484-1-git-send-email-ville.syrjala@linux.intel.com> <1373014667-19484-7-git-send-email-ville.syrjala@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga03.intel.com (mga03.intel.com [143.182.124.21]) by gabe.freedesktop.org (Postfix) with ESMTP id 83D24E7F05 for ; Thu, 1 Aug 2013 01:02:02 -0700 (PDT) Content-Disposition: inline In-Reply-To: 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: Paulo Zanoni Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Tue, Jul 30, 2013 at 04:49:18PM -0300, Paulo Zanoni wrote: > 2013/7/5 : > > From: Ville Syrj=E4l=E4 > > > > Passing the level insted of "is_lp" seems easier. The end result is the > > same though. > > > > Signed-off-by: Ville Syrj=E4l=E4 > > --- > > drivers/gpu/drm/i915/intel_pm.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/int= el_pm.c > > index 6b820c4..f178e26 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -2185,7 +2185,7 @@ enum hsw_data_buf_partitioning { > > /* For both WM_PIPE and WM_LP. */ > > static uint32_t ilk_compute_pri_wm(struct hsw_pipe_wm_parameters *para= ms, > > uint32_t mem_value, > > - bool is_lp) > > + int level) > > { > > uint32_t method1, method2; > > > > @@ -2197,7 +2197,7 @@ static uint32_t ilk_compute_pri_wm(struct hsw_pip= e_wm_parameters *params, > > params->pri_bytes_per_pixel, > > mem_value); > > > > - if (!is_lp) > > + if (level =3D=3D 0) > > return method1; > > > > method2 =3D ilk_wm_method2(params->pixel_rate, > > @@ -2266,7 +2266,7 @@ static bool hsw_compute_lp_wm(uint32_t mem_value,= struct hsw_wm_maximums *max, > > for (pipe =3D PIPE_A; pipe <=3D PIPE_C; pipe++) { > > struct hsw_pipe_wm_parameters *p =3D ¶ms[pipe]; > > > > - pri_val[pipe] =3D ilk_compute_pri_wm(p, mem_value, true= ); > > + pri_val[pipe] =3D ilk_compute_pri_wm(p, mem_value, 1); > = > But now even when you're processing levels 2/3/4 you're telling the > function that you're doing level 1, which is not true. We don't know > what kind of code changes we'll be doing in January 2023, so we may > use that incorrect "level" variable to do something wrong. IMHO, > either we keep the current code or we pass the actual level. Yeah. I guess we could keep it as bool now. The original reason for passing the level was that I handled the IVB cursor latency W/A inside the wm compute funcs, but now with the pre-populated latency info in dev_priv passing the level makes less sense. > = > = > > spr_val[pipe] =3D ilk_compute_spr_wm(p, mem_value); > > cur_val[pipe] =3D ilk_compute_cur_wm(p, mem_value); > > fbc_val[pipe] =3D ilk_compute_fbc_wm(p, pri_val[pipe], = mem_value); > > @@ -2296,7 +2296,7 @@ static uint32_t hsw_compute_wm_pipe(struct drm_i9= 15_private *dev_priv, > > { > > uint32_t pri_val, cur_val, spr_val; > > > > - pri_val =3D ilk_compute_pri_wm(params, mem_value, false); > > + pri_val =3D ilk_compute_pri_wm(params, mem_value, 0); > > spr_val =3D ilk_compute_spr_wm(params, mem_value); > > cur_val =3D ilk_compute_cur_wm(params, mem_value); > > > > -- > > 1.8.1.5 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > = > = > = > -- = > Paulo Zanoni -- = Ville Syrj=E4l=E4 Intel OTC