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

[Ilpo Järvinen]

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@xxxxxxxxxxxxxxx>
> ---
>  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);
>