Re: [PATCH 9/9] drm/i915/wm: Allow SAGV with multiple pipes on pre-icl

["Govindapillai, Vinod" <vinod.govindapillai@intel.com>]

On Tue, 2026-03-24 at 15:48 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> There was never any documented reason for limiting SAGV to
> single active pipe configuration on pre-icl. Allow SAGV
> with multiple active pipes.
> 
> At least my CFL NUC seems happy with this when using
> multiple displays. The machine actually has working
> SAGV because the memory clock can be observed changing
> via SA_PERF_STATUS/mchbar:0x5918.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bw.c | 40 -----------------------
> --
>  1 file changed, 40 deletions(-)
> 

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

> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c
> b/drivers/gpu/drm/i915/display/intel_bw.c
> index 07b4531a4376..bf7683ddcb67 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -28,9 +28,6 @@ struct intel_bw_state {
>  	 */
>  	u8 pipe_sagv_reject;
>  
> -	/* bitmask of active pipes */
> -	u8 active_pipes;
> -
>  	/*
>  	 * From MTL onwards, to lock a QGV point, punit expects the
> peak BW of
>  	 * the selected QGV point as the parameter in multiples of
> 100MB/s
> @@ -1265,31 +1262,6 @@ static int intel_bw_check_data_rate(struct
> intel_atomic_state *state, bool *chan
>  	return 0;
>  }
>  
> -static int intel_bw_modeset_checks(struct intel_atomic_state *state)
> -{
> -	const struct intel_bw_state *old_bw_state;
> -	struct intel_bw_state *new_bw_state;
> -	int ret;
> -
> -	if (!intel_any_crtc_active_changed(state))
> -		return 0;
> -
> -	new_bw_state = intel_atomic_get_bw_state(state);
> -	if (IS_ERR(new_bw_state))
> -		return PTR_ERR(new_bw_state);
> -
> -	old_bw_state = intel_atomic_get_old_bw_state(state);
> -
> -	new_bw_state->active_pipes =
> -		intel_calc_active_pipes(state, old_bw_state-
> >active_pipes);
> -
> -	ret = intel_atomic_lock_global_state(&new_bw_state->base);
> -	if (ret)
> -		return ret;
> -
> -	return 0;
> -}
> -
>  static int intel_bw_check_sagv_mask(struct intel_atomic_state
> *state)
>  {
>  	struct intel_display *display = to_intel_display(state);
> @@ -1346,10 +1318,6 @@ int intel_bw_atomic_check(struct
> intel_atomic_state *state)
>  	if (DISPLAY_VER(display) < 9)
>  		return 0;
>  
> -	ret = intel_bw_modeset_checks(state);
> -	if (ret)
> -		return ret;
> -
>  	ret = intel_bw_check_sagv_mask(state);
>  	if (ret)
>  		return ret;
> @@ -1410,7 +1378,6 @@ void intel_bw_update_hw_state(struct
> intel_display *display)
>  	if (DISPLAY_VER(display) < 9)
>  		return;
>  
> -	bw_state->active_pipes = 0;
>  	bw_state->pipe_sagv_reject = 0;
>  
>  	for_each_intel_crtc(display->drm, crtc) {
> @@ -1418,9 +1385,6 @@ void intel_bw_update_hw_state(struct
> intel_display *display)
>  			to_intel_crtc_state(crtc->base.state);
>  		enum pipe pipe = crtc->pipe;
>  
> -		if (crtc_state->hw.active)
> -			bw_state->active_pipes |= BIT(pipe);
> -
>  		if (DISPLAY_VER(display) >= 11)
>  			intel_bw_crtc_update(bw_state, crtc_state);
>  
> @@ -1504,10 +1468,6 @@ bool intel_bw_pmdemand_needs_update(struct
> intel_atomic_state *state)
>  bool intel_bw_can_enable_sagv(struct intel_display *display,
>  			      const struct intel_bw_state *bw_state)
>  {
> -	if (DISPLAY_VER(display) < 11 &&
> -	    bw_state->active_pipes && !is_power_of_2(bw_state-
> >active_pipes))
> -		return false;
> -
>  	return bw_state->pipe_sagv_reject == 0;
>  }
>