2013/8/2 Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>:
> On Fri, Aug 02, 2013 at 12:16:52PM -0300, Paulo Zanoni wrote:
>> 2013/8/1 <ville.syrjala@xxxxxxxxxxxxxxx>:
>> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>> >
>> > 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@xxxxxxxxxxxxxxx>
>> > ---
>> > 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 :)
Oops, sorry....
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>
> Or do you mean I should add another comment here w/ the w/a name?
No need as the WA is "implemented" in the place that has the "* 2"
multiplication.
>
>> Everything else looks correct. With that fixed: Reviewed-by: Paulo
>> Zanoni <paulo.r.zanoni@xxxxxxxxx>.
>>
>>
>> > &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@xxxxxxxxxxxxxxxxxxxxx
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>>
>>
>> --
>> Paulo Zanoni
>
> --
> Ville Syrjälä
> Intel OTC
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
