Re: [PATCH 3/9] drm/i915/scaler: Pass the whole atomic state into intel_atomic_setup_scalers()

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 29 Oct 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> intel_atomic_setup_scalers() currently digs out the full atomic
> state from the crtc state. Flip that on its head so that we instead
> pass in the full atomic state and dig out the crtc state (and whatever
> else we need). This is generallte the better approach as it works
> in all phases of the atomic commit, whereas the other apporoach only
> really works during .atomic_check().
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_display.c |  2 +-
>  drivers/gpu/drm/i915/display/skl_scaler.c    | 19 +++++++++----------
>  drivers/gpu/drm/i915/display/skl_scaler.h    |  7 +++----
>  3 files changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 0e6d6c8354ef..dce3a20fa69f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -4581,7 +4581,7 @@ static int intel_crtc_atomic_check(struct intel_atomic_state *state,
>  				return ret;
>  		}
>  
> -		ret = intel_atomic_setup_scalers(dev_priv, crtc, crtc_state);
> +		ret = intel_atomic_setup_scalers(state, crtc);
>  		if (ret)
>  			return ret;
>  	}
> diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
> index b6a4effee5de..29fa4a14400b 100644
> --- a/drivers/gpu/drm/i915/display/skl_scaler.c
> +++ b/drivers/gpu/drm/i915/display/skl_scaler.c
> @@ -426,9 +426,8 @@ static int intel_atomic_setup_scaler(struct intel_crtc_scaler_state *scaler_stat
>  
>  /**
>   * intel_atomic_setup_scalers() - setup scalers for crtc per staged requests
> - * @dev_priv: i915 device
> - * @crtc: intel crtc
> - * @crtc_state: incoming crtc_state to validate and setup scalers
> + * @intel_state: atomic state
> + * @crtc: crtc
>   *
>   * This function sets up scalers based on staged scaling requests for
>   * a @crtc and its planes. It is called from crtc level check path. If request
> @@ -441,16 +440,16 @@ static int intel_atomic_setup_scaler(struct intel_crtc_scaler_state *scaler_stat
>   *         0 - scalers were setup successfully
>   *         error code - otherwise
>   */
> -int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv,
> -			       struct intel_crtc *crtc,
> -			       struct intel_crtc_state *crtc_state)
> +int intel_atomic_setup_scalers(struct intel_atomic_state *intel_state,
> +			       struct intel_crtc *crtc)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	struct intel_crtc_state *crtc_state =
> +		intel_atomic_get_new_crtc_state(intel_state, crtc);
>  	struct drm_plane *plane = NULL;
>  	struct intel_plane *intel_plane;
>  	struct intel_crtc_scaler_state *scaler_state =
>  		&crtc_state->scaler_state;
> -	struct drm_atomic_state *drm_state = crtc_state->uapi.state;
> -	struct intel_atomic_state *intel_state = to_intel_atomic_state(drm_state);
>  	int num_scalers_need;
>  	int i;
>  
> @@ -498,7 +497,7 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv,
>  
>  			/* plane scaler case: assign as a plane scaler */
>  			/* find the plane that set the bit as scaler_user */
> -			plane = drm_state->planes[i].ptr;
> +			plane = intel_state->base.planes[i].ptr;
>  
>  			/*
>  			 * to enable/disable hq mode, add planes that are using scaler
> @@ -516,7 +515,7 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv,
>  					continue;
>  
>  				plane = drm_plane_from_index(&dev_priv->drm, i);
> -				state = drm_atomic_get_plane_state(drm_state, plane);
> +				state = drm_atomic_get_plane_state(&intel_state->base, plane);
>  				if (IS_ERR(state)) {
>  					drm_dbg_kms(&dev_priv->drm,
>  						    "Failed to add [PLANE:%d] to drm_state\n",
> diff --git a/drivers/gpu/drm/i915/display/skl_scaler.h b/drivers/gpu/drm/i915/display/skl_scaler.h
> index 73fa59da09f9..4d2e2dbb1666 100644
> --- a/drivers/gpu/drm/i915/display/skl_scaler.h
> +++ b/drivers/gpu/drm/i915/display/skl_scaler.h
> @@ -5,7 +5,7 @@
>  #ifndef INTEL_SCALER_H
>  #define INTEL_SCALER_H
>  
> -struct drm_i915_private;
> +struct intel_atomic_state;
>  struct intel_crtc;
>  struct intel_crtc_state;
>  struct intel_plane;
> @@ -16,9 +16,8 @@ int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
>  int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
>  			    struct intel_plane_state *plane_state);
>  
> -int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv,
> -			       struct intel_crtc *crtc,
> -			       struct intel_crtc_state *crtc_state);
> +int intel_atomic_setup_scalers(struct intel_atomic_state *state,
> +			       struct intel_crtc *crtc);
>  
>  void skl_pfit_enable(const struct intel_crtc_state *crtc_state);

-- 
Jani Nikula, Intel