On Fri, 2025-09-05 at 17:58 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> 
> The watermark algorithm sometimes produces results where higher
> watermark levels have smaller blocks/lines watermarks than the lower
> levels. That doesn't really make sense as the corresponding latencies
> are supposed to be non-decreasing. It's unclear how the hardware
> responds to such watermark values, so it seems better to avoid that
> case and just make sure the values are always non-decreasing.
> 
> Here's an example how things change for such a case on a GLK NUC:
>  [PLANE:70:cursor A]   level  wm0, wm1, wm2, wm3, wm4, wm5, wm6, wm7, twm, 
> swm, stwm -> *wm0,*wm1,*wm2,*wm3,*wm4,*wm5,*wm6,*wm7,*twm, swm, stwm
>  [PLANE:70:cursor A]   lines    0,   0,   0,   0,   0,   0,   0,   0,   0,   
> 0,    0 ->    4,   4,   4,   2,   2,   2,   2,   2,   0,   0,    0
>  [PLANE:70:cursor A]  blocks    0,   0,   0,   0,   0,   0,   0,   0,   0,   
> 0,    0 ->   11,  11,  12,   7,   7,   7,   7,   7,  25,   0,    0
>  [PLANE:70:cursor A] min_ddb    0,   0,   0,   0,   0,   0,   0,   0,   0,   
> 0,    0 ->   12,  12,  13,   8,   8,   8,   8,   8,  26,   0,    0
> ->
>  [PLANE:70:cursor A]   level  wm0, wm1, wm2, wm3, wm4, wm5, wm6, wm7, twm, 
> swm, stwm -> *wm0,*wm1,*wm2,*wm3,*wm4,*wm5,*wm6,*wm7,*twm, swm, stwm
>  [PLANE:70:cursor A]   lines    0,   0,   0,   0,   0,   0,   0,   0,   0,   
> 0,    0 ->    4,   4,   4,   4,   4,   4,   4,   4,   0,   0,    0
>  [PLANE:70:cursor A]  blocks    0,   0,   0,   0,   0,   0,   0,   0,   0,   
> 0,    0 ->   11,  11,  12,  12,  12,  12,  12,  12,  25,   0,    0
>  [PLANE:70:cursor A] min_ddb    0,   0,   0,   0,   0,   0,   0,   0,   0,   
> 0,    0 ->   12,  12,  13,  13,  13,  13,  13,  13,  26,   0,    0
> 
> Whether this actually helps on any display blinking issues is unclear.
> 
> References: https://gitlab.freedesktop.org/drm/intel/-/issues/8683
> Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> ---

Okay, maybe this answers my question to the other monotonic patch.

Reviewed-by: Luca Coelho <luciano.coe...@intel.com>

--
Cheers,
Luca.



>  drivers/gpu/drm/i915/display/skl_watermark.c | 21 +++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c 
> b/drivers/gpu/drm/i915/display/skl_watermark.c
> index a40113aa3f3e..6e268836f5c6 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -1878,18 +1878,21 @@ static void skl_compute_plane_wm(const struct 
> intel_crtc_state *crtc_state,
>                       } else {
>                               blocks++;
>                       }
> -
> -                     /*
> -                      * Make sure result blocks for higher latency levels are
> -                      * at least as high as level below the current level.
> -                      * Assumption in DDB algorithm optimization for special
> -                      * cases. Also covers Display WA #1125 for RC.
> -                      */
> -                     if (result_prev->blocks > blocks)
> -                             blocks = result_prev->blocks;
>               }
>       }
>  
> +     /*
> +      * Make sure result blocks for higher latency levels are
> +      * at least as high as level below the current level.
> +      * Assumption in DDB algorithm optimization for special
> +      * cases. Also covers Display WA #1125 for RC.
> +      *
> +      * Let's always do this as the algorithm can give non
> +      * monotonic results on any platform.
> +      */
> +     blocks = max_t(u32, blocks, result_prev->blocks);
> +     lines = max_t(u32, lines, result_prev->lines);
> +
>       if (DISPLAY_VER(display) >= 11) {
>               if (wp->y_tiled) {
>                       int extra_lines;

Reply via email to