[PATCH v3] platform/x86/amd/pmc: adjust getting DRAM size behavior

[PATCH v3] platform/x86/amd/pmc: adjust getting DRAM size behavior

[Shyam Sundar S K]

After talking to the PMFW team, its understood that the "get dram size"
mbox command would only be supported on specific platforms (like Mendocino)
and not all. So, adjust getting DRAM size behavior such that,

- if that's Rembrandt or Mendocino and the underlying PMFW knows how
to execute the "get dram size" command it shall give the custom dram size.

- if the underlying FW does not report the dram size, we just proceed
further and assign the default dram size.

Cc: Mark Hasemeyer <markhas@chromium.org>
Link: https://lore.kernel.org/platform-driver-x86/3b224c62-a1d8-41bd-aced-5825f5f20e66@amd.com/
Fixes: be8325fb3d8c ("platform/x86/amd: pmc: Get STB DRAM size from PMFW")
Suggested-by: Sanket Goswami <Sanket.Goswami@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
v3:
- Based on review-ilpo branch
- Remove amd_pmc_get_dram_size() function
- Remove prints that are noisy

v2:
- Based on review-ilpo branch
- Drop calling get smu version from probe.

 drivers/platform/x86/amd/pmc/pmc.c | 31 ++----------------------------
 1 file changed, 2 insertions(+), 29 deletions(-)

diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
index cd6ac04c1468..c3104714b480 100644
--- a/drivers/platform/x86/amd/pmc/pmc.c
+++ b/drivers/platform/x86/amd/pmc/pmc.c
@@ -964,33 +964,6 @@ static const struct pci_device_id pmc_pci_ids[] = {
 	{ }
 };
 
-static int amd_pmc_get_dram_size(struct amd_pmc_dev *dev)
-{
-	int ret;
-
-	switch (dev->cpu_id) {
-	case AMD_CPU_ID_YC:
-		if (!(dev->major > 90 || (dev->major == 90 && dev->minor > 39))) {
-			ret = -EINVAL;
-			goto err_dram_size;
-		}
-		break;
-	default:
-		ret = -EINVAL;
-		goto err_dram_size;
-	}
-
-	ret = amd_pmc_send_cmd(dev, S2D_DRAM_SIZE, &dev->dram_size, dev->s2d_msg_id, true);
-	if (ret || !dev->dram_size)
-		goto err_dram_size;
-
-	return 0;
-
-err_dram_size:
-	dev_err(dev->dev, "DRAM size command not supported for this platform\n");
-	return ret;
-}
-
 static int amd_pmc_s2d_init(struct amd_pmc_dev *dev)
 {
 	u32 phys_addr_low, phys_addr_hi;
@@ -1009,8 +982,8 @@ static int amd_pmc_s2d_init(struct amd_pmc_dev *dev)
 		return -EIO;
 
 	/* Get DRAM size */
-	ret = amd_pmc_get_dram_size(dev);
-	if (ret)
+	ret = amd_pmc_send_cmd(dev, S2D_DRAM_SIZE, &dev->dram_size, dev->s2d_msg_id, true);
+	if (ret || !dev->dram_size)
 		dev->dram_size = S2D_TELEMETRY_DRAMBYTES_MAX;
 
 	/* Get STB DRAM address */
-- 
2.25.1

Re: [PATCH v3] platform/x86/amd/pmc: adjust getting DRAM size behavior

[Ilpo Järvinen]

On Thu, 16 Nov 2023, Shyam Sundar S K wrote:

> After talking to the PMFW team, its understood that the "get dram size"
> mbox command would only be supported on specific platforms (like Mendocino)
> and not all. So, adjust getting DRAM size behavior such that,
> 
> - if that's Rembrandt or Mendocino and the underlying PMFW knows how
> to execute the "get dram size" command it shall give the custom dram size.
> 
> - if the underlying FW does not report the dram size, we just proceed
> further and assign the default dram size.

Hi,

This still talks only about the solution but I gave you a specific 
instruction to include the description of the problem that warrants
Fixes tag. :-(

So please include something along the lines you had in v1 to describe how 
the current code fails because the version numbers are not initialized.

> Cc: Mark Hasemeyer <markhas@chromium.org>

Promote Mark to Reported-by.

Codewise, this is seems fine and is IMO nice simplification.

-- 
 i.

> Link: https://lore.kernel.org/platform-driver-x86/3b224c62-a1d8-41bd-aced-5825f5f20e66@amd.com/
> Fixes: be8325fb3d8c ("platform/x86/amd: pmc: Get STB DRAM size from PMFW")
> Suggested-by: Sanket Goswami <Sanket.Goswami@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
> v3:
> - Based on review-ilpo branch
> - Remove amd_pmc_get_dram_size() function
> - Remove prints that are noisy
> 
> v2:
> - Based on review-ilpo branch
> - Drop calling get smu version from probe.
> 
>  drivers/platform/x86/amd/pmc/pmc.c | 31 ++----------------------------
>  1 file changed, 2 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
> index cd6ac04c1468..c3104714b480 100644
> --- a/drivers/platform/x86/amd/pmc/pmc.c
> +++ b/drivers/platform/x86/amd/pmc/pmc.c
> @@ -964,33 +964,6 @@ static const struct pci_device_id pmc_pci_ids[] = {
>  	{ }
>  };
>  
> -static int amd_pmc_get_dram_size(struct amd_pmc_dev *dev)
> -{
> -	int ret;
> -
> -	switch (dev->cpu_id) {
> -	case AMD_CPU_ID_YC:
> -		if (!(dev->major > 90 || (dev->major == 90 && dev->minor > 39))) {
> -			ret = -EINVAL;
> -			goto err_dram_size;
> -		}
> -		break;
> -	default:
> -		ret = -EINVAL;
> -		goto err_dram_size;
> -	}
> -
> -	ret = amd_pmc_send_cmd(dev, S2D_DRAM_SIZE, &dev->dram_size, dev->s2d_msg_id, true);
> -	if (ret || !dev->dram_size)
> -		goto err_dram_size;
> -
> -	return 0;
> -
> -err_dram_size:
> -	dev_err(dev->dev, "DRAM size command not supported for this platform\n");
> -	return ret;
> -}
> -
>  static int amd_pmc_s2d_init(struct amd_pmc_dev *dev)
>  {
>  	u32 phys_addr_low, phys_addr_hi;
> @@ -1009,8 +982,8 @@ static int amd_pmc_s2d_init(struct amd_pmc_dev *dev)
>  		return -EIO;
>  
>  	/* Get DRAM size */
> -	ret = amd_pmc_get_dram_size(dev);
> -	if (ret)
> +	ret = amd_pmc_send_cmd(dev, S2D_DRAM_SIZE, &dev->dram_size, dev->s2d_msg_id, true);
> +	if (ret || !dev->dram_size)
>  		dev->dram_size = S2D_TELEMETRY_DRAMBYTES_MAX;
>  
>  	/* Get STB DRAM address */
>