From patchwork Tue May 30 16:26:31 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicholas Piggin X-Patchwork-Id: 9754853 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id F1883602BF for ; Tue, 30 May 2017 16:26:49 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E3CBC26538 for ; Tue, 30 May 2017 16:26:49 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D851B27CEA; Tue, 30 May 2017 16:26:49 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 621D826538 for ; Tue, 30 May 2017 16:26:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751086AbdE3Q0p (ORCPT ); Tue, 30 May 2017 12:26:45 -0400 Received: from mail-pf0-f196.google.com ([209.85.192.196]:33406 "EHLO mail-pf0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751063AbdE3Q0n (ORCPT ); Tue, 30 May 2017 12:26:43 -0400 Received: by mail-pf0-f196.google.com with SMTP id f27so19019239pfe.0 for ; Tue, 30 May 2017 09:26:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=yoYAnUkWW2YkHH4MDW2kpDcGmf/sQycbzBRnyPve/+I=; b=Ylk+BtrKsDOrnG2OqThf3HGmTcq++ctGktgzve9wD38laD9xDjTikIb6y8Zc844HcH 1KpyiwAz+78IgZIirYJYpLZGhomzlGeY6PzCDoQcbVNwi0vOMgweO2G1ncl55wrEVtIT eieZPNkrATRYoVggm6w2U6ZrKpi8xdME65tc3fKzEN04kMg2znMNdoL3S2yBSTaBGnN8 eXJZuTNNcoEsr/hhbQ/1AH5a/+XumjApHRV7iFTwMLfx8w6YWvNAhCsJxTyyf1GFLhJr iuR4zW99IOL1fzWD7AaRFgkyTjfByAhofhUdmyFps/tyPUt+/t/Zc5AzWOfHCLCF9kke /54g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=yoYAnUkWW2YkHH4MDW2kpDcGmf/sQycbzBRnyPve/+I=; b=LM99hAq5iM3nmMF0xou2iOVVXo1xjP9vtarq7YgAsZ9EnrzBTODLMGTK/cPl73K9xz i0YiPHNfIzUMdarA8RzWR5N2mcfZ7gbGF6ichtWxv7sy3nSLP4hABsGNlMs3BCz06Hn2 VV4gQXP5pGyB7uPBdsxpVn4onnS2Pytl685WXiDNnwP/TjbhSGZPXxzp/jY/u8BWmYJR u5UpIH+ZT3bwspQIsT2R1UuHPVeKLhDT0SY1hccR8eWmXY5zC4gCEFPG3xr30FINzA9q Zx5mwOV2HKr0g3ogqkL8LXiHnfCNsyRZHD+XO8rR6TzZHWW6n76pFPP54eAlpvHk7BxC YZ8w== X-Gm-Message-State: AODbwcBZYyxP5QhY9ED9okEmO5baIXGnXc1EiO8DKPNqZzKGs0SSr3tm utTTPRG/lShLBg== X-Received: by 10.84.131.41 with SMTP id 38mr34065336pld.182.1496161602358; Tue, 30 May 2017 09:26:42 -0700 (PDT) Received: from roar.au.ibm.com (14-202-186-188.tpgi.com.au. [14.202.186.188]) by smtp.gmail.com with ESMTPSA id u67sm20376076pgb.24.2017.05.30.09.26.38 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 30 May 2017 09:26:41 -0700 (PDT) From: Nicholas Piggin To: "Rafael J. Wysocki" Cc: Nicholas Piggin , Daniel Lezcano , Vaidyanathan Srinivasan , linux-pm@vger.kernel.org Subject: [RFC] cpuidle: menu: nearby timer use lightest state; allow state 0 to be disabled Date: Wed, 31 May 2017 02:26:31 +1000 Message-Id: <20170530162631.9073-1-npiggin@gmail.com> X-Mailer: git-send-email 2.11.0 Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP I've found the menu driver does not allow state0 to properly be disabled because of all this poll logic selecting the first state and then trying to iterate over subsequent states. Ripping most of that out and simplifying it solved that issue but raised more questions about polling logic. Firstly polling logic is there only on architectures which define ARCH_HAS_CPU_RELAX, which is only x86. Seems like if we think a timer is so close that no powersave should be done, then surely just picking the lightest mode (whether that is polling or something else) would be best. But looking further into it, it seems maybe like some x86 hack (as the comments and changelog in 7884084f3bcc and subsequent attempts to work around Atom and broken firmware suggests). I would have thought such broken hard/firmware should get workarounds applied to fix the state values rather than add such logic? On the other hand, if (CPUIDLE_DRIVER_STATE_START > 0) is shorthand for if (x86 hacks), that's fine I'm happy to leave that alone and just work with the else parts... This is just a draft, but it does what I want at least with regard to disabling state0 for testing. Thanks, Nick --- drivers/cpuidle/governors/menu.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index b2330fd69e34..05de5d345ac7 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c @@ -286,6 +286,8 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev) struct device *device = get_cpu_device(dev->cpu); int latency_req = pm_qos_request(PM_QOS_CPU_DMA_LATENCY); int i; + int first_idx; + int idx; unsigned int interactivity_req; unsigned int expected_interval; unsigned long nr_iowaiters, cpu_load; @@ -318,7 +320,6 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev) data->predicted_us = DIV_ROUND_CLOSEST_ULL((uint64_t)data->next_timer_us * data->correction_factor[data->bucket], RESOLUTION * DECAY); - expected_interval = get_typical_interval(data); expected_interval = min(expected_interval, data->next_timer_us); @@ -327,19 +328,19 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev) unsigned int polling_threshold; /* - * We want to default to C1 (hlt), not to busy polling - * unless the timer is happening really really soon, or - * C1's exit latency exceeds the user configured limit. + * x86 wants to avoid busy polling unless the timer is + * happening really really soon, or C1's exit latency exceeds + * the uesr configured limit. */ polling_threshold = max_t(unsigned int, 20, s->target_residency); if (data->next_timer_us > polling_threshold && latency_req > s->exit_latency && !s->disabled && !dev->states_usage[CPUIDLE_DRIVER_STATE_START].disable) - data->last_state_idx = CPUIDLE_DRIVER_STATE_START; + first_idx = CPUIDLE_DRIVER_STATE_START; else - data->last_state_idx = CPUIDLE_DRIVER_STATE_START - 1; + first_idx = CPUIDLE_DRIVER_STATE_START - 1; } else { - data->last_state_idx = CPUIDLE_DRIVER_STATE_START; + first_idx = 0; } /* @@ -359,7 +360,8 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev) * Find the idle state with the lowest power while satisfying * our constraints. */ - for (i = data->last_state_idx + 1; i < drv->state_count; i++) { + idx = -1; + for (i = first_idx; i < drv->state_count; i++) { struct cpuidle_state *s = &drv->states[i]; struct cpuidle_state_usage *su = &dev->states_usage[i]; @@ -370,9 +372,14 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev) if (s->exit_latency > latency_req) break; - data->last_state_idx = i; + idx = i; } + if (idx == -1) /* no states */ + idx = 0; + + data->last_state_idx = idx; + return data->last_state_idx; }