On Wed, 29 Jan 2025, Imre Deak <imre.d...@intel.com> wrote:
> From: Imre Deak <imre.d...@gmail.com>
>
> Add the missing PHY lane stagger delay programming for ICL-ADL
> platforms on TypeC DP outputs.
>
> Bspec: 7534, 49533
> Signed-off-by: Imre Deak <imre.d...@gmail.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 18 ++++++++++++++++++
>  drivers/gpu/drm/i915/i915_reg.h          |  3 +++
>  2 files changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c 
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 76e8296cb134b..6192c0d3c87a5 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -328,9 +328,21 @@ static u32 ddi_buf_phy_link_rate(int port_clock)
>       }
>  }
>  
> +static int dp_phy_lane_stagger_delay(int port_clock)
> +{
> +     /*
> +      * Return the number of link symbols per 100 ns:
> +      * port_clock (10 kHz) -> bits    / 100 us
> +      * / symbol_size       -> symbols / 100 us
> +      * / 1000              -> symbols / 100 ns
> +      */
> +     return DIV_ROUND_UP(port_clock, intel_dp_link_symbol_size(port_clock) * 
> 1000);

Okay, this checks out, but it was incredibly difficult (for me) to
follow, even with the comment. The meaning of 100 ns is also not
documented i.e. we want a lane stagger delay of 100 ns or greater, and
we must express this in terms of symbols in the register.

Reviewed-by: Jani Nikula <jani.nik...@intel.com>

> +}
> +
>  static void intel_ddi_init_dp_buf_reg(struct intel_encoder *encoder,
>                                     const struct intel_crtc_state *crtc_state)
>  {
> +     struct intel_display *display = to_intel_display(encoder);
>       struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>       struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>       struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> @@ -356,6 +368,12 @@ static void intel_ddi_init_dp_buf_reg(struct 
> intel_encoder *encoder,
>               if (!intel_tc_port_in_tbt_alt_mode(dig_port))
>                       intel_dp->DP |= DDI_BUF_CTL_TC_PHY_OWNERSHIP;
>       }
> +
> +     if (IS_DISPLAY_VER(display, 11, 13) && intel_encoder_is_tc(encoder)) {
> +             int delay = dp_phy_lane_stagger_delay(crtc_state->port_clock);
> +
> +             intel_dp->DP |= DDI_BUF_LANE_STAGGER_DELAY(delay);
> +     }
>  }
>  
>  static int icl_calc_tbt_pll_link(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 04e47d0a8ab92..7fe4e71fc08ec 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3632,6 +3632,9 @@ enum skl_power_gate {
>  #define  DDI_BUF_PORT_DATA_20BIT             
> REG_FIELD_PREP(DDI_BUF_PORT_DATA_MASK, 1)
>  #define  DDI_BUF_PORT_DATA_40BIT             
> REG_FIELD_PREP(DDI_BUF_PORT_DATA_MASK, 2)
>  #define  DDI_BUF_PORT_REVERSAL                       (1 << 16)
> +#define  DDI_BUF_LANE_STAGGER_DELAY_MASK     REG_GENMASK(15, 8)
> +#define  DDI_BUF_LANE_STAGGER_DELAY(symbols) 
> REG_FIELD_PREP(DDI_BUF_LANE_STAGGER_DELAY_MASK, \
> +                                                            symbols)
>  #define  DDI_BUF_IS_IDLE                     (1 << 7)
>  #define  DDI_BUF_CTL_TC_PHY_OWNERSHIP                REG_BIT(6)
>  #define  DDI_A_4_LANES                               (1 << 4)

-- 
Jani Nikula, Intel

Reply via email to