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.Cui at 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. > } > > 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. > > 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;