[PATCH 1/2] drm amdgpu: Skip tmr load for SRIOV

[Luben Tuikov]

Thanks for this patch.

On 2020-07-28 1:12 a.m., Liu ChengZhe wrote:
> From: root <ChengZhe.Liu at amd.com>

You should fix your Git setup to show proper user name,
not "root". I've prepared a Confluence page which shows
a way to do it, and a few other things along the way:

http://confluence.amd.com/display/~ltuikov/Git+Setup

> 
>     1. For Navi12, CHIP_SIENNA_CICHLID, skip tmr load operation;
>     2. Check pointer before release firmware.
> 
>     v2: use CHIP_SIENNA_CICHLID instead
>     v3: remove local "bool ret"; fix grammer issue
> Signed-off-by: root <ChengZhe.Liu at amd.com>

You're missing an empty line between your commit message
and the Signed-off-by: line.

Also please do not indent your commit message. "git log" already
indents it and it would look too indented to the right.

Below for more:

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 35 ++++++++++++++++++++-----
>  1 file changed, 29 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index a053b7af0680..7f18286a0cc2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -193,12 +193,18 @@ static int psp_sw_fini(void *handle)
>  	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>  
>  	psp_memory_training_fini(&adev->psp);
> -	release_firmware(adev->psp.sos_fw);
> -	adev->psp.sos_fw = NULL;
> -	release_firmware(adev->psp.asd_fw);
> -	adev->psp.asd_fw = NULL;
> -	release_firmware(adev->psp.ta_fw);
> -	adev->psp.ta_fw = NULL;
> +	if (adev->psp.sos_fw) {
> +		release_firmware(adev->psp.sos_fw);
> +		adev->psp.sos_fw = NULL;
> +	}
> +	if (adev->psp.asd_fw) {
> +		release_firmware(adev->psp.asd_fw);
> +		adev->psp.asd_fw = NULL;
> +	}
> +	if (adev->psp.ta_fw) {
> +		release_firmware(adev->psp.ta_fw);
> +		adev->psp.ta_fw = NULL;
> +	}
>  
>  	if (adev->asic_type == CHIP_NAVI10)
>  		psp_sysfs_fini(adev);
> @@ -409,11 +415,28 @@ static int psp_clear_vf_fw(struct psp_context *psp)
>  	return ret;
>  }
>  
> +static bool psp_skip_tmr(struct psp_context *psp)
> +{
> +	switch (psp->adev->asic_type) {
> +	case CHIP_NAVI12:
> +	case CHIP_SIENNA_CICHLID:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
>  static int psp_tmr_load(struct psp_context *psp)
>  {
>  	int ret;
>  	struct psp_gfx_cmd_resp *cmd;
>  
> +	/* for Navi12 and CHIP_SIENNA_CICHLID SRIOV, do not set up TMR
> +	 * (already setup by host driver)

Thanks for fixing noun "setup" to verb "set up". But there
is another "already setup by" should be "already set up by the host driver".

Thanks and regards,
Luben


> +	 */
> +	if (amdgpu_sriov_vf(psp->adev) && psp_skip_tmr(psp))
> +		return 0;
> +
>  	cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL);
>  	if (!cmd)
>  		return -ENOMEM;
>