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 <j...@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 <j...@suse.cz>
>>>> Signed-off-by: Hans Verkuil <hans.verk...@cisco.com>
>>>> [hans.verk...@cisco.com: remove unused struct omap_vout_device *vout
>>>> variable]
>>>>
>>>> Signed-off-by: Mauro Carvalho Chehab <mche...@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(¤t->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(¤t->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
signature.asc
Description: OpenPGP digital signature
