On Wed, 05 Jul 2023, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Call *_calc_dpll_params() even in cases where the encoder has
> computed the DPLL params for us.
>
> The SDVO TV output code doesn't populate crtc_state->dpll.dot
> leading to the dotclock getting calculated as zero, and that
> leads to all kinds of real problems. The g4x DP code also
> doesn't populate the derived dividers nor .vco, which could
> also create some confusion.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
It's entirely possible there are corner cases I missed, but overall
makes sense.
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_dpll.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll.c b/drivers/gpu/drm/i915/display/intel_dpll.c
> index 71bfeea4cef2..2255ad651486 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll.c
> @@ -1182,6 +1182,8 @@ static int ilk_crtc_compute_clock(struct intel_atomic_state *state,
> refclk, NULL, &crtc_state->dpll))
> return -EINVAL;
>
> + i9xx_calc_dpll_params(refclk, &crtc_state->dpll);
> +
> ilk_compute_dpll(crtc_state, &crtc_state->dpll,
> &crtc_state->dpll);
>
> @@ -1256,6 +1258,8 @@ static int chv_crtc_compute_clock(struct intel_atomic_state *state,
> refclk, NULL, &crtc_state->dpll))
> return -EINVAL;
>
> + chv_calc_dpll_params(refclk, &crtc_state->dpll);
> +
> chv_compute_dpll(crtc_state);
>
> /* FIXME this is a mess */
> @@ -1278,9 +1282,10 @@ static int vlv_crtc_compute_clock(struct intel_atomic_state *state,
>
> if (!crtc_state->clock_set &&
> !vlv_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
> - refclk, NULL, &crtc_state->dpll)) {
> + refclk, NULL, &crtc_state->dpll))
> return -EINVAL;
> - }
> +
> + vlv_calc_dpll_params(refclk, &crtc_state->dpll);
>
> vlv_compute_dpll(crtc_state);
>
> @@ -1330,6 +1335,8 @@ static int g4x_crtc_compute_clock(struct intel_atomic_state *state,
> refclk, NULL, &crtc_state->dpll))
> return -EINVAL;
>
> + i9xx_calc_dpll_params(refclk, &crtc_state->dpll);
> +
> i9xx_compute_dpll(crtc_state, &crtc_state->dpll,
> &crtc_state->dpll);
>
> @@ -1368,6 +1375,8 @@ static int pnv_crtc_compute_clock(struct intel_atomic_state *state,
> refclk, NULL, &crtc_state->dpll))
> return -EINVAL;
>
> + pnv_calc_dpll_params(refclk, &crtc_state->dpll);
> +
> i9xx_compute_dpll(crtc_state, &crtc_state->dpll,
> &crtc_state->dpll);
>
> @@ -1404,6 +1413,8 @@ static int i9xx_crtc_compute_clock(struct intel_atomic_state *state,
> refclk, NULL, &crtc_state->dpll))
> return -EINVAL;
>
> + i9xx_calc_dpll_params(refclk, &crtc_state->dpll);
> +
> i9xx_compute_dpll(crtc_state, &crtc_state->dpll,
> &crtc_state->dpll);
>
> @@ -1444,6 +1455,8 @@ static int i8xx_crtc_compute_clock(struct intel_atomic_state *state,
> refclk, NULL, &crtc_state->dpll))
> return -EINVAL;
>
> + i9xx_calc_dpll_params(refclk, &crtc_state->dpll);
> +
> i8xx_compute_dpll(crtc_state, &crtc_state->dpll,
> &crtc_state->dpll);
--
Jani Nikula, Intel Open Source Graphics Center
