>>> 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.leichtzumer...@gmail.com> Sent: Tuesday, April 21, 2020 5:52 PM To: Tao, Yintian <yintian....@amd.com>; Liu, Monk <monk....@amd.com>; He, Jacob <jacob...@amd.com>; amd-gfx@lists.freedesktop.org Cc: Gu, Frans <frans...@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.leichtzumer...@gmail.com> > Sent: 2020年4月21日 17:10 > To: Liu, Monk <monk....@amd.com>; Tao, Yintian <yintian....@amd.com>; > He, Jacob <jacob...@amd.com>; amd-gfx@lists.freedesktop.org > Cc: Gu, Frans <frans...@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.koe...@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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx