diff mbox

[2/9,media] media: omap_vout: Convert omap_vout_uservirt_to_phys() to use get_vaddr_pfns()

Message ID 0bec810973e08df0e66260e84d2dcea055a3fad7.1433927458.git.mchehab@osg.samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mauro Carvalho Chehab June 10, 2015, 9:20 a.m. UTC
From: Jan Kara <jack@suse.cz>

Convert omap_vout_uservirt_to_phys() to use get_vaddr_pfns() instead of
hand made mapping of virtual address to physical address. Also the
function leaked page reference from get_user_pages() so fix that by
properly release the reference when omap_vout_buffer_release() is
called.

Signed-off-by: Jan Kara <jack@suse.cz>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
[hans.verkuil@cisco.com: remove unused struct omap_vout_device *vout variable]

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

Comments

Laurent Pinchart June 11, 2015, 4:21 a.m. UTC | #1
Hello,

(CC'ing Tomi Valkeinen)

On Wednesday 10 June 2015 06:20:45 Mauro Carvalho Chehab wrote:
> From: Jan Kara <jack@suse.cz>
> 
> Convert omap_vout_uservirt_to_phys() to use get_vaddr_pfns() instead of
> hand made mapping of virtual address to physical address. Also the
> function leaked page reference from get_user_pages() so fix that by
> properly release the reference when omap_vout_buffer_release() is
> called.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> [hans.verkuil@cisco.com: remove unused struct omap_vout_device *vout
> variable]
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> diff --git a/drivers/media/platform/omap/omap_vout.c
> b/drivers/media/platform/omap/omap_vout.c index f09c5f17a42f..7feb6394f111
> 100644
> --- a/drivers/media/platform/omap/omap_vout.c
> +++ b/drivers/media/platform/omap/omap_vout.c
> @@ -195,46 +195,34 @@ static int omap_vout_try_format(struct v4l2_pix_format
> *pix) }
> 
>  /*
> - * omap_vout_uservirt_to_phys: This inline function is used to convert user
> - * space virtual address to physical address.
> + * omap_vout_get_userptr: Convert user space virtual address to physical
> + * address.
>   */
> -static unsigned long omap_vout_uservirt_to_phys(unsigned long virtp)
> +static int omap_vout_get_userptr(struct videobuf_buffer *vb, u32 virtp,
> +				 u32 *physp)
>  {
> -	unsigned long physp = 0;
> -	struct vm_area_struct *vma;
> -	struct mm_struct *mm = current->mm;
> +	struct frame_vector *vec;
> +	int ret;
> 
>  	/* For kernel direct-mapped memory, take the easy way */
> -	if (virtp >= PAGE_OFFSET)
> -		return virt_to_phys((void *) virtp);
> -
> -	down_read(&current->mm->mmap_sem);
> -	vma = find_vma(mm, virtp);
> -	if (vma && (vma->vm_flags & VM_IO) && vma->vm_pgoff) {
> -		/* this will catch, kernel-allocated, mmaped-to-usermode
> -		   addresses */
> -		physp = (vma->vm_pgoff << PAGE_SHIFT) + (virtp - vma->vm_start);
> -		up_read(&current->mm->mmap_sem);
> -	} else {
> -		/* otherwise, use get_user_pages() for general userland pages */
> -		int res, nr_pages = 1;
> -		struct page *pages;
> +	if (virtp >= PAGE_OFFSET) {
> +		*physp = virt_to_phys((void *)virtp);

Lovely. virtp comes from userspace and as far as I know it arrives here 
completely unchecked. The problem isn't introduced by this patch, but 
omap_vout buffer management seems completely broken to me, and nobody seems to 
care about the driver. Given that omapdrm should now provide the video output 
capabilities that are missing from omapfb and resulted in the development of 
omap_vout, shouldn't we drop the omap_vout driver ?

Tomi, any opinion on this ? Do you see any omap_vout capability missing from 
omapdrm ?

> +		return 0;
> +	}
> 
> -		res = get_user_pages(current, current->mm, virtp, nr_pages, 1,
> -				0, &pages, NULL);
> -		up_read(&current->mm->mmap_sem);
> +	vec = frame_vector_create(1);
> +	if (!vec)
> +		return -ENOMEM;
> 
> -		if (res == nr_pages) {
> -			physp =  __pa(page_address(&pages[0]) +
> -					(virtp & ~PAGE_MASK));
> -		} else {
> -			printk(KERN_WARNING VOUT_NAME
> -					"get_user_pages failed\n");
> -			return 0;
> -		}
> +	ret = get_vaddr_frames(virtp, 1, true, false, vec);
> +	if (ret != 1) {
> +		frame_vector_destroy(vec);
> +		return -EINVAL;
>  	}
> +	*physp = __pfn_to_phys(frame_vector_pfns(vec)[0]);
> +	vb->priv = vec;
> 
> -	return physp;
> +	return 0;
>  }
> 
>  /*
> @@ -784,11 +772,15 @@ static int omap_vout_buffer_prepare(struct
> videobuf_queue *q, * address of the buffer
>  	 */
>  	if (V4L2_MEMORY_USERPTR == vb->memory) {
> +		int ret;
> +
>  		if (0 == vb->baddr)
>  			return -EINVAL;
>  		/* Physical address */
> -		vout->queued_buf_addr[vb->i] = (u8 *)
> -			omap_vout_uservirt_to_phys(vb->baddr);
> +		ret = omap_vout_get_userptr(vb, vb->baddr,
> +				(u32 *)&vout->queued_buf_addr[vb->i]);
> +		if (ret < 0)
> +			return ret;
>  	} else {
>  		unsigned long addr, dma_addr;
>  		unsigned long size;
> @@ -834,12 +826,13 @@ static void omap_vout_buffer_queue(struct
> videobuf_queue *q, static void omap_vout_buffer_release(struct
> videobuf_queue *q,
>  			    struct videobuf_buffer *vb)
>  {
> -	struct omap_vout_device *vout = q->priv_data;
> -
>  	vb->state = VIDEOBUF_NEEDS_INIT;
> +	if (vb->memory == V4L2_MEMORY_USERPTR && vb->priv) {
> +		struct frame_vector *vec = vb->priv;
> 
> -	if (V4L2_MEMORY_MMAP != vout->memory)
> -		return;
> +		put_vaddr_frames(vec);
> +		frame_vector_destroy(vec);
> +	}
>  }
> 
>  /*
Hans Verkuil (hansverk) June 11, 2015, 5:23 p.m. UTC | #2
On 06/11/15 06:21, Laurent Pinchart wrote:
> Hello,
> 
> (CC'ing Tomi Valkeinen)
> 
> On Wednesday 10 June 2015 06:20:45 Mauro Carvalho Chehab wrote:
>> From: Jan Kara <jack@suse.cz>
>>
>> Convert omap_vout_uservirt_to_phys() to use get_vaddr_pfns() instead of
>> hand made mapping of virtual address to physical address. Also the
>> function leaked page reference from get_user_pages() so fix that by
>> properly release the reference when omap_vout_buffer_release() is
>> called.
>>
>> Signed-off-by: Jan Kara <jack@suse.cz>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> [hans.verkuil@cisco.com: remove unused struct omap_vout_device *vout
>> variable]
>>
>> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>>
>> diff --git a/drivers/media/platform/omap/omap_vout.c
>> b/drivers/media/platform/omap/omap_vout.c index f09c5f17a42f..7feb6394f111
>> 100644
>> --- a/drivers/media/platform/omap/omap_vout.c
>> +++ b/drivers/media/platform/omap/omap_vout.c
>> @@ -195,46 +195,34 @@ static int omap_vout_try_format(struct v4l2_pix_format
>> *pix) }
>>
>>  /*
>> - * omap_vout_uservirt_to_phys: This inline function is used to convert user
>> - * space virtual address to physical address.
>> + * omap_vout_get_userptr: Convert user space virtual address to physical
>> + * address.
>>   */
>> -static unsigned long omap_vout_uservirt_to_phys(unsigned long virtp)
>> +static int omap_vout_get_userptr(struct videobuf_buffer *vb, u32 virtp,
>> +				 u32 *physp)
>>  {
>> -	unsigned long physp = 0;
>> -	struct vm_area_struct *vma;
>> -	struct mm_struct *mm = current->mm;
>> +	struct frame_vector *vec;
>> +	int ret;
>>
>>  	/* For kernel direct-mapped memory, take the easy way */
>> -	if (virtp >= PAGE_OFFSET)
>> -		return virt_to_phys((void *) virtp);
>> -
>> -	down_read(&current->mm->mmap_sem);
>> -	vma = find_vma(mm, virtp);
>> -	if (vma && (vma->vm_flags & VM_IO) && vma->vm_pgoff) {
>> -		/* this will catch, kernel-allocated, mmaped-to-usermode
>> -		   addresses */
>> -		physp = (vma->vm_pgoff << PAGE_SHIFT) + (virtp - vma->vm_start);
>> -		up_read(&current->mm->mmap_sem);
>> -	} else {
>> -		/* otherwise, use get_user_pages() for general userland pages */
>> -		int res, nr_pages = 1;
>> -		struct page *pages;
>> +	if (virtp >= PAGE_OFFSET) {
>> +		*physp = virt_to_phys((void *)virtp);
> 
> Lovely. virtp comes from userspace and as far as I know it arrives here 
> completely unchecked. The problem isn't introduced by this patch, but 
> omap_vout buffer management seems completely broken to me, and nobody seems to 
> care about the driver. Given that omapdrm should now provide the video output 
> capabilities that are missing from omapfb and resulted in the development of 
> omap_vout, shouldn't we drop the omap_vout driver ?

In addition it uses the old videobuf framework which at some point will be reason
for removal if nobody cares enough to convert to videobuf2.

So I would also be in favor of removal if there are no objections (well, probably
first to staging for one or two kernel cycles followed by removal).

	Hans

> 
> Tomi, any opinion on this ? Do you see any omap_vout capability missing from 
> omapdrm ?
> 
>> +		return 0;
>> +	}
>>
>> -		res = get_user_pages(current, current->mm, virtp, nr_pages, 1,
>> -				0, &pages, NULL);
>> -		up_read(&current->mm->mmap_sem);
>> +	vec = frame_vector_create(1);
>> +	if (!vec)
>> +		return -ENOMEM;
>>
>> -		if (res == nr_pages) {
>> -			physp =  __pa(page_address(&pages[0]) +
>> -					(virtp & ~PAGE_MASK));
>> -		} else {
>> -			printk(KERN_WARNING VOUT_NAME
>> -					"get_user_pages failed\n");
>> -			return 0;
>> -		}
>> +	ret = get_vaddr_frames(virtp, 1, true, false, vec);
>> +	if (ret != 1) {
>> +		frame_vector_destroy(vec);
>> +		return -EINVAL;
>>  	}
>> +	*physp = __pfn_to_phys(frame_vector_pfns(vec)[0]);
>> +	vb->priv = vec;
>>
>> -	return physp;
>> +	return 0;
>>  }
>>
>>  /*
>> @@ -784,11 +772,15 @@ static int omap_vout_buffer_prepare(struct
>> videobuf_queue *q, * address of the buffer
>>  	 */
>>  	if (V4L2_MEMORY_USERPTR == vb->memory) {
>> +		int ret;
>> +
>>  		if (0 == vb->baddr)
>>  			return -EINVAL;
>>  		/* Physical address */
>> -		vout->queued_buf_addr[vb->i] = (u8 *)
>> -			omap_vout_uservirt_to_phys(vb->baddr);
>> +		ret = omap_vout_get_userptr(vb, vb->baddr,
>> +				(u32 *)&vout->queued_buf_addr[vb->i]);
>> +		if (ret < 0)
>> +			return ret;
>>  	} else {
>>  		unsigned long addr, dma_addr;
>>  		unsigned long size;
>> @@ -834,12 +826,13 @@ static void omap_vout_buffer_queue(struct
>> videobuf_queue *q, static void omap_vout_buffer_release(struct
>> videobuf_queue *q,
>>  			    struct videobuf_buffer *vb)
>>  {
>> -	struct omap_vout_device *vout = q->priv_data;
>> -
>>  	vb->state = VIDEOBUF_NEEDS_INIT;
>> +	if (vb->memory == V4L2_MEMORY_USERPTR && vb->priv) {
>> +		struct frame_vector *vec = vb->priv;
>>
>> -	if (V4L2_MEMORY_MMAP != vout->memory)
>> -		return;
>> +		put_vaddr_frames(vec);
>> +		frame_vector_destroy(vec);
>> +	}
>>  }
>>
>>  /*
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomi Valkeinen June 12, 2015, 9:21 a.m. UTC | #3
On 11/06/15 07:21, Laurent Pinchart wrote:
> Hello,
> 
> (CC'ing Tomi Valkeinen)
> 
> On Wednesday 10 June 2015 06:20:45 Mauro Carvalho Chehab wrote:
>> From: Jan Kara <jack@suse.cz>
>>
>> Convert omap_vout_uservirt_to_phys() to use get_vaddr_pfns() instead of
>> hand made mapping of virtual address to physical address. Also the
>> function leaked page reference from get_user_pages() so fix that by
>> properly release the reference when omap_vout_buffer_release() is
>> called.
>>
>> Signed-off-by: Jan Kara <jack@suse.cz>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> [hans.verkuil@cisco.com: remove unused struct omap_vout_device *vout
>> variable]
>>
>> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>>
>> diff --git a/drivers/media/platform/omap/omap_vout.c
>> b/drivers/media/platform/omap/omap_vout.c index f09c5f17a42f..7feb6394f111
>> 100644
>> --- a/drivers/media/platform/omap/omap_vout.c
>> +++ b/drivers/media/platform/omap/omap_vout.c
>> @@ -195,46 +195,34 @@ static int omap_vout_try_format(struct v4l2_pix_format
>> *pix) }
>>
>>  /*
>> - * omap_vout_uservirt_to_phys: This inline function is used to convert user
>> - * space virtual address to physical address.
>> + * omap_vout_get_userptr: Convert user space virtual address to physical
>> + * address.
>>   */
>> -static unsigned long omap_vout_uservirt_to_phys(unsigned long virtp)
>> +static int omap_vout_get_userptr(struct videobuf_buffer *vb, u32 virtp,
>> +				 u32 *physp)
>>  {
>> -	unsigned long physp = 0;
>> -	struct vm_area_struct *vma;
>> -	struct mm_struct *mm = current->mm;
>> +	struct frame_vector *vec;
>> +	int ret;
>>
>>  	/* For kernel direct-mapped memory, take the easy way */
>> -	if (virtp >= PAGE_OFFSET)
>> -		return virt_to_phys((void *) virtp);
>> -
>> -	down_read(&current->mm->mmap_sem);
>> -	vma = find_vma(mm, virtp);
>> -	if (vma && (vma->vm_flags & VM_IO) && vma->vm_pgoff) {
>> -		/* this will catch, kernel-allocated, mmaped-to-usermode
>> -		   addresses */
>> -		physp = (vma->vm_pgoff << PAGE_SHIFT) + (virtp - vma->vm_start);
>> -		up_read(&current->mm->mmap_sem);
>> -	} else {
>> -		/* otherwise, use get_user_pages() for general userland pages */
>> -		int res, nr_pages = 1;
>> -		struct page *pages;
>> +	if (virtp >= PAGE_OFFSET) {
>> +		*physp = virt_to_phys((void *)virtp);
> 
> Lovely. virtp comes from userspace and as far as I know it arrives here 
> completely unchecked. The problem isn't introduced by this patch, but 
> omap_vout buffer management seems completely broken to me, and nobody seems to 
> care about the driver. Given that omapdrm should now provide the video output 
> capabilities that are missing from omapfb and resulted in the development of 
> omap_vout, shouldn't we drop the omap_vout driver ?
> 
> Tomi, any opinion on this ? Do you see any omap_vout capability missing from 
> omapdrm ?

I've never used omap_vout, so I don't even know what it offers. I do
know it supports VRFB rotation (omap3), which we don't have on omapdrm,
though. Whether anyone uses that (or omap_vout), I have no idea...

I'd personally love to get rid of omap_vout, as it causes complications
on omapfb/omapdss side.

 Tomi
Laurent Pinchart June 12, 2015, 9:26 a.m. UTC | #4
Hi Tomi,

On Friday 12 June 2015 12:21:13 Tomi Valkeinen wrote:
> On 11/06/15 07:21, Laurent Pinchart wrote:
> > On Wednesday 10 June 2015 06:20:45 Mauro Carvalho Chehab wrote:
> >> From: Jan Kara <jack@suse.cz>
> >> 
> >> Convert omap_vout_uservirt_to_phys() to use get_vaddr_pfns() instead of
> >> hand made mapping of virtual address to physical address. Also the
> >> function leaked page reference from get_user_pages() so fix that by
> >> properly release the reference when omap_vout_buffer_release() is
> >> called.
> >> 
> >> Signed-off-by: Jan Kara <jack@suse.cz>
> >> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> >> [hans.verkuil@cisco.com: remove unused struct omap_vout_device *vout
> >> variable]
> >> 
> >> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> >> 
> >> diff --git a/drivers/media/platform/omap/omap_vout.c
> >> b/drivers/media/platform/omap/omap_vout.c index
> >> f09c5f17a42f..7feb6394f111
> >> 100644
> >> --- a/drivers/media/platform/omap/omap_vout.c
> >> +++ b/drivers/media/platform/omap/omap_vout.c
> >> @@ -195,46 +195,34 @@ static int omap_vout_try_format(struct
> >> v4l2_pix_format *pix) }
> >> 
> >>  /*
> >> 
> >> - * omap_vout_uservirt_to_phys: This inline function is used to convert
> >> user - * space virtual address to physical address.
> >> + * omap_vout_get_userptr: Convert user space virtual address to physical
> >> + * address.
> >> 
> >>   */
> >> 
> >> -static unsigned long omap_vout_uservirt_to_phys(unsigned long virtp)
> >> +static int omap_vout_get_userptr(struct videobuf_buffer *vb, u32 virtp,
> >> +				 u32 *physp)
> >> 
> >>  {
> >> 
> >> -	unsigned long physp = 0;
> >> -	struct vm_area_struct *vma;
> >> -	struct mm_struct *mm = current->mm;
> >> +	struct frame_vector *vec;
> >> +	int ret;
> >> 
> >>  	/* For kernel direct-mapped memory, take the easy way */
> >> 
> >> -	if (virtp >= PAGE_OFFSET)
> >> -		return virt_to_phys((void *) virtp);
> >> -
> >> -	down_read(&current->mm->mmap_sem);
> >> -	vma = find_vma(mm, virtp);
> >> -	if (vma && (vma->vm_flags & VM_IO) && vma->vm_pgoff) {
> >> -		/* this will catch, kernel-allocated, mmaped-to-usermode
> >> -		   addresses */
> >> -		physp = (vma->vm_pgoff << PAGE_SHIFT) + (virtp - vma->vm_start);
> >> -		up_read(&current->mm->mmap_sem);
> >> -	} else {
> >> -		/* otherwise, use get_user_pages() for general userland pages */
> >> -		int res, nr_pages = 1;
> >> -		struct page *pages;
> >> +	if (virtp >= PAGE_OFFSET) {
> >> +		*physp = virt_to_phys((void *)virtp);
> > 
> > Lovely. virtp comes from userspace and as far as I know it arrives here
> > completely unchecked. The problem isn't introduced by this patch, but
> > omap_vout buffer management seems completely broken to me, and nobody
> > seems to care about the driver. Given that omapdrm should now provide the
> > video output capabilities that are missing from omapfb and resulted in
> > the development of omap_vout, shouldn't we drop the omap_vout driver ?
> > 
> > Tomi, any opinion on this ? Do you see any omap_vout capability missing
> > from omapdrm ?
> 
> I've never used omap_vout, so I don't even know what it offers. I do
> know it supports VRFB rotation (omap3), which we don't have on omapdrm,
> though. Whether anyone uses that (or omap_vout), I have no idea...
> 
> I'd personally love to get rid of omap_vout, as it causes complications
> on omapfb/omapdss side.

How difficult would it be to implement VRFB rotation in omapdrm ?
Tomi Valkeinen June 12, 2015, 9:44 a.m. UTC | #5
On 12/06/15 12:26, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Friday 12 June 2015 12:21:13 Tomi Valkeinen wrote:
>> On 11/06/15 07:21, Laurent Pinchart wrote:
>>> On Wednesday 10 June 2015 06:20:45 Mauro Carvalho Chehab wrote:
>>>> From: Jan Kara <jack@suse.cz>
>>>>
>>>> Convert omap_vout_uservirt_to_phys() to use get_vaddr_pfns() instead of
>>>> hand made mapping of virtual address to physical address. Also the
>>>> function leaked page reference from get_user_pages() so fix that by
>>>> properly release the reference when omap_vout_buffer_release() is
>>>> called.
>>>>
>>>> Signed-off-by: Jan Kara <jack@suse.cz>
>>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>>> [hans.verkuil@cisco.com: remove unused struct omap_vout_device *vout
>>>> variable]
>>>>
>>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>>>>
>>>> diff --git a/drivers/media/platform/omap/omap_vout.c
>>>> b/drivers/media/platform/omap/omap_vout.c index
>>>> f09c5f17a42f..7feb6394f111
>>>> 100644
>>>> --- a/drivers/media/platform/omap/omap_vout.c
>>>> +++ b/drivers/media/platform/omap/omap_vout.c
>>>> @@ -195,46 +195,34 @@ static int omap_vout_try_format(struct
>>>> v4l2_pix_format *pix) }
>>>>
>>>>  /*
>>>>
>>>> - * omap_vout_uservirt_to_phys: This inline function is used to convert
>>>> user - * space virtual address to physical address.
>>>> + * omap_vout_get_userptr: Convert user space virtual address to physical
>>>> + * address.
>>>>
>>>>   */
>>>>
>>>> -static unsigned long omap_vout_uservirt_to_phys(unsigned long virtp)
>>>> +static int omap_vout_get_userptr(struct videobuf_buffer *vb, u32 virtp,
>>>> +				 u32 *physp)
>>>>
>>>>  {
>>>>
>>>> -	unsigned long physp = 0;
>>>> -	struct vm_area_struct *vma;
>>>> -	struct mm_struct *mm = current->mm;
>>>> +	struct frame_vector *vec;
>>>> +	int ret;
>>>>
>>>>  	/* For kernel direct-mapped memory, take the easy way */
>>>>
>>>> -	if (virtp >= PAGE_OFFSET)
>>>> -		return virt_to_phys((void *) virtp);
>>>> -
>>>> -	down_read(&current->mm->mmap_sem);
>>>> -	vma = find_vma(mm, virtp);
>>>> -	if (vma && (vma->vm_flags & VM_IO) && vma->vm_pgoff) {
>>>> -		/* this will catch, kernel-allocated, mmaped-to-usermode
>>>> -		   addresses */
>>>> -		physp = (vma->vm_pgoff << PAGE_SHIFT) + (virtp - vma->vm_start);
>>>> -		up_read(&current->mm->mmap_sem);
>>>> -	} else {
>>>> -		/* otherwise, use get_user_pages() for general userland pages */
>>>> -		int res, nr_pages = 1;
>>>> -		struct page *pages;
>>>> +	if (virtp >= PAGE_OFFSET) {
>>>> +		*physp = virt_to_phys((void *)virtp);
>>>
>>> Lovely. virtp comes from userspace and as far as I know it arrives here
>>> completely unchecked. The problem isn't introduced by this patch, but
>>> omap_vout buffer management seems completely broken to me, and nobody
>>> seems to care about the driver. Given that omapdrm should now provide the
>>> video output capabilities that are missing from omapfb and resulted in
>>> the development of omap_vout, shouldn't we drop the omap_vout driver ?
>>>
>>> Tomi, any opinion on this ? Do you see any omap_vout capability missing
>>> from omapdrm ?
>>
>> I've never used omap_vout, so I don't even know what it offers. I do
>> know it supports VRFB rotation (omap3), which we don't have on omapdrm,
>> though. Whether anyone uses that (or omap_vout), I have no idea...
>>
>> I'd personally love to get rid of omap_vout, as it causes complications
>> on omapfb/omapdss side.
> 
> How difficult would it be to implement VRFB rotation in omapdrm ?

I don't know... drivers/video/fbdev/omap2/vrfb.c contains the code to
handle the VRFB hardware, but it does require certain amount of book
keeping and tweaking of the fb size etc from the omapdrm side. And
probably a new parameter to somehow enable vrfb for a buffer, as you
don't want it to be used by default.

 Tomi
diff mbox

Patch

diff --git a/drivers/media/platform/omap/omap_vout.c b/drivers/media/platform/omap/omap_vout.c
index f09c5f17a42f..7feb6394f111 100644
--- a/drivers/media/platform/omap/omap_vout.c
+++ b/drivers/media/platform/omap/omap_vout.c
@@ -195,46 +195,34 @@  static int omap_vout_try_format(struct v4l2_pix_format *pix)
 }
 
 /*
- * omap_vout_uservirt_to_phys: This inline function is used to convert user
- * space virtual address to physical address.
+ * omap_vout_get_userptr: Convert user space virtual address to physical
+ * address.
  */
-static unsigned long omap_vout_uservirt_to_phys(unsigned long virtp)
+static int omap_vout_get_userptr(struct videobuf_buffer *vb, u32 virtp,
+				 u32 *physp)
 {
-	unsigned long physp = 0;
-	struct vm_area_struct *vma;
-	struct mm_struct *mm = current->mm;
+	struct frame_vector *vec;
+	int ret;
 
 	/* For kernel direct-mapped memory, take the easy way */
-	if (virtp >= PAGE_OFFSET)
-		return virt_to_phys((void *) virtp);
-
-	down_read(&current->mm->mmap_sem);
-	vma = find_vma(mm, virtp);
-	if (vma && (vma->vm_flags & VM_IO) && vma->vm_pgoff) {
-		/* this will catch, kernel-allocated, mmaped-to-usermode
-		   addresses */
-		physp = (vma->vm_pgoff << PAGE_SHIFT) + (virtp - vma->vm_start);
-		up_read(&current->mm->mmap_sem);
-	} else {
-		/* otherwise, use get_user_pages() for general userland pages */
-		int res, nr_pages = 1;
-		struct page *pages;
+	if (virtp >= PAGE_OFFSET) {
+		*physp = virt_to_phys((void *)virtp);
+		return 0;
+	}
 
-		res = get_user_pages(current, current->mm, virtp, nr_pages, 1,
-				0, &pages, NULL);
-		up_read(&current->mm->mmap_sem);
+	vec = frame_vector_create(1);
+	if (!vec)
+		return -ENOMEM;
 
-		if (res == nr_pages) {
-			physp =  __pa(page_address(&pages[0]) +
-					(virtp & ~PAGE_MASK));
-		} else {
-			printk(KERN_WARNING VOUT_NAME
-					"get_user_pages failed\n");
-			return 0;
-		}
+	ret = get_vaddr_frames(virtp, 1, true, false, vec);
+	if (ret != 1) {
+		frame_vector_destroy(vec);
+		return -EINVAL;
 	}
+	*physp = __pfn_to_phys(frame_vector_pfns(vec)[0]);
+	vb->priv = vec;
 
-	return physp;
+	return 0;
 }
 
 /*
@@ -784,11 +772,15 @@  static int omap_vout_buffer_prepare(struct videobuf_queue *q,
 	 * address of the buffer
 	 */
 	if (V4L2_MEMORY_USERPTR == vb->memory) {
+		int ret;
+
 		if (0 == vb->baddr)
 			return -EINVAL;
 		/* Physical address */
-		vout->queued_buf_addr[vb->i] = (u8 *)
-			omap_vout_uservirt_to_phys(vb->baddr);
+		ret = omap_vout_get_userptr(vb, vb->baddr,
+				(u32 *)&vout->queued_buf_addr[vb->i]);
+		if (ret < 0)
+			return ret;
 	} else {
 		unsigned long addr, dma_addr;
 		unsigned long size;
@@ -834,12 +826,13 @@  static void omap_vout_buffer_queue(struct videobuf_queue *q,
 static void omap_vout_buffer_release(struct videobuf_queue *q,
 			    struct videobuf_buffer *vb)
 {
-	struct omap_vout_device *vout = q->priv_data;
-
 	vb->state = VIDEOBUF_NEEDS_INIT;
+	if (vb->memory == V4L2_MEMORY_USERPTR && vb->priv) {
+		struct frame_vector *vec = vb->priv;
 
-	if (V4L2_MEMORY_MMAP != vout->memory)
-		return;
+		put_vaddr_frames(vec);
+		frame_vector_destroy(vec);
+	}
 }
 
 /*