On Thu, Dec 19, 2024 at 11:34:02PM +0200, Jani Nikula wrote:
> We'll want to distinguish 128b/132b SST and MST modes at state
> readout. There's a catch, though. From the hardware perspective,
> 128b/132b SST and MST programming are pretty much the same. And we can't
> really ask the sink at this point.
>
> If we have more than one transcoder in 128b/132b mode associated with
> the port, we can safely assume it's MST. But for MST with only a single
> stream enabled, we are pretty much out of luck. Let's fall back to
> looking at the software state, i.e. intel_dp->is_mst. It should be fine
> for the state checker, but for hardware takeover at probe, we'll have to
> trust the GOP has only enabled SST.
>
> TODO: Not sure how this *or* our current code handles 128b/132b enabled
> by GOP.
>
> Cc: Imre Deak <imre.deak@xxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 29 +++++++++++++++++++-----
> 1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 7b739b9c5a06..04118f2eea94 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -786,7 +786,7 @@ static void intel_ddi_get_encoder_pipes(struct intel_encoder *encoder,
> intel_wakeref_t wakeref;
> enum pipe p;
> u32 tmp;
> - u8 mst_pipe_mask;
> + u8 mst_pipe_mask = 0, dp128b132b_pipe_mask = 0;
>
> *pipe_mask = 0;
> *is_dp_mst = false;
> @@ -823,7 +823,6 @@ static void intel_ddi_get_encoder_pipes(struct intel_encoder *encoder,
> goto out;
> }
>
> - mst_pipe_mask = 0;
> for_each_pipe(dev_priv, p) {
> enum transcoder cpu_transcoder = (enum transcoder)p;
> u32 port_mask, ddi_select, ddi_mode;
> @@ -852,9 +851,10 @@ static void intel_ddi_get_encoder_pipes(struct intel_encoder *encoder,
>
> ddi_mode = tmp & TRANS_DDI_MODE_SELECT_MASK;
>
> - if (ddi_mode == TRANS_DDI_MODE_SELECT_DP_MST ||
> - (ddi_mode == TRANS_DDI_MODE_SELECT_FDI_OR_128B132B && HAS_DP20(display)))
> + if (ddi_mode == TRANS_DDI_MODE_SELECT_DP_MST)
> mst_pipe_mask |= BIT(p);
> + else if (ddi_mode == TRANS_DDI_MODE_SELECT_FDI_OR_128B132B && HAS_DP20(display))
> + dp128b132b_pipe_mask |= BIT(p);
>
> *pipe_mask |= BIT(p);
> }
> @@ -864,6 +864,23 @@ static void intel_ddi_get_encoder_pipes(struct intel_encoder *encoder,
> "No pipe for [ENCODER:%d:%s] found\n",
> encoder->base.base.id, encoder->base.name);
>
> + if (!mst_pipe_mask && dp128b132b_pipe_mask) {
> + struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> +
8b10b and 128b132b can't be mixed on one link, so the above could make
this clear (and more robust) by
if (dp128b132b_pipe_mask) {
if (WARN(mst_pipe_mask))
mst_pipe_mask = 0;
In any case the patch is correct, so either way:
Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx>
> + /*
> + * If we don't have 8b/10b MST, but have more than one
> + * transcoder in 128b/132b mode, we know it must be 128b/132b
> + * MST.
> + *
> + * Otherwise, we fall back to checking the current MST
> + * state. It's not accurate for hardware takeover at probe, but
> + * we don't expect MST to have been enabled at that point, and
> + * can assume it's SST.
> + */
> + if (hweight8(dp128b132b_pipe_mask) > 1 || intel_dp->is_mst)
> + mst_pipe_mask = dp128b132b_pipe_mask;
> + }
> +
> if (!mst_pipe_mask && hweight8(*pipe_mask) > 1) {
> drm_dbg_kms(&dev_priv->drm,
> "Multiple pipes for [ENCODER:%d:%s] (pipe_mask %02x)\n",
> @@ -874,9 +891,9 @@ static void intel_ddi_get_encoder_pipes(struct intel_encoder *encoder,
>
> if (mst_pipe_mask && mst_pipe_mask != *pipe_mask)
> drm_dbg_kms(&dev_priv->drm,
> - "Conflicting MST and non-MST state for [ENCODER:%d:%s] (pipe_mask %02x mst_pipe_mask %02x)\n",
> + "Conflicting MST and non-MST state for [ENCODER:%d:%s] (pipe masks: all %02x, MST %02x, 128b/132b %02x)\n",
> encoder->base.base.id, encoder->base.name,
> - *pipe_mask, mst_pipe_mask);
> + *pipe_mask, mst_pipe_mask, dp128b132b_pipe_mask);
> else
> *is_dp_mst = mst_pipe_mask;
>
> --
> 2.39.5
>
