RE: [PATCH] drm/amdgpu: cleanup SPM VMID update

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



>>> Could only be that the firmware updates the bits to something non default, I'm going to double check that on a Vega10.

I think that will be a sure answer, otherwise why we need those field if we always write 0 to them and reader always expect 0 reading back from them ??

Those fields are kind of performance counters 

_____________________________________
Monk Liu|GPU Virtualization Team |AMD


-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@xxxxxxxxx> 
Sent: Tuesday, April 21, 2020 5:52 PM
To: Tao, Yintian <Yintian.Tao@xxxxxxx>; Liu, Monk <Monk.Liu@xxxxxxx>; He, Jacob <Jacob.He@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Gu, Frans <Frans.Gu@xxxxxxx>
Subject: Re: [PATCH] drm/amdgpu: cleanup SPM VMID update

Am 21.04.20 um 11:45 schrieb Tao, Yintian:
>
> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken@xxxxxxxxx>
> Sent: 2020年4月21日 17:10
> To: Liu, Monk <Monk.Liu@xxxxxxx>; Tao, Yintian <Yintian.Tao@xxxxxxx>; 
> He, Jacob <Jacob.He@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Gu, Frans <Frans.Gu@xxxxxxx>
> Subject: [PATCH] drm/amdgpu: cleanup SPM VMID update
>
> The RLC SPM configuration register contains the information how the memory access is made (VMID, MTYPE, etc....) which should always be consistent.
>
> So instead of a read modify write cycle of the VMID always update the whole register.
>
> Signed-off-by: Christian König <christian.koenig@xxxxxxx>
> ---
>   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 7 +------  drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  | 7 +------  drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 7 +------  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  | 7 +------
>   4 files changed, 4 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> index 0a03e2ad5d95..2a6556371046 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> @@ -7030,12 +7030,7 @@ static int 
> gfx_v10_0_update_gfx_clock_gating(struct amdgpu_device *adev,
>   
>   static void gfx_v10_0_update_spm_vmid(struct amdgpu_device *adev, unsigned vmid)  {
> -	u32 data;
> -
> -	data = RREG32_SOC15(GC, 0, mmRLC_SPM_MC_CNTL);
> -
> -	data &= ~RLC_SPM_MC_CNTL__RLC_SPM_VMID_MASK;
> -	data |= (vmid & RLC_SPM_MC_CNTL__RLC_SPM_VMID_MASK) << RLC_SPM_MC_CNTL__RLC_SPM_VMID__SHIFT;
> +	u32 data = REG_SET_FIELD(0, RLC_SPM_MC_CNTL, RLC_SPM_VMID, vmid);
> [yttao]: The orig_val is 0 which means except VMID field other reset fields will be set to 0. Whether it is legal?

According to the register specification that is the default value for those bits on gfx9/10.

Could only be that the firmware updates the bits to something non default, I'm going to double check that on a Vega10.

Regards,
Christian.

>   
>   	WREG32_SOC15(GC, 0, mmRLC_SPM_MC_CNTL, data);  } diff --git 
> a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> index b2f10e39eff1..a92486cd038f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> @@ -3570,12 +3570,7 @@ static int gfx_v7_0_rlc_resume(struct 
> amdgpu_device *adev)
>   
>   static void gfx_v7_0_update_spm_vmid(struct amdgpu_device *adev, unsigned vmid)  {
> -	u32 data;
> -
> -	data = RREG32(mmRLC_SPM_VMID);
> -
> -	data &= ~RLC_SPM_VMID__RLC_SPM_VMID_MASK;
> -	data |= (vmid & RLC_SPM_VMID__RLC_SPM_VMID_MASK) << RLC_SPM_VMID__RLC_SPM_VMID__SHIFT;
> +	u32 data = REG_SET_FIELD(0, RLC_SPM_VMID, RLC_SPM_VMID, vmid);
>   
>   	WREG32(mmRLC_SPM_VMID, data);
>   }
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index fc6c2f2bc76c..44fdda68db98 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -5613,12 +5613,7 @@ static void gfx_v8_0_unset_safe_mode(struct 
> amdgpu_device *adev)
>   
>   static void gfx_v8_0_update_spm_vmid(struct amdgpu_device *adev, unsigned vmid)  {
> -	u32 data;
> -
> -	data = RREG32(mmRLC_SPM_VMID);
> -
> -	data &= ~RLC_SPM_VMID__RLC_SPM_VMID_MASK;
> -	data |= (vmid & RLC_SPM_VMID__RLC_SPM_VMID_MASK) << RLC_SPM_VMID__RLC_SPM_VMID__SHIFT;
> +	u32 data = REG_SET_FIELD(0, RLC_SPM_VMID, RLC_SPM_VMID, vmid);
>   
>   	WREG32(mmRLC_SPM_VMID, data);
>   }
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 54eded9a6ac5..b36fbf991313 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -4950,12 +4950,7 @@ static int 
> gfx_v9_0_update_gfx_clock_gating(struct amdgpu_device *adev,
>   
>   static void gfx_v9_0_update_spm_vmid(struct amdgpu_device *adev, unsigned vmid)  {
> -	u32 data;
> -
> -	data = RREG32_SOC15(GC, 0, mmRLC_SPM_MC_CNTL);
> -
> -	data &= ~RLC_SPM_MC_CNTL__RLC_SPM_VMID_MASK;
> -	data |= (vmid & RLC_SPM_MC_CNTL__RLC_SPM_VMID_MASK) << RLC_SPM_MC_CNTL__RLC_SPM_VMID__SHIFT;
> +	u32 data = REG_SET_FIELD(0, RLC_SPM_MC_CNTL, RLC_SPM_VMID, vmid);
>   
>   	WREG32_SOC15(GC, 0, mmRLC_SPM_MC_CNTL, data);  }
> --
> 2.17.1
>

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux