> Subject: [PATCH v2 30/32] drm/i915/cx0: Get encoder configuration for C10
> and C20 PHY PLLs
>
> For DDI initialization get configuration for C10 and C20 chips.
>
> v2: Getting configuration either for a C10 or on the PTL port B
> eDP on TypeC PHY case for a C20 PHY PLL. Hence refer to this
> case as "non_tc_phy" instead of "c10phy".
>
> Signed-off-by: Imre Deak <[email protected]>
> Signed-off-by: Mika Kahola <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 81 ++++++++++++++++++++++--
> 1 file changed, 75 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index be25a6fdd491..689bd3224919 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4273,6 +4273,77 @@ static void mtl_ddi_get_config(struct intel_encoder
> *encoder,
> intel_ddi_get_config(encoder, crtc_state); }
>
> +static bool icl_ddi_tc_pll_is_tbt(const struct intel_dpll *pll) {
> + return pll->info->id == DPLL_ID_ICL_TBTPLL; }
> +
> +static void mtl_ddi_cx0_get_config(struct intel_encoder *encoder,
> + struct intel_crtc_state *crtc_state,
> + enum icl_port_dpll_id port_dpll_id,
> + enum intel_dpll_id pll_id)
> +{
> + struct intel_display *display = to_intel_display(encoder);
> + struct icl_port_dpll *port_dpll;
> + struct intel_dpll *pll;
> + bool pll_active;
> +
> + port_dpll = &crtc_state->icl_port_dplls[port_dpll_id];
> + pll = intel_get_dpll_by_id(display, pll_id);
> +
> + if (drm_WARN_ON(display->drm, !pll))
> + return;
> +
> + port_dpll->pll = pll;
> + pll_active = intel_dpll_get_hw_state(display, pll,
> &port_dpll->hw_state);
> + drm_WARN_ON(display->drm, !pll_active);
> +
> + icl_set_active_port_dpll(crtc_state, port_dpll_id);
> +
> + if (icl_ddi_tc_pll_is_tbt(crtc_state->intel_dpll))
> + crtc_state->port_clock = intel_mtl_tbt_calc_port_clock(encoder);
> + else
> + crtc_state->port_clock = intel_dpll_get_freq(display,
> crtc_state-
> >intel_dpll,
> + &crtc_state-
> >dpll_hw_state);
> +
> + intel_ddi_get_config(encoder, crtc_state); }
> +
> +/*
> + * Get the configuration for either a port using a C10 PHY PLL, or in
> +the case of
> + * the PTL port B eDP on TypeC PHY case the configuration of a port
> +using a C20
> + * PHY PLL.
> + */
> +static void mtl_ddi_non_tc_phy_get_config(struct intel_encoder *encoder,
> + struct intel_crtc_state
> *crtc_state) {
> + struct intel_display *display = to_intel_display(encoder);
> +
> + /* TODO: Remove when the PLL manager is in place. */
Is the comment needed anymore
> + mtl_ddi_get_config(encoder, crtc_state);
> + return;
Why the early return code after this point then serves no purpose.
> +
> + mtl_ddi_cx0_get_config(encoder, crtc_state, ICL_PORT_DPLL_DEFAULT,
> + mtl_port_to_pll_id(display, encoder->port)); }
Have the pll id in its own variable.
> +
> +static void mtl_ddi_tc_phy_get_config(struct intel_encoder *encoder,
> + struct intel_crtc_state *crtc_state) {
> + struct intel_display *display = to_intel_display(encoder);
> +
> + /* TODO: Remove when the PLL manager is in place. */
No need for this comment
> + mtl_ddi_get_config(encoder, crtc_state);
> + return;
Same question why the early return ?
> +
> + if (intel_tc_port_in_tbt_alt_mode(enc_to_dig_port(encoder)))
> + mtl_ddi_cx0_get_config(encoder, crtc_state,
> ICL_PORT_DPLL_DEFAULT,
> + DPLL_ID_ICL_TBTPLL);
> + else
> + mtl_ddi_cx0_get_config(encoder, crtc_state,
> ICL_PORT_DPLL_MG_PHY,
> + mtl_port_to_pll_id(display, encoder-
> >port)); }
You can have the pll id as its one variable
In fact you can call mtl_ddi_cx0_get_config just once if you have both port and
pll id variables assigned
After checking if intel_tc_port_in_tbt_alt_mode
Regards,
Suraj Kandpal
> +
> static void dg2_ddi_get_config(struct intel_encoder *encoder,
> struct intel_crtc_state *crtc_state) { @@ -
> 4310,11 +4381,6 @@ static void icl_ddi_combo_get_config(struct
> intel_encoder *encoder,
> intel_ddi_get_config(encoder, crtc_state); }
>
> -static bool icl_ddi_tc_pll_is_tbt(const struct intel_dpll *pll) -{
> - return pll->info->id == DPLL_ID_ICL_TBTPLL;
> -}
> -
> static enum icl_port_dpll_id
> icl_ddi_tc_port_pll_type(struct intel_encoder *encoder,
> const struct intel_crtc_state *crtc_state) @@ -5260,7
> +5326,10 @@ void intel_ddi_init(struct intel_display *display,
> encoder->enable_clock = intel_mtl_pll_enable_clock;
> encoder->disable_clock = intel_mtl_pll_disable_clock;
> encoder->port_pll_type = intel_mtl_port_pll_type;
> - encoder->get_config = mtl_ddi_get_config;
> + if (intel_encoder_is_tc(encoder))
> + encoder->get_config = mtl_ddi_tc_phy_get_config;
> + else
> + encoder->get_config = mtl_ddi_non_tc_phy_get_config;
> } else if (display->platform.dg2) {
> encoder->enable_clock = intel_mpllb_enable;
> encoder->disable_clock = intel_mpllb_disable;
> --
> 2.34.1
