Re: [PATCH] drm/amdgpu: remove dummy codes

[Flora Cui <flora.cui-5C7GfCeVMHo@public.gmane.org>]

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.Cui@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
> 
> >  		}
> >  		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(&params, 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