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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.