From patchwork Tue Oct 22 06:41:30 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomi Valkeinen X-Patchwork-Id: 13845219 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 16212D1CDA8 for ; Tue, 22 Oct 2024 06:43:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Cc:To:Message-Id: Content-Transfer-Encoding:Content-Type:MIME-Version:Subject:Date:From: Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender :Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References:List-Owner; bh=GQkpGkxaN5na3Deoq7e6WNGOHOVo62v3rmhbhlOiZxs=; b=Tk2FrOl4KNwK/a0KguRrUSv5A2 idJyQjo2Zcsw0CmZOhIjb/4n6C16uPDW364Pqzsb3vyfL9pCCmIpyLFw9HHGvFmvNnEnqlZ7exM/s 97ZG7Sfw8Uow2Ip12gIcY8lhCW0erBn1XSp2359/9kKqw1KMFOi6LlOy3FJ9bajpvqaR53m6BtdZI fpCNwJ9/QiD2HueF4G3bNLNwwJVdcWcCTm29KLhCc7+phHjNMia/oH7rB32fz3sxroeWpyYZ8qTxE zuzx9St56VEq7K8IDKuGCpDHltckr9hSeu3GRoiZW5qnfmGI+YZsi3Ngd5uLPITSB0wlGlK0Z43B1 Rsmgue5g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1t38c9-00000009r08-1ctq; Tue, 22 Oct 2024 06:43:33 +0000 Received: from perceval.ideasonboard.com ([213.167.242.64]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1t38ac-00000009qZT-0eVx for linux-arm-kernel@lists.infradead.org; Tue, 22 Oct 2024 06:41:59 +0000 Received: from [127.0.1.1] (91-157-155-49.elisa-laajakaista.fi [91.157.155.49]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 215CF49E; Tue, 22 Oct 2024 08:40:07 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1729579207; bh=Q/m6JeHkE9NHRmu3I+Eg8Y5KqSdt7FPgW1BJMS1oWLA=; h=From:Date:Subject:To:Cc:From; b=Z1l6JpDWrpUera0OMR4LUP2gafNWWrk2fX320xOmkU8YUA6Gl7QTaMhCIoZ0C2FPo YJnlkPMYfrul+89mg4RO4m/WhLYdRAxWMRHbnAzwBlu7ud1IBoePMy+w7JHin+/ys4 dqS9DqHk/Hk+ZyqEJ3YL/qcyklrUFs4/NOFTbOWE= From: Tomi Valkeinen Date: Tue, 22 Oct 2024 09:41:30 +0300 Subject: [PATCH RFC] pmdomain: ti-sci: Set PD on/off state according to the HW state MIME-Version: 1.0 Message-Id: <20241022-tisci-pd-boot-state-v1-1-849a6384131b@ideasonboard.com> X-B4-Tracking: v=1; b=H4sIABlJF2cC/6tWKk4tykwtVrJSqFYqSi3LLM7MzwNyDHUUlJIzE vPSU3UzU4B8JSMDIxNDAyMj3ZLM4uRM3YIU3aT8/BLd4pLEklRdY+PkNAOj1LQUY3MLJaDOgqL UtMwKsKnRSkFuzkqxtbUAEg/EnGoAAAA= To: Nishanth Menon , Tero Kristo , Santosh Shilimkar , Ulf Hansson Cc: linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, Kevin Hilman , vishalm@ti.com, sebin.francis@ti.com, d-gole@ti.com, Devarsh Thakkar , Vignesh Raghavendra , Tomi Valkeinen X-Mailer: b4 0.13.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=4375; i=tomi.valkeinen@ideasonboard.com; h=from:subject:message-id; bh=Q/m6JeHkE9NHRmu3I+Eg8Y5KqSdt7FPgW1BJMS1oWLA=; b=owEBbQKS/ZANAwAIAfo9qoy8lh71AcsmYgBnF0kt6AEWHMs+jTfYRAakF2U0Uqrtn+VgopsFa /z4rHlhUPSJAjMEAAEIAB0WIQTEOAw+ll79gQef86f6PaqMvJYe9QUCZxdJLQAKCRD6PaqMvJYe 9YIiD/4uByZFHf/N4T/D0FM++g4zR+G38U4XJSs9Qmc0duIglDf/zb1tTeAhXGAzXv9YTAcX1BO kmPU3cx68Bq5mBjuCoq7tEwGbYHx+Dzf08V3M9lGfIzVGkFLtNNe827Za3lLmdN3w8udmgK4SUH ubV6G9IcfyXetf0DQBXNRCoCOZPMaaThW4nyNIprkkOAg2BckN7rKd54hDtKvZjPt38z/P8VKKg Skj8+OKlyu8hhbfbKXonO96YUuu34hD8+P+qHlkmeXtv2fw4j1jPEwteXCSAdYk6SIP4Uzd+uVt 1CO+uxFEfevMH/6I3XINKl3WKmG8V6AQFibtb6TJkGM/JYxrPMYvjeGktzTUi5r6Pdv5uhDT9ls cwltLmKdmnNIbnNvhpsIcqGsCqsUYrmdLMLpirmYMxTu1Ne0WRUiXj2JUIVpOcYslVzDBxzKOo5 473KtWB+UKENRCDKjJI2XBEB0aDLdInx8naNaene4PmuDoIORX47sTVxEwYDdedWCBKiQHILPot uYFwuGHW3cHmA8SWI8rZ/6Z7Y6Pk6inr6qzusR+FHRnn7inISndSKsf5YC6e7/D2ZBWytQoeJyI QQeRahv6E2suGlg7nloqs8WYDMFQJke9nire/rCOKqHsllmaDtzZhZXgRHds0VcTudey4rVeMTb eLJotADLNFX5M9A== X-Developer-Key: i=tomi.valkeinen@ideasonboard.com; a=openpgp; fpr=C4380C3E965EFD81079FF3A7FA3DAA8CBC961EF5 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20241021_234158_374773_70F917C9 X-CRM114-Status: GOOD ( 26.43 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org At the moment the driver sets the power state of all the PDs it creates to off, regardless of the actual HW state. This has two drawbacks: 1) The kernel cannot disable unused PDs automatically for power saving, as it thinks they are off already 2) A more specific case (but perhaps applicable to other scenarios also): bootloader enabled splash-screen cannot be kept on the screen. The issue in 2) is that the driver framework automatically enables the device's PD before calling probe() and disables it after the probe(). This means that when the display subsystem (DSS) driver probes, but e.g. fails due to deferred probing, the DSS PD gets turned off and the driver cannot do anything to affect that. Solving the 2) requires more changes to actually keep the PD on during the boot, but a prerequisite for it is to have the correct power state for the PD. The downside with this patch is that it takes time to call the 'is_on' op, and we need to call it for each PD. In my tests with AM62 SK, using defconfig, I see an increase from ~3.5ms to ~7ms. However, the added feature is valuable, so in my opinion it's worth it. The performance could probably be improved with a new firmware API which returns the power states of all the PDs. There's also a related HW issue at play here: if the DSS IP is enabled and active, and its PD is turned off without first disabling the DSS display outputs, the DSS IP will hang and causes the kernel to halt if and when the DSS driver accesses the DSS registers the next time. With the current upstream kernel, with this patch applied, this means that if the bootloader enables the display, and the DSS driver is compiled as a module, the kernel will at some point disable unused PDs, including the DSS PD. When the DSS module is later loaded, it will hang the kernel. The same issue is already there, even without this patch, as the DSS driver may hit deferred probing, which causes the PD to be turned off, and leading to kernel halt when the DSS driver is probed again. This issue has been made quite rare with some arrangements in the DSS driver's probe, but it's still there. So, because of the DSS hang issues, I think this patch is still an RFC. Hopefully we can sort out all the issues, but this patch (or similar) will be part of the solution so I'd like to get some acks/nacks/comments for this. Also, this change might have side effects to other devices too, if the drivers expect the PD to be on, so testing is needed. Signed-off-by: Tomi Valkeinen Reviewed-by: Kevin Hilman Tested-by: Kevin Hilman --- drivers/pmdomain/ti/ti_sci_pm_domains.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) --- base-commit: 98f7e32f20d28ec452afb208f9cffc08448a2652 change-id: 20241022-tisci-pd-boot-state-33cf02efd378 Best regards, diff --git a/drivers/pmdomain/ti/ti_sci_pm_domains.c b/drivers/pmdomain/ti/ti_sci_pm_domains.c index 1510d5ddae3d..14c51a395d7e 100644 --- a/drivers/pmdomain/ti/ti_sci_pm_domains.c +++ b/drivers/pmdomain/ti/ti_sci_pm_domains.c @@ -126,6 +126,23 @@ static bool ti_sci_pm_idx_exists(struct ti_sci_genpd_provider *pd_provider, u32 return false; } +static bool ti_sci_pm_pd_is_on(struct ti_sci_genpd_provider *pd_provider, + int pd_idx) +{ + bool is_on; + int ret; + + if (!pd_provider->ti_sci->ops.dev_ops.is_on) + return false; + + ret = pd_provider->ti_sci->ops.dev_ops.is_on(pd_provider->ti_sci, + pd_idx, NULL, &is_on); + if (ret) + return false; + + return is_on; +} + static int ti_sci_pm_domain_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -161,6 +178,8 @@ static int ti_sci_pm_domain_probe(struct platform_device *pdev) break; if (args.args_count >= 1 && args.np == dev->of_node) { + bool is_on; + if (args.args[0] > max_id) { max_id = args.args[0]; } else { @@ -189,7 +208,10 @@ static int ti_sci_pm_domain_probe(struct platform_device *pdev) pd->idx = args.args[0]; pd->parent = pd_provider; - pm_genpd_init(&pd->pd, NULL, true); + is_on = ti_sci_pm_pd_is_on(pd_provider, + pd->idx); + + pm_genpd_init(&pd->pd, NULL, !is_on); list_add(&pd->node, &pd_provider->pd_list); }