[Intel-gfx] [PATCH 06/39] drm/i915: move wm_disp funcs to display.funcs

[Murthy, Arun R]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Jani
> Nikula
> Sent: Thursday, August 11, 2022 8:37 PM
> To: intel-gfx at lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula at intel.com>; De Marchi, Lucas
> <lucas.demarchi at intel.com>
> Subject: [Intel-gfx] [PATCH 06/39] drm/i915: move wm_disp funcs to
> display.funcs
> 
> Move display related members under drm_i915_private display sub-struct.
> 
> Rename struct drm_i915_wm_disp_funcs to intel_wm_funcs while at it.
> 
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c  | 34 +++++++--------
> .../gpu/drm/i915/display/intel_display_core.h | 21 ++++++++++
>  drivers/gpu/drm/i915/i915_drv.h               | 22 ----------
>  drivers/gpu/drm/i915/intel_pm.c               | 42 +++++++++----------
>  4 files changed, 59 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 24ab1501beea..7db4ac27364d 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -164,16 +164,16 @@ static void ilk_pfit_enable(const struct
> intel_crtc_state *crtc_state);
>   */
>  void intel_update_watermarks(struct drm_i915_private *dev_priv)  {
> -	if (dev_priv->wm_disp->update_wm)
> -		dev_priv->wm_disp->update_wm(dev_priv);
> +	if (dev_priv->display.funcs.wm->update_wm)
> +		dev_priv->display.funcs.wm->update_wm(dev_priv);
>  }
> 
>  static int intel_compute_pipe_wm(struct intel_atomic_state *state,
>  				 struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> -	if (dev_priv->wm_disp->compute_pipe_wm)
> -		return dev_priv->wm_disp->compute_pipe_wm(state, crtc);
> +	if (dev_priv->display.funcs.wm->compute_pipe_wm)
> +		return dev_priv->display.funcs.wm-
> >compute_pipe_wm(state, crtc);
>  	return 0;
>  }
> 
> @@ -181,20 +181,20 @@ static int intel_compute_intermediate_wm(struct
> intel_atomic_state *state,
>  					 struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> -	if (!dev_priv->wm_disp->compute_intermediate_wm)
> +	if (!dev_priv->display.funcs.wm->compute_intermediate_wm)
>  		return 0;
>  	if (drm_WARN_ON(&dev_priv->drm,
> -			!dev_priv->wm_disp->compute_pipe_wm))
> +			!dev_priv->display.funcs.wm->compute_pipe_wm))
>  		return 0;
> -	return dev_priv->wm_disp->compute_intermediate_wm(state, crtc);
> +	return dev_priv->display.funcs.wm-
> >compute_intermediate_wm(state,
> +crtc);
>  }
> 
>  static bool intel_initial_watermarks(struct intel_atomic_state *state,
>  				     struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> -	if (dev_priv->wm_disp->initial_watermarks) {
> -		dev_priv->wm_disp->initial_watermarks(state, crtc);
> +	if (dev_priv->display.funcs.wm->initial_watermarks) {
> +		dev_priv->display.funcs.wm->initial_watermarks(state, crtc);
>  		return true;
>  	}
>  	return false;
> @@ -204,23 +204,23 @@ static void intel_atomic_update_watermarks(struct
> intel_atomic_state *state,
>  					   struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> -	if (dev_priv->wm_disp->atomic_update_watermarks)
> -		dev_priv->wm_disp->atomic_update_watermarks(state,
> crtc);
> +	if (dev_priv->display.funcs.wm->atomic_update_watermarks)
> +		dev_priv->display.funcs.wm-
> >atomic_update_watermarks(state, crtc);
>  }
> 
>  static void intel_optimize_watermarks(struct intel_atomic_state *state,
>  				      struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> -	if (dev_priv->wm_disp->optimize_watermarks)
> -		dev_priv->wm_disp->optimize_watermarks(state, crtc);
> +	if (dev_priv->display.funcs.wm->optimize_watermarks)
> +		dev_priv->display.funcs.wm->optimize_watermarks(state,
> crtc);
>  }
> 
>  static int intel_compute_global_watermarks(struct intel_atomic_state
> *state)  {
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> -	if (dev_priv->wm_disp->compute_global_watermarks)
> -		return dev_priv->wm_disp-
> >compute_global_watermarks(state);
> +	if (dev_priv->display.funcs.wm->compute_global_watermarks)
> +		return dev_priv->display.funcs.wm-
> >compute_global_watermarks(state);
>  	return 0;
>  }
> 
> @@ -2400,7 +2400,7 @@ static void i9xx_crtc_disable(struct
> intel_atomic_state *state,
>  	if (DISPLAY_VER(dev_priv) != 2)
>  		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe,
> false);
> 
> -	if (!dev_priv->wm_disp->initial_watermarks)
> +	if (!dev_priv->display.funcs.wm->initial_watermarks)
>  		intel_update_watermarks(dev_priv);
> 
>  	/* clock the pipe down to 640x480 at 60 to potentially save power */
> @@ -8454,7 +8454,7 @@ static void sanitize_watermarks(struct
> drm_i915_private *dev_priv)
>  	int i;
> 
>  	/* Only supported on platforms that use atomic watermark design */
> -	if (!dev_priv->wm_disp->optimize_watermarks)
> +	if (!dev_priv->display.funcs.wm->optimize_watermarks)
>  		return;
> 
>  	state = drm_atomic_state_alloc(&dev_priv->drm);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h
> b/drivers/gpu/drm/i915/display/intel_display_core.h
> index 98c6ccdc9100..a6843ebcca5a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> @@ -8,6 +8,7 @@
> 
>  #include <linux/types.h>
> 
> +struct drm_i915_private;
>  struct intel_atomic_state;
>  struct intel_cdclk_funcs;
>  struct intel_clock_gating_funcs;
> @@ -31,6 +32,23 @@ struct intel_display_funcs {
>  	void (*commit_modeset_enables)(struct intel_atomic_state *state);
> };
> 
> +/* functions used for watermark calcs for display. */ struct
> +intel_wm_funcs {
> +	/* update_wm is for legacy wm management */
> +	void (*update_wm)(struct drm_i915_private *dev_priv);
> +	int (*compute_pipe_wm)(struct intel_atomic_state *state,
> +			       struct intel_crtc *crtc);
> +	int (*compute_intermediate_wm)(struct intel_atomic_state *state,
> +				       struct intel_crtc *crtc);
> +	void (*initial_watermarks)(struct intel_atomic_state *state,
> +				   struct intel_crtc *crtc);
> +	void (*atomic_update_watermarks)(struct intel_atomic_state *state,
> +					 struct intel_crtc *crtc);
> +	void (*optimize_watermarks)(struct intel_atomic_state *state,
> +				    struct intel_crtc *crtc);
> +	int (*compute_global_watermarks)(struct intel_atomic_state *state);
> };
> +
>  struct intel_display {
>  	/* Display functions */
>  	struct {
> @@ -48,6 +66,9 @@ struct intel_display {
> 
>  		/* pm private clock gating functions */
>  		const struct intel_clock_gating_funcs *clock_gating;
> +
> +		/* pm display functions */
> +		const struct intel_wm_funcs *wm;
>  	} funcs;

Can the wm, dbuf, clock related move to a struct intel_pm ? which makes it more meaningful else again we end up creating a struct intel_display a long one like i915_private.

Thanks and Regards,
Arun R Murthy
--------------------