* [PATCH 1/5] drm/i915/mst: Debug log connector name in destroy_connector()
@ 2017-09-18 22:21 Dhinakaran Pandiyan
2017-09-18 22:21 ` [PATCH 2/5] drm/i915/mst: Print active mst links after update Dhinakaran Pandiyan
` (6 more replies)
0 siblings, 7 replies; 17+ messages in thread
From: Dhinakaran Pandiyan @ 2017-09-18 22:21 UTC (permalink / raw)
To: intel-gfx; +Cc: Pandiyan, Dhinakaran
From: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
Print connector name in destroy_connect() and this doesn't add any extra
lines to dmesg. The debug macro has been moved before the unregister
call so that we don't lose the connector name and id.
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: James Ausmus <james.ausmus@intel.com>
---
drivers/gpu/drm/i915/intel_dp_mst.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 8e3aad0ea60b..88d1d2b9ac56 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -494,6 +494,7 @@ static void intel_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
struct intel_connector *intel_connector = to_intel_connector(connector);
struct drm_i915_private *dev_priv = to_i915(connector->dev);
+ DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id, connector->name);
drm_connector_unregister(connector);
if (dev_priv->fbdev)
@@ -505,7 +506,6 @@ static void intel_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
drm_modeset_unlock(&connector->dev->mode_config.connection_mutex);
drm_connector_unreference(connector);
- DRM_DEBUG_KMS("\n");
}
static void intel_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr)
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/5] drm/i915/mst: Print active mst links after update
2017-09-18 22:21 [PATCH 1/5] drm/i915/mst: Debug log connector name in destroy_connector() Dhinakaran Pandiyan
@ 2017-09-18 22:21 ` Dhinakaran Pandiyan
2017-09-18 22:21 ` [PATCH 3/5] drm/i915/dp: Fix buffer size for sink_irq_esi read Dhinakaran Pandiyan
` (5 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Dhinakaran Pandiyan @ 2017-09-18 22:21 UTC (permalink / raw)
To: intel-gfx; +Cc: Pandiyan, Dhinakaran
From: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
Both mst_disable_dp and mst_post_disable_dp print number of active links
before the variable has been updated. Move the print statement in
mst_disable_dp after the decrement so that the printed values indicate
the disabing of a mst connector. Also, add some text to clarify what we
are printing.
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: James Ausmus <james.ausmus@intel.com>
---
drivers/gpu/drm/i915/intel_dp_mst.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 88d1d2b9ac56..9a396f483f8b 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -133,7 +133,7 @@ static void intel_mst_disable_dp(struct intel_encoder *encoder,
to_intel_connector(old_conn_state->connector);
int ret;
- DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
+ DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
drm_dp_mst_reset_vcpi_slots(&intel_dp->mst_mgr, connector->port);
@@ -155,8 +155,6 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
struct intel_connector *connector =
to_intel_connector(old_conn_state->connector);
- DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
-
/* this can fail */
drm_dp_check_act_status(&intel_dp->mst_mgr);
/* and this can also fail */
@@ -173,6 +171,7 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
}
+ DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
}
static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,
@@ -195,7 +194,7 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,
connector->encoder = encoder;
intel_mst->connector = connector;
- DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
+ DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
if (intel_dp->active_mst_links == 0)
intel_dig_port->base.pre_enable(&intel_dig_port->base,
@@ -229,7 +228,7 @@ static void intel_mst_enable_dp(struct intel_encoder *encoder,
enum port port = intel_dig_port->port;
int ret;
- DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
+ DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
if (intel_wait_for_register(dev_priv,
DP_TP_STATUS(port),
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/5] drm/i915/dp: Fix buffer size for sink_irq_esi read
2017-09-18 22:21 [PATCH 1/5] drm/i915/mst: Debug log connector name in destroy_connector() Dhinakaran Pandiyan
2017-09-18 22:21 ` [PATCH 2/5] drm/i915/mst: Print active mst links after update Dhinakaran Pandiyan
@ 2017-09-18 22:21 ` Dhinakaran Pandiyan
2017-09-18 22:21 ` [PATCH 4/5] drm/i915/dp: Clean up intel_dp_check_mst_status Dhinakaran Pandiyan
` (4 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Dhinakaran Pandiyan @ 2017-09-18 22:21 UTC (permalink / raw)
To: intel-gfx; +Cc: Pandiyan, Dhinakaran
From: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
The buffer size defined is 16 bytes whereas only 14 bytes are read. Add a
macro to avoid this discrepancy.
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: James Ausmus <james.ausmus@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 887953c0f495..98e7b96ca826 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -42,6 +42,7 @@
#include "i915_drv.h"
#define DP_LINK_CHECK_TIMEOUT (10 * 1000)
+#define DP_DPRX_ESI_LEN 14
/* Compliance test status bits */
#define INTEL_DP_RESOLUTION_SHIFT_MASK 0
@@ -3991,15 +3992,9 @@ intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector)
static bool
intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector)
{
- int ret;
-
- ret = drm_dp_dpcd_read(&intel_dp->aux,
- DP_SINK_COUNT_ESI,
- sink_irq_vector, 14);
- if (ret != 14)
- return false;
-
- return true;
+ return drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_COUNT_ESI,
+ sink_irq_vector, DP_DPRX_ESI_LEN) ==
+ DP_DPRX_ESI_LEN;
}
static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp)
@@ -4199,7 +4194,7 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
bool bret;
if (intel_dp->is_mst) {
- u8 esi[16] = { 0 };
+ u8 esi[DP_DPRX_ESI_LEN] = { 0 };
int ret = 0;
int retry;
bool handled;
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/5] drm/i915/dp: Clean up intel_dp_check_mst_status
2017-09-18 22:21 [PATCH 1/5] drm/i915/mst: Debug log connector name in destroy_connector() Dhinakaran Pandiyan
2017-09-18 22:21 ` [PATCH 2/5] drm/i915/mst: Print active mst links after update Dhinakaran Pandiyan
2017-09-18 22:21 ` [PATCH 3/5] drm/i915/dp: Fix buffer size for sink_irq_esi read Dhinakaran Pandiyan
@ 2017-09-18 22:21 ` Dhinakaran Pandiyan
2017-09-20 19:11 ` Ausmus, James
2017-09-18 22:21 ` [PATCH 5/5] drm/i915/dp: Remove useless debug about TPS3 support Dhinakaran Pandiyan
` (3 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Dhinakaran Pandiyan @ 2017-09-18 22:21 UTC (permalink / raw)
To: intel-gfx; +Cc: Jani Nikula, Dhinakaran Pandiyan
Rewriting this code without the goto, I believe, makes it more readable.
One functional change that has been included is the handling of failed ESI
register reads. Instead of disabling MST only for the first failed read, we
now disable MST on subsequent failed reads too. A failed ESI read is
problematic irrespective of whether it is the first or not.
Cc: James Ausmus <james.ausmus@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 75 +++++++++++++++++------------------------
1 file changed, 31 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 98e7b96ca826..cc129aa444ac 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4191,57 +4191,44 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
static int
intel_dp_check_mst_status(struct intel_dp *intel_dp)
{
- bool bret;
+ u8 esi[DP_DPRX_ESI_LEN] = { 0 };
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
- if (intel_dp->is_mst) {
- u8 esi[DP_DPRX_ESI_LEN] = { 0 };
- int ret = 0;
- int retry;
+ if (!intel_dp->is_mst)
+ return -EINVAL;
+
+ while (intel_dp_get_sink_irq_esi(intel_dp, esi)) {
+ int ret, retry;
bool handled;
- bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
-go_again:
- if (bret == true) {
-
- /* check link status - esi[10] = 0x200c */
- if (intel_dp->active_mst_links &&
- !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
- DRM_DEBUG_KMS("channel EQ not ok, retraining\n");
- intel_dp_start_link_train(intel_dp);
- intel_dp_stop_link_train(intel_dp);
- }
- DRM_DEBUG_KMS("got esi %3ph\n", esi);
- ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled);
-
- if (handled) {
- for (retry = 0; retry < 3; retry++) {
- int wret;
- wret = drm_dp_dpcd_write(&intel_dp->aux,
- DP_SINK_COUNT_ESI+1,
- &esi[1], 3);
- if (wret == 3) {
- break;
- }
- }
+ DRM_DEBUG_KMS("ESI %3ph\n", esi);
- bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
- if (bret == true) {
- DRM_DEBUG_KMS("got esi2 %3ph\n", esi);
- goto go_again;
- }
- } else
- ret = 0;
+ /* check link status - esi[10] = 0x200c */
+ if (intel_dp->active_mst_links &&
+ !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
+ intel_dp_start_link_train(intel_dp);
+ intel_dp_stop_link_train(intel_dp);
+ }
- return ret;
- } else {
- struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
- DRM_DEBUG_KMS("failed to get ESI - device may have failed\n");
- intel_dp->is_mst = false;
- drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
- /* send a hotplug event */
- drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev);
+ ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled);
+ if (!handled)
+ return 0;
+
+ for (retry = 0; retry < 3; retry++) {
+ int wret;
+
+ wret = drm_dp_dpcd_write(&intel_dp->aux,
+ DP_SINK_COUNT_ESI + 1, &esi[1],
+ 3);
+ if (wret == 3)
+ break;
}
}
+
+ DRM_DEBUG_KMS("failed to get ESI - device may have failed\n");
+ intel_dp->is_mst = false;
+ drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
+ drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev);
return -EINVAL;
}
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/5] drm/i915/dp: Remove useless debug about TPS3 support
2017-09-18 22:21 [PATCH 1/5] drm/i915/mst: Debug log connector name in destroy_connector() Dhinakaran Pandiyan
` (2 preceding siblings ...)
2017-09-18 22:21 ` [PATCH 4/5] drm/i915/dp: Clean up intel_dp_check_mst_status Dhinakaran Pandiyan
@ 2017-09-18 22:21 ` Dhinakaran Pandiyan
2017-09-19 10:13 ` Jani Nikula
2017-09-19 12:17 ` ✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915/mst: Debug log connector name in destroy_connector() Patchwork
` (2 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Dhinakaran Pandiyan @ 2017-09-18 22:21 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan
We already print training pattern used during link training and also
print if the source or sink does not support TPS3 for HBR2 link rates,
see intel_dp_training_pattern().
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index cc129aa444ac..2d41cd684557 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4718,10 +4718,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
if (intel_encoder->type != INTEL_OUTPUT_EDP)
intel_encoder->type = INTEL_OUTPUT_DP;
- DRM_DEBUG_KMS("Display Port TPS3 support: source %s, sink %s\n",
- yesno(intel_dp_source_supports_hbr2(intel_dp)),
- yesno(drm_dp_tps3_supported(intel_dp->dpcd)));
-
if (intel_dp->reset_link_params) {
/* Initial max link lane count */
intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 5/5] drm/i915/dp: Remove useless debug about TPS3 support
2017-09-18 22:21 ` [PATCH 5/5] drm/i915/dp: Remove useless debug about TPS3 support Dhinakaran Pandiyan
@ 2017-09-19 10:13 ` Jani Nikula
0 siblings, 0 replies; 17+ messages in thread
From: Jani Nikula @ 2017-09-19 10:13 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan
On Mon, 18 Sep 2017, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> We already print training pattern used during link training and also
> print if the source or sink does not support TPS3 for HBR2 link rates,
> see intel_dp_training_pattern().
Yeah, this was useful a long time ago.
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index cc129aa444ac..2d41cd684557 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4718,10 +4718,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
> if (intel_encoder->type != INTEL_OUTPUT_EDP)
> intel_encoder->type = INTEL_OUTPUT_DP;
>
> - DRM_DEBUG_KMS("Display Port TPS3 support: source %s, sink %s\n",
> - yesno(intel_dp_source_supports_hbr2(intel_dp)),
> - yesno(drm_dp_tps3_supported(intel_dp->dpcd)));
> -
> if (intel_dp->reset_link_params) {
> /* Initial max link lane count */
> intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
--
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] 17+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915/mst: Debug log connector name in destroy_connector()
2017-09-18 22:21 [PATCH 1/5] drm/i915/mst: Debug log connector name in destroy_connector() Dhinakaran Pandiyan
` (3 preceding siblings ...)
2017-09-18 22:21 ` [PATCH 5/5] drm/i915/dp: Remove useless debug about TPS3 support Dhinakaran Pandiyan
@ 2017-09-19 12:17 ` Patchwork
2017-09-19 16:18 ` ✗ Fi.CI.IGT: warning " Patchwork
2017-09-21 19:18 ` ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/mst: Debug log connector name in destroy_connector() (rev2) Patchwork
6 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2017-09-19 12:17 UTC (permalink / raw)
To: Pandiyan, Dhinakaran; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/5] drm/i915/mst: Debug log connector name in destroy_connector()
URL : https://patchwork.freedesktop.org/series/30556/
State : success
== Summary ==
Series 30556v1 series starting with [1/5] drm/i915/mst: Debug log connector name in destroy_connector()
https://patchwork.freedesktop.org/api/1.0/series/30556/revisions/1/mbox/
Test gem_exec_reloc:
Subgroup basic-write-cpu:
dmesg-warn -> PASS (fi-kbl-7500u) fdo#102849
Test gem_exec_suspend:
Subgroup basic-s3:
incomplete -> PASS (fi-kbl-7500u) fdo#102850
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-atomic:
pass -> FAIL (fi-snb-2600) fdo#100215 +1
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
notrun -> INCOMPLETE (fi-kbl-7500u)
fdo#102849 https://bugs.freedesktop.org/show_bug.cgi?id=102849
fdo#102850 https://bugs.freedesktop.org/show_bug.cgi?id=102850
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:444s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:472s
fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:418s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:515s
fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:279s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:512s
fi-byt-j1900 total:289 pass:254 dwarn:1 dfail:0 fail:0 skip:34 time:498s
fi-byt-n2820 total:289 pass:250 dwarn:1 dfail:0 fail:0 skip:38 time:494s
fi-cfl-s total:289 pass:223 dwarn:34 dfail:0 fail:0 skip:32 time:545s
fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:418s
fi-glk-1 total:289 pass:259 dwarn:1 dfail:0 fail:0 skip:29 time:565s
fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:425s
fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:407s
fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:427s
fi-ivb-3520m total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:477s
fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:464s
fi-kbl-7500u total:245 pass:223 dwarn:1 dfail:0 fail:0 skip:20
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:585s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:588s
fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:548s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:453s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:753s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:493s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:478s
fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:567s
fi-snb-2600 total:289 pass:248 dwarn:0 dfail:0 fail:2 skip:39 time:424s
a7b0939454cd452880780131dd4c121d89325ed7 drm-tip: 2017y-09m-19d-09h-18m-06s UTC integration manifest
f668a812f549 drm/i915/dp: Remove useless debug about TPS3 support
b435e32a4afd drm/i915/dp: Clean up intel_dp_check_mst_status
5c9b671f1321 drm/i915/dp: Fix buffer size for sink_irq_esi read
4c2d2d7bc3b6 drm/i915/mst: Print active mst links after update
8bba69fd0f37 drm/i915/mst: Debug log connector name in destroy_connector()
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5743/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* ✗ Fi.CI.IGT: warning for series starting with [1/5] drm/i915/mst: Debug log connector name in destroy_connector()
2017-09-18 22:21 [PATCH 1/5] drm/i915/mst: Debug log connector name in destroy_connector() Dhinakaran Pandiyan
` (4 preceding siblings ...)
2017-09-19 12:17 ` ✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915/mst: Debug log connector name in destroy_connector() Patchwork
@ 2017-09-19 16:18 ` Patchwork
2017-09-21 19:18 ` ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/mst: Debug log connector name in destroy_connector() (rev2) Patchwork
6 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2017-09-19 16:18 UTC (permalink / raw)
To: Pandiyan, Dhinakaran; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/5] drm/i915/mst: Debug log connector name in destroy_connector()
URL : https://patchwork.freedesktop.org/series/30556/
State : warning
== Summary ==
Test kms_plane:
Subgroup plane-panning-bottom-right-suspend-pipe-A-planes:
pass -> SKIP (shard-hsw)
Test gem_cpu_reloc:
Subgroup full:
incomplete -> PASS (shard-hsw)
Test kms_flip:
Subgroup flip-vs-modeset-interruptible:
pass -> DMESG-WARN (shard-hsw) fdo#102557
Test perf:
Subgroup blocking:
pass -> FAIL (shard-hsw) fdo#102252 +1
Test kms_setmode:
Subgroup basic:
pass -> FAIL (shard-hsw) fdo#99912
fdo#102557 https://bugs.freedesktop.org/show_bug.cgi?id=102557
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
shard-hsw total:2314 pass:1242 dwarn:1 dfail:0 fail:14 skip:1057 time:9597s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5743/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] drm/i915/dp: Clean up intel_dp_check_mst_status
2017-09-18 22:21 ` [PATCH 4/5] drm/i915/dp: Clean up intel_dp_check_mst_status Dhinakaran Pandiyan
@ 2017-09-20 19:11 ` Ausmus, James
2017-09-20 19:55 ` Pandiyan, Dhinakaran
0 siblings, 1 reply; 17+ messages in thread
From: Ausmus, James @ 2017-09-20 19:11 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: Jani Nikula, Intel GFX
On Mon, Sep 18, 2017 at 3:21 PM, Dhinakaran Pandiyan
<dhinakaran.pandiyan@intel.com> wrote:
> Rewriting this code without the goto, I believe, makes it more readable.
> One functional change that has been included is the handling of failed ESI
> register reads. Instead of disabling MST only for the first failed read, we
> now disable MST on subsequent failed reads too. A failed ESI read is
> problematic irrespective of whether it is the first or not.
>
> Cc: James Ausmus <james.ausmus@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 75 +++++++++++++++++------------------------
> 1 file changed, 31 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 98e7b96ca826..cc129aa444ac 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4191,57 +4191,44 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
> static int
> intel_dp_check_mst_status(struct intel_dp *intel_dp)
> {
> - bool bret;
> + u8 esi[DP_DPRX_ESI_LEN] = { 0 };
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>
> - if (intel_dp->is_mst) {
> - u8 esi[DP_DPRX_ESI_LEN] = { 0 };
> - int ret = 0;
> - int retry;
> + if (!intel_dp->is_mst)
> + return -EINVAL;
> +
> + while (intel_dp_get_sink_irq_esi(intel_dp, esi)) {
It looks like if the underlying drm_dp_dpcd_read fails and returns
-EIO, for instance, you'll get true back from
intel_dp_get_sink_irq_esi, and you'll still go in to the while, but
with a potentially invalid esi. Granted, this is a problem in the
original code as well, but it seems like something that should be
fixed during the refactoring.
> + int ret, retry;
> bool handled;
> - bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
> -go_again:
> - if (bret == true) {
> -
> - /* check link status - esi[10] = 0x200c */
> - if (intel_dp->active_mst_links &&
> - !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
> - DRM_DEBUG_KMS("channel EQ not ok, retraining\n");
> - intel_dp_start_link_train(intel_dp);
> - intel_dp_stop_link_train(intel_dp);
> - }
>
> - DRM_DEBUG_KMS("got esi %3ph\n", esi);
> - ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled);
> -
> - if (handled) {
> - for (retry = 0; retry < 3; retry++) {
> - int wret;
> - wret = drm_dp_dpcd_write(&intel_dp->aux,
> - DP_SINK_COUNT_ESI+1,
> - &esi[1], 3);
> - if (wret == 3) {
> - break;
> - }
> - }
> + DRM_DEBUG_KMS("ESI %3ph\n", esi);
>
> - bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
> - if (bret == true) {
> - DRM_DEBUG_KMS("got esi2 %3ph\n", esi);
> - goto go_again;
> - }
> - } else
> - ret = 0;
> + /* check link status - esi[10] = 0x200c */
> + if (intel_dp->active_mst_links &&
> + !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
> + intel_dp_start_link_train(intel_dp);
> + intel_dp_stop_link_train(intel_dp);
> + }
>
> - return ret;
> - } else {
> - struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> - DRM_DEBUG_KMS("failed to get ESI - device may have failed\n");
> - intel_dp->is_mst = false;
> - drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
> - /* send a hotplug event */
> - drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev);
> + ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled);
You're no longer using the value returned by drm_dp_mst_hpd_irq
> + if (!handled)
> + return 0;
> +
> + for (retry = 0; retry < 3; retry++) {
> + int wret;
> +
> + wret = drm_dp_dpcd_write(&intel_dp->aux,
> + DP_SINK_COUNT_ESI + 1, &esi[1],
> + 3);
> + if (wret == 3)
> + break;
> }
> }
> +
> + DRM_DEBUG_KMS("failed to get ESI - device may have failed\n");
> + intel_dp->is_mst = false;
> + drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
> + drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev);
> return -EINVAL;
> }
>
> --
> 2.11.0
>
--
James Ausmus
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] drm/i915/dp: Clean up intel_dp_check_mst_status
2017-09-20 19:11 ` Ausmus, James
@ 2017-09-20 19:55 ` Pandiyan, Dhinakaran
2017-09-20 20:02 ` Ausmus, James
0 siblings, 1 reply; 17+ messages in thread
From: Pandiyan, Dhinakaran @ 2017-09-20 19:55 UTC (permalink / raw)
To: Ausmus, James; +Cc: Nikula, Jani, intel-gfx
On Wed, 2017-09-20 at 12:11 -0700, Ausmus, James wrote:
> On Mon, Sep 18, 2017 at 3:21 PM, Dhinakaran Pandiyan
> <dhinakaran.pandiyan@intel.com> wrote:
> > Rewriting this code without the goto, I believe, makes it more readable.
> > One functional change that has been included is the handling of failed ESI
> > register reads. Instead of disabling MST only for the first failed read, we
> > now disable MST on subsequent failed reads too. A failed ESI read is
> > problematic irrespective of whether it is the first or not.
> >
> > Cc: James Ausmus <james.ausmus@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 75 +++++++++++++++++------------------------
> > 1 file changed, 31 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 98e7b96ca826..cc129aa444ac 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4191,57 +4191,44 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
> > static int
> > intel_dp_check_mst_status(struct intel_dp *intel_dp)
> > {
> > - bool bret;
> > + u8 esi[DP_DPRX_ESI_LEN] = { 0 };
> > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >
> > - if (intel_dp->is_mst) {
> > - u8 esi[DP_DPRX_ESI_LEN] = { 0 };
> > - int ret = 0;
> > - int retry;
> > + if (!intel_dp->is_mst)
> > + return -EINVAL;
> > +
> > + while (intel_dp_get_sink_irq_esi(intel_dp, esi)) {
>
> It looks like if the underlying drm_dp_dpcd_read fails and returns
> -EIO, for instance, you'll get true back from
> intel_dp_get_sink_irq_esi,
Wait, anything other than 14 from that dpcd read is a false, isn't it?
> and you'll still go in to the while, but
> with a potentially invalid esi. Granted, this is a problem in the
> original code as well, but it seems like something that should be
> fixed during the refactoring.
>
>
> > + int ret, retry;
> > bool handled;
> > - bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
> > -go_again:
> > - if (bret == true) {
> > -
> > - /* check link status - esi[10] = 0x200c */
> > - if (intel_dp->active_mst_links &&
> > - !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
> > - DRM_DEBUG_KMS("channel EQ not ok, retraining\n");
> > - intel_dp_start_link_train(intel_dp);
> > - intel_dp_stop_link_train(intel_dp);
> > - }
> >
> > - DRM_DEBUG_KMS("got esi %3ph\n", esi);
> > - ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled);
> > -
> > - if (handled) {
> > - for (retry = 0; retry < 3; retry++) {
> > - int wret;
> > - wret = drm_dp_dpcd_write(&intel_dp->aux,
> > - DP_SINK_COUNT_ESI+1,
> > - &esi[1], 3);
> > - if (wret == 3) {
> > - break;
> > - }
> > - }
> > + DRM_DEBUG_KMS("ESI %3ph\n", esi);
> >
> > - bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
> > - if (bret == true) {
> > - DRM_DEBUG_KMS("got esi2 %3ph\n", esi);
> > - goto go_again;
> > - }
> > - } else
> > - ret = 0;
> > + /* check link status - esi[10] = 0x200c */
> > + if (intel_dp->active_mst_links &&
> > + !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
> > + intel_dp_start_link_train(intel_dp);
> > + intel_dp_stop_link_train(intel_dp);
> > + }
> >
> > - return ret;
> > - } else {
> > - struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > - DRM_DEBUG_KMS("failed to get ESI - device may have failed\n");
> > - intel_dp->is_mst = false;
> > - drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
> > - /* send a hotplug event */
> > - drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev);
> > + ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled);
>
> You're no longer using the value returned by drm_dp_mst_hpd_irq
The way the code was originally written, the return from
drm_dp_mst_hpd_irq() was
a) changed to 0 when handled == false
b) discarded and a new return value was obtained if handled == true and
intel_dp_get_sink_irq_esi() is true the second time.
So the only case when the return value was returned back to the caller
is when handled == true and the second intel_dp_get_sink_irq_esi()
returned false.
But this does not make sense. If the second intel_dp_get_sink_irq_esi()
is false, then we should still have to disable MST. This is the
functional change I noted in the commit message.
>
> > + if (!handled)
> > + return 0;
> > +
> > + for (retry = 0; retry < 3; retry++) {
> > + int wret;
> > +
> > + wret = drm_dp_dpcd_write(&intel_dp->aux,
> > + DP_SINK_COUNT_ESI + 1, &esi[1],
> > + 3);
> > + if (wret == 3)
> > + break;
> > }
> > }
> > +
> > + DRM_DEBUG_KMS("failed to get ESI - device may have failed\n");
> > + intel_dp->is_mst = false;
> > + drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
> > + drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev);
> > return -EINVAL;
> > }
> >
> > --
> > 2.11.0
> >
>
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] drm/i915/dp: Clean up intel_dp_check_mst_status
2017-09-20 19:55 ` Pandiyan, Dhinakaran
@ 2017-09-20 20:02 ` Ausmus, James
2017-09-20 22:47 ` Pandiyan, Dhinakaran
0 siblings, 1 reply; 17+ messages in thread
From: Ausmus, James @ 2017-09-20 20:02 UTC (permalink / raw)
To: Pandiyan, Dhinakaran; +Cc: Nikula, Jani, intel-gfx
On Wed, Sep 20, 2017 at 12:55 PM, Pandiyan, Dhinakaran
<dhinakaran.pandiyan@intel.com> wrote:
> On Wed, 2017-09-20 at 12:11 -0700, Ausmus, James wrote:
>> On Mon, Sep 18, 2017 at 3:21 PM, Dhinakaran Pandiyan
>> <dhinakaran.pandiyan@intel.com> wrote:
>> > Rewriting this code without the goto, I believe, makes it more readable.
>> > One functional change that has been included is the handling of failed ESI
>> > register reads. Instead of disabling MST only for the first failed read, we
>> > now disable MST on subsequent failed reads too. A failed ESI read is
>> > problematic irrespective of whether it is the first or not.
>> >
>> > Cc: James Ausmus <james.ausmus@intel.com>
>> > Cc: Jani Nikula <jani.nikula@intel.com>
>> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> > ---
>> > drivers/gpu/drm/i915/intel_dp.c | 75 +++++++++++++++++------------------------
>> > 1 file changed, 31 insertions(+), 44 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> > index 98e7b96ca826..cc129aa444ac 100644
>> > --- a/drivers/gpu/drm/i915/intel_dp.c
>> > +++ b/drivers/gpu/drm/i915/intel_dp.c
>> > @@ -4191,57 +4191,44 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>> > static int
>> > intel_dp_check_mst_status(struct intel_dp *intel_dp)
>> > {
>> > - bool bret;
>> > + u8 esi[DP_DPRX_ESI_LEN] = { 0 };
>> > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> >
>> > - if (intel_dp->is_mst) {
>> > - u8 esi[DP_DPRX_ESI_LEN] = { 0 };
>> > - int ret = 0;
>> > - int retry;
>> > + if (!intel_dp->is_mst)
>> > + return -EINVAL;
>> > +
>> > + while (intel_dp_get_sink_irq_esi(intel_dp, esi)) {
>>
>> It looks like if the underlying drm_dp_dpcd_read fails and returns
>> -EIO, for instance, you'll get true back from
>> intel_dp_get_sink_irq_esi,
>
> Wait, anything other than 14 from that dpcd read is a false, isn't it?
D'oh! You're right - I completely glossed over the whole " ==
DP_DPRX_ESI_LEN" bit - sorry for the noise...
>
>> and you'll still go in to the while, but
>> with a potentially invalid esi. Granted, this is a problem in the
>> original code as well, but it seems like something that should be
>> fixed during the refactoring.
>>
>>
>> > + int ret, retry;
>> > bool handled;
>> > - bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
>> > -go_again:
>> > - if (bret == true) {
>> > -
>> > - /* check link status - esi[10] = 0x200c */
>> > - if (intel_dp->active_mst_links &&
>> > - !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
>> > - DRM_DEBUG_KMS("channel EQ not ok, retraining\n");
>> > - intel_dp_start_link_train(intel_dp);
>> > - intel_dp_stop_link_train(intel_dp);
>> > - }
>> >
>> > - DRM_DEBUG_KMS("got esi %3ph\n", esi);
>> > - ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled);
>> > -
>> > - if (handled) {
>> > - for (retry = 0; retry < 3; retry++) {
>> > - int wret;
>> > - wret = drm_dp_dpcd_write(&intel_dp->aux,
>> > - DP_SINK_COUNT_ESI+1,
>> > - &esi[1], 3);
>> > - if (wret == 3) {
>> > - break;
>> > - }
>> > - }
>> > + DRM_DEBUG_KMS("ESI %3ph\n", esi);
>> >
>> > - bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
>> > - if (bret == true) {
>> > - DRM_DEBUG_KMS("got esi2 %3ph\n", esi);
>> > - goto go_again;
>> > - }
>> > - } else
>> > - ret = 0;
>> > + /* check link status - esi[10] = 0x200c */
>> > + if (intel_dp->active_mst_links &&
>> > + !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
>> > + intel_dp_start_link_train(intel_dp);
>> > + intel_dp_stop_link_train(intel_dp);
>> > + }
>> >
>> > - return ret;
>> > - } else {
>> > - struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> > - DRM_DEBUG_KMS("failed to get ESI - device may have failed\n");
>> > - intel_dp->is_mst = false;
>> > - drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
>> > - /* send a hotplug event */
>> > - drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev);
>> > + ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled);
>>
>> You're no longer using the value returned by drm_dp_mst_hpd_irq
>
> The way the code was originally written, the return from
> drm_dp_mst_hpd_irq() was
> a) changed to 0 when handled == false
> b) discarded and a new return value was obtained if handled == true and
> intel_dp_get_sink_irq_esi() is true the second time.
>
>
> So the only case when the return value was returned back to the caller
> is when handled == true and the second intel_dp_get_sink_irq_esi()
> returned false.
>
> But this does not make sense. If the second intel_dp_get_sink_irq_esi()
> is false, then we should still have to disable MST. This is the
> functional change I noted in the commit message.
>
Certainly, but you aren't actually using ret for anything anymore, so
the variable can be dropped
>
>>
>> > + if (!handled)
>> > + return 0;
>> > +
>> > + for (retry = 0; retry < 3; retry++) {
>> > + int wret;
>> > +
>> > + wret = drm_dp_dpcd_write(&intel_dp->aux,
>> > + DP_SINK_COUNT_ESI + 1, &esi[1],
>> > + 3);
>> > + if (wret == 3)
>> > + break;
>> > }
>> > }
>> > +
>> > + DRM_DEBUG_KMS("failed to get ESI - device may have failed\n");
>> > + intel_dp->is_mst = false;
>> > + drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
>> > + drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev);
>> > return -EINVAL;
>> > }
>> >
>> > --
>> > 2.11.0
>> >
>>
>>
>>
--
James Ausmus
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] drm/i915/dp: Clean up intel_dp_check_mst_status
2017-09-20 20:02 ` Ausmus, James
@ 2017-09-20 22:47 ` Pandiyan, Dhinakaran
2017-09-20 22:53 ` Ausmus, James
0 siblings, 1 reply; 17+ messages in thread
From: Pandiyan, Dhinakaran @ 2017-09-20 22:47 UTC (permalink / raw)
To: Ausmus, James; +Cc: Nikula, Jani, intel-gfx
On Wed, 2017-09-20 at 13:02 -0700, Ausmus, James wrote:
> On Wed, Sep 20, 2017 at 12:55 PM, Pandiyan, Dhinakaran
> <dhinakaran.pandiyan@intel.com> wrote:
> > On Wed, 2017-09-20 at 12:11 -0700, Ausmus, James wrote:
> >> On Mon, Sep 18, 2017 at 3:21 PM, Dhinakaran Pandiyan
> >> <dhinakaran.pandiyan@intel.com> wrote:
> >> > Rewriting this code without the goto, I believe, makes it more readable.
> >> > One functional change that has been included is the handling of failed ESI
> >> > register reads. Instead of disabling MST only for the first failed read, we
> >> > now disable MST on subsequent failed reads too. A failed ESI read is
> >> > problematic irrespective of whether it is the first or not.
> >> >
> >> > Cc: James Ausmus <james.ausmus@intel.com>
> >> > Cc: Jani Nikula <jani.nikula@intel.com>
> >> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> >> > ---
> >> > drivers/gpu/drm/i915/intel_dp.c | 75 +++++++++++++++++------------------------
> >> > 1 file changed, 31 insertions(+), 44 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >> > index 98e7b96ca826..cc129aa444ac 100644
> >> > --- a/drivers/gpu/drm/i915/intel_dp.c
> >> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> > @@ -4191,57 +4191,44 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
> >> > static int
> >> > intel_dp_check_mst_status(struct intel_dp *intel_dp)
> >> > {
> >> > - bool bret;
> >> > + u8 esi[DP_DPRX_ESI_LEN] = { 0 };
> >> > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >> >
> >> > - if (intel_dp->is_mst) {
> >> > - u8 esi[DP_DPRX_ESI_LEN] = { 0 };
> >> > - int ret = 0;
> >> > - int retry;
> >> > + if (!intel_dp->is_mst)
> >> > + return -EINVAL;
> >> > +
> >> > + while (intel_dp_get_sink_irq_esi(intel_dp, esi)) {
> >>
> >> It looks like if the underlying drm_dp_dpcd_read fails and returns
> >> -EIO, for instance, you'll get true back from
> >> intel_dp_get_sink_irq_esi,
> >
> > Wait, anything other than 14 from that dpcd read is a false, isn't it?
>
> D'oh! You're right - I completely glossed over the whole " ==
> DP_DPRX_ESI_LEN" bit - sorry for the noise...
>
> >
> >> and you'll still go in to the while, but
> >> with a potentially invalid esi. Granted, this is a problem in the
> >> original code as well, but it seems like something that should be
> >> fixed during the refactoring.
> >>
> >>
> >> > + int ret, retry;
> >> > bool handled;
> >> > - bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
> >> > -go_again:
> >> > - if (bret == true) {
> >> > -
> >> > - /* check link status - esi[10] = 0x200c */
> >> > - if (intel_dp->active_mst_links &&
> >> > - !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
> >> > - DRM_DEBUG_KMS("channel EQ not ok, retraining\n");
> >> > - intel_dp_start_link_train(intel_dp);
> >> > - intel_dp_stop_link_train(intel_dp);
> >> > - }
> >> >
> >> > - DRM_DEBUG_KMS("got esi %3ph\n", esi);
> >> > - ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled);
> >> > -
> >> > - if (handled) {
> >> > - for (retry = 0; retry < 3; retry++) {
> >> > - int wret;
> >> > - wret = drm_dp_dpcd_write(&intel_dp->aux,
> >> > - DP_SINK_COUNT_ESI+1,
> >> > - &esi[1], 3);
> >> > - if (wret == 3) {
> >> > - break;
> >> > - }
> >> > - }
> >> > + DRM_DEBUG_KMS("ESI %3ph\n", esi);
> >> >
> >> > - bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
> >> > - if (bret == true) {
> >> > - DRM_DEBUG_KMS("got esi2 %3ph\n", esi);
> >> > - goto go_again;
> >> > - }
> >> > - } else
> >> > - ret = 0;
> >> > + /* check link status - esi[10] = 0x200c */
> >> > + if (intel_dp->active_mst_links &&
> >> > + !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
> >> > + intel_dp_start_link_train(intel_dp);
> >> > + intel_dp_stop_link_train(intel_dp);
> >> > + }
> >> >
> >> > - return ret;
> >> > - } else {
> >> > - struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >> > - DRM_DEBUG_KMS("failed to get ESI - device may have failed\n");
> >> > - intel_dp->is_mst = false;
> >> > - drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
> >> > - /* send a hotplug event */
> >> > - drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev);
> >> > + ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled);
> >>
> >> You're no longer using the value returned by drm_dp_mst_hpd_irq
> >
> > The way the code was originally written, the return from
> > drm_dp_mst_hpd_irq() was
> > a) changed to 0 when handled == false
> > b) discarded and a new return value was obtained if handled == true and
> > intel_dp_get_sink_irq_esi() is true the second time.
> >
> >
> > So the only case when the return value was returned back to the caller
> > is when handled == true and the second intel_dp_get_sink_irq_esi()
> > returned false.
> >
> > But this does not make sense. If the second intel_dp_get_sink_irq_esi()
> > is false, then we should still have to disable MST. This is the
> > functional change I noted in the commit message.
> >
>
> Certainly, but you aren't actually using ret for anything anymore, so
> the variable can be dropped
>
>
> >
> >>
> >> > + if (!handled)
> >> > + return 0;
> >> > +
if (ret)
DRM_DEBUG_KMS("MST irq_hpd handling failed %d\n", ret);
I was thinking of adding something like this, but then the drm helper
functions _handle_down_rep() and _handle_up_req() only ever return 0. I
also don't want to get rid of 'ret' because that may make it seem like
the underlying helpers don't return anything. So, we should either
change the helpers to return something useful or modify their
signatures. Both options need a bit more thought.
For now, I guess we could add just the debug message. Let me know what
you think.
-DK
> >> > + for (retry = 0; retry < 3; retry++) {
> >> > + int wret;
> >> > +
> >> > + wret = drm_dp_dpcd_write(&intel_dp->aux,
> >> > + DP_SINK_COUNT_ESI + 1, &esi[1],
> >> > + 3);
> >> > + if (wret == 3)
> >> > + break;
> >> > }
> >> > }
> >> > +
> >> > + DRM_DEBUG_KMS("failed to get ESI - device may have failed\n");
> >> > + intel_dp->is_mst = false;
> >> > + drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
> >> > + drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev);
> >> > return -EINVAL;
> >> > }
> >> >
> >> > --
> >> > 2.11.0
> >> >
> >>
> >>
> >>
>
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] drm/i915/dp: Clean up intel_dp_check_mst_status
2017-09-20 22:47 ` Pandiyan, Dhinakaran
@ 2017-09-20 22:53 ` Ausmus, James
2017-09-21 18:54 ` [PATCH v2] " Dhinakaran Pandiyan
0 siblings, 1 reply; 17+ messages in thread
From: Ausmus, James @ 2017-09-20 22:53 UTC (permalink / raw)
To: Pandiyan, Dhinakaran; +Cc: Nikula, Jani, intel-gfx
On Wed, Sep 20, 2017 at 3:47 PM, Pandiyan, Dhinakaran
<dhinakaran.pandiyan@intel.com> wrote:
> On Wed, 2017-09-20 at 13:02 -0700, Ausmus, James wrote:
>> On Wed, Sep 20, 2017 at 12:55 PM, Pandiyan, Dhinakaran
>> <dhinakaran.pandiyan@intel.com> wrote:
>> > On Wed, 2017-09-20 at 12:11 -0700, Ausmus, James wrote:
>> >> On Mon, Sep 18, 2017 at 3:21 PM, Dhinakaran Pandiyan
>> >> <dhinakaran.pandiyan@intel.com> wrote:
>> >> > Rewriting this code without the goto, I believe, makes it more readable.
>> >> > One functional change that has been included is the handling of failed ESI
>> >> > register reads. Instead of disabling MST only for the first failed read, we
>> >> > now disable MST on subsequent failed reads too. A failed ESI read is
>> >> > problematic irrespective of whether it is the first or not.
>> >> >
>> >> > Cc: James Ausmus <james.ausmus@intel.com>
>> >> > Cc: Jani Nikula <jani.nikula@intel.com>
>> >> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> >> > ---
>> >> > drivers/gpu/drm/i915/intel_dp.c | 75 +++++++++++++++++------------------------
>> >> > 1 file changed, 31 insertions(+), 44 deletions(-)
>> >> >
>> >> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> >> > index 98e7b96ca826..cc129aa444ac 100644
>> >> > --- a/drivers/gpu/drm/i915/intel_dp.c
>> >> > +++ b/drivers/gpu/drm/i915/intel_dp.c
>> >> > @@ -4191,57 +4191,44 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>> >> > static int
>> >> > intel_dp_check_mst_status(struct intel_dp *intel_dp)
>> >> > {
>> >> > - bool bret;
>> >> > + u8 esi[DP_DPRX_ESI_LEN] = { 0 };
>> >> > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> >> >
>> >> > - if (intel_dp->is_mst) {
>> >> > - u8 esi[DP_DPRX_ESI_LEN] = { 0 };
>> >> > - int ret = 0;
>> >> > - int retry;
>> >> > + if (!intel_dp->is_mst)
>> >> > + return -EINVAL;
>> >> > +
>> >> > + while (intel_dp_get_sink_irq_esi(intel_dp, esi)) {
>> >>
>> >> It looks like if the underlying drm_dp_dpcd_read fails and returns
>> >> -EIO, for instance, you'll get true back from
>> >> intel_dp_get_sink_irq_esi,
>> >
>> > Wait, anything other than 14 from that dpcd read is a false, isn't it?
>>
>> D'oh! You're right - I completely glossed over the whole " ==
>> DP_DPRX_ESI_LEN" bit - sorry for the noise...
>>
>> >
>> >> and you'll still go in to the while, but
>> >> with a potentially invalid esi. Granted, this is a problem in the
>> >> original code as well, but it seems like something that should be
>> >> fixed during the refactoring.
>> >>
>> >>
>> >> > + int ret, retry;
>> >> > bool handled;
>> >> > - bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
>> >> > -go_again:
>> >> > - if (bret == true) {
>> >> > -
>> >> > - /* check link status - esi[10] = 0x200c */
>> >> > - if (intel_dp->active_mst_links &&
>> >> > - !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
>> >> > - DRM_DEBUG_KMS("channel EQ not ok, retraining\n");
>> >> > - intel_dp_start_link_train(intel_dp);
>> >> > - intel_dp_stop_link_train(intel_dp);
>> >> > - }
>> >> >
>> >> > - DRM_DEBUG_KMS("got esi %3ph\n", esi);
>> >> > - ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled);
>> >> > -
>> >> > - if (handled) {
>> >> > - for (retry = 0; retry < 3; retry++) {
>> >> > - int wret;
>> >> > - wret = drm_dp_dpcd_write(&intel_dp->aux,
>> >> > - DP_SINK_COUNT_ESI+1,
>> >> > - &esi[1], 3);
>> >> > - if (wret == 3) {
>> >> > - break;
>> >> > - }
>> >> > - }
>> >> > + DRM_DEBUG_KMS("ESI %3ph\n", esi);
>> >> >
>> >> > - bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
>> >> > - if (bret == true) {
>> >> > - DRM_DEBUG_KMS("got esi2 %3ph\n", esi);
>> >> > - goto go_again;
>> >> > - }
>> >> > - } else
>> >> > - ret = 0;
>> >> > + /* check link status - esi[10] = 0x200c */
>> >> > + if (intel_dp->active_mst_links &&
>> >> > + !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
>> >> > + intel_dp_start_link_train(intel_dp);
>> >> > + intel_dp_stop_link_train(intel_dp);
>> >> > + }
>> >> >
>> >> > - return ret;
>> >> > - } else {
>> >> > - struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> >> > - DRM_DEBUG_KMS("failed to get ESI - device may have failed\n");
>> >> > - intel_dp->is_mst = false;
>> >> > - drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
>> >> > - /* send a hotplug event */
>> >> > - drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev);
>> >> > + ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled);
>> >>
>> >> You're no longer using the value returned by drm_dp_mst_hpd_irq
>> >
>> > The way the code was originally written, the return from
>> > drm_dp_mst_hpd_irq() was
>> > a) changed to 0 when handled == false
>> > b) discarded and a new return value was obtained if handled == true and
>> > intel_dp_get_sink_irq_esi() is true the second time.
>> >
>> >
>> > So the only case when the return value was returned back to the caller
>> > is when handled == true and the second intel_dp_get_sink_irq_esi()
>> > returned false.
>> >
>> > But this does not make sense. If the second intel_dp_get_sink_irq_esi()
>> > is false, then we should still have to disable MST. This is the
>> > functional change I noted in the commit message.
>> >
>>
>> Certainly, but you aren't actually using ret for anything anymore, so
>> the variable can be dropped
>>
>>
>> >
>> >>
>> >> > + if (!handled)
>> >> > + return 0;
>> >> > +
> if (ret)
> DRM_DEBUG_KMS("MST irq_hpd handling failed %d\n", ret);
>
>
> I was thinking of adding something like this, but then the drm helper
> functions _handle_down_rep() and _handle_up_req() only ever return 0. I
> also don't want to get rid of 'ret' because that may make it seem like
> the underlying helpers don't return anything. So, we should either
> change the helpers to return something useful or modify their
> signatures. Both options need a bit more thought.
>
> For now, I guess we could add just the debug message. Let me know what
> you think.
A debug message is better than just ignoring the value - I think
that's a good option until the underlying helpers start returning
anything useful...
>
> -DK
>
>> >> > + for (retry = 0; retry < 3; retry++) {
>> >> > + int wret;
>> >> > +
>> >> > + wret = drm_dp_dpcd_write(&intel_dp->aux,
>> >> > + DP_SINK_COUNT_ESI + 1, &esi[1],
>> >> > + 3);
>> >> > + if (wret == 3)
>> >> > + break;
>> >> > }
>> >> > }
>> >> > +
>> >> > + DRM_DEBUG_KMS("failed to get ESI - device may have failed\n");
>> >> > + intel_dp->is_mst = false;
>> >> > + drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
>> >> > + drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev);
>> >> > return -EINVAL;
>> >> > }
>> >> >
>> >> > --
>> >> > 2.11.0
>> >> >
>> >>
>> >>
>> >>
>>
>>
>>
--
James Ausmus
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2] drm/i915/dp: Clean up intel_dp_check_mst_status
2017-09-20 22:53 ` Ausmus, James
@ 2017-09-21 18:54 ` Dhinakaran Pandiyan
2017-09-22 17:53 ` Ausmus, James
2017-09-25 12:39 ` Jani Nikula
0 siblings, 2 replies; 17+ messages in thread
From: Dhinakaran Pandiyan @ 2017-09-21 18:54 UTC (permalink / raw)
To: intel-gfx; +Cc: Jani Nikula, Dhinakaran Pandiyan
Rewriting this code without the goto, I believe, makes it more readable.
One functional change that has been included is the handling of failed ESI
register reads. Instead of disabling MST only for the first failed read, we
now disable MST on subsequent failed reads too. A failed ESI read is
problematic irrespective of whether it is the first or not.
v2: Don't ignore return from _mst_hpd_irq() (James)
Cc: James Ausmus <james.ausmus@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 78 ++++++++++++++++++-----------------------
1 file changed, 34 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 98e7b96ca826..aa97bd825369 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4191,57 +4191,47 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
static int
intel_dp_check_mst_status(struct intel_dp *intel_dp)
{
- bool bret;
+ u8 esi[DP_DPRX_ESI_LEN] = { 0 };
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
- if (intel_dp->is_mst) {
- u8 esi[DP_DPRX_ESI_LEN] = { 0 };
- int ret = 0;
- int retry;
+ if (!intel_dp->is_mst)
+ return -EINVAL;
+
+ while (intel_dp_get_sink_irq_esi(intel_dp, esi)) {
+ int ret, retry;
bool handled;
- bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
-go_again:
- if (bret == true) {
-
- /* check link status - esi[10] = 0x200c */
- if (intel_dp->active_mst_links &&
- !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
- DRM_DEBUG_KMS("channel EQ not ok, retraining\n");
- intel_dp_start_link_train(intel_dp);
- intel_dp_stop_link_train(intel_dp);
- }
- DRM_DEBUG_KMS("got esi %3ph\n", esi);
- ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled);
-
- if (handled) {
- for (retry = 0; retry < 3; retry++) {
- int wret;
- wret = drm_dp_dpcd_write(&intel_dp->aux,
- DP_SINK_COUNT_ESI+1,
- &esi[1], 3);
- if (wret == 3) {
- break;
- }
- }
+ DRM_DEBUG_KMS("ESI %3ph\n", esi);
- bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
- if (bret == true) {
- DRM_DEBUG_KMS("got esi2 %3ph\n", esi);
- goto go_again;
- }
- } else
- ret = 0;
+ /* check link status - esi[10] = 0x200c */
+ if (intel_dp->active_mst_links &&
+ !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
+ intel_dp_start_link_train(intel_dp);
+ intel_dp_stop_link_train(intel_dp);
+ }
- return ret;
- } else {
- struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
- DRM_DEBUG_KMS("failed to get ESI - device may have failed\n");
- intel_dp->is_mst = false;
- drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
- /* send a hotplug event */
- drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev);
+ ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled);
+ if (!handled)
+ return 0;
+
+ if (ret)
+ DRM_DEBUG_KMS("error handling MST IRQ_HPD %d\n", ret);
+
+ for (retry = 0; retry < 3; retry++) {
+ int wret;
+
+ wret = drm_dp_dpcd_write(&intel_dp->aux,
+ DP_SINK_COUNT_ESI + 1, &esi[1],
+ 3);
+ if (wret == 3)
+ break;
}
}
+
+ DRM_DEBUG_KMS("failed to get ESI - device may have failed\n");
+ intel_dp->is_mst = false;
+ drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
+ drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev);
return -EINVAL;
}
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/mst: Debug log connector name in destroy_connector() (rev2)
2017-09-18 22:21 [PATCH 1/5] drm/i915/mst: Debug log connector name in destroy_connector() Dhinakaran Pandiyan
` (5 preceding siblings ...)
2017-09-19 16:18 ` ✗ Fi.CI.IGT: warning " Patchwork
@ 2017-09-21 19:18 ` Patchwork
6 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2017-09-21 19:18 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/5] drm/i915/mst: Debug log connector name in destroy_connector() (rev2)
URL : https://patchwork.freedesktop.org/series/30556/
State : failure
== Summary ==
Series 30556v2 series starting with [1/5] drm/i915/mst: Debug log connector name in destroy_connector()
https://patchwork.freedesktop.org/api/1.0/series/30556/revisions/2/mbox/
Test chamelium:
Subgroup dp-crc-fast:
fail -> PASS (fi-kbl-7500u) fdo#102514
Test gem_exec_store:
Subgroup basic-all:
dmesg-warn -> PASS (fi-kbl-7500u) fdo#102849 +1
Test gem_exec_suspend:
Subgroup basic-s3:
incomplete -> PASS (fi-kbl-7500u) fdo#102850
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-atomic:
pass -> FAIL (fi-snb-2600) fdo#100215
Test kms_frontbuffer_tracking:
Subgroup basic:
pass -> DMESG-WARN (fi-bdw-5557u) fdo#102473
Test pm_rpm:
Subgroup basic-rte:
dmesg-warn -> PASS (fi-cfl-s) fdo#102294
Test drv_module_reload:
Subgroup basic-reload:
pass -> INCOMPLETE (fi-bsw-n3050)
dmesg-warn -> PASS (fi-glk-1) fdo#102777
fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fdo#102849 https://bugs.freedesktop.org/show_bug.cgi?id=102849
fdo#102850 https://bugs.freedesktop.org/show_bug.cgi?id=102850
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#102473 https://bugs.freedesktop.org/show_bug.cgi?id=102473
fdo#102294 https://bugs.freedesktop.org/show_bug.cgi?id=102294
fdo#102777 https://bugs.freedesktop.org/show_bug.cgi?id=102777
fi-bdw-5557u total:289 pass:267 dwarn:1 dfail:0 fail:0 skip:21 time:439s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:473s
fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:418s
fi-bsw-n3050 total:286 pass:240 dwarn:0 dfail:0 fail:0 skip:45
fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:278s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:504s
fi-byt-j1900 total:289 pass:254 dwarn:1 dfail:0 fail:0 skip:34 time:492s
fi-byt-n2820 total:289 pass:250 dwarn:1 dfail:0 fail:0 skip:38 time:495s
fi-cfl-s total:289 pass:223 dwarn:34 dfail:0 fail:0 skip:32 time:547s
fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:420s
fi-glk-1 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:566s
fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:424s
fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:408s
fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:430s
fi-ivb-3520m total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:480s
fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:462s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:471s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:579s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:585s
fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:541s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:451s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:745s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:487s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:475s
fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:564s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:1 skip:39 time:417s
35f6d27736f70caa580b7a6030528afa765bd76a drm-tip: 2017y-09m-21d-16h-20m-08s UTC integration manifest
8a5e56ae75b5 drm/i915/dp: Remove useless debug about TPS3 support
bc901982aecf drm/i915/dp: Clean up intel_dp_check_mst_status
6f309a9c64b4 drm/i915/dp: Fix buffer size for sink_irq_esi read
395f35441b58 drm/i915/mst: Print active mst links after update
d16014a27d54 drm/i915/mst: Debug log connector name in destroy_connector()
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5785/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] drm/i915/dp: Clean up intel_dp_check_mst_status
2017-09-21 18:54 ` [PATCH v2] " Dhinakaran Pandiyan
@ 2017-09-22 17:53 ` Ausmus, James
2017-09-25 12:39 ` Jani Nikula
1 sibling, 0 replies; 17+ messages in thread
From: Ausmus, James @ 2017-09-22 17:53 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: Jani Nikula, Intel GFX
On Thu, Sep 21, 2017 at 11:54 AM, Dhinakaran Pandiyan
<dhinakaran.pandiyan@intel.com> wrote:
> Rewriting this code without the goto, I believe, makes it more readable.
> One functional change that has been included is the handling of failed ESI
> register reads. Instead of disabling MST only for the first failed read, we
> now disable MST on subsequent failed reads too. A failed ESI read is
> problematic irrespective of whether it is the first or not.
>
> v2: Don't ignore return from _mst_hpd_irq() (James)
>
> Cc: James Ausmus <james.ausmus@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: James Ausmus <james.ausmus@intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 78 ++++++++++++++++++-----------------------
> 1 file changed, 34 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 98e7b96ca826..aa97bd825369 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4191,57 +4191,47 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
> static int
> intel_dp_check_mst_status(struct intel_dp *intel_dp)
> {
> - bool bret;
> + u8 esi[DP_DPRX_ESI_LEN] = { 0 };
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>
> - if (intel_dp->is_mst) {
> - u8 esi[DP_DPRX_ESI_LEN] = { 0 };
> - int ret = 0;
> - int retry;
> + if (!intel_dp->is_mst)
> + return -EINVAL;
> +
> + while (intel_dp_get_sink_irq_esi(intel_dp, esi)) {
> + int ret, retry;
> bool handled;
> - bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
> -go_again:
> - if (bret == true) {
> -
> - /* check link status - esi[10] = 0x200c */
> - if (intel_dp->active_mst_links &&
> - !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
> - DRM_DEBUG_KMS("channel EQ not ok, retraining\n");
> - intel_dp_start_link_train(intel_dp);
> - intel_dp_stop_link_train(intel_dp);
> - }
>
> - DRM_DEBUG_KMS("got esi %3ph\n", esi);
> - ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled);
> -
> - if (handled) {
> - for (retry = 0; retry < 3; retry++) {
> - int wret;
> - wret = drm_dp_dpcd_write(&intel_dp->aux,
> - DP_SINK_COUNT_ESI+1,
> - &esi[1], 3);
> - if (wret == 3) {
> - break;
> - }
> - }
> + DRM_DEBUG_KMS("ESI %3ph\n", esi);
>
> - bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
> - if (bret == true) {
> - DRM_DEBUG_KMS("got esi2 %3ph\n", esi);
> - goto go_again;
> - }
> - } else
> - ret = 0;
> + /* check link status - esi[10] = 0x200c */
> + if (intel_dp->active_mst_links &&
> + !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
> + intel_dp_start_link_train(intel_dp);
> + intel_dp_stop_link_train(intel_dp);
> + }
>
> - return ret;
> - } else {
> - struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> - DRM_DEBUG_KMS("failed to get ESI - device may have failed\n");
> - intel_dp->is_mst = false;
> - drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
> - /* send a hotplug event */
> - drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev);
> + ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled);
> + if (!handled)
> + return 0;
> +
> + if (ret)
> + DRM_DEBUG_KMS("error handling MST IRQ_HPD %d\n", ret);
> +
> + for (retry = 0; retry < 3; retry++) {
> + int wret;
> +
> + wret = drm_dp_dpcd_write(&intel_dp->aux,
> + DP_SINK_COUNT_ESI + 1, &esi[1],
> + 3);
> + if (wret == 3)
> + break;
> }
> }
> +
> + DRM_DEBUG_KMS("failed to get ESI - device may have failed\n");
> + intel_dp->is_mst = false;
> + drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
> + drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev);
> return -EINVAL;
> }
>
> --
> 2.11.0
>
--
James Ausmus
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] drm/i915/dp: Clean up intel_dp_check_mst_status
2017-09-21 18:54 ` [PATCH v2] " Dhinakaran Pandiyan
2017-09-22 17:53 ` Ausmus, James
@ 2017-09-25 12:39 ` Jani Nikula
1 sibling, 0 replies; 17+ messages in thread
From: Jani Nikula @ 2017-09-25 12:39 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan
On Thu, 21 Sep 2017, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> Rewriting this code without the goto, I believe, makes it more readable.
> One functional change that has been included is the handling of failed ESI
> register reads. Instead of disabling MST only for the first failed read, we
> now disable MST on subsequent failed reads too. A failed ESI read is
> problematic irrespective of whether it is the first or not.
>
> v2: Don't ignore return from _mst_hpd_irq() (James)
>
> Cc: James Ausmus <james.ausmus@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
I pushed everything *except* this patch in the series, please repost in
a new thread for clean CI results.
Thanks,
Jani.
> ---
> drivers/gpu/drm/i915/intel_dp.c | 78 ++++++++++++++++++-----------------------
> 1 file changed, 34 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 98e7b96ca826..aa97bd825369 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4191,57 +4191,47 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
> static int
> intel_dp_check_mst_status(struct intel_dp *intel_dp)
> {
> - bool bret;
> + u8 esi[DP_DPRX_ESI_LEN] = { 0 };
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>
> - if (intel_dp->is_mst) {
> - u8 esi[DP_DPRX_ESI_LEN] = { 0 };
> - int ret = 0;
> - int retry;
> + if (!intel_dp->is_mst)
> + return -EINVAL;
> +
> + while (intel_dp_get_sink_irq_esi(intel_dp, esi)) {
> + int ret, retry;
> bool handled;
> - bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
> -go_again:
> - if (bret == true) {
> -
> - /* check link status - esi[10] = 0x200c */
> - if (intel_dp->active_mst_links &&
> - !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
> - DRM_DEBUG_KMS("channel EQ not ok, retraining\n");
> - intel_dp_start_link_train(intel_dp);
> - intel_dp_stop_link_train(intel_dp);
> - }
>
> - DRM_DEBUG_KMS("got esi %3ph\n", esi);
> - ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled);
> -
> - if (handled) {
> - for (retry = 0; retry < 3; retry++) {
> - int wret;
> - wret = drm_dp_dpcd_write(&intel_dp->aux,
> - DP_SINK_COUNT_ESI+1,
> - &esi[1], 3);
> - if (wret == 3) {
> - break;
> - }
> - }
> + DRM_DEBUG_KMS("ESI %3ph\n", esi);
>
> - bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
> - if (bret == true) {
> - DRM_DEBUG_KMS("got esi2 %3ph\n", esi);
> - goto go_again;
> - }
> - } else
> - ret = 0;
> + /* check link status - esi[10] = 0x200c */
> + if (intel_dp->active_mst_links &&
> + !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
> + intel_dp_start_link_train(intel_dp);
> + intel_dp_stop_link_train(intel_dp);
> + }
>
> - return ret;
> - } else {
> - struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> - DRM_DEBUG_KMS("failed to get ESI - device may have failed\n");
> - intel_dp->is_mst = false;
> - drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
> - /* send a hotplug event */
> - drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev);
> + ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled);
> + if (!handled)
> + return 0;
> +
> + if (ret)
> + DRM_DEBUG_KMS("error handling MST IRQ_HPD %d\n", ret);
> +
> + for (retry = 0; retry < 3; retry++) {
> + int wret;
> +
> + wret = drm_dp_dpcd_write(&intel_dp->aux,
> + DP_SINK_COUNT_ESI + 1, &esi[1],
> + 3);
> + if (wret == 3)
> + break;
> }
> }
> +
> + DRM_DEBUG_KMS("failed to get ESI - device may have failed\n");
> + intel_dp->is_mst = false;
> + drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
> + drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev);
> return -EINVAL;
> }
--
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] 17+ messages in thread
end of thread, other threads:[~2017-09-25 12:40 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-18 22:21 [PATCH 1/5] drm/i915/mst: Debug log connector name in destroy_connector() Dhinakaran Pandiyan
2017-09-18 22:21 ` [PATCH 2/5] drm/i915/mst: Print active mst links after update Dhinakaran Pandiyan
2017-09-18 22:21 ` [PATCH 3/5] drm/i915/dp: Fix buffer size for sink_irq_esi read Dhinakaran Pandiyan
2017-09-18 22:21 ` [PATCH 4/5] drm/i915/dp: Clean up intel_dp_check_mst_status Dhinakaran Pandiyan
2017-09-20 19:11 ` Ausmus, James
2017-09-20 19:55 ` Pandiyan, Dhinakaran
2017-09-20 20:02 ` Ausmus, James
2017-09-20 22:47 ` Pandiyan, Dhinakaran
2017-09-20 22:53 ` Ausmus, James
2017-09-21 18:54 ` [PATCH v2] " Dhinakaran Pandiyan
2017-09-22 17:53 ` Ausmus, James
2017-09-25 12:39 ` Jani Nikula
2017-09-18 22:21 ` [PATCH 5/5] drm/i915/dp: Remove useless debug about TPS3 support Dhinakaran Pandiyan
2017-09-19 10:13 ` Jani Nikula
2017-09-19 12:17 ` ✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915/mst: Debug log connector name in destroy_connector() Patchwork
2017-09-19 16:18 ` ✗ Fi.CI.IGT: warning " Patchwork
2017-09-21 19:18 ` ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/mst: Debug log connector name in destroy_connector() (rev2) 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.