From patchwork Thu Jul 18 23:43:14 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ulf Hansson X-Patchwork-Id: 13736783 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 18476C3DA49 for ; Thu, 18 Jul 2024 23:44:19 +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:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=+zb2TnLEPy/yYn4OHpN9GAbJBX2HlvGglgTvnLaEwow=; b=ROz60hJWdcLV+5KYljFOlleEAR b1EcIVcxpEnDnYkJRU7Z7TSMMx3vN19q4hraEaT50m7734BaD8W2hakqh0zQBIIWB1RsOTcn3DVGz I+kPcfzV8WtLslCG2BSs9zGg1hKjiMUzGW5hpbR1gMWFMMz1VXhpAhYKf8jd9/XPvAqNxM9DTQhC4 4+swuMCfHHPpqCQ2+Dz7ESFH+SbmAKdxX4C6kD7dS9TjccM0dQoDSYGuhr27U9uZuN98Etf4n8pPp yMiRRQFwoW+3vdjtEIcRe9f0ejHSwjbeko7ECUSz5EvWZkYXnvSubjNvpB98tKxhUkH/WhpYUt+Jl fmxEkvoQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sUan9-000000011G7-2OCL; Thu, 18 Jul 2024 23:44:07 +0000 Received: from mail-lf1-x135.google.com ([2a00:1450:4864:20::135]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sUamV-0000000114G-289u for linux-arm-kernel@lists.infradead.org; Thu, 18 Jul 2024 23:43:29 +0000 Received: by mail-lf1-x135.google.com with SMTP id 2adb3069b0e04-52e976208f8so937949e87.2 for ; Thu, 18 Jul 2024 16:43:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1721346206; x=1721951006; darn=lists.infradead.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=+zb2TnLEPy/yYn4OHpN9GAbJBX2HlvGglgTvnLaEwow=; b=pEpMo4CFgDfWgdWd2RbjC5//0n/ZqK+CCMjZ2ETufDWzc1pjFXzivc5nRtkc2r8d14 bfXp0tWNyBsfMx4JCBp0l6wQ2Fj9DV1gdYuxKNq7Z4KA96o97Bn47WRiOgPNP0o9BsUG kOgP4I6hFuqdMskFksjkigk3Mixm5+TGfKnyZBVD36NZjnCt4uesh3dnVQOgtcFrcz7b XAJRfuyPuiJQWVoRQitu/ohOeOdMnzWs3mEknodAhYQpFeZ47MYJQ/FXaINyMKemX9ar IGl6BFj0Wandah0jsuNeQkntndI+V/rqu7ntLgo9lz9H0mWD8btQUiymQqajIcwffwTd Gmrg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721346206; x=1721951006; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=+zb2TnLEPy/yYn4OHpN9GAbJBX2HlvGglgTvnLaEwow=; b=nwwQzotqre2QUTBRCcUxUjtW4YRpU5Tnr9J8gktPFOYxgSQBsfhBrJd1jTF7IwP5Kk aHdfyt3ewiSz5WdKsAR63lHYCxOjhU1RFCj/6BovEPTrODu914ZT/OnJI9YdjCnZErfr z/c1sJRRab43KFdCOnvK394vsDosOo0fyhV29tVhFa2p/VttpdHV+1OAlrXciq4F3zKD OLSg9/YMrihxBJMXioGY3sKU5hbS+AnQ/C6eceiz+N9dqgYNNQEvrvIKZVmeXKzoor28 2tE71Sk7YUCGYvTYWEVRHGTUravBC52uqgNf8/xTO0bI46DSCLZr3jFzCoG8T9Yv49+b Cafg== X-Forwarded-Encrypted: i=1; AJvYcCUte36y+6ojq7rpRU6gDyycT+TauJ+cKim+Jfbbv6eO5D5ygKW1bJ5kY2Hl6u6bn+rJBmo/i+WmL7VloCDQhbnDejjo0JlYRQdTcdS9OrJEBqmVXmQ= X-Gm-Message-State: AOJu0YyhaqG8R9eSAw/B7fkKRcc0I2CJKqggewGVeFosGKEYz1CcwSLN O/xYVGd2kFC6RU6e6kFgqQ5g0qYZXS1ViBJAgLwBNb92zR4j5D/Z/7plwUlrdJM= X-Google-Smtp-Source: AGHT+IFF8smI3aMAQT3AbKX1NQGZpHaJaCkvum5gaxbfN2nJOzdqHbIsk7af5xBZTpN9HOnmw05rOw== X-Received: by 2002:a05:6512:ac7:b0:52b:9c8a:734f with SMTP id 2adb3069b0e04-52ee5411e3dmr5098927e87.50.1721346205884; Thu, 18 Jul 2024 16:43:25 -0700 (PDT) Received: from uffe-tuxpro14.. (h-178-174-189-39.A498.priv.bahnhof.se. [178.174.189.39]) by smtp.gmail.com with ESMTPSA id 2adb3069b0e04-52ef556b4fbsm22491e87.139.2024.07.18.16.43.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 Jul 2024 16:43:25 -0700 (PDT) From: Ulf Hansson To: Viresh Kumar , Nishanth Menon , Stephen Boyd Cc: Bjorn Andersson , Konrad Dybcio , Nikunj Kela , Prasad Sodagudi , Thierry Reding , Jonathan Hunter , Ulf Hansson , linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org Subject: [PATCH v2 1/6] OPP: Fix support for required OPPs for multiple PM domains Date: Fri, 19 Jul 2024 01:43:14 +0200 Message-Id: <20240718234319.356451-2-ulf.hansson@linaro.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240718234319.356451-1-ulf.hansson@linaro.org> References: <20240718234319.356451-1-ulf.hansson@linaro.org> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240718_164327_584241_50942A0C X-CRM114-Status: GOOD ( 29.76 ) 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 It has turned out that having _set_required_opps() to recursively call dev_pm_opp_set_opp() to set the required OPPs, doesn't really work as well as we expected. More precisely, at each recursive call to dev_pm_opp_set_opp() we are changing the OPP for a genpd's OPP table for a device that has been attached to it. The problem with this, is that we may have several devices being attached to the same genpd, thus sharing the same OPP-table that is being used for their required OPPs. So, typically we may have several active requests simultaneously for different OPPs for a genpd's OPP table. This may lead to that the per device vote for a performance-state (opp-level) for a genpd doesn't get requested accordingly. Moreover, dev_pm_opp_set_opp() doesn't get called for a required OPP when a device has been attached to a single PM domain. Even if a consumer driver would attempt to assign the required-devs, via _opp_attach_genpd() or _opp_set_required_devs() it would not be possible, as there is no separate virtual device at hand to use in this case. The above said, let's fix the problem by replacing the call to dev_pm_opp_set_opp() in _set_required_opps() by a call to _set_opp_level(). At the moment there's no drawback doing this, as there is no need to manage anything but the performance-state of the genpd. If it later turns out that another resource needs to be managed for a required-OPP, it can still be extended without having to call dev_pm_opp_set_opp(). Fixes: e37440e7e2c2 ("OPP: Call dev_pm_opp_set_opp() for required OPPs") Cc: stable@vger.kernel.org Signed-off-by: Ulf Hansson --- Changes in v2: - Clarified the commitmsg. - Addressed some comments from Viresh. - Drop calls to _add_opp_dev() for required_devs. --- drivers/opp/core.c | 56 ++++++++++++++++++---------------------------- 1 file changed, 22 insertions(+), 34 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 5f4598246a87..494f8860220d 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -1061,6 +1061,27 @@ static int _set_opp_bw(const struct opp_table *opp_table, return 0; } +static int _set_opp_level(struct device *dev, struct dev_pm_opp *opp) +{ + unsigned int level = 0; + int ret = 0; + + if (opp) { + if (opp->level == OPP_LEVEL_UNSET) + return 0; + + level = opp->level; + } + + /* Request a new performance state through the device's PM domain. */ + ret = dev_pm_domain_set_performance_state(dev, level); + if (ret) + dev_err(dev, "Failed to set performance state %u (%d)\n", level, + ret); + + return ret; +} + /* This is only called for PM domain for now */ static int _set_required_opps(struct device *dev, struct opp_table *opp_table, struct dev_pm_opp *opp, bool up) @@ -1091,7 +1112,7 @@ static int _set_required_opps(struct device *dev, struct opp_table *opp_table, if (devs[index]) { required_opp = opp ? opp->required_opps[index] : NULL; - ret = dev_pm_opp_set_opp(devs[index], required_opp); + ret = _set_opp_level(devs[index], required_opp); if (ret) return ret; } @@ -1102,27 +1123,6 @@ static int _set_required_opps(struct device *dev, struct opp_table *opp_table, return 0; } -static int _set_opp_level(struct device *dev, struct dev_pm_opp *opp) -{ - unsigned int level = 0; - int ret = 0; - - if (opp) { - if (opp->level == OPP_LEVEL_UNSET) - return 0; - - level = opp->level; - } - - /* Request a new performance state through the device's PM domain. */ - ret = dev_pm_domain_set_performance_state(dev, level); - if (ret) - dev_err(dev, "Failed to set performance state %u (%d)\n", level, - ret); - - return ret; -} - static void _find_current_opp(struct device *dev, struct opp_table *opp_table) { struct dev_pm_opp *opp = ERR_PTR(-ENODEV); @@ -2457,18 +2457,6 @@ static int _opp_attach_genpd(struct opp_table *opp_table, struct device *dev, } } - /* - * Add the virtual genpd device as a user of the OPP table, so - * we can call dev_pm_opp_set_opp() on it directly. - * - * This will be automatically removed when the OPP table is - * removed, don't need to handle that here. - */ - if (!_add_opp_dev(virt_dev, opp_table->required_opp_tables[index])) { - ret = -ENOMEM; - goto err; - } - opp_table->required_devs[index] = virt_dev; index++; name++;