Re: [PATCH] arch_topology: Update user supplied capacity to possible cpus in cluster

From: Chandra Sekhar Lingutla <clingutla@codeaurora.org>
To: Sudeep Holla <sudeep.holla@arm.com>,
	Quentin Perret <quentin.perret@arm.com>
Cc: catalin.marinas@arm.com, will.deacon@arm.com,
	Jeremy Linton <jeremy.linton@arm.com>,
	Morten Rasmussen <morten.rasmussen@arm.com>,
	dietmar.eggemann@arm.com, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] arch_topology: Update user supplied capacity to possible cpus in cluster
Date: Tue, 5 Mar 2019 21:23:24 +0530	[thread overview]
Message-ID: <f5c3765a-f8a9-3b35-2c42-51430e562f5f@codeaurora.org> (raw)
In-Reply-To: <20190305113633.GA2066@e107155-lin>


On 3/5/2019 5:06 PM, Sudeep Holla wrote:
> On Tue, Mar 05, 2019 at 11:29:55AM +0000, Quentin Perret wrote:
>> On Tuesday 05 Mar 2019 at 11:13:21 (+0000), Sudeep Holla wrote:
>> [...]
>>> While I like the idea, I am afraid that linking this to cpufreq policy
>>> may not be good. How will we deal with it on systems without CPUfreq ?
>>
>> Maybe something like this ?
>>
>> 	policy = cpufreq_cpu_get(cpu);
>> 	if (policy) {
>> 		for_each_cpu(i, policy->related_cpus) {
>> 			/* Update capacity for @i*/
>> 		}
>> 		cpufreq_cpu_put(policy);
>> 	} else {
>> 		/* Update capacity for @cpu*/
>> 	}
>>
>> I think it's OK to assume per-cpu capacities w/o CPUFreq. The only
>> case where it makes sense to 'bind' the capacity of several CPUs
>> together is when they're part of the same perf domain, I think. If you
>> don't know what the perf domains are, then there's nothing sensible you
>> can do.
>>
> 
> Makes sense.
> 
>> And for the dependency, a large part of the arch_topology driver is
>> already dependent on CPUFreq -- it registers a CPUFreq notifier on boot
>> to re-scale the CPU capacities depending on the max freq of the various
>> policies and so on. So the dependency is already there somehow.
>>
> 
> Sorry when I mentioned dependency, I meant absence of it needs to be
> dealt with. Your suggestion looks good.
> 
>> [...]
>>
>>> I think if there are no valid users of this, we *must* remove it. As I
>>> have pointed out in past, giving user such access will need platform
>>> knowledge. Though it's debatable topic, firmware providing this
>>> information is the only correct solution IMO.
>>
>> Yeah, if nobody is using it then maybe we can just remove it. Or at
>> least we can give it a go and if somebody complains then we can 'fix' it
>> with something like my snippet above :-)
>>
> 
> Happy to Ack code removing it ;). The argument that it can't be provided
> by firmware is no longer valid. We already have some dependency on DVFS
> data from the firmware for this to be functional correctly.
> 
If at all nobody uses it, making it as read-only (under debugfs) would be good ?
or else we are ok to update patch with related_cpus ?

> --
> Regards,
> Sudeep
> 

-- Chandrasekhar L, 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
 a Linux Foundation Collaborative Project.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel