diff mbox series

[v3,15/17] driver/cpufreq: enable Hygon support to cpufreq driver

Message ID 36b1edf4bd9e203f8e14fa5b90a5a197fdf13898.1533989493.git.puwen@hygon.cn (mailing list archive)
State Not Applicable, archived
Headers show
Series Add support for Hygon Dhyana Family 18h processor | expand

Commit Message

Pu Wen Aug. 11, 2018, 1:29 p.m. UTC 
Enable ACPI cpufreq driver support for Hygon by adding family ID check
along with AMD.

As Hygon platforms have SMBus device(PCI device ID 0x790b), enable Hygon
support to function amd_freq_sensitivity_init().

Signed-off-by: Pu Wen <puwen@hygon.cn>
---
 drivers/cpufreq/acpi-cpufreq.c         | 5 +++++
 drivers/cpufreq/amd_freq_sensitivity.c | 9 +++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

Comments

Rafael J. Wysocki Aug. 12, 2018, 9:55 a.m. UTC | #1 
On Sat, Aug 11, 2018 at 3:36 PM Pu Wen <puwen@hygon.cn> wrote:
>
> Enable ACPI cpufreq driver support for Hygon by adding family ID check
> along with AMD.
>
> As Hygon platforms have SMBus device(PCI device ID 0x790b), enable Hygon
> support to function amd_freq_sensitivity_init().
>
> Signed-off-by: Pu Wen <puwen@hygon.cn>

Is there any technical difference between HYGON and AMD?

You seem to be mechanically adding X86_VENDOR_HYGON wherever
X86_VENDOR_AMD is used.

> ---
>  drivers/cpufreq/acpi-cpufreq.c         | 5 +++++
>  drivers/cpufreq/amd_freq_sensitivity.c | 9 +++++++--
>  2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index b61f4ec..d62fd37 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -61,6 +61,7 @@ enum {
>
>  #define INTEL_MSR_RANGE                (0xffff)
>  #define AMD_MSR_RANGE          (0x7)
> +#define HYGON_MSR_RANGE                (0x7)
>
>  #define MSR_K7_HWCR_CPB_DIS    (1ULL << 25)
>
> @@ -95,6 +96,7 @@ static bool boost_state(unsigned int cpu)
>                 rdmsr_on_cpu(cpu, MSR_IA32_MISC_ENABLE, &lo, &hi);
>                 msr = lo | ((u64)hi << 32);
>                 return !(msr & MSR_IA32_MISC_ENABLE_TURBO_DISABLE);
> +       case X86_VENDOR_HYGON:
>         case X86_VENDOR_AMD:
>                 rdmsr_on_cpu(cpu, MSR_K7_HWCR, &lo, &hi);
>                 msr = lo | ((u64)hi << 32);
> @@ -113,6 +115,7 @@ static int boost_set_msr(bool enable)
>                 msr_addr = MSR_IA32_MISC_ENABLE;
>                 msr_mask = MSR_IA32_MISC_ENABLE_TURBO_DISABLE;
>                 break;
> +       case X86_VENDOR_HYGON:
>         case X86_VENDOR_AMD:
>                 msr_addr = MSR_K7_HWCR;
>                 msr_mask = MSR_K7_HWCR_CPB_DIS;
> @@ -225,6 +228,8 @@ static unsigned extract_msr(struct cpufreq_policy *policy, u32 msr)
>
>         if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
>                 msr &= AMD_MSR_RANGE;
> +       else if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON)
> +               msr &= HYGON_MSR_RANGE;
>         else
>                 msr &= INTEL_MSR_RANGE;
>
> diff --git a/drivers/cpufreq/amd_freq_sensitivity.c b/drivers/cpufreq/amd_freq_sensitivity.c
> index be926d9..4ac7c3c 100644
> --- a/drivers/cpufreq/amd_freq_sensitivity.c
> +++ b/drivers/cpufreq/amd_freq_sensitivity.c
> @@ -111,11 +111,16 @@ static int __init amd_freq_sensitivity_init(void)
>  {
>         u64 val;
>         struct pci_dev *pcidev;
> +       unsigned int pci_vendor;
>
> -       if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
> +       if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
> +               pci_vendor = PCI_VENDOR_ID_AMD;
> +       else if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON)
> +               pci_vendor = PCI_VENDOR_ID_HYGON;
> +       else
>                 return -ENODEV;
>
> -       pcidev = pci_get_device(PCI_VENDOR_ID_AMD,
> +       pcidev = pci_get_device(pci_vendor,
>                         PCI_DEVICE_ID_AMD_KERNCZ_SMBUS, NULL);
>
>         if (!pcidev) {
> --
> 2.7.4
>
Pu Wen Aug. 13, 2018, 4:22 p.m. UTC | #2 
On 2018/8/12 17:55, Rafael J. Wysocki wrote:
> On Sat, Aug 11, 2018 at 3:36 PM Pu Wen <puwen@hygon.cn> wrote:
>>
>> Enable ACPI cpufreq driver support for Hygon by adding family ID check
>> along with AMD.
>>
>> As Hygon platforms have SMBus device(PCI device ID 0x790b), enable Hygon
>> support to function amd_freq_sensitivity_init().
>>
>> Signed-off-by: Pu Wen <puwen@hygon.cn>
> 
> Is there any technical difference between HYGON and AMD?

For ACPI cpufreq patch, you are right. But for the whole point of view,
there has some technical difference between Hygon Dhyana and AMD Family
17h.
For cpufreq if not added X86_VENDOR_HYGON codes, this driver will not
work functionally on Hygon platforms.

> 
> You seem to be mechanically adding X86_VENDOR_HYGON wherever
> X86_VENDOR_AMD is used.

X86_VENDOR_HYGON is not mechanically added wherever X86_VENDOR_AMD is
used, we have reviewed and tested the features wherever X86_VENDOR_HYGON
is needed.

As Hygon Dhyana can share code path with AMD family 17h, to minimize the
code duplication, we choose to reuse the AMD's codes here.

Thanks,
Pu Wen

> 
>> ---
>>   drivers/cpufreq/acpi-cpufreq.c         | 5 +++++
>>   drivers/cpufreq/amd_freq_sensitivity.c | 9 +++++++--
>>   2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
>> index b61f4ec..d62fd37 100644
>> --- a/drivers/cpufreq/acpi-cpufreq.c
>> +++ b/drivers/cpufreq/acpi-cpufreq.c
>> @@ -61,6 +61,7 @@ enum {
>>
>>   #define INTEL_MSR_RANGE                (0xffff)
>>   #define AMD_MSR_RANGE          (0x7)
>> +#define HYGON_MSR_RANGE                (0x7)
>>
>>   #define MSR_K7_HWCR_CPB_DIS    (1ULL << 25)
>>
>> @@ -95,6 +96,7 @@ static bool boost_state(unsigned int cpu)
>>                  rdmsr_on_cpu(cpu, MSR_IA32_MISC_ENABLE, &lo, &hi);
>>                  msr = lo | ((u64)hi << 32);
>>                  return !(msr & MSR_IA32_MISC_ENABLE_TURBO_DISABLE);
>> +       case X86_VENDOR_HYGON:
>>          case X86_VENDOR_AMD:
>>                  rdmsr_on_cpu(cpu, MSR_K7_HWCR, &lo, &hi);
>>                  msr = lo | ((u64)hi << 32);
Rafael J. Wysocki Aug. 14, 2018, 10:11 a.m. UTC | #3 
On Monday, August 13, 2018 6:22:16 PM CEST Pu Wen wrote:
> On 2018/8/12 17:55, Rafael J. Wysocki wrote:
> > On Sat, Aug 11, 2018 at 3:36 PM Pu Wen <puwen@hygon.cn> wrote:
> >>
> >> Enable ACPI cpufreq driver support for Hygon by adding family ID check
> >> along with AMD.
> >>
> >> As Hygon platforms have SMBus device(PCI device ID 0x790b), enable Hygon
> >> support to function amd_freq_sensitivity_init().
> >>
> >> Signed-off-by: Pu Wen <puwen@hygon.cn>
> > 
> > Is there any technical difference between HYGON and AMD?
> 
> For ACPI cpufreq patch, you are right. But for the whole point of view,
> there has some technical difference between Hygon Dhyana and AMD Family
> 17h.
> For cpufreq if not added X86_VENDOR_HYGON codes, this driver will not
> work functionally on Hygon platforms.
> 
> > 
> > You seem to be mechanically adding X86_VENDOR_HYGON wherever
> > X86_VENDOR_AMD is used.
> 
> X86_VENDOR_HYGON is not mechanically added wherever X86_VENDOR_AMD is
> used, we have reviewed and tested the features wherever X86_VENDOR_HYGON
> is needed.
> 
> As Hygon Dhyana can share code path with AMD family 17h, to minimize the
> code duplication, we choose to reuse the AMD's codes here.

OK

The cpufreq changes are fine by me.

Thanks!
Rafael J. Wysocki Aug. 14, 2018, 10:12 a.m. UTC | #4 
On Saturday, August 11, 2018 3:29:52 PM CEST Pu Wen wrote:
> Enable ACPI cpufreq driver support for Hygon by adding family ID check
> along with AMD.
> 
> As Hygon platforms have SMBus device(PCI device ID 0x790b), enable Hygon
> support to function amd_freq_sensitivity_init().
> 
> Signed-off-by: Pu Wen <puwen@hygon.cn>
> ---
>  drivers/cpufreq/acpi-cpufreq.c         | 5 +++++
>  drivers/cpufreq/amd_freq_sensitivity.c | 9 +++++++--
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index b61f4ec..d62fd37 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -61,6 +61,7 @@ enum {
>  
>  #define INTEL_MSR_RANGE		(0xffff)
>  #define AMD_MSR_RANGE		(0x7)
> +#define HYGON_MSR_RANGE		(0x7)
>  
>  #define MSR_K7_HWCR_CPB_DIS	(1ULL << 25)
>  
> @@ -95,6 +96,7 @@ static bool boost_state(unsigned int cpu)
>  		rdmsr_on_cpu(cpu, MSR_IA32_MISC_ENABLE, &lo, &hi);
>  		msr = lo | ((u64)hi << 32);
>  		return !(msr & MSR_IA32_MISC_ENABLE_TURBO_DISABLE);
> +	case X86_VENDOR_HYGON:
>  	case X86_VENDOR_AMD:
>  		rdmsr_on_cpu(cpu, MSR_K7_HWCR, &lo, &hi);
>  		msr = lo | ((u64)hi << 32);
> @@ -113,6 +115,7 @@ static int boost_set_msr(bool enable)
>  		msr_addr = MSR_IA32_MISC_ENABLE;
>  		msr_mask = MSR_IA32_MISC_ENABLE_TURBO_DISABLE;
>  		break;
> +	case X86_VENDOR_HYGON:
>  	case X86_VENDOR_AMD:
>  		msr_addr = MSR_K7_HWCR;
>  		msr_mask = MSR_K7_HWCR_CPB_DIS;
> @@ -225,6 +228,8 @@ static unsigned extract_msr(struct cpufreq_policy *policy, u32 msr)
>  
>  	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
>  		msr &= AMD_MSR_RANGE;
> +	else if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON)
> +		msr &= HYGON_MSR_RANGE;
>  	else
>  		msr &= INTEL_MSR_RANGE;
>  
> diff --git a/drivers/cpufreq/amd_freq_sensitivity.c b/drivers/cpufreq/amd_freq_sensitivity.c
> index be926d9..4ac7c3c 100644
> --- a/drivers/cpufreq/amd_freq_sensitivity.c
> +++ b/drivers/cpufreq/amd_freq_sensitivity.c
> @@ -111,11 +111,16 @@ static int __init amd_freq_sensitivity_init(void)
>  {
>  	u64 val;
>  	struct pci_dev *pcidev;
> +	unsigned int pci_vendor;
>  
> -	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
> +	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
> +		pci_vendor = PCI_VENDOR_ID_AMD;
> +	else if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON)
> +		pci_vendor = PCI_VENDOR_ID_HYGON;
> +	else
>  		return -ENODEV;
>  
> -	pcidev = pci_get_device(PCI_VENDOR_ID_AMD,
> +	pcidev = pci_get_device(pci_vendor,
>  			PCI_DEVICE_ID_AMD_KERNCZ_SMBUS, NULL);
>  
>  	if (!pcidev) {
> 

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
diff mbox series

Patch

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index b61f4ec..d62fd37 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -61,6 +61,7 @@  enum {
 
 #define INTEL_MSR_RANGE		(0xffff)
 #define AMD_MSR_RANGE		(0x7)
+#define HYGON_MSR_RANGE		(0x7)
 
 #define MSR_K7_HWCR_CPB_DIS	(1ULL << 25)
 
@@ -95,6 +96,7 @@  static bool boost_state(unsigned int cpu)
 		rdmsr_on_cpu(cpu, MSR_IA32_MISC_ENABLE, &lo, &hi);
 		msr = lo | ((u64)hi << 32);
 		return !(msr & MSR_IA32_MISC_ENABLE_TURBO_DISABLE);
+	case X86_VENDOR_HYGON:
 	case X86_VENDOR_AMD:
 		rdmsr_on_cpu(cpu, MSR_K7_HWCR, &lo, &hi);
 		msr = lo | ((u64)hi << 32);
@@ -113,6 +115,7 @@  static int boost_set_msr(bool enable)
 		msr_addr = MSR_IA32_MISC_ENABLE;
 		msr_mask = MSR_IA32_MISC_ENABLE_TURBO_DISABLE;
 		break;
+	case X86_VENDOR_HYGON:
 	case X86_VENDOR_AMD:
 		msr_addr = MSR_K7_HWCR;
 		msr_mask = MSR_K7_HWCR_CPB_DIS;
@@ -225,6 +228,8 @@  static unsigned extract_msr(struct cpufreq_policy *policy, u32 msr)
 
 	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
 		msr &= AMD_MSR_RANGE;
+	else if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON)
+		msr &= HYGON_MSR_RANGE;
 	else
 		msr &= INTEL_MSR_RANGE;
 
diff --git a/drivers/cpufreq/amd_freq_sensitivity.c b/drivers/cpufreq/amd_freq_sensitivity.c
index be926d9..4ac7c3c 100644
--- a/drivers/cpufreq/amd_freq_sensitivity.c
+++ b/drivers/cpufreq/amd_freq_sensitivity.c
@@ -111,11 +111,16 @@  static int __init amd_freq_sensitivity_init(void)
 {
 	u64 val;
 	struct pci_dev *pcidev;
+	unsigned int pci_vendor;
 
-	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
+	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
+		pci_vendor = PCI_VENDOR_ID_AMD;
+	else if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON)
+		pci_vendor = PCI_VENDOR_ID_HYGON;
+	else
 		return -ENODEV;
 
-	pcidev = pci_get_device(PCI_VENDOR_ID_AMD,
+	pcidev = pci_get_device(pci_vendor,
 			PCI_DEVICE_ID_AMD_KERNCZ_SMBUS, NULL);
 
 	if (!pcidev) {