From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 05/16] drm/i915: Move some computations out from hsw_compute_wm_parameters() Date: Fri, 11 Oct 2013 11:26:29 +0300 Message-ID: <20131011082629.GR13047@intel.com> References: <1381335490-4906-1-git-send-email-ville.syrjala@linux.intel.com> <1381335490-4906-6-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 mga14.intel.com (mga14.intel.com [143.182.124.37]) by gabe.freedesktop.org (Postfix) with ESMTP id 9F639E5C6C for ; Fri, 11 Oct 2013 01:26:33 -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 Graphics Development List-Id: intel-gfx@lists.freedesktop.org On Thu, Oct 10, 2013 at 07:34:38PM -0300, Paulo Zanoni wrote: > 2013/10/9 : > > From: Ville Syrj=E4l=E4 > > > > Move the watermark max computations into haswell_update_wm(). This > > allows keeping the 1/2 vs. 5/6 split code in one place, and avoid having > > to pass around so many things. We also save a bit of stack space by only > > requiring one copy of struct hsw_wm_maximums. > > > > Also move the intel_wm_config out from hsw_compute_wm_parameters() and > > pass it it. We'll have some need for it in haswell_update_wm() later. > > > > Signed-off-by: Ville Syrj=E4l=E4 > > --- > > drivers/gpu/drm/i915/intel_pm.c | 33 ++++++++++++++------------------- > > 1 file changed, 14 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/int= el_pm.c > > index b09715f..0fe6c36 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -2586,13 +2586,11 @@ static void intel_setup_wm_latency(struct drm_d= evice *dev) > > > > static void hsw_compute_wm_parameters(struct drm_crtc *crtc, > > struct hsw_pipe_wm_parameters *p, > > - struct hsw_wm_maximums *lp_max_1_= 2, > > - struct hsw_wm_maximums *lp_max_5_= 6) > > + struct intel_wm_config *config) > > { > > struct drm_device *dev =3D crtc->dev; > > struct intel_crtc *intel_crtc =3D to_intel_crtc(crtc); > > enum pipe pipe =3D intel_crtc->pipe; > > - struct intel_wm_config config =3D {}; > > struct drm_plane *plane; > > > > p->active =3D intel_crtc_active(crtc); > > @@ -2609,7 +2607,7 @@ static void hsw_compute_wm_parameters(struct drm_= crtc *crtc, > > } > > > > list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) > > - config.num_pipes_active +=3D intel_crtc_active(crtc); > > + config->num_pipes_active +=3D intel_crtc_active(crtc); > > > > list_for_each_entry(plane, &dev->mode_config.plane_list, head) { > > struct intel_plane *intel_plane =3D to_intel_plane(plan= e); > > @@ -2617,17 +2615,9 @@ static void hsw_compute_wm_parameters(struct drm= _crtc *crtc, > > if (intel_plane->pipe =3D=3D pipe) > > p->spr =3D intel_plane->wm; > > > > - config.sprites_enabled |=3D intel_plane->wm.enabled; > > - config.sprites_scaled |=3D intel_plane->wm.scaled; > > + config->sprites_enabled |=3D intel_plane->wm.enabled; > > + config->sprites_scaled |=3D intel_plane->wm.scaled; > > } > > - > > - ilk_wm_max(dev, 1, &config, INTEL_DDB_PART_1_2, lp_max_1_2); > > - > > - /* 5/6 split only in single pipe config on IVB+ */ > > - if (INTEL_INFO(dev)->gen >=3D 7 && config.num_pipes_active <=3D= 1) > > - ilk_wm_max(dev, 1, &config, INTEL_DDB_PART_5_6, lp_max_= 5_6); > > - else > > - *lp_max_5_6 =3D *lp_max_1_2; > > } > > > > /* Compute new watermarks for the pipe */ > > @@ -2889,14 +2879,15 @@ static void haswell_update_wm(struct drm_crtc *= crtc) > > struct intel_crtc *intel_crtc =3D to_intel_crtc(crtc); > > struct drm_device *dev =3D crtc->dev; > > struct drm_i915_private *dev_priv =3D dev->dev_private; > > - struct hsw_wm_maximums lp_max_1_2, lp_max_5_6; > > + struct hsw_wm_maximums max; > > struct hsw_pipe_wm_parameters params =3D {}; > > 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 {}, *best_= lp_wm; > > + struct intel_wm_config config =3D {}; > > > > - hsw_compute_wm_parameters(crtc, ¶ms, &lp_max_1_2, &lp_max_5= _6); > > + hsw_compute_wm_parameters(crtc, ¶ms, &config); > > > > intel_compute_pipe_wm(crtc, ¶ms, &pipe_wm); > > > > @@ -2905,10 +2896,14 @@ static void haswell_update_wm(struct drm_crtc *= crtc) > > > > intel_crtc->wm.active =3D pipe_wm; > > > > - ilk_wm_merge(dev, &lp_max_1_2, &lp_wm_1_2); > > - ilk_wm_merge(dev, &lp_max_5_6, &lp_wm_5_6); > > + ilk_wm_max(dev, 1, &config, INTEL_DDB_PART_1_2, &max); > > + ilk_wm_merge(dev, &max, &lp_wm_1_2); > > + > > + /* 5/6 split only in single pipe config on IVB+ */ > > + if (INTEL_INFO(dev)->gen >=3D 7 && config.num_pipes_active <=3D= 1) { > = > No need to calculate 5_6 on zero pipes, I guess. > = > = > > + ilk_wm_max(dev, 1, &config, INTEL_DDB_PART_5_6, &max); > > + ilk_wm_merge(dev, &max, &lp_wm_5_6); > = > Oh, so now you've moved ilk_wm_merge to the "if" statement, as I > requested on the review to a previous patch :) > = > > > > - if (lp_max_1_2.pri !=3D lp_max_5_6.pri) { > = > By removing this check, you're now also calculating 5_6 watermarks for > the case where we just have 1 pipe but the sprites are disabled. Hmm, right. I guess we should just add a more explicit config.sprites_enabled check here to avoid that overhead. > = > = > > best_lp_wm =3D hsw_find_best_result(dev, &lp_wm_1_2, &l= p_wm_5_6); > > } else { > > best_lp_wm =3D &lp_wm_1_2; > > -- > > 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