From patchwork Fri Sep 5 13:41:05 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Abraham X-Patchwork-Id: 4852121 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 8EADCC033A for ; Fri, 5 Sep 2014 13:46:56 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 14FF620268 for ; Fri, 5 Sep 2014 13:46:55 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 63B352026D for ; Fri, 5 Sep 2014 13:46:49 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1XPtlk-00022s-Ae; Fri, 05 Sep 2014 13:41:40 +0000 Received: from mail-lb0-f178.google.com ([209.85.217.178]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1XPtlh-0001yi-1g for linux-arm-kernel@lists.infradead.org; Fri, 05 Sep 2014 13:41:39 +0000 Received: by mail-lb0-f178.google.com with SMTP id v6so13428761lbi.23 for ; Fri, 05 Sep 2014 06:41:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=u83pIvl4SSOl91CazkDLIm619v5TJKGWGLIcYPNbhhM=; b=apLqzx5BUhU4wPxH0O8R6d65bSlyQgAsuIYj8uU8rZZIsFY8Q4a0ApZJU3g/8ATbAx LM32aS7HQxwgloWzMMT14/miqlDPa0U5qB8EoSQ2RNpuJTipaxa8I/I0fn2hXzbOYb5r TGuh25aVdl1oDxWuW0+8Nzr1pzSW56uh7PyWsvDcsXQNR6e0KY+kXzFy8q0tNmwwHqhr Z/19JPgFufFaeNxAX9fvotayiuNVDOV2vWmbh38VIoh7WS133HgufE04xOHP6M9Eg80U RqTRVwWCzCR+Z4XEr+/ahree69hRsK4aNSjgyqi1664BTSq1RFGTFNbsq3F2RvQ2sLR/ PuMQ== MIME-Version: 1.0 X-Received: by 10.152.87.82 with SMTP id v18mr11959424laz.83.1409924465795; Fri, 05 Sep 2014 06:41:05 -0700 (PDT) Received: by 10.114.69.78 with HTTP; Fri, 5 Sep 2014 06:41:05 -0700 (PDT) In-Reply-To: <7hfvg74ioq.fsf@paris.lan> References: <1406707663-16656-1-git-send-email-thomas.ab@samsung.com> <1406707663-16656-5-git-send-email-thomas.ab@samsung.com> <53DA8BB9.6020702@samsung.com> <53DA8D85.3050106@gmail.com> <7hbnrc6px3.fsf@paris.lan> <53F7DA13.6090305@gmail.com> <7h8umc60we.fsf@paris.lan> <7hmwah6cop.fsf@paris.lan> <7hsik847p2.fsf@paris.lan> <7hfvg74ioq.fsf@paris.lan> Date: Fri, 5 Sep 2014 19:11:05 +0530 Message-ID: Subject: Re: [PATCH v9 4/6] ARM: Exynos: switch to using generic cpufreq driver for Exynos4210/5250/5420 From: Thomas Abraham To: Kevin Hilman X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20140905_064137_469039_FBE006DD X-CRM114-Status: GOOD ( 30.33 ) X-Spam-Score: -0.8 (/) Cc: Chander Kashyap , Lukasz Majewski , Kukjin Kim , Mike Turquette , =?UTF-8?Q?Heiko_St=C3=BCbner?= , "linux-pm@vger.kernel.org" , Viresh Kumar , Bartlomiej Zolnierkiewicz , Tomasz Figa , Amit Kucheria , Chanwoo Choi , "linaro-kernel@lists.linaro.org" , "linux-samsung-soc@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , Marek Szyprowski X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-3.5 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Sep 4, 2014 at 7:00 PM, Kevin Hilman wrote: > Thomas Abraham writes: > >> On Thu, Sep 4, 2014 at 4:45 AM, Kevin Hilman wrote: >>> Hi Thomas, >>> >>> Thomas Abraham writes: >>> >>> [...] >>> >>>> A new branch [1] has been created using commits from exynos5-v3.17-rc1 >>>> branch + cpufreq + regulator + temp fixes. I have tested this branch >>>> on Exynos5800 Chromebook2 and cpufreq works fine with both ondemand >>>> and performance governors. Please let me know if there are any issues >>>> with this new branch. It is based on v3.17-rc3. >>> >>> Excellent! Thank you. >>> >>> The only thing missing now is the CPUidle support for 5800, and all >>> that's needed for that is the compatible string patch[1] which Daniel >>> has queued up. >>> >>> With that patch, display + CPUidle + CPUfreq are working pretty well on >>> my exynos5800/chromebook2 with the big.LITTLE switcher disabled. If I >>> turn on the switcher, it boots OK, but as soon as I try to run powertop >>> (upstream v2.6.1) it gets stuck. Have you tried this branch with the >>> switcher enabled? >> >> Yes, I have tested switcher + cpufreq + cpuidle with this branch and >> there are no issues found. I haven't tested with powertop yet. I will >> try and do that and let you know the result. >> >> You mentioned that when you run powertop, it gets stuck. When that >> happens, is there any log on the console or does system just turn >> unresponsive? > > The console is not responsive, but kernel seems busy because I see > periodic timeout messages from the samsung clock driver. Ok, I haven't tried to recreate this issue. I will try and do that. > > Note that I see these messages when things are functioning normally > also. I did notice this a few times before but today I was able to reproduce this consistently with other test cases. This timeout is because the CPU clock blocks of the cluster that has been turned down are being reconfigured, which on Exynos will not work. The following is a temporary patch which solves this issue. From b0c4057d428134fe12446431ede1d9a579fd1d05 Mon Sep 17 00:00:00 2001 From: Thomas Abraham Date: Fri, 5 Sep 2014 17:11:10 +0530 Subject: [PATCH] TEMP: cpufreq/bL: let the CPU switch complete before scaling frequency On Exynos5420/Exyons5800, the clock blocks that make up the CPU clock supply do no operate when the cluster in which they belong is powered off. The CPU clock supply path is PLL -> Mux/Dividers -> CPU_clock. In the arm_big_little CPUfreq driver, the frequency is scaled first and then the CPU is switched to the new cluster. In case the switch was for the first-man CPU in that new cluster, the frequency scaling step in arm_big_little CPUfreq driver would not work for Exynos since the in-bound cluster is powered off at that point. Note: On Exynos, the cluster is powered off when all the CPUs in that cluster are powered off which implies that the CPU clock blocks for that cluster do not operate anymore. So when using arm_big_little CPUfreq driver for Exynos, two changes are required. The first change is to let the CPU to switch to the new cluster before scaling the frequency. The second change is to ensure that the switch has been completed before scaling the frequency. With these changes, the message "wait_until_divider_stable: timeout in divider stabilization" is not seen anymore. Signed-off-by: Thomas Abraham --- arch/arm/include/asm/bL_switcher.h | 15 ++++++++++++++- drivers/cpufreq/arm_big_little.c | 9 ++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) if (bLs) { @@ -167,7 +174,7 @@ bL_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate) __func__, cpu, old_cluster, new_cluster); /* Switch cluster */ - bL_switch_request(cpu, new_cluster); + /*bL_switch_request(cpu, new_cluster);*/ mutex_lock(&cluster_lock[old_cluster]); diff --git a/arch/arm/include/asm/bL_switcher.h b/arch/arm/include/asm/bL_switcher.h index 1714800..d609b86 100644 --- a/arch/arm/include/asm/bL_switcher.h +++ b/arch/arm/include/asm/bL_switcher.h @@ -20,9 +20,22 @@ typedef void (*bL_switch_completion_handler)(void *cookie); int bL_switch_request_cb(unsigned int cpu, unsigned int new_cluster_id, bL_switch_completion_handler completer, void *completer_cookie); + +static void bL_switch_complete_cb(void *cookie) +{ + struct completion *switch_complete = (struct completion *) cookie; + complete(switch_complete); +} + static inline int bL_switch_request(unsigned int cpu, unsigned int new_cluster_id) { - return bL_switch_request_cb(cpu, new_cluster_id, NULL, NULL); + struct completion complete; + + init_completion(&complete); + bL_switch_request_cb(cpu, new_cluster_id, &bL_switch_complete_cb, + &complete); + wait_for_completion(&complete); + return 0; } /* diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c index a46c223..baeff47 100644 --- a/drivers/cpufreq/arm_big_little.c +++ b/drivers/cpufreq/arm_big_little.c @@ -129,6 +129,13 @@ bL_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate) int ret; bool bLs = is_bL_switching_enabled(); + /* Switch cluster */ + if (old_cluster != new_cluster) { + pr_debug("%s: cpu: %d, old cluster: %d, new cluster: %d\n", + __func__, cpu, old_cluster, new_cluster); + bL_switch_request(cpu, new_cluster); + } + mutex_lock(&cluster_lock[new_cluster]);