Re: [PATCH v2 2/4] drm/i915/display: Add DG2 MPLLB DPLL manager support

On Tue, 2026-06-30 at 11:53 +0000, Mika Kahola wrote:
> Add DG2-specific DPLL manager infrastructure for MPLLB-backed port
> PLLs.
> 
> Introduce PLL id mapping, DG2-specific DPLL callbacks for
> enable/disable/readout/frequency calculation, and manager callbacks
> for compute/get/dump/compare. The platform is not switched over yet.
> 
> DG2's PLL-to-encoder lookup prefers the active encoder using the PLL,
> but fall back to a matching encoder for the fixed port-to-PLL mapping
> when no active user can be identified.
> 
> This keeps the active-user fix for operational paths while allowing
> readout and verification paths to resolve the PHY even when
> pll->active_mask does not identify an active user.
> 
> v2: Use intel_encoder_is_dig_port() to filter out
>     MST virtual encoders (Imre)
> 
> Assisted-by: Copilot:claude-sonnet-4-6
> Signed-off-by: Mika Kahola <[email protected]>
> Reviewed-by: Michał Grzelak <[email protected]> (v1)
> ---
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 210
> ++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.h |  22 ++
>  2 files changed, 232 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index 0f933101b7dd..3f9ed7f63eb2 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -42,6 +42,7 @@
>  #include "intel_lt_phy.h"
>  #include "intel_mg_phy_regs.h"
>  #include "intel_pch_refclk.h"
> +#include "intel_snps_phy.h"
>  #include "intel_tc.h"
>  
>  /**
> @@ -4735,6 +4736,215 @@ static const struct intel_dpll_mgr
> xe3plpd_pll_mgr = {
>  	.compare_hw_state = xe3plpd_compare_hw_state,
>  };
>  
> +enum intel_dpll_id dg2_port_to_pll_id(enum port port)
> +{
> +	switch (port) {
> +	case PORT_A:
> +		return DPLL_ID_DG2_DPLL_A;
> +	case PORT_B:
> +		return DPLL_ID_DG2_DPLL_B;
> +	case PORT_C:
> +		return DPLL_ID_DG2_DPLL_C;
> +	case PORT_D_XELPD:
> +		return DPLL_ID_DG2_DPLL_D;
> +	case PORT_TC1:
> +		return DPLL_ID_DG2_DPLL_E;
> +	default:
> +		MISSING_CASE(port);
> +		return DPLL_ID_DG2_DPLL_A;
> +	}
> +}
> +
> +static struct intel_encoder *dg2_get_intel_encoder(struct
> intel_display *display,
> +						   const struct
> intel_dpll *pll)
> +{
> +	struct intel_encoder *encoder;
> +
> +	for_each_intel_encoder(display->drm, encoder) {
> +		if (dg2_port_to_pll_id(encoder->port) == pll->info-
> >id &&
> +		    intel_encoder_is_dig_port(encoder))
> +			return encoder;
> +	}
> +
> +	return NULL;
> +}
> +

Reviewed-by: Vinod Govindapillai <[email protected]>

> +static void dg2_mpllb_enable(struct intel_display *display,
> +			     struct intel_dpll *pll,
> +			     const struct intel_dpll_hw_state
> *dpll_hw_state)
> +{
> +	struct intel_encoder *encoder =
> dg2_get_intel_encoder(display, pll);
> +
> +	if (drm_WARN_ON(display->drm, !encoder))
> +		return;
> +
> +	intel_mpllb_enable_phy(encoder, &dpll_hw_state->mpllb);
> +}
> +
> +static void dg2_mpllb_disable(struct intel_display *display,
> +			      struct intel_dpll *pll)
> +{
> +	struct intel_encoder *encoder =
> dg2_get_intel_encoder(display, pll);
> +
> +	if (drm_WARN_ON(display->drm, !encoder))
> +		return;
> +
> +	intel_mpllb_disable(encoder);
> +}
> +
> +static bool dg2_mpllb_get_hw_state(struct intel_display *display,
> +				   struct intel_dpll *pll,
> +				   struct intel_dpll_hw_state
> *dpll_hw_state)
> +{
> +	struct intel_encoder *encoder =
> dg2_get_intel_encoder(display, pll);
> +	enum phy phy;
> +	i915_reg_t enable_reg;
> +	struct ref_tracker *wakeref;
> +	bool ret = false;
> +	u32 val;
> +
> +	if (!encoder)
> +		return false;
> +
> +	wakeref = intel_display_power_get_if_enabled(display,
> +						    
> POWER_DOMAIN_DISPLAY_CORE);
> +	if (!wakeref)
> +		return false;
> +
> +	phy = intel_encoder_to_phy(encoder);
> +	enable_reg = (phy <= PHY_D ? DG2_PLL_ENABLE(phy) :
> MG_PLL_ENABLE(0));
> +
> +	val = intel_de_read(display, enable_reg);
> +	if (!(val & PLL_ENABLE))
> +		goto out;
> +
> +	intel_mpllb_readout_hw_state(encoder, &dpll_hw_state-
> >mpllb);
> +	ret = true;
> +
> +out:
> +	intel_display_power_put(display, POWER_DOMAIN_DISPLAY_CORE,
> wakeref);
> +	return ret;
> +}
> +
> +static int dg2_mpllb_get_freq(struct intel_display *display,
> +			      const struct intel_dpll *pll,
> +			      const struct intel_dpll_hw_state
> *dpll_hw_state)
> +{
> +	struct intel_encoder *encoder =
> dg2_get_intel_encoder(display, pll);
> +
> +	if (drm_WARN_ON(display->drm, !encoder))
> +		return 0;
> +
> +	return intel_mpllb_calc_port_clock(encoder, &dpll_hw_state-
> >mpllb);
> +}
> +
> +static const struct intel_dpll_funcs mpllb_pll_funcs = {
> +	.enable = dg2_mpllb_enable,
> +	.disable = dg2_mpllb_disable,
> +	.get_hw_state = dg2_mpllb_get_hw_state,
> +	.get_freq = dg2_mpllb_get_freq,
> +};
> +
> +static const struct dpll_info dg2_plls[] = {
> +	{ .name = "MPLLB A", .funcs = &mpllb_pll_funcs, .id =
> DPLL_ID_DG2_DPLL_A, },
> +	{ .name = "MPLLB B", .funcs = &mpllb_pll_funcs, .id =
> DPLL_ID_DG2_DPLL_B, },
> +	{ .name = "MPLLB C", .funcs = &mpllb_pll_funcs, .id =
> DPLL_ID_DG2_DPLL_C, },
> +	{ .name = "MPLLB D", .funcs = &mpllb_pll_funcs, .id =
> DPLL_ID_DG2_DPLL_D, },
> +	{ .name = "MPLLB E", .funcs = &mpllb_pll_funcs, .id =
> DPLL_ID_DG2_DPLL_E, },
> +	{}
> +};
> +
> +static int dg2_compute_dplls(struct intel_atomic_state *state,
> +			     struct intel_crtc *crtc,
> +			     struct intel_encoder *encoder)
> +{
> +	struct intel_crtc_state *crtc_state =
> +		intel_atomic_get_new_crtc_state(state, crtc);
> +	struct icl_port_dpll *port_dpll =
> +		&crtc_state->icl_port_dplls[ICL_PORT_DPLL_DEFAULT];
> +	int ret;
> +
> +	ret = intel_mpllb_calc_state(crtc_state, encoder);
> +	if (ret)
> +		return ret;
> +
> +	port_dpll->hw_state = crtc_state->dpll_hw_state;
> +
> +	/* this is mainly for the fastset check */
> +	icl_set_active_port_dpll(crtc_state, ICL_PORT_DPLL_DEFAULT);
> +
> +	crtc_state->port_clock =
> intel_mpllb_calc_port_clock(encoder,
> +							    
> &port_dpll->hw_state.mpllb);
> +
> +	return 0;
> +}
> +
> +static int dg2_get_dplls(struct intel_atomic_state *state,
> +			 struct intel_crtc *crtc,
> +			 struct intel_encoder *encoder)
> +{
> +	struct intel_crtc_state *crtc_state =
> +		intel_atomic_get_new_crtc_state(state, crtc);
> +	struct icl_port_dpll *port_dpll =
> +		&crtc_state->icl_port_dplls[ICL_PORT_DPLL_DEFAULT];
> +	enum intel_dpll_id dpll_id = dg2_port_to_pll_id(encoder-
> >port);
> +
> +	port_dpll->pll = intel_find_dpll(state, crtc,
> +					 &port_dpll->hw_state,
> +					 BIT(dpll_id));
> +	if (!port_dpll->pll)
> +		return -EINVAL;
> +
> +	intel_reference_dpll(state, crtc,
> +			     port_dpll->pll, &port_dpll->hw_state);
> +
> +	icl_set_active_port_dpll(crtc_state, ICL_PORT_DPLL_DEFAULT);
> +
> +	return 0;
> +}
> +
> +static void dg2_dump_hw_state(struct drm_printer *p,
> +			      const struct intel_dpll_hw_state
> *dpll_hw_state)
> +{
> +	const struct intel_mpllb_state *hw_state = &dpll_hw_state-
> >mpllb;
> +
> +	drm_printf(p, "dpll_hw_state: mpllb_cp: 0x%x, mpllb_div:
> 0x%x, "
> +		   "mpllb_div2: 0x%x, mpllb_fracn1: 0x%x, "
> +		   "mpllb_fracn2: 0x%x, mpllb_sscen: 0x%x, "
> +		   "mpllb_sscstep: 0x%x, ref_control: 0x%x\n",
> +		   hw_state->mpllb_cp, hw_state->mpllb_div,
> +		   hw_state->mpllb_div2, hw_state->mpllb_fracn1,
> +		   hw_state->mpllb_fracn2, hw_state->mpllb_sscen,
> +		   hw_state->mpllb_sscstep, hw_state->ref_control);
> +}
> +
> +static bool dg2_compare_hw_state(const struct intel_dpll_hw_state
> *_a,
> +				 const struct intel_dpll_hw_state
> *_b)
> +{
> +	const struct intel_mpllb_state *a = &_a->mpllb;
> +	const struct intel_mpllb_state *b = &_b->mpllb;
> +
> +	return a->mpllb_cp == b->mpllb_cp &&
> +		a->mpllb_div == b->mpllb_div &&
> +		a->mpllb_div2 == b->mpllb_div2 &&
> +		a->mpllb_fracn1 == b->mpllb_fracn1 &&
> +		a->mpllb_fracn2 == b->mpllb_fracn2 &&
> +		a->mpllb_sscen == b->mpllb_sscen &&
> +		a->mpllb_sscstep == b->mpllb_sscstep;
> +}
> +
> +__maybe_unused
> +static const struct intel_dpll_mgr dg2_pll_mgr = {
> +	.dpll_info = dg2_plls,
> +	.compute_dplls = dg2_compute_dplls,
> +	.get_dplls = dg2_get_dplls,
> +	.put_dplls = icl_put_dplls,
> +	.update_active_dpll = icl_update_active_dpll,
> +	.update_ref_clks = icl_update_dpll_ref_clks,
> +	.dump_hw_state = dg2_dump_hw_state,
> +	.compare_hw_state = dg2_compare_hw_state,
> +};
> +
>  /**
>   * intel_dpll_init - Initialize DPLLs
>   * @display: intel_display device
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> index d408ccf6f902..f4b84733a58c 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> @@ -169,9 +169,31 @@ enum intel_dpll_id {
>  	 * @DPLL_ID_DG1_DPLL3: DG1 combo PHY DPLL3
>  	 */
>  	DPLL_ID_DG1_DPLL3 = 3,
> +
> +	/**
> +	 * @DPLL_ID_DG2_DPLL_A: DG2 port PLL for PHY A (PORT_A)
> +	 */
> +	DPLL_ID_DG2_DPLL_A = 0,
> +	/**
> +	 * @DPLL_ID_DG2_DPLL_B: DG2 port PLL for PHY B (PORT_B)
> +	 */
> +	DPLL_ID_DG2_DPLL_B = 1,
> +	/**
> +	 * @DPLL_ID_DG2_DPLL_C: DG2 port PLL for PHY C (PORT_C)
> +	 */
> +	DPLL_ID_DG2_DPLL_C = 2,
> +	/**
> +	 * @DPLL_ID_DG2_DPLL_D: DG2 port PLL for PHY D
> (PORT_D_XELPD)
> +	 */
> +	DPLL_ID_DG2_DPLL_D = 3,
> +	/**
> +	 * @DPLL_ID_DG2_DPLL_E: DG2 port PLL for PHY F (PORT_TC1)
> +	 */
> +	DPLL_ID_DG2_DPLL_E = 4,
>  };
>  
>  #define I915_NUM_PLLS 9
> +enum intel_dpll_id dg2_port_to_pll_id(enum port port);
>  
>  enum icl_port_dpll_id {
>  	ICL_PORT_DPLL_DEFAULT,