Re: [PATCH] drm/amdgpu: Skip some registers config for SRIOV
Reviewed-by: Luben Tuikov
On 2020-08-07 04:48, Liu ChengZhe wrote:
> Some registers are not accessible to virtual function setup, so
> skip their initialization when in VF-SRIOV mode.
>
> v2: move SRIOV VF check into specify functions;
> modify commit description and comment.
>
> Signed-off-by: Liu ChengZhe
> ---
> drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c | 19 +++
> drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c | 19 +++
> 2 files changed, 38 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
> b/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
> index 1f6112b7fa49..80c906a0383f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
> @@ -182,6 +182,12 @@ static void gfxhub_v2_1_init_cache_regs(struct
> amdgpu_device *adev)
> {
> uint32_t tmp;
>
> + /* These registers are not accessible to VF-SRIOV.
> + * The PF will program them instead.
> + */
> + if (amdgpu_sriov_vf(adev))
> + return;
> +
> /* Setup L2 cache */
> tmp = RREG32_SOC15(GC, 0, mmGCVM_L2_CNTL);
> tmp = REG_SET_FIELD(tmp, GCVM_L2_CNTL, ENABLE_L2_CACHE, 1);
> @@ -237,6 +243,12 @@ static void gfxhub_v2_1_enable_system_domain(struct
> amdgpu_device *adev)
>
> static void gfxhub_v2_1_disable_identity_aperture(struct amdgpu_device *adev)
> {
> + /* These registers are not accessible to VF-SRIOV.
> + * The PF will program them instead.
> + */
> + if (amdgpu_sriov_vf(adev))
> + return;
> +
> WREG32_SOC15(GC, 0, mmGCVM_L2_CONTEXT1_IDENTITY_APERTURE_LOW_ADDR_LO32,
>0x);
> WREG32_SOC15(GC, 0, mmGCVM_L2_CONTEXT1_IDENTITY_APERTURE_LOW_ADDR_HI32,
> @@ -373,6 +385,13 @@ void gfxhub_v2_1_set_fault_enable_default(struct
> amdgpu_device *adev,
> bool value)
> {
> u32 tmp;
> +
> + /* These registers are not accessible to VF-SRIOV.
> + * The PF will program them instead.
> + */
> + if (amdgpu_sriov_vf(adev))
> + return;
> +
> tmp = RREG32_SOC15(GC, 0, mmGCVM_L2_PROTECTION_FAULT_CNTL);
> tmp = REG_SET_FIELD(tmp, GCVM_L2_PROTECTION_FAULT_CNTL,
> RANGE_PROTECTION_FAULT_ENABLE_DEFAULT, value);
> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
> b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
> index d83912901f73..8acb3b625afe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
> @@ -181,6 +181,12 @@ static void mmhub_v2_0_init_cache_regs(struct
> amdgpu_device *adev)
> {
> uint32_t tmp;
>
> + /* These registers are not accessible to VF-SRIOV.
> + * The PF will program them instead.
> + */
> + if (amdgpu_sriov_vf(adev))
> + return;
> +
> /* Setup L2 cache */
> tmp = RREG32_SOC15(MMHUB, 0, mmMMVM_L2_CNTL);
> tmp = REG_SET_FIELD(tmp, MMVM_L2_CNTL, ENABLE_L2_CACHE, 1);
> @@ -236,6 +242,12 @@ static void mmhub_v2_0_enable_system_domain(struct
> amdgpu_device *adev)
>
> static void mmhub_v2_0_disable_identity_aperture(struct amdgpu_device *adev)
> {
> + /* These registers are not accessible to VF-SRIOV.
> + * The PF will program them instead.
> + */
> + if (amdgpu_sriov_vf(adev))
> + return;
> +
> WREG32_SOC15(MMHUB, 0,
>mmMMVM_L2_CONTEXT1_IDENTITY_APERTURE_LOW_ADDR_LO32,
>0x);
> @@ -365,6 +377,13 @@ void mmhub_v2_0_gart_disable(struct amdgpu_device *adev)
> void mmhub_v2_0_set_fault_enable_default(struct amdgpu_device *adev, bool
> value)
> {
> u32 tmp;
> +
> + /* These registers are not accessible to VF-SRIOV.
> + * The PF will program them instead.
> + */
> + if (amdgpu_sriov_vf(adev))
> + return;
> +
> tmp = RREG32_SOC15(MMHUB, 0, mmMMVM_L2_PROTECTION_FAULT_CNTL);
> tmp = REG_SET_FIELD(tmp, MMVM_L2_PROTECTION_FAULT_CNTL,
> RANGE_PROTECTION_FAULT_ENABLE_DEFAULT, value);
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: Skip some registers config for SRIOV
Some registers are not accessible to virtual function setup, so
skip their initialization when in VF-SRIOV mode.
v2: move SRIOV VF check into specify functions;
modify commit description and comment.
Signed-off-by: Liu ChengZhe
---
drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c | 19 +++
drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c | 19 +++
2 files changed, 38 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
b/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
index 1f6112b7fa49..80c906a0383f 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
@@ -182,6 +182,12 @@ static void gfxhub_v2_1_init_cache_regs(struct
amdgpu_device *adev)
{
uint32_t tmp;
+ /* These registers are not accessible to VF-SRIOV.
+* The PF will program them instead.
+*/
+ if (amdgpu_sriov_vf(adev))
+ return;
+
/* Setup L2 cache */
tmp = RREG32_SOC15(GC, 0, mmGCVM_L2_CNTL);
tmp = REG_SET_FIELD(tmp, GCVM_L2_CNTL, ENABLE_L2_CACHE, 1);
@@ -237,6 +243,12 @@ static void gfxhub_v2_1_enable_system_domain(struct
amdgpu_device *adev)
static void gfxhub_v2_1_disable_identity_aperture(struct amdgpu_device *adev)
{
+ /* These registers are not accessible to VF-SRIOV.
+* The PF will program them instead.
+*/
+ if (amdgpu_sriov_vf(adev))
+ return;
+
WREG32_SOC15(GC, 0, mmGCVM_L2_CONTEXT1_IDENTITY_APERTURE_LOW_ADDR_LO32,
0x);
WREG32_SOC15(GC, 0, mmGCVM_L2_CONTEXT1_IDENTITY_APERTURE_LOW_ADDR_HI32,
@@ -373,6 +385,13 @@ void gfxhub_v2_1_set_fault_enable_default(struct
amdgpu_device *adev,
bool value)
{
u32 tmp;
+
+ /* These registers are not accessible to VF-SRIOV.
+* The PF will program them instead.
+*/
+ if (amdgpu_sriov_vf(adev))
+ return;
+
tmp = RREG32_SOC15(GC, 0, mmGCVM_L2_PROTECTION_FAULT_CNTL);
tmp = REG_SET_FIELD(tmp, GCVM_L2_PROTECTION_FAULT_CNTL,
RANGE_PROTECTION_FAULT_ENABLE_DEFAULT, value);
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
index d83912901f73..8acb3b625afe 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
@@ -181,6 +181,12 @@ static void mmhub_v2_0_init_cache_regs(struct
amdgpu_device *adev)
{
uint32_t tmp;
+ /* These registers are not accessible to VF-SRIOV.
+* The PF will program them instead.
+*/
+ if (amdgpu_sriov_vf(adev))
+ return;
+
/* Setup L2 cache */
tmp = RREG32_SOC15(MMHUB, 0, mmMMVM_L2_CNTL);
tmp = REG_SET_FIELD(tmp, MMVM_L2_CNTL, ENABLE_L2_CACHE, 1);
@@ -236,6 +242,12 @@ static void mmhub_v2_0_enable_system_domain(struct
amdgpu_device *adev)
static void mmhub_v2_0_disable_identity_aperture(struct amdgpu_device *adev)
{
+ /* These registers are not accessible to VF-SRIOV.
+* The PF will program them instead.
+*/
+ if (amdgpu_sriov_vf(adev))
+ return;
+
WREG32_SOC15(MMHUB, 0,
mmMMVM_L2_CONTEXT1_IDENTITY_APERTURE_LOW_ADDR_LO32,
0x);
@@ -365,6 +377,13 @@ void mmhub_v2_0_gart_disable(struct amdgpu_device *adev)
void mmhub_v2_0_set_fault_enable_default(struct amdgpu_device *adev, bool
value)
{
u32 tmp;
+
+ /* These registers are not accessible to VF-SRIOV.
+* The PF will program them instead.
+*/
+ if (amdgpu_sriov_vf(adev))
+ return;
+
tmp = RREG32_SOC15(MMHUB, 0, mmMMVM_L2_PROTECTION_FAULT_CNTL);
tmp = REG_SET_FIELD(tmp, MMVM_L2_PROTECTION_FAULT_CNTL,
RANGE_PROTECTION_FAULT_ENABLE_DEFAULT, value);
--
2.25.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Skip some registers config for SRIOV
On 2020-08-06 3:40 a.m., Liu ChengZhe wrote:
> For VF, registers L2_CNTL, L2_CONTEXT1_IDENTITY_APERTURE
> L2_PROTECTION_FAULT_CNTL are not accesible, skip the
Spelling: "accessible".
I'd rather move the "For VF" to the end, after "accessible",
like this. I also believe it should be "to VF" as opposed
to "for VF". Both are correct, but mean different things.
Maybe like this:
Some registers are not accessible to virtual function
setup, so skip their initialization when in VF-SRIOV mode.
> configuration for them in SRIOV mode.
>
> Signed-off-by: Liu ChengZhe
> ---
> drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c | 12 ++--
> drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c | 12 ++--
> 2 files changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
> b/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
> index 1f6112b7fa49..6b96f45fde2a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
> @@ -330,10 +330,13 @@ int gfxhub_v2_1_gart_enable(struct amdgpu_device *adev)
> gfxhub_v2_1_init_gart_aperture_regs(adev);
> gfxhub_v2_1_init_system_aperture_regs(adev);
> gfxhub_v2_1_init_tlb_regs(adev);
> - gfxhub_v2_1_init_cache_regs(adev);
> + if (!amdgpu_sriov_vf(adev))
> + gfxhub_v2_1_init_cache_regs(adev);
Yes, that's the literal meaning of the commit description,
but it would be cleaner if gfxhub_v2_1_init_cache_regs(adev)
did that check instead. (Some might even argue it'd be more
object-oriented...)
So then, this code would look like a sequence of
statements, unchanged, and each method of initialization
would know/check whether it is applicable to "adev".
It also makes it more centralized, less code duplication
and less code clutter.
>
> gfxhub_v2_1_enable_system_domain(adev);
> - gfxhub_v2_1_disable_identity_aperture(adev);
> + if (!amdgpu_sriov_vf(adev))
> + gfxhub_v2_1_disable_identity_aperture(adev);
> +
Ditto.
> gfxhub_v2_1_setup_vmid_config(adev);
> gfxhub_v2_1_program_invalidation(adev);
>
> @@ -372,7 +375,12 @@ void gfxhub_v2_1_gart_disable(struct amdgpu_device *adev)
> void gfxhub_v2_1_set_fault_enable_default(struct amdgpu_device *adev,
> bool value)
> {
> + /*These regs are not accessible for VF, PF will program in SRIOV */
Add a space after the asterisk and before the beginning of the sentence.
Format the comment in one of the two acceptable Linux Kernel Coding styles.
End the sentence with a full stop. It also helps to spell out "registers".
Like this perhaps:
/* These registers are not accessible to VF-SRIOV.
* The PF will program them instead.
*/
> + if (amdgpu_sriov_vf(adev))
> + return;
> +
> u32 tmp;
The definition of this automatic variable should precede
all other code in this function. The compiler will optimize
it away anyway.
> +
> tmp = RREG32_SOC15(GC, 0, mmGCVM_L2_PROTECTION_FAULT_CNTL);
> tmp = REG_SET_FIELD(tmp, GCVM_L2_PROTECTION_FAULT_CNTL,
> RANGE_PROTECTION_FAULT_ENABLE_DEFAULT, value);
> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
> b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
> index d83912901f73..9cfde9b81600 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
> @@ -321,10 +321,13 @@ int mmhub_v2_0_gart_enable(struct amdgpu_device *adev)
> mmhub_v2_0_init_gart_aperture_regs(adev);
> mmhub_v2_0_init_system_aperture_regs(adev);
> mmhub_v2_0_init_tlb_regs(adev);
> - mmhub_v2_0_init_cache_regs(adev);
> + if (!amdgpu_sriov_vf(adev))
> + mmhub_v2_0_init_cache_regs(adev);
Move this check inside said function.
>
> mmhub_v2_0_enable_system_domain(adev);
> - mmhub_v2_0_disable_identity_aperture(adev);
> + if (!amdgpu_sriov_vf(adev))
> + mmhub_v2_0_disable_identity_aperture(adev);
> +
Ditto.
> mmhub_v2_0_setup_vmid_config(adev);
> mmhub_v2_0_program_invalidation(adev);
>
> @@ -364,7 +367,12 @@ void mmhub_v2_0_gart_disable(struct amdgpu_device *adev)
> */
> void mmhub_v2_0_set_fault_enable_default(struct amdgpu_device *adev, bool
> value)
> {
> + /*These regs are not accessible for VF, PF will program in SRIOV */
You can duplicate this comment from the one above.
> + if (amdgpu_sriov_vf(adev))
> + return;
> +
> u32 tmp;
Should be the first thing in the function body, as noted above.
Regards,
Luben
> +
> tmp = RREG32_SOC15(MMHUB, 0, mmMMVM_L2_PROTECTION_FAULT_CNTL);
> tmp = REG_SET_FIELD(tmp, MMVM_L2_PROTECTION_FAULT_CNTL,
> RANGE_PROTECTION_FAULT_ENABLE_DEFAULT, value);
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: Skip some registers config for SRIOV
For VF, registers L2_CNTL, L2_CONTEXT1_IDENTITY_APERTURE
L2_PROTECTION_FAULT_CNTL are not accesible, skip the
configuration for them in SRIOV mode.
Signed-off-by: Liu ChengZhe
---
drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c | 12 ++--
drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c | 12 ++--
2 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
b/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
index 1f6112b7fa49..6b96f45fde2a 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
@@ -330,10 +330,13 @@ int gfxhub_v2_1_gart_enable(struct amdgpu_device *adev)
gfxhub_v2_1_init_gart_aperture_regs(adev);
gfxhub_v2_1_init_system_aperture_regs(adev);
gfxhub_v2_1_init_tlb_regs(adev);
- gfxhub_v2_1_init_cache_regs(adev);
+ if (!amdgpu_sriov_vf(adev))
+ gfxhub_v2_1_init_cache_regs(adev);
gfxhub_v2_1_enable_system_domain(adev);
- gfxhub_v2_1_disable_identity_aperture(adev);
+ if (!amdgpu_sriov_vf(adev))
+ gfxhub_v2_1_disable_identity_aperture(adev);
+
gfxhub_v2_1_setup_vmid_config(adev);
gfxhub_v2_1_program_invalidation(adev);
@@ -372,7 +375,12 @@ void gfxhub_v2_1_gart_disable(struct amdgpu_device *adev)
void gfxhub_v2_1_set_fault_enable_default(struct amdgpu_device *adev,
bool value)
{
+ /*These regs are not accessible for VF, PF will program in SRIOV */
+ if (amdgpu_sriov_vf(adev))
+ return;
+
u32 tmp;
+
tmp = RREG32_SOC15(GC, 0, mmGCVM_L2_PROTECTION_FAULT_CNTL);
tmp = REG_SET_FIELD(tmp, GCVM_L2_PROTECTION_FAULT_CNTL,
RANGE_PROTECTION_FAULT_ENABLE_DEFAULT, value);
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
index d83912901f73..9cfde9b81600 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
@@ -321,10 +321,13 @@ int mmhub_v2_0_gart_enable(struct amdgpu_device *adev)
mmhub_v2_0_init_gart_aperture_regs(adev);
mmhub_v2_0_init_system_aperture_regs(adev);
mmhub_v2_0_init_tlb_regs(adev);
- mmhub_v2_0_init_cache_regs(adev);
+ if (!amdgpu_sriov_vf(adev))
+ mmhub_v2_0_init_cache_regs(adev);
mmhub_v2_0_enable_system_domain(adev);
- mmhub_v2_0_disable_identity_aperture(adev);
+ if (!amdgpu_sriov_vf(adev))
+ mmhub_v2_0_disable_identity_aperture(adev);
+
mmhub_v2_0_setup_vmid_config(adev);
mmhub_v2_0_program_invalidation(adev);
@@ -364,7 +367,12 @@ void mmhub_v2_0_gart_disable(struct amdgpu_device *adev)
*/
void mmhub_v2_0_set_fault_enable_default(struct amdgpu_device *adev, bool
value)
{
+ /*These regs are not accessible for VF, PF will program in SRIOV */
+ if (amdgpu_sriov_vf(adev))
+ return;
+
u32 tmp;
+
tmp = RREG32_SOC15(MMHUB, 0, mmMMVM_L2_PROTECTION_FAULT_CNTL);
tmp = REG_SET_FIELD(tmp, MMVM_L2_PROTECTION_FAULT_CNTL,
RANGE_PROTECTION_FAULT_ENABLE_DEFAULT, value);
--
2.25.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
