[PATCH 10/19] drm/i915: Extract intel_cdclk_update_hw_state()

[Govindapillai, Vinod]

On Tue, 2025-02-18 at 23:19 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Hoist the cdclk stuff into a separate function from
> intel_modeset_readout_hw_state() so that the details
> are better hidden inside intel_cdclk.c.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c    | 29 ++++++++++++++++++-
>  drivers/gpu/drm/i915/display/intel_cdclk.h    |  2 +-
>  .../drm/i915/display/intel_modeset_setup.c    | 16 ++--------
>  3 files changed, 31 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 4b7058e65588..947833a96ab7 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -2788,7 +2788,7 @@ static int intel_planes_min_cdclk(const struct intel_crtc_state *crtc_state)
>  	return min_cdclk;
>  }
>  
> -int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
> +static int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>  {
>  	int min_cdclk;
>  
> @@ -3340,6 +3340,33 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
>  	return 0;
>  }
>  
> +void intel_cdclk_update_hw_state(struct intel_display *display)
> +{
> +	struct intel_cdclk_state *cdclk_state =
> +		to_intel_cdclk_state(display->cdclk.obj.state);
> +	struct intel_crtc *crtc;
> +
> +	cdclk_state->active_pipes = 0;

As active_pipes are already calculated in intel_modeset_readout_hw_state, wonder will it be useful
if we pass active_pipes as a parameter to this function and use it above? Same applies to bwstate-
>active_pipes as well in couple of patches later.

Anyway, 

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

> +
> +	for_each_intel_crtc(display->drm, crtc) {
> +		const struct intel_crtc_state *crtc_state =
> +			to_intel_crtc_state(crtc->base.state);
> +		enum pipe pipe = crtc->pipe;
> +		int min_cdclk = 0;
> +
> +		if (crtc_state->hw.active) {
> +			cdclk_state->active_pipes |= BIT(pipe);
> +
> +			min_cdclk = intel_crtc_compute_min_cdclk(crtc_state);
> +			if (drm_WARN_ON(display->drm, min_cdclk < 0))
> +				min_cdclk = 0;
> +		}
> +
> +		cdclk_state->min_cdclk[pipe] = min_cdclk;
> +		cdclk_state->min_voltage_level[pipe] = crtc_state->min_voltage_level;
> +	}
> +}
> +
>  void intel_cdclk_crtc_disable_noatomic(struct intel_crtc *crtc)
>  {
>  	struct intel_display *display = to_intel_display(crtc);
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h
> b/drivers/gpu/drm/i915/display/intel_cdclk.h
> index 689e12e2196b..a1cefd455d92 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> @@ -59,7 +59,6 @@ struct intel_cdclk_state {
>  	bool disable_pipes;
>  };
>  
> -int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state);
>  void intel_cdclk_init_hw(struct intel_display *display);
>  void intel_cdclk_uninit_hw(struct intel_display *display);
>  void intel_init_cdclk_hooks(struct intel_display *display);
> @@ -84,6 +83,7 @@ int intel_cdclk_atomic_check(struct intel_atomic_state *state,
>  int intel_cdclk_state_set_joined_mbus(struct intel_atomic_state *state, bool joined_mbus);
>  struct intel_cdclk_state *
>  intel_atomic_get_cdclk_state(struct intel_atomic_state *state);
> +void intel_cdclk_update_hw_state(struct intel_display *display);
>  void intel_cdclk_crtc_disable_noatomic(struct intel_crtc *crtc);
>  
>  #define to_intel_cdclk_state(global_state) \
> diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> index e9b0533526f6..1cfa03bd3224 100644
> --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> @@ -693,8 +693,6 @@ static void readout_plane_state(struct drm_i915_private *i915)
>  static void intel_modeset_readout_hw_state(struct drm_i915_private *i915)
>  {
>  	struct intel_display *display = &i915->display;
> -	struct intel_cdclk_state *cdclk_state =
> -		to_intel_cdclk_state(i915->display.cdclk.obj.state);
>  	struct intel_dbuf_state *dbuf_state =
>  		to_intel_dbuf_state(i915->display.dbuf.obj.state);
>  	struct intel_pmdemand_state *pmdemand_state =
> @@ -730,7 +728,6 @@ static void intel_modeset_readout_hw_state(struct drm_i915_private *i915)
>  			    str_enabled_disabled(crtc_state->hw.active));
>  	}
>  
> -	cdclk_state->active_pipes = active_pipes;
>  	dbuf_state->active_pipes = active_pipes;
>  
>  	readout_plane_state(i915);
> @@ -833,7 +830,6 @@ static void intel_modeset_readout_hw_state(struct drm_i915_private *i915)
>  		struct intel_crtc_state *crtc_state =
>  			to_intel_crtc_state(crtc->base.state);
>  		struct intel_plane *plane;
> -		int min_cdclk = 0;
>  
>  		if (crtc_state->hw.active) {
>  			/*
> @@ -882,22 +878,14 @@ static void intel_modeset_readout_hw_state(struct drm_i915_private *i915)
>  				    crtc_state->min_cdclk[plane->id]);
>  		}
>  
> -		if (crtc_state->hw.active) {
> -			min_cdclk = intel_crtc_compute_min_cdclk(crtc_state);
> -			if (drm_WARN_ON(&i915->drm, min_cdclk < 0))
> -				min_cdclk = 0;
> -		}
> -
> -		cdclk_state->min_cdclk[crtc->pipe] = min_cdclk;
> -		cdclk_state->min_voltage_level[crtc->pipe] =
> -			crtc_state->min_voltage_level;
> -
>  		intel_pmdemand_update_port_clock(display, pmdemand_state, pipe,
>  						 crtc_state->port_clock);
>  
>  		intel_bw_crtc_update(bw_state, crtc_state);
>  	}
>  
> +	intel_cdclk_update_hw_state(display);
> +
>  	intel_pmdemand_init_pmdemand_params(display, pmdemand_state);
>  }
>