Re: [PATCH v2] DRM: add drm gem cma helper

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,

• unsigned int size)

+{

• struct drm_gem_cma_object *cma_obj;
• struct drm_gem_object *gem_obj;
• int ret;
• size = round_up(size, PAGE_SIZE);
• cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
• if (!cma_obj)
• return ERR_PTR(-ENOMEM);
• cma_obj->vaddr = dma_alloc_writecombine(drm->dev, size,
• &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.

...

...

• struct drm_gem_object *gem_obj;
• struct drm_gem_cma_object *cma_obj;
• int ret;
• ret = drm_gem_mmap(filp, vma);
• if (ret)
• return ret;
• gem_obj = vma->vm_private_data;
• cma_obj = 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) { error message; and some exceptions; }

This is already done in drm_gem_mmap().

Sascha