* [PATCH v2] drm/amdgpu/sriov: For sriov runtime, use kiq to do invalidate tlb
@ 2018-08-14 10:56 Emily Deng
[not found] ` <1534244211-4717-1-git-send-email-Emily.Deng-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Emily Deng @ 2018-08-14 10:56 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Emily Deng
To avoid the tlb flush not interrupted by world switch, use kiq and one
command to do tlb invalidate.
v2:
Add firmware version checking.
SWDEV-161497
Signed-off-by: Emily Deng <Emily.Deng@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 63 ++++++++++++++++++++++++++++++++
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 2 +
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 ++++
3 files changed, 72 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
index 21adb1b6..436030c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
@@ -233,6 +233,69 @@ void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v)
pr_err("failed to write reg:%x\n", reg);
}
+signed long amdgpu_virt_kiq_invalidate_tlb(struct amdgpu_device *adev, struct amdgpu_vmhub *hub,
+ unsigned eng, u32 req, uint32_t vmid)
+{
+ signed long r, cnt = 0;
+ unsigned long flags;
+ uint32_t seq;
+ struct amdgpu_kiq *kiq = &adev->gfx.kiq;
+ struct amdgpu_ring *ring = &kiq->ring;
+ struct drm_amdgpu_info_firmware fw_info;
+
+ if (ring->me == 1) {
+ fw_info.ver = adev->gfx.mec_fw_version;
+ fw_info.feature = adev->gfx.mec_feature_version;
+ } else {
+ fw_info.ver = adev->gfx.mec2_fw_version;
+ fw_info.feature = adev->gfx.mec2_feature_version;
+ }
+
+ if (fw_info.ver < 0x00000190 || fw_info.feature < 42)
+ return -EPERM;
+
+ BUG_ON(!ring->funcs->emit_reg_write_reg_wait);
+
+ spin_lock_irqsave(&kiq->ring_lock, flags);
+ amdgpu_ring_alloc(ring, 32);
+ amdgpu_ring_emit_reg_write_reg_wait(ring, hub->vm_inv_eng0_req + eng,
+ hub->vm_inv_eng0_ack + eng,
+ req, 1 << vmid);
+ amdgpu_fence_emit_polling(ring, &seq);
+ amdgpu_ring_commit(ring);
+ spin_unlock_irqrestore(&kiq->ring_lock, flags);
+
+ r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
+
+ /* don't wait anymore for gpu reset case because this way may
+ * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
+ * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
+ * never return if we keep waiting in virt_kiq_rreg, which cause
+ * gpu_recover() hang there.
+ *
+ * also don't wait anymore for IRQ context
+ * */
+ if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
+ goto failed_kiq;
+
+ if (in_interrupt())
+ might_sleep();
+
+ while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
+ msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
+ r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
+ }
+
+ if (cnt > MAX_KIQ_REG_TRY)
+ goto failed_kiq;
+
+ return 0;
+
+failed_kiq:
+ pr_err("failed to invalidate tlb with kiq\n");
+ return r;
+}
+
/**
* amdgpu_virt_request_full_gpu() - request full gpu access
* @amdgpu: amdgpu device.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
index 880ac11..8908ff9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
@@ -288,6 +288,8 @@ void amdgpu_free_static_csa(struct amdgpu_device *adev);
void amdgpu_virt_init_setting(struct amdgpu_device *adev);
uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t reg);
void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v);
+signed long amdgpu_virt_kiq_invalidate_tlb(struct amdgpu_device *adev, struct amdgpu_vmhub *hub,
+ unsigned eng, u32 req, uint32_t vmid);
int amdgpu_virt_request_full_gpu(struct amdgpu_device *adev, bool init);
int amdgpu_virt_release_full_gpu(struct amdgpu_device *adev, bool init);
int amdgpu_virt_reset_gpu(struct amdgpu_device *adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 6999042..812f71e 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -331,6 +331,7 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev,
/* Use register 17 for GART */
const unsigned eng = 17;
unsigned i, j;
+ int r;
spin_lock(&adev->gmc.invalidate_lock);
@@ -338,6 +339,12 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev,
struct amdgpu_vmhub *hub = &adev->vmhub[i];
u32 tmp = gmc_v9_0_get_invalidate_req(vmid);
+ if (amdgpu_sriov_vf(adev) && amdgpu_sriov_runtime(adev)) {
+ r = amdgpu_virt_kiq_invalidate_tlb(adev, hub, eng, tmp, vmid);
+ if (!r)
+ continue;
+ }
+
WREG32_NO_KIQ(hub->vm_inv_eng0_req + eng, tmp);
/* Busy wait for ACK.*/
--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/amdgpu/sriov: For sriov runtime, use kiq to do invalidate tlb
[not found] ` <1534244211-4717-1-git-send-email-Emily.Deng-5C7GfCeVMHo@public.gmane.org>
@ 2018-08-14 12:45 ` Christian König
[not found] ` <a865d2c4-2373-1101-2dd1-ca3a8f0792eb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Christian König @ 2018-08-14 12:45 UTC (permalink / raw)
To: Emily Deng, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Am 14.08.2018 um 12:56 schrieb Emily Deng:
> To avoid the tlb flush not interrupted by world switch, use kiq and one
> command to do tlb invalidate.
>
> v2:
> Add firmware version checking.
>
> SWDEV-161497
>
> Signed-off-by: Emily Deng <Emily.Deng@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 63 ++++++++++++++++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 2 +
> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 ++++
> 3 files changed, 72 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index 21adb1b6..436030c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -233,6 +233,69 @@ void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v)
> pr_err("failed to write reg:%x\n", reg);
> }
>
> +signed long amdgpu_virt_kiq_invalidate_tlb(struct amdgpu_device *adev, struct amdgpu_vmhub *hub,
> + unsigned eng, u32 req, uint32_t vmid)
Drop that function here. It is Vega specific and doesn't belong into
common code.
> +{
> + signed long r, cnt = 0;
> + unsigned long flags;
> + uint32_t seq;
> + struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> + struct amdgpu_ring *ring = &kiq->ring;
> + struct drm_amdgpu_info_firmware fw_info;
> +
> + if (ring->me == 1) {
> + fw_info.ver = adev->gfx.mec_fw_version;
> + fw_info.feature = adev->gfx.mec_feature_version;
> + } else {
> + fw_info.ver = adev->gfx.mec2_fw_version;
> + fw_info.feature = adev->gfx.mec2_feature_version;
> + }
> +
> + if (fw_info.ver < 0x00000190 || fw_info.feature < 42)
> + return -EPERM;
Please move that check into gfx_v9_0_ring_emit_reg_write_reg_wait().
> +
> + BUG_ON(!ring->funcs->emit_reg_write_reg_wait);
That check is superfluous.
> +
> + spin_lock_irqsave(&kiq->ring_lock, flags);
> + amdgpu_ring_alloc(ring, 32);
> + amdgpu_ring_emit_reg_write_reg_wait(ring, hub->vm_inv_eng0_req + eng,
> + hub->vm_inv_eng0_ack + eng,
> + req, 1 << vmid);
> + amdgpu_fence_emit_polling(ring, &seq);
> + amdgpu_ring_commit(ring);
> + spin_unlock_irqrestore(&kiq->ring_lock, flags);
> +
> + r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> +
> + /* don't wait anymore for gpu reset case because this way may
> + * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
> + * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
> + * never return if we keep waiting in virt_kiq_rreg, which cause
> + * gpu_recover() hang there.
> + *
> + * also don't wait anymore for IRQ context
> + * */
> + if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
> + goto failed_kiq;
> +
> + if (in_interrupt())
> + might_sleep();
> +
> + while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
> + msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
> + r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> + }
> +
> + if (cnt > MAX_KIQ_REG_TRY)
> + goto failed_kiq;
> +
> + return 0;
> +
> +failed_kiq:
> + pr_err("failed to invalidate tlb with kiq\n");
> + return r;
> +}
> +
> /**
> * amdgpu_virt_request_full_gpu() - request full gpu access
> * @amdgpu: amdgpu device.
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> index 880ac11..8908ff9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> @@ -288,6 +288,8 @@ void amdgpu_free_static_csa(struct amdgpu_device *adev);
> void amdgpu_virt_init_setting(struct amdgpu_device *adev);
> uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t reg);
> void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v);
> +signed long amdgpu_virt_kiq_invalidate_tlb(struct amdgpu_device *adev, struct amdgpu_vmhub *hub,
> + unsigned eng, u32 req, uint32_t vmid);
> int amdgpu_virt_request_full_gpu(struct amdgpu_device *adev, bool init);
> int amdgpu_virt_release_full_gpu(struct amdgpu_device *adev, bool init);
> int amdgpu_virt_reset_gpu(struct amdgpu_device *adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 6999042..812f71e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -331,6 +331,7 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev,
> /* Use register 17 for GART */
> const unsigned eng = 17;
> unsigned i, j;
> + int r;
>
> spin_lock(&adev->gmc.invalidate_lock);
>
> @@ -338,6 +339,12 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev,
> struct amdgpu_vmhub *hub = &adev->vmhub[i];
> u32 tmp = gmc_v9_0_get_invalidate_req(vmid);
>
> + if (amdgpu_sriov_vf(adev) && amdgpu_sriov_runtime(adev)) {
> + r = amdgpu_virt_kiq_invalidate_tlb(adev, hub, eng, tmp, vmid);
> + if (!r)
> + continue;
> + }
> +
Drop that SRIOV check here. We don't want specialized code path for SRIOV.
Christian.
> WREG32_NO_KIQ(hub->vm_inv_eng0_req + eng, tmp);
>
> /* Busy wait for ACK.*/
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v2] drm/amdgpu/sriov: For sriov runtime, use kiq to do invalidate tlb
[not found] ` <a865d2c4-2373-1101-2dd1-ca3a8f0792eb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2018-08-15 2:28 ` Deng, Emily
[not found] ` <CY4PR12MB11250F5E45EBEBDD519056BB8F3F0-rpdhrqHFk07v2MZdTKcfDgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Deng, Emily @ 2018-08-15 2:28 UTC (permalink / raw)
To: Koenig, Christian, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Thanks for your review, will send a patch to review again as your suggestion. I think it will be better to use the amdgpu_sriov_vf(adev).
As need to let somebody to test all your bare metal test cases, such as vega10, and other asics, as I don't have those environments, and only could test sriov.
If passed those bare metal test cases and fixed the bugs if occurs, then could remove the amdgpu_sriov_vf(adev). Do you think so?
Best wishes
Emily Deng
>-----Original Message-----
>From: Christian König <ckoenig.leichtzumerken@gmail.com>
>Sent: Tuesday, August 14, 2018 8:46 PM
>To: Deng, Emily <Emily.Deng@amd.com>; amd-gfx@lists.freedesktop.org
>Subject: Re: [PATCH v2] drm/amdgpu/sriov: For sriov runtime, use kiq to do
>invalidate tlb
>
>Am 14.08.2018 um 12:56 schrieb Emily Deng:
>> To avoid the tlb flush not interrupted by world switch, use kiq and
>> one command to do tlb invalidate.
>>
>> v2:
>> Add firmware version checking.
>>
>> SWDEV-161497
>>
>> Signed-off-by: Emily Deng <Emily.Deng@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 63
>++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 2 +
>> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 ++++
>> 3 files changed, 72 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>> index 21adb1b6..436030c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>> @@ -233,6 +233,69 @@ void amdgpu_virt_kiq_wreg(struct amdgpu_device
>*adev, uint32_t reg, uint32_t v)
>> pr_err("failed to write reg:%x\n", reg);
>> }
>>
>> +signed long amdgpu_virt_kiq_invalidate_tlb(struct amdgpu_device *adev,
>struct amdgpu_vmhub *hub,
>> + unsigned eng, u32 req, uint32_t vmid)
>
>Drop that function here. It is Vega specific and doesn't belong into common
>code.
>
>> +{
>> + signed long r, cnt = 0;
>> + unsigned long flags;
>> + uint32_t seq;
>> + struct amdgpu_kiq *kiq = &adev->gfx.kiq;
>> + struct amdgpu_ring *ring = &kiq->ring;
>> + struct drm_amdgpu_info_firmware fw_info;
>> +
>> + if (ring->me == 1) {
>> + fw_info.ver = adev->gfx.mec_fw_version;
>> + fw_info.feature = adev->gfx.mec_feature_version;
>> + } else {
>> + fw_info.ver = adev->gfx.mec2_fw_version;
>> + fw_info.feature = adev->gfx.mec2_feature_version;
>> + }
>> +
>> + if (fw_info.ver < 0x00000190 || fw_info.feature < 42)
>> + return -EPERM;
>
>Please move that check into gfx_v9_0_ring_emit_reg_write_reg_wait().
>
>> +
>> + BUG_ON(!ring->funcs->emit_reg_write_reg_wait);
>
>That check is superfluous.
>
>> +
>> + spin_lock_irqsave(&kiq->ring_lock, flags);
>> + amdgpu_ring_alloc(ring, 32);
>> + amdgpu_ring_emit_reg_write_reg_wait(ring, hub->vm_inv_eng0_req +
>eng,
>> + hub->vm_inv_eng0_ack + eng,
>> + req, 1 << vmid);
>> + amdgpu_fence_emit_polling(ring, &seq);
>> + amdgpu_ring_commit(ring);
>> + spin_unlock_irqrestore(&kiq->ring_lock, flags);
>> +
>> + r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
>> +
>> + /* don't wait anymore for gpu reset case because this way may
>> + * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
>> + * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
>> + * never return if we keep waiting in virt_kiq_rreg, which cause
>> + * gpu_recover() hang there.
>> + *
>> + * also don't wait anymore for IRQ context
>> + * */
>> + if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
>> + goto failed_kiq;
>> +
>> + if (in_interrupt())
>> + might_sleep();
>> +
>> + while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
>> + msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
>> + r = amdgpu_fence_wait_polling(ring, seq,
>MAX_KIQ_REG_WAIT);
>> + }
>> +
>> + if (cnt > MAX_KIQ_REG_TRY)
>> + goto failed_kiq;
>> +
>> + return 0;
>> +
>> +failed_kiq:
>> + pr_err("failed to invalidate tlb with kiq\n");
>> + return r;
>> +}
>> +
>> /**
>> * amdgpu_virt_request_full_gpu() - request full gpu access
>> * @amdgpu: amdgpu device.
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>> index 880ac11..8908ff9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>> @@ -288,6 +288,8 @@ void amdgpu_free_static_csa(struct amdgpu_device
>*adev);
>> void amdgpu_virt_init_setting(struct amdgpu_device *adev);
>> uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t reg);
>> void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg,
>> uint32_t v);
>> +signed long amdgpu_virt_kiq_invalidate_tlb(struct amdgpu_device *adev,
>struct amdgpu_vmhub *hub,
>> + unsigned eng, u32 req, uint32_t vmid);
>> int amdgpu_virt_request_full_gpu(struct amdgpu_device *adev, bool init);
>> int amdgpu_virt_release_full_gpu(struct amdgpu_device *adev, bool init);
>> int amdgpu_virt_reset_gpu(struct amdgpu_device *adev); diff --git
>> a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> index 6999042..812f71e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -331,6 +331,7 @@ static void gmc_v9_0_flush_gpu_tlb(struct
>amdgpu_device *adev,
>> /* Use register 17 for GART */
>> const unsigned eng = 17;
>> unsigned i, j;
>> + int r;
>>
>> spin_lock(&adev->gmc.invalidate_lock);
>>
>> @@ -338,6 +339,12 @@ static void gmc_v9_0_flush_gpu_tlb(struct
>amdgpu_device *adev,
>> struct amdgpu_vmhub *hub = &adev->vmhub[i];
>> u32 tmp = gmc_v9_0_get_invalidate_req(vmid);
>>
>> + if (amdgpu_sriov_vf(adev) && amdgpu_sriov_runtime(adev)) {
>> + r = amdgpu_virt_kiq_invalidate_tlb(adev, hub, eng, tmp,
>vmid);
>> + if (!r)
>> + continue;
>> + }
>> +
>
>Drop that SRIOV check here. We don't want specialized code path for SRIOV.
>
>Christian.
>
>> WREG32_NO_KIQ(hub->vm_inv_eng0_req + eng, tmp);
>>
>> /* Busy wait for ACK.*/
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/amdgpu/sriov: For sriov runtime, use kiq to do invalidate tlb
[not found] ` <CY4PR12MB11250F5E45EBEBDD519056BB8F3F0-rpdhrqHFk07v2MZdTKcfDgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2018-08-15 7:07 ` Christian König
[not found] ` <2e0a1f3e-4e8a-b9ae-bfe6-46b4b74144ce-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Christian König @ 2018-08-15 7:07 UTC (permalink / raw)
To: Deng, Emily, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
> As need to let somebody to test all your bare metal test cases, such as vega10, and other asics, as I don't have those environments, and only could test sriov.
That is not much of a problem. We have plenty of people on the mailing
list (including me) who can test this on both Vega10 and Raven.
Just remove the SRIOV check and send the patch out with a request for
testing it on bare metal.
Christian.
Am 15.08.2018 um 04:28 schrieb Deng, Emily:
> Thanks for your review, will send a patch to review again as your suggestion. I think it will be better to use the amdgpu_sriov_vf(adev).
> As need to let somebody to test all your bare metal test cases, such as vega10, and other asics, as I don't have those environments, and only could test sriov.
> If passed those bare metal test cases and fixed the bugs if occurs, then could remove the amdgpu_sriov_vf(adev). Do you think so?
>
> Best wishes
> Emily Deng
>> -----Original Message-----
>> From: Christian König <ckoenig.leichtzumerken@gmail.com>
>> Sent: Tuesday, August 14, 2018 8:46 PM
>> To: Deng, Emily <Emily.Deng@amd.com>; amd-gfx@lists.freedesktop.org
>> Subject: Re: [PATCH v2] drm/amdgpu/sriov: For sriov runtime, use kiq to do
>> invalidate tlb
>>
>> Am 14.08.2018 um 12:56 schrieb Emily Deng:
>>> To avoid the tlb flush not interrupted by world switch, use kiq and
>>> one command to do tlb invalidate.
>>>
>>> v2:
>>> Add firmware version checking.
>>>
>>> SWDEV-161497
>>>
>>> Signed-off-by: Emily Deng <Emily.Deng@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 63
>> ++++++++++++++++++++++++++++++++
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 2 +
>>> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 ++++
>>> 3 files changed, 72 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>>> index 21adb1b6..436030c 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>>> @@ -233,6 +233,69 @@ void amdgpu_virt_kiq_wreg(struct amdgpu_device
>> *adev, uint32_t reg, uint32_t v)
>>> pr_err("failed to write reg:%x\n", reg);
>>> }
>>>
>>> +signed long amdgpu_virt_kiq_invalidate_tlb(struct amdgpu_device *adev,
>> struct amdgpu_vmhub *hub,
>>> + unsigned eng, u32 req, uint32_t vmid)
>> Drop that function here. It is Vega specific and doesn't belong into common
>> code.
>>
>>> +{
>>> + signed long r, cnt = 0;
>>> + unsigned long flags;
>>> + uint32_t seq;
>>> + struct amdgpu_kiq *kiq = &adev->gfx.kiq;
>>> + struct amdgpu_ring *ring = &kiq->ring;
>>> + struct drm_amdgpu_info_firmware fw_info;
>>> +
>>> + if (ring->me == 1) {
>>> + fw_info.ver = adev->gfx.mec_fw_version;
>>> + fw_info.feature = adev->gfx.mec_feature_version;
>>> + } else {
>>> + fw_info.ver = adev->gfx.mec2_fw_version;
>>> + fw_info.feature = adev->gfx.mec2_feature_version;
>>> + }
>>> +
>>> + if (fw_info.ver < 0x00000190 || fw_info.feature < 42)
>>> + return -EPERM;
>> Please move that check into gfx_v9_0_ring_emit_reg_write_reg_wait().
>>
>>> +
>>> + BUG_ON(!ring->funcs->emit_reg_write_reg_wait);
>> That check is superfluous.
>>
>>> +
>>> + spin_lock_irqsave(&kiq->ring_lock, flags);
>>> + amdgpu_ring_alloc(ring, 32);
>>> + amdgpu_ring_emit_reg_write_reg_wait(ring, hub->vm_inv_eng0_req +
>> eng,
>>> + hub->vm_inv_eng0_ack + eng,
>>> + req, 1 << vmid);
>>> + amdgpu_fence_emit_polling(ring, &seq);
>>> + amdgpu_ring_commit(ring);
>>> + spin_unlock_irqrestore(&kiq->ring_lock, flags);
>>> +
>>> + r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
>>> +
>>> + /* don't wait anymore for gpu reset case because this way may
>>> + * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
>>> + * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
>>> + * never return if we keep waiting in virt_kiq_rreg, which cause
>>> + * gpu_recover() hang there.
>>> + *
>>> + * also don't wait anymore for IRQ context
>>> + * */
>>> + if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
>>> + goto failed_kiq;
>>> +
>>> + if (in_interrupt())
>>> + might_sleep();
>>> +
>>> + while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
>>> + msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
>>> + r = amdgpu_fence_wait_polling(ring, seq,
>> MAX_KIQ_REG_WAIT);
>>> + }
>>> +
>>> + if (cnt > MAX_KIQ_REG_TRY)
>>> + goto failed_kiq;
>>> +
>>> + return 0;
>>> +
>>> +failed_kiq:
>>> + pr_err("failed to invalidate tlb with kiq\n");
>>> + return r;
>>> +}
>>> +
>>> /**
>>> * amdgpu_virt_request_full_gpu() - request full gpu access
>>> * @amdgpu: amdgpu device.
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>>> index 880ac11..8908ff9 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>>> @@ -288,6 +288,8 @@ void amdgpu_free_static_csa(struct amdgpu_device
>> *adev);
>>> void amdgpu_virt_init_setting(struct amdgpu_device *adev);
>>> uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t reg);
>>> void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg,
>>> uint32_t v);
>>> +signed long amdgpu_virt_kiq_invalidate_tlb(struct amdgpu_device *adev,
>> struct amdgpu_vmhub *hub,
>>> + unsigned eng, u32 req, uint32_t vmid);
>>> int amdgpu_virt_request_full_gpu(struct amdgpu_device *adev, bool init);
>>> int amdgpu_virt_release_full_gpu(struct amdgpu_device *adev, bool init);
>>> int amdgpu_virt_reset_gpu(struct amdgpu_device *adev); diff --git
>>> a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> index 6999042..812f71e 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> @@ -331,6 +331,7 @@ static void gmc_v9_0_flush_gpu_tlb(struct
>> amdgpu_device *adev,
>>> /* Use register 17 for GART */
>>> const unsigned eng = 17;
>>> unsigned i, j;
>>> + int r;
>>>
>>> spin_lock(&adev->gmc.invalidate_lock);
>>>
>>> @@ -338,6 +339,12 @@ static void gmc_v9_0_flush_gpu_tlb(struct
>> amdgpu_device *adev,
>>> struct amdgpu_vmhub *hub = &adev->vmhub[i];
>>> u32 tmp = gmc_v9_0_get_invalidate_req(vmid);
>>>
>>> + if (amdgpu_sriov_vf(adev) && amdgpu_sriov_runtime(adev)) {
>>> + r = amdgpu_virt_kiq_invalidate_tlb(adev, hub, eng, tmp,
>> vmid);
>>> + if (!r)
>>> + continue;
>>> + }
>>> +
>> Drop that SRIOV check here. We don't want specialized code path for SRIOV.
>>
>> Christian.
>>
>>> WREG32_NO_KIQ(hub->vm_inv_eng0_req + eng, tmp);
>>>
>>> /* Busy wait for ACK.*/
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v2] drm/amdgpu/sriov: For sriov runtime, use kiq to do invalidate tlb
[not found] ` <2e0a1f3e-4e8a-b9ae-bfe6-46b4b74144ce-5C7GfCeVMHo@public.gmane.org>
@ 2018-08-15 7:56 ` Deng, Emily
2018-08-15 10:21 ` Zhu, Rex
1 sibling, 0 replies; 6+ messages in thread
From: Deng, Emily @ 2018-08-15 7:56 UTC (permalink / raw)
To: Koenig, Christian, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
>-----Original Message-----
>From: Koenig, Christian
>Sent: Wednesday, August 15, 2018 3:08 PM
>To: Deng, Emily <Emily.Deng@amd.com>; amd-gfx@lists.freedesktop.org
>Subject: Re: [PATCH v2] drm/amdgpu/sriov: For sriov runtime, use kiq to do
>invalidate tlb
>
>> As need to let somebody to test all your bare metal test cases, such as vega10,
>and other asics, as I don't have those environments, and only could test sriov.
>That is not much of a problem. We have plenty of people on the mailing list
>(including me) who can test this on both Vega10 and Raven.
>
>Just remove the SRIOV check and send the patch out with a request for testing
>it on bare metal.
Ok, I am testing it on SRIOV, and then sent it to review.
>Christian.
>
>Am 15.08.2018 um 04:28 schrieb Deng, Emily:
>> Thanks for your review, will send a patch to review again as your suggestion. I
>think it will be better to use the amdgpu_sriov_vf(adev).
>> As need to let somebody to test all your bare metal test cases, such as vega10,
>and other asics, as I don't have those environments, and only could test sriov.
>> If passed those bare metal test cases and fixed the bugs if occurs, then could
>remove the amdgpu_sriov_vf(adev). Do you think so?
>>
>> Best wishes
>> Emily Deng
>>> -----Original Message-----
>>> From: Christian König <ckoenig.leichtzumerken@gmail.com>
>>> Sent: Tuesday, August 14, 2018 8:46 PM
>>> To: Deng, Emily <Emily.Deng@amd.com>; amd-gfx@lists.freedesktop.org
>>> Subject: Re: [PATCH v2] drm/amdgpu/sriov: For sriov runtime, use kiq
>>> to do invalidate tlb
>>>
>>> Am 14.08.2018 um 12:56 schrieb Emily Deng:
>>>> To avoid the tlb flush not interrupted by world switch, use kiq and
>>>> one command to do tlb invalidate.
>>>>
>>>> v2:
>>>> Add firmware version checking.
>>>>
>>>> SWDEV-161497
>>>>
>>>> Signed-off-by: Emily Deng <Emily.Deng@amd.com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 63
>>> ++++++++++++++++++++++++++++++++
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 2 +
>>>> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 ++++
>>>> 3 files changed, 72 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>>>> index 21adb1b6..436030c 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>>>> @@ -233,6 +233,69 @@ void amdgpu_virt_kiq_wreg(struct
>amdgpu_device
>>> *adev, uint32_t reg, uint32_t v)
>>>> pr_err("failed to write reg:%x\n", reg);
>>>> }
>>>>
>>>> +signed long amdgpu_virt_kiq_invalidate_tlb(struct amdgpu_device
>>>> +*adev,
>>> struct amdgpu_vmhub *hub,
>>>> + unsigned eng, u32 req, uint32_t vmid)
>>> Drop that function here. It is Vega specific and doesn't belong into
>>> common code.
>>>
>>>> +{
>>>> + signed long r, cnt = 0;
>>>> + unsigned long flags;
>>>> + uint32_t seq;
>>>> + struct amdgpu_kiq *kiq = &adev->gfx.kiq;
>>>> + struct amdgpu_ring *ring = &kiq->ring;
>>>> + struct drm_amdgpu_info_firmware fw_info;
>>>> +
>>>> + if (ring->me == 1) {
>>>> + fw_info.ver = adev->gfx.mec_fw_version;
>>>> + fw_info.feature = adev->gfx.mec_feature_version;
>>>> + } else {
>>>> + fw_info.ver = adev->gfx.mec2_fw_version;
>>>> + fw_info.feature = adev->gfx.mec2_feature_version;
>>>> + }
>>>> +
>>>> + if (fw_info.ver < 0x00000190 || fw_info.feature < 42)
>>>> + return -EPERM;
>>> Please move that check into gfx_v9_0_ring_emit_reg_write_reg_wait().
>>>
>>>> +
>>>> + BUG_ON(!ring->funcs->emit_reg_write_reg_wait);
>>> That check is superfluous.
>>>
>>>> +
>>>> + spin_lock_irqsave(&kiq->ring_lock, flags);
>>>> + amdgpu_ring_alloc(ring, 32);
>>>> + amdgpu_ring_emit_reg_write_reg_wait(ring, hub->vm_inv_eng0_req +
>>> eng,
>>>> + hub->vm_inv_eng0_ack + eng,
>>>> + req, 1 << vmid);
>>>> + amdgpu_fence_emit_polling(ring, &seq);
>>>> + amdgpu_ring_commit(ring);
>>>> + spin_unlock_irqrestore(&kiq->ring_lock, flags);
>>>> +
>>>> + r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
>>>> +
>>>> + /* don't wait anymore for gpu reset case because this way may
>>>> + * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
>>>> + * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
>>>> + * never return if we keep waiting in virt_kiq_rreg, which cause
>>>> + * gpu_recover() hang there.
>>>> + *
>>>> + * also don't wait anymore for IRQ context
>>>> + * */
>>>> + if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
>>>> + goto failed_kiq;
>>>> +
>>>> + if (in_interrupt())
>>>> + might_sleep();
>>>> +
>>>> + while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
>>>> + msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
>>>> + r = amdgpu_fence_wait_polling(ring, seq,
>>> MAX_KIQ_REG_WAIT);
>>>> + }
>>>> +
>>>> + if (cnt > MAX_KIQ_REG_TRY)
>>>> + goto failed_kiq;
>>>> +
>>>> + return 0;
>>>> +
>>>> +failed_kiq:
>>>> + pr_err("failed to invalidate tlb with kiq\n");
>>>> + return r;
>>>> +}
>>>> +
>>>> /**
>>>> * amdgpu_virt_request_full_gpu() - request full gpu access
>>>> * @amdgpu: amdgpu device.
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>>>> index 880ac11..8908ff9 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>>>> @@ -288,6 +288,8 @@ void amdgpu_free_static_csa(struct
>amdgpu_device
>>> *adev);
>>>> void amdgpu_virt_init_setting(struct amdgpu_device *adev);
>>>> uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t
>reg);
>>>> void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t
>>>> reg, uint32_t v);
>>>> +signed long amdgpu_virt_kiq_invalidate_tlb(struct amdgpu_device
>>>> +*adev,
>>> struct amdgpu_vmhub *hub,
>>>> + unsigned eng, u32 req, uint32_t vmid);
>>>> int amdgpu_virt_request_full_gpu(struct amdgpu_device *adev, bool init);
>>>> int amdgpu_virt_release_full_gpu(struct amdgpu_device *adev, bool init);
>>>> int amdgpu_virt_reset_gpu(struct amdgpu_device *adev); diff --git
>>>> a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> index 6999042..812f71e 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> @@ -331,6 +331,7 @@ static void gmc_v9_0_flush_gpu_tlb(struct
>>> amdgpu_device *adev,
>>>> /* Use register 17 for GART */
>>>> const unsigned eng = 17;
>>>> unsigned i, j;
>>>> + int r;
>>>>
>>>> spin_lock(&adev->gmc.invalidate_lock);
>>>>
>>>> @@ -338,6 +339,12 @@ static void gmc_v9_0_flush_gpu_tlb(struct
>>> amdgpu_device *adev,
>>>> struct amdgpu_vmhub *hub = &adev->vmhub[i];
>>>> u32 tmp = gmc_v9_0_get_invalidate_req(vmid);
>>>>
>>>> + if (amdgpu_sriov_vf(adev) && amdgpu_sriov_runtime(adev)) {
>>>> + r = amdgpu_virt_kiq_invalidate_tlb(adev, hub, eng, tmp,
>>> vmid);
>>>> + if (!r)
>>>> + continue;
>>>> + }
>>>> +
>>> Drop that SRIOV check here. We don't want specialized code path for SRIOV.
>>>
>>> Christian.
>>>
>>>> WREG32_NO_KIQ(hub->vm_inv_eng0_req + eng, tmp);
>>>>
>>>> /* Busy wait for ACK.*/
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/amdgpu/sriov: For sriov runtime, use kiq to do invalidate tlb
[not found] ` <2e0a1f3e-4e8a-b9ae-bfe6-46b4b74144ce-5C7GfCeVMHo@public.gmane.org>
2018-08-15 7:56 ` Deng, Emily
@ 2018-08-15 10:21 ` Zhu, Rex
1 sibling, 0 replies; 6+ messages in thread
From: Zhu, Rex @ 2018-08-15 10:21 UTC (permalink / raw)
To: Koenig, Christian, Deng, Emily, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
[-- Attachment #1.1: Type: text/plain, Size: 8231 bytes --]
Remove the sriov check, The patch can work on both vega10 and Raven under bare metal with tip firmware.
Best Regards
Rex
________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org>
Sent: Wednesday, August 15, 2018 3:07 PM
To: Deng, Emily; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH v2] drm/amdgpu/sriov: For sriov runtime, use kiq to do invalidate tlb
> As need to let somebody to test all your bare metal test cases, such as vega10, and other asics, as I don't have those environments, and only could test sriov.
That is not much of a problem. We have plenty of people on the mailing
list (including me) who can test this on both Vega10 and Raven.
Just remove the SRIOV check and send the patch out with a request for
testing it on bare metal.
Christian.
Am 15.08.2018 um 04:28 schrieb Deng, Emily:
> Thanks for your review, will send a patch to review again as your suggestion. I think it will be better to use the amdgpu_sriov_vf(adev).
> As need to let somebody to test all your bare metal test cases, such as vega10, and other asics, as I don't have those environments, and only could test sriov.
> If passed those bare metal test cases and fixed the bugs if occurs, then could remove the amdgpu_sriov_vf(adev). Do you think so?
>
> Best wishes
> Emily Deng
>> -----Original Message-----
>> From: Christian König <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Sent: Tuesday, August 14, 2018 8:46 PM
>> To: Deng, Emily <Emily.Deng-5C7GfCeVMHo@public.gmane.org>; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>> Subject: Re: [PATCH v2] drm/amdgpu/sriov: For sriov runtime, use kiq to do
>> invalidate tlb
>>
>> Am 14.08.2018 um 12:56 schrieb Emily Deng:
>>> To avoid the tlb flush not interrupted by world switch, use kiq and
>>> one command to do tlb invalidate.
>>>
>>> v2:
>>> Add firmware version checking.
>>>
>>> SWDEV-161497
>>>
>>> Signed-off-by: Emily Deng <Emily.Deng-5C7GfCeVMHo@public.gmane.org>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 63
>> ++++++++++++++++++++++++++++++++
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 2 +
>>> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 ++++
>>> 3 files changed, 72 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>>> index 21adb1b6..436030c 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>>> @@ -233,6 +233,69 @@ void amdgpu_virt_kiq_wreg(struct amdgpu_device
>> *adev, uint32_t reg, uint32_t v)
>>> pr_err("failed to write reg:%x\n", reg);
>>> }
>>>
>>> +signed long amdgpu_virt_kiq_invalidate_tlb(struct amdgpu_device *adev,
>> struct amdgpu_vmhub *hub,
>>> + unsigned eng, u32 req, uint32_t vmid)
>> Drop that function here. It is Vega specific and doesn't belong into common
>> code.
>>
>>> +{
>>> + signed long r, cnt = 0;
>>> + unsigned long flags;
>>> + uint32_t seq;
>>> + struct amdgpu_kiq *kiq = &adev->gfx.kiq;
>>> + struct amdgpu_ring *ring = &kiq->ring;
>>> + struct drm_amdgpu_info_firmware fw_info;
>>> +
>>> + if (ring->me == 1) {
>>> + fw_info.ver = adev->gfx.mec_fw_version;
>>> + fw_info.feature = adev->gfx.mec_feature_version;
>>> + } else {
>>> + fw_info.ver = adev->gfx.mec2_fw_version;
>>> + fw_info.feature = adev->gfx.mec2_feature_version;
>>> + }
>>> +
>>> + if (fw_info.ver < 0x00000190 || fw_info.feature < 42)
>>> + return -EPERM;
>> Please move that check into gfx_v9_0_ring_emit_reg_write_reg_wait().
>>
>>> +
>>> + BUG_ON(!ring->funcs->emit_reg_write_reg_wait);
>> That check is superfluous.
>>
>>> +
>>> + spin_lock_irqsave(&kiq->ring_lock, flags);
>>> + amdgpu_ring_alloc(ring, 32);
>>> + amdgpu_ring_emit_reg_write_reg_wait(ring, hub->vm_inv_eng0_req +
>> eng,
>>> + hub->vm_inv_eng0_ack + eng,
>>> + req, 1 << vmid);
>>> + amdgpu_fence_emit_polling(ring, &seq);
>>> + amdgpu_ring_commit(ring);
>>> + spin_unlock_irqrestore(&kiq->ring_lock, flags);
>>> +
>>> + r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
>>> +
>>> + /* don't wait anymore for gpu reset case because this way may
>>> + * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
>>> + * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
>>> + * never return if we keep waiting in virt_kiq_rreg, which cause
>>> + * gpu_recover() hang there.
>>> + *
>>> + * also don't wait anymore for IRQ context
>>> + * */
>>> + if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
>>> + goto failed_kiq;
>>> +
>>> + if (in_interrupt())
>>> + might_sleep();
>>> +
>>> + while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
>>> + msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
>>> + r = amdgpu_fence_wait_polling(ring, seq,
>> MAX_KIQ_REG_WAIT);
>>> + }
>>> +
>>> + if (cnt > MAX_KIQ_REG_TRY)
>>> + goto failed_kiq;
>>> +
>>> + return 0;
>>> +
>>> +failed_kiq:
>>> + pr_err("failed to invalidate tlb with kiq\n");
>>> + return r;
>>> +}
>>> +
>>> /**
>>> * amdgpu_virt_request_full_gpu() - request full gpu access
>>> * @amdgpu: amdgpu device.
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>>> index 880ac11..8908ff9 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>>> @@ -288,6 +288,8 @@ void amdgpu_free_static_csa(struct amdgpu_device
>> *adev);
>>> void amdgpu_virt_init_setting(struct amdgpu_device *adev);
>>> uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t reg);
>>> void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg,
>>> uint32_t v);
>>> +signed long amdgpu_virt_kiq_invalidate_tlb(struct amdgpu_device *adev,
>> struct amdgpu_vmhub *hub,
>>> + unsigned eng, u32 req, uint32_t vmid);
>>> int amdgpu_virt_request_full_gpu(struct amdgpu_device *adev, bool init);
>>> int amdgpu_virt_release_full_gpu(struct amdgpu_device *adev, bool init);
>>> int amdgpu_virt_reset_gpu(struct amdgpu_device *adev); diff --git
>>> a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> index 6999042..812f71e 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> @@ -331,6 +331,7 @@ static void gmc_v9_0_flush_gpu_tlb(struct
>> amdgpu_device *adev,
>>> /* Use register 17 for GART */
>>> const unsigned eng = 17;
>>> unsigned i, j;
>>> + int r;
>>>
>>> spin_lock(&adev->gmc.invalidate_lock);
>>>
>>> @@ -338,6 +339,12 @@ static void gmc_v9_0_flush_gpu_tlb(struct
>> amdgpu_device *adev,
>>> struct amdgpu_vmhub *hub = &adev->vmhub[i];
>>> u32 tmp = gmc_v9_0_get_invalidate_req(vmid);
>>>
>>> + if (amdgpu_sriov_vf(adev) && amdgpu_sriov_runtime(adev)) {
>>> + r = amdgpu_virt_kiq_invalidate_tlb(adev, hub, eng, tmp,
>> vmid);
>>> + if (!r)
>>> + continue;
>>> + }
>>> +
>> Drop that SRIOV check here. We don't want specialized code path for SRIOV.
>>
>> Christian.
>>
>>> WREG32_NO_KIQ(hub->vm_inv_eng0_req + eng, tmp);
>>>
>>> /* Busy wait for ACK.*/
_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
amd-gfx Info Page - freedesktop.org<https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
lists.freedesktop.org
Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the following form. Use of all freedesktop.org lists is subject to our Code of Conduct.
[-- Attachment #1.2: Type: text/html, Size: 16456 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-08-15 10:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-14 10:56 [PATCH v2] drm/amdgpu/sriov: For sriov runtime, use kiq to do invalidate tlb Emily Deng
[not found] ` <1534244211-4717-1-git-send-email-Emily.Deng-5C7GfCeVMHo@public.gmane.org>
2018-08-14 12:45 ` Christian König
[not found] ` <a865d2c4-2373-1101-2dd1-ca3a8f0792eb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-08-15 2:28 ` Deng, Emily
[not found] ` <CY4PR12MB11250F5E45EBEBDD519056BB8F3F0-rpdhrqHFk07v2MZdTKcfDgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2018-08-15 7:07 ` Christian König
[not found] ` <2e0a1f3e-4e8a-b9ae-bfe6-46b4b74144ce-5C7GfCeVMHo@public.gmane.org>
2018-08-15 7:56 ` Deng, Emily
2018-08-15 10:21 ` Zhu, Rex
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.