Re: [RFC/PATCH v6 02/12] media: Media device
Laurent Pinchart wrote:
+struct media_device {
...
+ u8 model[32];
+ u8 serial[40];
+ u8 bus_info[32];
All drivers and userspace applications have to treat this as char[], so
why u8[]?
Regards,
Clemens
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v6 03/12] [alsa-devel] media: Entities, pads and links
Laurent Pinchart wrote:
A link is a point-to-point oriented connection between two pads, either
on the same entity or on different entities. Data flows from a source
pad to a sink pad.
Links are stored in the source entity.
In the descriptors of USB Audio and HDAudio devices, the links are
stored only in the sink. But AFAICS this doesn't matter in the media
driver API.
+Links have flags that describe the link capabilities and state.
+
+ MEDIA_LINK_FLAG_ACTIVE indicates that the link is active and can be
+ used to transfer media data. When two or more links target a sink pad,
+ only one of them can be active at a time.
+ MEDIA_LINK_FLAG_IMMUTABLE indicates that the link active state can't
+ be modified at runtime. If MEDIA_LINK_FLAG_IMMUTABLE is set, then
+ MEDIA_LINK_FLAG_ACTIVE must also be set since an immutable link is
+ always active.
So the intended userspace API for controlling routing is to change the
ACTIVE flag of links?
In USB and HD audio devices, all links are immutable, and the routing
is controlled by 'selector' entities that activate exactly one of their
input pads. In userspace, this entity shows up as a mixer control.
I guess it would be possible to map the ACTIVE flag onto these controls.
Alternatively, entities can have 'mute' mixer controls associated with
their pads. In this case, multiple unmuted inputs would be mixed
together.
+#define MEDIA_ENTITY_TYPE_MASK 0x00ff
+#define MEDIA_ENTITY_SUBTYPE_MASK0x
+
+#define MEDIA_ENTITY_TYPE_NODE (1 MEDIA_ENTITY_TYPE_SHIFT)
...
+#define MEDIA_ENTITY_TYPE_NODE_ALSA (MEDIA_ENTITY_TYPE_NODE + 3)
ALSA has PCM and MIDI devices, and several types of mixer controls.
(It also has hardware dependent and timer devices, but I don't think
these would need topology information.) So we need at least these:
MEDIA_ENTITY_TYPE_NODE_ALSA_PCM
MEDIA_ENTITY_TYPE_NODE_ALSA_MIDI
MEDIA_ENTITY_TYPE_SUBDEV_ALSA_CONTROL
Furthermore, topology information is also needed for entities not
associated with a mixer control, such as microphones, speakers, jacks/
connectors, and effect units. These entities are defined in the USB and
HD audio specifications, but are not yet handled by ALSA.
+struct media_entity {
...
+ union {
+ /* Node specifications */
+ struct {
+ u32 major;
+ u32 minor;
+ } v4l;
+ struct {
+ u32 major;
+ u32 minor;
+ } fb;
+ int alsa;
+ int dvb;
+
+ /* Sub-device specifications */
+ /* Nothing needed yet */
+ };
ALSA devices are not addressed by their device node but with card/device/
subdevice numbers; mixer controls have numeric IDs, unique per card:
struct {
int card;
int device;
int subdevice;
} alsa_device;
struct {
int card;
int numid;
} alsa_control;
Regards,
Clemens
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/7] v4l: add videobuf2 Video for Linux 2 driver framework
Hello,
On Thursday, November 25, 2010 2:17 AM Laurent Pinchart wrote:
On Friday 19 November 2010 16:55:38 Marek Szyprowski wrote:
From: Pawel Osciak p.osc...@samsung.com
Videobuf2 is a Video for Linux 2 API-compatible driver framework for
multimedia devices. It acts as an intermediate layer between userspace
applications and device drivers. It also provides low-level, modular
memory management functions for drivers.
Videobuf2 eases driver development, reduces drivers' code size and aids in
proper and consistent implementation of V4L2 API in drivers.
Videobuf2 memory management backend is fully modular. This allows custom
memory management routines for devices and platforms with non-standard
memory management requirements to be plugged in, without changing the
high-level buffer management functions and API.
The framework provides:
- implementations of streaming I/O V4L2 ioctls and file operations
- high-level video buffer, video queue and state management functions
- video buffer memory allocation and management
Excellent job. I'm really pleased by the outstanding code quality.
Nevertheless I got a bunch of comments (I wonder if I'm known as an annoying
nit-picking reviewer in the community :-)).
I've reviewed the patch from bottom to top (starting at the header file), so
comments at the bottom can also apply to functions at the top when I got tired
repeating the same information. Sorry about that weird order.
Feel free to disagree with my comments, I've probably been overzealous during
the review to make sure everything I had a doubt about would be properly
addressed.
It's now past 2AM and I don't have enough courage to review the other patches.
I'm afraid they will have to wait until tomorrow. If they're of the same
quality as this one I'm sure they will be good.
Thanks for your hard work! I can imagine how much time you had to spend on
catching
all these things.
BTW, where's the patch for Documentation/video4linux ? :-)
Well, I hope it will be ready one day ;) What kind of documentation do you
expect
there? Vb2 structures and functions are already documented directly in the
source.
One last comment, why was the decision to remove all locking from vb2 taken ?
The idea came from Hans after posting version 1 and I really like it. It
simplifies
a lot of things in the drivers. The original idea of irq_spinlock and vb_lock
was
horrible. Having more than one queue in the driver meant that you need to make a
lot of nasty hacks to ensure proper locking, because in some cases you had to
take
locks from all queues. The irq_lock usage was even worse. Videobuf used it to
silently mess around the buffers that have been already queued to the hardware.
Now, after Hans changes to v4l2 core (BKL removal in favor of simple mutex)
almost
all this mess can be simply removed. Proper locking can be easily ensured by the
new mutex in the v4l2 core. There is no need to have any locks inside vb2. By
removing irq_lock the design of the drivers is easier to understand, because
there
is no implicit actions on buffers that are processed by the hardware.
snip
+/**
+ * __vb2_buf_userptr_put() - release userspace memory associated
associated + * with a USERPTR buffer
+ */
+static void __vb2_buf_userptr_put(struct vb2_buffer *vb)
+{
+ struct vb2_queue *q = vb-vb2_queue;
+ unsigned int plane;
+
+ for (plane = 0; plane vb-num_planes; ++plane) {
+ call_memop(q, plane, put_userptr, vb-planes[plane].mem_priv);
+ vb-planes[plane].mem_priv = NULL;
Any reason to initialize mem_priv to NULL here but not in __vb2_buf_mem_free ?
Yes, __vb2_buf_userptr_put is called when user calls QBUF with different address
than in the previous call, so the buffer is reinitialized without a call to
__vb2_buf_mem_free.
snip
+/**
+ * __vb2_queue_alloc() - allocate videobuf buffer structures and (for MMAP
type) + * video buffer memory for all buffers/planes on the queue and
initializes the + * queue
+ *
+ * Returns the number of buffers successfully allocated.
+ */
+static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
+unsigned int num_buffers, unsigned int num_planes)
+{
+ unsigned long plane_sizes[VIDEO_MAX_PLANES];
+ unsigned int buffer, plane;
+ struct vb2_buffer *vb;
+ int ret;
+
+ /* Get requested plane sizes from the driver */
+ for (plane = 0; plane num_planes; ++plane) {
+ ret = call_qop(q, plane_setup, q, plane, plane_sizes[plane]);
+ if (ret) {
+ dprintk(1, Plane setup failed\n);
+ return ret;
+ }
+ }
+
+ for (buffer = 0; buffer num_buffers; ++buffer) {
+ /* Allocate videobuf buffer structures */
+ vb = __vb2_buf_alloc(q);
+ if (!vb) {
+ dprintk(1, Memory alloc for buffer struct failed\n);
Re: [PATCH 3/7] v4l: videobuf2: add vmalloc allocator
On Friday, November 19, 2010 16:55:40 Marek Szyprowski wrote:
From: Pawel Osciak p.osc...@samsung.com
Add an implementation of contiguous virtual memory allocator and handling
routines for videobuf2, implemented on top of vmalloc()/vfree() calls.
Signed-off-by: Pawel Osciak p.osc...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
CC: Pawel Osciak pa...@osciak.com
---
drivers/media/video/Kconfig |5 +
drivers/media/video/Makefile|1 +
drivers/media/video/videobuf2-vmalloc.c | 177
+++
include/media/videobuf2-vmalloc.h | 21
4 files changed, 204 insertions(+), 0 deletions(-)
create mode 100644 drivers/media/video/videobuf2-vmalloc.c
create mode 100644 include/media/videobuf2-vmalloc.h
diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index 83ce858..9351423 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -55,6 +55,11 @@ config VIDEOBUF2_CORE
config VIDEOBUF2_MEMOPS
tristate
+config VIDEOBUF2_VMALLOC
+ select VIDEOBUF2_CORE
+ select VIDEOBUF2_MEMOPS
+ tristate
+
#
# Multimedia Video device configuration
#
diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
index a97a2a0..538bee9 100644
--- a/drivers/media/video/Makefile
+++ b/drivers/media/video/Makefile
@@ -116,6 +116,7 @@ obj-$(CONFIG_VIDEO_BTCX) += btcx-risc.o
obj-$(CONFIG_VIDEOBUF2_CORE) += videobuf2-core.o
obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o
+obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o
obj-$(CONFIG_V4L2_MEM2MEM_DEV) += v4l2-mem2mem.o
diff --git a/drivers/media/video/videobuf2-vmalloc.c
b/drivers/media/video/videobuf2-vmalloc.c
new file mode 100644
index 000..3310900
--- /dev/null
+++ b/drivers/media/video/videobuf2-vmalloc.c
@@ -0,0 +1,177 @@
+/*
+ * videobuf2-vmalloc.c - vmalloc memory allocator for videobuf2
+ *
+ * Copyright (C) 2010 Samsung Electronics
+ *
+ * Author: Pawel Osciak p.osc...@samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation.
+ */
+
+#include linux/module.h
+#include linux/mm.h
+#include linux/slab.h
+#include linux/vmalloc.h
+
+#include media/videobuf2-core.h
+#include media/videobuf2-memops.h
+
+struct vb2_vmalloc_conf {
+ struct vb2_alloc_ctxalloc_ctx;
+};
+
+struct vb2_vmalloc_buf {
+ void*vaddr;
+ unsigned long size;
+ unsigned intrefcount;
+};
+
+static void *vb2_vmalloc_alloc(const struct vb2_alloc_ctx *alloc_ctx,
+ unsigned long size)
+{
+ struct vb2_vmalloc_buf *buf;
+
+ buf = kzalloc(sizeof *buf, GFP_KERNEL);
+ if (!buf)
+ return NULL;
+
+ buf-size = size;
+ buf-vaddr = vmalloc_user(buf-size);
+ if (!buf-vaddr) {
+ printk(KERN_ERR vmalloc of size %ld failed\n, buf-size);
+ kfree(buf);
+ return NULL;
+ }
+
+ buf-refcount++;
+ printk(KERN_DEBUG Allocated vmalloc buffer of size %ld at vaddr=%p\n,
+ buf-size, buf-vaddr);
+
+ return buf;
+}
+
+static void vb2_vmalloc_put(void *buf_priv)
+{
+ struct vb2_vmalloc_buf *buf = buf_priv;
+
+ buf-refcount--;
+
+ if (0 == buf-refcount) {
I think I would feel happier if refcount was of type atomic_t and you would use
the atomic decrease-and-test operation here. I know that this is almost
certainly
called with some lock, but still...
+ printk(KERN_DEBUG %s: Freeing vmalloc mem at vaddr=%p\n,
+ __func__, buf-vaddr);
+ vfree(buf-vaddr);
+ kfree(buf);
+ }
+}
+
+static void *vb2_vmalloc_vaddr(void *buf_priv)
+{
+ struct vb2_vmalloc_buf *buf = buf_priv;
+
+ BUG_ON(!buf);
+
+ if (!buf-vaddr) {
+ printk(KERN_ERR Address of an unallocated
+ plane requested\n);
+ return NULL;
+ }
+
+ return buf-vaddr;
+}
+
+static unsigned int vb2_vmalloc_num_users(void *buf_priv)
+{
+ struct vb2_vmalloc_buf *buf = buf_priv;
+
+ return buf-refcount;
+}
+
+/* TODO generalize and extract to core as much as possible */
+static void vb2_vmalloc_vm_open(struct vm_area_struct *vma)
+{
+ struct vb2_vmalloc_buf *buf = vma-vm_private_data;
+
+ printk(KERN_DEBUG %s vmalloc_priv: %p, refcount: %d,
+ vma: %08lx-%08lx\n, __func__, buf, buf-refcount,
+ vma-vm_start, vma-vm_end);
+
+ buf-refcount++;
+}
+
+static void vb2_vmalloc_vm_close(struct vm_area_struct *vma)
+{
+ struct vb2_vmalloc_buf *buf =
Re: [PATCH 5/7] v4l: videobuf2: add read() and write() emulator
On Friday, November 19, 2010 16:55:42 Marek Szyprowski wrote:
Add a generic file io (read and write) emulator for videobuf2. It uses
MMAP memory type buffers and generic vb2 calls: req_bufs, qbuf and
dqbuf. Video date is being copied from mmap buffers to userspace with
standard copy_to_user() function. To add support for file io the driver
needs to provide an additional callback - read_setup or write_setup. It
should provide the default number of buffers used by emulator and flags.
With these flags one can detemine the style of read() or write()
emulation. By default 'streaming' style is used. With
VB2_FILEIO_READ_ONCE flag one can select 'one shot' mode for read()
emulator. With VB2_FILEIO_WRITE_IMMEDIATE flag one can select immediate
conversion of write calls to qbuf for write() emulator, so the vb2 will
not wait until each buffer is filled completely before queueing it to
the driver.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
CC: Pawel Osciak pa...@osciak.com
---
drivers/media/video/videobuf2-core.c | 396
+-
include/media/videobuf2-core.h | 31 +++
2 files changed, 426 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/videobuf2-core.c
b/drivers/media/video/videobuf2-core.c
index 828803f..bc497e3 100644
--- a/drivers/media/video/videobuf2-core.c
+++ b/drivers/media/video/videobuf2-core.c
snip
+/**
+ * __vb2_init_fileio() - initialize file io emulator
+ * @q: videobuf2 queue
+ * @read:mode selector (1 means read, 0 means write)
+ */
+static int __vb2_init_fileio(struct vb2_queue *q, int read)
+{
+ struct vb2_fileio_data *fileio;
+ int i, ret;
+ unsigned int count=0, flags=0;
+
+ /*
+ * Sanity check
+ */
+ if ((read !q-ops-read_setup) || (!read !q-ops-write_setup))
+ BUG();
+
+ /*
+ * Check if device supports mapping buffers to kernel virtual space.
+ */
+ if (!q-alloc_ctx[0]-mem_ops-vaddr)
+ return -EBUSY;
+
+ /*
+ * Check if steaming api has not been already activated.
typo: steaming - streaming :-)
+ */
+ if (q-streaming || q-num_buffers 0)
+ return -EBUSY;
+
+ /*
+ * Basic checks done, lets try to set up file io emulator
+ */
+ if (read)
+ ret = call_qop(q, read_setup, q, count, flags);
+ else
+ ret = call_qop(q, write_setup, q, count, flags);
+ if (ret)
+ return ret;
+
+ dprintk(3, setting up file io: mode %s, count %d, flags %08x\n,
+ (read) ? read : write, count, flags);
+
+ fileio = kzalloc(sizeof(struct vb2_fileio_data), GFP_KERNEL);
+ if (fileio == NULL)
+ return -ENOMEM;
+
+ fileio-flags = flags;
+
+ /*
+ * Request buffers and use MMAP type to force driver
+ * to allocate buffers by itself.
+ */
+ fileio-req.count = count;
+ fileio-req.memory = V4L2_MEMORY_MMAP;
+ fileio-req.type = q-type;
+ ret = vb2_reqbufs(q, fileio-req);
+ if (ret)
+ goto err_kfree;
+
+ /*
+ * Check if plane_count is correct
+ * (multiplane buffers are not supported).
+ */
+ if (q-bufs[0]-num_planes != 1) {
+ fileio-req.count = 0;
+ ret = -EBUSY;
I'm not sure about this error code. I think EINVAL is better although it's not
ideal either. A debug message would certainly help here.
+ goto err_reqbufs;
+ }
+
+ /*
+ * Get kernel address of each buffer.
+ */
+ for (i = 0; i q-num_buffers; i++) {
+ fileio-bufs[i].vaddr = vb2_plane_vaddr(q-bufs[i], 0);
+ if (fileio-bufs[i].vaddr == NULL)
+ goto err_reqbufs;
+ fileio-bufs[i].size = vb2_plane_size(q-bufs[i], 0);
+ }
+
+ /*
+ * Read mode requires pre queuing of all buffers.
+ */
+ if (read) {
+ /*
+ * Queue all buffers.
+ */
+ for (i = 0; i q-num_buffers; i++) {
+ struct v4l2_buffer *b = fileio-b;
+ memset(b, 0, sizeof(*b));
+ b-type = q-type;
+ b-memory = q-memory;
+ b-index = i;
+ ret = vb2_qbuf(q, b);
+ if (ret)
+ goto err_reqbufs;
+ fileio-bufs[i].queued = 1;
+ }
+
+ /*
+ * Start streaming.
+ */
+ ret = vb2_streamon(q, q-type);
+ if (ret)
+ goto err_reqbufs;
+ }
+
+ q-fileio = fileio;
+
+ return ret;
+
+err_reqbufs:
+ vb2_reqbufs(q, fileio-req);
+
+err_kfree:
+ kfree(fileio);
+ return ret;
+}
+
+/**
+ *
RE: [PATCH 3/7] v4l: videobuf2: add vmalloc allocator
Hello,
On Thursday, November 25, 2010 11:07 AM Hans Verkuil wrote:
On Friday, November 19, 2010 16:55:40 Marek Szyprowski wrote:
From: Pawel Osciak p.osc...@samsung.com
Add an implementation of contiguous virtual memory allocator and handling
routines for videobuf2, implemented on top of vmalloc()/vfree() calls.
Signed-off-by: Pawel Osciak p.osc...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
CC: Pawel Osciak pa...@osciak.com
---
drivers/media/video/Kconfig |5 +
drivers/media/video/Makefile|1 +
drivers/media/video/videobuf2-vmalloc.c | 177
+++
include/media/videobuf2-vmalloc.h | 21
4 files changed, 204 insertions(+), 0 deletions(-)
create mode 100644 drivers/media/video/videobuf2-vmalloc.c
create mode 100644 include/media/videobuf2-vmalloc.h
diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index 83ce858..9351423 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -55,6 +55,11 @@ config VIDEOBUF2_CORE
config VIDEOBUF2_MEMOPS
tristate
+config VIDEOBUF2_VMALLOC
+ select VIDEOBUF2_CORE
+ select VIDEOBUF2_MEMOPS
+ tristate
+
#
# Multimedia Video device configuration
#
diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
index a97a2a0..538bee9 100644
--- a/drivers/media/video/Makefile
+++ b/drivers/media/video/Makefile
@@ -116,6 +116,7 @@ obj-$(CONFIG_VIDEO_BTCX) += btcx-risc.o
obj-$(CONFIG_VIDEOBUF2_CORE) += videobuf2-core.o
obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o
+obj-$(CONFIG_VIDEOBUF2_VMALLOC)+= videobuf2-vmalloc.o
obj-$(CONFIG_V4L2_MEM2MEM_DEV) += v4l2-mem2mem.o
diff --git a/drivers/media/video/videobuf2-vmalloc.c
b/drivers/media/video/videobuf2-vmalloc.c
new file mode 100644
index 000..3310900
--- /dev/null
+++ b/drivers/media/video/videobuf2-vmalloc.c
@@ -0,0 +1,177 @@
+/*
+ * videobuf2-vmalloc.c - vmalloc memory allocator for videobuf2
+ *
+ * Copyright (C) 2010 Samsung Electronics
+ *
+ * Author: Pawel Osciak p.osc...@samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation.
+ */
+
+#include linux/module.h
+#include linux/mm.h
+#include linux/slab.h
+#include linux/vmalloc.h
+
+#include media/videobuf2-core.h
+#include media/videobuf2-memops.h
+
+struct vb2_vmalloc_conf {
+ struct vb2_alloc_ctxalloc_ctx;
+};
+
+struct vb2_vmalloc_buf {
+ void*vaddr;
+ unsigned long size;
+ unsigned intrefcount;
+};
+
+static void *vb2_vmalloc_alloc(const struct vb2_alloc_ctx *alloc_ctx,
+ unsigned long size)
+{
+ struct vb2_vmalloc_buf *buf;
+
+ buf = kzalloc(sizeof *buf, GFP_KERNEL);
+ if (!buf)
+ return NULL;
+
+ buf-size = size;
+ buf-vaddr = vmalloc_user(buf-size);
+ if (!buf-vaddr) {
+ printk(KERN_ERR vmalloc of size %ld failed\n, buf-size);
+ kfree(buf);
+ return NULL;
+ }
+
+ buf-refcount++;
+ printk(KERN_DEBUG Allocated vmalloc buffer of size %ld at vaddr=%p\n,
+ buf-size, buf-vaddr);
+
+ return buf;
+}
+
+static void vb2_vmalloc_put(void *buf_priv)
+{
+ struct vb2_vmalloc_buf *buf = buf_priv;
+
+ buf-refcount--;
+
+ if (0 == buf-refcount) {
I think I would feel happier if refcount was of type atomic_t and you would
use
the atomic decrease-and-test operation here. I know that this is almost
certainly
called with some lock, but still...
Yes, it is called with mm_sem taken in all cases. Maybe a comment in the source
will
be enough to indicate that this variable does not need to be atomic_t type?
I can change it to atomic_t if this is really required anyway.
snip
Best regards
--
Marek Szyprowski
Samsung Poland RD Center
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] v4l: vivi: port to videobuf2
On Friday, November 19, 2010 16:55:43 Marek Szyprowski wrote:
Make vivi use videobuf2 in place of videobuf.
Signed-off-by: Pawel Osciak p.osc...@samsung.com
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
CC: Pawel Osciak pa...@osciak.com
snip
-static void vivi_stop_generating(struct file *file)
+static void vivi_stop_generating(struct vivi_dev *dev)
{
- struct vivi_dev *dev = video_drvdata(file);
struct vivi_dmaqueue *dma_q = dev-vidq;
dprintk(dev, 1, %s\n, __func__);
- if (!file-private_data)
- return;
- if (!test_and_clear_bit(0, dev-generating))
- return;
-
/* shutdown control thread */
if (dma_q-kthread) {
kthread_stop(dma_q-kthread);
dma_q-kthread = NULL;
}
- videobuf_stop(dev-vb_vidq);
- videobuf_mmap_free(dev-vb_vidq);
-}
-static int vivi_is_generating(struct vivi_dev *dev)
-{
- return test_bit(0, dev-generating);
+ /*
+ * Typical driver might need to wait here until dma engine stops.
+ * In this case we can abort imiedetly, so it's just a noop.
typo: imiedetly - immediately
snip
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by Cisco
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] v4l: videobuf2: add vmalloc allocator
On Thursday, November 25, 2010 11:26:56 Marek Szyprowski wrote:
Hello,
On Thursday, November 25, 2010 11:07 AM Hans Verkuil wrote:
On Friday, November 19, 2010 16:55:40 Marek Szyprowski wrote:
From: Pawel Osciak p.osc...@samsung.com
Add an implementation of contiguous virtual memory allocator and handling
routines for videobuf2, implemented on top of vmalloc()/vfree() calls.
Signed-off-by: Pawel Osciak p.osc...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
CC: Pawel Osciak pa...@osciak.com
---
drivers/media/video/Kconfig |5 +
drivers/media/video/Makefile|1 +
drivers/media/video/videobuf2-vmalloc.c | 177
+++
include/media/videobuf2-vmalloc.h | 21
4 files changed, 204 insertions(+), 0 deletions(-)
create mode 100644 drivers/media/video/videobuf2-vmalloc.c
create mode 100644 include/media/videobuf2-vmalloc.h
diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index 83ce858..9351423 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -55,6 +55,11 @@ config VIDEOBUF2_CORE
config VIDEOBUF2_MEMOPS
tristate
+config VIDEOBUF2_VMALLOC
+ select VIDEOBUF2_CORE
+ select VIDEOBUF2_MEMOPS
+ tristate
+
#
# Multimedia Video device configuration
#
diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
index a97a2a0..538bee9 100644
--- a/drivers/media/video/Makefile
+++ b/drivers/media/video/Makefile
@@ -116,6 +116,7 @@ obj-$(CONFIG_VIDEO_BTCX) += btcx-risc.o
obj-$(CONFIG_VIDEOBUF2_CORE) += videobuf2-core.o
obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o
+obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o
obj-$(CONFIG_V4L2_MEM2MEM_DEV) += v4l2-mem2mem.o
diff --git a/drivers/media/video/videobuf2-vmalloc.c
b/drivers/media/video/videobuf2-vmalloc.c
new file mode 100644
index 000..3310900
--- /dev/null
+++ b/drivers/media/video/videobuf2-vmalloc.c
@@ -0,0 +1,177 @@
+/*
+ * videobuf2-vmalloc.c - vmalloc memory allocator for videobuf2
+ *
+ * Copyright (C) 2010 Samsung Electronics
+ *
+ * Author: Pawel Osciak p.osc...@samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation.
+ */
+
+#include linux/module.h
+#include linux/mm.h
+#include linux/slab.h
+#include linux/vmalloc.h
+
+#include media/videobuf2-core.h
+#include media/videobuf2-memops.h
+
+struct vb2_vmalloc_conf {
+ struct vb2_alloc_ctxalloc_ctx;
+};
+
+struct vb2_vmalloc_buf {
+ void*vaddr;
+ unsigned long size;
+ unsigned intrefcount;
+};
+
+static void *vb2_vmalloc_alloc(const struct vb2_alloc_ctx *alloc_ctx,
+ unsigned long size)
+{
+ struct vb2_vmalloc_buf *buf;
+
+ buf = kzalloc(sizeof *buf, GFP_KERNEL);
+ if (!buf)
+ return NULL;
+
+ buf-size = size;
+ buf-vaddr = vmalloc_user(buf-size);
+ if (!buf-vaddr) {
+ printk(KERN_ERR vmalloc of size %ld failed\n, buf-size);
+ kfree(buf);
+ return NULL;
+ }
+
+ buf-refcount++;
+ printk(KERN_DEBUG Allocated vmalloc buffer of size %ld at vaddr=%p\n,
+ buf-size, buf-vaddr);
+
+ return buf;
+}
+
+static void vb2_vmalloc_put(void *buf_priv)
+{
+ struct vb2_vmalloc_buf *buf = buf_priv;
+
+ buf-refcount--;
+
+ if (0 == buf-refcount) {
I think I would feel happier if refcount was of type atomic_t and you would
use
the atomic decrease-and-test operation here. I know that this is almost
certainly
called with some lock, but still...
Yes, it is called with mm_sem taken in all cases. Maybe a comment in the
source will
be enough to indicate that this variable does not need to be atomic_t type?
I can change it to atomic_t if this is really required anyway.
It definitely needs to be documented. But I wonder if what you say is true
anyway:
vb2_mmap is called without mm_sem taken as far as I can tell, this calls the
mmap
callback, vb2_vmalloc_mmap calls vb2_vmalloc_vm_open directly which does
buf_refcount++.
Making refcount atomic would make it easier to prove correctness IMHO.
Anyway, this leads me to a second question: the documentation (either separate
or in the
header) should document which ops need to be called with some lock set. That
way driver
writers have some guidelines regarding locking. videobuf used to lock
internally, but
that's now moved to the driver (and rightly so), but that does mean that the
driver writer
needs pointers
Re: [RFC/PATCH v3 6/7] omap3: Export omap3isp platform device structure
Hi Felipe,
On Thursday 25 November 2010 08:02:41 Felipe Balbi wrote:
On Thu, Nov 25, 2010 at 03:54:37AM +0100, Laurent Pinchart wrote:
From: Stanimir Varbanov svarba...@mm-sol.com
The omap3isp platform device requires platform data. As the data can be
provided by a kernel module, the device can't be registered during arch
initialization.
Remove the omap3isp platform device registration from
omap_init_camera(), and export the platform device structure to let
board code register/unregister it.
instead, why don't you...
diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index d5da345..c2275d3 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -34,6 +34,8 @@
#include mux.h
#include control.h
+#include devices.h
+
#if defined(CONFIG_VIDEO_OMAP2) || defined(CONFIG_VIDEO_OMAP2_MODULE)
static struct resource cam_resources[] = {
@@ -144,16 +146,28 @@ static struct resource omap3isp_resources[] = {
}
};
-static struct platform_device omap3isp_device = {
+static void omap3isp_release(struct device *dev)
+{
+/* Zero the device structure to avoid re-initialization complaints from
+ * kobject when the device will be re-registered.
+ */
+memset(dev, 0, sizeof(*dev));
+dev-release = omap3isp_release;
+}
+
+struct platform_device omap3isp_device = {
.name = omap3isp,
.id = -1,
.num_resources = ARRAY_SIZE(omap3isp_resources),
.resource = omap3isp_resources,
+.dev = {
+.release= omap3isp_release,
+},
};
+EXPORT_SYMBOL_GPL(omap3isp_device);
static inline void omap_init_camera(void)
pass platform_data as an argument to this call ? Then remove the static
inline and export this one ?
Yes indeed, why ? :-)
I guess things like that are difficult to spot when you've had your nose on
the code for too long. Thanks for the review, I'll fix this.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 10/10] ux500: MCDE: Add platform specific data
Hi,
+
+ if (ddev-id == PRIMARY_DISPLAY_ID rotate_main) {
+ swap(width, height);
+#ifdef CONFIG_DISPLAY_GENERIC_DSI_PRIMARY_ROTATE_180_DEGREES
+ rotate = FB_ROTATE_CCW;
+#else
+ rotate = FB_ROTATE_CW;
+#endif
+ }
+
+ virtual_width = width;
+ virtual_height = height * 2;
+#ifdef CONFIG_DISPLAY_GENERIC_DSI_PRIMARY_AUTO_SYNC
+ if (ddev-id == PRIMARY_DISPLAY_ID)
+ virtual_height = height;
+#endif
+
The contents of the hardware description should really not
be configuration dependent, because that breaks booting the same
kernel on machines that have different requirements.
This is something that should be passed down from the boot loader.
The defines above is more configuration of the display driver than hardware
dependant defines.
The define CONFIG_DISPLAY_GENERIC_DSI_PRIMARY_AUTO_SYNC is not hardware
dependant.
It is used to tell mcde that it should not rely on pan_display for refreshing
the display, it should rather refresh the display itself, currently using
software triggers and it will also tell mcde that only one frame buffer should
be used.
This mode can be used if the application system is for example X.
CONFIG_DISPLAY_GENERIC_DSI_PRIMARY_ROTATE_180_DEGREES is used for some boards
that have the display rotated 90 instead of 270 degrees, so in a sense it is
hardware dependant, but it will not break the kernel. This defines is going to
be removed and replaced by one define that handles all rotations, 0, 90, 180,
270 degrees.
+static void mcde_epod_enable(void)
+{
+ /* Power on DSS mem */
+ writel(PRCMU_ENABLE_DSS_MEM, PRCM_EPOD_C_SET);
+ mdelay(PRCMU_MCDE_DELAY);
+ /* Power on DSS logic */
+ writel(PRCMU_ENABLE_DSS_LOGIC, PRCM_EPOD_C_SET);
+ mdelay(PRCMU_MCDE_DELAY);
+}
In general, try to avoid using mdelay. Keeping the CPU busy for
miliseconds
or even microseconds for no reason is just wrong.
Reasonable hardware will not require this and do the right thing
anyway.
multiple writel calls are by design strictly ordered on the bus. If
that is
not the case on your hardware, you should find a proper way to ensure
ordering and create a small wrapper for it with a comment that explains
the breakage. Better get the hardware designers to fix their crap
before
releasing a product ;-)
If there is not even a way to reorder I/O by accessing other registers,
use msleep() to let the CPU do something useful in the meantime and
complain
even more to the HW people.
Arnd
These registers are normally not accessed by the cpu and therefore it is
required to use some delays between them, however I agree that msleep is better
to use.
We are investigating how this can be removed but for now we have to keep it.
/Jimmy
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v3 6/7] omap3: Export omap3isp platform device structure
Hi, On Thu, Nov 25, 2010 at 12:17:59PM +0100, Laurent Pinchart wrote: pass platform_data as an argument to this call ? Then remove the static inline and export this one ? Yes indeed, why ? :-) I guess things like that are difficult to spot when you've had your nose on the code for too long. Thanks for the review, I'll fix this. no problem :-) -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/7] v4l: add videobuf2 Video for Linux 2 driver framework
On Thursday, November 25, 2010 10:48:39 Marek Szyprowski wrote:
Hello,
On Thursday, November 25, 2010 2:17 AM Laurent Pinchart wrote:
On Friday 19 November 2010 16:55:38 Marek Szyprowski wrote:
From: Pawel Osciak p.osc...@samsung.com
Videobuf2 is a Video for Linux 2 API-compatible driver framework for
multimedia devices. It acts as an intermediate layer between userspace
applications and device drivers. It also provides low-level, modular
memory management functions for drivers.
Videobuf2 eases driver development, reduces drivers' code size and aids in
proper and consistent implementation of V4L2 API in drivers.
Videobuf2 memory management backend is fully modular. This allows custom
memory management routines for devices and platforms with non-standard
memory management requirements to be plugged in, without changing the
high-level buffer management functions and API.
The framework provides:
- implementations of streaming I/O V4L2 ioctls and file operations
- high-level video buffer, video queue and state management functions
- video buffer memory allocation and management
Excellent job. I'm really pleased by the outstanding code quality.
Nevertheless I got a bunch of comments (I wonder if I'm known as an annoying
nit-picking reviewer in the community :-)).
I've reviewed the patch from bottom to top (starting at the header file), so
comments at the bottom can also apply to functions at the top when I got
tired
repeating the same information. Sorry about that weird order.
Feel free to disagree with my comments, I've probably been overzealous
during
the review to make sure everything I had a doubt about would be properly
addressed.
It's now past 2AM and I don't have enough courage to review the other
patches.
I'm afraid they will have to wait until tomorrow. If they're of the same
quality as this one I'm sure they will be good.
Thanks for your hard work! I can imagine how much time you had to spend on
catching
all these things.
BTW, where's the patch for Documentation/video4linux ? :-)
Well, I hope it will be ready one day ;) What kind of documentation do you
expect
there? Vb2 structures and functions are already documented directly in the
source.
One last comment, why was the decision to remove all locking from vb2 taken
?
The idea came from Hans after posting version 1 and I really like it. It
simplifies
a lot of things in the drivers. The original idea of irq_spinlock and vb_lock
was
horrible. Having more than one queue in the driver meant that you need to
make a
lot of nasty hacks to ensure proper locking, because in some cases you had to
take
locks from all queues. The irq_lock usage was even worse. Videobuf used it to
silently mess around the buffers that have been already queued to the
hardware.
Now, after Hans changes to v4l2 core (BKL removal in favor of simple mutex)
almost
all this mess can be simply removed. Proper locking can be easily ensured by
the
new mutex in the v4l2 core. There is no need to have any locks inside vb2. By
removing irq_lock the design of the drivers is easier to understand, because
there
is no implicit actions on buffers that are processed by the hardware.
The BKL removal stuff has nothing to do with this. The main reason is just to
simplify
locking in drivers. Things get really complex if you have multiple nested locks.
Moving locking out of vb2 and into the driver gives the control back to the
driver.
+/**
+ * vb2_has_consumers() - return true if the userspace is waiting for a
buffer + * @q:videobuf2 queue
+ *
+ * This function returns true if a userspace application is waiting for a
buffer + * to be ready to dequeue (on which a hardware operation has been
finished). + */
+bool vb2_has_consumers(struct vb2_queue *q)
+{
+ return waitqueue_active(q-done_wq);
+}
+EXPORT_SYMBOL_GPL(vb2_has_consumers);
What is this for ? Do you have use cases in mind ?
The vivi driver is using it, but frankly it should be redesigned to use some
other check.
It is a bit of an odd function. I'm also not sure how useful it is.
+ * @plane_setup: called before memory allocation num_planes times;
+ * driver should return the required size of plane
number
+ * plane_no
+ * @unlock: release any locks taken while calling vb2
functions;
+ * it is called before poll_wait function in
vb2_poll
+ * implementation; required to avoid deadlock when
vb2_poll
+ * function waits for a buffer
+ * @lock:reacquire all locks released in the previous
callback;
+ * required to continue operation after sleeping in
+ * poll_wait function
Those names were not very
RE: [PATCH 02/10] MCDE: Add configuration registers
Hi,
This patch adds the configuration registers found in MCDE.
+
+#define MCDE_VAL2REG(__reg, __fld, __val) \
+ (((__val) __reg##_##__fld##_SHIFT)
__reg##_##__fld##_MASK)
+#define MCDE_REG2VAL(__reg, __fld, __val) \
+ (((__val) __reg##_##__fld##_MASK)
__reg##_##__fld##_SHIFT)
+
+#define MCDE_CR 0x
+#define MCDE_CR_DSICMD2_EN_V1_SHIFT 0
+#define MCDE_CR_DSICMD2_EN_V1_MASK 0x0001
+#define MCDE_CR_DSICMD2_EN_V1(__x) \
+ MCDE_VAL2REG(MCDE_CR, DSICMD2_EN_V1, __x)
+#define MCDE_CR_DSICMD1_EN_V1_SHIFT 1
+#define MCDE_CR_DSICMD1_EN_V1_MASK 0x0002
+#define MCDE_CR_DSICMD1_EN_V1(__x) \
+ MCDE_VAL2REG(MCDE_CR, DSICMD1_EN_V1, __x)
+#define MCDE_CR_DSI0_EN_V3_SHIFT 0
+#define MCDE_CR_DSI0_EN_V3_MASK 0x0001
+#define MCDE_CR_DSI0_EN_V3(__x) \
+ MCDE_VAL2REG(MCDE_CR, DSI0_EN_V3, __x)
This looks all rather unreadable. The easiest way is usually to just
define the bit mask, i.e. the second line of each register definition,
which you can use to mask the bits. It's also useful to indent the
lines
so you can easily tell the register offsets apart from the contents:
#define MCDE_CR 0x
#define MCDE_CR_DSICMD2_EN_V1 0x0001
#define MCDE_CR_DSICMD1_EN_V1 0x0002
Some people prefer to express all this in C instead of macros:
struct mcde_registers {
enum {
mcde_cr_dsicmd2_en = 0x0001,
mcde_cr_dsicmd1_en = 0x0002,
...
} cr;
enum {
mcde_conf0_syncmux0 = 0x0001,
...
} conf0;
...
};
This gives you better type safety, but which one you choose is your
decision.
Arnd
All these header files,
Configuration, pixel processing, formatter, dsi link registers are auto
generated from an xml file.
This is made because there are many registers which a prone to change for new
hardware revisions and there is a possibility to exclude registers that are not
used.
The chance of manual errors are less this way.
I also believe that these helper macros makes the source code easier to read.
For example.
#define MCDE_CR_DSICMD2_EN_V1_SHIFT 0
#define MCDE_CR_DSICMD2_EN_V1_MASK 0x0001
#define MCDE_CR_DSICMD2_EN_V1(__x) \
MCDE_VAL2REG(MCDE_CR, DSICMD2_EN_V1, __x)
Writing a single field in the register MCDE_CR can e.g. be done like this:
mcde_wfld(MCDE_CR, DSICMD1_EN_V1, true);
and for a whole register (a totally other register but just to show):
mcde_wreg(MCDE_ROTACONF + chnl_id * MCDE_ROTACONF_GROUPOFFSET,
MCDE_ROTACONF_ROTBURSTSIZE_ENUM(8W) |
MCDE_ROTACONF_ROTBURSTSIZE_HW(1) |
MCDE_ROTACONF_ROTDIR(regs-rotdir) |
MCDE_ROTACONF_STRIP_WIDTH_ENUM(16PIX) |
MCDE_ROTACONF_RD_MAXOUT_ENUM(4_REQ) |
MCDE_ROTACONF_WR_MAXOUT_ENUM(8_REQ));
I agree that the header files looks a bit unreadable I will try indent as you
suggested I am just worried about the file size. (Patch limit 100kbyte)
/Jimmy
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v6 11/12] v4l: Make video_device inherit from media_entity
On Thursday, November 25, 2010 03:28:18 Laurent Pinchart wrote:
V4L2 devices are media entities. As such they need to inherit from
(include) the media_entity structure.
When registering/unregistering the device, the media entity is
automatically registered/unregistered. The entity is acquired on device
open and released on device close.
Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
Signed-off-by: Sakari Ailus sakari.ai...@maxwell.research.nokia.com
---
Documentation/video4linux/v4l2-framework.txt | 38 +++--
drivers/media/video/v4l2-dev.c | 47
+++---
include/media/v4l2-dev.h |7
3 files changed, 84 insertions(+), 8 deletions(-)
snip
diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
index 035db52..511e1ee 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -278,6 +278,9 @@ static int v4l2_mmap(struct file *filp, struct
vm_area_struct *vm)
static int v4l2_open(struct inode *inode, struct file *filp)
{
struct video_device *vdev;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ struct media_entity *entity = NULL;
+#endif
int ret = 0;
/* Check if the video device is available */
@@ -291,6 +294,16 @@ static int v4l2_open(struct inode *inode, struct file
*filp)
/* and increase the device refcount */
video_get(vdev);
mutex_unlock(videodev_lock);
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ if (vdev-v4l2_dev vdev-v4l2_dev-mdev) {
+ entity = media_entity_get(vdev-entity);
+ if (!entity) {
+ ret = -EBUSY;
+ video_put(vdev);
+ return ret;
+ }
+ }
+#endif
if (vdev-fops-open) {
if (vdev-lock)
mutex_lock(vdev-lock);
@@ -303,8 +316,13 @@ static int v4l2_open(struct inode *inode, struct file
*filp)
}
/* decrease the refcount in case of an error */
- if (ret)
+ if (ret) {
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ if (vdev-v4l2_dev vdev-v4l2_dev-mdev)
+ media_entity_put(entity);
+#endif
video_put(vdev);
+ }
return ret;
}
@@ -321,7 +339,10 @@ static int v4l2_release(struct inode *inode, struct file
*filp)
if (vdev-lock)
mutex_unlock(vdev-lock);
}
-
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ if (vdev-v4l2_dev vdev-v4l2_dev-mdev)
+ media_entity_put(vdev-entity);
+#endif
/* decrease the refcount unconditionally since the release()
return value is ignored. */
video_put(vdev);
@@ -558,12 +579,25 @@ int __video_register_device(struct video_device *vdev,
int type, int nr,
if (nr != -1 nr != vdev-num warn_if_nr_in_use)
printk(KERN_WARNING %s: requested %s%d, got %s\n, __func__,
name_base, nr, video_device_node_name(vdev));
-
- /* Part 5: Activate this minor. The char device can now be used. */
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ /* Part 5: Register the entity. */
+ if (vdev-v4l2_dev vdev-v4l2_dev-mdev) {
+ vdev-entity.type = MEDIA_ENTITY_TYPE_NODE_V4L;
+ vdev-entity.name = vdev-name;
+ vdev-entity.v4l.major = VIDEO_MAJOR;
+ vdev-entity.v4l.minor = vdev-minor;
+ ret = media_device_register_entity(vdev-v4l2_dev-mdev,
+ vdev-entity);
+ if (ret 0)
+ printk(KERN_ERR error\n); /* TODO */
Was this forgotten, or will this be fixed in the next version? It looks
out-of-place...
+ }
+#endif
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by Cisco
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 08/10] MCDE: Add frame buffer device
Hi,
On Wednesday 10 November 2010, Jimmy Rubin wrote:
+
+static struct platform_device mcde_fb_device = {
+ .name = mcde_fb,
+ .id = -1,
+};
Do not introduce new static devices. We are trying to remove them and
they will stop working. Why do you even need a device here if there is
only one of them?
+struct fb_info *mcde_fb_create(struct mcde_display_device *ddev,
+ u16 w, u16 h, u16 vw, u16 vh, enum mcde_ovly_pix_fmt
pix_fmt,
+ u32 rotate)
+{
Here you have another device, which you could just use!
I will do that.
+/* Overlay fbs' platform device */
+static int mcde_fb_probe(struct platform_device *pdev)
+{
+ return 0;
+}
+
+static int mcde_fb_remove(struct platform_device *pdev)
+{
+ return 0;
+}
+
+static struct platform_driver mcde_fb_driver = {
+ .probe = mcde_fb_probe,
+ .remove = mcde_fb_remove,
+ .driver = {
+ .name = mcde_fb,
+ .owner = THIS_MODULE,
+ },
+};
+
+/* MCDE fb init */
+
+int __init mcde_fb_init(void)
+{
+ int ret;
+
+ ret = platform_driver_register(mcde_fb_driver);
+ if (ret)
+ goto fb_driver_failed;
+ ret = platform_device_register(mcde_fb_device);
+ if (ret)
+ goto fb_device_failed;
+
+ goto out;
+fb_device_failed:
+ platform_driver_unregister(mcde_fb_driver);
+fb_driver_failed:
+out:
+ return ret;
+}
+
+void mcde_fb_exit(void)
+{
+ platform_device_unregister(mcde_fb_device);
+ platform_driver_unregister(mcde_fb_driver);
+}
This appears to be an entirely useless registration for something that
does not exist and that you are not using anywhere ...
Will look into this and remove it.
+
+#include linux/fb.h
+#include linux/ioctl.h
+#if !defined(__KERNEL__) !defined(_KERNEL)
+#include stdint.h
+#else
+#include linux/types.h
+#endif
+
+#ifdef __KERNEL__
+#include mcde_dss.h
+#endif
+
+#ifdef __KERNEL__
+#define to_mcde_fb(x) ((struct mcde_fb *)(x)-par)
Everything in this file is enclosed in #ifdef __KERNEL__, and the file
is not even exported. You can remove the #ifdef and the #else path
everywhere AFAICT.
Agree, no IOCTLs there for the moment so only kernel include.
/Jimmy
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 09/10] MCDE: Add build files and bus
Hi Arnd,
Comments inline.
-Original Message-
From: Arnd Bergmann [mailto:a...@arndb.de]
Sent: den 12 november 2010 17:23
To: linux-arm-ker...@lists.infradead.org
Cc: Jimmy RUBIN; linux-fb...@vger.kernel.org; linux-
me...@vger.kernel.org; Dan JOHANSSON; Linus WALLEIJ
Subject: Re: [PATCH 09/10] MCDE: Add build files and bus
On Wednesday 10 November 2010, Jimmy Rubin wrote:
This patch adds support for the MCDE, Memory-to-display controller,
found in the ST-Ericsson ux500 products.
This patch adds the necessary build files for MCDE and the bus that
all displays are connected to.
Can you explain why you need a bus for this?
The idea of the bus is to make it easier to add new panel/display support using
the normal device/driver model. Boards add devices at init, and drivers are
selected in config. If we were to model the real physical bus structure it
would be very complex, hard to use. We use our own bus, but it's really a
virtual bus for adding display devices and drivers to MCDE host. Is there any
generic virtual bus type we should use instead of our own type? If you have
another idea of how to add display devices and drivers without MCDE code
modifications, please let us know. A virtual bus just give us a nice framework
to do this.
With the code you currently have, there is only a single driver
associated
with this bus type, and also just a single device that gets registered
here!
And yes, currently there's only one single driver. But there's a lot more
coming in the pipe. Like some LCD, DPI, DBI, DSI display drivers. And once all
different u8500 boards are pushed, there will be multiple boards registering
devices with different setups. But one thing at a time.
+static int __init mcde_subsystem_init(void)
+{
+ int ret;
+ pr_info(MCDE subsystem init begin\n);
+
+ /* MCDE module init sequence */
+ ret = mcde_init();
+ if (ret)
+ return ret;
+ ret = mcde_display_init();
+ if (ret)
+ goto mcde_display_failed;
+ ret = mcde_dss_init();
+ if (ret)
+ goto mcde_dss_failed;
+ ret = mcde_fb_init();
+ if (ret)
+ goto mcde_fb_failed;
+ pr_info(MCDE subsystem init done\n);
+
+ return 0;
+mcde_fb_failed:
+ mcde_dss_exit();
+mcde_dss_failed:
+ mcde_display_exit();
+mcde_display_failed:
+ mcde_exit();
+ return ret;
+}
Splitting up the module into four sub-modules and then initializing
everything from one place indicates that something is done wrong
on a global scale.
If you indeed need a bus, that should be a separate module that gets
loaded first and then has the other modules build on top of.
Yes, that's the general idea. We don't completely understand the correct way of
making sure how one module gets loaded before another, without selecting init
level, like the fs_initcall below you commented on. I guess most of our
submodules should be initialized during subsys_init. But we have not found how
to specify the module init order inside subsys init level. Maybe you can shed
some light on this? Makefile order seems like a fragile way of defining init
order dependencies.
Do you think static init calls from machine subsys init are a better solution?
I'm not sure how the other parts layer on top of one another, can you
provide some more insight?
++
| mcde_fb/mcde_kms/mcde_v4l2 |
+---++
|mcde_dss |
+ +---+
| | disp drvs |
+---+---+
|mcde hw|
+---+
| HW |
+---+
From what I understood so far, you have a single multi-channel display
controller (mcde_hw.c) that drives the hardware.
Each controller can have multiple frame buffers attached to it, which
in turn can have multiple displays attached to each of them, but your
current configuration only has one of each, right?
Correct, channels A/B (crtcs) can have two blended framebuffers plus
background color, channels C0/C1 can have one framebuffer.
Right now you have a single top-level bus device for the displays,
maybe that can be integrated into the controller so the displays are
properly rooted below the hardware that drives them.
Not sure I understand 100%. Routing is independent of bus structure. Routing
could change dynamically during runtime reconfiguration using future KMS for
example. Bus is only for connecting display devices with drivers. Of course we
could have one bus per port/connector. But then the code would be more complex
and we would end up with one bus/device/driver per connector (or in some rare
cases 2-4 on DBI/DSI connectors).
The frame buffer device also looks weird. Right now you only seem
to have a single frame buffer registered to a driver in the same
module. Is that frame buffer not dependent on a controller?
MCDE framebuffers are only depending on MCDE DSS. DSS
[PATCH] [media] msp3400: fix mute audio regression
This patch applies 2.6.37 commit 0310871d8f71da4ad8643687fbc40f219a0dac4d to the
2.6.36 stable series.
It was just too late to be included in 2.6.36 at the time, but it should be
added
to 2.6.36 since it fixes broken audio on this fairly popular audio receiver
chip.
Regards,
Hans
From 0310871d8f71da4ad8643687fbc40f219a0dac4d Mon Sep 17 00:00:00 2001
Message-Id:
0310871d8f71da4ad8643687fbc40f219a0dac4d.1290686265.git.hverk...@xs4all.nl
From: Hans Verkuil hverk...@xs4all.nl
Date: Sun, 17 Oct 2010 07:24:20 -0300
Subject: [PATCH] [media] msp3400: fix mute audio regression
To: linux-media@vger.kernel.org
The switch to the new control framework caused a regression where the audio was
no longer unmuted after the carrier scan finished.
The original code attempted to set the volume control to its current value in
order to have the set-volume control code to be called that handles the volume
and muting. However, the framework will not call that code unless the new volume
value is different from the old.
Instead we now call msp_s_ctrl directly.
It is a bit of a hack: we really need a v4l2_ctrl_refresh_ctrl function for this
(or something along those lines).
Thanks to Andy Walls for bisecting this and to Shane Shrybman for reporting it!
Reported-by: Shane Shrybman shryb...@teksavvy.com
Thanks-to: Andy Walls awa...@md.metrocast.net
Signed-off-by: Hans Verkuil hverk...@xs4all.nl
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
diff --git a/drivers/media/video/msp3400-driver.c
b/drivers/media/video/msp3400-driver.c
index fe18a0a..b1763ac 100644
--- a/drivers/media/video/msp3400-driver.c
+++ b/drivers/media/video/msp3400-driver.c
@@ -381,7 +381,12 @@ static int msp_s_ctrl(struct v4l2_ctrl *ctrl)
void msp_update_volume(struct msp_state *state)
{
- v4l2_ctrl_s_ctrl(state-volume, v4l2_ctrl_g_ctrl(state-volume));
+ /* Force an update of the volume/mute cluster */
+ v4l2_ctrl_lock(state-volume);
+ state-volume-val = state-volume-cur.val;
+ state-muted-val = state-muted-cur.val;
+ msp_s_ctrl(state-volume);
+ v4l2_ctrl_unlock(state-volume);
}
/* --- v4l2 ioctls --- */
--
1.7.0.4
--
Hans Verkuil - video4linux developer - sponsored by Cisco
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v6 03/12] media: Entities, pads and links
On Thu, Nov 25, 2010 at 03:28:10AM +0100, Laurent Pinchart wrote:
+Links have flags that describe the link capabilities and state.
+ MEDIA_LINK_FLAG_ACTIVE indicates that the link is active and can be
+ used to transfer media data. When two or more links target a sink pad,
+ only one of them can be active at a time.
Is this supposed to reflect the current state (if the link is carrying
data right now) or if it's possible for the link to carry data?
+struct media_entity {
+ struct list_head list;
+ struct media_device *parent;/* Media device this entity belongs to*/
+ u32 id; /* Entity ID, unique in the parent media
+ * device context */
+ const char *name; /* Entity name */
+ u32 type; /* Entity type (MEDIA_ENTITY_TYPE_*) */
+ u32 revision; /* Entity revision, driver specific */
+ unsigned long flags;/* Entity flags (MEDIA_ENTITY_FLAG_*) */
+ u32 group_id; /* Entity group ID */
+
+ u16 num_pads; /* Number of input and output pads */
+ u16 num_links; /* Number of existing links, both active
+ * and inactive */
+ u16 num_backlinks; /* Number of backlinks */
+ u16 max_links; /* Maximum number of links */
+
+ struct media_pad *pads; /* Pads array (num_pads elements) */
+ struct media_link *links; /* Links array (max_links elements)*/
Hrm. This is getting kind of large, especially considering the volume
of data we're holding per node and link already in ASoC. On the other
hand we may over time be able to refactor some of the existing stuff
(especially the link management) to use this structure.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v6 03/12] [alsa-devel] media: Entities, pads and links
On Thu, Nov 25, 2010 at 10:38:05AM +0100, Clemens Ladisch wrote:
In USB and HD audio devices, all links are immutable, and the routing
is controlled by 'selector' entities that activate exactly one of their
input pads. In userspace, this entity shows up as a mixer control.
I guess it would be possible to map the ACTIVE flag onto these controls.
Ditto for ASoC, mostly.
Alternatively, entities can have 'mute' mixer controls associated with
their pads. In this case, multiple unmuted inputs would be mixed
together.
ALSA has PCM and MIDI devices, and several types of mixer controls.
(It also has hardware dependent and timer devices, but I don't think
these would need topology information.) So we need at least these:
MEDIA_ENTITY_TYPE_NODE_ALSA_PCM
MEDIA_ENTITY_TYPE_NODE_ALSA_MIDI
MEDIA_ENTITY_TYPE_SUBDEV_ALSA_CONTROL
Furthermore, topology information is also needed for entities not
associated with a mixer control, such as microphones, speakers, jacks/
connectors, and effect units. These entities are defined in the USB and
HD audio specifications, but are not yet handled by ALSA.
All this and more in the embedded case - digital audio link nodes and DSP
I/O nodes (could possibly do those as digital audio ones) spring to
mind. Also bear in mind that embedded devices can get *very* large - a
mobile phone audio system can have of the order of 100 nodes in the
graph.
ALSA devices are not addressed by their device node but with card/device/
subdevice numbers; mixer controls have numeric IDs, unique per card:
struct {
int card;
int device;
int subdevice;
} alsa_device;
struct {
int card;
int numid;
} alsa_control;
For the embedded stuff we also have a bunch of stuff in the graph which
may not be visible to userspace at all at present and would just have a
string based identifier.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v6 05/12] media: Reference count and power handling
On Thu, Nov 25, 2010 at 03:28:12AM +0100, Laurent Pinchart wrote: If the entity is of node type, the power change is distributed to all connected entities. For non-nodes it only affects that very node. A mutex is used to serialise access to the entity graph. ASoC has its own power management stuff which doesn't *quite* map onto this one as-is. The power determination stuff is essentially identical (and this is actually nicer) but we have a separate postprocessing stage that actually applies the changes in a sequence which minimises audible issues caused by doing things in a bad order (eg, power down a PGA before you turn off inputs). This is noddy enough to implement, though - we just need a pre and post run notifications to set up and implement the changes I think. BTW, I notice you've not CCed the ALSA list, Liam, Takashi or Jaroslav on any of this - might be good. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v6 09/12] media: Entity locking and pipeline management
On Thu, Nov 25, 2010 at 03:28:16AM +0100, Laurent Pinchart wrote: Link states must not be modified while streaming is in progress on a graph they belong or connect to. The entity locking API helps drivers enforcing that requirement. This is not desirable for embedded audio - for example, it is very common to wish to switch from headphone to speaker and back mid playback, or to move between headset, speakerphone and handset modes on a phone. This is normally implemented by reconfiguring the output paths at runtime without interrupting applications. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v6 00/12] Media controller (core and V4L2)
On Thu, Nov 25, 2010 at 03:28:07AM +0100, Laurent Pinchart wrote: I want to emphasize that the media controller API does *not* replace the V4L, DVB or ALSA APIs. It complements them. Overall this looks relatively good and should be mappable onto the embedded audio stack. I'd need to sit down and actually do that to go into too much specific detail but the only real issue I see is working out how the control aspects of this interact with the control in ALSA. The simplest thing would be to have the userspace API be read only for ALSA devices, I guess. One potential issue -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v6 11/12] v4l: Make video_device inherit from media_entity
Hi Hans,
Thanks for the review.
On Thursday 25 November 2010 12:38:15 Hans Verkuil wrote:
On Thursday, November 25, 2010 03:28:18 Laurent Pinchart wrote:
V4L2 devices are media entities. As such they need to inherit from
(include) the media_entity structure.
When registering/unregistering the device, the media entity is
automatically registered/unregistered. The entity is acquired on device
open and released on device close.
Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
Signed-off-by: Sakari Ailus sakari.ai...@maxwell.research.nokia.com
---
Documentation/video4linux/v4l2-framework.txt | 38
+++-- drivers/media/video/v4l2-dev.c |
47 +++--- include/media/v4l2-dev.h
|7
3 files changed, 84 insertions(+), 8 deletions(-)
snip
diff --git a/drivers/media/video/v4l2-dev.c
b/drivers/media/video/v4l2-dev.c index 035db52..511e1ee 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
[snip]
@@ -558,12 +579,25 @@ int __video_register_device(struct video_device
*vdev, int type, int nr,
if (nr != -1 nr != vdev-num warn_if_nr_in_use)
printk(KERN_WARNING %s: requested %s%d, got %s\n, __func__,
name_base, nr, video_device_node_name(vdev));
-
- /* Part 5: Activate this minor. The char device can now be used. */
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ /* Part 5: Register the entity. */
+ if (vdev-v4l2_dev vdev-v4l2_dev-mdev) {
+ vdev-entity.type = MEDIA_ENTITY_TYPE_NODE_V4L;
+ vdev-entity.name = vdev-name;
+ vdev-entity.v4l.major = VIDEO_MAJOR;
+ vdev-entity.v4l.minor = vdev-minor;
+ ret = media_device_register_entity(vdev-v4l2_dev-mdev,
+ vdev-entity);
+ if (ret 0)
+ printk(KERN_ERR error\n); /* TODO */
Was this forgotten, or will this be fixed in the next version? It looks
out-of-place...
OOPS. I totally forgot about that one. I'll fix it for the next version.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v6 02/12] media: Media device
Hi Clemens,
Thanks for the review.
On Thursday 25 November 2010 10:33:02 Clemens Ladisch wrote:
Laurent Pinchart wrote:
+struct media_device {
...
+ u8 model[32];
+ u8 serial[40];
+ u8 bus_info[32];
All drivers and userspace applications have to treat this as char[], so
why u8[]?
Good question. I've copied the V4L2 practice of using u8 (or __u8) for fixed-
length strings in structures. I can't think of any reason for that.
I will replace u8 with char unless someone comes up with a good reason to keep
u8.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/6] s5p-fimc driver fixes for 2.6.37
Hello, the following are a few bugfix patches for s5p-fimc driver. Two of them are related to recent BKL removal efforts and others are minor fixes for V4L2 API conformance and correct handling of different IP revisions. The patch series contains: [PATCH 1/6] [media] s5p-fimc: BKL lock removal - compilation fix [PATCH 2/6] [media] s5p-fimc: Fix vidioc_g_crop/cropcap on camera sensor [PATCH 3/6] [media] s5p-fimc: Explicitly add required header file [PATCH 4/6] [media] s5p-fimc: Convert m2m driver to unlocked_ioctl [PATCH 5/6] [media] s5p-fimc: Use correct fourcc code for 32-bit RGB format [PATCH 6/6] [media] s5p-fimc: Fix output DMA handling in S5PV310 IP revisions It has been rebased onto linuxtv/staging-2.6.37-rc1 branch at git://linuxtv.org/media_tree.git. Thanks, Sylwester -- Sylwester Nawrocki Linux Platform Group Samsung Poland RD Center -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] [media] s5p-fimc: BKL lock removal - compilation fix
Adapt to recent videobuf_queue_dma_contig_init signature change.
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin kyungmin.p...@samsung.com
---
drivers/media/video/s5p-fimc/fimc-capture.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/s5p-fimc/fimc-capture.c
b/drivers/media/video/s5p-fimc/fimc-capture.c
index e8f13d3..4c70fba 100644
--- a/drivers/media/video/s5p-fimc/fimc-capture.c
+++ b/drivers/media/video/s5p-fimc/fimc-capture.c
@@ -785,7 +785,7 @@ int fimc_register_capture_device(struct fimc_dev *fimc)
videobuf_queue_dma_contig_init(vid_cap-vbq, fimc_qops,
vid_cap-v4l2_dev.dev, fimc-irqlock,
V4L2_BUF_TYPE_VIDEO_CAPTURE, V4L2_FIELD_NONE,
- sizeof(struct fimc_vid_buffer), (void *)ctx);
+ sizeof(struct fimc_vid_buffer), (void *)ctx, NULL);
ret = video_register_device(vfd, VFL_TYPE_GRABBER, -1);
if (ret) {
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/6] [media] s5p-fimc: Fix vidioc_g_crop/cropcap on camera sensor
Create separate vidioc_g_crop/vidioc_s_crop handlers for capture
video node and so image cropping parameters are properly queried
at FIMC input (image sensor) and not at FIMC output.
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/video/s5p-fimc/fimc-capture.c | 48 +-
drivers/media/video/s5p-fimc/fimc-core.c| 26 +++---
drivers/media/video/s5p-fimc/fimc-core.h|4 --
3 files changed, 59 insertions(+), 19 deletions(-)
diff --git a/drivers/media/video/s5p-fimc/fimc-capture.c
b/drivers/media/video/s5p-fimc/fimc-capture.c
index 4c70fba..c020bf6 100644
--- a/drivers/media/video/s5p-fimc/fimc-capture.c
+++ b/drivers/media/video/s5p-fimc/fimc-capture.c
@@ -652,6 +652,50 @@ static int fimc_cap_s_ctrl(struct file *file, void *priv,
return ret;
}
+static int fimc_cap_cropcap(struct file *file, void *fh,
+ struct v4l2_cropcap *cr)
+{
+ struct fimc_frame *f;
+ struct fimc_ctx *ctx = fh;
+ struct fimc_dev *fimc = ctx-fimc_dev;
+
+ if (cr-type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ return -EINVAL;
+
+ if (mutex_lock_interruptible(fimc-lock))
+ return -ERESTARTSYS;
+
+ f = ctx-s_frame;
+ cr-bounds.left = 0;
+ cr-bounds.top = 0;
+ cr-bounds.width= f-o_width;
+ cr-bounds.height = f-o_height;
+ cr-defrect = cr-bounds;
+
+ mutex_unlock(fimc-lock);
+ return 0;
+}
+
+static int fimc_cap_g_crop(struct file *file, void *fh, struct v4l2_crop *cr)
+{
+ struct fimc_frame *f;
+ struct fimc_ctx *ctx = file-private_data;
+ struct fimc_dev *fimc = ctx-fimc_dev;
+
+
+ if (mutex_lock_interruptible(fimc-lock))
+ return -ERESTARTSYS;
+
+ f = ctx-s_frame;
+ cr-c.left = f-offs_h;
+ cr-c.top = f-offs_v;
+ cr-c.width = f-width;
+ cr-c.height= f-height;
+
+ mutex_unlock(fimc-lock);
+ return 0;
+}
+
static int fimc_cap_s_crop(struct file *file, void *fh,
struct v4l2_crop *cr)
{
@@ -716,9 +760,9 @@ static const struct v4l2_ioctl_ops fimc_capture_ioctl_ops =
{
.vidioc_g_ctrl = fimc_vidioc_g_ctrl,
.vidioc_s_ctrl = fimc_cap_s_ctrl,
- .vidioc_g_crop = fimc_vidioc_g_crop,
+ .vidioc_g_crop = fimc_cap_g_crop,
.vidioc_s_crop = fimc_cap_s_crop,
- .vidioc_cropcap = fimc_vidioc_cropcap,
+ .vidioc_cropcap = fimc_cap_cropcap,
.vidioc_enum_input = fimc_cap_enum_input,
.vidioc_s_input = fimc_cap_s_input,
diff --git a/drivers/media/video/s5p-fimc/fimc-core.c
b/drivers/media/video/s5p-fimc/fimc-core.c
index 2e7c547..f45970d 100644
--- a/drivers/media/video/s5p-fimc/fimc-core.c
+++ b/drivers/media/video/s5p-fimc/fimc-core.c
@@ -1115,7 +1115,7 @@ static int fimc_m2m_s_ctrl(struct file *file, void *priv,
return 0;
}
-int fimc_vidioc_cropcap(struct file *file, void *fh,
+static int fimc_m2m_cropcap(struct file *file, void *fh,
struct v4l2_cropcap *cr)
{
struct fimc_frame *frame;
@@ -1139,7 +1139,7 @@ int fimc_vidioc_cropcap(struct file *file, void *fh,
return 0;
}
-int fimc_vidioc_g_crop(struct file *file, void *fh, struct v4l2_crop *cr)
+static int fimc_m2m_g_crop(struct file *file, void *fh, struct v4l2_crop *cr)
{
struct fimc_frame *frame;
struct fimc_ctx *ctx = file-private_data;
@@ -1167,22 +1167,22 @@ int fimc_try_crop(struct fimc_ctx *ctx, struct
v4l2_crop *cr)
struct fimc_frame *f;
u32 min_size, halign;
- f = (cr-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) ?
- ctx-s_frame : ctx-d_frame;
-
if (cr-c.top 0 || cr-c.left 0) {
v4l2_err(fimc-m2m.v4l2_dev,
doesn't support negative values for top left\n);
return -EINVAL;
}
- f = ctx_get_frame(ctx, cr-type);
- if (IS_ERR(f))
- return PTR_ERR(f);
+ if (cr-type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ f = (ctx-state FIMC_CTX_CAP) ? ctx-s_frame : ctx-d_frame;
+ else if (cr-type == V4L2_BUF_TYPE_VIDEO_OUTPUT
+ctx-state FIMC_CTX_M2M)
+ f = ctx-s_frame;
+ else
+ return -EINVAL;
- min_size = (cr-type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
- ? fimc-variant-min_inp_pixsize
- : fimc-variant-min_out_pixsize;
+ min_size = (f == ctx-s_frame) ?
+ fimc-variant-min_inp_pixsize : fimc-variant-min_out_pixsize;
if (ctx-state FIMC_CTX_M2M) {
if (fimc-id == 1 fimc-variant-pix_hoff)
@@ -1285,9 +1285,9 @@ static const struct v4l2_ioctl_ops
[PATCH 3/6] [media] s5p-fimc: Explicitly add required header file
Reported by: Dan Carpenter erro...@gmail.com Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/video/s5p-fimc/fimc-core.h |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/drivers/media/video/s5p-fimc/fimc-core.h b/drivers/media/video/s5p-fimc/fimc-core.h index afafebc..6137340 100644 --- a/drivers/media/video/s5p-fimc/fimc-core.h +++ b/drivers/media/video/s5p-fimc/fimc-core.h @@ -13,13 +13,15 @@ /*#define DEBUG*/ +#include linux/sched.h #include linux/types.h +#include linux/videodev2.h #include media/videobuf-core.h #include media/v4l2-device.h #include media/v4l2-mem2mem.h #include media/v4l2-mediabus.h #include media/s3c_fimc.h -#include linux/videodev2.h + #include regs-fimc.h #define err(fmt, args...) \ -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/6] [media] s5p-fimc: Convert m2m driver to unlocked_ioctl
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/video/s5p-fimc/fimc-core.c | 22 --
1 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/media/video/s5p-fimc/fimc-core.c
b/drivers/media/video/s5p-fimc/fimc-core.c
index f45970d..fa82a2a 100644
--- a/drivers/media/video/s5p-fimc/fimc-core.c
+++ b/drivers/media/video/s5p-fimc/fimc-core.c
@@ -983,6 +983,7 @@ int fimc_vidioc_queryctrl(struct file *file, void *priv,
{
struct fimc_ctx *ctx = priv;
struct v4l2_queryctrl *c;
+ int ret = -EINVAL;
c = get_ctrl(qc-id);
if (c) {
@@ -990,10 +991,14 @@ int fimc_vidioc_queryctrl(struct file *file, void *priv,
return 0;
}
- if (ctx-state FIMC_CTX_CAP)
- return v4l2_subdev_call(ctx-fimc_dev-vid_cap.sd,
+ if (ctx-state FIMC_CTX_CAP) {
+ if (mutex_lock_interruptible(ctx-fimc_dev-lock))
+ return -ERESTARTSYS;
+ ret = v4l2_subdev_call(ctx-fimc_dev-vid_cap.sd,
core, queryctrl, qc);
- return -EINVAL;
+ mutex_unlock(ctx-fimc_dev-lock);
+ }
+ return ret;
}
int fimc_vidioc_g_ctrl(struct file *file, void *priv,
@@ -1233,6 +1238,9 @@ static int fimc_m2m_s_crop(struct file *file, void *fh,
struct v4l2_crop *cr)
f = (cr-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) ?
ctx-s_frame : ctx-d_frame;
+ if (mutex_lock_interruptible(fimc-lock))
+ return -ERESTARTSYS;
+
spin_lock_irqsave(ctx-slock, flags);
if (~ctx-state (FIMC_SRC_FMT | FIMC_DST_FMT)) {
/* Check to see if scaling ratio is within supported range */
@@ -1241,9 +1249,9 @@ static int fimc_m2m_s_crop(struct file *file, void *fh,
struct v4l2_crop *cr)
else
ret = fimc_check_scaler_ratio(cr-c, ctx-s_frame);
if (ret) {
- spin_unlock_irqrestore(ctx-slock, flags);
v4l2_err(fimc-m2m.v4l2_dev, Out of scaler range);
- return -EINVAL;
+ ret = -EINVAL;
+ goto scr_unlock;
}
}
ctx-state |= FIMC_PARAMS;
@@ -1253,7 +1261,9 @@ static int fimc_m2m_s_crop(struct file *file, void *fh,
struct v4l2_crop *cr)
f-width = cr-c.width;
f-height = cr-c.height;
+scr_unlock:
spin_unlock_irqrestore(ctx-slock, flags);
+ mutex_unlock(fimc-lock);
return 0;
}
@@ -1396,7 +1406,7 @@ static const struct v4l2_file_operations fimc_m2m_fops = {
.open = fimc_m2m_open,
.release= fimc_m2m_release,
.poll = fimc_m2m_poll,
- .ioctl = video_ioctl2,
+ .unlocked_ioctl = video_ioctl2,
.mmap = fimc_m2m_mmap,
};
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] [media] s5p-fimc: Use correct fourcc code for 32-bit RGB format
Replace V4L2_PIX_FMT_RGB24 code with V4L2_PIX_FMT_RGB32
since the hardware uses 24-bits for actual pixel data but pixels
are 4-byte aligned in memory.
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/video/s5p-fimc/fimc-core.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/video/s5p-fimc/fimc-core.c
b/drivers/media/video/s5p-fimc/fimc-core.c
index fa82a2a..f538eb5 100644
--- a/drivers/media/video/s5p-fimc/fimc-core.c
+++ b/drivers/media/video/s5p-fimc/fimc-core.c
@@ -50,8 +50,8 @@ static struct fimc_fmt fimc_formats[] = {
.planes_cnt = 1,
.flags = FMT_FLAGS_M2M,
}, {
- .name = XRGB-8-8-8-8, 24 bpp,
- .fourcc = V4L2_PIX_FMT_RGB24,
+ .name = XRGB-8-8-8-8, 32 bpp,
+ .fourcc = V4L2_PIX_FMT_RGB32,
.depth = 32,
.color = S5P_FIMC_RGB888,
.buff_cnt = 1,
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6] [media] s5p-fimc: Fix output DMA handling in S5PV310 IP revisions
FIMC IP in S5Pv310 series has extended DMA status registers
and some bit fields are marked as reserved comparing to S5PC100/110.
Use correct registers for getting DMA write pointer in each SoC variant
supported by the driver.
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/video/s5p-fimc/fimc-capture.c |1 +
drivers/media/video/s5p-fimc/fimc-core.c|2 ++
drivers/media/video/s5p-fimc/fimc-core.h| 16 +---
drivers/media/video/s5p-fimc/regs-fimc.h|3 +++
4 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/media/video/s5p-fimc/fimc-capture.c
b/drivers/media/video/s5p-fimc/fimc-capture.c
index c020bf6..ba18acb 100644
--- a/drivers/media/video/s5p-fimc/fimc-capture.c
+++ b/drivers/media/video/s5p-fimc/fimc-capture.c
@@ -522,6 +522,7 @@ static int fimc_cap_streamon(struct file *file, void *priv,
INIT_LIST_HEAD(fimc-vid_cap.active_buf_q);
fimc-vid_cap.active_buf_cnt = 0;
fimc-vid_cap.frame_count = 0;
+ fimc-vid_cap.buf_index = fimc_hw_get_frame_index(fimc);
set_bit(ST_CAPT_PEND, fimc-state);
ret = videobuf_streamon(fimc-vid_cap.vbq);
diff --git a/drivers/media/video/s5p-fimc/fimc-core.c
b/drivers/media/video/s5p-fimc/fimc-core.c
index f538eb5..bb99f2d 100644
--- a/drivers/media/video/s5p-fimc/fimc-core.c
+++ b/drivers/media/video/s5p-fimc/fimc-core.c
@@ -1746,6 +1746,7 @@ static struct samsung_fimc_variant fimc0_variant_s5pv310
= {
.pix_hoff= 1,
.has_inp_rot = 1,
.has_out_rot = 1,
+ .has_cistatus2 = 1,
.min_inp_pixsize = 16,
.min_out_pixsize = 16,
.hor_offs_align = 1,
@@ -1755,6 +1756,7 @@ static struct samsung_fimc_variant fimc0_variant_s5pv310
= {
static struct samsung_fimc_variant fimc2_variant_s5pv310 = {
.pix_hoff= 1,
+ .has_cistatus2 = 1,
.min_inp_pixsize = 16,
.min_out_pixsize = 16,
.hor_offs_align = 1,
diff --git a/drivers/media/video/s5p-fimc/fimc-core.h
b/drivers/media/video/s5p-fimc/fimc-core.h
index 6137340..4f047d3 100644
--- a/drivers/media/video/s5p-fimc/fimc-core.h
+++ b/drivers/media/video/s5p-fimc/fimc-core.h
@@ -371,6 +371,7 @@ struct fimc_pix_limit {
* @pix_hoff: indicate whether horizontal offset is in pixels or in bytes
* @has_inp_rot: set if has input rotator
* @has_out_rot: set if has output rotator
+ * @has_cistatus2: 1 if CISTATUS2 register is present in this IP revision
* @pix_limit: pixel size constraints for the scaler
* @min_inp_pixsize: minimum input pixel size
* @min_out_pixsize: minimum output pixel size
@@ -381,6 +382,7 @@ struct samsung_fimc_variant {
unsigned intpix_hoff:1;
unsigned inthas_inp_rot:1;
unsigned inthas_out_rot:1;
+ unsigned inthas_cistatus2:1;
struct fimc_pix_limit *pix_limit;
u16 min_inp_pixsize;
u16 min_out_pixsize;
@@ -556,11 +558,19 @@ static inline struct fimc_frame *ctx_get_frame(struct
fimc_ctx *ctx,
return frame;
}
+/* Return an index to the buffer actually being written. */
static inline u32 fimc_hw_get_frame_index(struct fimc_dev *dev)
{
- u32 reg = readl(dev-regs + S5P_CISTATUS);
- return (reg S5P_CISTATUS_FRAMECNT_MASK)
- S5P_CISTATUS_FRAMECNT_SHIFT;
+ u32 reg;
+
+ if (dev-variant-has_cistatus2) {
+ reg = readl(dev-regs + S5P_CISTATUS2) 0x3F;
+ return reg 0 ? --reg : reg;
+ } else {
+ reg = readl(dev-regs + S5P_CISTATUS);
+ return (reg S5P_CISTATUS_FRAMECNT_MASK)
+ S5P_CISTATUS_FRAMECNT_SHIFT;
+ }
}
/* -*/
diff --git a/drivers/media/video/s5p-fimc/regs-fimc.h
b/drivers/media/video/s5p-fimc/regs-fimc.h
index a57daed..57e33f8 100644
--- a/drivers/media/video/s5p-fimc/regs-fimc.h
+++ b/drivers/media/video/s5p-fimc/regs-fimc.h
@@ -165,6 +165,9 @@
#define S5P_CISTATUS_VVALID_A (1 15)
#define S5P_CISTATUS_VVALID_B (1 14)
+/* Indexes to the last and the currently processed buffer. */
+#define S5P_CISTATUS2 0x68
+
/* Image capture control */
#define S5P_CIIMGCPT 0xc0
#define S5P_CIIMGCPT_IMGCPTEN (1 31)
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-dvb] [bug] AF9015 message WARNING: tuning failed!!!
On 11/23/2010 10:11 PM, Paul Gover wrote: On Wednesday 06 October 2010 22:29:35 Antti Palosaari wrote: On 10/06/2010 11:36 PM, dave cunningham wrote: In message4cacd0f3.6030...@iki.fi, Antti Palosaari wrote It is QT1010 tuner driver issue. None is working for that currently or in near future. Feel free to fix :] The wiki appears to show this stick as working. http://linuxtv.org/wiki/index.php/Afatech_AF9015. Is this information incorrect or is it hit and miss depending on the host system? It works but performance is poor. Usually it locks when RF signal is weak. If you fix bug around line 381 in qt1010.c it will work much better. But if you fix that it will break devices zl10353+qt1010 since zl10353 driver misses AGC configuration. Antti Antti, I took a look at qt1010.c, but didn't see what the bug was. I was hoping to see a FIXME or BUG or some comment; any comment ;-) Look this, I think it does have that fixed. http://linuxtv.org/hg/~anttip/qt1010/ But then I went back to my traces. They said my AF9015 detected an MT2060 tuner, not a QT1010. Does this mean the QT1010 comment was wrong? The detection code that said so looks to be part of the AF9015/9013 support, maybe they use the same code. Whatever, you will know, and I certainly don't. The tuner code certainly uses the QT1010 driver and not the MT2060 driver, if I understood the traces correctly. You surely misunderstand something now. Look your first post: Quantek QT1010 successfully identified. FWIW, I think you are right about AGC; when Kaffeine scans for channels, it gives a few fleeting signal levels about 50% but fails to identify anything. My previous DVB-T stick, different older chip, produced 100% signal solidly, and Kaffeine identified more than 80 channels. I think this bug renders the AF9015 in this configuration virtually useless in Linux; the signal is enough for the Windoze tuner bloatware supplied with it to find all the channels, but not a thing in Linux. Sorry for coming back on this so much later; I've been busy doing house repair. Also, I stopped following the mailing list - I was getting swamped with stuff and it seems to make yahoo break KDE PIM. If I should post this via the list, please say, and I'll start obeying the rules! Thanks in advance for any guidance. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- http://palosaari.fi/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v6 03/12] [alsa-devel] media: Entities, pads and links
On Thu, Nov 25, 2010 at 04:21:38PM +0100, Laurent Pinchart wrote: On Thursday 25 November 2010 10:38:05 Clemens Ladisch wrote: ALSA has PCM and MIDI devices, and several types of mixer controls. (It also has hardware dependent and timer devices, but I don't think these would need topology information.) So we need at least these: MEDIA_ENTITY_TYPE_NODE_ALSA_PCM MEDIA_ENTITY_TYPE_NODE_ALSA_MIDI MEDIA_ENTITY_TYPE_SUBDEV_ALSA_CONTROL I agree about PCM and MIDI, but I'm not sure about controls. If controls are part of an entity, the entity will be reported through the media controller API. If information about that entity can be queried through existing APIs (ALSA, V4L, ...) those APIs should be used. For instance, on the V4L side, V4L2 sub-devices are mapped to entities and also have a device node which can be used to enumerate the controls supported by the subdev. The media controller only reports the entity - major:minor mapping to let applications find the device and query it directly. For audio we don't currently have a sensible API for associating controls with any sort of map of how the device is laid out, userspace has to play guessing games. I think we will need a new ioctl in the media controller API to report advanced information about an entity. This could be used to report controls implemented by an ALSA element if ALSA doesn't provide a way to do that directly. Another use case, which make me think that such an ioctl would be needed, is to report UVC extension units type (16-byte GUID) to userspace. That seems reasonable. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v6 03/12] [alsa-devel] media: Entities, pads and links
Hi Mark,
On Thursday 25 November 2010 14:41:35 Mark Brown wrote:
On Thu, Nov 25, 2010 at 10:38:05AM +0100, Clemens Ladisch wrote:
In USB and HD audio devices, all links are immutable, and the routing
is controlled by 'selector' entities that activate exactly one of their
input pads. In userspace, this entity shows up as a mixer control.
I guess it would be possible to map the ACTIVE flag onto these controls.
Ditto for ASoC, mostly.
Alternatively, entities can have 'mute' mixer controls associated with
their pads. In this case, multiple unmuted inputs would be mixed
together.
ALSA has PCM and MIDI devices, and several types of mixer controls.
(It also has hardware dependent and timer devices, but I don't think
these would need topology information.) So we need at least these:
MEDIA_ENTITY_TYPE_NODE_ALSA_PCM
MEDIA_ENTITY_TYPE_NODE_ALSA_MIDI
MEDIA_ENTITY_TYPE_SUBDEV_ALSA_CONTROL
Furthermore, topology information is also needed for entities not
associated with a mixer control, such as microphones, speakers, jacks/
connectors, and effect units. These entities are defined in the USB and
HD audio specifications, but are not yet handled by ALSA.
All this and more in the embedded case - digital audio link nodes and DSP
I/O nodes (could possibly do those as digital audio ones) spring to
mind. Also bear in mind that embedded devices can get *very* large - a
mobile phone audio system can have of the order of 100 nodes in the
graph.
It depends on how you define nodes. I can certainly imagine a graph with 100
controls, but maybe several controls can be part of the same node ? On the
video side we've decided to split entities depending on the possible data
paths configurations. As I'm not a fluent ascii-art speaker, please have a
look at pages 4 and 5 of http://www.ideasonboard.org/media/20101103-lpc-
media.pdf
Page 4 shows the internal topology of the OMAP3 ISP. The major blocks in that
diagram are reported as entities. Page 5 shows the internal topology of one of
the blocks, the OMAP3 ISP preview engine. As you can see the pipeline is made
of sub-blocks that implement a single image processing function. As the
pipeline is linear (don't worry about the non-linear part in the beginning,
it's just there to take into account link configurability at the higher level)
we don't export all the sub-blocks as entities, but we expose the controls on
the preview engine entity instead.
ALSA devices are not addressed by their device node but with card/device/
subdevice numbers; mixer controls have numeric IDs, unique per card:
struct {
int card;
int device;
int subdevice;
} alsa_device;
struct {
int card;
int numid;
} alsa_control;
For the embedded stuff we also have a bunch of stuff in the graph which
may not be visible to userspace at all at present and would just have a
string based identifier.
That could be easily added (provided the string is not too long).
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v6 03/12] [alsa-devel] media: Entities, pads and links
On Thu, Nov 25, 2010 at 04:29:53PM +0100, Laurent Pinchart wrote: It depends on how you define nodes. I can certainly imagine a graph with 100 controls, but maybe several controls can be part of the same node ? On the video side we've decided to split entities depending on the possible data paths configurations. As I'm not a fluent ascii-art speaker, please have a look at pages 4 and 5 of http://www.ideasonboard.org/media/20101103-lpc- media.pdf Please take a look at: http://www.wolfsonmicro.com/products/WM8994 for an example of a modern smartphone CODEC. This has in the ballpark of 75-100 nodes (I've not counted exactly) represented in the routing graph in the driver today. This does not include any audio routing available in other components such as the CPU. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v6 05/12] media: Reference count and power handling
Hi Mark, On Thursday 25 November 2010 14:49:08 Mark Brown wrote: On Thu, Nov 25, 2010 at 03:28:12AM +0100, Laurent Pinchart wrote: If the entity is of node type, the power change is distributed to all connected entities. For non-nodes it only affects that very node. A mutex is used to serialise access to the entity graph. ASoC has its own power management stuff which doesn't *quite* map onto this one as-is. The power determination stuff is essentially identical (and this is actually nicer) but we have a separate postprocessing stage that actually applies the changes in a sequence which minimises audible issues caused by doing things in a bad order (eg, power down a PGA before you turn off inputs). This is noddy enough to implement, though - we just need a pre and post run notifications to set up and implement the changes I think. That sounds feasible. Sakari, you've implemented power management, what do you think about this ? BTW, I notice you've not CCed the ALSA list, Liam, Takashi or Jaroslav on any of this - might be good. I've just subscrived to the alsa-devel list. I will make sure to CC it, as well as Liam, Takashi and Jaroslav, on the next version of the patches. Sorry about the missing CC's in these e-mails. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v6 09/12] media: Entity locking and pipeline management
Hi Mark, On Thursday 25 November 2010 14:53:51 Mark Brown wrote: On Thu, Nov 25, 2010 at 03:28:16AM +0100, Laurent Pinchart wrote: Link states must not be modified while streaming is in progress on a graph they belong or connect to. The entity locking API helps drivers enforcing that requirement. This is not desirable for embedded audio - for example, it is very common to wish to switch from headphone to speaker and back mid playback, or to move between headset, speakerphone and handset modes on a phone. This is normally implemented by reconfiguring the output paths at runtime without interrupting applications. Agreed. The locking should be made optional. I'll work on that. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v6 03/12] media: Entities, pads and links
On Thu, Nov 25, 2010 at 04:40:41PM +0100, Laurent Pinchart wrote: On Thursday 25 November 2010 14:36:50 Mark Brown wrote: On Thu, Nov 25, 2010 at 03:28:10AM +0100, Laurent Pinchart wrote: + MEDIA_LINK_FLAG_ACTIVE indicates that the link is active and can be + used to transfer media data. When two or more links target a sink pad, + only one of them can be active at a time. Is this supposed to reflect the current state (if the link is carrying data right now) or if it's possible for the link to carry data? It's supposed to reflect whether the link can carry data. Think of the active flag as a valve on a pipe. If the valve is open, the link is active. If the valve is closed, the link is inactive. This is unrelated to whether water actually flows through the pipe. This seems a confusing name, then - I'd expect an active link to be one which is actually carrying data rather than one which is available to carry data. How a more neutrally worded name such as connected (which is what ASoC uses currently)? This also falls through into the power management stuff, we don't want to be powering things up unless they're actually doing something right now. Immutable links have no valve (in theory you could have an inactive immutable link, but that's not very useful, unless we define immutable as no user- controllable valve, in which case it might be better to rename it as read- only, or create separate immutable and read-only flags - just brainstorming here). That was what I was expecting immutable to mean - no user control. A link that's permanantly wired can have the data flow controlled through its inputs and outputs, even if it is not itself controllable. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/10] MCDE: Add configuration registers
On Thursday 25 November 2010, Jimmy RUBIN wrote:
All these header files,
Configuration, pixel processing, formatter, dsi link registers are auto
generated from an xml file.
This actually may or may not be a legal problem, because it means that the
distributed source code is not the preferred form for making modifications
as the GPL intends, you might want to ask a lawyer. Distributing the XML
file and the script with the kernel would solve that, but people might
consider that ugly ;-).
This is made because there are many registers which a prone to change for new
hardware revisions and there is a possibility to exclude registers that are
not used.
The chance of manual errors are less this way.
I also believe that these helper macros makes the source code easier to read.
For example.
#define MCDE_CR_DSICMD2_EN_V1_SHIFT 0
#define MCDE_CR_DSICMD2_EN_V1_MASK 0x0001
#define MCDE_CR_DSICMD2_EN_V1(__x) \
MCDE_VAL2REG(MCDE_CR, DSICMD2_EN_V1, __x)
Writing a single field in the register MCDE_CR can e.g. be done like this:
mcde_wfld(MCDE_CR, DSICMD1_EN_V1, true);
and for a whole register (a totally other register but just to show):
mcde_wreg(MCDE_ROTACONF + chnl_id * MCDE_ROTACONF_GROUPOFFSET,
MCDE_ROTACONF_ROTBURSTSIZE_ENUM(8W) |
MCDE_ROTACONF_ROTBURSTSIZE_HW(1) |
MCDE_ROTACONF_ROTDIR(regs-rotdir) |
MCDE_ROTACONF_STRIP_WIDTH_ENUM(16PIX) |
MCDE_ROTACONF_RD_MAXOUT_ENUM(4_REQ) |
MCDE_ROTACONF_WR_MAXOUT_ENUM(8_REQ));
I see what you mean, though I would probably still prefer an inline
function for doing the same:
static inline void mcde_write_rotaconf(struct mcde_chnl *chnl, unsigned
burstsize,
unsigned rotdir, unsigned strip_width,
unsigned rd_maxout, wr_maxout)
{
unsigned __iomem *reg = chnl-base + MCDE_ROTACONF +
chnl-id * MCDE_ROTACONF_GROUPOFFSET;
writel(reg, burstsize 20 | rotdir 12 | strip_width 8 |
rd_maxout 4 | wr_maxout);
}
Anyway, it's not really important how you do it, this is mostly a matter
of personal preference. If you can find a way to make it more readable,
that would be really good, but it's really a minor issue compared to
getting the overall layering inside the driver right.
I agree that the header files looks a bit unreadable I will try indent as you
suggested I am just worried about the file size. (Patch limit 100kbyte)
Don't worry about the patch size limit too much, that is not the real
problem here ;-). For review, you can always cut parts of these files
since nobody will notice a problem in the middle of 2000 almost identical
source lines. When you ask for a git pull, the size no longer matters.
Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Media framework backwards compatibility
Hi Lane, On Wednesday 24 November 2010 01:14:13 Lane Brooks wrote: On 11/23/2010 04:45 PM, Laurent Pinchart wrote: On Tuesday 23 November 2010 23:29:10 Lane Brooks wrote:Laurent, If the links are setup to the resizer, then it seems that user space applications should be able to talk the resizer output (/dev/video3) like a traditional V4L2 device and need not worry about the new media framework. It even seems possible for the resizer to allow the final link format to be adjusted so that the user space application can actually adjust the resizer subdev output format across the range of valid resizer options based on the format of the resizer input pad. If the resizer output device node worked this way, then our camera would work with all the existing V4L2 applications with the simple caveat that the user has to run a separate setup application first. The resizer output device node does not currently behave this way, and I am not sure why. These are the reasons that I can think of as to why: 1. It has not been implemented this way yet. 2. I am doing something incorrectly with the media-ctl application. 3. It not intended to work this way (by the new media framework design principles). 4. It cannot work this way because of some reason I am not considering. It's probably a combination of 1 and it cannot work this way because of reasons I can't remember at 1AM :-) The ISP video device nodes implementation doesn't initialize vfh-format when the device node is opened. I think this should be fixed by querying to connected subdevice for its current format. Of course there could be no connected subdevice when the video device node is opened, in which case the format can't be initialized. Pure V4L2 applications must not try to use the video device nodes before the pipeline is initialized. I'll look into implementing this. This is mostly what I am looking for and hopefully won't be too involved to implement. Regarding adjusting the format at the output of the connected subdevice when the video device node format is set, that might be possible to implement, but we will run into several issues. One of them is that applications currently can open the video device nodes, set the format and request buffers without influencing the ISP at all. The format set on the video device node will be checked against the format on the connected pad at streamon time. This allows preallocating buffers for snapshot capture to lower snapshot latency. Making set_format configure the connected subdev directly would break this How does calling set_format on the subdev pad at the same as the device node prevent preallocating buffers? I don't really understand the ISP buffering, so I think at this point I will look into implementing the previous option and then perhaps I will have a better understanding of the issue you raise here. I think it is only the resizer that would need this capability. I am bringing it up as a nice to have, but we can certainly live without it if it does not fit into the design goals of the framework. The preallocate buffers the driver needs to know the buffer size, and that information is computed using the format set on the video device node. If you want to preallocate two sets of buffers you need to call VIDIOC_S_FMT with different sizes on the file handles before calling VIDIOC_REQBUFS. That's a hack, and that's why VIDIOC_S_FMT on the video device nodes does not configure the connected pad. At some point in the future we will need to brainstorm a buffers management API that will solve this problem in a clean way. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/10] MCDE: Add build files and bus
On Thursday 25 November 2010, Marcus LORENTZON wrote:
From: Arnd Bergmann [mailto:a...@arndb.de]
On Wednesday 10 November 2010, Jimmy Rubin wrote:
This patch adds support for the MCDE, Memory-to-display controller,
found in the ST-Ericsson ux500 products.
This patch adds the necessary build files for MCDE and the bus that
all displays are connected to.
Can you explain why you need a bus for this?
The idea of the bus is to make it easier to add new panel/display support
using the normal device/driver model. Boards add devices at init, and
drivers are selected in config. If we were to model the real physical
bus structure it would be very complex, hard to use. We use our own bus,
but it's really a virtual bus for adding display devices and drivers to
MCDE host. Is there any generic virtual bus type we should use instead
of our own type? If you have another idea of how to add display devices
and drivers without MCDE code modifications, please let us know. A virtual
bus just give us a nice framework to do this.
All devices that you cannot probe by asking hardware or firmware are normally
considered platform devices. Then again, a platform device is usally
identified by its resources, i.e. MMIO addresses and interrupts, which
I guess your display does not have.
With the code you currently have, there is only a single driver
associated
with this bus type, and also just a single device that gets registered
here!
And yes, currently there's only one single driver. But there's a lot more
coming in the pipe. Like some LCD, DPI, DBI, DSI display drivers. And once
all different u8500 boards are pushed, there will be multiple boards
registering devices with different setups. But one thing at a time.
Your approach is making it hard to review the code in context. Adding a
device driver that uses existing infrastructure is usually straightforward,
but adding infrastructure is a hard thing and needs to be done with great
care, especially if you add infrastructure before it actually is needed.
Staging it in a way that adds all the display drivers later than the
base driver is a good idea, but it would be helpful to also add the
infrastructure at the later stage. Maybe you can try to simplify the
code for now by hardcoding the single display and remove the dynamic
registration. You still have the the code, so once the base code looks
good for inclusion, we can talk about it in the context of adding
dynamic display support back in, possibly in exactly the way you are
proposing now, but perhaps in an entirely different way if we come up
with a better solution.
On a more abstract level, when you want to add large chunks of code
to the kernel, you often cannot find anyone to review and understand
all of it at once. Splitting a subsystem into ten patches by file
level rarely helps, so instead you should ideally have a series of
patches that each add working code that enable more features.
This is of course more work for you, especially if you did not consider
it while writing the code in the first place. Still, you should
always try hard to make code easy to review as much as possible,
because you need to work with reviewers both to get code in and to
make sure you make the quality ends up as good as possible.
+static int __init mcde_subsystem_init(void)
+{
+ int ret;
+ pr_info(MCDE subsystem init begin\n);
+
+ /* MCDE module init sequence */
+ ret = mcde_init();
+ if (ret)
+ return ret;
+ ret = mcde_display_init();
+ if (ret)
+ goto mcde_display_failed;
+ ret = mcde_dss_init();
+ if (ret)
+ goto mcde_dss_failed;
+ ret = mcde_fb_init();
+ if (ret)
+ goto mcde_fb_failed;
+ pr_info(MCDE subsystem init done\n);
+
+ return 0;
+mcde_fb_failed:
+ mcde_dss_exit();
+mcde_dss_failed:
+ mcde_display_exit();
+mcde_display_failed:
+ mcde_exit();
+ return ret;
+}
Splitting up the module into four sub-modules and then initializing
everything from one place indicates that something is done wrong
on a global scale.
If you indeed need a bus, that should be a separate module that gets
loaded first and then has the other modules build on top of.
Yes, that's the general idea. We don't completely understand the correct
way of making sure how one module gets loaded before another, without
selecting init level, like the fs_initcall below you commented on. I
guess most of our submodules should be initialized during subsys_init.
But we have not found how to specify the module init order inside subsys
init level. Maybe you can shed some light on this? Makefile order seems
like a fragile way of defining init order dependencies.
Do you think static init calls from machine subsys init are a better solution?
In general, the idea with loadable modules is that you
Re: [PATCH/RFC] v4l: Add subdev sensor g_skip_frames operation
Hi Eino-Ville, On Tuesday 23 November 2010 00:18:18 Eino-Ville Talvala wrote: On 11/19/2010 6:22 AM, Laurent Pinchart wrote: On Friday 19 November 2010 15:15:11 Guennadi Liakhovetski wrote: On Fri, 19 Nov 2010, Laurent Pinchart wrote: On Friday 19 November 2010 14:42:31 Hans Verkuil wrote: On Friday 19 November 2010 14:26:42 Laurent Pinchart wrote: Some buggy sensors generate corrupt frames when the stream is started. This new operation returns the number of corrupt frames to skip when starting the stream. Looks OK, but perhaps the two should be combined to one function? I'm fine with both. Guennadi, any opinion ? Same as before;) I think, there can be many more such micro parameters, that we'll want to collect from the sensor. So, if we had a good idea - what those parameters are like, we could implement just one API call to get them all, or even just pass one object with this information - if it is constant. If we don't have a good idea yet, what to expect there, it might be best to wait and first collect a more complete understanding of this kind of information. A few days late on this, but I wanted to add my two cents. When using V4L2 for still photography applications (like what's done on the N900, or on our custom Frankencamera), it becomes a lot more important to know what data is good or bad, when typically you capture only one or a few frames at a high resolution, before dropping back down to a viewfinding mode. One doesn't want to throw any frames on the floor if it isn't absolutely neccessary ( 100 ms extra shutter lag for every frame you have to drop, on the N900). So having some reasonable way to know when the data becomes good is very helpful. I agree with this. To make things worse, as Guennadi noted, the world isn't black and white and we have lots of shades of grey between good and bad. Depending on the use case the same image could be considered good or bad. That's why we will need to pass metadata to userspace at some point (providing the kernel can receive the metadata from the hardware of course). This patch aims at solving a subset of the good-bad problem, namely sensors that always produce a fixed number of completely corrupted frames when the stream starts. Sensor data sheets are also often rather vague, and state things like changing regions of interest, digital zoom, etc, 'may cause a bad frame'. It'd be great if once somebody figures out what 'may' translates to, not everyone has to figure it out again for every application. I totally agree. We would need better cooperation from sensor vendors. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v6 02/12] media: Media device
The signedness of char is ambiguous for 8 bit data, which is why an API would
normally use u8 (or s8, I guess).
Since this is known to be character data, I would think char would be fine. I
am assuming C compilers would never assume multibyte chars.
Regards,
Andy
Laurent Pinchart laurent.pinch...@ideasonboard.com wrote:
Hi Clemens,
Thanks for the review.
On Thursday 25 November 2010 10:33:02 Clemens Ladisch wrote:
Laurent Pinchart wrote:
+struct media_device {
...
+ u8 model[32];
+ u8 serial[40];
+ u8 bus_info[32];
All drivers and userspace applications have to treat this as char[], so
why u8[]?
Good question. I've copied the V4L2 practice of using u8 (or __u8) for fixed-
length strings in structures. I can't think of any reason for that.
I will replace u8 with char unless someone comes up with a good reason to keep
u8.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
N�r��yb�X��ǧv�^�){.n�+{���bj)w* jg��� �ݢj/���z�ޖ��2�ޙ�)ߡ�a���� �G���h��j:+v���w��٥
Re: [RFC/PATCH v6 05/12] media: Reference count and power handling
Laurent Pinchart wrote: Hi Mark, Hi Laurent, others, On Thursday 25 November 2010 14:49:08 Mark Brown wrote: On Thu, Nov 25, 2010 at 03:28:12AM +0100, Laurent Pinchart wrote: If the entity is of node type, the power change is distributed to all connected entities. For non-nodes it only affects that very node. A mutex is used to serialise access to the entity graph. ASoC has its own power management stuff which doesn't *quite* map onto this one as-is. The power determination stuff is essentially identical (and this is actually nicer) but we have a separate postprocessing stage that actually applies the changes in a sequence which minimises audible issues caused by doing things in a bad order (eg, power down a PGA before you turn off inputs). This is noddy enough to implement, though - we just need a pre and post run notifications to set up and implement the changes I think. That sounds feasible. Sakari, you've implemented power management, what do you think about this ? I first have to admit that I don't know ALSA almost at all. Currently when two media entities are connected they will be powered up for the duration of the link setup, just in case the drivers would need to access hardware for some reason. I don't think we have a need for that at the moment, it's so just because it seemed to be the right thing to do. Essentially the idea is that the drivers do not need to be involved with the power state handling and the device would be powered always when they are run, but not be powered when it's not necessary. That feature put aside, then what's left is what Laurent described; the entities will be powered up based on opening subdev and video nodes (V4L2 case, for ALSA it'd be different kind of ALSA nodes). But I don't think there's necessarily even need to remove this feature. Subdev is a V4L2 specific concept and I don't know if ALSA would benefit from something similar. If you want to be able to access the individual enties from user space, then similar arrangement might be useful. There are use cases to only use subdev nodes on V4L2: camera LED flash in torch mode, for example. There is no need to power on the rest of the system. I don't see a problem in applying restrictions on power on / off sequence. They would probably be useful also on the V4L2 side in the future. Could the restrictions be described as dependencies only, i.e. entity 1 power-up requires entity 2 to be powered first, also meaning that entity 2 could be powered down only after entity 1 has been powered down? This type of restrictions would fit quite well to the current model as it would essentially mean just media_entity_get() for the dependent entities, and media_entity_put() when the original entity is powered down. This would work recursively. In the case of audio, most if not all devices (right?) would be always powered up in certain order. At the same time this would be good for the flash in torch mode case. Comments, opinions? :-) Best regards, -- Sakari Ailus sakari.ai...@maxwell.research.nokia.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 09/10] MCDE: Add build files and bus
-Original Message-
From: Arnd Bergmann [mailto:a...@arndb.de]
Sent: den 25 november 2010 17:48
To: Marcus LORENTZON
Cc: linux-arm-ker...@lists.infradead.org; Jimmy RUBIN; linux-
me...@vger.kernel.org; Dan JOHANSSON; Linus WALLEIJ
Subject: Re: [PATCH 09/10] MCDE: Add build files and bus
On Thursday 25 November 2010, Marcus LORENTZON wrote:
From: Arnd Bergmann [mailto:a...@arndb.de]
On Wednesday 10 November 2010, Jimmy Rubin wrote:
This patch adds support for the MCDE, Memory-to-display
controller,
found in the ST-Ericsson ux500 products.
This patch adds the necessary build files for MCDE and the bus
that
all displays are connected to.
Can you explain why you need a bus for this?
The idea of the bus is to make it easier to add new panel/display
support
using the normal device/driver model. Boards add devices at init, and
drivers are selected in config. If we were to model the real
physical
bus structure it would be very complex, hard to use. We use our own
bus,
but it's really a virtual bus for adding display devices and drivers
to
MCDE host. Is there any generic virtual bus type we should use
instead
of our own type? If you have another idea of how to add display
devices
and drivers without MCDE code modifications, please let us know. A
virtual
bus just give us a nice framework to do this.
All devices that you cannot probe by asking hardware or firmware are
normally
considered platform devices. Then again, a platform device is usally
identified by its resources, i.e. MMIO addresses and interrupts, which
I guess your display does not have.
Then we might be on right track to model them as devices on a platform bus.
Since most displays/panels can't be plug-n-play probed, instead devices has
to be statically declared in board-xx.c files in mach-ux500 folder. Or is
platform bus a singleton? Or can we define a new platform bus device?
Displays like HDMI TV-sets are not considered for device/driver in mcde.
Instead there will be a hdmi-chip-device/driver on the mcde bus. So all devices
and drivers on this bus are static.
With the code you currently have, there is only a single driver
associated
with this bus type, and also just a single device that gets
registered
here!
And yes, currently there's only one single driver. But there's a lot
more
coming in the pipe. Like some LCD, DPI, DBI, DSI display drivers. And
once
all different u8500 boards are pushed, there will be multiple boards
registering devices with different setups. But one thing at a time.
Your approach is making it hard to review the code in context. Adding a
device driver that uses existing infrastructure is usually
straightforward,
but adding infrastructure is a hard thing and needs to be done with
great
care, especially if you add infrastructure before it actually is
needed.
Staging it in a way that adds all the display drivers later than the
base driver is a good idea, but it would be helpful to also add the
infrastructure at the later stage. Maybe you can try to simplify the
code for now by hardcoding the single display and remove the dynamic
registration. You still have the the code, so once the base code looks
good for inclusion, we can talk about it in the context of adding
dynamic display support back in, possibly in exactly the way you are
proposing now, but perhaps in an entirely different way if we come up
with a better solution.
What about starting with MCDE HW, which is the core HW driver doing all real
work? And then continue with the infrastructure + some displays + drivers ...
Only problem is that we then have a driver that can't be used from user space,
because I don't think I can find anyone with enough time to write a display
driver + framebuffer on top of mcde_hw (which is what the existing code does).
On a more abstract level, when you want to add large chunks of code
to the kernel, you often cannot find anyone to review and understand
all of it at once. Splitting a subsystem into ten patches by file
level rarely helps, so instead you should ideally have a series of
patches that each add working code that enable more features.
This is of course more work for you, especially if you did not consider
it while writing the code in the first place. Still, you should
always try hard to make code easy to review as much as possible,
because you need to work with reviewers both to get code in and to
make sure you make the quality ends up as good as possible.
+static int __init mcde_subsystem_init(void)
+{
+ int ret;
+ pr_info(MCDE subsystem init begin\n);
+
+ /* MCDE module init sequence */
+ ret = mcde_init();
+ if (ret)
+ return ret;
+ ret = mcde_display_init();
+ if (ret)
+ goto mcde_display_failed;
+ ret = mcde_dss_init();
+ if (ret)
+ goto
Re: [RFC/PATCH v6 02/12] media: Media device
On Thu, 25 Nov 2010 12:20:31 -0500 Andy Walls awa...@md.metrocast.net wrote: The signedness of char is ambiguous for 8 bit data, which is why an API would normally use u8 (or s8, I guess). Since this is known to be character data, I would think char would be fine. I am assuming C compilers would never assume multibyte chars. char is 8bit in all modern C. I have used C compilers with configurable 9 or 7 bit char and such a machine is never going to run Linux without serious insanity. Even grep in 9bit char is not fun... The advantage of using u8 is that any casting goes the way you expect whereas when working with UTF-8 things like int x = name[0]; may not produce the expected result otherwise. Alan -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[cron job] v4l-dvb daily build: WARNINGS
This message is generated daily by a cron job that builds v4l-dvb for the kernels and architectures in the list below. Results of the daily build of v4l-dvb: date:Thu Nov 25 19:01:43 CET 2010 git master: 59365d136d205cc20fe666ca7f89b1c5001b0d5a git media-master: gcc version: i686-linux-gcc (GCC) 4.5.1 host hardware:x86_64 host os: 2.6.32.5 linux-git-armv5: WARNINGS linux-git-armv5-davinci: WARNINGS linux-git-armv5-ixp: WARNINGS linux-git-armv5-omap2: WARNINGS linux-git-i686: WARNINGS linux-git-m32r: WARNINGS linux-git-mips: WARNINGS linux-git-powerpc64: WARNINGS linux-git-x86_64: WARNINGS spec-git: OK sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.tar.bz2 The V4L-DVB specification from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] kconfig: add an option to determine a menu's visibility
On Thu, Nov 25, 2010 at 12:06:46PM -0500, Arnaud Lacombe wrote: Hi folks, On Sat, Nov 6, 2010 at 5:30 PM, Arnaud Lacombe lacom...@gmail.com wrote: This option is aimed to add the possibility to control a menu's visibility without adding dependency to the expression to all the submenu. Signed-off-by: Arnaud Lacombe lacom...@gmail.com --- scripts/kconfig/expr.h | 1 + scripts/kconfig/lkc.h | 1 + scripts/kconfig/menu.c | 11 +++ scripts/kconfig/zconf.gperf | 1 + scripts/kconfig/zconf.y | 21 ++--- 5 files changed, 32 insertions(+), 3 deletions(-) As there seem to be no interested from Michal to either, ACK, NACK, or Hi Arnaud. I have these sitting in my TODO pile. But I have other things that I need to do first and I expect these other things to take a couple of weeks. [Not Linux related - just busy at my day-job/night-job] Sam -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: fix casting issues in timberdale logiwin
On 11/24/2010 10:41 AM, Richard Röjfors wrote:
On 11/23/2010 09:39 PM, Jarod Wilson wrote:
I get warnings about casting to and from pointers and integers of
different sizes w/current code, this silences them.
Signed-off-by: Jarod Wilson ja...@redhat.com
Looks good, and works.
I did a mistake and tested the wrong binary, this patch is NOT ok.
Mauro please to do apply this patch.
Acked-by: Richard Röjfors richard.rojf...@pelagicore.com
---
drivers/media/video/timblogiw.c |6 --
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/media/video/timblogiw.c
b/drivers/media/video/timblogiw.c
index 48989e5..4da3625 100644
--- a/drivers/media/video/timblogiw.c
+++ b/drivers/media/video/timblogiw.c
@@ -148,7 +148,9 @@ static void timblogiw_dma_cb(void *data)
static bool timblogiw_dma_filter_fn(struct dma_chan *chan, void
*filter_param)
{
-return chan-chan_id == (int)filter_param;
+int chan_id = *(int *)filter_param;
+
+return chan-chan_id == chan_id;
}
This part dereferences the pointer, which actually is a type casted int!
/* IOCTL functions */
@@ -670,7 +672,7 @@ static int timblogiw_open(struct file *file)
/* find the DMA channel */
fh-chan = dma_request_channel(mask, timblogiw_dma_filter_fn,
-(void *)lw-pdata.dma_channel);
+(void *)(unsigned long)lw-pdata.dma_channel);
You see here, the channel is type casted to a pointer, not something we want to
dereference as above.
--Richard
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] rc-core: add loopback driver
This patch adds a loopback driver to rc-core which I've found useful for
running scripted tests of different parts of rc-core without having to
fiddle with real hardware.
Basically it emulates hardware with a learning and a non-learning
receiver and two transmitters (which correspond to the two
receivers). TX data that is sent is fed back as input on the
corresponding receiver, which allows for debugging of IR decoders,
keymaps, etc.
Signed-off-by: David Härdeman da...@hardeman.nu
---
drivers/media/rc/Kconfig | 13 ++
drivers/media/rc/Makefile |1
drivers/media/rc/rc-loopback.c | 260
3 files changed, 274 insertions(+), 0 deletions(-)
create mode 100644 drivers/media/rc/rc-loopback.c
diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index 42b4feb..3785162 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -177,4 +177,17 @@ config IR_WINBOND_CIR
To compile this driver as a module, choose M here: the module will
be called winbond_cir.
+config RC_LOOPBACK
+ tristate Remote Control Loopback Driver
+ depends on RC_CORE
+ ---help---
+ Say Y here if you want support for the remote control loopback
+ driver which allows TX data to be sent back as RX data.
+ This is mostly useful for debugging purposes.
+
+ If you're not sure, select N here.
+
+ To compile this driver as a module, choose M here: the module will
+ be called rc_loopback.
+
endif #RC_CORE
diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
index 78ac8c5..67b4f7f 100644
--- a/drivers/media/rc/Makefile
+++ b/drivers/media/rc/Makefile
@@ -19,3 +19,4 @@ obj-$(CONFIG_IR_NUVOTON) += nuvoton-cir.o
obj-$(CONFIG_IR_ENE) += ene_ir.o
obj-$(CONFIG_IR_STREAMZAP) += streamzap.o
obj-$(CONFIG_IR_WINBOND_CIR) += winbond-cir.o
+obj-$(CONFIG_RC_LOOPBACK) += rc-loopback.o
diff --git a/drivers/media/rc/rc-loopback.c b/drivers/media/rc/rc-loopback.c
new file mode 100644
index 000..49cee61
--- /dev/null
+++ b/drivers/media/rc/rc-loopback.c
@@ -0,0 +1,260 @@
+/*
+ * Loopback driver for rc-core,
+ *
+ * Copyright (c) 2010 David Härdeman da...@hardeman.nu
+ *
+ * This driver receives TX data and passes it back as RX data,
+ * which is useful for (scripted) debugging of rc-core without
+ * having to use actual hardware.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#include linux/device.h
+#include linux/module.h
+#include linux/sched.h
+#include media/rc-core.h
+
+#define DRIVER_NAMErc-loopback
+#define dprintk(x...) if (debug) printk(KERN_INFO DRIVER_NAME : x)
+#define RXMASK_REGULAR 0x1
+#define RXMASK_LEARNING0x2
+
+static bool debug;
+
+struct loopback_dev {
+ struct rc_dev *dev;
+ u32 txmask;
+ u32 txcarrier;
+ u32 txduty;
+ bool idle;
+ bool learning;
+ bool carrierreport;
+ u32 rxcarriermin;
+ u32 rxcarriermax;
+};
+
+static struct loopback_dev loopdev;
+
+static int loop_set_tx_mask(struct rc_dev *dev, u32 mask)
+{
+ struct loopback_dev *lodev = dev-priv;
+
+ if ((mask (RXMASK_REGULAR | RXMASK_LEARNING)) != mask) {
+ dprintk(invalid tx mask: %u\n, mask);
+ return -EINVAL;
+ }
+
+ dprintk(setting tx mask: %u\n, mask);
+ lodev-txmask = mask;
+ return 0;
+}
+
+static int loop_set_tx_carrier(struct rc_dev *dev, u32 carrier)
+{
+ struct loopback_dev *lodev = dev-priv;
+
+ dprintk(setting tx carrier: %u\n, carrier);
+ lodev-txcarrier = carrier;
+ return 0;
+}
+
+static int loop_set_tx_duty_cycle(struct rc_dev *dev, u32 duty_cycle)
+{
+ struct loopback_dev *lodev = dev-priv;
+
+ if (duty_cycle 1 || duty_cycle 99) {
+ dprintk(invalid duty cycle: %u\n, duty_cycle);
+ return -EINVAL;
+ }
+
+ dprintk(setting duty cycle: %u\n, duty_cycle);
+ lodev-txduty = duty_cycle;
+ return 0;
+}
+
+static int loop_set_rx_carrier_range(struct rc_dev *dev, u32 min, u32 max)
+{
+ struct loopback_dev *lodev = dev-priv;
+
+ if (min 1 || min max) {
+ dprintk(invalid rx carrier range %u to %u\n, min, max);
+ return -EINVAL;
+ }
+
+
Re: [RFC/PATCH v6 05/12] media: Reference count and power handling
On Thu, Nov 25, 2010 at 07:49:05PM +0200, Sakari Ailus wrote: Currently when two media entities are connected they will be powered up for the duration of the link setup, just in case the drivers would need to access hardware for some reason. I don't think we have a need for that at the moment, it's so just because it seemed to be the right thing to do. This is *really* bad for audio, powering things on and off needlessly can cause audible effects for users. If the individual drivers need to do something they can go and implement that but powering things up by default seems like it'll at best waste power most of the time. Essentially the idea is that the drivers do not need to be involved with the power state handling and the device would be powered always when they are run, but not be powered when it's not necessary. This is what DAPM does in ASoC at the minute. What it does is that every time there is a change in the device configuration which might have affected the power state it walks the graph of power nodes in the system. If there has been a change in the state the core will generate a power transition sequence and apply it. The devices have no knowledge of this (though they can insert manual sequences for nodes if they need to), for the most part they just describe the graph and the register bits that control the power for the nodes and the routing. Subdev is a V4L2 specific concept and I don't know if ALSA would benefit from something similar. If you want to be able to access the individual enties from user space, then similar arrangement might be useful. ALSA already has this feature (at least in the embedded case, HDA has some of these features too). I don't see a problem in applying restrictions on power on / off sequence. They would probably be useful also on the V4L2 side in the future. Could the restrictions be described as dependencies only, i.e. entity 1 power-up requires entity 2 to be powered first, also meaning that entity 2 could be powered down only after entity 1 has been powered down? No. Audio power sequencing tends to work better if sorted by the type of object rather than the route (though using the route as a lower order sort key can be useful). It's also useful to coalesce the I/O on the hardware for performance (both speed and user experience). The way we're currently doing it the sequencing is actually separated from the graph walk - the result of the graph walk is a set of things that need doing which is then implemented in a separate step. I think this would be the easiest way to integrate with what you're doing at the minute, keep the same split and then ASoC can do the postprocessing of the results of the graph walk in the same way as it does currently. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ATSC Tuner in KWorld PC120-U PCI Hybrid ATSC (17de:a134)
Hi, Am Donnerstag, den 25.11.2010, 00:56 -0500 schrieb Hooman B.: Thanks, I see the drivers for both TDA18271HDC2 and TDA8290 loaded. I thought TDA18271HDC2 was the digital channel decoder, isn't it? Is the digital channel decoder different from the digital tuner?? Should be looking for a different chip? they are detected by chip IDs. The tda8290 is the IF demodulator for global analog TV. In case of the saa7135 it is an extra chip on the PCB, which most often can also control the tuner over an i2c gate. The tda18271hdc2 is a global hybrid tuner for analog and digital terrestrial TV. It can also provide a FM radio IF. Further processing and stereo separation for that is done on capable bridges like saa7133/35/31e, not on the tda 8290. Yes, for any terrestrial digital TV you need an extra channel decoder and all known details about it. http://linuxtv.org/wiki/index.php/Category:ATSC_PCI_Cards In case of terrestrial ATSC, it must be able to deal with 8VSB modulation. IIRC, on the saa713x we have that tuner only in combination with the tda10048 and DVB-T for now, but other combinations seem to be already around. Hermann Based on these to chips, I added my card in saa7134_tda8290_callback to call saa7134_tda8290_18271_callback and here is the output of my dmesg: [ 828.879454] dvb_init() allocating 1 frontend [ 829.180234] nxt200x: nxt200x_readbytes: i2c read error (addr 0x0a, err == -5) [ 829.180244] Unknown/Unsupported NXT chip: 00 00 00 00 00 [ 829.180542] saa7133[0]/dvb: frontend initialization failed Hooman On Wed, Nov 24, 2010 at 7:21 PM, hermann pitton hermann-pit...@arcor.de wrote: Hi, Am Dienstag, den 23.11.2010, 17:34 -0500 schrieb Hooman B.: Hello! I've been trying to get the ATSC tuner in my KWorld PC120-U PCI Hybrid ATSC (17de:a134) to work with the latest v4l drivers from source (in Ubuntu). Right now, everything [capture, analog, radio, even IR] works - except the ATSC tuner (there is no front-end device). But that's the one thing I need working :-( OK. Here's the output of lspci -nnvv 03:01.0 Multimedia controller [0480]: Philips Semiconductors SAA7131/SAA7133/SAA7135 Video Broadcast Decoder [1131:7133] (rev d1) Subsystem: KWorld Computer Co. Ltd. Device [17de:a134] Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium TAbort- TAbort- MAbort- SERR- PERR- INTx- Latency: 255 (63750ns min, 63750ns max) Interrupt: pin A routed to IRQ 16 Region 0: Memory at fdaff000 (32-bit, non-prefetchable) [size=2K] Capabilities: access denied Kernel driver in use: saa7134 Kernel modules: saa7134 This is the most similar card that I forced in saa7134-cards.c: .vendor = PCI_VENDOR_ID_PHILIPS, .device = PCI_DEVICE_ID_PHILIPS_SAA7133, .subvendor= 0x17de, .subdevice= 0xa134, .driver_data = SAA7134_BOARD_MSI_TVATANYWHERE_PLUS, The chips are : SAA7135HL/203 and TDA18271HDC2 Both variants of the MSI t...@nywhere Plus do not have any support for digital TV. You need to find out the type of digital channel decoder on your board at first. Then you check saa7134-dvb.c, if it is already supported on other cards. You have to investigate the details of how the channel decoder is employed, but with some luck can try with already supported cards. If i2c traffic locks up and chips disappear from the bus, a cold boot might be necessary to continue with testing. Cheers, Hermann -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
