diff mbox series

[RFC] cpufreq / cppc: Work around for Hisilicon CPPC cpufreq

Message ID 1547722811-18052-1-git-send-email-wangxiongfeng2@huawei.com (mailing list archive)
State RFC, archived
Headers show
Series [RFC] cpufreq / cppc: Work around for Hisilicon CPPC cpufreq | expand

Commit Message

Xiongfeng Wang Jan. 17, 2019, 11 a.m. UTC
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@huawei.com>
---
 drivers/acpi/cppc_acpi.c       | 29 ++++++++++++++++++++
 drivers/cpufreq/Kconfig.arm    |  7 +++++
 drivers/cpufreq/cppc_cpufreq.c | 62 ++++++++++++++++++++++++++++++++++++++++++
 include/acpi/cppc_acpi.h       |  4 +++
 4 files changed, 102 insertions(+)

Comments

Viresh Kumar Jan. 24, 2019, 6:56 a.m. UTC | #1
+George/Prashanth.

Guys please see if you have any objections to this patch. I am not
very familiar with this stuff and it would be good to get some
feedback from you guys.

@Rafael: Do you have any comments on this ?

On 17-01-19, 19:00, Xiongfeng Wang 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@huawei.com>
> ---
>  drivers/acpi/cppc_acpi.c       | 29 ++++++++++++++++++++
>  drivers/cpufreq/Kconfig.arm    |  7 +++++
>  drivers/cpufreq/cppc_cpufreq.c | 62 ++++++++++++++++++++++++++++++++++++++++++
>  include/acpi/cppc_acpi.h       |  4 +++
>  4 files changed, 102 insertions(+)
> 
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 217a782..0cdaf7e 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1050,6 +1050,35 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
>  	return ret_val;
>  }
>  
> +#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
> +/*
> + * We reuse the desired performance register to store the real performance
> + * calculated by the platform.
> + */
> +u64 hisi_cppc_get_real_perf(unsigned int cpunum)
> +{
> +	int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
> +	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpunum);
> +	struct cpc_register_resource *desired_reg;
> +	u64 desired_perf;
> +	int ret;
> +
> +	/*
> +	 * Make sure the platform has finished the frequency adjust
> +	 * and wrote the real performance in desired performance register
> +	 */
> +	ret = check_pcc_chan(pcc_ss_id, false);
> +	if (ret)
> +		return 0;
> +
> +	desired_reg = &cpc_desc->cpc_regs[DESIRED_PERF];
> +	cpc_read(cpunum, desired_reg, &desired_perf);
> +
> +	return desired_perf;
> +}
> +EXPORT_SYMBOL_GPL(hisi_cppc_get_real_perf);
> +#endif
> +
>  /**
>   * cppc_get_perf_caps - Get a CPUs performance capabilities.
>   * @cpunum: CPU from which to get capabilities info.
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 688f102..236bd07 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -18,6 +18,13 @@ config ACPI_CPPC_CPUFREQ
>  
>  	  If in doubt, say N.
>  
> +config HISILICON_CPPC_CPUFREQ_WORKAROUND
> +	bool "Workaround for Hisilicon CPPC Cpufreq"
> +	default y
> +	depends on ACPI_CPPC_CPUFREQ && ARM64
> +	help
> +	  This option enables a workaround for Hisilicon CPPC Cpufreq.
> +
>  config ARM_ARMADA_37XX_CPUFREQ
>  	tristate "Armada 37xx CPUFreq support"
>  	depends on ARCH_MVEBU && CPUFREQ_DT
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index fd25c21c..b910e84 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -33,6 +33,13 @@
>  /* Offest in the DMI processor structure for the max frequency */
>  #define DMI_PROCESSOR_MAX_SPEED  0x14
>  
> +struct cppc_get_rate_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)(unsigned int cpu);
> +};
> +
>  /*
>   * These structs contain information parsed from per CPU
>   * ACPI _CPC structures.
> @@ -357,6 +364,59 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpunum)
>  	.name = "cppc_cpufreq",
>  };
>  
> +#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
> +/*
> + * When the platform does not support delivered performance counter or
> + * 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 *cpu = all_cpu_data[cpunum];
> +	u64 desired_perf = hisi_cppc_get_real_perf(cpunum);
> +
> +	return cppc_cpufreq_perf_to_khz(cpu, desired_perf);
> +}
> +#endif
> +
> +static struct cppc_get_rate_workaround_info wa_info[] = {
> +#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
> +	{
> +		.oem_id		= "HISI  ",
> +		.oem_table_id	= "HIP07   ",
> +		.oem_revision	= 0,
> +		.get = hisi_cppc_cpufreq_get_rate,
> +	},
> +	{

This should be:

        }, {

> +		.oem_id		= "HISI  ",
> +		.oem_table_id	= "HIP08   ",
> +		.oem_revision	= 0,
> +		.get = hisi_cppc_cpufreq_get_rate,
> +	},
> +#endif
> +	{},
> +};
> +
> +static void cppc_check_get_rate_workaround(struct cpufreq_driver *cppc_cpufreq_driver)
> +{
> +	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;
> +
> +	for (i = 0; i < ARRAY_SIZE(wa_info) - 1; 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_cpufreq_driver->get = wa_info[i].get;
> +			return;
> +		}
> +	}
> +}
>  static int __init cppc_cpufreq_init(void)
>  {
>  	int i, ret = 0;
> @@ -386,6 +446,8 @@ static int __init cppc_cpufreq_init(void)
>  		goto out;
>  	}
>  
> +	cppc_check_get_rate_workaround(&cppc_cpufreq_driver);
> +
>  	ret = cpufreq_register_driver(&cppc_cpufreq_driver);
>  	if (ret)
>  		goto out;
> diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
> index 4f34734..be7400b 100644
> --- a/include/acpi/cppc_acpi.h
> +++ b/include/acpi/cppc_acpi.h
> @@ -146,4 +146,8 @@ struct cppc_cpudata {
>  extern int cpc_read_ffh(int cpunum, struct cpc_reg *reg, u64 *val);
>  extern int cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val);
>  
> +#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
> +u64 hisi_cppc_get_real_perf(unsigned int cpunum);
> +#endif
> +
>  #endif /* _CPPC_ACPI_H*/
> -- 
> 1.7.12.4
Rafael J. Wysocki Jan. 24, 2019, 9:43 a.m. UTC | #2
On Thu, Jan 24, 2019 at 7:56 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> +George/Prashanth.
>
> Guys please see if you have any objections to this patch. I am not
> very familiar with this stuff and it would be good to get some
> feedback from you guys.
>
> @Rafael: Do you have any comments on this ?

Not yet, give me some time to noodle on it, please.
George Cherian Jan. 25, 2019, 7:07 a.m. UTC | #3
Hi Wang,

On Thu, Jan 24, 2019 at 12:27 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> +George/Prashanth.
>
> Guys please see if you have any objections to this patch. I am not
> very familiar with this stuff and it would be good to get some
> feedback from you guys.
>
> @Rafael: Do you have any comments on this ?
>
> On 17-01-19, 19:00, Xiongfeng Wang 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.
Does your platform support Autonomous Selection mode?
This register is not valid when autonomous mode is enabled. In such case
how are you calculating the frequency?
> >
> > Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com>
> > ---
> >  drivers/acpi/cppc_acpi.c       | 29 ++++++++++++++++++++
> >  drivers/cpufreq/Kconfig.arm    |  7 +++++
> >  drivers/cpufreq/cppc_cpufreq.c | 62 ++++++++++++++++++++++++++++++++++++++++++
> >  include/acpi/cppc_acpi.h       |  4 +++
> >  4 files changed, 102 insertions(+)
> >
> > diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> > index 217a782..0cdaf7e 100644
> > --- a/drivers/acpi/cppc_acpi.c
> > +++ b/drivers/acpi/cppc_acpi.c
> > @@ -1050,6 +1050,35 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
> >       return ret_val;
> >  }
> >
> > +#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
> > +/*
> > + * We reuse the desired performance register to store the real performance
> > + * calculated by the platform.
> > + */
> > +u64 hisi_cppc_get_real_perf(unsigned int cpunum)
> > +{
> > +     int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
> > +     struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpunum);
> > +     struct cpc_register_resource *desired_reg;
> > +     u64 desired_perf;
> > +     int ret;
> > +
> > +     /*
> > +      * Make sure the platform has finished the frequency adjust
> > +      * and wrote the real performance in desired performance register
> > +      */
> > +     ret = check_pcc_chan(pcc_ss_id, false);
> > +     if (ret)
> > +             return 0;
If there is a pending command in the channel then returning zero
will give bogus frequency value. You may return the previous written value.
> > +
> > +     desired_reg = &cpc_desc->cpc_regs[DESIRED_PERF];
> > +     cpc_read(cpunum, desired_reg, &desired_perf);
> > +
> > +     return desired_perf;
> > +}
> > +EXPORT_SYMBOL_GPL(hisi_cppc_get_real_perf);
> > +#endif
> > +
> >  /**
> >   * cppc_get_perf_caps - Get a CPUs performance capabilities.
> >   * @cpunum: CPU from which to get capabilities info.
> > diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> > index 688f102..236bd07 100644
> > --- a/drivers/cpufreq/Kconfig.arm
> > +++ b/drivers/cpufreq/Kconfig.arm
> > @@ -18,6 +18,13 @@ config ACPI_CPPC_CPUFREQ
> >
> >         If in doubt, say N.
> >
> > +config HISILICON_CPPC_CPUFREQ_WORKAROUND
> > +     bool "Workaround for Hisilicon CPPC Cpufreq"
> > +     default y
> > +     depends on ACPI_CPPC_CPUFREQ && ARM64
Do you really want this to be applied to all ARM64? or just only
for affected HISI platforms?
> > +     help
> > +       This option enables a workaround for Hisilicon CPPC Cpufreq.
> > +
> >  config ARM_ARMADA_37XX_CPUFREQ
> >       tristate "Armada 37xx CPUFreq support"
> >       depends on ARCH_MVEBU && CPUFREQ_DT
> > diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> > index fd25c21c..b910e84 100644
> > --- a/drivers/cpufreq/cppc_cpufreq.c
> > +++ b/drivers/cpufreq/cppc_cpufreq.c
> > @@ -33,6 +33,13 @@
> >  /* Offest in the DMI processor structure for the max frequency */
> >  #define DMI_PROCESSOR_MAX_SPEED  0x14
> >
> > +struct cppc_get_rate_workaround_info {
If your intention is to make a generic framework for future extensions
then you might need to name it differently. Something like
struct cppc_workaround_info, so that you can extend the same for any
future workarounds.
> > +     char oem_id[ACPI_OEM_ID_SIZE +1];
> > +     char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
> > +     u32 oem_revision;
> > +     unsigned int (*get)(unsigned int cpu);
This can be  unsigned int (*get_rate)(unsigned int cpu);
> > +};
> > +
> >  /*
> >   * These structs contain information parsed from per CPU
> >   * ACPI _CPC structures.
> > @@ -357,6 +364,59 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpunum)
> >       .name = "cppc_cpufreq",
> >  };
> >
> > +#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
> > +/*
> > + * When the platform does not support delivered performance counter or
> > + * 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 *cpu = all_cpu_data[cpunum];
> > +     u64 desired_perf = hisi_cppc_get_real_perf(cpunum);
> > +
> > +     return cppc_cpufreq_perf_to_khz(cpu, desired_perf);
> > +}
> > +#endif
> > +
> > +static struct cppc_get_rate_workaround_info wa_info[] = {
> > +#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
> > +     {
> > +             .oem_id         = "HISI  ",
> > +             .oem_table_id   = "HIP07   ",
> > +             .oem_revision   = 0,
> > +             .get = hisi_cppc_cpufreq_get_rate,
> > +     },
> > +     {
>
> This should be:
>
>         }, {
>
> > +             .oem_id         = "HISI  ",
> > +             .oem_table_id   = "HIP08   ",
> > +             .oem_revision   = 0,
> > +             .get = hisi_cppc_cpufreq_get_rate,
> > +     },
> > +#endif
> > +     {},
> > +};
> > +
> > +static void cppc_check_get_rate_workaround(struct cpufreq_driver *cppc_cpufreq_driver)
This function can be renamed to cppc_check_workaround to make it
generic also can have a
proper return value so that the caller can validate.
> > +{
> > +     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;
> > +
> > +     for (i = 0; i < ARRAY_SIZE(wa_info) - 1; 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_cpufreq_driver->get = wa_info[i].get;
> > +                     return;
> > +             }
> > +     }
> > +}
> >  static int __init cppc_cpufreq_init(void)
> >  {
> >       int i, ret = 0;
> > @@ -386,6 +446,8 @@ static int __init cppc_cpufreq_init(void)
> >               goto out;
> >       }
> >
> > +     cppc_check_get_rate_workaround(&cppc_cpufreq_driver);
> > +
> >       ret = cpufreq_register_driver(&cppc_cpufreq_driver);
> >       if (ret)
> >               goto out;
> > diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
> > index 4f34734..be7400b 100644
> > --- a/include/acpi/cppc_acpi.h
> > +++ b/include/acpi/cppc_acpi.h
> > @@ -146,4 +146,8 @@ struct cppc_cpudata {
> >  extern int cpc_read_ffh(int cpunum, struct cpc_reg *reg, u64 *val);
> >  extern int cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val);
> >
> > +#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
> > +u64 hisi_cppc_get_real_perf(unsigned int cpunum);
> > +#endif
> > +
> >  #endif /* _CPPC_ACPI_H*/
> > --
> > 1.7.12.4
>
> --
> viresh
-George
Xiongfeng Wang Jan. 26, 2019, 9:44 a.m. UTC | #4
Hi George,

Thanks for your reply !

On 2019/1/25 15:07, George Cherian wrote:
> Hi Wang,
> 
> On Thu, Jan 24, 2019 at 12:27 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>>
>> +George/Prashanth.
>>
>> Guys please see if you have any objections to this patch. I am not
>> very familiar with this stuff and it would be good to get some
>> feedback from you guys.
>>
>> @Rafael: Do you have any comments on this ?
>>
>> On 17-01-19, 19:00, Xiongfeng Wang 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.
> Does your platform support Autonomous Selection mode?
> This register is not valid when autonomous mode is enabled. In such case
> how are you calculating the frequency?

Our platform does not support Autonomous Selection mode.
When it's enabled, this method won't work. But I think the current doesn't support
Autonomous Selection mode. I only found a defition 'AUTO_SEL_ENABLE' in enum cppc_regs
and there is no one using it.

>>>
>>> Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com>
>>> ---
>>>  drivers/acpi/cppc_acpi.c       | 29 ++++++++++++++++++++
>>>  drivers/cpufreq/Kconfig.arm    |  7 +++++
>>>  drivers/cpufreq/cppc_cpufreq.c | 62 ++++++++++++++++++++++++++++++++++++++++++
>>>  include/acpi/cppc_acpi.h       |  4 +++
>>>  4 files changed, 102 insertions(+)
>>>
>>> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>>> index 217a782..0cdaf7e 100644
>>> --- a/drivers/acpi/cppc_acpi.c
>>> +++ b/drivers/acpi/cppc_acpi.c
>>> @@ -1050,6 +1050,35 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
>>>       return ret_val;
>>>  }
>>>
>>> +#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
>>> +/*
>>> + * We reuse the desired performance register to store the real performance
>>> + * calculated by the platform.
>>> + */
>>> +u64 hisi_cppc_get_real_perf(unsigned int cpunum)
>>> +{
>>> +     int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
>>> +     struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpunum);
>>> +     struct cpc_register_resource *desired_reg;
>>> +     u64 desired_perf;
>>> +     int ret;
>>> +
>>> +     /*
>>> +      * Make sure the platform has finished the frequency adjust
>>> +      * and wrote the real performance in desired performance register
>>> +      */
>>> +     ret = check_pcc_chan(pcc_ss_id, false);
>>> +     if (ret)
>>> +             return 0;
> If there is a pending command in the channel then returning zero
> will give bogus frequency value. You may return the previous written value.

How about returning a error code here ?
In current kernel, when 'cppc_cpufreq_get_rate' returns -EFAULT, 'cpuinfo_cur_freq'
shows a very big value. I was thinking about writing a patch to fix it.

>>> +
>>> +     desired_reg = &cpc_desc->cpc_regs[DESIRED_PERF];
>>> +     cpc_read(cpunum, desired_reg, &desired_perf);
>>> +
>>> +     return desired_perf;
>>> +}
>>> +EXPORT_SYMBOL_GPL(hisi_cppc_get_real_perf);
>>> +#endif
>>> +
>>>  /**
>>>   * cppc_get_perf_caps - Get a CPUs performance capabilities.
>>>   * @cpunum: CPU from which to get capabilities info.
>>> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
>>> index 688f102..236bd07 100644
>>> --- a/drivers/cpufreq/Kconfig.arm
>>> +++ b/drivers/cpufreq/Kconfig.arm
>>> @@ -18,6 +18,13 @@ config ACPI_CPPC_CPUFREQ
>>>
>>>         If in doubt, say N.
>>>
>>> +config HISILICON_CPPC_CPUFREQ_WORKAROUND
>>> +     bool "Workaround for Hisilicon CPPC Cpufreq"
>>> +     default y
>>> +     depends on ACPI_CPPC_CPUFREQ && ARM64
> Do you really want this to be applied to all ARM64? or just only
> for affected HISI platforms?

I was thinking about applying the framwork to all ARM64.
CONF_HISILICON_CPPC_CPUFREQ_WORKAROUND adds the OEM ID for HISI.

>>> +     help
>>> +       This option enables a workaround for Hisilicon CPPC Cpufreq.
>>> +
>>>  config ARM_ARMADA_37XX_CPUFREQ
>>>       tristate "Armada 37xx CPUFreq support"
>>>       depends on ARCH_MVEBU && CPUFREQ_DT
>>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>>> index fd25c21c..b910e84 100644
>>> --- a/drivers/cpufreq/cppc_cpufreq.c
>>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>>> @@ -33,6 +33,13 @@
>>>  /* Offest in the DMI processor structure for the max frequency */
>>>  #define DMI_PROCESSOR_MAX_SPEED  0x14
>>>
>>> +struct cppc_get_rate_workaround_info {
> If your intention is to make a generic framework for future extensions
> then you might need to name it differently. Something like
> struct cppc_workaround_info, so that you can extend the same for any
> future workarounds.

Thanks for your advice. I will change it in the next version.

>>> +     char oem_id[ACPI_OEM_ID_SIZE +1];
>>> +     char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
>>> +     u32 oem_revision;
>>> +     unsigned int (*get)(unsigned int cpu);
> This can be  unsigned int (*get_rate)(unsigned int cpu);
>>> +};
>>> +
>>>  /*
>>>   * These structs contain information parsed from per CPU
>>>   * ACPI _CPC structures.
>>> @@ -357,6 +364,59 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpunum)
>>>       .name = "cppc_cpufreq",
>>>  };
>>>
>>> +#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
>>> +/*
>>> + * When the platform does not support delivered performance counter or
>>> + * 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 *cpu = all_cpu_data[cpunum];
>>> +     u64 desired_perf = hisi_cppc_get_real_perf(cpunum);
>>> +
>>> +     return cppc_cpufreq_perf_to_khz(cpu, desired_perf);
>>> +}
>>> +#endif
>>> +
>>> +static struct cppc_get_rate_workaround_info wa_info[] = {
>>> +#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
>>> +     {
>>> +             .oem_id         = "HISI  ",
>>> +             .oem_table_id   = "HIP07   ",
>>> +             .oem_revision   = 0,
>>> +             .get = hisi_cppc_cpufreq_get_rate,
>>> +     },
>>> +     {
>>
>> This should be:
>>
>>         }, {
>>
>>> +             .oem_id         = "HISI  ",
>>> +             .oem_table_id   = "HIP08   ",
>>> +             .oem_revision   = 0,
>>> +             .get = hisi_cppc_cpufreq_get_rate,
>>> +     },
>>> +#endif
>>> +     {},
>>> +};
>>> +
>>> +static void cppc_check_get_rate_workaround(struct cpufreq_driver *cppc_cpufreq_driver)
> This function can be renamed to cppc_check_workaround to make it
> generic also can have a
> proper return value so that the caller can validate.

Thanks, I will change it and all above.
If I want to apply it to ARM64, I think I will need a CONFIG_ARM64 here.

>>> +{
>>> +     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;
>>> +
>>> +     for (i = 0; i < ARRAY_SIZE(wa_info) - 1; 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_cpufreq_driver->get = wa_info[i].get;
>>> +                     return;
>>> +             }
>>> +     }
>>> +}
>>>  static int __init cppc_cpufreq_init(void)
>>>  {
>>>       int i, ret = 0;
>>> @@ -386,6 +446,8 @@ static int __init cppc_cpufreq_init(void)
>>>               goto out;
>>>       }
>>>
>>> +     cppc_check_get_rate_workaround(&cppc_cpufreq_driver);
>>> +
>>>       ret = cpufreq_register_driver(&cppc_cpufreq_driver);
>>>       if (ret)
>>>               goto out;
>>> diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
>>> index 4f34734..be7400b 100644
>>> --- a/include/acpi/cppc_acpi.h
>>> +++ b/include/acpi/cppc_acpi.h
>>> @@ -146,4 +146,8 @@ struct cppc_cpudata {
>>>  extern int cpc_read_ffh(int cpunum, struct cpc_reg *reg, u64 *val);
>>>  extern int cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val);
>>>
>>> +#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
>>> +u64 hisi_cppc_get_real_perf(unsigned int cpunum);
>>> +#endif
>>> +
>>>  #endif /* _CPPC_ACPI_H*/
>>> --
>>> 1.7.12.4
>>
>> --
>> viresh
> -George
> 
> .
> 
Thanks,
Xiongfeng
Rafael J. Wysocki Jan. 30, 2019, 11:42 p.m. UTC | #5
On Thursday, January 17, 2019 12:00:11 PM CET Xiongfeng Wang 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@huawei.com>

Overall, I would do it differently.

> ---
>  drivers/acpi/cppc_acpi.c       | 29 ++++++++++++++++++++
>  drivers/cpufreq/Kconfig.arm    |  7 +++++
>  drivers/cpufreq/cppc_cpufreq.c | 62 ++++++++++++++++++++++++++++++++++++++++++
>  include/acpi/cppc_acpi.h       |  4 +++
>  4 files changed, 102 insertions(+)
> 
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 217a782..0cdaf7e 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1050,6 +1050,35 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
>  	return ret_val;
>  }
>  
> +#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
> +/*
> + * We reuse the desired performance register to store the real performance
> + * calculated by the platform.
> + */
> +u64 hisi_cppc_get_real_perf(unsigned int cpunum)
> +{
> +	int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
> +	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpunum);
> +	struct cpc_register_resource *desired_reg;
> +	u64 desired_perf;
> +	int ret;
> +
> +	/*
> +	 * Make sure the platform has finished the frequency adjust
> +	 * and wrote the real performance in desired performance register
> +	 */
> +	ret = check_pcc_chan(pcc_ss_id, false);
> +	if (ret)
> +		return 0;
> +
> +	desired_reg = &cpc_desc->cpc_regs[DESIRED_PERF];
> +	cpc_read(cpunum, desired_reg, &desired_perf);
> +
> +	return desired_perf;
> +}
> +EXPORT_SYMBOL_GPL(hisi_cppc_get_real_perf);
> +#endif

I would provide library routine to read desired_perf and it doesn't
need to have anything to do with HiSi at this level.  Also, it doesn't
need to be compiled conditionally.  Just call it cppc_get_desired_perf()
or similar.

Then, you can use it to implement the quirk in the cpufreq driver.

> +
>  /**
>   * cppc_get_perf_caps - Get a CPUs performance capabilities.
>   * @cpunum: CPU from which to get capabilities info.
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 688f102..236bd07 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -18,6 +18,13 @@ config ACPI_CPPC_CPUFREQ
>  
>  	  If in doubt, say N.
>  
> +config HISILICON_CPPC_CPUFREQ_WORKAROUND
> +	bool "Workaround for Hisilicon CPPC Cpufreq"

What is the reason for this to be a user-selectable option?

> +	default y
> +	depends on ACPI_CPPC_CPUFREQ && ARM64
> +	help
> +	  This option enables a workaround for Hisilicon CPPC Cpufreq.
> +
>  config ARM_ARMADA_37XX_CPUFREQ
>  	tristate "Armada 37xx CPUFreq support"
>  	depends on ARCH_MVEBU && CPUFREQ_DT
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index fd25c21c..b910e84 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -33,6 +33,13 @@
>  /* Offest in the DMI processor structure for the max frequency */
>  #define DMI_PROCESSOR_MAX_SPEED  0x14
>  
> +struct cppc_get_rate_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)(unsigned int cpu);
> +};
> +
>  /*
>   * These structs contain information parsed from per CPU
>   * ACPI _CPC structures.
> @@ -357,6 +364,59 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpunum)
>  	.name = "cppc_cpufreq",
>  };
>  
> +#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
> +/*
> + * When the platform does not support delivered performance counter or
> + * 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 *cpu = all_cpu_data[cpunum];
> +	u64 desired_perf = hisi_cppc_get_real_perf(cpunum);
> +
> +	return cppc_cpufreq_perf_to_khz(cpu, desired_perf);
> +}
> +#endif
> +
> +static struct cppc_get_rate_workaround_info wa_info[] = {
> +#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
> +	{
> +		.oem_id		= "HISI  ",
> +		.oem_table_id	= "HIP07   ",
> +		.oem_revision	= 0,
> +		.get = hisi_cppc_cpufreq_get_rate,
> +	},
> +	{
> +		.oem_id		= "HISI  ",
> +		.oem_table_id	= "HIP08   ",
> +		.oem_revision	= 0,
> +		.get = hisi_cppc_cpufreq_get_rate,
> +	},
> +#endif

Is this #ifdef really necessary?

> +	{},
> +};
> +
> +static void cppc_check_get_rate_workaround(struct cpufreq_driver *cppc_cpufreq_driver)
> +{
> +	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;
> +
> +	for (i = 0; i < ARRAY_SIZE(wa_info) - 1; 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_cpufreq_driver->get = wa_info[i].get;
> +			return;
> +		}
> +	}
> +}
>  static int __init cppc_cpufreq_init(void)
>  {
>  	int i, ret = 0;
> @@ -386,6 +446,8 @@ static int __init cppc_cpufreq_init(void)
>  		goto out;
>  	}
>  
> +	cppc_check_get_rate_workaround(&cppc_cpufreq_driver);
> +
>  	ret = cpufreq_register_driver(&cppc_cpufreq_driver);
>  	if (ret)
>  		goto out;
> diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
> index 4f34734..be7400b 100644
> --- a/include/acpi/cppc_acpi.h
> +++ b/include/acpi/cppc_acpi.h
> @@ -146,4 +146,8 @@ struct cppc_cpudata {
>  extern int cpc_read_ffh(int cpunum, struct cpc_reg *reg, u64 *val);
>  extern int cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val);
>  
> +#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
> +u64 hisi_cppc_get_real_perf(unsigned int cpunum);
> +#endif
> +
>  #endif /* _CPPC_ACPI_H*/
>
Xiongfeng Wang Feb. 2, 2019, 8:30 a.m. UTC | #6
Hi, Rafael

Thanks for your reply and advice below.
Sorry, I'am out of the office because of the Chinese spring festival.
I will send another version when I come back. It's 14th February.

Thanks,
Xiongfeng

On 2019/1/31 7:42, Rafael J. Wysocki wrote:
> On Thursday, January 17, 2019 12:00:11 PM CET Xiongfeng Wang 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@huawei.com>
> 
> Overall, I would do it differently.
> 
>> ---
>>  drivers/acpi/cppc_acpi.c       | 29 ++++++++++++++++++++
>>  drivers/cpufreq/Kconfig.arm    |  7 +++++
>>  drivers/cpufreq/cppc_cpufreq.c | 62 ++++++++++++++++++++++++++++++++++++++++++
>>  include/acpi/cppc_acpi.h       |  4 +++
>>  4 files changed, 102 insertions(+)
>>
>> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>> index 217a782..0cdaf7e 100644
>> --- a/drivers/acpi/cppc_acpi.c
>> +++ b/drivers/acpi/cppc_acpi.c
>> @@ -1050,6 +1050,35 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
>>  	return ret_val;
>>  }
>>  
>> +#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
>> +/*
>> + * We reuse the desired performance register to store the real performance
>> + * calculated by the platform.
>> + */
>> +u64 hisi_cppc_get_real_perf(unsigned int cpunum)
>> +{
>> +	int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
>> +	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpunum);
>> +	struct cpc_register_resource *desired_reg;
>> +	u64 desired_perf;
>> +	int ret;
>> +
>> +	/*
>> +	 * Make sure the platform has finished the frequency adjust
>> +	 * and wrote the real performance in desired performance register
>> +	 */
>> +	ret = check_pcc_chan(pcc_ss_id, false);
>> +	if (ret)
>> +		return 0;
>> +
>> +	desired_reg = &cpc_desc->cpc_regs[DESIRED_PERF];
>> +	cpc_read(cpunum, desired_reg, &desired_perf);
>> +
>> +	return desired_perf;
>> +}
>> +EXPORT_SYMBOL_GPL(hisi_cppc_get_real_perf);
>> +#endif
> 
> I would provide library routine to read desired_perf and it doesn't
> need to have anything to do with HiSi at this level.  Also, it doesn't
> need to be compiled conditionally.  Just call it cppc_get_desired_perf()
> or similar.
> 
> Then, you can use it to implement the quirk in the cpufreq driver.
> 
>> +
>>  /**
>>   * cppc_get_perf_caps - Get a CPUs performance capabilities.
>>   * @cpunum: CPU from which to get capabilities info.
>> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
>> index 688f102..236bd07 100644
>> --- a/drivers/cpufreq/Kconfig.arm
>> +++ b/drivers/cpufreq/Kconfig.arm
>> @@ -18,6 +18,13 @@ config ACPI_CPPC_CPUFREQ
>>  
>>  	  If in doubt, say N.
>>  
>> +config HISILICON_CPPC_CPUFREQ_WORKAROUND
>> +	bool "Workaround for Hisilicon CPPC Cpufreq"
> 
> What is the reason for this to be a user-selectable option?
> 
>> +	default y
>> +	depends on ACPI_CPPC_CPUFREQ && ARM64
>> +	help
>> +	  This option enables a workaround for Hisilicon CPPC Cpufreq.
>> +
>>  config ARM_ARMADA_37XX_CPUFREQ
>>  	tristate "Armada 37xx CPUFreq support"
>>  	depends on ARCH_MVEBU && CPUFREQ_DT
>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>> index fd25c21c..b910e84 100644
>> --- a/drivers/cpufreq/cppc_cpufreq.c
>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>> @@ -33,6 +33,13 @@
>>  /* Offest in the DMI processor structure for the max frequency */
>>  #define DMI_PROCESSOR_MAX_SPEED  0x14
>>  
>> +struct cppc_get_rate_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)(unsigned int cpu);
>> +};
>> +
>>  /*
>>   * These structs contain information parsed from per CPU
>>   * ACPI _CPC structures.
>> @@ -357,6 +364,59 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpunum)
>>  	.name = "cppc_cpufreq",
>>  };
>>  
>> +#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
>> +/*
>> + * When the platform does not support delivered performance counter or
>> + * 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 *cpu = all_cpu_data[cpunum];
>> +	u64 desired_perf = hisi_cppc_get_real_perf(cpunum);
>> +
>> +	return cppc_cpufreq_perf_to_khz(cpu, desired_perf);
>> +}
>> +#endif
>> +
>> +static struct cppc_get_rate_workaround_info wa_info[] = {
>> +#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
>> +	{
>> +		.oem_id		= "HISI  ",
>> +		.oem_table_id	= "HIP07   ",
>> +		.oem_revision	= 0,
>> +		.get = hisi_cppc_cpufreq_get_rate,
>> +	},
>> +	{
>> +		.oem_id		= "HISI  ",
>> +		.oem_table_id	= "HIP08   ",
>> +		.oem_revision	= 0,
>> +		.get = hisi_cppc_cpufreq_get_rate,
>> +	},
>> +#endif
> 
> Is this #ifdef really necessary?
> 
>> +	{},
>> +};
>> +
>> +static void cppc_check_get_rate_workaround(struct cpufreq_driver *cppc_cpufreq_driver)
>> +{
>> +	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;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(wa_info) - 1; 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_cpufreq_driver->get = wa_info[i].get;
>> +			return;
>> +		}
>> +	}
>> +}
>>  static int __init cppc_cpufreq_init(void)
>>  {
>>  	int i, ret = 0;
>> @@ -386,6 +446,8 @@ static int __init cppc_cpufreq_init(void)
>>  		goto out;
>>  	}
>>  
>> +	cppc_check_get_rate_workaround(&cppc_cpufreq_driver);
>> +
>>  	ret = cpufreq_register_driver(&cppc_cpufreq_driver);
>>  	if (ret)
>>  		goto out;
>> diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
>> index 4f34734..be7400b 100644
>> --- a/include/acpi/cppc_acpi.h
>> +++ b/include/acpi/cppc_acpi.h
>> @@ -146,4 +146,8 @@ struct cppc_cpudata {
>>  extern int cpc_read_ffh(int cpunum, struct cpc_reg *reg, u64 *val);
>>  extern int cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val);
>>  
>> +#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
>> +u64 hisi_cppc_get_real_perf(unsigned int cpunum);
>> +#endif
>> +
>>  #endif /* _CPPC_ACPI_H*/
>>
> 
> 
> 
> .
>
diff mbox series

Patch

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 217a782..0cdaf7e 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -1050,6 +1050,35 @@  static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
 	return ret_val;
 }
 
+#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
+/*
+ * We reuse the desired performance register to store the real performance
+ * calculated by the platform.
+ */
+u64 hisi_cppc_get_real_perf(unsigned int cpunum)
+{
+	int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
+	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpunum);
+	struct cpc_register_resource *desired_reg;
+	u64 desired_perf;
+	int ret;
+
+	/*
+	 * Make sure the platform has finished the frequency adjust
+	 * and wrote the real performance in desired performance register
+	 */
+	ret = check_pcc_chan(pcc_ss_id, false);
+	if (ret)
+		return 0;
+
+	desired_reg = &cpc_desc->cpc_regs[DESIRED_PERF];
+	cpc_read(cpunum, desired_reg, &desired_perf);
+
+	return desired_perf;
+}
+EXPORT_SYMBOL_GPL(hisi_cppc_get_real_perf);
+#endif
+
 /**
  * cppc_get_perf_caps - Get a CPUs performance capabilities.
  * @cpunum: CPU from which to get capabilities info.
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 688f102..236bd07 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -18,6 +18,13 @@  config ACPI_CPPC_CPUFREQ
 
 	  If in doubt, say N.
 
+config HISILICON_CPPC_CPUFREQ_WORKAROUND
+	bool "Workaround for Hisilicon CPPC Cpufreq"
+	default y
+	depends on ACPI_CPPC_CPUFREQ && ARM64
+	help
+	  This option enables a workaround for Hisilicon CPPC Cpufreq.
+
 config ARM_ARMADA_37XX_CPUFREQ
 	tristate "Armada 37xx CPUFreq support"
 	depends on ARCH_MVEBU && CPUFREQ_DT
diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index fd25c21c..b910e84 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -33,6 +33,13 @@ 
 /* Offest in the DMI processor structure for the max frequency */
 #define DMI_PROCESSOR_MAX_SPEED  0x14
 
+struct cppc_get_rate_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)(unsigned int cpu);
+};
+
 /*
  * These structs contain information parsed from per CPU
  * ACPI _CPC structures.
@@ -357,6 +364,59 @@  static unsigned int cppc_cpufreq_get_rate(unsigned int cpunum)
 	.name = "cppc_cpufreq",
 };
 
+#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
+/*
+ * When the platform does not support delivered performance counter or
+ * 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 *cpu = all_cpu_data[cpunum];
+	u64 desired_perf = hisi_cppc_get_real_perf(cpunum);
+
+	return cppc_cpufreq_perf_to_khz(cpu, desired_perf);
+}
+#endif
+
+static struct cppc_get_rate_workaround_info wa_info[] = {
+#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
+	{
+		.oem_id		= "HISI  ",
+		.oem_table_id	= "HIP07   ",
+		.oem_revision	= 0,
+		.get = hisi_cppc_cpufreq_get_rate,
+	},
+	{
+		.oem_id		= "HISI  ",
+		.oem_table_id	= "HIP08   ",
+		.oem_revision	= 0,
+		.get = hisi_cppc_cpufreq_get_rate,
+	},
+#endif
+	{},
+};
+
+static void cppc_check_get_rate_workaround(struct cpufreq_driver *cppc_cpufreq_driver)
+{
+	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;
+
+	for (i = 0; i < ARRAY_SIZE(wa_info) - 1; 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_cpufreq_driver->get = wa_info[i].get;
+			return;
+		}
+	}
+}
 static int __init cppc_cpufreq_init(void)
 {
 	int i, ret = 0;
@@ -386,6 +446,8 @@  static int __init cppc_cpufreq_init(void)
 		goto out;
 	}
 
+	cppc_check_get_rate_workaround(&cppc_cpufreq_driver);
+
 	ret = cpufreq_register_driver(&cppc_cpufreq_driver);
 	if (ret)
 		goto out;
diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
index 4f34734..be7400b 100644
--- a/include/acpi/cppc_acpi.h
+++ b/include/acpi/cppc_acpi.h
@@ -146,4 +146,8 @@  struct cppc_cpudata {
 extern int cpc_read_ffh(int cpunum, struct cpc_reg *reg, u64 *val);
 extern int cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val);
 
+#ifdef CONFIG_HISILICON_CPPC_CPUFREQ_WORKAROUND
+u64 hisi_cppc_get_real_perf(unsigned int cpunum);
+#endif
+
 #endif /* _CPPC_ACPI_H*/