From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755009AbcBDIZo (ORCPT ); Thu, 4 Feb 2016 03:25:44 -0500 Received: from mail-pf0-f171.google.com ([209.85.192.171]:35895 "EHLO mail-pf0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752197AbcBDIZm (ORCPT ); Thu, 4 Feb 2016 03:25:42 -0500 Date: Thu, 4 Feb 2016 13:55:38 +0530 From: Viresh Kumar To: ego@linux.vnet.ibm.com Cc: "Rafael J. Wysocki" , Linux PM list , Linux Kernel Mailing List , Srinivas Pandruvada , Juri Lelli , Steve Muckle , Saravana Kannan Subject: Re: [PATCH 3/11] cpufreq: governor: Use common global_dbs_data pointer Message-ID: <20160204082538.GB3469@vireshk> References: <3705929.bslqXH980s@vostro.rjw.lan> <1876466.AY9fn15fDn@vostro.rjw.lan> <20160204053614.GV3469@vireshk> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 04-02-16, 13:44, Gautham R Shenoy wrote: > In a a two policy system, to run ondemand on one and conservative on the other, >  won't the driver have CPUFREQ_HAVE_GOVERNOR_PER_POLICY set?   No. CPUFREQ_HAVE_GOVERNOR_PER_POLICY is not about the facility of using separate governor-type for each policy, that is always available to the user. CPUFREQ_HAVE_GOVERNOR_PER_POLICY was initially added for platforms with different type of CPUs on the same chip, though others can benefit from it as well. For example, on a 4 core ARM big LITTLE platform, we will have: - 2 A7 (low performance/low power) - 2 A15 (high performance/high power) The A7's share a policy and A15's share another one. Without CPUFREQ_HAVE_GOVERNOR_PER_POLICY, if ondemand is selected for both the policies, the we used to get a single directory (and a set of tunables) at /sys/devices/system/cpu/cpufreq/ondemand/ . That used to force us to use same tunables, like sampling rate, etc for both the policies. But because the CPUs were so different, we really wanted independent control. So, we designed CPUFREQ_HAVE_GOVERNOR_PER_POLICY, so that in such cases, each policy will have a set of tunables for the same governor type. Hope that makes it clear. If the below questionnaire is still valid, please let me know :) > If yes, then the changes in this patch won't come into play. > > Also in cpufreq_governor.c, we set cdata->gdbs_data only when  > !have_governor_per_policy(). cdata->gdbs_data is NULL otherwise. > A cursory inspection doesn't show any other place in the cpufreq codebase >  where cdata->gdbs_data is set. Unless I have missed one such initialization,  > based on my reading of the patch, instead of doing an extra hop to get the  > governor data via cdata->gdbs_data, we can simply record it in a global > variable. > > Also, if your concern is regarding the use of show_##file_name##_gov_sys in > case > of governor per policy, then the existing code is also broken, since we would > be  > accessing _gov##_dbs_cdata.gdbs_data->tuners where _gov##_dbs_cdata.gdbs_data  > will be NULL!. > > What am I missing ? -- viresh