On Thu, 22 Sep 2022, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> We always allocate two DPLLs (TC and TBT) for TC ports. This
> is because we can't know ahead of time wherher we need to put
> the PHY into DP-Alt or TBT mode.
>
> However during readout we can obviously only read out the state
> of the DPLL that the port is actually using. Thus the state after
> readout will not have both DPLLs populated.
>
> We run into problems if during readout the TC port is in DP-Alt
> mode, but we then perform a modeset on the port without going
> through the full .compute_config() machinery, and during said
> modeset the port cannot be switched back into DP-Alt mode and
> we need to take the TBT fallback path. Such a modeset can
> happen eg. due to cdclk reprogramming.
>
> This wasn't a problem earlier because we did all the DPLL
> calculations much later in the modeset. So even if flagged
> a modeset very late we'd still have gone through the DPLL
> calculations. But now all the DPLL calculations happen much
> earlier and so we need to deal with it, or else we'll attempt
> a modeset without a DPLL.
>
> To guarantee that we always have both DPLLs fully cal/ulated
> for TC ports force a full modeset computation during the
> initial commit.
>
> Reported-by: Lee Shawn C <shawn.c.lee@xxxxxxxxx>
> Fixes: b000abd3b3d2 ("drm/i915: Do .crtc_compute_clock() earlier")
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 643832d55c28..6278b8ea5bf1 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3600,10 +3600,21 @@ static void intel_ddi_sync_state(struct intel_encoder *encoder,
> static bool intel_ddi_initial_fastset_check(struct intel_encoder *encoder,
> struct intel_crtc_state *crtc_state)
> {
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> + enum phy phy = intel_port_to_phy(i915, encoder->port);
> + bool ret = true;
> +
> + if (intel_phy_is_tc(i915, phy)) {
> + drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s] Forcing full modeset to compute TC port DPLLs\n",
> + encoder->base.base.id, encoder->base.name);
> + crtc_state->uapi.mode_changed = true;
> + ret = false;
> + }
> +
> if (intel_crtc_has_dp_encoder(crtc_state))
> - return intel_dp_initial_fastset_check(encoder, crtc_state);
> + ret &= intel_dp_initial_fastset_check(encoder, crtc_state);
I think there have been static checker warnings about mixing bitwise and
boolean AND like this. I guess there's implicit type conversion to int
and back to bool.
BR,
Jani.
>
> - return true;
> + return ret;
> }
>
> static enum intel_output_type
--
Jani Nikula, Intel Open Source Graphics Center
