From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 60C01CA9EC0 for ; Mon, 28 Oct 2019 22:40:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1C90B21835 for ; Mon, 28 Oct 2019 22:40:54 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="uoJWfdHB" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729037AbfJ1Wkx (ORCPT ); Mon, 28 Oct 2019 18:40:53 -0400 Received: from mail-vs1-f65.google.com ([209.85.217.65]:45144 "EHLO mail-vs1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726175AbfJ1Wkx (ORCPT ); Mon, 28 Oct 2019 18:40:53 -0400 Received: by mail-vs1-f65.google.com with SMTP id l5so7443744vsh.12 for ; Mon, 28 Oct 2019 15:40:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=LbHWlc0+nebJGqRHkft0So8Qf87tbPWPZEk2OCB5L7k=; b=uoJWfdHBg4e2UJvDhf80dB6X0XM296yPXsBBn8t1aMMhnV0Pg6grSjQP1tA43WMskh XQL+YEgY9PWdYNA1/44uXNmZpLiQr74EbBMO0kd4dmU5U+IMpLd1+M+CgII+vKGdSZIH EDAKM20nlfrWO8zLEv5ssqfvicW1SKpoIuDbY7b2aCpX5r7Gz5u0Kl1CCXQKITvU3wRY LnXwkWgTpdCHvBd5UHWQAv9i0MUNv5KXskdLGafGW4I8h6IZ7tRji4sYIqBgdXXAhzlO NoHAqlkiHMqh86odCWrKAgptg42lB75m5wUj+Ri3GHzOgBlnKT6RxbEoY9O/Z0/Hyt0U P8FA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=LbHWlc0+nebJGqRHkft0So8Qf87tbPWPZEk2OCB5L7k=; b=Z7LF/8ytcYx8JEKJunj1kR0vewLurP8Tnn6jNE43RpA+WC2soA/g9Ut0SZzlxAf10s Gz4RQPDd5tmyjXwpVBEIBCMqzjWfWux/0fR+3DzPFcKM3QYSH3qb5LH49xGLiQV4iRV8 GULf9b5zNulDWnd9FA9/JtRWAr+R9mzx//KHTP0bymLRjjaCAtW0SF00d4trC7hyzMYa gMzKt6s9Xd/xlsGIO73I0uqU5FY1LTTD+G2Fxln5HGA78oW/Agsx1c1dQWq3Rw4DKtEA TOyaE3Vl8Di59g0dGP8aU6jQjxklA9p2TmMT0rmcHFiSFGM9UZHcYgoE1gxMl8gwQPy4 GaYg== X-Gm-Message-State: APjAAAVJsWVHos4G7aFOJJtMepBUkrIZKLdNAPTcRbNF6zlI+52Law2B v0H2OcCA1iTdIjNl8MFDoJz/Jx72noM9maWb9se7Jw== X-Google-Smtp-Source: APXvYqwvgrSKNvgD0SzVjUsiTobckOY2eSt0wr5gmUCcZBLHvL1zQq+ew7oqw1cZhVYfkSqkHRRumj6qS7VkI+zjNOo= X-Received: by 2002:a67:fb5a:: with SMTP id e26mr6327260vsr.200.1572302452026; Mon, 28 Oct 2019 15:40:52 -0700 (PDT) MIME-Version: 1.0 References: <20191010113937.15962-1-ulf.hansson@linaro.org> <20191010113937.15962-13-ulf.hansson@linaro.org> <20191024163257.GC22036@bogus> <20191027023414.GE18111@e107533-lin.cambridge.arm.com> In-Reply-To: <20191027023414.GE18111@e107533-lin.cambridge.arm.com> From: Ulf Hansson Date: Mon, 28 Oct 2019 23:40:16 +0100 Message-ID: Subject: Re: [PATCH 12/13] cpuidle: psci: Manage runtime PM in the idle path To: Sudeep Holla Cc: "Rafael J . Wysocki" , Daniel Lezcano , Lorenzo Pieralisi , Mark Rutland , Lina Iyer , Linux PM , Rob Herring , Vincent Guittot , Stephen Boyd , Bjorn Andersson , Kevin Hilman , Linux ARM , linux-arm-msm Content-Type: text/plain; charset="UTF-8" Sender: linux-arm-msm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org On Sun, 27 Oct 2019 at 03:34, Sudeep Holla wrote: > > On Thu, Oct 24, 2019 at 07:00:38PM +0200, Ulf Hansson wrote: > > On Thu, 24 Oct 2019 at 18:33, Sudeep Holla wrote: > > > > > > On Thu, Oct 10, 2019 at 01:39:36PM +0200, Ulf Hansson wrote: > > > > In case we have succeeded to attach a CPU to its PM domain, let's deploy > > > > runtime PM support for the corresponding attached device, to allow the CPU > > > > to be powered-managed accordingly. > > > > > > > > To set the triggering point for when runtime PM reference counting should > > > > be done, let's store the index of deepest idle state for the CPU in the per > > > > CPU struct. Then use this index to compare the selected idle state index > > > > when entering idle, as to understand whether runtime PM reference counting > > > > is needed or not. > > > > > > > > Note that, from the hierarchical point view, there may be good reasons to > > > > do runtime PM reference counting even on shallower idle states, but at this > > > > point this isn't supported, mainly due to limitations set by the generic PM > > > > domain. > > > > > > > > Signed-off-by: Ulf Hansson > > > > --- > > > > drivers/cpuidle/cpuidle-psci.c | 21 +++++++++++++++++++-- > > > > 1 file changed, 19 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/cpuidle/cpuidle-psci.c b/drivers/cpuidle/cpuidle-psci.c > > > > index 1510422c7a53..0919b40c1a85 100644 > > > > --- a/drivers/cpuidle/cpuidle-psci.c > > > > +++ b/drivers/cpuidle/cpuidle-psci.c > > > > @@ -16,6 +16,7 @@ > > > > #include > > > > #include > > > > #include > > > > +#include > > > > #include > > > > > > > > #include > > > > @@ -25,6 +26,7 @@ > > > > > > > > struct psci_cpuidle_data { > > > > u32 *psci_states; > > > > + u32 rpm_state_id; > > > > struct device *dev; > > > > }; > > > > > > > > @@ -50,14 +52,28 @@ static int psci_enter_idle_state(struct cpuidle_device *dev, > > > > struct cpuidle_driver *drv, int idx) > > > > { > > > > int ret; > > > > - u32 *states = __this_cpu_read(psci_cpuidle_data.psci_states); > > > > - u32 state = psci_get_domain_state(); > > > > + struct psci_cpuidle_data *data = this_cpu_ptr(&psci_cpuidle_data); > > > > + u32 *states = data->psci_states; > > > > + struct device *pd_dev = data->dev; > > > > + bool runtime_pm = (pd_dev && data->rpm_state_id == idx); > > > > + u32 state; > > > > > > Wonder if we can have separate psci_enter_idle_state for OSI mode so > > > that all these runtime extra check can be reduced ? It will also make > > > sure there's no additional latency for PC mode because of OSI. > > > > Good idea, that's the plan. See previous answer. > > > > Perhaps if I add a patch on top, implementing your suggestion, would > > you be happy with that? > > No, I prefer to amend this itself to keep it easy to be able to bisect. Alright! So I explored this a little bit more - and it actually forced me to re-order some of the patches in the series, but it seems to have turned out well. In the new approach I have taken, I haven't replaced the actual callback for the idle state, but instead make an early decision in psci_enter_idle_state(), based on one single read of a per CPU variable/struct. This tell what path to go. I am running some final test, but should be able to post a new version tomorrow. Although, if you still don't think the new approach is good enough, we can always invent a callback, that we can assign when the CPU is attached to PM domain. In any case, you will have to tell what you think, after I posted the new version, just wanted to give you a heads up. Kind regards Uffe From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B3DF7CA9EC0 for ; Mon, 28 Oct 2019 22:41:04 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 7C23621835 for ; Mon, 28 Oct 2019 22:41:04 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="nMVZBxFy"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="uoJWfdHB" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7C23621835 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=8wvjm797EK4WdCHC+rNHpZO4Vm0PakSpg2QqheQs5xw=; b=nMVZBxFyEKH02W xiRaQns24o6nrjkjc5VDQFDDui7CNlFo/NX0pF6UFeCAOYTY3mEd55C+shdpLU/vrA5C3hbyFcjDN 7fk9mJM39hEHQ2c4bwZ/Tf/XFP7rW14SZuG+MtkmxPDRya6363JBUEFEp5rOgz0KYojCXJ2uSBaCm GC+by7NhR2ruGM4Yn5hTt1yCtEkNtM6U/tPoW5eXlC7N/OClVBUfxuImS6FWyBbaPOW9z3viPT462 icwAzFOpNrGa7BGRFn8ipAHSBtN2KYYQcQb0kDJqyjeNN6EyOVYn/nGURzXQrdJ2YqA0xa9YaoSgR wXr6jjQwsHVxujV+77rQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1iPDh2-0003YR-OS; Mon, 28 Oct 2019 22:40:56 +0000 Received: from mail-vs1-xe43.google.com ([2607:f8b0:4864:20::e43]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1iPDgz-0003Xv-Hk for linux-arm-kernel@lists.infradead.org; Mon, 28 Oct 2019 22:40:55 +0000 Received: by mail-vs1-xe43.google.com with SMTP id f8so1326520vsg.1 for ; Mon, 28 Oct 2019 15:40:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=LbHWlc0+nebJGqRHkft0So8Qf87tbPWPZEk2OCB5L7k=; b=uoJWfdHBg4e2UJvDhf80dB6X0XM296yPXsBBn8t1aMMhnV0Pg6grSjQP1tA43WMskh XQL+YEgY9PWdYNA1/44uXNmZpLiQr74EbBMO0kd4dmU5U+IMpLd1+M+CgII+vKGdSZIH EDAKM20nlfrWO8zLEv5ssqfvicW1SKpoIuDbY7b2aCpX5r7Gz5u0Kl1CCXQKITvU3wRY LnXwkWgTpdCHvBd5UHWQAv9i0MUNv5KXskdLGafGW4I8h6IZ7tRji4sYIqBgdXXAhzlO NoHAqlkiHMqh86odCWrKAgptg42lB75m5wUj+Ri3GHzOgBlnKT6RxbEoY9O/Z0/Hyt0U P8FA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=LbHWlc0+nebJGqRHkft0So8Qf87tbPWPZEk2OCB5L7k=; b=Ynj9DiGgeSGpqvysPhy7c/AkjuqoH9h2TqB5fAI4fAp+2O+5JNlJ8lYqCmTeGYkK5U CIRJI/lNeYYs5XaEh1LTfTTvdVl0Pu1nZYiwevXX4u+jaF2fvUOYZLdSRWq18nAzemAj oJbZea6GamS0C8kTkk9acb/ERa3/IpvyIn92r4gZe1Lb5JAUeEs29m0eIWrU9MAdOYXu oR51pwX/N3Qb29CpjAcYu8UtFjQUmd6aywetCBZq4Bzdprh2xjVwIsri5utlTi49YZI6 aj3psWuW/oiLmAQsmU15sOAdHjfcmDEtjBnYX+i/oFFu8B3HGLkE32ZH9GpNIsfWgiPd I/nw== X-Gm-Message-State: APjAAAVu0J7Is/4p9466TlEbvFKzTLjImk687mNwILe2/baipIG7gwxG OX9Yxh/lx2XoNNpvV08dxfhEWCdWBDB8l6WXp9ITDA== X-Google-Smtp-Source: APXvYqwvgrSKNvgD0SzVjUsiTobckOY2eSt0wr5gmUCcZBLHvL1zQq+ew7oqw1cZhVYfkSqkHRRumj6qS7VkI+zjNOo= X-Received: by 2002:a67:fb5a:: with SMTP id e26mr6327260vsr.200.1572302452026; Mon, 28 Oct 2019 15:40:52 -0700 (PDT) MIME-Version: 1.0 References: <20191010113937.15962-1-ulf.hansson@linaro.org> <20191010113937.15962-13-ulf.hansson@linaro.org> <20191024163257.GC22036@bogus> <20191027023414.GE18111@e107533-lin.cambridge.arm.com> In-Reply-To: <20191027023414.GE18111@e107533-lin.cambridge.arm.com> From: Ulf Hansson Date: Mon, 28 Oct 2019 23:40:16 +0100 Message-ID: Subject: Re: [PATCH 12/13] cpuidle: psci: Manage runtime PM in the idle path To: Sudeep Holla X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20191028_154053_615583_8992614B X-CRM114-Status: GOOD ( 28.49 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , Lorenzo Pieralisi , Linux PM , Stephen Boyd , linux-arm-msm , Daniel Lezcano , "Rafael J . Wysocki" , Lina Iyer , Bjorn Andersson , Kevin Hilman , Rob Herring , Linux ARM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Sun, 27 Oct 2019 at 03:34, Sudeep Holla wrote: > > On Thu, Oct 24, 2019 at 07:00:38PM +0200, Ulf Hansson wrote: > > On Thu, 24 Oct 2019 at 18:33, Sudeep Holla wrote: > > > > > > On Thu, Oct 10, 2019 at 01:39:36PM +0200, Ulf Hansson wrote: > > > > In case we have succeeded to attach a CPU to its PM domain, let's deploy > > > > runtime PM support for the corresponding attached device, to allow the CPU > > > > to be powered-managed accordingly. > > > > > > > > To set the triggering point for when runtime PM reference counting should > > > > be done, let's store the index of deepest idle state for the CPU in the per > > > > CPU struct. Then use this index to compare the selected idle state index > > > > when entering idle, as to understand whether runtime PM reference counting > > > > is needed or not. > > > > > > > > Note that, from the hierarchical point view, there may be good reasons to > > > > do runtime PM reference counting even on shallower idle states, but at this > > > > point this isn't supported, mainly due to limitations set by the generic PM > > > > domain. > > > > > > > > Signed-off-by: Ulf Hansson > > > > --- > > > > drivers/cpuidle/cpuidle-psci.c | 21 +++++++++++++++++++-- > > > > 1 file changed, 19 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/cpuidle/cpuidle-psci.c b/drivers/cpuidle/cpuidle-psci.c > > > > index 1510422c7a53..0919b40c1a85 100644 > > > > --- a/drivers/cpuidle/cpuidle-psci.c > > > > +++ b/drivers/cpuidle/cpuidle-psci.c > > > > @@ -16,6 +16,7 @@ > > > > #include > > > > #include > > > > #include > > > > +#include > > > > #include > > > > > > > > #include > > > > @@ -25,6 +26,7 @@ > > > > > > > > struct psci_cpuidle_data { > > > > u32 *psci_states; > > > > + u32 rpm_state_id; > > > > struct device *dev; > > > > }; > > > > > > > > @@ -50,14 +52,28 @@ static int psci_enter_idle_state(struct cpuidle_device *dev, > > > > struct cpuidle_driver *drv, int idx) > > > > { > > > > int ret; > > > > - u32 *states = __this_cpu_read(psci_cpuidle_data.psci_states); > > > > - u32 state = psci_get_domain_state(); > > > > + struct psci_cpuidle_data *data = this_cpu_ptr(&psci_cpuidle_data); > > > > + u32 *states = data->psci_states; > > > > + struct device *pd_dev = data->dev; > > > > + bool runtime_pm = (pd_dev && data->rpm_state_id == idx); > > > > + u32 state; > > > > > > Wonder if we can have separate psci_enter_idle_state for OSI mode so > > > that all these runtime extra check can be reduced ? It will also make > > > sure there's no additional latency for PC mode because of OSI. > > > > Good idea, that's the plan. See previous answer. > > > > Perhaps if I add a patch on top, implementing your suggestion, would > > you be happy with that? > > No, I prefer to amend this itself to keep it easy to be able to bisect. Alright! So I explored this a little bit more - and it actually forced me to re-order some of the patches in the series, but it seems to have turned out well. In the new approach I have taken, I haven't replaced the actual callback for the idle state, but instead make an early decision in psci_enter_idle_state(), based on one single read of a per CPU variable/struct. This tell what path to go. I am running some final test, but should be able to post a new version tomorrow. Although, if you still don't think the new approach is good enough, we can always invent a callback, that we can assign when the CPU is attached to PM domain. In any case, you will have to tell what you think, after I posted the new version, just wanted to give you a heads up. Kind regards Uffe _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel