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=-0.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no 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 2AE3FC2D0B1 for ; Thu, 6 Feb 2020 08:46:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E78CA217BA for ; Thu, 6 Feb 2020 08:46:12 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="pbRhghe/" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728079AbgBFIqM (ORCPT ); Thu, 6 Feb 2020 03:46:12 -0500 Received: from mail-vk1-f193.google.com ([209.85.221.193]:43306 "EHLO mail-vk1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727864AbgBFIqM (ORCPT ); Thu, 6 Feb 2020 03:46:12 -0500 Received: by mail-vk1-f193.google.com with SMTP id m195so1385134vkh.10 for ; Thu, 06 Feb 2020 00:46:10 -0800 (PST) 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=8SHAgobSEb546iBt5cNLjHLEI9u6dAs7lm7+/SJg7Jk=; b=pbRhghe/7yBE+QvA+2niPDdMyXYY6Uxi+o3Wd1l4gPLoxZLbexr+qrjM5w3ZoBCt71 uNDS8Nkfc/dCOTAbqlK5ZmkYXagP5tP0mwoqBLCpzy5mw3Zrd6Yszfho47x35dVGJIMF T7lfuzUC2HyXaoiJSSCiTovaSnpG0sTVbJUOkRbDo6wjoeJTjoRdEv6O4dISSBEqm4k/ 7eaw08U4iH2SVIe8cr/3jE7QnA7VEoatzdj2wy7igjBZ0nMLOHbzwuwkmSJR/pev96bN UTINzh3dw4ubVF2YD3m7ZZecM9W0CbXKJHw3zXH5OC5JyhUNglK1iD7WsOafjQms2sux jR2A== 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=8SHAgobSEb546iBt5cNLjHLEI9u6dAs7lm7+/SJg7Jk=; b=IZksu89mxBv35ITLjBV2Suv89f3DINLyLlvHyI5WrMBcT8OfMdvmUtQgvagWmUb2b1 4XuMCez6oYO+woXo2FL/1yQC1g6VURdLlzbeDosuF4JotAssQQm/qPx0U4IaVDaMS5IX P6Qoq1lXm0W3aMEPI88HSgnLcAGx2onATBE2GvLoxcsob3nXX+Ptrsum3WL1PDtmiP/g t53/Vf6tvOOkjUyREPcEc2cXNB2jMTZzL42SWGBS6IeNuojP6a98Y7oi38wh0KOyuu0x /dx5aw0jTgq7cSc/uaoGORCOUde/upybcF1CbZkEsqLF2s6bIxTTf8GAVSi5riERHC11 cTIQ== X-Gm-Message-State: APjAAAUTpwsL4HS98CvRkVHGYvOAp7PUyO/d9kU7bLUX+a6q7QPAi2ft lOBRyBO0rPxQJQORGJuDZt/XB3cgbfLv8pD0hu8CnQ== X-Google-Smtp-Source: APXvYqyhaBupFPdxR9GuF8DCLqL3bWPkA59GP7Uflc2DvigCdd8wZzZQzSTTp4q1qRVGG3M64NLke2IWZ0RBxoBZK38= X-Received: by 2002:ac5:c844:: with SMTP id g4mr1195612vkm.25.1580978770270; Thu, 06 Feb 2020 00:46:10 -0800 (PST) MIME-Version: 1.0 References: <1580736940-6985-1-git-send-email-mkshah@codeaurora.org> <1580736940-6985-6-git-send-email-mkshah@codeaurora.org> <20200203170832.GA38466@bogus> <0d7f7ade-3a1e-5428-d851-f1a886f58712@codeaurora.org> <20200204152132.GA44858@bogus> <6ff7c82d-4204-a339-4070-0154ab4515f1@codeaurora.org> <20200205140603.GB38466@bogus> <20200205161816.GD38466@bogus> In-Reply-To: <20200205161816.GD38466@bogus> From: Ulf Hansson Date: Thu, 6 Feb 2020 09:45:34 +0100 Message-ID: Subject: Re: [PATCH v3 5/7] drivers: firmware: psci: Add hierarchical domain idle states converter To: Sudeep Holla Cc: Maulik Shah , Stephen Boyd , Andy Gross , David Brown , Lorenzo Pieralisi , linux-arm-msm , Linux Kernel Mailing List , Linux PM , Linux ARM , Bjorn Andersson , Evan Green , Doug Anderson , Rajendra Nayak , Lina Iyer , lsrao@codeaurora.org, "Rafael J. Wysocki" 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 Wed, 5 Feb 2020 at 17:18, Sudeep Holla wrote: > > On Wed, Feb 05, 2020 at 04:55:17PM +0100, Ulf Hansson wrote: > > On Wed, 5 Feb 2020 at 15:06, Sudeep Holla wrote: > > > > > > On Wed, Feb 05, 2020 at 05:53:00PM +0530, Maulik Shah wrote: > > > > > > > > On 2/4/2020 8:51 PM, Sudeep Holla wrote: > > > > > On Tue, Feb 04, 2020 at 10:22:42AM +0530, Maulik Shah wrote: > > > > > > On 2/3/2020 10:38 PM, Sudeep Holla wrote: > > > > > > > On Mon, Feb 03, 2020 at 07:05:38PM +0530, Maulik Shah wrote: > > > > > > > > From: Ulf Hansson > > > > > > > > > > > > > > > > If the hierarchical CPU topology is used, but the OS initiated mode isn't > > > > > > > > supported, we need to rely solely on the regular cpuidle framework to > > > > > > > > manage the idle state selection, rather than using genpd and its > > > > > > > > governor. > > > > > > > > > > > > > > > > For this reason, introduce a new PSCI DT helper function, > > > > > > > > psci_dt_pm_domains_parse_states(), which parses and converts the > > > > > > > > hierarchically described domain idle states from DT, into regular flattened > > > > > > > > cpuidle states. The converted states are added to the existing cpuidle > > > > > > > > driver's array of idle states, which make them available for cpuidle. > > > > > > > > > > > > > > > And what's the main motivation for this if OSI is not supported in the > > > > > > > firmware ? > > > > > > Hi Sudeep, > > > > > > > > > > > > Main motivation is to do last-man activities before the CPU cluster can > > > > > > enter a deep idle state. > > > > > > > > > > > Details on those last-man activities will help the discussion. Basically > > > > > I am wondering what they are and why they need to done in OSPM ? > > > > > > > > Hi Sudeep, > > > > > > > > there are cases like, > > > > > > > > Last cpu going to deepest idle mode need to lower various resoruce > > > > requirements (for eg DDR freq). > > > > > > > > > > In PC mode, only PSCI implementation knows the last man and there shouldn't > > > be any notion of it in OS. If you need it, you may need OSI. You are still > > > mixing up the things. NACK for any such approach, sorry. > > > > Sudeep, I don't quite agree with your NACK to this. At least not yet. :-) > > > > OK, I am not surprised :-) Apologize for troubling you again. :-) > > > I do agree that the best suited solution seems to be OSI, as to > > support this kind of SoC requirements. > > > > That's the main point. We need to draw some line as what we want to do > with PC and OSI mode. If we plan to take up all last man responsibility > in the kernel, what's the point in not supporting OSI in the firmware > then ? I can't buy it yet. > > > However, if for some reason the PC mode is being used, we could still > > allow Linux to control "last-man activities" as it knows what each CPU > > has voted for when going idle. Yes, the PSCI FW decides in the end, > > but that doesn't really matter. Or is there another technical reason > > to why you object? > > > > Precisely, FW decides and let it. Just because we can do in the kernel > doesn't mean we must do it. It's clear in the spec and doing it in the > kernel will be sub-optimal if PSCI f/w aborted entering the deeper > state that required some action in the first place. Yes, it may be suboptimal for PC-mode. On the other hand, we already fire CPU PM notifiers while exit/enter idle states (except for WFI). Those may also be suboptimal for kind of the similar reasons. Maybe it's not the best argument, but it sounds like allowing us to control cluster power on/off notifications for last-man activities, would just conform to the similar behaviour we already have. No? > > > As a matter of fact, if we allow support for PC mode with > > "last-man-activities", it would allow us to make a fair > > performance/energy comparison between the two PSCI CPU suspend modes, > > for the same SoC. I would be thrilled about looking into doing such > > tests, I bet you are as well!? > > > > I was, but not anymore, especially if we want such changes in the kernel > to do so. > > Just use OSI as that was the point of adding all these after years of > discussion claiming it's more optimal compared to PC. Now telling that > you need more changes to compare it with PC just doesn't make any sense > at all to me. Fair enough. I was just pondering over if there are other reasons to why we may want this. One other thing that could be problematic to support, is when are other resources, I/O controllers for example, sharing the same power rail as a cluster. When such controller is in use, idle states of the cluster must be prevented. Without using genpd to model the CPU topology, it may be difficult to deal with this. Of course, using PC mode when trying to deal with this platform/board-requirement would also be suboptimal. In other words, your argument about when using OSI vs PC mode, still stands. 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=1.8 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,URIBL_DBL_ABUSE_MALW autolearn=no 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 8AADDC2D0B1 for ; Thu, 6 Feb 2020 08:46:20 +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 5E84121741 for ; Thu, 6 Feb 2020 08:46:20 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="kfL3Oasg"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="pbRhghe/" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5E84121741 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=FqIxgyXJkEb0w1b7kfRNkFDV3JWWC+PsSg+AT2bmP78=; b=kfL3OasghbL6Fr lzo0f34xokK6XwtqmXBN1mBQ9yVY+DiMzf97Nipm4LaVGNn7sxO/Ct1nL+CO4tjLgdbklrc37W67K L7oktELqkju9j454+n56irXKWsg8l8SsLUAz1jG4+QiOWOqhzJuNLrJry+jNhW3vSMm0blR9pG8Nf hoWzQd7pkd05912zrXlPZEj7Z93ic13EE6lN2h+4x/9aIvE7Ao8LUziB8+edWQbR6MPCi6T3vjStC 2D+LH3RIxt/XYB+lFpYxchu4ocfRCQL8NUPZyDD+v9aqy8cZWvYyfXc15T8ffFNv7GAfWRd3BRUMX 3zdVYKx/Impkyeq8peJg==; 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 1izcnf-0004pf-NS; Thu, 06 Feb 2020 08:46:15 +0000 Received: from mail-vk1-xa41.google.com ([2607:f8b0:4864:20::a41]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1izcnb-0004pD-Ix for linux-arm-kernel@lists.infradead.org; Thu, 06 Feb 2020 08:46:13 +0000 Received: by mail-vk1-xa41.google.com with SMTP id b69so1391219vke.9 for ; Thu, 06 Feb 2020 00:46:11 -0800 (PST) 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=8SHAgobSEb546iBt5cNLjHLEI9u6dAs7lm7+/SJg7Jk=; b=pbRhghe/7yBE+QvA+2niPDdMyXYY6Uxi+o3Wd1l4gPLoxZLbexr+qrjM5w3ZoBCt71 uNDS8Nkfc/dCOTAbqlK5ZmkYXagP5tP0mwoqBLCpzy5mw3Zrd6Yszfho47x35dVGJIMF T7lfuzUC2HyXaoiJSSCiTovaSnpG0sTVbJUOkRbDo6wjoeJTjoRdEv6O4dISSBEqm4k/ 7eaw08U4iH2SVIe8cr/3jE7QnA7VEoatzdj2wy7igjBZ0nMLOHbzwuwkmSJR/pev96bN UTINzh3dw4ubVF2YD3m7ZZecM9W0CbXKJHw3zXH5OC5JyhUNglK1iD7WsOafjQms2sux jR2A== 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=8SHAgobSEb546iBt5cNLjHLEI9u6dAs7lm7+/SJg7Jk=; b=jXfMKDzvAgEmOCI2+md9ysD3bvysvu4Mkm22x33aHKKWn4dOhHzwxjh04sXvYQ2YQE jJcVaSnG+CSl4KUhcFXhWA2lXeVHdvRdCJAtaIqaHcjnFJ+UoGJb+fI4+RcC5+UPBA3e 1Zn7xgnq+WpJWenfwBYXSI8sF0TIC5JmGMIP1x0OZQLLtoTaWGAMvc4bfqv6uWe0qK3w yggAUBRXKGPaIViQ5LWShZPxcL9/nY4HdsqaEF4ndf6jU/EOyEqL0fHzG38RYIXNiJxn 406J+1nfYeoRIak4gfaVCvUfrPXzm5jo3A8spmIcyYqvXeXjFVjlVd7SxDBuEzPkX63p QEMA== X-Gm-Message-State: APjAAAUzTPbsxJIPbgVQFv1fGadz6PFN8qhysZt5piA789IwvXrzMsxJ sS2xYj8yagge1H0uoRjiX1IQAuUG+fn4jtCq7NxJVg== X-Google-Smtp-Source: APXvYqyhaBupFPdxR9GuF8DCLqL3bWPkA59GP7Uflc2DvigCdd8wZzZQzSTTp4q1qRVGG3M64NLke2IWZ0RBxoBZK38= X-Received: by 2002:ac5:c844:: with SMTP id g4mr1195612vkm.25.1580978770270; Thu, 06 Feb 2020 00:46:10 -0800 (PST) MIME-Version: 1.0 References: <1580736940-6985-1-git-send-email-mkshah@codeaurora.org> <1580736940-6985-6-git-send-email-mkshah@codeaurora.org> <20200203170832.GA38466@bogus> <0d7f7ade-3a1e-5428-d851-f1a886f58712@codeaurora.org> <20200204152132.GA44858@bogus> <6ff7c82d-4204-a339-4070-0154ab4515f1@codeaurora.org> <20200205140603.GB38466@bogus> <20200205161816.GD38466@bogus> In-Reply-To: <20200205161816.GD38466@bogus> From: Ulf Hansson Date: Thu, 6 Feb 2020 09:45:34 +0100 Message-ID: Subject: Re: [PATCH v3 5/7] drivers: firmware: psci: Add hierarchical domain idle states converter To: Sudeep Holla X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200206_004611_688420_0B579E42 X-CRM114-Status: GOOD ( 33.38 ) 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: Maulik Shah , lsrao@codeaurora.org, Lorenzo Pieralisi , Rajendra Nayak , Linux PM , linux-arm-msm , "Rafael J. Wysocki" , Linux Kernel Mailing List , Evan Green , Stephen Boyd , David Brown , Andy Gross , Lina Iyer , Doug Anderson , Bjorn Andersson , 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 Wed, 5 Feb 2020 at 17:18, Sudeep Holla wrote: > > On Wed, Feb 05, 2020 at 04:55:17PM +0100, Ulf Hansson wrote: > > On Wed, 5 Feb 2020 at 15:06, Sudeep Holla wrote: > > > > > > On Wed, Feb 05, 2020 at 05:53:00PM +0530, Maulik Shah wrote: > > > > > > > > On 2/4/2020 8:51 PM, Sudeep Holla wrote: > > > > > On Tue, Feb 04, 2020 at 10:22:42AM +0530, Maulik Shah wrote: > > > > > > On 2/3/2020 10:38 PM, Sudeep Holla wrote: > > > > > > > On Mon, Feb 03, 2020 at 07:05:38PM +0530, Maulik Shah wrote: > > > > > > > > From: Ulf Hansson > > > > > > > > > > > > > > > > If the hierarchical CPU topology is used, but the OS initiated mode isn't > > > > > > > > supported, we need to rely solely on the regular cpuidle framework to > > > > > > > > manage the idle state selection, rather than using genpd and its > > > > > > > > governor. > > > > > > > > > > > > > > > > For this reason, introduce a new PSCI DT helper function, > > > > > > > > psci_dt_pm_domains_parse_states(), which parses and converts the > > > > > > > > hierarchically described domain idle states from DT, into regular flattened > > > > > > > > cpuidle states. The converted states are added to the existing cpuidle > > > > > > > > driver's array of idle states, which make them available for cpuidle. > > > > > > > > > > > > > > > And what's the main motivation for this if OSI is not supported in the > > > > > > > firmware ? > > > > > > Hi Sudeep, > > > > > > > > > > > > Main motivation is to do last-man activities before the CPU cluster can > > > > > > enter a deep idle state. > > > > > > > > > > > Details on those last-man activities will help the discussion. Basically > > > > > I am wondering what they are and why they need to done in OSPM ? > > > > > > > > Hi Sudeep, > > > > > > > > there are cases like, > > > > > > > > Last cpu going to deepest idle mode need to lower various resoruce > > > > requirements (for eg DDR freq). > > > > > > > > > > In PC mode, only PSCI implementation knows the last man and there shouldn't > > > be any notion of it in OS. If you need it, you may need OSI. You are still > > > mixing up the things. NACK for any such approach, sorry. > > > > Sudeep, I don't quite agree with your NACK to this. At least not yet. :-) > > > > OK, I am not surprised :-) Apologize for troubling you again. :-) > > > I do agree that the best suited solution seems to be OSI, as to > > support this kind of SoC requirements. > > > > That's the main point. We need to draw some line as what we want to do > with PC and OSI mode. If we plan to take up all last man responsibility > in the kernel, what's the point in not supporting OSI in the firmware > then ? I can't buy it yet. > > > However, if for some reason the PC mode is being used, we could still > > allow Linux to control "last-man activities" as it knows what each CPU > > has voted for when going idle. Yes, the PSCI FW decides in the end, > > but that doesn't really matter. Or is there another technical reason > > to why you object? > > > > Precisely, FW decides and let it. Just because we can do in the kernel > doesn't mean we must do it. It's clear in the spec and doing it in the > kernel will be sub-optimal if PSCI f/w aborted entering the deeper > state that required some action in the first place. Yes, it may be suboptimal for PC-mode. On the other hand, we already fire CPU PM notifiers while exit/enter idle states (except for WFI). Those may also be suboptimal for kind of the similar reasons. Maybe it's not the best argument, but it sounds like allowing us to control cluster power on/off notifications for last-man activities, would just conform to the similar behaviour we already have. No? > > > As a matter of fact, if we allow support for PC mode with > > "last-man-activities", it would allow us to make a fair > > performance/energy comparison between the two PSCI CPU suspend modes, > > for the same SoC. I would be thrilled about looking into doing such > > tests, I bet you are as well!? > > > > I was, but not anymore, especially if we want such changes in the kernel > to do so. > > Just use OSI as that was the point of adding all these after years of > discussion claiming it's more optimal compared to PC. Now telling that > you need more changes to compare it with PC just doesn't make any sense > at all to me. Fair enough. I was just pondering over if there are other reasons to why we may want this. One other thing that could be problematic to support, is when are other resources, I/O controllers for example, sharing the same power rail as a cluster. When such controller is in use, idle states of the cluster must be prevented. Without using genpd to model the CPU topology, it may be difficult to deal with this. Of course, using PC mode when trying to deal with this platform/board-requirement would also be suboptimal. In other words, your argument about when using OSI vs PC mode, still stands. Kind regards Uffe _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel