From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Lezcano Subject: Re: [PATCH v7 2/6] ACPI / processor_idle: Add support for Low Power Idle(LPI) states Date: Fri, 1 Jul 2016 15:07:33 +0200 Message-ID: <57766B15.4090407@linaro.org> References: <1467122152-5604-1-git-send-email-sudeep.holla@arm.com> <1467122152-5604-3-git-send-email-sudeep.holla@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <1467122152-5604-3-git-send-email-sudeep.holla@arm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Sudeep Holla , linux-acpi@vger.kernel.org, "Rafael J . Wysocki" Cc: Lorenzo Pieralisi , Vincent Guittot , PrashanthPrakash , Al Stone , Vikas Sajjan , linux-kernel@vger.kernel.org, Ashwin Chaugule , linux-arm-kernel@lists.infradead.org, Sunil List-Id: linux-acpi@vger.kernel.org T24gMDYvMjgvMjAxNiAwMzo1NSBQTSwgU3VkZWVwIEhvbGxhIHdyb3RlOgo+IEFDUEkgNi4wIGlu dHJvZHVjZWQgYW4gb3B0aW9uYWwgb2JqZWN0IF9MUEkgdGhhdCBwcm92aWRlcyBhbiBhbHRlcm5h dGUKPiBtZXRob2QgdG8gZGVzY3JpYmUgTG93IFBvd2VyIElkbGUgc3RhdGVzLiBJdCBkZWZpbmVz IHRoZSBsb2NhbCBwb3dlcgo+IHN0YXRlcyBmb3IgZWFjaCBub2RlIGluIGEgaGllcmFyY2hpY2Fs IHByb2Nlc3NvciB0b3BvbG9neS4gVGhlIE9TUE0gY2FuCj4gdXNlIF9MUEkgb2JqZWN0IHRvIHNl bGVjdCBhIGxvY2FsIHBvd2VyIHN0YXRlIGZvciBlYWNoIGxldmVsIG9mIHByb2Nlc3Nvcgo+IGhp ZXJhcmNoeSBpbiB0aGUgc3lzdGVtLiBUaGV5IHVzZWQgdG8gcHJvZHVjZSBhIGNvbXBvc2l0ZSBw b3dlciBzdGF0ZQo+IHJlcXVlc3QgdGhhdCBpcyBwcmVzZW50ZWQgdG8gdGhlIHBsYXRmb3JtIGJ5 IHRoZSBPU1BNLgo+Cj4gU2luY2UgbXVsdGlwbGUgcHJvY2Vzc29ycyBhZmZlY3QgdGhlIGlkbGUg c3RhdGUgZm9yIGFueSBub24tbGVhZiBoaWVyYXJjaHkKPiBub2RlLCBjb29yZGluYXRpb24gb2Yg aWRsZSBzdGF0ZSByZXF1ZXN0cyBiZXR3ZWVuIHRoZSBwcm9jZXNzb3JzIGlzCj4gcmVxdWlyZWQu IEFDUEkgc3VwcG9ydHMgdHdvIGRpZmZlcmVudCBjb29yZGluYXRpb24gc2NoZW1lczogUGxhdGZv cm0KPiBjb29yZGluYXRlZCBhbmQgIE9TIGluaXRpYXRlZC4KPgo+IFRoaXMgcGF0Y2ggYWRkcyBp bml0aWFsIHN1cHBvcnQgZm9yIFBsYXRmb3JtIGNvb3JkaW5hdGlvbiBzY2hlbWUgb2YgTFBJLgo+ Cj4gQ2M6ICJSYWZhZWwgSi4gV3lzb2NraSIgPHJqd0Byand5c29ja2kubmV0Pgo+IFNpZ25lZC1v ZmYtYnk6IFN1ZGVlcCBIb2xsYSA8c3VkZWVwLmhvbGxhQGFybS5jb20+Cj4gLS0tCgpIaSBTdWRl ZXAsCgpJIGxvb2tlZCBhdCB0aGUgYWNwaSBwcm9jZXNzb3IgaWRsZSBjb2RlIHNvbWV0aW1lIGFn byBhbmQgZnJvbSBteSBQT1YsIAppdCB3YXMgYXdmdWwsIHVubmVjZXNzYXJ5IGNvbXBsZXggYW5k IHZlcnkgZGlmZmljdWx0IHRvIG1haW50YWluLiBUaGUgCnVzYWdlIG9mIGZsYWdzIGFsbCBvdmVy IHRoZSBwbGFjZXMgaXMgc2lnbmlmaWNhbnQgb2YgdGhlIGxhY2sgb2YgY29udHJvbCAKb2YgdGhl IHdyaXR0ZW4gY29kZS4KCkV2ZW4gaWYgeW91IGFyZSBub3QgcmVzcG9uc2libGUgb2YgdGhpcyBp bXBsZW1lbnRhdGlvbiwgdGhlIGN1cnJlbnQgCnNpdHVhdGlvbiBmb3JjZXMgeW91IHRvIGFkZCBt b3JlIGF3ZnVsIGNvZGUgb24gdG9wIG9mIHRoYXQsIHdoaWNoIGlzIApjbGVhcmx5IGFnYWluc3Qg Im1ha2luZyBMaW51eCBiZXR0ZXIiLgoKSU1PLCB0aGUgY3VycmVudCBjb2RlIGRlc2VydmVzIGEg aHVnZSBjbGVhbnVwIGJlZm9yZSBhcHBseWluZyBhbnl0aGluZyAKbmV3IDogY3N0YXRlIGFuZCBs cGkgc2hvdWxkIGJlIGludmVzdGlnYXRlZCB0byBiZSBzZWxmLWNvbnRhaW5lZCBpbiAKdGhlaXIg cmVzcGVjdGl2ZSBmaWxlIGFuZCBjb25zb2xpZGF0ZWQsIHRoZSBnbG9iYWwgdmFyaWFibGUgdXNh Z2Ugc2hvdWxkIApiZSBraWxsZWQsIHJlZHVuZGFudCBmbGFnIGNoZWNraW5nIHJlbW92ZWQgYnkg cmVjYXB0dXJpbmcgdGhlIGNvZGUgZmxvdywgCmV0YyAuLi4gSSBiZWxpZXZlIHRoZSB1c2FnZSBv ZiBhY3BpIHByb2JlIHRhYmxlIGNvdWxkIGJlIG9uZSBlbnRyeSBwb2ludCAKdG8gYmVnaW4gdGhp cyBjbGVhbnVwLgoKCgoKLS0gCiAgPGh0dHA6Ly93d3cubGluYXJvLm9yZy8+IExpbmFyby5vcmcg 4pSCIE9wZW4gc291cmNlIHNvZnR3YXJlIGZvciBBUk0gU29DcwoKRm9sbG93IExpbmFybzogIDxo dHRwOi8vd3d3LmZhY2Vib29rLmNvbS9wYWdlcy9MaW5hcm8+IEZhY2Vib29rIHwKPGh0dHA6Ly90 d2l0dGVyLmNvbS8jIS9saW5hcm9vcmc+IFR3aXR0ZXIgfAo8aHR0cDovL3d3dy5saW5hcm8ub3Jn L2xpbmFyby1ibG9nLz4gQmxvZwoKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fCmxpbnV4LWFybS1rZXJuZWwgbWFpbGluZyBsaXN0CmxpbnV4LWFybS1rZXJu ZWxAbGlzdHMuaW5mcmFkZWFkLm9yZwpodHRwOi8vbGlzdHMuaW5mcmFkZWFkLm9yZy9tYWlsbWFu L2xpc3RpbmZvL2xpbnV4LWFybS1rZXJuZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 From: daniel.lezcano@linaro.org (Daniel Lezcano) Date: Fri, 1 Jul 2016 15:07:33 +0200 Subject: [PATCH v7 2/6] ACPI / processor_idle: Add support for Low Power Idle(LPI) states In-Reply-To: <1467122152-5604-3-git-send-email-sudeep.holla@arm.com> References: <1467122152-5604-1-git-send-email-sudeep.holla@arm.com> <1467122152-5604-3-git-send-email-sudeep.holla@arm.com> Message-ID: <57766B15.4090407@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 06/28/2016 03:55 PM, Sudeep Holla wrote: > ACPI 6.0 introduced an optional object _LPI that provides an alternate > method to describe Low Power Idle states. It defines the local power > states for each node in a hierarchical processor topology. The OSPM can > use _LPI object to select a local power state for each level of processor > hierarchy in the system. They used to produce a composite power state > request that is presented to the platform by the OSPM. > > Since multiple processors affect the idle state for any non-leaf hierarchy > node, coordination of idle state requests between the processors is > required. ACPI supports two different coordination schemes: Platform > coordinated and OS initiated. > > This patch adds initial support for Platform coordination scheme of LPI. > > Cc: "Rafael J. Wysocki" > Signed-off-by: Sudeep Holla > --- Hi Sudeep, I looked at the acpi processor idle code sometime ago and from my POV, it was awful, unnecessary complex and very difficult to maintain. The usage of flags all over the places is significant of the lack of control of the written code. Even if you are not responsible of this implementation, the current situation forces you to add more awful code on top of that, which is clearly against "making Linux better". IMO, the current code deserves a huge cleanup before applying anything new : cstate and lpi should be investigated to be self-contained in their respective file and consolidated, the global variable usage should be killed, redundant flag checking removed by recapturing the code flow, etc ... I believe the usage of acpi probe table could be one entry point to begin this cleanup. -- Linaro.org ? Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752549AbcGANM6 (ORCPT ); Fri, 1 Jul 2016 09:12:58 -0400 Received: from mail-wm0-f52.google.com ([74.125.82.52]:37214 "EHLO mail-wm0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750803AbcGANM4 (ORCPT ); Fri, 1 Jul 2016 09:12:56 -0400 Subject: Re: [PATCH v7 2/6] ACPI / processor_idle: Add support for Low Power Idle(LPI) states To: Sudeep Holla , linux-acpi@vger.kernel.org, "Rafael J . Wysocki" References: <1467122152-5604-1-git-send-email-sudeep.holla@arm.com> <1467122152-5604-3-git-send-email-sudeep.holla@arm.com> Cc: Vikas Sajjan , Sunil , Lorenzo Pieralisi , PrashanthPrakash , Al Stone , Ashwin Chaugule , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Vincent Guittot From: Daniel Lezcano Message-ID: <57766B15.4090407@linaro.org> Date: Fri, 1 Jul 2016 15:07:33 +0200 User-Agent: Mozilla/5.0 (X11; Linux i686; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <1467122152-5604-3-git-send-email-sudeep.holla@arm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/28/2016 03:55 PM, Sudeep Holla wrote: > ACPI 6.0 introduced an optional object _LPI that provides an alternate > method to describe Low Power Idle states. It defines the local power > states for each node in a hierarchical processor topology. The OSPM can > use _LPI object to select a local power state for each level of processor > hierarchy in the system. They used to produce a composite power state > request that is presented to the platform by the OSPM. > > Since multiple processors affect the idle state for any non-leaf hierarchy > node, coordination of idle state requests between the processors is > required. ACPI supports two different coordination schemes: Platform > coordinated and OS initiated. > > This patch adds initial support for Platform coordination scheme of LPI. > > Cc: "Rafael J. Wysocki" > Signed-off-by: Sudeep Holla > --- Hi Sudeep, I looked at the acpi processor idle code sometime ago and from my POV, it was awful, unnecessary complex and very difficult to maintain. The usage of flags all over the places is significant of the lack of control of the written code. Even if you are not responsible of this implementation, the current situation forces you to add more awful code on top of that, which is clearly against "making Linux better". IMO, the current code deserves a huge cleanup before applying anything new : cstate and lpi should be investigated to be self-contained in their respective file and consolidated, the global variable usage should be killed, redundant flag checking removed by recapturing the code flow, etc ... I believe the usage of acpi probe table could be one entry point to begin this cleanup. -- Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog