diff mbox series

[02/20] drm/i915/dp: Move link train params to a substruct in intel_dp

Message ID 20240514191418.2863344-3-imre.deak@intel.com (mailing list archive)
State New
Headers show
Series drm/i915/dp_mst: Enable link training fallback | expand

Commit Message

Imre Deak May 14, 2024, 7:14 p.m. UTC 
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.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 .../drm/i915/display/intel_display_types.h    | 12 ++++----
 drivers/gpu/drm/i915/display/intel_dp.c       | 30 +++++++++----------
 2 files changed, 22 insertions(+), 20 deletions(-)

Comments

Jani Nikula May 15, 2024, 12:52 p.m. UTC | #1 
On Tue, 14 May 2024, Imre Deak <imre.deak@intel.com> 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@intel.com>
> ---
>  .../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,
Imre Deak May 15, 2024, 1:26 p.m. UTC | #2 
On Wed, May 15, 2024 at 03:52:33PM +0300, Jani Nikula wrote:
> On Tue, 14 May 2024, Imre Deak <imre.deak@intel.com> 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"?

It was link training I've been thinking about and that it's important to
know which state gets updated during LT and gets reset after a hotplug.

> You could have three groups: source, sink and link.

Ok, makes sense and link_train could be instead a more generic link
container.

> 
> BR,
> Jani.
> 
> 
> >
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  .../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
diff mbox series

Patch

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,