All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab@redhat.com>
To: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: linux-media@vger.kernel.org, pawel@osciak.com,
	laurent.pinchart@ideasonboard.com
Subject: Re: [GIT PULL] Selection API and fixes for v3.2
Date: Tue, 27 Sep 2011 08:40:54 -0300	[thread overview]
Message-ID: <4E81B646.2010400@redhat.com> (raw)
In-Reply-To: <000001cc7cee$c465d350$4d3179f0$%szyprowski@samsung.com>

Em 27-09-2011 05:23, Marek Szyprowski escreveu:
> Hello,
> 
> On Monday, September 26, 2011 3:30 PM Mauro Carvalho Chehab wrote:
> 
>> Em 26-09-2011 08:21, Marek Szyprowski escreveu:
>>> Hello,
>>>
>>> On Monday, September 26, 2011 1:14 PM Mauro Carvalho Chehab wrote:
>>>
>>>>> Scott Jiang (1):
>>>>>       vb2: add vb2_get_unmapped_area in vb2 core
>>>>
>>>>> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
>>>>> index ea55c08..977410b 100644
>>>>> --- a/include/media/videobuf2-core.h
>>>>> +++ b/include/media/videobuf2-core.h
>>>>> @@ -309,6 +309,13 @@ int vb2_streamon(struct vb2_queue *q, enum v4l2_buf_type type);
>>>>>  int vb2_streamoff(struct vb2_queue *q, enum v4l2_buf_type type);
>>>>>
>>>>>  int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma);
>>>>> +#ifndef CONFIG_MMU
>>>>> +unsigned long vb2_get_unmapped_area(struct vb2_queue *q,
>>>>> +				    unsigned long addr,
>>>>> +				    unsigned long len,
>>>>> +				    unsigned long pgoff,
>>>>> +				    unsigned long flags);
>>>>> +#endif
>>>>>  unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait);
>>>>>  size_t vb2_read(struct vb2_queue *q, char __user *data, size_t count,
>>>>>  		loff_t *ppos, int nonblock);
>>>>
>>>> This sounds me like a hack, as it is passing the problem of working with a non-mmu
>>>> capable hardware to the driver, inserting architecture-dependent bits on them.
>>>>
>>>> The proper way to do it is to provide a vb2 core support to handle the non-mmu case
>>>> inside it.
>>>
>>> This is exactly what this patch does - it provides generic vb2 implementation for
>>> fops->get_unmapped_area callback which any vb2 ready driver can use. This operation
>>> is used only on NON-MMU systems. Please check drivers/media/video/v4l2-dev.c file and
>>> the implementation of get_unmapped_area there. Similar code is used by uvc driver.
>>
>> At least there, there is a:
>> #ifdef CONFIG_MMU
>> #define v4l2_get_unmapped_area NULL
>> #else
>> ...
>> #endif
>>
>> block, so, in thesis, a driver can be written to support both cases without inserting
>> #ifdefs inside it.
> 
> videobuf2 functions are not meant to be used as direct callbacks in fops so defining it
> as NULL makes no sense at all.
> 
>> Ideally, I would prefer if all those iommu-specific calls would be inside the core.
>> A driver should not need to do anything special in order to support a different
>> (sub)architecture.
> 
> It is not about IOMMU at all. Some (embedded) systems don't have MMU at all. Drivers on
> such systems cannot do regular mmap operation. Instead it is emulated with 
> get_unmapped_area fops callback and some (u)libC magic. This patch just provides
> vb2_get_unmapped_area function. Drivers have to call it from their respective
> my_driver_get_unmapped_area() function provided in its fops. Implementing it makes
> sense only on NO-MMU systems. I really see no other way of dealing with this.

Ok. Feel free to add this patch again on a next request.

Thanks,
Mauro
> 
> Best regards


      reply	other threads:[~2011-09-27 11:41 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-22 15:13 [GIT PULL] Selection API and fixes for v3.2 Marek Szyprowski
2011-09-24  3:58 ` Mauro Carvalho Chehab
2011-09-26  8:42   ` Tomasz Stanislawski
2011-09-26 12:10     ` Mauro Carvalho Chehab
2011-09-26 12:17       ` Mauro Carvalho Chehab
2011-09-27 13:02       ` Tomasz Stanislawski
2011-09-27 14:10         ` Mauro Carvalho Chehab
2011-09-27 16:46           ` Tomasz Stanislawski
2011-09-28  8:01             ` Hans Verkuil
2011-09-28  8:29               ` Laurent Pinchart
2011-09-28  9:00                 ` Hans Verkuil
2011-09-28  9:59               ` Tomasz Stanislawski
2011-09-28 10:40                 ` Mauro Carvalho Chehab
2011-09-28 15:17                   ` Tomasz Stanislawski
2011-09-28 11:20                 ` Hans Verkuil
2011-09-26 10:03   ` Marek Szyprowski
2011-09-26 11:18     ` Mauro Carvalho Chehab
2011-09-26 12:45   ` Hans Verkuil
2011-09-26 11:13 ` Mauro Carvalho Chehab
2011-09-26 11:21   ` Marek Szyprowski
2011-09-26 13:30     ` Mauro Carvalho Chehab
2011-09-26 14:41       ` Laurent Pinchart
2011-09-27  8:23       ` Marek Szyprowski
2011-09-27 11:40         ` Mauro Carvalho Chehab [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E81B646.2010400@redhat.com \
    --to=mchehab@redhat.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=pawel@osciak.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.