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@xxxxxxx>
> >>
> >> 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@xxxxxxx>
> >> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> >> [hans.verkuil@xxxxxxxxx: remove unused struct omap_vout_device *vout
> >> variable]
> >>
> >> Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxx>
> >>
> >> 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 ?
--
Regards,
Laurent Pinchart
Attachment:
signature.asc
Description: This is a digitally signed message part.
