On Fri, Feb 02, 2024 at 04:05:32PM +0200, Jani Nikula wrote:
> Currently we've split MST capability detection in two places,
> intel_dp_can_mst() and intel_dp_configure_mst(). They check essentially
> the same things.
>
> Move bulk of the work, including logging, to intel_dp_can_mst() and
> rename it intel_dp_mst_detect(). Set intel_dp->is_mst there to avoid
> duplicate work.
This seems confusing. is_mst is supposed to reflect the state
of the topology manager, nothing more.
>
> Rename intel_dp_configure_mst() to intel_dp_mst_configure(), and only
> set the topology manager state there.
>
> The main functional difference is that the DP_MSTM_CAP DPCD register is
> now only read once at detect, unconditionally, and the MST support is
> always logged. Everything else should remain the same.
>
> Cc: Arun R Murthy <arun.r.murthy@xxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 36 ++++++++++---------------
> 1 file changed, 14 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 7af09f2c008d..e0b8ee6bde79 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4004,23 +4004,15 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
> intel_dp->downstream_ports) == 0;
> }
>
> -static bool
> -intel_dp_can_mst(struct intel_dp *intel_dp)
> +static bool intel_dp_mst_detect(struct intel_dp *intel_dp)
> {
> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> + struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> + bool sink_can_mst = drm_dp_read_mst_cap(&intel_dp->aux, intel_dp->dpcd) == DP_MST_CAPABLE;
>
> - return i915->display.params.enable_dp_mst &&
> + intel_dp->is_mst = i915->display.params.enable_dp_mst &&
> intel_dp_mst_source_support(intel_dp) &&
> - drm_dp_read_mst_cap(&intel_dp->aux, intel_dp->dpcd) == DP_MST_CAPABLE;
> -}
> -
> -static void
> -intel_dp_configure_mst(struct intel_dp *intel_dp)
> -{
> - struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> - struct intel_encoder *encoder =
> - &dp_to_dig_port(intel_dp)->base;
> - bool sink_can_mst = drm_dp_read_mst_cap(&intel_dp->aux, intel_dp->dpcd) == DP_MST_CAPABLE;
> + sink_can_mst;
>
> drm_dbg_kms(&i915->drm,
> "[ENCODER:%d:%s] MST support: port: %s, sink: %s, modparam: %s\n",
> @@ -4029,14 +4021,14 @@ intel_dp_configure_mst(struct intel_dp *intel_dp)
> str_yes_no(sink_can_mst),
> str_yes_no(i915->display.params.enable_dp_mst));
>
> - if (!intel_dp_mst_source_support(intel_dp))
> - return;
> -
> - intel_dp->is_mst = sink_can_mst &&
> - i915->display.params.enable_dp_mst;
> + return intel_dp->is_mst;
> +}
>
> - drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
> - intel_dp->is_mst);
> +static void intel_dp_mst_configure(struct intel_dp *intel_dp)
> +{
> + if (intel_dp_mst_source_support(intel_dp))
> + drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
> + intel_dp->is_mst);
> }
>
> static bool
> @@ -5387,7 +5379,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
> connector_status_connected : connector_status_disconnected;
> }
>
> - if (intel_dp_can_mst(intel_dp))
> + if (intel_dp_mst_detect(intel_dp))
> return connector_status_connected;
>
> /* If no HPD, poke DDC gently */
> @@ -5706,7 +5698,7 @@ intel_dp_detect(struct drm_connector *connector,
>
> intel_dp_detect_dsc_caps(intel_dp, intel_connector);
>
> - intel_dp_configure_mst(intel_dp);
> + intel_dp_mst_configure(intel_dp);
>
> /*
> * TODO: Reset link params when switching to MST mode, until MST
> --
> 2.39.2
--
Ville Syrjälä
Intel
