All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@intel.com>
To: Suraj Kandpal <suraj.kandpal@intel.com>,
	intel-xe@lists.freedesktop.org, intel-gfx@lists.freedesktop.org
Cc: ankit.k.nautiyal@intel.com, Suraj Kandpal <suraj.kandpal@intel.com>
Subject: Re: [PATCH 9/9] drm/i915/dpll: Replace all other leftover drm_i915_private
Date: Tue, 11 Feb 2025 15:17:17 +0200	[thread overview]
Message-ID: <87h6501uzm.fsf@intel.com> (raw)
In-Reply-To: <20250211104857.3501566-10-suraj.kandpal@intel.com>

On Tue, 11 Feb 2025, Suraj Kandpal <suraj.kandpal@intel.com> wrote:
> Replace all other left over drm_i915_private with intel_display
> in dpll_mgr.c.
>
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 99 +++++++++----------
>  1 file changed, 48 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index 26b6b9372fa3..96abb7e295a2 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -117,11 +117,10 @@ struct intel_dpll_mgr {
>  };
>  
>  static void
> -intel_atomic_duplicate_dpll_state(struct drm_i915_private *i915,
> +intel_atomic_duplicate_dpll_state(struct intel_display *display,
>  				  struct intel_shared_dpll_state *shared_dpll)
>  {
>  	struct intel_shared_dpll *pll;
> -	struct intel_display *display = to_intel_display(&i915->drm);
>  	int i;
>  
>  	/* Copy shared dpll state */
> @@ -139,7 +138,7 @@ intel_atomic_get_shared_dpll_state(struct drm_atomic_state *s)
>  	if (!state->dpll_set) {
>  		state->dpll_set = true;
>  
> -		intel_atomic_duplicate_dpll_state(to_i915(s->dev),
> +		intel_atomic_duplicate_dpll_state(to_intel_display(state),

Please do not add inline to_intel_display() usages, ever, anywhere. Add
a local display variable instead.

With that fixed,

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


>  						  state->shared_dpll);
>  	}
>  
> @@ -420,13 +419,13 @@ intel_reference_shared_dpll_crtc(const struct intel_crtc *crtc,
>  				 const struct intel_shared_dpll *pll,
>  				 struct intel_shared_dpll_state *shared_dpll_state)
>  {
> -	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +	struct intel_display *display = to_intel_display(crtc);
>  
> -	drm_WARN_ON(&i915->drm, (shared_dpll_state->pipe_mask & BIT(crtc->pipe)) != 0);
> +	drm_WARN_ON(display->drm, (shared_dpll_state->pipe_mask & BIT(crtc->pipe)) != 0);
>  
>  	shared_dpll_state->pipe_mask |= BIT(crtc->pipe);
>  
> -	drm_dbg_kms(&i915->drm, "[CRTC:%d:%s] reserving %s\n",
> +	drm_dbg_kms(display->drm, "[CRTC:%d:%s] reserving %s\n",
>  		    crtc->base.base.id, crtc->base.name, pll->info->name);
>  }
>  
> @@ -459,13 +458,13 @@ intel_unreference_shared_dpll_crtc(const struct intel_crtc *crtc,
>  				   const struct intel_shared_dpll *pll,
>  				   struct intel_shared_dpll_state *shared_dpll_state)
>  {
> -	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +	struct intel_display *display = to_intel_display(crtc);
>  
> -	drm_WARN_ON(&i915->drm, (shared_dpll_state->pipe_mask & BIT(crtc->pipe)) == 0);
> +	drm_WARN_ON(display->drm, (shared_dpll_state->pipe_mask & BIT(crtc->pipe)) == 0);
>  
>  	shared_dpll_state->pipe_mask &= ~BIT(crtc->pipe);
>  
> -	drm_dbg_kms(&i915->drm, "[CRTC:%d:%s] releasing %s\n",
> +	drm_dbg_kms(display->drm, "[CRTC:%d:%s] releasing %s\n",
>  		    crtc->base.base.id, crtc->base.name, pll->info->name);
>  }
>  
> @@ -545,9 +544,8 @@ static bool ibx_pch_dpll_get_hw_state(struct intel_display *display,
>  	return val & DPLL_VCO_ENABLE;
>  }
>  
> -static void ibx_assert_pch_refclk_enabled(struct drm_i915_private *i915)
> +static void ibx_assert_pch_refclk_enabled(struct intel_display *display)
>  {
> -	struct intel_display *display = &i915->display;
>  	u32 val;
>  	bool enabled;
>  
> @@ -562,12 +560,11 @@ static void ibx_pch_dpll_enable(struct intel_display *display,
>  				struct intel_shared_dpll *pll,
>  				const struct intel_dpll_hw_state *dpll_hw_state)
>  {
> -	struct drm_i915_private *i915 = to_i915(display->drm);
>  	const struct i9xx_dpll_hw_state *hw_state = &dpll_hw_state->i9xx;
>  	const enum intel_dpll_id id = pll->info->id;
>  
>  	/* PCH refclock must be enabled first */
> -	ibx_assert_pch_refclk_enabled(i915);
> +	ibx_assert_pch_refclk_enabled(display);
>  
>  	intel_de_write(display, PCH_FP0(id), hw_state->fp0);
>  	intel_de_write(display, PCH_FP1(id), hw_state->fp1);
> @@ -1074,7 +1071,7 @@ hsw_ddi_wrpll_get_dpll(struct intel_atomic_state *state,
>  static int
>  hsw_ddi_lcpll_compute_dpll(struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> +	struct intel_display *display = to_intel_display(crtc_state);
>  	int clock = crtc_state->port_clock;
>  
>  	switch (clock / 2) {
> @@ -1083,7 +1080,7 @@ hsw_ddi_lcpll_compute_dpll(struct intel_crtc_state *crtc_state)
>  	case 270000:
>  		return 0;
>  	default:
> -		drm_dbg_kms(&i915->drm, "Invalid clock for DP: %d\n",
> +		drm_dbg_kms(display->drm, "Invalid clock for DP: %d\n",
>  			    clock);
>  		return -EINVAL;
>  	}
> @@ -2255,7 +2252,7 @@ static int
>  bxt_ddi_hdmi_pll_dividers(struct intel_crtc_state *crtc_state,
>  			  struct dpll *clk_div)
>  {
> -	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> +	struct intel_display *display = to_intel_display(crtc_state);
>  
>  	/* Calculate HDMI div */
>  	/*
> @@ -2265,7 +2262,7 @@ bxt_ddi_hdmi_pll_dividers(struct intel_crtc_state *crtc_state,
>  	if (!bxt_find_best_dpll(crtc_state, clk_div))
>  		return -EINVAL;
>  
> -	drm_WARN_ON(&i915->drm, clk_div->m1 != 2);
> +	drm_WARN_ON(display->drm, clk_div->m1 != 2);
>  
>  	return 0;
>  }
> @@ -2273,7 +2270,7 @@ bxt_ddi_hdmi_pll_dividers(struct intel_crtc_state *crtc_state,
>  static void bxt_ddi_dp_pll_dividers(struct intel_crtc_state *crtc_state,
>  				    struct dpll *clk_div)
>  {
> -	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> +	struct intel_display *display = to_intel_display(crtc_state);
>  	int i;
>  
>  	*clk_div = bxt_dp_clk_val[0];
> @@ -2284,16 +2281,16 @@ static void bxt_ddi_dp_pll_dividers(struct intel_crtc_state *crtc_state,
>  		}
>  	}
>  
> -	chv_calc_dpll_params(i915->display.dpll.ref_clks.nssc, clk_div);
> +	chv_calc_dpll_params(display->dpll.ref_clks.nssc, clk_div);
>  
> -	drm_WARN_ON(&i915->drm, clk_div->vco == 0 ||
> +	drm_WARN_ON(display->drm, clk_div->vco == 0 ||
>  		    clk_div->dot != crtc_state->port_clock);
>  }
>  
>  static int bxt_ddi_set_dpll_hw_state(struct intel_crtc_state *crtc_state,
>  				     const struct dpll *clk_div)
>  {
> -	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> +	struct intel_display *display = to_intel_display(crtc_state);
>  	struct bxt_dpll_hw_state *hw_state = &crtc_state->dpll_hw_state.bxt;
>  	int clock = crtc_state->port_clock;
>  	int vco = clk_div->vco;
> @@ -2317,7 +2314,7 @@ static int bxt_ddi_set_dpll_hw_state(struct intel_crtc_state *crtc_state,
>  		gain_ctl = 1;
>  		targ_cnt = 9;
>  	} else {
> -		drm_err(&i915->drm, "Invalid VCO\n");
> +		drm_err(display->drm, "Invalid VCO\n");
>  		return -EINVAL;
>  	}
>  
> @@ -2700,9 +2697,9 @@ static const struct skl_wrpll_params tgl_tbt_pll_24MHz_values = {
>  static int icl_calc_dp_combo_pll(struct intel_crtc_state *crtc_state,
>  				 struct skl_wrpll_params *pll_params)
>  {
> -	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> +	struct intel_display *display = to_intel_display(crtc_state);
>  	const struct icl_combo_pll_params *params =
> -		i915->display.dpll.ref_clks.nssc == 24000 ?
> +		display->dpll.ref_clks.nssc == 24000 ?
>  		icl_dp_combo_pll_24MHz_values :
>  		icl_dp_combo_pll_19_2MHz_values;
>  	int clock = crtc_state->port_clock;
> @@ -2722,12 +2719,12 @@ static int icl_calc_dp_combo_pll(struct intel_crtc_state *crtc_state,
>  static int icl_calc_tbt_pll(struct intel_crtc_state *crtc_state,
>  			    struct skl_wrpll_params *pll_params)
>  {
> -	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> +	struct intel_display *display = to_intel_display(crtc_state);
>  
> -	if (DISPLAY_VER(i915) >= 12) {
> -		switch (i915->display.dpll.ref_clks.nssc) {
> +	if (DISPLAY_VER(display) >= 12) {
> +		switch (display->dpll.ref_clks.nssc) {
>  		default:
> -			MISSING_CASE(i915->display.dpll.ref_clks.nssc);
> +			MISSING_CASE(display->dpll.ref_clks.nssc);
>  			fallthrough;
>  		case 19200:
>  		case 38400:
> @@ -2738,9 +2735,9 @@ static int icl_calc_tbt_pll(struct intel_crtc_state *crtc_state,
>  			break;
>  		}
>  	} else {
> -		switch (i915->display.dpll.ref_clks.nssc) {
> +		switch (display->dpll.ref_clks.nssc) {
>  		default:
> -			MISSING_CASE(i915->display.dpll.ref_clks.nssc);
> +			MISSING_CASE(display->dpll.ref_clks.nssc);
>  			fallthrough;
>  		case 19200:
>  		case 38400:
> @@ -2998,9 +2995,9 @@ static int icl_mg_pll_find_divisors(int clock_khz, bool is_dp, bool use_ssc,
>  static int icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state,
>  				 struct intel_dpll_hw_state *dpll_hw_state)
>  {
> -	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> +	struct intel_display *display = to_intel_display(crtc_state);
>  	struct icl_dpll_hw_state *hw_state = &dpll_hw_state->icl;
> -	int refclk_khz = i915->display.dpll.ref_clks.nssc;
> +	int refclk_khz = display->dpll.ref_clks.nssc;
>  	int clock = crtc_state->port_clock;
>  	u32 dco_khz, m1div, m2div_int, m2div_rem, m2div_frac;
>  	u32 iref_ndiv, iref_trim, iref_pulse_w;
> @@ -3010,7 +3007,7 @@ static int icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state,
>  	u64 tmp;
>  	bool use_ssc = false;
>  	bool is_dp = !intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI);
> -	bool is_dkl = DISPLAY_VER(i915) >= 12;
> +	bool is_dkl = DISPLAY_VER(display) >= 12;
>  	int ret;
>  
>  	ret = icl_mg_pll_find_divisors(clock, is_dp, use_ssc, &dco_khz,
> @@ -3108,8 +3105,8 @@ static int icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state,
>  					 DKL_PLL_DIV0_PROP_COEFF(prop_coeff) |
>  					 DKL_PLL_DIV0_FBPREDIV(m1div) |
>  					 DKL_PLL_DIV0_FBDIV_INT(m2div_int);
> -		if (i915->display.vbt.override_afc_startup) {
> -			u8 val = i915->display.vbt.override_afc_startup_val;
> +		if (display->vbt.override_afc_startup) {
> +			u8 val = display->vbt.override_afc_startup_val;
>  
>  			hw_state->mg_pll_div0 |= DKL_PLL_DIV0_AFC_STARTUP(val);
>  		}
> @@ -3347,7 +3344,6 @@ static int icl_get_combo_phy_dpll(struct intel_atomic_state *state,
>  				  struct intel_encoder *encoder)
>  {
>  	struct intel_display *display = to_intel_display(crtc);
> -	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
>  	struct intel_crtc_state *crtc_state =
>  		intel_atomic_get_new_crtc_state(state, crtc);
>  	struct icl_port_dpll *port_dpll =
> @@ -3355,13 +3351,13 @@ static int icl_get_combo_phy_dpll(struct intel_atomic_state *state,
>  	enum port port = encoder->port;
>  	unsigned long dpll_mask;
>  
> -	if (IS_ALDERLAKE_S(i915)) {
> +	if (display->platform.alderlake_s) {
>  		dpll_mask =
>  			BIT(DPLL_ID_DG1_DPLL3) |
>  			BIT(DPLL_ID_DG1_DPLL2) |
>  			BIT(DPLL_ID_ICL_DPLL1) |
>  			BIT(DPLL_ID_ICL_DPLL0);
> -	} else if (IS_DG1(i915)) {
> +	} else if (display->platform.dg1) {
>  		if (port == PORT_D || port == PORT_E) {
>  			dpll_mask =
>  				BIT(DPLL_ID_DG1_DPLL2) |
> @@ -3371,12 +3367,13 @@ static int icl_get_combo_phy_dpll(struct intel_atomic_state *state,
>  				BIT(DPLL_ID_DG1_DPLL0) |
>  				BIT(DPLL_ID_DG1_DPLL1);
>  		}
> -	} else if (IS_ROCKETLAKE(i915)) {
> +	} else if (display->platform.rocketlake) {
>  		dpll_mask =
>  			BIT(DPLL_ID_EHL_DPLL4) |
>  			BIT(DPLL_ID_ICL_DPLL1) |
>  			BIT(DPLL_ID_ICL_DPLL0);
> -	} else if ((IS_JASPERLAKE(i915) || IS_ELKHARTLAKE(i915)) &&
> +	} else if ((display->platform.jasperlake ||
> +		    display->platform.elkhartlake) &&
>  		   port != PORT_A) {
>  		dpll_mask =
>  			BIT(DPLL_ID_EHL_DPLL4) |
> @@ -4381,10 +4378,10 @@ int intel_compute_shared_dplls(struct intel_atomic_state *state,
>  			       struct intel_crtc *crtc,
>  			       struct intel_encoder *encoder)
>  {
> -	struct drm_i915_private *i915 = to_i915(state->base.dev);
> -	const struct intel_dpll_mgr *dpll_mgr = i915->display.dpll.mgr;
> +	struct intel_display *display = to_intel_display(state);
> +	const struct intel_dpll_mgr *dpll_mgr = display->dpll.mgr;
>  
> -	if (drm_WARN_ON(&i915->drm, !dpll_mgr))
> +	if (drm_WARN_ON(display->drm, !dpll_mgr))
>  		return -EINVAL;
>  
>  	return dpll_mgr->compute_dplls(state, crtc, encoder);
> @@ -4414,10 +4411,10 @@ int intel_reserve_shared_dplls(struct intel_atomic_state *state,
>  			       struct intel_crtc *crtc,
>  			       struct intel_encoder *encoder)
>  {
> -	struct drm_i915_private *i915 = to_i915(state->base.dev);
> -	const struct intel_dpll_mgr *dpll_mgr = i915->display.dpll.mgr;
> +	struct intel_display *display = to_intel_display(state);
> +	const struct intel_dpll_mgr *dpll_mgr = display->dpll.mgr;
>  
> -	if (drm_WARN_ON(&i915->drm, !dpll_mgr))
> +	if (drm_WARN_ON(display->drm, !dpll_mgr))
>  		return -EINVAL;
>  
>  	return dpll_mgr->get_dplls(state, crtc, encoder);
> @@ -4437,8 +4434,8 @@ int intel_reserve_shared_dplls(struct intel_atomic_state *state,
>  void intel_release_shared_dplls(struct intel_atomic_state *state,
>  				struct intel_crtc *crtc)
>  {
> -	struct drm_i915_private *i915 = to_i915(state->base.dev);
> -	const struct intel_dpll_mgr *dpll_mgr = i915->display.dpll.mgr;
> +	struct intel_display *display = to_intel_display(state);
> +	const struct intel_dpll_mgr *dpll_mgr = display->dpll.mgr;
>  
>  	/*
>  	 * FIXME: this function is called for every platform having a
> @@ -4466,10 +4463,10 @@ void intel_update_active_dpll(struct intel_atomic_state *state,
>  			      struct intel_crtc *crtc,
>  			      struct intel_encoder *encoder)
>  {
> -	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> -	const struct intel_dpll_mgr *dpll_mgr = i915->display.dpll.mgr;
> +	struct intel_display *display = to_intel_display(encoder);
> +	const struct intel_dpll_mgr *dpll_mgr = display->dpll.mgr;
>  
> -	if (drm_WARN_ON(&i915->drm, !dpll_mgr))
> +	if (drm_WARN_ON(display->drm, !dpll_mgr))
>  		return;
>  
>  	dpll_mgr->update_active_dpll(state, crtc, encoder);

-- 
Jani Nikula, Intel

  reply	other threads:[~2025-02-11 13:17 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-11 10:48 [PATCH 0/9] drm_i915_private to intel_display cleanup Suraj Kandpal
2025-02-11 10:48 ` [PATCH 1/9] drm/i915/display_debug_fs: Use intel_display wherever possible Suraj Kandpal
2025-02-11 12:51   ` Jani Nikula
2025-02-11 12:52   ` Jani Nikula
2025-02-11 10:48 ` [PATCH 2/9] drm/i915/display_debug_fs: Prefer using display->platform Suraj Kandpal
2025-02-11 12:53   ` Jani Nikula
2025-02-11 10:48 ` [PATCH 3/9] drm/i915/dpll: Change param to intel_display in for_each_shared_dpll Suraj Kandpal
2025-02-11 12:56   ` Jani Nikula
2025-02-11 10:48 ` [PATCH 4/9] drm/i915/dpll: Use intel_display for dpll dump and compare hw state Suraj Kandpal
2025-02-11 12:59   ` Jani Nikula
2025-02-11 10:48 ` [PATCH 5/9] drm/i915/dpll: Use intel_display possible in shared_dpll_mgr hooks Suraj Kandpal
2025-02-11 13:10   ` Jani Nikula
2025-02-11 10:48 ` [PATCH 6/9] drm/i915/dpll: Use intel_display for asserting pll Suraj Kandpal
2025-02-11 13:12   ` Jani Nikula
2025-02-11 10:48 ` [PATCH 7/9] drm/i915/dpll: Use intel_display for update_refclk hook Suraj Kandpal
2025-02-11 13:12   ` Jani Nikula
2025-02-11 10:48 ` [PATCH 8/9] drm/i915/dpll: Accept intel_display as argument for shared_dpll_init Suraj Kandpal
2025-02-11 13:14   ` Jani Nikula
2025-02-11 14:23     ` Kandpal, Suraj
2025-02-11 16:57       ` Jani Nikula
2025-02-12  7:35         ` Kandpal, Suraj
2025-02-11 10:48 ` [PATCH 9/9] drm/i915/dpll: Replace all other leftover drm_i915_private Suraj Kandpal
2025-02-11 13:17   ` Jani Nikula [this message]
2025-02-11 11:50 ` ✓ CI.Patch_applied: success for drm_i915_private to intel_display cleanup (rev2) Patchwork
2025-02-11 11:51 ` ✗ CI.checkpatch: warning " Patchwork
2025-02-11 11:52 ` ✓ CI.KUnit: success " Patchwork
2025-02-11 12:09 ` ✓ CI.Build: " Patchwork
2025-02-11 12:10 ` ✗ Fi.CI.CHECKPATCH: warning " Patchwork
2025-02-11 12:10 ` ✗ Fi.CI.SPARSE: " Patchwork
2025-02-11 12:10 ` ✗ CI.Hooks: failure " Patchwork
2025-02-11 12:11 ` ✗ CI.checksparse: warning " Patchwork
2025-02-11 12:31 ` ✓ Xe.CI.BAT: success " Patchwork
2025-02-11 13:45 ` ✓ i915.CI.BAT: " Patchwork
2025-02-11 16:23 ` ✗ i915.CI.Full: failure " Patchwork
2025-02-11 20:46 ` ✗ Xe.CI.Full: " Patchwork
2025-02-12  9:54 ` [PATCH 0/9] drm_i915_private to intel_display cleanup Kandpal, Suraj
  -- strict thread matches above, loose matches on Subject: below --
2025-02-10 12:39 Suraj Kandpal
2025-02-10 12:39 ` [PATCH 9/9] drm/i915/dpll: Replace all other leftover drm_i915_private Suraj Kandpal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h6501uzm.fsf@intel.com \
    --to=jani.nikula@intel.com \
    --cc=ankit.k.nautiyal@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=suraj.kandpal@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.