On Thu, Feb 14, 2019 at 8:46 AM Xiongfeng Wang
<wangxiongfeng2@xxxxxxxxxx> wrote:
>
> Hisilicon chips do not support delivered performance counter register
> and reference performance counter register. But the platform can
> calculate the real performance using its own method. This patch provide
> a workaround for this problem, and other platforms can also use this
> workaround framework. We reuse the desired performance register to
> store the real performance calculated by the platform. After the
> platform finished the frequency adjust, it gets the real performance and
> writes it into desired performance register. OS can use it to calculate
> the real frequency.
>
> Signed-off-by: Xiongfeng Wang <wangxiongfeng2@xxxxxxxxxx>
> ---
> drivers/cpufreq/cppc_cpufreq.c | 70 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 70 insertions(+)
>
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index fd25c21c..da96fec 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -33,6 +33,16 @@
> /* Offest in the DMI processor structure for the max frequency */
> #define DMI_PROCESSOR_MAX_SPEED 0x14
>
> +struct cppc_workaround_info {
> + char oem_id[ACPI_OEM_ID_SIZE +1];
> + char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
> + u32 oem_revision;
> + unsigned int (*get_rate)(unsigned int cpu);
> +};
> +
> +/* CPPC workaround for get_rate callback */
> +unsigned int (*cppc_wa_get_rate)(unsigned int cpu);
> +
First off, please don't split the workaround material into two parts.
IOW, the other new function added below can go here just fine IMO.
> /*
> * These structs contain information parsed from per CPU
> * ACPI _CPC structures.
> @@ -334,6 +344,9 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpunum)
> struct cppc_cpudata *cpu = all_cpu_data[cpunum];
> int ret;
>
> + if (cppc_wa_get_rate)
> + return cppc_wa_get_rate(cpunum);
Second, what is the value of using the function pointer above?
All we need for now is a flag to indicate whether or not to call
hisi_cppc_cpufreq_get_rate() here and return its return value.
> +
> ret = cppc_get_perf_ctrs(cpunum, &fb_ctrs_t0);
> if (ret)
> return ret;
> @@ -357,6 +370,61 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpunum)
> .name = "cppc_cpufreq",
> };
>
> +/*
> + * HISI platform does not support delivered performance counter and
> + * reference performance counter. It can calculate the performance using the
> + * platform specific mechanism. We reuse the desired performance register to
> + * store the real performance calculated by the platform.
> + */
> +static unsigned int hisi_cppc_cpufreq_get_rate(unsigned int cpunum)
> +{
> + struct cppc_cpudata *cpudata = all_cpu_data[cpunum];
> + u64 desired_perf;
> + int ret;
> +
> + ret = cppc_get_desired_perf(cpunum, &desired_perf);
> + if (ret < 0)
> + return -EIO;
> +
> + return cppc_cpufreq_perf_to_khz(cpudata, desired_perf);
> +}
> +
> +static struct cppc_workaround_info wa_info[] = {
> + {
> + .oem_id = "HISI ",
> + .oem_table_id = "HIP07 ",
> + .oem_revision = 0,
> + .get_rate = hisi_cppc_cpufreq_get_rate,
> + }, {
> + .oem_id = "HISI ",
> + .oem_table_id = "HIP08 ",
> + .oem_revision = 0,
> + .get_rate = hisi_cppc_cpufreq_get_rate,
> + }
> +};
> +
> +static int cppc_check_workaround(void)
And this function can be a void one.
> +{
> + struct acpi_table_header *tbl;
> + acpi_status status = AE_OK;
> + int i;
> +
> + status = acpi_get_table(ACPI_SIG_PCCT, 0, &tbl);
> + if (ACPI_FAILURE(status) || !tbl)
> + return -EINVAL;
> +
> + for (i = 0; i < ARRAY_SIZE(wa_info); i++) {
> + if (!memcmp(wa_info[i].oem_id, tbl->oem_id, ACPI_OEM_ID_SIZE) &&
> + !memcmp(wa_info[i].oem_table_id, tbl->oem_table_id, ACPI_OEM_TABLE_ID_SIZE) &&
> + wa_info[i].oem_revision == tbl->oem_revision) {
> + cppc_wa_get_rate = wa_info[i].get_rate;
> + return 0;
> + }
> + }
> +
> + return -ENODEV;
> +}
> +
> static int __init cppc_cpufreq_init(void)
> {
> int i, ret = 0;
> @@ -386,6 +454,8 @@ static int __init cppc_cpufreq_init(void)
> goto out;
> }
>
> + cppc_check_workaround();
> +
> ret = cpufreq_register_driver(&cppc_cpufreq_driver);
> if (ret)
> goto out;
> --
