Re: [PATCH 9/9] drm/exynos: Convert g2d_userptr_get_dma_addr() to use get_vaddr_frames()

[Jan Kara <jack@suse.cz>]

[-- Attachment #1: Type: text/plain, Size: 2255 bytes --]

On Sat 18-07-15 12:14:12, Inki Dae wrote:
> On 2015e?? 07i?? 17i? 1/4  19:31, Hans Verkuil wrote:
> > On 07/17/2015 12:29 PM, Inki Dae wrote:
> >> On 2015e?? 07i?? 17i? 1/4  19:20, Hans Verkuil wrote:
> >>> On 07/13/2015 04:55 PM, Jan Kara wrote:
> >>>> From: Jan Kara <jack@suse.cz>
> >>>>
> >>>> Convert g2d_userptr_get_dma_addr() to pin pages using get_vaddr_frames().
> >>>> This removes the knowledge about vmas and mmap_sem locking from exynos
> >>>> driver. Also it fixes a problem that the function has been mapping user
> >>>> provided address without holding mmap_sem.
> >>>
> >>> I'd like to see an Ack from one of the exynos drm driver maintainers before
> >>> I merge this.
> >>>
> >>> Inki, Marek?
> >>
> >> I already gave Ack but it seems that Jan missed it while updating.
> >>
> >> Anyway,
> >> Acked-by: Inki Dae <inki.dae@samsung.com>
> > 
> > Thanks!
> 
> Oops, sorry. This patch would incur a build warning. Below is my comment.
> 
> >>>> @@ -456,65 +455,38 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct drm_device *drm_dev,
> >>>>  		return ERR_PTR(-ENOMEM);
> >>>>  
> >>>>  	atomic_set(&g2d_userptr->refcount, 1);
> >>>> +	g2d_userptr->size = size;
> >>>>  
> >>>>  	start = userptr & PAGE_MASK;
> >>>>  	offset = userptr & ~PAGE_MASK;
> >>>>  	end = PAGE_ALIGN(userptr + size);
> >>>>  	npages = (end - start) >> PAGE_SHIFT;
> >>>> -	g2d_userptr->npages = npages;
> >>>> -
> >>>> -	pages = drm_calloc_large(npages, sizeof(struct page *));
> >>>> -	if (!pages) {
> >>>> -		DRM_ERROR("failed to allocate pages.\n");
> >>>> -		ret = -ENOMEM;
> >>>> +	g2d_userptr->vec = frame_vector_create(npages);
> >>>> +	if (!g2d_userptr->vec)
> 
> You would need ret = -EFAULT here. And below is a patch posted already,
> 	http://www.spinics.net/lists/dri-devel/msg85321.html

The error should IMHO be -ENOMEM because frame_vector_create() fails only
if we fail to allocate the structure. Attached is the updated version of
the patch. Hans, can you please pick this one?
 
> ps. please, ignore the codes related to build error in the patch.
> 
> With the change, Acked-by: Inki Dae <inki.dae@samsung.com>

Thanks and sorry for making so many stupid mistakes in the Exynos driver.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[-- Attachment #2: 0009-drm-exynos-Convert-g2d_userptr_get_dma_addr-to-use-g.patch --]
[-- Type: text/x-patch, Size: 0 bytes --]