From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 48/62] drm/i915/bdw: Add Broadwell display FIFO limits Date: Mon, 4 Nov 2013 15:59:37 +0200 Message-ID: <20131104135937.GQ13047@intel.com> References: <1383451680-11173-1-git-send-email-benjamin.widawsky@intel.com> <1383451680-11173-49-git-send-email-benjamin.widawsky@intel.com> <87a9hkcxoz.fsf@intel.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 4D868131E8B for ; Mon, 4 Nov 2013 05:59:41 -0800 (PST) Content-Disposition: inline In-Reply-To: <87a9hkcxoz.fsf@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Jani Nikula Cc: Daniel Vetter , Intel GFX , Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org On Mon, Nov 04, 2013 at 11:39:56AM +0200, Jani Nikula wrote: > On Sun, 03 Nov 2013, Ben Widawsky wrote: > > From: Ville Syrj=E4l=E4 > > > > Broadwell has bigger display FIFOs than Haswell. Otherwise the > > two are very similar. > > > > v2: Fix FBC WM_LP shift for BDW > > > > v3: Rebase on top of the big Haswell wm rework. > > > > Signed-off-by: Ville Syrj=E4l=E4 (v2) > > Signed-off-by: Daniel Vetter > > --- > > drivers/gpu/drm/i915/i915_reg.h | 1 + > > drivers/gpu/drm/i915/intel_pm.c | 33 ++++++++++++++++++++++++--------- > > 2 files changed, 25 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i91= 5_reg.h > > index 6f834b3..2a65f92 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -3366,6 +3366,7 @@ > > #define WM1_LP_LATENCY_MASK (0x7f<<24) > > #define WM1_LP_FBC_MASK (0xf<<20) > > #define WM1_LP_FBC_SHIFT 20 > > +#define WM1_LP_FBC_SHIFT_BDW 19 > > #define WM1_LP_SR_MASK (0x7ff<<8) > = > Meh, the above _MASKs would need some love too. WM1_LP_SR_MASK is wrong > for HSW already I think. But nobody's using them, so not a blocker for > this patch. > = > > #define WM1_LP_SR_SHIFT 8 > > #define WM1_LP_CURSOR_MASK (0xff) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/int= el_pm.c > > index 68dc363..6d14182 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -2291,7 +2291,9 @@ static uint32_t ilk_compute_fbc_wm(const struct h= sw_pipe_wm_parameters *params, > > = > > static unsigned int ilk_display_fifo_size(const struct drm_device *dev) > > { > > - if (INTEL_INFO(dev)->gen >=3D 7) > > + if (INTEL_INFO(dev)->gen >=3D 8) > > + return 3072; > = > It's probably just me, but I couldn't find this in the spec, so can't > verify. Looks like it's not spelled out there anymore. But you can figure it out by looking at the single pipe primary:sprite 1:1 split numbers (1536 * 2 =3D 3072) or the multi pipe primary only numbers (1024 * 3 =3D 30= 72). > Apart from that, > = > Reviewed-by: Jani Nikula > = > ...but read on, some non-blocking bikeshedding below. > = > > + else if (INTEL_INFO(dev)->gen >=3D 7) > > return 768; > > else > > return 512; > > @@ -2336,7 +2338,9 @@ static unsigned int ilk_plane_wm_max(const struct= drm_device *dev, > > } > > = > > /* clamp to max that the registers can hold */ > > - if (INTEL_INFO(dev)->gen >=3D 7) > > + if (INTEL_INFO(dev)->gen >=3D 8) > > + max =3D level =3D=3D 0 ? 255 : 2047; > = > Not having looked at the WM stuff before, it took me a while to find the > registers and check the maximums. Which made me wonder why we don't fix > the masks and use them here, so it would be bloody obvious what we're > referring to? > = > if (level) > max =3D WM1_LP_SR_MASK_BDW >> WM1_LP_SR_SHIFT_BDW; > else > max =3D WM0_PIPE_PLANE_MASK_BDW >> WM0_PIPE_PLANE_SHIFT; I just extended the masks to cover all platforms. That makes hw state readout a bit simpler since I don't need to worry about the differences between generations there. But that means the masks don't necessarily correspond to any specific platform, and so we can't use them here. I could define per-platforms masks too, but that seems rather pointless since there would be but one user. > = > > + else if (INTEL_INFO(dev)->gen >=3D 7) > > /* IVB/HSW primary/sprite plane watermarks */ > > max =3D level =3D=3D 0 ? 127 : 1023; > > else if (!is_sprite) > > @@ -2366,10 +2370,13 @@ static unsigned int ilk_cursor_wm_max(const str= uct drm_device *dev, > > } > > = > > /* Calculate the maximum FBC watermark */ > > -static unsigned int ilk_fbc_wm_max(void) > > +static unsigned int ilk_fbc_wm_max(struct drm_device *dev) > > { > > /* max that registers can hold */ > > - return 15; > > + if (INTEL_INFO(dev)->gen >=3D 8) > > + return 31; > > + else > > + return 15; > > } > > = > > static void ilk_compute_wm_maximums(struct drm_device *dev, > > @@ -2381,7 +2388,7 @@ static void ilk_compute_wm_maximums(struct drm_de= vice *dev, > > max->pri =3D ilk_plane_wm_max(dev, level, config, ddb_partitioning, f= alse); > > max->spr =3D ilk_plane_wm_max(dev, level, config, ddb_partitioning, t= rue); > > max->cur =3D ilk_cursor_wm_max(dev, level, config); > > - max->fbc =3D ilk_fbc_wm_max(); > > + max->fbc =3D ilk_fbc_wm_max(dev); > > } > > = > > static bool ilk_validate_wm_level(int level, > > @@ -2722,10 +2729,18 @@ static void hsw_compute_wm_results(struct drm_d= evice *dev, > > if (!r->enable) > > break; > > = > > - results->wm_lp[wm_lp - 1] =3D HSW_WM_LP_VAL(level * 2, > > - r->fbc_val, > > - r->pri_val, > > - r->cur_val); > = > This leaves HSW_WM_LP_VAL() macro unused. Yeah. I should just kill it. > = > > + results->wm_lp[wm_lp - 1] =3D WM3_LP_EN | > > + ((level * 2) << WM1_LP_LATENCY_SHIFT) | > > + (r->pri_val << WM1_LP_SR_SHIFT) | > > + r->cur_val; > > + > > + if (INTEL_INFO(dev)->gen >=3D 8) > > + results->wm_lp[wm_lp - 1] |=3D > > + r->fbc_val << WM1_LP_FBC_SHIFT_BDW; > > + else > > + results->wm_lp[wm_lp - 1] |=3D > > + r->fbc_val << WM1_LP_FBC_SHIFT; > > + > > results->wm_lp_spr[wm_lp - 1] =3D r->spr_val; > > } > > = > > -- = > > 1.8.4.2 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > = > -- = > Jani Nikula, Intel Open Source Technology Center > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Ville Syrj=E4l=E4 Intel OTC