> -----Original Message-----
> From: Intel-gfx <[email protected]> On Behalf Of Mika
> Kahola
> Sent: Tuesday, March 10, 2026 9:08 PM
> To: [email protected]; [email protected]
> Cc: Kahola, Mika <[email protected]>
> Subject: [PATCH v3 04/24] drm/i915/lt_phy: Refactor LT PHY PLL handling to use
> explicit PLL state
> 
> The LT PHY implementation currently pulls PLL and port_clock information
> directly from the CRTC state. This ties the PHY programming logic too tightly 
> to
> the CRTC state and makes it harder to clearly express the PHY’s own PLL
> configuration.
> 
> Introduce an explicit "struct intel_lt_phy_pll_state" argument for the PHY
> functions and update callers accordingly.
> 
> No functional change is intended — this is a preparatory cleanup for to bring 
> LT
> PHY PLL handling as part of PLL framework.
> 
> v2:  DP, HDMI 2.0, and HDMI FRL modes are port of the VDR configuration 0
>     register. These modes are defined by bits 2:0. Decode these to
>     differentiate DP and HDMI modes when programming PLL's. (Imre, Suraj)
> 
> BSpec: 744921

No new line needed here

> 
> Signed-off-by: Mika Kahola <[email protected]>
> ---
>  drivers/gpu/drm/i915/display/intel_lt_phy.c | 67 ++++++++++++++-------
>  1 file changed, 46 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_lt_phy.c
> b/drivers/gpu/drm/i915/display/intel_lt_phy.c
> index 8fe61cfdb706..76acffb2e840 100644
> --- a/drivers/gpu/drm/i915/display/intel_lt_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_lt_phy.c
> @@ -32,6 +32,7 @@
>                                        INTEL_LT_PHY_LANE0)
>  #define MODE_DP                              3
>  #define MODE_HDMI_20                 4
> +#define MODE_HDMI_FRL                        5
>  #define Q32_TO_INT(x)        ((x) >> 32)
>  #define Q32_TO_FRAC(x)       ((x) & 0xFFFFFFFF)
>  #define DCO_MIN_FREQ_MHZ     11850
> @@ -1176,9 +1177,30 @@ intel_lt_phy_lane_reset(struct intel_encoder
> *encoder,
>       intel_de_rmw(display, XELPDP_PORT_BUF_CTL2(display, port),
> lane_phy_pulse_status, 0);  }
> 
> +static bool intel_lt_phy_is_hdmi(const struct intel_lt_phy_pll_state
> +*ltpll) {
> +     u8 mode = REG_FIELD_GET8(LT_PHY_VDR_MODE_ENCODING_MASK,
> +ltpll->config[0]);
> +
> +     if (mode == MODE_HDMI_20 || mode == MODE_HDMI_FRL)
> +             return true;
> +
> +     return false;
> +}
> +
> +static bool intel_lt_phy_is_dp(const struct intel_lt_phy_pll_state
> +*ltpll) {
> +     u8 mode = REG_FIELD_GET8(LT_PHY_VDR_MODE_ENCODING_MASK,
> +ltpll->config[0]);
> +
> +     if (mode == MODE_DP)
> +             return true;
> +
> +     return false;
> +}
> +
>  static void
>  intel_lt_phy_program_port_clock_ctl(struct intel_encoder *encoder,
> -                                 const struct intel_crtc_state *crtc_state,
> +                                 const struct intel_lt_phy_pll_state *ltpll,
> +                                 int port_clock,
>                                   bool lane_reversal)
>  {
>       struct intel_display *display = to_intel_display(encoder); @@ -1195,17
> +1217,16 @@ intel_lt_phy_program_port_clock_ctl(struct intel_encoder
> *encoder,
>        * but since the register bits still remain the same we use
>        * the same definition
>        */
> -     if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI) &&
> -         intel_hdmi_is_frl(crtc_state->port_clock))
> +     if (intel_lt_phy_is_hdmi(ltpll) && intel_hdmi_is_frl(port_clock))
>               val |= XELPDP_DDI_CLOCK_SELECT_PREP(display,
> XELPDP_DDI_CLOCK_SELECT_DIV18CLK);
>       else
>               val |= XELPDP_DDI_CLOCK_SELECT_PREP(display,
> XELPDP_DDI_CLOCK_SELECT_MAXPCLK);
> 
>        /* DP2.0 10G and 20G rates enable MPLLA*/
> -     if (crtc_state->port_clock == 1000000 || crtc_state->port_clock ==
> 2000000)
> +     if (port_clock == 1000000 || port_clock == 2000000)
>               val |= XELPDP_SSC_ENABLE_PLLA;
>       else
> -             val |= crtc_state->dpll_hw_state.ltpll.ssc_enabled ?
> XELPDP_SSC_ENABLE_PLLB : 0;
> +             val |= ltpll->ssc_enabled ? XELPDP_SSC_ENABLE_PLLB : 0;
> 
>       intel_de_rmw(display, XELPDP_PORT_CLOCK_CTL(display, encoder-
> >port),
>                    XELPDP_LANE1_PHY_CLOCK_SELECT |
> XELPDP_FORWARD_CLOCK_UNGATE | @@ -1248,10 +1269,12 @@ static u32
> intel_lt_phy_get_dp_clock(u8 rate)
> 
>  static bool
>  intel_lt_phy_config_changed(struct intel_encoder *encoder,
> -                         const struct intel_crtc_state *crtc_state)
> +                         const struct intel_lt_phy_pll_state *ltpll)

Why are we changing this?

>  {
> +     struct intel_display *display = to_intel_display(encoder);
>       u8 val, rate;
>       u32 clock;
> +     u32 port_clock = intel_lt_phy_calc_port_clock(display, ltpll);

No need to have this recalculated again
If we really want to change the arguments to send ltpll state
Then I recommend sending the crtc_state->port_clock as an argument

Otherwise rest looks good to me here

Regards,
Suraj Kandpal
 
> 
>       val = intel_lt_phy_read(encoder, INTEL_LT_PHY_LANE0,
>                               LT_PHY_VDR_0_CONFIG);
> @@ -1262,9 +1285,9 @@ intel_lt_phy_config_changed(struct intel_encoder
> *encoder,
>        * using 1.62 Gbps clock since PHY PLL defaults to that
>        * otherwise we always need to reconfigure it.
>        */
> -     if (intel_crtc_has_dp_encoder(crtc_state)) {
> +     if (intel_lt_phy_is_dp(ltpll)) {
>               clock = intel_lt_phy_get_dp_clock(rate);
> -             if (crtc_state->port_clock == 1620000 && crtc_state-
> >port_clock == clock)
> +             if (port_clock == 1620000 && port_clock == clock)
>                       return false;
>       }
> 
> @@ -1759,41 +1782,41 @@ intel_lt_phy_pll_calc_state(struct intel_crtc_state
> *crtc_state,
> 
>  static void
>  intel_lt_phy_program_pll(struct intel_encoder *encoder,
> -                      const struct intel_crtc_state *crtc_state)
> +                      const struct intel_lt_phy_pll_state *ltpll)
>  {
>       u8 owned_lane_mask = intel_lt_phy_get_owned_lane_mask(encoder);
>       int i, j, k;
> 
>       intel_lt_phy_write(encoder, owned_lane_mask,
> LT_PHY_VDR_0_CONFIG,
> -                        crtc_state->dpll_hw_state.ltpll.config[0],
> MB_WRITE_COMMITTED);
> +                        ltpll->config[0], MB_WRITE_COMMITTED);
>       intel_lt_phy_write(encoder, INTEL_LT_PHY_LANE0,
> LT_PHY_VDR_1_CONFIG,
> -                        crtc_state->dpll_hw_state.ltpll.config[1],
> MB_WRITE_COMMITTED);
> +                        ltpll->config[1], MB_WRITE_COMMITTED);
>       intel_lt_phy_write(encoder, owned_lane_mask,
> LT_PHY_VDR_2_CONFIG,
> -                        crtc_state->dpll_hw_state.ltpll.config[2],
> MB_WRITE_COMMITTED);
> +                        ltpll->config[2], MB_WRITE_COMMITTED);
> 
>       for (i = 0; i <= 12; i++) {
>               intel_lt_phy_write(encoder, INTEL_LT_PHY_LANE0,
> LT_PHY_VDR_X_ADDR_MSB(i),
> -                                crtc_state->dpll_hw_state.ltpll.addr_msb[i],
> +                                ltpll->addr_msb[i],
>                                  MB_WRITE_COMMITTED);
>               intel_lt_phy_write(encoder, INTEL_LT_PHY_LANE0,
> LT_PHY_VDR_X_ADDR_LSB(i),
> -                                crtc_state->dpll_hw_state.ltpll.addr_lsb[i],
> +                                ltpll->addr_lsb[i],
>                                  MB_WRITE_COMMITTED);
> 
>               for (j = 3, k = 0; j >= 0; j--, k++)
>                       intel_lt_phy_write(encoder, INTEL_LT_PHY_LANE0,
>                                          LT_PHY_VDR_X_DATAY(i, j),
> -                                        crtc_state-
> >dpll_hw_state.ltpll.data[i][k],
> +                                        ltpll->data[i][k],
>                                          MB_WRITE_COMMITTED);
>       }
>  }
> 
>  static void
>  intel_lt_phy_enable_disable_tx(struct intel_encoder *encoder,
> -                            const struct intel_crtc_state *crtc_state)
> +                            const struct intel_lt_phy_pll_state *ltpll,
> +                            u8 lane_count)
>  {
>       struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
>       bool lane_reversal = dig_port->lane_reversal;
> -     u8 lane_count = crtc_state->lane_count;
>       bool is_dp_alt =
>               intel_tc_port_in_dp_alt_mode(dig_port);
>       enum intel_tc_pin_assignment tc_pin =
> @@ -1895,7 +1918,8 @@ void intel_lt_phy_pll_enable(struct intel_encoder
> *encoder,
>       intel_lt_phy_lane_reset(encoder, crtc_state->lane_count);
> 
>       /* 2. Program PORT_CLOCK_CTL register to configure clock muxes,
> gating, and SSC. */
> -     intel_lt_phy_program_port_clock_ctl(encoder, crtc_state,
> lane_reversal);
> +     intel_lt_phy_program_port_clock_ctl(encoder, &crtc_state-
> >dpll_hw_state.ltpll,
> +                                         crtc_state->port_clock,
> lane_reversal);
> 
>       /* 3. Change owned PHY lanes power to Ready state. */
>       intel_lt_phy_powerdown_change_sequence(encoder,
> owned_lane_mask, @@ -1905,12 +1929,12 @@ void
> intel_lt_phy_pll_enable(struct intel_encoder *encoder,
>        * 4. Read the PHY message bus VDR register PHY_VDR_0_Config check
> enabled PLL type,
>        * encoded rate and encoded mode.
>        */
> -     if (intel_lt_phy_config_changed(encoder, crtc_state)) {
> +     if (intel_lt_phy_config_changed(encoder,
> +&crtc_state->dpll_hw_state.ltpll)) {
>               /*
>                * 5. Program the PHY internal PLL registers over PHY message
> bus for the desired
>                * frequency and protocol type
>                */
> -             intel_lt_phy_program_pll(encoder, crtc_state);
> +             intel_lt_phy_program_pll(encoder, &crtc_state-
> >dpll_hw_state.ltpll);
> 
>               /* 6. Use the P2P transaction flow */
>               /*
> @@ -2001,7 +2025,8 @@ void intel_lt_phy_pll_enable(struct intel_encoder
> *encoder,
>       intel_lt_phy_powerdown_change_sequence(encoder,
> owned_lane_mask,
>                                              XELPDP_P0_STATE_ACTIVE);
> 
> -     intel_lt_phy_enable_disable_tx(encoder, crtc_state);
> +     intel_lt_phy_enable_disable_tx(encoder, &crtc_state-
> >dpll_hw_state.ltpll,
> +                                    crtc_state->lane_count);
>       intel_lt_phy_transaction_end(encoder, wakeref);  }
> 
> --
> 2.43.0

Reply via email to