From patchwork Fri Jun 5 09:16:21 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Imre Deak X-Patchwork-Id: 11589231 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1127A739 for ; Fri, 5 Jun 2020 09:16:27 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id EC3E12075B for ; Fri, 5 Jun 2020 09:16:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org EC3E12075B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 824C16E8C8; Fri, 5 Jun 2020 09:16:26 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by gabe.freedesktop.org (Postfix) with ESMTPS id DBEBF6E8C8 for ; Fri, 5 Jun 2020 09:16:25 +0000 (UTC) IronPort-SDR: y5BeMXHatjC8jficWLxvd1/jDeBqLlDY0t2E62BEgb6zNoJA/5vHGStxyQuSCegf74wMAtQFXT F0VGhtqw3eSg== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Jun 2020 02:16:24 -0700 IronPort-SDR: kis+ottAtSCmmUNVJ7Oou1JH185LRoUTA2UfAlz4AZFf6hib/gpEMoDgunnELXqi8lzHRNV7oi eMEg/UtP9pVw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,475,1583222400"; d="scan'208";a="269705909" Received: from ideak-desk.fi.intel.com ([10.237.72.183]) by orsmga003.jf.intel.com with ESMTP; 05 Jun 2020 02:16:23 -0700 From: Imre Deak To: intel-gfx@lists.freedesktop.org Date: Fri, 5 Jun 2020 12:16:21 +0300 Message-Id: <20200605091621.17026-1-imre.deak@intel.com> X-Mailer: git-send-email 2.23.1 In-Reply-To: <20200604184500.23730-1-imre.deak@intel.com> References: <20200604184500.23730-1-imre.deak@intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH v3 1/3] drm/i915/dp_mst: Fix disabling MST on a port X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" Currently MST on a port can get enabled/disabled from the hotplug work and get disabled from the short pulse work in a racy way. Fix this by relying on the MST state checking in the hotplug work and just schedule a hotplug work from the short pulse handler if some problem happened during the MST interrupt handling. This removes the explicit MST disabling in case of an AUX failure, but if AUX fails, then probably the detection will also fail during the scheduled hotplug work and it's not guaranteed that we'll see intermittent errors anyway. While at it also simplify the error checking of the MST interrupt handler. v2: - Convert intel_dp_check_mst_status() to return bool. (Ville) - Change the intel_dp->is_mst check to an assert, since after this patch the condition can't change after we checked it previously. - Document the return value from intel_dp_check_mst_status(). v3: - Remove the intel_dp->is_mst check from intel_dp_check_mst_status(). There is no point in checking the same condition twice, even though there is a chance that the hotplug work running concurrently changes it. Cc: José Roberto de Souza Cc: Ville Syrjälä Reviewed-by: José Roberto de Souza (v1) Signed-off-by: Imre Deak --- drivers/gpu/drm/i915/display/intel_dp.c | 66 ++++++++++--------------- 1 file changed, 26 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 7ef60af8308b..ade21157f29b 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -5556,14 +5556,24 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp) "Could not write test response to sink\n"); } -static int +/** + * intel_dp_check_mst_status - service any pending MST interrupts, check link status + * @intel_dp: Intel DP struct + * + * Read any pending MST interrupts, call MST core to handle these and ack the + * interrupts. Check if the main and AUX link state is ok. + * + * Returns: + * - %true if pending interrupts were serviced (or no interrupts were + * pending) w/o detecting an error condition. + * - %false if an error condition - like AUX failure or a loss of link - is + * detected, which needs servicing from the hotplug work. + */ +static bool intel_dp_check_mst_status(struct intel_dp *intel_dp) { struct drm_i915_private *i915 = dp_to_i915(intel_dp); - bool need_retrain = false; - - if (!intel_dp->is_mst) - return -EINVAL; + bool link_ok = true; drm_WARN_ON_ONCE(&i915->drm, intel_dp->active_mst_links < 0); @@ -5591,22 +5601,23 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp) for (;;) { u8 esi[DP_DPRX_ESI_LEN] = {}; - bool bret, handled; + bool handled; int retry; - bret = intel_dp_get_sink_irq_esi(intel_dp, esi); - if (!bret) { + if (!intel_dp_get_sink_irq_esi(intel_dp, esi)) { drm_dbg_kms(&i915->drm, "failed to get ESI - device may have failed\n"); - return -EINVAL; + link_ok = false; + + break; } /* check link status - esi[10] = 0x200c */ - if (intel_dp->active_mst_links > 0 && !need_retrain && + if (intel_dp->active_mst_links > 0 && link_ok && !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) { drm_dbg_kms(&i915->drm, "channel EQ not ok, retraining\n"); - need_retrain = true; + link_ok = false; } drm_dbg_kms(&i915->drm, "got esi %3ph\n", esi); @@ -5626,7 +5637,7 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp) } } - return need_retrain; + return link_ok; } static bool @@ -7277,35 +7288,10 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) } if (intel_dp->is_mst) { - switch (intel_dp_check_mst_status(intel_dp)) { - case -EINVAL: - /* - * If we were in MST mode, and device is not - * there, get out of MST mode - */ - drm_dbg_kms(&i915->drm, - "MST device may have disappeared %d vs %d\n", - intel_dp->is_mst, - intel_dp->mst_mgr.mst_state); - intel_dp->is_mst = false; - drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, - intel_dp->is_mst); - - return IRQ_NONE; - case 1: - return IRQ_NONE; - default: - break; - } - } - - if (!intel_dp->is_mst) { - bool handled; - - handled = intel_dp_short_pulse(intel_dp); - - if (!handled) + if (!intel_dp_check_mst_status(intel_dp)) return IRQ_NONE; + } else if (!intel_dp_short_pulse(intel_dp)) { + return IRQ_NONE; } return IRQ_HANDLED;