On Wed, 01 Oct 2025, Mika Kahola <[email protected]> wrote:
> From: Imre Deak <[email protected]>
>
> The Cx0 PLL enable programming needs to know if the PLL is in DP or HDMI
> mode. The PLL manager framework doesn't pass the CRTC state to the PLL's
> enable hook, so prepare here for the conversion to use the PLL manager
> for Cx0 PHY PLLs by determining the DP/HDMI mode from the PLL state.
>
> For C10 PHYs use the fact that the HDMI divider value in the PLL
> registers are set if and only if the PLL is in HDMI mode.
>
> For C20 PHYs use the DP mode flag programmed to the VDR SERDES register,
> which is set if and only if the PLL is in DP mode.
>
> Assert that the above PLL/VDR SERDES register values match the DP/HDMI
> mode being configured already during state computation.
>
> This also allows dropping the is_dp param from the
> __intel_cx0pll_enable() function, since it can retrieve this now from
> the PLL state.
>
> Signed-off-by: Imre Deak <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_cx0_phy.c | 43 ++++++++++++++++----
> 1 file changed, 36 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> index 93e37b7ac3d9..f2fd766343d5 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> @@ -2090,6 +2090,24 @@ static void intel_c10pll_update_pll(struct
> intel_encoder *encoder,
> pll_state->c10.pll[i] = 0;
> }
>
> +static bool c10pll_state_is_dp(const struct intel_c10pll_state *pll_state)
> +{
> + return !REG_FIELD_GET8(C10_PLL15_HDMIDIV_MASK, pll_state->pll[15]);
> +}
> +
> +static bool c20pll_state_is_dp(const struct intel_c20pll_state *pll_state)
> +{
> + return pll_state->vdr.serdes_rate & PHY_C20_IS_DP;
Wouldn't need this if software state was the logical state instead of
hardware state that you need to mask. It could be just
pll_state->vdr.is_dp, and no function needed.
> +}
> +
> +static bool cx0pll_state_is_dp(const struct intel_cx0pll_state *pll_state)
> +{
> + if (pll_state->use_c10)
> + return c10pll_state_is_dp(&pll_state->c10);
> +
> + return c20pll_state_is_dp(&pll_state->c20);
> +}
> +
> /*
> * TODO: Convert the following align with intel_c20pll_find_table() and
> * intel_c20pll_calc_state_from_table().
> @@ -2099,6 +2117,7 @@ static int intel_c10pll_calc_state_from_table(struct
> intel_encoder *encoder,
> bool is_dp, int port_clock, int
> lane_count,
> struct intel_cx0pll_state
> *pll_state)
> {
> + struct intel_display *display = to_intel_display(encoder);
> int i;
>
> for (i = 0; tables[i]; i++) {
> @@ -2110,6 +2129,8 @@ static int intel_c10pll_calc_state_from_table(struct
> intel_encoder *encoder,
> pll_state->use_c10 = true;
> pll_state->lane_count = lane_count;
>
> + drm_WARN_ON(display->drm, is_dp !=
> c10pll_state_is_dp(&pll_state->c10));
> +
> return 0;
> }
> }
> @@ -2120,6 +2141,8 @@ static int intel_c10pll_calc_state_from_table(struct
> intel_encoder *encoder,
> static int intel_c10pll_calc_state(struct intel_crtc_state *crtc_state,
> struct intel_encoder *encoder)
> {
> + struct intel_display *display = to_intel_display(encoder);
> + bool is_dp = intel_crtc_has_dp_encoder(crtc_state);
> const struct intel_c10pll_state * const *tables;
> int err;
>
> @@ -2127,8 +2150,7 @@ static int intel_c10pll_calc_state(struct
> intel_crtc_state *crtc_state,
> if (!tables)
> return -EINVAL;
>
> - err = intel_c10pll_calc_state_from_table(encoder, tables,
> -
> intel_crtc_has_dp_encoder(crtc_state),
> + err = intel_c10pll_calc_state_from_table(encoder, tables, is_dp,
> crtc_state->port_clock,
> crtc_state->lane_count,
>
> &crtc_state->dpll_hw_state.cx0pll);
>
> @@ -2143,6 +2165,9 @@ static int intel_c10pll_calc_state(struct
> intel_crtc_state *crtc_state,
> crtc_state->dpll_hw_state.cx0pll.use_c10 = true;
> crtc_state->dpll_hw_state.cx0pll.lane_count = crtc_state->lane_count;
>
> + drm_WARN_ON(display->drm,
> + is_dp !=
> c10pll_state_is_dp(&crtc_state->dpll_hw_state.cx0pll.c10));
> +
> return 0;
> }
>
> @@ -2643,6 +2668,7 @@ static int intel_c20pll_calc_state_from_table(struct
> intel_crtc_state *crtc_stat
> static int intel_c20pll_calc_state(struct intel_crtc_state *crtc_state,
> struct intel_encoder *encoder)
> {
> + struct intel_display *display = to_intel_display(encoder);
> bool is_dp = intel_crtc_has_dp_encoder(crtc_state);
> int err = -ENOENT;
>
> @@ -2663,6 +2689,9 @@ static int intel_c20pll_calc_state(struct
> intel_crtc_state *crtc_state,
> intel_c20_calc_vdr_params(&crtc_state->dpll_hw_state.cx0pll.c20.vdr,
> is_dp, crtc_state->port_clock);
>
> + drm_WARN_ON(display->drm,
> + is_dp !=
> c20pll_state_is_dp(&crtc_state->dpll_hw_state.cx0pll.c20));
> +
> return 0;
> }
>
> @@ -2929,10 +2958,11 @@ static void intel_c20_pll_program(struct
> intel_display *display,
>
> static void intel_program_port_clock_ctl(struct intel_encoder *encoder,
> const struct intel_cx0pll_state
> *pll_state,
> - bool is_dp, int port_clock,
> + int port_clock,
> bool lane_reversal)
> {
> struct intel_display *display = to_intel_display(encoder);
> + bool is_dp = cx0pll_state_is_dp(pll_state);
> u32 val = 0;
>
> intel_de_rmw(display, XELPDP_PORT_BUF_CTL1(display, encoder->port),
> @@ -3178,7 +3208,7 @@ static u32 intel_cx0_get_pclk_pll_ack(u8 lane_mask)
>
> static void __intel_cx0pll_enable(struct intel_encoder *encoder,
> const struct intel_cx0pll_state *pll_state,
> - bool is_dp, int port_clock)
> + int port_clock)
> {
> struct intel_display *display = to_intel_display(encoder);
> enum phy phy = intel_encoder_to_phy(encoder);
> @@ -3192,7 +3222,7 @@ static void __intel_cx0pll_enable(struct intel_encoder
> *encoder,
> * 1. Program PORT_CLOCK_CTL REGISTER to configure
> * clock muxes, gating and SSC
> */
> - intel_program_port_clock_ctl(encoder, pll_state, is_dp, port_clock,
> lane_reversal);
> + intel_program_port_clock_ctl(encoder, pll_state, port_clock,
> lane_reversal);
>
> /* 2. Bring PHY out of reset. */
> intel_cx0_phy_lane_reset(encoder, lane_reversal);
> @@ -3262,7 +3292,6 @@ static void intel_cx0pll_enable(struct intel_encoder
> *encoder,
> const struct intel_crtc_state *crtc_state)
> {
> __intel_cx0pll_enable(encoder, &crtc_state->dpll_hw_state.cx0pll,
> - intel_crtc_has_dp_encoder(crtc_state),
> crtc_state->port_clock);
> }
>
> @@ -3818,7 +3847,7 @@ void intel_cx0_pll_power_save_wa(struct intel_display
> *display)
> "[ENCODER:%d:%s] Applying power saving workaround
> on disabled PLL\n",
> encoder->base.base.id, encoder->base.name);
>
> - __intel_cx0pll_enable(encoder, &pll_state, true, port_clock);
> + __intel_cx0pll_enable(encoder, &pll_state, port_clock);
> intel_cx0pll_disable(encoder);
> }
> }
--
Jani Nikula, Intel
