Hi Christian,
> > + if (likely(amdgpu_bo_reserve(*bo, false) == 0)) {
> > + amdgpu_bo_kunmap(*bo);
>
> We only kmap it in amdgpu_bo_create_kernel() when there was a CPU address
> given.
>
> I think we should mirror that here or otherwise might run into problems
> calling
> kunmap() more often than kmap().
Thanks for your comments.
I consider it too when creating the patch.
Actually looking into the amdgpu_bo_kunmap(), it will check if the bo is really
mapped by if(bo->kptr == NULL).
But with the judgment of cpu address, it looks obvious in code.
I will update it and commit these patches.
Regards,
Jerry (Junwei Zhang)
SRDC SW Development
AMD Shanghai
_____________________________________
> -----Original Message-----
> From: Christian König [mailto:[email protected]]
> Sent: Thursday, September 08, 2016 0:19
> To: Zhang, Jerry; [email protected]
> Subject: Re: [PATCH 1/2] drm/amdgpu: add a new helper to free a BO in kernel
> allocations
>
> Am 07.09.2016 um 16:43 schrieb Junwei Zhang:
> > Free the BO allocated by amdgpu_bo_create_kernel()
> >
> > Change-Id: I1c7248cfb00bcc51fb86d89760522ca36095cdd2
> > Signed-off-by: Junwei Zhang <[email protected]>
>
> Really nice cleanup, but one comment below.
>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 26
> ++++++++++++++++++++++++++
> > drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 2 ++
> > 2 files changed, 28 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > index 53e4ac5..81dd4f1 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > @@ -299,6 +299,32 @@ error_free:
> > return r;
> > }
> >
> > +/**
> > + * amdgpu_bo_free_kernel - free BO for kernel use
> > + *
> > + * @bo: amdgpu BO to free
> > + *
> > + * unmaps and unpin a BO for kernel internal use.
> > + */
> > +void amdgpu_bo_free_kernel(struct amdgpu_bo **bo, u64 *gpu_addr,
> > + void **cpu_addr)
> > +{
> > + if (*bo == NULL)
> > + return;
> > +
> > + if (likely(amdgpu_bo_reserve(*bo, false) == 0)) {
> > + amdgpu_bo_kunmap(*bo);
>
> We only kmap it in amdgpu_bo_create_kernel() when there was a CPU address
> given.
>
> I think we should mirror that here or otherwise might run into problems
> calling
> kunmap() more often than kmap().
>
> With that fixed both patches are Reviewed-by: Christian König
> <[email protected]>.
>
> Regards,
> Christian.
>
> > + amdgpu_bo_unpin(*bo);
> > + amdgpu_bo_unreserve(*bo);
> > + }
> > + amdgpu_bo_unref(bo);
> > +
> > + if (gpu_addr)
> > + *gpu_addr = 0;
> > + if (cpu_addr)
> > + *cpu_addr = NULL;
> > +}
> > +
> > int amdgpu_bo_create_restricted(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_object.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> > index 72be7d0..c3459fc 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> > @@ -132,6 +132,8 @@ int amdgpu_bo_create_kernel(struct amdgpu_device
> *adev,
> > unsigned long size, int align,
> > u32 domain, struct amdgpu_bo **bo_ptr,
> > u64 *gpu_addr, void **cpu_addr);
> > +void amdgpu_bo_free_kernel(struct amdgpu_bo **bo, u64 *gpu_addr,
> > + void **cpu_addr);
> > int amdgpu_bo_kmap(struct amdgpu_bo *bo, void **ptr);
> > void amdgpu_bo_kunmap(struct amdgpu_bo *bo);
> > struct amdgpu_bo *amdgpu_bo_ref(struct amdgpu_bo *bo);
>
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
