[PATCH] drm/amdgpu: cleanup SPM VMID update

Tue Apr 21 11:51:47 UTC 2020

Hi Monk,

at least on Vega that should be fine. If the RLC should use anything 
else than 0 here we should update that together with the VMID.

Regards,
Christian.

Am 21.04.20 um 11:54 schrieb Liu, Monk:
>>>> 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 at gmail.com>
> Sent: Tuesday, April 21, 2020 5:52 PM
> To: Tao, Yintian <Yintian.Tao at amd.com>; Liu, Monk <Monk.Liu at amd.com>; He, Jacob <Jacob.He at amd.com>; amd-gfx at lists.freedesktop.org
> Cc: Gu, Frans <Frans.Gu at amd.com>
> 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 at gmail.com>
>> Sent: 2020年4月21日 17:10
>> To: Liu, Monk <Monk.Liu at amd.com>; Tao, Yintian <Yintian.Tao at amd.com>;
>> He, Jacob <Jacob.He at amd.com>; amd-gfx at lists.freedesktop.org
>> Cc: Gu, Frans <Frans.Gu at amd.com>
>> 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 at amd.com>
>> ---
>>    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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx