Re: [PATCH 09/13] drm/amdgpu:fix gmc_v9 vm fault process for SRIOV

[Zhang, Jerry (Junwei)]

On 03/27/2017 02:39 PM, Christian König wrote:

Am 27.03.2017 um 03:51 schrieb Zhang, Jerry (Junwei):

On 03/24/2017 06:38 PM, Monk Liu wrote:

for SRIOV we cannot use access register when in IRQ routine
with regular KIQ method

Change-Id: Ifae3164cf12311b851ae131f58175f6ec3174f82
Signed-off-by: Monk Liu 
---
  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 24 
  1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 51a1919..88221bb 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -138,20 +138,28 @@ static int gmc_v9_0_process_interrupt(struct
amdgpu_device *adev,
  addr = (u64)entry->src_data[0] << 12;
  addr |= ((u64)entry->src_data[1] & 0xf) << 44;

-if (entry->vm_id_src) {
-status = RREG32(mmhub->vm_l2_pro_fault_status);
-WREG32_P(mmhub->vm_l2_pro_fault_cntl, 1, ~1);
-} else {
-status = RREG32(gfxhub->vm_l2_pro_fault_status);
-WREG32_P(gfxhub->vm_l2_pro_fault_cntl, 1, ~1);
-}
+if (!amdgpu_sriov_vf(adev)) {
+if (entry->vm_id_src) {
+status = RREG32(mmhub->vm_l2_pro_fault_status);
+WREG32_P(mmhub->vm_l2_pro_fault_cntl, 1, ~1);
+} else {
+status = RREG32(gfxhub->vm_l2_pro_fault_status);
+WREG32_P(gfxhub->vm_l2_pro_fault_cntl, 1, ~1);
+}


Even though SRIOV don't use status info, is it needed to clear vm L2 fault
cntl regs?


Actually it is forbidden to clear that register under SRIOV. So the answer is
no we shouldn't clear it.


Thanks for your clarification.
That's fine.

Jerry





If not,
Reviewed-by: Junwei Zhang 


Reviewed-by: Christian König  as well.

Regards,
Christian.





-DRM_ERROR("[%s]VMC page fault (src_id:%u ring:%u vm_id:%u pas_id:%u) "
+DRM_ERROR("[%s]VMC page fault (src_id:%u ring:%u vm_id:%u pas_id:%u) "
"at page 0x%016llx from %d\n"
"VM_L2_PROTECTION_FAULT_STATUS:0x%08X\n",
entry->vm_id_src ? "mmhub" : "gfxhub",
entry->src_id, entry->ring_id, entry->vm_id, entry->pas_id,
addr, entry->client_id, status);
+} else {
+DRM_ERROR("[%s]VMC page fault (src_id:%u ring:%u vm_id:%u pas_id:%u) "
+  "at page 0x%016llx from %d\n",
+  entry->vm_id_src ? "mmhub" : "gfxhub",
+  entry->src_id, entry->ring_id, entry->vm_id, entry->pas_id,
+  addr, entry->client_id);
+}

  return 0;
  }


___
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

Re: [PATCH 09/13] drm/amdgpu:fix gmc_v9 vm fault process for SRIOV

[Christian König]

Am 27.03.2017 um 03:51 schrieb Zhang, Jerry (Junwei):

On 03/24/2017 06:38 PM, Monk Liu wrote:

for SRIOV we cannot use access register when in IRQ routine
with regular KIQ method

Change-Id: Ifae3164cf12311b851ae131f58175f6ec3174f82
Signed-off-by: Monk Liu 
---
  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 24 
  1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c

index 51a1919..88221bb 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -138,20 +138,28 @@ static int gmc_v9_0_process_interrupt(struct 
amdgpu_device *adev,

  addr = (u64)entry->src_data[0] << 12;
  addr |= ((u64)entry->src_data[1] & 0xf) << 44;

-if (entry->vm_id_src) {
-status = RREG32(mmhub->vm_l2_pro_fault_status);
-WREG32_P(mmhub->vm_l2_pro_fault_cntl, 1, ~1);
-} else {
-status = RREG32(gfxhub->vm_l2_pro_fault_status);
-WREG32_P(gfxhub->vm_l2_pro_fault_cntl, 1, ~1);
-}
+if (!amdgpu_sriov_vf(adev)) {
+if (entry->vm_id_src) {
+status = RREG32(mmhub->vm_l2_pro_fault_status);
+WREG32_P(mmhub->vm_l2_pro_fault_cntl, 1, ~1);
+} else {
+status = RREG32(gfxhub->vm_l2_pro_fault_status);
+WREG32_P(gfxhub->vm_l2_pro_fault_cntl, 1, ~1);
+}


Even though SRIOV don't use status info, is it needed to clear vm L2 
fault cntl regs?


Actually it is forbidden to clear that register under SRIOV. So the 
answer is no we shouldn't clear it.




If not,
Reviewed-by: Junwei Zhang 


Reviewed-by: Christian König  as well.

Regards,
Christian.





-DRM_ERROR("[%s]VMC page fault (src_id:%u ring:%u vm_id:%u 
pas_id:%u) "
+DRM_ERROR("[%s]VMC page fault (src_id:%u ring:%u vm_id:%u 
pas_id:%u) "

"at page 0x%016llx from %d\n"
"VM_L2_PROTECTION_FAULT_STATUS:0x%08X\n",
entry->vm_id_src ? "mmhub" : "gfxhub",
entry->src_id, entry->ring_id, entry->vm_id, entry->pas_id,
addr, entry->client_id, status);
+} else {
+DRM_ERROR("[%s]VMC page fault (src_id:%u ring:%u vm_id:%u 
pas_id:%u) "

+  "at page 0x%016llx from %d\n",
+  entry->vm_id_src ? "mmhub" : "gfxhub",
+  entry->src_id, entry->ring_id, entry->vm_id, entry->pas_id,
+  addr, entry->client_id);
+}

  return 0;
  }


___
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

Re: [PATCH 09/13] drm/amdgpu:fix gmc_v9 vm fault process for SRIOV

[Zhang, Jerry (Junwei)]

On 03/24/2017 06:38 PM, Monk Liu wrote:

for SRIOV we cannot use access register when in IRQ routine
with regular KIQ method

Change-Id: Ifae3164cf12311b851ae131f58175f6ec3174f82
Signed-off-by: Monk Liu 
---
  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 24 
  1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 51a1919..88221bb 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -138,20 +138,28 @@ static int gmc_v9_0_process_interrupt(struct 
amdgpu_device *adev,
addr = (u64)entry->src_data[0] << 12;
addr |= ((u64)entry->src_data[1] & 0xf) << 44;

-   if (entry->vm_id_src) {
-   status = RREG32(mmhub->vm_l2_pro_fault_status);
-   WREG32_P(mmhub->vm_l2_pro_fault_cntl, 1, ~1);
-   } else {
-   status = RREG32(gfxhub->vm_l2_pro_fault_status);
-   WREG32_P(gfxhub->vm_l2_pro_fault_cntl, 1, ~1);
-   }
+   if (!amdgpu_sriov_vf(adev)) {
+   if (entry->vm_id_src) {
+   status = RREG32(mmhub->vm_l2_pro_fault_status);
+   WREG32_P(mmhub->vm_l2_pro_fault_cntl, 1, ~1);
+   } else {
+   status = RREG32(gfxhub->vm_l2_pro_fault_status);
+   WREG32_P(gfxhub->vm_l2_pro_fault_cntl, 1, ~1);
+   }


Even though SRIOV don't use status info, is it needed to clear vm L2 fault cntl 
regs?


If not,
Reviewed-by: Junwei Zhang 



-   DRM_ERROR("[%s]VMC page fault (src_id:%u ring:%u vm_id:%u pas_id:%u) "
+   DRM_ERROR("[%s]VMC page fault (src_id:%u ring:%u vm_id:%u pas_id:%u) 
"
  "at page 0x%016llx from %d\n"
  "VM_L2_PROTECTION_FAULT_STATUS:0x%08X\n",
  entry->vm_id_src ? "mmhub" : "gfxhub",
  entry->src_id, entry->ring_id, entry->vm_id, entry->pas_id,
  addr, entry->client_id, status);
+   } else {
+   DRM_ERROR("[%s]VMC page fault (src_id:%u ring:%u vm_id:%u pas_id:%u) 
"
+ "at page 0x%016llx from %d\n",
+ entry->vm_id_src ? "mmhub" : "gfxhub",
+ entry->src_id, entry->ring_id, entry->vm_id, entry->pas_id,
+ addr, entry->client_id);
+   }

return 0;
  }


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

RE: [PATCH 09/13] drm/amdgpu:fix gmc_v9 vm fault process for SRIOV

[Deucher, Alexander]

> -Original Message-
> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Monk Liu
> Sent: Friday, March 24, 2017 6:38 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Liu, Monk
> Subject: [PATCH 09/13] drm/amdgpu:fix gmc_v9 vm fault process for SRIOV
> 
> for SRIOV we cannot use access register when in IRQ routine
> with regular KIQ method
> 
> Change-Id: Ifae3164cf12311b851ae131f58175f6ec3174f82
> Signed-off-by: Monk Liu 
> ---
>  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 24 
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 51a1919..88221bb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -138,20 +138,28 @@ static int gmc_v9_0_process_interrupt(struct
> amdgpu_device *adev,
>   addr = (u64)entry->src_data[0] << 12;
>   addr |= ((u64)entry->src_data[1] & 0xf) << 44;
> 
> - if (entry->vm_id_src) {
> - status = RREG32(mmhub->vm_l2_pro_fault_status);
> - WREG32_P(mmhub->vm_l2_pro_fault_cntl, 1, ~1);
> - } else {
> - status = RREG32(gfxhub->vm_l2_pro_fault_status);
> - WREG32_P(gfxhub->vm_l2_pro_fault_cntl, 1, ~1);
> - }
> + if (!amdgpu_sriov_vf(adev)) {
> + if (entry->vm_id_src) {
> + status = RREG32(mmhub->vm_l2_pro_fault_status);
> + WREG32_P(mmhub->vm_l2_pro_fault_cntl, 1, ~1);
> + } else {
> + status = RREG32(gfxhub->vm_l2_pro_fault_status);
> + WREG32_P(gfxhub->vm_l2_pro_fault_cntl, 1, ~1);
> + }
> 
> - DRM_ERROR("[%s]VMC page fault (src_id:%u ring:%u vm_id:%u
> pas_id:%u) "
> + DRM_ERROR("[%s]VMC page fault (src_id:%u ring:%u
> vm_id:%u pas_id:%u) "
> "at page 0x%016llx from %d\n"
> "VM_L2_PROTECTION_FAULT_STATUS:0x%08X\n",
> entry->vm_id_src ? "mmhub" : "gfxhub",
> entry->src_id, entry->ring_id, entry->vm_id, entry->pas_id,
> addr, entry->client_id, status);

Fix the indentation here.

> + } else {
> + DRM_ERROR("[%s]VMC page fault (src_id:%u ring:%u
> vm_id:%u pas_id:%u) "
> +   "at page 0x%016llx from %d\n",
> +   entry->vm_id_src ? "mmhub" : "gfxhub",
> +   entry->src_id, entry->ring_id, entry->vm_id, entry->pas_id,
> +   addr, entry->client_id);

And here.  With that fixed:
Reviewed-by: Alex Deucher 


> + }
> 
>   return 0;
>  }
> --
> 2.7.4
> 
> ___
> 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