Am 04.11.2016 um 10:22 schrieb Flora Cui:
On Fri, Nov 04, 2016 at 09:52:16AM +0100, Christian König wrote:
Well clearly a NAK on this, why do you think this is just dummy code?
Am 04.11.2016 um 08:32 schrieb Flora Cui:
Change-Id: I3238a2520c9161b1c2f9cf176158bdeb9cb21cbb
Signed-off-by: Flora Cui <flora....@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 15 +--------------
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 4 ----
2 files changed, 1 insertion(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 4068504..be2ad79 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -917,13 +917,6 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev,
memcpy(ib->ptr, kptr, chunk_ib->ib_bytes);
amdgpu_bo_kunmap(aobj);
- } else {
- r = amdgpu_ib_get(adev, vm, 0, ib);
- if (r) {
- DRM_ERROR("Failed to get ib !\n");
- return r;
- }
-
This is the IB allocation and initialization for the VM case and vital to
command submission.
flora: amdgpu_ib_get do nothing with param size=0
That's clearly a bug. The function should at least clear the structure
members.
Christian.
}
ib->gpu_addr = chunk_ib->va_start;
@@ -1008,21 +1001,15 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
job = p->job;
p->job = NULL;
- r = amd_sched_job_init(&job->base, &ring->sched, entity, p->filp);
+ r = amdgpu_job_submit(job, ring, entity, p->filp, &p->fence);
if (r) {
amdgpu_job_free(job);
return r;
}
- job->owner = p->filp;
- job->fence_ctx = entity->fence_context;
- p->fence = 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;
- amdgpu_job_free_resources(job);
-
trace_amdgpu_cs_ioctl(job);
- amd_sched_entity_push_job(&job->base);
This results in a race condition because the job might already be freed
after amdgpu_job_submit() succeeded.
flora: OK. I'll drop this.
return 0;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index d6c2839..c6448e1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -960,10 +960,6 @@ static int amdgpu_vm_bo_update_mapping(struct
amdgpu_device *adev,
ring = container_of(vm->entity.sched, struct amdgpu_ring, sched);
- memset(¶ms, 0, sizeof(params));
- params.adev = adev;
- params.src = src;
-
This indeed looks like a duplicate, probably a leftover merge issue.
Regards,
Christian.
/* sync to everything on unmapping */
if (!(flags & AMDGPU_PTE_VALID))
owner = AMDGPU_FENCE_OWNER_UNDEFINED;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
