From patchwork Wed Apr 11 22:54:45 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Lyude Paul X-Patchwork-Id: 10337563 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 1685D6053C for ; Wed, 11 Apr 2018 22:56:32 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0530C2843C for ; Wed, 11 Apr 2018 22:56:32 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id ED1602844C; Wed, 11 Apr 2018 22:56:31 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 747C12843C for ; Wed, 11 Apr 2018 22:56:31 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 06B3F89B46; Wed, 11 Apr 2018 22:55:41 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by gabe.freedesktop.org (Postfix) with ESMTPS id 56E438967A; Wed, 11 Apr 2018 22:55:35 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8B18E11E2F3; Wed, 11 Apr 2018 22:55:34 +0000 (UTC) Received: from malachite.bss.redhat.com (dhcp-10-20-1-55.bss.redhat.com [10.20.1.55]) by smtp.corp.redhat.com (Postfix) with ESMTP id 362832026985; Wed, 11 Apr 2018 22:55:34 +0000 (UTC) From: Lyude Paul To: intel-gfx@lists.freedesktop.org Date: Wed, 11 Apr 2018 18:54:45 -0400 Message-Id: <20180411225501.26751-9-lyude@redhat.com> In-Reply-To: <20180411225501.26751-1-lyude@redhat.com> References: <20180411225501.26751-1-lyude@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Wed, 11 Apr 2018 22:55:34 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Wed, 11 Apr 2018 22:55:34 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lyude@redhat.com' RCPT:'' Subject: [Intel-gfx] [PATCH v7 08/10] drm/i915: Make intel_dp_mst_atomic_check() idempotent X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: David Airlie , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Rodrigo Vivi Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP The current way of handling changing VCPI allocations doesn't make a whole ton of sense. Since drm_atomic_helper_check_modeset() can be called multiple times which means that intel_dp_mst_atomic_check() can also be called multiple times. However, since we make the silly mistake of trying to free VCPI slots based off the /new/ atomic state instead of the old atomic state, we'll end up potentially double freeing the vcpi slots for the ports. This also has another unintended consequence that came back up while implementing MST fallback retraining: if a modeset is forced on a connector that's already part of the state, but it's atomic_check() has already been run once and doesn't get run again, we'll end up not freeing the VCPI allocations on the connector at all. The easiest way to solve this is to be clever and, with the exception of connectors that are being disabled and thus will never have compute_config() ran on them, move vcpi freeing out of the atomic check and into compute_config(). Cc: Manasi Navare Cc: Ville Syrjälä Signed-off-by: Lyude Paul V7: - Fix CHECKPATCH errors Signed-off-by: Lyude Paul --- drivers/gpu/drm/i915/intel_dp_mst.c | 84 +++++++++++++++++++++++++++---------- 1 file changed, 63 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index 19de0b5a7a40..31b37722cd89 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -30,6 +30,38 @@ #include #include +static int +intel_dp_mst_atomic_release_vcpi_slots(struct drm_atomic_state *state, + struct drm_crtc_state *crtc_state, + struct drm_connector_state *conn_state) +{ + struct drm_crtc_state *new_crtc_state; + struct intel_crtc_state *intel_crtc_state = + to_intel_crtc_state(crtc_state); + struct drm_encoder *encoder; + struct drm_dp_mst_topology_mgr *mgr; + int slots, ret; + + slots = intel_crtc_state->dp_m_n.tu; + if (slots <= 0) + return 0; + + encoder = conn_state->best_encoder; + mgr = &enc_to_mst(encoder)->primary->dp.mst_mgr; + + ret = drm_dp_atomic_release_vcpi_slots(state, mgr, slots); + if (ret) { + DRM_DEBUG_KMS("failed releasing %d vcpi slots:%d\n", + slots, ret); + } else { + new_crtc_state = drm_atomic_get_crtc_state(state, + crtc_state->crtc); + to_intel_crtc_state(new_crtc_state)->dp_m_n.tu = 0; + } + + return ret; +} + static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, struct intel_crtc_state *pipe_config, struct drm_connector_state *conn_state) @@ -44,10 +76,14 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, struct drm_dp_mst_topology_state *mst_state = drm_atomic_dp_mst_get_topology_state(state, &intel_dp->mst_mgr); struct intel_dp_mst_topology_state *intel_mst_state; + struct drm_connector_state *old_conn_state = + drm_atomic_get_old_connector_state(state, &connector->base); + struct drm_crtc *old_crtc; int bpp; int slots; const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; int mst_pbn; + int ret; bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_LIMITED_M_N); @@ -80,6 +116,21 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp); pipe_config->pbn = mst_pbn; + /* Free any VCPI allocations on this connector from the previous + * state + */ + old_crtc = old_conn_state->crtc; + if (old_crtc) { + struct drm_crtc_state *old_crtc_state = + drm_atomic_get_old_crtc_state(state, old_crtc); + + ret = intel_dp_mst_atomic_release_vcpi_slots(state, + old_crtc_state, + old_conn_state); + if (ret) + return ret; + } + slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp->mst_mgr, connector->port, mst_pbn); if (slots < 0) { @@ -109,31 +160,22 @@ static int intel_dp_mst_atomic_check(struct drm_connector *connector, { struct drm_atomic_state *state = new_conn_state->state; struct drm_connector_state *old_conn_state; - struct drm_crtc *old_crtc; - struct drm_crtc_state *crtc_state; - int slots, ret = 0; + struct drm_crtc_state *new_crtc_state; + int ret = 0; old_conn_state = drm_atomic_get_old_connector_state(state, connector); - old_crtc = old_conn_state->crtc; - if (!old_crtc) - return ret; - crtc_state = drm_atomic_get_new_crtc_state(state, old_crtc); - slots = to_intel_crtc_state(crtc_state)->dp_m_n.tu; - if (drm_atomic_crtc_needs_modeset(crtc_state) && slots > 0) { - struct drm_dp_mst_topology_mgr *mgr; - struct drm_encoder *old_encoder; - - old_encoder = old_conn_state->best_encoder; - mgr = &enc_to_mst(old_encoder)->primary->dp.mst_mgr; + /* Only free VCPI here if we're not going to be detaching the + * connector's current CRTC, since no config will be computed + */ + if (new_conn_state->crtc || !old_conn_state->crtc) + return ret; - ret = drm_dp_atomic_release_vcpi_slots(state, mgr, slots); - if (ret) - DRM_DEBUG_KMS("failed releasing %d vcpi slots:%d\n", slots, ret); - else - to_intel_crtc_state(crtc_state)->dp_m_n.tu = 0; - } - return ret; + new_crtc_state = drm_atomic_get_new_crtc_state(state, + old_conn_state->crtc); + return intel_dp_mst_atomic_release_vcpi_slots(state, + new_crtc_state, + old_conn_state); } static void intel_mst_disable_dp(struct intel_encoder *encoder,