Message ID | 20230926225955.386553-3-mario.limonciello@amd.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:cae8:0:b0:403:3b70:6f57 with SMTP id r8csp3676485vqu; Thu, 28 Sep 2023 16:57:16 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHwbbmHsRZSHC8mGeRORVEbdufUEkspfe8v524HsmPTGzbz+8sCbvcoVI04jYFA+IHeag1M X-Received: by 2002:a05:6a00:993:b0:68f:cf6f:e21e with SMTP id u19-20020a056a00099300b0068fcf6fe21emr2922319pfg.28.1695945436268; Thu, 28 Sep 2023 16:57:16 -0700 (PDT) ARC-Seal: i=2; a=rsa-sha256; t=1695945436; cv=pass; d=google.com; s=arc-20160816; b=aT13wrtWY0CCzNy2Oxv++Mppy87S75Y61tR3Ip8lCxysnGB4YKYWLypBZcQi+bkO2l NVZ+T/9l3/weKFqZVhjHhrBdB0xDeh2o/24dGMv9ISULr3Ot4KFtxf7uSMnMB8A614yq XJGILrgyTflpDnxFFnrt/eNJCjUwn21iD0Pm7oksjBLS6KeKOHUHBujKQ+Aoz+9xY25C DCPib//TmRh2OtrV2SOFPV3LpYipSf8fkYzVVqyrvqH0sbTr0Ps8ber7qIqoHua76Nbf lXQxf4h4jo7D1TL0ylnIy8XWzJP2ha75BUxvPiCnW2IOg25sJ6geCpw7gPWgJZSUdig9 MD7w== ARC-Message-Signature: i=2; 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=3Def7aua7Saf96hnHEYbIARDNTIxX/ofd10KyHqIMrQ=; fh=ABVm0psAHlO3wUFuG4B/8TQE9V6/c2W7vr8KVwJc/z8=; b=O4Ecn3sP2YZybT15o4y6xaN+byt+Gk8fRgDpQKo2b4G2/Eyi8o9V9x5RjW18Gv8SHR v5ynRepJoC0mhKEaCgPLJM4lkhSFHElXsd8ZOKXAfEIDkUcC96grQ8n8ftyPHeKgRaTg rwzhfCrXMkF+fJXn5yy+idPdxaVrCOqnehAW7xKrRDGxbXAaexYKInBct/kBxr+Ta8jL jUW4tSlmnliu/Pyv2sBxeOTZVBk5tQ09BnNDlM4s8tmyUXvb3ozIyVVsdrzyiAWt4C7u V+DpDZ4c6vdg5DTcYkR4B/z/0ZdlRfClX9yaU2fJbO0lsozKB1YG6RLpTAkRsLKsUpRP vYoA== ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@amd.com header.s=selector1 header.b=fOoj6RoX; arc=pass (i=1 spf=pass spfdomain=amd.com dmarc=pass fromdomain=amd.com); spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=amd.com Received: from lipwig.vger.email (lipwig.vger.email. [2620:137:e000::3:3]) by mx.google.com with ESMTPS id k17-20020a056a00135100b0068fba252466si19336278pfu.169.2023.09.28.16.57.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 Sep 2023 16:57:16 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3; Authentication-Results: mx.google.com; dkim=pass header.i=@amd.com header.s=selector1 header.b=fOoj6RoX; arc=pass (i=1 spf=pass spfdomain=amd.com dmarc=pass fromdomain=amd.com); spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=amd.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by lipwig.vger.email (Postfix) with ESMTP id D0F1D805367E; Thu, 28 Sep 2023 09:36:13 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at lipwig.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231617AbjI1Qff (ORCPT <rfc822;pwkd43@gmail.com> + 21 others); Thu, 28 Sep 2023 12:35:35 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58056 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231397AbjI1Qf1 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 28 Sep 2023 12:35:27 -0400 Received: from NAM12-BN8-obe.outbound.protection.outlook.com (mail-bn8nam12on2079.outbound.protection.outlook.com [40.107.237.79]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 539C8193; Thu, 28 Sep 2023 09:35:25 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=njyOKal7PmHJuKyvRiNLdxZl1ymhP+zoAC6VJssFvtRdNbw3qwoFwNmJpj930k8XpC35gA7schffkVtEAq0dA4/mgxirzvY/1goljtEuwoADfjrhG1CLmQcCFycYw9kx61PfaxSTZVrTv4BN91aUXKiatM/guruNe5GWrhUz/adx4gpARL23246U0wVqiGqjewlwMPKf/eT7jqK/X+r/wDS8V7oBJzzEjuKpbikWWe922LZxSFiplLysH1XC3CUICyQdVl+6+r8V+XMRJiBlbSRLe0EW4Yr/WCjivcifNP0kvUoE9gc4Aa4FotlrVC/kj/rktl8PDU/mbqh/0Bu/Xw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=3Def7aua7Saf96hnHEYbIARDNTIxX/ofd10KyHqIMrQ=; b=Ys3SCl5cB407jRudITy2lRnkbraI+clJK4leO2RFd5x71qpw1D36A51sKoJ8Fqf+ZzHbf7iAe0U1474Dj2MpUS6ArQ2hcDSZv9RWqrdXAf8Yq4ZetTL8fdJN3HzVEvLtlxr/QFJSP5BADuUAxLUZkmPhca3qAHVmH+zVc+nOgtyLI81LdoXim9V3yOvIU6mnAeHvR1A4Bzg4K+SRF9jbCJOU4yQXylfrwkkDNIwhRuDe1xVJ10u7SaHhHw9BF4Xdugb9QIpvKQLbTXqV7ofR27KRQOoJ2RWWLjHjIkKChtB32/m0aW8J3nPbrPO3a9f/n2DJT7zdzAVgugOsdAupFA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=lists.freedesktop.org smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=3Def7aua7Saf96hnHEYbIARDNTIxX/ofd10KyHqIMrQ=; b=fOoj6RoXw7VdiBI+j3MeT3aVDXuL92amtAKZ7OxpVHZpTfL9Oix02+2vjUpUJ4NeQqC3KvQ/6eiW7LR+emddXdCLFcv+L8D6/WZFTBT62CisCO1D8bGZR3Gs05K8d3FrxGX1WF/VMQDZE9BxYGWOl2oceXS6/4OXhcLjWIcz/AU= Received: from SA1PR02CA0011.namprd02.prod.outlook.com (2603:10b6:806:2cf::28) by LV2PR12MB5750.namprd12.prod.outlook.com (2603:10b6:408:17e::6) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6838.22; Thu, 28 Sep 2023 16:35:23 +0000 Received: from SA2PEPF00001508.namprd04.prod.outlook.com (2603:10b6:806:2cf:cafe::3f) by SA1PR02CA0011.outlook.office365.com (2603:10b6:806:2cf::28) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6838.25 via Frontend Transport; Thu, 28 Sep 2023 16:35:22 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 165.204.84.17) smtp.mailfrom=amd.com; dkim=none (message not signed) header.d=none;dmarc=pass action=none header.from=amd.com; Received-SPF: Pass (protection.outlook.com: domain of amd.com designates 165.204.84.17 as permitted sender) receiver=protection.outlook.com; client-ip=165.204.84.17; helo=SATLEXMB04.amd.com; pr=C Received: from SATLEXMB04.amd.com (165.204.84.17) by SA2PEPF00001508.mail.protection.outlook.com (10.167.242.40) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.20.6838.14 via Frontend Transport; Thu, 28 Sep 2023 16:35:22 +0000 Received: from AUS-P9-MLIMONCI.amd.com (10.180.168.240) by SATLEXMB04.amd.com (10.181.40.145) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.27; Thu, 28 Sep 2023 11:35:21 -0500 From: Mario Limonciello <mario.limonciello@amd.com> To: <amd-gfx@lists.freedesktop.org>, Sebastian Reichel <sre@kernel.org>, "Alex Deucher" <alexander.deucher@amd.com> CC: <linux-pm@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <Jun.ma2@amd.com>, Mario Limonciello <mario.limonciello@amd.com> Subject: [PATCH 2/3] power: supply: Don't count 'unknown' scope power supplies Date: Tue, 26 Sep 2023 17:59:54 -0500 Message-ID: <20230926225955.386553-3-mario.limonciello@amd.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230926225955.386553-1-mario.limonciello@amd.com> References: <20230926225955.386553-1-mario.limonciello@amd.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Originating-IP: [10.180.168.240] X-ClientProxiedBy: SATLEXMB03.amd.com (10.181.40.144) To SATLEXMB04.amd.com (10.181.40.145) X-EOPAttributedMessage: 0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: SA2PEPF00001508:EE_|LV2PR12MB5750:EE_ X-MS-Office365-Filtering-Correlation-Id: 2df7feb7-0847-4b46-876d-08dbc040e96e X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: Lqn2EKFmjAVeUN2fIA5wwgdjEjzyBqJXxPfTH0O6CStFMMy0h+g54WiojlOD90Xj6Ms/WqhUelO5P3+Y/MKumXnG5TQIrwIJQeqT7OjMz4QYA1pzsNWKj+H+6Af5nd+xgXTlT/S85Ef6Omf3Dp6Gginit2WUAdUj9kYRoTCDdpobR96RfuzCwAh1cQ1Kfbuhe5ABnjwVZea1SgS281IlkVZczFGLyYs1k5exA71Lx1bUhoWMh0B6+KBdm7xjWld2y/+kjcXu9Fwzq93DmD8xxUV5R4l1sF1dQU/TBAlGuZOypil6k0U7qm1L7TjcUMEwTcqmxtwFU/Z+TZm4VCkIIyFaKdRIgD8Kzg5dJG6BRf+tes6aoem29daS//TJvh3ROe6OH20bvRnQMbwFUuLOipT3GCIySDCQkkwhbWnwUyMtrM29cyTVnDMtn5ZIsjWmpRxDu0mK++R/2bG0WAqtvaA8lijwB2Gc3Eh6DgggDhDfqz4yvTZeHDA/0QgLo9CsFTB5mjdHZ0VzdvmNJDUN/9Tbloj++IPhGcckGr4RMzv95VEScL5HyPMn3bcCVCPcPLuqbN2dL7DpMffDsMMq3Ini5pIJpd/a4bQU5be6Ts/+uWi/xhHKCT/Qz+O92Lrut9ayZMKbArKtbgZ2paPCWRz+qkfRFVkkpSCCOfC/mpC0UKalLOytC+Vlw9Ya0CJftzMKJWLXY9xoMcTRvpSZRI1JLOeDMSuAuVphElDuoLA2GxOusa+PkOavvBJYZGlIu5dGxdHr3nV4NTClichKaA== X-Forefront-Antispam-Report: CIP:165.204.84.17;CTRY:US;LANG:en;SCL:1;SRV:;IPV:CAL;SFV:NSPM;H:SATLEXMB04.amd.com;PTR:InfoDomainNonexistent;CAT:NONE;SFS:(13230031)(4636009)(346002)(376002)(396003)(136003)(39860400002)(230922051799003)(186009)(451199024)(1800799009)(64100799003)(82310400011)(46966006)(36840700001)(40470700004)(2906002)(44832011)(5660300002)(4326008)(8936002)(41300700001)(8676002)(26005)(70206006)(6636002)(54906003)(110136005)(316002)(16526019)(1076003)(70586007)(40460700003)(478600001)(7696005)(82740400003)(83380400001)(36756003)(6666004)(336012)(426003)(2616005)(36860700001)(81166007)(47076005)(40480700001)(356005)(86362001)(36900700001);DIR:OUT;SFP:1101; X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Sep 2023 16:35:22.9363 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 2df7feb7-0847-4b46-876d-08dbc040e96e X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d;Ip=[165.204.84.17];Helo=[SATLEXMB04.amd.com] X-MS-Exchange-CrossTenant-AuthSource: SA2PEPF00001508.namprd04.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV2PR12MB5750 X-Spam-Status: No, score=-0.4 required=5.0 tests=DATE_IN_PAST_24_48, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Thu, 28 Sep 2023 09:36:13 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1778327682015110312 X-GMAIL-MSGID: 1778327682015110312 |
Series |
Fix Navi3x boot and hotplug problems
|
|
Commit Message
Mario Limonciello
Sept. 26, 2023, 10:59 p.m. UTC
On some systems AMD Navi3x dGPU triggers RAS errors on startup; but only if the amdgpu kernel module is not part of the initramfs. This is because the hardware is not properly programmed for the AC/DC state of the system when it is loaded later in boot. The AC/DC state of the system is incorrect specifically when UCSI power supplies have been initialized. These power supplies are marked as POWER_SUPPLY_SCOPE_UNKNOWN scope. As they're 'offline' the power supply count is increased but the resultant return value is power_supply_is_system_supplied() 0. To fix this look explicitly for `POWER_SUPPLY_SCOPE_SYSTEM` power supplies before incrementing the count. If no system power supply is found then the system is assumed to be on AC. Cc: stable@vger.kernel.org Tested-by: David Perry <David.Perry@amd.com> Fixes: 95339f40a8b6 ("power: supply: Fix logic checking if system is running from battery") Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> --- drivers/power/supply/power_supply_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Comments
Hi, On Tue, Sep 26, 2023 at 05:59:54PM -0500, Mario Limonciello wrote: > On some systems AMD Navi3x dGPU triggers RAS errors on startup; but > only if the amdgpu kernel module is not part of the initramfs. > This is because the hardware is not properly programmed for the > AC/DC state of the system when it is loaded later in boot. I don't understand the last sentence. As far as I can see i2c_dw_pci_probe() either does not registers UCSI at all or with the dGPU properties (and thus scope) set. > The AC/DC state of the system is incorrect specifically when UCSI power > supplies have been initialized. These power supplies are marked as > POWER_SUPPLY_SCOPE_UNKNOWN scope. As they're 'offline' the power > supply count is increased but the resultant return value is > power_supply_is_system_supplied() 0. > > To fix this look explicitly for `POWER_SUPPLY_SCOPE_SYSTEM` power > supplies before incrementing the count. If no system power supply > is found then the system is assumed to be on AC. > > Cc: stable@vger.kernel.org > Tested-by: David Perry <David.Perry@amd.com> > Fixes: 95339f40a8b6 ("power: supply: Fix logic checking if system is running from battery") > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- This effectively fully disables supply detection for UCSI, because it is never set to POWER_SUPPLY_SCOPE_SYSTEM. Please fix the amdgpu init part instead. -- Sebastian > drivers/power/supply/power_supply_core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c > index d325e6dbc770..3de6e6d00815 100644 > --- a/drivers/power/supply/power_supply_core.c > +++ b/drivers/power/supply/power_supply_core.c > @@ -349,7 +349,7 @@ static int __power_supply_is_system_supplied(struct device *dev, void *data) > unsigned int *count = data; > > if (!psy->desc->get_property(psy, POWER_SUPPLY_PROP_SCOPE, &ret)) > - if (ret.intval == POWER_SUPPLY_SCOPE_DEVICE) > + if (ret.intval != POWER_SUPPLY_SCOPE_SYSTEM) > return 0; > > (*count)++; > -- > 2.34.1 >
On 9/30/2023 15:18, Sebastian Reichel wrote: > Hi, > > On Tue, Sep 26, 2023 at 05:59:54PM -0500, Mario Limonciello wrote: >> On some systems AMD Navi3x dGPU triggers RAS errors on startup; but >> only if the amdgpu kernel module is not part of the initramfs. >> This is because the hardware is not properly programmed for the >> AC/DC state of the system when it is loaded later in boot. > > I don't understand the last sentence. As far as I can see > i2c_dw_pci_probe() either does not registers UCSI at all or > with the dGPU properties (and thus scope) set. I'll explain it better below. > >> The AC/DC state of the system is incorrect specifically when UCSI power >> supplies have been initialized. These power supplies are marked as >> POWER_SUPPLY_SCOPE_UNKNOWN scope. As they're 'offline' the power >> supply count is increased but the resultant return value is >> power_supply_is_system_supplied() 0. >> >> To fix this look explicitly for `POWER_SUPPLY_SCOPE_SYSTEM` power >> supplies before incrementing the count. If no system power supply >> is found then the system is assumed to be on AC. >> >> Cc: stable@vger.kernel.org >> Tested-by: David Perry <David.Perry@amd.com> >> Fixes: 95339f40a8b6 ("power: supply: Fix logic checking if system is running from battery") >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >> --- > > This effectively fully disables supply detection for UCSI, because > it is never set to POWER_SUPPLY_SCOPE_SYSTEM. Please fix the amdgpu > init part instead. I don't think my commit message did a good job conveying why this is a core bug. Let me try to add more detail. This is an OEM system that has 3 USB type C ports. It's an Intel system, but this doesn't matter for the issue. * when ucsi_acpi is not loaded there are no power supplies in the system and it reports power_supply_is_system_supplied() as AC. * When ucsi_acpi is loaded 3 power supplies will be registered. power_supply_is_system_supplied() reports as DC. Now when you add in a Navi3x AMD dGPU to the system the power supplies don't change. This particular dGPU model doesn't contain a USB-C port, so there is no UCSI power supply registered. As amdgpu is loaded it looks at device initialization whether the system is powered by AC or DC. Here is how it looks: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c?h=linux-6.5.y#n3834 On the OEM system if amdgpu loads before the ucsi_acpi driver (such as in the initramfs) then the right value is returned for power_supply_is_system_supplied() - AC. If amdgpu is loaded after the ucsi_acpi driver, the wrong value is returned for power_supply_is_system_supplied() - DC. This value is very important to set up the dGPU properly. If the wrong value is returned, the wrong value will be notified to the hardware and the hardware will not behave properly. On the OEM system this is a "black screen" at bootup along with RAS errors emitted by the dGPU. With no changes to a malfunctioning kernel or initramfs binaries I can add modprobe.blacklist=ucsi_acpi to kernel command line avoid registering those 3 power supplies and the system behaves properly. So I think it's inappropriate for "UNKNOWN" scope power supplies to be registered and treated as system supplies, at least as it pertains to power_supply_is_system_supplied(). > > -- Sebastian > >> drivers/power/supply/power_supply_core.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c >> index d325e6dbc770..3de6e6d00815 100644 >> --- a/drivers/power/supply/power_supply_core.c >> +++ b/drivers/power/supply/power_supply_core.c >> @@ -349,7 +349,7 @@ static int __power_supply_is_system_supplied(struct device *dev, void *data) >> unsigned int *count = data; >> >> if (!psy->desc->get_property(psy, POWER_SUPPLY_PROP_SCOPE, &ret)) >> - if (ret.intval == POWER_SUPPLY_SCOPE_DEVICE) >> + if (ret.intval != POWER_SUPPLY_SCOPE_SYSTEM) >> return 0; >> >> (*count)++; >> -- >> 2.34.1 >>
Hi, On Sun, Oct 01, 2023 at 07:00:11PM -0500, Mario Limonciello wrote: > Let me try to add more detail. > > This is an OEM system that has 3 USB type C ports. It's an Intel system, > but this doesn't matter for the issue. > * when ucsi_acpi is not loaded there are no power supplies in the system and > it reports power_supply_is_system_supplied() as AC. > * When ucsi_acpi is loaded 3 power supplies will be registered. > power_supply_is_system_supplied() reports as DC. > > Now when you add in a Navi3x AMD dGPU to the system the power supplies don't > change. This particular dGPU model doesn't contain a USB-C port, so there > is no UCSI power supply registered. > > As amdgpu is loaded it looks at device initialization whether the system is > powered by AC or DC. Here is how it looks: > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c?h=linux-6.5.y#n3834 > > On the OEM system if amdgpu loads before the ucsi_acpi driver (such as in > the initramfs) then the right value is returned for > power_supply_is_system_supplied() - AC. > > If amdgpu is loaded after the ucsi_acpi driver, the wrong value is returned > for power_supply_is_system_supplied() - DC. > > This value is very important to set up the dGPU properly. If the wrong > value is returned, the wrong value will be notified to the hardware and the > hardware will not behave properly. On the OEM system this is a "black > screen" at bootup along with RAS errors emitted by the dGPU. > > With no changes to a malfunctioning kernel or initramfs binaries I can add > modprobe.blacklist=ucsi_acpi to kernel command line avoid registering those > 3 power supplies and the system behaves properly. > > So I think it's inappropriate for "UNKNOWN" scope power supplies to be > registered and treated as system supplies, at least as it pertains to > power_supply_is_system_supplied(). So the main issue is, that the ucsi_acpi registers a bunch of power-supply chargers with unknown scope on a desktop systems and that results in the system assumed to be supplied from battery. The problem with your change is, that many of the charger drivers don't set a scope at all (and thus report unknown scope). Those obviously should not be skipped. Probably most of these drivers could be changed to properly set the scope, but it needs to be checked on a case-by-case basis. With your current patch they would regress in the oposite direction of your use-case. Ideally ucsi is changed to properly describe the scope, but I suppose this information is not available in ACPI? Assuming that the above are not solvable easily, my idea would be to only count the number of POWER_SUPPLY_TYPE_BATTERY device, which have !POWER_SUPPLY_SCOPE_DEVICE and exit early if there are none. Basically change __power_supply_is_system_supplied(), so that it looks like this: ... if (!psy->desc->get_property(psy, POWER_SUPPLY_PROP_SCOPE, &ret)) if (ret.intval == POWER_SUPPLY_SCOPE_DEVICE) return 0; if (psy->desc->type == POWER_SUPPLY_TYPE_BATTERY) (*count)++; else if (!psy->desc->get_property(psy, POWER_SUPPLY_PROP_ONLINE, &ret)) return ret.intval; ... That should work in both cases. -- Sebastian > > > drivers/power/supply/power_supply_core.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c > > > index d325e6dbc770..3de6e6d00815 100644 > > > --- a/drivers/power/supply/power_supply_core.c > > > +++ b/drivers/power/supply/power_supply_core.c > > > @@ -349,7 +349,7 @@ static int __power_supply_is_system_supplied(struct device *dev, void *data) > > > unsigned int *count = data; > > > if (!psy->desc->get_property(psy, POWER_SUPPLY_PROP_SCOPE, &ret)) > > > - if (ret.intval == POWER_SUPPLY_SCOPE_DEVICE) > > > + if (ret.intval != POWER_SUPPLY_SCOPE_SYSTEM) > > > return 0; > > > (*count)++; > > > -- > > > 2.34.1 > > > >
On 10/4/2023 18:10, Sebastian Reichel wrote: > Hi, > > On Sun, Oct 01, 2023 at 07:00:11PM -0500, Mario Limonciello wrote: >> Let me try to add more detail. >> >> This is an OEM system that has 3 USB type C ports. It's an Intel system, >> but this doesn't matter for the issue. >> * when ucsi_acpi is not loaded there are no power supplies in the system and >> it reports power_supply_is_system_supplied() as AC. >> * When ucsi_acpi is loaded 3 power supplies will be registered. >> power_supply_is_system_supplied() reports as DC. >> >> Now when you add in a Navi3x AMD dGPU to the system the power supplies don't >> change. This particular dGPU model doesn't contain a USB-C port, so there >> is no UCSI power supply registered. >> >> As amdgpu is loaded it looks at device initialization whether the system is >> powered by AC or DC. Here is how it looks: >> >> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c?h=linux-6.5.y#n3834 >> >> On the OEM system if amdgpu loads before the ucsi_acpi driver (such as in >> the initramfs) then the right value is returned for >> power_supply_is_system_supplied() - AC. >> >> If amdgpu is loaded after the ucsi_acpi driver, the wrong value is returned >> for power_supply_is_system_supplied() - DC. >> >> This value is very important to set up the dGPU properly. If the wrong >> value is returned, the wrong value will be notified to the hardware and the >> hardware will not behave properly. On the OEM system this is a "black >> screen" at bootup along with RAS errors emitted by the dGPU. >> >> With no changes to a malfunctioning kernel or initramfs binaries I can add >> modprobe.blacklist=ucsi_acpi to kernel command line avoid registering those >> 3 power supplies and the system behaves properly. >> >> So I think it's inappropriate for "UNKNOWN" scope power supplies to be >> registered and treated as system supplies, at least as it pertains to >> power_supply_is_system_supplied(). > > So the main issue is, that the ucsi_acpi registers a bunch of > power-supply chargers with unknown scope on a desktop systems > and that results in the system assumed to be supplied from battery. > > The problem with your change is, that many of the charger drivers > don't set a scope at all (and thus report unknown scope). Those > obviously should not be skipped. Probably most of these drivers > could be changed to properly set the scope, but it needs to be > checked on a case-by-case basis. With your current patch they would > regress in the oposite direction of your use-case. > > Ideally ucsi is changed to properly describe the scope, but I > suppose this information is not available in ACPI? > > Assuming that the above are not solvable easily, my idea would be to > only count the number of POWER_SUPPLY_TYPE_BATTERY device, which have > !POWER_SUPPLY_SCOPE_DEVICE and exit early if there are none. > Basically change __power_supply_is_system_supplied(), so that it > looks like this: > > ... > if (!psy->desc->get_property(psy, POWER_SUPPLY_PROP_SCOPE, &ret)) > if (ret.intval == POWER_SUPPLY_SCOPE_DEVICE) > return 0; > > if (psy->desc->type == POWER_SUPPLY_TYPE_BATTERY) > (*count)++; > else > if (!psy->desc->get_property(psy, POWER_SUPPLY_PROP_ONLINE, > &ret)) > return ret.intval; > ... > > That should work in both cases. > I tested both your suggestion as well as modifying UCSI driver to set the scope. Both worked. I've sent out v2 modifying the scope for UCSI driver. If for some reason that ends up not working out we can revert to your generic suggestion. https://lore.kernel.org/linux-usb/20231005175230.232764-1-mario.limonciello@amd.com/T/#m9543f1f2c3767c0e88135c2e3f15ced65cfdf004 > -- Sebastian > >>>> drivers/power/supply/power_supply_core.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c >>>> index d325e6dbc770..3de6e6d00815 100644 >>>> --- a/drivers/power/supply/power_supply_core.c >>>> +++ b/drivers/power/supply/power_supply_core.c >>>> @@ -349,7 +349,7 @@ static int __power_supply_is_system_supplied(struct device *dev, void *data) >>>> unsigned int *count = data; >>>> if (!psy->desc->get_property(psy, POWER_SUPPLY_PROP_SCOPE, &ret)) >>>> - if (ret.intval == POWER_SUPPLY_SCOPE_DEVICE) >>>> + if (ret.intval != POWER_SUPPLY_SCOPE_SYSTEM) >>>> return 0; >>>> (*count)++; >>>> -- >>>> 2.34.1 >>>> >>
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c index d325e6dbc770..3de6e6d00815 100644 --- a/drivers/power/supply/power_supply_core.c +++ b/drivers/power/supply/power_supply_core.c @@ -349,7 +349,7 @@ static int __power_supply_is_system_supplied(struct device *dev, void *data) unsigned int *count = data; if (!psy->desc->get_property(psy, POWER_SUPPLY_PROP_SCOPE, &ret)) - if (ret.intval == POWER_SUPPLY_SCOPE_DEVICE) + if (ret.intval != POWER_SUPPLY_SCOPE_SYSTEM) return 0; (*count)++;