[PATCH v3 00/14] drm/i915/dp: link rate and lane count refactoring

[PATCH v3 00/14] drm/i915/dp: link rate and lane count refactoring

[Jani Nikula]

v3 of [1], rebased and review addressed.

This might still conflict with some of Manasi's pending work, but I
think for the most parts we could start pushing these. The scales are
tipping, it'll be easier to rebase Manasi's stuff than this. Indeed some
of it may be easier to understand after the changes here.

BR,
Jani.


[1] http://mid.mail-archive.com/cover.1486131408.git.jani.nikula@intel.com


Jani Nikula (14):
  drm/i915/dp: use known correct array size in rate_to_index
  drm/i915/dp: return errors from rate_to_index()
  drm/i915/dp: rename rate_to_index() to intel_dp_rate_index() and reuse
  drm/i915/dp: cache source rates at init
  drm/i915/dp: generate and cache sink rate array for all DP, not just
    eDP 1.4
  drm/i915/dp: use the sink rates array for max sink rates
  drm/i915/dp: cache common rates with sink rates
  drm/i915/dp: do not limit rate seek when not needed
  drm/i915/dp: don't call the link parameters sink parameters
  drm/i915/dp: add functions for max common link rate and lane count
  drm/i915/mst: use max link not sink lane count
  drm/i915/dp: localize link rate index variable more
  drm/i915/dp: use readb and writeb calls for single byte DPCD access
  drm/i915/dp: read sink count to a temporary variable first

 drivers/gpu/drm/i915/intel_dp.c               | 291 ++++++++++++++------------
 drivers/gpu/drm/i915/intel_dp_link_training.c |   3 +-
 drivers/gpu/drm/i915/intel_dp_mst.c           |   4 +-
 drivers/gpu/drm/i915/intel_drv.h              |  20 +-
 4 files changed, 179 insertions(+), 139 deletions(-)

-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 01/14] drm/i915/dp: use known correct array size in rate_to_index

[Jani Nikula]

I can't think of a real world bug this could cause now, but this will be
required in follow-up work. While at it, change the parameter order to
be slightly more sensible.

Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index fd96a6cf7326..88c708b07c70 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1538,12 +1538,12 @@ bool intel_dp_read_desc(struct intel_dp *intel_dp)
 	return true;
 }
 
-static int rate_to_index(int find, const int *rates)
+static int rate_to_index(const int *rates, int len, int rate)
 {
-	int i = 0;
+	int i;
 
-	for (i = 0; i < DP_MAX_SUPPORTED_RATES; ++i)
-		if (find == rates[i])
+	for (i = 0; i < len; i++)
+		if (rate == rates[i])
 			break;
 
 	return i;
@@ -1564,7 +1564,8 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp)
 
 int intel_dp_rate_select(struct intel_dp *intel_dp, int rate)
 {
-	return rate_to_index(rate, intel_dp->sink_rates);
+	return rate_to_index(intel_dp->sink_rates, intel_dp->num_sink_rates,
+			     rate);
 }
 
 void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 02/14] drm/i915/dp: return errors from rate_to_index()

[Jani Nikula]

We shouldn't silently use the first element if we can't find the rate
we're looking for. Make rate_to_index() more generally useful, and
fallback to the first element in the caller, with a big warning.

Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 88c708b07c70..0e200a37b75b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1544,9 +1544,9 @@ static int rate_to_index(const int *rates, int len, int rate)
 
 	for (i = 0; i < len; i++)
 		if (rate == rates[i])
-			break;
+			return i;
 
-	return i;
+	return -1;
 }
 
 int
@@ -1564,8 +1564,13 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp)
 
 int intel_dp_rate_select(struct intel_dp *intel_dp, int rate)
 {
-	return rate_to_index(intel_dp->sink_rates, intel_dp->num_sink_rates,
-			     rate);
+	int i = rate_to_index(intel_dp->sink_rates, intel_dp->num_sink_rates,
+			      rate);
+
+	if (WARN_ON(i < 0))
+		i = 0;
+
+	return i;
 }
 
 void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 03/14] drm/i915/dp: rename rate_to_index() to intel_dp_rate_index() and reuse

[Jani Nikula]

Rename the function, move it at the top, and reuse in
intel_dp_link_rate_index(). If there was a reason in the past to use
reverse search order here, there isn't now.

The names may be slightly confusing now, but intel_dp_link_rate_index()
will go away in follow-up patches.

v2: Use name intel_dp_rate_index (Dhinakaran)

Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 34 +++++++++++++++-------------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0e200a37b75b..9fc066dda4e0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -266,6 +266,18 @@ static int intersect_rates(const int *source_rates, int source_len,
 	return k;
 }
 
+/* return index of rate in rates array, or -1 if not found */
+static int intel_dp_rate_index(const int *rates, int len, int rate)
+{
+	int i;
+
+	for (i = 0; i < len; i++)
+		if (rate == rates[i])
+			return i;
+
+	return -1;
+}
+
 static int intel_dp_common_rates(struct intel_dp *intel_dp,
 				 int *common_rates)
 {
@@ -284,15 +296,10 @@ static int intel_dp_link_rate_index(struct intel_dp *intel_dp,
 				    int *common_rates, int link_rate)
 {
 	int common_len;
-	int index;
 
 	common_len = intel_dp_common_rates(intel_dp, common_rates);
-	for (index = 0; index < common_len; index++) {
-		if (link_rate == common_rates[common_len - index - 1])
-			return common_len - index - 1;
-	}
 
-	return -1;
+	return intel_dp_rate_index(common_rates, common_len, link_rate);
 }
 
 int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
@@ -1538,17 +1545,6 @@ bool intel_dp_read_desc(struct intel_dp *intel_dp)
 	return true;
 }
 
-static int rate_to_index(const int *rates, int len, int rate)
-{
-	int i;
-
-	for (i = 0; i < len; i++)
-		if (rate == rates[i])
-			return i;
-
-	return -1;
-}
-
 int
 intel_dp_max_link_rate(struct intel_dp *intel_dp)
 {
@@ -1564,8 +1560,8 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp)
 
 int intel_dp_rate_select(struct intel_dp *intel_dp, int rate)
 {
-	int i = rate_to_index(intel_dp->sink_rates, intel_dp->num_sink_rates,
-			      rate);
+	int i = intel_dp_rate_index(intel_dp->sink_rates,
+				    intel_dp->num_sink_rates, rate);
 
 	if (WARN_ON(i < 0))
 		i = 0;
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 04/14] drm/i915/dp: cache source rates at init

[Jani Nikula]

We need the source rates array so often that it makes sense to set it
once at init. This reduces function calls when we need the rates, making
the code easier to follow.

Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 35 +++++++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_drv.h |  3 +++
 2 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 9fc066dda4e0..e9bd75ff3904 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -218,21 +218,25 @@ intel_dp_sink_rates(struct intel_dp *intel_dp, const int **sink_rates)
 	return (intel_dp->max_sink_link_bw >> 3) + 1;
 }
 
-static int
-intel_dp_source_rates(struct intel_dp *intel_dp, const int **source_rates)
+static void
+intel_dp_set_source_rates(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
+	const int *source_rates;
 	int size;
 
+	/* This should only be done once */
+	WARN_ON(intel_dp->source_rates || intel_dp->num_source_rates);
+
 	if (IS_GEN9_LP(dev_priv)) {
-		*source_rates = bxt_rates;
+		source_rates = bxt_rates;
 		size = ARRAY_SIZE(bxt_rates);
 	} else if (IS_GEN9_BC(dev_priv)) {
-		*source_rates = skl_rates;
+		source_rates = skl_rates;
 		size = ARRAY_SIZE(skl_rates);
 	} else {
-		*source_rates = default_rates;
+		source_rates = default_rates;
 		size = ARRAY_SIZE(default_rates);
 	}
 
@@ -240,7 +244,8 @@ intel_dp_source_rates(struct intel_dp *intel_dp, const int **source_rates)
 	if (!intel_dp_source_supports_hbr2(intel_dp))
 		size--;
 
-	return size;
+	intel_dp->source_rates = source_rates;
+	intel_dp->num_source_rates = size;
 }
 
 static int intersect_rates(const int *source_rates, int source_len,
@@ -281,13 +286,13 @@ static int intel_dp_rate_index(const int *rates, int len, int rate)
 static int intel_dp_common_rates(struct intel_dp *intel_dp,
 				 int *common_rates)
 {
-	const int *source_rates, *sink_rates;
-	int source_len, sink_len;
+	const int *sink_rates;
+	int sink_len;
 
 	sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
-	source_len = intel_dp_source_rates(intel_dp, &source_rates);
 
-	return intersect_rates(source_rates, source_len,
+	return intersect_rates(intel_dp->source_rates,
+			       intel_dp->num_source_rates,
 			       sink_rates, sink_len,
 			       common_rates);
 }
@@ -1493,16 +1498,16 @@ static void snprintf_int_array(char *str, size_t len,
 
 static void intel_dp_print_rates(struct intel_dp *intel_dp)
 {
-	const int *source_rates, *sink_rates;
-	int source_len, sink_len, common_len;
+	const int *sink_rates;
+	int sink_len, common_len;
 	int common_rates[DP_MAX_SUPPORTED_RATES];
 	char str[128]; /* FIXME: too big for stack? */
 
 	if ((drm_debug & DRM_UT_KMS) == 0)
 		return;
 
-	source_len = intel_dp_source_rates(intel_dp, &source_rates);
-	snprintf_int_array(str, sizeof(str), source_rates, source_len);
+	snprintf_int_array(str, sizeof(str),
+			   intel_dp->source_rates, intel_dp->num_source_rates);
 	DRM_DEBUG_KMS("source rates: %s\n", str);
 
 	sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
@@ -5943,6 +5948,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 		 intel_dig_port->max_lanes, port_name(port)))
 		return false;
 
+	intel_dp_set_source_rates(intel_dp);
+
 	intel_dp->reset_link_params = true;
 	intel_dp->pps_pipe = INVALID_PIPE;
 	intel_dp->active_pipe = INVALID_PIPE;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e24641b559e2..f59574261dc6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -949,6 +949,9 @@ struct intel_dp {
 	uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
 	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
 	uint8_t edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
+	/* source rates */
+	int num_source_rates;
+	const int *source_rates;
 	/* sink rates as reported by DP_SUPPORTED_LINK_RATES */
 	uint8_t num_sink_rates;
 	int sink_rates[DP_MAX_SUPPORTED_RATES];
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 05/14] drm/i915/dp: generate and cache sink rate array for all DP, not just eDP 1.4

[Jani Nikula]

There is some conflation related to sink rates, making this change more
complicated than it would otherwise have to be. There are three changes
here that are rather difficult to split up:

1) Use the intel_dp->sink_rates array for all DP, not just eDP 1.4. We
   initialize it from DPCD on eDP 1.4 like before, but generate it based
   on DP_MAX_LINK_RATE on others. This reduces code complexity when we
   need to use the sink rates; they are all always in the sink_rates
   array.

2) Update the sink rate array whenever we read DPCD, and use the
   information from there. This increases code readability when we need
   the sink rates.

3) Disentangle fallback rate limiting from sink rates. In the code, the
   max rate is a dynamic property of the *link*, not of the *sink*. Do
   the limiting after intersecting the source and sink rates, which are
   static properties of the devices.

This paves the way for follow-up refactoring that I've refrained from
doing here to keep this change as simple as it possibly can.

v2: introduce use_rate_select and handle non-confirming eDP (Ville)

v3: don't clobber cached eDP rates on short pulse (Ville)

Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c               | 81 ++++++++++++++++++---------
 drivers/gpu/drm/i915/intel_dp_link_training.c |  3 +-
 drivers/gpu/drm/i915/intel_drv.h              |  5 +-
 3 files changed, 61 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e9bd75ff3904..b38cba7d5abc 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -133,6 +133,34 @@ static void vlv_steal_power_sequencer(struct drm_device *dev,
 				      enum pipe pipe);
 static void intel_dp_unset_edid(struct intel_dp *intel_dp);
 
+static int intel_dp_num_rates(u8 link_bw_code)
+{
+	switch (link_bw_code) {
+	default:
+		WARN(1, "invalid max DP link bw val %x, using 1.62Gbps\n",
+		     link_bw_code);
+	case DP_LINK_BW_1_62:
+		return 1;
+	case DP_LINK_BW_2_7:
+		return 2;
+	case DP_LINK_BW_5_4:
+		return 3;
+	}
+}
+
+/* update sink rates from dpcd */
+static void intel_dp_set_sink_rates(struct intel_dp *intel_dp)
+{
+	int i, num_rates;
+
+	num_rates = intel_dp_num_rates(intel_dp->dpcd[DP_MAX_LINK_RATE]);
+
+	for (i = 0; i < num_rates; i++)
+		intel_dp->sink_rates[i] = default_rates[i];
+
+	intel_dp->num_sink_rates = num_rates;
+}
+
 static int
 intel_dp_max_link_bw(struct intel_dp  *intel_dp)
 {
@@ -205,19 +233,6 @@ intel_dp_downstream_max_dotclock(struct intel_dp *intel_dp)
 	return max_dotclk;
 }
 
-static int
-intel_dp_sink_rates(struct intel_dp *intel_dp, const int **sink_rates)
-{
-	if (intel_dp->num_sink_rates) {
-		*sink_rates = intel_dp->sink_rates;
-		return intel_dp->num_sink_rates;
-	}
-
-	*sink_rates = default_rates;
-
-	return (intel_dp->max_sink_link_bw >> 3) + 1;
-}
-
 static void
 intel_dp_set_source_rates(struct intel_dp *intel_dp)
 {
@@ -286,15 +301,22 @@ static int intel_dp_rate_index(const int *rates, int len, int rate)
 static int intel_dp_common_rates(struct intel_dp *intel_dp,
 				 int *common_rates)
 {
-	const int *sink_rates;
-	int sink_len;
+	int max_rate = drm_dp_bw_code_to_link_rate(intel_dp->max_sink_link_bw);
+	int i, common_len;
 
-	sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
+	common_len = intersect_rates(intel_dp->source_rates,
+				     intel_dp->num_source_rates,
+				     intel_dp->sink_rates,
+				     intel_dp->num_sink_rates,
+				     common_rates);
+
+	/* Limit results by potentially reduced max rate */
+	for (i = 0; i < common_len; i++) {
+		if (common_rates[common_len - i - 1] <= max_rate)
+			return common_len - i;
+	}
 
-	return intersect_rates(intel_dp->source_rates,
-			       intel_dp->num_source_rates,
-			       sink_rates, sink_len,
-			       common_rates);
+	return 0;
 }
 
 static int intel_dp_link_rate_index(struct intel_dp *intel_dp,
@@ -1498,8 +1520,7 @@ static void snprintf_int_array(char *str, size_t len,
 
 static void intel_dp_print_rates(struct intel_dp *intel_dp)
 {
-	const int *sink_rates;
-	int sink_len, common_len;
+	int common_len;
 	int common_rates[DP_MAX_SUPPORTED_RATES];
 	char str[128]; /* FIXME: too big for stack? */
 
@@ -1510,8 +1531,8 @@ static void intel_dp_print_rates(struct intel_dp *intel_dp)
 			   intel_dp->source_rates, intel_dp->num_source_rates);
 	DRM_DEBUG_KMS("source rates: %s\n", str);
 
-	sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
-	snprintf_int_array(str, sizeof(str), sink_rates, sink_len);
+	snprintf_int_array(str, sizeof(str),
+			   intel_dp->sink_rates, intel_dp->num_sink_rates);
 	DRM_DEBUG_KMS("sink rates: %s\n", str);
 
 	common_len = intel_dp_common_rates(intel_dp, common_rates);
@@ -1577,7 +1598,8 @@ int intel_dp_rate_select(struct intel_dp *intel_dp, int rate)
 void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
 			   uint8_t *link_bw, uint8_t *rate_select)
 {
-	if (intel_dp->num_sink_rates) {
+	/* eDP 1.4 rate select method. */
+	if (intel_dp->use_rate_select) {
 		*link_bw = 0;
 		*rate_select =
 			intel_dp_rate_select(intel_dp, port_clock);
@@ -3702,6 +3724,11 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)
 		intel_dp->num_sink_rates = i;
 	}
 
+	if (intel_dp->num_sink_rates)
+		intel_dp->use_rate_select = true;
+	else
+		intel_dp_set_sink_rates(intel_dp);
+
 	return true;
 }
 
@@ -3712,6 +3739,10 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 	if (!intel_dp_read_dpcd(intel_dp))
 		return false;
 
+	/* Don't clobber cached eDP rates. */
+	if (!is_edp(intel_dp))
+		intel_dp_set_sink_rates(intel_dp);
+
 	if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_COUNT,
 			     &intel_dp->sink_count, 1) < 0)
 		return false;
diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
index 0048b520baf7..694ad0ffb523 100644
--- a/drivers/gpu/drm/i915/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
@@ -146,7 +146,8 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
 		link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
 	drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_BW_SET, link_config, 2);
 
-	if (intel_dp->num_sink_rates)
+	/* eDP 1.4 rate select method. */
+	if (!link_bw)
 		drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_RATE_SET,
 				  &rate_select, 1);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f59574261dc6..77a94aaa98c2 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -952,9 +952,10 @@ struct intel_dp {
 	/* source rates */
 	int num_source_rates;
 	const int *source_rates;
-	/* sink rates as reported by DP_SUPPORTED_LINK_RATES */
-	uint8_t num_sink_rates;
+	/* sink rates as reported by DP_MAX_LINK_RATE/DP_SUPPORTED_LINK_RATES */
+	int num_sink_rates;
 	int sink_rates[DP_MAX_SUPPORTED_RATES];
+	bool use_rate_select;
 	/* Max lane count for the sink as per DPCD registers */
 	uint8_t max_sink_lane_count;
 	/* Max link BW for the sink as per DPCD registers */
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 06/14] drm/i915/dp: use the sink rates array for max sink rates

[Jani Nikula]

Looking at DPCD DP_MAX_LINK_RATE may be completely bogus for eDP 1.4
which is allowed to use link rate select method and have 0 in max link
rate. With this change, it makes sense to store the max rate as the
actual rate rather than as a bw code.

Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 28 +++++++---------------------
 drivers/gpu/drm/i915/intel_drv.h |  2 +-
 2 files changed, 8 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b38cba7d5abc..e4650f1625cc 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -161,23 +161,9 @@ static void intel_dp_set_sink_rates(struct intel_dp *intel_dp)
 	intel_dp->num_sink_rates = num_rates;
 }
 
-static int
-intel_dp_max_link_bw(struct intel_dp  *intel_dp)
+static int intel_dp_max_sink_rate(struct intel_dp *intel_dp)
 {
-	int max_link_bw = intel_dp->dpcd[DP_MAX_LINK_RATE];
-
-	switch (max_link_bw) {
-	case DP_LINK_BW_1_62:
-	case DP_LINK_BW_2_7:
-	case DP_LINK_BW_5_4:
-		break;
-	default:
-		WARN(1, "invalid max DP link bw val %x, using 1.62Gbps\n",
-		     max_link_bw);
-		max_link_bw = DP_LINK_BW_1_62;
-		break;
-	}
-	return max_link_bw;
+	return intel_dp->sink_rates[intel_dp->num_sink_rates - 1];
 }
 
 static u8 intel_dp_max_lane_count(struct intel_dp *intel_dp)
@@ -301,7 +287,7 @@ static int intel_dp_rate_index(const int *rates, int len, int rate)
 static int intel_dp_common_rates(struct intel_dp *intel_dp,
 				 int *common_rates)
 {
-	int max_rate = drm_dp_bw_code_to_link_rate(intel_dp->max_sink_link_bw);
+	int max_rate = intel_dp->max_sink_link_rate;
 	int i, common_len;
 
 	common_len = intersect_rates(intel_dp->source_rates,
@@ -339,10 +325,10 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
 						   common_rates,
 						   link_rate);
 	if (link_rate_index > 0) {
-		intel_dp->max_sink_link_bw = drm_dp_link_rate_to_bw_code(common_rates[link_rate_index - 1]);
+		intel_dp->max_sink_link_rate = common_rates[link_rate_index - 1];
 		intel_dp->max_sink_lane_count = lane_count;
 	} else if (lane_count > 1) {
-		intel_dp->max_sink_link_bw = intel_dp_max_link_bw(intel_dp);
+		intel_dp->max_sink_link_rate = intel_dp_max_sink_rate(intel_dp);
 		intel_dp->max_sink_lane_count = lane_count >> 1;
 	} else {
 		DRM_ERROR("Link Training Unsuccessful\n");
@@ -4652,8 +4638,8 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 		/* Set the max lane count for sink */
 		intel_dp->max_sink_lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
 
-		/* Set the max link BW for sink */
-		intel_dp->max_sink_link_bw = intel_dp_max_link_bw(intel_dp);
+		/* Set the max link rate for sink */
+		intel_dp->max_sink_link_rate = intel_dp_max_sink_rate(intel_dp);
 
 		intel_dp->reset_link_params = false;
 	}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 77a94aaa98c2..f8140b06679d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -959,7 +959,7 @@ struct intel_dp {
 	/* Max lane count for the sink as per DPCD registers */
 	uint8_t max_sink_lane_count;
 	/* Max link BW for the sink as per DPCD registers */
-	int max_sink_link_bw;
+	int max_sink_link_rate;
 	/* sink or branch descriptor */
 	struct intel_dp_desc desc;
 	struct drm_dp_aux aux;
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 07/14] drm/i915/dp: cache common rates with sink rates

[Jani Nikula]

Now that source rates are static and sink rates are updated whenever
DPCD is updated, we can do and cache the intersection of them whenever
sink rates are updated. This reduces code complexity, as we don't have
to keep calling the functions to intersect. We also get rid of several
common rates arrays on stack.

Limiting the common rates by a max link rate can be done by picking the
first N elements of the cached common rates.

v2: get rid of the local common_rates variable (Manasi)
v3: don't clobber cached eDP rates on short pulse (Ville)

Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 75 ++++++++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_drv.h |  3 ++
 2 files changed, 45 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e4650f1625cc..1808af6d635d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -284,17 +284,29 @@ static int intel_dp_rate_index(const int *rates, int len, int rate)
 	return -1;
 }
 
-static int intel_dp_common_rates(struct intel_dp *intel_dp,
-				 int *common_rates)
+static void intel_dp_set_common_rates(struct intel_dp *intel_dp)
 {
-	int max_rate = intel_dp->max_sink_link_rate;
-	int i, common_len;
+	WARN_ON(!intel_dp->num_source_rates || !intel_dp->num_sink_rates);
 
-	common_len = intersect_rates(intel_dp->source_rates,
-				     intel_dp->num_source_rates,
-				     intel_dp->sink_rates,
-				     intel_dp->num_sink_rates,
-				     common_rates);
+	intel_dp->num_common_rates = intersect_rates(intel_dp->source_rates,
+						     intel_dp->num_source_rates,
+						     intel_dp->sink_rates,
+						     intel_dp->num_sink_rates,
+						     intel_dp->common_rates);
+
+	/* Paranoia, there should always be something in common. */
+	if (WARN_ON(intel_dp->num_common_rates == 0)) {
+		intel_dp->common_rates[0] = default_rates[0];
+		intel_dp->num_common_rates = 1;
+	}
+}
+
+/* get length of common rates potentially limited by max_rate */
+static int intel_dp_common_len_rate_limit(struct intel_dp *intel_dp,
+					  int max_rate)
+{
+	const int *common_rates = intel_dp->common_rates;
+	int i, common_len = intel_dp->num_common_rates;
 
 	/* Limit results by potentially reduced max rate */
 	for (i = 0; i < common_len; i++) {
@@ -305,25 +317,23 @@ static int intel_dp_common_rates(struct intel_dp *intel_dp,
 	return 0;
 }
 
-static int intel_dp_link_rate_index(struct intel_dp *intel_dp,
-				    int *common_rates, int link_rate)
+static int intel_dp_link_rate_index(struct intel_dp *intel_dp, int link_rate)
 {
 	int common_len;
 
-	common_len = intel_dp_common_rates(intel_dp, common_rates);
+	common_len = intel_dp_common_len_rate_limit(intel_dp,
+						    intel_dp->max_sink_link_rate);
 
-	return intel_dp_rate_index(common_rates, common_len, link_rate);
+	return intel_dp_rate_index(intel_dp->common_rates, common_len, link_rate);
 }
 
 int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
 					    int link_rate, uint8_t lane_count)
 {
-	int common_rates[DP_MAX_SUPPORTED_RATES];
+	const int *common_rates = intel_dp->common_rates;
 	int link_rate_index;
 
-	link_rate_index = intel_dp_link_rate_index(intel_dp,
-						   common_rates,
-						   link_rate);
+	link_rate_index = intel_dp_link_rate_index(intel_dp, link_rate);
 	if (link_rate_index > 0) {
 		intel_dp->max_sink_link_rate = common_rates[link_rate_index - 1];
 		intel_dp->max_sink_lane_count = lane_count;
@@ -1506,8 +1516,6 @@ static void snprintf_int_array(char *str, size_t len,
 
 static void intel_dp_print_rates(struct intel_dp *intel_dp)
 {
-	int common_len;
-	int common_rates[DP_MAX_SUPPORTED_RATES];
 	char str[128]; /* FIXME: too big for stack? */
 
 	if ((drm_debug & DRM_UT_KMS) == 0)
@@ -1521,8 +1529,8 @@ static void intel_dp_print_rates(struct intel_dp *intel_dp)
 			   intel_dp->sink_rates, intel_dp->num_sink_rates);
 	DRM_DEBUG_KMS("sink rates: %s\n", str);
 
-	common_len = intel_dp_common_rates(intel_dp, common_rates);
-	snprintf_int_array(str, sizeof(str), common_rates, common_len);
+	snprintf_int_array(str, sizeof(str),
+			   intel_dp->common_rates, intel_dp->num_common_rates);
 	DRM_DEBUG_KMS("common rates: %s\n", str);
 }
 
@@ -1560,14 +1568,14 @@ bool intel_dp_read_desc(struct intel_dp *intel_dp)
 int
 intel_dp_max_link_rate(struct intel_dp *intel_dp)
 {
-	int rates[DP_MAX_SUPPORTED_RATES] = {};
 	int len;
 
-	len = intel_dp_common_rates(intel_dp, rates);
+	len = intel_dp_common_len_rate_limit(intel_dp,
+					     intel_dp->max_sink_link_rate);
 	if (WARN_ON(len <= 0))
 		return 162000;
 
-	return rates[len - 1];
+	return intel_dp->common_rates[len - 1];
 }
 
 int intel_dp_rate_select(struct intel_dp *intel_dp, int rate)
@@ -1636,11 +1644,11 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	int link_rate_index;
 	int bpp, mode_rate;
 	int link_avail, link_clock;
-	int common_rates[DP_MAX_SUPPORTED_RATES] = {};
 	int common_len;
 	uint8_t link_bw, rate_select;
 
-	common_len = intel_dp_common_rates(intel_dp, common_rates);
+	common_len = intel_dp_common_len_rate_limit(intel_dp,
+						    intel_dp->max_sink_link_rate);
 
 	/* No common link rates between source and sink */
 	WARN_ON(common_len <= 0);
@@ -1678,7 +1686,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	/* Use values requested by Compliance Test Request */
 	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
 		link_rate_index = intel_dp_link_rate_index(intel_dp,
-							   common_rates,
 							   intel_dp->compliance.test_link_rate);
 		if (link_rate_index >= 0)
 			min_clock = max_clock = link_rate_index;
@@ -1686,7 +1693,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	}
 	DRM_DEBUG_KMS("DP link computation with max lane count %i "
 		      "max bw %d pixel clock %iKHz\n",
-		      max_lane_count, common_rates[max_clock],
+		      max_lane_count, intel_dp->common_rates[max_clock],
 		      adjusted_mode->crtc_clock);
 
 	/* Walk through all bpp values. Luckily they're all nicely spaced with 2
@@ -1722,7 +1729,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 				lane_count <= max_lane_count;
 				lane_count <<= 1) {
 
-				link_clock = common_rates[clock];
+				link_clock = intel_dp->common_rates[clock];
 				link_avail = intel_dp_max_data_rate(link_clock,
 								    lane_count);
 
@@ -1754,7 +1761,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	pipe_config->lane_count = lane_count;
 
 	pipe_config->pipe_bpp = bpp;
-	pipe_config->port_clock = common_rates[clock];
+	pipe_config->port_clock = intel_dp->common_rates[clock];
 
 	intel_dp_compute_rate(intel_dp, pipe_config->port_clock,
 			      &link_bw, &rate_select);
@@ -3715,6 +3722,8 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)
 	else
 		intel_dp_set_sink_rates(intel_dp);
 
+	intel_dp_set_common_rates(intel_dp);
+
 	return true;
 }
 
@@ -3726,8 +3735,10 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 		return false;
 
 	/* Don't clobber cached eDP rates. */
-	if (!is_edp(intel_dp))
+	if (!is_edp(intel_dp)) {
 		intel_dp_set_sink_rates(intel_dp);
+		intel_dp_set_common_rates(intel_dp);
+	}
 
 	if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_COUNT,
 			     &intel_dp->sink_count, 1) < 0)
@@ -3950,7 +3961,6 @@ static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp)
 {
 	int status = 0;
 	int min_lane_count = 1;
-	int common_rates[DP_MAX_SUPPORTED_RATES] = {};
 	int link_rate_index, test_link_rate;
 	uint8_t test_lane_count, test_link_bw;
 	/* (DP CTS 1.2)
@@ -3979,7 +3989,6 @@ static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp)
 	/* Validate the requested link rate */
 	test_link_rate = drm_dp_bw_code_to_link_rate(test_link_bw);
 	link_rate_index = intel_dp_link_rate_index(intel_dp,
-						   common_rates,
 						   test_link_rate);
 	if (link_rate_index < 0)
 		return DP_TEST_NAK;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f8140b06679d..ec8985b20616 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -956,6 +956,9 @@ struct intel_dp {
 	int num_sink_rates;
 	int sink_rates[DP_MAX_SUPPORTED_RATES];
 	bool use_rate_select;
+	/* intersection of source and sink rates */
+	int num_common_rates;
+	int common_rates[DP_MAX_SUPPORTED_RATES];
 	/* Max lane count for the sink as per DPCD registers */
 	uint8_t max_sink_lane_count;
 	/* Max link BW for the sink as per DPCD registers */
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 08/14] drm/i915/dp: do not limit rate seek when not needed

[Jani Nikula]

In link training fallback, we're trying to find a rate that we know is
in a sorted array of common link rates. We don't need to limit the array
using the max rate. For test request, the DP CTS doesn't say we should
limit the rate based on earlier fallback.

Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 31 ++++++++++++-------------------
 1 file changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1808af6d635d..8c061c54d481 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -317,25 +317,16 @@ static int intel_dp_common_len_rate_limit(struct intel_dp *intel_dp,
 	return 0;
 }
 
-static int intel_dp_link_rate_index(struct intel_dp *intel_dp, int link_rate)
-{
-	int common_len;
-
-	common_len = intel_dp_common_len_rate_limit(intel_dp,
-						    intel_dp->max_sink_link_rate);
-
-	return intel_dp_rate_index(intel_dp->common_rates, common_len, link_rate);
-}
-
 int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
 					    int link_rate, uint8_t lane_count)
 {
-	const int *common_rates = intel_dp->common_rates;
-	int link_rate_index;
+	int index;
 
-	link_rate_index = intel_dp_link_rate_index(intel_dp, link_rate);
-	if (link_rate_index > 0) {
-		intel_dp->max_sink_link_rate = common_rates[link_rate_index - 1];
+	index = intel_dp_rate_index(intel_dp->common_rates,
+				    intel_dp->num_common_rates,
+				    link_rate);
+	if (index > 0) {
+		intel_dp->max_sink_link_rate = intel_dp->common_rates[index - 1];
 		intel_dp->max_sink_lane_count = lane_count;
 	} else if (lane_count > 1) {
 		intel_dp->max_sink_link_rate = intel_dp_max_sink_rate(intel_dp);
@@ -1685,8 +1676,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 
 	/* Use values requested by Compliance Test Request */
 	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
-		link_rate_index = intel_dp_link_rate_index(intel_dp,
-							   intel_dp->compliance.test_link_rate);
+		link_rate_index = intel_dp_rate_index(intel_dp->common_rates,
+						      intel_dp->num_common_rates,
+						      intel_dp->compliance.test_link_rate);
 		if (link_rate_index >= 0)
 			min_clock = max_clock = link_rate_index;
 		min_lane_count = max_lane_count = intel_dp->compliance.test_lane_count;
@@ -3988,8 +3980,9 @@ static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp)
 	}
 	/* Validate the requested link rate */
 	test_link_rate = drm_dp_bw_code_to_link_rate(test_link_bw);
-	link_rate_index = intel_dp_link_rate_index(intel_dp,
-						   test_link_rate);
+	link_rate_index = intel_dp_rate_index(intel_dp->common_rates,
+					      intel_dp->num_common_rates,
+					      test_link_rate);
 	if (link_rate_index < 0)
 		return DP_TEST_NAK;
 
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 09/14] drm/i915/dp: don't call the link parameters sink parameters

[Jani Nikula]

If we modify these on the fly depending on the link conditions, don't
pretend they are sink properties.

Some link vs. sink confusion still remains, but we'll take care of them
in follow-up patches.

Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 25 ++++++++++++-------------
 drivers/gpu/drm/i915/intel_drv.h |  8 ++++----
 2 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8c061c54d481..a0082a3784e8 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -172,7 +172,7 @@ static u8 intel_dp_max_lane_count(struct intel_dp *intel_dp)
 	u8 source_max, sink_max;
 
 	source_max = intel_dig_port->max_lanes;
-	sink_max = intel_dp->max_sink_lane_count;
+	sink_max = intel_dp->max_link_lane_count;
 
 	return min(source_max, sink_max);
 }
@@ -326,11 +326,11 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
 				    intel_dp->num_common_rates,
 				    link_rate);
 	if (index > 0) {
-		intel_dp->max_sink_link_rate = intel_dp->common_rates[index - 1];
-		intel_dp->max_sink_lane_count = lane_count;
+		intel_dp->max_link_rate = intel_dp->common_rates[index - 1];
+		intel_dp->max_link_lane_count = lane_count;
 	} else if (lane_count > 1) {
-		intel_dp->max_sink_link_rate = intel_dp_max_sink_rate(intel_dp);
-		intel_dp->max_sink_lane_count = lane_count >> 1;
+		intel_dp->max_link_rate = intel_dp_max_sink_rate(intel_dp);
+		intel_dp->max_link_lane_count = lane_count >> 1;
 	} else {
 		DRM_ERROR("Link Training Unsuccessful\n");
 		return -1;
@@ -1561,8 +1561,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_sink_link_rate);
+	len = intel_dp_common_len_rate_limit(intel_dp, intel_dp->max_link_rate);
 	if (WARN_ON(len <= 0))
 		return 162000;
 
@@ -1639,7 +1638,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	uint8_t link_bw, rate_select;
 
 	common_len = intel_dp_common_len_rate_limit(intel_dp,
-						    intel_dp->max_sink_link_rate);
+						    intel_dp->max_link_rate);
 
 	/* No common link rates between source and sink */
 	WARN_ON(common_len <= 0);
@@ -3969,7 +3968,7 @@ static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp)
 	test_lane_count &= DP_MAX_LANE_COUNT_MASK;
 	/* Validate the requested lane count */
 	if (test_lane_count < min_lane_count ||
-	    test_lane_count > intel_dp->max_sink_lane_count)
+	    test_lane_count > intel_dp->max_link_lane_count)
 		return DP_TEST_NAK;
 
 	status = drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_LINK_RATE,
@@ -4637,11 +4636,11 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 		      yesno(drm_dp_tps3_supported(intel_dp->dpcd)));
 
 	if (intel_dp->reset_link_params) {
-		/* Set the max lane count for sink */
-		intel_dp->max_sink_lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
+		/* Set the max lane count for link */
+		intel_dp->max_link_lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
 
-		/* Set the max link rate for sink */
-		intel_dp->max_sink_link_rate = intel_dp_max_sink_rate(intel_dp);
+		/* Set the max link rate for link */
+		intel_dp->max_link_rate = intel_dp_max_sink_rate(intel_dp);
 
 		intel_dp->reset_link_params = false;
 	}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ec8985b20616..9141515e4204 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -959,10 +959,10 @@ 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 sink as per DPCD registers */
-	uint8_t max_sink_lane_count;
-	/* Max link BW for the sink as per DPCD registers */
-	int max_sink_link_rate;
+	/* Max lane count for the current link */
+	int max_link_lane_count;
+	/* Max rate for the current link */
+	int max_link_rate;
 	/* sink or branch descriptor */
 	struct intel_dp_desc desc;
 	struct drm_dp_aux aux;
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 10/14] drm/i915/dp: add functions for max common link rate and lane count

[Jani Nikula]

These are the theoretical maximums common for source and sink. These are
the maximums we should start with. They may be degraded in case of link
training failures, and the dynamic link values are stored separately.

Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a0082a3784e8..b3df2082eac9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -161,22 +161,27 @@ static void intel_dp_set_sink_rates(struct intel_dp *intel_dp)
 	intel_dp->num_sink_rates = num_rates;
 }
 
-static int intel_dp_max_sink_rate(struct intel_dp *intel_dp)
+/* Theoretical max between source and sink */
+static int intel_dp_max_common_rate(struct intel_dp *intel_dp)
 {
-	return intel_dp->sink_rates[intel_dp->num_sink_rates - 1];
+	return intel_dp->common_rates[intel_dp->num_common_rates - 1];
 }
 
-static u8 intel_dp_max_lane_count(struct intel_dp *intel_dp)
+/* Theoretical max between source and sink */
+static int intel_dp_max_common_lane_count(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
-	u8 source_max, sink_max;
-
-	source_max = intel_dig_port->max_lanes;
-	sink_max = intel_dp->max_link_lane_count;
+	int source_max = intel_dig_port->max_lanes;
+	int sink_max = drm_dp_max_lane_count(intel_dp->dpcd);
 
 	return min(source_max, sink_max);
 }
 
+static int intel_dp_max_lane_count(struct intel_dp *intel_dp)
+{
+	return intel_dp->max_link_lane_count;
+}
+
 int
 intel_dp_link_required(int pixel_clock, int bpp)
 {
@@ -329,7 +334,7 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
 		intel_dp->max_link_rate = intel_dp->common_rates[index - 1];
 		intel_dp->max_link_lane_count = lane_count;
 	} else if (lane_count > 1) {
-		intel_dp->max_link_rate = intel_dp_max_sink_rate(intel_dp);
+		intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
 		intel_dp->max_link_lane_count = lane_count >> 1;
 	} else {
 		DRM_ERROR("Link Training Unsuccessful\n");
@@ -4636,11 +4641,11 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 		      yesno(drm_dp_tps3_supported(intel_dp->dpcd)));
 
 	if (intel_dp->reset_link_params) {
-		/* Set the max lane count for link */
-		intel_dp->max_link_lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
+		/* Initial max link lane count */
+		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
 
-		/* Set the max link rate for link */
-		intel_dp->max_link_rate = intel_dp_max_sink_rate(intel_dp);
+		/* Initial max link rate */
+		intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
 
 		intel_dp->reset_link_params = false;
 	}
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 11/14] drm/i915/mst: use max link not sink lane count

[Jani Nikula]

The source might not support as many lanes as the sink, or the link
training might have failed at higher lane counts. Take these into
account.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c     | 2 +-
 drivers/gpu/drm/i915/intel_dp_mst.c | 4 ++--
 drivers/gpu/drm/i915/intel_drv.h    | 1 +
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b3df2082eac9..95f2278700e3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -177,7 +177,7 @@ static int intel_dp_max_common_lane_count(struct intel_dp *intel_dp)
 	return min(source_max, sink_max);
 }
 
-static int intel_dp_max_lane_count(struct intel_dp *intel_dp)
+int intel_dp_max_lane_count(struct intel_dp *intel_dp)
 {
 	return intel_dp->max_link_lane_count;
 }
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index c1f62eb07c07..3451e2abb23b 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -56,7 +56,7 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
 	 * for MST we always configure max link bw - the spec doesn't
 	 * seem to suggest we should do otherwise.
 	 */
-	lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
+	lane_count = intel_dp_max_lane_count(intel_dp);
 
 	pipe_config->lane_count = lane_count;
 
@@ -343,7 +343,7 @@ intel_dp_mst_mode_valid(struct drm_connector *connector,
 	int max_rate, mode_rate, max_lanes, max_link_clock;
 
 	max_link_clock = intel_dp_max_link_rate(intel_dp);
-	max_lanes = drm_dp_max_lane_count(intel_dp->dpcd);
+	max_lanes = intel_dp_max_lane_count(intel_dp);
 
 	max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes);
 	mode_rate = intel_dp_link_required(mode->clock, bpp);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9141515e4204..0c037295459b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1507,6 +1507,7 @@ void intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *co
 void intel_dp_mst_suspend(struct drm_device *dev);
 void intel_dp_mst_resume(struct drm_device *dev);
 int intel_dp_max_link_rate(struct intel_dp *intel_dp);
+int intel_dp_max_lane_count(struct intel_dp *intel_dp);
 int intel_dp_rate_select(struct intel_dp *intel_dp, int rate);
 void intel_dp_hot_plug(struct intel_encoder *intel_encoder);
 void intel_power_sequencer_reset(struct drm_i915_private *dev_priv);
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 12/14] drm/i915/dp: localize link rate index variable more

[Jani Nikula]

Localize link_rate_index to the if block, and rename to just index to
reduce indent.

Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 95f2278700e3..6f743490855b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1636,7 +1636,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	/* Conveniently, the link BW constants become indices with a shift...*/
 	int min_clock = 0;
 	int max_clock;
-	int link_rate_index;
 	int bpp, mode_rate;
 	int link_avail, link_clock;
 	int common_len;
@@ -1680,11 +1679,13 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 
 	/* Use values requested by Compliance Test Request */
 	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
-		link_rate_index = intel_dp_rate_index(intel_dp->common_rates,
-						      intel_dp->num_common_rates,
-						      intel_dp->compliance.test_link_rate);
-		if (link_rate_index >= 0)
-			min_clock = max_clock = link_rate_index;
+		int index;
+
+		index = intel_dp_rate_index(intel_dp->common_rates,
+					    intel_dp->num_common_rates,
+					    intel_dp->compliance.test_link_rate);
+		if (index >= 0)
+			min_clock = max_clock = index;
 		min_lane_count = max_lane_count = intel_dp->compliance.test_lane_count;
 	}
 	DRM_DEBUG_KMS("DP link computation with max lane count %i "
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 13/14] drm/i915/dp: use readb and writeb calls for single byte DPCD access

[Jani Nikula]

This is what we have the readb and writeb variants for. Do some minor
return value and variable cleanup while at it.

Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 37 +++++++++++++++++--------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 6f743490855b..81682fd2804b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3661,9 +3661,9 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)
 		uint8_t frame_sync_cap;
 
 		dev_priv->psr.sink_support = true;
-		drm_dp_dpcd_read(&intel_dp->aux,
-				 DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
-				 &frame_sync_cap, 1);
+		drm_dp_dpcd_readb(&intel_dp->aux,
+				  DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
+				  &frame_sync_cap);
 		dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false;
 		/* PSR2 needs frame sync as well */
 		dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync;
@@ -3737,8 +3737,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 		intel_dp_set_common_rates(intel_dp);
 	}
 
-	if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_COUNT,
-			     &intel_dp->sink_count, 1) < 0)
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_COUNT,
+			      &intel_dp->sink_count) <= 0)
 		return false;
 
 	/*
@@ -3775,7 +3775,7 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 static bool
 intel_dp_can_mst(struct intel_dp *intel_dp)
 {
-	u8 buf[1];
+	u8 mstm_cap;
 
 	if (!i915.enable_dp_mst)
 		return false;
@@ -3786,10 +3786,10 @@ intel_dp_can_mst(struct intel_dp *intel_dp)
 	if (intel_dp->dpcd[DP_DPCD_REV] < 0x12)
 		return false;
 
-	if (drm_dp_dpcd_read(&intel_dp->aux, DP_MSTM_CAP, buf, 1) != 1)
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_MSTM_CAP, &mstm_cap) != 1)
 		return false;
 
-	return buf[0] & DP_MST_CAP;
+	return mstm_cap & DP_MST_CAP;
 }
 
 static void
@@ -3935,9 +3935,8 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
 static bool
 intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector)
 {
-	return drm_dp_dpcd_read(&intel_dp->aux,
-				       DP_DEVICE_SERVICE_IRQ_VECTOR,
-				       sink_irq_vector, 1) == 1;
+	return drm_dp_dpcd_readb(&intel_dp->aux, DP_DEVICE_SERVICE_IRQ_VECTOR,
+				 sink_irq_vector) == 1;
 }
 
 static bool
@@ -4000,13 +3999,13 @@ static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp)
 static uint8_t intel_dp_autotest_video_pattern(struct intel_dp *intel_dp)
 {
 	uint8_t test_pattern;
-	uint16_t test_misc;
+	uint8_t test_misc;
 	__be16 h_width, v_height;
 	int status = 0;
 
 	/* Read the TEST_PATTERN (DP CTS 3.1.5) */
-	status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_PATTERN,
-				  &test_pattern, 1);
+	status = drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_PATTERN,
+				   &test_pattern);
 	if (status <= 0) {
 		DRM_DEBUG_KMS("Test pattern read failed\n");
 		return DP_TEST_NAK;
@@ -4028,8 +4027,8 @@ static uint8_t intel_dp_autotest_video_pattern(struct intel_dp *intel_dp)
 		return DP_TEST_NAK;
 	}
 
-	status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_MISC0,
-				  &test_misc, 1);
+	status = drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_MISC0,
+				   &test_misc);
 	if (status <= 0) {
 		DRM_DEBUG_KMS("TEST MISC read failed\n");
 		return DP_TEST_NAK;
@@ -4088,10 +4087,8 @@ static uint8_t intel_dp_autotest_edid(struct intel_dp *intel_dp)
 		 */
 		block += intel_connector->detect_edid->extensions;
 
-		if (!drm_dp_dpcd_write(&intel_dp->aux,
-					DP_TEST_EDID_CHECKSUM,
-					&block->checksum,
-					1))
+		if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_EDID_CHECKSUM,
+				       block->checksum) <= 0)
 			DRM_DEBUG_KMS("Failed to write EDID checksum\n");
 
 		test_result = DP_TEST_ACK | DP_TEST_EDID_CHECKSUM_WRITE;
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 14/14] drm/i915/dp: read sink count to a temporary variable first

[Jani Nikula]

Don't clobber intel_dp->sink_count with the raw value.

Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 81682fd2804b..3c5c80da9ea3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3728,6 +3728,8 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)
 static bool
 intel_dp_get_dpcd(struct intel_dp *intel_dp)
 {
+	u8 sink_count;
+
 	if (!intel_dp_read_dpcd(intel_dp))
 		return false;
 
@@ -3737,8 +3739,7 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 		intel_dp_set_common_rates(intel_dp);
 	}
 
-	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_COUNT,
-			      &intel_dp->sink_count) <= 0)
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_COUNT, &sink_count) <= 0)
 		return false;
 
 	/*
@@ -3746,7 +3747,7 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 	 * a member variable in intel_dp will track any changes
 	 * between short pulse interrupts.
 	 */
-	intel_dp->sink_count = DP_GET_SINK_COUNT(intel_dp->sink_count);
+	intel_dp->sink_count = DP_GET_SINK_COUNT(sink_count);
 
 	/*
 	 * SINK_COUNT == 0 and DOWNSTREAM_PORT_PRESENT == 1 implies that
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/dp: link rate and lane count refactoring (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915/dp: link rate and lane count refactoring (rev3)
URL   : https://patchwork.freedesktop.org/series/18359/
State : success

== Summary ==

Series 18359v3 drm/i915/dp: link rate and lane count refactoring
https://patchwork.freedesktop.org/api/1.0/series/18359/revisions/3/mbox/

Test gem_exec_suspend:
        Subgroup basic-s3:
                incomplete -> PASS       (fi-skl-6260u)
        Subgroup basic-s4-devices:
                dmesg-warn -> PASS       (fi-kbl-7560u) fdo#100428

fdo#100428 https://bugs.freedesktop.org/show_bug.cgi?id=100428

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 462s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time: 462s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 580s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 542s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 581s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 505s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 503s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 432s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 440s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 437s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 520s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 494s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 485s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 591s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 475s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 606s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 489s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 517s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time: 465s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 556s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 429s

dabd992961047cf26698036f563aa86a083284ac drm-tip: 2017y-03m-28d-15h-25m-54s UTC integration manifest
9198096 drm/i915/dp: read sink count to a temporary variable first
73c8492 drm/i915/dp: use readb and writeb calls for single byte DPCD access
0dfcae5 drm/i915/dp: localize link rate index variable more
927edbe drm/i915/mst: use max link not sink lane count
ad7b47c drm/i915/dp: add functions for max common link rate and lane count
8e49432 drm/i915/dp: don't call the link parameters sink parameters
f282076 drm/i915/dp: do not limit rate seek when not needed
c6d445d drm/i915/dp: cache common rates with sink rates
2b0af8d drm/i915/dp: use the sink rates array for max sink rates
31f8e23 drm/i915/dp: generate and cache sink rate array for all DP, not just eDP 1.4
1c46c61 drm/i915/dp: cache source rates at init
3ec14d5 drm/i915/dp: rename rate_to_index() to intel_dp_rate_index() and reuse
9e6a1d9 drm/i915/dp: return errors from rate_to_index()
deb82e8 drm/i915/dp: use known correct array size in rate_to_index

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4326/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 02/14] drm/i915/dp: return errors from rate_to_index()

[Manasi Navare]

Won't it make more sense to squash this patch with Patch 01 in this series?
When i was reading Patch 1, I almost was going to comment about
handling the case where we dont find the index..

Regards
Manasi

On Tue, Mar 28, 2017 at 05:59:02PM +0300, Jani Nikula wrote:
> We shouldn't silently use the first element if we can't find the rate
> we're looking for. Make rate_to_index() more generally useful, and
> fallback to the first element in the caller, with a big warning.
> 
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 88c708b07c70..0e200a37b75b 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1544,9 +1544,9 @@ static int rate_to_index(const int *rates, int len, int rate)
>  
>  	for (i = 0; i < len; i++)
>  		if (rate == rates[i])
> -			break;
> +			return i;
>  
> -	return i;
> +	return -1;
>  }
>  
>  int
> @@ -1564,8 +1564,13 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp)
>  
>  int intel_dp_rate_select(struct intel_dp *intel_dp, int rate)
>  {
> -	return rate_to_index(intel_dp->sink_rates, intel_dp->num_sink_rates,
> -			     rate);
> +	int i = rate_to_index(intel_dp->sink_rates, intel_dp->num_sink_rates,
> +			      rate);
> +
> +	if (WARN_ON(i < 0))
> +		i = 0;
> +
> +	return i;
>  }
>  
>  void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 08/14] drm/i915/dp: do not limit rate seek when not needed

[Manasi Navare]

On Tue, Mar 28, 2017 at 05:59:08PM +0300, Jani Nikula wrote:
> In link training fallback, we're trying to find a rate that we know is
> in a sorted array of common link rates. We don't need to limit the array
> using the max rate. For test request, the DP CTS doesn't say we should
> limit the rate based on earlier fallback.
>

The commit message should probably mention that you are removing
intel_dp_link_rate_index function completely and using
intel_dp_rate_index() instead.

Regards
Manasi

 
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 31 ++++++++++++-------------------
>  1 file changed, 12 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 1808af6d635d..8c061c54d481 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -317,25 +317,16 @@ static int intel_dp_common_len_rate_limit(struct intel_dp *intel_dp,
>  	return 0;
>  }
>  
> -static int intel_dp_link_rate_index(struct intel_dp *intel_dp, int link_rate)
> -{
> -	int common_len;
> -
> -	common_len = intel_dp_common_len_rate_limit(intel_dp,
> -						    intel_dp->max_sink_link_rate);
> -
> -	return intel_dp_rate_index(intel_dp->common_rates, common_len, link_rate);
> -}
> -
>  int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
>  					    int link_rate, uint8_t lane_count)
>  {
> -	const int *common_rates = intel_dp->common_rates;
> -	int link_rate_index;
> +	int index;
>  
> -	link_rate_index = intel_dp_link_rate_index(intel_dp, link_rate);
> -	if (link_rate_index > 0) {
> -		intel_dp->max_sink_link_rate = common_rates[link_rate_index - 1];
> +	index = intel_dp_rate_index(intel_dp->common_rates,
> +				    intel_dp->num_common_rates,
> +				    link_rate);
> +	if (index > 0) {
> +		intel_dp->max_sink_link_rate = intel_dp->common_rates[index - 1];
>  		intel_dp->max_sink_lane_count = lane_count;
>  	} else if (lane_count > 1) {
>  		intel_dp->max_sink_link_rate = intel_dp_max_sink_rate(intel_dp);
> @@ -1685,8 +1676,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  
>  	/* Use values requested by Compliance Test Request */
>  	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
> -		link_rate_index = intel_dp_link_rate_index(intel_dp,
> -							   intel_dp->compliance.test_link_rate);
> +		link_rate_index = intel_dp_rate_index(intel_dp->common_rates,
> +						      intel_dp->num_common_rates,
> +						      intel_dp->compliance.test_link_rate);
>  		if (link_rate_index >= 0)
>  			min_clock = max_clock = link_rate_index;
>  		min_lane_count = max_lane_count = intel_dp->compliance.test_lane_count;
> @@ -3988,8 +3980,9 @@ static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp)
>  	}
>  	/* Validate the requested link rate */
>  	test_link_rate = drm_dp_bw_code_to_link_rate(test_link_bw);
> -	link_rate_index = intel_dp_link_rate_index(intel_dp,
> -						   test_link_rate);
> +	link_rate_index = intel_dp_rate_index(intel_dp->common_rates,
> +					      intel_dp->num_common_rates,
> +					      test_link_rate);
>  	if (link_rate_index < 0)
>  		return DP_TEST_NAK;
>  
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 09/14] drm/i915/dp: don't call the link parameters sink parameters

[Manasi Navare]

I agree this definitely minimizes the confusion! Thanks for this patch.

Regards
Manasi

On Tue, Mar 28, 2017 at 05:59:09PM +0300, Jani Nikula wrote:
> If we modify these on the fly depending on the link conditions, don't
> pretend they are sink properties.
> 
> Some link vs. sink confusion still remains, but we'll take care of them
> in follow-up patches.
> 
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
 Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>


> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 25 ++++++++++++-------------
>  drivers/gpu/drm/i915/intel_drv.h |  8 ++++----
>  2 files changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 8c061c54d481..a0082a3784e8 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -172,7 +172,7 @@ static u8 intel_dp_max_lane_count(struct intel_dp *intel_dp)
>  	u8 source_max, sink_max;
>  
>  	source_max = intel_dig_port->max_lanes;
> -	sink_max = intel_dp->max_sink_lane_count;
> +	sink_max = intel_dp->max_link_lane_count;
>  
>  	return min(source_max, sink_max);
>  }
> @@ -326,11 +326,11 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
>  				    intel_dp->num_common_rates,
>  				    link_rate);
>  	if (index > 0) {
> -		intel_dp->max_sink_link_rate = intel_dp->common_rates[index - 1];
> -		intel_dp->max_sink_lane_count = lane_count;
> +		intel_dp->max_link_rate = intel_dp->common_rates[index - 1];
> +		intel_dp->max_link_lane_count = lane_count;
>  	} else if (lane_count > 1) {
> -		intel_dp->max_sink_link_rate = intel_dp_max_sink_rate(intel_dp);
> -		intel_dp->max_sink_lane_count = lane_count >> 1;
> +		intel_dp->max_link_rate = intel_dp_max_sink_rate(intel_dp);
> +		intel_dp->max_link_lane_count = lane_count >> 1;
>  	} else {
>  		DRM_ERROR("Link Training Unsuccessful\n");
>  		return -1;
> @@ -1561,8 +1561,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_sink_link_rate);
> +	len = intel_dp_common_len_rate_limit(intel_dp, intel_dp->max_link_rate);
>  	if (WARN_ON(len <= 0))
>  		return 162000;
>  
> @@ -1639,7 +1638,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	uint8_t link_bw, rate_select;
>  
>  	common_len = intel_dp_common_len_rate_limit(intel_dp,
> -						    intel_dp->max_sink_link_rate);
> +						    intel_dp->max_link_rate);
>  
>  	/* No common link rates between source and sink */
>  	WARN_ON(common_len <= 0);
> @@ -3969,7 +3968,7 @@ static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp)
>  	test_lane_count &= DP_MAX_LANE_COUNT_MASK;
>  	/* Validate the requested lane count */
>  	if (test_lane_count < min_lane_count ||
> -	    test_lane_count > intel_dp->max_sink_lane_count)
> +	    test_lane_count > intel_dp->max_link_lane_count)
>  		return DP_TEST_NAK;
>  
>  	status = drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_LINK_RATE,
> @@ -4637,11 +4636,11 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>  		      yesno(drm_dp_tps3_supported(intel_dp->dpcd)));
>  
>  	if (intel_dp->reset_link_params) {
> -		/* Set the max lane count for sink */
> -		intel_dp->max_sink_lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
> +		/* Set the max lane count for link */
> +		intel_dp->max_link_lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
>  
> -		/* Set the max link rate for sink */
> -		intel_dp->max_sink_link_rate = intel_dp_max_sink_rate(intel_dp);
> +		/* Set the max link rate for link */
> +		intel_dp->max_link_rate = intel_dp_max_sink_rate(intel_dp);
>  
>  		intel_dp->reset_link_params = false;
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index ec8985b20616..9141515e4204 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -959,10 +959,10 @@ 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 sink as per DPCD registers */
> -	uint8_t max_sink_lane_count;
> -	/* Max link BW for the sink as per DPCD registers */
> -	int max_sink_link_rate;
> +	/* Max lane count for the current link */
> +	int max_link_lane_count;
> +	/* Max rate for the current link */
> +	int max_link_rate;
>  	/* sink or branch descriptor */
>  	struct intel_dp_desc desc;
>  	struct drm_dp_aux aux;
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 10/14] drm/i915/dp: add functions for max common link rate and lane count

[Manasi Navare]

On Tue, Mar 28, 2017 at 05:59:10PM +0300, Jani Nikula wrote:
> These are the theoretical maximums common for source and sink. These are
> the maximums we should start with. They may be degraded in case of link
> training failures, and the dynamic link values are stored separately.
> 
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

 Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 29 +++++++++++++++++------------
>  1 file changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index a0082a3784e8..b3df2082eac9 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -161,22 +161,27 @@ static void intel_dp_set_sink_rates(struct intel_dp *intel_dp)
>  	intel_dp->num_sink_rates = num_rates;
>  }
>  
> -static int intel_dp_max_sink_rate(struct intel_dp *intel_dp)
> +/* Theoretical max between source and sink */
> +static int intel_dp_max_common_rate(struct intel_dp *intel_dp)
>  {
> -	return intel_dp->sink_rates[intel_dp->num_sink_rates - 1];
> +	return intel_dp->common_rates[intel_dp->num_common_rates - 1];
>  }
>  
> -static u8 intel_dp_max_lane_count(struct intel_dp *intel_dp)
> +/* Theoretical max between source and sink */
> +static int intel_dp_max_common_lane_count(struct intel_dp *intel_dp)
>  {
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> -	u8 source_max, sink_max;
> -
> -	source_max = intel_dig_port->max_lanes;
> -	sink_max = intel_dp->max_link_lane_count;
> +	int source_max = intel_dig_port->max_lanes;
> +	int sink_max = drm_dp_max_lane_count(intel_dp->dpcd);
>  
>  	return min(source_max, sink_max);
>  }
>  
> +static int intel_dp_max_lane_count(struct intel_dp *intel_dp)
> +{
> +	return intel_dp->max_link_lane_count;
> +}
> +
>  int
>  intel_dp_link_required(int pixel_clock, int bpp)
>  {
> @@ -329,7 +334,7 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
>  		intel_dp->max_link_rate = intel_dp->common_rates[index - 1];
>  		intel_dp->max_link_lane_count = lane_count;
>  	} else if (lane_count > 1) {
> -		intel_dp->max_link_rate = intel_dp_max_sink_rate(intel_dp);
> +		intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
>  		intel_dp->max_link_lane_count = lane_count >> 1;
>  	} else {
>  		DRM_ERROR("Link Training Unsuccessful\n");
> @@ -4636,11 +4641,11 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>  		      yesno(drm_dp_tps3_supported(intel_dp->dpcd)));
>  
>  	if (intel_dp->reset_link_params) {
> -		/* Set the max lane count for link */
> -		intel_dp->max_link_lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
> +		/* Initial max link lane count */
> +		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
>  
> -		/* Set the max link rate for link */
> -		intel_dp->max_link_rate = intel_dp_max_sink_rate(intel_dp);
> +		/* Initial max link rate */
> +		intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
>  
>  		intel_dp->reset_link_params = false;
>  	}
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 11/14] drm/i915/mst: use max link not sink lane count

[Manasi Navare]

On Tue, Mar 28, 2017 at 05:59:11PM +0300, Jani Nikula wrote:
> The source might not support as many lanes as the sink, or the link
> training might have failed at higher lane counts. Take these into
> account.
>

Yes that is true for link fallback to work correctly for MST.
So Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Regards
Manasi


> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c     | 2 +-
>  drivers/gpu/drm/i915/intel_dp_mst.c | 4 ++--
>  drivers/gpu/drm/i915/intel_drv.h    | 1 +
>  3 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b3df2082eac9..95f2278700e3 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -177,7 +177,7 @@ static int intel_dp_max_common_lane_count(struct intel_dp *intel_dp)
>  	return min(source_max, sink_max);
>  }
>  
> -static int intel_dp_max_lane_count(struct intel_dp *intel_dp)
> +int intel_dp_max_lane_count(struct intel_dp *intel_dp)
>  {
>  	return intel_dp->max_link_lane_count;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index c1f62eb07c07..3451e2abb23b 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -56,7 +56,7 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  	 * for MST we always configure max link bw - the spec doesn't
>  	 * seem to suggest we should do otherwise.
>  	 */
> -	lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
> +	lane_count = intel_dp_max_lane_count(intel_dp);
>  
>  	pipe_config->lane_count = lane_count;
>  
> @@ -343,7 +343,7 @@ intel_dp_mst_mode_valid(struct drm_connector *connector,
>  	int max_rate, mode_rate, max_lanes, max_link_clock;
>  
>  	max_link_clock = intel_dp_max_link_rate(intel_dp);
> -	max_lanes = drm_dp_max_lane_count(intel_dp->dpcd);
> +	max_lanes = intel_dp_max_lane_count(intel_dp);
>  
>  	max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes);
>  	mode_rate = intel_dp_link_required(mode->clock, bpp);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 9141515e4204..0c037295459b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1507,6 +1507,7 @@ void intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *co
>  void intel_dp_mst_suspend(struct drm_device *dev);
>  void intel_dp_mst_resume(struct drm_device *dev);
>  int intel_dp_max_link_rate(struct intel_dp *intel_dp);
> +int intel_dp_max_lane_count(struct intel_dp *intel_dp);
>  int intel_dp_rate_select(struct intel_dp *intel_dp, int rate);
>  void intel_dp_hot_plug(struct intel_encoder *intel_encoder);
>  void intel_power_sequencer_reset(struct drm_i915_private *dev_priv);
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 12/14] drm/i915/dp: localize link rate index variable more

[Manasi Navare]

Why not squash this with patch 08/14? and call it cleanup for obtaining the
rate index using intel_dp_rate_index()
Just a thought..

Regards
Manasi

On Tue, Mar 28, 2017 at 05:59:12PM +0300, Jani Nikula wrote:
> Localize link_rate_index to the if block, and rename to just index to
> reduce indent.
> 
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 95f2278700e3..6f743490855b 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1636,7 +1636,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	/* Conveniently, the link BW constants become indices with a shift...*/
>  	int min_clock = 0;
>  	int max_clock;
> -	int link_rate_index;
>  	int bpp, mode_rate;
>  	int link_avail, link_clock;
>  	int common_len;
> @@ -1680,11 +1679,13 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  
>  	/* Use values requested by Compliance Test Request */
>  	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
> -		link_rate_index = intel_dp_rate_index(intel_dp->common_rates,
> -						      intel_dp->num_common_rates,
> -						      intel_dp->compliance.test_link_rate);
> -		if (link_rate_index >= 0)
> -			min_clock = max_clock = link_rate_index;
> +		int index;
> +
> +		index = intel_dp_rate_index(intel_dp->common_rates,
> +					    intel_dp->num_common_rates,
> +					    intel_dp->compliance.test_link_rate);
> +		if (index >= 0)
> +			min_clock = max_clock = index;
>  		min_lane_count = max_lane_count = intel_dp->compliance.test_lane_count;
>  	}
>  	DRM_DEBUG_KMS("DP link computation with max lane count %i "
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 02/14] drm/i915/dp: return errors from rate_to_index()

[Jani Nikula]

On Tue, 28 Mar 2017, Manasi Navare <manasi.d.navare@intel.com> wrote:
> Won't it make more sense to squash this patch with Patch 01 in this series?
> When i was reading Patch 1, I almost was going to comment about
> handling the case where we dont find the index..

Matter of taste. You might then look at patch 3 and figure it should be
squashed too... you have to draw the line somewhere, and I think it's
always easier to review smaller pieces and be sure they are each
correct. If you find an issue with the 2nd patch, the 1st one is still
valid, and the follow-up review is also easier.

BR,
Jani.


>
> Regards
> Manasi
>
> On Tue, Mar 28, 2017 at 05:59:02PM +0300, Jani Nikula wrote:
>> We shouldn't silently use the first element if we can't find the rate
>> we're looking for. Make rate_to_index() more generally useful, and
>> fallback to the first element in the caller, with a big warning.
>> 
>> Cc: Manasi Navare <manasi.d.navare@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c | 13 +++++++++----
>>  1 file changed, 9 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 88c708b07c70..0e200a37b75b 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -1544,9 +1544,9 @@ static int rate_to_index(const int *rates, int len, int rate)
>>  
>>  	for (i = 0; i < len; i++)
>>  		if (rate == rates[i])
>> -			break;
>> +			return i;
>>  
>> -	return i;
>> +	return -1;
>>  }
>>  
>>  int
>> @@ -1564,8 +1564,13 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp)
>>  
>>  int intel_dp_rate_select(struct intel_dp *intel_dp, int rate)
>>  {
>> -	return rate_to_index(intel_dp->sink_rates, intel_dp->num_sink_rates,
>> -			     rate);
>> +	int i = rate_to_index(intel_dp->sink_rates, intel_dp->num_sink_rates,
>> +			      rate);
>> +
>> +	if (WARN_ON(i < 0))
>> +		i = 0;
>> +
>> +	return i;
>>  }
>>  
>>  void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
>> -- 
>> 2.1.4
>> 

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 12/14] drm/i915/dp: localize link rate index variable more

[Jani Nikula]

On Wed, 29 Mar 2017, Manasi Navare <manasi.d.navare@intel.com> wrote:
> Why not squash this with patch 08/14? and call it cleanup for obtaining the
> rate index using intel_dp_rate_index()
> Just a thought..

Again, usually too big patches are the problem, not the other way
around.

BR,
Jani.

>
> Regards
> Manasi
>
> On Tue, Mar 28, 2017 at 05:59:12PM +0300, Jani Nikula wrote:
>> Localize link_rate_index to the if block, and rename to just index to
>> reduce indent.
>> 
>> Cc: Manasi Navare <manasi.d.navare@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c | 13 +++++++------
>>  1 file changed, 7 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 95f2278700e3..6f743490855b 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -1636,7 +1636,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>>  	/* Conveniently, the link BW constants become indices with a shift...*/
>>  	int min_clock = 0;
>>  	int max_clock;
>> -	int link_rate_index;
>>  	int bpp, mode_rate;
>>  	int link_avail, link_clock;
>>  	int common_len;
>> @@ -1680,11 +1679,13 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>>  
>>  	/* Use values requested by Compliance Test Request */
>>  	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
>> -		link_rate_index = intel_dp_rate_index(intel_dp->common_rates,
>> -						      intel_dp->num_common_rates,
>> -						      intel_dp->compliance.test_link_rate);
>> -		if (link_rate_index >= 0)
>> -			min_clock = max_clock = link_rate_index;
>> +		int index;
>> +
>> +		index = intel_dp_rate_index(intel_dp->common_rates,
>> +					    intel_dp->num_common_rates,
>> +					    intel_dp->compliance.test_link_rate);
>> +		if (index >= 0)
>> +			min_clock = max_clock = index;
>>  		min_lane_count = max_lane_count = intel_dp->compliance.test_lane_count;
>>  	}
>>  	DRM_DEBUG_KMS("DP link computation with max lane count %i "
>> -- 
>> 2.1.4
>> 

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915/dp: do not limit rate seek when not needed

[Jani Nikula]

In link training fallback, we're trying to find a rate that we know is
in a sorted array of common link rates. We don't need to limit the array
using the max rate. For test request, the DP CTS doesn't say we should
limit the rate based on earlier fallback. This lets us get rid of
intel_dp_link_rate_index() and use intel_dp_rate_index() instead.

Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>

---

updated commit message, no changes
---
 drivers/gpu/drm/i915/intel_dp.c | 31 ++++++++++++-------------------
 1 file changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1808af6d635d..8c061c54d481 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -317,25 +317,16 @@ static int intel_dp_common_len_rate_limit(struct intel_dp *intel_dp,
 	return 0;
 }
 
-static int intel_dp_link_rate_index(struct intel_dp *intel_dp, int link_rate)
-{
-	int common_len;
-
-	common_len = intel_dp_common_len_rate_limit(intel_dp,
-						    intel_dp->max_sink_link_rate);
-
-	return intel_dp_rate_index(intel_dp->common_rates, common_len, link_rate);
-}
-
 int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
 					    int link_rate, uint8_t lane_count)
 {
-	const int *common_rates = intel_dp->common_rates;
-	int link_rate_index;
+	int index;
 
-	link_rate_index = intel_dp_link_rate_index(intel_dp, link_rate);
-	if (link_rate_index > 0) {
-		intel_dp->max_sink_link_rate = common_rates[link_rate_index - 1];
+	index = intel_dp_rate_index(intel_dp->common_rates,
+				    intel_dp->num_common_rates,
+				    link_rate);
+	if (index > 0) {
+		intel_dp->max_sink_link_rate = intel_dp->common_rates[index - 1];
 		intel_dp->max_sink_lane_count = lane_count;
 	} else if (lane_count > 1) {
 		intel_dp->max_sink_link_rate = intel_dp_max_sink_rate(intel_dp);
@@ -1685,8 +1676,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 
 	/* Use values requested by Compliance Test Request */
 	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
-		link_rate_index = intel_dp_link_rate_index(intel_dp,
-							   intel_dp->compliance.test_link_rate);
+		link_rate_index = intel_dp_rate_index(intel_dp->common_rates,
+						      intel_dp->num_common_rates,
+						      intel_dp->compliance.test_link_rate);
 		if (link_rate_index >= 0)
 			min_clock = max_clock = link_rate_index;
 		min_lane_count = max_lane_count = intel_dp->compliance.test_lane_count;
@@ -3988,8 +3980,9 @@ static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp)
 	}
 	/* Validate the requested link rate */
 	test_link_rate = drm_dp_bw_code_to_link_rate(test_link_bw);
-	link_rate_index = intel_dp_link_rate_index(intel_dp,
-						   test_link_rate);
+	link_rate_index = intel_dp_rate_index(intel_dp->common_rates,
+					      intel_dp->num_common_rates,
+					      test_link_rate);
 	if (link_rate_index < 0)
 		return DP_TEST_NAK;
 
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/dp: link rate and lane count refactoring (rev4)

[Patchwork]

== Series Details ==

Series: drm/i915/dp: link rate and lane count refactoring (rev4)
URL   : https://patchwork.freedesktop.org/series/18359/
State : success

== Summary ==

Series 18359v4 drm/i915/dp: link rate and lane count refactoring
https://patchwork.freedesktop.org/api/1.0/series/18359/revisions/4/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                fail       -> PASS       (fi-snb-2600) fdo#100007
Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                dmesg-warn -> PASS       (fi-bxt-t5700) fdo#100125
Test kms_force_connector_basic:
        Subgroup force-connector-state:
                skip       -> PASS       (fi-ivb-3520m)
        Subgroup force-edid:
                skip       -> PASS       (fi-ivb-3520m)
        Subgroup force-load-detect:
                skip       -> PASS       (fi-ivb-3520m)
        Subgroup prune-stale-modes:
                skip       -> PASS       (fi-ivb-3520m)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                dmesg-warn -> PASS       (fi-kbl-7560u)

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 467s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time: 459s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 592s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 533s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 588s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 505s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 431s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 433s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 447s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 506s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 497s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 484s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 588s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 479s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 597s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 488s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 525s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time: 462s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 552s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 423s

486e7f49dc0d278c62a809532e4b645ddf9a3c25 drm-tip: 2017y-03m-29d-08h-01m-04s UTC integration manifest
1f9f4e8 drm/i915/dp: read sink count to a temporary variable first
0d220ad drm/i915/dp: use readb and writeb calls for single byte DPCD access
a703dc7 drm/i915/dp: localize link rate index variable more
10fc0d2 drm/i915/mst: use max link not sink lane count
8534706 drm/i915/dp: add functions for max common link rate and lane count
c1b096f drm/i915/dp: don't call the link parameters sink parameters
73552e9 drm/i915/dp: do not limit rate seek when not needed
4e48a12 drm/i915/dp: cache common rates with sink rates
37f9a56 drm/i915/dp: use the sink rates array for max sink rates
0342da6 drm/i915/dp: generate and cache sink rate array for all DP, not just eDP 1.4
a1d0fa1 drm/i915/dp: cache source rates at init
e3793b2 drm/i915/dp: rename rate_to_index() to intel_dp_rate_index() and reuse
32d79ba drm/i915/dp: return errors from rate_to_index()
25bff2e drm/i915/dp: use known correct array size in rate_to_index

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4335/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 01/14] drm/i915/dp: use known correct array size in rate_to_index

[Manasi Navare]

Looks good to me.

Reviewed-by: <manasi.d.navare@intel.com>

Manasi

On Tue, Mar 28, 2017 at 05:59:01PM +0300, Jani Nikula wrote:
> I can't think of a real world bug this could cause now, but this will be
> required in follow-up work. While at it, change the parameter order to
> be slightly more sensible.
> 
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index fd96a6cf7326..88c708b07c70 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1538,12 +1538,12 @@ bool intel_dp_read_desc(struct intel_dp *intel_dp)
>  	return true;
>  }
>  
> -static int rate_to_index(int find, const int *rates)
> +static int rate_to_index(const int *rates, int len, int rate)
>  {
> -	int i = 0;
> +	int i;
>  
> -	for (i = 0; i < DP_MAX_SUPPORTED_RATES; ++i)
> -		if (find == rates[i])
> +	for (i = 0; i < len; i++)
> +		if (rate == rates[i])
>  			break;
>  
>  	return i;
> @@ -1564,7 +1564,8 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp)
>  
>  int intel_dp_rate_select(struct intel_dp *intel_dp, int rate)
>  {
> -	return rate_to_index(rate, intel_dp->sink_rates);
> +	return rate_to_index(intel_dp->sink_rates, intel_dp->num_sink_rates,
> +			     rate);
>  }
>  
>  void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 02/14] drm/i915/dp: return errors from rate_to_index()

[Manasi Navare]

On Wed, Mar 29, 2017 at 10:17:41AM +0300, Jani Nikula wrote:
> On Tue, 28 Mar 2017, Manasi Navare <manasi.d.navare@intel.com> wrote:
> > Won't it make more sense to squash this patch with Patch 01 in this series?
> > When i was reading Patch 1, I almost was going to comment about
> > handling the case where we dont find the index..
> 
> Matter of taste. You might then look at patch 3 and figure it should be
> squashed too... you have to draw the line somewhere, and I think it's
> always easier to review smaller pieces and be sure they are each
> correct. If you find an issue with the 2nd patch, the 1st one is still
> valid, and the follow-up review is also easier.
> 
> BR,
> Jani.
>

OK. Agreed.

Regards
Manasi 
> 
> >
> > Regards
> > Manasi
> >
> > On Tue, Mar 28, 2017 at 05:59:02PM +0300, Jani Nikula wrote:
> >> We shouldn't silently use the first element if we can't find the rate
> >> we're looking for. Make rate_to_index() more generally useful, and
> >> fallback to the first element in the caller, with a big warning.
> >> 
> >> Cc: Manasi Navare <manasi.d.navare@intel.com>
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
  Reviewed-by: <manasi.d.navare@intel.com>

> >> ---
> >>  drivers/gpu/drm/i915/intel_dp.c | 13 +++++++++----
> >>  1 file changed, 9 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >> index 88c708b07c70..0e200a37b75b 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -1544,9 +1544,9 @@ static int rate_to_index(const int *rates, int len, int rate)
> >>  
> >>  	for (i = 0; i < len; i++)
> >>  		if (rate == rates[i])
> >> -			break;
> >> +			return i;
> >>  
> >> -	return i;
> >> +	return -1;
> >>  }
> >>  
> >>  int
> >> @@ -1564,8 +1564,13 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp)
> >>  
> >>  int intel_dp_rate_select(struct intel_dp *intel_dp, int rate)
> >>  {
> >> -	return rate_to_index(intel_dp->sink_rates, intel_dp->num_sink_rates,
> >> -			     rate);
> >> +	int i = rate_to_index(intel_dp->sink_rates, intel_dp->num_sink_rates,
> >> +			      rate);
> >> +
> >> +	if (WARN_ON(i < 0))
> >> +		i = 0;
> >> +
> >> +	return i;
> >>  }
> >>  
> >>  void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
> >> -- 
> >> 2.1.4
> >> 
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 03/14] drm/i915/dp: rename rate_to_index() to intel_dp_rate_index() and reuse

[Manasi Navare]

On Tue, Mar 28, 2017 at 05:59:03PM +0300, Jani Nikula wrote:
> Rename the function, move it at the top, and reuse in
> intel_dp_link_rate_index(). If there was a reason in the past to use
> reverse search order here, there isn't now.
> 
> The names may be slightly confusing now, but intel_dp_link_rate_index()
> will go away in follow-up patches.
> 
> v2: Use name intel_dp_rate_index (Dhinakaran)
> 
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
   Reviewed-by: <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 34 +++++++++++++++-------------------
>  1 file changed, 15 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 0e200a37b75b..9fc066dda4e0 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -266,6 +266,18 @@ static int intersect_rates(const int *source_rates, int source_len,
>  	return k;
>  }
>  
> +/* return index of rate in rates array, or -1 if not found */
> +static int intel_dp_rate_index(const int *rates, int len, int rate)
> +{
> +	int i;
> +
> +	for (i = 0; i < len; i++)
> +		if (rate == rates[i])
> +			return i;
> +
> +	return -1;
> +}
> +
>  static int intel_dp_common_rates(struct intel_dp *intel_dp,
>  				 int *common_rates)
>  {
> @@ -284,15 +296,10 @@ static int intel_dp_link_rate_index(struct intel_dp *intel_dp,
>  				    int *common_rates, int link_rate)
>  {
>  	int common_len;
> -	int index;
>  
>  	common_len = intel_dp_common_rates(intel_dp, common_rates);
> -	for (index = 0; index < common_len; index++) {
> -		if (link_rate == common_rates[common_len - index - 1])
> -			return common_len - index - 1;
> -	}
>  
> -	return -1;
> +	return intel_dp_rate_index(common_rates, common_len, link_rate);
>  }
>  
>  int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> @@ -1538,17 +1545,6 @@ bool intel_dp_read_desc(struct intel_dp *intel_dp)
>  	return true;
>  }
>  
> -static int rate_to_index(const int *rates, int len, int rate)
> -{
> -	int i;
> -
> -	for (i = 0; i < len; i++)
> -		if (rate == rates[i])
> -			return i;
> -
> -	return -1;
> -}
> -
>  int
>  intel_dp_max_link_rate(struct intel_dp *intel_dp)
>  {
> @@ -1564,8 +1560,8 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp)
>  
>  int intel_dp_rate_select(struct intel_dp *intel_dp, int rate)
>  {
> -	int i = rate_to_index(intel_dp->sink_rates, intel_dp->num_sink_rates,
> -			      rate);
> +	int i = intel_dp_rate_index(intel_dp->sink_rates,
> +				    intel_dp->num_sink_rates, rate);
>  
>  	if (WARN_ON(i < 0))
>  		i = 0;
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 04/14] drm/i915/dp: cache source rates at init

[Manasi Navare]

Reviewed-by: <manasi.d.navare@intel.com>

Manasi

On Tue, Mar 28, 2017 at 05:59:04PM +0300, Jani Nikula wrote:
> We need the source rates array so often that it makes sense to set it
> once at init. This reduces function calls when we need the rates, making
> the code easier to follow.
> 
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 35 +++++++++++++++++++++--------------
>  drivers/gpu/drm/i915/intel_drv.h |  3 +++
>  2 files changed, 24 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 9fc066dda4e0..e9bd75ff3904 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -218,21 +218,25 @@ intel_dp_sink_rates(struct intel_dp *intel_dp, const int **sink_rates)
>  	return (intel_dp->max_sink_link_bw >> 3) + 1;
>  }
>  
> -static int
> -intel_dp_source_rates(struct intel_dp *intel_dp, const int **source_rates)
> +static void
> +intel_dp_set_source_rates(struct intel_dp *intel_dp)
>  {
>  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>  	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> +	const int *source_rates;
>  	int size;
>  
> +	/* This should only be done once */
> +	WARN_ON(intel_dp->source_rates || intel_dp->num_source_rates);
> +
>  	if (IS_GEN9_LP(dev_priv)) {
> -		*source_rates = bxt_rates;
> +		source_rates = bxt_rates;
>  		size = ARRAY_SIZE(bxt_rates);
>  	} else if (IS_GEN9_BC(dev_priv)) {
> -		*source_rates = skl_rates;
> +		source_rates = skl_rates;
>  		size = ARRAY_SIZE(skl_rates);
>  	} else {
> -		*source_rates = default_rates;
> +		source_rates = default_rates;
>  		size = ARRAY_SIZE(default_rates);
>  	}
>  
> @@ -240,7 +244,8 @@ intel_dp_source_rates(struct intel_dp *intel_dp, const int **source_rates)
>  	if (!intel_dp_source_supports_hbr2(intel_dp))
>  		size--;
>  
> -	return size;
> +	intel_dp->source_rates = source_rates;
> +	intel_dp->num_source_rates = size;
>  }
>  
>  static int intersect_rates(const int *source_rates, int source_len,
> @@ -281,13 +286,13 @@ static int intel_dp_rate_index(const int *rates, int len, int rate)
>  static int intel_dp_common_rates(struct intel_dp *intel_dp,
>  				 int *common_rates)
>  {
> -	const int *source_rates, *sink_rates;
> -	int source_len, sink_len;
> +	const int *sink_rates;
> +	int sink_len;
>  
>  	sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
> -	source_len = intel_dp_source_rates(intel_dp, &source_rates);
>  
> -	return intersect_rates(source_rates, source_len,
> +	return intersect_rates(intel_dp->source_rates,
> +			       intel_dp->num_source_rates,
>  			       sink_rates, sink_len,
>  			       common_rates);
>  }
> @@ -1493,16 +1498,16 @@ static void snprintf_int_array(char *str, size_t len,
>  
>  static void intel_dp_print_rates(struct intel_dp *intel_dp)
>  {
> -	const int *source_rates, *sink_rates;
> -	int source_len, sink_len, common_len;
> +	const int *sink_rates;
> +	int sink_len, common_len;
>  	int common_rates[DP_MAX_SUPPORTED_RATES];
>  	char str[128]; /* FIXME: too big for stack? */
>  
>  	if ((drm_debug & DRM_UT_KMS) == 0)
>  		return;
>  
> -	source_len = intel_dp_source_rates(intel_dp, &source_rates);
> -	snprintf_int_array(str, sizeof(str), source_rates, source_len);
> +	snprintf_int_array(str, sizeof(str),
> +			   intel_dp->source_rates, intel_dp->num_source_rates);
>  	DRM_DEBUG_KMS("source rates: %s\n", str);
>  
>  	sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
> @@ -5943,6 +5948,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>  		 intel_dig_port->max_lanes, port_name(port)))
>  		return false;
>  
> +	intel_dp_set_source_rates(intel_dp);
> +
>  	intel_dp->reset_link_params = true;
>  	intel_dp->pps_pipe = INVALID_PIPE;
>  	intel_dp->active_pipe = INVALID_PIPE;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index e24641b559e2..f59574261dc6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -949,6 +949,9 @@ struct intel_dp {
>  	uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
>  	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
>  	uint8_t edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
> +	/* source rates */
> +	int num_source_rates;
> +	const int *source_rates;
>  	/* sink rates as reported by DP_SUPPORTED_LINK_RATES */
>  	uint8_t num_sink_rates;
>  	int sink_rates[DP_MAX_SUPPORTED_RATES];
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 05/14] drm/i915/dp: generate and cache sink rate array for all DP, not just eDP 1.4

[Manasi Navare]

I agree with disentangling the fallback rate limiting
from sink rates and instead just limiting the common_rates array
based on max link rate. 

Reviewed-by: <manasi.d.navare@intel.com>

Manasi

On Tue, Mar 28, 2017 at 05:59:05PM +0300, Jani Nikula wrote:
> There is some conflation related to sink rates, making this change more
> complicated than it would otherwise have to be. There are three changes
> here that are rather difficult to split up:
> 
> 1) Use the intel_dp->sink_rates array for all DP, not just eDP 1.4. We
>    initialize it from DPCD on eDP 1.4 like before, but generate it based
>    on DP_MAX_LINK_RATE on others. This reduces code complexity when we
>    need to use the sink rates; they are all always in the sink_rates
>    array.
> 
> 2) Update the sink rate array whenever we read DPCD, and use the
>    information from there. This increases code readability when we need
>    the sink rates.
> 
> 3) Disentangle fallback rate limiting from sink rates. In the code, the
>    max rate is a dynamic property of the *link*, not of the *sink*. Do
>    the limiting after intersecting the source and sink rates, which are
>    static properties of the devices.
> 
> This paves the way for follow-up refactoring that I've refrained from
> doing here to keep this change as simple as it possibly can.
> 
> v2: introduce use_rate_select and handle non-confirming eDP (Ville)
> 
> v3: don't clobber cached eDP rates on short pulse (Ville)
> 
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c               | 81 ++++++++++++++++++---------
>  drivers/gpu/drm/i915/intel_dp_link_training.c |  3 +-
>  drivers/gpu/drm/i915/intel_drv.h              |  5 +-
>  3 files changed, 61 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index e9bd75ff3904..b38cba7d5abc 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -133,6 +133,34 @@ static void vlv_steal_power_sequencer(struct drm_device *dev,
>  				      enum pipe pipe);
>  static void intel_dp_unset_edid(struct intel_dp *intel_dp);
>  
> +static int intel_dp_num_rates(u8 link_bw_code)
> +{
> +	switch (link_bw_code) {
> +	default:
> +		WARN(1, "invalid max DP link bw val %x, using 1.62Gbps\n",
> +		     link_bw_code);
> +	case DP_LINK_BW_1_62:
> +		return 1;
> +	case DP_LINK_BW_2_7:
> +		return 2;
> +	case DP_LINK_BW_5_4:
> +		return 3;
> +	}
> +}
> +
> +/* update sink rates from dpcd */
> +static void intel_dp_set_sink_rates(struct intel_dp *intel_dp)
> +{
> +	int i, num_rates;
> +
> +	num_rates = intel_dp_num_rates(intel_dp->dpcd[DP_MAX_LINK_RATE]);
> +
> +	for (i = 0; i < num_rates; i++)
> +		intel_dp->sink_rates[i] = default_rates[i];
> +
> +	intel_dp->num_sink_rates = num_rates;
> +}
> +
>  static int
>  intel_dp_max_link_bw(struct intel_dp  *intel_dp)
>  {
> @@ -205,19 +233,6 @@ intel_dp_downstream_max_dotclock(struct intel_dp *intel_dp)
>  	return max_dotclk;
>  }
>  
> -static int
> -intel_dp_sink_rates(struct intel_dp *intel_dp, const int **sink_rates)
> -{
> -	if (intel_dp->num_sink_rates) {
> -		*sink_rates = intel_dp->sink_rates;
> -		return intel_dp->num_sink_rates;
> -	}
> -
> -	*sink_rates = default_rates;
> -
> -	return (intel_dp->max_sink_link_bw >> 3) + 1;
> -}
> -
>  static void
>  intel_dp_set_source_rates(struct intel_dp *intel_dp)
>  {
> @@ -286,15 +301,22 @@ static int intel_dp_rate_index(const int *rates, int len, int rate)
>  static int intel_dp_common_rates(struct intel_dp *intel_dp,
>  				 int *common_rates)
>  {
> -	const int *sink_rates;
> -	int sink_len;
> +	int max_rate = drm_dp_bw_code_to_link_rate(intel_dp->max_sink_link_bw);
> +	int i, common_len;
>  
> -	sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
> +	common_len = intersect_rates(intel_dp->source_rates,
> +				     intel_dp->num_source_rates,
> +				     intel_dp->sink_rates,
> +				     intel_dp->num_sink_rates,
> +				     common_rates);
> +
> +	/* Limit results by potentially reduced max rate */
> +	for (i = 0; i < common_len; i++) {
> +		if (common_rates[common_len - i - 1] <= max_rate)
> +			return common_len - i;
> +	}
>  
> -	return intersect_rates(intel_dp->source_rates,
> -			       intel_dp->num_source_rates,
> -			       sink_rates, sink_len,
> -			       common_rates);
> +	return 0;
>  }
>  
>  static int intel_dp_link_rate_index(struct intel_dp *intel_dp,
> @@ -1498,8 +1520,7 @@ static void snprintf_int_array(char *str, size_t len,
>  
>  static void intel_dp_print_rates(struct intel_dp *intel_dp)
>  {
> -	const int *sink_rates;
> -	int sink_len, common_len;
> +	int common_len;
>  	int common_rates[DP_MAX_SUPPORTED_RATES];
>  	char str[128]; /* FIXME: too big for stack? */
>  
> @@ -1510,8 +1531,8 @@ static void intel_dp_print_rates(struct intel_dp *intel_dp)
>  			   intel_dp->source_rates, intel_dp->num_source_rates);
>  	DRM_DEBUG_KMS("source rates: %s\n", str);
>  
> -	sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
> -	snprintf_int_array(str, sizeof(str), sink_rates, sink_len);
> +	snprintf_int_array(str, sizeof(str),
> +			   intel_dp->sink_rates, intel_dp->num_sink_rates);
>  	DRM_DEBUG_KMS("sink rates: %s\n", str);
>  
>  	common_len = intel_dp_common_rates(intel_dp, common_rates);
> @@ -1577,7 +1598,8 @@ int intel_dp_rate_select(struct intel_dp *intel_dp, int rate)
>  void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
>  			   uint8_t *link_bw, uint8_t *rate_select)
>  {
> -	if (intel_dp->num_sink_rates) {
> +	/* eDP 1.4 rate select method. */
> +	if (intel_dp->use_rate_select) {
>  		*link_bw = 0;
>  		*rate_select =
>  			intel_dp_rate_select(intel_dp, port_clock);
> @@ -3702,6 +3724,11 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)
>  		intel_dp->num_sink_rates = i;
>  	}
>  
> +	if (intel_dp->num_sink_rates)
> +		intel_dp->use_rate_select = true;
> +	else
> +		intel_dp_set_sink_rates(intel_dp);
> +
>  	return true;
>  }
>  
> @@ -3712,6 +3739,10 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  	if (!intel_dp_read_dpcd(intel_dp))
>  		return false;
>  
> +	/* Don't clobber cached eDP rates. */
> +	if (!is_edp(intel_dp))
> +		intel_dp_set_sink_rates(intel_dp);
> +
>  	if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_COUNT,
>  			     &intel_dp->sink_count, 1) < 0)
>  		return false;
> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
> index 0048b520baf7..694ad0ffb523 100644
> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> @@ -146,7 +146,8 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
>  		link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
>  	drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_BW_SET, link_config, 2);
>  
> -	if (intel_dp->num_sink_rates)
> +	/* eDP 1.4 rate select method. */
> +	if (!link_bw)
>  		drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_RATE_SET,
>  				  &rate_select, 1);
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f59574261dc6..77a94aaa98c2 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -952,9 +952,10 @@ struct intel_dp {
>  	/* source rates */
>  	int num_source_rates;
>  	const int *source_rates;
> -	/* sink rates as reported by DP_SUPPORTED_LINK_RATES */
> -	uint8_t num_sink_rates;
> +	/* sink rates as reported by DP_MAX_LINK_RATE/DP_SUPPORTED_LINK_RATES */
> +	int num_sink_rates;
>  	int sink_rates[DP_MAX_SUPPORTED_RATES];
> +	bool use_rate_select;
>  	/* Max lane count for the sink as per DPCD registers */
>  	uint8_t max_sink_lane_count;
>  	/* Max link BW for the sink as per DPCD registers */
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 07/14] drm/i915/dp: cache common rates with sink rates

[Manasi Navare]

Reviewed-by: <manasi.d.navare@intel.com>

Manasi

On Tue, Mar 28, 2017 at 05:59:07PM +0300, Jani Nikula wrote:
> Now that source rates are static and sink rates are updated whenever
> DPCD is updated, we can do and cache the intersection of them whenever
> sink rates are updated. This reduces code complexity, as we don't have
> to keep calling the functions to intersect. We also get rid of several
> common rates arrays on stack.
> 
> Limiting the common rates by a max link rate can be done by picking the
> first N elements of the cached common rates.
> 
> v2: get rid of the local common_rates variable (Manasi)
> v3: don't clobber cached eDP rates on short pulse (Ville)
> 
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 75 ++++++++++++++++++++++------------------
>  drivers/gpu/drm/i915/intel_drv.h |  3 ++
>  2 files changed, 45 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index e4650f1625cc..1808af6d635d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -284,17 +284,29 @@ static int intel_dp_rate_index(const int *rates, int len, int rate)
>  	return -1;
>  }
>  
> -static int intel_dp_common_rates(struct intel_dp *intel_dp,
> -				 int *common_rates)
> +static void intel_dp_set_common_rates(struct intel_dp *intel_dp)
>  {
> -	int max_rate = intel_dp->max_sink_link_rate;
> -	int i, common_len;
> +	WARN_ON(!intel_dp->num_source_rates || !intel_dp->num_sink_rates);
>  
> -	common_len = intersect_rates(intel_dp->source_rates,
> -				     intel_dp->num_source_rates,
> -				     intel_dp->sink_rates,
> -				     intel_dp->num_sink_rates,
> -				     common_rates);
> +	intel_dp->num_common_rates = intersect_rates(intel_dp->source_rates,
> +						     intel_dp->num_source_rates,
> +						     intel_dp->sink_rates,
> +						     intel_dp->num_sink_rates,
> +						     intel_dp->common_rates);
> +
> +	/* Paranoia, there should always be something in common. */
> +	if (WARN_ON(intel_dp->num_common_rates == 0)) {
> +		intel_dp->common_rates[0] = default_rates[0];
> +		intel_dp->num_common_rates = 1;
> +	}
> +}
> +
> +/* get length of common rates potentially limited by max_rate */
> +static int intel_dp_common_len_rate_limit(struct intel_dp *intel_dp,
> +					  int max_rate)
> +{
> +	const int *common_rates = intel_dp->common_rates;
> +	int i, common_len = intel_dp->num_common_rates;
>  
>  	/* Limit results by potentially reduced max rate */
>  	for (i = 0; i < common_len; i++) {
> @@ -305,25 +317,23 @@ static int intel_dp_common_rates(struct intel_dp *intel_dp,
>  	return 0;
>  }
>  
> -static int intel_dp_link_rate_index(struct intel_dp *intel_dp,
> -				    int *common_rates, int link_rate)
> +static int intel_dp_link_rate_index(struct intel_dp *intel_dp, int link_rate)
>  {
>  	int common_len;
>  
> -	common_len = intel_dp_common_rates(intel_dp, common_rates);
> +	common_len = intel_dp_common_len_rate_limit(intel_dp,
> +						    intel_dp->max_sink_link_rate);
>  
> -	return intel_dp_rate_index(common_rates, common_len, link_rate);
> +	return intel_dp_rate_index(intel_dp->common_rates, common_len, link_rate);
>  }
>  
>  int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
>  					    int link_rate, uint8_t lane_count)
>  {
> -	int common_rates[DP_MAX_SUPPORTED_RATES];
> +	const int *common_rates = intel_dp->common_rates;
>  	int link_rate_index;
>  
> -	link_rate_index = intel_dp_link_rate_index(intel_dp,
> -						   common_rates,
> -						   link_rate);
> +	link_rate_index = intel_dp_link_rate_index(intel_dp, link_rate);
>  	if (link_rate_index > 0) {
>  		intel_dp->max_sink_link_rate = common_rates[link_rate_index - 1];
>  		intel_dp->max_sink_lane_count = lane_count;
> @@ -1506,8 +1516,6 @@ static void snprintf_int_array(char *str, size_t len,
>  
>  static void intel_dp_print_rates(struct intel_dp *intel_dp)
>  {
> -	int common_len;
> -	int common_rates[DP_MAX_SUPPORTED_RATES];
>  	char str[128]; /* FIXME: too big for stack? */
>  
>  	if ((drm_debug & DRM_UT_KMS) == 0)
> @@ -1521,8 +1529,8 @@ static void intel_dp_print_rates(struct intel_dp *intel_dp)
>  			   intel_dp->sink_rates, intel_dp->num_sink_rates);
>  	DRM_DEBUG_KMS("sink rates: %s\n", str);
>  
> -	common_len = intel_dp_common_rates(intel_dp, common_rates);
> -	snprintf_int_array(str, sizeof(str), common_rates, common_len);
> +	snprintf_int_array(str, sizeof(str),
> +			   intel_dp->common_rates, intel_dp->num_common_rates);
>  	DRM_DEBUG_KMS("common rates: %s\n", str);
>  }
>  
> @@ -1560,14 +1568,14 @@ bool intel_dp_read_desc(struct intel_dp *intel_dp)
>  int
>  intel_dp_max_link_rate(struct intel_dp *intel_dp)
>  {
> -	int rates[DP_MAX_SUPPORTED_RATES] = {};
>  	int len;
>  
> -	len = intel_dp_common_rates(intel_dp, rates);
> +	len = intel_dp_common_len_rate_limit(intel_dp,
> +					     intel_dp->max_sink_link_rate);
>  	if (WARN_ON(len <= 0))
>  		return 162000;
>  
> -	return rates[len - 1];
> +	return intel_dp->common_rates[len - 1];
>  }
>  
>  int intel_dp_rate_select(struct intel_dp *intel_dp, int rate)
> @@ -1636,11 +1644,11 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	int link_rate_index;
>  	int bpp, mode_rate;
>  	int link_avail, link_clock;
> -	int common_rates[DP_MAX_SUPPORTED_RATES] = {};
>  	int common_len;
>  	uint8_t link_bw, rate_select;
>  
> -	common_len = intel_dp_common_rates(intel_dp, common_rates);
> +	common_len = intel_dp_common_len_rate_limit(intel_dp,
> +						    intel_dp->max_sink_link_rate);
>  
>  	/* No common link rates between source and sink */
>  	WARN_ON(common_len <= 0);
> @@ -1678,7 +1686,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	/* Use values requested by Compliance Test Request */
>  	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
>  		link_rate_index = intel_dp_link_rate_index(intel_dp,
> -							   common_rates,
>  							   intel_dp->compliance.test_link_rate);
>  		if (link_rate_index >= 0)
>  			min_clock = max_clock = link_rate_index;
> @@ -1686,7 +1693,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	}
>  	DRM_DEBUG_KMS("DP link computation with max lane count %i "
>  		      "max bw %d pixel clock %iKHz\n",
> -		      max_lane_count, common_rates[max_clock],
> +		      max_lane_count, intel_dp->common_rates[max_clock],
>  		      adjusted_mode->crtc_clock);
>  
>  	/* Walk through all bpp values. Luckily they're all nicely spaced with 2
> @@ -1722,7 +1729,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  				lane_count <= max_lane_count;
>  				lane_count <<= 1) {
>  
> -				link_clock = common_rates[clock];
> +				link_clock = intel_dp->common_rates[clock];
>  				link_avail = intel_dp_max_data_rate(link_clock,
>  								    lane_count);
>  
> @@ -1754,7 +1761,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	pipe_config->lane_count = lane_count;
>  
>  	pipe_config->pipe_bpp = bpp;
> -	pipe_config->port_clock = common_rates[clock];
> +	pipe_config->port_clock = intel_dp->common_rates[clock];
>  
>  	intel_dp_compute_rate(intel_dp, pipe_config->port_clock,
>  			      &link_bw, &rate_select);
> @@ -3715,6 +3722,8 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)
>  	else
>  		intel_dp_set_sink_rates(intel_dp);
>  
> +	intel_dp_set_common_rates(intel_dp);
> +
>  	return true;
>  }
>  
> @@ -3726,8 +3735,10 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  		return false;
>  
>  	/* Don't clobber cached eDP rates. */
> -	if (!is_edp(intel_dp))
> +	if (!is_edp(intel_dp)) {
>  		intel_dp_set_sink_rates(intel_dp);
> +		intel_dp_set_common_rates(intel_dp);
> +	}
>  
>  	if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_COUNT,
>  			     &intel_dp->sink_count, 1) < 0)
> @@ -3950,7 +3961,6 @@ static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp)
>  {
>  	int status = 0;
>  	int min_lane_count = 1;
> -	int common_rates[DP_MAX_SUPPORTED_RATES] = {};
>  	int link_rate_index, test_link_rate;
>  	uint8_t test_lane_count, test_link_bw;
>  	/* (DP CTS 1.2)
> @@ -3979,7 +3989,6 @@ static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp)
>  	/* Validate the requested link rate */
>  	test_link_rate = drm_dp_bw_code_to_link_rate(test_link_bw);
>  	link_rate_index = intel_dp_link_rate_index(intel_dp,
> -						   common_rates,
>  						   test_link_rate);
>  	if (link_rate_index < 0)
>  		return DP_TEST_NAK;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f8140b06679d..ec8985b20616 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -956,6 +956,9 @@ struct intel_dp {
>  	int num_sink_rates;
>  	int sink_rates[DP_MAX_SUPPORTED_RATES];
>  	bool use_rate_select;
> +	/* intersection of source and sink rates */
> +	int num_common_rates;
> +	int common_rates[DP_MAX_SUPPORTED_RATES];
>  	/* Max lane count for the sink as per DPCD registers */
>  	uint8_t max_sink_lane_count;
>  	/* Max link BW for the sink as per DPCD registers */
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dp: do not limit rate seek when not needed

[Manasi Navare]

The commit message looks good now.

Reviewed-by: <manasi.d.navare@intel.com>

Manasi

On Wed, Mar 29, 2017 at 12:23:10PM +0300, Jani Nikula wrote:
> In link training fallback, we're trying to find a rate that we know is
> in a sorted array of common link rates. We don't need to limit the array
> using the max rate. For test request, the DP CTS doesn't say we should
> limit the rate based on earlier fallback. This lets us get rid of
> intel_dp_link_rate_index() and use intel_dp_rate_index() instead.
> 
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> 
> ---
> 
> updated commit message, no changes
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 31 ++++++++++++-------------------
>  1 file changed, 12 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 1808af6d635d..8c061c54d481 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -317,25 +317,16 @@ static int intel_dp_common_len_rate_limit(struct intel_dp *intel_dp,
>  	return 0;
>  }
>  
> -static int intel_dp_link_rate_index(struct intel_dp *intel_dp, int link_rate)
> -{
> -	int common_len;
> -
> -	common_len = intel_dp_common_len_rate_limit(intel_dp,
> -						    intel_dp->max_sink_link_rate);
> -
> -	return intel_dp_rate_index(intel_dp->common_rates, common_len, link_rate);
> -}
> -
>  int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
>  					    int link_rate, uint8_t lane_count)
>  {
> -	const int *common_rates = intel_dp->common_rates;
> -	int link_rate_index;
> +	int index;
>  
> -	link_rate_index = intel_dp_link_rate_index(intel_dp, link_rate);
> -	if (link_rate_index > 0) {
> -		intel_dp->max_sink_link_rate = common_rates[link_rate_index - 1];
> +	index = intel_dp_rate_index(intel_dp->common_rates,
> +				    intel_dp->num_common_rates,
> +				    link_rate);
> +	if (index > 0) {
> +		intel_dp->max_sink_link_rate = intel_dp->common_rates[index - 1];
>  		intel_dp->max_sink_lane_count = lane_count;
>  	} else if (lane_count > 1) {
>  		intel_dp->max_sink_link_rate = intel_dp_max_sink_rate(intel_dp);
> @@ -1685,8 +1676,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  
>  	/* Use values requested by Compliance Test Request */
>  	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
> -		link_rate_index = intel_dp_link_rate_index(intel_dp,
> -							   intel_dp->compliance.test_link_rate);
> +		link_rate_index = intel_dp_rate_index(intel_dp->common_rates,
> +						      intel_dp->num_common_rates,
> +						      intel_dp->compliance.test_link_rate);
>  		if (link_rate_index >= 0)
>  			min_clock = max_clock = link_rate_index;
>  		min_lane_count = max_lane_count = intel_dp->compliance.test_lane_count;
> @@ -3988,8 +3980,9 @@ static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp)
>  	}
>  	/* Validate the requested link rate */
>  	test_link_rate = drm_dp_bw_code_to_link_rate(test_link_bw);
> -	link_rate_index = intel_dp_link_rate_index(intel_dp,
> -						   test_link_rate);
> +	link_rate_index = intel_dp_rate_index(intel_dp->common_rates,
> +					      intel_dp->num_common_rates,
> +					      test_link_rate);
>  	if (link_rate_index < 0)
>  		return DP_TEST_NAK;
>  
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 12/14] drm/i915/dp: localize link rate index variable more

[Manasi Navare]

On Wed, Mar 29, 2017 at 10:22:32AM +0300, Jani Nikula wrote:
> On Wed, 29 Mar 2017, Manasi Navare <manasi.d.navare@intel.com> wrote:
> > Why not squash this with patch 08/14? and call it cleanup for obtaining the
> > rate index using intel_dp_rate_index()
> > Just a thought..
> 
> Again, usually too big patches are the problem, not the other way
> around.
> 
> BR,
> Jani.
>

OK, your call. Functionality wise looks good and I have tested this.


> >
> > Regards
> > Manasi
> >
> > On Tue, Mar 28, 2017 at 05:59:12PM +0300, Jani Nikula wrote:
> >> Localize link_rate_index to the if block, and rename to just index to
> >> reduce indent.
> >> 
> >> Cc: Manasi Navare <manasi.d.navare@intel.com>
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
    Reviewed-by: <manasi.d.navare@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_dp.c | 13 +++++++------
> >>  1 file changed, 7 insertions(+), 6 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >> index 95f2278700e3..6f743490855b 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -1636,7 +1636,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> >>  	/* Conveniently, the link BW constants become indices with a shift...*/
> >>  	int min_clock = 0;
> >>  	int max_clock;
> >> -	int link_rate_index;
> >>  	int bpp, mode_rate;
> >>  	int link_avail, link_clock;
> >>  	int common_len;
> >> @@ -1680,11 +1679,13 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> >>  
> >>  	/* Use values requested by Compliance Test Request */
> >>  	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
> >> -		link_rate_index = intel_dp_rate_index(intel_dp->common_rates,
> >> -						      intel_dp->num_common_rates,
> >> -						      intel_dp->compliance.test_link_rate);
> >> -		if (link_rate_index >= 0)
> >> -			min_clock = max_clock = link_rate_index;
> >> +		int index;
> >> +
> >> +		index = intel_dp_rate_index(intel_dp->common_rates,
> >> +					    intel_dp->num_common_rates,
> >> +					    intel_dp->compliance.test_link_rate);
> >> +		if (index >= 0)
> >> +			min_clock = max_clock = index;
> >>  		min_lane_count = max_lane_count = intel_dp->compliance.test_lane_count;
> >>  	}
> >>  	DRM_DEBUG_KMS("DP link computation with max lane count %i "
> >> -- 
> >> 2.1.4
> >> 
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 13/14] drm/i915/dp: use readb and writeb calls for single byte DPCD access

[Manasi Navare]

Yes this is a good optimization.

Manasi

On Tue, Mar 28, 2017 at 05:59:13PM +0300, Jani Nikula wrote:
> This is what we have the readb and writeb variants for. Do some minor
> return value and variable cleanup while at it.
> 
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
  Reviewed-by: <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 37 +++++++++++++++++--------------------
>  1 file changed, 17 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 6f743490855b..81682fd2804b 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3661,9 +3661,9 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)
>  		uint8_t frame_sync_cap;
>  
>  		dev_priv->psr.sink_support = true;
> -		drm_dp_dpcd_read(&intel_dp->aux,
> -				 DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
> -				 &frame_sync_cap, 1);
> +		drm_dp_dpcd_readb(&intel_dp->aux,
> +				  DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
> +				  &frame_sync_cap);
>  		dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false;
>  		/* PSR2 needs frame sync as well */
>  		dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync;
> @@ -3737,8 +3737,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  		intel_dp_set_common_rates(intel_dp);
>  	}
>  
> -	if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_COUNT,
> -			     &intel_dp->sink_count, 1) < 0)
> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_COUNT,
> +			      &intel_dp->sink_count) <= 0)
>  		return false;
>  
>  	/*
> @@ -3775,7 +3775,7 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  static bool
>  intel_dp_can_mst(struct intel_dp *intel_dp)
>  {
> -	u8 buf[1];
> +	u8 mstm_cap;
>  
>  	if (!i915.enable_dp_mst)
>  		return false;
> @@ -3786,10 +3786,10 @@ intel_dp_can_mst(struct intel_dp *intel_dp)
>  	if (intel_dp->dpcd[DP_DPCD_REV] < 0x12)
>  		return false;
>  
> -	if (drm_dp_dpcd_read(&intel_dp->aux, DP_MSTM_CAP, buf, 1) != 1)
> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_MSTM_CAP, &mstm_cap) != 1)
>  		return false;
>  
> -	return buf[0] & DP_MST_CAP;
> +	return mstm_cap & DP_MST_CAP;
>  }
>  
>  static void
> @@ -3935,9 +3935,8 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
>  static bool
>  intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector)
>  {
> -	return drm_dp_dpcd_read(&intel_dp->aux,
> -				       DP_DEVICE_SERVICE_IRQ_VECTOR,
> -				       sink_irq_vector, 1) == 1;
> +	return drm_dp_dpcd_readb(&intel_dp->aux, DP_DEVICE_SERVICE_IRQ_VECTOR,
> +				 sink_irq_vector) == 1;
>  }
>  
>  static bool
> @@ -4000,13 +3999,13 @@ static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp)
>  static uint8_t intel_dp_autotest_video_pattern(struct intel_dp *intel_dp)
>  {
>  	uint8_t test_pattern;
> -	uint16_t test_misc;
> +	uint8_t test_misc;
>  	__be16 h_width, v_height;
>  	int status = 0;
>  
>  	/* Read the TEST_PATTERN (DP CTS 3.1.5) */
> -	status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_PATTERN,
> -				  &test_pattern, 1);
> +	status = drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_PATTERN,
> +				   &test_pattern);
>  	if (status <= 0) {
>  		DRM_DEBUG_KMS("Test pattern read failed\n");
>  		return DP_TEST_NAK;
> @@ -4028,8 +4027,8 @@ static uint8_t intel_dp_autotest_video_pattern(struct intel_dp *intel_dp)
>  		return DP_TEST_NAK;
>  	}
>  
> -	status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_MISC0,
> -				  &test_misc, 1);
> +	status = drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_MISC0,
> +				   &test_misc);
>  	if (status <= 0) {
>  		DRM_DEBUG_KMS("TEST MISC read failed\n");
>  		return DP_TEST_NAK;
> @@ -4088,10 +4087,8 @@ static uint8_t intel_dp_autotest_edid(struct intel_dp *intel_dp)
>  		 */
>  		block += intel_connector->detect_edid->extensions;
>  
> -		if (!drm_dp_dpcd_write(&intel_dp->aux,
> -					DP_TEST_EDID_CHECKSUM,
> -					&block->checksum,
> -					1))
> +		if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_EDID_CHECKSUM,
> +				       block->checksum) <= 0)
>  			DRM_DEBUG_KMS("Failed to write EDID checksum\n");
>  
>  		test_result = DP_TEST_ACK | DP_TEST_EDID_CHECKSUM_WRITE;
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 05/14] drm/i915/dp: generate and cache sink rate array for all DP, not just eDP 1.4

[Jani Nikula]

On Wed, 05 Apr 2017, Manasi Navare <manasi.d.navare@intel.com> wrote:
> I agree with disentangling the fallback rate limiting
> from sink rates and instead just limiting the common_rates array
> based on max link rate. 
>
> Reviewed-by: <manasi.d.navare@intel.com>

Pushed patches 1-5 to drm-intel-next-queued, thanks for the review.

BR,
Jani.


>
> Manasi
>
> On Tue, Mar 28, 2017 at 05:59:05PM +0300, Jani Nikula wrote:
>> There is some conflation related to sink rates, making this change more
>> complicated than it would otherwise have to be. There are three changes
>> here that are rather difficult to split up:
>> 
>> 1) Use the intel_dp->sink_rates array for all DP, not just eDP 1.4. We
>>    initialize it from DPCD on eDP 1.4 like before, but generate it based
>>    on DP_MAX_LINK_RATE on others. This reduces code complexity when we
>>    need to use the sink rates; they are all always in the sink_rates
>>    array.
>> 
>> 2) Update the sink rate array whenever we read DPCD, and use the
>>    information from there. This increases code readability when we need
>>    the sink rates.
>> 
>> 3) Disentangle fallback rate limiting from sink rates. In the code, the
>>    max rate is a dynamic property of the *link*, not of the *sink*. Do
>>    the limiting after intersecting the source and sink rates, which are
>>    static properties of the devices.
>> 
>> This paves the way for follow-up refactoring that I've refrained from
>> doing here to keep this change as simple as it possibly can.
>> 
>> v2: introduce use_rate_select and handle non-confirming eDP (Ville)
>> 
>> v3: don't clobber cached eDP rates on short pulse (Ville)
>> 
>> Cc: Manasi Navare <manasi.d.navare@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c               | 81 ++++++++++++++++++---------
>>  drivers/gpu/drm/i915/intel_dp_link_training.c |  3 +-
>>  drivers/gpu/drm/i915/intel_drv.h              |  5 +-
>>  3 files changed, 61 insertions(+), 28 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index e9bd75ff3904..b38cba7d5abc 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -133,6 +133,34 @@ static void vlv_steal_power_sequencer(struct drm_device *dev,
>>  				      enum pipe pipe);
>>  static void intel_dp_unset_edid(struct intel_dp *intel_dp);
>>  
>> +static int intel_dp_num_rates(u8 link_bw_code)
>> +{
>> +	switch (link_bw_code) {
>> +	default:
>> +		WARN(1, "invalid max DP link bw val %x, using 1.62Gbps\n",
>> +		     link_bw_code);
>> +	case DP_LINK_BW_1_62:
>> +		return 1;
>> +	case DP_LINK_BW_2_7:
>> +		return 2;
>> +	case DP_LINK_BW_5_4:
>> +		return 3;
>> +	}
>> +}
>> +
>> +/* update sink rates from dpcd */
>> +static void intel_dp_set_sink_rates(struct intel_dp *intel_dp)
>> +{
>> +	int i, num_rates;
>> +
>> +	num_rates = intel_dp_num_rates(intel_dp->dpcd[DP_MAX_LINK_RATE]);
>> +
>> +	for (i = 0; i < num_rates; i++)
>> +		intel_dp->sink_rates[i] = default_rates[i];
>> +
>> +	intel_dp->num_sink_rates = num_rates;
>> +}
>> +
>>  static int
>>  intel_dp_max_link_bw(struct intel_dp  *intel_dp)
>>  {
>> @@ -205,19 +233,6 @@ intel_dp_downstream_max_dotclock(struct intel_dp *intel_dp)
>>  	return max_dotclk;
>>  }
>>  
>> -static int
>> -intel_dp_sink_rates(struct intel_dp *intel_dp, const int **sink_rates)
>> -{
>> -	if (intel_dp->num_sink_rates) {
>> -		*sink_rates = intel_dp->sink_rates;
>> -		return intel_dp->num_sink_rates;
>> -	}
>> -
>> -	*sink_rates = default_rates;
>> -
>> -	return (intel_dp->max_sink_link_bw >> 3) + 1;
>> -}
>> -
>>  static void
>>  intel_dp_set_source_rates(struct intel_dp *intel_dp)
>>  {
>> @@ -286,15 +301,22 @@ static int intel_dp_rate_index(const int *rates, int len, int rate)
>>  static int intel_dp_common_rates(struct intel_dp *intel_dp,
>>  				 int *common_rates)
>>  {
>> -	const int *sink_rates;
>> -	int sink_len;
>> +	int max_rate = drm_dp_bw_code_to_link_rate(intel_dp->max_sink_link_bw);
>> +	int i, common_len;
>>  
>> -	sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
>> +	common_len = intersect_rates(intel_dp->source_rates,
>> +				     intel_dp->num_source_rates,
>> +				     intel_dp->sink_rates,
>> +				     intel_dp->num_sink_rates,
>> +				     common_rates);
>> +
>> +	/* Limit results by potentially reduced max rate */
>> +	for (i = 0; i < common_len; i++) {
>> +		if (common_rates[common_len - i - 1] <= max_rate)
>> +			return common_len - i;
>> +	}
>>  
>> -	return intersect_rates(intel_dp->source_rates,
>> -			       intel_dp->num_source_rates,
>> -			       sink_rates, sink_len,
>> -			       common_rates);
>> +	return 0;
>>  }
>>  
>>  static int intel_dp_link_rate_index(struct intel_dp *intel_dp,
>> @@ -1498,8 +1520,7 @@ static void snprintf_int_array(char *str, size_t len,
>>  
>>  static void intel_dp_print_rates(struct intel_dp *intel_dp)
>>  {
>> -	const int *sink_rates;
>> -	int sink_len, common_len;
>> +	int common_len;
>>  	int common_rates[DP_MAX_SUPPORTED_RATES];
>>  	char str[128]; /* FIXME: too big for stack? */
>>  
>> @@ -1510,8 +1531,8 @@ static void intel_dp_print_rates(struct intel_dp *intel_dp)
>>  			   intel_dp->source_rates, intel_dp->num_source_rates);
>>  	DRM_DEBUG_KMS("source rates: %s\n", str);
>>  
>> -	sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
>> -	snprintf_int_array(str, sizeof(str), sink_rates, sink_len);
>> +	snprintf_int_array(str, sizeof(str),
>> +			   intel_dp->sink_rates, intel_dp->num_sink_rates);
>>  	DRM_DEBUG_KMS("sink rates: %s\n", str);
>>  
>>  	common_len = intel_dp_common_rates(intel_dp, common_rates);
>> @@ -1577,7 +1598,8 @@ int intel_dp_rate_select(struct intel_dp *intel_dp, int rate)
>>  void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
>>  			   uint8_t *link_bw, uint8_t *rate_select)
>>  {
>> -	if (intel_dp->num_sink_rates) {
>> +	/* eDP 1.4 rate select method. */
>> +	if (intel_dp->use_rate_select) {
>>  		*link_bw = 0;
>>  		*rate_select =
>>  			intel_dp_rate_select(intel_dp, port_clock);
>> @@ -3702,6 +3724,11 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)
>>  		intel_dp->num_sink_rates = i;
>>  	}
>>  
>> +	if (intel_dp->num_sink_rates)
>> +		intel_dp->use_rate_select = true;
>> +	else
>> +		intel_dp_set_sink_rates(intel_dp);
>> +
>>  	return true;
>>  }
>>  
>> @@ -3712,6 +3739,10 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>>  	if (!intel_dp_read_dpcd(intel_dp))
>>  		return false;
>>  
>> +	/* Don't clobber cached eDP rates. */
>> +	if (!is_edp(intel_dp))
>> +		intel_dp_set_sink_rates(intel_dp);
>> +
>>  	if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_COUNT,
>>  			     &intel_dp->sink_count, 1) < 0)
>>  		return false;
>> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
>> index 0048b520baf7..694ad0ffb523 100644
>> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
>> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
>> @@ -146,7 +146,8 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
>>  		link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
>>  	drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_BW_SET, link_config, 2);
>>  
>> -	if (intel_dp->num_sink_rates)
>> +	/* eDP 1.4 rate select method. */
>> +	if (!link_bw)
>>  		drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_RATE_SET,
>>  				  &rate_select, 1);
>>  
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index f59574261dc6..77a94aaa98c2 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -952,9 +952,10 @@ struct intel_dp {
>>  	/* source rates */
>>  	int num_source_rates;
>>  	const int *source_rates;
>> -	/* sink rates as reported by DP_SUPPORTED_LINK_RATES */
>> -	uint8_t num_sink_rates;
>> +	/* sink rates as reported by DP_MAX_LINK_RATE/DP_SUPPORTED_LINK_RATES */
>> +	int num_sink_rates;
>>  	int sink_rates[DP_MAX_SUPPORTED_RATES];
>> +	bool use_rate_select;
>>  	/* Max lane count for the sink as per DPCD registers */
>>  	uint8_t max_sink_lane_count;
>>  	/* Max link BW for the sink as per DPCD registers */
>> -- 
>> 2.1.4
>> 

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx