From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 25/40] drm/i915: Fill out the FWx watermark register defines Date: Fri, 1 Aug 2014 14:26:46 +0300 Message-ID: <20140801112646.GV4193@intel.com> References: <1403910271-24984-1-git-send-email-ville.syrjala@linux.intel.com> <1403910271-24984-26-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 mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 4C03B6E19B for ; Fri, 1 Aug 2014 04:26:50 -0700 (PDT) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Paulo Zanoni Cc: Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org On Thu, Jul 31, 2014 at 05:16:21PM -0300, Paulo Zanoni wrote: > 2014-06-27 20:04 GMT-03:00 : > > From: Ville Syrj=E4l=E4 > > > > Add defines for all the watermark registers on modernish gmch platforms. > > > > VLV has increased the number of bits available for certain watermaks so > > expand the masks appropriately. Also vlv and chv have added some extra > > FW registers. > > > > Not sure what happened on chv because a new register called FW9 is now > > at the offset where FW7 was on vlv, while FW7 and FW8 (another new > > register) have been moved off somewhere else. Oh well, well just need > > two defines for FW7 then. > > > > Signed-off-by: Ville Syrj=E4l=E4 > > --- > > drivers/gpu/drm/i915/i915_reg.h | 138 ++++++++++++++++++++++++++++++++= +++----- > > drivers/gpu/drm/i915/intel_pm.c | 11 ++-- > > 2 files changed, 130 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i91= 5_reg.h > > index 7ab5a03..9fab647 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -3884,28 +3884,136 @@ enum punit_power_well { > > #define DSPARB_BEND_SHIFT 9 /* on 855 */ > > #define DSPARB_AEND_SHIFT 0 > > > > +/* pnv/gen4/g4x/vlv/chv */ > > #define DSPFW1 (dev_priv->info.display_mmio_offset + 0= x70034) > > -#define DSPFW_SR_SHIFT 23 > > -#define DSPFW_SR_MASK (0x1ff<<23) > > -#define DSPFW_CURSORB_SHIFT 16 > > -#define DSPFW_CURSORB_MASK (0x3f<<16) > > -#define DSPFW_PLANEB_SHIFT 8 > > -#define DSPFW_PLANEB_MASK (0x7f<<8) > > -#define DSPFW_PLANEA_MASK (0x7f) > > +#define DSPFW_SR_SHIFT 23 > > +#define DSPFW_SR_MASK (0x1ff<<23) > > +#define DSPFW_CURSORB_SHIFT 16 > > +#define DSPFW_CURSORB_MASK (0x3f<<16) > > +#define DSPFW_PLANEB_SHIFT 8 > > +#define DSPFW_PLANEB_MASK (0x7f<<8) > > +#define DSPFW_PLANEB_MASK_VLV (0xff<<8) /* vlv/chv */ > > +#define DSPFW_PLANEA_SHIFT 0 > > +#define DSPFW_PLANEA_MASK (0x7f<<0) > > +#define DSPFW_PLANEA_MASK_VLV (0xff<<0) /* vlv/chv */ > > #define DSPFW2 (dev_priv->info.display_mmio_offset + 0= x70038) > > -#define DSPFW_CURSORA_MASK 0x00003f00 > > -#define DSPFW_CURSORA_SHIFT 8 > > -#define DSPFW_PLANEC_MASK (0x7f) > > +#define DSPFW_FBC_SR_EN (1<<31) /* g4x */ > > +#define DSPFW_FBC_SR_SHIFT 28 > > +#define DSPFW_FBC_SR_MASK (0x7<<28) /* g4x */ > > +#define DSPFW_FBC_HPLL_SR_SHIFT 24 > > +#define DSPFW_FBC_HPLL_SR_MASK (0xf<<24) /* g4x */ > > +#define DSPFW_SPRITEB_SHIFT (16) > > +#define DSPFW_SPRITEB_MASK (0x7f<<16) /* g4x */ > > +#define DSPFW_SPRITEB_MASK_VLV (0xff<<16) /* vlv/chv */ > > +#define DSPFW_CURSORA_SHIFT 8 > > +#define DSPFW_CURSORA_MASK (0x3f<<8) > > +#define DSPFW_PLANEC_SHIFT_OLD 0 > > +#define DSPFW_PLANEC_MASK_OLD (0x7f<<0) /* pre-gen4 s= prite C */ > > +#define DSPFW_SPRITEA_SHIFT 0 > > +#define DSPFW_SPRITEA_MASK (0x7f<<0) /* g4x */ > > +#define DSPFW_SPRITEA_MASK_VLV (0xff<<0) /* vlv/chv */ > > #define DSPFW3 (dev_priv->info.display_mmio_offset + 0= x7003c) > > -#define DSPFW_HPLL_SR_EN (1<<31) > > -#define DSPFW_CURSOR_SR_SHIFT 24 > > +#define DSPFW_HPLL_SR_EN (1<<31) > > #define PINEVIEW_SELF_REFRESH_EN (1<<30) > > +#define DSPFW_CURSOR_SR_SHIFT 24 > > #define DSPFW_CURSOR_SR_MASK (0x3f<<24) > > #define DSPFW_HPLL_CURSOR_SHIFT 16 > > #define DSPFW_HPLL_CURSOR_MASK (0x3f<<16) > > -#define DSPFW_HPLL_SR_MASK (0x1ff) > > -#define DSPFW4 (dev_priv->info.display_mmio_offset + 0= x70070) > > -#define DSPFW7 (dev_priv->info.display_mmio_offset + 0= x7007c) > > +#define DSPFW_HPLL_SR_SHIFT 0 > > +#define DSPFW_HPLL_SR_MASK (0x1ff<<0) > > + > > +/* vlv/chv */ > > +#define DSPFW4 (VLV_DISPLAY_BASE + 0x70070) > > +#define DSPFW_SPRITEB_WM1_SHIFT 16 > > +#define DSPFW_SPRITEB_WM1_MASK (0xff<<16) > > +#define DSPFW_CURSORA_WM1_SHIFT 8 > > +#define DSPFW_CURSORA_WM1_MASK (0x3f<<8) > > +#define DSPFW_SPRITEA_WM1_SHIFT 0 > > +#define DSPFW_SPRITEA_WM1_MASK (0xff<<0) > > +#define DSPFW5 (VLV_DISPLAY_BASE + 0x70074) > > +#define DSPFW_PLANEB_WM1_SHIFT 24 > > +#define DSPFW_PLANEB_WM1_MASK (0xff<<24) > > +#define DSPFW_PLANEA_WM1_SHIFT 16 > > +#define DSPFW_PLANEA_WM1_MASK (0xff<<16) > > +#define DSPFW_CURSORB_WM1_SHIFT 8 > > +#define DSPFW_CURSORB_WM1_MASK (0x3f<<8) > > +#define DSPFW_CURSOR_SR_WM1_SHIFT 0 > > +#define DSPFW_CURSOR_SR_WM1_MASK (0x3f<<0) > > +#define DSPFW6 (VLV_DISPLAY_BASE + 0x70078) > > +#define DSPFW_SR_WM1_SHIFT 0 > > +#define DSPFW_SR_WM1_MASK (0x1ff<<0) > > +#define DSPFW7 (VLV_DISPLAY_BASE + 0x7007c) > > +#define DSPFW7_CHV (VLV_DISPLAY_BASE + 0x700b4) /* wtf #1?= */ > > +#define DSPFW_SPRITED_WM1_SHIFT 24 > > +#define DSPFW_SPRITED_WM1_MASK (0xff<<24) > > +#define DSPFW_SPRITED_SHIFT 16 > > +#define DSPFW_SPRITED_MASK (0xff<<16) > > +#define DSPFW_SPRITEC_WM1_SHIFT 8 > > +#define DSPFW_SPRITEC_WM1_MASK (0xff<<8) > > +#define DSPFW_SPRITEC_SHIFT 0 > > +#define DSPFW_SPRITEC_MASK (0xff<<0) > > +#define DSPFW8_CHV (VLV_DISPLAY_BASE + 0x700b8) > > +#define DSPFW_SPRITEF_WM1_SHIFT 24 > > +#define DSPFW_SPRITEF_WM1_MASK (0xff<<24) > > +#define DSPFW_SPRITEF_SHIFT 16 > > +#define DSPFW_SPRITEF_MASK (0xff<<16) > > +#define DSPFW_SPRITEE_WM1_SHIFT 8 > > +#define DSPFW_SPRITEE_WM1_MASK (0xff<<8) > > +#define DSPFW_SPRITEE_SHIFT 0 > > +#define DSPFW_SPRITEE_MASK (0xff<<0) > > +#define DSPFW9_CHV (VLV_DISPLAY_BASE + 0x7007c) /* wtf #2?= */ > > +#define DSPFW_PLANEC_WM1_SHIFT 24 > > +#define DSPFW_PLANEC_WM1_MASK (0xff<<24) > > +#define DSPFW_PLANEC_SHIFT 16 > > +#define DSPFW_PLANEC_MASK (0xff<<16) > > +#define DSPFW_CURSORC_WM1_SHIFT 8 > > +#define DSPFW_CURSORC_WM1_MASK (0x3f<<16) > > +#define DSPFW_CURSORC_SHIFT 0 > > +#define DSPFW_CURSORC_MASK (0x3f<<0) > > + > > +/* vlv/chv high order bits */ > > +#define DSPHOWM (VLV_DISPLAY_BASE + 0x70064) > > +#define DSPFW_SR_HI_SHIFT 24 > > +#define DSPFW_SR_HI_MASK (1<<24) > > +#define DSPFW_SPRITEF_HI_SHIFT 23 > > +#define DSPFW_SPRITEF_HI_MASK (1<<23) > > +#define DSPFW_SPRITEE_HI_SHIFT 22 > > +#define DSPFW_SPRITEE_HI_MASK (1<<22) > > +#define DSPFW_PLANEC_HI_SHIFT 21 > > +#define DSPFW_PLANEC_HI_MASK (1<<21) > = > It looks like bits 23:21 were removed. They are just marked as reserved n= ow. I see them in the CHV doc. They're for pipe C planes, so CHV only. > = > = > > +#define DSPFW_SPRITED_HI_SHIFT 20 > > +#define DSPFW_SPRITED_HI_MASK (1<<20) > > +#define DSPFW_SPRITEC_HI_SHIFT 16 > > +#define DSPFW_SPRITEC_HI_MASK (1<<16) > > +#define DSPFW_PLANEB_HI_SHIFT 12 > > +#define DSPFW_PLANEB_HI_MASK (1<<12) > > +#define DSPFW_SPRITEB_HI_SHIFT 8 > > +#define DSPFW_SPRITEB_HI_MASK (1<<8) > > +#define DSPFW_SPRITEA_HI_SHIFT 4 > > +#define DSPFW_SPRITEA_HI_MASK (1<<4) > > +#define DSPFW_PLANEA_HI_SHIFT 0 > > +#define DSPFW_PLANEA_HI_MASK (1<<0) > > +#define DSPHOWM1 (VLV_DISPLAY_BASE + 0x70064) > = > Should be 0x70068. Nice catch. Will fix. > = > = > > +#define DSPFW_SR_WM1_HI_SHIFT 24 > > +#define DSPFW_SR_WM1_HI_MASK (1<<24) > > +#define DSPFW_SPRITEF_WM1_HI_SHIFT 23 > > +#define DSPFW_SPRITEF_WM1_HI_MASK (1<<23) > > +#define DSPFW_SPRITEE_WM1_HI_SHIFT 22 > > +#define DSPFW_SPRITEE_WM1_HI_MASK (1<<22) > > +#define DSPFW_PLANEC_WM1_HI_SHIFT 21 > > +#define DSPFW_PLANEC_WM1_HI_MASK (1<<21) > = > Same story about 23:21 here. > = > Everything else looks correct. With the details above > fixed/addressed/explained: Reviewed-by: Paulo Zanoni > . > = > > +#define DSPFW_SPRITED_WM1_HI_SHIFT 20 > > +#define DSPFW_SPRITED_WM1_HI_MASK (1<<20) > > +#define DSPFW_SPRITEC_WM1_HI_SHIFT 16 > > +#define DSPFW_SPRITEC_WM1_HI_MASK (1<<16) > > +#define DSPFW_PLANEB_WM1_HI_SHIFT 12 > > +#define DSPFW_PLANEB_WM1_HI_MASK (1<<12) > > +#define DSPFW_SPRITEB_WM1_HI_SHIFT 8 > > +#define DSPFW_SPRITEB_WM1_HI_MASK (1<<8) > > +#define DSPFW_SPRITEA_WM1_HI_SHIFT 4 > > +#define DSPFW_SPRITEA_WM1_HI_MASK (1<<4) > > +#define DSPFW_PLANEA_WM1_HI_SHIFT 0 > > +#define DSPFW_PLANEA_WM1_HI_MASK (1<<0) > > > > /* drain latency register values*/ > > #define DRAIN_LATENCY_PRECISION_32 32 > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/int= el_pm.c > > index 3aa7959..dc858b5 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -1360,7 +1360,7 @@ static void valleyview_update_wm(struct drm_crtc = *crtc) > > (plane_sr << DSPFW_SR_SHIFT) | > > (cursorb_wm << DSPFW_CURSORB_SHIFT) | > > (planeb_wm << DSPFW_PLANEB_SHIFT) | > > - planea_wm); > > + (planea_wm << DSPFW_PLANEA_SHIFT)); > > I915_WRITE(DSPFW2, > > (I915_READ(DSPFW2) & ~DSPFW_CURSORA_MASK) | > > (cursora_wm << DSPFW_CURSORA_SHIFT)); > > @@ -1412,7 +1412,7 @@ static void g4x_update_wm(struct drm_crtc *crtc) > > (plane_sr << DSPFW_SR_SHIFT) | > > (cursorb_wm << DSPFW_CURSORB_SHIFT) | > > (planeb_wm << DSPFW_PLANEB_SHIFT) | > > - planea_wm); > > + (planea_wm << DSPFW_PLANEA_SHIFT)); > > I915_WRITE(DSPFW2, > > (I915_READ(DSPFW2) & ~DSPFW_CURSORA_MASK) | > > (cursora_wm << DSPFW_CURSORA_SHIFT)); > > @@ -1484,8 +1484,11 @@ static void i965_update_wm(struct drm_crtc *unus= ed_crtc) > > > > /* 965 has limitations... */ > > I915_WRITE(DSPFW1, (srwm << DSPFW_SR_SHIFT) | > > - (8 << 16) | (8 << 8) | (8 << 0)); > > - I915_WRITE(DSPFW2, (8 << 8) | (8 << 0)); > > + (8 << DSPFW_CURSORB_SHIFT) | > > + (8 << DSPFW_PLANEB_SHIFT) | > > + (8 << DSPFW_PLANEA_SHIFT)); > > + I915_WRITE(DSPFW2, (8 << DSPFW_CURSORA_SHIFT) | > > + (8 << DSPFW_PLANEC_SHIFT_OLD)); > > /* update cursor SR watermark */ > > I915_WRITE(DSPFW3, (cursor_sr << DSPFW_CURSOR_SR_SHIFT)); > > } > > -- > > 1.8.5.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