Re: [PATCH V4 10/19] accel/amdxdna: Refactor AIE4 hardware initialization sequence

[Lizhi Hou <lizhi.hou@amd.com>]

On 10/9/26 15:53, David Zhang wrote:
> Reorganize AIE4 hardware initialization into distinct phases, next to
> the existing aie4_config_fw():
> - aie4_query_fw(): Query NPU and CERT firmware versions.
> - aie4_setup_aie(): Query AIE version, metadata, initialize DPM frequency
>    table, and initialize partitions.
>
> Update aie4_pf_hw_start(), aie4_vf_hw_start(), and aie4_classic_hw_start()
> to use these phases and unify error unwinding labels. As part of this,
> aie4_pf_hw_start() now also calls aie4_query_fw(), which it previously
> did not do.
>
> Signed-off-by: David Zhang <yidong.zhang@amd.com>
> ---
>   drivers/accel/amdxdna/aie4_pci.c | 79 +++++++++++++++++++-------------
>   1 file changed, 46 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/accel/amdxdna/aie4_pci.c b/drivers/accel/amdxdna/aie4_pci.c
> index 446ab2f47c7d..ffe78c0c4e8b 100644
> --- a/drivers/accel/amdxdna/aie4_pci.c
> +++ b/drivers/accel/amdxdna/aie4_pci.c
> @@ -50,7 +50,7 @@ static int aie4_fw_is_alive(struct amdxdna_dev *xdna)
>   {
>   	const struct amdxdna_dev_priv *npriv = xdna->dev_info->dev_priv;
>   	struct amdxdna_dev_hdl *ndev = xdna->dev_handle;
> -	u32 __iomem *src;
> +	void __iomem *src;
>   	u32 fw_is_valid;
>   	int ret;
>   
> @@ -280,7 +280,8 @@ static void aie4_partition_fini(struct amdxdna_dev_hdl *ndev)
>   		XDNA_ERR(xdna, "partition fini failed: %d", ret);
>   }
>   
> -static int aie4_query(struct amdxdna_dev_hdl *ndev)
> +/* Verify CERT protocol compatibility before starting hardware. */
> +static int aie4_query_fw(struct amdxdna_dev_hdl *ndev)
>   {
>   	struct amdxdna_dev *xdna = ndev->aie.xdna;
>   	int ret;
> @@ -293,25 +294,6 @@ static int aie4_query(struct amdxdna_dev_hdl *ndev)
>   	if (ret)
>   		return ret;
>   
> -	ret = aie4_query_aie_version(ndev, &ndev->aie.version);
> -	if (ret)
> -		return ret;
> -
> -	ret = aie4_query_aie_metadata(ndev, &ndev->aie.metadata);
> -	if (ret)
> -		return ret;
> -
> -	ndev->total_col = min_t(u32, AIE4_TOTAL_COLUMN, ndev->aie.metadata.cols);
> -
> -	ret = aie4_init_dpm_freq_table(ndev);
> -	if (ret) {
> -		if (ret == -ETIME)
> -			return ret;
> -		/* if query dpm from fw failed, using default value */
> -		if (ndev->priv->hw_ops && ndev->priv->hw_ops->set_dpm)
> -			(void)ndev->priv->hw_ops->set_dpm(&ndev->aie, 0);
> -	}
> -
>   	return 0;
>   }
>   
> @@ -340,6 +322,32 @@ static int aie4_config_fw(struct amdxdna_dev_hdl *ndev)
>   	return 0;
>   }
>   
> +static int aie4_setup_aie(struct amdxdna_dev_hdl *ndev)
> +{
> +	int ret;
> +
> +	ret = aie4_query_aie_version(ndev, &ndev->aie.version);
> +	if (ret)
> +		return ret;
> +
> +	ret = aie4_query_aie_metadata(ndev, &ndev->aie.metadata);
> +	if (ret)
> +		return ret;
> +
> +	ndev->total_col = min_t(u32, AIE4_TOTAL_COLUMN, ndev->aie.metadata.cols);
> +
> +	ret = aie4_init_dpm_freq_table(ndev);
> +	if (ret) {
> +		if (ret == -ETIME)
> +			return ret;
> +		/* if query dpm from fw failed, using default value */
> +		if (ndev->priv->hw_ops && ndev->priv->hw_ops->set_dpm)
> +			(void)ndev->priv->hw_ops->set_dpm(&ndev->aie, 0);
> +	}
> +
> +	return aie4_partition_init(ndev);
> +}
> +
>   static int aie4_pf_hw_start(struct amdxdna_dev_hdl *ndev)
>   {
>   	int ret;
> @@ -352,6 +360,10 @@ static int aie4_pf_hw_start(struct amdxdna_dev_hdl *ndev)
>   	if (ret)
>   		goto stop_fw;
>   
> +	ret = aie4_query_fw(ndev);
> +	if (ret)
> +		goto mbox_fini;
> +
>   	ret = aie4_config_fw(ndev);
>   	if (ret)
>   		goto mbox_fini;
> @@ -389,13 +401,13 @@ static int aie4_vf_hw_start(struct amdxdna_dev_hdl *ndev)
>   	if (ret)
>   		return ret;
>   
> -	ret = aie4_query(ndev);
> +	ret = aie4_query_fw(ndev);
>   	if (ret)
> -		goto mailbox_fini;
> +		goto mbox_fini;
>   
> -	ret = aie4_partition_init(ndev);
> +	ret = aie4_setup_aie(ndev);
>   	if (ret)
> -		goto mailbox_fini;
> +		goto mbox_fini;
>   
>   	ret = aie4_restore_power_mode(ndev);
>   	if (ret)
> @@ -405,7 +417,7 @@ static int aie4_vf_hw_start(struct amdxdna_dev_hdl *ndev)
>   
>   partition_fini:
>   	aie4_partition_fini(ndev);
> -mailbox_fini:
> +mbox_fini:
>   	aie4_mailbox_fini(ndev);
>   	return ret;
>   }
> @@ -432,17 +444,17 @@ static int aie4_classic_hw_start(struct amdxdna_dev_hdl *ndev)
>   	if (ret)
>   		goto stop_fw;
>   
> -	ret = aie4_query(ndev);
> +	ret = aie4_query_fw(ndev);
>   	if (ret)
> -		goto mailbox_fini;
> +		goto mbox_fini;
>   
>   	ret = aie4_config_fw(ndev);
>   	if (ret)
> -		goto mailbox_fini;
> +		goto mbox_fini;
>   
> -	ret = aie4_partition_init(ndev);
> +	ret = aie4_setup_aie(ndev);
>   	if (ret)
> -		goto mailbox_fini;
> +		goto mbox_fini;
>   
>   	ret = aie4_restore_power_mode(ndev);
>   	if (ret)
> @@ -452,10 +464,11 @@ static int aie4_classic_hw_start(struct amdxdna_dev_hdl *ndev)
>   
>   partition_fini:
>   	aie4_partition_fini(ndev);
> -mailbox_fini:
> +mbox_fini:
>   	aie4_mailbox_fini(ndev);
>   stop_fw:
>   	aie4_fw_stop(ndev);
> +
>   	return ret;
>   }
>   
> @@ -529,8 +542,8 @@ static int aie4_prepare_firmware(struct amdxdna_dev_hdl *ndev,
>   				 void __iomem *tbl[PCI_NUM_RESOURCES])
>   {
>   	struct amdxdna_dev *xdna = ndev->aie.xdna;
> +	struct smu_config smu_conf = {};
>   	struct psp_config psp_conf;
> -	struct smu_config smu_conf;
Reviewed-by: Lizhi Hou <lizhi.hou@amd.com>
>   	int i;
>   
>   	psp_conf.fw_size = npufw->size;