From patchwork Thu Sep 21 14:15:12 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 13393952 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 96C51E71072 for ; Thu, 21 Sep 2023 14:15:30 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id CE7C110E137; Thu, 21 Sep 2023 14:15:29 +0000 (UTC) Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3E06710E5AF for ; Thu, 21 Sep 2023 14:15:27 +0000 (UTC) From: Sebastian Andrzej Siewior DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1695305725; 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=FOWN2bP3bL+i8WU1AdRgy2WvLGaYMKjJkqdur8ZMT5U=; b=zORUY5Xkv3wz1yjY4DDKkQzFNInGAaoCeKEqG8+SzR8M7zDT0N1ruCuMNVKDpM/fKlgh73 fCJ9m049d4T1G6He4zIdup4YW9DyLZfJ3UttPGqrGWJ9oE7ZDP7ycRW5rcLKKikjQCRGET jCLgZa1VMJ7KQVKgCsAC3egfUFh3s+VfG4ZqUPCPd3sjB2aTLTTj+gpVxI0tR5NdXqVO5l 3bXUalpS8v/0gDxhZJLRMkBHXbg1kii1QG768G1dMAVf5JAGh1f0A/30F6o1mOJcITctew //OuRxjrxXhzO68wK9aob0OVtekkra0z7oi6HKnN5NN1ekbE6Ih5O43FeqhxxQ== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1695305725; 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=FOWN2bP3bL+i8WU1AdRgy2WvLGaYMKjJkqdur8ZMT5U=; b=sHscKoKcbk8ILIhokgaU/401GuFMDm+ywMOvfMNuLAGr4ihRWdmUt0LzNlUx4RJMoS+GcD MZ3VQCQlC4Y4mODQ== To: dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org Subject: [PATCH 1/5] drm/amd/display: Remove migrate_en/dis from dc_fpu_begin(). Date: Thu, 21 Sep 2023 16:15:12 +0200 Message-Id: <20230921141516.520471-2-bigeasy@linutronix.de> In-Reply-To: <20230921141516.520471-1-bigeasy@linutronix.de> References: <20230921141516.520471-1-bigeasy@linutronix.de> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Tianci Yin , Leo Li , Sebastian Andrzej Siewior , "Pan, Xinhui" , Rodrigo Siqueira , Peter Zijlstra , Aurabindo Pillai , Alex Deucher , Thomas Gleixner , =?utf-8?q?Christian_K=C3=B6nig?= Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" This is a revert of the commit mentioned below while it is not wrong, as in the kernel will explode, having migrate_disable() here it is complete waste of resources. Additionally commit message is plain wrong the review tag does not make it any better. The migrate_disable() interface has a fat comment describing it and it includes the word "undesired" in the headline which should tickle people to read it before using it. Initially I assumed it is worded too harsh but now I beg to differ. The reviewer of the original commit, even not understanding what migrate_disable() does should ask the following: - migrate_disable() is added only to the CONFIG_X86 block and it claims to protect fpu_recursion_depth. Why are the other the architectures excluded? - migrate_disable() is added after fpu_recursion_depth was modified. Shouldn't it be added before the modification or referencing takes place? Moving on. Disabling preemption DOES prevent CPU migration. A task, that can not be pushed away from the CPU by the scheduler (due to disabled preemption) can not be pushed or migrated to another CPU. Disabling migration DOES NOT ensure consistency of per-CPU variables. It only ensures that the task acts always on the same per-CPU variable. The task remains preemptible meaning multiple tasks can access the same per-CPU variable. This in turn leads to inconsistency for the statement *pcpu -= 1; with two tasks on one CPU and a preemption point during the RMW operation: Task A Task B read pcpu to reg # 0 inc reg # 0 -> 1 read pcpu to reg # 0 inc reg # 0 -> 1 write reg to pcpu # 1 write reg to pcpu # 1 At the end pcpu reads 1 but should read 2 instead. Boom. get_cpu_ptr() already contains a preempt_disable() statement. That means that the per-CPU variable can only be referenced by a single task which is currently running. The only inconsistency that can occur if the variable is additionally accessed from an interrupt. Remove migrate_disable/enable() from dc_fpu_begin/end(). Cc: Tianci Yin Cc: Aurabindo Pillai Fixes: 0c316556d1249 ("drm/amd/display: Disable migration to ensure consistency of per-CPU variable") Signed-off-by: Sebastian Andrzej Siewior --- drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c index 172aa10a8800f..86f4c0e046548 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c @@ -91,7 +91,6 @@ void dc_fpu_begin(const char *function_name, const int line) if (*pcpu == 1) { #if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH) - migrate_disable(); kernel_fpu_begin(); #elif defined(CONFIG_PPC64) if (cpu_has_feature(CPU_FTR_VSX_COMP)) { @@ -132,7 +131,6 @@ void dc_fpu_end(const char *function_name, const int line) if (*pcpu <= 0) { #if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH) kernel_fpu_end(); - migrate_enable(); #elif defined(CONFIG_PPC64) if (cpu_has_feature(CPU_FTR_VSX_COMP)) { disable_kernel_vsx(); From patchwork Thu Sep 21 14:15:13 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 13393953 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id D865AE71072 for ; Thu, 21 Sep 2023 14:15:36 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 312A610E5AD; Thu, 21 Sep 2023 14:15:30 +0000 (UTC) Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3CE0210E5AD for ; Thu, 21 Sep 2023 14:15:27 +0000 (UTC) From: Sebastian Andrzej Siewior DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1695305725; 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=sVGBlAnrz6UmtRjv0t+ZHRq3KiWMNmFKvIlfE1mXOc0=; b=NBrV6T0NWs4HKzSdAAji57rmWnzTpkb3H4ynAlflPDBpnoJVaYCFOCJR7JAtYs7xCKlw1O aHOiVXEJIRgQlJDHIj9X63fyESExpsAqAphAq8JZBF7fM4dvSNrq9dfj68R3UTg7Pj4gHS dYecNrlPSYLk5WHvLGCKx7+THlfL9SA1jFN1PcCBnBXY/7TD43OPOkNJ27jJhY3uElIOwD sAJZLCMeiX84HlySp39rlzMx+oS3HmNbqAyBKKXD2Cyz5VmVcYjRX5gIQXN24M+LAvFBRx DGPpvETrFskDDAQ/l21MKpZiMXwbkmK6dzoDzKJkND5mqSeiYQu/APfhpIgNcw== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1695305725; 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=sVGBlAnrz6UmtRjv0t+ZHRq3KiWMNmFKvIlfE1mXOc0=; b=F0m9BVTlImsDc20k/SwfKtjeER2Q8iQ551xbjpO/Bc45kt7ZLyR0obyRHla6Qcv/v8WqR4 vhKrQAG6v4348rCA== To: dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org Subject: [PATCH 2/5] drm/amd/display: Simplify the per-CPU usage. Date: Thu, 21 Sep 2023 16:15:13 +0200 Message-Id: <20230921141516.520471-3-bigeasy@linutronix.de> In-Reply-To: <20230921141516.520471-1-bigeasy@linutronix.de> References: <20230921141516.520471-1-bigeasy@linutronix.de> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Leo Li , Sebastian Andrzej Siewior , "Pan, Xinhui" , Rodrigo Siqueira , Peter Zijlstra , Alex Deucher , Thomas Gleixner , =?utf-8?q?Christian_K=C3=B6nig?= Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" The fpu_recursion_depth counter is used to ensure that dc_fpu_begin() can be invoked multiple times while the FPU-disable function itself is only invoked once. Also the counter part (dc_fpu_end()) is ballanced properly. Instead of using the get_cpu_ptr() dance around the inc it is simpler to increment the per-CPU variable directly. Also the per-CPU variable has to be incremented and decremented on the same CPU. This is ensured by the inner-part which disables preemption. This is kind of not obvious, works and the preempt-counter is touched a few times for no reason. Disable preemption before incrementing fpu_recursion_depth for the first time. Keep preemption disabled until dc_fpu_end() where the counter is decremented making it obvious that the preemption has to stay disabled while the counter is non-zero. Use simple inc/dec functions. Remove the nested preempt_disable/enable functions which are now not needed. Signed-off-by: Sebastian Andrzej Siewior --- .../gpu/drm/amd/display/amdgpu_dm/dc_fpu.c | 50 ++++++++----------- 1 file changed, 20 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c index 86f4c0e046548..8bd5926b47e06 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c @@ -60,11 +60,9 @@ static DEFINE_PER_CPU(int, fpu_recursion_depth); */ inline void dc_assert_fp_enabled(void) { - int *pcpu, depth = 0; + int depth; - pcpu = get_cpu_ptr(&fpu_recursion_depth); - depth = *pcpu; - put_cpu_ptr(&fpu_recursion_depth); + depth = __this_cpu_read(fpu_recursion_depth); ASSERT(depth >= 1); } @@ -84,32 +82,27 @@ inline void dc_assert_fp_enabled(void) */ void dc_fpu_begin(const char *function_name, const int line) { - int *pcpu; + int depth; - pcpu = get_cpu_ptr(&fpu_recursion_depth); - *pcpu += 1; + preempt_disable(); + depth = __this_cpu_inc_return(fpu_recursion_depth); - if (*pcpu == 1) { + if (depth == 1) { #if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH) kernel_fpu_begin(); #elif defined(CONFIG_PPC64) - if (cpu_has_feature(CPU_FTR_VSX_COMP)) { - preempt_disable(); + if (cpu_has_feature(CPU_FTR_VSX_COMP)) enable_kernel_vsx(); - } else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) { - preempt_disable(); + else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) enable_kernel_altivec(); - } else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) { - preempt_disable(); + else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) enable_kernel_fp(); - } #elif defined(CONFIG_ARM64) kernel_neon_begin(); #endif } - TRACE_DCN_FPU(true, function_name, line, *pcpu); - put_cpu_ptr(&fpu_recursion_depth); + TRACE_DCN_FPU(true, function_name, line, depth); } /** @@ -124,29 +117,26 @@ void dc_fpu_begin(const char *function_name, const int line) */ void dc_fpu_end(const char *function_name, const int line) { - int *pcpu; + int depth; - pcpu = get_cpu_ptr(&fpu_recursion_depth); - *pcpu -= 1; - if (*pcpu <= 0) { + depth = __this_cpu_dec_return(fpu_recursion_depth); + if (depth == 0) { #if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH) kernel_fpu_end(); #elif defined(CONFIG_PPC64) - if (cpu_has_feature(CPU_FTR_VSX_COMP)) { + if (cpu_has_feature(CPU_FTR_VSX_COMP)) disable_kernel_vsx(); - preempt_enable(); - } else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) { + else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) disable_kernel_altivec(); - preempt_enable(); - } else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) { + else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) disable_kernel_fp(); - preempt_enable(); - } #elif defined(CONFIG_ARM64) kernel_neon_end(); #endif + } else { + WARN_ON_ONCE(depth < 0); } - TRACE_DCN_FPU(false, function_name, line, *pcpu); - put_cpu_ptr(&fpu_recursion_depth); + TRACE_DCN_FPU(false, function_name, line, depth); + preempt_enable(); } From patchwork Thu Sep 21 14:15:14 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 13393955 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 5AE86E7107A for ; Thu, 21 Sep 2023 14:15:43 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 757A310E5B0; Thu, 21 Sep 2023 14:15:40 +0000 (UTC) Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by gabe.freedesktop.org (Postfix) with ESMTPS id E48F810E137 for ; Thu, 21 Sep 2023 14:15:28 +0000 (UTC) From: Sebastian Andrzej Siewior DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1695305725; 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=dQNP4qnayxb8k0G9c9g8gb+q5OeXVIV16Tw418a50N0=; b=R1e2ZUboVrm1yLGexIoE7faY8XdhJ19BfrSYjPAWiIRFQOLkWNxMcfW9XmVilqgg2FrFYb 1ZEhc4pSVXT24om0L5YH7l7/uLNWlDhr9Djae/phlQUQL/9SGKFzMeyLyMVc3M6TRbrokX WryG7D43QXGitxnMYbCOkYvJOqLc66mppzoTzubvW0q4Hr3FWOe9x7P4U4PL0nLASaiy2i XA9IAOHEEkGy1yrxp+7N7UqE5gH9MTnV2KVz6ARq1iqIkZszh0jqPH6CbjwOa9egPVhPOP JwZOZYr8PQ1O5oTk7I+lRjeopRQ8cqhkPJtK1uOXf39codTf2yYh9oDi8iMvWw== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1695305725; 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=dQNP4qnayxb8k0G9c9g8gb+q5OeXVIV16Tw418a50N0=; b=ITLG7Vr8nA2Acv93CdsFEZspA29eBMF5N9dgjI2frMX6U341lbdKyZqjTl26y24Y8XcIyr PAOk7arYcvyD+2Cw== To: dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org Subject: [PATCH 3/5] drm/amd/display: Add a warning if the FPU is used outside from task context. Date: Thu, 21 Sep 2023 16:15:14 +0200 Message-Id: <20230921141516.520471-4-bigeasy@linutronix.de> In-Reply-To: <20230921141516.520471-1-bigeasy@linutronix.de> References: <20230921141516.520471-1-bigeasy@linutronix.de> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Leo Li , Sebastian Andrzej Siewior , "Pan, Xinhui" , Rodrigo Siqueira , Peter Zijlstra , Alex Deucher , Thomas Gleixner , =?utf-8?q?Christian_K=C3=B6nig?= Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Add a warning if the FPU is used from any context other than task context. This is only precaution since the code is not able to be used from softirq while the API allows it on x86 for instance. Signed-off-by: Sebastian Andrzej Siewior --- drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c index 8bd5926b47e06..4ae4720535a56 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c @@ -84,6 +84,7 @@ void dc_fpu_begin(const char *function_name, const int line) { int depth; + WARN_ON_ONCE(!in_task()); preempt_disable(); depth = __this_cpu_inc_return(fpu_recursion_depth); From patchwork Thu Sep 21 14:15:15 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 13393956 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id E703FE7107D for ; Thu, 21 Sep 2023 14:15:45 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 8749110E5B3; Thu, 21 Sep 2023 14:15:40 +0000 (UTC) Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by gabe.freedesktop.org (Postfix) with ESMTPS id 77D6A10E5B0 for ; Thu, 21 Sep 2023 14:15:27 +0000 (UTC) From: Sebastian Andrzej Siewior DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1695305726; 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=nf89mi48+BTDl9P9LgC24tFBtIdB1s51h5vIjiQv4aM=; b=nfJnajQm2EfomGeimuqmYKDLizXeJivS+k4dgKb5mHbY+A2J42EWPVgFFt4MrmJVQhdT1T 01VAKFj7bsGz2SJfb+oeL1anUzjfdCM/NE8Aji8jXMlCfk2jwaynq5No7HC5Dham2aRIxi abZ3iQoR1W+TTxJ3I03KrTrSnWlirrjUY0cxjAByjybfYigGilZ31Jq3QOoxZ+WgUmIgGS iTQjyZSEr7vowPWotLkrxVbR4m3uouJObGYygKHeAhhTV5IjggswjqMh2wT0FYhkTLXUCY 4PtAdNzSkDmj1kC2Sns6pEv34qGcO4oVxDWt88MUXykKE5amRr+UZYyad+89Hg== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1695305726; 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=nf89mi48+BTDl9P9LgC24tFBtIdB1s51h5vIjiQv4aM=; b=vDdumqZ+OuhqqEqPf5tBX4Z38hg1tC9FoJ+b7EXxM+auQmdl0twrEvTK0dwjFvUhccxKDI j0SwiWJVTdARuDCw== To: dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org Subject: [PATCH 4/5] drm/amd/display: Move the memory allocation out of dcn21_validate_bandwidth_fp(). Date: Thu, 21 Sep 2023 16:15:15 +0200 Message-Id: <20230921141516.520471-5-bigeasy@linutronix.de> In-Reply-To: <20230921141516.520471-1-bigeasy@linutronix.de> References: <20230921141516.520471-1-bigeasy@linutronix.de> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Leo Li , Sebastian Andrzej Siewior , "Pan, Xinhui" , Rodrigo Siqueira , Peter Zijlstra , Alex Deucher , Thomas Gleixner , =?utf-8?q?Christian_K=C3=B6nig?= Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" dcn21_validate_bandwidth_fp() is invoked while FPU access has been enabled. FPU access requires disabling preemption even on PREEMPT_RT. It is not possible to allocate memory with disabled preemption even with GFP_ATOMIC on PREEMPT_RT. Move the memory allocation before FPU access is enabled. Link: https://bugzilla.kernel.org/show_bug.cgi?id=217928 Signed-off-by: Sebastian Andrzej Siewior --- drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c | 10 +++++++++- drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c | 7 ++----- drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.h | 5 ++--- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c b/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c index d1a25fe6c44fa..5674c3450fc36 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c @@ -953,9 +953,17 @@ static bool dcn21_validate_bandwidth(struct dc *dc, struct dc_state *context, bool fast_validate) { bool voltage_supported; + display_e2e_pipe_params_st *pipes; + + pipes = kcalloc(dc->res_pool->pipe_count, sizeof(display_e2e_pipe_params_st), GFP_KERNEL); + if (!pipes) + return false; + DC_FP_START(); - voltage_supported = dcn21_validate_bandwidth_fp(dc, context, fast_validate); + voltage_supported = dcn21_validate_bandwidth_fp(dc, context, fast_validate, pipes); DC_FP_END(); + + kfree(pipes); return voltage_supported; } diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c index 5805fb02af14e..8b2038162a7e1 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c @@ -2216,9 +2216,8 @@ static void dcn21_calculate_wm(struct dc *dc, struct dc_state *context, &context->bw_ctx.dml, pipes, pipe_cnt); } -bool dcn21_validate_bandwidth_fp(struct dc *dc, - struct dc_state *context, - bool fast_validate) +bool dcn21_validate_bandwidth_fp(struct dc *dc, struct dc_state *context, + bool fast_validate, display_e2e_pipe_params_st *pipes) { bool out = false; @@ -2227,7 +2226,6 @@ bool dcn21_validate_bandwidth_fp(struct dc *dc, int vlevel = 0; int pipe_split_from[MAX_PIPES]; int pipe_cnt = 0; - display_e2e_pipe_params_st *pipes = kzalloc(dc->res_pool->pipe_count * sizeof(display_e2e_pipe_params_st), GFP_ATOMIC); DC_LOGGER_INIT(dc->ctx->logger); BW_VAL_TRACE_COUNT(); @@ -2267,7 +2265,6 @@ bool dcn21_validate_bandwidth_fp(struct dc *dc, out = false; validate_out: - kfree(pipes); BW_VAL_TRACE_FINISH(); diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.h b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.h index c51badf7b68a9..a81a0b9e68842 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.h +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.h @@ -77,9 +77,8 @@ int dcn21_populate_dml_pipes_from_context(struct dc *dc, struct dc_state *context, display_e2e_pipe_params_st *pipes, bool fast_validate); -bool dcn21_validate_bandwidth_fp(struct dc *dc, - struct dc_state *context, - bool fast_validate); +bool dcn21_validate_bandwidth_fp(struct dc *dc, struct dc_state *context, bool + fast_validate, display_e2e_pipe_params_st *pipes); void dcn21_update_bw_bounding_box(struct dc *dc, struct clk_bw_params *bw_params); void dcn21_clk_mgr_set_bw_params_wm_table(struct clk_bw_params *bw_params); From patchwork Thu Sep 21 14:15:16 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 13393957 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 002D7E71072 for ; Thu, 21 Sep 2023 14:15:49 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 8E68210E5B5; Thu, 21 Sep 2023 14:15:40 +0000 (UTC) Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by gabe.freedesktop.org (Postfix) with ESMTPS id 4B68A10E5AF for ; Thu, 21 Sep 2023 14:15:30 +0000 (UTC) From: Sebastian Andrzej Siewior DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1695305726; 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=3QKfJEY1uLi87VIrjkLudvRExB2VG8jcVhhoK+qWqrU=; b=zE/czbZ4vhjGgipLwIvYHBCSOwaXOsr2FHzPwUl12r9qkPAilJqqhw04tSHieJiqr+nrjB HFV+3rx8nqby67Q8tVSb5ihXER9ip/f9Yn8kp4Pf0/l8r9CWmCESe9y0ebW3Gjwp9lVTNV Zq7HP6jFjUvpfI+2bXsGtgFsBu+a3vi5G+LqoxybmlCAe58GPgb6FN2Rh/vhU/ToULLgRX 9CSRWlAt8Gj4urqYo75pU0daqpFK5SbyFhXaPZQI7PURhYyZKZWBGDxCiPiKnnzUIMzTjj HMrGBErt3D49XlwnZRM/Pl+/mnm9UPBT3CKvGCXMaEj5sz5MYBQFMvXkO55I6g== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1695305726; 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=3QKfJEY1uLi87VIrjkLudvRExB2VG8jcVhhoK+qWqrU=; b=DZ/H+yLvkKfU6D2WwvKVS0fxhhgMN7NtQ3+NP/OLmnyTklcdRT0JyqBbvhMrSqxDJY9QEV 72CINHkotZEzKEAw== To: dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org Subject: [PATCH 5/5] drm/amd/display: Move the memory allocation out of dcn20_validate_bandwidth_fp(). Date: Thu, 21 Sep 2023 16:15:16 +0200 Message-Id: <20230921141516.520471-6-bigeasy@linutronix.de> In-Reply-To: <20230921141516.520471-1-bigeasy@linutronix.de> References: <20230921141516.520471-1-bigeasy@linutronix.de> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Leo Li , Sebastian Andrzej Siewior , "Pan, Xinhui" , Rodrigo Siqueira , Peter Zijlstra , Alex Deucher , Thomas Gleixner , =?utf-8?q?Christian_K=C3=B6nig?= Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" dcn20_validate_bandwidth_fp() is invoked while FPU access has been enabled. FPU access requires disabling preemption even on PREEMPT_RT. It is not possible to allocate memory with disabled preemption even with GFP_ATOMIC on PREEMPT_RT. Move the memory allocation before FPU access is enabled. To preserve previous "clean" state of "pipes" add a memset() before the second invocation of dcn20_validate_bandwidth_internal() where the variable is used. Signed-off-by: Sebastian Andrzej Siewior --- .../drm/amd/display/dc/dcn20/dcn20_resource.c | 10 +++++++++- .../gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c | 16 +++++++--------- .../gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.h | 5 ++--- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c index d587f807dfd7c..5036a3e608324 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c @@ -2141,9 +2141,17 @@ bool dcn20_validate_bandwidth(struct dc *dc, struct dc_state *context, bool fast_validate) { bool voltage_supported; + display_e2e_pipe_params_st *pipes; + + pipes = kcalloc(dc->res_pool->pipe_count, sizeof(display_e2e_pipe_params_st), GFP_KERNEL); + if (!pipes) + return false; + DC_FP_START(); - voltage_supported = dcn20_validate_bandwidth_fp(dc, context, fast_validate); + voltage_supported = dcn20_validate_bandwidth_fp(dc, context, fast_validate, pipes); DC_FP_END(); + + kfree(pipes); return voltage_supported; } diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c index 8b2038162a7e1..2ad92497b9bf2 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c @@ -1923,7 +1923,7 @@ void dcn20_patch_bounding_box(struct dc *dc, struct _vcs_dpi_soc_bounding_box_st } static bool dcn20_validate_bandwidth_internal(struct dc *dc, struct dc_state *context, - bool fast_validate) + bool fast_validate, display_e2e_pipe_params_st *pipes) { bool out = false; @@ -1932,7 +1932,6 @@ static bool dcn20_validate_bandwidth_internal(struct dc *dc, struct dc_state *co int vlevel = 0; int pipe_split_from[MAX_PIPES]; int pipe_cnt = 0; - display_e2e_pipe_params_st *pipes = kzalloc(dc->res_pool->pipe_count * sizeof(display_e2e_pipe_params_st), GFP_ATOMIC); DC_LOGGER_INIT(dc->ctx->logger); BW_VAL_TRACE_COUNT(); @@ -1967,16 +1966,14 @@ static bool dcn20_validate_bandwidth_internal(struct dc *dc, struct dc_state *co out = false; validate_out: - kfree(pipes); BW_VAL_TRACE_FINISH(); return out; } -bool dcn20_validate_bandwidth_fp(struct dc *dc, - struct dc_state *context, - bool fast_validate) +bool dcn20_validate_bandwidth_fp(struct dc *dc, struct dc_state *context, + bool fast_validate, display_e2e_pipe_params_st *pipes) { bool voltage_supported = false; bool full_pstate_supported = false; @@ -1995,11 +1992,11 @@ bool dcn20_validate_bandwidth_fp(struct dc *dc, ASSERT(context != dc->current_state); if (fast_validate) { - return dcn20_validate_bandwidth_internal(dc, context, true); + return dcn20_validate_bandwidth_internal(dc, context, true, pipes); } // Best case, we support full UCLK switch latency - voltage_supported = dcn20_validate_bandwidth_internal(dc, context, false); + voltage_supported = dcn20_validate_bandwidth_internal(dc, context, false, pipes); full_pstate_supported = context->bw_ctx.bw.dcn.clk.p_state_change_support; if (context->bw_ctx.dml.soc.dummy_pstate_latency_us == 0 || @@ -2011,7 +2008,8 @@ bool dcn20_validate_bandwidth_fp(struct dc *dc, // Fallback: Try to only support G6 temperature read latency context->bw_ctx.dml.soc.dram_clock_change_latency_us = context->bw_ctx.dml.soc.dummy_pstate_latency_us; - voltage_supported = dcn20_validate_bandwidth_internal(dc, context, false); + memset(pipes, 0, dc->res_pool->pipe_count * sizeof(display_e2e_pipe_params_st)); + voltage_supported = dcn20_validate_bandwidth_internal(dc, context, false, pipes); dummy_pstate_supported = context->bw_ctx.bw.dcn.clk.p_state_change_support; if (voltage_supported && (dummy_pstate_supported || !(context->stream_count))) { diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.h b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.h index a81a0b9e68842..b6c34198ddc86 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.h +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.h @@ -61,9 +61,8 @@ void dcn20_update_bounding_box(struct dc *dc, unsigned int num_states); void dcn20_patch_bounding_box(struct dc *dc, struct _vcs_dpi_soc_bounding_box_st *bb); -bool dcn20_validate_bandwidth_fp(struct dc *dc, - struct dc_state *context, - bool fast_validate); +bool dcn20_validate_bandwidth_fp(struct dc *dc, struct dc_state *context, + bool fast_validate, display_e2e_pipe_params_st *pipes); void dcn20_fpu_set_wm_ranges(int i, struct pp_smu_wm_range_sets *ranges, struct _vcs_dpi_soc_bounding_box_st *loaded_bb);