From mboxrd@z Thu Jan 1 00:00:00 1970 From: InKi Dae Subject: Re: [PATCH v2] DRM: add drm gem cma helper Date: Tue, 5 Jun 2012 22:54:10 +0900 Message-ID: References: <1338451734-20232-1-git-send-email-s.hauer@pengutronix.de> <2093871.VgT5F32Ubz@avalon> <20120605075621.GP30400@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-pb0-f49.google.com (mail-pb0-f49.google.com [209.85.160.49]) by gabe.freedesktop.org (Postfix) with ESMTP id 314259E861 for ; Tue, 5 Jun 2012 06:54:11 -0700 (PDT) Received: by pbbrq13 with SMTP id rq13so8114902pbb.36 for ; Tue, 05 Jun 2012 06:54:11 -0700 (PDT) In-Reply-To: <20120605075621.GP30400@pengutronix.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Sascha Hauer Cc: Laurent Pinchart , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org 2012/6/5 Sascha Hauer : > On Fri, Jun 01, 2012 at 12:29:47AM +0900, InKi Dae wrote: >> Hi Sascha, >> >> >> +struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm, >> >> + =A0 =A0 =A0 =A0 =A0 =A0 unsigned int size) >> >> +{ >> >> + =A0 =A0 struct drm_gem_cma_object *cma_obj; >> >> + =A0 =A0 struct drm_gem_object *gem_obj; >> >> + =A0 =A0 int ret; >> >> + >> >> + =A0 =A0 size =3D round_up(size, PAGE_SIZE); >> >> + >> >> + =A0 =A0 cma_obj =3D kzalloc(sizeof(*cma_obj), GFP_KERNEL); >> >> + =A0 =A0 if (!cma_obj) >> >> + =A0 =A0 =A0 =A0 =A0 =A0 return ERR_PTR(-ENOMEM); >> >> + >> >> + =A0 =A0 cma_obj->vaddr =3D dma_alloc_writecombine(drm->dev, size, >> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 &cma_obj->paddr, GFP_KERNEL= | __GFP_NOWARN); >> >> how about calling drm_gem_cma_buf_allocate() instead of >> dma_alloc_wirtecombine() for consistency in using dma api so its call >> flow would be "dma_gem_cma_buf_allocate() -> >> dma_alloc_writecombine()". > > What do you mean? There is no drm_gem_cma_buf_allocate() function. > I mean have a pair with drm_gem_cma_buf_destroy() otherwise how about removing dem_gem_cma_buf_destroy() and calling dma_free_writecombine() directly? drm_gem_cma_buf_destroy() is just a wrapper to dma_free_writecombine(). >> >> + =A0 =A0 struct drm_gem_object *gem_obj; >> >> + =A0 =A0 struct drm_gem_cma_object *cma_obj; >> >> + =A0 =A0 int ret; >> >> + >> >> + =A0 =A0 ret =3D drm_gem_mmap(filp, vma); >> >> + =A0 =A0 if (ret) >> >> + =A0 =A0 =A0 =A0 =A0 =A0 return ret; >> >> + >> >> + =A0 =A0 gem_obj =3D vma->vm_private_data; >> >> + =A0 =A0 cma_obj =3D to_drm_gem_cma_obj(gem_obj); >> >> it's likely to need checking if user space size is valid or not here. >> like this; >> if (vma->end - vma->start > gem_obj->size) { >> =A0 =A0 =A0 =A0 =A0 error message; >> =A0 =A0 =A0 =A0 =A0and some exceptions; >> } > > This is already done in drm_gem_mmap(). > > Sascha > > -- > Pengutronix e.K. =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 | = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 | > Industrial Linux Solutions =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 | http://www.p= engutronix.de/ =A0| > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 =A0= =A0| > Amtsgericht Hildesheim, HRA 2686 =A0 =A0 =A0 =A0 =A0 | Fax: =A0 +49-5121-= 206917-5555 |