> Subject: [CI 04/32] drm/i915/display: Sanitize calculating C20 PLL state from
> tables
> 
> From: Imre Deak <[email protected]>
> 
> A follow up change adds a computation for the C20 PLL VDR state, which is
> common to both the HDMI algorithmic and DP/HDMI table based method.
> To prepare for that streamline the code. The C10 counterpart would benefit
> from the same change, leave that for later adding a TODO comment.
> 
> Signed-off-by: Imre Deak <[email protected]>
> Signed-off-by: Mika Kahola <[email protected]>
> ---
>  drivers/gpu/drm/i915/display/intel_cx0_phy.c | 68 ++++++++++++++------
>  1 file changed, 47 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> index dd4cf335f3ae..0dd367457f93 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> @@ -2077,6 +2077,10 @@ static void intel_c10pll_update_pll(struct
> intel_encoder *encoder,
>               pll_state->c10.pll[i] = 0;
>  }
> 
> +/*
> + * TODO: Convert the following align with intel_c20pll_find_table() and
> + * intel_c20pll_calc_state_from_table().

* " following to align with..."

> + */
>  static int intel_c10pll_calc_state_from_table(struct intel_encoder *encoder,
>                                             const struct intel_c10pll_state *
> const *tables,
>                                             bool is_dp, int port_clock,
> @@ -2330,7 +2334,7 @@ static int
> intel_c20_compute_hdmi_tmds_pll(struct intel_crtc_state *crtc_state)  }
> 
>  static const struct intel_c20pll_state * const * 
> -intel_c20_pll_tables_get(struct
> intel_crtc_state *crtc_state,
> +intel_c20_pll_tables_get(const struct intel_crtc_state *crtc_state,
>                        struct intel_encoder *encoder)
>  {
>       struct intel_display *display = to_intel_display(crtc_state); @@ -
> 2358,35 +2362,57 @@ intel_c20_pll_tables_get(struct intel_crtc_state
> *crtc_state,
>       return NULL;
>  }
> 
> -static int intel_c20pll_calc_state(struct intel_crtc_state *crtc_state,
> -                                struct intel_encoder *encoder)
> +static const struct intel_c20pll_state * intel_c20_pll_find_table(const
> +struct intel_crtc_state *crtc_state,
> +                      struct intel_encoder *encoder)
>  {
>       const struct intel_c20pll_state * const *tables;
>       int i;
> 
> -     crtc_state->dpll_hw_state.cx0pll.use_c10 = false;
> -
> -     /* try computed C20 HDMI tables before using consolidated tables */
> -     if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
> -             if (intel_c20_compute_hdmi_tmds_pll(crtc_state) == 0)
> -                     return 0;
> -     }
> -
>       tables = intel_c20_pll_tables_get(crtc_state, encoder);
>       if (!tables)
> +             return NULL;
> +
> +     for (i = 0; tables[i]; i++)
> +             if (crtc_state->port_clock == tables[i]->clock)
> +                     return tables[i];
> +
> +     return NULL;
> +}
> +
> +static int intel_c20pll_calc_state_from_table(struct intel_crtc_state
> *crtc_state,
> +                                           struct intel_encoder *encoder) {
> +     const struct intel_c20pll_state *table;
> +
> +     table = intel_c20_pll_find_table(crtc_state, encoder);
> +     if (!table)
>               return -EINVAL;
> 
> -     for (i = 0; tables[i]; i++) {
> -             if (crtc_state->port_clock == tables[i]->clock) {
> -                     crtc_state->dpll_hw_state.cx0pll.c20 = *tables[i];
> -                     intel_cx0pll_update_ssc(encoder,
> -                                             &crtc_state-
> >dpll_hw_state.cx0pll,
> -
>       intel_crtc_has_dp_encoder(crtc_state));
> -                     return 0;
> -             }
> -     }
> +     crtc_state->dpll_hw_state.cx0pll.c20 = *table;
> 
> -     return -EINVAL;
> +     intel_cx0pll_update_ssc(encoder, &crtc_state->dpll_hw_state.cx0pll,
> +                             intel_crtc_has_dp_encoder(crtc_state));
> +
> +     return 0;
> +}
> +
> +static int intel_c20pll_calc_state(struct intel_crtc_state *crtc_state,
> +                                struct intel_encoder *encoder)
> +{
> +     int err = -ENOENT;
> +
> +     crtc_state->dpll_hw_state.cx0pll.use_c10 = false;
> +
> +     /* try computed C20 HDMI tables before using consolidated tables */
> +     if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> +             /* TODO: Update SSC state for HDMI as well */
> +             err = intel_c20_compute_hdmi_tmds_pll(crtc_state);
> +
> +     if (err)
> +             err = intel_c20pll_calc_state_from_table(crtc_state, encoder);


So this is something I have been meaning to fix we should really be using the 
HDMI tables already defined
Computing them ourselves, that should be reserved for only when we do not have 
any HDMI table for the said port clock available.
Also if we use the computed tables directly that means we never end up using 
the defined tables.

SO the flow here should be

err = intel_c20pll_calc_state_from_table(crtc_state, encoder);

if (err && intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)))
        err = intel_c20_compute_hdmi_tmds_pll(crtc_state);

something like this.

Regards,
Suraj Kandpal
 
> +
> +     return err;
>  }
> 
>  int intel_cx0pll_calc_state(struct intel_crtc_state *crtc_state,
> --
> 2.34.1

Reply via email to