> Subject: [PATCH v2 08/15] drm/i915/display: Add helper function for fuzzy 
> clock
> check
> 
> The hard coded clock rate stored in the PLL state will be removed by a follow-
> up change. Prepare for that here to calculate instead the clock from the PLL

Maybe fix the sentence up here a bit

> divider values in the PLL state. Since this calculated clock can have a +-1 
> kHz
> difference wrt. the request clock, use a fuzzy check when looking up the
> corresponding table entry.
> 
> v2:
> - Use the stricter +-1 kHz allowed difference.
> - Derive the clock from PLL dividers in intel_cx0pll_enable().
> - Move corresponding fuzzy check for LT PHY PLLs to this patch.
> 
> Signed-off-by: Mika Kahola <[email protected]>
> ---
>  drivers/gpu/drm/i915/display/intel_cx0_phy.c | 26 +++++++++++++++++---
> drivers/gpu/drm/i915/display/intel_cx0_phy.h |  1 +
> drivers/gpu/drm/i915/display/intel_lt_phy.c  |  6 ++++-
>  3 files changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> index ab402bd8a125..4d006c14c049 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> @@ -2211,6 +2211,11 @@ static int intel_c20pll_calc_port_clock(const struct
> intel_c20pll_state *pll_sta
>       return vco << tx_rate_mult >> tx_clk_div >> tx_rate;  }
> 
> +bool intel_cx0pll_clock_matches(int clock1, int clock2) {
> +     return abs(clock1 - clock2) <= 1;
> +}
> +
>  /*
>   * TODO: Convert the following to align with intel_c20pll_find_table() and
>   * intel_c20pll_calc_state_from_table().
> @@ -2224,7 +2229,10 @@ static int intel_c10pll_calc_state_from_table(struct
> intel_encoder *encoder,
>       int i;
> 
>       for (i = 0; tables[i].name; i++) {
> -             if (port_clock == tables[i].clock_rate) {
> +             int clock = intel_c10pll_calc_port_clock(tables[i].c10);
> +
> +             drm_WARN_ON(display->drm,
> !intel_cx0pll_clock_matches(clock, tables[i].clock_rate));
> +             if (intel_cx0pll_clock_matches(port_clock, clock)) {
>                       pll_state->c10 = *tables[i].c10;
>                       intel_cx0pll_update_ssc(encoder, pll_state, is_dp);
>                       intel_c10pll_update_pll(encoder, pll_state); @@ -
> 2710,6 +2718,7 @@ static const struct intel_cx0pll_params *
> intel_c20_pll_find_table(const struct intel_crtc_state *crtc_state,
>                        struct intel_encoder *encoder)
>  {
> +     struct intel_display *display = to_intel_display(crtc_state);
>       const struct intel_cx0pll_params *tables;
>       int i;
> 
> @@ -2717,9 +2726,13 @@ intel_c20_pll_find_table(const struct
> intel_crtc_state *crtc_state,
>       if (!tables)
>               return NULL;
> 
> -     for (i = 0; tables[i].name; i++)
> -             if (crtc_state->port_clock == tables[i].clock_rate)
> +     for (i = 0; tables[i].name; i++) {
> +             int clock = intel_c20pll_calc_port_clock(tables[i].c20);
> +
> +             drm_WARN_ON(display->drm,
> !intel_cx0pll_clock_matches(clock, tables[i].clock_rate));
> +             if (intel_cx0pll_clock_matches(crtc_state->port_clock, clock))
>                       return &tables[i];
> +     }
> 
>       return NULL;
>  }
> @@ -3252,7 +3265,6 @@ 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)  {
> -     int port_clock = pll_state->use_c10 ? pll_state->c10.clock : pll_state-
> >c20.clock;
>       struct intel_display *display = to_intel_display(encoder);
>       enum phy phy = intel_encoder_to_phy(encoder);
>       struct intel_digital_port *dig_port = enc_to_dig_port(encoder); @@ -
> 3260,6 +3272,12 @@ static void intel_cx0pll_enable(struct intel_encoder
> *encoder,
>       u8 maxpclk_lane = lane_reversal ? INTEL_CX0_LANE1 :
>                                         INTEL_CX0_LANE0;
>       struct ref_tracker *wakeref =
> intel_cx0_phy_transaction_begin(encoder);
> +     int port_clock;
> +
> +     if (pll_state->use_c10)
> +             port_clock = intel_c10pll_calc_port_clock(&pll_state->c10);
> +     else
> +             port_clock = intel_c20pll_calc_port_clock(&pll_state->c20);
> 
>       /*
>        * Lane reversal is never used in DP-alt mode, in that case the diff 
> --git
> a/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> b/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> index 9f10113e2d18..3d9c580eb562 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> @@ -43,6 +43,7 @@ void intel_mtl_tbt_pll_enable_clock(struct intel_encoder
> *encoder,
>                                   int port_clock);
>  void intel_mtl_tbt_pll_disable_clock(struct intel_encoder *encoder);
> 
> +bool intel_cx0pll_clock_matches(int clock1, int clock2);
>  int intel_cx0pll_calc_state(const struct intel_crtc_state *crtc_state,
>                           struct intel_encoder *encoder,
>                           struct intel_dpll_hw_state *hw_state); diff --git
> a/drivers/gpu/drm/i915/display/intel_lt_phy.c
> b/drivers/gpu/drm/i915/display/intel_lt_phy.c
> index 64e223f35fdf..e33f6f48a6ce 100644
> --- a/drivers/gpu/drm/i915/display/intel_lt_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_lt_phy.c
> @@ -1792,6 +1792,7 @@ int
>  intel_lt_phy_pll_calc_state(struct intel_crtc_state *crtc_state,
>                           struct intel_encoder *encoder)
>  {
> +     struct intel_display *display = to_intel_display(crtc_state);
>       const struct intel_lt_phy_pll_params *tables;
>       int i;
> 
> @@ -1800,7 +1801,10 @@ intel_lt_phy_pll_calc_state(struct intel_crtc_state
> *crtc_state,
>               return -EINVAL;
> 
>       for (i = 0; tables[i].name; i++) {
> -             if (crtc_state->port_clock == tables[i].clock_rate) {
> +             int clock = intel_lt_phy_calc_port_clock(display, 
> tables[i].state);
> +
> +             drm_WARN_ON(display->drm,
> !intel_cx0pll_clock_matches(clock, tables[i].clock_rate));
> +             if (intel_cx0pll_clock_matches(crtc_state->port_clock, clock)) {

Having cx0pll_clock_matches get called in lt phy seems a bit weird
Was thinking if this function belong in Intel_dpll.c intel_dpll_clock_matches() 
sounds
Better and generalized

Regards,
Suraj Kandpal

>                       crtc_state->dpll_hw_state.ltpll = *tables[i].state;
>                       if (intel_crtc_has_dp_encoder(crtc_state)) {
>                               if (intel_crtc_has_type(crtc_state,
> INTEL_OUTPUT_EDP))
> --
> 2.34.1

Reply via email to