Re: [PATCH 01/18] drm/amdgpu/sriov:fix missing error handling
Am 18.09.2017 um 08:11 schrieb Monk Liu: Change-Id: Ifc6942ed0221f3134bfba4d66fde743484191da3 Signed-off-by: Monk Liu Reviewed-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index e390c01..d1ac27d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -841,8 +841,11 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv) if (amdgpu_sriov_vf(adev)) { r = amdgpu_map_static_csa(adev, &fpriv->vm, &fpriv->csa_va); - if (r) + if (r) { + amdgpu_vm_fini(adev, &fpriv->vm); + kfree(fpriv); goto out_suspend; + } } mutex_init(&fpriv->bo_list_lock); ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 02/18] drm/amdgpu:no kiq in IH
Am 18.09.2017 um 08:11 schrieb Monk Liu: Change-Id: I4deb65675d2531236b2f4e2bc6f015c657546464 Signed-off-by: Monk Liu Reviewed-by: Christian König --- drivers/gpu/drm/amd/amdgpu/vega10_ih.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c index 67610f7..c291e33 100644 --- a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c +++ b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c @@ -219,9 +219,9 @@ static u32 vega10_ih_get_wptr(struct amdgpu_device *adev) wptr, adev->irq.ih.rptr, tmp); adev->irq.ih.rptr = tmp; - tmp = RREG32(SOC15_REG_OFFSET(OSSSYS, 0, mmIH_RB_CNTL)); + tmp = RREG32_NO_KIQ(SOC15_REG_OFFSET(OSSSYS, 0, mmIH_RB_CNTL)); tmp = REG_SET_FIELD(tmp, IH_RB_CNTL, WPTR_OVERFLOW_CLEAR, 1); - WREG32(SOC15_REG_OFFSET(OSSSYS, 0, mmIH_RB_CNTL), tmp); + WREG32_NO_KIQ(SOC15_REG_OFFSET(OSSSYS, 0, mmIH_RB_CNTL), tmp); } return (wptr & adev->irq.ih.ptr_mask); } ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 03/18] drm/amdgpu/sriov:move in_reset to adev and rename
Am 18.09.2017 um 08:11 schrieb Monk Liu: currently in_reset is only used in sriov gpu reset, and it will be used for other non-gfx hw component later, like PSP, so move it from gfx to adev and rename to in_sriov_reset make more sense. Change-Id: Ibb8546f6e4635a1cca740e57f6244f158c70a1e6 Signed-off-by: Monk Liu Reviewed-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu.h| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 ++-- drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 6 +++--- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index a34c4cb..cc9a232 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1019,7 +1019,6 @@ struct amdgpu_gfx { /* reset mask */ uint32_tgrbm_soft_reset; uint32_tsrbm_soft_reset; - boolin_reset; /* s3/s4 mask */ boolin_suspend; /* NGG */ @@ -1588,6 +1587,7 @@ struct amdgpu_device { /* record last mm index being written through WREG32*/ unsigned long last_mm_index; + boolin_sriov_reset; }; static inline struct amdgpu_device *amdgpu_ttm_adev(struct ttm_bo_device *bdev) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 3467179..298a241 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -2757,7 +2757,7 @@ int amdgpu_sriov_gpu_reset(struct amdgpu_device *adev, struct amdgpu_job *job) mutex_lock(&adev->virt.lock_reset); atomic_inc(&adev->gpu_reset_counter); - adev->gfx.in_reset = true; + adev->in_sriov_reset = true; /* block TTM */ resched = ttm_bo_lock_delayed_workqueue(&adev->mman.bdev); @@ -2868,7 +2868,7 @@ int amdgpu_sriov_gpu_reset(struct amdgpu_device *adev, struct amdgpu_job *job) dev_info(adev->dev, "GPU reset successed!\n"); } - adev->gfx.in_reset = false; + adev->in_sriov_reset = false; mutex_unlock(&adev->virt.lock_reset); return r; } diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c index 6ee348e..3f511a9 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c @@ -4810,7 +4810,7 @@ static int gfx_v8_0_kiq_init_queue(struct amdgpu_ring *ring) gfx_v8_0_kiq_setting(ring); - if (adev->gfx.in_reset) { /* for GPU_RESET case */ + if (adev->in_sriov_reset) { /* for GPU_RESET case */ /* reset MQD to a clean status */ if (adev->gfx.mec.mqd_backup[mqd_idx]) memcpy(mqd, adev->gfx.mec.mqd_backup[mqd_idx], sizeof(struct vi_mqd_allocation)); @@ -4847,7 +4847,7 @@ static int gfx_v8_0_kcq_init_queue(struct amdgpu_ring *ring) struct vi_mqd *mqd = ring->mqd_ptr; int mqd_idx = ring - &adev->gfx.compute_ring[0]; - if (!adev->gfx.in_reset && !adev->gfx.in_suspend) { + if (!adev->in_sriov_reset && !adev->gfx.in_suspend) { memset((void *)mqd, 0, sizeof(struct vi_mqd_allocation)); ((struct vi_mqd_allocation *)mqd)->dynamic_cu_mask = 0x; ((struct vi_mqd_allocation *)mqd)->dynamic_rb_mask = 0x; @@ -4859,7 +4859,7 @@ static int gfx_v8_0_kcq_init_queue(struct amdgpu_ring *ring) if (adev->gfx.mec.mqd_backup[mqd_idx]) memcpy(adev->gfx.mec.mqd_backup[mqd_idx], mqd, sizeof(struct vi_mqd_allocation)); - } else if (adev->gfx.in_reset) { /* for GPU_RESET case */ + } else if (adev->in_sriov_reset) { /* for GPU_RESET case */ /* reset MQD to a clean status */ if (adev->gfx.mec.mqd_backup[mqd_idx]) memcpy(mqd, adev->gfx.mec.mqd_backup[mqd_idx], sizeof(struct vi_mqd_allocation)); diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index c133c85..21838f4 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -2698,7 +2698,7 @@ static int gfx_v9_0_kiq_init_queue(struct amdgpu_ring *ring) gfx_v9_0_kiq_setting(ring); - if (adev->gfx.in_reset) { /* for GPU_RESET case */ + if (adev->in_sriov_reset) { /* for GPU_RESET case */ /* reset MQD to a clean status */ if (adev->gfx.mec.mqd_backup[mqd_idx]) memcpy(mqd, adev->gfx.mec.mqd_backup[mqd_idx], sizeof(struct v9_mqd_allocation)); @@ -2736,7 +2736,7 @@ static int gfx_v9_0_kcq_init_queue(struct amdgpu_ring *ring) struct v9_mqd *mqd = ring->mqd_ptr; int mqd_idx = ring - &adev->gfx.compute_ring[0]; - if (!adev->gfx.in_reset &&
Re: [PATCH 04/18] drm/amdgpu/sriov:don't load psp fw during gpu reset
Am 18.09.2017 um 08:11 schrieb Monk Liu: At least for SRIOV we found reload PSP fw during gpu reset cause PSP hang. Change-Id: I5f273187a10bb8571b77651dfba7656ce0429af0 Signed-off-by: Monk Liu Acked-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c index 8a1ee97..4eee2ef 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c @@ -253,15 +253,18 @@ static int psp_asd_load(struct psp_context *psp) static int psp_hw_start(struct psp_context *psp) { + struct amdgpu_device *adev = psp->adev; int ret; - ret = psp_bootloader_load_sysdrv(psp); - if (ret) - return ret; + if (amdgpu_sriov_vf(adev) && !adev->in_sriov_reset) { + ret = psp_bootloader_load_sysdrv(psp); + if (ret) + return ret; - ret = psp_bootloader_load_sos(psp); - if (ret) - return ret; + ret = psp_bootloader_load_sos(psp); + if (ret) + return ret; + } ret = psp_ring_create(psp, PSP_RING_TYPE__KM); if (ret) ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 05/18] drm/amdgpu:make ctx_add_fence interruptible
Am 18.09.2017 um 08:11 schrieb Monk Liu: otherwise a gpu hang will make application couldn't be killed Change-Id: I6051b5b3ae1188983f49325a2438c84a6c12374a Signed-off-by: Monk Liu --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 4 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 12 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 14 +- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index cc9a232..6ff2959 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -736,8 +736,8 @@ struct amdgpu_ctx_mgr { struct amdgpu_ctx *amdgpu_ctx_get(struct amdgpu_fpriv *fpriv, uint32_t id); int amdgpu_ctx_put(struct amdgpu_ctx *ctx); -uint64_t amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring, - struct dma_fence *fence); +int amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring, + struct dma_fence *fence, uint64_t *seq); struct dma_fence *amdgpu_ctx_get_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring, uint64_t seq); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index b59749d..4ac7a92 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -1043,6 +1043,8 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, struct amd_sched_entity *entity = &p->ctx->rings[ring->idx].entity; struct amdgpu_job *job; unsigned i; + uint64_t seq; + int r; amdgpu_mn_lock(p->mn); @@ -1071,8 +1073,14 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, job->owner = p->filp; job->fence_ctx = entity->fence_context; p->fence = dma_fence_get(&job->base.s_fence->finished); - cs->out.handle = amdgpu_ctx_add_fence(p->ctx, ring, p->fence); - job->uf_sequence = cs->out.handle; + r = amdgpu_ctx_add_fence(p->ctx, ring, p->fence, &seq); + if (r) { + dma_fence_put(p->fence); + return r; This will memory leak the job and you need to call amdgpu_mn_unlock() before returning. + } + + cs->out.handle = seq; + job->uf_sequence = seq; amdgpu_job_free_resources(job); trace_amdgpu_cs_ioctl(job); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c index a11e443..97f8be4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c @@ -246,8 +246,8 @@ int amdgpu_ctx_put(struct amdgpu_ctx *ctx) return 0; } -uint64_t amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring, - struct dma_fence *fence) +int amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring, + struct dma_fence *fence, uint64_t* handler) { struct amdgpu_ctx_ring *cring = & ctx->rings[ring->idx]; uint64_t seq = cring->sequence; @@ -258,9 +258,11 @@ uint64_t amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring, other = cring->fences[idx]; if (other) { signed long r; - r = dma_fence_wait_timeout(other, false, MAX_SCHEDULE_TIMEOUT); - if (r < 0) + r = dma_fence_wait_timeout(other, true, MAX_SCHEDULE_TIMEOUT); + if (r < 0) { DRM_ERROR("Error (%ld) waiting for fence!\n", r); Drop the extra error message here. Receiving an signal is not something that should trigger an extra message in the logs + return -ERESTARTSYS; And return the original error code here. Apart from that looks good to me, Christian. + } } dma_fence_get(fence); @@ -271,8 +273,10 @@ uint64_t amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring, spin_unlock(&ctx->ring_lock); dma_fence_put(other); + if (handler) + *handler = seq; - return seq; + return 0; } struct dma_fence *amdgpu_ctx_get_fence(struct amdgpu_ctx *ctx, ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH 14/18] drm/amdgpu: Fix amdgpu reload failure under SRIOV
NAK, Tonga has no this problem, please keep the patch into internal branch for temporally. -Original Message- From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of Monk Liu Sent: Monday, September 18, 2017 2:12 PM To: amd-gfx@lists.freedesktop.org Cc: Chen, Horace ; Liu, Monk Subject: [PATCH 14/18] drm/amdgpu: Fix amdgpu reload failure under SRIOV From: Horace Chen Kernel will set the PCI power state to UNKNOWN after unloading, Since SRIOV has faked PCI config space so the UNKNOWN state will be kept forever. In driver reload if the power state is UNKNOWN then enabling msi will fail. forcely set it to D0 for SRIOV to fix this kernel flawness. Change-Id: I6a72d5fc9b653b21c3c98167515a511c5edeb91c Signed-off-by: Horace Chen Signed-off-by: Monk Liu --- drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c index 914c5bf..345406a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c @@ -229,7 +229,15 @@ int amdgpu_irq_init(struct amdgpu_device *adev) adev->irq.msi_enabled = false; if (amdgpu_msi_ok(adev)) { - int ret = pci_enable_msi(adev->pdev); + int ret; + if (amdgpu_sriov_vf(adev) && + adev->pdev->current_state == PCI_UNKNOWN){ + /* If pci power state is unknown on the SRIOV platform, +* it may be set in the remove device. We need to forcely +* set it to D0 to enable the msi*/ + adev->pdev->current_state = PCI_D0; + } + ret = pci_enable_msi(adev->pdev); if (!ret) { adev->irq.msi_enabled = true; dev_info(adev->dev, "amdgpu: using MSI.\n"); -- 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
Re: [PATCH 06/18] drm/amdgpu/sriov:fix memory leak after gpu reset
Am 18.09.2017 um 08:11 schrieb Monk Liu: doing gpu reset will rerun all hw_init and thus ucode_init_bo is invoked again, so we need to skip the fw_buf allocation during sriov gpu reset to avoid memory leak. Change-Id: I31131eda1bd45ea2f5bdc50c5da5fc5a9fe9027d Signed-off-by: Monk Liu --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 64 +++ 2 files changed, 35 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 6ff2959..3d0c633 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1185,6 +1185,9 @@ struct amdgpu_firmware { /* gpu info firmware data pointer */ const struct firmware *gpu_info_fw; + + void *fw_buf_ptr; + uint64_t fw_buf_mc; }; /* diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c index f306374..6564902 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c @@ -360,8 +360,6 @@ static int amdgpu_ucode_patch_jt(struct amdgpu_firmware_info *ucode, int amdgpu_ucode_init_bo(struct amdgpu_device *adev) { struct amdgpu_bo **bo = &adev->firmware.fw_buf; - uint64_t fw_mc_addr; - void *fw_buf_ptr = NULL; uint64_t fw_offset = 0; int i, err; struct amdgpu_firmware_info *ucode = NULL; @@ -372,37 +370,39 @@ int amdgpu_ucode_init_bo(struct amdgpu_device *adev) return 0; } - err = amdgpu_bo_create(adev, adev->firmware.fw_size, PAGE_SIZE, true, - amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT, - AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS, - NULL, NULL, 0, bo); - if (err) { - dev_err(adev->dev, "(%d) Firmware buffer allocate failed\n", err); - goto failed; - } + if (!amdgpu_sriov_vf(adev) || !adev->in_sriov_reset) { Instead of all this better use amdgpu_bo_create_kernel(), this should already include most of the handling necessary here. Christian. + err = amdgpu_bo_create(adev, adev->firmware.fw_size, PAGE_SIZE, true, + amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT, + AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS, + NULL, NULL, 0, bo); + if (err) { + dev_err(adev->dev, "(%d) Firmware buffer allocate failed\n", err); + goto failed; + } - err = amdgpu_bo_reserve(*bo, false); - if (err) { - dev_err(adev->dev, "(%d) Firmware buffer reserve failed\n", err); - goto failed_reserve; - } + err = amdgpu_bo_reserve(*bo, false); + if (err) { + dev_err(adev->dev, "(%d) Firmware buffer reserve failed\n", err); + goto failed_reserve; + } - err = amdgpu_bo_pin(*bo, amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT, - &fw_mc_addr); - if (err) { - dev_err(adev->dev, "(%d) Firmware buffer pin failed\n", err); - goto failed_pin; - } + err = amdgpu_bo_pin(*bo, amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT, + &adev->firmware.fw_buf_mc); + if (err) { + dev_err(adev->dev, "(%d) Firmware buffer pin failed\n", err); + goto failed_pin; + } - err = amdgpu_bo_kmap(*bo, &fw_buf_ptr); - if (err) { - dev_err(adev->dev, "(%d) Firmware buffer kmap failed\n", err); - goto failed_kmap; - } + err = amdgpu_bo_kmap(*bo, &adev->firmware.fw_buf_ptr); + if (err) { + dev_err(adev->dev, "(%d) Firmware buffer kmap failed\n", err); + goto failed_kmap; + } - amdgpu_bo_unreserve(*bo); + amdgpu_bo_unreserve(*bo); + } - memset(fw_buf_ptr, 0, adev->firmware.fw_size); + memset(adev->firmware.fw_buf_ptr, 0, adev->firmware.fw_size); /* * if SMU loaded firmware, it needn't add SMC, UVD, and VCE @@ -421,14 +421,14 @@ int amdgpu_ucode_init_bo(struct amdgpu_device *adev) ucode = &adev->firmware.ucode[i]; if (ucode->fw) { header = (const struct common_firmware_header *)ucode->fw->data; - amdgpu_ucode_init_single_fw(adev, ucode, fw_mc_addr + fw_offset, - (void *)((uint8_t *)fw_buf_ptr + fw_offset)); + a
Re: [PATCH 07/18] drm/amdgpu:add hdp golden setting register name hint
Am 18.09.2017 um 08:11 schrieb Monk Liu: Change-Id: I3a43901f5757b9fab629824a74ad9a4770a47b38 Signed-off-by: Monk Liu --- drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c index 7ca9cbe..7a20ba8 100644 --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c @@ -59,16 +59,16 @@ static const u32 golden_settings_vega10_hdp[] = { - 0xf64, 0x0fff, 0x, - 0xf65, 0x0fff, 0x, - 0xf66, 0x0fff, 0x, - 0xf67, 0x0fff, 0x, - 0xf68, 0x0fff, 0x, - 0xf6a, 0x0fff, 0x, - 0xf6b, 0x0fff, 0x, - 0xf6c, 0x0fff, 0x, - 0xf6d, 0x0fff, 0x, - 0xf6e, 0x0fff, 0x, + 0xf64, 0x0fff, 0x,//surface0_low_bound + 0xf65, 0x0fff, 0x,//surface0_upper_bound + 0xf66, 0x0fff, 0x,//surface0_base + 0xf67, 0x0fff, 0x,//surface0_info + 0xf68, 0x0fff, 0x,//surface0_base_hi + 0xf6a, 0x0fff, 0x,//surface1_low_bound + 0xf6b, 0x0fff, 0x,//surface1_upper_bound + 0xf6c, 0x0fff, 0x,//surface1_base + 0xf6d, 0x0fff, 0x,//surface1_info + 0xf6e, 0x0fff, 0x,//surface1_base_hi Don't use "//" in kernel code. Christian. }; static int gmc_v9_0_vm_fault_interrupt_state(struct amdgpu_device *adev, ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 08/18] drm/amdgpu:halt when vm fault
Am 18.09.2017 um 08:11 schrieb Monk Liu: only with this way we can debug the VMC page fault issue Change-Id: Ifc8373c3c3c40d54ae94dedf1be74d6314faeb10 Signed-off-by: Monk Liu Please make this behavior depend on the vm_fault_stop module parameter just like it is handled on older generations. Apart from that it looks like a really good idea to me, Christian. --- drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c | 6 ++ drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c | 7 +++ 2 files changed, 13 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c index 6c8040e..c17996e 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c @@ -319,6 +319,12 @@ void gfxhub_v1_0_set_fault_enable_default(struct amdgpu_device *adev, WRITE_PROTECTION_FAULT_ENABLE_DEFAULT, value); tmp = REG_SET_FIELD(tmp, VM_L2_PROTECTION_FAULT_CNTL, EXECUTE_PROTECTION_FAULT_ENABLE_DEFAULT, value); + if (!value) { + tmp = REG_SET_FIELD(tmp, VM_L2_PROTECTION_FAULT_CNTL, + CRASH_ON_NO_RETRY_FAULT, 1); + tmp = REG_SET_FIELD(tmp, VM_L2_PROTECTION_FAULT_CNTL, + CRASH_ON_RETRY_FAULT, 1); +} WREG32_SOC15(GC, 0, mmVM_L2_PROTECTION_FAULT_CNTL, tmp); } diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c index 7ff7076..cc21c4b 100644 --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c @@ -561,6 +561,13 @@ void mmhub_v1_0_set_fault_enable_default(struct amdgpu_device *adev, bool value) WRITE_PROTECTION_FAULT_ENABLE_DEFAULT, value); tmp = REG_SET_FIELD(tmp, VM_L2_PROTECTION_FAULT_CNTL, EXECUTE_PROTECTION_FAULT_ENABLE_DEFAULT, value); + if (!value) { + tmp = REG_SET_FIELD(tmp, VM_L2_PROTECTION_FAULT_CNTL, + CRASH_ON_NO_RETRY_FAULT, 1); + tmp = REG_SET_FIELD(tmp, VM_L2_PROTECTION_FAULT_CNTL, + CRASH_ON_RETRY_FAULT, 1); +} + WREG32_SOC15(MMHUB, 0, mmVM_L2_PROTECTION_FAULT_CNTL, tmp); } ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 09/18] drm/amdgpu:insert TMZ_BEGIN
Am 18.09.2017 um 08:11 schrieb Monk Liu: FRAME_CONTROL(begin) is needed for vega10 due to ucode logic change, it can fix some CTS random fail under gfx preemption enabled mode. Change-Id: I0442337f6cde13ed2a33f033badcb522e0f35e2d Signed-off-by: Monk Liu Acked-by: Christian König --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index 21838f4..3306667 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -3764,6 +3764,12 @@ static void gfx_v9_0_ring_emit_de_meta(struct amdgpu_ring *ring) amdgpu_ring_write_multiple(ring, (void *)&de_payload, sizeof(de_payload) >> 2); } +static void gfx_v9_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start) +{ + amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0)); + amdgpu_ring_write(ring, FRAME_CMD(start ? 0 : 1)); /* frame_end */ +} + static void gfx_v9_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags) { uint32_t dw2 = 0; @@ -3771,6 +3777,8 @@ static void gfx_v9_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags) if (amdgpu_sriov_vf(ring->adev)) gfx_v9_0_ring_emit_ce_meta(ring); + gfx_v9_0_ring_emit_tmz(ring, true); + dw2 |= 0x8000; /* set load_enable otherwise this package is just NOPs */ if (flags & AMDGPU_HAVE_CTX_SWITCH) { /* set load_global_config & load_global_uconfig */ @@ -3821,12 +3829,6 @@ static void gfx_v9_0_ring_emit_patch_cond_exec(struct amdgpu_ring *ring, unsigne ring->ring[offset] = (ring->ring_size>>2) - offset + cur; } -static void gfx_v9_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start) -{ - amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0)); - amdgpu_ring_write(ring, FRAME_CMD(start ? 0 : 1)); /* frame_end */ -} - static void gfx_v9_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg) { struct amdgpu_device *adev = ring->adev; ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 10/18] drm/amdgpu:hdp flush should be put it initialized
Am 18.09.2017 um 08:11 schrieb Monk Liu: Change-Id: I635271ba4c89189017daa302a7fe5cd65c3eef06 Signed-off-by: Monk Liu Acked-by: Christian König --- drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c index 7a20ba8..3d035a6 100644 --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c @@ -696,12 +696,6 @@ static int gmc_v9_0_gart_enable(struct amdgpu_device *adev) if (r) return r; - /* After HDP is initialized, flush HDP.*/ - if (adev->flags & AMD_IS_APU) - nbio_v7_0_hdp_flush(adev); - else - nbio_v6_1_hdp_flush(adev); - switch (adev->asic_type) { case CHIP_RAVEN: mmhub_v1_0_initialize_power_gating(adev); @@ -724,6 +718,12 @@ static int gmc_v9_0_gart_enable(struct amdgpu_device *adev) tmp = RREG32_SOC15(HDP, 0, mmHDP_HOST_PATH_CNTL); WREG32_SOC15(HDP, 0, mmHDP_HOST_PATH_CNTL, tmp); + /* After HDP is initialized, flush HDP.*/ + if (adev->flags & AMD_IS_APU) + nbio_v7_0_hdp_flush(adev); + else + nbio_v6_1_hdp_flush(adev); + if (amdgpu_vm_fault_stop == AMDGPU_VM_FAULT_STOP_ALWAYS) value = false; else ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 11/18] drm/amdgpu:add vgt_flush for gfx9
Am 18.09.2017 um 08:11 schrieb Monk Liu: Change-Id: I584572cfb9145ee1b8d11d69ba2989bd6acfd706 Signed-off-by: Monk Liu I could be wrong, but wasn't the consensus that this should be done by the UMD? Marek, please comment. Christian. --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index 3306667..f201510 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -3499,6 +3499,17 @@ static void gfx_v9_0_ring_set_wptr_gfx(struct amdgpu_ring *ring) } } +static void gfx_v9_0_ring_emit_vgt_flush(struct amdgpu_ring *ring) +{ + amdgpu_ring_write(ring, PACKET3(PACKET3_EVENT_WRITE, 0)); + amdgpu_ring_write(ring, EVENT_TYPE(VS_PARTIAL_FLUSH) | + EVENT_INDEX(4)); + + amdgpu_ring_write(ring, PACKET3(PACKET3_EVENT_WRITE, 0)); + amdgpu_ring_write(ring, EVENT_TYPE(VGT_FLUSH) | + EVENT_INDEX(0)); +} + static void gfx_v9_0_ring_emit_hdp_flush(struct amdgpu_ring *ring) { u32 ref_and_mask, reg_mem_engine; @@ -3530,6 +3541,9 @@ static void gfx_v9_0_ring_emit_hdp_flush(struct amdgpu_ring *ring) nbio_hf_reg->hdp_flush_req_offset, nbio_hf_reg->hdp_flush_done_offset, ref_and_mask, ref_and_mask, 0x20); + + if (ring->funcs->type == AMDGPU_RING_TYPE_GFX) + gfx_v9_0_ring_emit_vgt_flush(ring); } static void gfx_v9_0_ring_emit_hdp_invalidate(struct amdgpu_ring *ring) ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 12/18] drm/amdgpu:use formal register to trigger hdp invalidate
Am 18.09.2017 um 08:11 schrieb Monk Liu: Change-Id: I61dc02ea6a450f9acfa3bae07aa20244261f5369 Signed-off-by: Monk Liu Reviewed-by: Christian König Please scan the code once more, we most likely have used mmHDP_DEBUG0 for this at even more places. Christian. --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 +- drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index f201510..44960b3 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -3549,7 +3549,7 @@ static void gfx_v9_0_ring_emit_hdp_flush(struct amdgpu_ring *ring) static void gfx_v9_0_ring_emit_hdp_invalidate(struct amdgpu_ring *ring) { gfx_v9_0_write_data_to_reg(ring, 0, true, - SOC15_REG_OFFSET(HDP, 0, mmHDP_DEBUG0), 1); + SOC15_REG_OFFSET(HDP, 0, mmHDP_READ_CACHE_INVALIDATE), 1); } static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring, diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c index fd7c72a..d5f3848 100644 --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c @@ -398,7 +398,7 @@ static void sdma_v4_0_ring_emit_hdp_invalidate(struct amdgpu_ring *ring) { amdgpu_ring_write(ring, SDMA_PKT_HEADER_OP(SDMA_OP_SRBM_WRITE) | SDMA_PKT_SRBM_WRITE_HEADER_BYTE_EN(0xf)); - amdgpu_ring_write(ring, SOC15_REG_OFFSET(HDP, 0, mmHDP_DEBUG0)); + amdgpu_ring_write(ring, SOC15_REG_OFFSET(HDP, 0, mmHDP_READ_CACHE_INVALIDATE)); amdgpu_ring_write(ring, 1); } ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 16/18] drm/amdgpu: increate mailbox polling timeout to 12s.
Am 18.09.2017 um 08:12 schrieb Monk Liu: From: Horace Chen Because there may have multiple FLR waiting for done, the waiting time of events may be long, add the time to 12s to reduce timeout failure. Change-Id: I6b33170ba7dedf781b99ba6095127efce403af81 Signed-off-by: Horace Chen Acked-by: Christian König --- drivers/gpu/drm/amd/amdgpu/mxgpu_ai.h | 2 +- drivers/gpu/drm/amd/amdgpu/mxgpu_vi.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.h b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.h index 1e91b9a..67e7857 100644 --- a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.h +++ b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.h @@ -24,7 +24,7 @@ #ifndef __MXGPU_AI_H__ #define __MXGPU_AI_H__ -#define AI_MAILBOX_TIMEDOUT 5000 +#define AI_MAILBOX_TIMEDOUT12000 enum idh_request { IDH_REQ_GPU_INIT_ACCESS = 1, diff --git a/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.h b/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.h index c791d73..f13dc6c 100644 --- a/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.h +++ b/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.h @@ -23,7 +23,7 @@ #ifndef __MXGPU_VI_H__ #define __MXGPU_VI_H__ -#define VI_MAILBOX_TIMEDOUT 5000 +#define VI_MAILBOX_TIMEDOUT12000 #define VI_MAILBOX_RESET_TIME 12 /* VI mailbox messages request */ ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 15/18] drm/amdgpu/sriov: fix page fault issue of driver unload
Am 18.09.2017 um 08:11 schrieb Monk Liu: bo_free on csa is too late to put in amdgpu_fini because that time ttm is already finished, Move it earlier to avoid the page fault. Change-Id: Id9c3f6aa8720cabbc9936ce21d8cf98af6e23bee Signed-off-by: Monk Liu Signed-off-by: Horace Chen Reviewed-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 +--- drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 1 + 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 298a241..e0a17bd 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -1795,10 +1795,8 @@ static int amdgpu_fini(struct amdgpu_device *adev) adev->ip_blocks[i].status.late_initialized = false; } - if (amdgpu_sriov_vf(adev)) { - amdgpu_bo_free_kernel(&adev->virt.csa_obj, &adev->virt.csa_vmid0_addr, NULL); + if (amdgpu_sriov_vf(adev)) amdgpu_virt_release_full_gpu(adev, false); - } return 0; } diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c index 3f511a9..40e5865 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c @@ -2113,6 +2113,7 @@ static int gfx_v8_0_sw_fini(void *handle) amdgpu_gfx_compute_mqd_sw_fini(adev); amdgpu_gfx_kiq_free_ring(&adev->gfx.kiq.ring, &adev->gfx.kiq.irq); amdgpu_gfx_kiq_fini(adev); + amdgpu_bo_free_kernel(&adev->virt.csa_obj, &adev->virt.csa_vmid0_addr, NULL); gfx_v8_0_mec_fini(adev); gfx_v8_0_rlc_fini(adev); ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 17/18] drm/amdgpu:fix uvd ring fini routine
Am 18.09.2017 um 08:12 schrieb Monk Liu: fix missing finish uvd enc_ring and wrongly finish uvd ring Change-Id: Ib74237ca5adcb3b128c9b751fced0b7db7b09e86 Signed-off-by: Monk Liu --- drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c index 331e34a..63b00eb 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c @@ -269,6 +269,8 @@ int amdgpu_uvd_sw_init(struct amdgpu_device *adev) int amdgpu_uvd_sw_fini(struct amdgpu_device *adev) { + struct amdgpu_ring *ring; + int i; A newline should be between declaration and code. kfree(adev->uvd.saved_bo); amd_sched_entity_fini(&adev->uvd.ring.sched, &adev->uvd.entity); @@ -277,7 +279,15 @@ int amdgpu_uvd_sw_fini(struct amdgpu_device *adev) &adev->uvd.gpu_addr, (void **)&adev->uvd.cpu_addr); - amdgpu_ring_fini(&adev->uvd.ring); + ring = &adev->uvd.ring; + if (ring->adev) No need for that, the first thing that amdgpu_ring_fini() does is checking ring->adev, so that is just duplicated. Reviewed-by: Christian König with those two minor issues fixed. Regards, Christian. + amdgpu_ring_fini(ring); + + for (i = 0; i < AMDGPU_MAX_UVD_ENC_RINGS; ++i) { + ring = &adev->uvd.ring_enc[i]; + if (ring->adev) + amdgpu_ring_fini(ring); + } release_firmware(adev->uvd.fw); ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 13/18] drm/amdgpu:fix driver unloading bug
Am 18.09.2017 um 08:11 schrieb Monk Liu: [SWDEV-126631] - fix hypervisor save_vf fail that occured after driver removed: 1. Because the KIQ and KCQ were not ummapped, save_vf will fail if driver freed mqd of KIQ and KCQ. 2. KIQ can't be unmapped since RLCV always need it, the bo_free on KIQ should be skipped 3. KCQ can be unmapped, and should be unmapped during hw_fini, 4. RLCV still need to access other mc address from some hw even after driver unloaded, So we should not unbind gart for VF. Change-Id: I320487a9a848f41484c5f8cc11be34aca807b424 Signed-off-by: Horace Chen Signed-off-by: Monk Liu I absolutely can't judge if this is correct or not, but keeping the GART and KIQ alive after the driver is unloaded sounds really fishy to me. Isn't there any other clean way of handling this? Christian. --- drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 3 +- drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 5 +++ drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c| 60 +++- 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c index f437008..2fee071 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c @@ -394,7 +394,8 @@ int amdgpu_gart_init(struct amdgpu_device *adev) */ void amdgpu_gart_fini(struct amdgpu_device *adev) { - if (adev->gart.ready) { + /* gart is still used by other hw under SRIOV, don't unbind it */ + if (adev->gart.ready && !amdgpu_sriov_vf(adev)) { /* unbind pages */ amdgpu_gart_unbind(adev, 0, adev->gart.num_cpu_pages); } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c index 4f6c68f..bf6656f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c @@ -309,6 +309,11 @@ void amdgpu_gfx_compute_mqd_sw_fini(struct amdgpu_device *adev) &ring->mqd_ptr); } + /* don't deallocate KIQ mqd because the bo is still used by RLCV even + the guest VM is shutdown */ + if (amdgpu_sriov_vf(adev)) + return; + ring = &adev->gfx.kiq.ring; kfree(adev->gfx.mec.mqd_backup[AMDGPU_MAX_COMPUTE_RINGS]); amdgpu_bo_free_kernel(&ring->mqd_obj, diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index 44960b3..a577bbc 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -2892,14 +2892,72 @@ static int gfx_v9_0_hw_init(void *handle) return r; } +static int gfx_v9_0_kcq_disable(struct amdgpu_ring *kiq_ring,struct amdgpu_ring *ring) +{ + struct amdgpu_device *adev = kiq_ring->adev; + uint32_t scratch, tmp = 0; + int r, i; + + r = amdgpu_gfx_scratch_get(adev, &scratch); + if (r) { + DRM_ERROR("Failed to get scratch reg (%d).\n", r); + return r; + } + WREG32(scratch, 0xCAFEDEAD); + + r = amdgpu_ring_alloc(kiq_ring, 10); + if (r) { + DRM_ERROR("Failed to lock KIQ (%d).\n", r); + amdgpu_gfx_scratch_free(adev, scratch); + return r; + } + + /* unmap queues */ + amdgpu_ring_write(kiq_ring, PACKET3(PACKET3_UNMAP_QUEUES, 4)); + amdgpu_ring_write(kiq_ring, /* Q_sel: 0, vmid: 0, engine: 0, num_Q: 1 */ + PACKET3_UNMAP_QUEUES_ACTION(1) | /* RESET_QUEUES */ + PACKET3_UNMAP_QUEUES_QUEUE_SEL(0) | + PACKET3_UNMAP_QUEUES_ENGINE_SEL(0) | + PACKET3_UNMAP_QUEUES_NUM_QUEUES(1)); + amdgpu_ring_write(kiq_ring, PACKET3_UNMAP_QUEUES_DOORBELL_OFFSET0(ring->doorbell_index)); + amdgpu_ring_write(kiq_ring, 0); + amdgpu_ring_write(kiq_ring, 0); + amdgpu_ring_write(kiq_ring, 0); + /* write to scratch for completion */ + amdgpu_ring_write(kiq_ring, PACKET3(PACKET3_SET_UCONFIG_REG, 1)); + amdgpu_ring_write(kiq_ring, (scratch - PACKET3_SET_UCONFIG_REG_START)); + amdgpu_ring_write(kiq_ring, 0xDEADBEEF); + amdgpu_ring_commit(kiq_ring); + + for (i = 0; i < adev->usec_timeout; i++) { + tmp = RREG32(scratch); + if (tmp == 0xDEADBEEF) + break; + DRM_UDELAY(1); + } + if (i >= adev->usec_timeout) { + DRM_ERROR("KCQ disabled failed (scratch(0x%04X)=0x%08X)\n", scratch, tmp); + r = -EINVAL; + } + amdgpu_gfx_scratch_free(adev, scratch); + return r; +} + + static int gfx_v9_0_hw_fini(void *handle) { struct amdgpu_device *adev = (struct amdgpu_device *)handle; + int i, r; amdgpu_irq_put(adev, &adev->gfx.priv_reg_irq, 0);
Re: [PATCH 14/18] drm/amdgpu: Fix amdgpu reload failure under SRIOV
Am 18.09.2017 um 08:11 schrieb Monk Liu: From: Horace Chen Kernel will set the PCI power state to UNKNOWN after unloading, Since SRIOV has faked PCI config space so the UNKNOWN state will be kept forever. In driver reload if the power state is UNKNOWN then enabling msi will fail. forcely set it to D0 for SRIOV to fix this kernel flawness. Change-Id: I6a72d5fc9b653b21c3c98167515a511c5edeb91c Signed-off-by: Horace Chen Signed-off-by: Monk Liu Acked-by: Christian König , but better wait for Alex to have a look as well on this before pushing it. Christian. --- drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c index 914c5bf..345406a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c @@ -229,7 +229,15 @@ int amdgpu_irq_init(struct amdgpu_device *adev) adev->irq.msi_enabled = false; if (amdgpu_msi_ok(adev)) { - int ret = pci_enable_msi(adev->pdev); + int ret; + if (amdgpu_sriov_vf(adev) && + adev->pdev->current_state == PCI_UNKNOWN){ + /* If pci power state is unknown on the SRIOV platform, +* it may be set in the remove device. We need to forcely +* set it to D0 to enable the msi*/ + adev->pdev->current_state = PCI_D0; + } + ret = pci_enable_msi(adev->pdev); if (!ret) { adev->irq.msi_enabled = true; dev_info(adev->dev, "amdgpu: using MSI.\n"); ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH 13/18] drm/amdgpu:fix driver unloading bug
Christian, Let's discuss this patch and the one follows which skip the KIQ MQD free to avoid SAVE_FAIL issue. For skipping KIQ MQD deallocation patch, I think I will drop it and use a new way: We allocate KIQ MQD in VRAM domain and this BO can be safely freed after driver unloaded, because after driver unloaded no one will change the data in this BO *usually*. e.g. some root app can map visible vram and alter the value in it for this patch "to skipping unbind the GART mapping to keep KIQ MQD always valid": Since hypervisor side always have couple hw component working, and they rely on GMC kept alive, so this is very different with BARE-METAL. That's to say we can only do like this way. Besides, we'll have more patches in future for L1 secure mode, which forbidden VF access GMC registers, so under L1 secure mode driver will always skip GMC programing under SRIOV both in init and fini, but that will come later BR Monk -Original Message- From: Christian König [mailto:ckoenig.leichtzumer...@gmail.com] Sent: 2017年9月18日 17:28 To: Liu, Monk ; amd-gfx@lists.freedesktop.org Cc: Chen, Horace Subject: Re: [PATCH 13/18] drm/amdgpu:fix driver unloading bug Am 18.09.2017 um 08:11 schrieb Monk Liu: > [SWDEV-126631] - fix hypervisor save_vf fail that occured after driver > removed: > 1. Because the KIQ and KCQ were not ummapped, save_vf will fail if driver > freed mqd of KIQ and KCQ. > 2. KIQ can't be unmapped since RLCV always need it, the bo_free on KIQ > should be skipped 3. KCQ can be unmapped, and should be unmapped > during hw_fini, 4. RLCV still need to access other mc address from some hw > even after driver unloaded, > So we should not unbind gart for VF. > > Change-Id: I320487a9a848f41484c5f8cc11be34aca807b424 > Signed-off-by: Horace Chen > Signed-off-by: Monk Liu I absolutely can't judge if this is correct or not, but keeping the GART and KIQ alive after the driver is unloaded sounds really fishy to me. Isn't there any other clean way of handling this? Christian. > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 3 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 5 +++ > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c| 60 > +++- > 3 files changed, 66 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c > index f437008..2fee071 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c > @@ -394,7 +394,8 @@ int amdgpu_gart_init(struct amdgpu_device *adev) >*/ > void amdgpu_gart_fini(struct amdgpu_device *adev) > { > - if (adev->gart.ready) { > + /* gart is still used by other hw under SRIOV, don't unbind it */ > + if (adev->gart.ready && !amdgpu_sriov_vf(adev)) { > /* unbind pages */ > amdgpu_gart_unbind(adev, 0, adev->gart.num_cpu_pages); > } > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c > index 4f6c68f..bf6656f 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c > @@ -309,6 +309,11 @@ void amdgpu_gfx_compute_mqd_sw_fini(struct amdgpu_device > *adev) > &ring->mqd_ptr); > } > > + /* don't deallocate KIQ mqd because the bo is still used by RLCV even > + the guest VM is shutdown */ > + if (amdgpu_sriov_vf(adev)) > + return; > + > ring = &adev->gfx.kiq.ring; > kfree(adev->gfx.mec.mqd_backup[AMDGPU_MAX_COMPUTE_RINGS]); > amdgpu_bo_free_kernel(&ring->mqd_obj, > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > index 44960b3..a577bbc 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > @@ -2892,14 +2892,72 @@ static int gfx_v9_0_hw_init(void *handle) > return r; > } > > +static int gfx_v9_0_kcq_disable(struct amdgpu_ring *kiq_ring,struct > +amdgpu_ring *ring) { > + struct amdgpu_device *adev = kiq_ring->adev; > + uint32_t scratch, tmp = 0; > + int r, i; > + > + r = amdgpu_gfx_scratch_get(adev, &scratch); > + if (r) { > + DRM_ERROR("Failed to get scratch reg (%d).\n", r); > + return r; > + } > + WREG32(scratch, 0xCAFEDEAD); > + > + r = amdgpu_ring_alloc(kiq_ring, 10); > + if (r) { > + DRM_ERROR("Failed to lock KIQ (%d).\n", r); > + amdgpu_gfx_scratch_free(adev, scratch); > + return r; > + } > + > + /* unmap queues */ > + amdgpu_ring_write(kiq_ring, PACKET3(PACKET3_UNMAP_QUEUES, 4)); > + amdgpu_ring_write(kiq_ring, /* Q_sel: 0, vmid: 0, engine: 0, num_Q: 1 */ > + PACKET3_UNMAP_QUEUES_ACTION(1) > | /* RESET_QUEUES */ > + > PACKET3_UNMAP_QUEUES_QUEUE_SEL(0) | > +
RE: [PATCH 06/18] drm/amdgpu/sriov:fix memory leak after gpu reset
I didn't get your point... how could bo_create_kernel solve my issue ? The thing here is during gpu reset we invoke hw_init for every hw component, and by design hw_init shouldn't doing anything software related, thus the BO allocating in hw_init is wrong, Even switch to bo_create_kernel won't address the issue ... BR Monk -Original Message- From: Christian König [mailto:ckoenig.leichtzumer...@gmail.com] Sent: 2017年9月18日 17:13 To: Liu, Monk ; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH 06/18] drm/amdgpu/sriov:fix memory leak after gpu reset Am 18.09.2017 um 08:11 schrieb Monk Liu: > doing gpu reset will rerun all hw_init and thus ucode_init_bo is > invoked again, so we need to skip the fw_buf allocation during sriov > gpu reset to avoid memory leak. > > Change-Id: I31131eda1bd45ea2f5bdc50c5da5fc5a9fe9027d > Signed-off-by: Monk Liu > --- > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 ++ > drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 64 > +++ > 2 files changed, 35 insertions(+), 32 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > index 6ff2959..3d0c633 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > @@ -1185,6 +1185,9 @@ struct amdgpu_firmware { > > /* gpu info firmware data pointer */ > const struct firmware *gpu_info_fw; > + > + void *fw_buf_ptr; > + uint64_t fw_buf_mc; > }; > > /* > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c > index f306374..6564902 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c > @@ -360,8 +360,6 @@ static int amdgpu_ucode_patch_jt(struct > amdgpu_firmware_info *ucode, > int amdgpu_ucode_init_bo(struct amdgpu_device *adev) > { > struct amdgpu_bo **bo = &adev->firmware.fw_buf; > - uint64_t fw_mc_addr; > - void *fw_buf_ptr = NULL; > uint64_t fw_offset = 0; > int i, err; > struct amdgpu_firmware_info *ucode = NULL; @@ -372,37 +370,39 @@ > int amdgpu_ucode_init_bo(struct amdgpu_device *adev) > return 0; > } > > - err = amdgpu_bo_create(adev, adev->firmware.fw_size, PAGE_SIZE, true, > - amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM > : AMDGPU_GEM_DOMAIN_GTT, > - AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS, > - NULL, NULL, 0, bo); > - if (err) { > - dev_err(adev->dev, "(%d) Firmware buffer allocate failed\n", > err); > - goto failed; > - } > + if (!amdgpu_sriov_vf(adev) || !adev->in_sriov_reset) { Instead of all this better use amdgpu_bo_create_kernel(), this should already include most of the handling necessary here. Christian. > + err = amdgpu_bo_create(adev, adev->firmware.fw_size, PAGE_SIZE, > true, > + amdgpu_sriov_vf(adev) ? > AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT, > + AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS, > + NULL, NULL, 0, bo); > + if (err) { > + dev_err(adev->dev, "(%d) Firmware buffer allocate > failed\n", err); > + goto failed; > + } > > - err = amdgpu_bo_reserve(*bo, false); > - if (err) { > - dev_err(adev->dev, "(%d) Firmware buffer reserve failed\n", > err); > - goto failed_reserve; > - } > + err = amdgpu_bo_reserve(*bo, false); > + if (err) { > + dev_err(adev->dev, "(%d) Firmware buffer reserve > failed\n", err); > + goto failed_reserve; > + } > > - err = amdgpu_bo_pin(*bo, amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM > : AMDGPU_GEM_DOMAIN_GTT, > - &fw_mc_addr); > - if (err) { > - dev_err(adev->dev, "(%d) Firmware buffer pin failed\n", err); > - goto failed_pin; > - } > + err = amdgpu_bo_pin(*bo, amdgpu_sriov_vf(adev) ? > AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT, > + &adev->firmware.fw_buf_mc); > + if (err) { > + dev_err(adev->dev, "(%d) Firmware buffer pin failed\n", > err); > + goto failed_pin; > + } > > - err = amdgpu_bo_kmap(*bo, &fw_buf_ptr); > - if (err) { > - dev_err(adev->dev, "(%d) Firmware buffer kmap failed\n", err); > - goto failed_kmap; > - } > + err = amdgpu_bo_kmap(*bo, &adev->firmware.fw_buf_ptr); > + if (err) { > + dev_err(adev->dev, "(%d) Firmware buffer kmap > failed\n", err); > + goto failed_kmap; > + } > > - amdgpu_bo_unreserve(*bo); > + amdgp
RE: [PATCH 12/18] drm/amdgpu:use formal register to trigger hdp invalidate
Only vega10 has this register -Original Message- From: Christian König [mailto:ckoenig.leichtzumer...@gmail.com] Sent: 2017年9月18日 17:20 To: Liu, Monk ; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH 12/18] drm/amdgpu:use formal register to trigger hdp invalidate Am 18.09.2017 um 08:11 schrieb Monk Liu: > Change-Id: I61dc02ea6a450f9acfa3bae07aa20244261f5369 > Signed-off-by: Monk Liu Reviewed-by: Christian König Please scan the code once more, we most likely have used mmHDP_DEBUG0 for this at even more places. Christian. > --- > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 +- > drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > index f201510..44960b3 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > @@ -3549,7 +3549,7 @@ static void gfx_v9_0_ring_emit_hdp_flush(struct > amdgpu_ring *ring) > static void gfx_v9_0_ring_emit_hdp_invalidate(struct amdgpu_ring *ring) > { > gfx_v9_0_write_data_to_reg(ring, 0, true, > -SOC15_REG_OFFSET(HDP, 0, mmHDP_DEBUG0), 1); > +SOC15_REG_OFFSET(HDP, 0, > mmHDP_READ_CACHE_INVALIDATE), 1); > } > > static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring, diff > --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c > b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c > index fd7c72a..d5f3848 100644 > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c > @@ -398,7 +398,7 @@ static void sdma_v4_0_ring_emit_hdp_invalidate(struct > amdgpu_ring *ring) > { > amdgpu_ring_write(ring, SDMA_PKT_HEADER_OP(SDMA_OP_SRBM_WRITE) | > SDMA_PKT_SRBM_WRITE_HEADER_BYTE_EN(0xf)); > - amdgpu_ring_write(ring, SOC15_REG_OFFSET(HDP, 0, mmHDP_DEBUG0)); > + amdgpu_ring_write(ring, SOC15_REG_OFFSET(HDP, 0, > +mmHDP_READ_CACHE_INVALIDATE)); > amdgpu_ring_write(ring, 1); > } > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 06/18] drm/amdgpu/sriov:fix memory leak after gpu reset
Am 18.09.2017 um 12:47 schrieb Liu, Monk: I didn't get your point... how could bo_create_kernel solve my issue ? It doesn't solve the underlying issue, you just need less code for your workaround. With bo_create_kernel you can do create/pin/kmap in just one function call. The thing here is during gpu reset we invoke hw_init for every hw component, and by design hw_init shouldn't doing anything software related, thus the BO allocating in hw_init is wrong, Yeah, but your patch doesn't fix that either as far as I can see. Even switch to bo_create_kernel won't address the issue ... See the implementation of bo_create_kernel(): if (!*bo_ptr) { r = amdgpu_bo_create(adev, size, align, true, domain, } r = amdgpu_bo_pin(*bo_ptr, domain, gpu_addr); ... if (cpu_addr) { r = amdgpu_bo_kmap(*bo_ptr, cpu_addr); ... } Creating is actually optional, but the function always pins the BO once more and figures out it's CPU address. As far as I can see that should solve your problem for now. Christian. BR Monk -Original Message- From: Christian König [mailto:ckoenig.leichtzumer...@gmail.com] Sent: 2017年9月18日 17:13 To: Liu, Monk ; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH 06/18] drm/amdgpu/sriov:fix memory leak after gpu reset Am 18.09.2017 um 08:11 schrieb Monk Liu: doing gpu reset will rerun all hw_init and thus ucode_init_bo is invoked again, so we need to skip the fw_buf allocation during sriov gpu reset to avoid memory leak. Change-Id: I31131eda1bd45ea2f5bdc50c5da5fc5a9fe9027d Signed-off-by: Monk Liu --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 64 +++ 2 files changed, 35 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 6ff2959..3d0c633 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1185,6 +1185,9 @@ struct amdgpu_firmware { /* gpu info firmware data pointer */ const struct firmware *gpu_info_fw; + + void *fw_buf_ptr; + uint64_t fw_buf_mc; }; /* diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c index f306374..6564902 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c @@ -360,8 +360,6 @@ static int amdgpu_ucode_patch_jt(struct amdgpu_firmware_info *ucode, int amdgpu_ucode_init_bo(struct amdgpu_device *adev) { struct amdgpu_bo **bo = &adev->firmware.fw_buf; - uint64_t fw_mc_addr; - void *fw_buf_ptr = NULL; uint64_t fw_offset = 0; int i, err; struct amdgpu_firmware_info *ucode = NULL; @@ -372,37 +370,39 @@ int amdgpu_ucode_init_bo(struct amdgpu_device *adev) return 0; } - err = amdgpu_bo_create(adev, adev->firmware.fw_size, PAGE_SIZE, true, - amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT, - AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS, - NULL, NULL, 0, bo); - if (err) { - dev_err(adev->dev, "(%d) Firmware buffer allocate failed\n", err); - goto failed; - } + if (!amdgpu_sriov_vf(adev) || !adev->in_sriov_reset) { Instead of all this better use amdgpu_bo_create_kernel(), this should already include most of the handling necessary here. Christian. + err = amdgpu_bo_create(adev, adev->firmware.fw_size, PAGE_SIZE, true, + amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT, + AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS, + NULL, NULL, 0, bo); + if (err) { + dev_err(adev->dev, "(%d) Firmware buffer allocate failed\n", err); + goto failed; + } - err = amdgpu_bo_reserve(*bo, false); - if (err) { - dev_err(adev->dev, "(%d) Firmware buffer reserve failed\n", err); - goto failed_reserve; - } + err = amdgpu_bo_reserve(*bo, false); + if (err) { + dev_err(adev->dev, "(%d) Firmware buffer reserve failed\n", err); + goto failed_reserve; + } - err = amdgpu_bo_pin(*bo, amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT, - &fw_mc_addr); - if (err) { - dev_err(adev->dev, "(%d) Firmware buffer pin failed\n", err); - goto failed_pin; - } + err = amdgpu_bo_pin(*bo, amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT, + &adev->firm
Re: [PATCH 12/18] drm/amdgpu:use formal register to trigger hdp invalidate
Yeah, but Vega10 has UVD7 and in uvd_v7_0.c we have: static void uvd_v7_0_ring_emit_hdp_invalidate(struct amdgpu_ring *ring) { amdgpu_ring_write(ring, PACKET0(SOC15_REG_OFFSET(HDP, 0, mmHDP_DEBUG0), 0)); amdgpu_ring_write(ring, 1); } That should probably be fixed as well. Regards, Christian. Am 18.09.2017 um 13:03 schrieb Liu, Monk: Only vega10 has this register -Original Message- From: Christian König [mailto:ckoenig.leichtzumer...@gmail.com] Sent: 2017年9月18日 17:20 To: Liu, Monk ; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH 12/18] drm/amdgpu:use formal register to trigger hdp invalidate Am 18.09.2017 um 08:11 schrieb Monk Liu: Change-Id: I61dc02ea6a450f9acfa3bae07aa20244261f5369 Signed-off-by: Monk Liu Reviewed-by: Christian König Please scan the code once more, we most likely have used mmHDP_DEBUG0 for this at even more places. Christian. --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 +- drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index f201510..44960b3 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -3549,7 +3549,7 @@ static void gfx_v9_0_ring_emit_hdp_flush(struct amdgpu_ring *ring) static void gfx_v9_0_ring_emit_hdp_invalidate(struct amdgpu_ring *ring) { gfx_v9_0_write_data_to_reg(ring, 0, true, - SOC15_REG_OFFSET(HDP, 0, mmHDP_DEBUG0), 1); + SOC15_REG_OFFSET(HDP, 0, mmHDP_READ_CACHE_INVALIDATE), 1); } static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring, diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c index fd7c72a..d5f3848 100644 --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c @@ -398,7 +398,7 @@ static void sdma_v4_0_ring_emit_hdp_invalidate(struct amdgpu_ring *ring) { amdgpu_ring_write(ring, SDMA_PKT_HEADER_OP(SDMA_OP_SRBM_WRITE) | SDMA_PKT_SRBM_WRITE_HEADER_BYTE_EN(0xf)); - amdgpu_ring_write(ring, SOC15_REG_OFFSET(HDP, 0, mmHDP_DEBUG0)); + amdgpu_ring_write(ring, SOC15_REG_OFFSET(HDP, 0, +mmHDP_READ_CACHE_INVALIDATE)); amdgpu_ring_write(ring, 1); } ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 13/18] drm/amdgpu:fix driver unloading bug
Am 18.09.2017 um 12:12 schrieb Liu, Monk: Christian, Let's discuss this patch and the one follows which skip the KIQ MQD free to avoid SAVE_FAIL issue. For skipping KIQ MQD deallocation patch, I think I will drop it and use a new way: We allocate KIQ MQD in VRAM domain and this BO can be safely freed after driver unloaded, because after driver unloaded no one will change the data in this BO *usually*. e.g. some root app can map visible vram and alter the value in it That sounds at least a bit better. But my question is why doesn't this work like it does on Tonga, e.g. correctly clean things up? for this patch "to skipping unbind the GART mapping to keep KIQ MQD always valid": Since hypervisor side always have couple hw component working, and they rely on GMC kept alive, so this is very different with BARE-METAL. That's to say we can only do like this way. Yeah, gut keeping the GART mapping alive is complete nonsense. When the driver unloads all memory should be returned to the OS. So we either keep a GART mapping to pages which are about to be reused and overwritten, or we leak memory on driver shutdown. Neither options sounds very good, Christian. Besides, we'll have more patches in future for L1 secure mode, which forbidden VF access GMC registers, so under L1 secure mode driver will always skip GMC programing under SRIOV both in init and fini, but that will come later BR Monk -Original Message- From: Christian König [mailto:ckoenig.leichtzumer...@gmail.com] Sent: 2017年9月18日 17:28 To: Liu, Monk ; amd-gfx@lists.freedesktop.org Cc: Chen, Horace Subject: Re: [PATCH 13/18] drm/amdgpu:fix driver unloading bug Am 18.09.2017 um 08:11 schrieb Monk Liu: [SWDEV-126631] - fix hypervisor save_vf fail that occured after driver removed: 1. Because the KIQ and KCQ were not ummapped, save_vf will fail if driver freed mqd of KIQ and KCQ. 2. KIQ can't be unmapped since RLCV always need it, the bo_free on KIQ should be skipped 3. KCQ can be unmapped, and should be unmapped during hw_fini, 4. RLCV still need to access other mc address from some hw even after driver unloaded, So we should not unbind gart for VF. Change-Id: I320487a9a848f41484c5f8cc11be34aca807b424 Signed-off-by: Horace Chen Signed-off-by: Monk Liu I absolutely can't judge if this is correct or not, but keeping the GART and KIQ alive after the driver is unloaded sounds really fishy to me. Isn't there any other clean way of handling this? Christian. --- drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 3 +- drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 5 +++ drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c| 60 +++- 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c index f437008..2fee071 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c @@ -394,7 +394,8 @@ int amdgpu_gart_init(struct amdgpu_device *adev) */ void amdgpu_gart_fini(struct amdgpu_device *adev) { - if (adev->gart.ready) { + /* gart is still used by other hw under SRIOV, don't unbind it */ + if (adev->gart.ready && !amdgpu_sriov_vf(adev)) { /* unbind pages */ amdgpu_gart_unbind(adev, 0, adev->gart.num_cpu_pages); } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c index 4f6c68f..bf6656f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c @@ -309,6 +309,11 @@ void amdgpu_gfx_compute_mqd_sw_fini(struct amdgpu_device *adev) &ring->mqd_ptr); } + /* don't deallocate KIQ mqd because the bo is still used by RLCV even + the guest VM is shutdown */ + if (amdgpu_sriov_vf(adev)) + return; + ring = &adev->gfx.kiq.ring; kfree(adev->gfx.mec.mqd_backup[AMDGPU_MAX_COMPUTE_RINGS]); amdgpu_bo_free_kernel(&ring->mqd_obj, diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index 44960b3..a577bbc 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -2892,14 +2892,72 @@ static int gfx_v9_0_hw_init(void *handle) return r; } +static int gfx_v9_0_kcq_disable(struct amdgpu_ring *kiq_ring,struct +amdgpu_ring *ring) { + struct amdgpu_device *adev = kiq_ring->adev; + uint32_t scratch, tmp = 0; + int r, i; + + r = amdgpu_gfx_scratch_get(adev, &scratch); + if (r) { + DRM_ERROR("Failed to get scratch reg (%d).\n", r); + return r; + } + WREG32(scratch, 0xCAFEDEAD); + + r = amdgpu_ring_alloc(kiq_ring, 10); + if (r) { + DRM_ERROR("Failed to lock KIQ (%d).\n", r); + amdgpu_gfx_scratch_free(adev, scratch); + return r; +
[PATCH] drm/amdgpu: use 2MB fragment size for GFX6,7 and 8
From: Christian König Use 2MB fragment size by default for older hardware generations as well. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 2 +- drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 2 +- drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c index 5be9c83..2d1f3f6 100644 --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c @@ -831,7 +831,7 @@ static int gmc_v6_0_sw_init(void *handle) if (r) return r; - amdgpu_vm_adjust_size(adev, 64, 4); + amdgpu_vm_adjust_size(adev, 64, 9); adev->vm_manager.max_pfn = adev->vm_manager.vm_size << 18; adev->mc.mc_mask = 0xffULL; diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c index eace9e7..2256277 100644 --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c @@ -970,7 +970,7 @@ static int gmc_v7_0_sw_init(void *handle) * Currently set to 4GB ((1 << 20) 4k pages). * Max GPUVM size for cayman and SI is 40 bits. */ - amdgpu_vm_adjust_size(adev, 64, 4); + amdgpu_vm_adjust_size(adev, 64, 9); adev->vm_manager.max_pfn = adev->vm_manager.vm_size << 18; /* Set the internal MC address mask diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c index 3b3326d..114671b 100644 --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c @@ -1067,7 +1067,7 @@ static int gmc_v8_0_sw_init(void *handle) * Currently set to 4GB ((1 << 20) 4k pages). * Max GPUVM size for cayman and SI is 40 bits. */ - amdgpu_vm_adjust_size(adev, 64, 4); + amdgpu_vm_adjust_size(adev, 64, 9); adev->vm_manager.max_pfn = adev->vm_manager.vm_size << 18; /* Set the internal MC address mask -- 2.7.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 4/4] drm/ttm: Remove TTM dma tracepoint since it's not required anymore
Signed-off-by: Tom St Denis --- drivers/gpu/drm/ttm/Makefile | 2 +- drivers/gpu/drm/ttm/ttm_debug.c | 74 - drivers/gpu/drm/ttm/ttm_trace.h | 87 --- drivers/gpu/drm/ttm/ttm_tracepoints.c | 45 -- 4 files changed, 1 insertion(+), 207 deletions(-) delete mode 100644 drivers/gpu/drm/ttm/ttm_debug.c delete mode 100644 drivers/gpu/drm/ttm/ttm_trace.h delete mode 100644 drivers/gpu/drm/ttm/ttm_tracepoints.c diff --git a/drivers/gpu/drm/ttm/Makefile b/drivers/gpu/drm/ttm/Makefile index ab2bef1219e5..4d0c938ff4b2 100644 --- a/drivers/gpu/drm/ttm/Makefile +++ b/drivers/gpu/drm/ttm/Makefile @@ -4,7 +4,7 @@ ttm-y := ttm_memory.o ttm_tt.o ttm_bo.o \ ttm_bo_util.o ttm_bo_vm.o ttm_module.o \ ttm_object.o ttm_lock.o ttm_execbuf_util.o ttm_page_alloc.o \ - ttm_bo_manager.o ttm_page_alloc_dma.o ttm_debug.o ttm_tracepoints.o + ttm_bo_manager.o ttm_page_alloc_dma.o ttm-$(CONFIG_AGP) += ttm_agp_backend.o obj-$(CONFIG_DRM_TTM) += ttm.o diff --git a/drivers/gpu/drm/ttm/ttm_debug.c b/drivers/gpu/drm/ttm/ttm_debug.c deleted file mode 100644 index ef5f0d090154.. --- a/drivers/gpu/drm/ttm/ttm_debug.c +++ /dev/null @@ -1,74 +0,0 @@ -/** - * - * Copyright (c) 2017 Advanced Micro Devices, Inc. - * All Rights Reserved. - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the - * "Software"), to deal in the Software without restriction, including - * without limitation the rights to use, copy, modify, merge, publish, - * distribute, sub license, and/or sell copies of the Software, and to - * permit persons to whom the Software is furnished to do so, subject to - * the following conditions: - * - * The above copyright notice and this permission notice (including the - * next paragraph) shall be included in all copies or substantial portions - * of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL - * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, - * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR - * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE - * USE OR OTHER DEALINGS IN THE SOFTWARE. - * - **/ -/* - * Authors: Tom St Denis - */ -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include "ttm_trace.h" - -void ttm_trace_dma_map(struct device *dev, struct ttm_dma_tt *tt) -{ - unsigned i; - - if (unlikely(trace_ttm_dma_map_enabled())) { - for (i = 0; i < tt->ttm.num_pages; i++) { - trace_ttm_dma_map( - dev, - tt->ttm.pages[i], - tt->dma_address[i]); - } - } -} -EXPORT_SYMBOL(ttm_trace_dma_map); - -void ttm_trace_dma_unmap(struct device *dev, struct ttm_dma_tt *tt) -{ - unsigned i; - - if (unlikely(trace_ttm_dma_unmap_enabled())) { - for (i = 0; i < tt->ttm.num_pages; i++) { - trace_ttm_dma_unmap( - dev, - tt->ttm.pages[i], - tt->dma_address[i]); - } - } -} -EXPORT_SYMBOL(ttm_trace_dma_unmap); - diff --git a/drivers/gpu/drm/ttm/ttm_trace.h b/drivers/gpu/drm/ttm/ttm_trace.h deleted file mode 100644 index 715ce68b7b33.. --- a/drivers/gpu/drm/ttm/ttm_trace.h +++ /dev/null @@ -1,87 +0,0 @@ -/** - * - * Copyright (c) 2017 Advanced Micro Devices, Inc. - * All Rights Reserved. - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the - * "Software"), to deal in the Software without restriction, including - * without limitation the rights to use, copy, modify, merge, publish, - * distribute, sub license, and/or sell copies of the Software, and to - * permit persons to whom the Software is furnished to do so, subject to - * the following conditions: - * - * The above copyright notice and this permission notice (including the - * next paragraph) shall be included in all copies or substantial portions - * of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NON-IN
[PATCH 2/4] drm/amd/amdgpu: add support for iova_to_phys to replace TTM trace
Signed-off-by: Tom St Denis --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 32 1 file changed, 32 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 7848ffa99eb4..b4c298925e2a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -43,6 +43,7 @@ #include #include #include +#include #include "amdgpu.h" #include "amdgpu_trace.h" #include "bif/bif_4_1_d.h" @@ -1810,6 +1811,36 @@ static const struct file_operations amdgpu_ttm_gtt_fops = { #endif +static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user *buf, + size_t size, loff_t *pos) +{ + struct amdgpu_device *adev = file_inode(f)->i_private; + int r; + uint64_t phys; + + // always return 8 bytes + if (size != 8) + return -EINVAL; + + // only accept page addresses + if (*pos & 0xFFF) + return -EINVAL; + + + phys = iommu_iova_to_phys(iommu_get_domain_for_dev(adev->dev), *pos); + + r = copy_to_user(buf, &phys, 8); + if (r) + return -EFAULT; + + return 8; +} + +static const struct file_operations amdgpu_ttm_iova_fops = { + .owner = THIS_MODULE, + .read = amdgpu_iova_to_phys_read, + .llseek = default_llseek +}; static const struct { char *name; @@ -1819,6 +1850,7 @@ static const struct { #ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS { "amdgpu_gtt", &amdgpu_ttm_gtt_fops }, #endif + { "amdgpu_iova", &amdgpu_ttm_iova_fops }, }; #endif -- 2.12.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 3/4] drm/amd/amdgpu: remove usage of ttm trace
Signed-off-by: Tom St Denis --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 36 +++-- 1 file changed, 3 insertions(+), 33 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index b4c298925e2a..e0c37fe4d043 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -34,7 +34,6 @@ #include #include #include -#include #include #include #include @@ -704,22 +703,6 @@ void amdgpu_ttm_tt_mark_user_pages(struct ttm_tt *ttm) } } -static void amdgpu_trace_dma_map(struct ttm_tt *ttm) -{ - struct amdgpu_device *adev = amdgpu_ttm_adev(ttm->bdev); - struct amdgpu_ttm_tt *gtt = (void *)ttm; - - ttm_trace_dma_map(adev->dev, >t->ttm); -} - -static void amdgpu_trace_dma_unmap(struct ttm_tt *ttm) -{ - struct amdgpu_device *adev = amdgpu_ttm_adev(ttm->bdev); - struct amdgpu_ttm_tt *gtt = (void *)ttm; - - ttm_trace_dma_unmap(adev->dev, >t->ttm); -} - /* prepare the sg table with the user pages */ static int amdgpu_ttm_tt_pin_userptr(struct ttm_tt *ttm) { @@ -746,8 +729,6 @@ static int amdgpu_ttm_tt_pin_userptr(struct ttm_tt *ttm) drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages, gtt->ttm.dma_address, ttm->num_pages); - amdgpu_trace_dma_map(ttm); - return 0; release_sg: @@ -773,8 +754,6 @@ static void amdgpu_ttm_tt_unpin_userptr(struct ttm_tt *ttm) amdgpu_ttm_tt_mark_user_pages(ttm); - amdgpu_trace_dma_unmap(ttm); - sg_free_table(ttm->sg); } @@ -958,7 +937,6 @@ static int amdgpu_ttm_tt_populate(struct ttm_tt *ttm) { struct amdgpu_device *adev = amdgpu_ttm_adev(ttm->bdev); struct amdgpu_ttm_tt *gtt = (void *)ttm; - int r; bool slave = !!(ttm->page_flags & TTM_PAGE_FLAG_SG); if (ttm->state != tt_unpopulated) @@ -978,22 +956,16 @@ static int amdgpu_ttm_tt_populate(struct ttm_tt *ttm) drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages, gtt->ttm.dma_address, ttm->num_pages); ttm->state = tt_unbound; - r = 0; - goto trace_mappings; + return 0; } #ifdef CONFIG_SWIOTLB if (swiotlb_nr_tbl()) { - r = ttm_dma_populate(>t->ttm, adev->dev); - goto trace_mappings; + return ttm_dma_populate(>t->ttm, adev->dev); } #endif - r = ttm_populate_and_map_pages(adev->dev, >t->ttm); -trace_mappings: - if (likely(!r)) - amdgpu_trace_dma_map(ttm); - return r; + return ttm_populate_and_map_pages(adev->dev, >t->ttm); } static void amdgpu_ttm_tt_unpopulate(struct ttm_tt *ttm) @@ -1014,8 +986,6 @@ static void amdgpu_ttm_tt_unpopulate(struct ttm_tt *ttm) adev = amdgpu_ttm_adev(ttm->bdev); - amdgpu_trace_dma_unmap(ttm); - #ifdef CONFIG_SWIOTLB if (swiotlb_nr_tbl()) { ttm_dma_unpopulate(>t->ttm, adev->dev); -- 2.12.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Remove TTM trace and use proper IOMMU api
These patches tidy up the amdgpu_ttm debugfs creation, add an iova_to_phys interface and then remove the TTM trace from both amdgpu and drm/ttm. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 1/4] drm/amd/amdgpu: Fold TTM debugfs entries into array
Signed-off-by: Tom St Denis --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 53 ++--- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 4 +-- 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 8ee16dfdb8af..7848ffa99eb4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1809,6 +1809,18 @@ static const struct file_operations amdgpu_ttm_gtt_fops = { #endif + + +static const struct { + char *name; + const struct file_operations *fops; +} ttm_debugfs_entries[] = { + { "amdgpu_vram", &amdgpu_ttm_vram_fops }, +#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS + { "amdgpu_gtt", &amdgpu_ttm_gtt_fops }, +#endif +}; + #endif static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev) @@ -1819,22 +1831,21 @@ static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev) struct drm_minor *minor = adev->ddev->primary; struct dentry *ent, *root = minor->debugfs_root; - ent = debugfs_create_file("amdgpu_vram", S_IFREG | S_IRUGO, root, - adev, &amdgpu_ttm_vram_fops); - if (IS_ERR(ent)) - return PTR_ERR(ent); - i_size_write(ent->d_inode, adev->mc.mc_vram_size); - adev->mman.vram = ent; - -#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS - ent = debugfs_create_file("amdgpu_gtt", S_IFREG | S_IRUGO, root, - adev, &amdgpu_ttm_gtt_fops); - if (IS_ERR(ent)) - return PTR_ERR(ent); - i_size_write(ent->d_inode, adev->mc.gart_size); - adev->mman.gtt = ent; + for (count = 0; count < ARRAY_SIZE(ttm_debugfs_entries); count++) { + ent = debugfs_create_file( + ttm_debugfs_entries[count].name, + S_IFREG | S_IRUGO, root, + adev, + ttm_debugfs_entries[count].fops); + if (IS_ERR(ent)) + return PTR_ERR(ent); + if (!strcmp(ttm_debugfs_entries[count].name, "amdgpu_vram")) + i_size_write(ent->d_inode, adev->mc.mc_vram_size); + else if (!strcmp(ttm_debugfs_entries[count].name, "amdgpu_gtt")) + i_size_write(ent->d_inode, adev->mc.gart_size); + adev->mman.debugfs_entries[count] = ent; + } -#endif count = ARRAY_SIZE(amdgpu_ttm_debugfs_list); #ifdef CONFIG_SWIOTLB @@ -1844,7 +1855,6 @@ static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev) return amdgpu_debugfs_add_files(adev, amdgpu_ttm_debugfs_list, count); #else - return 0; #endif } @@ -1852,14 +1862,9 @@ static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev) static void amdgpu_ttm_debugfs_fini(struct amdgpu_device *adev) { #if defined(CONFIG_DEBUG_FS) + unsigned i; - debugfs_remove(adev->mman.vram); - adev->mman.vram = NULL; - -#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS - debugfs_remove(adev->mman.gtt); - adev->mman.gtt = NULL; -#endif - + for (i = 0; i < ARRAY_SIZE(ttm_debugfs_entries); i++) + debugfs_remove(adev->mman.debugfs_entries[i]); #endif } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h index 64709e041d5b..7abae6867339 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h @@ -24,6 +24,7 @@ #ifndef __AMDGPU_TTM_H__ #define __AMDGPU_TTM_H__ +#include "amdgpu.h" #include "gpu_scheduler.h" #define AMDGPU_PL_GDS (TTM_PL_PRIV + 0) @@ -45,8 +46,7 @@ struct amdgpu_mman { boolinitialized; #if defined(CONFIG_DEBUG_FS) - struct dentry *vram; - struct dentry *gtt; + struct dentry *debugfs_entries[8]; #endif /* buffer handling */ -- 2.12.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH umr] switch over from ttm trace to iova debugfs entry
Signed-off-by: Tom St Denis --- src/lib/CMakeLists.txt | 1 - src/lib/close_asic.c | 2 +- src/lib/discover.c | 3 + src/lib/free_maps.c| 44 - src/lib/read_vram.c| 167 ++--- src/umr.h | 14 + 6 files changed, 10 insertions(+), 221 deletions(-) delete mode 100644 src/lib/free_maps.c diff --git a/src/lib/CMakeLists.txt b/src/lib/CMakeLists.txt index 78d827ac1bf1..a21fdf8eea2d 100644 --- a/src/lib/CMakeLists.txt +++ b/src/lib/CMakeLists.txt @@ -14,7 +14,6 @@ add_library(umrcore STATIC discover.c dump_ib.c find_reg.c - free_maps.c mmio.c query_drm.c read_sensor.c diff --git a/src/lib/close_asic.c b/src/lib/close_asic.c index d532a11fa671..a140409e617b 100644 --- a/src/lib/close_asic.c +++ b/src/lib/close_asic.c @@ -29,7 +29,6 @@ void umr_free_asic(struct umr_asic *asic) { int x; - umr_free_maps(asic); if (asic->pci.mem != NULL) { // free PCI mapping pci_device_unmap_range(asic->pci.pdevice, asic->pci.mem, asic->pci.pdevice->regions[asic->pci.region].size); @@ -57,6 +56,7 @@ void umr_close_asic(struct umr_asic *asic) cond_close(asic->fd.vram); cond_close(asic->fd.gpr); cond_close(asic->fd.drm); + cond_close(asic->fd.iova); umr_free_asic(asic); } } diff --git a/src/lib/discover.c b/src/lib/discover.c index dcc212fc39e4..ff7950e4e6ba 100644 --- a/src/lib/discover.c +++ b/src/lib/discover.c @@ -222,6 +222,8 @@ struct umr_asic *umr_discover_asic(struct umr_options *options) asic->fd.vram = open(fname, O_RDWR); snprintf(fname, sizeof(fname)-1, "/sys/kernel/debug/dri/%d/amdgpu_gpr", asic->instance); asic->fd.gpr = open(fname, O_RDWR); + snprintf(fname, sizeof(fname)-1, "/sys/kernel/debug/dri/%d/amdgpu_iova", asic->instance); + asic->fd.iova = open(fname, O_RDWR); asic->fd.drm = -1; // default to closed // if appending to the fd list remember to update close_asic() and discover_by_did()... } else { @@ -235,6 +237,7 @@ struct umr_asic *umr_discover_asic(struct umr_options *options) asic->fd.vram = -1; asic->fd.gpr = -1; asic->fd.drm = -1; + asic->fd.iova = -1; } if (options->use_pci) { diff --git a/src/lib/free_maps.c b/src/lib/free_maps.c deleted file mode 100644 index e1d27cb177f7.. --- a/src/lib/free_maps.c +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright 2017 Advanced Micro Devices, Inc. - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the "Software"), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR - * OTHER DEALINGS IN THE SOFTWARE. - * - * Authors: Tom St Denis - * - */ -#include "umr.h" - -static void recurse_free(struct umr_map *map) -{ - if (map->left) - recurse_free(map->left); - if (map->right) - recurse_free(map->right); - free(map); -} - -void umr_free_maps(struct umr_asic *asic) -{ - if (!asic->maps) - return; - - recurse_free(asic->maps->maps); - free(asic->maps); - asic->maps = NULL; -} diff --git a/src/lib/read_vram.c b/src/lib/read_vram.c index 6e8f1f931895..7890c90c51df 100644 --- a/src/lib/read_vram.c +++ b/src/lib/read_vram.c @@ -25,161 +25,13 @@ #include "umrapp.h" #include -// find a mapping or create node for it -static struct umr_map *find_map(struct umr_dma_maps *maps, uint64_t dma_addr, int create) -{ - struct umr_map *n = maps->maps, **nn; - uint64_t key; - - // addresses aren't terribly random - // so if we use an identity function on the search - // key we'll end up with a really unbalanced tree - // so mix up address a bit to randomize keys - key = d
Re: [PATCH 1/4] drm/amd/amdgpu: Fold TTM debugfs entries into array
Am 18.09.2017 um 14:35 schrieb Tom St Denis: Signed-off-by: Tom St Denis --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 53 ++--- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 4 +-- 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 8ee16dfdb8af..7848ffa99eb4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1809,6 +1809,18 @@ static const struct file_operations amdgpu_ttm_gtt_fops = { #endif + + +static const struct { + char *name; + const struct file_operations *fops; +} ttm_debugfs_entries[] = { + { "amdgpu_vram", &amdgpu_ttm_vram_fops }, +#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS + { "amdgpu_gtt", &amdgpu_ttm_gtt_fops }, +#endif +}; + #endif static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev) @@ -1819,22 +1831,21 @@ static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev) struct drm_minor *minor = adev->ddev->primary; struct dentry *ent, *root = minor->debugfs_root; - ent = debugfs_create_file("amdgpu_vram", S_IFREG | S_IRUGO, root, - adev, &amdgpu_ttm_vram_fops); - if (IS_ERR(ent)) - return PTR_ERR(ent); - i_size_write(ent->d_inode, adev->mc.mc_vram_size); - adev->mman.vram = ent; - -#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS - ent = debugfs_create_file("amdgpu_gtt", S_IFREG | S_IRUGO, root, - adev, &amdgpu_ttm_gtt_fops); - if (IS_ERR(ent)) - return PTR_ERR(ent); - i_size_write(ent->d_inode, adev->mc.gart_size); - adev->mman.gtt = ent; + for (count = 0; count < ARRAY_SIZE(ttm_debugfs_entries); count++) { + ent = debugfs_create_file( + ttm_debugfs_entries[count].name, + S_IFREG | S_IRUGO, root, + adev, + ttm_debugfs_entries[count].fops); + if (IS_ERR(ent)) + return PTR_ERR(ent); + if (!strcmp(ttm_debugfs_entries[count].name, "amdgpu_vram")) + i_size_write(ent->d_inode, adev->mc.mc_vram_size); + else if (!strcmp(ttm_debugfs_entries[count].name, "amdgpu_gtt")) + i_size_write(ent->d_inode, adev->mc.gart_size); Uff, string compare? That is screaming break me by typo. Maybe but the domain type into the struct as well? Apart from that looks good to me, Christian. + adev->mman.debugfs_entries[count] = ent; + } -#endif count = ARRAY_SIZE(amdgpu_ttm_debugfs_list); #ifdef CONFIG_SWIOTLB @@ -1844,7 +1855,6 @@ static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev) return amdgpu_debugfs_add_files(adev, amdgpu_ttm_debugfs_list, count); #else - return 0; #endif } @@ -1852,14 +1862,9 @@ static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev) static void amdgpu_ttm_debugfs_fini(struct amdgpu_device *adev) { #if defined(CONFIG_DEBUG_FS) + unsigned i; - debugfs_remove(adev->mman.vram); - adev->mman.vram = NULL; - -#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS - debugfs_remove(adev->mman.gtt); - adev->mman.gtt = NULL; -#endif - + for (i = 0; i < ARRAY_SIZE(ttm_debugfs_entries); i++) + debugfs_remove(adev->mman.debugfs_entries[i]); #endif } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h index 64709e041d5b..7abae6867339 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h @@ -24,6 +24,7 @@ #ifndef __AMDGPU_TTM_H__ #define __AMDGPU_TTM_H__ +#include "amdgpu.h" #include "gpu_scheduler.h" #define AMDGPU_PL_GDS (TTM_PL_PRIV + 0) @@ -45,8 +46,7 @@ struct amdgpu_mman { boolinitialized; #if defined(CONFIG_DEBUG_FS) - struct dentry *vram; - struct dentry *gtt; + struct dentry *debugfs_entries[8]; #endif /* buffer handling */ ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/4] drm/amd/amdgpu: add support for iova_to_phys to replace TTM trace
Am 18.09.2017 um 14:35 schrieb Tom St Denis: Signed-off-by: Tom St Denis --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 32 1 file changed, 32 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 7848ffa99eb4..b4c298925e2a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -43,6 +43,7 @@ #include #include #include +#include #include "amdgpu.h" #include "amdgpu_trace.h" #include "bif/bif_4_1_d.h" @@ -1810,6 +1811,36 @@ static const struct file_operations amdgpu_ttm_gtt_fops = { #endif +static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user *buf, + size_t size, loff_t *pos) +{ + struct amdgpu_device *adev = file_inode(f)->i_private; + int r; + uint64_t phys; + + // always return 8 bytes + if (size != 8) + return -EINVAL; + + // only accept page addresses + if (*pos & 0xFFF) + return -EINVAL; + + + phys = iommu_iova_to_phys(iommu_get_domain_for_dev(adev->dev), *pos); Well how about adding directly read/write support for the page behind the address? This way we won't need to fiddle with /dev/mem any more either. Christian. + + r = copy_to_user(buf, &phys, 8); + if (r) + return -EFAULT; + + return 8; +} + +static const struct file_operations amdgpu_ttm_iova_fops = { + .owner = THIS_MODULE, + .read = amdgpu_iova_to_phys_read, + .llseek = default_llseek +}; static const struct { char *name; @@ -1819,6 +1850,7 @@ static const struct { #ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS { "amdgpu_gtt", &amdgpu_ttm_gtt_fops }, #endif + { "amdgpu_iova", &amdgpu_ttm_iova_fops }, }; #endif ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/4] drm/amd/amdgpu: add support for iova_to_phys to replace TTM trace
On 18/09/17 08:52 AM, Christian König wrote: Am 18.09.2017 um 14:35 schrieb Tom St Denis: Signed-off-by: Tom St Denis --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 32 1 file changed, 32 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 7848ffa99eb4..b4c298925e2a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -43,6 +43,7 @@ #include #include #include +#include #include "amdgpu.h" #include "amdgpu_trace.h" #include "bif/bif_4_1_d.h" @@ -1810,6 +1811,36 @@ static const struct file_operations amdgpu_ttm_gtt_fops = { #endif +static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user *buf, + size_t size, loff_t *pos) +{ + struct amdgpu_device *adev = file_inode(f)->i_private; + int r; + uint64_t phys; + + // always return 8 bytes + if (size != 8) + return -EINVAL; + + // only accept page addresses + if (*pos & 0xFFF) + return -EINVAL; + + + phys = iommu_iova_to_phys(iommu_get_domain_for_dev(adev->dev), *pos); Well how about adding directly read/write support for the page behind the address? This way we won't need to fiddle with /dev/mem any more either. Given the flak we got from requesting this from the iommu team I'm worried that might not be appreciated by the community (even though we maintain this part of the tree) and hurt our abilities to upstream. I agree that adding a read/write method would be better though since we don't need config changes of /dev/fmem anymore. Tom ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/4] drm/amd/amdgpu: Fold TTM debugfs entries into array
On 18/09/17 08:48 AM, Christian König wrote: Am 18.09.2017 um 14:35 schrieb Tom St Denis: Signed-off-by: Tom St Denis --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 53 ++--- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 4 +-- 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 8ee16dfdb8af..7848ffa99eb4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1809,6 +1809,18 @@ static const struct file_operations amdgpu_ttm_gtt_fops = { #endif + + +static const struct { + char *name; + const struct file_operations *fops; +} ttm_debugfs_entries[] = { + { "amdgpu_vram", &amdgpu_ttm_vram_fops }, +#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS + { "amdgpu_gtt", &amdgpu_ttm_gtt_fops }, +#endif +}; + #endif static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev) @@ -1819,22 +1831,21 @@ static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev) struct drm_minor *minor = adev->ddev->primary; struct dentry *ent, *root = minor->debugfs_root; - ent = debugfs_create_file("amdgpu_vram", S_IFREG | S_IRUGO, root, - adev, &amdgpu_ttm_vram_fops); - if (IS_ERR(ent)) - return PTR_ERR(ent); - i_size_write(ent->d_inode, adev->mc.mc_vram_size); - adev->mman.vram = ent; - -#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS - ent = debugfs_create_file("amdgpu_gtt", S_IFREG | S_IRUGO, root, - adev, &amdgpu_ttm_gtt_fops); - if (IS_ERR(ent)) - return PTR_ERR(ent); - i_size_write(ent->d_inode, adev->mc.gart_size); - adev->mman.gtt = ent; + for (count = 0; count < ARRAY_SIZE(ttm_debugfs_entries); count++) { + ent = debugfs_create_file( + ttm_debugfs_entries[count].name, + S_IFREG | S_IRUGO, root, + adev, + ttm_debugfs_entries[count].fops); + if (IS_ERR(ent)) + return PTR_ERR(ent); + if (!strcmp(ttm_debugfs_entries[count].name, "amdgpu_vram")) + i_size_write(ent->d_inode, adev->mc.mc_vram_size); + else if (!strcmp(ttm_debugfs_entries[count].name, "amdgpu_gtt")) + i_size_write(ent->d_inode, adev->mc.gart_size); Uff, string compare? That is screaming break me by typo. Maybe but the domain type into the struct as well? Apart from that looks good to me, Sure, a quick grep didn't turn up any defines/enums for VRAM vs GTT though so just make some up? Tom ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/4] drm/amd/amdgpu: Fold TTM debugfs entries into array
Am 18.09.2017 um 15:01 schrieb Tom St Denis: On 18/09/17 08:48 AM, Christian König wrote: Am 18.09.2017 um 14:35 schrieb Tom St Denis: Signed-off-by: Tom St Denis --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 53 ++--- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 4 +-- 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 8ee16dfdb8af..7848ffa99eb4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1809,6 +1809,18 @@ static const struct file_operations amdgpu_ttm_gtt_fops = { #endif + + +static const struct { +char *name; +const struct file_operations *fops; +} ttm_debugfs_entries[] = { +{ "amdgpu_vram", &amdgpu_ttm_vram_fops }, +#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS +{ "amdgpu_gtt", &amdgpu_ttm_gtt_fops }, +#endif +}; + #endif static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev) @@ -1819,22 +1831,21 @@ static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev) struct drm_minor *minor = adev->ddev->primary; struct dentry *ent, *root = minor->debugfs_root; -ent = debugfs_create_file("amdgpu_vram", S_IFREG | S_IRUGO, root, - adev, &amdgpu_ttm_vram_fops); -if (IS_ERR(ent)) -return PTR_ERR(ent); -i_size_write(ent->d_inode, adev->mc.mc_vram_size); -adev->mman.vram = ent; - -#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS -ent = debugfs_create_file("amdgpu_gtt", S_IFREG | S_IRUGO, root, - adev, &amdgpu_ttm_gtt_fops); -if (IS_ERR(ent)) -return PTR_ERR(ent); -i_size_write(ent->d_inode, adev->mc.gart_size); -adev->mman.gtt = ent; +for (count = 0; count < ARRAY_SIZE(ttm_debugfs_entries); count++) { +ent = debugfs_create_file( +ttm_debugfs_entries[count].name, +S_IFREG | S_IRUGO, root, +adev, +ttm_debugfs_entries[count].fops); +if (IS_ERR(ent)) +return PTR_ERR(ent); +if (!strcmp(ttm_debugfs_entries[count].name, "amdgpu_vram")) +i_size_write(ent->d_inode, adev->mc.mc_vram_size); +else if (!strcmp(ttm_debugfs_entries[count].name, "amdgpu_gtt")) +i_size_write(ent->d_inode, adev->mc.gart_size); Uff, string compare? That is screaming break me by typo. Maybe but the domain type into the struct as well? Apart from that looks good to me, Sure, a quick grep didn't turn up any defines/enums for VRAM vs GTT though so just make some up? Just use TTM_PL_VRAM and TTM_PL_TT for this. Christian. Tom ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 09/11] drm/amdkfd: Fix kernel-queue wrapping bugs
On 2017-09-17 08:03 AM, Oded Gabbay wrote: > On Sat, Sep 16, 2017 at 2:43 AM, Felix Kuehling > wrote: >> From: Yong Zhao >> >> Avoid intermediate negative numbers when doing calculations with a mix >> of signed and unsigned variables where implicit conversions can lead >> to unexpected results. >> >> When kernel queue buffer wraps around to 0, we need to check that rptr >> won't be overwritten by the new packet. >> >> Signed-off-by: Yong Zhao >> Signed-off-by: Felix Kuehling >> --- >> drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 18 +++--- >> 1 file changed, 15 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c >> b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c >> index 9ebb4c1..1c66334 100644 >> --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c >> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c >> @@ -210,6 +210,11 @@ static int acquire_packet_buffer(struct kernel_queue >> *kq, >> uint32_t wptr, rptr; >> unsigned int *queue_address; >> >> + /* When rptr == wptr, the buffer is empty. > Start comment text in a new line. First line should be just /* > >> +* When rptr == wptr + 1, the buffer is full. >> +* It is always rptr that advances to the position of wptr, rather >> than >> +* the opposite. So we can only use up to queue_size_dwords - 1 >> dwords. >> +*/ >> rptr = *kq->rptr_kernel; >> wptr = *kq->wptr_kernel; >> queue_address = (unsigned int *)kq->pq_kernel_addr; >> @@ -219,11 +224,10 @@ static int acquire_packet_buffer(struct kernel_queue >> *kq, >> pr_debug("wptr: %d\n", wptr); >> pr_debug("queue_address 0x%p\n", queue_address); >> >> - available_size = (rptr - 1 - wptr + queue_size_dwords) % >> + available_size = (rptr + queue_size_dwords - 1 - wptr) % >> queue_size_dwords; >> >> - if (packet_size_in_dwords >= queue_size_dwords || >> - packet_size_in_dwords >= available_size) { >> + if (packet_size_in_dwords > available_size) { >> /* >> * make sure calling functions know >> * acquire_packet_buffer() failed >> @@ -233,6 +237,14 @@ static int acquire_packet_buffer(struct kernel_queue >> *kq, >> } >> >> if (wptr + packet_size_in_dwords >= queue_size_dwords) { >> + /* make sure after rolling back to position 0, there is >> +* still enough space. >> +*/ >> + if (packet_size_in_dwords >= rptr) { >> + *buffer_ptr = NULL; >> + return -ENOMEM; >> + } > I don't think the condition is correct. > Suppose, queue_size_dwords == 100, wptr == rptr == 50 (queue is empty) > and we have a new packet with size of 70. > Now, wptr + size is 120, which is >= 100 > However, 70 >= rptr (50) which will give us -ENOMEM, but this is not > correct condition, because the packet *does* have enough room in the > queue. Not really. We need 70 consecutive dwords. So the last 50 dwords of the queue are not enough. Therefore we decide to wrap around to the beginning of the queue and fill the last 50 dwords with NOPs. That means we really need 70+50 dwords, including those NOPs. Now we only have 50 dwords left in the queue for the actual packet, which is not enough. So we have no choice but to fail with ENOMEM. I think the consequence is, that we can't guarantee any successful allocations from the queue that need more than half of the queue. Regards, Felix > > I think the condition should be: > if (packet_size_in_dwords - (queue_size_dwords - wptr) >= rptr) > but please check this. > >> + /* fill nops, roll back and start at position 0 */ >> while (wptr > 0) { >> queue_address[wptr] = kq->nop_packet; >> wptr = (wptr + 1) % queue_size_dwords; >> -- >> 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
Re: [PATCH 09/11] drm/amdkfd: Fix kernel-queue wrapping bugs
On 2017-09-17 08:03 AM, Oded Gabbay wrote: > On Sat, Sep 16, 2017 at 2:43 AM, Felix Kuehling > wrote: >> From: Yong Zhao >> >> Avoid intermediate negative numbers when doing calculations with a mix >> of signed and unsigned variables where implicit conversions can lead >> to unexpected results. >> >> When kernel queue buffer wraps around to 0, we need to check that rptr >> won't be overwritten by the new packet. >> >> Signed-off-by: Yong Zhao >> Signed-off-by: Felix Kuehling >> --- >> drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 18 +++--- >> 1 file changed, 15 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c >> b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c >> index 9ebb4c1..1c66334 100644 >> --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c >> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c >> @@ -210,6 +210,11 @@ static int acquire_packet_buffer(struct kernel_queue >> *kq, >> uint32_t wptr, rptr; >> unsigned int *queue_address; >> >> + /* When rptr == wptr, the buffer is empty. > Start comment text in a new line. First line should be just /* That's a new rule to me, and it seems a bit arbitrary. checkpatch.pl doesn't complain about this. checkpatch.pl does complain about the closing */ not being on it's own line, but checkpatch has always been OK with multiline comments starting on the same line as the opening /*. There are also plenty of examples of multiline comments starting on the same line as /*. For example run this grep on the include/linux: grep -A3 '/\*[^*]\+[^/]$' *.h Regards, Felix > >> +* When rptr == wptr + 1, the buffer is full. >> +* It is always rptr that advances to the position of wptr, rather >> than >> +* the opposite. So we can only use up to queue_size_dwords - 1 >> dwords. >> +*/ >> rptr = *kq->rptr_kernel; >> wptr = *kq->wptr_kernel; >> queue_address = (unsigned int *)kq->pq_kernel_addr; >> @@ -219,11 +224,10 @@ static int acquire_packet_buffer(struct kernel_queue >> *kq, >> pr_debug("wptr: %d\n", wptr); >> pr_debug("queue_address 0x%p\n", queue_address); >> >> - available_size = (rptr - 1 - wptr + queue_size_dwords) % >> + available_size = (rptr + queue_size_dwords - 1 - wptr) % >> queue_size_dwords; >> >> - if (packet_size_in_dwords >= queue_size_dwords || >> - packet_size_in_dwords >= available_size) { >> + if (packet_size_in_dwords > available_size) { >> /* >> * make sure calling functions know >> * acquire_packet_buffer() failed >> @@ -233,6 +237,14 @@ static int acquire_packet_buffer(struct kernel_queue >> *kq, >> } >> >> if (wptr + packet_size_in_dwords >= queue_size_dwords) { >> + /* make sure after rolling back to position 0, there is >> +* still enough space. >> +*/ >> + if (packet_size_in_dwords >= rptr) { >> + *buffer_ptr = NULL; >> + return -ENOMEM; >> + } > I don't think the condition is correct. > Suppose, queue_size_dwords == 100, wptr == rptr == 50 (queue is empty) > and we have a new packet with size of 70. > Now, wptr + size is 120, which is >= 100 > However, 70 >= rptr (50) which will give us -ENOMEM, but this is not > correct condition, because the packet *does* have enough room in the > queue. > > I think the condition should be: > if (packet_size_in_dwords - (queue_size_dwords - wptr) >= rptr) > but please check this. > >> + /* fill nops, roll back and start at position 0 */ >> while (wptr > 0) { >> queue_address[wptr] = kq->nop_packet; >> wptr = (wptr + 1) % queue_size_dwords; >> -- >> 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
RE: [PATCH 09/11] drm/amdkfd: Fix kernel-queue wrapping bugs
Correct (coming from the guy who did all of the checkpatch cleanup for KFD). For multi-line comments, /* Can be on its own, or on the same line as the comment. */ has to be on its own. https://github.com/torvalds/linux/blob/master/scripts/checkpatch.pl#L3042 Kent -Original Message- From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of Felix Kuehling Sent: Monday, September 18, 2017 11:22 AM To: Oded Gabbay Cc: Zhao, Yong; amd-gfx list Subject: Re: [PATCH 09/11] drm/amdkfd: Fix kernel-queue wrapping bugs On 2017-09-17 08:03 AM, Oded Gabbay wrote: > On Sat, Sep 16, 2017 at 2:43 AM, Felix Kuehling > wrote: >> From: Yong Zhao >> >> Avoid intermediate negative numbers when doing calculations with a >> mix of signed and unsigned variables where implicit conversions can >> lead to unexpected results. >> >> When kernel queue buffer wraps around to 0, we need to check that >> rptr won't be overwritten by the new packet. >> >> Signed-off-by: Yong Zhao >> Signed-off-by: Felix Kuehling >> --- >> drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 18 >> +++--- >> 1 file changed, 15 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c >> b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c >> index 9ebb4c1..1c66334 100644 >> --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c >> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c >> @@ -210,6 +210,11 @@ static int acquire_packet_buffer(struct kernel_queue >> *kq, >> uint32_t wptr, rptr; >> unsigned int *queue_address; >> >> + /* When rptr == wptr, the buffer is empty. > Start comment text in a new line. First line should be just /* That's a new rule to me, and it seems a bit arbitrary. checkpatch.pl doesn't complain about this. checkpatch.pl does complain about the closing */ not being on it's own line, but checkpatch has always been OK with multiline comments starting on the same line as the opening /*. There are also plenty of examples of multiline comments starting on the same line as /*. For example run this grep on the include/linux: grep -A3 '/\*[^*]\+[^/]$' *.h Regards, Felix > >> +* When rptr == wptr + 1, the buffer is full. >> +* It is always rptr that advances to the position of wptr, rather >> than >> +* the opposite. So we can only use up to queue_size_dwords - 1 >> dwords. >> +*/ >> rptr = *kq->rptr_kernel; >> wptr = *kq->wptr_kernel; >> queue_address = (unsigned int *)kq->pq_kernel_addr; @@ >> -219,11 +224,10 @@ static int acquire_packet_buffer(struct kernel_queue *kq, >> pr_debug("wptr: %d\n", wptr); >> pr_debug("queue_address 0x%p\n", queue_address); >> >> - available_size = (rptr - 1 - wptr + queue_size_dwords) % >> + available_size = (rptr + queue_size_dwords - 1 - wptr) % >> >> queue_size_dwords; >> >> - if (packet_size_in_dwords >= queue_size_dwords || >> - packet_size_in_dwords >= available_size) { >> + if (packet_size_in_dwords > available_size) { >> /* >> * make sure calling functions know >> * acquire_packet_buffer() failed @@ -233,6 +237,14 >> @@ static int acquire_packet_buffer(struct kernel_queue *kq, >> } >> >> if (wptr + packet_size_in_dwords >= queue_size_dwords) { >> + /* make sure after rolling back to position 0, there is >> +* still enough space. >> +*/ >> + if (packet_size_in_dwords >= rptr) { >> + *buffer_ptr = NULL; >> + return -ENOMEM; >> + } > I don't think the condition is correct. > Suppose, queue_size_dwords == 100, wptr == rptr == 50 (queue is empty) > and we have a new packet with size of 70. > Now, wptr + size is 120, which is >= 100 However, 70 >= rptr (50) > which will give us -ENOMEM, but this is not correct condition, because > the packet *does* have enough room in the queue. > > I think the condition should be: > if (packet_size_in_dwords - (queue_size_dwords - wptr) >= rptr) but > please check this. > >> + /* fill nops, roll back and start at position 0 */ >> while (wptr > 0) { >> queue_address[wptr] = kq->nop_packet; >> wptr = (wptr + 1) % queue_size_dwords; >> -- >> 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 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH 09/11] drm/amdkfd: Fix kernel-queue wrapping bugs
Though to be fair, we should probably consolidate the comment style so that it's actually consistent through the KFD. Kent -Original Message- From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of Russell, Kent Sent: Monday, September 18, 2017 11:28 AM To: Kuehling, Felix; Oded Gabbay Cc: Zhao, Yong; amd-gfx list Subject: RE: [PATCH 09/11] drm/amdkfd: Fix kernel-queue wrapping bugs Correct (coming from the guy who did all of the checkpatch cleanup for KFD). For multi-line comments, /* Can be on its own, or on the same line as the comment. */ has to be on its own. https://github.com/torvalds/linux/blob/master/scripts/checkpatch.pl#L3042 Kent -Original Message- From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of Felix Kuehling Sent: Monday, September 18, 2017 11:22 AM To: Oded Gabbay Cc: Zhao, Yong; amd-gfx list Subject: Re: [PATCH 09/11] drm/amdkfd: Fix kernel-queue wrapping bugs On 2017-09-17 08:03 AM, Oded Gabbay wrote: > On Sat, Sep 16, 2017 at 2:43 AM, Felix Kuehling > wrote: >> From: Yong Zhao >> >> Avoid intermediate negative numbers when doing calculations with a >> mix of signed and unsigned variables where implicit conversions can >> lead to unexpected results. >> >> When kernel queue buffer wraps around to 0, we need to check that >> rptr won't be overwritten by the new packet. >> >> Signed-off-by: Yong Zhao >> Signed-off-by: Felix Kuehling >> --- >> drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 18 >> +++--- >> 1 file changed, 15 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c >> b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c >> index 9ebb4c1..1c66334 100644 >> --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c >> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c >> @@ -210,6 +210,11 @@ static int acquire_packet_buffer(struct kernel_queue >> *kq, >> uint32_t wptr, rptr; >> unsigned int *queue_address; >> >> + /* When rptr == wptr, the buffer is empty. > Start comment text in a new line. First line should be just /* That's a new rule to me, and it seems a bit arbitrary. checkpatch.pl doesn't complain about this. checkpatch.pl does complain about the closing */ not being on it's own line, but checkpatch has always been OK with multiline comments starting on the same line as the opening /*. There are also plenty of examples of multiline comments starting on the same line as /*. For example run this grep on the include/linux: grep -A3 '/\*[^*]\+[^/]$' *.h Regards, Felix > >> +* When rptr == wptr + 1, the buffer is full. >> +* It is always rptr that advances to the position of wptr, rather >> than >> +* the opposite. So we can only use up to queue_size_dwords - 1 >> dwords. >> +*/ >> rptr = *kq->rptr_kernel; >> wptr = *kq->wptr_kernel; >> queue_address = (unsigned int *)kq->pq_kernel_addr; @@ >> -219,11 +224,10 @@ static int acquire_packet_buffer(struct kernel_queue *kq, >> pr_debug("wptr: %d\n", wptr); >> pr_debug("queue_address 0x%p\n", queue_address); >> >> - available_size = (rptr - 1 - wptr + queue_size_dwords) % >> + available_size = (rptr + queue_size_dwords - 1 - wptr) % >> >> queue_size_dwords; >> >> - if (packet_size_in_dwords >= queue_size_dwords || >> - packet_size_in_dwords >= available_size) { >> + if (packet_size_in_dwords > available_size) { >> /* >> * make sure calling functions know >> * acquire_packet_buffer() failed @@ -233,6 +237,14 >> @@ static int acquire_packet_buffer(struct kernel_queue *kq, >> } >> >> if (wptr + packet_size_in_dwords >= queue_size_dwords) { >> + /* make sure after rolling back to position 0, there is >> +* still enough space. >> +*/ >> + if (packet_size_in_dwords >= rptr) { >> + *buffer_ptr = NULL; >> + return -ENOMEM; >> + } > I don't think the condition is correct. > Suppose, queue_size_dwords == 100, wptr == rptr == 50 (queue is empty) > and we have a new packet with size of 70. > Now, wptr + size is 120, which is >= 100 However, 70 >= rptr (50) > which will give us -ENOMEM, but this is not correct condition, because > the packet *does* have enough room in the queue. > > I think the condition should be: > if (packet_size_in_dwords - (queue_size_dwords - wptr) >= rptr) but > please check this. > >> + /* fill nops, roll back and start at position 0 */ >> while (wptr > 0) { >> queue_address[wptr] = kq->nop_packet; >> wptr = (wptr + 1) % queue_size_dwords; >> -- >> 2.7.4 >> >> ___ >> amd-gfx mailing li
Re: [PATCH 11/18] drm/amdgpu:add vgt_flush for gfx9
Yes, the UMD does it. Marek On Mon, Sep 18, 2017 at 11:18 AM, Christian König wrote: > Am 18.09.2017 um 08:11 schrieb Monk Liu: >> >> Change-Id: I584572cfb9145ee1b8d11d69ba2989bd6acfd706 >> Signed-off-by: Monk Liu > > > I could be wrong, but wasn't the consensus that this should be done by the > UMD? > > Marek, please comment. > > Christian. > > >> --- >> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 14 ++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >> index 3306667..f201510 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >> @@ -3499,6 +3499,17 @@ static void gfx_v9_0_ring_set_wptr_gfx(struct >> amdgpu_ring *ring) >> } >> } >> +static void gfx_v9_0_ring_emit_vgt_flush(struct amdgpu_ring *ring) >> +{ >> + amdgpu_ring_write(ring, PACKET3(PACKET3_EVENT_WRITE, 0)); >> + amdgpu_ring_write(ring, EVENT_TYPE(VS_PARTIAL_FLUSH) | >> + EVENT_INDEX(4)); >> + >> + amdgpu_ring_write(ring, PACKET3(PACKET3_EVENT_WRITE, 0)); >> + amdgpu_ring_write(ring, EVENT_TYPE(VGT_FLUSH) | >> + EVENT_INDEX(0)); >> +} >> + >> static void gfx_v9_0_ring_emit_hdp_flush(struct amdgpu_ring *ring) >> { >> u32 ref_and_mask, reg_mem_engine; >> @@ -3530,6 +3541,9 @@ static void gfx_v9_0_ring_emit_hdp_flush(struct >> amdgpu_ring *ring) >> nbio_hf_reg->hdp_flush_req_offset, >> nbio_hf_reg->hdp_flush_done_offset, >> ref_and_mask, ref_and_mask, 0x20); >> + >> + if (ring->funcs->type == AMDGPU_RING_TYPE_GFX) >> + gfx_v9_0_ring_emit_vgt_flush(ring); >> } >> static void gfx_v9_0_ring_emit_hdp_invalidate(struct amdgpu_ring >> *ring) > > > > ___ > 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
[PATCH 3/4] drm/amd/amdgpu: remove usage of ttm trace
Signed-off-by: Tom St Denis --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 36 +++-- 1 file changed, 3 insertions(+), 33 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 02ae32378e1c..b41d03226c26 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -34,7 +34,6 @@ #include #include #include -#include #include #include #include @@ -704,22 +703,6 @@ void amdgpu_ttm_tt_mark_user_pages(struct ttm_tt *ttm) } } -static void amdgpu_trace_dma_map(struct ttm_tt *ttm) -{ - struct amdgpu_device *adev = amdgpu_ttm_adev(ttm->bdev); - struct amdgpu_ttm_tt *gtt = (void *)ttm; - - ttm_trace_dma_map(adev->dev, >t->ttm); -} - -static void amdgpu_trace_dma_unmap(struct ttm_tt *ttm) -{ - struct amdgpu_device *adev = amdgpu_ttm_adev(ttm->bdev); - struct amdgpu_ttm_tt *gtt = (void *)ttm; - - ttm_trace_dma_unmap(adev->dev, >t->ttm); -} - /* prepare the sg table with the user pages */ static int amdgpu_ttm_tt_pin_userptr(struct ttm_tt *ttm) { @@ -746,8 +729,6 @@ static int amdgpu_ttm_tt_pin_userptr(struct ttm_tt *ttm) drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages, gtt->ttm.dma_address, ttm->num_pages); - amdgpu_trace_dma_map(ttm); - return 0; release_sg: @@ -773,8 +754,6 @@ static void amdgpu_ttm_tt_unpin_userptr(struct ttm_tt *ttm) amdgpu_ttm_tt_mark_user_pages(ttm); - amdgpu_trace_dma_unmap(ttm); - sg_free_table(ttm->sg); } @@ -958,7 +937,6 @@ static int amdgpu_ttm_tt_populate(struct ttm_tt *ttm) { struct amdgpu_device *adev = amdgpu_ttm_adev(ttm->bdev); struct amdgpu_ttm_tt *gtt = (void *)ttm; - int r; bool slave = !!(ttm->page_flags & TTM_PAGE_FLAG_SG); if (ttm->state != tt_unpopulated) @@ -978,22 +956,16 @@ static int amdgpu_ttm_tt_populate(struct ttm_tt *ttm) drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages, gtt->ttm.dma_address, ttm->num_pages); ttm->state = tt_unbound; - r = 0; - goto trace_mappings; + return 0; } #ifdef CONFIG_SWIOTLB if (swiotlb_nr_tbl()) { - r = ttm_dma_populate(>t->ttm, adev->dev); - goto trace_mappings; + return ttm_dma_populate(>t->ttm, adev->dev); } #endif - r = ttm_populate_and_map_pages(adev->dev, >t->ttm); -trace_mappings: - if (likely(!r)) - amdgpu_trace_dma_map(ttm); - return r; + return ttm_populate_and_map_pages(adev->dev, >t->ttm); } static void amdgpu_ttm_tt_unpopulate(struct ttm_tt *ttm) @@ -1014,8 +986,6 @@ static void amdgpu_ttm_tt_unpopulate(struct ttm_tt *ttm) adev = amdgpu_ttm_adev(ttm->bdev); - amdgpu_trace_dma_unmap(ttm); - #ifdef CONFIG_SWIOTLB if (swiotlb_nr_tbl()) { ttm_dma_unpopulate(>t->ttm, adev->dev); -- 2.12.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
remove ttm trace and add iova debugfs (v2)
In this respin I add some changes per feedback and make the iova entry have proper read/write methods which access pages mapped by amdgpu. So there is no need for /dev/mem or /dev/fmem anymore when reading system memory. Patches 3/4 are unchanged and remove the TTM trace from amdgpu and from TTM itself. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 1/4] drm/amd/amdgpu: Fold TTM debugfs entries into array (v2)
Signed-off-by: Tom St Denis (v2): add domains and avoid strcmp --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 54 ++--- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 4 +-- 2 files changed, 32 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 8ee16dfdb8af..50d20903de4f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1809,6 +1809,19 @@ static const struct file_operations amdgpu_ttm_gtt_fops = { #endif + + +static const struct { + char *name; + const struct file_operations *fops; + int domain; +} ttm_debugfs_entries[] = { + { "amdgpu_vram", &amdgpu_ttm_vram_fops, TTM_PL_VRAM }, +#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS + { "amdgpu_gtt", &amdgpu_ttm_gtt_fops, TTM_PL_TT }, +#endif +}; + #endif static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev) @@ -1819,22 +1832,21 @@ static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev) struct drm_minor *minor = adev->ddev->primary; struct dentry *ent, *root = minor->debugfs_root; - ent = debugfs_create_file("amdgpu_vram", S_IFREG | S_IRUGO, root, - adev, &amdgpu_ttm_vram_fops); - if (IS_ERR(ent)) - return PTR_ERR(ent); - i_size_write(ent->d_inode, adev->mc.mc_vram_size); - adev->mman.vram = ent; - -#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS - ent = debugfs_create_file("amdgpu_gtt", S_IFREG | S_IRUGO, root, - adev, &amdgpu_ttm_gtt_fops); - if (IS_ERR(ent)) - return PTR_ERR(ent); - i_size_write(ent->d_inode, adev->mc.gart_size); - adev->mman.gtt = ent; + for (count = 0; count < ARRAY_SIZE(ttm_debugfs_entries); count++) { + ent = debugfs_create_file( + ttm_debugfs_entries[count].name, + S_IFREG | S_IRUGO, root, + adev, + ttm_debugfs_entries[count].fops); + if (IS_ERR(ent)) + return PTR_ERR(ent); + if (ttm_debugfs_entries[count].domain == TTM_PL_VRAM) + i_size_write(ent->d_inode, adev->mc.mc_vram_size); + else if (ttm_debugfs_entries[count].domain == TTM_PL_TT) + i_size_write(ent->d_inode, adev->mc.gart_size); + adev->mman.debugfs_entries[count] = ent; + } -#endif count = ARRAY_SIZE(amdgpu_ttm_debugfs_list); #ifdef CONFIG_SWIOTLB @@ -1844,7 +1856,6 @@ static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev) return amdgpu_debugfs_add_files(adev, amdgpu_ttm_debugfs_list, count); #else - return 0; #endif } @@ -1852,14 +1863,9 @@ static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev) static void amdgpu_ttm_debugfs_fini(struct amdgpu_device *adev) { #if defined(CONFIG_DEBUG_FS) + unsigned i; - debugfs_remove(adev->mman.vram); - adev->mman.vram = NULL; - -#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS - debugfs_remove(adev->mman.gtt); - adev->mman.gtt = NULL; -#endif - + for (i = 0; i < ARRAY_SIZE(ttm_debugfs_entries); i++) + debugfs_remove(adev->mman.debugfs_entries[i]); #endif } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h index 64709e041d5b..7abae6867339 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h @@ -24,6 +24,7 @@ #ifndef __AMDGPU_TTM_H__ #define __AMDGPU_TTM_H__ +#include "amdgpu.h" #include "gpu_scheduler.h" #define AMDGPU_PL_GDS (TTM_PL_PRIV + 0) @@ -45,8 +46,7 @@ struct amdgpu_mman { boolinitialized; #if defined(CONFIG_DEBUG_FS) - struct dentry *vram; - struct dentry *gtt; + struct dentry *debugfs_entries[8]; #endif /* buffer handling */ -- 2.12.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 2/4] drm/amd/amdgpu: add support for iova_to_phys to replace TTM trace (v3)
Signed-off-by: Tom St Denis (v2): Add domain to iova debugfs (v3): Add true read/write methods to access system memory of pages mapped to the device --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 104 1 file changed, 104 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 50d20903de4f..02ae32378e1c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -43,6 +43,7 @@ #include #include #include +#include #include "amdgpu.h" #include "amdgpu_trace.h" #include "bif/bif_4_1_d.h" @@ -1810,6 +1811,108 @@ static const struct file_operations amdgpu_ttm_gtt_fops = { #endif +static void *transform_page(uint64_t phys) +{ + if (PageHighMem(pfn_to_page(PFN_DOWN(phys + return kmap(pfn_to_page(PFN_DOWN(phys))); + else + return __va(phys); +} + +static void untransform_page(uint64_t phys) +{ + if (PageHighMem(pfn_to_page(PFN_DOWN(phys + return kunmap(pfn_to_page(PFN_DOWN(phys))); +} + +static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user *buf, + size_t size, loff_t *pos) +{ + struct amdgpu_device *adev = file_inode(f)->i_private; + ssize_t result, n; + int r; + uint64_t phys; + void *ptr; + + result = 0; + while (size) { + // get physical address and map + phys = iommu_iova_to_phys(iommu_get_domain_for_dev(adev->dev), *pos); + + // copy upto one page + if (size > PAGE_SIZE) + n = PAGE_SIZE; + else + n = size; + + // to end of the page + if (((*pos & (PAGE_SIZE - 1)) + n) >= PAGE_SIZE) + n = PAGE_SIZE - (*pos & (PAGE_SIZE - 1)); + + ptr = transform_page(phys); + if (!ptr) + return -EFAULT; + + r = copy_to_user(buf, ptr, n); + untransform_page(phys); + if (r) + return -EFAULT; + + *pos += n; + size -= n; + result += n; + } + + return result; +} + +static ssize_t amdgpu_iova_to_phys_write(struct file *f, const char __user *buf, + size_t size, loff_t *pos) +{ + struct amdgpu_device *adev = file_inode(f)->i_private; + ssize_t result, n; + int r; + uint64_t phys; + void *ptr; + + result = 0; + while (size) { + // get physical address and map + phys = iommu_iova_to_phys(iommu_get_domain_for_dev(adev->dev), *pos); + + // copy upto one page + if (size > PAGE_SIZE) + n = PAGE_SIZE; + else + n = size; + + // to end of the page + if (((*pos & (PAGE_SIZE - 1)) + n) >= PAGE_SIZE) + n = PAGE_SIZE - (*pos & (PAGE_SIZE - 1)); + + ptr = transform_page(phys); + if (!ptr) + return -EFAULT; + + r = copy_from_user(ptr, buf, n); + untransform_page(phys); + if (r) + return -EFAULT; + + *pos += n; + size -= n; + result += n; + } + + return result; +} + +static const struct file_operations amdgpu_ttm_iova_fops = { + .owner = THIS_MODULE, + .read = amdgpu_iova_to_phys_read, + .write = amdgpu_iova_to_phys_write, + .llseek = default_llseek +}; static const struct { char *name; @@ -1820,6 +1923,7 @@ static const struct { #ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS { "amdgpu_gtt", &amdgpu_ttm_gtt_fops, TTM_PL_TT }, #endif + { "amdgpu_iova", &amdgpu_ttm_iova_fops, TTM_PL_SYSTEM }, }; #endif -- 2.12.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 4/4] drm/ttm: Remove TTM dma tracepoint since it's not required anymore
Signed-off-by: Tom St Denis --- drivers/gpu/drm/ttm/Makefile | 2 +- drivers/gpu/drm/ttm/ttm_debug.c | 74 - drivers/gpu/drm/ttm/ttm_trace.h | 87 --- drivers/gpu/drm/ttm/ttm_tracepoints.c | 45 -- 4 files changed, 1 insertion(+), 207 deletions(-) delete mode 100644 drivers/gpu/drm/ttm/ttm_debug.c delete mode 100644 drivers/gpu/drm/ttm/ttm_trace.h delete mode 100644 drivers/gpu/drm/ttm/ttm_tracepoints.c diff --git a/drivers/gpu/drm/ttm/Makefile b/drivers/gpu/drm/ttm/Makefile index ab2bef1219e5..4d0c938ff4b2 100644 --- a/drivers/gpu/drm/ttm/Makefile +++ b/drivers/gpu/drm/ttm/Makefile @@ -4,7 +4,7 @@ ttm-y := ttm_memory.o ttm_tt.o ttm_bo.o \ ttm_bo_util.o ttm_bo_vm.o ttm_module.o \ ttm_object.o ttm_lock.o ttm_execbuf_util.o ttm_page_alloc.o \ - ttm_bo_manager.o ttm_page_alloc_dma.o ttm_debug.o ttm_tracepoints.o + ttm_bo_manager.o ttm_page_alloc_dma.o ttm-$(CONFIG_AGP) += ttm_agp_backend.o obj-$(CONFIG_DRM_TTM) += ttm.o diff --git a/drivers/gpu/drm/ttm/ttm_debug.c b/drivers/gpu/drm/ttm/ttm_debug.c deleted file mode 100644 index ef5f0d090154.. --- a/drivers/gpu/drm/ttm/ttm_debug.c +++ /dev/null @@ -1,74 +0,0 @@ -/** - * - * Copyright (c) 2017 Advanced Micro Devices, Inc. - * All Rights Reserved. - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the - * "Software"), to deal in the Software without restriction, including - * without limitation the rights to use, copy, modify, merge, publish, - * distribute, sub license, and/or sell copies of the Software, and to - * permit persons to whom the Software is furnished to do so, subject to - * the following conditions: - * - * The above copyright notice and this permission notice (including the - * next paragraph) shall be included in all copies or substantial portions - * of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL - * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, - * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR - * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE - * USE OR OTHER DEALINGS IN THE SOFTWARE. - * - **/ -/* - * Authors: Tom St Denis - */ -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include "ttm_trace.h" - -void ttm_trace_dma_map(struct device *dev, struct ttm_dma_tt *tt) -{ - unsigned i; - - if (unlikely(trace_ttm_dma_map_enabled())) { - for (i = 0; i < tt->ttm.num_pages; i++) { - trace_ttm_dma_map( - dev, - tt->ttm.pages[i], - tt->dma_address[i]); - } - } -} -EXPORT_SYMBOL(ttm_trace_dma_map); - -void ttm_trace_dma_unmap(struct device *dev, struct ttm_dma_tt *tt) -{ - unsigned i; - - if (unlikely(trace_ttm_dma_unmap_enabled())) { - for (i = 0; i < tt->ttm.num_pages; i++) { - trace_ttm_dma_unmap( - dev, - tt->ttm.pages[i], - tt->dma_address[i]); - } - } -} -EXPORT_SYMBOL(ttm_trace_dma_unmap); - diff --git a/drivers/gpu/drm/ttm/ttm_trace.h b/drivers/gpu/drm/ttm/ttm_trace.h deleted file mode 100644 index 715ce68b7b33.. --- a/drivers/gpu/drm/ttm/ttm_trace.h +++ /dev/null @@ -1,87 +0,0 @@ -/** - * - * Copyright (c) 2017 Advanced Micro Devices, Inc. - * All Rights Reserved. - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the - * "Software"), to deal in the Software without restriction, including - * without limitation the rights to use, copy, modify, merge, publish, - * distribute, sub license, and/or sell copies of the Software, and to - * permit persons to whom the Software is furnished to do so, subject to - * the following conditions: - * - * The above copyright notice and this permission notice (including the - * next paragraph) shall be included in all copies or substantial portions - * of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NON-IN
RE: [PATCH] drm/amdgpu: use 2MB fragment size for GFX6,7 and 8
Acked-by: John Bridgman >-Original Message- >From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of >Christian König >Sent: Monday, September 18, 2017 8:34 AM >To: amd-gfx@lists.freedesktop.org >Subject: [PATCH] drm/amdgpu: use 2MB fragment size for GFX6,7 and 8 > >From: Christian König > >Use 2MB fragment size by default for older hardware generations as well. > >Signed-off-by: Christian König >--- > drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 2 +- >drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 2 +- >drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > >diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c >b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c >index 5be9c83..2d1f3f6 100644 >--- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c >+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c >@@ -831,7 +831,7 @@ static int gmc_v6_0_sw_init(void *handle) > if (r) > return r; > >- amdgpu_vm_adjust_size(adev, 64, 4); >+ amdgpu_vm_adjust_size(adev, 64, 9); > adev->vm_manager.max_pfn = adev->vm_manager.vm_size << 18; > > adev->mc.mc_mask = 0xffULL; >diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c >b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c >index eace9e7..2256277 100644 >--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c >+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c >@@ -970,7 +970,7 @@ static int gmc_v7_0_sw_init(void *handle) >* Currently set to 4GB ((1 << 20) 4k pages). >* Max GPUVM size for cayman and SI is 40 bits. >*/ >- amdgpu_vm_adjust_size(adev, 64, 4); >+ amdgpu_vm_adjust_size(adev, 64, 9); > adev->vm_manager.max_pfn = adev->vm_manager.vm_size << 18; > > /* Set the internal MC address mask >diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c >b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c >index 3b3326d..114671b 100644 >--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c >+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c >@@ -1067,7 +1067,7 @@ static int gmc_v8_0_sw_init(void *handle) >* Currently set to 4GB ((1 << 20) 4k pages). >* Max GPUVM size for cayman and SI is 40 bits. >*/ >- amdgpu_vm_adjust_size(adev, 64, 4); >+ amdgpu_vm_adjust_size(adev, 64, 9); > adev->vm_manager.max_pfn = adev->vm_manager.vm_size << 18; > > /* Set the internal MC address mask >-- >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
[PATCH umr] switch over from ttm trace to iova debugfs entry (v2)
Signed-off-by: Tom St Denis (v2): Use iova debugfs for read/write not address translation --- src/lib/CMakeLists.txt | 1 - src/lib/close_asic.c | 2 +- src/lib/discover.c | 3 + src/lib/free_maps.c| 44 -- src/lib/read_vram.c| 213 - src/umr.h | 14 +--- 6 files changed, 23 insertions(+), 254 deletions(-) delete mode 100644 src/lib/free_maps.c diff --git a/src/lib/CMakeLists.txt b/src/lib/CMakeLists.txt index 78d827ac1bf1..a21fdf8eea2d 100644 --- a/src/lib/CMakeLists.txt +++ b/src/lib/CMakeLists.txt @@ -14,7 +14,6 @@ add_library(umrcore STATIC discover.c dump_ib.c find_reg.c - free_maps.c mmio.c query_drm.c read_sensor.c diff --git a/src/lib/close_asic.c b/src/lib/close_asic.c index d532a11fa671..a140409e617b 100644 --- a/src/lib/close_asic.c +++ b/src/lib/close_asic.c @@ -29,7 +29,6 @@ void umr_free_asic(struct umr_asic *asic) { int x; - umr_free_maps(asic); if (asic->pci.mem != NULL) { // free PCI mapping pci_device_unmap_range(asic->pci.pdevice, asic->pci.mem, asic->pci.pdevice->regions[asic->pci.region].size); @@ -57,6 +56,7 @@ void umr_close_asic(struct umr_asic *asic) cond_close(asic->fd.vram); cond_close(asic->fd.gpr); cond_close(asic->fd.drm); + cond_close(asic->fd.iova); umr_free_asic(asic); } } diff --git a/src/lib/discover.c b/src/lib/discover.c index dcc212fc39e4..ff7950e4e6ba 100644 --- a/src/lib/discover.c +++ b/src/lib/discover.c @@ -222,6 +222,8 @@ struct umr_asic *umr_discover_asic(struct umr_options *options) asic->fd.vram = open(fname, O_RDWR); snprintf(fname, sizeof(fname)-1, "/sys/kernel/debug/dri/%d/amdgpu_gpr", asic->instance); asic->fd.gpr = open(fname, O_RDWR); + snprintf(fname, sizeof(fname)-1, "/sys/kernel/debug/dri/%d/amdgpu_iova", asic->instance); + asic->fd.iova = open(fname, O_RDWR); asic->fd.drm = -1; // default to closed // if appending to the fd list remember to update close_asic() and discover_by_did()... } else { @@ -235,6 +237,7 @@ struct umr_asic *umr_discover_asic(struct umr_options *options) asic->fd.vram = -1; asic->fd.gpr = -1; asic->fd.drm = -1; + asic->fd.iova = -1; } if (options->use_pci) { diff --git a/src/lib/free_maps.c b/src/lib/free_maps.c deleted file mode 100644 index e1d27cb177f7.. --- a/src/lib/free_maps.c +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright 2017 Advanced Micro Devices, Inc. - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the "Software"), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR - * OTHER DEALINGS IN THE SOFTWARE. - * - * Authors: Tom St Denis - * - */ -#include "umr.h" - -static void recurse_free(struct umr_map *map) -{ - if (map->left) - recurse_free(map->left); - if (map->right) - recurse_free(map->right); - free(map); -} - -void umr_free_maps(struct umr_asic *asic) -{ - if (!asic->maps) - return; - - recurse_free(asic->maps->maps); - free(asic->maps); - asic->maps = NULL; -} diff --git a/src/lib/read_vram.c b/src/lib/read_vram.c index 6e8f1f931895..928467a0a3a8 100644 --- a/src/lib/read_vram.c +++ b/src/lib/read_vram.c @@ -25,163 +25,6 @@ #include "umrapp.h" #include -// find a mapping or create node for it -static struct umr_map *find_map(struct umr_dma_maps *maps, uint64_t dma_addr, int create) -{ - struct umr_map *n = maps->maps, **nn; - uint64_t key; - - // addresses aren't terribly random - // so if we use an identity function on the search - // key we'll end up with a really unbalanced tree - //
RE: [PATCH v2 3/3] drm/amd/display: Reduce DC chattiness
> -Original Message- > From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf > Of Harry Wentland > Sent: Friday, September 15, 2017 7:52 PM > To: amd-gfx@lists.freedesktop.org > Cc: Grodzovsky, Andrey; Wentland, Harry; Kuehling, Felix; Deucher, > Alexander; Cheng, Tony; Koenig, Christian > Subject: [PATCH v2 3/3] drm/amd/display: Reduce DC chattiness > > Log DC init but default log level to 0 (default for > amdgpu_dc_log) otherwise. Bug reporters can still make > DC more chatty by using the dc_log module param. > amdgpu.dc_log = 1 > > v2: Only provide runtime option, no compile time config > > Change-Id: Icdfb849fa678225e2460519fbd8066540feb451a > Signed-off-by: Harry Wentland Series is: Reviewed-by: Alex Deucher > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 74 > -- > drivers/gpu/drm/amd/display/include/logger_types.h | 3 + > 2 files changed, 42 insertions(+), 35 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index abe89e3fed5b..846cff8bff17 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -333,7 +333,6 @@ int amdgpu_dm_init(struct amdgpu_device *adev) > adev->dm.ddev = adev->ddev; > adev->dm.adev = adev; > > - DRM_INFO("DAL is enabled\n"); > /* Zero all the fields */ > memset(&init_data, 0, sizeof(init_data)); > > @@ -373,7 +372,10 @@ int amdgpu_dm_init(struct amdgpu_device *adev) > > init_data.dce_environment = DCE_ENV_PRODUCTION_DRV; > > - init_data.log_mask = DC_DEFAULT_LOG_MASK; > + if (amdgpu_dc_log) > + init_data.log_mask = DC_DEFAULT_LOG_MASK; > + else > + init_data.log_mask = DC_MIN_LOG_MASK; > > #ifdef ENABLE_FBC > if (adev->family == FAMILY_CZ) > @@ -383,7 +385,9 @@ int amdgpu_dm_init(struct amdgpu_device *adev) > /* Display Core create. */ > adev->dm.dc = dc_create(&init_data); > > - if (!adev->dm.dc) > + if (adev->dm.dc) > + DRM_INFO("Display Core initialized!\n"); > + else > DRM_INFO("Display Core failed to initialize!\n"); > > INIT_WORK(&adev->dm.mst_hotplug_work, > hotplug_notify_work_func); > @@ -393,7 +397,7 @@ int amdgpu_dm_init(struct amdgpu_device *adev) > DRM_ERROR( > "amdgpu: failed to initialize freesync_module.\n"); > } else > - DRM_INFO("amdgpu: freesync_module init done %p.\n", > + DRM_DEBUG_DRIVER("amdgpu: freesync_module init done > %p.\n", > adev->dm.freesync_module); > > if (amdgpu_dm_initialize_drm_device(adev)) { > @@ -417,7 +421,7 @@ int amdgpu_dm_init(struct amdgpu_device *adev) > goto error; > } > > - DRM_INFO("KMS initialized.\n"); > + DRM_DEBUG_DRIVER("KMS initialized.\n"); > > return 0; > error: > @@ -475,7 +479,7 @@ static int detect_mst_link_for_all_connectors(struct > drm_device *dev) > list_for_each_entry(connector, &dev->mode_config.connector_list, > head) { > aconnector = to_amdgpu_dm_connector(connector); > if (aconnector->dc_link->type == > dc_connection_mst_branch) { > - DRM_INFO("DM_MST: starting TM on aconnector: > %p [id: %d]\n", > + DRM_DEBUG_DRIVER("DM_MST: starting TM on > aconnector: %p [id: %d]\n", > aconnector, aconnector- > >base.base.id); > > ret = > drm_dp_mst_topology_mgr_set_mst(&aconnector->mst_mgr, true); > @@ -819,12 +823,12 @@ void > amdgpu_dm_update_connector_after_detect( > if (aconnector->dc_sink == sink) { > /* We got a DP short pulse (Link Loss, DP CTS, etc...). >* Do nothing!! */ > - DRM_INFO("DCHPD: connector_id=%d: dc_sink didn't > change.\n", > + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: dc_sink > didn't change.\n", > aconnector->connector_id); > return; > } > > - DRM_INFO("DCHPD: connector_id=%d: Old sink=%p New > sink=%p\n", > + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: Old sink=%p New > sink=%p\n", > aconnector->connector_id, aconnector->dc_sink, sink); > > mutex_lock(&dev->mode_config.mutex); > @@ -926,7 +930,7 @@ static void dm_handle_hpd_rx_irq(struct > amdgpu_dm_connector *aconnector) > > process_count++; > > - DRM_DEBUG_KMS("ESI %02x %02x %02x\n", esi[0], esi[1], > esi[2]); > + DRM_DEBUG_DRIVER("ESI %02x %02x %02x\n", esi[0], > esi[1], esi[2]); > /* handle HPD short pulse irq */ > if (aconnector->mst_mgr.mst_state) > drm_dp_mst_hpd_irq( > @@ -964,7 +968,7 @@ static void dm_handle_hpd_rx_irq(struct > amdgpu_dm_connector *aconnector) > } > > if (p
[PATCH 2/2] drm/amdgpu: Fix a bug in amdgpu_fill_buffer()
When max_bytes is not 8 bytes aligned and bo size is larger than max_bytes, the last 8 bytes in a ttm node may be left unchanged. For example, on pre SDMA 4.0, max_bytes = 0x1f, and the bo size is 0x20, the problem will happen. In order to fix the problem, we store the max nums of PTEs/PDEs a single operation can set separately in amdgpu_vm_pte_funcs structure. Change-Id: I37c588a57cb63f1a8251fb5ead2eff4b39e047c9 Signed-off-by: Yong Zhao --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 6 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 8 drivers/gpu/drm/amd/amdgpu/cik_sdma.c | 3 +++ drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c | 3 +++ drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 3 +++ drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 3 +++ drivers/gpu/drm/amd/amdgpu/si_dma.c | 3 +++ 7 files changed, 25 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index a34c4cb..91bb111 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -309,6 +309,12 @@ struct amdgpu_vm_pte_funcs { uint64_t pe, uint64_t addr, unsigned count, uint32_t incr, uint64_t flags); + + /* maximum nums of PTEs/PDEs in a single operation */ + uint32_tset_max_nums_pte_pde; + + /* number of dw to reserve per operation */ + unsignedset_pte_pde_num_dw; }; /* provided by the gmc block */ diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 2180ed3..8685b0c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1556,8 +1556,8 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo, struct dma_fence **fence) { struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev); - /* max_bytes applies to SDMA_OP_PTEPDE as well as SDMA_OP_CONST_FILL*/ - uint32_t max_bytes = adev->mman.buffer_funcs->fill_max_bytes; + uint32_t max_bytes = 8 * + adev->vm_manager.vm_pte_funcs->set_max_nums_pte_pde; struct amdgpu_ring *ring = adev->mman.buffer_funcs_ring; struct drm_mm_node *mm_node; @@ -1589,8 +1589,8 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo, ++mm_node; } - /* 10 double words for each SDMA_OP_PTEPDE cmd */ - num_dw = num_loops * 10; + /* num of dwords for each SDMA_OP_PTEPDE cmd */ + num_dw = num_loops * adev->vm_manager.vm_pte_funcs->set_pte_pde_num_dw; /* for IB padding */ num_dw += 64; diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c index f508f4d..ff59351 100644 --- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c +++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c @@ -1390,6 +1390,9 @@ static const struct amdgpu_vm_pte_funcs cik_sdma_vm_pte_funcs = { .copy_pte = cik_sdma_vm_copy_pte, .write_pte = cik_sdma_vm_write_pte, .set_pte_pde = cik_sdma_vm_set_pte_pde, + + .set_max_nums_pte_pde = 0x1f >> 3, + .set_pte_pde_num_dw = 10, }; static void cik_sdma_set_vm_pte_funcs(struct amdgpu_device *adev) diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c index f2d0710..aec3586 100644 --- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c @@ -1327,6 +1327,9 @@ static const struct amdgpu_vm_pte_funcs sdma_v2_4_vm_pte_funcs = { .copy_pte = sdma_v2_4_vm_copy_pte, .write_pte = sdma_v2_4_vm_write_pte, .set_pte_pde = sdma_v2_4_vm_set_pte_pde, + + .set_max_nums_pte_pde = 0x1f >> 3, + .set_pte_pde_num_dw = 10, }; static void sdma_v2_4_set_vm_pte_funcs(struct amdgpu_device *adev) diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c index 4e7fe07..7610272 100644 --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c @@ -1751,6 +1751,9 @@ static const struct amdgpu_vm_pte_funcs sdma_v3_0_vm_pte_funcs = { .copy_pte = sdma_v3_0_vm_copy_pte, .write_pte = sdma_v3_0_vm_write_pte, .set_pte_pde = sdma_v3_0_vm_set_pte_pde, + + .set_max_nums_pte_pde = 0x3fffe0 >> 3, + .set_pte_pde_num_dw = 10, }; static void sdma_v3_0_set_vm_pte_funcs(struct amdgpu_device *adev) diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c index fd7c72a..6e1e0c0 100644 --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c @@ -1717,6 +1717,9 @@ static const struct amdgpu_vm_pte_funcs sdma_v4_0_vm_pte_funcs = { .copy_pte = sdma_v4_0_vm_copy_pte, .write_pte = sdma_v4_0_vm_write_pte, .set_pte_pde = sdma_v4_0_vm_set_pte_pde, + + .set_max_nums_pte_pde = 0x40 >> 3, + .set_pte_pde_num_dw = 10, }; static voi
[PATCH 1/2] drm/amdgpu: Correct bytes limit for SDMA 3.0 copy and fill
Change-Id: I10fc5efbc303056c5c5c4dc4f4dd2c3186595a91 Signed-off-by: Yong Zhao --- drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c index 728c0d8..4e7fe07 100644 --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c @@ -1730,11 +1730,11 @@ static void sdma_v3_0_emit_fill_buffer(struct amdgpu_ib *ib, } static const struct amdgpu_buffer_funcs sdma_v3_0_buffer_funcs = { - .copy_max_bytes = 0x1f, + .copy_max_bytes = 0x3fffe0, /* not 0x3f due to HW limitation*/ .copy_num_dw = 7, .emit_copy_buffer = sdma_v3_0_emit_copy_buffer, - .fill_max_bytes = 0x1f, + .fill_max_bytes = 0x3fffe0, /* not 0x3f due to HW limitation*/ .fill_num_dw = 5, .emit_fill_buffer = sdma_v3_0_emit_fill_buffer, }; -- 2.7.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 14/18] drm/amdgpu: Fix amdgpu reload failure under SRIOV
On Mon, Sep 18, 2017 at 5:31 AM, Christian König wrote: > Am 18.09.2017 um 08:11 schrieb Monk Liu: >> >> From: Horace Chen >> >> Kernel will set the PCI power state to UNKNOWN after unloading, >> Since SRIOV has faked PCI config space so the UNKNOWN state >> will be kept forever. >> >> In driver reload if the power state is UNKNOWN then enabling msi >> will fail. >> >> forcely set it to D0 for SRIOV to fix this kernel flawness. >> >> Change-Id: I6a72d5fc9b653b21c3c98167515a511c5edeb91c >> Signed-off-by: Horace Chen >> Signed-off-by: Monk Liu > > > Acked-by: Christian König , but better wait for > Alex to have a look as well on this before pushing it. > Seems reasonable to me barring Xiangliang's comment. Acked-by: Alex Deucher > Christian. > >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 10 +- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c >> index 914c5bf..345406a 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c >> @@ -229,7 +229,15 @@ int amdgpu_irq_init(struct amdgpu_device *adev) >> adev->irq.msi_enabled = false; >> if (amdgpu_msi_ok(adev)) { >> - int ret = pci_enable_msi(adev->pdev); >> + int ret; >> + if (amdgpu_sriov_vf(adev) && >> + adev->pdev->current_state == PCI_UNKNOWN){ >> + /* If pci power state is unknown on the SRIOV >> platform, >> +* it may be set in the remove device. We need to >> forcely >> +* set it to D0 to enable the msi*/ >> + adev->pdev->current_state = PCI_D0; >> + } >> + ret = pci_enable_msi(adev->pdev); >> if (!ret) { >> adev->irq.msi_enabled = true; >> dev_info(adev->dev, "amdgpu: using MSI.\n"); > > > > ___ > 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
[PATCH] radv: disable implicit sync for radv allocated bos
Implicit sync kicks in when a buffer is used by two different amdgpu contexts simultaneously. Jobs that use explicit synchronization mechanisms end up needlessly waiting to be scheduled for long periods of time in order to achieve serialized execution. This patch disables implicit synchronization for all radv allocations. The only systems that require implicit synchronization are DRI2/3 and PRIME. Signed-off-by: Andres Rodriguez --- src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.c b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.c index 325f875..9dc7559 100644 --- a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.c +++ b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.c @@ -330,6 +330,7 @@ radv_amdgpu_winsys_bo_create(struct radeon_winsys *_ws, request.flags |= AMDGPU_GEM_CREATE_NO_CPU_ACCESS; if (flags & RADEON_FLAG_GTT_WC) request.flags |= AMDGPU_GEM_CREATE_CPU_GTT_USWC; + request.flags |= AMDGPU_GEM_CREATE_EXPLICIT_SYNC; r = amdgpu_bo_alloc(ws->dev, &request, &buf_handle); if (r) { -- 2.9.3 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH libdrm] amdgpu: update headers
For testing the kernel commit --- include/drm/amdgpu_drm.h | 4 1 file changed, 4 insertions(+) diff --git a/include/drm/amdgpu_drm.h b/include/drm/amdgpu_drm.h index da2ade6..c01abaa 100644 --- a/include/drm/amdgpu_drm.h +++ b/include/drm/amdgpu_drm.h @@ -89,6 +89,10 @@ extern "C" { #define AMDGPU_GEM_CREATE_SHADOW (1 << 4) /* Flag that allocating the BO should use linear VRAM */ #define AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS (1 << 5) +/* Flag that BO is always valid in this VM */ +#define AMDGPU_GEM_CREATE_VM_ALWAYS_VALID (1 << 6) +/* Flag that BO sharing will be explicitely sync'd */ +#define AMDGPU_GEM_CREATE_EXPLICIT_SYNC(1 << 7) struct drm_amdgpu_gem_create_in { /** the requested memory size */ -- 2.9.3 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] Add flag to disable implicit synchronization
Implicit synchronization of jobs that access a shared bo is always enabled. Currently this behaviour is required for DRI2/3 and PRIME use cases, where the protocol doesn't provide a mechanism to shared an synchronization primitive alongside the surface. This patch series aims to provide a mechanism to allow userspace to disable implicit synchronization when it is not required. Following is an example of some async compute work getting delayed for 2.12ms due to implicit synchronization: https://drive.google.com/open?id=0B2ygSoZuj3IMRzFCYzBxaDFpaFk Following is the same workload but AMDGPU_GEM_CREATE_EXPLICIT_SYNC enabled: https://drive.google.com/open?id=0B2ygSoZuj3IMb0pTZEJRQmNwVHM In the second case we can see that hellovr_vulkan and the steamvr compositor can access the same surface simultaneously, without the gpu scheduler introducing any implicit waits. Gpuvis traces for these two scenarios can be found here: https://drive.google.com/open?id=0B2ygSoZuj3IMRklfM1llbTJqTnc The libdrm and radv patches are included for reference. Andres Rodriguez (1): drm/amdgpu: introduce AMDGPU_GEM_CREATE_EXPLICIT_SYNC drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c| 4 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 8 drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 8 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 6 -- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 10 ++ include/uapi/drm/amdgpu_drm.h | 2 ++ 8 files changed, 34 insertions(+), 11 deletions(-) -- 2.9.3 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: introduce AMDGPU_GEM_CREATE_EXPLICIT_SYNC
Introduce a flag to signal that access to a BO will be synchronized through an external mechanism. Currently all buffers shared between contexts are subject to implicit synchronization. However, this is only required for protocols that currently don't support an explicit synchronization mechanism (DRI2/3). This patch introduces the AMDGPU_GEM_CREATE_EXPLICIT_SYNC, so that users can specify when it is safe to disable implicit sync. Signed-off-by: Andres Rodriguez --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c| 4 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 8 drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 8 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 6 -- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 10 ++ include/uapi/drm/amdgpu_drm.h | 2 ++ 8 files changed, 34 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index db97e78..107533f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -704,7 +704,9 @@ static int amdgpu_cs_sync_rings(struct amdgpu_cs_parser *p) list_for_each_entry(e, &p->validated, tv.head) { struct reservation_object *resv = e->robj->tbo.resv; - r = amdgpu_sync_resv(p->adev, &p->job->sync, resv, p->filp); + r = amdgpu_sync_resv(p->adev, &p->job->sync, resv, +p->filp, +amdgpu_bo_explicit_sync(e->robj)); if (r) return r; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index b0d45c8..21e9936 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -212,7 +212,9 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data, AMDGPU_GEM_CREATE_NO_CPU_ACCESS | AMDGPU_GEM_CREATE_CPU_GTT_USWC | AMDGPU_GEM_CREATE_VRAM_CLEARED | - AMDGPU_GEM_CREATE_VM_ALWAYS_VALID)) + AMDGPU_GEM_CREATE_VM_ALWAYS_VALID | + AMDGPU_GEM_CREATE_EXPLICIT_SYNC)) + return -EINVAL; /* reject invalid gem domains */ diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h index c26ef53..428aae0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h @@ -193,6 +193,14 @@ static inline bool amdgpu_bo_gpu_accessible(struct amdgpu_bo *bo) } } +/** + * amdgpu_bo_explicit_sync - return whether the bo is explicitly synced + */ +static inline bool amdgpu_bo_explicit_sync(struct amdgpu_bo *bo) +{ + return bo->flags & AMDGPU_GEM_CREATE_EXPLICIT_SYNC; +} + int amdgpu_bo_create(struct amdgpu_device *adev, unsigned long size, int byte_align, bool kernel, u32 domain, u64 flags, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c index c586f44..6bf4bed 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c @@ -169,14 +169,15 @@ int amdgpu_sync_fence(struct amdgpu_device *adev, struct amdgpu_sync *sync, * * @sync: sync object to add fences from reservation object to * @resv: reservation object with embedded fence - * @shared: true if we should only sync to the exclusive fence + * @explicit_sync: true if we should only sync to the exclusive fence * * Sync to the fence */ int amdgpu_sync_resv(struct amdgpu_device *adev, struct amdgpu_sync *sync, struct reservation_object *resv, -void *owner) +void *owner, +bool explicit_sync) { struct reservation_object_list *flist; struct dma_fence *f; @@ -191,6 +192,9 @@ int amdgpu_sync_resv(struct amdgpu_device *adev, f = reservation_object_get_excl(resv); r = amdgpu_sync_fence(adev, sync, f); + if (explicit_sync) + return r; + flist = reservation_object_get_list(resv); if (!flist || r) return r; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h index dc76879..70d7e3a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h @@ -45,7 +45,8 @@ int amdgpu_sync_fence(struct amdgpu_device *adev, struct amdgpu_sync *sync, int amdgpu_sync_resv(struct amdgpu_device *adev, struct amdgpu_sync *sync, struct reservation_object *resv, -void *owner); +void *owner, +bool explicit_
Re: remove ttm trace and add iova debugfs (v2)
Should add I was able to read/write system memory mapped by amdgpu with these patches in place on my polaris10 device (with iommu enabled of course). From: amd-gfx on behalf of Tom St Denis Sent: Monday, September 18, 2017 13:33 To: amd-gfx@lists.freedesktop.org Cc: dri-de...@lists.freedesktop.org Subject: remove ttm trace and add iova debugfs (v2) In this respin I add some changes per feedback and make the iova entry have proper read/write methods which access pages mapped by amdgpu. So there is no need for /dev/mem or /dev/fmem anymore when reading system memory. Patches 3/4 are unchanged and remove the TTM trace from amdgpu and from TTM itself. ___ 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 0/8] Retry page fault handling for Vega10
Thanks for the reviews. I rebased this on amd-staging-drm-next, retested and submitted. Christian, do you want to do some graphics PASID and VMFault work on top of that? I think I'll be working on more KFD upstreaming this week and maybe look at this subject again next week. Regards, Felix On 2017-09-12 07:05 PM, Felix Kuehling wrote: > Rebased on adeucher/amd-staging-4.13 and tested on Vega10 (graphics) > and Kaveri (KFD). Meaningful graphics tests with retry faults enabled > will only be possible after PASID support is added to amdgpu_cs. > > The chash table was moved to drivers/gpu/drm/amd/lib for now but is > ready to move to lib if needed. I have not got any feedback on LKLM > and I don't want that to hold up the patch series. > > TODO: > * Finish upstreaming KFD > * Allocate PASIDs for graphics contexts > * Setup VMID-PASID mapping during graphics command submission > * Confirm that graphics page faults have the correct PASID in the IV > > > Felix Kuehling (8): > drm/amdgpu: Fix error handling in amdgpu_vm_init > drm/amdgpu: Add PASID management > drm/radeon: Add PASID manager for KFD > drm/amdkfd: Separate doorbell allocation from PASID > drm/amdkfd: Use PASID manager from KGD > drm/amdgpu: Add prescreening stage in IH processing > drm/amd: Closed hash table with low overhead > drm/amdgpu: Track pending retry faults in IH and VM (v2) > > drivers/gpu/drm/Kconfig | 3 + > drivers/gpu/drm/Makefile | 1 + > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 + > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c | 2 + > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c | 2 + > drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c| 82 +++ > drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h| 12 + > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 84 ++- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h| 21 +- > drivers/gpu/drm/amd/amdgpu/cik_ih.c | 14 + > drivers/gpu/drm/amd/amdgpu/cz_ih.c| 14 + > drivers/gpu/drm/amd/amdgpu/iceland_ih.c | 14 + > drivers/gpu/drm/amd/amdgpu/si_ih.c| 14 + > drivers/gpu/drm/amd/amdgpu/tonga_ih.c | 14 + > drivers/gpu/drm/amd/amdgpu/vega10_ih.c| 90 > drivers/gpu/drm/amd/amdkfd/kfd_device.c | 7 - > drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c | 50 +- > drivers/gpu/drm/amd/amdkfd/kfd_module.c | 6 - > drivers/gpu/drm/amd/amdkfd/kfd_pasid.c| 90 ++-- > drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 10 +- > drivers/gpu/drm/amd/amdkfd/kfd_process.c | 6 + > drivers/gpu/drm/amd/include/kgd_kfd_interface.h | 6 + > drivers/gpu/drm/amd/include/linux/chash.h | 358 + > drivers/gpu/drm/amd/lib/Kconfig | 27 + > drivers/gpu/drm/amd/lib/Makefile | 11 + > drivers/gpu/drm/amd/lib/chash.c | 622 > ++ > drivers/gpu/drm/radeon/radeon_kfd.c | 31 ++ > 28 files changed, 1504 insertions(+), 91 deletions(-) > create mode 100644 drivers/gpu/drm/amd/include/linux/chash.h > create mode 100644 drivers/gpu/drm/amd/lib/Kconfig > create mode 100644 drivers/gpu/drm/amd/lib/Makefile > create mode 100644 drivers/gpu/drm/amd/lib/chash.c > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] lib: Closed hash table with low overhead
On 2017-09-15 05:14 PM, Andrew Morton wrote: > On Mon, 28 Aug 2017 21:53:10 -0400 Felix Kuehling > wrote: > >> This adds a statically sized closed hash table implementation with >> low memory and CPU overhead. The API is inspired by kfifo. >> >> Storing, retrieving and deleting data does not involve any dynamic >> memory management, which makes it ideal for use in interrupt context. >> Static memory usage per entry comprises a 32 or 64 bit hash key, two >> bits for occupancy tracking and the value size stored in the table. >> No list heads or pointers are needed. Therefore this data structure >> should be quite cache-friendly, too. >> >> It uses linear probing and lazy deletion. During lookups free space >> is reclaimed and entries relocated to speed up future lookups. > I haven't looked at the implementation (yet), but I'm wondering if you > have identified hash table users (or implementations) elsewhere in the > kernel which might be migrated to use this? If so, such conversions > can be used to determine/demonstrate the desirability of the patch. I haven't looked into this. I did a quick search for where hash tables are currently used in the kernel tree. But I'm not sufficiently familiar with the subsystems to easily decide which ones could benefit from my work. My implementation has some constraints because the hash table is not resizable (by design). I think it may be useful for some applications in drivers, where the amount of data in the hash table is limited by HW constraints. Data stored in the table should also be quite small. For larger data structures that need to be allocated with kmalloc, you may as well use hashtable or rhashtable. To see a performance impact, you'd need very frequent lookups. For now I've settled on checking the code into the amdgpu driver so I can make progress. If someone finds another application for it, it can be moved to lib/ easily. Regards, Felix ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH 14/18] drm/amdgpu: Fix amdgpu reload failure under SRIOV
pci_enable_device will set power state to D0. This patch is just work around the issue, not address the root cause. -Original Message- From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of Alex Deucher Sent: Tuesday, September 19, 2017 5:07 AM To: Koenig, Christian Cc: Chen, Horace ; amd-gfx list ; Liu, Monk Subject: Re: [PATCH 14/18] drm/amdgpu: Fix amdgpu reload failure under SRIOV On Mon, Sep 18, 2017 at 5:31 AM, Christian König wrote: > Am 18.09.2017 um 08:11 schrieb Monk Liu: >> >> From: Horace Chen >> >> Kernel will set the PCI power state to UNKNOWN after unloading, Since >> SRIOV has faked PCI config space so the UNKNOWN state will be kept >> forever. >> >> In driver reload if the power state is UNKNOWN then enabling msi will >> fail. >> >> forcely set it to D0 for SRIOV to fix this kernel flawness. >> >> Change-Id: I6a72d5fc9b653b21c3c98167515a511c5edeb91c >> Signed-off-by: Horace Chen >> Signed-off-by: Monk Liu > > > Acked-by: Christian König , but better wait > for Alex to have a look as well on this before pushing it. > Seems reasonable to me barring Xiangliang's comment. Acked-by: Alex Deucher > Christian. > >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 10 +- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c >> index 914c5bf..345406a 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c >> @@ -229,7 +229,15 @@ int amdgpu_irq_init(struct amdgpu_device *adev) >> adev->irq.msi_enabled = false; >> if (amdgpu_msi_ok(adev)) { >> - int ret = pci_enable_msi(adev->pdev); >> + int ret; >> + if (amdgpu_sriov_vf(adev) && >> + adev->pdev->current_state == PCI_UNKNOWN){ >> + /* If pci power state is unknown on the SRIOV >> platform, >> +* it may be set in the remove device. We >> + need to >> forcely >> +* set it to D0 to enable the msi*/ >> + adev->pdev->current_state = PCI_D0; >> + } >> + ret = pci_enable_msi(adev->pdev); >> if (!ret) { >> adev->irq.msi_enabled = true; >> dev_info(adev->dev, "amdgpu: using MSI.\n"); > > > > ___ > 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 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amdgpu: use 2MB fragment size for GFX6,7 and 8
Reviewed-by: Roger He Thanks Roger(Hongbo.He) -Original Message- From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of Christian K?nig Sent: Monday, September 18, 2017 8:34 PM To: amd-gfx@lists.freedesktop.org Subject: [PATCH] drm/amdgpu: use 2MB fragment size for GFX6,7 and 8 From: Christian König Use 2MB fragment size by default for older hardware generations as well. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 2 +- drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 2 +- drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c index 5be9c83..2d1f3f6 100644 --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c @@ -831,7 +831,7 @@ static int gmc_v6_0_sw_init(void *handle) if (r) return r; - amdgpu_vm_adjust_size(adev, 64, 4); + amdgpu_vm_adjust_size(adev, 64, 9); adev->vm_manager.max_pfn = adev->vm_manager.vm_size << 18; adev->mc.mc_mask = 0xffULL; diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c index eace9e7..2256277 100644 --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c @@ -970,7 +970,7 @@ static int gmc_v7_0_sw_init(void *handle) * Currently set to 4GB ((1 << 20) 4k pages). * Max GPUVM size for cayman and SI is 40 bits. */ - amdgpu_vm_adjust_size(adev, 64, 4); + amdgpu_vm_adjust_size(adev, 64, 9); adev->vm_manager.max_pfn = adev->vm_manager.vm_size << 18; /* Set the internal MC address mask diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c index 3b3326d..114671b 100644 --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c @@ -1067,7 +1067,7 @@ static int gmc_v8_0_sw_init(void *handle) * Currently set to 4GB ((1 << 20) 4k pages). * Max GPUVM size for cayman and SI is 40 bits. */ - amdgpu_vm_adjust_size(adev, 64, 4); + amdgpu_vm_adjust_size(adev, 64, 9); adev->vm_manager.max_pfn = adev->vm_manager.vm_size << 18; /* Set the internal MC address mask -- 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
Re: [PATCH] drm/amdgpu: introduce AMDGPU_GEM_CREATE_EXPLICIT_SYNC
On 2017年09月19日 07:16, Andres Rodriguez wrote: Introduce a flag to signal that access to a BO will be synchronized through an external mechanism. Currently all buffers shared between contexts are subject to implicit synchronization. However, this is only required for protocols that currently don't support an explicit synchronization mechanism (DRI2/3). This patch introduces the AMDGPU_GEM_CREATE_EXPLICIT_SYNC, so that users can specify when it is safe to disable implicit sync. Signed-off-by: Andres Rodriguez --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c| 4 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 8 drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 8 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 6 -- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 10 ++ include/uapi/drm/amdgpu_drm.h | 2 ++ 8 files changed, 34 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index db97e78..107533f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -704,7 +704,9 @@ static int amdgpu_cs_sync_rings(struct amdgpu_cs_parser *p) list_for_each_entry(e, &p->validated, tv.head) { struct reservation_object *resv = e->robj->tbo.resv; - r = amdgpu_sync_resv(p->adev, &p->job->sync, resv, p->filp); + r = amdgpu_sync_resv(p->adev, &p->job->sync, resv, +p->filp, +amdgpu_bo_explicit_sync(e->robj)); if (r) return r; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index b0d45c8..21e9936 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -212,7 +212,9 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data, AMDGPU_GEM_CREATE_NO_CPU_ACCESS | AMDGPU_GEM_CREATE_CPU_GTT_USWC | AMDGPU_GEM_CREATE_VRAM_CLEARED | - AMDGPU_GEM_CREATE_VM_ALWAYS_VALID)) + AMDGPU_GEM_CREATE_VM_ALWAYS_VALID | + AMDGPU_GEM_CREATE_EXPLICIT_SYNC)) + return -EINVAL; /* reject invalid gem domains */ diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h index c26ef53..428aae0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h @@ -193,6 +193,14 @@ static inline bool amdgpu_bo_gpu_accessible(struct amdgpu_bo *bo) } } +/** + * amdgpu_bo_explicit_sync - return whether the bo is explicitly synced + */ +static inline bool amdgpu_bo_explicit_sync(struct amdgpu_bo *bo) +{ + return bo->flags & AMDGPU_GEM_CREATE_EXPLICIT_SYNC; +} + int amdgpu_bo_create(struct amdgpu_device *adev, unsigned long size, int byte_align, bool kernel, u32 domain, u64 flags, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c index c586f44..6bf4bed 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c @@ -169,14 +169,15 @@ int amdgpu_sync_fence(struct amdgpu_device *adev, struct amdgpu_sync *sync, * * @sync: sync object to add fences from reservation object to * @resv: reservation object with embedded fence - * @shared: true if we should only sync to the exclusive fence + * @explicit_sync: true if we should only sync to the exclusive fence * * Sync to the fence */ int amdgpu_sync_resv(struct amdgpu_device *adev, struct amdgpu_sync *sync, struct reservation_object *resv, -void *owner) +void *owner, +bool explicit_sync) Could you move explicit_sync inside function? like: bool explicit_sync = bo->flags & AMDGPU_GEM_CREATE_EXPLICIT_SYNC; { struct reservation_object_list *flist; struct dma_fence *f; @@ -191,6 +192,9 @@ int amdgpu_sync_resv(struct amdgpu_device *adev, f = reservation_object_get_excl(resv); r = amdgpu_sync_fence(adev, sync, f); + if (explicit_sync) + return r; + Do you need to move return before syncing excl? Regards, David Zhou flist = reservation_object_get_list(resv); if (!flist || r) return r; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h index dc76879..70d7e3a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h @@ -45,7 +45,8 @@ int amdgpu_sync_fence(struct amdgpu_device *adev, struct amdgpu_sync *sync, int amdgp
Re: [PATCH] drm/radeon: properly initialize r600_audio_status() data
On Sat, Sep 16, 2017 at 8:20 AM, Christian König wrote: > Am 15.09.2017 um 22:06 schrieb Arnd Bergmann: >> >> The structure returned from r600_audio_status() is only partially >> initialized, and older gcc versions (4.3 and 4.4) warn about this: >> >> drivers/gpu/drm/radeon/r600_hdmi.c: In function 'r600_audio_status': >> drivers/gpu/drm/radeon/r600_hdmi.c:108: error: 'status.id' is used >> uninitialized in this function >> drivers/gpu/drm/radeon/r600_hdmi.c:108: error: 'status.connected' is used >> uninitialized in this function >> drivers/gpu/drm/radeon/r600_hdmi.c:108: error: 'status.offset' is used >> uninitialized in this function >> >> This is harmless and surprisingly correct in C99, as the caller >> only accesses the fields that got initialized, so newer compilers >> don't warn about it, but initializing the entire structure feels >> like the right thing to do here and avoids the warning. >> >> Signed-off-by: Arnd Bergmann > > > Reviewed-by: Christian König Applied. thanks! Alex > >> --- >> drivers/gpu/drm/radeon/r600_hdmi.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/radeon/r600_hdmi.c >> b/drivers/gpu/drm/radeon/r600_hdmi.c >> index e82a99cb2459..ab32830c4e23 100644 >> --- a/drivers/gpu/drm/radeon/r600_hdmi.c >> +++ b/drivers/gpu/drm/radeon/r600_hdmi.c >> @@ -58,7 +58,7 @@ enum r600_hdmi_iec_status_bits { >> static struct r600_audio_pin r600_audio_status(struct radeon_device >> *rdev) >> { >> - struct r600_audio_pin status; >> + struct r600_audio_pin status = {}; >> uint32_t value; >> value = RREG32(R600_AUDIO_RATE_BPS_CHANNEL); > > > > ___ > 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] radv: disable implicit sync for radv allocated bos
Got some feedback from Dave, and this patch doesn't handle dri3 use cases correctly. Regards, Andres On 2017-09-18 07:16 PM, Andres Rodriguez wrote: Implicit sync kicks in when a buffer is used by two different amdgpu contexts simultaneously. Jobs that use explicit synchronization mechanisms end up needlessly waiting to be scheduled for long periods of time in order to achieve serialized execution. This patch disables implicit synchronization for all radv allocations. The only systems that require implicit synchronization are DRI2/3 and PRIME. Signed-off-by: Andres Rodriguez --- src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.c b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.c index 325f875..9dc7559 100644 --- a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.c +++ b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.c @@ -330,6 +330,7 @@ radv_amdgpu_winsys_bo_create(struct radeon_winsys *_ws, request.flags |= AMDGPU_GEM_CREATE_NO_CPU_ACCESS; if (flags & RADEON_FLAG_GTT_WC) request.flags |= AMDGPU_GEM_CREATE_CPU_GTT_USWC; + request.flags |= AMDGPU_GEM_CREATE_EXPLICIT_SYNC; r = amdgpu_bo_alloc(ws->dev, &request, &buf_handle); if (r) { ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: introduce AMDGPU_GEM_CREATE_EXPLICIT_SYNC
On 2017-09-18 10:47 PM, zhoucm1 wrote: On 2017年09月19日 07:16, Andres Rodriguez wrote: Introduce a flag to signal that access to a BO will be synchronized through an external mechanism. Currently all buffers shared between contexts are subject to implicit synchronization. However, this is only required for protocols that currently don't support an explicit synchronization mechanism (DRI2/3). This patch introduces the AMDGPU_GEM_CREATE_EXPLICIT_SYNC, so that users can specify when it is safe to disable implicit sync. Signed-off-by: Andres Rodriguez --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 8 drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 8 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 6 -- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 10 ++ include/uapi/drm/amdgpu_drm.h | 2 ++ 8 files changed, 34 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index db97e78..107533f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -704,7 +704,9 @@ static int amdgpu_cs_sync_rings(struct amdgpu_cs_parser *p) list_for_each_entry(e, &p->validated, tv.head) { struct reservation_object *resv = e->robj->tbo.resv; - r = amdgpu_sync_resv(p->adev, &p->job->sync, resv, p->filp); + r = amdgpu_sync_resv(p->adev, &p->job->sync, resv, + p->filp, + amdgpu_bo_explicit_sync(e->robj)); if (r) return r; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index b0d45c8..21e9936 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -212,7 +212,9 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data, AMDGPU_GEM_CREATE_NO_CPU_ACCESS | AMDGPU_GEM_CREATE_CPU_GTT_USWC | AMDGPU_GEM_CREATE_VRAM_CLEARED | - AMDGPU_GEM_CREATE_VM_ALWAYS_VALID)) + AMDGPU_GEM_CREATE_VM_ALWAYS_VALID | + AMDGPU_GEM_CREATE_EXPLICIT_SYNC)) + return -EINVAL; /* reject invalid gem domains */ diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h index c26ef53..428aae0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h @@ -193,6 +193,14 @@ static inline bool amdgpu_bo_gpu_accessible(struct amdgpu_bo *bo) } } +/** + * amdgpu_bo_explicit_sync - return whether the bo is explicitly synced + */ +static inline bool amdgpu_bo_explicit_sync(struct amdgpu_bo *bo) +{ + return bo->flags & AMDGPU_GEM_CREATE_EXPLICIT_SYNC; +} + int amdgpu_bo_create(struct amdgpu_device *adev, unsigned long size, int byte_align, bool kernel, u32 domain, u64 flags, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c index c586f44..6bf4bed 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c @@ -169,14 +169,15 @@ int amdgpu_sync_fence(struct amdgpu_device *adev, struct amdgpu_sync *sync, * * @sync: sync object to add fences from reservation object to * @resv: reservation object with embedded fence - * @shared: true if we should only sync to the exclusive fence + * @explicit_sync: true if we should only sync to the exclusive fence * * Sync to the fence */ int amdgpu_sync_resv(struct amdgpu_device *adev, struct amdgpu_sync *sync, struct reservation_object *resv, - void *owner) + void *owner, + bool explicit_sync) Could you move explicit_sync inside function? like: bool explicit_sync = bo->flags & AMDGPU_GEM_CREATE_EXPLICIT_SYNC; I was thinking of doing something like this originally. Extract the ttm bo from the resv object, and then get the abo from that. Would've been a pretty tiny and clean patch. However, the reservation object is a pointer instead of being embedded inside the ttm bo. So that path doesn't work. Passing in a pointer to the full bo is overkill I think. And the function might be used in cases where the reservation object is not associated with a specific bo (at least the current interface allows for that). That is why I ended up choosing this interface, even though it made the patch a lot more verbose. But if you, or anyone, has any suggestions on how to simplify this patch let me know. { struct reservation_object_list *flist; struct dma_fence *f; @@ -191,6 +192,9 @@ int amdgpu_sync_resv(struct amdgpu_device *adev, f = reservation_object_get_excl(resv); r = amd
[PATCH] drm/amdgpu: add helper to convert a ttm bo to amdgpu_bo
Signed-off-by: Andres Rodriguez --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 8 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 5 + drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 9 + 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 726a662..73eedd3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -40,9 +40,7 @@ static void amdgpu_ttm_bo_destroy(struct ttm_buffer_object *tbo) { struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev); - struct amdgpu_bo *bo; - - bo = container_of(tbo, struct amdgpu_bo, tbo); + struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo); amdgpu_bo_kunmap(bo); @@ -891,7 +889,7 @@ void amdgpu_bo_move_notify(struct ttm_buffer_object *bo, if (!amdgpu_ttm_bo_is_amdgpu_bo(bo)) return; - abo = container_of(bo, struct amdgpu_bo, tbo); + abo = ttm_to_amdgpu_bo(bo); amdgpu_vm_bo_invalidate(adev, abo, evict); amdgpu_bo_kunmap(abo); @@ -918,7 +916,7 @@ int amdgpu_bo_fault_reserve_notify(struct ttm_buffer_object *bo) if (!amdgpu_ttm_bo_is_amdgpu_bo(bo)) return 0; - abo = container_of(bo, struct amdgpu_bo, tbo); + abo = ttm_to_amdgpu_bo(bo); /* Remember that this BO was accessed by the CPU */ abo->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h index 39b6bf6..c26ef53 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h @@ -94,6 +94,11 @@ struct amdgpu_bo { }; }; +static inline struct amdgpu_bo *ttm_to_amdgpu_bo(struct ttm_buffer_object *tbo) +{ + return container_of(tbo, struct amdgpu_bo, tbo); +} + /** * amdgpu_mem_type_to_domain - return domain corresponding to mem_type * @mem_type: ttm memory type diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index b2b11e1..c9c059d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -44,6 +44,7 @@ #include #include #include "amdgpu.h" +#include "amdgpu_object.h" #include "amdgpu_trace.h" #include "bif/bif_4_1_d.h" @@ -209,7 +210,7 @@ static void amdgpu_evict_flags(struct ttm_buffer_object *bo, placement->num_busy_placement = 1; return; } - abo = container_of(bo, struct amdgpu_bo, tbo); + abo = ttm_to_amdgpu_bo(bo); switch (bo->mem.mem_type) { case TTM_PL_VRAM: if (adev->mman.buffer_funcs && @@ -257,7 +258,7 @@ static void amdgpu_evict_flags(struct ttm_buffer_object *bo, static int amdgpu_verify_access(struct ttm_buffer_object *bo, struct file *filp) { - struct amdgpu_bo *abo = container_of(bo, struct amdgpu_bo, tbo); + struct amdgpu_bo *abo = ttm_to_amdgpu_bo(bo); if (amdgpu_ttm_tt_get_usermm(bo->ttm)) return -EPERM; @@ -484,7 +485,7 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, int r; /* Can't move a pinned BO */ - abo = container_of(bo, struct amdgpu_bo, tbo); + abo = ttm_to_amdgpu_bo(bo); if (WARN_ON_ONCE(abo->pin_count > 0)) return -EINVAL; @@ -1172,7 +1173,7 @@ static int amdgpu_ttm_access_memory(struct ttm_buffer_object *bo, unsigned long offset, void *buf, int len, int write) { - struct amdgpu_bo *abo = container_of(bo, struct amdgpu_bo, tbo); + struct amdgpu_bo *abo = ttm_to_amdgpu_bo(bo); struct amdgpu_device *adev = amdgpu_ttm_adev(abo->tbo.bdev); struct drm_mm_node *nodes = abo->tbo.mem.mm_node; uint32_t value = 0; -- 2.9.3 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: add helper to convert a ttm bo to amdgpu_bo
This is a small cleanup patch from my initial naive attempt at extracting a TTM bo in amdgpu_sync_resv(). It didn't end up being useful in that specific case, but I thought I'd send it out anyways in case you find it useful. Regards, Andres On 2017-09-18 11:17 PM, Andres Rodriguez wrote: Signed-off-by: Andres Rodriguez --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 8 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 5 + drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 9 + 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 726a662..73eedd3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -40,9 +40,7 @@ static void amdgpu_ttm_bo_destroy(struct ttm_buffer_object *tbo) { struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev); - struct amdgpu_bo *bo; - - bo = container_of(tbo, struct amdgpu_bo, tbo); + struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo); amdgpu_bo_kunmap(bo); @@ -891,7 +889,7 @@ void amdgpu_bo_move_notify(struct ttm_buffer_object *bo, if (!amdgpu_ttm_bo_is_amdgpu_bo(bo)) return; - abo = container_of(bo, struct amdgpu_bo, tbo); + abo = ttm_to_amdgpu_bo(bo); amdgpu_vm_bo_invalidate(adev, abo, evict); amdgpu_bo_kunmap(abo); @@ -918,7 +916,7 @@ int amdgpu_bo_fault_reserve_notify(struct ttm_buffer_object *bo) if (!amdgpu_ttm_bo_is_amdgpu_bo(bo)) return 0; - abo = container_of(bo, struct amdgpu_bo, tbo); + abo = ttm_to_amdgpu_bo(bo); /* Remember that this BO was accessed by the CPU */ abo->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h index 39b6bf6..c26ef53 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h @@ -94,6 +94,11 @@ struct amdgpu_bo { }; }; +static inline struct amdgpu_bo *ttm_to_amdgpu_bo(struct ttm_buffer_object *tbo) +{ + return container_of(tbo, struct amdgpu_bo, tbo); +} + /** * amdgpu_mem_type_to_domain - return domain corresponding to mem_type * @mem_type: ttm memory type diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index b2b11e1..c9c059d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -44,6 +44,7 @@ #include #include #include "amdgpu.h" +#include "amdgpu_object.h" #include "amdgpu_trace.h" #include "bif/bif_4_1_d.h" @@ -209,7 +210,7 @@ static void amdgpu_evict_flags(struct ttm_buffer_object *bo, placement->num_busy_placement = 1; return; } - abo = container_of(bo, struct amdgpu_bo, tbo); + abo = ttm_to_amdgpu_bo(bo); switch (bo->mem.mem_type) { case TTM_PL_VRAM: if (adev->mman.buffer_funcs && @@ -257,7 +258,7 @@ static void amdgpu_evict_flags(struct ttm_buffer_object *bo, static int amdgpu_verify_access(struct ttm_buffer_object *bo, struct file *filp) { - struct amdgpu_bo *abo = container_of(bo, struct amdgpu_bo, tbo); + struct amdgpu_bo *abo = ttm_to_amdgpu_bo(bo); if (amdgpu_ttm_tt_get_usermm(bo->ttm)) return -EPERM; @@ -484,7 +485,7 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, int r; /* Can't move a pinned BO */ - abo = container_of(bo, struct amdgpu_bo, tbo); + abo = ttm_to_amdgpu_bo(bo); if (WARN_ON_ONCE(abo->pin_count > 0)) return -EINVAL; @@ -1172,7 +1173,7 @@ static int amdgpu_ttm_access_memory(struct ttm_buffer_object *bo, unsigned long offset, void *buf, int len, int write) { - struct amdgpu_bo *abo = container_of(bo, struct amdgpu_bo, tbo); + struct amdgpu_bo *abo = ttm_to_amdgpu_bo(bo); struct amdgpu_device *adev = amdgpu_ttm_adev(abo->tbo.bdev); struct drm_mm_node *nodes = abo->tbo.mem.mm_node; uint32_t value = 0; ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH 12/18] drm/amdgpu:use formal register to trigger hdp invalidate
Yeah, vnc1_0 and uvd_v7_0 Thanks -Original Message- From: Koenig, Christian Sent: 2017年9月18日 19:39 To: Liu, Monk ; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH 12/18] drm/amdgpu:use formal register to trigger hdp invalidate Yeah, but Vega10 has UVD7 and in uvd_v7_0.c we have: > static void uvd_v7_0_ring_emit_hdp_invalidate(struct amdgpu_ring > *ring) { > amdgpu_ring_write(ring, PACKET0(SOC15_REG_OFFSET(HDP, 0, > mmHDP_DEBUG0), 0)); > amdgpu_ring_write(ring, 1); > } That should probably be fixed as well. Regards, Christian. Am 18.09.2017 um 13:03 schrieb Liu, Monk: > Only vega10 has this register > > -Original Message- > From: Christian König [mailto:ckoenig.leichtzumer...@gmail.com] > Sent: 2017年9月18日 17:20 > To: Liu, Monk ; amd-gfx@lists.freedesktop.org > Subject: Re: [PATCH 12/18] drm/amdgpu:use formal register to trigger > hdp invalidate > > Am 18.09.2017 um 08:11 schrieb Monk Liu: >> Change-Id: I61dc02ea6a450f9acfa3bae07aa20244261f5369 >> Signed-off-by: Monk Liu > Reviewed-by: Christian König > > Please scan the code once more, we most likely have used mmHDP_DEBUG0 for > this at even more places. > > Christian. > >> --- >>drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 +- >>drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 2 +- >>2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >> index f201510..44960b3 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >> @@ -3549,7 +3549,7 @@ static void gfx_v9_0_ring_emit_hdp_flush(struct >> amdgpu_ring *ring) >>static void gfx_v9_0_ring_emit_hdp_invalidate(struct amdgpu_ring *ring) >>{ >> gfx_v9_0_write_data_to_reg(ring, 0, true, >> - SOC15_REG_OFFSET(HDP, 0, mmHDP_DEBUG0), 1); >> + SOC15_REG_OFFSET(HDP, 0, >> mmHDP_READ_CACHE_INVALIDATE), 1); >>} >> >>static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring, >> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c >> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c >> index fd7c72a..d5f3848 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c >> @@ -398,7 +398,7 @@ static void sdma_v4_0_ring_emit_hdp_invalidate(struct >> amdgpu_ring *ring) >>{ >> amdgpu_ring_write(ring, SDMA_PKT_HEADER_OP(SDMA_OP_SRBM_WRITE) | >>SDMA_PKT_SRBM_WRITE_HEADER_BYTE_EN(0xf)); >> -amdgpu_ring_write(ring, SOC15_REG_OFFSET(HDP, 0, mmHDP_DEBUG0)); >> +amdgpu_ring_write(ring, SOC15_REG_OFFSET(HDP, 0, >> +mmHDP_READ_CACHE_INVALIDATE)); >> amdgpu_ring_write(ring, 1); >>} >> > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH 13/18] drm/amdgpu:fix driver unloading bug
Christian, > That sounds at least a bit better. But my question is why doesn't this work > like it does on Tonga, e.g. correctly clean things up? Tonga also suffer with this issue, just that we fixed it in the branch for CSP customer and staging code usually behind our private branch ... > Yeah, gut keeping the GART mapping alive is complete nonsense. When the > driver unloads all memory should be returned to the OS. >So we either keep a GART mapping to pages which are about to be reused and >overwritten, or we leak memory on driver shutdown. >Neither options sounds very good, Unbinding the GART mapping makes CPC hang if it run MQD commands, and CPC must run MQD commands because RLCV always Requires CPC do that when RLCV doing SAVE_VF commands, Do you have way to fix above circle ? BR Monk -Original Message- From: Christian König [mailto:ckoenig.leichtzumer...@gmail.com] Sent: 2017年9月18日 19:54 To: Liu, Monk ; Koenig, Christian ; amd-gfx@lists.freedesktop.org Cc: Chen, Horace Subject: Re: [PATCH 13/18] drm/amdgpu:fix driver unloading bug Am 18.09.2017 um 12:12 schrieb Liu, Monk: > Christian, > > Let's discuss this patch and the one follows which skip the KIQ MQD free to > avoid SAVE_FAIL issue. > > > For skipping KIQ MQD deallocation patch, I think I will drop it and use a new > way: > We allocate KIQ MQD in VRAM domain and this BO can be safely freed after > driver unloaded, because after driver unloaded no one will change the data in > this BO *usually*. > e.g. some root app can map visible vram and alter the value in it That sounds at least a bit better. But my question is why doesn't this work like it does on Tonga, e.g. correctly clean things up? > > for this patch "to skipping unbind the GART mapping to keep KIQ MQD always > valid": > Since hypervisor side always have couple hw component working, and they rely > on GMC kept alive, so this is very different with BARE-METAL. That's to say > we can only do like this way. Yeah, gut keeping the GART mapping alive is complete nonsense. When the driver unloads all memory should be returned to the OS. So we either keep a GART mapping to pages which are about to be reused and overwritten, or we leak memory on driver shutdown. Neither options sounds very good, Christian. > > Besides, we'll have more patches in future for L1 secure mode, which > forbidden VF access GMC registers, so under L1 secure mode driver will > always skip GMC programing under SRIOV both in init and fini, but that > will come later > > BR Monk > > > > -Original Message- > From: Christian König [mailto:ckoenig.leichtzumer...@gmail.com] > Sent: 2017年9月18日 17:28 > To: Liu, Monk ; amd-gfx@lists.freedesktop.org > Cc: Chen, Horace > Subject: Re: [PATCH 13/18] drm/amdgpu:fix driver unloading bug > > Am 18.09.2017 um 08:11 schrieb Monk Liu: >> [SWDEV-126631] - fix hypervisor save_vf fail that occured after >> driver >> removed: >> 1. Because the KIQ and KCQ were not ummapped, save_vf will fail if driver >> freed mqd of KIQ and KCQ. >> 2. KIQ can't be unmapped since RLCV always need it, the bo_free on >> KIQ should be skipped 3. KCQ can be unmapped, and should be unmapped >> during hw_fini, 4. RLCV still need to access other mc address from some hw >> even after driver unloaded, >> So we should not unbind gart for VF. >> >> Change-Id: I320487a9a848f41484c5f8cc11be34aca807b424 >> Signed-off-by: Horace Chen >> Signed-off-by: Monk Liu > I absolutely can't judge if this is correct or not, but keeping the GART and > KIQ alive after the driver is unloaded sounds really fishy to me. > > Isn't there any other clean way of handling this? > > Christian. > >> --- >>drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 3 +- >>drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 5 +++ >>drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c| 60 >> +++- >>3 files changed, 66 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c >> index f437008..2fee071 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c >> @@ -394,7 +394,8 @@ int amdgpu_gart_init(struct amdgpu_device *adev) >> */ >>void amdgpu_gart_fini(struct amdgpu_device *adev) >>{ >> -if (adev->gart.ready) { >> +/* gart is still used by other hw under SRIOV, don't unbind it */ >> +if (adev->gart.ready && !amdgpu_sriov_vf(adev)) { >> /* unbind pages */ >> amdgpu_gart_unbind(adev, 0, adev->gart.num_cpu_pages); >> } >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c >> index 4f6c68f..bf6656f 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c >> @@ -309,6 +309,11 @@ void amdgpu_gfx_compute_mqd_sw_fini(struct >> amdgpu_device *adev) >>&ring->mqd_ptr); >>
RE: [PATCH 12/18] drm/amdgpu:use formal register to trigger hdp invalidate
Please answer my question as I raised in another thread, otherwise I will give a NAK on this! Regards, David Zhou -Original Message- From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of Liu, Monk Sent: Tuesday, September 19, 2017 12:04 PM To: Koenig, Christian ; amd-gfx@lists.freedesktop.org Subject: RE: [PATCH 12/18] drm/amdgpu:use formal register to trigger hdp invalidate Yeah, vnc1_0 and uvd_v7_0 Thanks -Original Message- From: Koenig, Christian Sent: 2017年9月18日 19:39 To: Liu, Monk ; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH 12/18] drm/amdgpu:use formal register to trigger hdp invalidate Yeah, but Vega10 has UVD7 and in uvd_v7_0.c we have: > static void uvd_v7_0_ring_emit_hdp_invalidate(struct amdgpu_ring > *ring) { > amdgpu_ring_write(ring, PACKET0(SOC15_REG_OFFSET(HDP, 0, > mmHDP_DEBUG0), 0)); > amdgpu_ring_write(ring, 1); > } That should probably be fixed as well. Regards, Christian. Am 18.09.2017 um 13:03 schrieb Liu, Monk: > Only vega10 has this register > > -Original Message- > From: Christian König [mailto:ckoenig.leichtzumer...@gmail.com] > Sent: 2017年9月18日 17:20 > To: Liu, Monk ; amd-gfx@lists.freedesktop.org > Subject: Re: [PATCH 12/18] drm/amdgpu:use formal register to trigger > hdp invalidate > > Am 18.09.2017 um 08:11 schrieb Monk Liu: >> Change-Id: I61dc02ea6a450f9acfa3bae07aa20244261f5369 >> Signed-off-by: Monk Liu > Reviewed-by: Christian König > > Please scan the code once more, we most likely have used mmHDP_DEBUG0 for > this at even more places. > > Christian. > >> --- >>drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 +- >>drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 2 +- >>2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >> index f201510..44960b3 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >> @@ -3549,7 +3549,7 @@ static void gfx_v9_0_ring_emit_hdp_flush(struct >> amdgpu_ring *ring) >>static void gfx_v9_0_ring_emit_hdp_invalidate(struct amdgpu_ring *ring) >>{ >> gfx_v9_0_write_data_to_reg(ring, 0, true, >> - SOC15_REG_OFFSET(HDP, 0, mmHDP_DEBUG0), 1); >> + SOC15_REG_OFFSET(HDP, 0, >> mmHDP_READ_CACHE_INVALIDATE), 1); >>} >> >>static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring, >> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c >> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c >> index fd7c72a..d5f3848 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c >> @@ -398,7 +398,7 @@ static void sdma_v4_0_ring_emit_hdp_invalidate(struct >> amdgpu_ring *ring) >>{ >> amdgpu_ring_write(ring, SDMA_PKT_HEADER_OP(SDMA_OP_SRBM_WRITE) | >>SDMA_PKT_SRBM_WRITE_HEADER_BYTE_EN(0xf)); >> -amdgpu_ring_write(ring, SOC15_REG_OFFSET(HDP, 0, mmHDP_DEBUG0)); >> +amdgpu_ring_write(ring, SOC15_REG_OFFSET(HDP, 0, >> +mmHDP_READ_CACHE_INVALIDATE)); >> amdgpu_ring_write(ring, 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
[PATCH 1/3] drm/amdgpu:make ctx_add_fence interruptible(v2)
otherwise a gpu hang will make application couldn't be killed under timedout=0 mode v2: Fix memoryleak job/job->s_fence issue unlock mn remove the ERROR msg after waiting being interrupted Change-Id: I6051b5b3ae1188983f49325a2438c84a6c12374a Signed-off-by: Monk Liu --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 4 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 17 +++-- drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 12 +++- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index cc9a232..6ff2959 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -736,8 +736,8 @@ struct amdgpu_ctx_mgr { struct amdgpu_ctx *amdgpu_ctx_get(struct amdgpu_fpriv *fpriv, uint32_t id); int amdgpu_ctx_put(struct amdgpu_ctx *ctx); -uint64_t amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring, - struct dma_fence *fence); +int amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring, + struct dma_fence *fence, uint64_t *seq); struct dma_fence *amdgpu_ctx_get_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring, uint64_t seq); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index b59749d..9bd4834 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -1043,6 +1043,8 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, struct amd_sched_entity *entity = &p->ctx->rings[ring->idx].entity; struct amdgpu_job *job; unsigned i; + uint64_t seq; + int r; amdgpu_mn_lock(p->mn); @@ -1071,8 +1073,19 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, job->owner = p->filp; job->fence_ctx = entity->fence_context; p->fence = dma_fence_get(&job->base.s_fence->finished); - cs->out.handle = amdgpu_ctx_add_fence(p->ctx, ring, p->fence); - job->uf_sequence = cs->out.handle; + r = amdgpu_ctx_add_fence(p->ctx, ring, p->fence, &seq); + if (r) { + /* release job include the sched fence as well */ + dma_fence_put(&job->base.s_fence->finished); + dma_fence_put(&job->base.s_fence->scheduled); + amdgpu_job_free(job); + amdgpu_mn_unlock(p->mn); + dma_fence_put(p->fence); + return r; + } + + cs->out.handle = seq; + job->uf_sequence = seq; amdgpu_job_free_resources(job); trace_amdgpu_cs_ioctl(job); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c index a11e443..551f114 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c @@ -246,8 +246,8 @@ int amdgpu_ctx_put(struct amdgpu_ctx *ctx) return 0; } -uint64_t amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring, - struct dma_fence *fence) +int amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring, + struct dma_fence *fence, uint64_t* handler) { struct amdgpu_ctx_ring *cring = & ctx->rings[ring->idx]; uint64_t seq = cring->sequence; @@ -258,9 +258,9 @@ uint64_t amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring, other = cring->fences[idx]; if (other) { signed long r; - r = dma_fence_wait_timeout(other, false, MAX_SCHEDULE_TIMEOUT); + r = dma_fence_wait_timeout(other, true, MAX_SCHEDULE_TIMEOUT); if (r < 0) - DRM_ERROR("Error (%ld) waiting for fence!\n", r); + return -ERESTARTSYS; } dma_fence_get(fence); @@ -271,8 +271,10 @@ uint64_t amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring, spin_unlock(&ctx->ring_lock); dma_fence_put(other); + if (handler) + *handler = seq; - return seq; + return 0; } struct dma_fence *amdgpu_ctx_get_fence(struct amdgpu_ctx *ctx, -- 2.7.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 2/3] drm/amdgpu/sriov:fix memory leak after gpu reset
GPU reset will require all hw doing hw_init thus ucode_init_bo will be invoked again, which lead to memory leak skip the fw_buf allocation during sriov gpu reset to avoid memory leak. Change-Id: I31131eda1bd45ea2f5bdc50c5da5fc5a9fe9027d Signed-off-by: Monk Liu --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 64 +++ 2 files changed, 35 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 6ff2959..3d0c633 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1185,6 +1185,9 @@ struct amdgpu_firmware { /* gpu info firmware data pointer */ const struct firmware *gpu_info_fw; + + void *fw_buf_ptr; + uint64_t fw_buf_mc; }; /* diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c index f306374..6564902 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c @@ -360,8 +360,6 @@ static int amdgpu_ucode_patch_jt(struct amdgpu_firmware_info *ucode, int amdgpu_ucode_init_bo(struct amdgpu_device *adev) { struct amdgpu_bo **bo = &adev->firmware.fw_buf; - uint64_t fw_mc_addr; - void *fw_buf_ptr = NULL; uint64_t fw_offset = 0; int i, err; struct amdgpu_firmware_info *ucode = NULL; @@ -372,37 +370,39 @@ int amdgpu_ucode_init_bo(struct amdgpu_device *adev) return 0; } - err = amdgpu_bo_create(adev, adev->firmware.fw_size, PAGE_SIZE, true, - amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT, - AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS, - NULL, NULL, 0, bo); - if (err) { - dev_err(adev->dev, "(%d) Firmware buffer allocate failed\n", err); - goto failed; - } + if (!amdgpu_sriov_vf(adev) || !adev->in_sriov_reset) { + err = amdgpu_bo_create(adev, adev->firmware.fw_size, PAGE_SIZE, true, + amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT, + AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS, + NULL, NULL, 0, bo); + if (err) { + dev_err(adev->dev, "(%d) Firmware buffer allocate failed\n", err); + goto failed; + } - err = amdgpu_bo_reserve(*bo, false); - if (err) { - dev_err(adev->dev, "(%d) Firmware buffer reserve failed\n", err); - goto failed_reserve; - } + err = amdgpu_bo_reserve(*bo, false); + if (err) { + dev_err(adev->dev, "(%d) Firmware buffer reserve failed\n", err); + goto failed_reserve; + } - err = amdgpu_bo_pin(*bo, amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT, - &fw_mc_addr); - if (err) { - dev_err(adev->dev, "(%d) Firmware buffer pin failed\n", err); - goto failed_pin; - } + err = amdgpu_bo_pin(*bo, amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT, + &adev->firmware.fw_buf_mc); + if (err) { + dev_err(adev->dev, "(%d) Firmware buffer pin failed\n", err); + goto failed_pin; + } - err = amdgpu_bo_kmap(*bo, &fw_buf_ptr); - if (err) { - dev_err(adev->dev, "(%d) Firmware buffer kmap failed\n", err); - goto failed_kmap; - } + err = amdgpu_bo_kmap(*bo, &adev->firmware.fw_buf_ptr); + if (err) { + dev_err(adev->dev, "(%d) Firmware buffer kmap failed\n", err); + goto failed_kmap; + } - amdgpu_bo_unreserve(*bo); + amdgpu_bo_unreserve(*bo); + } - memset(fw_buf_ptr, 0, adev->firmware.fw_size); + memset(adev->firmware.fw_buf_ptr, 0, adev->firmware.fw_size); /* * if SMU loaded firmware, it needn't add SMC, UVD, and VCE @@ -421,14 +421,14 @@ int amdgpu_ucode_init_bo(struct amdgpu_device *adev) ucode = &adev->firmware.ucode[i]; if (ucode->fw) { header = (const struct common_firmware_header *)ucode->fw->data; - amdgpu_ucode_init_single_fw(adev, ucode, fw_mc_addr + fw_offset, - (void *)((uint8_t *)fw_buf_ptr + fw_offset)); + amdgpu_ucode_init_single_fw(adev, ucode, adev->firmware.fw_buf_mc + fw_offset, + adev->firmwa
[PATCH 3/3] drm/amdgpu:fix typo
previously a patch has typo error, correct it Change-Id: I91bcefd7148b5db1c7d957c868e13a46ca40ef74 Signed-off-by: Monk Liu --- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c index 4eee2ef..35cc5ee 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c @@ -256,7 +256,7 @@ static int psp_hw_start(struct psp_context *psp) struct amdgpu_device *adev = psp->adev; int ret; - if (amdgpu_sriov_vf(adev) && !adev->in_sriov_reset) { + if (!amdgpu_sriov_vf(adev) || !adev->in_sriov_reset) { ret = psp_bootloader_load_sysdrv(psp); if (ret) return ret; -- 2.7.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH 12/18] drm/amdgpu:use formal register to trigger hdp invalidate
What question ? please reply here -Original Message- From: Zhou, David(ChunMing) Sent: 2017年9月19日 12:25 To: Liu, Monk ; Koenig, Christian ; amd-gfx@lists.freedesktop.org Subject: RE: [PATCH 12/18] drm/amdgpu:use formal register to trigger hdp invalidate Please answer my question as I raised in another thread, otherwise I will give a NAK on this! Regards, David Zhou -Original Message- From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of Liu, Monk Sent: Tuesday, September 19, 2017 12:04 PM To: Koenig, Christian ; amd-gfx@lists.freedesktop.org Subject: RE: [PATCH 12/18] drm/amdgpu:use formal register to trigger hdp invalidate Yeah, vnc1_0 and uvd_v7_0 Thanks -Original Message- From: Koenig, Christian Sent: 2017年9月18日 19:39 To: Liu, Monk ; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH 12/18] drm/amdgpu:use formal register to trigger hdp invalidate Yeah, but Vega10 has UVD7 and in uvd_v7_0.c we have: > static void uvd_v7_0_ring_emit_hdp_invalidate(struct amdgpu_ring > *ring) { > amdgpu_ring_write(ring, PACKET0(SOC15_REG_OFFSET(HDP, 0, > mmHDP_DEBUG0), 0)); > amdgpu_ring_write(ring, 1); > } That should probably be fixed as well. Regards, Christian. Am 18.09.2017 um 13:03 schrieb Liu, Monk: > Only vega10 has this register > > -Original Message- > From: Christian König [mailto:ckoenig.leichtzumer...@gmail.com] > Sent: 2017年9月18日 17:20 > To: Liu, Monk ; amd-gfx@lists.freedesktop.org > Subject: Re: [PATCH 12/18] drm/amdgpu:use formal register to trigger > hdp invalidate > > Am 18.09.2017 um 08:11 schrieb Monk Liu: >> Change-Id: I61dc02ea6a450f9acfa3bae07aa20244261f5369 >> Signed-off-by: Monk Liu > Reviewed-by: Christian König > > Please scan the code once more, we most likely have used mmHDP_DEBUG0 for > this at even more places. > > Christian. > >> --- >>drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 +- >>drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 2 +- >>2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >> index f201510..44960b3 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >> @@ -3549,7 +3549,7 @@ static void gfx_v9_0_ring_emit_hdp_flush(struct >> amdgpu_ring *ring) >>static void gfx_v9_0_ring_emit_hdp_invalidate(struct amdgpu_ring *ring) >>{ >> gfx_v9_0_write_data_to_reg(ring, 0, true, >> - SOC15_REG_OFFSET(HDP, 0, mmHDP_DEBUG0), 1); >> + SOC15_REG_OFFSET(HDP, 0, >> mmHDP_READ_CACHE_INVALIDATE), 1); >>} >> >>static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring, >> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c >> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c >> index fd7c72a..d5f3848 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c >> @@ -398,7 +398,7 @@ static void sdma_v4_0_ring_emit_hdp_invalidate(struct >> amdgpu_ring *ring) >>{ >> amdgpu_ring_write(ring, SDMA_PKT_HEADER_OP(SDMA_OP_SRBM_WRITE) | >>SDMA_PKT_SRBM_WRITE_HEADER_BYTE_EN(0xf)); >> -amdgpu_ring_write(ring, SOC15_REG_OFFSET(HDP, 0, mmHDP_DEBUG0)); >> +amdgpu_ring_write(ring, SOC15_REG_OFFSET(HDP, 0, >> +mmHDP_READ_CACHE_INVALIDATE)); >> amdgpu_ring_write(ring, 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
Re: [PATCH 12/18] drm/amdgpu:use formal register to trigger hdp invalidate
Seems the change is more proper, but where do you find mmHDP_READ_CACHE_INVALIDATE? Could you double check if Windows driver has changed to use this? I'm confusing it, since mmHDP_DEBUG0 implementation is from windows as well. I even don't find mmHDP_READ_CACHE_INVALIDATE in register spec. Regards, David Zhou On 2017年09月19日 14:46, Liu, Monk wrote: What question ? please reply here -Original Message- From: Zhou, David(ChunMing) Sent: 2017年9月19日 12:25 To: Liu, Monk ; Koenig, Christian ; amd-gfx@lists.freedesktop.org Subject: RE: [PATCH 12/18] drm/amdgpu:use formal register to trigger hdp invalidate Please answer my question as I raised in another thread, otherwise I will give a NAK on this! Regards, David Zhou -Original Message- From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of Liu, Monk Sent: Tuesday, September 19, 2017 12:04 PM To: Koenig, Christian ; amd-gfx@lists.freedesktop.org Subject: RE: [PATCH 12/18] drm/amdgpu:use formal register to trigger hdp invalidate Yeah, vnc1_0 and uvd_v7_0 Thanks -Original Message- From: Koenig, Christian Sent: 2017年9月18日 19:39 To: Liu, Monk ; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH 12/18] drm/amdgpu:use formal register to trigger hdp invalidate Yeah, but Vega10 has UVD7 and in uvd_v7_0.c we have: static void uvd_v7_0_ring_emit_hdp_invalidate(struct amdgpu_ring *ring) { amdgpu_ring_write(ring, PACKET0(SOC15_REG_OFFSET(HDP, 0, mmHDP_DEBUG0), 0)); amdgpu_ring_write(ring, 1); } That should probably be fixed as well. Regards, Christian. Am 18.09.2017 um 13:03 schrieb Liu, Monk: Only vega10 has this register -Original Message- From: Christian König [mailto:ckoenig.leichtzumer...@gmail.com] Sent: 2017年9月18日 17:20 To: Liu, Monk ; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH 12/18] drm/amdgpu:use formal register to trigger hdp invalidate Am 18.09.2017 um 08:11 schrieb Monk Liu: Change-Id: I61dc02ea6a450f9acfa3bae07aa20244261f5369 Signed-off-by: Monk Liu Reviewed-by: Christian König Please scan the code once more, we most likely have used mmHDP_DEBUG0 for this at even more places. Christian. --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 +- drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index f201510..44960b3 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -3549,7 +3549,7 @@ static void gfx_v9_0_ring_emit_hdp_flush(struct amdgpu_ring *ring) static void gfx_v9_0_ring_emit_hdp_invalidate(struct amdgpu_ring *ring) { gfx_v9_0_write_data_to_reg(ring, 0, true, - SOC15_REG_OFFSET(HDP, 0, mmHDP_DEBUG0), 1); + SOC15_REG_OFFSET(HDP, 0, mmHDP_READ_CACHE_INVALIDATE), 1); } static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring, diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c index fd7c72a..d5f3848 100644 --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c @@ -398,7 +398,7 @@ static void sdma_v4_0_ring_emit_hdp_invalidate(struct amdgpu_ring *ring) { amdgpu_ring_write(ring, SDMA_PKT_HEADER_OP(SDMA_OP_SRBM_WRITE) | SDMA_PKT_SRBM_WRITE_HEADER_BYTE_EN(0xf)); - amdgpu_ring_write(ring, SOC15_REG_OFFSET(HDP, 0, mmHDP_DEBUG0)); + amdgpu_ring_write(ring, SOC15_REG_OFFSET(HDP, 0, +mmHDP_READ_CACHE_INVALIDATE)); amdgpu_ring_write(ring, 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