The confusing thing is that plane_blocks_per_line is listed as part of
the method 2 calculation but is also used for other things. We
calculated it in two different places and different ways: one inside
skl_wm_method2() and the other inside skl_compute_plane_wm(). The
skl_wm_method2() implementation is the one that matches the
specification.

With this patch we fix the skl_compute_plane_wm() calculation and just
pass it as a parameter to skl_wm_method2(). We also take care to not
modify the value of plane_bytes_per_line since we're going to rely on
it having a correct value in later patches.

This should affect the watermarks for Linear and Y-tiled.

From my analysis, it looks like the two plane_blocks_per_line
variables got out of sync on 0fda65680e92, but we can't really say
that commit was a regression, it looks like just an incomplete fix.
There's always the possibility that 0fda65680e92 matched our
specification at that time, and then later the specification changed.

v2: Try to add a "Fixes" tag (Maarten).

Fixes: 0fda65680e92 ("drm/i915/skl: Update watermarks for Y tiling")
Cc: sta...@vger.kernel.org
Cc: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
Reviewed-by: Lyude <cp...@redhat.com>
Reviewed-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zan...@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 39 +++++++++++++++------------------------
 1 file changed, 15 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a7f5f7f..a6ae7b7 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3494,30 +3494,14 @@ static uint32_t skl_wm_method1(uint32_t pixel_rate, 
uint8_t cpp, uint32_t latenc
 }
 
 static uint32_t skl_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal,
-                              uint32_t horiz_pixels, uint8_t cpp,
-                              uint64_t tiling, uint32_t latency,
-                              uint32_t y_min_scanlines)
+                              uint32_t latency, uint32_t plane_blocks_per_line)
 {
        uint32_t ret;
-       uint32_t plane_bytes_per_line, plane_blocks_per_line;
        uint32_t wm_intermediate_val;
 
        if (latency == 0)
                return UINT_MAX;
 
-       plane_bytes_per_line = horiz_pixels * cpp;
-
-       if (tiling == I915_FORMAT_MOD_Y_TILED ||
-           tiling == I915_FORMAT_MOD_Yf_TILED) {
-               plane_bytes_per_line *= y_min_scanlines;
-               plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
-               plane_blocks_per_line /= y_min_scanlines;
-       } else if (tiling == DRM_FORMAT_MOD_NONE) {
-               plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512) 
+ 1;
-       } else {
-               plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
-       }
-
        wm_intermediate_val = latency * pixel_rate;
        ret = DIV_ROUND_UP(wm_intermediate_val, pipe_htotal * 1000) *
                                plane_blocks_per_line;
@@ -3606,17 +3590,24 @@ static int skl_compute_plane_wm(const struct 
drm_i915_private *dev_priv,
                y_min_scanlines = 4;
        }
 
+       plane_bytes_per_line = width * cpp;
+       if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
+           fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {
+               plane_blocks_per_line =
+                     DIV_ROUND_UP(plane_bytes_per_line * y_min_scanlines, 512);
+               plane_blocks_per_line /= y_min_scanlines;
+       } else if (fb->modifier[0] == DRM_FORMAT_MOD_NONE) {
+               plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512)
+                                       + 1;
+       } else {
+               plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
+       }
+
        method1 = skl_wm_method1(plane_pixel_rate, cpp, latency);
        method2 = skl_wm_method2(plane_pixel_rate,
                                 cstate->base.adjusted_mode.crtc_htotal,
-                                width,
-                                cpp,
-                                fb->modifier[0],
                                 latency,
-                                y_min_scanlines);
-
-       plane_bytes_per_line = width * cpp;
-       plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
+                                plane_blocks_per_line);
 
        if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
            fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to