platform/x86/intel/pmc: Add Arrow Lake U/H support to intel_pmc_core driver

[PATCH] platform/x86/intel/pmc: Add Arrow Lake U/H support to intel_pmc_core driver

Posted by Rajvi Jingar 2 months, 3 weeks ago

Add Arrow Lake U and Arro Lake H support in intel_pmc_core driver

Signed-off-by: Rajvi Jingar <rajvi.jingar@linux.intel.com>
---
 drivers/platform/x86/intel/pmc/arl.c  | 65 ++++++++++++++++++++++-----
 drivers/platform/x86/intel/pmc/core.c |  2 +
 drivers/platform/x86/intel/pmc/core.h |  7 +++
 3 files changed, 64 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/x86/intel/pmc/arl.c b/drivers/platform/x86/intel/pmc/arl.c
index e10527c4e3e0..964f5f040dd9 100644
--- a/drivers/platform/x86/intel/pmc/arl.c
+++ b/drivers/platform/x86/intel/pmc/arl.c
@@ -16,6 +16,7 @@
 #define IOEP_LPM_REQ_GUID	0x5077612
 #define SOCS_LPM_REQ_GUID	0x8478657
 #define PCHS_LPM_REQ_GUID	0x9684572
+#define SOCM_LPM_REQ_GUID	0x2625030
 
 static const u8 ARL_LPM_REG_INDEX[] = {0, 4, 5, 6, 8, 9, 10, 11, 12, 13, 14, 15, 16, 20};
 
@@ -650,6 +651,7 @@ const struct pmc_reg_map arl_pchs_reg_map = {
 	.etr3_offset = ETR3_OFFSET,
 };
 
+#define PMC_DEVID_SOCM 0x777f
 #define PMC_DEVID_SOCS 0xae7f
 #define PMC_DEVID_IOEP 0x7ecf
 #define PMC_DEVID_PCHS 0x7f27
@@ -669,11 +671,17 @@ static struct pmc_info arl_pmc_info_list[] = {
 		.devid	= PMC_DEVID_PCHS,
 		.map	= &arl_pchs_reg_map,
 	},
+	{
+		.guid	= SOCM_LPM_REQ_GUID,
+		.devid	= PMC_DEVID_SOCM,
+		.map	= &mtl_socm_reg_map,
+	},
 	{}
 };
 
 #define ARL_NPU_PCI_DEV			0xad1d
 #define ARL_GNA_PCI_DEV			0xae4c
+#define ARL_H_GNA_PCI_DEV 		0x774c
 /*
  * Set power state of select devices that do not have drivers to D3
  * so that they do not block Package C entry.
@@ -684,6 +692,12 @@ static void arl_d3_fixup(void)
 	pmc_core_set_device_d3(ARL_GNA_PCI_DEV);
 }
 
+static void arl_h_d3_fixup(void)
+{
+	pmc_core_set_device_d3(ARL_NPU_PCI_DEV);
+	pmc_core_set_device_d3(ARL_H_GNA_PCI_DEV);
+}
+
 static int arl_resume(struct pmc_dev *pmcdev)
 {
 	arl_d3_fixup();
@@ -692,16 +706,47 @@ static int arl_resume(struct pmc_dev *pmcdev)
 	return pmc_core_resume_common(pmcdev);
 }
 
+static int arl_h_resume(struct pmc_dev *pmcdev)
+{
+	arl_h_d3_fixup();
+	pmc_core_send_ltr_ignore(pmcdev, 3, 0);
+
+	return pmc_core_resume_common(pmcdev);
+}
+
+int arl_h_core_init(struct pmc_dev *pmcdev)
+{
+	arl_h_d3_fixup();
+
+	return arl_core_generic_init(pmcdev, SOC_M);
+}
+
 int arl_core_init(struct pmc_dev *pmcdev)
 {
-	struct pmc *pmc = pmcdev->pmcs[PMC_IDX_SOC];
+	arl_d3_fixup();
+
+	return arl_core_generic_init(pmcdev, SOC_S);
+}
+
+int arl_core_generic_init(struct pmc_dev *pmcdev, int soc_tp)
+{
 	int ret;
-	int func = 0;
+	int func;
 	bool ssram_init = true;
+	struct pmc *pmc = pmcdev->pmcs[PMC_IDX_SOC];
 
-	arl_d3_fixup();
 	pmcdev->suspend = cnl_suspend;
-	pmcdev->resume = arl_resume;
+
+	if (soc_tp == SOC_M) {
+		func = 2;
+		pmcdev->resume = arl_h_resume;
+	} else if (soc_tp == SOC_S) {
+		func = 0;
+		pmcdev->resume = arl_resume;
+	} else {
+		return -EINVAL;
+	}
+
 	pmcdev->regmap_list = arl_pmc_info_list;
 
 	/*
@@ -711,7 +756,10 @@ int arl_core_init(struct pmc_dev *pmcdev)
 	ret = pmc_core_ssram_init(pmcdev, func);
 	if (ret) {
 		ssram_init = false;
-		pmc->map = &arl_socs_reg_map;
+		if (soc_tp == SOC_M)
+			pmc->map = &mtl_socm_reg_map;
+		else
+			pmc->map = &arl_socs_reg_map;
 
 		ret = get_primary_reg_base(pmc);
 		if (ret)
@@ -721,11 +769,8 @@ int arl_core_init(struct pmc_dev *pmcdev)
 	pmc_core_get_low_power_modes(pmcdev);
 	pmc_core_punit_pmt_init(pmcdev, ARL_PMT_DMU_GUID);
 
-	if (ssram_init)	{
-		ret = pmc_core_ssram_get_lpm_reqs(pmcdev);
-		if (ret)
-			return ret;
-	}
+	if (ssram_init)
+		return pmc_core_ssram_get_lpm_reqs(pmcdev);
 
 	return 0;
 }
diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
index 01ae71c6df59..046791f21619 100644
--- a/drivers/platform/x86/intel/pmc/core.c
+++ b/drivers/platform/x86/intel/pmc/core.c
@@ -1319,6 +1319,8 @@ static const struct x86_cpu_id intel_pmc_core_ids[] = {
 	X86_MATCH_VFM(INTEL_RAPTORLAKE_S,	adl_core_init),
 	X86_MATCH_VFM(INTEL_METEORLAKE_L,	mtl_core_init),
 	X86_MATCH_VFM(INTEL_ARROWLAKE,		arl_core_init),
+	X86_MATCH_VFM(INTEL_ARROWLAKE_H,	arl_h_core_init),
+	X86_MATCH_VFM(INTEL_ARROWLAKE_U,	arl_h_core_init),
 	X86_MATCH_VFM(INTEL_LUNARLAKE_M,	lnl_core_init),
 	{}
 };
diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
index ea04de7eb9e8..5771f40185c2 100644
--- a/drivers/platform/x86/intel/pmc/core.h
+++ b/drivers/platform/x86/intel/pmc/core.h
@@ -365,6 +365,11 @@ struct pmc_info {
 	const struct pmc_reg_map *map;
 };
 
+enum soc_type {
+	SOC_M,
+	SOC_S
+};
+
 /**
  * struct pmc - pmc private info structure
  * @base_addr:		contains pmc base address
@@ -599,6 +604,8 @@ int tgl_core_generic_init(struct pmc_dev *pmcdev, int pch_tp);
 int adl_core_init(struct pmc_dev *pmcdev);
 int mtl_core_init(struct pmc_dev *pmcdev);
 int arl_core_init(struct pmc_dev *pmcdev);
+int arl_h_core_init(struct pmc_dev *pmcdev);
+int arl_core_generic_init(struct pmc_dev *pmcdev, int pch_tp);
 int lnl_core_init(struct pmc_dev *pmcdev);
 
 void cnl_suspend(struct pmc_dev *pmcdev);
-- 
2.43.0

Re: [PATCH] platform/x86/intel/pmc: Add Arrow Lake U/H support to intel_pmc_core driver

Posted by Ilpo Järvinen 2 months, 2 weeks ago

On Mon, 9 Sep 2024, Rajvi Jingar wrote:

> Add Arrow Lake U and Arro Lake H support in intel_pmc_core driver

Arrow

Add . to the end of sentence.

> Signed-off-by: Rajvi Jingar <rajvi.jingar@linux.intel.com>
> ---
>  drivers/platform/x86/intel/pmc/arl.c  | 65 ++++++++++++++++++++++-----
>  drivers/platform/x86/intel/pmc/core.c |  2 +
>  drivers/platform/x86/intel/pmc/core.h |  7 +++
>  3 files changed, 64 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/pmc/arl.c b/drivers/platform/x86/intel/pmc/arl.c
> index e10527c4e3e0..964f5f040dd9 100644
> --- a/drivers/platform/x86/intel/pmc/arl.c
> +++ b/drivers/platform/x86/intel/pmc/arl.c
> @@ -16,6 +16,7 @@
>  #define IOEP_LPM_REQ_GUID	0x5077612
>  #define SOCS_LPM_REQ_GUID	0x8478657
>  #define PCHS_LPM_REQ_GUID	0x9684572
> +#define SOCM_LPM_REQ_GUID	0x2625030
>  
>  static const u8 ARL_LPM_REG_INDEX[] = {0, 4, 5, 6, 8, 9, 10, 11, 12, 13, 14, 15, 16, 20};
>  
> @@ -650,6 +651,7 @@ const struct pmc_reg_map arl_pchs_reg_map = {
>  	.etr3_offset = ETR3_OFFSET,
>  };
>  
> +#define PMC_DEVID_SOCM 0x777f
>  #define PMC_DEVID_SOCS 0xae7f
>  #define PMC_DEVID_IOEP 0x7ecf
>  #define PMC_DEVID_PCHS 0x7f27
> @@ -669,11 +671,17 @@ static struct pmc_info arl_pmc_info_list[] = {
>  		.devid	= PMC_DEVID_PCHS,
>  		.map	= &arl_pchs_reg_map,
>  	},
> +	{
> +		.guid	= SOCM_LPM_REQ_GUID,
> +		.devid	= PMC_DEVID_SOCM,
> +		.map	= &mtl_socm_reg_map,
> +	},
>  	{}
>  };
>  
>  #define ARL_NPU_PCI_DEV			0xad1d
>  #define ARL_GNA_PCI_DEV			0xae4c
> +#define ARL_H_GNA_PCI_DEV 		0x774c
>  /*
>   * Set power state of select devices that do not have drivers to D3
>   * so that they do not block Package C entry.
> @@ -684,6 +692,12 @@ static void arl_d3_fixup(void)
>  	pmc_core_set_device_d3(ARL_GNA_PCI_DEV);
>  }
>  
> +static void arl_h_d3_fixup(void)
> +{
> +	pmc_core_set_device_d3(ARL_NPU_PCI_DEV);
> +	pmc_core_set_device_d3(ARL_H_GNA_PCI_DEV);
> +}
> +
>  static int arl_resume(struct pmc_dev *pmcdev)
>  {
>  	arl_d3_fixup();
> @@ -692,16 +706,47 @@ static int arl_resume(struct pmc_dev *pmcdev)
>  	return pmc_core_resume_common(pmcdev);
>  }
>  
> +static int arl_h_resume(struct pmc_dev *pmcdev)
> +{
> +	arl_h_d3_fixup();
> +	pmc_core_send_ltr_ignore(pmcdev, 3, 0);
> +
> +	return pmc_core_resume_common(pmcdev);
> +}
> +
> +int arl_h_core_init(struct pmc_dev *pmcdev)
> +{
> +	arl_h_d3_fixup();
> +
> +	return arl_core_generic_init(pmcdev, SOC_M);
> +}
> +
>  int arl_core_init(struct pmc_dev *pmcdev)
>  {
> -	struct pmc *pmc = pmcdev->pmcs[PMC_IDX_SOC];
> +	arl_d3_fixup();
> +
> +	return arl_core_generic_init(pmcdev, SOC_S);
> +}
> +
> +int arl_core_generic_init(struct pmc_dev *pmcdev, int soc_tp)

This function has no callers outside or arl.c and should be made static 
and the code should be reordered such that no prototype is needed.

> +{
>  	int ret;
> -	int func = 0;
> +	int func;
>  	bool ssram_init = true;
> +	struct pmc *pmc = pmcdev->pmcs[PMC_IDX_SOC];
>  
> -	arl_d3_fixup();
>  	pmcdev->suspend = cnl_suspend;
> -	pmcdev->resume = arl_resume;
> +
> +	if (soc_tp == SOC_M) {
> +		func = 2;
> +		pmcdev->resume = arl_h_resume;
> +	} else if (soc_tp == SOC_S) {
> +		func = 0;
> +		pmcdev->resume = arl_resume;

It would be preferrable to make an info structure to describe this kind 
of platform variations so that if () forests like this are avoided.

> +	} else {
> +		return -EINVAL;
> +	}
> +
>  	pmcdev->regmap_list = arl_pmc_info_list;
>  
>  	/*
> @@ -711,7 +756,10 @@ int arl_core_init(struct pmc_dev *pmcdev)
>  	ret = pmc_core_ssram_init(pmcdev, func);
>  	if (ret) {
>  		ssram_init = false;
> -		pmc->map = &arl_socs_reg_map;
> +		if (soc_tp == SOC_M)
> +			pmc->map = &mtl_socm_reg_map;
> +		else
> +			pmc->map = &arl_socs_reg_map;

As with above, use an info struct.

-- 
 i.

>  
>  		ret = get_primary_reg_base(pmc);
>  		if (ret)
> @@ -721,11 +769,8 @@ int arl_core_init(struct pmc_dev *pmcdev)
>  	pmc_core_get_low_power_modes(pmcdev);
>  	pmc_core_punit_pmt_init(pmcdev, ARL_PMT_DMU_GUID);
>  
> -	if (ssram_init)	{
> -		ret = pmc_core_ssram_get_lpm_reqs(pmcdev);
> -		if (ret)
> -			return ret;
> -	}
> +	if (ssram_init)
> +		return pmc_core_ssram_get_lpm_reqs(pmcdev);
>  
>  	return 0;
>  }
> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
> index 01ae71c6df59..046791f21619 100644
> --- a/drivers/platform/x86/intel/pmc/core.c
> +++ b/drivers/platform/x86/intel/pmc/core.c
> @@ -1319,6 +1319,8 @@ static const struct x86_cpu_id intel_pmc_core_ids[] = {
>  	X86_MATCH_VFM(INTEL_RAPTORLAKE_S,	adl_core_init),
>  	X86_MATCH_VFM(INTEL_METEORLAKE_L,	mtl_core_init),
>  	X86_MATCH_VFM(INTEL_ARROWLAKE,		arl_core_init),
> +	X86_MATCH_VFM(INTEL_ARROWLAKE_H,	arl_h_core_init),
> +	X86_MATCH_VFM(INTEL_ARROWLAKE_U,	arl_h_core_init),
>  	X86_MATCH_VFM(INTEL_LUNARLAKE_M,	lnl_core_init),
>  	{}
>  };
> diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
> index ea04de7eb9e8..5771f40185c2 100644
> --- a/drivers/platform/x86/intel/pmc/core.h
> +++ b/drivers/platform/x86/intel/pmc/core.h
> @@ -365,6 +365,11 @@ struct pmc_info {
>  	const struct pmc_reg_map *map;
>  };
>  
> +enum soc_type {
> +	SOC_M,
> +	SOC_S
> +};
> +
>  /**
>   * struct pmc - pmc private info structure
>   * @base_addr:		contains pmc base address
> @@ -599,6 +604,8 @@ int tgl_core_generic_init(struct pmc_dev *pmcdev, int pch_tp);
>  int adl_core_init(struct pmc_dev *pmcdev);
>  int mtl_core_init(struct pmc_dev *pmcdev);
>  int arl_core_init(struct pmc_dev *pmcdev);
> +int arl_h_core_init(struct pmc_dev *pmcdev);
> +int arl_core_generic_init(struct pmc_dev *pmcdev, int pch_tp);
>  int lnl_core_init(struct pmc_dev *pmcdev);
>  
>  void cnl_suspend(struct pmc_dev *pmcdev);
>

Re: [PATCH] platform/x86/intel/pmc: Add Arrow Lake U/H support to intel_pmc_core driver

Posted by rajvi.jingar 2 months, 2 weeks ago

Hi Ilpo,

Thanks for reviewing the patch.

Restructuring the code to handle platform variations won't be that 
straightforward, so we will be sending the next version in next cycle rc1.

Thanks,

Rajvi

On 9/10/2024 2:32 AM, Ilpo Järvinen wrote:

> On Mon, 9 Sep 2024, Rajvi Jingar wrote:
>
>> Add Arrow Lake U and Arro Lake H support in intel_pmc_core driver
> Arrow
>
> Add . to the end of sentence.
>
>> Signed-off-by: Rajvi Jingar <rajvi.jingar@linux.intel.com>
>> ---
>>   drivers/platform/x86/intel/pmc/arl.c  | 65 ++++++++++++++++++++++-----
>>   drivers/platform/x86/intel/pmc/core.c |  2 +
>>   drivers/platform/x86/intel/pmc/core.h |  7 +++
>>   3 files changed, 64 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/platform/x86/intel/pmc/arl.c b/drivers/platform/x86/intel/pmc/arl.c
>> index e10527c4e3e0..964f5f040dd9 100644
>> --- a/drivers/platform/x86/intel/pmc/arl.c
>> +++ b/drivers/platform/x86/intel/pmc/arl.c
>> @@ -16,6 +16,7 @@
>>   #define IOEP_LPM_REQ_GUID	0x5077612
>>   #define SOCS_LPM_REQ_GUID	0x8478657
>>   #define PCHS_LPM_REQ_GUID	0x9684572
>> +#define SOCM_LPM_REQ_GUID	0x2625030
>>   
>>   static const u8 ARL_LPM_REG_INDEX[] = {0, 4, 5, 6, 8, 9, 10, 11, 12, 13, 14, 15, 16, 20};
>>   
>> @@ -650,6 +651,7 @@ const struct pmc_reg_map arl_pchs_reg_map = {
>>   	.etr3_offset = ETR3_OFFSET,
>>   };
>>   
>> +#define PMC_DEVID_SOCM 0x777f
>>   #define PMC_DEVID_SOCS 0xae7f
>>   #define PMC_DEVID_IOEP 0x7ecf
>>   #define PMC_DEVID_PCHS 0x7f27
>> @@ -669,11 +671,17 @@ static struct pmc_info arl_pmc_info_list[] = {
>>   		.devid	= PMC_DEVID_PCHS,
>>   		.map	= &arl_pchs_reg_map,
>>   	},
>> +	{
>> +		.guid	= SOCM_LPM_REQ_GUID,
>> +		.devid	= PMC_DEVID_SOCM,
>> +		.map	= &mtl_socm_reg_map,
>> +	},
>>   	{}
>>   };
>>   
>>   #define ARL_NPU_PCI_DEV			0xad1d
>>   #define ARL_GNA_PCI_DEV			0xae4c
>> +#define ARL_H_GNA_PCI_DEV 		0x774c
>>   /*
>>    * Set power state of select devices that do not have drivers to D3
>>    * so that they do not block Package C entry.
>> @@ -684,6 +692,12 @@ static void arl_d3_fixup(void)
>>   	pmc_core_set_device_d3(ARL_GNA_PCI_DEV);
>>   }
>>   
>> +static void arl_h_d3_fixup(void)
>> +{
>> +	pmc_core_set_device_d3(ARL_NPU_PCI_DEV);
>> +	pmc_core_set_device_d3(ARL_H_GNA_PCI_DEV);
>> +}
>> +
>>   static int arl_resume(struct pmc_dev *pmcdev)
>>   {
>>   	arl_d3_fixup();
>> @@ -692,16 +706,47 @@ static int arl_resume(struct pmc_dev *pmcdev)
>>   	return pmc_core_resume_common(pmcdev);
>>   }
>>   
>> +static int arl_h_resume(struct pmc_dev *pmcdev)
>> +{
>> +	arl_h_d3_fixup();
>> +	pmc_core_send_ltr_ignore(pmcdev, 3, 0);
>> +
>> +	return pmc_core_resume_common(pmcdev);
>> +}
>> +
>> +int arl_h_core_init(struct pmc_dev *pmcdev)
>> +{
>> +	arl_h_d3_fixup();
>> +
>> +	return arl_core_generic_init(pmcdev, SOC_M);
>> +}
>> +
>>   int arl_core_init(struct pmc_dev *pmcdev)
>>   {
>> -	struct pmc *pmc = pmcdev->pmcs[PMC_IDX_SOC];
>> +	arl_d3_fixup();
>> +
>> +	return arl_core_generic_init(pmcdev, SOC_S);
>> +}
>> +
>> +int arl_core_generic_init(struct pmc_dev *pmcdev, int soc_tp)
> This function has no callers outside or arl.c and should be made static
> and the code should be reordered such that no prototype is needed.
>
>> +{
>>   	int ret;
>> -	int func = 0;
>> +	int func;
>>   	bool ssram_init = true;
>> +	struct pmc *pmc = pmcdev->pmcs[PMC_IDX_SOC];
>>   
>> -	arl_d3_fixup();
>>   	pmcdev->suspend = cnl_suspend;
>> -	pmcdev->resume = arl_resume;
>> +
>> +	if (soc_tp == SOC_M) {
>> +		func = 2;
>> +		pmcdev->resume = arl_h_resume;
>> +	} else if (soc_tp == SOC_S) {
>> +		func = 0;
>> +		pmcdev->resume = arl_resume;
> It would be preferrable to make an info structure to describe this kind
> of platform variations so that if () forests like this are avoided.
>
>> +	} else {
>> +		return -EINVAL;
>> +	}
>> +
>>   	pmcdev->regmap_list = arl_pmc_info_list;
>>   
>>   	/*
>> @@ -711,7 +756,10 @@ int arl_core_init(struct pmc_dev *pmcdev)
>>   	ret = pmc_core_ssram_init(pmcdev, func);
>>   	if (ret) {
>>   		ssram_init = false;
>> -		pmc->map = &arl_socs_reg_map;
>> +		if (soc_tp == SOC_M)
>> +			pmc->map = &mtl_socm_reg_map;
>> +		else
>> +			pmc->map = &arl_socs_reg_map;
> As with above, use an info struct.
>