Bill Dirks wrote:
> Gerd Knorr wrote:
> > ...you can vmalloc() a chunk of memory and then remap that to
> > userspace (like most drivers do today).
>
> For which v4l2_vmalloc_to_page() is still useful.
Yes. Only the v4l2_vmalloc_to_bus() is obsolete and must be either
simply dropped or replaced with something which wraps the pci_*
functions, the other two mm helper functions are still useful as-is.
> > Idea was to make it easier to handle v4l1 backward compatibility.
>
> I understand, but I disagree that that is a valid reason to change the
> API. It doesn't add new capability, provide a benefit to the
> application, or solve an API design defect.
The v4l2 drivers have to deal with both new-style v4l2 and old-style
v4l1 mappings then. Ok, I can live with that.
I don't like the way this is handled currently throuth: The compatibily
layer tried to translate the VIDIOCGMBUF ioctl into VIDIOC_REQBUFS and
VIDIOC_QUERYBUF, failes to do so for more than one buffer, and introduces
the V4L2_BUF_REQ_CONTIG flag to workaround that. I don't think that
is very useful. If we can't translate that particular ioctl anyway, the
compatibility layer should not try to do that in the first place.
IMHO it better and less confusing to handle it this way:
* drop the V4L2_BUF_REQ_CONTIG flag.
* v4l2 drivers should handle VIDIOCGMBUF directly (maybe let the
compatibility layer provide one single buffer as fallback).
* mmap() will not be translated by the compatibility layer.
rationale: the driver has to special case the v4l1 mappings
anyway, there is no point in trying to hide that from the driver.
* the v4l2 driver should never ever use zero as offset magic (so
it will notice whenever a application uses the v4l1 or v4l2
mappings).
It also is useful if the driver takes care that the buffers are
page-aligned (vmalloc does that anyway I think) and the length
is a multiple of PAGE_SIZE. It can easily remap multiple vmalloc()ed
buffers into one contignous virtual mapping for v4l1 compatibility
then.
> I used different locks for hardware and lists. One lock for each list,
> and one lock for each card.
/me uses just one lock[1] per device which is used for both hardware access
and assorted lists. As I often have to access both lists and hardware
at the same time (when switching capture buffers for example) I don't
think it is useful to have separate locks for them. Locking needs
resources too, you don't get locks for free.
Gerd
[1] One spinlock and one semaphore to be exact. The spinlock to lock
stuff which needs to be locked against access from the irq handler
as you can't use the semaphores within the irq handler. The
semaphore to lock other stuff (which might sleep what you can't do
while holding a spinlock).
--
Man muß die Software wacker hüten
weil in der Welt die Hacker wüten
