Re: [PATCH 1/8] drm/i915/display: update intel_enabled_dbuf_slices_mask to use intel_display

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

On Tue, 05 Nov 2024, Vinod Govindapillai <vinod.govindapillai@intel.com> wrote:
> Update intel_enabled_dbuf_slices_mask to use intel_display instead
> of drm_i915_private object. This is a prepratory patch for the next
> patch in the series, where all intel_de_read calls in skl_watermarks.c
> are updated to use intel_display instead of drm_i915_private.
>
> Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_power.c      | 2 +-
>  drivers/gpu/drm/i915/display/intel_display_power_well.c | 2 +-
>  drivers/gpu/drm/i915/display/skl_watermark.c            | 9 +++++----
>  drivers/gpu/drm/i915/display/skl_watermark.h            | 3 ++-
>  4 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 2766fd9208b0..62e0faffca40 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -1090,7 +1090,7 @@ static void gen9_dbuf_enable(struct drm_i915_private *dev_priv)
>  	u8 slices_mask;
>  
>  	dev_priv->display.dbuf.enabled_slices =
> -		intel_enabled_dbuf_slices_mask(dev_priv);
> +		intel_enabled_dbuf_slices_mask(&dev_priv->display);

Please add a local struct intel_display *display variable and pass that
to intel_enabled_dbuf_slices_mask().

The point is, all of the i915/dev_priv references need to go, and if you
add &dev_priv->display, this line needs to be updated again.

>  
>  	slices_mask = BIT(DBUF_S1) | dev_priv->display.dbuf.enabled_slices;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power_well.c b/drivers/gpu/drm/i915/display/intel_display_power_well.c
> index f0131dd853de..f792db191fcf 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
> @@ -973,7 +973,7 @@ static bool gen9_dc_off_power_well_enabled(struct drm_i915_private *dev_priv,
>  
>  static void gen9_assert_dbuf_enabled(struct drm_i915_private *dev_priv)
>  {
> -	u8 hw_enabled_dbuf_slices = intel_enabled_dbuf_slices_mask(dev_priv);
> +	u8 hw_enabled_dbuf_slices = intel_enabled_dbuf_slices_mask(&dev_priv->display);

Ditto.

>  	u8 enabled_dbuf_slices = dev_priv->display.dbuf.enabled_slices;
>  
>  	drm_WARN(&dev_priv->drm,
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
> index 3b0e87edbacf..d9d7238f0fb4 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -52,13 +52,13 @@ struct skl_wm_params {
>  	u32 dbuf_block_size;
>  };
>  
> -u8 intel_enabled_dbuf_slices_mask(struct drm_i915_private *i915)
> +u8 intel_enabled_dbuf_slices_mask(struct intel_display *display)
>  {
>  	u8 enabled_slices = 0;
>  	enum dbuf_slice slice;
>  
> -	for_each_dbuf_slice(i915, slice) {
> -		if (intel_de_read(i915, DBUF_CTL_S(slice)) & DBUF_POWER_STATE)
> +	for_each_dbuf_slice(display, slice) {
> +		if (intel_de_read(display, DBUF_CTL_S(slice)) & DBUF_POWER_STATE)
>  			enabled_slices |= BIT(slice);
>  	}
>  
> @@ -3126,6 +3126,7 @@ void intel_wm_state_verify(struct intel_atomic_state *state,
>  			   struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *i915 = to_i915(state->base.dev);
> +	struct intel_display *display = to_intel_display(state);

Please prefer putting the display variable first if at all possible.


>  	const struct intel_crtc_state *new_crtc_state =
>  		intel_atomic_get_new_crtc_state(state, crtc);
>  	struct skl_hw_state {
> @@ -3149,7 +3150,7 @@ void intel_wm_state_verify(struct intel_atomic_state *state,
>  
>  	skl_pipe_ddb_get_hw_state(crtc, hw->ddb, hw->ddb_y);
>  
> -	hw_enabled_slices = intel_enabled_dbuf_slices_mask(i915);
> +	hw_enabled_slices = intel_enabled_dbuf_slices_mask(display);
>  
>  	if (DISPLAY_VER(i915) >= 11 &&
>  	    hw_enabled_slices != i915->display.dbuf.enabled_slices)
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h b/drivers/gpu/drm/i915/display/skl_watermark.h
> index e73baec94873..990793e36272 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.h
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.h
> @@ -17,11 +17,12 @@ struct intel_atomic_state;
>  struct intel_bw_state;
>  struct intel_crtc;
>  struct intel_crtc_state;
> +struct intel_display;
>  struct intel_plane;
>  struct skl_pipe_wm;
>  struct skl_wm_level;
>  
> -u8 intel_enabled_dbuf_slices_mask(struct drm_i915_private *i915);
> +u8 intel_enabled_dbuf_slices_mask(struct intel_display *display);
>  
>  void intel_sagv_pre_plane_update(struct intel_atomic_state *state);
>  void intel_sagv_post_plane_update(struct intel_atomic_state *state);

-- 
Jani Nikula, Intel