* [PATCH] drm/amdgpu: cleanup SPM VMID update
@ 2020-04-21 9:10 Christian König
2020-04-21 9:45 ` Tao, Yintian
2020-04-21 9:50 ` Liu, Monk
0 siblings, 2 replies; 10+ messages in thread
From: Christian König @ 2020-04-21 9:10 UTC (permalink / raw)
To: Monk.Liu, Yintian.Tao, Jacob.He, amd-gfx; +Cc: Frans.Gu
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@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);
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* RE: [PATCH] drm/amdgpu: cleanup SPM VMID update
2020-04-21 9:10 [PATCH] drm/amdgpu: cleanup SPM VMID update Christian König
@ 2020-04-21 9:45 ` Tao, Yintian
2020-04-21 9:52 ` Christian König
2020-04-21 9:50 ` Liu, Monk
1 sibling, 1 reply; 10+ messages in thread
From: Tao, Yintian @ 2020-04-21 9:45 UTC (permalink / raw)
To: Christian König, Liu, Monk, He, Jacob, amd-gfx; +Cc: Gu, Frans
-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@gmail.com>
Sent: 2020年4月21日 17:10
To: Liu, Monk <Monk.Liu@amd.com>; Tao, Yintian <Yintian.Tao@amd.com>; He, Jacob <Jacob.He@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Gu, Frans <Frans.Gu@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@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?
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* RE: [PATCH] drm/amdgpu: cleanup SPM VMID update
2020-04-21 9:10 [PATCH] drm/amdgpu: cleanup SPM VMID update Christian König
2020-04-21 9:45 ` Tao, Yintian
@ 2020-04-21 9:50 ` Liu, Monk
1 sibling, 0 replies; 10+ messages in thread
From: Liu, Monk @ 2020-04-21 9:50 UTC (permalink / raw)
To: Christian König, Tao, Yintian, He, Jacob, amd-gfx; +Cc: Gu, Frans
Christian
Many fields looks not like going to be still value at all, e.g.:
RLC_SPM_PERF_CNTR 5 0x0 PERF_CNTR that is used by RLC for memory transactions
By your change you always set above filed to 0, is it right ? I really doubt it
Beside: to make SRIOV VF less painful please use NOKIQ version read/write if "one vf mode" is detected :
amdgpu_sriov_is_pp_one_vf(adev)
_____________________________________
Monk Liu|GPU Virtualization Team |AMD
-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@gmail.com>
Sent: Tuesday, April 21, 2020 5:10 PM
To: Liu, Monk <Monk.Liu@amd.com>; Tao, Yintian <Yintian.Tao@amd.com>; He, Jacob <Jacob.He@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Gu, Frans <Frans.Gu@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@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);
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: cleanup SPM VMID update
2020-04-21 9:45 ` Tao, Yintian
@ 2020-04-21 9:52 ` Christian König
2020-04-21 9:54 ` Liu, Monk
0 siblings, 1 reply; 10+ messages in thread
From: Christian König @ 2020-04-21 9:52 UTC (permalink / raw)
To: Tao, Yintian, Liu, Monk, He, Jacob, amd-gfx; +Cc: Gu, Frans
Am 21.04.20 um 11:45 schrieb Tao, Yintian:
>
> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken@gmail.com>
> Sent: 2020年4月21日 17:10
> To: Liu, Monk <Monk.Liu@amd.com>; Tao, Yintian <Yintian.Tao@amd.com>; He, Jacob <Jacob.He@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Gu, Frans <Frans.Gu@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@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
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] drm/amdgpu: cleanup SPM VMID update
2020-04-21 9:52 ` Christian König
@ 2020-04-21 9:54 ` Liu, Monk
2020-04-21 11:51 ` Christian König
0 siblings, 1 reply; 10+ messages in thread
From: Liu, Monk @ 2020-04-21 9:54 UTC (permalink / raw)
To: Koenig, Christian, Tao, Yintian, He, Jacob, amd-gfx; +Cc: Gu, Frans
>>> 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@gmail.com>
Sent: Tuesday, April 21, 2020 5:52 PM
To: Tao, Yintian <Yintian.Tao@amd.com>; Liu, Monk <Monk.Liu@amd.com>; He, Jacob <Jacob.He@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Gu, Frans <Frans.Gu@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@gmail.com>
> Sent: 2020年4月21日 17:10
> To: Liu, Monk <Monk.Liu@amd.com>; Tao, Yintian <Yintian.Tao@amd.com>;
> He, Jacob <Jacob.He@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Gu, Frans <Frans.Gu@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@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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: cleanup SPM VMID update
2020-04-21 9:54 ` Liu, Monk
@ 2020-04-21 11:51 ` Christian König
2020-04-21 13:34 ` Liu, Monk
0 siblings, 1 reply; 10+ messages in thread
From: Christian König @ 2020-04-21 11:51 UTC (permalink / raw)
To: Liu, Monk, Koenig, Christian, Tao, Yintian, He, Jacob, amd-gfx; +Cc: Gu, Frans
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@gmail.com>
> Sent: Tuesday, April 21, 2020 5:52 PM
> To: Tao, Yintian <Yintian.Tao@amd.com>; Liu, Monk <Monk.Liu@amd.com>; He, Jacob <Jacob.He@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Gu, Frans <Frans.Gu@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@gmail.com>
>> Sent: 2020年4月21日 17:10
>> To: Liu, Monk <Monk.Liu@amd.com>; Tao, Yintian <Yintian.Tao@amd.com>;
>> He, Jacob <Jacob.He@amd.com>; amd-gfx@lists.freedesktop.org
>> Cc: Gu, Frans <Frans.Gu@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@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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] drm/amdgpu: cleanup SPM VMID update
2020-04-21 11:51 ` Christian König
@ 2020-04-21 13:34 ` Liu, Monk
2020-04-21 13:38 ` Christian König
0 siblings, 1 reply; 10+ messages in thread
From: Liu, Monk @ 2020-04-21 13:34 UTC (permalink / raw)
To: Koenig, Christian, Tao, Yintian, He, Jacob, amd-gfx
The problem is some fields are increased by hardware, and RLC simply read its value, we cannot set those field together with VMID
Christian, we should stop arguing on this small feature, there is no way to have a worse solution compared with current logic ....
I think at least we should apply one change: we use NO_KIQ for SRIOV pp_one_vf_mode case to access this SPM register to avoid SRIOV KIQ flood
_____________________________________
Monk Liu|GPU Virtualization Team |AMD
-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Christian K?nig
Sent: Tuesday, April 21, 2020 7:52 PM
To: Liu, Monk <Monk.Liu@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Tao, Yintian <Yintian.Tao@amd.com>; He, Jacob <Jacob.He@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Gu, Frans <Frans.Gu@amd.com>
Subject: Re: [PATCH] drm/amdgpu: cleanup SPM VMID update
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@gmail.com>
> Sent: Tuesday, April 21, 2020 5:52 PM
> To: Tao, Yintian <Yintian.Tao@amd.com>; Liu, Monk <Monk.Liu@amd.com>;
> He, Jacob <Jacob.He@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Gu, Frans <Frans.Gu@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@gmail.com>
>> Sent: 2020年4月21日 17:10
>> To: Liu, Monk <Monk.Liu@amd.com>; Tao, Yintian <Yintian.Tao@amd.com>;
>> He, Jacob <Jacob.He@amd.com>; amd-gfx@lists.freedesktop.org
>> Cc: Gu, Frans <Frans.Gu@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@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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cmo
> nk.liu%40amd.com%7C2efa2541a56f4716b9bd08d7e5ea630d%7C3dd8961fe4884e60
> 8e11a82d994e183d%7C0%7C0%7C637230667158960812&sdata=uoHIufAtSFzCf4
> P8VpxwBi0%2ByddIa0QVw32W3ZCJAmk%3D&reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cmonk.liu%40amd.com%7C2efa2541a56f4716b9bd08d7e5ea630d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637230667158960812&sdata=uoHIufAtSFzCf4P8VpxwBi0%2ByddIa0QVw32W3ZCJAmk%3D&reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: cleanup SPM VMID update
2020-04-21 13:34 ` Liu, Monk
@ 2020-04-21 13:38 ` Christian König
2020-04-21 13:46 ` Tao, Yintian
0 siblings, 1 reply; 10+ messages in thread
From: Christian König @ 2020-04-21 13:38 UTC (permalink / raw)
To: Liu, Monk, Tao, Yintian, He, Jacob, amd-gfx
> The problem is some fields are increased by hardware
What are you talking about? The bits control what is used in the MC
interface, there is no increment or anything here.
> I think at least we should apply one change: we use NO_KIQ for SRIOV pp_one_vf_mode case to access this SPM register to avoid SRIOV KIQ flood
Agreed that sounds like a good idea to me as well no matter if we use
RMW or just a write.
Regards,
Christian.
Am 21.04.20 um 15:34 schrieb Liu, Monk:
> The problem is some fields are increased by hardware, and RLC simply read its value, we cannot set those field together with VMID
>
> Christian, we should stop arguing on this small feature, there is no way to have a worse solution compared with current logic ....
>
> I think at least we should apply one change: we use NO_KIQ for SRIOV pp_one_vf_mode case to access this SPM register to avoid SRIOV KIQ flood
>
> _____________________________________
> Monk Liu|GPU Virtualization Team |AMD
>
>
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Christian K?nig
> Sent: Tuesday, April 21, 2020 7:52 PM
> To: Liu, Monk <Monk.Liu@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Tao, Yintian <Yintian.Tao@amd.com>; He, Jacob <Jacob.He@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Gu, Frans <Frans.Gu@amd.com>
> Subject: Re: [PATCH] drm/amdgpu: cleanup SPM VMID update
>
> 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@gmail.com>
>> Sent: Tuesday, April 21, 2020 5:52 PM
>> To: Tao, Yintian <Yintian.Tao@amd.com>; Liu, Monk <Monk.Liu@amd.com>;
>> He, Jacob <Jacob.He@amd.com>; amd-gfx@lists.freedesktop.org
>> Cc: Gu, Frans <Frans.Gu@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@gmail.com>
>>> Sent: 2020年4月21日 17:10
>>> To: Liu, Monk <Monk.Liu@amd.com>; Tao, Yintian <Yintian.Tao@amd.com>;
>>> He, Jacob <Jacob.He@amd.com>; amd-gfx@lists.freedesktop.org
>>> Cc: Gu, Frans <Frans.Gu@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@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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
>> s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cmo
>> nk.liu%40amd.com%7C2efa2541a56f4716b9bd08d7e5ea630d%7C3dd8961fe4884e60
>> 8e11a82d994e183d%7C0%7C0%7C637230667158960812&sdata=uoHIufAtSFzCf4
>> P8VpxwBi0%2ByddIa0QVw32W3ZCJAmk%3D&reserved=0
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cmonk.liu%40amd.com%7C2efa2541a56f4716b9bd08d7e5ea630d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637230667158960812&sdata=uoHIufAtSFzCf4P8VpxwBi0%2ByddIa0QVw32W3ZCJAmk%3D&reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] drm/amdgpu: cleanup SPM VMID update
2020-04-21 13:38 ` Christian König
@ 2020-04-21 13:46 ` Tao, Yintian
2020-04-21 13:52 ` Liu, Monk
0 siblings, 1 reply; 10+ messages in thread
From: Tao, Yintian @ 2020-04-21 13:46 UTC (permalink / raw)
To: Koenig, Christian, Liu, Monk, He, Jacob, amd-gfx
Hi Christian
Great. Then can you modify the patch according to Monk's suggestion?
We need this patch for one important project.
Best Regards
Yintian Tao
-----Original Message-----
From: Koenig, Christian <Christian.Koenig@amd.com>
Sent: 2020年4月21日 21:38
To: Liu, Monk <Monk.Liu@amd.com>; Tao, Yintian <Yintian.Tao@amd.com>; He, Jacob <Jacob.He@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: cleanup SPM VMID update
> The problem is some fields are increased by hardware
What are you talking about? The bits control what is used in the MC interface, there is no increment or anything here.
> I think at least we should apply one change: we use NO_KIQ for SRIOV
> pp_one_vf_mode case to access this SPM register to avoid SRIOV KIQ
> flood
Agreed that sounds like a good idea to me as well no matter if we use RMW or just a write.
Regards,
Christian.
Am 21.04.20 um 15:34 schrieb Liu, Monk:
> The problem is some fields are increased by hardware, and RLC simply
> read its value, we cannot set those field together with VMID
>
> Christian, we should stop arguing on this small feature, there is no way to have a worse solution compared with current logic ....
>
> I think at least we should apply one change: we use NO_KIQ for SRIOV
> pp_one_vf_mode case to access this SPM register to avoid SRIOV KIQ
> flood
>
> _____________________________________
> Monk Liu|GPU Virtualization Team |AMD
>
>
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Christian K?nig
> Sent: Tuesday, April 21, 2020 7:52 PM
> To: Liu, Monk <Monk.Liu@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Tao, Yintian <Yintian.Tao@amd.com>; He,
> Jacob <Jacob.He@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Gu, Frans <Frans.Gu@amd.com>
> Subject: Re: [PATCH] drm/amdgpu: cleanup SPM VMID update
>
> 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@gmail.com>
>> Sent: Tuesday, April 21, 2020 5:52 PM
>> To: Tao, Yintian <Yintian.Tao@amd.com>; Liu, Monk <Monk.Liu@amd.com>;
>> He, Jacob <Jacob.He@amd.com>; amd-gfx@lists.freedesktop.org
>> Cc: Gu, Frans <Frans.Gu@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@gmail.com>
>>> Sent: 2020年4月21日 17:10
>>> To: Liu, Monk <Monk.Liu@amd.com>; Tao, Yintian
>>> <Yintian.Tao@amd.com>; He, Jacob <Jacob.He@amd.com>;
>>> amd-gfx@lists.freedesktop.org
>>> Cc: Gu, Frans <Frans.Gu@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@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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
>> t
>> s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cm
>> o
>> nk.liu%40amd.com%7C2efa2541a56f4716b9bd08d7e5ea630d%7C3dd8961fe4884e6
>> 0
>> 8e11a82d994e183d%7C0%7C0%7C637230667158960812&sdata=uoHIufAtSFzCf
>> 4
>> P8VpxwBi0%2ByddIa0QVw32W3ZCJAmk%3D&reserved=0
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cmo
> nk.liu%40amd.com%7C2efa2541a56f4716b9bd08d7e5ea630d%7C3dd8961fe4884e60
> 8e11a82d994e183d%7C0%7C0%7C637230667158960812&sdata=uoHIufAtSFzCf4
> P8VpxwBi0%2ByddIa0QVw32W3ZCJAmk%3D&reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] drm/amdgpu: cleanup SPM VMID update
2020-04-21 13:46 ` Tao, Yintian
@ 2020-04-21 13:52 ` Liu, Monk
0 siblings, 0 replies; 10+ messages in thread
From: Liu, Monk @ 2020-04-21 13:52 UTC (permalink / raw)
To: Tao, Yintian, Koenig, Christian, He, Jacob, amd-gfx
> What are you talking about? The bits control what is used in the MC interface, there is no increment or anything here.
My bad , it is RLC_SPM_PERF_CNTR not RLC_SPM_PERF_COUNTER, I though it as COUNTER
>>Agreed that sounds like a good idea to me as well no matter if we use RMW or just a write.
If we go NOKIQ way then we always use MMIO to update it, no RMW package at all
Besides RMW is not available in KCQ ring ...
Looks we have no choice !
_____________________________________
Monk Liu|GPU Virtualization Team |AMD
-----Original Message-----
From: Tao, Yintian <Yintian.Tao@amd.com>
Sent: Tuesday, April 21, 2020 9:46 PM
To: Koenig, Christian <Christian.Koenig@amd.com>; Liu, Monk <Monk.Liu@amd.com>; He, Jacob <Jacob.He@amd.com>; amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH] drm/amdgpu: cleanup SPM VMID update
Hi Christian
Great. Then can you modify the patch according to Monk's suggestion?
We need this patch for one important project.
Best Regards
Yintian Tao
-----Original Message-----
From: Koenig, Christian <Christian.Koenig@amd.com>
Sent: 2020年4月21日 21:38
To: Liu, Monk <Monk.Liu@amd.com>; Tao, Yintian <Yintian.Tao@amd.com>; He, Jacob <Jacob.He@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: cleanup SPM VMID update
> The problem is some fields are increased by hardware
What are you talking about? The bits control what is used in the MC interface, there is no increment or anything here.
> I think at least we should apply one change: we use NO_KIQ for SRIOV
> pp_one_vf_mode case to access this SPM register to avoid SRIOV KIQ
> flood
Agreed that sounds like a good idea to me as well no matter if we use RMW or just a write.
Regards,
Christian.
Am 21.04.20 um 15:34 schrieb Liu, Monk:
> The problem is some fields are increased by hardware, and RLC simply
> read its value, we cannot set those field together with VMID
>
> Christian, we should stop arguing on this small feature, there is no way to have a worse solution compared with current logic ....
>
> I think at least we should apply one change: we use NO_KIQ for SRIOV
> pp_one_vf_mode case to access this SPM register to avoid SRIOV KIQ
> flood
>
> _____________________________________
> Monk Liu|GPU Virtualization Team |AMD
>
>
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Christian K?nig
> Sent: Tuesday, April 21, 2020 7:52 PM
> To: Liu, Monk <Monk.Liu@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Tao, Yintian <Yintian.Tao@amd.com>; He,
> Jacob <Jacob.He@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Gu, Frans <Frans.Gu@amd.com>
> Subject: Re: [PATCH] drm/amdgpu: cleanup SPM VMID update
>
> 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@gmail.com>
>> Sent: Tuesday, April 21, 2020 5:52 PM
>> To: Tao, Yintian <Yintian.Tao@amd.com>; Liu, Monk <Monk.Liu@amd.com>;
>> He, Jacob <Jacob.He@amd.com>; amd-gfx@lists.freedesktop.org
>> Cc: Gu, Frans <Frans.Gu@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@gmail.com>
>>> Sent: 2020年4月21日 17:10
>>> To: Liu, Monk <Monk.Liu@amd.com>; Tao, Yintian
>>> <Yintian.Tao@amd.com>; He, Jacob <Jacob.He@amd.com>;
>>> amd-gfx@lists.freedesktop.org
>>> Cc: Gu, Frans <Frans.Gu@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@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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
>> t
>> s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cm
>> o
>> nk.liu%40amd.com%7C2efa2541a56f4716b9bd08d7e5ea630d%7C3dd8961fe4884e6
>> 0
>> 8e11a82d994e183d%7C0%7C0%7C637230667158960812&sdata=uoHIufAtSFzCf
>> 4
>> P8VpxwBi0%2ByddIa0QVw32W3ZCJAmk%3D&reserved=0
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cmo
> nk.liu%40amd.com%7C2efa2541a56f4716b9bd08d7e5ea630d%7C3dd8961fe4884e60
> 8e11a82d994e183d%7C0%7C0%7C637230667158960812&sdata=uoHIufAtSFzCf4
> P8VpxwBi0%2ByddIa0QVw32W3ZCJAmk%3D&reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-04-21 13:52 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-21 9:10 [PATCH] drm/amdgpu: cleanup SPM VMID update Christian König
2020-04-21 9:45 ` Tao, Yintian
2020-04-21 9:52 ` Christian König
2020-04-21 9:54 ` Liu, Monk
2020-04-21 11:51 ` Christian König
2020-04-21 13:34 ` Liu, Monk
2020-04-21 13:38 ` Christian König
2020-04-21 13:46 ` Tao, Yintian
2020-04-21 13:52 ` Liu, Monk
2020-04-21 9:50 ` Liu, Monk
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.