Am 04.11.2016 um 11:49 schrieb Christian König:
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.
On the other hand we zero clear the IB array now anyway.
So what you could do is remove the size check from amdgpu_ib_get() and
then drop this code as well.
Regards,
Christian.
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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
