* [PATCH v4 00/11] drm/i915/dp: rest of refactoring, link rate fallback
@ 2017-04-06 13:44 Jani Nikula
2017-04-06 13:44 ` [PATCH v4 01/11] drm/i915/dp: use the sink rates array for max sink rates Jani Nikula
` (13 more replies)
0 siblings, 14 replies; 28+ messages in thread
From: Jani Nikula @ 2017-04-06 13:44 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, dhinakaran.pandiyan
v4 of [1], after patches 1-5 have been merged, and with Manasi's patches [2] and
[3] rebased on top.
BR,
Jani.
[1] http://mid.mail-archive.com/cover.1490712890.git.jani.nikula@intel.com
[2] http://patchwork.freedesktop.org/patch/msgid/1490651090-4263-1-git-send-email-manasi.d.navare@intel.com
[3] http://patchwork.freedesktop.org/patch/msgid/1489529511-7856-1-git-send-email-manasi.d.navare@intel.com
Jani Nikula (9):
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
Manasi Navare (2):
drm/i915/dp: Validate cached link rate and lane count before
retraining
drm/i915: Implement Link Rate fallback on Link training failure
drivers/gpu/drm/i915/intel_dp.c | 236 +++++++++++++++-----------
drivers/gpu/drm/i915/intel_dp_link_training.c | 22 ++-
drivers/gpu/drm/i915/intel_dp_mst.c | 4 +-
drivers/gpu/drm/i915/intel_drv.h | 15 +-
4 files changed, 172 insertions(+), 105 deletions(-)
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v4 01/11] drm/i915/dp: use the sink rates array for max sink rates
2017-04-06 13:44 [PATCH v4 00/11] drm/i915/dp: rest of refactoring, link rate fallback Jani Nikula
@ 2017-04-06 13:44 ` Jani Nikula
2017-04-06 14:24 ` Ville Syrjälä
2017-04-06 13:44 ` [PATCH v4 02/11] drm/i915/dp: cache common rates with " Jani Nikula
` (12 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Jani Nikula @ 2017-04-06 13:44 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, dhinakaran.pandiyan
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 7bc0c25b3396..92e353dc61bf 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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 02/11] drm/i915/dp: cache common rates with sink rates
2017-04-06 13:44 [PATCH v4 00/11] drm/i915/dp: rest of refactoring, link rate fallback Jani Nikula
2017-04-06 13:44 ` [PATCH v4 01/11] drm/i915/dp: use the sink rates array for max sink rates Jani Nikula
@ 2017-04-06 13:44 ` Jani Nikula
2017-04-06 13:44 ` [PATCH v4 03/11] drm/i915/dp: do not limit rate seek when not needed Jani Nikula
` (11 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2017-04-06 13:44 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, dhinakaran.pandiyan
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>
Reviewed-by: Manasi Navare <manasi.d.navare@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 92e353dc61bf..4a4bf9cb0d90 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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 03/11] drm/i915/dp: do not limit rate seek when not needed
2017-04-06 13:44 [PATCH v4 00/11] drm/i915/dp: rest of refactoring, link rate fallback Jani Nikula
2017-04-06 13:44 ` [PATCH v4 01/11] drm/i915/dp: use the sink rates array for max sink rates Jani Nikula
2017-04-06 13:44 ` [PATCH v4 02/11] drm/i915/dp: cache common rates with " Jani Nikula
@ 2017-04-06 13:44 ` Jani Nikula
2017-04-06 13:44 ` [PATCH v4 04/11] drm/i915/dp: don't call the link parameters sink parameters Jani Nikula
` (10 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2017-04-06 13:44 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, dhinakaran.pandiyan
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>
Reviewed-by: Manasi Navare <manasi.d.navare@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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 04/11] drm/i915/dp: don't call the link parameters sink parameters
2017-04-06 13:44 [PATCH v4 00/11] drm/i915/dp: rest of refactoring, link rate fallback Jani Nikula
` (2 preceding siblings ...)
2017-04-06 13:44 ` [PATCH v4 03/11] drm/i915/dp: do not limit rate seek when not needed Jani Nikula
@ 2017-04-06 13:44 ` Jani Nikula
2017-04-06 13:44 ` [PATCH v4 05/11] drm/i915/dp: add functions for max common link rate and lane count Jani Nikula
` (9 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2017-04-06 13:44 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, dhinakaran.pandiyan
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>
Reviewed-by: Manasi Navare <manasi.d.navare@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 4a4bf9cb0d90..f97603b74a28 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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 05/11] drm/i915/dp: add functions for max common link rate and lane count
2017-04-06 13:44 [PATCH v4 00/11] drm/i915/dp: rest of refactoring, link rate fallback Jani Nikula
` (3 preceding siblings ...)
2017-04-06 13:44 ` [PATCH v4 04/11] drm/i915/dp: don't call the link parameters sink parameters Jani Nikula
@ 2017-04-06 13:44 ` Jani Nikula
2017-04-06 13:44 ` [PATCH v4 06/11] drm/i915/mst: use max link not sink " Jani Nikula
` (8 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2017-04-06 13:44 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, dhinakaran.pandiyan
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>
Reviewed-by: Manasi Navare <manasi.d.navare@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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 06/11] drm/i915/mst: use max link not sink lane count
2017-04-06 13:44 [PATCH v4 00/11] drm/i915/dp: rest of refactoring, link rate fallback Jani Nikula
` (4 preceding siblings ...)
2017-04-06 13:44 ` [PATCH v4 05/11] drm/i915/dp: add functions for max common link rate and lane count Jani Nikula
@ 2017-04-06 13:44 ` Jani Nikula
2017-04-06 13:44 ` [PATCH v4 07/11] drm/i915/dp: localize link rate index variable more Jani Nikula
` (7 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2017-04-06 13:44 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, dhinakaran.pandiyan
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>
Reviewed-by: Manasi Navare <manasi.d.navare@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 f97603b74a28..5f6e1aad909d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1503,6 +1503,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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 07/11] drm/i915/dp: localize link rate index variable more
2017-04-06 13:44 [PATCH v4 00/11] drm/i915/dp: rest of refactoring, link rate fallback Jani Nikula
` (5 preceding siblings ...)
2017-04-06 13:44 ` [PATCH v4 06/11] drm/i915/mst: use max link not sink " Jani Nikula
@ 2017-04-06 13:44 ` Jani Nikula
2017-04-06 13:44 ` [PATCH v4 08/11] drm/i915/dp: use readb and writeb calls for single byte DPCD access Jani Nikula
` (6 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2017-04-06 13:44 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, dhinakaran.pandiyan
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>
Reviewed-by: Manasi Navare <manasi.d.navare@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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 08/11] drm/i915/dp: use readb and writeb calls for single byte DPCD access
2017-04-06 13:44 [PATCH v4 00/11] drm/i915/dp: rest of refactoring, link rate fallback Jani Nikula
` (6 preceding siblings ...)
2017-04-06 13:44 ` [PATCH v4 07/11] drm/i915/dp: localize link rate index variable more Jani Nikula
@ 2017-04-06 13:44 ` Jani Nikula
2017-04-06 13:44 ` [PATCH v4 09/11] drm/i915/dp: read sink count to a temporary variable first Jani Nikula
` (5 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2017-04-06 13:44 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, dhinakaran.pandiyan
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>
Reviewed-by: Manasi Navare <manasi.d.navare@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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 09/11] drm/i915/dp: read sink count to a temporary variable first
2017-04-06 13:44 [PATCH v4 00/11] drm/i915/dp: rest of refactoring, link rate fallback Jani Nikula
` (7 preceding siblings ...)
2017-04-06 13:44 ` [PATCH v4 08/11] drm/i915/dp: use readb and writeb calls for single byte DPCD access Jani Nikula
@ 2017-04-06 13:44 ` Jani Nikula
2017-04-06 14:25 ` Ville Syrjälä
2017-04-06 13:44 ` [PATCH v4 10/11] drm/i915/dp: Validate cached link rate and lane count before retraining Jani Nikula
` (4 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Jani Nikula @ 2017-04-06 13:44 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, dhinakaran.pandiyan
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 10/11] drm/i915/dp: Validate cached link rate and lane count before retraining
2017-04-06 13:44 [PATCH v4 00/11] drm/i915/dp: rest of refactoring, link rate fallback Jani Nikula
` (8 preceding siblings ...)
2017-04-06 13:44 ` [PATCH v4 09/11] drm/i915/dp: read sink count to a temporary variable first Jani Nikula
@ 2017-04-06 13:44 ` Jani Nikula
2017-04-06 14:31 ` Ville Syrjälä
2017-04-06 13:44 ` [PATCH v4 11/11] drm/i915: Implement Link Rate fallback on Link training failure Jani Nikula
` (3 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Jani Nikula @ 2017-04-06 13:44 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, dhinakaran.pandiyan
From: Manasi Navare <manasi.d.navare@intel.com>
Currently intel_dp_check_link_status() tries to retrain the link if
Clock recovery or Channel EQ for any of the lanes indicated by
intel_dp->lane_count is not set. However these values cached in intel_dp
structure can be stale if link training has failed for these values
during previous modeset. Or these values can get stale since we have
now re read the DPCD registers or it can be 0 in case of connected boot
case.
This patch validates these values against the common_rates and max lane
count values.
This is absolutely required incase the common_rates or max lane count
are now different due to link fallback.
v2:
* Include the FIXME commnet inside the function (Ville Syrjala)
* Remove the redundant parenthesis (Ville Syrjala)
v3 by Jani:
* rebase on the DP refactoring series
* rename intel_dp_link_params_is_valid to intel_dp_link_params_valid
* minor stylistic changes
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 30 +++++++++++++++++++++++++++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3c5c80da9ea3..6915d89a238c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -322,6 +322,28 @@ static int intel_dp_common_len_rate_limit(struct intel_dp *intel_dp,
return 0;
}
+static bool intel_dp_link_params_valid(struct intel_dp *intel_dp)
+{
+ int index;
+
+ /*
+ * FIXME: we need to synchronize the current link parameters with
+ * hardware readout. Currently fast link training doesn't work on
+ * boot-up.
+ */
+ index = intel_dp_rate_index(intel_dp->common_rates,
+ intel_dp->num_common_rates,
+ intel_dp->link_rate);
+ if (index < 0)
+ return false;
+
+ if (intel_dp->lane_count == 0 ||
+ intel_dp->lane_count > intel_dp_max_lane_count(intel_dp))
+ return false;
+
+ return true;
+}
+
int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
int link_rate, uint8_t lane_count)
{
@@ -4253,9 +4275,11 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
if (!to_intel_crtc(intel_encoder->base.crtc)->active)
return;
- /* FIXME: we need to synchronize this sort of stuff with hardware
- * readout. Currently fast link training doesn't work on boot-up. */
- if (!intel_dp->lane_count)
+ /*
+ * Validate the cached values of intel_dp->link_rate and
+ * intel_dp->lane_count before attempting to retrain.
+ */
+ if (!intel_dp_link_params_valid(intel_dp))
return;
/* Retrain if Channel EQ or CR not ok */
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 11/11] drm/i915: Implement Link Rate fallback on Link training failure
2017-04-06 13:44 [PATCH v4 00/11] drm/i915/dp: rest of refactoring, link rate fallback Jani Nikula
` (9 preceding siblings ...)
2017-04-06 13:44 ` [PATCH v4 10/11] drm/i915/dp: Validate cached link rate and lane count before retraining Jani Nikula
@ 2017-04-06 13:44 ` Jani Nikula
2017-04-13 17:25 ` Manasi Navare
2017-04-06 14:35 ` ✓ Fi.CI.BAT: success for drm/i915/dp: rest of refactoring, link rate fallback Patchwork
` (2 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Jani Nikula @ 2017-04-06 13:44 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, dhinakaran.pandiyan, Daniel Vetter
From: Manasi Navare <manasi.d.navare@intel.com>
If link training at a link rate optimal for a particular
mode fails during modeset's atomic commit phase, then we
let the modeset complete and then retry. We save the link rate
value at which link training failed, update the link status property
to "BAD" and use a lower link rate to prune the modes. It will redo
the modeset on the current mode at lower link rate or if the current
mode gets pruned due to lower link constraints then, it will send a
hotplug uevent for userspace to handle it.
This is also required to pass DP CTS tests 4.3.1.3, 4.3.1.4,
4.3.1.6.
This patch is a resend of the original commit id (233ce881dd91fb
"drm/i915: Implement Link Rate fallback on Link training failure")
which got reverted in this commit id (afc1ebf4562a14 Revert
"drm/i915: Implement Link Rate fallback on Link training failure")
due to CI failures.
After investigating the CI failures it was found that these
were essentially the failures which were always there but hidden because
they used to be DRM_DEBUG_KMS messages for link failures so never got
caught by CI. But now this patch actually throws DRM_ERROR if the link
training fails at RBR and 1 lane. So it caught these link train failures.
There were two failures:
1. On SKL 6700k this was because the machine in CI lab is a SKL desktop
without eDP on Port A. But our VBT initialization code in the driver writes
VBT defaults in a way that it always sets DP flag on Port A and this does
not get cleared after parsing the VBT outputs. This has been fixed in
commit id (bb1d132935c2f8 "drm/i915/vbt: split out defaults that are set
when there is no VBT) and (665788572c6410b "drm/i915/vbt: don't propagate
errors from intel_bios_init())
2. On ILK-650 desktop - This was happening because of a bad monitor desktop
combination. I switched the monitor in the CI lab and that helped get rid
of the link failures on ILK system.
v10:
* Rebase on drm-tip and resend after revert
v9:
* Use the trimmed max values of link rate/lane count based on
link train fallback (Daniel Vetter)
v8:
* Set link_status to BAD first and then call mode_valid (Jani Nikula)
v7:
Remove the redundant variable in previous patch itself
v6:
* Obtain link rate index from fallback_link_rate using
the helper intel_dp_link_rate_index (Jani Nikula)
* Include fallback within intel_dp_start_link_train (Jani Nikula)
v5:
* Move set link status to drm core (Daniel Vetter, Jani Nikula)
v4:
* Add fallback support for non DDI platforms too
* Set connector->link status inside set_link_status function
(Jani Nikula)
v3:
* Set link status property to BAd unconditionally (Jani Nikula)
* Dont use two separate variables link_train_failed and link_status
to indicate same thing (Jani Nikula)
v2:
* Squashed a few patches (Jani Nikula)
Acked-by: Tony Cheng <tony.cheng@amd.com>
Acked-by: Harry Wentland <Harry.wentland@amd.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 27 +++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_dp_link_training.c | 22 ++++++++++++++++++++--
drivers/gpu/drm/i915/intel_drv.h | 3 +++
3 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 6915d89a238c..f2d792a1b18e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5977,6 +5977,29 @@ intel_dp_init_connector_port_info(struct intel_digital_port *intel_dig_port)
}
}
+static void intel_dp_modeset_retry_work_fn(struct work_struct *work)
+{
+ struct intel_connector *intel_connector;
+ struct drm_connector *connector;
+
+ intel_connector = container_of(work, typeof(*intel_connector),
+ modeset_retry_work);
+ connector = &intel_connector->base;
+ DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id,
+ connector->name);
+
+ /* Grab the locks before changing connector property*/
+ mutex_lock(&connector->dev->mode_config.mutex);
+ /* Set connector link status to BAD and send a Uevent to notify
+ * userspace to do a modeset.
+ */
+ drm_mode_connector_set_link_status_property(connector,
+ DRM_MODE_LINK_STATUS_BAD);
+ mutex_unlock(&connector->dev->mode_config.mutex);
+ /* Send Hotplug uevent so userspace can reprobe */
+ drm_kms_helper_hotplug_event(connector->dev);
+}
+
bool
intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
struct intel_connector *intel_connector)
@@ -5989,6 +6012,10 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
enum port port = intel_dig_port->port;
int type;
+ /* Initialize the work for modeset in case of link train failure */
+ INIT_WORK(&intel_connector->modeset_retry_work,
+ intel_dp_modeset_retry_work_fn);
+
if (WARN(intel_dig_port->max_lanes < 1,
"Not enough lanes (%d) for DP on port %c\n",
intel_dig_port->max_lanes, port_name(port)))
diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
index 694ad0ffb523..b79c1c0e404c 100644
--- a/drivers/gpu/drm/i915/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
@@ -314,6 +314,24 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp)
void
intel_dp_start_link_train(struct intel_dp *intel_dp)
{
- intel_dp_link_training_clock_recovery(intel_dp);
- intel_dp_link_training_channel_equalization(intel_dp);
+ struct intel_connector *intel_connector = intel_dp->attached_connector;
+
+ if (!intel_dp_link_training_clock_recovery(intel_dp))
+ goto failure_handling;
+ if (!intel_dp_link_training_channel_equalization(intel_dp))
+ goto failure_handling;
+
+ DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d",
+ intel_dp->link_rate, intel_dp->lane_count);
+ return;
+
+ failure_handling:
+ DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d",
+ intel_dp->link_rate, intel_dp->lane_count);
+ if (!intel_dp_get_link_train_fallback_values(intel_dp,
+ intel_dp->link_rate,
+ intel_dp->lane_count))
+ /* Schedule a Hotplug Uevent to userspace to start modeset */
+ schedule_work(&intel_connector->modeset_retry_work);
+ return;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5f6e1aad909d..d36026eff7cd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -321,6 +321,9 @@ struct intel_connector {
void *port; /* store this opaque as its illegal to dereference it */
struct intel_dp *mst_port;
+
+ /* Work struct to schedule a uevent on link train failure */
+ struct work_struct modeset_retry_work;
};
struct dpll {
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v4 01/11] drm/i915/dp: use the sink rates array for max sink rates
2017-04-06 13:44 ` [PATCH v4 01/11] drm/i915/dp: use the sink rates array for max sink rates Jani Nikula
@ 2017-04-06 14:24 ` Ville Syrjälä
0 siblings, 0 replies; 28+ messages in thread
From: Ville Syrjälä @ 2017-04-06 14:24 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, dhinakaran.pandiyan
On Thu, Apr 06, 2017 at 04:44:09PM +0300, Jani Nikula wrote:
> 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>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.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 7bc0c25b3396..92e353dc61bf 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
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 09/11] drm/i915/dp: read sink count to a temporary variable first
2017-04-06 13:44 ` [PATCH v4 09/11] drm/i915/dp: read sink count to a temporary variable first Jani Nikula
@ 2017-04-06 14:25 ` Ville Syrjälä
2017-04-11 14:19 ` Jani Nikula
0 siblings, 1 reply; 28+ messages in thread
From: Ville Syrjälä @ 2017-04-06 14:25 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, dhinakaran.pandiyan
On Thu, Apr 06, 2017 at 04:44:17PM +0300, Jani Nikula wrote:
> 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>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.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
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 10/11] drm/i915/dp: Validate cached link rate and lane count before retraining
2017-04-06 13:44 ` [PATCH v4 10/11] drm/i915/dp: Validate cached link rate and lane count before retraining Jani Nikula
@ 2017-04-06 14:31 ` Ville Syrjälä
2017-04-06 18:29 ` Manasi Navare
2017-04-06 20:43 ` [PATCH v5] " Manasi Navare
0 siblings, 2 replies; 28+ messages in thread
From: Ville Syrjälä @ 2017-04-06 14:31 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, dhinakaran.pandiyan
On Thu, Apr 06, 2017 at 04:44:18PM +0300, Jani Nikula wrote:
> From: Manasi Navare <manasi.d.navare@intel.com>
>
> Currently intel_dp_check_link_status() tries to retrain the link if
> Clock recovery or Channel EQ for any of the lanes indicated by
> intel_dp->lane_count is not set. However these values cached in intel_dp
> structure can be stale if link training has failed for these values
> during previous modeset. Or these values can get stale since we have
> now re read the DPCD registers or it can be 0 in case of connected boot
> case.
>
> This patch validates these values against the common_rates and max lane
> count values.
>
> This is absolutely required incase the common_rates or max lane count
> are now different due to link fallback.
>
> v2:
> * Include the FIXME commnet inside the function (Ville Syrjala)
> * Remove the redundant parenthesis (Ville Syrjala)
>
> v3 by Jani:
> * rebase on the DP refactoring series
> * rename intel_dp_link_params_is_valid to intel_dp_link_params_valid
> * minor stylistic changes
>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 30 +++++++++++++++++++++++++++---
> 1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 3c5c80da9ea3..6915d89a238c 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -322,6 +322,28 @@ static int intel_dp_common_len_rate_limit(struct intel_dp *intel_dp,
> return 0;
> }
>
> +static bool intel_dp_link_params_valid(struct intel_dp *intel_dp)
> +{
> + int index;
> +
> + /*
> + * FIXME: we need to synchronize the current link parameters with
> + * hardware readout. Currently fast link training doesn't work on
> + * boot-up.
> + */
> + index = intel_dp_rate_index(intel_dp->common_rates,
> + intel_dp->num_common_rates,
> + intel_dp->link_rate);
> + if (index < 0)
> + return false;
Hmm. Isn't common_rates[] still just the intersection of the source
and sink rates? So it looks to me like this should actually just check
for 'link_rate == 0 || link_rate > max_link_rate', or something similar.
> +
> + if (intel_dp->lane_count == 0 ||
> + intel_dp->lane_count > intel_dp_max_lane_count(intel_dp))
> + return false;
> +
> + return true;
> +}
> +
> int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> int link_rate, uint8_t lane_count)
> {
> @@ -4253,9 +4275,11 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
> if (!to_intel_crtc(intel_encoder->base.crtc)->active)
> return;
>
> - /* FIXME: we need to synchronize this sort of stuff with hardware
> - * readout. Currently fast link training doesn't work on boot-up. */
> - if (!intel_dp->lane_count)
> + /*
> + * Validate the cached values of intel_dp->link_rate and
> + * intel_dp->lane_count before attempting to retrain.
> + */
> + if (!intel_dp_link_params_valid(intel_dp))
> return;
>
> /* Retrain if Channel EQ or CR not ok */
> --
> 2.1.4
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915/dp: rest of refactoring, link rate fallback
2017-04-06 13:44 [PATCH v4 00/11] drm/i915/dp: rest of refactoring, link rate fallback Jani Nikula
` (10 preceding siblings ...)
2017-04-06 13:44 ` [PATCH v4 11/11] drm/i915: Implement Link Rate fallback on Link training failure Jani Nikula
@ 2017-04-06 14:35 ` Patchwork
2017-04-06 20:42 ` ✗ Fi.CI.BAT: failure for drm/i915/dp: rest of refactoring, link rate fallback (rev2) Patchwork
2017-04-06 21:54 ` ✓ Fi.CI.BAT: success for drm/i915/dp: rest of refactoring, link rate fallback (rev3) Patchwork
13 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2017-04-06 14:35 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/dp: rest of refactoring, link rate fallback
URL : https://patchwork.freedesktop.org/series/22586/
State : success
== Summary ==
Series 22586v1 drm/i915/dp: rest of refactoring, link rate fallback
https://patchwork.freedesktop.org/api/1.0/series/22586/revisions/1/mbox/
fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 428s
fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time: 426s
fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time: 566s
fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 508s
fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time: 546s
fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time: 483s
fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 485s
fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 411s
fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 402s
fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 419s
fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 489s
fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 474s
fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 454s
fi-kbl-7560u total:278 pass:267 dwarn:1 dfail:0 fail:0 skip:10 time: 566s
fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 450s
fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 574s
fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 462s
fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 492s
fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time: 435s
fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 526s
fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 402s
e087f8395ca39c6988de8680bd6f80a20b08c0f4 drm-tip: 2017y-04m-06d-13h-28m-42s UTC integration manifest
32e033d drm/i915: Implement Link Rate fallback on Link training failure
7b669fb drm/i915/dp: Validate cached link rate and lane count before retraining
eb1b3252 drm/i915/dp: read sink count to a temporary variable first
e575e14 drm/i915/dp: use readb and writeb calls for single byte DPCD access
3df04b8 drm/i915/dp: localize link rate index variable more
dcd419b drm/i915/mst: use max link not sink lane count
c57a383 drm/i915/dp: add functions for max common link rate and lane count
688e493 drm/i915/dp: don't call the link parameters sink parameters
960daae drm/i915/dp: do not limit rate seek when not needed
2f414b3 drm/i915/dp: cache common rates with sink rates
f648e2e drm/i915/dp: use the sink rates array for max sink rates
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4420/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 10/11] drm/i915/dp: Validate cached link rate and lane count before retraining
2017-04-06 14:31 ` Ville Syrjälä
@ 2017-04-06 18:29 ` Manasi Navare
2017-04-06 20:43 ` [PATCH v5] " Manasi Navare
1 sibling, 0 replies; 28+ messages in thread
From: Manasi Navare @ 2017-04-06 18:29 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Jani Nikula, intel-gfx, dhinakaran.pandiyan
On Thu, Apr 06, 2017 at 05:31:07PM +0300, Ville Syrjälä wrote:
> On Thu, Apr 06, 2017 at 04:44:18PM +0300, Jani Nikula wrote:
> > From: Manasi Navare <manasi.d.navare@intel.com>
> >
> > Currently intel_dp_check_link_status() tries to retrain the link if
> > Clock recovery or Channel EQ for any of the lanes indicated by
> > intel_dp->lane_count is not set. However these values cached in intel_dp
> > structure can be stale if link training has failed for these values
> > during previous modeset. Or these values can get stale since we have
> > now re read the DPCD registers or it can be 0 in case of connected boot
> > case.
> >
> > This patch validates these values against the common_rates and max lane
> > count values.
> >
> > This is absolutely required incase the common_rates or max lane count
> > are now different due to link fallback.
> >
> > v2:
> > * Include the FIXME commnet inside the function (Ville Syrjala)
> > * Remove the redundant parenthesis (Ville Syrjala)
> >
> > v3 by Jani:
> > * rebase on the DP refactoring series
> > * rename intel_dp_link_params_is_valid to intel_dp_link_params_valid
> > * minor stylistic changes
> >
> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 30 +++++++++++++++++++++++++++---
> > 1 file changed, 27 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 3c5c80da9ea3..6915d89a238c 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -322,6 +322,28 @@ static int intel_dp_common_len_rate_limit(struct intel_dp *intel_dp,
> > return 0;
> > }
> >
> > +static bool intel_dp_link_params_valid(struct intel_dp *intel_dp)
> > +{
> > + int index;
> > +
> > + /*
> > + * FIXME: we need to synchronize the current link parameters with
> > + * hardware readout. Currently fast link training doesn't work on
> > + * boot-up.
> > + */
> > + index = intel_dp_rate_index(intel_dp->common_rates,
> > + intel_dp->num_common_rates,
> > + intel_dp->link_rate);
> > + if (index < 0)
> > + return false;
>
> Hmm. Isn't common_rates[] still just the intersection of the source
> and sink rates? So it looks to me like this should actually just check
> for 'link_rate == 0 || link_rate > max_link_rate', or something similar.
>
Yes that is true. Passing common_rates worked earlier to see if the link
rate was within the bounds and still less than the lowered fallback link rate
becuase we were changing the common_rates.
But now with this new series, you are right that we cannot use common_rates array
because it is still the intersection of just sink and source rates and wont
take into account the lowered fallback rate unless we pass the length of common
rates array limited by the max link rate by calling intel_dp_common_len_rate_limit()
or we could just check that it is greater than 0 and less than the max_link_rate.
Does that sound right to you?
I will update this logic and send a new revision.
Regards
Manasi
> > +
> > + if (intel_dp->lane_count == 0 ||
> > + intel_dp->lane_count > intel_dp_max_lane_count(intel_dp))
> > + return false;
> > +
> > + return true;
> > +}
> > +
> > int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> > int link_rate, uint8_t lane_count)
> > {
> > @@ -4253,9 +4275,11 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
> > if (!to_intel_crtc(intel_encoder->base.crtc)->active)
> > return;
> >
> > - /* FIXME: we need to synchronize this sort of stuff with hardware
> > - * readout. Currently fast link training doesn't work on boot-up. */
> > - if (!intel_dp->lane_count)
> > + /*
> > + * Validate the cached values of intel_dp->link_rate and
> > + * intel_dp->lane_count before attempting to retrain.
> > + */
> > + if (!intel_dp_link_params_valid(intel_dp))
> > return;
> >
> > /* Retrain if Channel EQ or CR not ok */
> > --
> > 2.1.4
>
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915/dp: rest of refactoring, link rate fallback (rev2)
2017-04-06 13:44 [PATCH v4 00/11] drm/i915/dp: rest of refactoring, link rate fallback Jani Nikula
` (11 preceding siblings ...)
2017-04-06 14:35 ` ✓ Fi.CI.BAT: success for drm/i915/dp: rest of refactoring, link rate fallback Patchwork
@ 2017-04-06 20:42 ` Patchwork
2017-04-06 21:54 ` ✓ Fi.CI.BAT: success for drm/i915/dp: rest of refactoring, link rate fallback (rev3) Patchwork
13 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2017-04-06 20:42 UTC (permalink / raw)
To: Navare, Manasi D; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/dp: rest of refactoring, link rate fallback (rev2)
URL : https://patchwork.freedesktop.org/series/22586/
State : failure
== Summary ==
CC [M] drivers/gpu/drm/i915/gvt/display.o
CC [M] drivers/gpu/drm/i915/gvt/edid.o
LD drivers/rtc/built-in.o
CC [M] drivers/gpu/drm/i915/gvt/execlist.o
CC [M] drivers/gpu/drm/i915/gvt/scheduler.o
CC [M] drivers/gpu/drm/i915/gvt/sched_policy.o
CC [M] drivers/gpu/drm/i915/gvt/render.o
CC [M] drivers/gpu/drm/i915/gvt/cmd_parser.o
LD [M] drivers/ssb/ssb.o
CC [M] drivers/gpu/drm/i915/intel_lpe_audio.o
AR lib/lib.a
LD net/xfrm/built-in.o
EXPORTS lib/lib-ksyms.o
LD net/packet/built-in.o
LD lib/built-in.o
LD drivers/mmc/core/mmc_core.o
LD drivers/mmc/host/built-in.o
LD drivers/pci/pcie/pcieportdrv.o
LD drivers/mmc/core/mmc_block.o
LD drivers/mmc/core/built-in.o
LD drivers/mmc/built-in.o
LD drivers/gpu/drm/drm.o
LD drivers/usb/storage/usb-storage.o
LD kernel/sched/built-in.o
LD drivers/usb/storage/built-in.o
LD kernel/built-in.o
LD drivers/tty/serial/8250/8250.o
LD drivers/pci/pcie/aer/aerdriver.o
LD drivers/pci/pcie/aer/built-in.o
LD drivers/pci/pcie/built-in.o
LD net/ipv6/ipv6.o
LD [M] drivers/usb/serial/usbserial.o
LD [M] drivers/net/ethernet/intel/igbvf/igbvf.o
LD [M] drivers/net/ethernet/broadcom/genet/genet.o
LD net/ipv6/built-in.o
LD drivers/video/fbdev/core/fb.o
LD drivers/thermal/thermal_sys.o
LD drivers/video/fbdev/core/built-in.o
LD drivers/thermal/built-in.o
LD drivers/pci/built-in.o
drivers/gpu/drm/i915/intel_dp.c: In function ‘intel_dp_link_params_valid’:
drivers/gpu/drm/i915/intel_dp.c:327:6: error: unused variable ‘index’ [-Werror=unused-variable]
int index;
^
LD drivers/video/fbdev/built-in.o
LD [M] drivers/misc/mei/mei-me.o
LD drivers/iommu/built-in.o
LD drivers/misc/built-in.o
LD drivers/usb/gadget/udc/udc-core.o
LD drivers/usb/gadget/udc/built-in.o
LD drivers/scsi/scsi_mod.o
LD drivers/tty/serial/8250/8250_base.o
LD drivers/tty/serial/8250/built-in.o
LD drivers/tty/serial/built-in.o
LD drivers/spi/built-in.o
LD drivers/video/console/built-in.o
LD drivers/video/built-in.o
LD drivers/usb/gadget/libcomposite.o
LD drivers/usb/gadget/built-in.o
LD net/ipv4/built-in.o
LD drivers/scsi/sd_mod.o
LD drivers/scsi/built-in.o
LD fs/btrfs/btrfs.o
LD fs/btrfs/built-in.o
LD drivers/tty/vt/built-in.o
LD drivers/tty/built-in.o
LD [M] drivers/net/ethernet/intel/e1000/e1000.o
CC arch/x86/kernel/cpu/capflags.o
LD net/core/built-in.o
LD arch/x86/kernel/cpu/built-in.o
LD arch/x86/kernel/built-in.o
LD drivers/usb/core/usbcore.o
LD net/built-in.o
LD drivers/usb/core/built-in.o
LD fs/ext4/ext4.o
LD arch/x86/built-in.o
LD fs/ext4/built-in.o
LD fs/built-in.o
LD drivers/md/md-mod.o
LD drivers/md/built-in.o
LD [M] drivers/net/ethernet/intel/igb/igb.o
LD drivers/usb/host/xhci-hcd.o
LD drivers/usb/host/built-in.o
cc1: all warnings being treated as errors
LD drivers/usb/built-in.o
scripts/Makefile.build:294: recipe for target 'drivers/gpu/drm/i915/intel_dp.o' failed
make[4]: *** [drivers/gpu/drm/i915/intel_dp.o] Error 1
make[4]: *** Waiting for unfinished jobs....
LD [M] drivers/net/ethernet/intel/e1000e/e1000e.o
scripts/Makefile.build:553: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:553: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:553: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
make[1]: *** Waiting for unfinished jobs....
LD drivers/net/ethernet/built-in.o
LD drivers/net/built-in.o
Makefile:1002: recipe for target 'drivers' failed
make: *** [drivers] Error 2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v5] drm/i915/dp: Validate cached link rate and lane count before retraining
2017-04-06 14:31 ` Ville Syrjälä
2017-04-06 18:29 ` Manasi Navare
@ 2017-04-06 20:43 ` Manasi Navare
2017-04-06 21:00 ` [PATCH v6] " Manasi Navare
1 sibling, 1 reply; 28+ messages in thread
From: Manasi Navare @ 2017-04-06 20:43 UTC (permalink / raw)
To: intel-gfx; +Cc: Jani Nikula
Currently intel_dp_check_link_status() tries to retrain the link if
Clock recovery or Channel EQ for any of the lanes indicated by
intel_dp->lane_count is not set. However these values cached in intel_dp
structure can be stale if link training has failed for these values
during previous modeset. Or these values can get stale since we have
now re read the DPCD registers or it can be 0 in case of connected boot
case.
This patch validates these values against the max link rate and max lane
count values.
This is absolutely required incase the common_rates or max lane count
are now different due to link fallback.
v2:
* Include the FIXME commnet inside the function (Ville Syrjala)
* Remove the redundant parenthesis (Ville Syrjala)
v3 by Jani:
* rebase on the DP refactoring series
* rename intel_dp_link_params_is_valid to intel_dp_link_params_valid
* minor stylistic changes
v4:
* Compare the link rate against max link rate not the
common_rates since common_rates does not account for the
lowered fallback link rate value. (Ville Syrjala)
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 28 +++++++++++++++++++++++++---
1 file changed, 25 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3c5c80d..f3cca3c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -322,6 +322,26 @@ static int intel_dp_common_len_rate_limit(struct intel_dp *intel_dp,
return 0;
}
+static bool intel_dp_link_params_valid(struct intel_dp *intel_dp)
+{
+ int index;
+
+ /*
+ * FIXME: we need to synchronize the current link parameters with
+ * hardware readout. Currently fast link training doesn't work on
+ * boot-up.
+ */
+ if (intel_dp->link_rate == 0 ||
+ intel_dp->link_rate > intel_dp->max_link_rate)
+ return false;
+
+ if (intel_dp->lane_count == 0 ||
+ intel_dp->lane_count > intel_dp_max_lane_count(intel_dp))
+ return false;
+
+ return true;
+}
+
int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
int link_rate, uint8_t lane_count)
{
@@ -4253,9 +4273,11 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
if (!to_intel_crtc(intel_encoder->base.crtc)->active)
return;
- /* FIXME: we need to synchronize this sort of stuff with hardware
- * readout. Currently fast link training doesn't work on boot-up. */
- if (!intel_dp->lane_count)
+ /*
+ * Validate the cached values of intel_dp->link_rate and
+ * intel_dp->lane_count before attempting to retrain.
+ */
+ if (!intel_dp_link_params_valid(intel_dp))
return;
/* Retrain if Channel EQ or CR not ok */
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v6] drm/i915/dp: Validate cached link rate and lane count before retraining
2017-04-06 20:43 ` [PATCH v5] " Manasi Navare
@ 2017-04-06 21:00 ` Manasi Navare
2017-04-12 14:00 ` Ville Syrjälä
0 siblings, 1 reply; 28+ messages in thread
From: Manasi Navare @ 2017-04-06 21:00 UTC (permalink / raw)
To: intel-gfx; +Cc: Jani Nikula
Currently intel_dp_check_link_status() tries to retrain the link if
Clock recovery or Channel EQ for any of the lanes indicated by
intel_dp->lane_count is not set. However these values cached in intel_dp
structure can be stale if link training has failed for these values
during previous modeset. Or these values can get stale since we have
now re read the DPCD registers or it can be 0 in case of connected boot
case.
This patch validates these values against the max link rate and max lane
count values.
This is absolutely required incase the common_rates or max lane count
are now different due to link fallback.
v2:
* Include the FIXME commnet inside the function (Ville Syrjala)
* Remove the redundant parenthesis (Ville Syrjala)
v3 by Jani:
* rebase on the DP refactoring series
* rename intel_dp_link_params_is_valid to intel_dp_link_params_valid
* minor stylistic changes
v4:
* Compare the link rate against max link rate not the
common_rates since common_rates does not account for the
lowered fallback link rate value. (Ville Syrjala)
v5:
* Fixed a warning for unused variable (Manasi)
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 26 +++++++++++++++++++++++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3c5c80d..36485be 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -322,6 +322,24 @@ static int intel_dp_common_len_rate_limit(struct intel_dp *intel_dp,
return 0;
}
+static bool intel_dp_link_params_valid(struct intel_dp *intel_dp)
+{
+ /*
+ * FIXME: we need to synchronize the current link parameters with
+ * hardware readout. Currently fast link training doesn't work on
+ * boot-up.
+ */
+ if (intel_dp->link_rate == 0 ||
+ intel_dp->link_rate > intel_dp->max_link_rate)
+ return false;
+
+ if (intel_dp->lane_count == 0 ||
+ intel_dp->lane_count > intel_dp_max_lane_count(intel_dp))
+ return false;
+
+ return true;
+}
+
int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
int link_rate, uint8_t lane_count)
{
@@ -4253,9 +4271,11 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
if (!to_intel_crtc(intel_encoder->base.crtc)->active)
return;
- /* FIXME: we need to synchronize this sort of stuff with hardware
- * readout. Currently fast link training doesn't work on boot-up. */
- if (!intel_dp->lane_count)
+ /*
+ * Validate the cached values of intel_dp->link_rate and
+ * intel_dp->lane_count before attempting to retrain.
+ */
+ if (!intel_dp_link_params_valid(intel_dp))
return;
/* Retrain if Channel EQ or CR not ok */
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915/dp: rest of refactoring, link rate fallback (rev3)
2017-04-06 13:44 [PATCH v4 00/11] drm/i915/dp: rest of refactoring, link rate fallback Jani Nikula
` (12 preceding siblings ...)
2017-04-06 20:42 ` ✗ Fi.CI.BAT: failure for drm/i915/dp: rest of refactoring, link rate fallback (rev2) Patchwork
@ 2017-04-06 21:54 ` Patchwork
13 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2017-04-06 21:54 UTC (permalink / raw)
To: Navare, Manasi D; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/dp: rest of refactoring, link rate fallback (rev3)
URL : https://patchwork.freedesktop.org/series/22586/
State : success
== Summary ==
Series 22586v3 drm/i915/dp: rest of refactoring, link rate fallback
https://patchwork.freedesktop.org/api/1.0/series/22586/revisions/3/mbox/
fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 432s
fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time: 425s
fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time: 577s
fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 514s
fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time: 541s
fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time: 486s
fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 482s
fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 408s
fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 405s
fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 424s
fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 492s
fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 487s
fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 452s
fi-kbl-7560u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 567s
fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 454s
fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 567s
fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 464s
fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 494s
fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time: 429s
fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 524s
fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 401s
7aafd5c8395fea9a79fbee82de4ffc63c04790d6 drm-tip: 2017y-04m-06d-21h-02m-00s UTC integration manifest
d09e78a drm/i915: Implement Link Rate fallback on Link training failure
91e58a6 drm/i915/dp: Validate cached link rate and lane count before retraining
d49bedb drm/i915/dp: read sink count to a temporary variable first
c5150ed drm/i915/dp: use readb and writeb calls for single byte DPCD access
5f81a04 drm/i915/dp: localize link rate index variable more
f7874d5 drm/i915/mst: use max link not sink lane count
ffeaa33 drm/i915/dp: add functions for max common link rate and lane count
dd6ff8d drm/i915/dp: don't call the link parameters sink parameters
c6ffc38 drm/i915/dp: do not limit rate seek when not needed
f9d73b0 drm/i915/dp: cache common rates with sink rates
e45eeb8 drm/i915/dp: use the sink rates array for max sink rates
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4431/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 09/11] drm/i915/dp: read sink count to a temporary variable first
2017-04-06 14:25 ` Ville Syrjälä
@ 2017-04-11 14:19 ` Jani Nikula
2017-04-11 15:44 ` Navare, Manasi D
0 siblings, 1 reply; 28+ messages in thread
From: Jani Nikula @ 2017-04-11 14:19 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, dhinakaran.pandiyan
On Thu, 06 Apr 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Apr 06, 2017 at 04:44:17PM +0300, Jani Nikula wrote:
>> 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>
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Thanks for the reviews, pushed patches 1-9 to drm-intel-next-queued.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 09/11] drm/i915/dp: read sink count to a temporary variable first
2017-04-11 14:19 ` Jani Nikula
@ 2017-04-11 15:44 ` Navare, Manasi D
0 siblings, 0 replies; 28+ messages in thread
From: Navare, Manasi D @ 2017-04-11 15:44 UTC (permalink / raw)
To: Nikula, Jani, Ville Syrjälä; +Cc: intel-gfx, Pandiyan, Dhinakaran
Thanks Jani for pushing patches 1-9.
Now we just need review on Patch 10 (Validate cached link rate and lane count), may
Be Ville can review that. I have submitted new revision based on his comments already.
And Patch 11 already has your R-b.
Regards
Manasi
On Thu, 06 Apr 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Apr 06, 2017 at 04:44:17PM +0300, Jani Nikula wrote:
>> 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>
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Thanks for the reviews, pushed patches 1-9 to drm-intel-next-queued.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v6] drm/i915/dp: Validate cached link rate and lane count before retraining
2017-04-06 21:00 ` [PATCH v6] " Manasi Navare
@ 2017-04-12 14:00 ` Ville Syrjälä
2017-04-12 14:13 ` Jani Nikula
0 siblings, 1 reply; 28+ messages in thread
From: Ville Syrjälä @ 2017-04-12 14:00 UTC (permalink / raw)
To: Manasi Navare; +Cc: Jani Nikula, intel-gfx
On Thu, Apr 06, 2017 at 02:00:12PM -0700, Manasi Navare wrote:
> Currently intel_dp_check_link_status() tries to retrain the link if
> Clock recovery or Channel EQ for any of the lanes indicated by
> intel_dp->lane_count is not set. However these values cached in intel_dp
> structure can be stale if link training has failed for these values
> during previous modeset. Or these values can get stale since we have
> now re read the DPCD registers or it can be 0 in case of connected boot
> case.
>
> This patch validates these values against the max link rate and max lane
> count values.
>
> This is absolutely required incase the common_rates or max lane count
> are now different due to link fallback.
>
> v2:
> * Include the FIXME commnet inside the function (Ville Syrjala)
> * Remove the redundant parenthesis (Ville Syrjala)
>
> v3 by Jani:
> * rebase on the DP refactoring series
> * rename intel_dp_link_params_is_valid to intel_dp_link_params_valid
> * minor stylistic changes
>
> v4:
> * Compare the link rate against max link rate not the
> common_rates since common_rates does not account for the
> lowered fallback link rate value. (Ville Syrjala)
>
> v5:
> * Fixed a warning for unused variable (Manasi)
>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 26 +++++++++++++++++++++++---
> 1 file changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 3c5c80d..36485be 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -322,6 +322,24 @@ static int intel_dp_common_len_rate_limit(struct intel_dp *intel_dp,
> return 0;
> }
>
> +static bool intel_dp_link_params_valid(struct intel_dp *intel_dp)
> +{
> + /*
> + * FIXME: we need to synchronize the current link parameters with
> + * hardware readout. Currently fast link training doesn't work on
> + * boot-up.
> + */
> + if (intel_dp->link_rate == 0 ||
> + intel_dp->link_rate > intel_dp->max_link_rate)
> + return false;
> +
> + if (intel_dp->lane_count == 0 ||
> + intel_dp->lane_count > intel_dp_max_lane_count(intel_dp))
> + return false;
> +
> + return true;
> +}
This lgtm
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> +
> int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> int link_rate, uint8_t lane_count)
> {
> @@ -4253,9 +4271,11 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
> if (!to_intel_crtc(intel_encoder->base.crtc)->active)
> return;
>
> - /* FIXME: we need to synchronize this sort of stuff with hardware
> - * readout. Currently fast link training doesn't work on boot-up. */
> - if (!intel_dp->lane_count)
> + /*
> + * Validate the cached values of intel_dp->link_rate and
> + * intel_dp->lane_count before attempting to retrain.
> + */
> + if (!intel_dp_link_params_valid(intel_dp))
> return;
>
> /* Retrain if Channel EQ or CR not ok */
> --
> 2.1.4
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v6] drm/i915/dp: Validate cached link rate and lane count before retraining
2017-04-12 14:00 ` Ville Syrjälä
@ 2017-04-12 14:13 ` Jani Nikula
2017-04-12 16:50 ` Navare, Manasi D
0 siblings, 1 reply; 28+ messages in thread
From: Jani Nikula @ 2017-04-12 14:13 UTC (permalink / raw)
To: Ville Syrjälä, Manasi Navare; +Cc: intel-gfx
On Wed, 12 Apr 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Apr 06, 2017 at 02:00:12PM -0700, Manasi Navare wrote:
>> Currently intel_dp_check_link_status() tries to retrain the link if
>> Clock recovery or Channel EQ for any of the lanes indicated by
>> intel_dp->lane_count is not set. However these values cached in intel_dp
>> structure can be stale if link training has failed for these values
>> during previous modeset. Or these values can get stale since we have
>> now re read the DPCD registers or it can be 0 in case of connected boot
>> case.
>>
>> This patch validates these values against the max link rate and max lane
>> count values.
>>
>> This is absolutely required incase the common_rates or max lane count
>> are now different due to link fallback.
>>
>> v2:
>> * Include the FIXME commnet inside the function (Ville Syrjala)
>> * Remove the redundant parenthesis (Ville Syrjala)
>>
>> v3 by Jani:
>> * rebase on the DP refactoring series
>> * rename intel_dp_link_params_is_valid to intel_dp_link_params_valid
>> * minor stylistic changes
>>
>> v4:
>> * Compare the link rate against max link rate not the
>> common_rates since common_rates does not account for the
>> lowered fallback link rate value. (Ville Syrjala)
>>
>> v5:
>> * Fixed a warning for unused variable (Manasi)
>>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_dp.c | 26 +++++++++++++++++++++++---
>> 1 file changed, 23 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 3c5c80d..36485be 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -322,6 +322,24 @@ static int intel_dp_common_len_rate_limit(struct intel_dp *intel_dp,
>> return 0;
>> }
>>
>> +static bool intel_dp_link_params_valid(struct intel_dp *intel_dp)
>> +{
>> + /*
>> + * FIXME: we need to synchronize the current link parameters with
>> + * hardware readout. Currently fast link training doesn't work on
>> + * boot-up.
>> + */
>> + if (intel_dp->link_rate == 0 ||
>> + intel_dp->link_rate > intel_dp->max_link_rate)
>> + return false;
>> +
>> + if (intel_dp->lane_count == 0 ||
>> + intel_dp->lane_count > intel_dp_max_lane_count(intel_dp))
>> + return false;
>> +
>> + return true;
>> +}
>
> This lgtm
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Pushed to drm-intel-next-queued, thanks for the patch and review.
BR,
Jani.
>
>> +
>> int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
>> int link_rate, uint8_t lane_count)
>> {
>> @@ -4253,9 +4271,11 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>> if (!to_intel_crtc(intel_encoder->base.crtc)->active)
>> return;
>>
>> - /* FIXME: we need to synchronize this sort of stuff with hardware
>> - * readout. Currently fast link training doesn't work on boot-up. */
>> - if (!intel_dp->lane_count)
>> + /*
>> + * Validate the cached values of intel_dp->link_rate and
>> + * intel_dp->lane_count before attempting to retrain.
>> + */
>> + if (!intel_dp_link_params_valid(intel_dp))
>> return;
>>
>> /* Retrain if Channel EQ or CR not ok */
>> --
>> 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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v6] drm/i915/dp: Validate cached link rate and lane count before retraining
2017-04-12 14:13 ` Jani Nikula
@ 2017-04-12 16:50 ` Navare, Manasi D
0 siblings, 0 replies; 28+ messages in thread
From: Navare, Manasi D @ 2017-04-12 16:50 UTC (permalink / raw)
To: Jani Nikula, Ville Syrjälä; +Cc: intel-gfx
Thanks Ville for the review and thanks Jani for pushing
this patch. Now we are down to 1 patch to get merged!
Regards
Manasi
On Wed, 12 Apr 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Apr 06, 2017 at 02:00:12PM -0700, Manasi Navare wrote:
>> Currently intel_dp_check_link_status() tries to retrain the link if
>> Clock recovery or Channel EQ for any of the lanes indicated by
>> intel_dp->lane_count is not set. However these values cached in
>> intel_dp structure can be stale if link training has failed for these
>> values during previous modeset. Or these values can get stale since
>> we have now re read the DPCD registers or it can be 0 in case of
>> connected boot case.
>>
>> This patch validates these values against the max link rate and max
>> lane count values.
>>
>> This is absolutely required incase the common_rates or max lane count
>> are now different due to link fallback.
>>
>> v2:
>> * Include the FIXME commnet inside the function (Ville Syrjala)
>> * Remove the redundant parenthesis (Ville Syrjala)
>>
>> v3 by Jani:
>> * rebase on the DP refactoring series
>> * rename intel_dp_link_params_is_valid to intel_dp_link_params_valid
>> * minor stylistic changes
>>
>> v4:
>> * Compare the link rate against max link rate not the common_rates
>> since common_rates does not account for the lowered fallback link
>> rate value. (Ville Syrjala)
>>
>> v5:
>> * Fixed a warning for unused variable (Manasi)
>>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_dp.c | 26 +++++++++++++++++++++++---
>> 1 file changed, 23 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>> b/drivers/gpu/drm/i915/intel_dp.c index 3c5c80d..36485be 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -322,6 +322,24 @@ static int intel_dp_common_len_rate_limit(struct intel_dp *intel_dp,
>> return 0;
>> }
>>
>> +static bool intel_dp_link_params_valid(struct intel_dp *intel_dp) {
>> + /*
>> + * FIXME: we need to synchronize the current link parameters with
>> + * hardware readout. Currently fast link training doesn't work on
>> + * boot-up.
>> + */
>> + if (intel_dp->link_rate == 0 ||
>> + intel_dp->link_rate > intel_dp->max_link_rate)
>> + return false;
>> +
>> + if (intel_dp->lane_count == 0 ||
>> + intel_dp->lane_count > intel_dp_max_lane_count(intel_dp))
>> + return false;
>> +
>> + return true;
>> +}
>
> This lgtm
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Pushed to drm-intel-next-queued, thanks for the patch and review.
BR,
Jani.
>
>> +
>> int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
>> int link_rate, uint8_t lane_count) { @@ -4253,9 +4271,11
>> @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>> if (!to_intel_crtc(intel_encoder->base.crtc)->active)
>> return;
>>
>> - /* FIXME: we need to synchronize this sort of stuff with hardware
>> - * readout. Currently fast link training doesn't work on boot-up. */
>> - if (!intel_dp->lane_count)
>> + /*
>> + * Validate the cached values of intel_dp->link_rate and
>> + * intel_dp->lane_count before attempting to retrain.
>> + */
>> + if (!intel_dp_link_params_valid(intel_dp))
>> return;
>>
>> /* Retrain if Channel EQ or CR not ok */
>> --
>> 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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 11/11] drm/i915: Implement Link Rate fallback on Link training failure
2017-04-06 13:44 ` [PATCH v4 11/11] drm/i915: Implement Link Rate fallback on Link training failure Jani Nikula
@ 2017-04-13 17:25 ` Manasi Navare
2017-04-13 19:16 ` Jani Nikula
0 siblings, 1 reply; 28+ messages in thread
From: Manasi Navare @ 2017-04-13 17:25 UTC (permalink / raw)
To: Jani Nikula; +Cc: Daniel Vetter, intel-gfx, dhinakaran.pandiyan
Hi Jani,
This patch has the necessary ACKs and R-bs, anything else
blocking this from getting merged?
Regards
Manasi
On Thu, Apr 06, 2017 at 04:44:19PM +0300, Jani Nikula wrote:
> From: Manasi Navare <manasi.d.navare@intel.com>
>
> If link training at a link rate optimal for a particular
> mode fails during modeset's atomic commit phase, then we
> let the modeset complete and then retry. We save the link rate
> value at which link training failed, update the link status property
> to "BAD" and use a lower link rate to prune the modes. It will redo
> the modeset on the current mode at lower link rate or if the current
> mode gets pruned due to lower link constraints then, it will send a
> hotplug uevent for userspace to handle it.
>
> This is also required to pass DP CTS tests 4.3.1.3, 4.3.1.4,
> 4.3.1.6.
>
> This patch is a resend of the original commit id (233ce881dd91fb
> "drm/i915: Implement Link Rate fallback on Link training failure")
> which got reverted in this commit id (afc1ebf4562a14 Revert
> "drm/i915: Implement Link Rate fallback on Link training failure")
> due to CI failures.
>
> After investigating the CI failures it was found that these
> were essentially the failures which were always there but hidden because
> they used to be DRM_DEBUG_KMS messages for link failures so never got
> caught by CI. But now this patch actually throws DRM_ERROR if the link
> training fails at RBR and 1 lane. So it caught these link train failures.
>
> There were two failures:
> 1. On SKL 6700k this was because the machine in CI lab is a SKL desktop
> without eDP on Port A. But our VBT initialization code in the driver writes
> VBT defaults in a way that it always sets DP flag on Port A and this does
> not get cleared after parsing the VBT outputs. This has been fixed in
> commit id (bb1d132935c2f8 "drm/i915/vbt: split out defaults that are set
> when there is no VBT) and (665788572c6410b "drm/i915/vbt: don't propagate
> errors from intel_bios_init())
>
> 2. On ILK-650 desktop - This was happening because of a bad monitor desktop
> combination. I switched the monitor in the CI lab and that helped get rid
> of the link failures on ILK system.
>
> v10:
> * Rebase on drm-tip and resend after revert
> v9:
> * Use the trimmed max values of link rate/lane count based on
> link train fallback (Daniel Vetter)
> v8:
> * Set link_status to BAD first and then call mode_valid (Jani Nikula)
> v7:
> Remove the redundant variable in previous patch itself
> v6:
> * Obtain link rate index from fallback_link_rate using
> the helper intel_dp_link_rate_index (Jani Nikula)
> * Include fallback within intel_dp_start_link_train (Jani Nikula)
> v5:
> * Move set link status to drm core (Daniel Vetter, Jani Nikula)
> v4:
> * Add fallback support for non DDI platforms too
> * Set connector->link status inside set_link_status function
> (Jani Nikula)
> v3:
> * Set link status property to BAd unconditionally (Jani Nikula)
> * Dont use two separate variables link_train_failed and link_status
> to indicate same thing (Jani Nikula)
> v2:
> * Squashed a few patches (Jani Nikula)
>
> Acked-by: Tony Cheng <tony.cheng@amd.com>
> Acked-by: Harry Wentland <Harry.wentland@amd.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 27 +++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_dp_link_training.c | 22 ++++++++++++++++++++--
> drivers/gpu/drm/i915/intel_drv.h | 3 +++
> 3 files changed, 50 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 6915d89a238c..f2d792a1b18e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5977,6 +5977,29 @@ intel_dp_init_connector_port_info(struct intel_digital_port *intel_dig_port)
> }
> }
>
> +static void intel_dp_modeset_retry_work_fn(struct work_struct *work)
> +{
> + struct intel_connector *intel_connector;
> + struct drm_connector *connector;
> +
> + intel_connector = container_of(work, typeof(*intel_connector),
> + modeset_retry_work);
> + connector = &intel_connector->base;
> + DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id,
> + connector->name);
> +
> + /* Grab the locks before changing connector property*/
> + mutex_lock(&connector->dev->mode_config.mutex);
> + /* Set connector link status to BAD and send a Uevent to notify
> + * userspace to do a modeset.
> + */
> + drm_mode_connector_set_link_status_property(connector,
> + DRM_MODE_LINK_STATUS_BAD);
> + mutex_unlock(&connector->dev->mode_config.mutex);
> + /* Send Hotplug uevent so userspace can reprobe */
> + drm_kms_helper_hotplug_event(connector->dev);
> +}
> +
> bool
> intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> struct intel_connector *intel_connector)
> @@ -5989,6 +6012,10 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> enum port port = intel_dig_port->port;
> int type;
>
> + /* Initialize the work for modeset in case of link train failure */
> + INIT_WORK(&intel_connector->modeset_retry_work,
> + intel_dp_modeset_retry_work_fn);
> +
> if (WARN(intel_dig_port->max_lanes < 1,
> "Not enough lanes (%d) for DP on port %c\n",
> intel_dig_port->max_lanes, port_name(port)))
> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
> index 694ad0ffb523..b79c1c0e404c 100644
> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> @@ -314,6 +314,24 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp)
> void
> intel_dp_start_link_train(struct intel_dp *intel_dp)
> {
> - intel_dp_link_training_clock_recovery(intel_dp);
> - intel_dp_link_training_channel_equalization(intel_dp);
> + struct intel_connector *intel_connector = intel_dp->attached_connector;
> +
> + if (!intel_dp_link_training_clock_recovery(intel_dp))
> + goto failure_handling;
> + if (!intel_dp_link_training_channel_equalization(intel_dp))
> + goto failure_handling;
> +
> + DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d",
> + intel_dp->link_rate, intel_dp->lane_count);
> + return;
> +
> + failure_handling:
> + DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d",
> + intel_dp->link_rate, intel_dp->lane_count);
> + if (!intel_dp_get_link_train_fallback_values(intel_dp,
> + intel_dp->link_rate,
> + intel_dp->lane_count))
> + /* Schedule a Hotplug Uevent to userspace to start modeset */
> + schedule_work(&intel_connector->modeset_retry_work);
> + return;
> }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 5f6e1aad909d..d36026eff7cd 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -321,6 +321,9 @@ struct intel_connector {
> void *port; /* store this opaque as its illegal to dereference it */
>
> struct intel_dp *mst_port;
> +
> + /* Work struct to schedule a uevent on link train failure */
> + struct work_struct modeset_retry_work;
> };
>
> struct dpll {
> --
> 2.1.4
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 11/11] drm/i915: Implement Link Rate fallback on Link training failure
2017-04-13 17:25 ` Manasi Navare
@ 2017-04-13 19:16 ` Jani Nikula
0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2017-04-13 19:16 UTC (permalink / raw)
To: Manasi Navare; +Cc: Daniel Vetter, intel-gfx, dhinakaran.pandiyan
On Thu, 13 Apr 2017, Manasi Navare <manasi.d.navare@intel.com> wrote:
> Hi Jani,
>
> This patch has the necessary ACKs and R-bs, anything else
> blocking this from getting merged?
Pushed, finally, sorry for the delay.
BR,
Jani.
>
> Regards
> Manasi
>
> On Thu, Apr 06, 2017 at 04:44:19PM +0300, Jani Nikula wrote:
>> From: Manasi Navare <manasi.d.navare@intel.com>
>>
>> If link training at a link rate optimal for a particular
>> mode fails during modeset's atomic commit phase, then we
>> let the modeset complete and then retry. We save the link rate
>> value at which link training failed, update the link status property
>> to "BAD" and use a lower link rate to prune the modes. It will redo
>> the modeset on the current mode at lower link rate or if the current
>> mode gets pruned due to lower link constraints then, it will send a
>> hotplug uevent for userspace to handle it.
>>
>> This is also required to pass DP CTS tests 4.3.1.3, 4.3.1.4,
>> 4.3.1.6.
>>
>> This patch is a resend of the original commit id (233ce881dd91fb
>> "drm/i915: Implement Link Rate fallback on Link training failure")
>> which got reverted in this commit id (afc1ebf4562a14 Revert
>> "drm/i915: Implement Link Rate fallback on Link training failure")
>> due to CI failures.
>>
>> After investigating the CI failures it was found that these
>> were essentially the failures which were always there but hidden because
>> they used to be DRM_DEBUG_KMS messages for link failures so never got
>> caught by CI. But now this patch actually throws DRM_ERROR if the link
>> training fails at RBR and 1 lane. So it caught these link train failures.
>>
>> There were two failures:
>> 1. On SKL 6700k this was because the machine in CI lab is a SKL desktop
>> without eDP on Port A. But our VBT initialization code in the driver writes
>> VBT defaults in a way that it always sets DP flag on Port A and this does
>> not get cleared after parsing the VBT outputs. This has been fixed in
>> commit id (bb1d132935c2f8 "drm/i915/vbt: split out defaults that are set
>> when there is no VBT) and (665788572c6410b "drm/i915/vbt: don't propagate
>> errors from intel_bios_init())
>>
>> 2. On ILK-650 desktop - This was happening because of a bad monitor desktop
>> combination. I switched the monitor in the CI lab and that helped get rid
>> of the link failures on ILK system.
>>
>> v10:
>> * Rebase on drm-tip and resend after revert
>> v9:
>> * Use the trimmed max values of link rate/lane count based on
>> link train fallback (Daniel Vetter)
>> v8:
>> * Set link_status to BAD first and then call mode_valid (Jani Nikula)
>> v7:
>> Remove the redundant variable in previous patch itself
>> v6:
>> * Obtain link rate index from fallback_link_rate using
>> the helper intel_dp_link_rate_index (Jani Nikula)
>> * Include fallback within intel_dp_start_link_train (Jani Nikula)
>> v5:
>> * Move set link status to drm core (Daniel Vetter, Jani Nikula)
>> v4:
>> * Add fallback support for non DDI platforms too
>> * Set connector->link status inside set_link_status function
>> (Jani Nikula)
>> v3:
>> * Set link status property to BAd unconditionally (Jani Nikula)
>> * Dont use two separate variables link_train_failed and link_status
>> to indicate same thing (Jani Nikula)
>> v2:
>> * Squashed a few patches (Jani Nikula)
>>
>> Acked-by: Tony Cheng <tony.cheng@amd.com>
>> Acked-by: Harry Wentland <Harry.wentland@amd.com>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Cc: Daniel Vetter <daniel.vetter@intel.com>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
>> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_dp.c | 27 +++++++++++++++++++++++++++
>> drivers/gpu/drm/i915/intel_dp_link_training.c | 22 ++++++++++++++++++++--
>> drivers/gpu/drm/i915/intel_drv.h | 3 +++
>> 3 files changed, 50 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 6915d89a238c..f2d792a1b18e 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -5977,6 +5977,29 @@ intel_dp_init_connector_port_info(struct intel_digital_port *intel_dig_port)
>> }
>> }
>>
>> +static void intel_dp_modeset_retry_work_fn(struct work_struct *work)
>> +{
>> + struct intel_connector *intel_connector;
>> + struct drm_connector *connector;
>> +
>> + intel_connector = container_of(work, typeof(*intel_connector),
>> + modeset_retry_work);
>> + connector = &intel_connector->base;
>> + DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id,
>> + connector->name);
>> +
>> + /* Grab the locks before changing connector property*/
>> + mutex_lock(&connector->dev->mode_config.mutex);
>> + /* Set connector link status to BAD and send a Uevent to notify
>> + * userspace to do a modeset.
>> + */
>> + drm_mode_connector_set_link_status_property(connector,
>> + DRM_MODE_LINK_STATUS_BAD);
>> + mutex_unlock(&connector->dev->mode_config.mutex);
>> + /* Send Hotplug uevent so userspace can reprobe */
>> + drm_kms_helper_hotplug_event(connector->dev);
>> +}
>> +
>> bool
>> intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>> struct intel_connector *intel_connector)
>> @@ -5989,6 +6012,10 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>> enum port port = intel_dig_port->port;
>> int type;
>>
>> + /* Initialize the work for modeset in case of link train failure */
>> + INIT_WORK(&intel_connector->modeset_retry_work,
>> + intel_dp_modeset_retry_work_fn);
>> +
>> if (WARN(intel_dig_port->max_lanes < 1,
>> "Not enough lanes (%d) for DP on port %c\n",
>> intel_dig_port->max_lanes, port_name(port)))
>> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
>> index 694ad0ffb523..b79c1c0e404c 100644
>> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
>> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
>> @@ -314,6 +314,24 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp)
>> void
>> intel_dp_start_link_train(struct intel_dp *intel_dp)
>> {
>> - intel_dp_link_training_clock_recovery(intel_dp);
>> - intel_dp_link_training_channel_equalization(intel_dp);
>> + struct intel_connector *intel_connector = intel_dp->attached_connector;
>> +
>> + if (!intel_dp_link_training_clock_recovery(intel_dp))
>> + goto failure_handling;
>> + if (!intel_dp_link_training_channel_equalization(intel_dp))
>> + goto failure_handling;
>> +
>> + DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d",
>> + intel_dp->link_rate, intel_dp->lane_count);
>> + return;
>> +
>> + failure_handling:
>> + DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d",
>> + intel_dp->link_rate, intel_dp->lane_count);
>> + if (!intel_dp_get_link_train_fallback_values(intel_dp,
>> + intel_dp->link_rate,
>> + intel_dp->lane_count))
>> + /* Schedule a Hotplug Uevent to userspace to start modeset */
>> + schedule_work(&intel_connector->modeset_retry_work);
>> + return;
>> }
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 5f6e1aad909d..d36026eff7cd 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -321,6 +321,9 @@ struct intel_connector {
>> void *port; /* store this opaque as its illegal to dereference it */
>>
>> struct intel_dp *mst_port;
>> +
>> + /* Work struct to schedule a uevent on link train failure */
>> + struct work_struct modeset_retry_work;
>> };
>>
>> struct dpll {
>> --
>> 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
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2017-04-13 19:16 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-06 13:44 [PATCH v4 00/11] drm/i915/dp: rest of refactoring, link rate fallback Jani Nikula
2017-04-06 13:44 ` [PATCH v4 01/11] drm/i915/dp: use the sink rates array for max sink rates Jani Nikula
2017-04-06 14:24 ` Ville Syrjälä
2017-04-06 13:44 ` [PATCH v4 02/11] drm/i915/dp: cache common rates with " Jani Nikula
2017-04-06 13:44 ` [PATCH v4 03/11] drm/i915/dp: do not limit rate seek when not needed Jani Nikula
2017-04-06 13:44 ` [PATCH v4 04/11] drm/i915/dp: don't call the link parameters sink parameters Jani Nikula
2017-04-06 13:44 ` [PATCH v4 05/11] drm/i915/dp: add functions for max common link rate and lane count Jani Nikula
2017-04-06 13:44 ` [PATCH v4 06/11] drm/i915/mst: use max link not sink " Jani Nikula
2017-04-06 13:44 ` [PATCH v4 07/11] drm/i915/dp: localize link rate index variable more Jani Nikula
2017-04-06 13:44 ` [PATCH v4 08/11] drm/i915/dp: use readb and writeb calls for single byte DPCD access Jani Nikula
2017-04-06 13:44 ` [PATCH v4 09/11] drm/i915/dp: read sink count to a temporary variable first Jani Nikula
2017-04-06 14:25 ` Ville Syrjälä
2017-04-11 14:19 ` Jani Nikula
2017-04-11 15:44 ` Navare, Manasi D
2017-04-06 13:44 ` [PATCH v4 10/11] drm/i915/dp: Validate cached link rate and lane count before retraining Jani Nikula
2017-04-06 14:31 ` Ville Syrjälä
2017-04-06 18:29 ` Manasi Navare
2017-04-06 20:43 ` [PATCH v5] " Manasi Navare
2017-04-06 21:00 ` [PATCH v6] " Manasi Navare
2017-04-12 14:00 ` Ville Syrjälä
2017-04-12 14:13 ` Jani Nikula
2017-04-12 16:50 ` Navare, Manasi D
2017-04-06 13:44 ` [PATCH v4 11/11] drm/i915: Implement Link Rate fallback on Link training failure Jani Nikula
2017-04-13 17:25 ` Manasi Navare
2017-04-13 19:16 ` Jani Nikula
2017-04-06 14:35 ` ✓ Fi.CI.BAT: success for drm/i915/dp: rest of refactoring, link rate fallback Patchwork
2017-04-06 20:42 ` ✗ Fi.CI.BAT: failure for drm/i915/dp: rest of refactoring, link rate fallback (rev2) Patchwork
2017-04-06 21:54 ` ✓ Fi.CI.BAT: success for drm/i915/dp: rest of refactoring, link rate fallback (rev3) Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.