On Wed, 05 Jul 2023, Ville Syrjala <[email protected]> wrote: > From: Ville Syrjälä <[email protected]> > > 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ä <[email protected]>
It's entirely possible there are corner cases I missed, but overall makes sense. Reviewed-by: Jani Nikula <[email protected]> > --- > 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
