From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 16/17] drm/i915: Use the watermark latency values from dev_priv for ILK/SNB/IVB too Date: Fri, 2 Aug 2013 18:23:54 +0300 Message-ID: <20130802152354.GN5004@intel.com> References: <1375363135-20618-1-git-send-email-ville.syrjala@linux.intel.com> <1375363135-20618-17-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 mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 1D1C4E667C for ; Fri, 2 Aug 2013 08:23:58 -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 Fri, Aug 02, 2013 at 12:16:52PM -0300, Paulo Zanoni wrote: > 2013/8/1 : > > From: Ville Syrj=E4l=E4 > > > > Adjust the current ILK/SNB/IVB watermark codepaths to use the > > pre-populated latency values from dev_priv instead of reading > > them out from the registers every time. > > > > Signed-off-by: Ville Syrj=E4l=E4 > > --- > > drivers/gpu/drm/i915/i915_reg.h | 9 ------- > > drivers/gpu/drm/i915/intel_pm.c | 57 +++++++++++++++++++--------------= -------- > > 2 files changed, 27 insertions(+), 39 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i91= 5_reg.h > > index 19941c6..17f6252 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -3202,9 +3202,6 @@ > > #define MLTR_WM2_SHIFT 8 > > /* the unit of memory self-refresh latency time is 0.5us */ > > #define ILK_SRLT_MASK 0x3f > > -#define ILK_LATENCY(shift) (I915_READ(MLTR_ILK) >> (shift) & ILK_S= RLT_MASK) > > -#define ILK_READ_WM1_LATENCY() ILK_LATENCY(MLTR_WM1_SHIFT) > > -#define ILK_READ_WM2_LATENCY() ILK_LATENCY(MLTR_WM2_SHIFT) > > > > /* define the fifo size on Ironlake */ > > #define ILK_DISPLAY_FIFO 128 > > @@ -3251,12 +3248,6 @@ > > #define SSKPD_WM2_SHIFT 16 > > #define SSKPD_WM3_SHIFT 24 > > > > -#define SNB_LATENCY(shift) (I915_READ(MCHBAR_MIRROR_BASE_SNB + SSK= PD) >> (shift) & SSKPD_WM_MASK) > > -#define SNB_READ_WM0_LATENCY() SNB_LATENCY(SSKPD_WM0_SHIFT) > > -#define SNB_READ_WM1_LATENCY() SNB_LATENCY(SSKPD_WM1_SHIFT) > > -#define SNB_READ_WM2_LATENCY() SNB_LATENCY(SSKPD_WM2_SHIFT) > > -#define SNB_READ_WM3_LATENCY() SNB_LATENCY(SSKPD_WM3_SHIFT) > > - > > /* > > * The two pipe frame counter registers are not synchronized, so > > * reading a stable value is somewhat tricky. The following code > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/int= el_pm.c > > index 149eb0a..5fe8c4e 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -1665,9 +1665,6 @@ static void i830_update_wm(struct drm_device *dev) > > I915_WRITE(FW_BLC, fwater_lo); > > } > > > > -#define ILK_LP0_PLANE_LATENCY 700 > > -#define ILK_LP0_CURSOR_LATENCY 1300 > > - > > /* > > * Check the wm result. > > * > > @@ -1782,9 +1779,9 @@ static void ironlake_update_wm(struct drm_device = *dev) > > enabled =3D 0; > > if (g4x_compute_wm0(dev, PIPE_A, > > &ironlake_display_wm_info, > > - ILK_LP0_PLANE_LATENCY, > > + dev_priv->wm.pri_latency[0] * 100, > > &ironlake_cursor_wm_info, > > - ILK_LP0_CURSOR_LATENCY, > > + dev_priv->wm.cur_latency[0] * 100, > > &plane_wm, &cursor_wm)) { > > I915_WRITE(WM0_PIPEA_ILK, > > (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_= wm); > > @@ -1796,9 +1793,9 @@ static void ironlake_update_wm(struct drm_device = *dev) > > > > if (g4x_compute_wm0(dev, PIPE_B, > > &ironlake_display_wm_info, > > - ILK_LP0_PLANE_LATENCY, > > + dev_priv->wm.pri_latency[0] * 100, > > &ironlake_cursor_wm_info, > > - ILK_LP0_CURSOR_LATENCY, > > + dev_priv->wm.cur_latency[0] * 100, > > &plane_wm, &cursor_wm)) { > > I915_WRITE(WM0_PIPEB_ILK, > > (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_= wm); > > @@ -1822,7 +1819,7 @@ static void ironlake_update_wm(struct drm_device = *dev) > > > > /* WM1 */ > > if (!ironlake_compute_srwm(dev, 1, enabled, > > - ILK_READ_WM1_LATENCY() * 500, > > + dev_priv->wm.pri_latency[1] * 500, > > &ironlake_display_srwm_info, > > &ironlake_cursor_srwm_info, > > &fbc_wm, &plane_wm, &cursor_wm)) > > @@ -1830,14 +1827,14 @@ static void ironlake_update_wm(struct drm_devic= e *dev) > > > > I915_WRITE(WM1_LP_ILK, > > WM1_LP_SR_EN | > > - (ILK_READ_WM1_LATENCY() << WM1_LP_LATENCY_SHIFT) | > > + (dev_priv->wm.pri_latency[1] << WM1_LP_LATENCY_SHIFT= ) | > > (fbc_wm << WM1_LP_FBC_SHIFT) | > > (plane_wm << WM1_LP_SR_SHIFT) | > > cursor_wm); > > > > /* WM2 */ > > if (!ironlake_compute_srwm(dev, 2, enabled, > > - ILK_READ_WM2_LATENCY() * 500, > > + dev_priv->wm.pri_latency[2] * 500, > > &ironlake_display_srwm_info, > > &ironlake_cursor_srwm_info, > > &fbc_wm, &plane_wm, &cursor_wm)) > > @@ -1845,7 +1842,7 @@ static void ironlake_update_wm(struct drm_device = *dev) > > > > I915_WRITE(WM2_LP_ILK, > > WM2_LP_EN | > > - (ILK_READ_WM2_LATENCY() << WM1_LP_LATENCY_SHIFT) | > > + (dev_priv->wm.pri_latency[2] << WM1_LP_LATENCY_SHIFT= ) | > > (fbc_wm << WM1_LP_FBC_SHIFT) | > > (plane_wm << WM1_LP_SR_SHIFT) | > > cursor_wm); > > @@ -1859,7 +1856,7 @@ static void ironlake_update_wm(struct drm_device = *dev) > > static void sandybridge_update_wm(struct drm_device *dev) > > { > > struct drm_i915_private *dev_priv =3D dev->dev_private; > > - int latency =3D SNB_READ_WM0_LATENCY() * 100; /* In unit 0.= 1us */ > > + int latency =3D dev_priv->wm.pri_latency[0] * 100; /* In= unit 0.1us */ > > u32 val; > > int fbc_wm, plane_wm, cursor_wm; > > unsigned int enabled; > > @@ -1914,7 +1911,7 @@ static void sandybridge_update_wm(struct drm_devi= ce *dev) > > > > /* WM1 */ > > if (!ironlake_compute_srwm(dev, 1, enabled, > > - SNB_READ_WM1_LATENCY() * 500, > > + dev_priv->wm.pri_latency[1] * 500, > > &sandybridge_display_srwm_info, > > &sandybridge_cursor_srwm_info, > > &fbc_wm, &plane_wm, &cursor_wm)) > > @@ -1922,14 +1919,14 @@ static void sandybridge_update_wm(struct drm_de= vice *dev) > > > > I915_WRITE(WM1_LP_ILK, > > WM1_LP_SR_EN | > > - (SNB_READ_WM1_LATENCY() << WM1_LP_LATENCY_SHIFT) | > > + (dev_priv->wm.pri_latency[1] << WM1_LP_LATENCY_SHIFT= ) | > > (fbc_wm << WM1_LP_FBC_SHIFT) | > > (plane_wm << WM1_LP_SR_SHIFT) | > > cursor_wm); > > > > /* WM2 */ > > if (!ironlake_compute_srwm(dev, 2, enabled, > > - SNB_READ_WM2_LATENCY() * 500, > > + dev_priv->wm.pri_latency[2] * 500, > > &sandybridge_display_srwm_info, > > &sandybridge_cursor_srwm_info, > > &fbc_wm, &plane_wm, &cursor_wm)) > > @@ -1937,14 +1934,14 @@ static void sandybridge_update_wm(struct drm_de= vice *dev) > > > > I915_WRITE(WM2_LP_ILK, > > WM2_LP_EN | > > - (SNB_READ_WM2_LATENCY() << WM1_LP_LATENCY_SHIFT) | > > + (dev_priv->wm.pri_latency[2] << WM1_LP_LATENCY_SHIFT= ) | > > (fbc_wm << WM1_LP_FBC_SHIFT) | > > (plane_wm << WM1_LP_SR_SHIFT) | > > cursor_wm); > > > > /* WM3 */ > > if (!ironlake_compute_srwm(dev, 3, enabled, > > - SNB_READ_WM3_LATENCY() * 500, > > + dev_priv->wm.pri_latency[3] * 500, > > &sandybridge_display_srwm_info, > > &sandybridge_cursor_srwm_info, > > &fbc_wm, &plane_wm, &cursor_wm)) > > @@ -1952,7 +1949,7 @@ static void sandybridge_update_wm(struct drm_devi= ce *dev) > > > > I915_WRITE(WM3_LP_ILK, > > WM3_LP_EN | > > - (SNB_READ_WM3_LATENCY() << WM1_LP_LATENCY_SHIFT) | > > + (dev_priv->wm.pri_latency[3] << WM1_LP_LATENCY_SHIFT= ) | > > (fbc_wm << WM1_LP_FBC_SHIFT) | > > (plane_wm << WM1_LP_SR_SHIFT) | > > cursor_wm); > > @@ -1961,7 +1958,7 @@ static void sandybridge_update_wm(struct drm_devi= ce *dev) > > static void ivybridge_update_wm(struct drm_device *dev) > > { > > struct drm_i915_private *dev_priv =3D dev->dev_private; > > - int latency =3D SNB_READ_WM0_LATENCY() * 100; /* In unit 0.= 1us */ > > + int latency =3D dev_priv->wm.pri_latency[0] * 100; /* In= unit 0.1us */ > > u32 val; > > int fbc_wm, plane_wm, cursor_wm; > > int ignore_fbc_wm, ignore_plane_wm, ignore_cursor_wm; > > @@ -2031,7 +2028,7 @@ static void ivybridge_update_wm(struct drm_device= *dev) > > > > /* WM1 */ > > if (!ironlake_compute_srwm(dev, 1, enabled, > > - SNB_READ_WM1_LATENCY() * 500, > > + dev_priv->wm.pri_latency[1] * 500, > > &sandybridge_display_srwm_info, > > &sandybridge_cursor_srwm_info, > > &fbc_wm, &plane_wm, &cursor_wm)) > > @@ -2039,14 +2036,14 @@ static void ivybridge_update_wm(struct drm_devi= ce *dev) > > > > I915_WRITE(WM1_LP_ILK, > > WM1_LP_SR_EN | > > - (SNB_READ_WM1_LATENCY() << WM1_LP_LATENCY_SHIFT) | > > + (dev_priv->wm.pri_latency[1] << WM1_LP_LATENCY_SHIFT= ) | > > (fbc_wm << WM1_LP_FBC_SHIFT) | > > (plane_wm << WM1_LP_SR_SHIFT) | > > cursor_wm); > > > > /* WM2 */ > > if (!ironlake_compute_srwm(dev, 2, enabled, > > - SNB_READ_WM2_LATENCY() * 500, > > + dev_priv->wm.pri_latency[2] * 500, > > &sandybridge_display_srwm_info, > > &sandybridge_cursor_srwm_info, > > &fbc_wm, &plane_wm, &cursor_wm)) > > @@ -2054,19 +2051,19 @@ static void ivybridge_update_wm(struct drm_devi= ce *dev) > > > > I915_WRITE(WM2_LP_ILK, > > WM2_LP_EN | > > - (SNB_READ_WM2_LATENCY() << WM1_LP_LATENCY_SHIFT) | > > + (dev_priv->wm.pri_latency[2] << WM1_LP_LATENCY_SHIFT= ) | > > (fbc_wm << WM1_LP_FBC_SHIFT) | > > (plane_wm << WM1_LP_SR_SHIFT) | > > cursor_wm); > > > > /* WM3, note we have to correct the cursor latency */ > > if (!ironlake_compute_srwm(dev, 3, enabled, > > - SNB_READ_WM3_LATENCY() * 500, > > + dev_priv->wm.pri_latency[3] * 500, > > &sandybridge_display_srwm_info, > > &sandybridge_cursor_srwm_info, > > &fbc_wm, &plane_wm, &ignore_cursor_w= m) || > > !ironlake_compute_srwm(dev, 3, enabled, > > - 2 * SNB_READ_WM3_LATENCY() * 500, > > + dev_priv->wm.cur_latency[3] * 500, > = > You've killed WaDoubleCursorLP3Latency here. You should probably > revive it and add the WA name in a comment. cur_latency[3] already contains the 2x adjustment. You just complained about it in the other patch :) Or do you mean I should add another comment here w/ the w/a name? > Everything else looks correct. With that fixed: Reviewed-by: Paulo > Zanoni . > = > = > > &sandybridge_display_srwm_info, > > &sandybridge_cursor_srwm_info, > > &ignore_fbc_wm, &ignore_plane_wm, &c= ursor_wm)) > > @@ -2074,7 +2071,7 @@ static void ivybridge_update_wm(struct drm_device= *dev) > > > > I915_WRITE(WM3_LP_ILK, > > WM3_LP_EN | > > - (SNB_READ_WM3_LATENCY() << WM1_LP_LATENCY_SHIFT) | > > + (dev_priv->wm.pri_latency[3] << WM1_LP_LATENCY_SHIFT= ) | > > (fbc_wm << WM1_LP_FBC_SHIFT) | > > (plane_wm << WM1_LP_SR_SHIFT) | > > cursor_wm); > > @@ -2818,7 +2815,7 @@ static void sandybridge_update_sprite_wm(struct d= rm_device *dev, int pipe, > > bool enabled, bool scaled) > > { > > struct drm_i915_private *dev_priv =3D dev->dev_private; > > - int latency =3D SNB_READ_WM0_LATENCY() * 100; /* In unit 0.= 1us */ > > + int latency =3D dev_priv->wm.spr_latency[0] * 100; /* In= unit 0.1us */ > > u32 val; > > int sprite_wm, reg; > > int ret; > > @@ -2858,7 +2855,7 @@ static void sandybridge_update_sprite_wm(struct d= rm_device *dev, int pipe, > > ret =3D sandybridge_compute_sprite_srwm(dev, pipe, sprite_width, > > pixel_size, > > &sandybridge_display_srwm= _info, > > - SNB_READ_WM1_LATENCY() * = 500, > > + dev_priv->wm.spr_latency[= 1] * 500, > > &sprite_wm); > > if (!ret) { > > DRM_DEBUG_KMS("failed to compute sprite lp1 wm on pipe = %c\n", > > @@ -2874,7 +2871,7 @@ static void sandybridge_update_sprite_wm(struct d= rm_device *dev, int pipe, > > ret =3D sandybridge_compute_sprite_srwm(dev, pipe, sprite_width, > > pixel_size, > > &sandybridge_display_srwm= _info, > > - SNB_READ_WM2_LATENCY() * = 500, > > + dev_priv->wm.spr_latency[= 2] * 500, > > &sprite_wm); > > if (!ret) { > > DRM_DEBUG_KMS("failed to compute sprite lp2 wm on pipe = %c\n", > > @@ -2886,7 +2883,7 @@ static void sandybridge_update_sprite_wm(struct d= rm_device *dev, int pipe, > > ret =3D sandybridge_compute_sprite_srwm(dev, pipe, sprite_width, > > pixel_size, > > &sandybridge_display_srwm= _info, > > - SNB_READ_WM3_LATENCY() * = 500, > > + dev_priv->wm.spr_latency[= 3] * 500, > > &sprite_wm); > > if (!ret) { > > DRM_DEBUG_KMS("failed to compute sprite lp3 wm on pipe = %c\n", > > -- > > 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