On Tue, 14 May 2024, Imre Deak <imre.deak@xxxxxxxxx> wrote:
> For clarity move the link training parameters updated during link
> training based on the pass/fail LT result under a substruct in intel_dp.
> This prepares for later patches in this patchset adding similar params
> here. Rename intel_dp_reset_max_link_params() to
> intel_dp_reset_link_train_params() to better reflect what state gets
> reset.
High level bikeshedding, why "link_train" instead of just "link"?
You could have three groups: source, sink and link.
BR,
Jani.
>
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> ---
> .../drm/i915/display/intel_display_types.h | 12 ++++----
> drivers/gpu/drm/i915/display/intel_dp.c | 30 +++++++++----------
> 2 files changed, 22 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index fec3de25ea54e..7edb533758416 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1738,7 +1738,6 @@ struct intel_dp {
> u8 lane_count;
> u8 sink_count;
> bool link_trained;
> - bool reset_link_params;
> bool use_max_params;
> u8 dpcd[DP_RECEIVER_CAP_SIZE];
> u8 psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
> @@ -1759,10 +1758,13 @@ struct intel_dp {
> /* intersection of source and sink rates */
> int num_common_rates;
> int common_rates[DP_MAX_SUPPORTED_RATES];
> - /* Max lane count for the current link */
> - int max_link_lane_count;
> - /* Max rate for the current link */
> - int max_link_rate;
> + struct {
> + /* Max lane count for the current link */
> + int max_lane_count;
> + /* Max rate for the current link */
> + int max_rate;
> + } link_train;
> + bool reset_link_params;
> int mso_link_count;
> int mso_pixel_overlap;
> /* sink or branch descriptor */
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 6b8a94d0ca999..ffa627c63e048 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -372,13 +372,13 @@ int intel_dp_max_common_lane_count(struct intel_dp *intel_dp)
>
> int intel_dp_max_lane_count(struct intel_dp *intel_dp)
> {
> - switch (intel_dp->max_link_lane_count) {
> + switch (intel_dp->link_train.max_lane_count) {
> case 1:
> case 2:
> case 4:
> - return intel_dp->max_link_lane_count;
> + return intel_dp->link_train.max_lane_count;
> default:
> - MISSING_CASE(intel_dp->max_link_lane_count);
> + MISSING_CASE(intel_dp->link_train.max_lane_count);
> return 1;
> }
> }
> @@ -644,7 +644,7 @@ static bool intel_dp_link_params_valid(struct intel_dp *intel_dp, int link_rate,
> * boot-up.
> */
> if (link_rate == 0 ||
> - link_rate > intel_dp->max_link_rate)
> + link_rate > intel_dp->link_train.max_rate)
> return false;
>
> if (lane_count == 0 ||
> @@ -705,8 +705,8 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> "Retrying Link training for eDP with same parameters\n");
> return 0;
> }
> - intel_dp->max_link_rate = intel_dp_common_rate(intel_dp, index - 1);
> - intel_dp->max_link_lane_count = lane_count;
> + intel_dp->link_train.max_rate = intel_dp_common_rate(intel_dp, index - 1);
> + intel_dp->link_train.max_lane_count = lane_count;
> } else if (lane_count > 1) {
> if (intel_dp_is_edp(intel_dp) &&
> !intel_dp_can_link_train_fallback_for_edp(intel_dp,
> @@ -716,8 +716,8 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> "Retrying Link training for eDP with same parameters\n");
> return 0;
> }
> - intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
> - intel_dp->max_link_lane_count = lane_count >> 1;
> + intel_dp->link_train.max_rate = intel_dp_max_common_rate(intel_dp);
> + intel_dp->link_train.max_lane_count = lane_count >> 1;
> } else {
> drm_err(&i915->drm, "Link Training Unsuccessful\n");
> return -1;
> @@ -1382,7 +1382,7 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp)
> {
> int len;
>
> - len = intel_dp_common_len_rate_limit(intel_dp, intel_dp->max_link_rate);
> + len = intel_dp_common_len_rate_limit(intel_dp, intel_dp->link_train.max_rate);
>
> return intel_dp_common_rate(intel_dp, len - 1);
> }
> @@ -3017,10 +3017,10 @@ void intel_dp_set_link_params(struct intel_dp *intel_dp,
> intel_dp->lane_count = lane_count;
> }
>
> -static void intel_dp_reset_max_link_params(struct intel_dp *intel_dp)
> +static void intel_dp_reset_link_train_params(struct intel_dp *intel_dp)
> {
> - intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> - intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
> + intel_dp->link_train.max_lane_count = intel_dp_max_common_lane_count(intel_dp);
> + intel_dp->link_train.max_rate = intel_dp_max_common_rate(intel_dp);
> }
>
> /* Enable backlight PWM and backlight PP control. */
> @@ -3355,7 +3355,7 @@ void intel_dp_sync_state(struct intel_encoder *encoder,
> intel_dp_tunnel_resume(intel_dp, crtc_state, dpcd_updated);
>
> if (crtc_state)
> - intel_dp_reset_max_link_params(intel_dp);
> + intel_dp_reset_link_train_params(intel_dp);
> }
>
> bool intel_dp_initial_fastset_check(struct intel_encoder *encoder,
> @@ -5888,7 +5888,7 @@ intel_dp_detect(struct drm_connector *connector,
> * supports link training fallback params.
> */
> if (intel_dp->reset_link_params || intel_dp->is_mst) {
> - intel_dp_reset_max_link_params(intel_dp);
> + intel_dp_reset_link_train_params(intel_dp);
> intel_dp->reset_link_params = false;
> }
>
> @@ -6740,7 +6740,7 @@ intel_dp_init_connector(struct intel_digital_port *dig_port,
>
> intel_dp_set_source_rates(intel_dp);
> intel_dp_set_common_rates(intel_dp);
> - intel_dp_reset_max_link_params(intel_dp);
> + intel_dp_reset_link_train_params(intel_dp);
>
> /* init MST on ports that can support it */
> intel_dp_mst_encoder_init(dig_port,
--
Jani Nikula, Intel
