<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"></style> </head> <body dir="ltr"> <div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" dir="ltr"> Remove the sriov check, The patch can work on both vega10 and Raven under bare metal with tip firmware. Best Regards Rex <div style="color: rgb(0, 0, 0);"> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Christian König <christian.koenig@amd.com> Sent: Wednesday, August 15, 2018 3:07 PM To: Deng, Emily; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH v2] drm/amdgpu/sriov: For sriov runtime, use kiq to do invalidate tlb <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">> 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 <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" id="LPlnk575449" class="OWAAutoLink" previewremoved="true">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> <div id="LPBorder_GT_15343271181480.00033885929112609237" style="margin-bottom: 20px; overflow: auto; width: 100%; text-indent: 0px;"> <table id="LPContainer_15343271181480.9816543027771137" style="width: 90%; background-color: rgb(255, 255, 255); position: relative; overflow: auto; padding-top: 20px; padding-bottom: 20px; margin-top: 20px; border-top: 1px dotted rgb(200, 200, 200); border-bottom: 1px dotted rgb(200, 200, 200);" role="presentation" cellspacing="0"> <tbody> <tr style="border-spacing: 0px;" valign="top"> <td id="TextCell_15343271181480.9952248566073559" style="vertical-align: top; position: relative; padding: 0px; display: table-cell;" colspan="2"> <div id="LPRemovePreviewContainer_15343271181480.875376248828901"></div> <div id="LPTitle_15343271181480.18428245518285646" style="top: 0px; color: rgb(0, 120, 215); font-weight: 400; font-size: 21px; font-family: "wf_segoe-ui_light", "Segoe UI Light", "Segoe WP Light", "Segoe UI", "Segoe WP", Tahoma, Arial, sans-serif; line-height: 21px;"> <a id="LPUrlAnchor_15343271181480.9572732071652488" style="text-decoration: none;" href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" target="_blank">amd-gfx Info Page - freedesktop.org</a></div> <div id="LPMetadata_15343271181480.5649528060753035" style="margin: 10px 0px 16px; color: rgb(102, 102, 102); font-weight: 400; font-family: "wf_segoe-ui_normal", "Segoe UI", "Segoe WP", Tahoma, Arial, sans-serif; font-size: 14px; line-height: 14px;"> lists.freedesktop.org</div> <div id="LPDescription_15343271181480.30659632369144363" style="display: block; color: rgb(102, 102, 102); font-weight: 400; font-family: "wf_segoe-ui_normal", "Segoe UI", "Segoe WP", Tahoma, Arial, sans-serif; font-size: 14px; line-height: 20px; max-height: 100px; overflow: hidden;"> 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.</div> </td> </tr> </tbody> </table> </div> </div> </div> </div> </div> </body> </html>