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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

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 */
>