Re: [PATCH 4/6] android: convert sync to fence api, v4
On Tue, Mar 04, 2014 at 08:50:38AM +0100, Maarten Lankhorst wrote:
op 03-03-14 22:11, Daniel Vetter schreef:
On Mon, Feb 17, 2014 at 04:57:19PM +0100, Maarten Lankhorst wrote:
Android syncpoints can be mapped to a timeline. This removes the need
to maintain a separate api for synchronization. I've left the android
trace events in place, but the core fence events should already be
sufficient for debugging.
v2:
- Call fence_remove_callback in sync_fence_free if not all fences have
fired.
v3:
- Merge Colin Cross' bugfixes, and the android fence merge optimization.
v4:
- Merge with the upstream fixes.
Signed-off-by: Maarten Lankhorst maarten.lankho...@canonical.com
---
Snipped everything but headers - Ian Lister from our android team is
signed up to have a more in-depth look at proper integration with android
syncpoints. Adding him to cc.
diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h
index 62e2255b1c1e..6036dbdc8e6f 100644
--- a/drivers/staging/android/sync.h
+++ b/drivers/staging/android/sync.h
@@ -21,6 +21,7 @@
#include linux/list.h
#include linux/spinlock.h
#include linux/wait.h
+#include linux/fence.h
struct sync_timeline;
struct sync_pt;
@@ -40,8 +41,6 @@ struct sync_fence;
* -1 if a will signal before b
* @free_pt: called before sync_pt is freed
* @release_obj: called before sync_timeline is freed
- * @print_obj: deprecated
- * @print_pt: deprecated
* @fill_driver_data: write implementation specific driver data to data.
* should return an error if there is not enough room
* as specified by size. This information is returned
@@ -67,13 +66,6 @@ struct sync_timeline_ops {
/* optional */
void (*release_obj)(struct sync_timeline *sync_timeline);
- /* deprecated */
- void (*print_obj)(struct seq_file *s,
- struct sync_timeline *sync_timeline);
-
- /* deprecated */
- void (*print_pt)(struct seq_file *s, struct sync_pt *sync_pt);
-
/* optional */
int (*fill_driver_data)(struct sync_pt *syncpt, void *data, int size);
@@ -104,42 +96,48 @@ struct sync_timeline {
/* protected by child_list_lock */
bool destroyed;
+ int context, value;
struct list_head child_list_head;
spinlock_t child_list_lock;
struct list_head active_list_head;
- spinlock_t active_list_lock;
+#ifdef CONFIG_DEBUG_FS
struct list_head sync_timeline_list;
+#endif
};
/**
* struct sync_pt - sync point
- * @parent: sync_timeline to which this sync_pt belongs
+ * @fence: base fence class
* @child_list: membership in sync_timeline.child_list_head
* @active_list: membership in sync_timeline.active_list_head
+ current
* @signaled_list: membership in temporary signaled_list on stack
* @fence: sync_fence to which the sync_pt belongs
* @pt_list: membership in sync_fence.pt_list_head
* @status: 1: signaled, 0:active, 0: error
* @timestamp: time which sync_pt status transitioned from active to
* signaled or error.
+===
+ patched
Conflict markers ...
Oops.
*/
struct sync_pt {
- struct sync_timeline *parent;
- struct list_head child_list;
+ struct fence base;
Hm, embedding feels wrong, since that still means that I'll need to
implement two kinds of fences in i915 - one using the seqno fence to make
dma-buf sync work, and one to implmenent sync_pt to make the android folks
happy.
If I can dream I think we should have a pointer to an underlying fence
here, i.e. a struct sync_pt would just be a userspace interface wrapper to
do explicit syncing using native fences, instead of implicit syncing like
with dma-bufs. But this is all drive-by comments from a very cursory
high-level look. I might be full of myself again ;-)
-Daniel
No, the idea is that because android syncpoint is simply another type of
dma-fence, that if you deal with normal fences then android can
automatically be handled too. The userspace fence api android exposes
could be very easily made to work for dma-fence, just pass a dma-fence
to sync_fence_create.
So exposing dma-fence would probably work for android too.
Hm, then why do we still have struct sync_pt around? Since it's just the
internal bit, with the userspace facing object being struct sync_fence,
I'd opt to shuffle any useful features into the core struct fence.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
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 4/6] android: convert sync to fence api, v4
op 04-03-14 09:14, Daniel Vetter schreef:
On Tue, Mar 04, 2014 at 08:50:38AM +0100, Maarten Lankhorst wrote:
op 03-03-14 22:11, Daniel Vetter schreef:
On Mon, Feb 17, 2014 at 04:57:19PM +0100, Maarten Lankhorst wrote:
Android syncpoints can be mapped to a timeline. This removes the need
to maintain a separate api for synchronization. I've left the android
trace events in place, but the core fence events should already be
sufficient for debugging.
v2:
- Call fence_remove_callback in sync_fence_free if not all fences have fired.
v3:
- Merge Colin Cross' bugfixes, and the android fence merge optimization.
v4:
- Merge with the upstream fixes.
Signed-off-by: Maarten Lankhorst maarten.lankho...@canonical.com
---
Snipped everything but headers - Ian Lister from our android team is
signed up to have a more in-depth look at proper integration with android
syncpoints. Adding him to cc.
diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h
index 62e2255b1c1e..6036dbdc8e6f 100644
--- a/drivers/staging/android/sync.h
+++ b/drivers/staging/android/sync.h
@@ -21,6 +21,7 @@
#include linux/list.h
#include linux/spinlock.h
#include linux/wait.h
+#include linux/fence.h
struct sync_timeline;
struct sync_pt;
@@ -40,8 +41,6 @@ struct sync_fence;
* -1 if a will signal before b
* @free_pt: called before sync_pt is freed
* @release_obj: called before sync_timeline is freed
- * @print_obj: deprecated
- * @print_pt: deprecated
* @fill_driver_data: write implementation specific driver data to data.
* should return an error if there is not enough room
* as specified by size. This information is returned
@@ -67,13 +66,6 @@ struct sync_timeline_ops {
/* optional */
void (*release_obj)(struct sync_timeline *sync_timeline);
- /* deprecated */
- void (*print_obj)(struct seq_file *s,
- struct sync_timeline *sync_timeline);
-
- /* deprecated */
- void (*print_pt)(struct seq_file *s, struct sync_pt *sync_pt);
-
/* optional */
int (*fill_driver_data)(struct sync_pt *syncpt, void *data, int size);
@@ -104,42 +96,48 @@ struct sync_timeline {
/* protected by child_list_lock */
bool destroyed;
+ int context, value;
struct list_head child_list_head;
spinlock_t child_list_lock;
struct list_head active_list_head;
- spinlock_t active_list_lock;
+#ifdef CONFIG_DEBUG_FS
struct list_head sync_timeline_list;
+#endif
};
/**
* struct sync_pt - sync point
- * @parent: sync_timeline to which this sync_pt belongs
+ * @fence: base fence class
* @child_list: membership in sync_timeline.child_list_head
* @active_list: membership in sync_timeline.active_list_head
+ current
* @signaled_list: membership in temporary signaled_list on stack
* @fence: sync_fence to which the sync_pt belongs
* @pt_list: membership in sync_fence.pt_list_head
* @status: 1: signaled, 0:active, 0: error
* @timestamp: time which sync_pt status transitioned from active to
* signaled or error.
+===
+ patched
Conflict markers ...
Oops.
*/
struct sync_pt {
- struct sync_timeline *parent;
- struct list_head child_list;
+ struct fence base;
Hm, embedding feels wrong, since that still means that I'll need to
implement two kinds of fences in i915 - one using the seqno fence to make
dma-buf sync work, and one to implmenent sync_pt to make the android folks
happy.
If I can dream I think we should have a pointer to an underlying fence
here, i.e. a struct sync_pt would just be a userspace interface wrapper to
do explicit syncing using native fences, instead of implicit syncing like
with dma-bufs. But this is all drive-by comments from a very cursory
high-level look. I might be full of myself again ;-)
-Daniel
No, the idea is that because android syncpoint is simply another type of
dma-fence, that if you deal with normal fences then android can
automatically be handled too. The userspace fence api android exposes
could be very easily made to work for dma-fence, just pass a dma-fence
to sync_fence_create.
So exposing dma-fence would probably work for android too.
Hm, then why do we still have struct sync_pt around? Since it's just the
internal bit, with the userspace facing object being struct sync_fence,
I'd opt to shuffle any useful features into the core struct fence.
-Daniel
To keep compatibility with the android api. I think that gradually converting
them is going to be
more useful than to force all drivers to use a new api all at once. They could
keep android
syncpoint api for exporting, as long as they accept dma-fence for
importing/waiting.
~Maarten
--
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 7/7] v4l: ti-vpe: Add crop support in VPE driver
Hi, On Tuesday 04 March 2014 01:13 PM, Hans Verkuil wrote: On 03/04/2014 08:38 AM, Archit Taneja wrote: Hi Hans, On Monday 03 March 2014 01:20 PM, Hans Verkuil wrote: Hi Archit! On 03/03/2014 08:33 AM, Archit Taneja wrote: Add crop ioctl ops. For VPE, cropping only makes sense with the input to VPE, or the V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE buffer type. For the CAPTURE type, a S_CROP ioctl results in setting the crop region as the whole image itself, hence making crop dimensions same as the pix dimensions. Setting the crop successfully should result in re-configuration of those registers which are affected when either source or destination dimensions change, set_srcdst_params() is called for this purpose. Some standard crop parameter checks are done in __vpe_try_crop(). Please use the selection ops instead: if you implement cropping with those then you'll support both the selection API and the old cropping API will be implemented by the v4l2 core using the selection ops. Two for the price of one... When using selection API, I was finding issues using the older cropping API. The v4l_s_crop() ioctl func assumes that crop means compose for output devices. However, for a m2m device. It probably makes sense to provide the following configuration: for V4L2_BUF_TYPE_VIDEO_OUTPUT (input to the mem to mem HW), use CROP target(to crop the input buffer) and, for V4L2_BUF_TYPE_VIDEO_CAPTURE(output of the mem to mem HW), use COMPOSE target(to place the HW output into a larger region) Don't you think forcing OUTPUT devices to 'COMPOSE' for older cropping API is a bit limiting? Yes, and that's why the selection API was created to work around that limitation :-) The old cropping API was insufficiently flexible for modern devices, so we came up with this replacement. Another reason why you have to implement the selection API: it's the only way to implement your functionality. Okay, I'll go ahead with the selection API then :) Archit -- 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 v2 0/7] v4l: ti-vpe: Some VPE fixes and enhancements
This patch set mainly consists of minor fixes for the VPE driver. These fixes ensure the following: - The VPE module can be inserted and removed successively. - Make sure that smaller resolutions like qcif work correctly. - Prevent race condition between firmware loading and an open call to the v4l2 device. - Prevent the possibility of output m2m queue not having sufficient 'ready' buffers. - Some VPDMA data descriptor fields weren't understood correctly before. They are now used correctly. The rest of the patches add some minor features like DMA buf support and cropping/composing. Reference branch: g...@github.com:boddob/linux.git vpe_for_315 Changes in v2: - selection API used instead of older cropping API. - Typo fix. - Some changes in patch 6/7 to support composing on the capture side of VPE. Archit Taneja (7): v4l: ti-vpe: Make sure in job_ready that we have the needed number of dst_bufs v4l: ti-vpe: register video device only when firmware is loaded v4l: ti-vpe: Use video_device_release_empty v4l: ti-vpe: Allow DMABUF buffer type support v4l: ti-vpe: Allow usage of smaller images v4l: ti-vpe: Fix some params in VPE data descriptors v4l: ti-vpe: Add selection API in VPE driver drivers/media/platform/ti-vpe/vpdma.c | 68 +++--- drivers/media/platform/ti-vpe/vpdma.h | 17 +-- drivers/media/platform/ti-vpe/vpe.c | 228 +- 3 files changed, 255 insertions(+), 58 deletions(-) -- 1.8.3.2 -- 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 v2 4/7] v4l: ti-vpe: Allow DMABUF buffer type support
For OMAP and DRA7x, we generally allocate video and graphics buffers through omapdrm since the corresponding omap-gem driver provides DMM-Tiler backed contiguous buffers. omapdrm is a dma-buf exporter. These buffers are used by other drivers in the video pipeline. Add VB2_DMABUF flag to the io_modes of the vb2 output and capture queues. This allows the driver to import dma shared buffers. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index bb275f4..915029b 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1768,7 +1768,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, memset(src_vq, 0, sizeof(*src_vq)); src_vq-type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE; - src_vq-io_modes = VB2_MMAP; + src_vq-io_modes = VB2_MMAP | VB2_DMABUF; src_vq-drv_priv = ctx; src_vq-buf_struct_size = sizeof(struct v4l2_m2m_buffer); src_vq-ops = vpe_qops; @@ -1781,7 +1781,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, memset(dst_vq, 0, sizeof(*dst_vq)); dst_vq-type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; - dst_vq-io_modes = VB2_MMAP; + dst_vq-io_modes = VB2_MMAP | VB2_DMABUF; dst_vq-drv_priv = ctx; dst_vq-buf_struct_size = sizeof(struct v4l2_m2m_buffer); dst_vq-ops = vpe_qops; -- 1.8.3.2 -- 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 v2 2/7] v4l: ti-vpe: register video device only when firmware is loaded
vpe fops(vpe_open in particular) should be called only when VPDMA firmware
is loaded. File operations on the video device are possible the moment it is
registered.
Currently, we register the video device for VPE at driver probe, after calling
a vpdma helper to initialize VPDMA and load firmware. This function is
non-blocking(it calls request_firmware_nowait()), and doesn't ensure that the
firmware is actually loaded when it returns.
We remove the device registration from vpe probe, and move it to a callback
provided by the vpe driver to the vpdma library, through vpdma_create().
The ready field in vpdma_data is no longer needed since we always have firmware
loaded before the device is registered.
A minor problem with this approach is that if the video_register_device
fails(which doesn't really happen), the vpe platform device would be registered.
however, there won't be any v4l2 device corresponding to it.
Signed-off-by: Archit Taneja arc...@ti.com
---
drivers/media/platform/ti-vpe/vpdma.c | 8 +++--
drivers/media/platform/ti-vpe/vpdma.h | 7 +++--
drivers/media/platform/ti-vpe/vpe.c | 55 ---
3 files changed, 41 insertions(+), 29 deletions(-)
diff --git a/drivers/media/platform/ti-vpe/vpdma.c
b/drivers/media/platform/ti-vpe/vpdma.c
index e8175e7..73dd38e 100644
--- a/drivers/media/platform/ti-vpe/vpdma.c
+++ b/drivers/media/platform/ti-vpe/vpdma.c
@@ -781,7 +781,7 @@ static void vpdma_firmware_cb(const struct firmware *f,
void *context)
/* already initialized */
if (read_field_reg(vpdma, VPDMA_LIST_ATTR, VPDMA_LIST_RDY_MASK,
VPDMA_LIST_RDY_SHFT)) {
- vpdma-ready = true;
+ vpdma-cb(vpdma-pdev);
return;
}
@@ -811,7 +811,7 @@ static void vpdma_firmware_cb(const struct firmware *f,
void *context)
goto free_buf;
}
- vpdma-ready = true;
+ vpdma-cb(vpdma-pdev);
free_buf:
vpdma_unmap_desc_buf(vpdma, fw_dma_buf);
@@ -839,7 +839,8 @@ static int vpdma_load_firmware(struct vpdma_data *vpdma)
return 0;
}
-struct vpdma_data *vpdma_create(struct platform_device *pdev)
+struct vpdma_data *vpdma_create(struct platform_device *pdev,
+ void (*cb)(struct platform_device *pdev))
{
struct resource *res;
struct vpdma_data *vpdma;
@@ -854,6 +855,7 @@ struct vpdma_data *vpdma_create(struct platform_device
*pdev)
}
vpdma-pdev = pdev;
+ vpdma-cb = cb;
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, vpdma);
if (res == NULL) {
diff --git a/drivers/media/platform/ti-vpe/vpdma.h
b/drivers/media/platform/ti-vpe/vpdma.h
index cf40f11..bf5f8bb 100644
--- a/drivers/media/platform/ti-vpe/vpdma.h
+++ b/drivers/media/platform/ti-vpe/vpdma.h
@@ -35,8 +35,8 @@ struct vpdma_data {
struct platform_device *pdev;
- /* tells whether vpdma firmware is loaded or not */
- bool ready;
+ /* callback to VPE driver when the firmware is loaded */
+ void (*cb)(struct platform_device *pdev);
};
enum vpdma_data_format_type {
@@ -208,6 +208,7 @@ void vpdma_set_frame_start_event(struct vpdma_data *vpdma,
void vpdma_dump_regs(struct vpdma_data *vpdma);
/* initialize vpdma, passed with VPE's platform device pointer */
-struct vpdma_data *vpdma_create(struct platform_device *pdev);
+struct vpdma_data *vpdma_create(struct platform_device *pdev,
+ void (*cb)(struct platform_device *pdev));
#endif
diff --git a/drivers/media/platform/ti-vpe/vpe.c
b/drivers/media/platform/ti-vpe/vpe.c
index 623e59e..4243687 100644
--- a/drivers/media/platform/ti-vpe/vpe.c
+++ b/drivers/media/platform/ti-vpe/vpe.c
@@ -1815,11 +1815,6 @@ static int vpe_open(struct file *file)
vpe_dbg(dev, vpe_open\n);
- if (!dev-vpdma-ready) {
- vpe_err(dev, vpdma firmware not loaded\n);
- return -ENODEV;
- }
-
ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
if (!ctx)
return -ENOMEM;
@@ -2037,10 +2032,40 @@ static void vpe_runtime_put(struct platform_device
*pdev)
WARN_ON(r 0 r != -ENOSYS);
}
+static void vpe_fw_cb(struct platform_device *pdev)
+{
+ struct vpe_dev *dev = platform_get_drvdata(pdev);
+ struct video_device *vfd;
+ int ret;
+
+ vfd = dev-vfd;
+ *vfd = vpe_videodev;
+ vfd-lock = dev-dev_mutex;
+ vfd-v4l2_dev = dev-v4l2_dev;
+
+ ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
+ if (ret) {
+ vpe_err(dev, Failed to register video device\n);
+
+ vpe_set_clock_enable(dev, 0);
+ vpe_runtime_put(pdev);
+ pm_runtime_disable(pdev-dev);
+ v4l2_m2m_release(dev-m2m_dev);
+ vb2_dma_contig_cleanup_ctx(dev-alloc_ctx);
+ v4l2_device_unregister(dev-v4l2_dev);
+
+ return;
+ }
+
+
[PATCH v2 3/7] v4l: ti-vpe: Use video_device_release_empty
The video_device struct is currently embedded in the driver data struct vpe_dev.
A vpe_dev instance is allocated by the driver, and the memory for the vfd is a
part of this struct.
The v4l2 core, however, manages the removal of the vfd region, through the
video_device's .release() op, which currently is the helper
video_device_release. This causes memory corruption, and leads to issues when
we try to re-insert the vpe module.
Use the video_device_release_empty helper function instead
Signed-off-by: Archit Taneja arc...@ti.com
---
drivers/media/platform/ti-vpe/vpe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/ti-vpe/vpe.c
b/drivers/media/platform/ti-vpe/vpe.c
index 4243687..bb275f4 100644
--- a/drivers/media/platform/ti-vpe/vpe.c
+++ b/drivers/media/platform/ti-vpe/vpe.c
@@ -1998,7 +1998,7 @@ static struct video_device vpe_videodev = {
.fops = vpe_fops,
.ioctl_ops = vpe_ioctl_ops,
.minor = -1,
- .release= video_device_release,
+ .release= video_device_release_empty,
.vfl_dir= VFL_DIR_M2M,
};
--
1.8.3.2
--
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 v2 1/7] v4l: ti-vpe: Make sure in job_ready that we have the needed number of dst_bufs
VPE has a ctrl parameter which decides how many mem to mem transactions the active job from the job queue can perform. The driver's job_ready() made sure that the number of ready source buffers are sufficient for the job to execute successfully. But it didn't make sure if there are sufficient ready destination buffers in the capture queue for the VPE output. If the time taken by VPE to process a single frame is really slow, then it's possible that we don't need to imply such a restriction on the dst queue, but really fast transactions(small resolution, no de-interlacing) may cause us to hit the condition where we don't have any free buffers for the VPE to write on. Add the extra check in job_ready() to make sure we have the sufficient amount of destination buffers. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 1296c53..623e59e 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -887,6 +887,9 @@ static int job_ready(void *priv) if (v4l2_m2m_num_src_bufs_ready(ctx-m2m_ctx) needed) return 0; + if (v4l2_m2m_num_dst_bufs_ready(ctx-m2m_ctx) needed) + return 0; + return 1; } -- 1.8.3.2 -- 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 v2 5/7] v4l: ti-vpe: Allow usage of smaller images
The minimum width and height for VPE input/output was kept as 128 pixels. VPE doesn't have a constraint on the image height, it requires the image width to be at least 16 bytes. Change the minimum supported dimensions to 32x32. This allows us to de-interlace qcif content. A smaller image size than 32x32 didn't make much sense, so stopped at this. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 915029b..3a610a6 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -49,8 +49,8 @@ #define VPE_MODULE_NAME vpe /* minimum and maximum frame sizes */ -#define MIN_W 128 -#define MIN_H 128 +#define MIN_W 32 +#define MIN_H 32 #define MAX_W 1920 #define MAX_H 1080 -- 1.8.3.2 -- 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 v2 7/7] v4l: ti-vpe: Add selection API in VPE driver
Add selection ioctl ops. For VPE, cropping makes sense only for the input to
VPE(or V4L2_BUF_TYPE_VIDEO_OUTPUT/MPLANE buffers) and composing makes sense
only for the output of VPE(or V4L2_BUF_TYPE_VIDEO_CAPTURE/MPLANE buffers).
For the CAPTURE type, V4L2_SEL_TGT_COMPOSE results in VPE writing the output
in a rectangle within the capture buffer. For the OUTPUT type, V4L2_SEL_TGT_CROP
results in selecting a rectangle region within the source buffer.
Setting the crop/compose rectangles should successfully result in
re-configuration of registers which are affected when either source or
destination dimensions change, set_srcdst_params() is called for this purpose.
Signed-off-by: Archit Taneja arc...@ti.com
---
drivers/media/platform/ti-vpe/vpe.c | 142
1 file changed, 142 insertions(+)
diff --git a/drivers/media/platform/ti-vpe/vpe.c
b/drivers/media/platform/ti-vpe/vpe.c
index 03a6846..b938590 100644
--- a/drivers/media/platform/ti-vpe/vpe.c
+++ b/drivers/media/platform/ti-vpe/vpe.c
@@ -410,8 +410,10 @@ static struct vpe_q_data *get_q_data(struct vpe_ctx *ctx,
{
switch (type) {
case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
+ case V4L2_BUF_TYPE_VIDEO_OUTPUT:
return ctx-q_data[Q_DATA_SRC];
case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
+ case V4L2_BUF_TYPE_VIDEO_CAPTURE:
return ctx-q_data[Q_DATA_DST];
default:
BUG();
@@ -1585,6 +1587,143 @@ static int vpe_s_fmt(struct file *file, void *priv,
struct v4l2_format *f)
return set_srcdst_params(ctx);
}
+static int __vpe_try_selection(struct vpe_ctx *ctx, struct v4l2_selection *s)
+{
+ struct vpe_q_data *q_data;
+
+ if ((s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ (s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT))
+ return -EINVAL;
+
+ q_data = get_q_data(ctx, s-type);
+ if (!q_data)
+ return -EINVAL;
+
+ switch (s-target) {
+ case V4L2_SEL_TGT_COMPOSE:
+ case V4L2_SEL_TGT_COMPOSE_DEFAULT:
+ case V4L2_SEL_TGT_COMPOSE_BOUNDS:
+ /*
+* COMPOSE target is only valid for capture buffer type, for
+* output buffer type, assign existing crop parameters to the
+* selection rectangle
+*/
+ if (s-type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
+ break;
+ } else {
+ s-r = q_data-c_rect;
+ return 0;
+ }
+
+ case V4L2_SEL_TGT_CROP:
+ case V4L2_SEL_TGT_CROP_DEFAULT:
+ case V4L2_SEL_TGT_CROP_BOUNDS:
+ /*
+* CROP target is only valid for output buffer type, for capture
+* buffer type, assign existing compose parameters to the
+* selection rectangle
+*/
+ if (s-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
+ break;
+ } else {
+ s-r = q_data-c_rect;
+ return 0;
+ }
+ default:
+ return -EINVAL;
+ }
+
+ if (s-r.top 0 || s-r.left 0) {
+ vpe_err(ctx-dev, negative values for top and left\n);
+ s-r.top = s-r.left = 0;
+ }
+
+ v4l_bound_align_image(s-r.width, MIN_W, q_data-width, 1,
+ s-r.height, MIN_H, q_data-height, H_ALIGN, S_ALIGN);
+
+ /* adjust left/top if cropping rectangle is out of bounds */
+ if (s-r.left + s-r.width q_data-width)
+ s-r.left = q_data-width - s-r.width;
+ if (s-r.top + s-r.height q_data-height)
+ s-r.top = q_data-height - s-r.height;
+
+ return 0;
+}
+
+static int vpe_g_selection(struct file *file, void *fh,
+ struct v4l2_selection *s)
+{
+ struct vpe_ctx *ctx = file2ctx(file);
+ struct vpe_q_data *q_data;
+
+ if ((s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ (s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT))
+ return -EINVAL;
+
+ q_data = get_q_data(ctx, s-type);
+ if (!q_data)
+ return -EINVAL;
+
+ switch (s-target) {
+ /* return width and height from S_FMT of the respective buffer type */
+ case V4L2_SEL_TGT_COMPOSE_DEFAULT:
+ case V4L2_SEL_TGT_COMPOSE_BOUNDS:
+ case V4L2_SEL_TGT_CROP_BOUNDS:
+ case V4L2_SEL_TGT_CROP_DEFAULT:
+ s-r.left = 0;
+ s-r.top = 0;
+ s-r.width = q_data-width;
+ s-r.height = q_data-height;
+ return 0;
+
+ /*
+* CROP target holds for the output buffer type, and COMPOSE target
+* holds for the capture buffer type. We still return the c_rect params
+* for both the target types.
+*/
+ case V4L2_SEL_TGT_COMPOSE:
+ case V4L2_SEL_TGT_CROP:
+ s-r.left = q_data-c_rect.left;
+ s-r.top =
[PATCH v2 6/7] v4l: ti-vpe: Fix some params in VPE data descriptors
Some parameters of the VPE descriptors were understood incorrectly. They are now
fixed. The fixes are explained as follows:
- When adding an inbound data descriptor to the VPDMA descriptor list, we intend
to use c_rect as the cropped region fetched by VPDMA. Therefore, c_rect-width
shouldn't be used to calculate the line stride, the original image width
should be used for that. We add a 'width' argument which gives the buffer
width in memory.
- frame_width and frame_height describe the complete width and height of the
client to which the channel is connected. If there are multiple channels
fetching data and providing to the same client, the above 2 arguments should
be the width and height of the region covered by all the channels. In the case
where there is only one channel providing pixel data to the client
(like in VPE), frame_width and frame_height should be the cropped width and
cropped height respectively. The calculation of these params is done in the
vpe driver now.
- start_h and start_v is also used in the case of multiple channels to describe
where each channel should start filling pixel data. We don't use this in VPE,
and pass 0s to the vpdma_add_in_dtd() helper.
- Some minor changes are made to the vpdma_add_out_dtd() helper. The c_rect
param is used for specifying the 'composition' target, and 'width' is added
to calculate the line stride.
Signed-off-by: Archit Taneja arc...@ti.com
---
drivers/media/platform/ti-vpe/vpdma.c | 60 +++
drivers/media/platform/ti-vpe/vpdma.h | 10 +++---
drivers/media/platform/ti-vpe/vpe.c | 18 +++
3 files changed, 64 insertions(+), 24 deletions(-)
diff --git a/drivers/media/platform/ti-vpe/vpdma.c
b/drivers/media/platform/ti-vpe/vpdma.c
index 73dd38e..a51a013 100644
--- a/drivers/media/platform/ti-vpe/vpdma.c
+++ b/drivers/media/platform/ti-vpe/vpdma.c
@@ -614,8 +614,17 @@ static void dump_dtd(struct vpdma_dtd *dtd)
/*
* append an outbound data transfer descriptor to the given descriptor list,
* this sets up a 'client to memory' VPDMA transfer for the given VPDMA channel
+ *
+ * @list: vpdma desc list to which we add this decriptor
+ * @width: width of the image in pixels in memory
+ * @c_rect: compose params of output image
+ * @fmt: vpdma data format of the buffer
+ * dma_addr: dma address as seen by VPDMA
+ * chan: VPDMA channel
+ * flags: VPDMA flags to configure some descriptor fileds
*/
-void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct v4l2_rect *c_rect,
+void vpdma_add_out_dtd(struct vpdma_desc_list *list, int width,
+ const struct v4l2_rect *c_rect,
const struct vpdma_data_format *fmt, dma_addr_t dma_addr,
enum vpdma_channel chan, u32 flags)
{
@@ -623,6 +632,7 @@ void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct
v4l2_rect *c_rect,
int field = 0;
int notify = 1;
int channel, next_chan;
+ struct v4l2_rect rect = *c_rect;
int depth = fmt-depth;
int stride;
struct vpdma_dtd *dtd;
@@ -630,11 +640,15 @@ void vpdma_add_out_dtd(struct vpdma_desc_list *list,
struct v4l2_rect *c_rect,
channel = next_chan = chan_info[chan].num;
if (fmt-type == VPDMA_DATA_FMT_TYPE_YUV
- fmt-data_type == DATA_TYPE_C420)
+ fmt-data_type == DATA_TYPE_C420) {
+ rect.height = 1;
+ rect.top = 1;
depth = 8;
+ }
- stride = ALIGN((depth * c_rect-width) 3, VPDMA_STRIDE_ALIGN);
- dma_addr += (c_rect-left * depth) 3;
+ stride = ALIGN((depth * width) 3, VPDMA_STRIDE_ALIGN);
+
+ dma_addr += rect.top * stride + (rect.left * depth 3);
dtd = list-next;
WARN_ON((void *)(dtd + 1) (list-buf.addr + list-buf.size));
@@ -664,31 +678,48 @@ void vpdma_add_out_dtd(struct vpdma_desc_list *list,
struct v4l2_rect *c_rect,
/*
* append an inbound data transfer descriptor to the given descriptor list,
* this sets up a 'memory to client' VPDMA transfer for the given VPDMA channel
+ *
+ * @list: vpdma desc list to which we add this decriptor
+ * @width: width of the image in pixels in memory(not the cropped width)
+ * @c_rect: crop params of input image
+ * @fmt: vpdma data format of the buffer
+ * dma_addr: dma address as seen by VPDMA
+ * chan: VPDMA channel
+ * field: top or bottom field info of the input image
+ * flags: VPDMA flags to configure some descriptor fileds
+ * frame_width/height: the complete width/height of the image presented to the
+ * client (this makes sense when multiple channels are
+ * connected to the same client, forming a larger frame)
+ * start_h, start_v: position where the given channel starts providing pixel
+ * data to the client (makes sense when multiple channels
+ * contribute to the client)
*/
-void vpdma_add_in_dtd(struct
Re: [PATCH v5 5/7] [media] of: move common endpoint parsing to drivers/of
Hi Philipp,
On 27/02/14 19:35, Philipp Zabel wrote:
This patch adds a new struct of_endpoint which is then embedded in struct
v4l2_of_endpoint and contains the endpoint properties that are not V4L2
(or even media) specific: the port number, endpoint id, local device tree
node and remote endpoint phandle. of_graph_parse_endpoint parses those
properties and is used by v4l2_of_parse_endpoint, which just adds the
V4L2 MBUS information to the containing v4l2_of_endpoint structure.
snip
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 8ecca7a..ba3cfca 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1985,6 +1985,37 @@ struct device_node *of_find_next_cache_node(const
struct device_node *np)
}
/**
+ * of_graph_parse_endpoint() - parse common endpoint node properties
+ * @node: pointer to endpoint device_node
+ * @endpoint: pointer to the OF endpoint data structure
+ *
+ * All properties are optional. If none are found, we don't set any flags.
+ * This means the port has a static configuration and no properties have
+ * to be specified explicitly.
+ * The caller should hold a reference to @node.
+ */
+int of_graph_parse_endpoint(const struct device_node *node,
+ struct of_endpoint *endpoint)
+{
+ struct device_node *port_node = of_get_parent(node);
Can port_node be NULL? Probably only if something is quite wrong, but
maybe it's safer to return error in that case.
+ memset(endpoint, 0, sizeof(*endpoint));
+
+ endpoint-local_node = node;
+ /*
+ * It doesn't matter whether the two calls below succeed.
+ * If they don't then the default value 0 is used.
+ */
+ of_property_read_u32(port_node, reg, endpoint-port);
+ of_property_read_u32(node, reg, endpoint-id);
If the endpoint does not have 'port' as parent (i.e. the shortened
format), the above will return the 'reg' of the device node (with
'device node' I mean the main node, with 'compatible' property).
And generally speaking, if struct of_endpoint is needed, maybe it would
be better to return the struct of_endpoint when iterating the ports and
endpoints. That way there's no need to do parsing afterwards, trying
to figure out if there's a parent port node, but the information is
already at hand.
+
+ of_node_put(port_node);
+
+ return 0;
+}
+EXPORT_SYMBOL(of_graph_parse_endpoint);
+
+/**
* of_graph_get_next_endpoint() - get next endpoint node
* @parent: pointer to the parent device node
* @prev: previous endpoint node, or NULL to get first
diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
index 3bbeb60..2b233db 100644
--- a/include/linux/of_graph.h
+++ b/include/linux/of_graph.h
@@ -14,7 +14,21 @@
#ifndef __LINUX_OF_GRAPH_H
#define __LINUX_OF_GRAPH_H
+/**
+ * struct of_endpoint - the OF graph endpoint data structure
+ * @port: identifier (value of reg property) of a port this endpoint belongs
to
+ * @id: identifier (value of reg property) of this endpoint
+ * @local_node: pointer to device_node of this endpoint
+ */
+struct of_endpoint {
+ unsigned int port;
+ unsigned int id;
+ const struct device_node *local_node;
+};
A few thoughts about the iteration, and the API in general.
In the omapdss version I separated iterating ports and endpoints, for
the two reasons:
1) I think there are cases where you may want to have properties in the
port node, for things that are common for all the port's endpoints.
2) if there are multiple ports, I think the driver code is cleaner if
you first take the port, decide what port that is and maybe call a
sub-function, and then iterate the endpoints for that port only.
Both of those are possible with the API in the series, but not very cleanly.
Also, if you just want to iterate the endpoints, it's easy to implement
a helper using the separate port and endpoint iterations.
Then, about the get_remote functions: I think there should be only one
function for that purpose, one that returns the device node that
contains the remote endpoint.
My reasoning is that the ports and endpoints, and their contents, should
be private to the device. So the only use to get the remote is to get
the actual device, to see if it's been probed, or maybe get some video
API for that device.
If the driver model used has some kind of master-driver, which goes
through all the display entities, I think the above is still valid. When
the master-driver follows the remote-link, it still needs to first get
the main device node, as the ports and endpoints make no sense without
the context of the main device node.
Tomi
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v5 6/7] of: Implement simplified graph binding for single port devices
On 27/02/14 19:35, Philipp Zabel wrote:
For simple devices with only one port, it can be made implicit.
The endpoint node can be a direct child of the device node.
snip
@@ -2105,9 +2112,11 @@ struct device_node *of_graph_get_remote_port_parent(
/* Get remote endpoint node. */
np = of_parse_phandle(node, remote-endpoint, 0);
- /* Walk 3 levels up only if there is 'ports' node. */
+ /* Walk 3 levels up only if there is 'ports' node */
for (depth = 3; depth np; depth--) {
np = of_get_next_parent(np);
+ if (depth == 3 of_node_cmp(np-name, port))
+ break;
if (depth == 2 of_node_cmp(np-name, ports))
break;
}
This function becomes a bit funny. Would it be clearer just to do
something like:
np = of_parse_phandle(node, remote-endpoint, 0);
np = of_get_next_parent(np);
if (of_node_cmp(np-name, port) != 0)
return np;
np = of_get_next_parent(np);
if (of_node_cmp(np-name, ports) != 0)
return np;
np = of_get_next_parent(np);
return np;
Tomi
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v2 7/7] v4l: ti-vpe: Add selection API in VPE driver
Hi Archit,
On 03/04/14 09:49, Archit Taneja wrote:
Add selection ioctl ops. For VPE, cropping makes sense only for the input to
VPE(or V4L2_BUF_TYPE_VIDEO_OUTPUT/MPLANE buffers) and composing makes sense
only for the output of VPE(or V4L2_BUF_TYPE_VIDEO_CAPTURE/MPLANE buffers).
For the CAPTURE type, V4L2_SEL_TGT_COMPOSE results in VPE writing the output
in a rectangle within the capture buffer. For the OUTPUT type,
V4L2_SEL_TGT_CROP
results in selecting a rectangle region within the source buffer.
Setting the crop/compose rectangles should successfully result in
re-configuration of registers which are affected when either source or
destination dimensions change, set_srcdst_params() is called for this purpose.
Signed-off-by: Archit Taneja arc...@ti.com
---
drivers/media/platform/ti-vpe/vpe.c | 142
1 file changed, 142 insertions(+)
diff --git a/drivers/media/platform/ti-vpe/vpe.c
b/drivers/media/platform/ti-vpe/vpe.c
index 03a6846..b938590 100644
--- a/drivers/media/platform/ti-vpe/vpe.c
+++ b/drivers/media/platform/ti-vpe/vpe.c
@@ -410,8 +410,10 @@ static struct vpe_q_data *get_q_data(struct vpe_ctx *ctx,
{
switch (type) {
case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
+ case V4L2_BUF_TYPE_VIDEO_OUTPUT:
return ctx-q_data[Q_DATA_SRC];
case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
+ case V4L2_BUF_TYPE_VIDEO_CAPTURE:
I noticed that the querycap implementation is wrong. It reports
V4L2_CAP_VIDEO_M2M instead of V4L2_CAP_VIDEO_M2M_MPLANE.
This driver is using the multiplanar formats, so the M2M_MPLANE cap should
be set.
This should be a separate patch.
BTW, did you test the driver with the v4l2-compliance tool? The latest version
(http://git.linuxtv.org/v4l-utils.git) has m2m support.
However, if you want to test streaming (the -s option), then you will probably
need to base your kernel on this tree:
http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/vb2-part1
That branch contains a pile of fixes for vb2 and without that v4l2-compliance -s
will fail a number of tests.
return ctx-q_data[Q_DATA_DST];
default:
BUG();
@@ -1585,6 +1587,143 @@ static int vpe_s_fmt(struct file *file, void *priv,
struct v4l2_format *f)
return set_srcdst_params(ctx);
}
+static int __vpe_try_selection(struct vpe_ctx *ctx, struct v4l2_selection *s)
+{
+ struct vpe_q_data *q_data;
+
+ if ((s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ (s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT))
+ return -EINVAL;
+
+ q_data = get_q_data(ctx, s-type);
+ if (!q_data)
+ return -EINVAL;
+
+ switch (s-target) {
+ case V4L2_SEL_TGT_COMPOSE:
+ case V4L2_SEL_TGT_COMPOSE_DEFAULT:
+ case V4L2_SEL_TGT_COMPOSE_BOUNDS:
+ /*
+ * COMPOSE target is only valid for capture buffer type, for
+ * output buffer type, assign existing crop parameters to the
+ * selection rectangle
+ */
+ if (s-type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
+ break;
+ } else {
No need for the 'else' keywork here.
+ s-r = q_data-c_rect;
+ return 0;
+ }
+
+ case V4L2_SEL_TGT_CROP:
+ case V4L2_SEL_TGT_CROP_DEFAULT:
+ case V4L2_SEL_TGT_CROP_BOUNDS:
+ /*
+ * CROP target is only valid for output buffer type, for capture
+ * buffer type, assign existing compose parameters to the
+ * selection rectangle
+ */
+ if (s-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
+ break;
+ } else {
Ditto.
+ s-r = q_data-c_rect;
+ return 0;
+ }
+ default:
+ return -EINVAL;
+ }
+
+ if (s-r.top 0 || s-r.left 0) {
+ vpe_err(ctx-dev, negative values for top and left\n);
+ s-r.top = s-r.left = 0;
+ }
+
+ v4l_bound_align_image(s-r.width, MIN_W, q_data-width, 1,
+ s-r.height, MIN_H, q_data-height, H_ALIGN, S_ALIGN);
+
+ /* adjust left/top if cropping rectangle is out of bounds */
+ if (s-r.left + s-r.width q_data-width)
+ s-r.left = q_data-width - s-r.width;
+ if (s-r.top + s-r.height q_data-height)
+ s-r.top = q_data-height - s-r.height;
+
+ return 0;
+}
+
+static int vpe_g_selection(struct file *file, void *fh,
+ struct v4l2_selection *s)
+{
+ struct vpe_ctx *ctx = file2ctx(file);
+ struct vpe_q_data *q_data;
+
+ if ((s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ (s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT))
+ return -EINVAL;
+
+ q_data = get_q_data(ctx, s-type);
+ if (!q_data)
+ return -EINVAL;
+
+ switch (s-target) {
+ /* return
Re: [PATCH 4/6] android: convert sync to fence api, v4
On Tue, Mar 04, 2014 at 09:20:58AM +0100, Maarten Lankhorst wrote:
op 04-03-14 09:14, Daniel Vetter schreef:
On Tue, Mar 04, 2014 at 08:50:38AM +0100, Maarten Lankhorst wrote:
op 03-03-14 22:11, Daniel Vetter schreef:
On Mon, Feb 17, 2014 at 04:57:19PM +0100, Maarten Lankhorst wrote:
Android syncpoints can be mapped to a timeline. This removes the need
to maintain a separate api for synchronization. I've left the android
trace events in place, but the core fence events should already be
sufficient for debugging.
v2:
- Call fence_remove_callback in sync_fence_free if not all fences have
fired.
v3:
- Merge Colin Cross' bugfixes, and the android fence merge optimization.
v4:
- Merge with the upstream fixes.
Signed-off-by: Maarten Lankhorst maarten.lankho...@canonical.com
---
Snipped everything but headers - Ian Lister from our android team is
signed up to have a more in-depth look at proper integration with android
syncpoints. Adding him to cc.
diff --git a/drivers/staging/android/sync.h
b/drivers/staging/android/sync.h
index 62e2255b1c1e..6036dbdc8e6f 100644
--- a/drivers/staging/android/sync.h
+++ b/drivers/staging/android/sync.h
@@ -21,6 +21,7 @@
#include linux/list.h
#include linux/spinlock.h
#include linux/wait.h
+#include linux/fence.h
struct sync_timeline;
struct sync_pt;
@@ -40,8 +41,6 @@ struct sync_fence;
* -1 if a will signal before b
* @free_pt: called before sync_pt is freed
* @release_obj: called before sync_timeline is freed
- * @print_obj: deprecated
- * @print_pt: deprecated
* @fill_driver_data: write implementation specific driver data to data.
* should return an error if there is not enough room
* as specified by size. This information is returned
@@ -67,13 +66,6 @@ struct sync_timeline_ops {
/* optional */
void (*release_obj)(struct sync_timeline *sync_timeline);
- /* deprecated */
- void (*print_obj)(struct seq_file *s,
- struct sync_timeline *sync_timeline);
-
- /* deprecated */
- void (*print_pt)(struct seq_file *s, struct sync_pt *sync_pt);
-
/* optional */
int (*fill_driver_data)(struct sync_pt *syncpt, void *data, int size);
@@ -104,42 +96,48 @@ struct sync_timeline {
/* protected by child_list_lock */
bool destroyed;
+ int context, value;
struct list_head child_list_head;
spinlock_t child_list_lock;
struct list_head active_list_head;
- spinlock_t active_list_lock;
+#ifdef CONFIG_DEBUG_FS
struct list_head sync_timeline_list;
+#endif
};
/**
* struct sync_pt - sync point
- * @parent: sync_timeline to which this sync_pt belongs
+ * @fence: base fence class
* @child_list: membership in sync_timeline.child_list_head
* @active_list: membership in sync_timeline.active_list_head
+ current
* @signaled_list: membership in temporary signaled_list on stack
* @fence: sync_fence to which the sync_pt belongs
* @pt_list: membership in sync_fence.pt_list_head
* @status: 1: signaled, 0:active, 0: error
* @timestamp: time which sync_pt status transitioned from active to
* signaled or error.
+===
+ patched
Conflict markers ...
Oops.
*/
struct sync_pt {
- struct sync_timeline *parent;
- struct list_head child_list;
+ struct fence base;
Hm, embedding feels wrong, since that still means that I'll need to
implement two kinds of fences in i915 - one using the seqno fence to make
dma-buf sync work, and one to implmenent sync_pt to make the android folks
happy.
If I can dream I think we should have a pointer to an underlying fence
here, i.e. a struct sync_pt would just be a userspace interface wrapper to
do explicit syncing using native fences, instead of implicit syncing like
with dma-bufs. But this is all drive-by comments from a very cursory
high-level look. I might be full of myself again ;-)
-Daniel
No, the idea is that because android syncpoint is simply another type of
dma-fence, that if you deal with normal fences then android can
automatically be handled too. The userspace fence api android exposes
could be very easily made to work for dma-fence, just pass a dma-fence
to sync_fence_create.
So exposing dma-fence would probably work for android too.
Hm, then why do we still have struct sync_pt around? Since it's just the
internal bit, with the userspace facing object being struct sync_fence,
I'd opt to shuffle any useful features into the core struct fence.
-Daniel
To keep compatibility with the android api. I think that gradually converting
them is going to be
more useful than to force all drivers to use a new api all at once. They
could keep android
syncpoint api for exporting, as long as they accept dma-fence for
importing/waiting.
We don't have any users of the android sync_pt stuff (outside of the
framework itself). So any considerations for existing drivers for
upstreaming are imo moot. At least for the in-kernel interfaces used. For
the actual
Re: [PATCH v5 6/7] of: Implement simplified graph binding for single port devices
Am Dienstag, den 04.03.2014, 11:06 +0200 schrieb Tomi Valkeinen:
On 27/02/14 19:35, Philipp Zabel wrote:
For simple devices with only one port, it can be made implicit.
The endpoint node can be a direct child of the device node.
snip
@@ -2105,9 +2112,11 @@ struct device_node *of_graph_get_remote_port_parent(
/* Get remote endpoint node. */
np = of_parse_phandle(node, remote-endpoint, 0);
- /* Walk 3 levels up only if there is 'ports' node. */
+ /* Walk 3 levels up only if there is 'ports' node */
for (depth = 3; depth np; depth--) {
np = of_get_next_parent(np);
+ if (depth == 3 of_node_cmp(np-name, port))
+ break;
if (depth == 2 of_node_cmp(np-name, ports))
break;
}
This function becomes a bit funny. Would it be clearer just to do
something like:
np = of_parse_phandle(node, remote-endpoint, 0);
np = of_get_next_parent(np);
if (of_node_cmp(np-name, port) != 0)
return np;
np = of_get_next_parent(np);
if (of_node_cmp(np-name, ports) != 0)
return np;
np = of_get_next_parent(np);
return np;
I'm not sure if this was initially crafted to reduce the number of
function calls, but rolled out it certainly is easier to read. I'll
change this as you suggest.
thanks
Philipp
--
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 v5 5/7] [media] of: move common endpoint parsing to drivers/of
Am Freitag, den 28.02.2014, 22:09 +0100 schrieb Sylwester Nawrocki: [...] --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -1985,6 +1985,37 @@ struct device_node *of_find_next_cache_node(const struct device_node *np) } /** + * of_graph_parse_endpoint() - parse common endpoint node properties + * @node: pointer to endpoint device_node + * @endpoint: pointer to the OF endpoint data structure + * + * All properties are optional. If none are found, we don't set any flags. + * This means the port has a static configuration and no properties have + * to be specified explicitly. I don't think these two sentences are needed, it's all described in the DT binding documentation. And struct of_endpoint doesn't contain any flags field. Thanks, I'll remove them. regards Philipp -- 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 v5 3/7] of: Warn if of_graph_get_next_endpoint is called with the root node
Am Freitag, den 28.02.2014, 22:09 +0100 schrieb Sylwester Nawrocki:
On 02/27/2014 06:35 PM, Philipp Zabel wrote:
If of_graph_get_next_endpoint is given a parentless node instead of an
endpoint node, it is clearly a bug.
Signed-off-by: Philipp Zabelp.za...@pengutronix.de
---
drivers/of/base.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index b2f223f..6e650cf 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2028,8 +2028,8 @@ struct device_node *of_graph_get_next_endpoint(const
struct device_node *parent,
of_node_put(node);
} else {
port = of_get_parent(prev);
- if (!port)
- /* Hm, has someone given us the root node ?... */
+ if (WARN_ONCE(!port, %s(): endpoint has no parent node\n,
+ __func__))
Perhaps we can add more information to this error log, e.g.
if (WARN_ONCE(!port, %s(): endpoint %s has no parent node\n,
__func__, prev-full_name))
?
Yes, I'll include this change next time.
thanks
Philipp
--
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 v5 2/7] Documentation: of: Document graph bindings
Hi Sylwester,
Am Freitag, den 28.02.2014, 22:08 +0100 schrieb Sylwester Nawrocki:
Hi Philipp,
Just couple minor comments...
Thanks, I'll fix all of those.
On 02/27/2014 06:35 PM, Philipp Zabel wrote:
The device tree graph bindings as used by V4L2 and documented in
Documentation/device-tree/bindings/media/video-interfaces.txt contain
generic parts that are not media specific but could be useful for any
subsystem with data flow between multiple devices. This document
describe the generic bindings.
s/describe/describes/
[...]
+These common bindings do not contain any information about the direction of
s/of/or/ ?
[...]
+device_1 {
+port {
+device_1_output: endpoint {
+remote-endpoint =device_2_input;
+};
+};
+};
+
+device_1 {
s/device_1/device_2/
But it might be better to use dashes instead of underscores
for the node names.
regards
Philipp
--
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
[REVIEWv4 PATCH 00/18] vb2: fixes, balancing callbacks (PART 1)
The fourth version, hopefully the last as well. Changes since REVIEWv3: - Split off the pwc patch into the buf_finish return type change and the patch that only decompresses the image if the buffer state is DONE. - Extend the comments in patch 07/18: explain that the buf_finish call in queue_cancel shouldn't be moved to __vb2_dqbuf, or we'll get the same bug that I introduced in an earlier version of this patch series. The actual code has been unchanged since v3. If there are no more comments, then I'll plan on making a pull request on Friday. Regards, Hans -- 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 4/6] android: convert sync to fence api, v4
op 04-03-14 11:00, Daniel Vetter schreef:
On Tue, Mar 04, 2014 at 09:20:58AM +0100, Maarten Lankhorst wrote:
op 04-03-14 09:14, Daniel Vetter schreef:
On Tue, Mar 04, 2014 at 08:50:38AM +0100, Maarten Lankhorst wrote:
op 03-03-14 22:11, Daniel Vetter schreef:
On Mon, Feb 17, 2014 at 04:57:19PM +0100, Maarten Lankhorst wrote:
Android syncpoints can be mapped to a timeline. This removes the need
to maintain a separate api for synchronization. I've left the android
trace events in place, but the core fence events should already be
sufficient for debugging.
v2:
- Call fence_remove_callback in sync_fence_free if not all fences have fired.
v3:
- Merge Colin Cross' bugfixes, and the android fence merge optimization.
v4:
- Merge with the upstream fixes.
Signed-off-by: Maarten Lankhorst maarten.lankho...@canonical.com
---
Snipped everything but headers - Ian Lister from our android team is
signed up to have a more in-depth look at proper integration with android
syncpoints. Adding him to cc.
diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h
index 62e2255b1c1e..6036dbdc8e6f 100644
--- a/drivers/staging/android/sync.h
+++ b/drivers/staging/android/sync.h
@@ -21,6 +21,7 @@
#include linux/list.h
#include linux/spinlock.h
#include linux/wait.h
+#include linux/fence.h
struct sync_timeline;
struct sync_pt;
@@ -40,8 +41,6 @@ struct sync_fence;
* -1 if a will signal before b
* @free_pt: called before sync_pt is freed
* @release_obj: called before sync_timeline is freed
- * @print_obj: deprecated
- * @print_pt: deprecated
* @fill_driver_data: write implementation specific driver data to data.
* should return an error if there is not enough room
* as specified by size. This information is returned
@@ -67,13 +66,6 @@ struct sync_timeline_ops {
/* optional */
void (*release_obj)(struct sync_timeline *sync_timeline);
- /* deprecated */
- void (*print_obj)(struct seq_file *s,
- struct sync_timeline *sync_timeline);
-
- /* deprecated */
- void (*print_pt)(struct seq_file *s, struct sync_pt *sync_pt);
-
/* optional */
int (*fill_driver_data)(struct sync_pt *syncpt, void *data, int size);
@@ -104,42 +96,48 @@ struct sync_timeline {
/* protected by child_list_lock */
bool destroyed;
+ int context, value;
struct list_head child_list_head;
spinlock_t child_list_lock;
struct list_head active_list_head;
- spinlock_t active_list_lock;
+#ifdef CONFIG_DEBUG_FS
struct list_head sync_timeline_list;
+#endif
};
/**
* struct sync_pt - sync point
- * @parent: sync_timeline to which this sync_pt belongs
+ * @fence: base fence class
* @child_list: membership in sync_timeline.child_list_head
* @active_list: membership in sync_timeline.active_list_head
+ current
* @signaled_list: membership in temporary signaled_list on stack
* @fence: sync_fence to which the sync_pt belongs
* @pt_list: membership in sync_fence.pt_list_head
* @status: 1: signaled, 0:active, 0: error
* @timestamp: time which sync_pt status transitioned from active to
* signaled or error.
+===
+ patched
Conflict markers ...
Oops.
*/
struct sync_pt {
- struct sync_timeline *parent;
- struct list_head child_list;
+ struct fence base;
Hm, embedding feels wrong, since that still means that I'll need to
implement two kinds of fences in i915 - one using the seqno fence to make
dma-buf sync work, and one to implmenent sync_pt to make the android folks
happy.
If I can dream I think we should have a pointer to an underlying fence
here, i.e. a struct sync_pt would just be a userspace interface wrapper to
do explicit syncing using native fences, instead of implicit syncing like
with dma-bufs. But this is all drive-by comments from a very cursory
high-level look. I might be full of myself again ;-)
-Daniel
No, the idea is that because android syncpoint is simply another type of
dma-fence, that if you deal with normal fences then android can
automatically be handled too. The userspace fence api android exposes
could be very easily made to work for dma-fence, just pass a dma-fence
to sync_fence_create.
So exposing dma-fence would probably work for android too.
Hm, then why do we still have struct sync_pt around? Since it's just the
internal bit, with the userspace facing object being struct sync_fence,
I'd opt to shuffle any useful features into the core struct fence.
-Daniel
To keep compatibility with the android api. I think that gradually converting
them is going to be
more useful than to force all drivers to use a new api all at once. They could
keep android
syncpoint api for exporting, as long as they accept dma-fence for
importing/waiting.
We don't have any users of the android sync_pt stuff (outside of the
framework itself). So any considerations for existing drivers for
upstreaming are imo moot. At least for the in-kernel interfaces used. For
the actual userspace interface I guess keeping the android syncpt ioctls
as-is has value,
[REVIEWv4 PATCH 03/18] vb2: fix PREPARE_BUF regression
Fix an incorrect test in vb2_internal_qbuf() where only DEQUEUED buffers
are allowed. But PREPARED buffers are also OK.
Introduced by commit 4138111a27859dcc56a5592c804dd16bb12a23d1
(vb2: simplify qbuf/prepare_buf by removing callback).
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
drivers/media/v4l2-core/videobuf2-core.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index f1a2857c..909f367 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1420,11 +1420,6 @@ static int vb2_internal_qbuf(struct vb2_queue *q, struct
v4l2_buffer *b)
return ret;
vb = q-bufs[b-index];
- if (vb-state != VB2_BUF_STATE_DEQUEUED) {
- dprintk(1, %s(): invalid buffer state %d\n, __func__,
- vb-state);
- return -EINVAL;
- }
switch (vb-state) {
case VB2_BUF_STATE_DEQUEUED:
@@ -1438,7 +1433,8 @@ static int vb2_internal_qbuf(struct vb2_queue *q, struct
v4l2_buffer *b)
dprintk(1, qbuf: buffer still being prepared\n);
return -EINVAL;
default:
- dprintk(1, qbuf: buffer already in use\n);
+ dprintk(1, %s(): invalid buffer state %d\n, __func__,
+ vb-state);
return -EINVAL;
}
--
1.9.0
--
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
[REVIEWv4 PATCH 02/18] vb2: fix read/write regression
From: Hans Verkuil hans.verk...@cisco.com
Commit 88e268702bfba78448abd20a31129458707383aa (vb2: Improve file I/O
emulation to handle buffers in any order) broke read/write support if
the size of the buffer being read/written is less than the size of the
image.
When the commit was tested originally I used qv4l2, which calls read()
with exactly the size of the image. But if you try 'cat /dev/video0'
then it will fail and typically hang after reading two buffers.
This patch fixes the behavior by adding a new cur_index field that
contains the index of the field currently being filled/read, or it
is num_buffers in which case a new buffer needs to be dequeued.
The old index field has been renamed to initial_index in order to be
a bit more descriptive.
This has been tested with both read and write.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
Tested-by: Hans Verkuil hans.verk...@cisco.com
Cc: Andy Walls awa...@md.metrocast.net
---
drivers/media/v4l2-core/videobuf2-core.c | 46 +++-
1 file changed, 40 insertions(+), 6 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index a127925..f1a2857c 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -2251,6 +2251,22 @@ struct vb2_fileio_buf {
/**
* struct vb2_fileio_data - queue context used by file io emulator
*
+ * @cur_index: the index of the buffer currently being read from or
+ * written to. If equal to q-num_buffers then a new buffer
+ * must be dequeued.
+ * @initial_index: in the read() case all buffers are queued up immediately
+ * in __vb2_init_fileio() and __vb2_perform_fileio() just cycles
+ * buffers. However, in the write() case no buffers are initially
+ * queued, instead whenever a buffer is full it is queued up by
+ * __vb2_perform_fileio(). Only once all available buffers have
+ * been queued up will __vb2_perform_fileio() start to dequeue
+ * buffers. This means that initially __vb2_perform_fileio()
+ * needs to know what buffer index to use when it is queuing up
+ * the buffers for the first time. That initial index is stored
+ * in this field. Once it is equal to q-num_buffers all
+ * available buffers have been queued and __vb2_perform_fileio()
+ * should start the normal dequeue/queue cycle.
+ *
* vb2 provides a compatibility layer and emulator of file io (read and
* write) calls on top of streaming API. For proper operation it required
* this structure to save the driver state between each call of the read
@@ -2260,7 +2276,8 @@ struct vb2_fileio_data {
struct v4l2_requestbuffers req;
struct v4l2_buffer b;
struct vb2_fileio_buf bufs[VIDEO_MAX_FRAME];
- unsigned int index;
+ unsigned int cur_index;
+ unsigned int initial_index;
unsigned int q_count;
unsigned int dq_count;
unsigned int flags;
@@ -2360,7 +2377,12 @@ static int __vb2_init_fileio(struct vb2_queue *q, int
read)
goto err_reqbufs;
fileio-bufs[i].queued = 1;
}
- fileio-index = q-num_buffers;
+ /*
+* All buffers have been queued, so mark that by setting
+* initial_index to q-num_buffers
+*/
+ fileio-initial_index = q-num_buffers;
+ fileio-cur_index = q-num_buffers;
}
/*
@@ -2439,7 +2461,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q,
char __user *data, size_
/*
* Check if we need to dequeue the buffer.
*/
- index = fileio-index;
+ index = fileio-cur_index;
if (index = q-num_buffers) {
/*
* Call vb2_dqbuf to get buffer back.
@@ -2453,7 +2475,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q,
char __user *data, size_
return ret;
fileio-dq_count += 1;
- index = fileio-b.index;
+ fileio-cur_index = index = fileio-b.index;
buf = fileio-bufs[index];
/*
@@ -2529,8 +2551,20 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q,
char __user *data, size_
buf-queued = 1;
buf-size = vb2_plane_size(q-bufs[index], 0);
fileio-q_count += 1;
- if (fileio-index q-num_buffers)
- fileio-index++;
+ /*
+* If we are queuing up buffers for the first time, then
+* increase initial_index by one.
+*/
+ if (fileio-initial_index q-num_buffers)
+ fileio-initial_index++;
+ /*
+* The next buffer to use is either a buffer that's
[REVIEWv4 PATCH 04/18] vb2: add debugging code to check for unbalanced ops
From: Hans Verkuil hans.verk...@cisco.com
When a vb2_queue is freed check if all the mem_ops and queue ops were balanced.
So the number of calls to e.g. buf_finish has to match the number of calls to
buf_prepare, etc.
This code is only enabled if CONFIG_VIDEO_ADV_DEBUG is set.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
Acked-by: Pawel Osciak pa...@osciak.com
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
drivers/media/v4l2-core/videobuf2-core.c | 233 ---
include/media/videobuf2-core.h | 43 ++
2 files changed, 226 insertions(+), 50 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 909f367..8f1578b 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -33,12 +33,63 @@ module_param(debug, int, 0644);
printk(KERN_DEBUG vb2: fmt, ## arg); \
} while (0)
-#define call_memop(q, op, args...) \
- (((q)-mem_ops-op) ? \
- ((q)-mem_ops-op(args)) : 0)
+#ifdef CONFIG_VIDEO_ADV_DEBUG
+
+/*
+ * If advanced debugging is on, then count how often each op is called,
+ * which can either be per-buffer or per-queue.
+ *
+ * If the op failed then the 'fail_' variant is called to decrease the
+ * counter. That makes it easy to check that the 'init' and 'cleanup'
+ * (and variations thereof) stay balanced.
+ */
+
+#define call_memop(vb, op, args...)\
+({ \
+ struct vb2_queue *_q = (vb)-vb2_queue; \
+ dprintk(2, call_memop(%p, %d, %s)%s\n,\
+ _q, (vb)-v4l2_buf.index, #op, \
+ _q-mem_ops-op ? : (nop)); \
+ (vb)-cnt_mem_ ## op++; \
+ _q-mem_ops-op ? _q-mem_ops-op(args) : 0;\
+})
+#define fail_memop(vb, op) ((vb)-cnt_mem_ ## op--)
+
+#define call_qop(q, op, args...) \
+({ \
+ dprintk(2, call_qop(%p, %s)%s\n, q, #op, \
+ (q)-ops-op ? : (nop)); \
+ (q)-cnt_ ## op++; \
+ (q)-ops-op ? (q)-ops-op(args) : 0; \
+})
+#define fail_qop(q, op) ((q)-cnt_ ## op--)
+
+#define call_vb_qop(vb, op, args...) \
+({ \
+ struct vb2_queue *_q = (vb)-vb2_queue; \
+ dprintk(2, call_vb_qop(%p, %d, %s)%s\n, \
+ _q, (vb)-v4l2_buf.index, #op, \
+ _q-ops-op ? : (nop)); \
+ (vb)-cnt_ ## op++; \
+ _q-ops-op ? _q-ops-op(args) : 0;\
+})
+#define fail_vb_qop(vb, op) ((vb)-cnt_ ## op--)
+
+#else
+
+#define call_memop(vb, op, args...)\
+ ((vb)-vb2_queue-mem_ops-op ? (vb)-vb2_queue-mem_ops-op(args) : 0)
+#define fail_memop(vb, op)
#define call_qop(q, op, args...) \
- (((q)-ops-op) ? ((q)-ops-op(args)) : 0)
+ ((q)-ops-op ? (q)-ops-op(args) : 0)
+#define fail_qop(q, op)
+
+#define call_vb_qop(vb, op, args...) \
+ ((vb)-vb2_queue-ops-op ? (vb)-vb2_queue-ops-op(args) : 0)
+#define fail_vb_qop(vb, op)
+
+#endif
#define V4L2_BUFFER_MASK_FLAGS (V4L2_BUF_FLAG_MAPPED | V4L2_BUF_FLAG_QUEUED | \
V4L2_BUF_FLAG_DONE | V4L2_BUF_FLAG_ERROR | \
@@ -61,7 +112,7 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
for (plane = 0; plane vb-num_planes; ++plane) {
unsigned long size = PAGE_ALIGN(q-plane_sizes[plane]);
- mem_priv = call_memop(q, alloc, q-alloc_ctx[plane],
+ mem_priv = call_memop(vb, alloc, q-alloc_ctx[plane],
size, q-gfp_flags);
if (IS_ERR_OR_NULL(mem_priv))
goto free;
@@ -73,9 +124,10 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
return 0;
free:
+ fail_memop(vb, alloc);
/* Free already allocated memory if one of the allocations failed */
for (; plane 0; --plane) {
- call_memop(q, put, vb-planes[plane - 1].mem_priv);
+ call_memop(vb, put, vb-planes[plane - 1].mem_priv);
vb-planes[plane - 1].mem_priv = NULL;
}
@@ -87,11 +139,10 @@ free:
*/
static void
[REVIEWv4 PATCH 01/18] vb2: Check if there are buffers before streamon
From: Ricardo Ribalda Delgado ricardo.riba...@gmail.com
This patch adds a test preventing streamon() if there is no buffer
ready.
Without this patch, a user could call streamon() before
preparing any buffer. This leads to a situation where if he calls
close() before calling streamoff() the device is kept streaming.
Signed-off-by: Ricardo Ribalda Delgado ricardo.riba...@gmail.com
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---
drivers/media/v4l2-core/videobuf2-core.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 5a5fb7f..a127925 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1776,6 +1776,11 @@ static int vb2_internal_streamon(struct vb2_queue *q,
enum v4l2_buf_type type)
return 0;
}
+ if (!q-num_buffers) {
+ dprintk(1, streamon: no buffers have been allocated\n);
+ return -EINVAL;
+ }
+
/*
* If any buffers were queued before streamon,
* we can now pass them to driver for processing.
--
1.9.0
--
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
[REVIEWv4 PATCH 13/18] vb2: properly clean up PREPARED and QUEUED buffers
From: Hans Verkuil hans.verk...@cisco.com If __reqbufs was called then existing buffers are freed. However, if that happens without ever having started STREAMON, but if buffers have been queued, then the buf_finish op is never called. Add a call to __vb2_queue_cancel in __reqbufs so that these buffers are cleaned up there as well. Signed-off-by: Hans Verkuil hans.verk...@cisco.com Acked-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/v4l2-core/videobuf2-core.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 1f6eccf..8dc8d50 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -96,6 +96,8 @@ module_param(debug, int, 0644); V4L2_BUF_FLAG_PREPARED | \ V4L2_BUF_FLAG_TIMESTAMP_MASK) +static void __vb2_queue_cancel(struct vb2_queue *q); + /** * __vb2_buf_mem_alloc() - allocate video memory for the given buffer */ @@ -789,6 +791,12 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req) return -EBUSY; } + /* +* Call queue_cancel to clean up any buffers in the PREPARED or +* QUEUED state which is possible if buffers were prepared or +* queued without ever calling STREAMON. +*/ + __vb2_queue_cancel(q); ret = __vb2_queue_free(q, q-num_buffers); if (ret) return ret; -- 1.9.0 -- 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
[REVIEWv4 PATCH 14/18] vb2: replace BUG by WARN_ON
From: Hans Verkuil hans.verk...@cisco.com No need to oops for this, WARN_ON is good enough. Signed-off-by: Hans Verkuil hans.verk...@cisco.com Acked-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/v4l2-core/videobuf2-core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 8dc8d50..e750769 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -2538,9 +2538,9 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read) /* * Sanity check */ - if ((read !(q-io_modes VB2_READ)) || - (!read !(q-io_modes VB2_WRITE))) - BUG(); + if (WARN_ON((read !(q-io_modes VB2_READ)) || + (!read !(q-io_modes VB2_WRITE + return -EINVAL; /* * Check if device supports mapping buffers to kernel virtual space. -- 1.9.0 -- 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
[REVIEWv4 PATCH 11/18] vb2: don't init the list if there are still buffers
From: Hans Verkuil hans.verk...@cisco.com
__vb2_queue_free() would init the queued_list at all times, even if
q-num_buffers 0. This should only happen if num_buffers == 0.
This situation can happen if a CREATE_BUFFERS call couldn't allocate
enough buffers and had to free those it did manage to allocate before
returning an error.
While we're at it: __vb2_queue_alloc() returns the number of buffers
allocated, not an error code. So stick the result in allocated_buffers
instead of ret as that's very confusing.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
drivers/media/v4l2-core/videobuf2-core.c | 29 +
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 018a132..07cce7f 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -452,9 +452,10 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned
int buffers)
}
q-num_buffers -= buffers;
- if (!q-num_buffers)
+ if (!q-num_buffers) {
q-memory = 0;
- INIT_LIST_HEAD(q-queued_list);
+ INIT_LIST_HEAD(q-queued_list);
+ }
return 0;
}
@@ -820,14 +821,12 @@ static int __reqbufs(struct vb2_queue *q, struct
v4l2_requestbuffers *req)
}
/* Finally, allocate buffers and video memory */
- ret = __vb2_queue_alloc(q, req-memory, num_buffers, num_planes);
- if (ret == 0) {
+ allocated_buffers = __vb2_queue_alloc(q, req-memory, num_buffers,
num_planes);
+ if (allocated_buffers == 0) {
dprintk(1, Memory allocation failed\n);
return -ENOMEM;
}
- allocated_buffers = ret;
-
/*
* Check if driver can handle the allocated number of buffers.
*/
@@ -851,6 +850,10 @@ static int __reqbufs(struct vb2_queue *q, struct
v4l2_requestbuffers *req)
q-num_buffers = allocated_buffers;
if (ret 0) {
+ /*
+* Note: __vb2_queue_free() will subtract 'allocated_buffers'
+* from q-num_buffers.
+*/
__vb2_queue_free(q, allocated_buffers);
return ret;
}
@@ -924,20 +927,18 @@ static int __create_bufs(struct vb2_queue *q, struct
v4l2_create_buffers *create
}
/* Finally, allocate buffers and video memory */
- ret = __vb2_queue_alloc(q, create-memory, num_buffers,
+ allocated_buffers = __vb2_queue_alloc(q, create-memory, num_buffers,
num_planes);
- if (ret == 0) {
+ if (allocated_buffers == 0) {
dprintk(1, Memory allocation failed\n);
return -ENOMEM;
}
- allocated_buffers = ret;
-
/*
* Check if driver can handle the so far allocated number of buffers.
*/
- if (ret num_buffers) {
- num_buffers = ret;
+ if (allocated_buffers num_buffers) {
+ num_buffers = allocated_buffers;
/*
* q-num_buffers contains the total number of buffers, that the
@@ -960,6 +961,10 @@ static int __create_bufs(struct vb2_queue *q, struct
v4l2_create_buffers *create
q-num_buffers += allocated_buffers;
if (ret 0) {
+ /*
+* Note: __vb2_queue_free() will subtract 'allocated_buffers'
+* from q-num_buffers.
+*/
__vb2_queue_free(q, allocated_buffers);
return -ENOMEM;
}
--
1.9.0
--
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
[REVIEWv4 PATCH 12/18] vb2: only call start_streaming if sufficient buffers are queued
From: Hans Verkuil hans.verk...@cisco.com
In commit 02f142ecd24aaf891324ffba8527284c1731b561 support was added to
start_streaming to return -ENOBUFS if insufficient buffers were queued
for the DMA engine to start. The vb2 core would attempt calling
start_streaming again if another buffer would be queued up.
Later analysis uncovered problems with the queue management if start_streaming
would return an error: the buffers are enqueued to the driver before the
start_streaming op is called, so after an error they are never returned to
the vb2 core. The solution for this is to let the driver return them to
the vb2 core in case of an error while starting the DMA engine. However,
in the case of -ENOBUFS that would be weird: it is not a real error, it
just says that more buffers are needed. Requiring start_streaming to give
them back only to have them requeued again the next time the application
calls QBUF is inefficient.
This patch changes this mechanism: it adds a 'min_buffers_needed' field
to vb2_queue that drivers can set with the minimum number of buffers
required to start the DMA engine. The start_streaming op is only called
if enough buffers are queued. The -ENOBUFS handling has been dropped in
favor of this new method.
Drivers are expected to return buffers back to vb2 core with state QUEUED
if start_streaming would return an error. The vb2 core checks for this
and produces a warning if that didn't happen and it will forcefully
reclaim such buffers to ensure that the internal vb2 core state remains
consistent and all buffer-related resources have been correctly freed
and all op calls have been balanced.
__reqbufs() has been updated to check that at least min_buffers_needed
buffers could be allocated. If fewer buffers were allocated then __reqbufs
will free what was allocated and return -ENOMEM. Based on a suggestion from
Pawel Osciak.
__create_bufs() doesn't do that check, since the use of __create_bufs
assumes some advance scenario where the user might want more control.
Instead streamon will check if enough buffers were allocated to prevent
streaming with fewer than the minimum required number of buffers.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---
drivers/media/platform/davinci/vpbe_display.c | 6 +-
drivers/media/platform/davinci/vpif_capture.c | 7 +-
drivers/media/platform/davinci/vpif_display.c | 7 +-
drivers/media/platform/s5p-tv/mixer_video.c | 6 +-
drivers/media/v4l2-core/videobuf2-core.c| 146
drivers/staging/media/davinci_vpfe/vpfe_video.c | 3 +-
include/media/videobuf2-core.h | 14 ++-
7 files changed, 116 insertions(+), 73 deletions(-)
diff --git a/drivers/media/platform/davinci/vpbe_display.c
b/drivers/media/platform/davinci/vpbe_display.c
index b02aba4..9231e48 100644
--- a/drivers/media/platform/davinci/vpbe_display.c
+++ b/drivers/media/platform/davinci/vpbe_display.c
@@ -344,11 +344,6 @@ static int vpbe_start_streaming(struct vb2_queue *vq,
unsigned int count)
struct vpbe_device *vpbe_dev = fh-disp_dev-vpbe_dev;
int ret;
- /* If buffer queue is empty, return error */
- if (list_empty(layer-dma_queue)) {
- v4l2_err(vpbe_dev-v4l2_dev, buffer queue is empty\n);
- return -ENOBUFS;
- }
/* Get the next frame from the buffer queue */
layer-next_frm = layer-cur_frm = list_entry(layer-dma_queue.next,
struct vpbe_disp_buffer, list);
@@ -1416,6 +1411,7 @@ static int vpbe_display_reqbufs(struct file *file, void
*priv,
q-mem_ops = vb2_dma_contig_memops;
q-buf_struct_size = sizeof(struct vpbe_disp_buffer);
q-timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+ q-min_buffers_needed = 1;
ret = vb2_queue_init(q);
if (ret) {
diff --git a/drivers/media/platform/davinci/vpif_capture.c
b/drivers/media/platform/davinci/vpif_capture.c
index 735ec47..9731ad4 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -272,13 +272,7 @@ static int vpif_start_streaming(struct vb2_queue *vq,
unsigned int count)
unsigned long flags;
int ret;
- /* If buffer queue is empty, return error */
spin_lock_irqsave(common-irqlock, flags);
- if (list_empty(common-dma_queue)) {
- spin_unlock_irqrestore(common-irqlock, flags);
- vpif_dbg(1, debug, buffer queue is empty\n);
- return -ENOBUFS;
- }
/* Get the next frame from the buffer queue */
common-cur_frm = common-next_frm = list_entry(common-dma_queue.next,
@@ -1024,6 +1018,7 @@ static int vpif_reqbufs(struct file *file, void *priv,
q-mem_ops = vb2_dma_contig_memops;
q-buf_struct_size = sizeof(struct vpif_cap_buffer);
q-timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+ q-min_buffers_needed = 1;
ret = vb2_queue_init(q);
[REVIEWv4 PATCH 05/18] vb2: change result code of buf_finish to void
From: Hans Verkuil hans.verk...@cisco.com
The buf_finish op should always work, so change the return type to void.
Update the few drivers that use it.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
Acked-by: Pawel Osciak pa...@osciak.com
Reviewed-by: Pawel Osciak pa...@osciak.com
---
drivers/media/parport/bw-qcam.c | 3 +--
drivers/media/pci/sta2x11/sta2x11_vip.c | 4 +---
drivers/media/platform/marvell-ccic/mcam-core.c | 3 +--
drivers/media/usb/pwc/pwc-if.c | 4 ++--
drivers/media/usb/uvc/uvc_queue.c | 3 +--
drivers/media/v4l2-core/videobuf2-core.c| 6 +-
drivers/staging/media/go7007/go7007-v4l2.c | 3 +--
include/media/videobuf2-core.h | 2 +-
8 files changed, 9 insertions(+), 19 deletions(-)
diff --git a/drivers/media/parport/bw-qcam.c b/drivers/media/parport/bw-qcam.c
index d12bd33..0166aef 100644
--- a/drivers/media/parport/bw-qcam.c
+++ b/drivers/media/parport/bw-qcam.c
@@ -667,7 +667,7 @@ static void buffer_queue(struct vb2_buffer *vb)
vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
}
-static int buffer_finish(struct vb2_buffer *vb)
+static void buffer_finish(struct vb2_buffer *vb)
{
struct qcam *qcam = vb2_get_drv_priv(vb-vb2_queue);
void *vbuf = vb2_plane_vaddr(vb, 0);
@@ -691,7 +691,6 @@ static int buffer_finish(struct vb2_buffer *vb)
if (len != size)
vb-state = VB2_BUF_STATE_ERROR;
vb2_set_plane_payload(vb, 0, len);
- return 0;
}
static struct vb2_ops qcam_video_qops = {
diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c
b/drivers/media/pci/sta2x11/sta2x11_vip.c
index e5cfb6c..e66556c 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -327,7 +327,7 @@ static void buffer_queue(struct vb2_buffer *vb)
}
spin_unlock(vip-lock);
}
-static int buffer_finish(struct vb2_buffer *vb)
+static void buffer_finish(struct vb2_buffer *vb)
{
struct sta2x11_vip *vip = vb2_get_drv_priv(vb-vb2_queue);
struct vip_buffer *vip_buf = to_vip_buffer(vb);
@@ -338,8 +338,6 @@ static int buffer_finish(struct vb2_buffer *vb)
spin_unlock(vip-lock);
vip_active_buf_next(vip);
-
- return 0;
}
static int start_streaming(struct vb2_queue *vq, unsigned int count)
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
b/drivers/media/platform/marvell-ccic/mcam-core.c
index 32fab30..8b34c48 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -1238,7 +1238,7 @@ static int mcam_vb_sg_buf_prepare(struct vb2_buffer *vb)
return 0;
}
-static int mcam_vb_sg_buf_finish(struct vb2_buffer *vb)
+static void mcam_vb_sg_buf_finish(struct vb2_buffer *vb)
{
struct mcam_camera *cam = vb2_get_drv_priv(vb-vb2_queue);
struct sg_table *sg_table = vb2_dma_sg_plane_desc(vb, 0);
@@ -1246,7 +1246,6 @@ static int mcam_vb_sg_buf_finish(struct vb2_buffer *vb)
if (sg_table)
dma_unmap_sg(cam-dev, sg_table-sgl,
sg_table-nents, DMA_FROM_DEVICE);
- return 0;
}
static void mcam_vb_sg_buf_cleanup(struct vb2_buffer *vb)
diff --git a/drivers/media/usb/pwc/pwc-if.c b/drivers/media/usb/pwc/pwc-if.c
index abf365a..619121f 100644
--- a/drivers/media/usb/pwc/pwc-if.c
+++ b/drivers/media/usb/pwc/pwc-if.c
@@ -614,7 +614,7 @@ static int buffer_prepare(struct vb2_buffer *vb)
return 0;
}
-static int buffer_finish(struct vb2_buffer *vb)
+static void buffer_finish(struct vb2_buffer *vb)
{
struct pwc_device *pdev = vb2_get_drv_priv(vb-vb2_queue);
struct pwc_frame_buf *buf = container_of(vb, struct pwc_frame_buf, vb);
@@ -624,7 +624,7 @@ static int buffer_finish(struct vb2_buffer *vb)
* filled, take the pwc data we've stored in buf-data and decompress
* it into a usable format, storing the result in the vb2_buffer
*/
- return pwc_decompress(pdev, buf);
+ pwc_decompress(pdev, buf);
}
static void buffer_cleanup(struct vb2_buffer *vb)
diff --git a/drivers/media/usb/uvc/uvc_queue.c
b/drivers/media/usb/uvc/uvc_queue.c
index cd962be..cca2c6e 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -104,7 +104,7 @@ static void uvc_buffer_queue(struct vb2_buffer *vb)
spin_unlock_irqrestore(queue-irqlock, flags);
}
-static int uvc_buffer_finish(struct vb2_buffer *vb)
+static void uvc_buffer_finish(struct vb2_buffer *vb)
{
struct uvc_video_queue *queue = vb2_get_drv_priv(vb-vb2_queue);
struct uvc_streaming *stream =
@@ -112,7 +112,6 @@ static int uvc_buffer_finish(struct vb2_buffer *vb)
struct uvc_buffer *buf = container_of(vb, struct uvc_buffer, buf);
uvc_video_clock_update(stream, vb-v4l2_buf, buf);
- return 0;
}
static void uvc_wait_prepare(struct vb2_queue *vq)
diff --git
[REVIEWv4 PATCH 18/18] vivi: fix ENUM_FRAMEINTERVALS implementation
From: Hans Verkuil hans.verk...@cisco.com This function never checked if width and height are correct. Add such a check so the v4l2-compliance tool returns OK again for vivi. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/platform/vivi.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c index 643937b..7360a84 100644 --- a/drivers/media/platform/vivi.c +++ b/drivers/media/platform/vivi.c @@ -1121,7 +1121,11 @@ static int vidioc_enum_frameintervals(struct file *file, void *priv, if (!fmt) return -EINVAL; - /* regarding width height - we support any */ + /* check for valid width/height */ + if (fival-width 48 || fival-width MAX_WIDTH || (fival-width 3)) + return -EINVAL; + if (fival-height 32 || fival-height MAX_HEIGHT) + return -EINVAL; fival-type = V4L2_FRMIVAL_TYPE_CONTINUOUS; -- 1.9.0 -- 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
[REVIEWv4 PATCH 08/18] vb2: consistent usage of periods in videobuf2-core.h
From: Hans Verkuil hans.verk...@cisco.com
Sometimes sentences in comments ended with a period, and sometimes they
didn't. Add periods. No other changes.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---
include/media/videobuf2-core.h | 40
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 3cb0bcf..06efa4a 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -34,49 +34,49 @@ struct vb2_fileio_data;
* usually will result in the allocator freeing the buffer (if
* no other users of this buffer are present); the buf_priv
* argument is the allocator private per-buffer structure
- * previously returned from the alloc callback
+ * previously returned from the alloc callback.
* @get_userptr: acquire userspace memory for a hardware operation; used for
* USERPTR memory types; vaddr is the address passed to the
* videobuf layer when queuing a video buffer of USERPTR type;
* should return an allocator private per-buffer structure
* associated with the buffer on success, NULL on failure;
* the returned private structure will then be passed as buf_priv
- * argument to other ops in this structure
+ * argument to other ops in this structure.
* @put_userptr: inform the allocator that a USERPTR buffer will no longer
- * be used
+ * be used.
* @attach_dmabuf: attach a shared struct dma_buf for a hardware operation;
*used for DMABUF memory types; alloc_ctx is the alloc context
*dbuf is the shared dma_buf; returns NULL on failure;
*allocator private per-buffer structure on success;
- *this needs to be used for further accesses to the buffer
+ *this needs to be used for further accesses to the buffer.
* @detach_dmabuf: inform the exporter of the buffer that the current DMABUF
*buffer is no longer used; the buf_priv argument is the
*allocator private per-buffer structure previously returned
- *from the attach_dmabuf callback
+ *from the attach_dmabuf callback.
* @map_dmabuf: request for access to the dmabuf from allocator; the allocator
* of dmabuf is informed that this driver is going to use the
- * dmabuf
+ * dmabuf.
* @unmap_dmabuf: releases access control to the dmabuf - allocator is notified
- * that this driver is done using the dmabuf for now
+ * that this driver is done using the dmabuf for now.
* @prepare: called every time the buffer is passed from userspace to the
- * driver, useful for cache synchronisation, optional
+ * driver, useful for cache synchronisation, optional.
* @finish:called every time the buffer is passed back from the driver
- * to the userspace, also optional
+ * to the userspace, also optional.
* @vaddr: return a kernel virtual address to a given memory buffer
* associated with the passed private structure or NULL if no
- * such mapping exists
+ * such mapping exists.
* @cookie:return allocator specific cookie for a given memory buffer
* associated with the passed private structure or NULL if not
- * available
+ * available.
* @num_users: return the current number of users of a memory buffer;
* return 1 if the videobuf layer (or actually the driver using
- * it) is the only user
+ * it) is the only user.
* @mmap: setup a userspace mapping for a given memory buffer under
- * the provided virtual memory region
+ * the provided virtual memory region.
*
* Required ops for USERPTR types: get_userptr, put_userptr.
* Required ops for MMAP types: alloc, put, num_users, mmap.
- * Required ops for read/write access types: alloc, put, num_users, vaddr
+ * Required ops for read/write access types: alloc, put, num_users, vaddr.
* Required ops for DMABUF types: attach_dmabuf, detach_dmabuf, map_dmabuf,
* unmap_dmabuf.
*/
@@ -258,22 +258,22 @@ struct vb2_buffer {
* @wait_prepare: release any locks taken while calling vb2 functions;
* it is called before an ioctl needs to wait for a new
* buffer to arrive; required to avoid a deadlock in
- * blocking access type
+ * blocking access type.
* @wait_finish: reacquire all locks released in the previous callback;
* required to continue operation after sleeping while
- * waiting for a new buffer to
[REVIEWv4 PATCH 17/18] vivi: correctly cleanup after a start_streaming failure
From: Hans Verkuil hans.verk...@cisco.com
If start_streaming fails then any queued buffers must be given back
to the vb2 core.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---
drivers/media/platform/vivi.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
index e9cd96e..643937b 100644
--- a/drivers/media/platform/vivi.c
+++ b/drivers/media/platform/vivi.c
@@ -889,10 +889,20 @@ static void buffer_queue(struct vb2_buffer *vb)
static int start_streaming(struct vb2_queue *vq, unsigned int count)
{
struct vivi_dev *dev = vb2_get_drv_priv(vq);
+ int err;
dprintk(dev, 1, %s\n, __func__);
dev-seq_count = 0;
- return vivi_start_generating(dev);
+ err = vivi_start_generating(dev);
+ if (err) {
+ struct vivi_buffer *buf, *tmp;
+
+ list_for_each_entry_safe(buf, tmp, dev-vidq.active, list) {
+ list_del(buf-list);
+ vb2_buffer_done(buf-vb, VB2_BUF_STATE_QUEUED);
+ }
+ }
+ return err;
}
/* abort streaming and wait for last buffer */
--
1.9.0
--
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
[REVIEWv4 PATCH 15/18] vb2: fix streamoff handling if streamon wasn't called.
From: Hans Verkuil hans.verk...@cisco.com
If you request buffers, then queue buffers and then call STREAMOFF
those buffers are not returned to their dequeued state because streamoff
will just return if q-streaming was 0.
This means that afterwards you can never QBUF that same buffer again unless
you do STREAMON, REQBUFS or close the filehandle first.
It is clear that if you do STREAMOFF even if no STREAMON was called before,
you still want to have all buffers returned to their proper dequeued state.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---
drivers/media/v4l2-core/videobuf2-core.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index e750769..3c00e22 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -2069,14 +2069,14 @@ static int vb2_internal_streamoff(struct vb2_queue *q,
enum v4l2_buf_type type)
return -EINVAL;
}
- if (!q-streaming) {
- dprintk(3, streamoff successful: not streaming\n);
- return 0;
- }
-
/*
* Cancel will pause streaming and remove all buffers from the driver
* and videobuf, effectively returning control over them to userspace.
+*
+* Note that we do this even if q-streaming == 0: if you prepare or
+* queue buffers, and then call streamoff without ever having called
+* streamon, you would still expect those buffers to be returned to
+* their normal dequeued state.
*/
__vb2_queue_cancel(q);
--
1.9.0
--
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
[REVIEWv4 PATCH 10/18] vb2: rename queued_count to owned_by_drv_count
From: Hans Verkuil hans.verk...@cisco.com
'queued_count' is a bit vague since it is not clear to which queue it
refers to: the vb2 internal list of buffers or the driver-owned list
of buffers.
Rename to make it explicit.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
Acked-by: Pawel Osciak pa...@osciak.com
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
drivers/media/v4l2-core/videobuf2-core.c | 10 +-
include/media/videobuf2-core.h | 4 ++--
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 7753abe..018a132 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1071,7 +1071,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum
vb2_buffer_state state)
spin_lock_irqsave(q-done_lock, flags);
vb-state = state;
list_add_tail(vb-done_entry, q-done_list);
- atomic_dec(q-queued_count);
+ atomic_dec(q-owned_by_drv_count);
spin_unlock_irqrestore(q-done_lock, flags);
/* Inform any processes that may be waiting for buffers */
@@ -1402,7 +1402,7 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
unsigned int plane;
vb-state = VB2_BUF_STATE_ACTIVE;
- atomic_inc(q-queued_count);
+ atomic_inc(q-owned_by_drv_count);
/* sync buffers */
for (plane = 0; plane vb-num_planes; ++plane)
@@ -1554,7 +1554,7 @@ static int vb2_start_streaming(struct vb2_queue *q)
int ret;
/* Tell the driver to start streaming */
- ret = call_qop(q, start_streaming, q, atomic_read(q-queued_count));
+ ret = call_qop(q, start_streaming, q,
atomic_read(q-owned_by_drv_count));
if (ret)
fail_qop(q, start_streaming);
@@ -1775,7 +1775,7 @@ int vb2_wait_for_all_buffers(struct vb2_queue *q)
}
if (!q-retry_start_streaming)
- wait_event(q-done_wq, !atomic_read(q-queued_count));
+ wait_event(q-done_wq, !atomic_read(q-owned_by_drv_count));
return 0;
}
EXPORT_SYMBOL_GPL(vb2_wait_for_all_buffers);
@@ -1907,7 +1907,7 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
* has not already dequeued before initiating cancel.
*/
INIT_LIST_HEAD(q-done_list);
- atomic_set(q-queued_count, 0);
+ atomic_set(q-owned_by_drv_count, 0);
wake_up_all(q-done_wq);
/*
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 06efa4a..4f8dce2 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -359,7 +359,7 @@ struct v4l2_fh;
* @bufs: videobuf buffer structures
* @num_buffers: number of allocated/used buffers
* @queued_list: list of buffers currently queued from userspace
- * @queued_count: number of buffers owned by the driver
+ * @owned_by_drv_count: number of buffers owned by the driver
* @done_list: list of buffers ready to be dequeued to userspace
* @done_lock: lock to protect done_list list
* @done_wq: waitqueue for processes waiting for buffers ready to be dequeued
@@ -391,7 +391,7 @@ struct vb2_queue {
struct list_headqueued_list;
- atomic_tqueued_count;
+ atomic_towned_by_drv_count;
struct list_headdone_list;
spinlock_t done_lock;
wait_queue_head_t done_wq;
--
1.9.0
--
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
[REVIEWv4 PATCH 09/18] vb2: fix buf_init/buf_cleanup call sequences
From: Hans Verkuil hans.verk...@cisco.com
Ensure that these ops are properly balanced.
There are two scenarios:
1) for MMAP buf_init is called when the buffers are created and buf_cleanup
must be called when the queue is finally freed. This scenario was always
working.
2) for USERPTR and DMABUF it is more complicated. When a buffer is queued
the code checks if all planes of this buffer have been acquired before.
If that's the case, then only buf_prepare has to be called. Otherwise
buf_cleanup needs to be called if the buffer was acquired before, then,
once all changed planes have been (re)acquired, buf_init has to be
called followed by buf_prepare. Should buf_prepare fail, then buf_cleanup
must be called on the newly acquired planes to release them in.
Finally, in __vb2_queue_free we have to check if the buffer was actually
acquired before calling buf_cleanup. While that it always true for MMAP
mode, it is not necessarily true for the other modes. E.g. if you just
call REQBUFS and close the file handle, then buffers were never queued and
so no buf_init was ever called.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---
drivers/media/v4l2-core/videobuf2-core.c | 100 +--
1 file changed, 67 insertions(+), 33 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index f42520b..7753abe 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -373,8 +373,10 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned
int buffers)
/* Call driver-provided cleanup function for each buffer, if provided */
for (buffer = q-num_buffers - buffers; buffer q-num_buffers;
++buffer) {
- if (q-bufs[buffer])
- call_vb_qop(q-bufs[buffer], buf_cleanup,
q-bufs[buffer]);
+ struct vb2_buffer *vb = q-bufs[buffer];
+
+ if (vb vb-planes[0].mem_priv)
+ call_vb_qop(vb, buf_cleanup, vb);
}
/* Release video buffer memory */
@@ -1161,6 +1163,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const
struct v4l2_buffer *b)
unsigned int plane;
int ret;
int write = !V4L2_TYPE_IS_OUTPUT(q-type);
+ bool reacquired = vb-planes[0].mem_priv == NULL;
/* Copy relevant information provided by the userspace */
__fill_vb2_buffer(vb, b, planes);
@@ -1186,12 +1189,16 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const
struct v4l2_buffer *b)
}
/* Release previously acquired memory if present */
- if (vb-planes[plane].mem_priv)
+ if (vb-planes[plane].mem_priv) {
+ if (!reacquired) {
+ reacquired = true;
+ call_vb_qop(vb, buf_cleanup, vb);
+ }
call_memop(vb, put_userptr, vb-planes[plane].mem_priv);
+ }
vb-planes[plane].mem_priv = NULL;
- vb-v4l2_planes[plane].m.userptr = 0;
- vb-v4l2_planes[plane].length = 0;
+ memset(vb-v4l2_planes[plane], 0, sizeof(struct v4l2_plane));
/* Acquire each plane's memory */
mem_priv = call_memop(vb, get_userptr, q-alloc_ctx[plane],
@@ -1208,23 +1215,34 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const
struct v4l2_buffer *b)
}
/*
-* Call driver-specific initialization on the newly acquired buffer,
-* if provided.
-*/
- ret = call_vb_qop(vb, buf_init, vb);
- if (ret) {
- dprintk(1, qbuf: buffer initialization failed\n);
- fail_vb_qop(vb, buf_init);
- goto err;
- }
-
- /*
* Now that everything is in order, copy relevant information
* provided by userspace.
*/
for (plane = 0; plane vb-num_planes; ++plane)
vb-v4l2_planes[plane] = planes[plane];
+ if (reacquired) {
+ /*
+* One or more planes changed, so we must call buf_init to do
+* the driver-specific initialization on the newly acquired
+* buffer, if provided.
+*/
+ ret = call_vb_qop(vb, buf_init, vb);
+ if (ret) {
+ dprintk(1, qbuf: buffer initialization failed\n);
+ fail_vb_qop(vb, buf_init);
+ goto err;
+ }
+ }
+
+ ret = call_vb_qop(vb, buf_prepare, vb);
+ if (ret) {
+ dprintk(1, qbuf: buffer preparation failed\n);
+ fail_vb_qop(vb, buf_prepare);
+ call_vb_qop(vb, buf_cleanup, vb);
+ goto err;
+ }
+
return 0;
err:
/* In case of errors, release planes that were already
[REVIEWv4 PATCH 07/18] vb2: call buf_finish from __queue_cancel.
From: Hans Verkuil hans.verk...@cisco.com
If a queue was canceled, then the buf_finish op was never called for the
pending buffers. So add this call to queue_cancel. Before calling buf_finish
set the buffer state to PREPARED, which is the correct state. That way the
states DONE and ERROR will only be seen in buf_finish if streaming is in
progress.
Since buf_finish can now be called from non-streaming state we need to
adapt the handful of drivers that actually need to know this.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
Acked-by: Sakari Ailus sakari.ai...@linux.intel.com
---
drivers/media/parport/bw-qcam.c | 3 +++
drivers/media/pci/sta2x11/sta2x11_vip.c | 3 ++-
drivers/media/usb/uvc/uvc_queue.c| 3 ++-
drivers/media/v4l2-core/videobuf2-core.c | 17 +++--
include/media/videobuf2-core.h | 10 +-
5 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/drivers/media/parport/bw-qcam.c b/drivers/media/parport/bw-qcam.c
index 0166aef..8d69bfb 100644
--- a/drivers/media/parport/bw-qcam.c
+++ b/drivers/media/parport/bw-qcam.c
@@ -674,6 +674,9 @@ static void buffer_finish(struct vb2_buffer *vb)
int size = vb-vb2_queue-plane_sizes[0];
int len;
+ if (!vb2_is_streaming(vb-vb2_queue))
+ return;
+
mutex_lock(qcam-lock);
parport_claim_or_block(qcam-pdev);
diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c
b/drivers/media/pci/sta2x11/sta2x11_vip.c
index e66556c..bb11443 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -337,7 +337,8 @@ static void buffer_finish(struct vb2_buffer *vb)
list_del_init(vip_buf-list);
spin_unlock(vip-lock);
- vip_active_buf_next(vip);
+ if (vb2_is_streaming(vb-vb2_queue))
+ vip_active_buf_next(vip);
}
static int start_streaming(struct vb2_queue *vq, unsigned int count)
diff --git a/drivers/media/usb/uvc/uvc_queue.c
b/drivers/media/usb/uvc/uvc_queue.c
index cca2c6e..ab9f96e 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -111,7 +111,8 @@ static void uvc_buffer_finish(struct vb2_buffer *vb)
container_of(queue, struct uvc_streaming, queue);
struct uvc_buffer *buf = container_of(vb, struct uvc_buffer, buf);
- uvc_video_clock_update(stream, vb-v4l2_buf, buf);
+ if (vb-state == VB2_BUF_STATE_DONE)
+ uvc_video_clock_update(stream, vb-v4l2_buf, buf);
}
static void uvc_wait_prepare(struct vb2_queue *vq)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 59bfd85..f42520b 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1878,9 +1878,22 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
/*
* Reinitialize all buffers for next use.
+* Make sure to call buf_finish for any queued buffers. Normally
+* that's done in dqbuf, but that's not going to happen when we
+* cancel the whole queue. Note: this code belongs here, not in
+* __vb2_dqbuf() since in vb2_internal_dqbuf() there is a critical
+* call to __fill_v4l2_buffer() after buf_finish(). That order can't
+* be changed, so we can't move the buf_finish() to __vb2_dqbuf().
*/
- for (i = 0; i q-num_buffers; ++i)
- __vb2_dqbuf(q-bufs[i]);
+ for (i = 0; i q-num_buffers; ++i) {
+ struct vb2_buffer *vb = q-bufs[i];
+
+ if (vb-state != VB2_BUF_STATE_DEQUEUED) {
+ vb-state = VB2_BUF_STATE_PREPARED;
+ call_vb_qop(vb, buf_finish, vb);
+ }
+ __vb2_dqbuf(vb);
+ }
}
static int vb2_internal_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index f443ce0..3cb0bcf 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -276,7 +276,15 @@ struct vb2_buffer {
* in driver; optional
* @buf_finish:called before every dequeue of the buffer back
to
* userspace; drivers may perform any operations required
- * before userspace accesses the buffer; optional
+ * before userspace accesses the buffer; optional. The
+ * buffer state can be one of the following: DONE and
+ * ERROR occur while streaming is in progress, and the
+ * PREPARED state occurs when the queue has been canceled
+ * and all pending buffers are being returned to their
+ * default DEQUEUED state. Typically you only have to do
+ * something if the state is VB2_BUF_STATE_DONE, since in
+ * all other cases the buffer contents
[REVIEWv4 PATCH 16/18] vb2: call buf_finish after the state check.
From: Hans Verkuil hans.verk...@cisco.com
Don't call buf_finish unless we know that the buffer is in a valid state.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
Acked-by: Sakari Ailus sakari.ai...@linux.intel.com
---
drivers/media/v4l2-core/videobuf2-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 3c00e22..54cea42 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1878,8 +1878,6 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct
v4l2_buffer *b, bool n
if (ret 0)
return ret;
- call_vb_qop(vb, buf_finish, vb);
-
switch (vb-state) {
case VB2_BUF_STATE_DONE:
dprintk(3, dqbuf: Returning done buffer\n);
@@ -1892,6 +1890,8 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct
v4l2_buffer *b, bool n
return -EINVAL;
}
+ call_vb_qop(vb, buf_finish, vb);
+
/* Fill buffer information for the userspace */
__fill_v4l2_buffer(vb, b);
/* Remove from videobuf queue */
--
1.9.0
--
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
[REVIEWv4 PATCH 06/18] pwc: do not decompress the image unless the state is DONE
From: Hans Verkuil hans.verk...@cisco.com
There is no point in trying to decompress a captured frame unless
the buffer state is OK. It won't be used in any other state, and
in fact the contents of the buffer might well be corrupt.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---
drivers/media/usb/pwc/pwc-if.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/media/usb/pwc/pwc-if.c b/drivers/media/usb/pwc/pwc-if.c
index 619121f..d13e7c9 100644
--- a/drivers/media/usb/pwc/pwc-if.c
+++ b/drivers/media/usb/pwc/pwc-if.c
@@ -619,12 +619,15 @@ static void buffer_finish(struct vb2_buffer *vb)
struct pwc_device *pdev = vb2_get_drv_priv(vb-vb2_queue);
struct pwc_frame_buf *buf = container_of(vb, struct pwc_frame_buf, vb);
- /*
-* Application has called dqbuf and is getting back a buffer we've
-* filled, take the pwc data we've stored in buf-data and decompress
-* it into a usable format, storing the result in the vb2_buffer
-*/
- pwc_decompress(pdev, buf);
+ if (vb-state == VB2_BUF_STATE_DONE) {
+ /*
+* Application has called dqbuf and is getting back a buffer
+* we've filled, take the pwc data we've stored in buf-data
+* and decompress it into a usable format, storing the result
+* in the vb2_buffer.
+*/
+ pwc_decompress(pdev, buf);
+ }
}
static void buffer_cleanup(struct vb2_buffer *vb)
--
1.9.0
--
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: [REVIEWv2 PATCH 39/34] v4l2-ctrls: allow HIDDEN controls in the user class
Acked-by: Ricardo Ribalda ricardo.riba...@gmail.com
(Sorry for the delay)
On Sat, Feb 15, 2014 at 1:04 PM, Hans Verkuil hverk...@xs4all.nl wrote:
Previously hidden controls were not allowed in the user class due to
backwards compatibility reasons (QUERYCTRL should not see them), but
by simply testing if QUERYCTRL found a hidden control and returning
-EINVAL this limitation can be lifted.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---
drivers/media/v4l2-core/v4l2-ctrls.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
b/drivers/media/v4l2-core/v4l2-ctrls.c
index bc30c50..859ac29 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1853,15 +1853,6 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct
v4l2_ctrl_handler *hdl,
/* Complex controls are always hidden */
if (is_matrix || type = V4L2_CTRL_COMPLEX_TYPES)
flags |= V4L2_CTRL_FLAG_HIDDEN;
- /*
-* No hidden controls are allowed in the USER class
-* due to backwards compatibility with old applications.
-*/
- if (V4L2_CTRL_ID2CLASS(id) == V4L2_CTRL_CLASS_USER
- (flags V4L2_CTRL_FLAG_HIDDEN)) {
- handler_set_err(hdl, -EINVAL);
- return NULL;
- }
err = check_range(type, min, max, step, def);
if (err) {
handler_set_err(hdl, err);
@@ -2469,6 +2460,9 @@ int v4l2_queryctrl(struct v4l2_ctrl_handler *hdl,
struct v4l2_queryctrl *qc)
if (rc)
return rc;
+ /* VIDIOC_QUERYCTRL is not allowed to see hidden controls */
+ if (qc-flags V4L2_CTRL_FLAG_HIDDEN)
+ return -EINVAL;
qc-id = qec.id;
qc-type = qec.type;
qc-flags = qec.flags;
--
1.8.5.2
--
Ricardo Ribalda
--
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: Dell XPS 12 USB camera bulk mode issues
Hi Mark,
On Friday 28 February 2014 10:34:24 Mark Ryan wrote:
On 02/26/2014 04:40 PM, Laurent Pinchart wrote:
[ ... ]
With the information I've given you, could you try to log more information
in the driver to try and find what goes wrong ? You could for instance
log the content of each header at the beginning of the
uvc_video_decode_start() function.
So maybe I have something here. I ran guvcview and set the format to
MJPEG running at a high resolution. I'm using the kernel 3.11.0-17 that
comes with Ubuntu 13.10. In the usbmon logs I see the following.
[...]
SETUP Host-to-device Class request to Interface
bRequest: SET CUR (01)
wValue: 0200
wIndex: INTF 1 ENTITY 0 (0001)
wLength: 001a
26 data bytes
bmHint 0x01
bFormatIndex 2
bFrameIndex 10
dwFrameInterval 33
wKeyFrameRate 0
wPFrameRate 0
wCompQuality 0
wCompWindowSize 0
wDelay 32
dwMaxVideoFrameSize 1843200
dwMaxPayloadTransferSize 34816
Note the dwMaxPayloadTransferSize of 34816
[...]
Now I have my first payload
16384 data bytes
0c8d863e 8c007d67.†Œ.}g
8e00b304 ffd8ffdbŽ.³.ÿØÿÛ
00430003 02020202.C..
02030202 02030303
16384 data bytes
fbf5aeff 00c25e12ûõ®ÿ.Â^.
5d1244d5 353b2437].DÕ5;$7
f92d06e6 24c28475ù-.æ$„u
c740c6b3 8bbb29adÇ@Ƴ‹»)
16384 data bytes
f1a3cc60 b8200c52ñ£Ì`¸ .R
0108eac3 1cd4610e..êÃ.Ôa.
ece45032 40c477a8ìäP2@Äw¨
e4f35b81 d05001fcäó[ÐP.ü
16384 data bytes
fb3528d1 bc4fcc56û5(ѼOÌV
fe2d9522 1d04b1e7þ-•..±ç
3fa1a9e5 2b9867f6?¡©å+˜gö
778e2dd0 7d9b57b5wŽ-Ð}›Wµ
16384 data bytes
c016baf6 99732c9aÀ.ºö™s,š
9db39b7b e6b995a4³›{湕¤
65751f78 649eb5edeu.xdžµí
9e2dfdb1 355fd9faž-ý±5_Ùú
16384 data bytes
a73d4053 8fad795c§=@Sy\
c9088170 ec1c1e54É.pì..T
9ce6a59a d89f5742œæ¥šØŸWB
82cf3297 26d95b18‚Ï2—Ù[.
16384 data bytes
b4d3b21b d4a53595´Ó².Ô¥5•
99cac904 2e1bb346™ÊÉ...³F
09fe555e 4d2b4b95.þU^M+K•
be6b488a 0e91b0ca¾kHŠ.‘°Ê
16384 data bytes
3adb9dbc 6323ad21:Û¼c#!
8aa85b9d b934d31eŠ¨[¹4Ó.
465885f6 a00ad25cFX…ö .Ò\
04fdd44b be43d08a.ýÔK¾CЊ
16384 data bytes
14f1cf4c 0ae43589.ñÏL.ä5‰
0acdbd0b c8afedc2.ͽ.ȯíÂ
f15d4339 8bfb5b2bñ]C9‹û[+
df39e42d 14ecb849ß9ä-.ì¸I
10466 data bytes
7fffd1fc af99e466ÿÑü¯™äf
c39cd475 cab500a2ÃœÔuʵ.¢
ad8ae147 d295ec0cŠáGÒ•ì.
bd639196 3cfa0ad5½c‘–ú.Õ
14 data bytes
0c8f863e 8c000fd7.†Œ..×
9300cb04 ffd9“.Ë.ÿÙ
The problem seems to be that the payload sent by the camera is much larger
than dwMaxPayloadTransferSize. For this reason the driver assumes that it
has found the end of the frame after processing the third URB. This test is
performed at the bottom of uvc_video_decode_bulk. It then expects URB 4 to
be the start of a new frame, which it isn't, and so it gets out of sync. If
I understand correctly, dwMaxPayloadTransferSize is set by the camera, so
perhaps the camera is at fault here.
Yes, I believe the camera violates the spec.
Interestingly, I checked some wireshark logs I took while using the camera
with the Dell XPS12 booted into Windows and I see the exact same thing. The
dwMaxPayloadTransferSize is set to 34816, but the payloads were much larger,
around 140kb.
What bulk URB size did Windows use ?
As the camera works fine on Windows, I'm guessing Windows is not relying on
the dwMaxPayloadTransferSize to detect the end of a payload. Perhaps it just
uses the FID. Does this sound plausible? If so, I might see if I can
replicate this behaviour locally, to see if it solves the issue.
I don't think it uses the FID, as that requires the presence of a header,
which is only present at the beginning of payloads, and thus require the
driver to be able to detect the payload in the first place.
With the three USB traces I've received so far for your camera it seems that
we could detect the end of a payload by a short URBs. Maybe that's what
Windows does. This would however probably break if the MJPEG data size +
header size happens to be a multiple of 16kB.
Have you looked at the YUYV capture traces ? What's the data pattern there ? I
would expect the last 14 bytes transfer not to be present for YUYV. Does the
camera use a single payload in that case as well ?
--
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 v2 7/7] v4l: ti-vpe: Add selection API in VPE driver
Hi,
On Tuesday 04 March 2014 03:10 PM, Hans Verkuil wrote:
Hi Archit,
On 03/04/14 09:49, Archit Taneja wrote:
Add selection ioctl ops. For VPE, cropping makes sense only for the input to
VPE(or V4L2_BUF_TYPE_VIDEO_OUTPUT/MPLANE buffers) and composing makes sense
only for the output of VPE(or V4L2_BUF_TYPE_VIDEO_CAPTURE/MPLANE buffers).
For the CAPTURE type, V4L2_SEL_TGT_COMPOSE results in VPE writing the output
in a rectangle within the capture buffer. For the OUTPUT type, V4L2_SEL_TGT_CROP
results in selecting a rectangle region within the source buffer.
Setting the crop/compose rectangles should successfully result in
re-configuration of registers which are affected when either source or
destination dimensions change, set_srcdst_params() is called for this purpose.
Signed-off-by: Archit Taneja arc...@ti.com
---
drivers/media/platform/ti-vpe/vpe.c | 142
1 file changed, 142 insertions(+)
diff --git a/drivers/media/platform/ti-vpe/vpe.c
b/drivers/media/platform/ti-vpe/vpe.c
index 03a6846..b938590 100644
--- a/drivers/media/platform/ti-vpe/vpe.c
+++ b/drivers/media/platform/ti-vpe/vpe.c
@@ -410,8 +410,10 @@ static struct vpe_q_data *get_q_data(struct vpe_ctx *ctx,
{
switch (type) {
case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
+ case V4L2_BUF_TYPE_VIDEO_OUTPUT:
return ctx-q_data[Q_DATA_SRC];
case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
+ case V4L2_BUF_TYPE_VIDEO_CAPTURE:
I noticed that the querycap implementation is wrong. It reports
V4L2_CAP_VIDEO_M2M instead of V4L2_CAP_VIDEO_M2M_MPLANE.
This driver is using the multiplanar formats, so the M2M_MPLANE cap should
be set.
This should be a separate patch.
Thanks for pointing this out, I'll make a patch for that.
BTW, did you test the driver with the v4l2-compliance tool? The latest version
(http://git.linuxtv.org/v4l-utils.git) has m2m support.
I haven't tested it with this yet.
However, if you want to test streaming (the -s option), then you will probably
need to base your kernel on this tree:
http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/vb2-part1
I can give it a try. It'll probably take a bit more time to try this
out. I'll need to port some minor DRA7x stuff.
Kamil,
Do you think you have some more time for the m2m pull request?
That branch contains a pile of fixes for vb2 and without that v4l2-compliance -s
will fail a number of tests.
return ctx-q_data[Q_DATA_DST];
default:
BUG();
@@ -1585,6 +1587,143 @@ static int vpe_s_fmt(struct file *file, void *priv,
struct v4l2_format *f)
return set_srcdst_params(ctx);
}
+static int __vpe_try_selection(struct vpe_ctx *ctx, struct v4l2_selection *s)
+{
+ struct vpe_q_data *q_data;
+
+ if ((s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ (s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT))
+ return -EINVAL;
+
+ q_data = get_q_data(ctx, s-type);
+ if (!q_data)
+ return -EINVAL;
+
+ switch (s-target) {
+ case V4L2_SEL_TGT_COMPOSE:
+ case V4L2_SEL_TGT_COMPOSE_DEFAULT:
+ case V4L2_SEL_TGT_COMPOSE_BOUNDS:
+ /*
+* COMPOSE target is only valid for capture buffer type, for
+* output buffer type, assign existing crop parameters to the
+* selection rectangle
+*/
+ if (s-type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
+ break;
+ } else {
No need for the 'else' keywork here.
+ s-r = q_data-c_rect;
+ return 0;
+ }
+
+ case V4L2_SEL_TGT_CROP:
+ case V4L2_SEL_TGT_CROP_DEFAULT:
+ case V4L2_SEL_TGT_CROP_BOUNDS:
+ /*
+* CROP target is only valid for output buffer type, for capture
+* buffer type, assign existing compose parameters to the
+* selection rectangle
+*/
+ if (s-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
+ break;
+ } else {
Ditto.
Thanks. I'll fix these.
I had a minor question about the selection API:
Are the V4L2_SET_TGT_CROP/COMPOSE_DEFAULT and the corresponding 'BOUNDS'
targets supposed to be used with VIDIOC_S_SELECTION? If so, what's the
expect behaviour?
Thanks,
Archit
--
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
[RFCv1 PATCH 1/4] v4l2: allow v4l2_subdev_edid to be used with video nodes
From: Hans Verkuil hans.verk...@cisco.com
Struct v4l2_subdev_edid and the VIDIOC_SUBDEV_G/S_EDID ioctls were
specific for subdevices, but for hardware with a simple video pipeline
you do not need/want to create subdevice nodes to just get/set the EDID.
Move the v4l2_subdev_edid struct to v4l2-common.h and rename as
v4l2_edid. Add the same ioctls to videodev2.h as well, thus allowing
this API to be used with both video nodes and v4l-subdev nodes.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---
include/uapi/linux/v4l2-common.h | 8
include/uapi/linux/v4l2-subdev.h | 14 +-
include/uapi/linux/videodev2.h | 2 ++
3 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/include/uapi/linux/v4l2-common.h b/include/uapi/linux/v4l2-common.h
index 4f0667e..270db89 100644
--- a/include/uapi/linux/v4l2-common.h
+++ b/include/uapi/linux/v4l2-common.h
@@ -68,4 +68,12 @@
#define V4L2_SUBDEV_SEL_FLAG_SIZE_LE V4L2_SEL_FLAG_LE
#define V4L2_SUBDEV_SEL_FLAG_KEEP_CONFIG V4L2_SEL_FLAG_KEEP_CONFIG
+struct v4l2_edid {
+ __u32 pad;
+ __u32 start_block;
+ __u32 blocks;
+ __u32 reserved[5];
+ __u8 __user *edid;
+};
+
#endif /* __V4L2_COMMON__ */
diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
index a33c4da..87e0515 100644
--- a/include/uapi/linux/v4l2-subdev.h
+++ b/include/uapi/linux/v4l2-subdev.h
@@ -148,13 +148,8 @@ struct v4l2_subdev_selection {
__u32 reserved[8];
};
-struct v4l2_subdev_edid {
- __u32 pad;
- __u32 start_block;
- __u32 blocks;
- __u32 reserved[5];
- __u8 __user *edid;
-};
+/* Backwards compatibility define --- to be removed */
+#define v4l2_subdev_edid v4l2_edid
#define VIDIOC_SUBDEV_G_FMT_IOWR('V', 4, struct v4l2_subdev_format)
#define VIDIOC_SUBDEV_S_FMT_IOWR('V', 5, struct v4l2_subdev_format)
@@ -174,7 +169,8 @@ struct v4l2_subdev_edid {
_IOWR('V', 61, struct v4l2_subdev_selection)
#define VIDIOC_SUBDEV_S_SELECTION \
_IOWR('V', 62, struct v4l2_subdev_selection)
-#define VIDIOC_SUBDEV_G_EDID _IOWR('V', 40, struct v4l2_subdev_edid)
-#define VIDIOC_SUBDEV_S_EDID _IOWR('V', 41, struct v4l2_subdev_edid)
+/* These two G/S_EDID ioctls are identical to the ioctls in videodev2.h */
+#define VIDIOC_SUBDEV_G_EDID _IOWR('V', 40, struct v4l2_edid)
+#define VIDIOC_SUBDEV_S_EDID _IOWR('V', 41, struct v4l2_edid)
#endif
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 6ae7bbe..55d976c 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1885,6 +1885,8 @@ struct v4l2_create_buffers {
#define VIDIOC_QUERYMENU _IOWR('V', 37, struct v4l2_querymenu)
#define VIDIOC_G_INPUT _IOR('V', 38, int)
#define VIDIOC_S_INPUT _IOWR('V', 39, int)
+#define VIDIOC_G_EDID _IOWR('V', 40, struct v4l2_edid)
+#define VIDIOC_S_EDID _IOWR('V', 41, struct v4l2_edid)
#define VIDIOC_G_OUTPUT _IOR('V', 46, int)
#define VIDIOC_S_OUTPUT_IOWR('V', 47, int)
#define VIDIOC_ENUMOUTPUT _IOWR('V', 48, struct v4l2_output)
--
1.8.4.rc3
--
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
[RFCv1 PATCH 3/4] adv*: replace the deprecated v4l2_subdev_edid by v4l2_edid.
From: Hans Verkuil hans.verk...@cisco.com
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---
drivers/media/i2c/ad9389b.c | 2 +-
drivers/media/i2c/adv7511.c | 2 +-
drivers/media/i2c/adv7604.c | 4 ++--
drivers/media/i2c/adv7842.c | 4 ++--
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/media/i2c/ad9389b.c b/drivers/media/i2c/ad9389b.c
index 83225d6..1b7ecfd 100644
--- a/drivers/media/i2c/ad9389b.c
+++ b/drivers/media/i2c/ad9389b.c
@@ -573,7 +573,7 @@ static const struct v4l2_subdev_core_ops ad9389b_core_ops =
{
/* -- PAD OPS -- */
-static int ad9389b_get_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid
*edid)
+static int ad9389b_get_edid(struct v4l2_subdev *sd, struct v4l2_edid *edid)
{
struct ad9389b_state *state = get_ad9389b_state(sd);
diff --git a/drivers/media/i2c/adv7511.c b/drivers/media/i2c/adv7511.c
index ee61894..942ca4b 100644
--- a/drivers/media/i2c/adv7511.c
+++ b/drivers/media/i2c/adv7511.c
@@ -597,7 +597,7 @@ static int adv7511_isr(struct v4l2_subdev *sd, u32 status,
bool *handled)
return 0;
}
-static int adv7511_get_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid
*edid)
+static int adv7511_get_edid(struct v4l2_subdev *sd, struct v4l2_edid *edid)
{
struct adv7511_state *state = get_adv7511_state(sd);
diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 71c8570..98cc540 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -1658,7 +1658,7 @@ static int adv7604_isr(struct v4l2_subdev *sd, u32
status, bool *handled)
return 0;
}
-static int adv7604_get_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid
*edid)
+static int adv7604_get_edid(struct v4l2_subdev *sd, struct v4l2_edid *edid)
{
struct adv7604_state *state = to_state(sd);
u8 *data = NULL;
@@ -1728,7 +1728,7 @@ static int get_edid_spa_location(const u8 *edid)
return -1;
}
-static int adv7604_set_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid
*edid)
+static int adv7604_set_edid(struct v4l2_subdev *sd, struct v4l2_edid *edid)
{
struct adv7604_state *state = to_state(sd);
int spa_loc;
diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c
index e04fe3f..4d1e07e 100644
--- a/drivers/media/i2c/adv7842.c
+++ b/drivers/media/i2c/adv7842.c
@@ -2014,7 +2014,7 @@ static int adv7842_isr(struct v4l2_subdev *sd, u32
status, bool *handled)
return 0;
}
-static int adv7842_get_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid
*edid)
+static int adv7842_get_edid(struct v4l2_subdev *sd, struct v4l2_edid *edid)
{
struct adv7842_state *state = to_state(sd);
u8 *data = NULL;
@@ -2054,7 +2054,7 @@ static int adv7842_get_edid(struct v4l2_subdev *sd,
struct v4l2_subdev_edid *edi
return 0;
}
-static int adv7842_set_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *e)
+static int adv7842_set_edid(struct v4l2_subdev *sd, struct v4l2_edid *e)
{
struct adv7842_state *state = to_state(sd);
int err = 0;
--
1.8.4.rc3
--
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
[RFCv1 PATCH 4/4] DocBook v4l2: update the G/S_EDID documentation
From: Hans Verkuil hans.verk...@cisco.com
Document that it is now possible to call G/S_EDID from video nodes, not
just sub-device nodes.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---
Documentation/DocBook/media/v4l/v4l2.xml | 2 +-
...{vidioc-subdev-g-edid.xml = vidioc-g-edid.xml} | 30 +-
2 files changed, 19 insertions(+), 13 deletions(-)
rename Documentation/DocBook/media/v4l/{vidioc-subdev-g-edid.xml =
vidioc-g-edid.xml} (82%)
diff --git a/Documentation/DocBook/media/v4l/v4l2.xml
b/Documentation/DocBook/media/v4l/v4l2.xml
index 6bf3532..9a4aeb6 100644
--- a/Documentation/DocBook/media/v4l/v4l2.xml
+++ b/Documentation/DocBook/media/v4l/v4l2.xml
@@ -596,6 +596,7 @@ and discussions on the V4L mailing list./revremark
sub-g-crop;
sub-g-ctrl;
sub-g-dv-timings;
+sub-g-edid;
sub-g-enc-index;
sub-g-ext-ctrls;
sub-g-fbuf;
@@ -627,7 +628,6 @@ and discussions on the V4L mailing list./revremark
sub-subdev-enum-frame-size;
sub-subdev-enum-mbus-code;
sub-subdev-g-crop;
-sub-subdev-g-edid;
sub-subdev-g-fmt;
sub-subdev-g-frame-interval;
sub-subdev-g-selection;
diff --git a/Documentation/DocBook/media/v4l/vidioc-subdev-g-edid.xml
b/Documentation/DocBook/media/v4l/vidioc-g-edid.xml
similarity index 82%
rename from Documentation/DocBook/media/v4l/vidioc-subdev-g-edid.xml
rename to Documentation/DocBook/media/v4l/vidioc-g-edid.xml
index bbd18f0..e04f39f 100644
--- a/Documentation/DocBook/media/v4l/vidioc-subdev-g-edid.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-g-edid.xml
@@ -1,12 +1,12 @@
-refentry id=vidioc-subdev-g-edid
+refentry id=vidioc-g-edid
refmeta
-refentrytitleioctl VIDIOC_SUBDEV_G_EDID,
VIDIOC_SUBDEV_S_EDID/refentrytitle
+refentrytitleioctl VIDIOC_G_EDID, VIDIOC_S_EDID/refentrytitle
manvol;
/refmeta
refnamediv
-refnameVIDIOC_SUBDEV_G_EDID/refname
-refnameVIDIOC_SUBDEV_S_EDID/refname
+refnameVIDIOC_G_EDID/refname
+refnameVIDIOC_S_EDID/refname
refpurposeGet or set the EDID of a video
receiver/transmitter/refpurpose
/refnamediv
@@ -16,7 +16,7 @@
funcdefint functionioctl/function/funcdef
paramdefint parameterfd/parameter/paramdef
paramdefint parameterrequest/parameter/paramdef
- paramdefstruct v4l2_subdev_edid
*parameterargp/parameter/paramdef
+ paramdefstruct v4l2_edid *parameterargp/parameter/paramdef
/funcprototype
/funcsynopsis
funcsynopsis
@@ -24,7 +24,7 @@
funcdefint functionioctl/function/funcdef
paramdefint parameterfd/parameter/paramdef
paramdefint parameterrequest/parameter/paramdef
- paramdefconst struct v4l2_subdev_edid
*parameterargp/parameter/paramdef
+ paramdefconst struct v4l2_edid *parameterargp/parameter/paramdef
/funcprototype
/funcsynopsis
/refsynopsisdiv
@@ -42,7 +42,7 @@
varlistentry
termparameterrequest/parameter/term
listitem
- paraVIDIOC_SUBDEV_G_EDID, VIDIOC_SUBDEV_S_EDID/para
+ paraVIDIOC_G_EDID, VIDIOC_S_EDID/para
/listitem
/varlistentry
varlistentry
@@ -57,11 +57,16 @@
refsect1
titleDescription/title
paraThese ioctls can be used to get or set an EDID associated with an
input pad
-from a receiver or an output pad of a transmitter subdevice./para
+from a receiver or an output pad of a transmitter subdevice. These ioctls
can be
+used with subdevice nodes or with video nodes. A video node should only
support
+these ioctls if it is unambiguous which receiver or transmitter is
referred to.
+This will typically be based on the current input or output.
+When used with video nodes the structfieldpad/structfield field shall
be set
+to 0, otherwise EINVAL; will be returned./para
paraTo get the EDID data the application has to fill in the
structfieldpad/structfield,
structfieldstart_block/structfield, structfieldblocks/structfield
and structfieldedid/structfield
-fields and call constantVIDIOC_SUBDEV_G_EDID/constant. The current
EDID from block
+fields and call constantVIDIOC_G_EDID/constant. The current EDID from
block
structfieldstart_block/structfield and of size
structfieldblocks/structfield
will be placed in the memory structfieldedid/structfield points to.
The structfieldedid/structfield
pointer must point to memory at least
structfieldblocks/structfieldnbsp;*nbsp;128 bytes
@@ -91,15 +96,16 @@
data in some way. In any case, the end result is the same: the EDID is no
longer available.
/para
-table pgwide=1 frame=none id=v4l2-subdev-edid
- titlestruct structnamev4l2_subdev_edid/structname/title
+table pgwide=1 frame=none id=v4l2-edid
+ titlestruct structnamev4l2_edid/structname/title
tgroup cols=3
cs-str;
tbody valign=top
row
entry__u32/entry
entrystructfieldpad/structfield/entry
-
[RFCv1 PATCH 2/4] v4l2: add VIDIOC_G/S_EDID support to the v4l2 core.
From: Hans Verkuil hans.verk...@cisco.com
Support this ioctl as part of the v4l2 core. Use the new ioctl
name and struct v4l2_edid type in the existing core code.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---
drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 32 +--
drivers/media/v4l2-core/v4l2-dev.c| 2 ++
drivers/media/v4l2-core/v4l2-ioctl.c | 16 +++---
drivers/media/v4l2-core/v4l2-subdev.c | 4 ++--
include/media/v4l2-ioctl.h| 2 ++
include/media/v4l2-subdev.h | 4 ++--
6 files changed, 37 insertions(+), 23 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index 1b18616..6463c87 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -740,7 +740,7 @@ static int put_v4l2_event32(struct v4l2_event *kp, struct
v4l2_event32 __user *u
return 0;
}
-struct v4l2_subdev_edid32 {
+struct v4l2_edid32 {
__u32 pad;
__u32 start_block;
__u32 blocks;
@@ -748,11 +748,11 @@ struct v4l2_subdev_edid32 {
compat_caddr_t edid;
};
-static int get_v4l2_subdev_edid32(struct v4l2_subdev_edid *kp, struct
v4l2_subdev_edid32 __user *up)
+static int get_v4l2_edid32(struct v4l2_edid *kp, struct v4l2_edid32 __user *up)
{
u32 tmp;
- if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_subdev_edid32)) ||
+ if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_edid32)) ||
get_user(kp-pad, up-pad) ||
get_user(kp-start_block, up-start_block) ||
get_user(kp-blocks, up-blocks) ||
@@ -763,11 +763,11 @@ static int get_v4l2_subdev_edid32(struct v4l2_subdev_edid
*kp, struct v4l2_subde
return 0;
}
-static int put_v4l2_subdev_edid32(struct v4l2_subdev_edid *kp, struct
v4l2_subdev_edid32 __user *up)
+static int put_v4l2_edid32(struct v4l2_edid *kp, struct v4l2_edid32 __user *up)
{
u32 tmp = (u32)((unsigned long)kp-edid);
- if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_subdev_edid32)) ||
+ if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_edid32)) ||
put_user(kp-pad, up-pad) ||
put_user(kp-start_block, up-start_block) ||
put_user(kp-blocks, up-blocks) ||
@@ -787,8 +787,8 @@ static int put_v4l2_subdev_edid32(struct v4l2_subdev_edid
*kp, struct v4l2_subde
#define VIDIOC_DQBUF32 _IOWR('V', 17, struct v4l2_buffer32)
#define VIDIOC_ENUMSTD32 _IOWR('V', 25, struct v4l2_standard32)
#define VIDIOC_ENUMINPUT32 _IOWR('V', 26, struct v4l2_input32)
-#define VIDIOC_SUBDEV_G_EDID32 _IOWR('V', 63, struct v4l2_subdev_edid32)
-#define VIDIOC_SUBDEV_S_EDID32 _IOWR('V', 64, struct v4l2_subdev_edid32)
+#define VIDIOC_G_EDID32_IOWR('V', 63, struct v4l2_edid32)
+#define VIDIOC_S_EDID32_IOWR('V', 64, struct v4l2_edid32)
#define VIDIOC_TRY_FMT32 _IOWR('V', 64, struct v4l2_format32)
#define VIDIOC_G_EXT_CTRLS32_IOWR('V', 71, struct v4l2_ext_controls32)
#define VIDIOC_S_EXT_CTRLS32_IOWR('V', 72, struct v4l2_ext_controls32)
@@ -816,7 +816,7 @@ static long do_video_ioctl(struct file *file, unsigned int
cmd, unsigned long ar
struct v4l2_ext_controls v2ecs;
struct v4l2_event v2ev;
struct v4l2_create_buffers v2crt;
- struct v4l2_subdev_edid v2edid;
+ struct v4l2_edid v2edid;
unsigned long vx;
int vi;
} karg;
@@ -849,8 +849,8 @@ static long do_video_ioctl(struct file *file, unsigned int
cmd, unsigned long ar
case VIDIOC_S_OUTPUT32: cmd = VIDIOC_S_OUTPUT; break;
case VIDIOC_CREATE_BUFS32: cmd = VIDIOC_CREATE_BUFS; break;
case VIDIOC_PREPARE_BUF32: cmd = VIDIOC_PREPARE_BUF; break;
- case VIDIOC_SUBDEV_G_EDID32: cmd = VIDIOC_SUBDEV_G_EDID; break;
- case VIDIOC_SUBDEV_S_EDID32: cmd = VIDIOC_SUBDEV_S_EDID; break;
+ case VIDIOC_G_EDID32: cmd = VIDIOC_G_EDID; break;
+ case VIDIOC_S_EDID32: cmd = VIDIOC_S_EDID; break;
}
switch (cmd) {
@@ -868,9 +868,9 @@ static long do_video_ioctl(struct file *file, unsigned int
cmd, unsigned long ar
compatible_arg = 0;
break;
- case VIDIOC_SUBDEV_G_EDID:
- case VIDIOC_SUBDEV_S_EDID:
- err = get_v4l2_subdev_edid32(karg.v2edid, up);
+ case VIDIOC_G_EDID:
+ case VIDIOC_S_EDID:
+ err = get_v4l2_edid32(karg.v2edid, up);
compatible_arg = 0;
break;
@@ -966,9 +966,9 @@ static long do_video_ioctl(struct file *file, unsigned int
cmd, unsigned long ar
err = put_v4l2_event32(karg.v2ev, up);
break;
- case VIDIOC_SUBDEV_G_EDID:
- case VIDIOC_SUBDEV_S_EDID:
- err =
[RFCv1 PATCH 0/4] add G/S_EDID support for video nodes
Currently the VIDIOC_SUBDEV_G/S_EDID and struct v4l2_subdev_edid are subdev APIs. However, that's in reality quite annoying since for simple video pipelines there is no need to create v4l-subdev device nodes for anything else except for setting or getting EDIDs. What happens in practice is that v4l2 bridge drivers add explicit support for VIDIOC_SUBDEV_G/S_EDID themselves, just to avoid having to create subdev device nodes just for this. So this patch series makes the ioctls available as regular ioctls as well. In that case the pad field should be set to 0 and the bridge driver will fill in the right pad value internally depending on the current input or output and pass it along to the actual subdev driver. Regards, Hans -- 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 v2 7/7] v4l: ti-vpe: Add selection API in VPE driver
On 03/04/14 12:25, Archit Taneja wrote: I had a minor question about the selection API: Are the V4L2_SET_TGT_CROP/COMPOSE_DEFAULT and the corresponding 'BOUNDS' targets supposed to be used with VIDIOC_S_SELECTION? If so, what's the expect behaviour? No, those are read only in practice. So only used with G_SELECTION, never with S_SELECTION. Regards, Hans -- 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 v5 5/7] [media] of: move common endpoint parsing to drivers/of
Hi Tomi,
Am Dienstag, den 04.03.2014, 10:58 +0200 schrieb Tomi Valkeinen:
[...]
+int of_graph_parse_endpoint(const struct device_node *node,
+ struct of_endpoint *endpoint)
+{
+ struct device_node *port_node = of_get_parent(node);
Can port_node be NULL? Probably only if something is quite wrong, but
maybe it's safer to return error in that case.
both of_property_read_u32 and of_node_put can handle port_node == NULL.
I'll add a WARN_ONCE here as for of_graph_get_next_endpoint and continue
on.
+ memset(endpoint, 0, sizeof(*endpoint));
+
+ endpoint-local_node = node;
+ /*
+* It doesn't matter whether the two calls below succeed.
+* If they don't then the default value 0 is used.
+*/
+ of_property_read_u32(port_node, reg, endpoint-port);
+ of_property_read_u32(node, reg, endpoint-id);
If the endpoint does not have 'port' as parent (i.e. the shortened
format), the above will return the 'reg' of the device node (with
'device node' I mean the main node, with 'compatible' property).
Yes, a check for the port_node name is in order.
And generally speaking, if struct of_endpoint is needed, maybe it would
be better to return the struct of_endpoint when iterating the ports and
endpoints. That way there's no need to do parsing afterwards, trying
to figure out if there's a parent port node, but the information is
already at hand.
I'd like to keep the iteration separate from parsing so we can
eventually introduce a for_each_endpoint_of_node helper macro around
of_graph_get_next_endpoint.
[...]
A few thoughts about the iteration, and the API in general.
In the omapdss version I separated iterating ports and endpoints, for
the two reasons:
1) I think there are cases where you may want to have properties in the
port node, for things that are common for all the port's endpoints.
2) if there are multiple ports, I think the driver code is cleaner if
you first take the port, decide what port that is and maybe call a
sub-function, and then iterate the endpoints for that port only.
It depends a bit on whether you are actually iterating over individual
ports, or if you are just walking the whole endpoint graph to find
remote devices that have to be added to the component master's waiting
list, for example.
Both of those are possible with the API in the series, but not very cleanly.
Also, if you just want to iterate the endpoints, it's easy to implement
a helper using the separate port and endpoint iterations.
I started out to move an existing (albeit lightly used) API to a common
place so others can use it and improve upon it, too. I'm happy to pile
on fixes directly in this series, but could we separate the improvement
step from the move, for the bigger modifications?
I had no immediate use for the port iteration, so I have taken no steps
to add a function for this. I see no problem to add this later when
somebody needs it, or even rewrite of_graph_get_next_endpoint to use it
if it is feasible. Iterating over endpoints on a given port needs no
helper, as you can just use for_each_child_of_node.
Then, about the get_remote functions: I think there should be only one
function for that purpose, one that returns the device node that
contains the remote endpoint.
My reasoning is that the ports and endpoints, and their contents, should
be private to the device. So the only use to get the remote is to get
the actual device, to see if it's been probed, or maybe get some video
API for that device.
of_graph_get_remote_port currently is used in the exynos4-is/fimc-is.c
v4l2 driver to get the mipi-csi channel id from the remote port, and
I've started using it in imx-drm-core.c for two cases:
- given an endpoint on the encoder, find the remote port connected to
it, get the associated drm_crtc, to obtain its the drm_crtc_mask
for encoder-possible_crtcs.
- given an encoder and a connected drm_crtc, walk all endpoints to find
the remote port associated with the drm_crtc, and then use the local
endpoint parent port to determine multiplexer settings.
If the driver model used has some kind of master-driver, which goes
through all the display entities, I think the above is still valid. When
the master-driver follows the remote-link, it still needs to first get
the main device node, as the ports and endpoints make no sense without
the context of the main device node.
I'm not sure about this. I might just need the remote port node
associated with a remote drm_crtc or drm_encoder structure to find out
which local endpoint I should look at to retrieve configuration.
regards
Philipp
--
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 v5 5/7] [media] of: move common endpoint parsing to drivers/of
On 04/03/14 13:36, Philipp Zabel wrote:
Hi Tomi,
Am Dienstag, den 04.03.2014, 10:58 +0200 schrieb Tomi Valkeinen:
[...]
+int of_graph_parse_endpoint(const struct device_node *node,
+ struct of_endpoint *endpoint)
+{
+ struct device_node *port_node = of_get_parent(node);
Can port_node be NULL? Probably only if something is quite wrong, but
maybe it's safer to return error in that case.
both of_property_read_u32 and of_node_put can handle port_node == NULL.
I'll add a WARN_ONCE here as for of_graph_get_next_endpoint and continue
on.
Isn't it better to return an error?
And generally speaking, if struct of_endpoint is needed, maybe it would
be better to return the struct of_endpoint when iterating the ports and
endpoints. That way there's no need to do parsing afterwards, trying
to figure out if there's a parent port node, but the information is
already at hand.
I'd like to keep the iteration separate from parsing so we can
eventually introduce a for_each_endpoint_of_node helper macro around
of_graph_get_next_endpoint.
[...]
A few thoughts about the iteration, and the API in general.
In the omapdss version I separated iterating ports and endpoints, for
the two reasons:
1) I think there are cases where you may want to have properties in the
port node, for things that are common for all the port's endpoints.
2) if there are multiple ports, I think the driver code is cleaner if
you first take the port, decide what port that is and maybe call a
sub-function, and then iterate the endpoints for that port only.
It depends a bit on whether you are actually iterating over individual
ports, or if you are just walking the whole endpoint graph to find
remote devices that have to be added to the component master's waiting
list, for example.
True, but the latter is easily implemented using the separate
port/endpoint iteration. So I see it as a more powerful API.
Both of those are possible with the API in the series, but not very cleanly.
Also, if you just want to iterate the endpoints, it's easy to implement
a helper using the separate port and endpoint iterations.
I started out to move an existing (albeit lightly used) API to a common
place so others can use it and improve upon it, too. I'm happy to pile
on fixes directly in this series, but could we separate the improvement
step from the move, for the bigger modifications?
Yes, I understand that. What I wonder is that which is easier: make it a
public API now, more or less as it was in v4l2, or make it a public API
only when all the improvements we can think of have been made.
So my fear is that the API is now made public, and you and others start
to use it. But I can't use it, as I need things like separate
port/endpoint iteration. I need to add those, which also means that I
need to change all the users of the API, making the task more difficult
than I'd like.
However, this is more of thinking out loud than I don't like the
series. It's a good series =).
I had no immediate use for the port iteration, so I have taken no steps
to add a function for this. I see no problem to add this later when
somebody needs it, or even rewrite of_graph_get_next_endpoint to use it
if it is feasible. Iterating over endpoints on a given port needs no
helper, as you can just use for_each_child_of_node.
I would have a helper, which should do some sanity checks, like that the
node names are endpoint.
Then, about the get_remote functions: I think there should be only one
function for that purpose, one that returns the device node that
contains the remote endpoint.
My reasoning is that the ports and endpoints, and their contents, should
be private to the device. So the only use to get the remote is to get
the actual device, to see if it's been probed, or maybe get some video
API for that device.
of_graph_get_remote_port currently is used in the exynos4-is/fimc-is.c
v4l2 driver to get the mipi-csi channel id from the remote port, and
I've started using it in imx-drm-core.c for two cases:
- given an endpoint on the encoder, find the remote port connected to
it, get the associated drm_crtc, to obtain its the drm_crtc_mask
for encoder-possible_crtcs.
- given an encoder and a connected drm_crtc, walk all endpoints to find
the remote port associated with the drm_crtc, and then use the local
endpoint parent port to determine multiplexer settings.
Ok.
In omapdss each driver handles only the ports and endpoints defined for
its device, and they can be considered private to that device. The only
reason to look for the remote endpoint is to find the remote device. To
me the omapdss model makes sense, and feels logical and sane =). So I
have to say I'm not really familiar with the model you're using.
Of course, the different get_remove_* funcs do no harm, even if we have
a bunch of them. My point was only about enforcing the correct use of
the model, where correct is of course
RE: [PATCH v2 7/7] v4l: ti-vpe: Add selection API in VPE driver
Hi Archit,
From: Archit Taneja [mailto:arc...@ti.com]
Sent: Tuesday, March 04, 2014 12:25 PM
Hi,
On Tuesday 04 March 2014 03:10 PM, Hans Verkuil wrote:
Hi Archit,
On 03/04/14 09:49, Archit Taneja wrote:
Add selection ioctl ops. For VPE, cropping makes sense only for the
input to VPE(or V4L2_BUF_TYPE_VIDEO_OUTPUT/MPLANE buffers) and
composing makes sense only for the output of VPE(or
V4L2_BUF_TYPE_VIDEO_CAPTURE/MPLANE buffers).
For the CAPTURE type, V4L2_SEL_TGT_COMPOSE results in VPE writing
the
output in a rectangle within the capture buffer. For the OUTPUT
type,
V4L2_SEL_TGT_CROP results in selecting a rectangle region within the
source buffer.
Setting the crop/compose rectangles should successfully result in
re-configuration of registers which are affected when either source
or destination dimensions change, set_srcdst_params() is called for
this purpose.
Signed-off-by: Archit Taneja arc...@ti.com
---
drivers/media/platform/ti-vpe/vpe.c | 142
1 file changed, 142 insertions(+)
diff --git a/drivers/media/platform/ti-vpe/vpe.c
b/drivers/media/platform/ti-vpe/vpe.c
index 03a6846..b938590 100644
--- a/drivers/media/platform/ti-vpe/vpe.c
+++ b/drivers/media/platform/ti-vpe/vpe.c
@@ -410,8 +410,10 @@ static struct vpe_q_data *get_q_data(struct
vpe_ctx *ctx,
{
switch (type) {
case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
+ case V4L2_BUF_TYPE_VIDEO_OUTPUT:
return ctx-q_data[Q_DATA_SRC];
case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
+ case V4L2_BUF_TYPE_VIDEO_CAPTURE:
I noticed that the querycap implementation is wrong. It reports
V4L2_CAP_VIDEO_M2M instead of V4L2_CAP_VIDEO_M2M_MPLANE.
This driver is using the multiplanar formats, so the M2M_MPLANE cap
should be set.
This should be a separate patch.
Thanks for pointing this out, I'll make a patch for that.
BTW, did you test the driver with the v4l2-compliance tool? The
latest
version
(http://git.linuxtv.org/v4l-utils.git) has m2m support.
I haven't tested it with this yet.
However, if you want to test streaming (the -s option), then you will
probably need to base your kernel on this tree:
http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/vb2
-part1
I can give it a try. It'll probably take a bit more time to try this
out. I'll need to port some minor DRA7x stuff.
Kamil,
Do you think you have some more time for the m2m pull request?
Yes, I would like to send the final pull request at the beginning of
the coming week.
That branch contains a pile of fixes for vb2 and without that
v4l2-compliance -s will fail a number of tests.
return ctx-q_data[Q_DATA_DST];
default:
BUG();
@@ -1585,6 +1587,143 @@ static int vpe_s_fmt(struct file *file, void
*priv, struct v4l2_format *f)
return set_srcdst_params(ctx);
}
+static int __vpe_try_selection(struct vpe_ctx *ctx, struct
+v4l2_selection *s) {
+ struct vpe_q_data *q_data;
+
+ if ((s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ (s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT))
+ return -EINVAL;
+
+ q_data = get_q_data(ctx, s-type);
+ if (!q_data)
+ return -EINVAL;
+
+ switch (s-target) {
+ case V4L2_SEL_TGT_COMPOSE:
+ case V4L2_SEL_TGT_COMPOSE_DEFAULT:
+ case V4L2_SEL_TGT_COMPOSE_BOUNDS:
+ /*
+ * COMPOSE target is only valid for capture buffer type,
for
+ * output buffer type, assign existing crop parameters to
the
+ * selection rectangle
+ */
+ if (s-type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
+ break;
+ } else {
No need for the 'else' keywork here.
+ s-r = q_data-c_rect;
+ return 0;
+ }
+
+ case V4L2_SEL_TGT_CROP:
+ case V4L2_SEL_TGT_CROP_DEFAULT:
+ case V4L2_SEL_TGT_CROP_BOUNDS:
+ /*
+ * CROP target is only valid for output buffer type, for
capture
+ * buffer type, assign existing compose parameters to the
+ * selection rectangle
+ */
+ if (s-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
+ break;
+ } else {
Ditto.
Thanks. I'll fix these.
I had a minor question about the selection API:
Are the V4L2_SET_TGT_CROP/COMPOSE_DEFAULT and the corresponding
'BOUNDS'
targets supposed to be used with VIDIOC_S_SELECTION? If so, what's the
expect behaviour?
Thanks,
Archit
Best wishes,
--
Kamil Debski
Samsung RD Institute Poland
--
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/2] [media] em28xx: Add LED support for Kworld UB45-Q v3
This device has a led at bit 7 of GPIO reg. 0x80 to indicate
when a DVB capture is happening.
Add support for it.
Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com
---
drivers/media/usb/em28xx/em28xx-cards.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c
b/drivers/media/usb/em28xx/em28xx-cards.c
index 5cd2df14bf1a..66d9c8798c82 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -516,6 +516,17 @@ static struct em28xx_led speedlink_vad_laplace_leds[] = {
{-1, 0, 0, 0},
};
+static struct em28xx_led kworld_ub435q_v3_leds[] = {
+ {
+ .role = EM28XX_LED_DIGITAL_CAPTURING,
+ .gpio_reg = EM2874_R80_GPIO_P0_CTRL,
+ .gpio_mask = 0x80,
+ .inverted = 1,
+ },
+ {-1, 0, 0, 0},
+};
+
+
/*
* Board definitions
*/
@@ -2159,6 +2170,7 @@ struct em28xx_board em28xx_boards[] = {
.def_i2c_bus= 1,
.i2c_speed = EM28XX_I2C_CLK_WAIT_ENABLE |
EM28XX_I2C_FREQ_100_KHZ,
+ .leds = kworld_ub435q_v3_leds,
},
[EM2874_BOARD_PCTV_HD_MINI_80E] = {
.name = Pinnacle PCTV HD Mini,
--
1.8.5.3
--
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/2] [media] em28xx: add support for DVB monitor led
Some devices have a LED to indicate when DVB capture started.
Add support for it.
Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com
---
drivers/media/usb/em28xx/em28xx-core.c | 26 ++
drivers/media/usb/em28xx/em28xx.h | 1 +
2 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-core.c
b/drivers/media/usb/em28xx/em28xx-core.c
index 6de41c6a0770..523d7e92bf47 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -619,6 +619,7 @@ EXPORT_SYMBOL_GPL(em28xx_find_led);
int em28xx_capture_start(struct em28xx *dev, int start)
{
int rc;
+ const struct em28xx_led *led = NULL;
if (dev-chip_id == CHIP_ID_EM2874 ||
dev-chip_id == CHIP_ID_EM2884 ||
@@ -643,6 +644,8 @@ int em28xx_capture_start(struct em28xx *dev, int start)
/* Enable video capture */
rc = em28xx_write_reg(dev, 0x48, 0x00);
+ if (rc 0)
+ return rc;
if (dev-mode == EM28XX_ANALOG_MODE)
rc = em28xx_write_reg(dev,
@@ -650,6 +653,8 @@ int em28xx_capture_start(struct em28xx *dev, int start)
else
rc = em28xx_write_reg(dev,
EM28XX_R12_VINENABLE, 0x37);
+ if (rc 0)
+ return rc;
msleep(6);
} else {
@@ -658,19 +663,16 @@ int em28xx_capture_start(struct em28xx *dev, int start)
}
}
- if (rc 0)
- return rc;
-
- /* Switch (explicitly controlled) analog capturing LED on/off */
- if (dev-mode == EM28XX_ANALOG_MODE) {
- const struct em28xx_led *led;
+ if (dev-mode == EM28XX_ANALOG_MODE)
led = em28xx_find_led(dev, EM28XX_LED_ANALOG_CAPTURING);
- if (led)
- em28xx_write_reg_bits(dev, led-gpio_reg,
- (!start ^ led-inverted) ?
- ~led-gpio_mask : led-gpio_mask,
- led-gpio_mask);
- }
+ else
+ led = em28xx_find_led(dev, EM28XX_LED_DIGITAL_CAPTURING);
+
+ if (led)
+ em28xx_write_reg_bits(dev, led-gpio_reg,
+ (!start ^ led-inverted) ?
+ ~led-gpio_mask : led-gpio_mask,
+ led-gpio_mask);
return rc;
}
diff --git a/drivers/media/usb/em28xx/em28xx.h
b/drivers/media/usb/em28xx/em28xx.h
index 3b08556376e3..9e44f5bfc48b 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -401,6 +401,7 @@ enum em28xx_adecoder {
enum em28xx_led_role {
EM28XX_LED_ANALOG_CAPTURING = 0,
+ EM28XX_LED_DIGITAL_CAPTURING,
EM28XX_LED_ILLUMINATION,
EM28XX_NUM_LED_ROLES, /* must be the last */
};
--
1.8.5.3
--
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 v5 5/7] [media] of: move common endpoint parsing to drivers/of
Am Dienstag, den 04.03.2014, 14:21 +0200 schrieb Tomi Valkeinen: On 04/03/14 13:36, Philipp Zabel wrote: [...] Can port_node be NULL? Probably only if something is quite wrong, but maybe it's safer to return error in that case. both of_property_read_u32 and of_node_put can handle port_node == NULL. I'll add a WARN_ONCE here as for of_graph_get_next_endpoint and continue on. Isn't it better to return an error? I am not sure. We can still correctly parse the endpoint properties of a parentless node. All current users ignore the return value anyway. So as long as we still do the memset and and set local_node and id, returning an error effectively won't change the current behaviour. [...] It depends a bit on whether you are actually iterating over individual ports, or if you are just walking the whole endpoint graph to find remote devices that have to be added to the component master's waiting list, for example. True, but the latter is easily implemented using the separate port/endpoint iteration. So I see it as a more powerful API. Indeed. I see no problem in adding an of_graph_get_next_port function. But I'd like to keep the current of_graph_get_next_endpoint function iterating over all endpoints of the device. Both of those are possible with the API in the series, but not very cleanly. Also, if you just want to iterate the endpoints, it's easy to implement a helper using the separate port and endpoint iterations. I started out to move an existing (albeit lightly used) API to a common place so others can use it and improve upon it, too. I'm happy to pile on fixes directly in this series, but could we separate the improvement step from the move, for the bigger modifications? Yes, I understand that. What I wonder is that which is easier: make it a public API now, more or less as it was in v4l2, or make it a public API only when all the improvements we can think of have been made. So my fear is that the API is now made public, and you and others start to use it. And I fear that this series might outgrow maintainer attention spans if I keep adding to it. But I can't use it, as I need things like separate port/endpoint iteration. I need to add those, which also means that I need to change all the users of the API, making the task more difficult than I'd like. However, this is more of thinking out loud than I don't like the series. It's a good series =). Thanks. How about I follow this up with a split port/endpoint parsing helpers after I get some acks? I had no immediate use for the port iteration, so I have taken no steps to add a function for this. I see no problem to add this later when somebody needs it, or even rewrite of_graph_get_next_endpoint to use it if it is feasible. Iterating over endpoints on a given port needs no helper, as you can just use for_each_child_of_node. I would have a helper, which should do some sanity checks, like that the node names are endpoint. I'd prefer this to be a generic function of_get_next_child_by_name and possibly a macro for_each_named_child_of_node wrapping that. [...] In omapdss each driver handles only the ports and endpoints defined for its device, and they can be considered private to that device. The only reason to look for the remote endpoint is to find the remote device. To me the omapdss model makes sense, and feels logical and sane =). So I have to say I'm not really familiar with the model you're using. The main difference I see is that a single IPU device will have two port nodes handled by the DRM driver and two port nodes handled by the V4L2 driver, so we can't go back up to the IPU device tree node and iterate over all its ports in either the DRM or V4L2 drivers. You could argue that all the device tree parsing should be done from the IPU drivers, and the DRM and V4L2 drivers should use preparsed internal graph structures. But then we are getting into using struct media_entity in DRM drivers territory, and rather not go there right now. regards Philipp -- 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 00/79] Add support for ATSC PCTV 80e USB stick
On Mon, Mar 3, 2014 at 5:05 AM, Mauro Carvalho Chehab m.che...@samsung.com wrote: This patch series finally merge a long waited driver, for Micronas/Trident DRX-J ATSC frontends. It is based on a previous work from Devin, who made the original port of the Trident driver and got license to ship the firmware. Latter, it got some attention from Patrick that tried to upstream it. However, this driver had very serious issues, and didn't fit into Linux Kernel code quality standards. So, I made some patches, back in 2012, in order to try helping to get this driver merged, but, as I didn't have the device, and Patrick never sent a final tested version, the patches got hibernated for a long time. Finally, this year, I got one hardware for me, and one for Shuah, as she offered her help to fix suspend/resume issues at the subsystem. This series is a result of this work: the driver got major cleanups, and several relevant bug fixes. The most serious one is that the device was not fully initializing the struct that was enabling the MPEG output. Worse than that, the routine that was setting the MPEG output was also rewriting the default values for a dirty set. The type of output was also wrong: on this board, PCTV 80e is wired for serial MPEG transfer, but the driver was initialized to parallel. All those bugs got fixed, and, at least on my tests with a PCTV 80e model 01134, the device is now properly working. Not sure if all patches will arrive the ML, as there are some very big ones here that could be biger than VGER's max limit for posts. Anyway, all patches can be found at: http://git.linuxtv.org/mchehab/experimental.git/shortlog/refs/heads/drx-j-v3 Have fun! Mauro Devin Heitmueller (2): [media] drx-j: add a driver for Trident drx-j frontend [media] drx-j: put under 3-clause BSD license Mauro Carvalho Chehab (74): [media] drx-j: CodingStyle fixes [media] drx-j: Fix compilation and un-comment it [media] drx-j: Fix CodingStyle [media] drx-j: get rid of the typedefs on bsp_i2c.h [media] drx-j: remove the const annotate on HICommand() [media] drx-j: get rid of the integer typedefs [media] drx-j: get rid of the other typedefs at bsp_types.h [media] drx-j: get rid of the bsp*.h headers [media] drx-j: get rid of most of the typedefs [media] drx-j: fix whitespacing on pointer parmameters [media] drx-j: Use checkpatch --fix to solve several issues [media] drx-j: Don't use CamelCase [media] drx-j: do more CodingStyle fixes [media] drx-j: remove the unused tuner_i2c_write_read() function [media] drx-j: Remove a bunch of unused but assigned vars [media] drx-j: Some minor CodingStyle fixes at headers [media] drx-j: make a few functions static [media] drx-j: Get rid of drx39xyj/bsp_tuner.h [media] drx-j: get rid of typedefs in drx_driver.h [media] drx-j: Get rid of typedefs on drxh.h [media] drx-j: a few more CodingStyle fixups [media] drx-j: Don't use buffer if an error occurs [media] drx-j: replace the ugly CHK_ERROR() macro [media] drx-j: don't use parenthesis on return [media] drx-j: Simplify logic expressions [media] drx-j: More CamelCase fixups [media] drx-j: Remove typedefs in drxj.c [media] drx-j: CodingStyle fixups on drxj.c [media] drx-j: Use the Linux error codes [media] drx-j: Replace printk's by pr_foo() [media] drx-j: get rid of some ugly macros [media] drx-j: remove typedefs at drx_driver.c [media] drx-j: remove drxj_options.h [media] drx-j: make checkpatch.pl happy [media] drx-j: remove the useless microcode_size [media] drx-j: Fix release and error path on drx39xxj.c [media] drx-j: Be sure that all allocated data are properly initialized [media] drx-j: dynamically load the firmware [media] drx-j: Split firmware size check from the main routine [media] em28xx: add support for PCTV 80e remote controller [media] drx-j: remove unused code from drx_driver.c [media] drx-j: get rid of its own be??_to_cpu() implementation [media] drx-j: reset the DVB scan configuration at powerup [media] drx-j: Allow standard selection [media] drx-j: Some cleanups at drx_driver.c source [media] drx-j: prepend function names with drx_ at drx_driver.c [media] drx-j: get rid of drx_driver.c [media] drx-j: Avoid any regressions by preserving old behavior [media] drx-j: Remove duplicated firmware upload code [media] drx-j: get rid of drx_ctrl [media] drx-j: get rid of the remaining drx generic functions [media] drx-j: move drx39xxj into drxj.c [media] drx-j: get rid of drxj_ctrl() [media] drx-j: comment or remove unused code [media] drx-j: remove some ugly bindings from drx39xxj_dummy.c [media] drx-j: get rid of tuner dummy get/set frequency [media] drx-j: be sure to use tuner's IF [media] drx-j: avoid calling power_down_foo twice [media] drx-j: call ctrl_set_standard even if a standard is powered [media]
Re: Dell XPS 12 USB camera bulk mode issues
Hi Laurent,
Thanks again for your reply.
On 03/04/2014 12:11 PM, Laurent Pinchart wrote:
Hi Mark,
On Friday 28 February 2014 10:34:24 Mark Ryan wrote:
On 02/26/2014 04:40 PM, Laurent Pinchart wrote:
[ ... ]
With the information I've given you, could you try to log more information
in the driver to try and find what goes wrong ? You could for instance
log the content of each header at the beginning of the
uvc_video_decode_start() function.
So maybe I have something here. I ran guvcview and set the format to
MJPEG running at a high resolution. I'm using the kernel 3.11.0-17 that
comes with Ubuntu 13.10. In the usbmon logs I see the following.
[...]
SETUP Host-to-device Class request to Interface
bRequest: SET CUR (01)
wValue: 0200
wIndex: INTF 1 ENTITY 0 (0001)
wLength: 001a
26 data bytes
bmHint 0x01
bFormatIndex 2
bFrameIndex 10
dwFrameInterval 33
wKeyFrameRate 0
wPFrameRate 0
wCompQuality 0
wCompWindowSize 0
wDelay 32
dwMaxVideoFrameSize 1843200
dwMaxPayloadTransferSize 34816
Note the dwMaxPayloadTransferSize of 34816
[...]
Now I have my first payload
16384 data bytes
0c8d863e 8c007d67.†Œ.}g
8e00b304 ffd8ffdbŽ.³.ÿØÿÛ
00430003 02020202.C..
02030202 02030303
16384 data bytes
fbf5aeff 00c25e12ûõ®ÿ.Â^.
5d1244d5 353b2437].DÕ5;$7
f92d06e6 24c28475ù-.æ$„u
c740c6b3 8bbb29adÇ@Ƴ‹»)
16384 data bytes
f1a3cc60 b8200c52ñ£Ì`¸ .R
0108eac3 1cd4610e..êÃ.Ôa.
ece45032 40c477a8ìäP2@Äw¨
e4f35b81 d05001fcäó[ÐP.ü
16384 data bytes
fb3528d1 bc4fcc56û5(ѼOÌV
fe2d9522 1d04b1e7þ-•..±ç
3fa1a9e5 2b9867f6?¡©å+˜gö
778e2dd0 7d9b57b5wŽ-Ð}›Wµ
16384 data bytes
c016baf6 99732c9aÀ.ºö™s,š
9db39b7b e6b995a4³›{湕¤
65751f78 649eb5edeu.xdžµí
9e2dfdb1 355fd9faž-ý±5_Ùú
16384 data bytes
a73d4053 8fad795c§=@Sy\
c9088170 ec1c1e54É.pì..T
9ce6a59a d89f5742œæ¥šØŸWB
82cf3297 26d95b18‚Ï2—Ù[.
16384 data bytes
b4d3b21b d4a53595´Ó².Ô¥5•
99cac904 2e1bb346™ÊÉ...³F
09fe555e 4d2b4b95.þU^M+K•
be6b488a 0e91b0ca¾kHŠ.‘°Ê
16384 data bytes
3adb9dbc 6323ad21:Û¼c#!
8aa85b9d b934d31eŠ¨[¹4Ó.
465885f6 a00ad25cFX…ö .Ò\
04fdd44b be43d08a.ýÔK¾CЊ
16384 data bytes
14f1cf4c 0ae43589.ñÏL.ä5‰
0acdbd0b c8afedc2.ͽ.ȯíÂ
f15d4339 8bfb5b2bñ]C9‹û[+
df39e42d 14ecb849ß9ä-.ì¸I
10466 data bytes
7fffd1fc af99e466ÿÑü¯™äf
c39cd475 cab500a2ÃœÔuʵ.¢
ad8ae147 d295ec0cŠáGÒ•ì.
bd639196 3cfa0ad5½c‘–ú.Õ
14 data bytes
0c8f863e 8c000fd7.†Œ..×
9300cb04 ffd9“.Ë.ÿÙ
The problem seems to be that the payload sent by the camera is much larger
than dwMaxPayloadTransferSize. For this reason the driver assumes that it
has found the end of the frame after processing the third URB. This test is
performed at the bottom of uvc_video_decode_bulk. It then expects URB 4 to
be the start of a new frame, which it isn't, and so it gets out of sync. If
I understand correctly, dwMaxPayloadTransferSize is set by the camera, so
perhaps the camera is at fault here.
Yes, I believe the camera violates the spec.
Interestingly, I checked some wireshark logs I took while using the camera
with the Dell XPS12 booted into Windows and I see the exact same thing. The
dwMaxPayloadTransferSize is set to 34816, but the payloads were much larger,
around 140kb.
What bulk URB size did Windows use ?
As the camera works fine on Windows, I'm guessing Windows is not relying on
the dwMaxPayloadTransferSize to detect the end of a payload. Perhaps it just
uses the FID. Does this sound plausible? If so, I might see if I can
replicate this behaviour locally, to see if it solves the issue.
I don't think it uses the FID, as that requires the presence of a header,
which is only present at the beginning of payloads, and thus require the
driver to be able to detect the payload in the first place.
With the three USB traces I've received so far for your camera it seems that
we could detect the end of a payload by a short URBs. Maybe that's what
Windows does. This would however probably break if the MJPEG data size +
header size happens to be a multiple of 16kB.
Have you looked at the YUYV capture traces ? What's the data pattern there ? I
would expect the last 14 bytes transfer not to be present for YUYV. Does the
camera use a single payload in that case as well ?
I have, and the behaviour seems to differ depending on the frameIndex.
I haven't tested all of the resolutions yet, but here are the two main
patterns I see for YVUV.
- For some resolutions, each frame is split up into multiple payloads,
each of which are equal in size to the dwMaxPayloadTransferSize, apart
perhaps from the last. Each payload consists of a single URB ( I hope
this is the correct terminology ) and has a header. The total
[no subject]
From a5cfa1881de152a887d195e8c880dcca3e6b766e Mon Sep 17 00:00:00 2001
From: Stefan Ringel linu...@stefanringel.de
Date: Tue, 4 Mar 2014 20:50:32 +0100
Subject: [PATCH] v4l-utils: bugfix memory chunk
Bug 1070855 - [abrt] v4l-utils: parse_string(): dvbv5-scan killed by SIGABRT
https://bugzilla.redhat.com/show_bug.cgi?id=1070855
Signed-off-by: Stefan Ringel linu...@stefanringel.de
---
lib/libdvbv5/descriptors/desc_frequency_list.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/libdvbv5/descriptors/desc_frequency_list.c b/lib/libdvbv5/descriptors/desc_frequency_list.c
index de6f9fd..0a06a4a 100644
--- a/lib/libdvbv5/descriptors/desc_frequency_list.c
+++ b/lib/libdvbv5/descriptors/desc_frequency_list.c
@@ -36,10 +36,10 @@ void dvb_desc_frequency_list_init(struct dvb_v5_fe_parms *parms, const uint8_t *
d-frequencies = (d-length - len) / sizeof(d-frequency[0]);
- d-frequency = calloc(1, sizeof(d-frequency));
+ d-frequency = calloc(d-frequencies, sizeof(d-frequency));
for (i = 0; i d-frequencies; i++) {
- d-frequency[i] = ((uint32_t *) buf)[i];
+ d-frequency[i] = ((uint32_t *) p)[i];
bswap32(d-frequency[i]);
switch (d-freq_type) {
case 1: /* satellite - to get kHz */
--
1.9.0
[no subject]
From a5cfa1881de152a887d195e8c880dcca3e6b766e Mon Sep 17 00:00:00 2001
From: Stefan Ringel linu...@stefanringel.de
Date: Tue, 4 Mar 2014 20:50:32 +0100
Subject: [PATCH] v4l-utils: bugfix memory chunk
Bug 1070855 - [abrt] v4l-utils: parse_string(): dvbv5-scan killed by SIGABRT
https://bugzilla.redhat.com/show_bug.cgi?id=1070855
Signed-off-by: Stefan Ringel linu...@stefanringel.de
---
lib/libdvbv5/descriptors/desc_frequency_list.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/libdvbv5/descriptors/desc_frequency_list.c b/lib/libdvbv5/descriptors/desc_frequency_list.c
index de6f9fd..0a06a4a 100644
--- a/lib/libdvbv5/descriptors/desc_frequency_list.c
+++ b/lib/libdvbv5/descriptors/desc_frequency_list.c
@@ -36,10 +36,10 @@ void dvb_desc_frequency_list_init(struct dvb_v5_fe_parms *parms, const uint8_t *
d-frequencies = (d-length - len) / sizeof(d-frequency[0]);
- d-frequency = calloc(1, sizeof(d-frequency));
+ d-frequency = calloc(d-frequencies, sizeof(d-frequency));
for (i = 0; i d-frequencies; i++) {
- d-frequency[i] = ((uint32_t *) buf)[i];
+ d-frequency[i] = ((uint32_t *) p)[i];
bswap32(d-frequency[i]);
switch (d-freq_type) {
case 1: /* satellite - to get kHz */
--
1.9.0
[PATCH] v4l-utils: bugfix memory chunk
From a5cfa1881de152a887d195e8c880dcca3e6b766e Mon Sep 17 00:00:00 2001
From: Stefan Ringel linu...@stefanringel.de
Date: Tue, 4 Mar 2014 20:50:32 +0100
Subject: [PATCH] v4l-utils: bugfix memory chunk
Bug 1070855 - [abrt] v4l-utils: parse_string(): dvbv5-scan killed by SIGABRT
https://bugzilla.redhat.com/show_bug.cgi?id=1070855
Signed-off-by: Stefan Ringel linu...@stefanringel.de
---
lib/libdvbv5/descriptors/desc_frequency_list.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/libdvbv5/descriptors/desc_frequency_list.c b/lib/libdvbv5/descriptors/desc_frequency_list.c
index de6f9fd..0a06a4a 100644
--- a/lib/libdvbv5/descriptors/desc_frequency_list.c
+++ b/lib/libdvbv5/descriptors/desc_frequency_list.c
@@ -36,10 +36,10 @@ void dvb_desc_frequency_list_init(struct dvb_v5_fe_parms *parms, const uint8_t *
d-frequencies = (d-length - len) / sizeof(d-frequency[0]);
- d-frequency = calloc(1, sizeof(d-frequency));
+ d-frequency = calloc(d-frequencies, sizeof(d-frequency));
for (i = 0; i d-frequencies; i++) {
- d-frequency[i] = ((uint32_t *) buf)[i];
+ d-frequency[i] = ((uint32_t *) p)[i];
bswap32(d-frequency[i]);
switch (d-freq_type) {
case 1: /* satellite - to get kHz */
--
1.9.0
[linuxtv-media:master 428/499] drivers/media/dvb-frontends/drx39xyj/drxj.c:1039:16: sparse: symbol 'drxj_default_aud_data_g' was not declared. Should it be static?
tree: git://linuxtv.org/media_tree.git master
head: 59432be1c7fbf2a4f608850855ff649bee0f7b3b
commit: 57afe2f0bb0cca758701679f141c9fa92a034415 [428/499] [media] drx-j: Don't
use CamelCase
reproduce: make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by )
drivers/media/dvb-frontends/drx39xyj/drxj.c:651:6: sparse: symbol
'drx_dap_drxj_module_name' was not declared. Should it be static?
drivers/media/dvb-frontends/drx39xyj/drxj.c:652:6: sparse: symbol
'drx_dap_drxj_version_text' was not declared. Should it be static?
drivers/media/dvb-frontends/drx39xyj/drxj.c:654:15: sparse: symbol
'drx_dap_drxj_version' was not declared. Should it be static?
drivers/media/dvb-frontends/drx39xyj/drxj.c:1039:16: sparse: symbol
'drxj_default_aud_data_g' was not declared. Should it be static?
drivers/media/dvb-frontends/drx39xyj/drxj.c:4208:1: sparse: symbol
'tuner_i2c_write_read' was not declared. Should it be static?
drivers/media/dvb-frontends/drx39xyj/drxj.c:10234:27: sparse: cast truncates
bits from constant value (00ff becomes ff)
drivers/media/dvb-frontends/drx39xyj/drxj.c:10251:24: sparse: cast truncates
bits from constant value (3fff becomes 3fff)
drivers/media/dvb-frontends/drx39xyj/drxj.c:11125:31: sparse: cast truncates
bits from constant value (00ff becomes ff)
drivers/media/dvb-frontends/drx39xyj/drxj.c:11167:26: sparse: cast truncates
bits from constant value ( becomes 0)
drivers/media/dvb-frontends/drx39xyj/drxj.c:11233:33: sparse: cast truncates
bits from constant value (7fff becomes 7fff)
drivers/media/dvb-frontends/drx39xyj/drxj.c:11777:26: sparse: cast truncates
bits from constant value (7fff becomes 7fff)
Please consider folding the attached diff :-)
---
0-DAY kernel build testing backend Open Source Technology Center
http://lists.01.org/mailman/listinfo/kbuild Intel Corporation
From: Fengguang Wu fengguang...@intel.com
Subject: [PATCH linuxtv-media] drx-j: drxj_default_aud_data_g can be static
TO: Mauro Carvalho Chehab m.che...@samsung.com
CC: linux-media@vger.kernel.org
CC: linux-media@vger.kernel.org
CC: linux-ker...@vger.kernel.org
CC: Mauro Carvalho Chehab m.che...@samsung.com
CC: linux-media@vger.kernel.org
Signed-off-by: Fengguang Wu fengguang...@intel.com
---
drxj.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/dvb-frontends/drx39xyj/drxj.c b/drivers/media/dvb-frontends/drx39xyj/drxj.c
index 3a63520..b7a2b84 100644
--- a/drivers/media/dvb-frontends/drx39xyj/drxj.c
+++ b/drivers/media/dvb-frontends/drx39xyj/drxj.c
@@ -1036,7 +1036,7 @@ drx_demod_instance_t drxj_default_demod_g = {
* This structure is DRXK specific.
*
*/
-drx_aud_data_t drxj_default_aud_data_g = {
+static drx_aud_data_t drxj_default_aud_data_g = {
false, /* audio_is_active */
DRX_AUD_STANDARD_AUTO, /* audio_standard */
[PATCH] [media] em28xx: only enable PCTV 80e led when streaming
Instead of keeping the led always on, use it to indicate
when DVB is streaming.
Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com
---
drivers/media/usb/em28xx/em28xx-cards.c | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c
b/drivers/media/usb/em28xx/em28xx-cards.c
index 66d9c8798c82..2fb300e882f0 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -228,8 +228,8 @@ static struct em28xx_reg_seq
terratec_cinergy_USB_XS_FR_digital[] = {
7: LED on, active high */
static struct em28xx_reg_seq em2874_pctv_80e_digital[] = {
{EM28XX_R06_I2C_CLK,0x45, 0xff, 10}, /*400 KHz*/
- {EM2874_R80_GPIO_P0_CTRL, 0x80, 0xff, 100},/*Demod reset*/
- {EM2874_R80_GPIO_P0_CTRL, 0xc0, 0xff, 10},
+ {EM2874_R80_GPIO_P0_CTRL, 0x00, 0xff, 100},/*Demod reset*/
+ {EM2874_R80_GPIO_P0_CTRL, 0x40, 0xff, 10},
{ -1, -1, -1, -1},
};
@@ -526,6 +526,16 @@ static struct em28xx_led kworld_ub435q_v3_leds[] = {
{-1, 0, 0, 0},
};
+static struct em28xx_led pctv_80e_leds[] = {
+ {
+ .role = EM28XX_LED_DIGITAL_CAPTURING,
+ .gpio_reg = EM2874_R80_GPIO_P0_CTRL,
+ .gpio_mask = 0x80,
+ .inverted = 0,
+ },
+ {-1, 0, 0, 0},
+};
+
/*
* Board definitions
@@ -2179,6 +2189,7 @@ struct em28xx_board em28xx_boards[] = {
.dvb_gpio = em2874_pctv_80e_digital,
.decoder = EM28XX_NODECODER,
.ir_codes = RC_MAP_PINNACLE_PCTV_HD,
+ .leds = pctv_80e_leds,
},
/* 1ae7:9003/9004 SpeedLink Vicious And Devine Laplace webcam
* Empia EM2765 + OmniVision OV2640 */
--
1.8.5.3
--
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
[linuxtv-media:master 463/499] drivers/media/dvb-frontends/drx39xyj/drxj.c:20670:34: sparse: cast to restricted __be16
tree: git://linuxtv.org/media_tree.git master head: 59432be1c7fbf2a4f608850855ff649bee0f7b3b commit: b240eacdd536bac23c9d48dfc3d527ed6870ddad [463/499] [media] drx-j: get rid of drx_driver.c reproduce: make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by ) drivers/media/dvb-frontends/drx39xyj/drxj.c:21196:1: sparse: no newline at end of file drivers/media/dvb-frontends/drx39xyj/drxj.c:611:6: sparse: symbol 'drx_dap_drxj_module_name' was not declared. Should it be static? drivers/media/dvb-frontends/drx39xyj/drxj.c:612:6: sparse: symbol 'drx_dap_drxj_version_text' was not declared. Should it be static? drivers/media/dvb-frontends/drx39xyj/drxj.c:614:20: sparse: symbol 'drx_dap_drxj_version' was not declared. Should it be static? drivers/media/dvb-frontends/drx39xyj/drxj.c:998:21: sparse: symbol 'drxj_default_aud_data_g' was not declared. Should it be static? drivers/media/dvb-frontends/drx39xyj/drxj.c:16591:68: sparse: dubious: x !y drivers/media/dvb-frontends/drx39xyj/drxj.c:16609:68: sparse: dubious: x !y drivers/media/dvb-frontends/drx39xyj/drxj.c:16628:68: sparse: dubious: x !y drivers/media/dvb-frontends/drx39xyj/drxj.c:20670:34: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20670:34: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20670:34: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20670:34: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20697:29: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20697:29: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20697:29: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20697:29: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20715:34: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20715:34: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20715:34: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20715:34: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20715:34: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20715:34: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20717:34: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20717:34: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20717:34: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20717:34: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20719:35: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20719:35: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20719:35: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20719:35: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20721:33: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20721:33: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20721:33: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20721:33: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20735:35: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20735:35: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20735:35: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20735:35: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20743:47: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20743:47: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20743:47: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20743:47: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20743:47: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20743:47: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20745:46: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20745:46: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20745:46: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20745:46: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20745:46: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20745:46: sparse: cast to restricted __be32
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Wed Mar 5 04:00:26 CET 2014 git branch: test git hash: 59432be1c7fbf2a4f608850855ff649bee0f7b3b gcc version:i686-linux-gcc (GCC) 4.8.2 sparse version: 0.4.5-rc1 host hardware: x86_64 host os:3.12-6.slh.2-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-exynos: WARNINGS linux-git-arm-mx: OK linux-git-arm-omap: OK linux-git-arm-omap1: OK linux-git-arm-pxa: OK linux-git-blackfin: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: ERRORS linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.31.14-i686: ERRORS linux-2.6.32.27-i686: WARNINGS linux-2.6.33.7-i686: ERRORS linux-2.6.34.7-i686: ERRORS linux-2.6.35.9-i686: ERRORS linux-2.6.36.4-i686: ERRORS linux-2.6.37.6-i686: ERRORS linux-2.6.38.8-i686: ERRORS linux-2.6.39.4-i686: ERRORS linux-3.0.60-i686: ERRORS linux-3.1.10-i686: ERRORS linux-3.2.37-i686: WARNINGS linux-3.3.8-i686: WARNINGS linux-3.4.27-i686: WARNINGS linux-3.5.7-i686: WARNINGS linux-3.6.11-i686: WARNINGS linux-3.7.4-i686: ERRORS linux-3.8-i686: ERRORS linux-3.9.2-i686: ERRORS linux-3.10.1-i686: ERRORS linux-3.11.1-i686: ERRORS linux-3.12-i686: ERRORS linux-3.13-i686: ERRORS linux-3.14-rc1-i686: ERRORS linux-2.6.31.14-x86_64: ERRORS linux-2.6.32.27-x86_64: WARNINGS linux-2.6.33.7-x86_64: WARNINGS linux-2.6.34.7-x86_64: ERRORS linux-2.6.35.9-x86_64: ERRORS linux-2.6.36.4-x86_64: ERRORS linux-2.6.37.6-x86_64: ERRORS linux-2.6.38.8-x86_64: ERRORS linux-2.6.39.4-x86_64: ERRORS linux-3.0.60-x86_64: ERRORS linux-3.1.10-x86_64: ERRORS linux-3.2.37-x86_64: WARNINGS linux-3.3.8-x86_64: WARNINGS linux-3.4.27-x86_64: WARNINGS linux-3.5.7-x86_64: WARNINGS linux-3.6.11-x86_64: WARNINGS linux-3.7.4-x86_64: ERRORS linux-3.8-x86_64: ERRORS linux-3.9.2-x86_64: ERRORS linux-3.10.1-x86_64: ERRORS linux-3.11.1-x86_64: ERRORS linux-3.12-x86_64: ERRORS linux-3.13-x86_64: ERRORS linux-3.14-rc1-x86_64: ERRORS apps: OK spec-git: OK sparse version: 0.4.5-rc1 sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2 The Media Infrastructure API 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
[linuxtv-media:master 485/499] drivers/media/dvb-frontends/drx39xyj/drxj.c:1672:65: sparse: Using plain integer as NULL pointer
tree: git://linuxtv.org/media_tree.git master head: 59432be1c7fbf2a4f608850855ff649bee0f7b3b commit: 9e4c509d7444e067d39d3ac96a3398721bca4f01 [485/499] [media] drx-j: Use single master mode reproduce: make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by ) drivers/media/dvb-frontends/drx39xyj/drxj.c:1672:65: sparse: Using plain integer as NULL pointer drivers/media/dvb-frontends/drx39xyj/drxj.c:1672:71: sparse: Using plain integer as NULL pointer drivers/media/dvb-frontends/drx39xyj/drxj.c:1674:52: sparse: Using plain integer as NULL pointer drivers/media/dvb-frontends/drx39xyj/drxj.c:1674:58: sparse: Using plain integer as NULL pointer drivers/media/dvb-frontends/drx39xyj/drxj.c:905:21: sparse: symbol 'drxj_default_aud_data_g' was not declared. Should it be static? drivers/media/dvb-frontends/drx39xyj/drxj.c:20248:25: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20248:25: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20248:25: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20248:25: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20250:25: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20250:25: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20250:25: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20250:25: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20274:34: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20274:34: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20274:34: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20274:34: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20274:34: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20274:34: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20276:34: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20276:34: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20276:34: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20276:34: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20278:35: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20278:35: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20278:35: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20278:35: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20280:33: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20280:33: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20280:33: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20280:33: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20087:34: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20087:34: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20087:34: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20087:34: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20114:29: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20114:29: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20114:29: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20114:29: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20132:34: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20132:34: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20132:34: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20132:34: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20132:34: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20132:34: sparse: cast to restricted __be32 drivers/media/dvb-frontends/drx39xyj/drxj.c:20134:34: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20134:34: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20134:34: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20134:34: sparse: cast to restricted __be16 drivers/media/dvb-frontends/drx39xyj/drxj.c:20136:35: sparse: cast to restricted __be16
