From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paulo Zanoni Subject: Re: [PATCH 04/16] drm/i915: Use intel_pipe_wm in hsw_find_best_results Date: Thu, 10 Oct 2013 19:20:09 -0300 Message-ID: References: <1381335490-4906-1-git-send-email-ville.syrjala@linux.intel.com> <1381335490-4906-5-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 mail-oa0-f48.google.com (mail-oa0-f48.google.com [209.85.219.48]) by gabe.freedesktop.org (Postfix) with ESMTP id A2284E62A2 for ; Thu, 10 Oct 2013 15:20:09 -0700 (PDT) Received: by mail-oa0-f48.google.com with SMTP id m6so1843553oag.7 for ; Thu, 10 Oct 2013 15:20:09 -0700 (PDT) In-Reply-To: <1381335490-4906-5-git-send-email-ville.syrjala@linux.intel.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: =?ISO-8859-1?Q?Ville_Syrj=E4l=E4?= Cc: Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org 2013/10/9 : > From: Ville Syrj=E4l=E4 > > Let's try to keep using the intermediate intel_pipe_wm representation > for as long as possible. It avoids subtle knowledge about the > internals of the hardware registers when trying to choose the > best watermark configuration. > > While at it replace the memset() w/ zero initialization. > > Signed-off-by: Ville Syrj=E4l=E4 Looks correct. These things are hard to review... So many structs! Reviewed-by: Paulo Zanoni > --- > drivers/gpu/drm/i915/intel_pm.c | 42 ++++++++++++++++++++---------------= ------ > 1 file changed, 21 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel= _pm.c > index c2d439f..b09715f 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2722,8 +2722,6 @@ static void hsw_compute_wm_results(struct drm_devic= e *dev, > struct intel_crtc *intel_crtc; > int level, wm_lp; > > - memset(results, 0, sizeof(*results)); > - > results->enable_fbc_wm =3D merged->fbc_wm_enabled; > > /* LP1+ register values */ > @@ -2763,24 +2761,26 @@ static void hsw_compute_wm_results(struct drm_dev= ice *dev, > > /* Find the result with the highest level enabled. Check for enable_fbc_= wm in > * case both are at the same level. Prefer r1 in case they're the same. = */ > -static struct hsw_wm_values *hsw_find_best_result(struct hsw_wm_values *= r1, > - struct hsw_wm_values *r= 2) > +static struct intel_pipe_wm *hsw_find_best_result(struct drm_device *dev, > + struct intel_pipe_wm *r= 1, > + struct intel_pipe_wm *r= 2) > { > - int i, val_r1 =3D 0, val_r2 =3D 0; > + int level, max_level =3D ilk_wm_max_level(dev); > + int level1 =3D 0, level2 =3D 0; > > - for (i =3D 0; i < 3; i++) { > - if (r1->wm_lp[i] & WM3_LP_EN) > - val_r1 =3D r1->wm_lp[i] & WM1_LP_LATENCY_MASK; > - if (r2->wm_lp[i] & WM3_LP_EN) > - val_r2 =3D r2->wm_lp[i] & WM1_LP_LATENCY_MASK; > + for (level =3D 1; level <=3D max_level; level++) { > + if (r1->wm[level].enable) > + level1 =3D level; > + if (r2->wm[level].enable) > + level2 =3D level; > } > > - if (val_r1 =3D=3D val_r2) { > - if (r2->enable_fbc_wm && !r1->enable_fbc_wm) > + if (level1 =3D=3D level2) { > + if (r2->fbc_wm_enabled && !r1->fbc_wm_enabled) > return r2; > else > return r1; > - } else if (val_r1 > val_r2) { > + } else if (level1 > level2) { > return r1; > } else { > return r2; > @@ -2891,10 +2891,10 @@ static void haswell_update_wm(struct drm_crtc *cr= tc) > struct drm_i915_private *dev_priv =3D dev->dev_private; > struct hsw_wm_maximums lp_max_1_2, lp_max_5_6; > struct hsw_pipe_wm_parameters params =3D {}; > - struct hsw_wm_values results_1_2, results_5_6, *best_results; > + struct hsw_wm_values results =3D {}; > enum intel_ddb_partitioning partitioning; > struct intel_pipe_wm pipe_wm =3D {}; > - struct intel_pipe_wm lp_wm_1_2 =3D {}, lp_wm_5_6 =3D {}; > + struct intel_pipe_wm lp_wm_1_2 =3D {}, lp_wm_5_6 =3D {}, *best_lp= _wm; > > hsw_compute_wm_parameters(crtc, ¶ms, &lp_max_1_2, &lp_max_5_6= ); > > @@ -2908,18 +2908,18 @@ static void haswell_update_wm(struct drm_crtc *cr= tc) > ilk_wm_merge(dev, &lp_max_1_2, &lp_wm_1_2); > ilk_wm_merge(dev, &lp_max_5_6, &lp_wm_5_6); > > - hsw_compute_wm_results(dev, &lp_wm_1_2, &results_1_2); > if (lp_max_1_2.pri !=3D lp_max_5_6.pri) { > - hsw_compute_wm_results(dev, &lp_wm_5_6, &results_5_6); > - best_results =3D hsw_find_best_result(&results_1_2, &resu= lts_5_6); > + best_lp_wm =3D hsw_find_best_result(dev, &lp_wm_1_2, &lp_= wm_5_6); > } else { > - best_results =3D &results_1_2; > + best_lp_wm =3D &lp_wm_1_2; > } > > - partitioning =3D (best_results =3D=3D &results_1_2) ? > + hsw_compute_wm_results(dev, best_lp_wm, &results); > + > + partitioning =3D (best_lp_wm =3D=3D &lp_wm_1_2) ? > INTEL_DDB_PART_1_2 : INTEL_DDB_PART_5_6; > > - hsw_write_wm_values(dev_priv, best_results, partitioning); > + hsw_write_wm_values(dev_priv, &results, partitioning); > } > > static void haswell_update_sprite_wm(struct drm_plane *plane, > -- > 1.8.1.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Paulo Zanoni