[PATCH 2/2 v4] drm/exynos: added userptr feature.

[Inki Dae]

> -----Original Message-----
> From: KOSAKI Motohiro [mailto:kosaki.motohiro at gmail.com]
> Sent: Monday, May 14, 2012 3:33 PM
> To: Inki Dae
> Cc: airlied at linux.ie; dri-devel at lists.freedesktop.org; j.glisse at 
> gmail.com;
> minchan at kernel.org; kyungmin.park at samsung.com; sw0312.kim at 
> samsung.com;
> jy0922.shim at samsung.com
> Subject: Re: [PATCH 2/2 v4] drm/exynos: added userptr feature.
> 
> > + ? ? ? npages = buf->size >> PAGE_SHIFT;
> 
> Why round down? usually we use round up.
> 

The size was already rounded up by exynos_drm_gem_userptr_ioctl so this is
just used to get page count.

> 
> > + ? ? ? down_read(&current->mm->mmap_sem);
> > +
> > + ? ? ? /*
> > + ? ? ? ?* Basically, all the pages from get_user_pages() can not be not
> only
> > + ? ? ? ?* migrated by CMA but also swapped out.
> > + ? ? ? ?*
> > + ? ? ? ?* The migration issue.
> > + ? ? ? ?* - Pages reserved by CMA for some device using DMA could be
> used by
> > + ? ? ? ?* kernel and if the device driver wants to use those pages
> > + ? ? ? ?* while being used by kernel then the pages are copied into
> > + ? ? ? ?* other ones allocated to migrate them and then finally,
> > + ? ? ? ?* the device driver can use the pages for itself.
> > + ? ? ? ?* Thus, migrated, the pages being accessed by DMA could be
> changed
> > + ? ? ? ?* to other so this situation may incur that DMA accesses any
> pages
> > + ? ? ? ?* it doesn't want.
> > + ? ? ? ?*
> > + ? ? ? ?* But the use of get_user_pages is safe from such magration
> issue
> > + ? ? ? ?* because all the pages from get_user_pages CAN NOT be not only
> > + ? ? ? ?* migrated, but also swapped out.
> > + ? ? ? ?*/
> > + ? ? ? get_npages = get_user_pages(current, current->mm, userptr,
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? npages, write, 1, buf->pages,
NULL);
> 
> Why force=1? It is almostly core-dump specific option. Why don't you
> return

I know that force indicates whether to force write access even  if user
mapping is readonly. so we just want to use pages from get_user_pages as
read/write permission.

> EFAULT when the page has write permission. IOW, Why your Xorg module
> don't map memory w/ PROT_WRITE?

No, Xorg can map memory w/ PROT_WRITE. Couldn't the Xorg map w/ PROT_WRITE
if force = 1? plz, let me know if there is my missing point.

Thanks,
Inki Dae

> 
> 
> > + ? ? ? up_read(&current->mm->mmap_sem);
> > + ? ? ? if (get_npages != npages)
> > + ? ? ? ? ? ? ? DRM_ERROR("failed to get user_pages.\n");