Re: [PATCH 2/2] CPUFreq: Add new sysfs attribute freqdomain_cpus for acpi-freq driver

[Lan Tianyu <tianyu.lan@intel.com>]

On 2013年06月25日 15:48, Viresh Kumar wrote:
> On 25 June 2013 12:24, Lan Tianyu <tianyu.lan@intel.com> wrote:
>> On 2013年06月25日 11:56, Viresh Kumar wrote:
> 
>>> This is generic file, don't add this information here. Add this in
>>> acpi-cpufreq file.
>> I don't find acpi-cpufreq.txt under
>> Documentation/cpu-freq/acpi-cpufreq.txt. So I should create it?
> 
> I meant acpi-cpufreq.c file
Ok. From my opinion, the new attribute is an ABI and it's better to add
descriptor under Document directory. The user can be easy to find how to
use it.

> 
>> Please see the acpi_processor_preregister_performance() in the
>> drivers/acpi/processor_perlib.c. All cpus in the same dependency domain
>> are stored in the perf->shared_cpu_map(including the reference cpu the
>> perf belongs to). Original code will copy shared_cpu_map to
>> policy->related_cpus, do cpumask_or between policy->related_cpus and
>> policy->cpus in the cpufreq_add_dev() and store the result into
>> policy->related_cpus. Expose the data via related_cpus.
>>
>> For acpi-cpufreq driver, the shared_cpu_map is the biggest subset since
>> it regardless the coordination type.
>>
>> After the commit aa77a5, only the cpus in the software or
>> software&hardware coordination type dependency domain will copy to
>> policy->cpus and finally store into policy->related_cpus in the
>> cpufreq_add_dev() by cpumask_or. And then we miss the cpus in the
>> hardware coordination type domain. But these info is still stored in the
>> policy->shared_cpu_map. Does this make sense?
> 
> I was concerned about this code that was present earlier. That changes
> related_cpus based on some conditions.

Please see the commit which add the code. Maybe, we should overwrite
shared_cpu_map by sibling_cpus for this case?

commit acd316248205d553594296f1895ba5196b89ffcc
Author: Andre Przywara <andre.przywara@amd.com>
Date:   Tue Sep 4 08:28:03 2012 +0000

    acpi-cpufreq: Add quirk to disable _PSD usage on all AMD CPUs

    To workaround some Windows specific behavior, the ACPI _PSD table
    on AMD desktop boards advertises all cores as dependent, meaning
    that they all can only use the same P-state. acpi-cpufreq strictly
    obeys this description, instantiating one CPU only and symlinking
    the others. But the hardware can have distinct frequencies for each
    core and powernow-k8 did it that way.
    So, in order to use the hardware to its full potential and keep the
    original powernow-k8 behavior, lets override the _PSD table setting
    on AMD hardware.
    We use the siblings table, as it matches the current hardware
    behavior.

    Signed-off-by: Andre Przywara <andre.przywara@amd.com>
    Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>


> 
>         if (check_amd_hwpstate_cpu(cpu) && !acpi_pstate_strict) {
>                 cpumask_clear(policy->cpus);
>                 cpumask_set_cpu(cpu, policy->cpus);
>                 cpumask_copy(policy->related_cpus, cpu_sibling_mask(cpu));
>                 policy->shared_type = CPUFREQ_SHARED_TYPE_HW;
>                 pr_info_once(PFX "overriding BIOS provided _PSD data\n");
>         }
> 


-- 
Best regards
Tianyu Lan