Re: [PATCH 2/9] drm/i915/wm: Don't compute separate SAGV watermarks for RKL

Govindapillai, Vinod
2026-04-08T11:48:21Z

On Tue, 2026-03-24 at 15:48 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> RKL is supposed to use the old SKL/ICL method for determining
> whether the watermarks tolerate SAGV or not, not the TGL+ method.
> Make it so.
>
> BSpec: 49325
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  .../gpu/drm/i915/display/intel_display_device.h   |  1 +
>  drivers/gpu/drm/i915/display/skl_watermark.c      | 15 ++++++++-----
> --
>  2 files changed, 9 insertions(+), 7 deletions(-)
>

Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com>

> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h
> b/drivers/gpu/drm/i915/display/intel_display_device.h
> index 1170ac346615..074e3ba8fb77 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> @@ -200,6 +200,7 @@ struct intel_display_platforms {
>  #define
> HAS_PSR_TRANS_PUSH_FRAME_CHANGE(__display) (DISPLAY_VER(__display) >=20)
>  #define HAS_SAGV(__display) (DISPLAY_VER(__display) >= 9
> && \
>   !(__display)-
> >platform.broxton && !(__display)->platform.geminilake)
> +#define HAS_SAGV_WM(__display) (DISPLAY_VER(__display) >=
> 12 && !(__display)->platform.rocketlake)
>  #define HAS_TRANSCODER(__display,
> trans) ((DISPLAY_RUNTIME_INFO(__display)->cpu_transcoder_mask & \
>     BIT(trans)) != 0)
>  #define
> HAS_UNCOMPRESSED_JOINER(__display) (DISPLAY_VER(__display) >= 13)
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c
> b/drivers/gpu/drm/i915/display/skl_watermark.c
> index 09988f46e083..bcdca1b99fe4 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -388,7 +388,7 @@ bool intel_crtc_can_enable_sagv(const struct
> intel_crtc_state *crtc_state)
>   if (crtc_state->inherited)
>   return false;
>
> - if (DISPLAY_VER(display) >= 12)
> + if (HAS_SAGV_WM(display))
>   return tgl_crtc_can_enable_sagv(crtc_state);
>   else
>   return skl_crtc_can_enable_sagv(crtc_state);
> @@ -1939,7 +1939,7 @@ static void skl_compute_plane_wm(const struct
> intel_crtc_state *crtc_state,
>   result->enable = true;
>   result->auto_min_alloc_wm_enable =
> xe3_auto_min_alloc_capable(plane, level);
>
> - if (DISPLAY_VER(display) < 12 && display-
> >sagv.block_time_us)
> + if (!HAS_SAGV_WM(display) && display->sagv.block_time_us)
>   result->can_sagv = latency >= display-
> >sagv.block_time_us;
>  }
>
> @@ -2065,7 +2065,7 @@ static int skl_build_plane_wm_single(struct
> intel_crtc_state *crtc_state,
>   skl_compute_transition_wm(display, &wm->trans_wm,
>     &wm->wm[0], &wm_params);
>
> - if (DISPLAY_VER(display) >= 12) {
> + if (HAS_SAGV_WM(display)) {
>   tgl_compute_sagv_wm(crtc_state, plane, &wm_params,
> wm);
>
>   skl_compute_transition_wm(display, &wm-
> >sagv.trans_wm,
> @@ -2324,7 +2324,7 @@ static int skl_wm_check_vblank(struct
> intel_crtc_state *crtc_state)
>   }
>   }
>
> - if (DISPLAY_VER(display) >= 12 &&
> + if (HAS_SAGV_WM(display) &&
>       display->sagv.block_time_us &&
>       skl_prefill_vblank_too_short(&ctx, crtc_state,
>   display-
> >sagv.block_time_us)) {
> @@ -2997,8 +2997,9 @@ skl_compute_wm(struct intel_atomic_state
> *state)
>   * other crtcs can't be allowed to use the more
> optimal
>   * normal (ie. non-SAGV) watermarks.
>   */
> - pipe_wm->use_sagv_wm = !HAS_HW_SAGV_WM(display) &&
> - DISPLAY_VER(display) >= 12 &&
> + pipe_wm->use_sagv_wm =
> + HAS_SAGV_WM(display) &&
> + !HAS_HW_SAGV_WM(display) &&
>   intel_crtc_can_enable_sagv(new_crtc_state);
>
>   ret = skl_wm_add_affected_planes(state, crtc);
> @@ -3064,7 +3065,7 @@ static void skl_pipe_wm_get_hw_state(struct
> intel_crtc *crtc,
>   val = intel_de_read(display,
> CUR_WM_SAGV_TRANS(pipe));
>
>   skl_wm_level_from_reg_val(display, val, &wm-
> >sagv.trans_wm);
> - } else if (DISPLAY_VER(display) >= 12) {
> + } else if (HAS_SAGV_WM(display)) {
>   wm->sagv.wm0 = wm->wm[0];
>   wm->sagv.trans_wm = wm->trans_wm;
>   }