From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Paulo Zanoni <przanoni@gmail.com>
Cc: intel-gfx@lists.freedesktop.org
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 [thread overview]
Message-ID: <20130802152354.GN5004@intel.com> (raw)
In-Reply-To: <CA+gsUGSE=rhta9oaskq5PFWT5d7kxA=6HSztCEarvt86w3nkhQ@mail.gmail.com>
On Fri, Aug 02, 2013 at 12:16:52PM -0300, Paulo Zanoni wrote:
> 2013/8/1 <ville.syrjala@linux.intel.com>:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > 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älä <ville.syrjala@linux.intel.com>
> > ---
> > 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/i915_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_SRLT_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 + SSKPD) >> (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/intel_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 = 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_device *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 = dev->dev_private;
> > - int latency = SNB_READ_WM0_LATENCY() * 100; /* In unit 0.1us */
> > + int latency = 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_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))
> > @@ -1922,14 +1919,14 @@ static void sandybridge_update_wm(struct drm_device *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_device *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_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);
> > @@ -1961,7 +1958,7 @@ static void sandybridge_update_wm(struct drm_device *dev)
> > static void ivybridge_update_wm(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > - int latency = SNB_READ_WM0_LATENCY() * 100; /* In unit 0.1us */
> > + int latency = 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_device *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_device *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_wm) ||
> > !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 <paulo.r.zanoni@intel.com>.
>
>
> > &sandybridge_display_srwm_info,
> > &sandybridge_cursor_srwm_info,
> > &ignore_fbc_wm, &ignore_plane_wm, &cursor_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 drm_device *dev, int pipe,
> > bool enabled, bool scaled)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > - int latency = SNB_READ_WM0_LATENCY() * 100; /* In unit 0.1us */
> > + int latency = 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 drm_device *dev, int pipe,
> > ret = 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 drm_device *dev, int pipe,
> > ret = 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 drm_device *dev, int pipe,
> > ret = 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älä
Intel OTC
next prev parent reply other threads:[~2013-08-02 15:23 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-01 13:18 [PATCH 0/17] drm/i915: ILK+ watermark prep patches ville.syrjala
2013-08-01 13:18 ` [PATCH v2 01/17] drm/i915: Add scaled paramater to update_sprite_watermarks() ville.syrjala
2013-08-05 16:23 ` Daniel Vetter
2013-08-01 13:18 ` [PATCH 02/17] drm/i915: Pass the actual sprite width to watermarks functions ville.syrjala
2013-08-01 13:18 ` [PATCH 03/17] drm/i915: Calculate the sprite WM based on the source width instead of the destination width ville.syrjala
2013-08-01 13:18 ` [PATCH 04/17] drm/i915: Rename hsw_wm_get_pixel_rate to ilk_pipe_pixel_rate ville.syrjala
2013-08-01 13:18 ` [PATCH 05/17] drm/i915: Rename most wm compute functions to ilk_ prefix ville.syrjala
2013-08-01 13:18 ` [PATCH 06/17] drm/i915: Don't pass "mem_value" to ilk_compute_fbc_wm ville.syrjala
2013-08-01 13:18 ` [PATCH 07/17] drm/i915: Change the watermark latency type to uint16_t ville.syrjala
2013-08-01 13:18 ` [PATCH 08/17] drm/i915: Split out reading of HSW watermark latency values ville.syrjala
2013-08-01 13:18 ` [PATCH 09/17] drm/i915: Don't multiply the watermark latency values too early ville.syrjala
2013-08-01 13:18 ` [PATCH 10/17] drm/i915: Add SNB/IVB support to intel_read_wm_latency ville.syrjala
2013-08-01 13:18 ` [PATCH 11/17] drm/i915: Add ILK " ville.syrjala
2013-08-02 14:16 ` Paulo Zanoni
2013-08-01 13:18 ` [PATCH v2 12/17] drm/i915: Store the watermark latency values in dev_priv ville.syrjala
2013-08-01 14:23 ` Chris Wilson
2013-08-05 16:25 ` Daniel Vetter
2013-08-05 16:41 ` Chris Wilson
2013-08-02 14:28 ` Paulo Zanoni
2013-08-01 13:18 ` [PATCH v2 13/17] drm/i915: Use the stored cursor and plane latencies properly ville.syrjala
2013-08-02 14:34 ` Paulo Zanoni
2013-08-01 13:18 ` [PATCH v2 14/17] drm/i915: Print the watermark latencies during init ville.syrjala
2013-08-02 14:41 ` Paulo Zanoni
2013-08-02 14:55 ` Ville Syrjälä
2013-08-01 13:18 ` [PATCH v2 15/17] drm/i915: Disable specific watermark levels when latency is zero ville.syrjala
2013-08-02 14:48 ` Paulo Zanoni
2013-08-05 16:31 ` Daniel Vetter
2013-08-01 13:18 ` [PATCH 16/17] drm/i915: Use the watermark latency values from dev_priv for ILK/SNB/IVB too ville.syrjala
2013-08-02 15:16 ` Paulo Zanoni
2013-08-02 15:23 ` Ville Syrjälä [this message]
2013-08-02 15:55 ` Paulo Zanoni
2013-08-01 13:18 ` [PATCH 17/17] drm/i915: Add comments about units of latency values ville.syrjala
2013-08-02 15:58 ` Paulo Zanoni
2013-08-02 16:09 ` Ville Syrjälä
2013-08-05 16:41 ` Daniel Vetter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130802152354.GN5004@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=przanoni@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox