* [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
* 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@lists.freedesktop.org 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@lists.freedesktop.org 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@lists.freedesktop.org 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@lists.freedesktop.org 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
* 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
* [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
* ✗ 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
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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox