From patchwork Mon Nov 14 22:17:54 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lyude Paul X-Patchwork-Id: 20053 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp2393073wru; Mon, 14 Nov 2022 14:22:39 -0800 (PST) X-Google-Smtp-Source: AA0mqf7wMXw3oIvpX7D13XBcvlJs+xWq0nACXM9iWlYQ8QrCwYAYu+vAld2yvqmtL3YxQ+FflNh6 X-Received: by 2002:a63:ec44:0:b0:45f:a2ff:6597 with SMTP id r4-20020a63ec44000000b0045fa2ff6597mr13526058pgj.271.1668464558847; Mon, 14 Nov 2022 14:22:38 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1668464558; cv=none; d=google.com; s=arc-20160816; b=qQZhmEExxgFHnDA78VD4E447rvhdXNZZXgN1H2NiiPZcmT/5grB18LOqSdYH/G4GQq GPVqaVMoqZ8RtisVdttIf0cEx81uQJG5nuOT7gmd8IK0gAssYtqHaI2+6RmXf3M/Zr1q 9QCCMwnbaOk8aBW8FIWu/vKJ0DJ90BsjkBeQOFldHG/lWlUZUheCbHpB6/H4jBI5hmPZ eTyzcMLgqCD7lJtduRkSDWqFphR5tUdtDoCAwvok+cudAie4fZ0kU6WIxGEWmmY17CAm pDF7ytgUxM9fjlgS5xridjFdDGM7DBen4pXCCR8+QmTkt5sI8iyxEM87tApJuMFpJVKl YrGA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=BwdYUydnhRZsWPP/sXvy2mAjn3fiCkV9s76CcNVZuMU=; b=TBg/iHCkQd4dLMfqSEhPyCI6WwcmkMeV3wNeBv67kE716sAu6ZULD7N3eQvVSlSMLU y6qC//DGh8UZ13/LXBoeOKhalTHd0sffLOgQzsjs1Q8TlNdJFONIrbSQHSwJvzg8GVUl cgOh9LSlfHQBh9a+HZXeH/C1lksihvE6tndA8J+br/tZX/GiisQW2s0MFlupFU0NsKp9 vVY89vqJYDC10rfm/3uc7L8yWkzOC4G680eiCaZJLllq37y+t/3Ckm8qsHM2yt5VDwx7 +tED/FbJdj6xZiqa8foe/CA/HrH+MeIdxLsjO38WJp6Hw8DbJf2lcaursUHJe3s1Dj/I kZAg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=B2tjL9ca; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id a192-20020a6390c9000000b0046b3dce8455si10870521pge.14.2022.11.14.14.22.23; Mon, 14 Nov 2022 14:22:38 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=B2tjL9ca; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237054AbiKNWUP (ORCPT + 99 others); Mon, 14 Nov 2022 17:20:15 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54150 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236828AbiKNWTr (ORCPT ); Mon, 14 Nov 2022 17:19:47 -0500 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7912765BD for ; Mon, 14 Nov 2022 14:18:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668464327; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=BwdYUydnhRZsWPP/sXvy2mAjn3fiCkV9s76CcNVZuMU=; b=B2tjL9cabeSVReTRjOxb7NLM3pnfBQEfYLwHucKJWN59VDSLAjJUxF5mwbQ6cIDPzs0X5X JRh0/iM0Uq30r+3m/kTkzpjUHxIT/hvt+kl16PO0Xbsql5zr9uLB5hx0xv5mAqzZ6poZBA 7KLE0n0umCHLgRBbwFnF5BsLUuJOT3M= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-507-lNYgjQTHOvisQdRJTmanfQ-1; Mon, 14 Nov 2022 17:18:42 -0500 X-MC-Unique: lNYgjQTHOvisQdRJTmanfQ-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 73C6B85A5A6; Mon, 14 Nov 2022 22:18:41 +0000 (UTC) Received: from emerald.lyude.net (unknown [10.22.18.150]) by smtp.corp.redhat.com (Postfix) with ESMTP id B28DE49BB60; Mon, 14 Nov 2022 22:18:40 +0000 (UTC) From: Lyude Paul To: amd-gfx@lists.freedesktop.org Cc: stable@vger.kernel.org, Harry Wentland , Leo Li , Rodrigo Siqueira , Alex Deucher , =?utf-8?q?Christian_K=C3=B6nig?= , "Pan, Xinhui" , David Airlie , Daniel Vetter , hersen wu , Fangzhi Zuo , Wayne Lin , Thomas Zimmermann , Roman Li , Hamza Mahfooz , Wenjing Liu , David Francis , Mikita Lipski , dri-devel@lists.freedesktop.org (open list:DRM DRIVERS), linux-kernel@vger.kernel.org (open list) Subject: [PATCH v2 3/4] drm/amdgpu/dm/mst: Use the correct topology mgr pointer in amdgpu_dm_connector Date: Mon, 14 Nov 2022 17:17:54 -0500 Message-Id: <20221114221754.385090-4-lyude@redhat.com> In-Reply-To: <20221114221754.385090-1-lyude@redhat.com> References: <20221114221754.385090-1-lyude@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1749511893204293237?= X-GMAIL-MSGID: =?utf-8?q?1749511893204293237?= This bug hurt me. Basically, it appears that we've been grabbing the entirely wrong mutex in the MST DSC computation code for amdgpu! While we've been grabbing: amdgpu_dm_connector->mst_mgr That's zero-initialized memory, because the only connectors we'll ever actually be doing DSC computations for are MST ports. Which have mst_mgr zero-initialized, and instead have the correct topology mgr pointer located at: amdgpu_dm_connector->mst_port->mgr; I'm a bit impressed that until now, this code has managed not to crash anyone's systems! It does seem to cause a warning in LOCKDEP though: [ 66.637670] DEBUG_LOCKS_WARN_ON(lock->magic != lock) This was causing the problems that appeared to have been introduced by: commit 4d07b0bc4034 ("drm/display/dp_mst: Move all payload info into the atomic state") This wasn't actually where they came from though. Presumably, before the only thing we were doing with the topology mgr pointer was attempting to grab mst_mgr->lock. Since the above commit however, we grab much more information from mst_mgr including the atomic MST state and respective modesetting locks. This patch also implies that up until now, it's quite likely we could be susceptible to race conditions when going through the MST topology state for DSC computations since we technically will not have grabbed any lock when going through it. So, let's fix this by adjusting all the respective code paths to look at the right pointer and skip things that aren't actual MST connectors from a topology. Signed-off-by: Lyude Paul Gitlab issue: https://gitlab.freedesktop.org/drm/amd/-/issues/2171 Fixes: 8c20a1ed9b4f ("drm/amd/display: MST DSC compute fair share") Cc: # v5.6+ --- .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 37 +++++++++---------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c index bba2e8aaa2c20..5196c9a0e432d 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c @@ -1117,6 +1117,7 @@ int compute_mst_dsc_configs_for_state(struct drm_atomic_state *state, struct dc_stream_state *stream; bool computed_streams[MAX_PIPES]; struct amdgpu_dm_connector *aconnector; + struct drm_dp_mst_topology_mgr *mst_mgr; int link_vars_start_index = 0; int ret = 0; @@ -1131,7 +1132,7 @@ int compute_mst_dsc_configs_for_state(struct drm_atomic_state *state, aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context; - if (!aconnector || !aconnector->dc_sink) + if (!aconnector || !aconnector->dc_sink || !aconnector->port) continue; if (!aconnector->dc_sink->dsc_caps.dsc_dec_caps.is_dsc_supported) @@ -1146,16 +1147,13 @@ int compute_mst_dsc_configs_for_state(struct drm_atomic_state *state, if (!is_dsc_need_re_compute(state, dc_state, stream->link)) continue; - mutex_lock(&aconnector->mst_mgr.lock); - - ret = compute_mst_dsc_configs_for_link(state, dc_state, stream->link, vars, - &aconnector->mst_mgr, + mst_mgr = aconnector->port->mgr; + mutex_lock(&mst_mgr->lock); + ret = compute_mst_dsc_configs_for_link(state, dc_state, stream->link, vars, mst_mgr, &link_vars_start_index); - if (ret != 0) { - mutex_unlock(&aconnector->mst_mgr.lock); + mutex_unlock(&mst_mgr->lock); + if (ret != 0) return ret; - } - mutex_unlock(&aconnector->mst_mgr.lock); for (j = 0; j < dc_state->stream_count; j++) { if (dc_state->streams[j]->link == stream->link) @@ -1182,6 +1180,7 @@ static int pre_compute_mst_dsc_configs_for_state(struct drm_atomic_state *state, struct dc_stream_state *stream; bool computed_streams[MAX_PIPES]; struct amdgpu_dm_connector *aconnector; + struct drm_dp_mst_topology_mgr *mst_mgr; int link_vars_start_index = 0; int ret; @@ -1196,7 +1195,7 @@ static int pre_compute_mst_dsc_configs_for_state(struct drm_atomic_state *state, aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context; - if (!aconnector || !aconnector->dc_sink) + if (!aconnector || !aconnector->dc_sink || !aconnector->port) continue; if (!aconnector->dc_sink->dsc_caps.dsc_dec_caps.is_dsc_supported) @@ -1208,15 +1207,13 @@ static int pre_compute_mst_dsc_configs_for_state(struct drm_atomic_state *state, if (!is_dsc_need_re_compute(state, dc_state, stream->link)) continue; - mutex_lock(&aconnector->mst_mgr.lock); - ret = compute_mst_dsc_configs_for_link(state, dc_state, stream->link, vars, - &aconnector->mst_mgr, + mst_mgr = aconnector->port->mgr; + mutex_lock(&mst_mgr->lock); + ret = compute_mst_dsc_configs_for_link(state, dc_state, stream->link, vars, mst_mgr, &link_vars_start_index); - if (ret != 0) { - mutex_unlock(&aconnector->mst_mgr.lock); + mutex_unlock(&mst_mgr->lock); + if (ret != 0) return ret; - } - mutex_unlock(&aconnector->mst_mgr.lock); for (j = 0; j < dc_state->stream_count; j++) { if (dc_state->streams[j]->link == stream->link) @@ -1419,6 +1416,7 @@ enum dc_status dm_dp_mst_is_port_support_mode( unsigned int upper_link_bw_in_kbps = 0, down_link_bw_in_kbps = 0; unsigned int max_compressed_bw_in_kbps = 0; struct dc_dsc_bw_range bw_range = {0}; + struct drm_dp_mst_topology_mgr *mst_mgr; /* * check if the mode could be supported if DSC pass-through is supported @@ -1427,7 +1425,8 @@ enum dc_status dm_dp_mst_is_port_support_mode( */ if (is_dsc_common_config_possible(stream, &bw_range) && aconnector->port->passthrough_aux) { - mutex_lock(&aconnector->mst_mgr.lock); + mst_mgr = aconnector->port->mgr; + mutex_lock(&mst_mgr->lock); cur_link_settings = stream->link->verified_link_cap; @@ -1440,7 +1439,7 @@ enum dc_status dm_dp_mst_is_port_support_mode( end_to_end_bw_in_kbps = min(upper_link_bw_in_kbps, down_link_bw_in_kbps); - mutex_unlock(&aconnector->mst_mgr.lock); + mutex_unlock(&mst_mgr->lock); /* * use the maximum dsc compression bandwidth as the required