Re: [PATCH 18/19] drm/i915: Move dbuf_state->active_piepes into skl_wm_get_hw_state()

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

On Tue, 2025-02-18 at 23:19 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Move the dbuf_state readout parts into skl_wm_get_hw_state()
> so that the details are better hidden from sight.
> 
> This will stop updating this on pre-skl, but that's what we want
> since the dbuf state is only used on skl+.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_modeset_setup.c | 8 --------
>  drivers/gpu/drm/i915/display/skl_watermark.c       | 5 ++++-
>  2 files changed, 4 insertions(+), 9 deletions(-)
> 
Again my previous suggestion of active_pipes in an earlier patch wont be relevant anymore after this
patch

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

> diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> index 6a4142d7d025..312b21b1ab59 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_dbuf_state *dbuf_state =
> -		to_intel_dbuf_state(i915->display.dbuf.obj.state);
>  	struct intel_pmdemand_state *pmdemand_state =
>  		to_intel_pmdemand_state(i915->display.pmdemand.obj.state);
>  	enum pipe pipe;
> @@ -702,7 +700,6 @@ static void intel_modeset_readout_hw_state(struct drm_i915_private *i915)
>  	struct intel_encoder *encoder;
>  	struct intel_connector *connector;
>  	struct drm_connector_list_iter conn_iter;
> -	u8 active_pipes = 0;
>  
>  	for_each_intel_crtc(&i915->drm, crtc) {
>  		struct intel_crtc_state *crtc_state =
> @@ -719,17 +716,12 @@ static void intel_modeset_readout_hw_state(struct drm_i915_private *i915)
>  		crtc->base.enabled = crtc_state->hw.enable;
>  		crtc->active = crtc_state->hw.active;
>  
> -		if (crtc_state->hw.active)
> -			active_pipes |= BIT(crtc->pipe);
> -
>  		drm_dbg_kms(&i915->drm,
>  			    "[CRTC:%d:%s] hw state readout: %s\n",
>  			    crtc->base.base.id, crtc->base.name,
>  			    str_enabled_disabled(crtc_state->hw.active));
>  	}
>  
> -	dbuf_state->active_pipes = active_pipes;
> -
>  	readout_plane_state(i915);
>  
>  	for_each_intel_encoder(&i915->drm, encoder) {
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c
> b/drivers/gpu/drm/i915/display/skl_watermark.c
> index 0bd7aa3b2877..2d0de1c63308 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -3157,6 +3157,7 @@ static void skl_wm_get_hw_state(struct drm_i915_private *i915)
>  		dbuf_state->joined_mbus = intel_de_read(display, MBUS_CTL) & MBUS_JOIN;
>  
>  	dbuf_state->mdclk_cdclk_ratio = intel_mdclk_cdclk_ratio(display, &display->cdclk.hw);
> +	dbuf_state->active_pipes = 0;
>  
>  	for_each_intel_crtc(display->drm, crtc) {
>  		struct intel_crtc_state *crtc_state =
> @@ -3168,8 +3169,10 @@ static void skl_wm_get_hw_state(struct drm_i915_private *i915)
>  
>  		memset(&crtc_state->wm.skl.optimal, 0,
>  		       sizeof(crtc_state->wm.skl.optimal));
> -		if (crtc_state->hw.active)
> +		if (crtc_state->hw.active) {
>  			skl_pipe_wm_get_hw_state(crtc, &crtc_state->wm.skl.optimal);
> +			dbuf_state->active_pipes |= BIT(pipe);
> +		}
>  		crtc_state->wm.skl.raw = crtc_state->wm.skl.optimal;
>  
>  		memset(&dbuf_state->ddb[pipe], 0, sizeof(dbuf_state->ddb[pipe]));