Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Michał Grzelak" <michal.grzelak@intel.com>
To: Mika Kahola <mika.kahola@intel.com>
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org
Subject: Re: [PATCH 2/4] drm/i915/display: Add DG2 MPLLB DPLL manager support
Date: Fri, 19 Jun 2026 15:18:15 +0200 (CEST)	[thread overview]
Message-ID: <642dbf88-144f-d007-e67d-fd0dced1f412@intel.com> (raw)
In-Reply-To: <20260518103642.3678448-3-mika.kahola@intel.com>

[-- Attachment #1: Type: text/plain, Size: 9820 bytes --]

On Mon, 18 May 2026, 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.
>
> Assisted-by: Copilot:claude-sonnet-4-6
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>

Reviewed-by: Michał Grzelak <michal.grzelak@intel.com>

BR,
Michał

> ---
> drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 227 ++++++++++++++++++
> drivers/gpu/drm/i915/display/intel_dpll_mgr.h |  22 ++
> 2 files changed, 249 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index bb487e647f76..c03560532d94 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,232 @@ 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;
> +	struct intel_encoder *fallback = NULL;
> +
> +	for_each_intel_encoder(display->drm, encoder) {
> +		struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
> +
> +		if (dg2_port_to_pll_id(encoder->port) != pll->info->id)
> +			continue;
> +
> +		/*
> +		 * Multiple encoder objects may exist for the same physical port.
> +		 * Prefer the encoder that is actively using this PLL.
> +		 */
> +		if (crtc && (pll->active_mask & BIT(crtc->pipe)))
> +			return encoder;
> +
> +		/*
> +		 * Fall back to a matching encoder so that readout paths can
> +		 * still resolve the PHY when active_mask does not identify an
> +		 * active user.
> +		 */
> +		if (!fallback)
> +			fallback = encoder;
> +	}
> +
> +	return fallback;
> +}
> +
> +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,
> -- 
> 2.43.0
>
>

  reply	other threads:[~2026-06-19 13:18 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-18 10:36 [PATCH 0/4] drm/i915/display: Switch DG2 MPLLB handling to the DPLL framework Mika Kahola
2026-05-18 10:36 ` [PATCH 1/4] drm/i915/display: Split out DG2 MPLLB enable helper Mika Kahola
2026-06-19 13:05   ` Michał Grzelak
2026-05-18 10:36 ` [PATCH 2/4] drm/i915/display: Add DG2 MPLLB DPLL manager support Mika Kahola
2026-06-19 13:18   ` Michał Grzelak [this message]
2026-06-19 13:57   ` Imre Deak
2026-06-22  9:20     ` Kahola, Mika
2026-05-18 10:36 ` [PATCH 3/4] drm/i915/display: Prepare DG2 DDI and compute paths for DPLL framework Mika Kahola
2026-06-19 13:18   ` Michał Grzelak
2026-05-18 10:36 ` [PATCH 4/4] drm/i915/display: Switch DG2 to use " Mika Kahola
2026-06-19 13:19   ` Michał Grzelak
2026-05-18 10:43 ` ✓ CI.KUnit: success for drm/i915/display: Switch DG2 MPLLB handling to the " Patchwork
2026-05-18 11:20 ` ✓ Xe.CI.BAT: " Patchwork
2026-05-18 14:00 ` ✗ Xe.CI.FULL: failure " Patchwork

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=642dbf88-144f-d007-e67d-fd0dced1f412@intel.com \
    --to=michal.grzelak@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=mika.kahola@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox