RE: [PATCH 10/10] drm/exynos: consider memory releasing to exported gem buffer into dmabuf

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Rob,

> -----Original Message-----
> From: robdclark@xxxxxxxxx [mailto:robdclark@xxxxxxxxx] On Behalf Of Rob
> Clark
> Sent: Wednesday, June 27, 2012 9:20 PM
> To: Inki Dae
> Cc: airlied@xxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx;
> kyungmin.park@xxxxxxxxxxx; sw0312.kim@xxxxxxxxxxx
> Subject: Re: [PATCH 10/10] drm/exynos: consider memory releasing to
> exported gem buffer into dmabuf
> 
> On Wed, Jun 27, 2012 at 3:03 AM, Inki Dae <inki.dae@xxxxxxxxxxx> wrote:
> > exported gem buffer into dmabuf should be released when a gem relese is
> > requested by user. with dma_buf_put() call, if file->f_count is 0 then
> > a release callback of exynos gem module(exporter) will be called to
> release
> > its own gem buffer.
> >
> > Signed-off-by: Inki Dae <inki.dae@xxxxxxxxxxx>
> > Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> > ---
> >  drivers/gpu/drm/exynos/exynos_drm_drv.c |    1 +
> >  drivers/gpu/drm/exynos/exynos_drm_gem.c |   16 ++++++++++++++++
> >  drivers/gpu/drm/exynos/exynos_drm_gem.h |    4 ++++
> >  3 files changed, 21 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> > index d6de2e0..1501dd2 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> > @@ -258,6 +258,7 @@ static struct drm_driver exynos_drm_driver = {
> >        .gem_init_object        = exynos_drm_gem_init_object,
> >        .gem_free_object        = exynos_drm_gem_free_object,
> >        .gem_vm_ops             = &exynos_drm_gem_vm_ops,
> > +       .gem_close_object       = exynos_drm_gem_close_object,
> >        .dumb_create            = exynos_drm_gem_dumb_create,
> >        .dumb_map_offset        = exynos_drm_gem_dumb_map_offset,
> >        .dumb_destroy           = exynos_drm_gem_dumb_destroy,
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> > index 411d82b..5ca8641 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> > @@ -27,6 +27,7 @@
> >  #include "drm.h"
> >
> >  #include <linux/shmem_fs.h>
> > +#include <linux/dma-buf.h>
> >  #include <drm/exynos_drm.h>
> >
> >  #include "exynos_drm_drv.h"
> > @@ -749,6 +750,21 @@ int exynos_drm_gem_dumb_destroy(struct drm_file
> *file_priv,
> >        return 0;
> >  }
> >
> > +void exynos_drm_gem_close_object(struct drm_gem_object *obj,
> > +                               struct drm_file *file)
> > +{
> > +       DRM_DEBUG_KMS("%s\n", __FILE__);
> > +
> > +       /*
> > +        * exported dmabuf should be released when a gem is released by
> user.
> > +        * with dma_buf_put() call, if file->f_count is 0 then a release
> > +        * callback of gem module(exporter) will be called to release
> > +        * its own gem buffer.
> > +        */
> > +       if (obj->export_dma_buf)
> > +               dma_buf_put(obj->export_dma_buf);
> 
> this doesn't seem quite right to me..  although I think the dmabuf
> release fxn needs to NULL out the obj->export_dma_buf.
> 
> If your GEM obj is getting released before your dmabuf then there is
> something going wrong with the ref cnt'ing.
> 
> BR,
> -R
> 

Could you look into below steps? I understood gem and dmabuf life cycle like below so thought we needs this patch. with this patch, the gem object isn't getting released before dmabuf and the gem will be released by dma_buf_put(). if there is my missing point then please give me any comment.

Reference count(step number)
====================================
gem object1   gem object2       dmabuf
------------------------------------
    1(1)
    2(2)                               1(2)
                     1(3)              2(3)
                     0(4)              1(4)
    1(5)                               0(5)
    0(5)
====================================

1. create gem1
2. export the gem1 into dmabuf
3. import the dmabuf into gem2
4. close gem2
5. close gem1


Thanks,
Inki Dae

> > +}
> > +
> >  int exynos_drm_gem_fault(struct vm_area_struct *vma, struct vm_fault
> *vmf)
> >  {
> >        struct drm_gem_object *obj = vma->vm_private_data;
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h
> b/drivers/gpu/drm/exynos/exynos_drm_gem.h
> > index 14d038b..4f1ba1a 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
> > @@ -162,4 +162,8 @@ int exynos_drm_gem_fault(struct vm_area_struct *vma,
> struct vm_fault *vmf);
> >  /* set vm_flags and we can change the vm attribute to other one at here.
> */
> >  int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
> >
> > +/* do extra release exynos gem module needs when gem close is called.
> */
> > +void exynos_drm_gem_close_object(struct drm_gem_object *obj,
> > +                               struct drm_file *file);
> > +
> >  #endif
> > --
> > 1.7.4.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux