Re: [PATCH 1/2] drm/amdgpu/gfx9: move update_spm_vmid() out of rlc_init()

["Christian König" <ckoenig.leichtzumerken@gmail.com>]

Am 06.07.23 um 20:55 schrieb Alex Deucher:
> rlc_init() is part of sw_init() so it should not touch hardware.
> Additionally, calling the rlc update_spm_vmid() callback
> directly invokes a gfx on/off cycle which could result in
> powergating being enabled before hw init is complete.  Split
> update_spm_vmid() into an internal implementation for local
> use without gfxoff interaction and then the rlc callback
> which includes gfxoff handling.  lbpw_init also touches
> hardware so mvoe that to rlc_resume as well.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

Acked-by: Christian König <christian.koenig@amd.com> for the series.

> ---
>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 34 ++++++++++++---------------
>   1 file changed, 15 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index d654bdd2037c9..7d992e4730db1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -762,6 +762,8 @@ static void gfx_v9_0_query_ras_error_count(struct amdgpu_device *adev,
>   static int gfx_v9_0_ras_error_inject(struct amdgpu_device *adev,
>   				     void *inject_if, uint32_t instance_mask);
>   static void gfx_v9_0_reset_ras_error_count(struct amdgpu_device *adev);
> +static void gfx_v9_0_update_spm_vmid_internal(struct amdgpu_device *adev,
> +					      unsigned vmid);
>   
>   static void gfx_v9_0_kiq_set_resources(struct amdgpu_ring *kiq_ring,
>   				uint64_t queue_mask)
> @@ -1669,22 +1671,6 @@ static int gfx_v9_0_rlc_init(struct amdgpu_device *adev)
>   			return r;
>   	}
>   
> -	switch (adev->ip_versions[GC_HWIP][0]) {
> -	case IP_VERSION(9, 2, 2):
> -	case IP_VERSION(9, 1, 0):
> -		gfx_v9_0_init_lbpw(adev);
> -		break;
> -	case IP_VERSION(9, 4, 0):
> -		gfx_v9_4_init_lbpw(adev);
> -		break;
> -	default:
> -		break;
> -	}
> -
> -	/* init spm vmid with 0xf */
> -	if (adev->gfx.rlc.funcs->update_spm_vmid)
> -		adev->gfx.rlc.funcs->update_spm_vmid(adev, 0xf);
> -
>   	return 0;
>   }
>   
> @@ -2944,12 +2930,14 @@ static int gfx_v9_0_rlc_resume(struct amdgpu_device *adev)
>   	switch (adev->ip_versions[GC_HWIP][0]) {
>   	case IP_VERSION(9, 2, 2):
>   	case IP_VERSION(9, 1, 0):
> +		gfx_v9_0_init_lbpw(adev);
>   		if (amdgpu_lbpw == 0)
>   			gfx_v9_0_enable_lbpw(adev, false);
>   		else
>   			gfx_v9_0_enable_lbpw(adev, true);
>   		break;
>   	case IP_VERSION(9, 4, 0):
> +		gfx_v9_4_init_lbpw(adev);
>   		if (amdgpu_lbpw > 0)
>   			gfx_v9_0_enable_lbpw(adev, true);
>   		else
> @@ -2959,6 +2947,8 @@ static int gfx_v9_0_rlc_resume(struct amdgpu_device *adev)
>   		break;
>   	}
>   
> +	gfx_v9_0_update_spm_vmid_internal(adev, 0xf);
> +
>   	adev->gfx.rlc.funcs->start(adev);
>   
>   	return 0;
> @@ -4883,12 +4873,11 @@ static int gfx_v9_0_update_gfx_clock_gating(struct amdgpu_device *adev,
>   	return 0;
>   }
>   
> -static void gfx_v9_0_update_spm_vmid(struct amdgpu_device *adev, unsigned vmid)
> +static void gfx_v9_0_update_spm_vmid_internal(struct amdgpu_device *adev,
> +					      unsigned vmid)
>   {
>   	u32 reg, data;
>   
> -	amdgpu_gfx_off_ctrl(adev, false);
> -
>   	reg = SOC15_REG_OFFSET(GC, 0, mmRLC_SPM_MC_CNTL);
>   	if (amdgpu_sriov_is_pp_one_vf(adev))
>   		data = RREG32_NO_KIQ(reg);
> @@ -4902,6 +4891,13 @@ static void gfx_v9_0_update_spm_vmid(struct amdgpu_device *adev, unsigned vmid)
>   		WREG32_SOC15_NO_KIQ(GC, 0, mmRLC_SPM_MC_CNTL, data);
>   	else
>   		WREG32_SOC15(GC, 0, mmRLC_SPM_MC_CNTL, data);
> +}
> +
> +static void gfx_v9_0_update_spm_vmid(struct amdgpu_device *adev, unsigned vmid)
> +{
> +	amdgpu_gfx_off_ctrl(adev, false);
> +
> +	gfx_v9_0_update_spm_vmid_internal(adev, vmid);
>   
>   	amdgpu_gfx_off_ctrl(adev, true);
>   }