Re: [RFC v5 10/11] [media] vb2: add out-fence support to QBUF

[Alexandre Courbot]

On Thursday, November 16, 2017 2:10:56 AM JST, Gustavo Padovan wrote:

From: Gustavo Padovan 

If V4L2_BUF_FLAG_OUT_FENCE flag is present on the QBUF call we create
an out_fence and send its fd to userspace on the fence_fd field as a
return arg for the QBUF call.

The fence is signaled on buffer_done(), when the job on the buffer is
finished.

With out-fences we do not allow drivers to requeue buffers through vb2,
instead we flag an error on the buffer, signals it fence with error and


s/it/its


return it to userspace.

v6
- get rid of the V4L2_EVENT_OUT_FENCE event. We always keep the
ordering in vb2 for queueing in the driver, so the event is not
necessary anymore and the out_fence_fd is sent back to userspace
on QBUF call return arg
- do not allow requeueing with out-fences, instead mark the buffer
with an error and wake up to userspace.
- send the out_fence_fd back to userspace on the fence_fd field

v5:
- delay fd_install to DQ_EVENT (Hans)
- if queue is fully ordered send OUT_FENCE event right away
(Brian)
- rename 'q->ordered' to 'q->ordered_in_driver'
- merge change to implement OUT_FENCE event here

v4:
- return the out_fence_fd in the BUF_QUEUED event(Hans)

v3: - add WARN_ON_ONCE(q->ordered) on requeueing (Hans)
- set the OUT_FENCE flag if there is a fence pending (Hans)
- call fd_install() after vb2_core_qbuf() (Hans)
- clean up fence if vb2_core_qbuf() fails (Hans)
- add list to store sync_file and fence for the next queued buffer

v2: check if the queue is ordered.

Signed-off-by: Gustavo Padovan 
---
 drivers/media/v4l2-core/videobuf2-core.c | 42 
+---

 drivers/media/v4l2-core/videobuf2-v4l2.c | 21 +++-
 2 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c

index 8b4f0e9bcb36..2eb5ffa8e028 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -354,6 +354,7 @@ static int __vb2_queue_alloc(struct 
vb2_queue *q, enum vb2_memory memory,

vb->planes[plane].length = plane_sizes[plane];
vb->planes[plane].min_length = plane_sizes[plane];
}
+   vb->out_fence_fd = -1;
q->bufs[vb->index] = vb;
 
 		/* Allocate video buffer memory for the MMAP type */
@@ -934,10 +935,26 @@ void vb2_buffer_done(struct vb2_buffer 
*vb, enum vb2_buffer_state state)

case VB2_BUF_STATE_QUEUED:
return;
case VB2_BUF_STATE_REQUEUEING:
-   if (q->start_streaming_called)
-   __enqueue_in_driver(vb);
-   return;
+
+   if (!vb->out_fence) {
+   if (q->start_streaming_called)
+   __enqueue_in_driver(vb);
+   return;
+   }
+
+   /* Do not allow requeuing with explicit synchronization,
+* report it as an error to userspace */
+   state = VB2_BUF_STATE_ERROR;
+
+   /* fall through */
default:
+   if (state == VB2_BUF_STATE_ERROR)
+   dma_fence_set_error(vb->out_fence, -EFAULT);
+   dma_fence_signal(vb->out_fence);
+   dma_fence_put(vb->out_fence);
+   vb->out_fence = NULL;
+   vb->out_fence_fd = -1;
+
/* Inform any processes that may be waiting for buffers */
wake_up(>done_wq);
break;
@@ -1325,12 +1342,18 @@ EXPORT_SYMBOL_GPL(vb2_core_prepare_buf);
 int vb2_setup_out_fence(struct vb2_queue *q, unsigned int index)
 {
struct vb2_buffer *vb;
+   u64 context;
 
 	vb = q->bufs[index];
 
 	vb->out_fence_fd = get_unused_fd_flags(O_CLOEXEC);
 
-	vb->out_fence = vb2_fence_alloc(q->out_fence_context);

+   if (q->ordered_in_driver)
+   context = q->out_fence_context;
+   else
+   context = dma_fence_context_alloc(1);
+
+   vb->out_fence = vb2_fence_alloc(context);
if (!vb->out_fence) {
put_unused_fd(vb->out_fence_fd);
return -ENOMEM;
@@ -1594,6 +1617,9 @@ int vb2_core_qbuf(struct vb2_queue *q, 
unsigned int index, void *pb,

if (pb)
call_void_bufop(q, fill_user_buffer, vb, pb);
 
+	fd_install(vb->out_fence_fd, vb->sync_file->file);


What happens if the buffer does not have an output fence? Shouldn't this be 
protected by a check on whether the buffer has been queued with 
V4L2_BUF_FLAG_OUT_FENCE?


(... or maybe move this to vb2_setup_out_fence() after all, where we know 
for sure an output fence exists).



+   vb->sync_file = NULL;
+
dprintk(2, "qbuf of buffer %d succeeded\n", vb->index);
   

Re: [RFC v5 09/11] [media] vb2: add infrastructure to support out-fences

[Alexandre Courbot]

On Friday, November 17, 2017 4:19:00 PM JST, Alexandre Courbot wrote:

On Thursday, November 16, 2017 2:10:55 AM JST, Gustavo Padovan wrote:

From: Gustavo Padovan 

Add vb2_setup_out_fence() and the needed members to struct vb2_buffer.

v3:
- Do not hold yet another ref to the out_fence (Brian Starkey)

v2: - change it to reflect fd_install at DQEVENT ...


out_fence_fd is allocated in this patch but not used anywhere 
for the moment.
For consistency, maybe move its allocation to the next patch, 
or move the call
to fd_install() here if that is possible? In both cases, the 
call to get_unused_fd() can be moved right before fd_install() 
so you don't need to

call put_unused_fd() in the error paths below.


Aha, just realized that fd_install() was called in qbuf() :) Other comments 
probably still hold though.




... same thing for sync_file too. Maybe this patch can just be merged into
the next one? The current patch just creates an incomplete 
version of vb2_setup_out_fence() for which no user exist yet.



+
+   vb->out_fence = vb2_fence_alloc(q->out_fence_context);
+   if (!vb->out_fence) {
+   put_unused_fd(vb->out_fence_fd);
+   return -ENOMEM; ...

Re: [RFC v5 09/11] [media] vb2: add infrastructure to support out-fences

[Alexandre Courbot]

On Thursday, November 16, 2017 2:10:55 AM JST, Gustavo Padovan wrote:

From: Gustavo Padovan 

Add vb2_setup_out_fence() and the needed members to struct vb2_buffer.

v3:
- Do not hold yet another ref to the out_fence (Brian Starkey)

v2: - change it to reflect fd_install at DQEVENT
- add fence context for out-fences

Signed-off-by: Gustavo Padovan 
---
 drivers/media/v4l2-core/videobuf2-core.c | 28 
 include/media/videobuf2-core.h   | 20 
 2 files changed, 48 insertions(+)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c

index 26de4c80717d..8b4f0e9bcb36 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -24,8 +24,10 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 

+#include 
 #include 
 
 #include 
@@ -1320,6 +1322,32 @@ int vb2_core_prepare_buf(struct 
vb2_queue *q, unsigned int index, void *pb)

 }
 EXPORT_SYMBOL_GPL(vb2_core_prepare_buf);
 
+int vb2_setup_out_fence(struct vb2_queue *q, unsigned int index)

+{
+   struct vb2_buffer *vb;
+
+   vb = q->bufs[index];
+
+   vb->out_fence_fd = get_unused_fd_flags(O_CLOEXEC);


out_fence_fd is allocated in this patch but not used anywhere for the 
moment.
For consistency, maybe move its allocation to the next patch, or move the 
call
to fd_install() here if that is possible? In both cases, the call to 
get_unused_fd() can be moved right before fd_install() so you don't need to

call put_unused_fd() in the error paths below.

... same thing for sync_file too. Maybe this patch can just be merged into
the next one? The current patch just creates an incomplete version of 
vb2_setup_out_fence() for which no user exist yet.



+
+   vb->out_fence = vb2_fence_alloc(q->out_fence_context);
+   if (!vb->out_fence) {
+   put_unused_fd(vb->out_fence_fd);
+   return -ENOMEM;
+   }
+
+   vb->sync_file = sync_file_create(vb->out_fence);
+   if (!vb->sync_file) {
+   put_unused_fd(vb->out_fence_fd);
+   dma_fence_put(vb->out_fence);
+   vb->out_fence = NULL;
+   return -ENOMEM;
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(vb2_setup_out_fence);
+
 /**
  * vb2_start_streaming() - Attempt to start streaming.
  * @q: videobuf2 queue
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 5f48c7be7770..a9b0697bd782 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -259,6 +259,10 @@ struct vb2_buffer {
 *  using the buffer (queueing to the driver)
 * fence_cb:fence callback information
 * fence_cb_lock:   protect callback signal/remove
+* out_fence_fd:the out_fence_fd to be shared with userspace.
+* out_fence:   the out-fence associated with the buffer once
+*  it is queued to the driver.
+* sync_file:   the sync file to wrap the out fence
 */
enum vb2_buffer_state   state;
 
@@ -269,6 +273,10 @@ struct vb2_buffer {

struct dma_fence_cb fence_cb;
spinlock_t  fence_cb_lock;
 
+	int			out_fence_fd;

+   struct dma_fence*out_fence;
+   struct sync_file*sync_file;
+
 #ifdef CONFIG_VIDEO_ADV_DEBUG
/*
 * Counters for how often these buffer-related ops are
@@ -514,6 +522,7 @@ struct vb2_buf_ops {
  * @last_buffer_dequeued: used in poll() and DQBUF to 
immediately return if the

  * last decoded buffer was already dequeued. Set for capture queues
  * when a buffer with the V4L2_BUF_FLAG_LAST is dequeued.
+ * @out_fence_context: the fence context for the out fences
  * @last_fence:last in-fence received. Used to keep ordering.
  * @fileio:file io emulator internal data, used only if emulator is active
  * @threadio:  thread io internal data, used only if thread is active
@@ -569,6 +578,7 @@ struct vb2_queue {
unsigned intcopy_timestamp:1;
unsigned intlast_buffer_dequeued:1;
 
+	u64out_fence_context;

struct dma_fence*last_fence;
 
 	struct vb2_fileio_data		*fileio;
@@ -740,6 +750,16 @@ int vb2_core_create_bufs(struct vb2_queue 
*q, enum vb2_memory memory,
 int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int 
index, void *pb);
 
 /**

+ * vb2_setup_out_fence() - setup new out-fence
+ * @q: The vb2_queue where to setup it
+ * @index: index of the buffer
+ *
+ * Setup the file descriptor, the fence and the sync_file for the next
+ * buffer to be queued and add everything to the tail of the 
q->out_fence_list.

+ */
+int vb2_setup_out_fence(struct vb2_queue *q, unsigned int index);
+
+/**
  * vb2_core_qbuf() - 

Re: [RFC v5 08/11] [media] vb2: add videobuf2 dma-buf fence helpers

[Alexandre Courbot]

On Friday, November 17, 2017 4:02:56 PM JST, Alexandre Courbot wrote:

On Thursday, November 16, 2017 2:10:54 AM JST, Gustavo Padovan wrote:

From: Javier Martinez Canillas 

Add a videobuf2-fence.h header file that contains different helpers
for DMA buffer sharing explicit fence support in videobuf2.

v2: - use fence context provided by the caller in vb2_fence_alloc()

Signed-off-by: Javier Martinez Canillas  ...


It is probably not a good idea to define that struct here since it will be
deduplicated for every source file that includes it.

Maybe change it to a simple declaration, and move the definition to
videobuf2-core.c or a dedicated videobuf2-fence.c file?


+
+static inline struct dma_fence *vb2_fence_alloc(u64 context)
+{
+   struct dma_fence *vb2_fence = kzalloc(sizeof(*vb2_fence), GFP_KERNEL);
+ ...


Not sure we gain a lot by having this function static inline, but your call.



Looking at the following patch, since it seems that this function is only 
to be

called from vb2_setup_out_fence() anyway, you may as well make it static in
videobuf2-core.c or even inline it in vb2_setup_out_fence() - it would only
add a few extra lines to this function.

Re: [RFC v5 08/11] [media] vb2: add videobuf2 dma-buf fence helpers

[Alexandre Courbot]

On Thursday, November 16, 2017 2:10:54 AM JST, Gustavo Padovan wrote:

From: Javier Martinez Canillas 

Add a videobuf2-fence.h header file that contains different helpers
for DMA buffer sharing explicit fence support in videobuf2.

v2: - use fence context provided by the caller in vb2_fence_alloc()

Signed-off-by: Javier Martinez Canillas 
Signed-off-by: Gustavo Padovan 
---
 include/media/videobuf2-fence.h | 48 
+

 1 file changed, 48 insertions(+)
 create mode 100644 include/media/videobuf2-fence.h

diff --git a/include/media/videobuf2-fence.h 
b/include/media/videobuf2-fence.h

new file mode 100644
index ..b49cc1bf6bb4
--- /dev/null
+++ b/include/media/videobuf2-fence.h
@@ -0,0 +1,48 @@
+/*
+ * videobuf2-fence.h - DMA buffer sharing fence helpers for videobuf 2
+ *
+ * Copyright (C) 2016 Samsung Electronics
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation.
+ */
+
+#include 
+#include 
+
+static DEFINE_SPINLOCK(vb2_fence_lock);
+
+static inline const char *vb2_fence_get_driver_name(struct 
dma_fence *fence)

+{
+   return "vb2_fence";
+}
+
+static inline const char *vb2_fence_get_timeline_name(struct 
dma_fence *fence)

+{
+   return "vb2_fence_timeline";
+}
+
+static inline bool vb2_fence_enable_signaling(struct dma_fence *fence)
+{
+   return true;
+}
+
+static const struct dma_fence_ops vb2_fence_ops = {
+   .get_driver_name = vb2_fence_get_driver_name,
+   .get_timeline_name = vb2_fence_get_timeline_name,
+   .enable_signaling = vb2_fence_enable_signaling,
+   .wait = dma_fence_default_wait,
+};


It is probably not a good idea to define that struct here since it will be
deduplicated for every source file that includes it.

Maybe change it to a simple declaration, and move the definition to
videobuf2-core.c or a dedicated videobuf2-fence.c file?


+
+static inline struct dma_fence *vb2_fence_alloc(u64 context)
+{
+   struct dma_fence *vb2_fence = kzalloc(sizeof(*vb2_fence), GFP_KERNEL);
+
+   if (!vb2_fence)
+   return NULL;
+
+   dma_fence_init(vb2_fence, _fence_ops, _fence_lock, context, 1);
+
+   return vb2_fence;
+}


Not sure we gain a lot by having this function static inline, but your 
call.

Re: [RFC v5 07/11] [media] vb2: add in-fence support to QBUF

[Alexandre Courbot]

Hi Gustavo,

I am coming a bit late in this series' review, so apologies if some of my
comments have already have been discussed in an earlier revision.

On Thursday, November 16, 2017 2:10:53 AM JST, Gustavo Padovan wrote:

From: Gustavo Padovan 

Receive in-fence from userspace and add support for waiting on them
before queueing the buffer to the driver. Buffers can't be queued to the
driver before its fences signal. And a buffer can't be queue to the driver
out of the order they were queued from userspace. That means that even if
it fence signal it must wait all other buffers, ahead of it in the queue,
to signal first.

To make that possible we use fence_array to keep that ordering. Basically
we create a fence_array that contains both the current fence and the fence
from the previous buffer (which might be a fence array as well). The base
fence class for the fence_array becomes the new buffer fence, waiting on
that one guarantees that it won't be queued out of order.

v6:
- With fences always keep the order userspace queues the buffers.
- Protect in_fence manipulation with a lock (Brian Starkey)
- check if fences have the same context before adding a fence array
- Fix last_fence ref unbalance in __set_in_fence() (Brian Starkey)
- Clean up fence if __set_in_fence() fails (Brian Starkey)
- treat -EINVAL from dma_fence_add_callback() (Brian Starkey)

v5: - use fence_array to keep buffers ordered in vb2 core when
needed (Brian Starkey)
- keep backward compat on the reserved2 field (Brian Starkey)
- protect fence callback removal with lock (Brian Starkey)

v4:
- Add a comment about dma_fence_add_callback() not returning a
error (Hans)
- Call dma_fence_put(vb->in_fence) if fence signaled (Hans)
- select SYNC_FILE under config VIDEOBUF2_CORE (Hans)
- Move dma_fence_is_signaled() check to __enqueue_in_driver() (Hans)
- Remove list_for_each_entry() in __vb2_core_qbuf() (Hans)
-  Remove if (vb->state != VB2_BUF_STATE_QUEUED) from
vb2_start_streaming() (Hans)
- set IN_FENCE flags on __fill_v4l2_buffer (Hans)
- Queue buffers to the driver as soon as they are ready (Hans)
- call fill_user_buffer() after queuing the buffer (Hans)
- add err: label to clean up fence
- add dma_fence_wait() before calling vb2_start_streaming()

v3: - document fence parameter
- remove ternary if at vb2_qbuf() return (Mauro)
- do not change if conditions behaviour (Mauro)

v2:
- fix vb2_queue_or_prepare_buf() ret check
- remove check for VB2_MEMORY_DMABUF only (Javier)
- check num of ready buffers to start streaming
- when queueing, start from the first ready buffer
- handle queue cancel

Signed-off-by: Gustavo Padovan 
---
 drivers/media/v4l2-core/Kconfig  |   1 +
 drivers/media/v4l2-core/videobuf2-core.c | 202 
---

 drivers/media/v4l2-core/videobuf2-v4l2.c |  29 -
 include/media/videobuf2-core.h   |  17 ++-
 4 files changed, 231 insertions(+), 18 deletions(-)

diff --git a/drivers/media/v4l2-core/Kconfig 
b/drivers/media/v4l2-core/Kconfig

index a35c33686abf..3f988c407c80 100644
--- a/drivers/media/v4l2-core/Kconfig
+++ b/drivers/media/v4l2-core/Kconfig
@@ -83,6 +83,7 @@ config VIDEOBUF_DVB
 # Used by drivers that need Videobuf2 modules
 config VIDEOBUF2_CORE
select DMA_SHARED_BUFFER
+   select SYNC_FILE
tristate
 
 config VIDEOBUF2_MEMOPS
diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c

index 60f8b582396a..26de4c80717d 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 

 #include 
@@ -346,6 +347,7 @@ static int __vb2_queue_alloc(struct 
vb2_queue *q, enum vb2_memory memory,

vb->index = q->num_buffers + buffer;
vb->type = q->type;
vb->memory = memory;
+   spin_lock_init(>fence_cb_lock);
for (plane = 0; plane < num_planes; ++plane) {
vb->planes[plane].length = plane_sizes[plane];
vb->planes[plane].min_length = plane_sizes[plane];
@@ -1222,6 +1224,9 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
 {
struct vb2_queue *q = vb->vb2_queue;
 
+	if (vb->in_fence && !dma_fence_is_signaled(vb->in_fence))

+   return;
+
vb->state = VB2_BUF_STATE_ACTIVE;
atomic_inc(>owned_by_drv_count);
 
@@ -1273,6 +1278,23 @@ static int __buf_prepare(struct 
vb2_buffer *vb, const void *pb)

return 0;
 }
 
+static int __get_num_ready_buffers(struct vb2_queue *q)

+{
+   struct vb2_buffer *vb;
+   int ready_count = 0;
+   unsigned long 

Re: [RFC v5 03/11] [media] vb2: add 'ordered_in_driver' property to queues

[Alexandre Courbot]

On Thursday, November 16, 2017 2:10:49 AM JST, Gustavo Padovan wrote:

From: Gustavo Padovan 

We use ordered_in_driver property to optimize for the case where
the driver can deliver the buffers in an ordered fashion. When it
is ordered we can use the same fence context for all fences, but
when it is not we need to a new context for each out-fence.


"we need to a new context" looks like it is missing a word.

cron job: media_tree daily build: WARNINGS

[Hans Verkuil]

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:   Fri Nov 17 05:00:25 CET 2017
media-tree git hash:30b4e122d71cbec2944a5f8b558b88936ee42f10
media_build git hash:   097aaf3e4e4bfdeff130db9697dec1befeb3221b
v4l-utils git hash: 58885f8fdd9a7ef5f0b7f0a8ff71f5a4ac0821b8
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: 0.5.1 (Debian: 0.5.1-2)
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.13.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: WARNINGS
linux-2.6.37.6-i686: WARNINGS
linux-2.6.38.8-i686: WARNINGS
linux-2.6.39.4-i686: WARNINGS
linux-3.0.60-i686: WARNINGS
linux-3.1.10-i686: WARNINGS
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: WARNINGS
linux-3.8-i686: WARNINGS
linux-3.9.2-i686: WARNINGS
linux-3.10.1-i686: WARNINGS
linux-3.11.1-i686: WARNINGS
linux-3.12.67-i686: WARNINGS
linux-3.13.11-i686: WARNINGS
linux-3.14.9-i686: WARNINGS
linux-3.15.2-i686: WARNINGS
linux-3.16.7-i686: WARNINGS
linux-3.17.8-i686: WARNINGS
linux-3.18.7-i686: WARNINGS
linux-3.19-i686: WARNINGS
linux-4.0.9-i686: WARNINGS
linux-4.1.33-i686: WARNINGS
linux-4.2.8-i686: WARNINGS
linux-4.3.6-i686: WARNINGS
linux-4.4.22-i686: WARNINGS
linux-4.5.7-i686: WARNINGS
linux-4.6.7-i686: WARNINGS
linux-4.7.5-i686: WARNINGS
linux-4.8-i686: OK
linux-4.9.26-i686: OK
linux-4.10.14-i686: OK
linux-4.11-i686: OK
linux-4.12.1-i686: OK
linux-4.13-i686: OK
linux-4.14-i686: OK
linux-2.6.36.4-x86_64: WARNINGS
linux-2.6.37.6-x86_64: WARNINGS
linux-2.6.38.8-x86_64: WARNINGS
linux-2.6.39.4-x86_64: WARNINGS
linux-3.0.60-x86_64: WARNINGS
linux-3.1.10-x86_64: WARNINGS
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: WARNINGS
linux-3.8-x86_64: WARNINGS
linux-3.9.2-x86_64: WARNINGS
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: WARNINGS
linux-3.12.67-x86_64: WARNINGS
linux-3.13.11-x86_64: WARNINGS
linux-3.14.9-x86_64: WARNINGS
linux-3.15.2-x86_64: WARNINGS
linux-3.16.7-x86_64: WARNINGS
linux-3.17.8-x86_64: WARNINGS
linux-3.18.7-x86_64: WARNINGS
linux-3.19-x86_64: WARNINGS
linux-4.0.9-x86_64: WARNINGS
linux-4.1.33-x86_64: WARNINGS
linux-4.2.8-x86_64: WARNINGS
linux-4.3.6-x86_64: WARNINGS
linux-4.4.22-x86_64: WARNINGS
linux-4.5.7-x86_64: WARNINGS
linux-4.6.7-x86_64: WARNINGS
linux-4.7.5-x86_64: WARNINGS
linux-4.8-x86_64: WARNINGS
linux-4.9.26-x86_64: WARNINGS
linux-4.10.14-x86_64: WARNINGS
linux-4.11-x86_64: WARNINGS
linux-4.12.1-x86_64: WARNINGS
linux-4.13-x86_64: OK
linux-4.14-x86_64: OK
apps: OK
spec-git: OK
sparse: WARNINGS

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Friday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Friday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html

Re: [PATCH] media: ABS macro parameter parenthesization

[Baruch Siach]

Hi Dan,

On Thu, Nov 16, 2017 at 06:09:20PM -0500, Dan Gopstein wrote:
> From: Dan Gopstein 
> 
> Two definitions of the ABS (absolute value) macro fail to parenthesize their
> parameter properly. This can lead to a bad expansion for low-precedence
> expression arguments. Add parens to protect against troublesome arguments.
> 
> Signed-off-by: Dan Gopstein 
> ---
> See an example bad usage in
> drivers/media/dvb-frontends/mb86a16.c b/drivers/media/dvb-frontends/mb86a16.c
> on line 1204:
> 
> ABS(prev_swp_freq[j] - swp_freq)
>
> For example: ABS(1-2) currently expands to ((1-2) < 0 ? (-1-2) : (1-2)) which
> evaluates to -3. But the correct expansion would be ((1-2) < 0 ? -(1-2) : 
> (1-2))
> which evaluates to 1.

This example would be nice to have in the commit log.

> I found this issue as part of the "Atoms of Confusion" research at NYU's 
> Secure
> Systems Lab. As the work continues, hopefully we'll be able to find more 
> issues
> like this one.
> 
> diff --git a/drivers/media/dvb-frontends/dibx000_common.h
> b/drivers/media/dvb-frontends/dibx000_common.h

Unfortunately, your email client (gmail?) mangled the patch by splitting lines 
like the above. So 'git am', or the 'patch' utility can't apply your patch as 
is.

I suggest you to use 'git send-email' to send patches. You can find gmail 
specific setup instructions in the git-send-email man page[1] under EXAMPLE.

[1] https://git-scm.com/docs/git-send-email

baruch

> index 8784af9..ae60f5d 100644
> --- a/drivers/media/dvb-frontends/dibx000_common.h
> +++ b/drivers/media/dvb-frontends/dibx000_common.h
> @@ -223,7 +223,7 @@ struct dvb_frontend_parametersContext {
> 
> #define FE_CALLBACK_TIME_NEVER 0x
> 
> -#define ABS(x) ((x < 0) ? (-x) : (x))
> +#define ABS(x) (((x) < 0) ? -(x) : (x))
> 
> #define DATA_BUS_ACCESS_MODE_8BIT 0x01
> #define DATA_BUS_ACCESS_MODE_16BIT0x02
> diff --git a/drivers/media/dvb-frontends/mb86a16.c
> b/drivers/media/dvb-frontends/mb86a16.c
> index dfe322e..2d921c7 100644
> --- a/drivers/media/dvb-frontends/mb86a16.c
> +++ b/drivers/media/dvb-frontends/mb86a16.c
> @@ -31,7 +31,7 @@
> static unsigned int verbose = 5;
> module_param(verbose, int, 0644);
> 
> -#define ABS(x) ((x) < 0 ? (-x) : (x))
> +#define ABS(x) ((x) < 0 ? -(x) : (x))
> 
> struct mb86a16_state {
> struct i2c_adapter  *i2c_adap;

-- 
 http://baruch.siach.name/blog/  ~. .~   Tk Open Systems
=}ooO--U--Ooo{=
   - bar...@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

Re: [RFCv1 PATCH 0/6] v4l2-ctrls: implement requests

[Alexandre Courbot]

On Thu, Nov 16, 2017 at 6:13 PM, Hans Verkuil  wrote:
> On 16/11/17 09:48, Alexandre Courbot wrote:
>> Hi Hans,
>>
>> On Wed, Nov 15, 2017 at 7:12 PM, Hans Verkuil  wrote:
>>> Hi Alexandre,
>>>
>>> On 15/11/17 10:38, Alexandre Courbot wrote:
 Hi Hans!

 Thanks for the patchset! It looks quite good at first sight, a few 
 comments and
 questions follow though.

 On Monday, November 13, 2017 11:34:02 PM JST, Hans Verkuil wrote:
> From: Hans Verkuil 
>
> Hi Alexandre,
>
> This is a first implementation of the request API in the
> control framework. It is fairly simplistic at the moment in that
> it just clones all the control values (so no refcounting yet for
> values as Laurent proposed, I will work on that later). But this
> should not be a problem for codecs since there aren't all that many
> controls involved.

 Regarding value refcounting, I think we can probably do without it if we 
 parse
 the requests queue when looking values up. It may be more practical 
 (having a
 kref for each v4l2_ctrl_ref in a request sounds overkill to me), and maybe 
 also
 more predictible since we would have less chance of having dangling old 
 values.

> The API is as follows:
>
> struct v4l2_ctrl_handler *v4l2_ctrl_request_alloc(void);
>
> This allocates a struct v4l2_ctrl_handler that is empty (i.e. has
> no controls) but is refcounted and is marked as representing a
> request.
>
> int v4l2_ctrl_request_clone(struct v4l2_ctrl_handler *hdl,
> const struct v4l2_ctrl_handler *from,
> bool (*filter)(const struct v4l2_ctrl *ctrl));
>
> Delete any existing controls in handler 'hdl', then clone the values
> from an existing handler 'from' into 'hdl'. If 'from' == NULL, then
> this just clears the handler. 'from' can either be another request
> control handler or a regular control handler in which case the
> current values are cloned. If 'filter' != NULL then you can
> filter which controls you want to clone.

 One thing that seems to be missing is, what happens if you try to set a 
 control
 on an empty request? IIUC this would currently fail because find_ref() 
 would
 not be able to find the control. The value ref should probably be created 
 in
 that case so we can create requests with a handful of controls.
>>>
>>> Wasn't the intention that we never have an empty request but always clone?
>>> I.e. in your code the _alloc call is always followed by a _clone call.
>>>
>>> The reason I have a separate _alloc function is that you use that when you
>>> want to create a new control handler ('new request'). If the user wants to 
>>> reuse an
>>> existing request, then _clone can be called directly on the existing 
>>> handler.
>>>
 Also, if you clone a handler that is not a request, I understand that all
 controls will be deduplicated, creating a full-state copy? That could be 
 useful,
 but since this is the only way to make the current code work, I hope that 
 the
 current impossibility to set a control on an empty request is a bug (or 
 misunderstanding from my part).
>>>
>>> I think it is a misunderstanding. Seen from userspace you'll never have an 
>>> empty
>>> request.
>>
>> It depends on what we want requests to be:
>>
>> (1) A representation of what the complete state of the device should
>> be when processing buffers, or
>>
>> (2) A set of controls to be applied before processing buffers.
>>
>> IIUC your patchset currently implements (1): as we clone a request (or
>> the current state of the device), we create a new ref for every
>> control that is not inherited. And when applying the request, we
>> consider all these controls again.
>>
>> There is the fact that on more complex pipelines, the number of
>> controls may be big enough that we may want to limit the number we
>> need to manage per request, but my main concern is that this will
>> probably disallow some use-cases that we discussed in Prague.
>>
>> For instance, let's say I create a request for a camera sensor by
>> cloning the device's control_handler, and set a few controls like
>> exposure that I want to be used for a give frame. But let's also say
>> that I have another thread taking care of focus on the same device -
>> this thread uses some other input to constantly adjust the focus by
>> calling S_EXT_CTRLS directly (i.e. without a request). When my request
>> gets processed, the focus will be reset to whatever it was when I
>> cloned the request, which we want to avoid (if you remember, I
>> interrupted Laurent to ask whether this is the behavior we wanted, and
>> we all agreed it was).
>>
>> Or maybe I am missing something in your implementation?
>
> Such things are simply not yet implemented. This 

Re: [PATCH 01/10] video/hdmi: Allow "empty" HDMI infoframes

[Sharma, Shashank]

Regards

Shashank


On 11/16/2017 9:46 PM, Ville Syrjälä wrote:

On Thu, Nov 16, 2017 at 08:06:18PM +0530, Sharma, Shashank wrote:

Regards

Shashank


On 11/13/2017 10:34 PM, Ville Syrjala wrote:

From: Ville Syrjälä 

HDMI 2.0 Appendix F suggest that we should keep sending the infoframe
when switching from 3D to 2D mode, even if the infoframe isn't strictly
necessary (ie. not needed to transmit the VIC or stereo information).
This is a workaround against some sinks that fail to realize that they
should switch from 3D to 2D mode when the source stop transmitting
the infoframe.

v2: Handle unpack() as well
  Pull the length calculation into a helper

Cc: Shashank Sharma 
Cc: Andrzej Hajda 
Cc: Thierry Reding 
Cc: Hans Verkuil 
Cc: linux-media@vger.kernel.org
Reviewed-by: Andrzej Hajda  #v1
Signed-off-by: Ville Syrjälä 
---
   drivers/video/hdmi.c | 51 +++
   1 file changed, 31 insertions(+), 20 deletions(-)

diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index 1cf907ecded4..111a0ab6280a 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -321,6 +321,17 @@ int hdmi_vendor_infoframe_init(struct 
hdmi_vendor_infoframe *frame)
   }
   EXPORT_SYMBOL(hdmi_vendor_infoframe_init);
   
+static int hdmi_vendor_infoframe_length(const struct hdmi_vendor_infoframe *frame)

+{
+   /* for side by side (half) we also need to provide 3D_Ext_Data */
+   if (frame->s3d_struct >= HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF)
+   return 6;
+   else if (frame->vic != 0 || frame->s3d_struct != 
HDMI_3D_STRUCTURE_INVALID)
+   return 5;
+   else
+   return 4;
+}
+
   /**
* hdmi_vendor_infoframe_pack() - write a HDMI vendor infoframe to binary 
buffer
* @frame: HDMI infoframe
@@ -341,19 +352,11 @@ ssize_t hdmi_vendor_infoframe_pack(struct 
hdmi_vendor_infoframe *frame,
u8 *ptr = buffer;
size_t length;
   
-	/* empty info frame */

-   if (frame->vic == 0 && frame->s3d_struct == HDMI_3D_STRUCTURE_INVALID)
-   return -EINVAL;
-
/* only one of those can be supplied */
if (frame->vic != 0 && frame->s3d_struct != HDMI_3D_STRUCTURE_INVALID)
return -EINVAL;
   
-	/* for side by side (half) we also need to provide 3D_Ext_Data */

-   if (frame->s3d_struct >= HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF)
-   frame->length = 6;
-   else
-   frame->length = 5;
+   frame->length = hdmi_vendor_infoframe_length(frame);
   
   	length = HDMI_INFOFRAME_HEADER_SIZE + frame->length;
   
@@ -372,14 +375,16 @@ ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,

ptr[5] = 0x0c;
ptr[6] = 0x00;
   
-	if (frame->vic) {

-   ptr[7] = 0x1 << 5;/* video format */
-   ptr[8] = frame->vic;
-   } else {
+   if (frame->s3d_struct != HDMI_3D_STRUCTURE_INVALID) {
ptr[7] = 0x2 << 5;/* video format */
ptr[8] = (frame->s3d_struct & 0xf) << 4;
if (frame->s3d_struct >= HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF)
ptr[9] = (frame->s3d_ext_data & 0xf) << 4;
+   } else if (frame->vic) {

Please correct me if I dint understand this properly, but for a HDMI 2.0
sink + 3D transmission, wouldn't I be sending
HDMI 2.0 VIC = 94 as well as some valid s3d flag (like side by side) ?

That vic will be in the AVI infoframe. Here we're concerned about the
VIC in the HDMI vendor infoframe.
Yep, I was thinking if there is any way we can cross check that there is 
a valid HDMI 2 vic

before we do anything here, but seems like a long shot.

Reviewed-by: Shashank Sharma 

- Shashank

+   ptr[7] = 0x1 << 5;/* video format */
+   ptr[8] = frame->vic;
+   } else {
+   ptr[7] = 0x0 << 5;/* video format */
}
   
   	hdmi_infoframe_set_checksum(buffer, length);

@@ -1165,7 +1170,7 @@ hdmi_vendor_any_infoframe_unpack(union 
hdmi_vendor_any_infoframe *frame,
   
   	if (ptr[0] != HDMI_INFOFRAME_TYPE_VENDOR ||

ptr[1] != 1 ||
-   (ptr[2] != 5 && ptr[2] != 6))
+   (ptr[2] != 4 && ptr[2] != 5 && ptr[2] != 6))
return -EINVAL;
   
   	length = ptr[2];

@@ -1193,16 +1198,22 @@ hdmi_vendor_any_infoframe_unpack(union 
hdmi_vendor_any_infoframe *frame,
   
   	hvf->length = length;
   
-	if (hdmi_video_format == 0x1) {

-   hvf->vic = ptr[4];
-   } else if (hdmi_video_format == 0x2) {
+   if (hdmi_video_format == 0x2) {
+   if (length != 5 && length != 6)
+   return -EINVAL;
hvf->s3d_struct = ptr[4] >> 4;
if (hvf->s3d_struct >= 

RE: [PATCH v4 00/12] Intel IPU3 ImgU patchset

[Mani, Rajmohan]

Hi Sakari and all,

> -Original Message-
> From: Zhi, Yong
> Sent: Tuesday, October 17, 2017 8:47 PM
> To: linux-media@vger.kernel.org; sakari.ai...@linux.intel.com
> Cc: Zheng, Jian Xu ; Mani, Rajmohan
> ; Toivonen, Tuukka
> ; Hu, Jerry W ;
> a...@arndb.de; h...@lst.de; robin.mur...@arm.com; iommu@lists.linux-
> foundation.org; Zhi, Yong 
> Subject: [PATCH v4 00/12] Intel IPU3 ImgU patchset
> 
> This patchset adds support for the Intel IPU3 (Image Processing Unit) ImgU
> which is essentially a modern memory-to-memory ISP. It implements raw
> Bayer to YUV image format conversion as well as a large number of other pixel
> processing algorithms for improving the image quality.
> 
> Meta data formats are defined for image statistics (3A, i.e. automatic white
> balance, exposure and focus, histogram and local area contrast
> enhancement) as well as for the pixel processing algorithm parameters.
> The documentation for these formats is currently not included in the patchset
> but will be added in a future version of this set.
> 

Here is an update on the IPU3 documentation that we are currently working on.

Image processing in IPU3 relies on the following.

1) HW configuration to enable ISP and
2) setting customer specific 3A Tuning / Algorithm Parameters to achieve 
desired image quality. 

We intend to provide documentation on ImgU driver programming interface to help 
users of this driver to configure and enable ISP HW to meet their needs.  This 
documentation will include details on complete V4L2 Kernel driver interface and 
IO-Control parameters, except for the ISP internal algorithm and its parameters 
(which is Intel proprietary IP).

We will also provide an user space library in binary form to help users of this 
driver, to convert the public 3A tuning parameters to IPU3 algorithm 
parameters. This tool will be released under NDA to the users of this driver.

> The algorithm parameters need to be considered specific to a given frame and
> typically a large number of these parameters change on frame to frame basis.
> Additionally, the parameters are highly structured (and not a flat space of
> independent configuration primitives). They also reflect the data structures
> used by the firmware and the hardware. On top of that, the algorithms require
> highly specialized user space to make meaningful use of them. For these
> reasons it has been chosen video buffers to pass the parameters to the device.
> 
> On individual patches:
> 
> The heart of ImgU is the CSS, or Camera Subsystem, which contains the image
> processors and HW accelerators.
> 
> The 3A statistics and other firmware parameter computation related functions
> are implemented in patch 8.
> 
> All h/w programming related code can be found in patch 9.
> 
> To access DDR via ImgU's own memory space, IPU3 is also equipped with its
> own MMU unit, the driver is implemented in patch 2.
> 
> Currently, the MMU driver has dependency on two exported symbols
> (iommu_group_ref_get and iommu_group_get_for_dev))to build as ko.
> 
> Patch 3 uses above IOMMU driver for DMA mem related functions.
> 
> Patch 5-10 are basically IPU3 CSS specific implementations:
> 
> 6 and 7 provide some utility functions and manage IPU3 fw download and
> install.
> 
> The firmware which is called ipu3-fw.bin can be downloaded from:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git
> (commit 2c27b0cb02f18c022d8378e0e1abaf8b7ae8188f)
> 
> Patch 9 and 10 are of the same file, the latter implements interface functions
> for access fw & hw capabilities defined in patch 8.
> 
> Patch 11 has a dependency on Sakari's V4L2_BUF_TYPE_META_OUTPUT work:
> 
> 
> 
> 
> Patch 12 uses Kconfig and Makefile created by IPU3 cio2 patch series.
> 
> Link to user space implementation:
> 
>  camera/+/master>
> 
> Device topology:
> 
> ./media-ctl -d /dev/media0 -p
> Media controller API version 4.14.0
> 
> Media device information
> 
> driver  ipu3-imgu
> model   ipu3-imgu
> serial
> bus info:00:05.0
> hw revision 0x0
> driver version  4.14.0
> 
> Device topology
> - entity 1: ipu3-imgu:0 (8 pads, 8 links)
> type V4L2 subdev subtype Unknown flags 0
> device node name /dev/v4l-subdev0
>   pad0: Sink
>   [fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown]
>   <- "input":0 [ENABLED,IMMUTABLE]
>   pad1: Sink
>   [fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown]
>   <- "parameters":0 []
>   pad2: Source
>   [fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown]
>   -> "output":0 []
>   pad3: Source
>   

Re: [PATCH v1 08/10] media: i2c: ov772x: Remove soc_camera dependencies

[Sakari Ailus]

Hi Jacopo,

On Wed, Nov 15, 2017 at 11:56:01AM +0100, Jacopo Mondi wrote:
> Remove soc_camera framework dependencies from ov772x sensor driver.
> - Handle clock directly
> - Register async subdevice
> - Add platform specific enable/disable functions
> - Adjust build system
> 
> This commit does not remove the original soc_camera based driver.
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  drivers/media/i2c/Kconfig  | 12 +++
>  drivers/media/i2c/Makefile |  1 +
>  drivers/media/i2c/ov772x.c | 88 
> +++---
>  include/media/i2c/ov772x.h |  3 ++
>  4 files changed, 76 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 9415389..ff251ce 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -629,6 +629,18 @@ config VIDEO_OV5670
> To compile this driver as a module, choose M here: the
> module will be called ov5670.
> 
> +config VIDEO_OV772X
> + tristate "OmniVision OV772x sensor support"
> + depends on I2C && VIDEO_V4L2
> + depends on MEDIA_CAMERA_SUPPORT
> + ---help---
> +   This is a Video4Linux2 sensor-level driver for the OmniVision
> +   OV772x camera.
> +
> +   To compile this driver as a module, choose M here: the
> +   module will be called ov772x.
> +
> +
>  config VIDEO_OV7640
>   tristate "OmniVision OV7640 sensor support"
>   depends on I2C && VIDEO_V4L2
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index f104650..b2459a1 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -66,6 +66,7 @@ obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
>  obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
>  obj-$(CONFIG_VIDEO_OV5670) += ov5670.o
>  obj-$(CONFIG_VIDEO_OV6650) += ov6650.o
> +obj-$(CONFIG_VIDEO_OV772X) += ov772x.o
>  obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
>  obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
>  obj-$(CONFIG_VIDEO_OV9650) += ov9650.o
> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
> index 8063835..9be7e4e 100644
> --- a/drivers/media/i2c/ov772x.c
> +++ b/drivers/media/i2c/ov772x.c
> @@ -15,6 +15,7 @@
>   * published by the Free Software Foundation.
>   */
> 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -25,8 +26,8 @@
>  #include 
> 
>  #include 
> -#include 
> -#include 
> +
> +#include 

Alphabetical order would be nice.

>  #include 
>  #include 
>  #include 
> @@ -393,7 +394,7 @@ struct ov772x_win_size {
>  struct ov772x_priv {
>   struct v4l2_subdevsubdev;
>   struct v4l2_ctrl_handler  hdl;
> - struct v4l2_clk  *clk;
> + struct clk   *clk;
>   struct ov772x_camera_info*info;
>   const struct ov772x_color_format *cfmt;
>   const struct ov772x_win_size *win;
> @@ -550,7 +551,7 @@ static int ov772x_reset(struct i2c_client *client)
>  }
> 
>  /*
> - * soc_camera_ops function
> + * subdev ops
>   */
> 
>  static int ov772x_s_stream(struct v4l2_subdev *sd, int enable)
> @@ -650,13 +651,36 @@ static int ov772x_s_register(struct v4l2_subdev *sd,
>  }
>  #endif
> 
> +static int ov772x_power_on(struct ov772x_priv *priv)
> +{
> + int ret;
> +
> + if (priv->info->platform_enable) {
> + ret = priv->info->platform_enable();
> + if (ret)
> + return ret;

What does this do, enable the regulator?

> + }
> +
> + /*  drivers/sh/clk/core.c returns -EINVAL if clk is NULL */
> + return clk_enable(priv->clk) <= 0 ? 0 : 1;
> +}
> +
> +static int ov772x_power_off(struct ov772x_priv *priv)
> +{
> + if (priv->info->platform_enable)
> + priv->info->platform_disable();
> +
> + clk_disable(priv->clk);
> +
> + return 0;
> +}
> +
>  static int ov772x_s_power(struct v4l2_subdev *sd, int on)
>  {
> - struct i2c_client *client = v4l2_get_subdevdata(sd);
> - struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
>   struct ov772x_priv *priv = to_ov772x(sd);
> 
> - return soc_camera_set_power(>dev, ssdd, priv->clk, on);
> + return on ? ov772x_power_on(priv) :
> + ov772x_power_off(priv);
>  }
> 
>  static const struct ov772x_win_size *ov772x_select_win(u32 width, u32 height)
> @@ -1000,14 +1024,10 @@ static int ov772x_enum_mbus_code(struct v4l2_subdev 
> *sd,
>  static int ov772x_g_mbus_config(struct v4l2_subdev *sd,
>   struct v4l2_mbus_config *cfg)
>  {
> - struct i2c_client *client = v4l2_get_subdevdata(sd);
> - struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
> -
>   cfg->flags = V4L2_MBUS_PCLK_SAMPLE_RISING | V4L2_MBUS_MASTER |
>   V4L2_MBUS_VSYNC_ACTIVE_HIGH | V4L2_MBUS_HSYNC_ACTIVE_HIGH |
>   V4L2_MBUS_DATA_ACTIVE_HIGH;
>   cfg->type = V4L2_MBUS_PARALLEL;
> - cfg->flags = soc_camera_apply_board_flags(ssdd, cfg);
> 
>   

Re: [PATCH v1 03/10] v4l: platform: Add Renesas CEU driver

[Sakari Ailus]

Hi Jacopo,

On Wed, Nov 15, 2017 at 03:25:11PM +0100, jacopo mondi wrote:
> Hi Sakari,
>thanks for review!

You're welcome!

> On Wed, Nov 15, 2017 at 02:45:51PM +0200, Sakari Ailus wrote:
> > Hi Jacopo,
> >
> > Could you remove the original driver and send the patch using git
> > send-email -C ? That way a single patch would address converting it to a
> > proper V4L2 driver as well as move it to the correct location. The changes
> > would be easier to review that way since then, well, it'd be easier to see
> > the changes. :-)
> 
> Actually I prefer not to remove the existing driver at the moment. See
> the cover letter for reasons why not to do so right now...

So it's about testing mostly? Does someone (possibly you) have those boards
to test? I'd like to see this patchset to remove that last remaining SoC
camera bridge driver. :-)

> 
> Also, there's not that much code from the old driver in here, surely
> less than the default 50% -C and -M options of 'git format-patch' use
> as a threshold for detecting copies iirc..

Oh, if that's so, then makes sense to review it as a new driver.

> 
> I would prefer this to be reviewed as new driver, I know it's a bit
> more painful, but irq handler and a couple of other routines apart,
> there's not that much code shared between the two...
> 
> >
> > The same goes for the two V4L2 SoC camera sensor / video decoder drivers at
> > the end of the set.
> >
> 
> Also in this case I prefer not to remove existing code, as long as
> there are platforms using it..

Couldn't they use this driver instead?

> 
> > On Wed, Nov 15, 2017 at 11:55:56AM +0100, Jacopo Mondi wrote:
> > > Add driver for Renesas Capture Engine Unit (CEU).
> > >
> > > The CEU interface supports capturing 'data' (YUV422) and 'images'
> > > (NV[12|21|16|61]).
> > >
> > > This driver aims to replace the soc_camera based sh_mobile_ceu one.
> > >
> > > Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ
> > > platform GR-Peach.
> > >
> > > Tested with ov7725 camera sensor on SH4 platform Migo-R.
> >
> > Nice!
> >
> > >
> > > Signed-off-by: Jacopo Mondi 
> > > ---
> > > +#include 
> >
> > Do you need this header? There would seem some that I wouldn't expect to be
> > needed below, such as linux/init.h.
> 
> It's probably a leftover, I'll remove it...
> 
> [snip]
> >
> > > +#if IS_ENABLED(CONFIG_OF)
> > > +static const struct of_device_id ceu_of_match[] = {
> > > + { .compatible = "renesas,renesas-ceu" },
> >
> > Even if you add support for new hardware, shouldn't you maintain support
> > for renesas,sh-mobile-ceu?
> >
> 
> As you noticed already, the old driver did not support OF, so there
> are no compatibility issues here

Yeah, I realised that only after reviewing this patch.

It'd be Super-cool if someone did the DT conversion. Perhaps Laurent? ;-)

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi

[PATCH] media: ABS macro parameter parenthesization

[Dan Gopstein]

From: Dan Gopstein 

Two definitions of the ABS (absolute value) macro fail to parenthesize their
parameter properly. This can lead to a bad expansion for low-precedence
expression arguments. Add parens to protect against troublesome arguments.

Signed-off-by: Dan Gopstein 
---
See an example bad usage in
drivers/media/dvb-frontends/mb86a16.c b/drivers/media/dvb-frontends/mb86a16.c
on line 1204:

ABS(prev_swp_freq[j] - swp_freq)

For example: ABS(1-2) currently expands to ((1-2) < 0 ? (-1-2) : (1-2)) which
evaluates to -3. But the correct expansion would be ((1-2) < 0 ? -(1-2) : (1-2))
which evaluates to 1.

I found this issue as part of the "Atoms of Confusion" research at NYU's Secure
Systems Lab. As the work continues, hopefully we'll be able to find more issues
like this one.

diff --git a/drivers/media/dvb-frontends/dibx000_common.h
b/drivers/media/dvb-frontends/dibx000_common.h
index 8784af9..ae60f5d 100644
--- a/drivers/media/dvb-frontends/dibx000_common.h
+++ b/drivers/media/dvb-frontends/dibx000_common.h
@@ -223,7 +223,7 @@ struct dvb_frontend_parametersContext {

#define FE_CALLBACK_TIME_NEVER 0x

-#define ABS(x) ((x < 0) ? (-x) : (x))
+#define ABS(x) (((x) < 0) ? -(x) : (x))

#define DATA_BUS_ACCESS_MODE_8BIT 0x01
#define DATA_BUS_ACCESS_MODE_16BIT0x02
diff --git a/drivers/media/dvb-frontends/mb86a16.c
b/drivers/media/dvb-frontends/mb86a16.c
index dfe322e..2d921c7 100644
--- a/drivers/media/dvb-frontends/mb86a16.c
+++ b/drivers/media/dvb-frontends/mb86a16.c
@@ -31,7 +31,7 @@
static unsigned int verbose = 5;
module_param(verbose, int, 0644);

-#define ABS(x) ((x) < 0 ? (-x) : (x))
+#define ABS(x) ((x) < 0 ? -(x) : (x))

struct mb86a16_state {
struct i2c_adapter  *i2c_adap;

[PATCH] media: rc: double keypresses due to timeout expiring to early

[Sean Young]

Since commit d57ea877af38 ("media: rc: per-protocol repeat period"),
double keypresses are reported on the ite-cir driver. This is due
two factors: that commit reduced the timeout used for some protocols
(it became protocol dependant) and the high default IR timeout used
by the ite-cir driver.

Some of the IR decoders wait for a trailing space, as that is
the only way to know if the bit stream has ended (e.g. rc-6 can be
16, 20 or 32 bits). The longer the IR timeout, the longer it will take
to receive the trailing space, delaying decoding and pushing it past the
keyup timeout.

So, add the IR timeout to the keyup timeout.

Cc:  # 4.14
Reported-by: Matthias Reichl 
Signed-off-by: Sean Young 
---
 drivers/media/rc/rc-main.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 17950e29d4e3..fae721534517 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -672,7 +672,8 @@ void rc_repeat(struct rc_dev *dev)
input_event(dev->input_dev, EV_MSC, MSC_SCAN, dev->last_scancode);
input_sync(dev->input_dev);
 
-   dev->keyup_jiffies = jiffies + msecs_to_jiffies(timeout);
+   dev->keyup_jiffies = jiffies + msecs_to_jiffies(timeout) +
+   nsecs_to_jiffies(dev->timeout);
mod_timer(>timer_keyup, dev->keyup_jiffies);
 
 out:
@@ -744,7 +745,8 @@ void rc_keydown(struct rc_dev *dev, enum rc_proto protocol, 
u32 scancode,
 
if (dev->keypressed) {
dev->keyup_jiffies = jiffies +
-   msecs_to_jiffies(protocols[protocol].repeat_period);
+   msecs_to_jiffies(protocols[protocol].repeat_period) +
+   nsecs_to_jiffies(dev->timeout);
mod_timer(>timer_keyup, dev->keyup_jiffies);
}
spin_unlock_irqrestore(>keylock, flags);
-- 
2.14.3

Re: [linux-sunxi] Cedrus driver

[Nicolas Dufresne]

Le jeudi 16 novembre 2017 à 12:02 +0100, Maxime Ripard a écrit :
> Assuming that the request API is in, we'd need to:
>   - Finish the MPEG4 support
>   - Work on more useful codecs (H264 comes to my mind)

For which we will have to review the tables and make sure they match
the spec (the easy part). But as an example, that branch uses a table
that merge Mpeg4 VOP and VOP Short Header. We need to make sure it does
not pause problems or split it up. On top of that, ST and Rockchip
teams should give some help and sync with these tables on their side.
We also need to consider decoder like Tegra 2. In H264, they don't need
frame parsing, but just the PPS/SPS data (might just be parsed in the
driver, like CODA ?). There is other mode of operation, specially in
H264/HEVC low latency, where the decoder will be similar, but will
accept and process slices right away, without waiting for the full
frame.

We also need some doc, to be able to tell the GStreamer and FFMPEG team
how to detect and handle these decoder. I doubt the libv4l2 proposed
approach will be used for these two projects since they already have
their own parser and would like to not parse twice. As an example, we
need to document that V4L2_PIX_FMT_MPEG2_FRAME implies using the
Request API and specific CID. We should probably also ping the Chrome
Devs, which probably have couple of pending branches around this.

regards,
Nicolas




signature.asc
Description: This is a digitally signed message part

Re: [PATCH 0/4] treewide: Fix line continuation formats

[Mimi Zohar]

On Thu, 2017-11-16 at 09:17 -0800, Joe Perches wrote:
> On Thu, 2017-11-16 at 12:11 -0500, Mimi Zohar wrote:
> > On Thu, 2017-11-16 at 07:27 -0800, Joe Perches wrote:
> > > Avoid using line continations in formats as that causes unexpected
> > > output.
> > 
> > Is having lines greater than 80 characters the preferred method?
> 
> yes.
> 
> >  Could you add quotes before the backlash and before the first word on
> > the next line instead?
> 
> coalesced formats are preferred.

In the future, please reference the commit 6f76b6fcaa60 "CodingStyle:
Document the exception of not splitting user-visible strings, for
grepping"

thanks,

Mimi

[PATCH v2 1/4] dt-bindings: media: rcar_vin: Reverse SoC part number list

[Fabrizio Castro]

Change the sorting of the part numbers from descending to ascending to
match with other documentation.

Signed-off-by: Fabrizio Castro 
Reviewed-by: Biju Das 
---
v1->v2:
* new patch triggered by Geert's comment, see the below link for details:
  https://www.mail-archive.com/linux-media@vger.kernel.org/msg121992.html

 Documentation/devicetree/bindings/media/rcar_vin.txt | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt 
b/Documentation/devicetree/bindings/media/rcar_vin.txt
index 6e4ef8c..98931f5 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -6,14 +6,14 @@ family of devices. The current blocks are always slaves and 
suppot one input
 channel which can be either RGB, YUYV or BT656.
 
  - compatible: Must be one or more of the following
-   - "renesas,vin-r8a7795" for the R8A7795 device
-   - "renesas,vin-r8a7794" for the R8A7794 device
-   - "renesas,vin-r8a7793" for the R8A7793 device
-   - "renesas,vin-r8a7792" for the R8A7792 device
-   - "renesas,vin-r8a7791" for the R8A7791 device
-   - "renesas,vin-r8a7790" for the R8A7790 device
-   - "renesas,vin-r8a7779" for the R8A7779 device
- "renesas,vin-r8a7778" for the R8A7778 device
+   - "renesas,vin-r8a7779" for the R8A7779 device
+   - "renesas,vin-r8a7790" for the R8A7790 device
+   - "renesas,vin-r8a7791" for the R8A7791 device
+   - "renesas,vin-r8a7792" for the R8A7792 device
+   - "renesas,vin-r8a7793" for the R8A7793 device
+   - "renesas,vin-r8a7794" for the R8A7794 device
+   - "renesas,vin-r8a7795" for the R8A7795 device
- "renesas,rcar-gen2-vin" for a generic R-Car Gen2 compatible device.
- "renesas,rcar-gen3-vin" for a generic R-Car Gen3 compatible device.
 
-- 
2.7.4

[PATCH v2 2/4] dt-bindings: media: rcar_vin: add device tree support for r8a774[35]

[Fabrizio Castro]

Add compatible strings for r8a7743 and r8a7745. No driver change
is needed as "renesas,rcar-gen2-vin" will activate the right code.
However, it is good practice to document compatible strings for the
specific SoC as this allows SoC specific changes to the driver if
needed, in addition to document SoC support and therefore allow
checkpatch.pl to validate compatible string values.

Signed-off-by: Fabrizio Castro 
Reviewed-by: Biju Das 
---
v1->v2:
* Fixed double "change" in changelog

 Documentation/devicetree/bindings/media/rcar_vin.txt | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt 
b/Documentation/devicetree/bindings/media/rcar_vin.txt
index 98931f5..ff9697e 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -6,6 +6,8 @@ family of devices. The current blocks are always slaves and 
suppot one input
 channel which can be either RGB, YUYV or BT656.
 
  - compatible: Must be one or more of the following
+   - "renesas,vin-r8a7743" for the R8A7743 device
+   - "renesas,vin-r8a7745" for the R8A7745 device
- "renesas,vin-r8a7778" for the R8A7778 device
- "renesas,vin-r8a7779" for the R8A7779 device
- "renesas,vin-r8a7790" for the R8A7790 device
@@ -14,7 +16,8 @@ channel which can be either RGB, YUYV or BT656.
- "renesas,vin-r8a7793" for the R8A7793 device
- "renesas,vin-r8a7794" for the R8A7794 device
- "renesas,vin-r8a7795" for the R8A7795 device
-   - "renesas,rcar-gen2-vin" for a generic R-Car Gen2 compatible device.
+   - "renesas,rcar-gen2-vin" for a generic R-Car Gen2 or RZ/G1 compatible
+ device.
- "renesas,rcar-gen3-vin" for a generic R-Car Gen3 compatible device.
 
When compatible with the generic version nodes must list the
-- 
2.7.4

[PATCH v2 0/4] Add VIN support to r8a774[35]

[Fabrizio Castro]

Hello,

this series documents VIN related dt-bindings for r8a774[35], adds VIN[012]
nodes to the r8a7743 SoC dtsi and adds VIN[01] nodes to the r8a7745 SoC dtsi.

Best regards,

Fabrizio Castro (4):
  dt-bindings: media: rcar_vin: Reverse SoC part number list
  dt-bindings: media: rcar_vin: add device tree support for r8a774[35]
  ARM: dts: r8a7743: add VIN dt support
  ARM: dts: r8a7745: add VIN dt support

 .../devicetree/bindings/media/rcar_vin.txt | 19 +++-
 arch/arm/boot/dts/r8a7743.dtsi | 36 ++
 arch/arm/boot/dts/r8a7745.dtsi | 24 +++
 3 files changed, 71 insertions(+), 8 deletions(-)

-- 
2.7.4

RE: [PATCH 1/2] dt-bindings: media: rcar_vin: add device tree support for r8a774[35]

[Fabrizio Castro]

Hi Geert,

> >>
> >> Please keep the list sorted by SoC part number.
> >
> > It is sorted, just in descending order. Do you want me to re-order the full 
> > list in ascending order?
>
> That may be a good idea, given the current order is non-standard and
> counter-intuitive.

sure, dropping this series and sending a v2 then.

Cheers,
Fab

>
> Thanks!
>
> Gr{oetje,eeting}s,
>
> Geert



Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, 
Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered 
No. 04586709.

Re: [PATCH v2 7/8] v4l: vsp1: Move video configuration to a cached dlb

[Kieran Bingham]

Hi Laurent,

Thankyou for the review, and your patience on the long awaited response on these
remaining patches.

On 17/08/17 19:10, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Monday 14 Aug 2017 16:13:30 Kieran Bingham wrote:
>> We are now able to configure a pipeline directly into a local display
>> list body. Take advantage of this fact, and create a cacheable body to
>> store the configuration of the pipeline in the video object.
>>
>> vsp1_video_pipeline_run() is now the last user of the pipe->dl object.
>> Convert this function to use the cached video->config body and obtain a
>> local display list reference.
>>
>> Attach the video->config body to the display list when needed before
>> committing to hardware.
>>
>> The pipe object is marked as un-configured when entering a suspend. This
>> ensures that upon resume, where the hardware is reset - our cached
>> configuration will be re-attached to the next committed DL.
>>
>> Signed-off-by: Kieran Bingham 
>> ---
>>
>> Our video DL usage now looks like the below output:
>>
>> dl->body0 contains our disposable runtime configuration. Max 41.
>> dl_child->body0 is our partition specific configuration. Max 12.
>> dl->fragments shows our constant configuration and LUTs.
>>
>>   These two are LUT/CLU:
>>  * dl->fragments[x]->num_entries 256 / max 256
>>  * dl->fragments[x]->num_entries 4914 / max 4914
>>
>> Which shows that our 'constant' configuration cache is currently
>> utilised to a maximum of 64 entries.
>>
>> trace-cmd report | \
>> grep max | sed 's/.*vsp1_dl_list_commit://g' | sort | uniq;
>>
>>   dl->body0->num_entries 13 / max 128
>>   dl->body0->num_entries 14 / max 128
>>   dl->body0->num_entries 16 / max 128
>>   dl->body0->num_entries 20 / max 128
>>   dl->body0->num_entries 27 / max 128
>>   dl->body0->num_entries 34 / max 128
>>   dl->body0->num_entries 41 / max 128
>>   dl_child->body0->num_entries 10 / max 128
>>   dl_child->body0->num_entries 12 / max 128
>>   dl->fragments[x]->num_entries 15 / max 128
>>   dl->fragments[x]->num_entries 16 / max 128
>>   dl->fragments[x]->num_entries 17 / max 128
>>   dl->fragments[x]->num_entries 18 / max 128
>>   dl->fragments[x]->num_entries 20 / max 128
>>   dl->fragments[x]->num_entries 21 / max 128
>>   dl->fragments[x]->num_entries 256 / max 256
>>   dl->fragments[x]->num_entries 31 / max 128
>>   dl->fragments[x]->num_entries 32 / max 128
>>   dl->fragments[x]->num_entries 39 / max 128
>>   dl->fragments[x]->num_entries 40 / max 128
>>   dl->fragments[x]->num_entries 47 / max 128
>>   dl->fragments[x]->num_entries 48 / max 128
>>   dl->fragments[x]->num_entries 4914 / max 4914
>>   dl->fragments[x]->num_entries 55 / max 128
>>   dl->fragments[x]->num_entries 56 / max 128
>>   dl->fragments[x]->num_entries 63 / max 128
>>   dl->fragments[x]->num_entries 64 / max 128
>> ---
>>  drivers/media/platform/vsp1/vsp1_pipe.c  |  4 +-
>>  drivers/media/platform/vsp1/vsp1_pipe.h  |  4 +-
>>  drivers/media/platform/vsp1/vsp1_video.c | 67 -
>>  drivers/media/platform/vsp1/vsp1_video.h |  2 +-
>>  4 files changed, 51 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c
>> b/drivers/media/platform/vsp1/vsp1_pipe.c index 5012643583b6..7d1f7ba43060
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_pipe.c
>> +++ b/drivers/media/platform/vsp1/vsp1_pipe.c
>> @@ -249,6 +249,7 @@ void vsp1_pipeline_run(struct vsp1_pipeline *pipe)
>>  vsp1_write(vsp1, VI6_CMD(pipe->output->entity.index),
>> VI6_CMD_STRCMD);
>>  pipe->state = VSP1_PIPELINE_RUNNING;
>> +pipe->configured = true;
>>  }
>>
>>  pipe->buffers_ready = 0;
>> @@ -430,6 +431,9 @@ void vsp1_pipelines_suspend(struct vsp1_device *vsp1)
>>  spin_lock_irqsave(>irqlock, flags);
>>  if (pipe->state == VSP1_PIPELINE_RUNNING)
>>  pipe->state = VSP1_PIPELINE_STOPPING;
>> +
>> +/* After a suspend, the hardware will be reset */
>> +pipe->configured = false;
> 
> It shouldn't make a difference in practice, but I think it would be more 
> logical to set the configured field to false after the hardware has been 
> reset. I'd move this to the resume handler and update the comment to "The 
> hardware might have been reset during suspend and need a full 
> reconfiguration". 

Agreed, and Done.


> 
>>  spin_unlock_irqrestore(>irqlock, flags);
>>  }
>>
>> diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h
>> b/drivers/media/platform/vsp1/vsp1_pipe.h index 90d29492b9b9..e7ad6211b4d0
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_pipe.h
>> +++ b/drivers/media/platform/vsp1/vsp1_pipe.h
>> @@ -90,6 +90,7 @@ struct vsp1_partition {
>>   * @irqlock: protects the pipeline state
>>   * @state: current state
>>   * @wq: wait queue to wait for state change completion
>> + * @configured: flag 

Re: [PATCH 0/4] treewide: Fix line continuation formats

[Joe Perches]

On Thu, 2017-11-16 at 12:11 -0500, Mimi Zohar wrote:
> On Thu, 2017-11-16 at 07:27 -0800, Joe Perches wrote:
> > Avoid using line continations in formats as that causes unexpected
> > output.
> 
> Is having lines greater than 80 characters the preferred method?

yes.

>  Could you add quotes before the backlash and before the first word on
> the next line instead?

coalesced formats are preferred.

Re: [PATCH 0/4] treewide: Fix line continuation formats

[Mimi Zohar]

On Thu, 2017-11-16 at 07:27 -0800, Joe Perches wrote:
> Avoid using line continations in formats as that causes unexpected
> output.

Is having lines greater than 80 characters the preferred method?
 Could you add quotes before the backlash and before the first word on
the next line instead?

Mimi

Re: 4.14 regression from commit d57ea877af38 media: rc: per-protocol repeat period

[Sean Young]

On Thu, Nov 16, 2017 at 04:27:01PM +0100, Matthias Reichl wrote:
> The following commit introduced a regression
> 
> commit d57ea877af38057b0ef31758cf3b99765dc33695
> Author: Sean Young 
> Date:   Wed Aug 9 13:19:16 2017 -0400
> 
> media: rc: per-protocol repeat period
> 
> CEC needs a keypress timeout of 550ms, which is too high for the IR
> protocols. Also fill in known repeat times, with 50ms error margin.
> 
> Also, combine all protocol data into one structure.
> 
> We received a report that an RC6 MCE remote used with the ite-cir
> produces "double events" on short button presses:
> 
> https://forum.kodi.tv/showthread.php?tid=298462=2667855#pid2667855
> 
> Looking at the ir-keytable -t output an additional key event is also
> generated after longer button presses:
> 
> # ir-keytable -t
> Testing events. Please, press CTRL-C to abort.
> 1510680591.697657: event type EV_MSC(0x04): scancode = 0x800f041f
> 1510680591.697657: event type EV_KEY(0x01) key_down: KEY_DOWN(0x006c)
> 1510680591.697657: event type EV_SYN(0x00).
> 1510680591.867355: event type EV_KEY(0x01) key_up: KEY_DOWN(0x006c)
> 1510680591.867355: event type EV_SYN(0x00).
> 1510680591.935026: event type EV_MSC(0x04): scancode = 0x800f041f
> 1510680591.935026: event type EV_KEY(0x01) key_down: KEY_DOWN(0x006c)
> 1510680591.935026: event type EV_SYN(0x00).
> 1510680592.104100: event type EV_KEY(0x01) key_up: KEY_DOWN(0x006c)
> 1510680592.104100: event type EV_SYN(0x00).
> 
> 1510680597.714055: event type EV_MSC(0x04): scancode = 0x800f0405
> 1510680597.714055: event type EV_KEY(0x01) key_down: KEY_NUMERIC_5(0x0205)
> 1510680597.714055: event type EV_SYN(0x00).
> 1510680597.819939: event type EV_MSC(0x04): scancode = 0x800f0405
> 1510680597.819939: event type EV_SYN(0x00).
> 1510680597.925614: event type EV_MSC(0x04): scancode = 0x800f0405
> 1510680597.925614: event type EV_SYN(0x00).
> 1510680598.032422: event type EV_MSC(0x04): scancode = 0x800f0405
> 1510680598.032422: event type EV_SYN(0x00).
> ...
> 1510680598.562114: event type EV_MSC(0x04): scancode = 0x800f0405
> 1510680598.562114: event type EV_SYN(0x00).
> 1510680598.630641: event type EV_KEY(0x01) key_down: KEY_NUMERIC_5(0x0205)
> 1510680598.630641: event type EV_SYN(0x00).
> 1510680598.667906: event type EV_MSC(0x04): scancode = 0x800f0405
> 1510680598.667906: event type EV_SYN(0x00).
> 1510680598.760760: event type EV_KEY(0x01) key_down: KEY_NUMERIC_5(0x0205)
> 1510680598.760760: event type EV_SYN(0x00).
> 1510680598.837412: event type EV_KEY(0x01) key_up: KEY_NUMERIC_5(0x0205)
> 1510680598.837412: event type EV_SYN(0x00).
> 1510680598.905003: event type EV_MSC(0x04): scancode = 0x800f0405
> 1510680598.905003: event type EV_KEY(0x01) key_down: KEY_NUMERIC_5(0x0205)
> 1510680598.905003: event type EV_SYN(0x00).
> 1510680599.074092: event type EV_KEY(0x01) key_up: KEY_NUMERIC_5(0x0205)
> 1510680599.074092: event type EV_SYN(0x00).
> 
> Looking at the timestamps of the scancode events it seems that
> signals are received every ~106ms but the last signal seems to be
> received 237ms after the last-but-one - which is then interpreted
> as a new key press cycle as the delay is longer than the 164ms
> "repeat_period" setting of the RC6 protocol (before that commit
> 250ms was used).
> 
> This 237ms delay seems to be coming from the 200ms timeout value
> of the ite-cir driver (237ms is in the ballpark of ~30ms rc6 signal
> time plus 200ms timeout).
> 
> The original author hasn't reported back yet but others confirmed
> that changing the timeout to 100ms (minimum idle timeout value
> of ite-cir) using "ir-ctl -t 10" fixes the issue.

Right, so some of the IR decoders wait for a trailing space, as that is
the only way to know if the bit stream has ended (e.g. rc-6 can be 
16 bits, 20 or 32). The longer the timeout, the longer it will take
to receive the trailing space, pushing decoding further along and
pushing it past the keyup timeout.

For any IR which is not the "last", decoding is triggered by the IR which
follows it.

I think the resolution is to add the rcdev timeout to the keyup
timeout; else this problem will start occuring again if someone sets
a high timeout (e.g. 300ms).

> I could locally reproduce this with gpio-ir-recv (which uses the
> default 125ms timeout) and the sony protocol (repeat_period = 100ms):
> 
> 1510838115.272021: event type EV_MSC(0x04): scancode = 0x110001
> 1510838115.272021: event type EV_KEY(0x01) key_down: KEY_2(0x0003)
> 1510838115.272021: event type EV_SYN(0x00).
> 1510838115.322014: event type EV_MSC(0x04): scancode = 0x110001
> 1510838115.322014: event type EV_SYN(0x00).
> 1510838115.362003: event type EV_MSC(0x04): scancode = 0x110001
> 1510838115.362003: event type EV_SYN(0x00).
> 1510838115.412002: event type EV_MSC(0x04): scancode = 0x110001
> 1510838115.412002: event type EV_SYN(0x00).
> 1510838115.521973: event type EV_KEY(0x01) key_up: KEY_2(0x0003)
> 1510838115.521973: event type EV_SYN(0x00).
> 

[PATCH v2] dvb_logfunc: add a user private parameter

[Rafaël Carré]

This is useful if an application is using 2 different devices,
and want logging to be split by device, or if application logging
needs some private context.

Care has been taken to not break the API/ABI and only add new functions,
or only modify private structures.

A drawback is that when dvb_fe_open_flags() is called, the custom function
can not be set yet, so dvb_fe_open_flags() will log errors using the old
function without private context.

Signed-off-by: Rafaël Carré 
---
 lib/include/libdvbv5/dvb-dev.h | 21 
 lib/include/libdvbv5/dvb-log.h | 73 --
 lib/libdvbv5/dvb-dev.c | 13 
 lib/libdvbv5/dvb-fe-priv.h |  3 ++
 lib/libdvbv5/dvb-fe.c  |  8 +
 utils/dvb/dvbv5-daemon.c   |  9 +++---
 6 files changed, 85 insertions(+), 42 deletions(-)

diff --git a/lib/include/libdvbv5/dvb-dev.h b/lib/include/libdvbv5/dvb-dev.h
index 55e0f065..6dbd2ae7 100644
--- a/lib/include/libdvbv5/dvb-dev.h
+++ b/lib/include/libdvbv5/dvb-dev.h
@@ -234,6 +234,27 @@ struct dvb_dev_list *dvb_get_dev_info(struct dvb_device 
*dvb,
  */
 void dvb_dev_stop_monitor(struct dvb_device *dvb);
 
+/**
+ * @brief Sets the DVB verbosity and log function with context private data
+ * @ingroup dvb_device
+ *
+ * @param dvb  pointer to struct dvb_device to be used
+ * @param verbose  Verbosity level of the messages that will be printed
+ * @param logfunc  Callback function to be called when a log event
+ * happens. Can either store the event into a file or
+ * to print it at the TUI/GUI. Can be null.
+ * @param logpriv   Private data for log function
+ *
+ * @details Sets the function to report log errors and to set the verbosity
+ * level of debug report messages. If not called, or if logfunc is
+ * NULL, the libdvbv5 will report error and debug messages via stderr,
+ * and will use colors for the debug messages.
+ *
+ */
+void dvb_dev_set_logpriv(struct dvb_device *dvb,
+unsigned verbose,
+dvb_logfunc_priv logfunc, void *logpriv);
+
 /**
  * @brief Sets the DVB verbosity and log function
  * @ingroup dvb_device
diff --git a/lib/include/libdvbv5/dvb-log.h b/lib/include/libdvbv5/dvb-log.h
index 181a23c8..23cceede 100644
--- a/lib/include/libdvbv5/dvb-log.h
+++ b/lib/include/libdvbv5/dvb-log.h
@@ -43,61 +43,58 @@
 
 typedef void (*dvb_logfunc)(int level, const char *fmt, ...) __attribute__ (( 
format( printf, 2, 3 )));
 
+/**
+ * @typedef void (*dvb_logfunc)(void *logpriv, int level, const char *fmt, ...)
+ * @brief typedef used by dvb_fe_open2 for the log function with private 
context
+ * @ingroup ancillary
+ */
+
+typedef void (*dvb_logfunc_priv)(void *logpriv, int level, const char *fmt, 
...);
+
 /*
  * Macros used internally inside libdvbv5 frontend part, to output logs
  */
 
 #ifndef _DOXYGEN
 
-#ifndef __DVB_FE_PRIV_H
+struct dvb_v5_fe_parms;
+/**
+ * @brief retrieve the logging function with private data from the private fe 
params.
+ */
+dvb_logfunc_priv dvb_get_log_priv(struct dvb_v5_fe_parms *, void **);
 
-#define dvb_log(fmt, arg...) do {\
-   parms->logfunc(LOG_INFO, fmt, ##arg); \
-} while (0)
-#define dvb_logerr(fmt, arg...) do {\
-   parms->logfunc(LOG_ERR, fmt, ##arg); \
-} while (0)
-#define dvb_logdbg(fmt, arg...) do {\
-   parms->logfunc(LOG_DEBUG, fmt, ##arg); \
-} while (0)
-#define dvb_logwarn(fmt, arg...) do {\
-   parms->logfunc(LOG_WARNING, fmt, ##arg); \
-} while (0)
-#define dvb_loginfo(fmt, arg...) do {\
-   parms->logfunc(LOG_NOTICE, fmt, ##arg); \
-} while (0)
+#ifndef __DVB_FE_PRIV_H
 
-#define dvb_perror(msg) do {\
-   parms->logfunc(LOG_ERR, "%s: %s", msg, strerror(errno)); \
+#define dvb_loglevel(level, fmt, arg...) do {\
+void *priv;\
+dvb_logfunc_priv f = dvb_get_log_priv(parms, );\
+if (f) {\
+f(priv, level, fmt, ##arg);\
+} else {\
+parms->logfunc(level, fmt, ##arg); \
+}\
 } while (0)
 
 #else
 
-#define dvb_log(fmt, arg...) do {\
-   parms->p.logfunc(LOG_INFO, fmt, ##arg); \
-} while (0)
-#define dvb_logerr(fmt, arg...) do {\
-   parms->p.logfunc(LOG_ERR, fmt, ##arg); \
-} while (0)
-#define dvb_logdbg(fmt, arg...) do {\
-   parms->p.logfunc(LOG_DEBUG, fmt, ##arg); \
-} while (0)
-#define dvb_logwarn(fmt, arg...) do {\
-   parms->p.logfunc(LOG_WARNING, fmt, ##arg); \
-} while (0)
-#define dvb_loginfo(fmt, arg...) do {\
-   parms->p.logfunc(LOG_NOTICE, fmt, ##arg); \
-} while (0)
 #define dvb_loglevel(level, fmt, arg...) do {\
-   parms->p.logfunc(level, fmt, ##arg); \
-} while (0)
-
-#define dvb_perror(msg) do {\
-   parms->p.logfunc(LOG_ERR, "%s: %s", msg, strerror(errno)); \
+if (parms->logfunc_priv) {\
+parms->logfunc_priv(parms->logpriv, level, fmt, ##arg);\
+} else {\
+parms->p.logfunc(level, fmt, ##arg); \
+}\
 } while (0)
 
 #endif
 
+#define dvb_log(fmt, arg...) 

Re: [PATCH 01/10] video/hdmi: Allow "empty" HDMI infoframes

[Ville Syrjälä]

On Thu, Nov 16, 2017 at 08:06:18PM +0530, Sharma, Shashank wrote:
> Regards
> 
> Shashank
> 
> 
> On 11/13/2017 10:34 PM, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> >
> > HDMI 2.0 Appendix F suggest that we should keep sending the infoframe
> > when switching from 3D to 2D mode, even if the infoframe isn't strictly
> > necessary (ie. not needed to transmit the VIC or stereo information).
> > This is a workaround against some sinks that fail to realize that they
> > should switch from 3D to 2D mode when the source stop transmitting
> > the infoframe.
> >
> > v2: Handle unpack() as well
> >  Pull the length calculation into a helper
> >
> > Cc: Shashank Sharma 
> > Cc: Andrzej Hajda 
> > Cc: Thierry Reding 
> > Cc: Hans Verkuil 
> > Cc: linux-media@vger.kernel.org
> > Reviewed-by: Andrzej Hajda  #v1
> > Signed-off-by: Ville Syrjälä 
> > ---
> >   drivers/video/hdmi.c | 51 
> > +++
> >   1 file changed, 31 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> > index 1cf907ecded4..111a0ab6280a 100644
> > --- a/drivers/video/hdmi.c
> > +++ b/drivers/video/hdmi.c
> > @@ -321,6 +321,17 @@ int hdmi_vendor_infoframe_init(struct 
> > hdmi_vendor_infoframe *frame)
> >   }
> >   EXPORT_SYMBOL(hdmi_vendor_infoframe_init);
> >   
> > +static int hdmi_vendor_infoframe_length(const struct hdmi_vendor_infoframe 
> > *frame)
> > +{
> > +   /* for side by side (half) we also need to provide 3D_Ext_Data */
> > +   if (frame->s3d_struct >= HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF)
> > +   return 6;
> > +   else if (frame->vic != 0 || frame->s3d_struct != 
> > HDMI_3D_STRUCTURE_INVALID)
> > +   return 5;
> > +   else
> > +   return 4;
> > +}
> > +
> >   /**
> >* hdmi_vendor_infoframe_pack() - write a HDMI vendor infoframe to binary 
> > buffer
> >* @frame: HDMI infoframe
> > @@ -341,19 +352,11 @@ ssize_t hdmi_vendor_infoframe_pack(struct 
> > hdmi_vendor_infoframe *frame,
> > u8 *ptr = buffer;
> > size_t length;
> >   
> > -   /* empty info frame */
> > -   if (frame->vic == 0 && frame->s3d_struct == HDMI_3D_STRUCTURE_INVALID)
> > -   return -EINVAL;
> > -
> > /* only one of those can be supplied */
> > if (frame->vic != 0 && frame->s3d_struct != HDMI_3D_STRUCTURE_INVALID)
> > return -EINVAL;
> >   
> > -   /* for side by side (half) we also need to provide 3D_Ext_Data */
> > -   if (frame->s3d_struct >= HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF)
> > -   frame->length = 6;
> > -   else
> > -   frame->length = 5;
> > +   frame->length = hdmi_vendor_infoframe_length(frame);
> >   
> > length = HDMI_INFOFRAME_HEADER_SIZE + frame->length;
> >   
> > @@ -372,14 +375,16 @@ ssize_t hdmi_vendor_infoframe_pack(struct 
> > hdmi_vendor_infoframe *frame,
> > ptr[5] = 0x0c;
> > ptr[6] = 0x00;
> >   
> > -   if (frame->vic) {
> > -   ptr[7] = 0x1 << 5;  /* video format */
> > -   ptr[8] = frame->vic;
> > -   } else {
> > +   if (frame->s3d_struct != HDMI_3D_STRUCTURE_INVALID) {
> > ptr[7] = 0x2 << 5;  /* video format */
> > ptr[8] = (frame->s3d_struct & 0xf) << 4;
> > if (frame->s3d_struct >= HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF)
> > ptr[9] = (frame->s3d_ext_data & 0xf) << 4;
> > +   } else if (frame->vic) {
> Please correct me if I dint understand this properly, but for a HDMI 2.0 
> sink + 3D transmission, wouldn't I be sending
> HDMI 2.0 VIC = 94 as well as some valid s3d flag (like side by side) ?

That vic will be in the AVI infoframe. Here we're concerned about the
VIC in the HDMI vendor infoframe.

> 
> - Shashank
> > +   ptr[7] = 0x1 << 5;  /* video format */
> > +   ptr[8] = frame->vic;
> > +   } else {
> > +   ptr[7] = 0x0 << 5;  /* video format */
> > }
> >   
> > hdmi_infoframe_set_checksum(buffer, length);
> > @@ -1165,7 +1170,7 @@ hdmi_vendor_any_infoframe_unpack(union 
> > hdmi_vendor_any_infoframe *frame,
> >   
> > if (ptr[0] != HDMI_INFOFRAME_TYPE_VENDOR ||
> > ptr[1] != 1 ||
> > -   (ptr[2] != 5 && ptr[2] != 6))
> > +   (ptr[2] != 4 && ptr[2] != 5 && ptr[2] != 6))
> > return -EINVAL;
> >   
> > length = ptr[2];
> > @@ -1193,16 +1198,22 @@ hdmi_vendor_any_infoframe_unpack(union 
> > hdmi_vendor_any_infoframe *frame,
> >   
> > hvf->length = length;
> >   
> > -   if (hdmi_video_format == 0x1) {
> > -   hvf->vic = ptr[4];
> > -   } else if (hdmi_video_format == 0x2) {
> > +   if (hdmi_video_format == 0x2) {
> > +   if (length != 5 && length != 6)
> > +   return -EINVAL;
> > hvf->s3d_struct = ptr[4] >> 4;
> > if (hvf->s3d_struct >= 

Re: [PATCH/RFC 2/2] v4l: rcar-vin: Wait for device access to complete before unplugging

[Niklas Söderlund]

Hi Sakari,

On 2017-11-16 14:36:24 +0200, Sakari Ailus wrote:
> On Thu, Nov 16, 2017 at 02:33:49AM +0200, Laurent Pinchart wrote:
> > To avoid races between device access and unplug, call the
> > video_device_unplug() function in the platform driver remove handler.
> > This will unsure that all device access completes before the remove
> > handler proceeds to free resources.
> > 
> > Signed-off-by: Laurent Pinchart 
> > ---
> >  drivers/media/platform/rcar-vin/rcar-core.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
> > b/drivers/media/platform/rcar-vin/rcar-core.c
> > index bd7976efa1fb..c5210f1d09ed 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-core.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> > @@ -1273,6 +1273,7 @@ static int rcar_vin_remove(struct platform_device 
> > *pdev)
> >  
> > pm_runtime_disable(>dev);
> >  
> > +   video_device_unplug(>vdev);
> 
> Does this depend on another patch?

I believe this patch is on top of the R-Car VIN Gen3 enablement series.

> 
> >  
> > if (!vin->info->use_mc) {
> > v4l2_async_notifier_unregister(>notifier);
> > -- 
> > Regards,
> > 
> > Laurent Pinchart
> > 
> 
> -- 
> Sakari Ailus
> e-mail: sakari.ai...@iki.fi

-- 
Regards,
Niklas Söderlund

4.14 regression from commit d57ea877af38 media: rc: per-protocol repeat period

[Matthias Reichl]

The following commit introduced a regression

commit d57ea877af38057b0ef31758cf3b99765dc33695
Author: Sean Young 
Date:   Wed Aug 9 13:19:16 2017 -0400

media: rc: per-protocol repeat period

CEC needs a keypress timeout of 550ms, which is too high for the IR
protocols. Also fill in known repeat times, with 50ms error margin.

Also, combine all protocol data into one structure.

We received a report that an RC6 MCE remote used with the ite-cir
produces "double events" on short button presses:

https://forum.kodi.tv/showthread.php?tid=298462=2667855#pid2667855

Looking at the ir-keytable -t output an additional key event is also
generated after longer button presses:

# ir-keytable -t
Testing events. Please, press CTRL-C to abort.
1510680591.697657: event type EV_MSC(0x04): scancode = 0x800f041f
1510680591.697657: event type EV_KEY(0x01) key_down: KEY_DOWN(0x006c)
1510680591.697657: event type EV_SYN(0x00).
1510680591.867355: event type EV_KEY(0x01) key_up: KEY_DOWN(0x006c)
1510680591.867355: event type EV_SYN(0x00).
1510680591.935026: event type EV_MSC(0x04): scancode = 0x800f041f
1510680591.935026: event type EV_KEY(0x01) key_down: KEY_DOWN(0x006c)
1510680591.935026: event type EV_SYN(0x00).
1510680592.104100: event type EV_KEY(0x01) key_up: KEY_DOWN(0x006c)
1510680592.104100: event type EV_SYN(0x00).

1510680597.714055: event type EV_MSC(0x04): scancode = 0x800f0405
1510680597.714055: event type EV_KEY(0x01) key_down: KEY_NUMERIC_5(0x0205)
1510680597.714055: event type EV_SYN(0x00).
1510680597.819939: event type EV_MSC(0x04): scancode = 0x800f0405
1510680597.819939: event type EV_SYN(0x00).
1510680597.925614: event type EV_MSC(0x04): scancode = 0x800f0405
1510680597.925614: event type EV_SYN(0x00).
1510680598.032422: event type EV_MSC(0x04): scancode = 0x800f0405
1510680598.032422: event type EV_SYN(0x00).
...
1510680598.562114: event type EV_MSC(0x04): scancode = 0x800f0405
1510680598.562114: event type EV_SYN(0x00).
1510680598.630641: event type EV_KEY(0x01) key_down: KEY_NUMERIC_5(0x0205)
1510680598.630641: event type EV_SYN(0x00).
1510680598.667906: event type EV_MSC(0x04): scancode = 0x800f0405
1510680598.667906: event type EV_SYN(0x00).
1510680598.760760: event type EV_KEY(0x01) key_down: KEY_NUMERIC_5(0x0205)
1510680598.760760: event type EV_SYN(0x00).
1510680598.837412: event type EV_KEY(0x01) key_up: KEY_NUMERIC_5(0x0205)
1510680598.837412: event type EV_SYN(0x00).
1510680598.905003: event type EV_MSC(0x04): scancode = 0x800f0405
1510680598.905003: event type EV_KEY(0x01) key_down: KEY_NUMERIC_5(0x0205)
1510680598.905003: event type EV_SYN(0x00).
1510680599.074092: event type EV_KEY(0x01) key_up: KEY_NUMERIC_5(0x0205)
1510680599.074092: event type EV_SYN(0x00).

Looking at the timestamps of the scancode events it seems that
signals are received every ~106ms but the last signal seems to be
received 237ms after the last-but-one - which is then interpreted
as a new key press cycle as the delay is longer than the 164ms
"repeat_period" setting of the RC6 protocol (before that commit
250ms was used).

This 237ms delay seems to be coming from the 200ms timeout value
of the ite-cir driver (237ms is in the ballpark of ~30ms rc6 signal
time plus 200ms timeout).

The original author hasn't reported back yet but others confirmed
that changing the timeout to 100ms (minimum idle timeout value
of ite-cir) using "ir-ctl -t 10" fixes the issue.

I could locally reproduce this with gpio-ir-recv (which uses the
default 125ms timeout) and the sony protocol (repeat_period = 100ms):

1510838115.272021: event type EV_MSC(0x04): scancode = 0x110001
1510838115.272021: event type EV_KEY(0x01) key_down: KEY_2(0x0003)
1510838115.272021: event type EV_SYN(0x00).
1510838115.322014: event type EV_MSC(0x04): scancode = 0x110001
1510838115.322014: event type EV_SYN(0x00).
1510838115.362003: event type EV_MSC(0x04): scancode = 0x110001
1510838115.362003: event type EV_SYN(0x00).
1510838115.412002: event type EV_MSC(0x04): scancode = 0x110001
1510838115.412002: event type EV_SYN(0x00).
1510838115.521973: event type EV_KEY(0x01) key_up: KEY_2(0x0003)
1510838115.521973: event type EV_SYN(0x00).
1510838115.532007: event type EV_MSC(0x04): scancode = 0x110001
1510838115.532007: event type EV_KEY(0x01) key_down: KEY_2(0x0003)
1510838115.532007: event type EV_SYN(0x00).
1510838115.641972: event type EV_KEY(0x01) key_up: KEY_2(0x0003)
1510838115.641972: event type EV_SYN(0x00).

Reducing the timeout to 20ms removes the addional key_down/up event.

Another test with a rc-5 remote on gpio-ir-recv worked fine at the
default 125ms timeout but with 200ms as on the ite-cir I again
got the additional key event.

so long,

Hias

[PATCH 3/4] [media] dibx000_common: Fix line continuation format

[Joe Perches]

Line continuations with excess spacing causes unexpected output.

Signed-off-by: Joe Perches 
---
 drivers/media/dvb-frontends/dibx000_common.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/dvb-frontends/dibx000_common.c 
b/drivers/media/dvb-frontends/dibx000_common.c
index bc28184c7fb0..d981233e458f 100644
--- a/drivers/media/dvb-frontends/dibx000_common.c
+++ b/drivers/media/dvb-frontends/dibx000_common.c
@@ -288,8 +288,8 @@ static int dibx000_i2c_gated_gpio67_xfer(struct i2c_adapter 
*i2c_adap,
int ret;
 
if (num > 32) {
-   dprintk("%s: too much I2C message to be transmitted (%i).\
-   Maximum is 32", __func__, num);
+   dprintk("%s: too much I2C message to be transmitted (%i). 
Maximum is 32",
+   __func__, num);
return -ENOMEM;
}
 
@@ -335,8 +335,8 @@ static int dibx000_i2c_gated_tuner_xfer(struct i2c_adapter 
*i2c_adap,
int ret;
 
if (num > 32) {
-   dprintk("%s: too much I2C message to be transmitted (%i).\
-   Maximum is 32", __func__, num);
+   dprintk("%s: too much I2C message to be transmitted (%i). 
Maximum is 32",
+   __func__, num);
return -ENOMEM;
}
 
-- 
2.15.0

[PATCH 0/4] treewide: Fix line continuation formats

[Joe Perches]

Avoid using line continations in formats as that causes unexpected
output.

Joe Perches (4):
  rk3399_dmc: Fix line continuation format
  drm: amd: Fix line continuation formats
  [media] dibx000_common: Fix line continuation format
  ima: Fix line continuation format

 drivers/devfreq/rk3399_dmc.c   |  4 ++--
 drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c   | 11 -
 .../amd/powerplay/hwmgr/process_pptables_v1_0.c|  6 ++---
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 27 --
 drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c   |  6 ++---
 .../gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c  |  9 +++-
 .../gpu/drm/amd/powerplay/smumgr/vega10_smumgr.c   |  6 ++---
 drivers/media/dvb-frontends/dibx000_common.c   |  8 +++
 security/integrity/ima/ima_template.c  | 11 -
 9 files changed, 33 insertions(+), 55 deletions(-)

-- 
2.15.0

Re: [PATCH/RFC 1/2] v4l: v4l2-dev: Add infrastructure to protect device unplug race

[Kieran Bingham]

Hi Laurent,

On 16/11/17 12:32, Sakari Ailus wrote:
> Hi Laurent,
> 
> Thank you for the initiative to bring up and address the matter!

I concur - this looks like a good start towards managing the issue.

One potential thing spotted on top of Sakari's review inline below, of course I
suspect this was more of a prototype/consideration patch.

> On Thu, Nov 16, 2017 at 02:33:48AM +0200, Laurent Pinchart wrote:
>> Device unplug being asynchronous, it naturally races with operations
>> performed by userspace through ioctls or other file operations on video
>> device nodes.
>>
>> This leads to potential access to freed memory or to other resources
>> during device access if unplug occurs during device access. To solve
>> this, we need to wait until all device access completes when unplugging
>> the device, and block all further access when the device is being
>> unplugged.
>>
>> Three new functions are introduced. The video_device_enter() and
>> video_device_exit() functions must be used to mark entry and exit from
>> all code sections where the device can be accessed. The
> 
> I wonder if it'd help splitting this patch into two: one that introduces
> the mechanism and the other that uses it. Up to you.
> 
> Nevertheless, it'd be better to have other system calls covered as well.
> 
>> video_device_unplug() function is then used in the unplug handler to
>> mark the device as being unplugged and wait for all access to complete.
>>
>> As an example mark the ioctl handler as a device access section. Other
>> file operations need to be protected too, and blocking ioctls (such as
>> VIDIOC_DQBUF) need to be handled as well.
>>
>> Signed-off-by: Laurent Pinchart 
>> ---
>>  drivers/media/v4l2-core/v4l2-dev.c | 57 
>> ++
>>  include/media/v4l2-dev.h   | 47 +++
>>  2 files changed, 104 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-dev.c 
>> b/drivers/media/v4l2-core/v4l2-dev.c
>> index c647ba648805..c73c6d49e7cf 100644
>> --- a/drivers/media/v4l2-core/v4l2-dev.c
>> +++ b/drivers/media/v4l2-core/v4l2-dev.c
>> @@ -156,6 +156,52 @@ void video_device_release_empty(struct video_device 
>> *vdev)
>>  }
>>  EXPORT_SYMBOL(video_device_release_empty);
>>  
>> +int video_device_enter(struct video_device *vdev)
>> +{
>> +bool unplugged;
>> +
>> +spin_lock(>unplug_lock);
>> +unplugged = vdev->unplugged;
>> +if (!unplugged)
>> +vdev->access_refcount++;
>> +spin_unlock(>unplug_lock);
>> +
>> +return unplugged ? -ENODEV : 0;
>> +}
>> +EXPORT_SYMBOL_GPL(video_device_enter);
>> +
>> +void video_device_exit(struct video_device *vdev)
>> +{
>> +bool wake_up;
>> +
>> +spin_lock(>unplug_lock);
>> +WARN_ON(--vdev->access_refcount < 0);
>> +wake_up = vdev->access_refcount == 0;
>> +spin_unlock(>unplug_lock);
>> +
>> +if (wake_up)
>> +wake_up(>unplug_wait);
>> +}
>> +EXPORT_SYMBOL_GPL(video_device_exit);
> 
> Is there a need to export the two, i.e. wouldn't you only call them from
> the framework, or the same module?
> 
>> +
>> +void video_device_unplug(struct video_device *vdev)
>> +{
>> +bool unplug_blocked;
>> +
>> +spin_lock(>unplug_lock);
>> +unplug_blocked = vdev->access_refcount > 0;
>> +vdev->unplugged = true;
> 
> Shouldn't this be set to false in video_register_device()?
> 
>> +spin_unlock(>unplug_lock);
>> +
>> +if (!unplug_blocked)
>> +return;
> 
> Not necessary, wait_event_timeout() handles this already.
> 
>> +
>> +if (!wait_event_timeout(vdev->unplug_wait, !vdev->access_refcount,
>> +msecs_to_jiffies(15)))
>> +WARN(1, "Timeout waiting for device access to complete\n");
> 
> Why a timeout? Does this get somehow less problematic over time? :-)
> 
>> +}
>> +EXPORT_SYMBOL_GPL(video_device_unplug);
>> +
>>  static inline void video_get(struct video_device *vdev)
>>  {
>>  get_device(>dev);
>> @@ -351,6 +397,10 @@ static long v4l2_ioctl(struct file *filp, unsigned int 
>> cmd, unsigned long arg)
>>  struct video_device *vdev = video_devdata(filp);
>>  int ret = -ENODEV;
> 
> You could leave ret unassigned here.
> 
>>  
>> +ret = video_device_enter(vdev);
>> +if (ret < 0)
>> +return ret;
>> +
>>  if (vdev->fops->unlocked_ioctl) {
>>  struct mutex *lock = v4l2_ioctl_get_lock(vdev, cmd);
>>  
>> @@ -358,11 +408,14 @@ static long v4l2_ioctl(struct file *filp, unsigned int 
>> cmd, unsigned long arg)
>>  return -ERESTARTSYS;


It looks like that return -ERESTARTSYS might need a video_device_exit() too?


>>  if (video_is_registered(vdev))
>>  ret = vdev->fops->unlocked_ioctl(filp, cmd, arg);
>> +else
>> +ret = -ENODEV;
>>  if (lock)
>>  mutex_unlock(lock);
>>  } else
>>  ret = 

Re: [PATCH 01/10] video/hdmi: Allow "empty" HDMI infoframes

[Sharma, Shashank]

Regards

Shashank


On 11/13/2017 10:34 PM, Ville Syrjala wrote:

From: Ville Syrjälä 

HDMI 2.0 Appendix F suggest that we should keep sending the infoframe
when switching from 3D to 2D mode, even if the infoframe isn't strictly
necessary (ie. not needed to transmit the VIC or stereo information).
This is a workaround against some sinks that fail to realize that they
should switch from 3D to 2D mode when the source stop transmitting
the infoframe.

v2: Handle unpack() as well
 Pull the length calculation into a helper

Cc: Shashank Sharma 
Cc: Andrzej Hajda 
Cc: Thierry Reding 
Cc: Hans Verkuil 
Cc: linux-media@vger.kernel.org
Reviewed-by: Andrzej Hajda  #v1
Signed-off-by: Ville Syrjälä 
---
  drivers/video/hdmi.c | 51 +++
  1 file changed, 31 insertions(+), 20 deletions(-)

diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index 1cf907ecded4..111a0ab6280a 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -321,6 +321,17 @@ int hdmi_vendor_infoframe_init(struct 
hdmi_vendor_infoframe *frame)
  }
  EXPORT_SYMBOL(hdmi_vendor_infoframe_init);
  
+static int hdmi_vendor_infoframe_length(const struct hdmi_vendor_infoframe *frame)

+{
+   /* for side by side (half) we also need to provide 3D_Ext_Data */
+   if (frame->s3d_struct >= HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF)
+   return 6;
+   else if (frame->vic != 0 || frame->s3d_struct != 
HDMI_3D_STRUCTURE_INVALID)
+   return 5;
+   else
+   return 4;
+}
+
  /**
   * hdmi_vendor_infoframe_pack() - write a HDMI vendor infoframe to binary 
buffer
   * @frame: HDMI infoframe
@@ -341,19 +352,11 @@ ssize_t hdmi_vendor_infoframe_pack(struct 
hdmi_vendor_infoframe *frame,
u8 *ptr = buffer;
size_t length;
  
-	/* empty info frame */

-   if (frame->vic == 0 && frame->s3d_struct == HDMI_3D_STRUCTURE_INVALID)
-   return -EINVAL;
-
/* only one of those can be supplied */
if (frame->vic != 0 && frame->s3d_struct != HDMI_3D_STRUCTURE_INVALID)
return -EINVAL;
  
-	/* for side by side (half) we also need to provide 3D_Ext_Data */

-   if (frame->s3d_struct >= HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF)
-   frame->length = 6;
-   else
-   frame->length = 5;
+   frame->length = hdmi_vendor_infoframe_length(frame);
  
  	length = HDMI_INFOFRAME_HEADER_SIZE + frame->length;
  
@@ -372,14 +375,16 @@ ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,

ptr[5] = 0x0c;
ptr[6] = 0x00;
  
-	if (frame->vic) {

-   ptr[7] = 0x1 << 5;/* video format */
-   ptr[8] = frame->vic;
-   } else {
+   if (frame->s3d_struct != HDMI_3D_STRUCTURE_INVALID) {
ptr[7] = 0x2 << 5;/* video format */
ptr[8] = (frame->s3d_struct & 0xf) << 4;
if (frame->s3d_struct >= HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF)
ptr[9] = (frame->s3d_ext_data & 0xf) << 4;
+   } else if (frame->vic) {
Please correct me if I dint understand this properly, but for a HDMI 2.0 
sink + 3D transmission, wouldn't I be sending

HDMI 2.0 VIC = 94 as well as some valid s3d flag (like side by side) ?

- Shashank

+   ptr[7] = 0x1 << 5;/* video format */
+   ptr[8] = frame->vic;
+   } else {
+   ptr[7] = 0x0 << 5;/* video format */
}
  
  	hdmi_infoframe_set_checksum(buffer, length);

@@ -1165,7 +1170,7 @@ hdmi_vendor_any_infoframe_unpack(union 
hdmi_vendor_any_infoframe *frame,
  
  	if (ptr[0] != HDMI_INFOFRAME_TYPE_VENDOR ||

ptr[1] != 1 ||
-   (ptr[2] != 5 && ptr[2] != 6))
+   (ptr[2] != 4 && ptr[2] != 5 && ptr[2] != 6))
return -EINVAL;
  
  	length = ptr[2];

@@ -1193,16 +1198,22 @@ hdmi_vendor_any_infoframe_unpack(union 
hdmi_vendor_any_infoframe *frame,
  
  	hvf->length = length;
  
-	if (hdmi_video_format == 0x1) {

-   hvf->vic = ptr[4];
-   } else if (hdmi_video_format == 0x2) {
+   if (hdmi_video_format == 0x2) {
+   if (length != 5 && length != 6)
+   return -EINVAL;
hvf->s3d_struct = ptr[4] >> 4;
if (hvf->s3d_struct >= HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF) {
-   if (length == 6)
-   hvf->s3d_ext_data = ptr[5] >> 4;
-   else
+   if (length != 6)
return -EINVAL;
+   hvf->s3d_ext_data = ptr[5] >> 4;
}
+   } else if (hdmi_video_format == 0x1) {
+   if (length != 5)
+   return -EINVAL;
+   hvf->vic = 

Re: [PATCH 1/2] dt-bindings: media: rcar_vin: add device tree support for r8a774[35]

[Geert Uytterhoeven]

Hi Fabrizio,

On Thu, Nov 16, 2017 at 2:45 PM, Fabrizio Castro
 wrote:
>> Subject: Re: [PATCH 1/2] dt-bindings: media: rcar_vin: add device tree 
>> support for r8a774[35]
>>
>> On Thu, Nov 16, 2017 at 1:11 PM, Fabrizio Castro
>>  wrote:
>> > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
>> > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
>> > @@ -14,7 +14,10 @@ channel which can be either RGB, YUYV or BT656.
>> > - "renesas,vin-r8a7790" for the R8A7790 device
>> > - "renesas,vin-r8a7779" for the R8A7779 device
>> > - "renesas,vin-r8a7778" for the R8A7778 device
>> > -   - "renesas,rcar-gen2-vin" for a generic R-Car Gen2 compatible device.
>> > +   - "renesas,vin-r8a7745" for the R8A7745 device
>> > +   - "renesas,vin-r8a7743" for the R8A7743 device
>>
>> Please keep the list sorted by SoC part number.
>
> It is sorted, just in descending order. Do you want me to re-order the full 
> list in ascending order?

That may be a good idea, given the current order is non-standard and
counter-intuitive.

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

RE: [PATCH 1/2] dt-bindings: media: rcar_vin: add device tree support for r8a774[35]

[Fabrizio Castro]

Hello Geert,

thank you for your comment!

> Subject: Re: [PATCH 1/2] dt-bindings: media: rcar_vin: add device tree 
> support for r8a774[35]
>
> On Thu, Nov 16, 2017 at 1:11 PM, Fabrizio Castro
>  wrote:
> > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> > @@ -14,7 +14,10 @@ channel which can be either RGB, YUYV or BT656.
> > - "renesas,vin-r8a7790" for the R8A7790 device
> > - "renesas,vin-r8a7779" for the R8A7779 device
> > - "renesas,vin-r8a7778" for the R8A7778 device
> > -   - "renesas,rcar-gen2-vin" for a generic R-Car Gen2 compatible device.
> > +   - "renesas,vin-r8a7745" for the R8A7745 device
> > +   - "renesas,vin-r8a7743" for the R8A7743 device
>
> Please keep the list sorted by SoC part number.
>

It is sorted, just in descending order. Do you want me to re-order the full 
list in ascending order?

Thanks,
Fab





Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, 
Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered 
No. 04586709.

Re: [linux-sunxi] Cedrus driver

[Giulio Benetti]

Hi,

Il 16/11/2017 14:39, Maxime Ripard ha scritto:

On Thu, Nov 16, 2017 at 02:17:08PM +0100, Giulio Benetti wrote:

Hi Hans,

Il 16/11/2017 14:12, Hans Verkuil ha scritto:

On 16/11/17 13:57, Giulio Benetti wrote:

Il 16/11/2017 13:53, Maxime Ripard ha scritto:

On Thu, Nov 16, 2017 at 01:30:52PM +0100, Giulio Benetti wrote:

On Thu, Nov 16, 2017 at 11:37:30AM +0100, Giulio Benetti wrote:

Il 16/11/2017 11:31, Andreas Baierl ha scritto:

Am 16.11.2017 um 11:13 schrieb Giulio Benetti:

Hello,


Hello,

I'm wondering why cedrus
https://github.com/FlorentRevest/linux-sunxi-cedrus has never been
merged with linux-sunxi sunxi-next.


Because it is not ready to be merged. It depends on the v4l2 request
API, which was not merged and which is re-worked atm.
Also, sunxi-cedrus itself is not in a finished state and is not as
feature-complete to be merged. Anyway it might be something for
staging... Has there been a [RFC] on the mailing list at all?


Where can I find a list of TODOs to get it ready to be merged?


Assuming that the request API is in, we'd need to:
  - Finish the MPEG4 support
  - Work on more useful codecs (H264 comes to my mind)
  - Implement the DRM planes support for the custom frame format
  - Implement the DRM planes support for scaling
  - Test it on more SoCs

Or something along those lines.


Lot of work to do


Well... If it was fast and easy it would have been done already :)


:))




I see it seems to be dead, no commit in 1 year.


Yes, because the author did this during an internship, which ended ...
Afaik nobody picked up his work yet.


That's not entirely true. Some work has been done by Thomas (in CC),
especially on the display engine side, but last time we talked his
work was not really upstreamable.

We will also resume that effort starting next march.


Is it possible a preview on a separate Reporitory to start working on now?
Expecially to start porting everything done by FlorentRevest to mainline,
admitted you've not already done.


I'm not sure what you're asking for. Florent's work *was* on mainline.


and then they took it off because it was unmantained?
You've spoken about Thomas(in CC) not ready,
maybe I could help on that if it's public to accelerate.
If I'm able to of course, this is my primary concern.

Otherwise, in which way can I help improving it to make it accept to 
linux-sunxi?
Starting from Florent's work and porting it to sunxi-next to begin?
And after that adding all features you've listed?
Tell me what I can do(I repeat, if I'm able to).


The bottleneck is that the Request API is not mainlined. We restarted work
on it after a meeting a few weeks back where we all agreed on the roadmap
so hopefully it will go into mainline Q1 or Q2 next year.

That said, you can use Florent's patch series for further development.
It should be relatively easy to convert it to the final version of the
Request API. Just note that the public API of the final Request API will
be somewhat different from the old version Florent's patch series is using.


So I'm going to try soon to :
1) adapt that patchset to sunxi-next
2) add A20 support
3) add A33 support
4) after mainlined APIs, merge


That sounds good. Thomas already has the support for the A20, and as I
was saying, there is someone that is going to work full time on this
in a couple monthes on our side.

I'll set up a git repo on github so that we can collaborate until the
request API is ready.


Great!
Write me when you've got news.

Thank you very much!



Maxime




--
Giulio Benetti
R Manager &
Advanced Research

MICRONOVA SRL
Sede: Via A. Niedda 3 - 35010 Vigonza (PD)
Tel. 049/8931563 - Fax 049/8931346
Cod.Fiscale - P.IVA 02663420285
Capitale Sociale € 26.000 i.v.
Iscritta al Reg. Imprese di Padova N. 02663420285
Numero R.E.A. 258642

[PATCH v1 2/4] media: ov5640: check chip id

[Hugues Fruchet]

Verify that chip identifier is correct before starting streaming

Signed-off-by: Hugues Fruchet 
---
 drivers/media/i2c/ov5640.c | 30 +-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 61071f5..a576d11 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -34,7 +34,8 @@
 
 #define OV5640_DEFAULT_SLAVE_ID 0x3c
 
-#define OV5640_REG_CHIP_ID 0x300a
+#define OV5640_REG_CHIP_ID_HIGH0x300a
+#define OV5640_REG_CHIP_ID_LOW 0x300b
 #define OV5640_REG_PAD_OUTPUT000x3019
 #define OV5640_REG_SC_PLL_CTRL00x3034
 #define OV5640_REG_SC_PLL_CTRL10x3035
@@ -926,6 +927,29 @@ static int ov5640_load_regs(struct ov5640_dev *sensor,
return ret;
 }
 
+static int ov5640_check_chip_id(struct ov5640_dev *sensor)
+{
+   struct i2c_client *client = sensor->i2c_client;
+   int ret;
+   u8 chip_id_h, chip_id_l;
+
+   ret = ov5640_read_reg(sensor, OV5640_REG_CHIP_ID_HIGH, _id_h);
+   if (ret)
+   return ret;
+
+   ret = ov5640_read_reg(sensor, OV5640_REG_CHIP_ID_LOW, _id_l);
+   if (ret)
+   return ret;
+
+   if (!(chip_id_h == 0x56 && chip_id_l == 0x40)) {
+   dev_err(>dev, "%s: wrong chip identifier, expected 
0x5640, got 0x%x%x\n",
+   __func__, chip_id_h, chip_id_l);
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
 /* read exposure, in number of line periods */
 static int ov5640_get_exposure(struct ov5640_dev *sensor)
 {
@@ -1562,6 +1586,10 @@ static int ov5640_set_power(struct ov5640_dev *sensor, 
bool on)
ov5640_reset(sensor);
ov5640_power(sensor, true);
 
+   ret = ov5640_check_chip_id(sensor);
+   if (ret)
+   goto power_off;
+
ret = ov5640_init_slave_id(sensor);
if (ret)
goto power_off;
-- 
1.9.1

[PATCH v1 0/4] Add OV5640 parallel interface and RGB565/YUYV support

[Hugues Fruchet]

Enhance OV5640 CSI driver to support also DVP parallel interface.
Add RGB565 (LE & BE) and YUV422 YUYV format in addition to existing
YUV422 UYVY format.
Some other improvements on chip identifier check and removal
of warnings in powering phase around gpio handling.

Hugues Fruchet (4):
  media: ov5640: switch to gpiod_set_value_cansleep()
  media: ov5640: check chip id
  media: ov5640: add support of DVP parallel interface
  media: ov5640: add support of RGB565 and YUYV formats

 drivers/media/i2c/ov5640.c | 229 +++--
 1 file changed, 198 insertions(+), 31 deletions(-)

-- 
1.9.1

[PATCH v1 3/4] media: ov5640: add support of DVP parallel interface

[Hugues Fruchet]

Add support of DVP parallel mode in addition of
existing MIPI CSI mode. The choice between two modes
and configuration is made through device tree.

Signed-off-by: Hugues Fruchet 
---
 drivers/media/i2c/ov5640.c | 112 +
 1 file changed, 94 insertions(+), 18 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index a576d11..fb519ad 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -34,14 +34,20 @@
 
 #define OV5640_DEFAULT_SLAVE_ID 0x3c
 
+#define OV5640_REG_SYS_CTRL0   0x3008
 #define OV5640_REG_CHIP_ID_HIGH0x300a
 #define OV5640_REG_CHIP_ID_LOW 0x300b
+#define OV5640_REG_IO_MIPI_CTRL00  0x300e
+#define OV5640_REG_PAD_OUTPUT_ENABLE01 0x3017
+#define OV5640_REG_PAD_OUTPUT_ENABLE02 0x3018
 #define OV5640_REG_PAD_OUTPUT000x3019
+#define OV5640_REG_SYSTEM_CONTROL1 0x302e
 #define OV5640_REG_SC_PLL_CTRL00x3034
 #define OV5640_REG_SC_PLL_CTRL10x3035
 #define OV5640_REG_SC_PLL_CTRL20x3036
 #define OV5640_REG_SC_PLL_CTRL30x3037
 #define OV5640_REG_SLAVE_ID0x3100
+#define OV5640_REG_SCCB_SYS_CTRL1  0x3103
 #define OV5640_REG_SYS_ROOT_DIVIDER0x3108
 #define OV5640_REG_AWB_R_GAIN  0x3400
 #define OV5640_REG_AWB_G_GAIN  0x3402
@@ -1006,7 +1012,71 @@ static int ov5640_get_gain(struct ov5640_dev *sensor)
return gain & 0x3ff;
 }
 
-static int ov5640_set_stream(struct ov5640_dev *sensor, bool on)
+static int ov5640_set_stream_dvp(struct ov5640_dev *sensor)
+{
+   int ret;
+
+   /*
+* MIPI CONTROL 00
+* 6:mipi lane debug
+* 4:PWDN PHY TX
+* 3:PWDN PHY RX
+*/
+   ret = ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00, 0x58);
+   if (ret)
+   return ret;
+
+   /* SYSTEM CONTROL, not recommended... */
+   ret = ov5640_write_reg(sensor, OV5640_REG_SYSTEM_CONTROL1, 0x00);
+   if (ret)
+   return ret;
+
+   /* SCCB SYSTEM CTRL1 1:system input clk from PLL, 0:debug enable */
+   ret = ov5640_write_reg(sensor, OV5640_REG_SCCB_SYS_CTRL1, 0x03);
+   if (ret)
+   return ret;
+
+   /* SYSTEM CTRL0 1:debug enable */
+   ret = ov5640_write_reg(sensor, OV5640_REG_SYS_CTRL0, 0x02);
+   if (ret)
+   return ret;
+
+   /*
+* SC PLL CONTRL1 0
+* - [7..4]:System clock divider = 4
+* - [3..0]:MIPI PCLK/SERCLK divider = 1
+*/
+   ret = ov5640_write_reg(sensor, OV5640_REG_SC_PLL_CTRL1, 0x41);
+   if (ret)
+   return ret;
+
+   /* OV5640_REG_SC_PLL_CTRL2 [7:0]: PLL multiplier (4~252) = 0x60 */
+   ret = ov5640_write_reg(sensor, OV5640_REG_SC_PLL_CTRL2, 0x60);
+   if (ret)
+   return ret;
+
+   /*
+* PAD OUTPUT ENABLE 01:
+* - 6: VSYNC output enable
+* - 5: HREF output enable
+* - 4: PCLK output enable
+* - [3:0]: D[9:6] output enable
+*/
+   ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT_ENABLE01, 0x7f);
+   if (ret)
+   return ret;
+
+   /*
+* PAD OUTPUT ENABLE 02:
+* - [7:4]: D[5:2] output enable
+*  0:1 are unused with 8 bits
+*  parallel mode (8 bits output
+*  are on D[9:2])
+*/
+   return ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT_ENABLE02, 0xf0);
+}
+
+static int ov5640_set_stream_mipi(struct ov5640_dev *sensor, bool on)
 {
int ret;
 
@@ -1598,17 +1668,24 @@ static int ov5640_set_power(struct ov5640_dev *sensor, 
bool on)
if (ret)
goto power_off;
 
-   /*
-* start streaming briefly followed by stream off in
-* order to coax the clock lane into LP-11 state.
-*/
-   ret = ov5640_set_stream(sensor, true);
-   if (ret)
-   goto power_off;
-   usleep_range(1000, 2000);
-   ret = ov5640_set_stream(sensor, false);
-   if (ret)
-   goto power_off;
+   if (sensor->ep.bus_type == V4L2_MBUS_CSI2) {
+   /*
+* start streaming briefly followed by stream off in
+* order to coax the clock lane into LP-11 state.
+*/
+   ret = ov5640_set_stream_mipi(sensor, true);
+   if (ret)
+   goto power_off;
+   usleep_range(1000, 2000);
+   ret = ov5640_set_stream_mipi(sensor, false);
+   if (ret)
+   goto power_off;
+   } else {
+   ret = 

[PATCH v1 1/4] media: ov5640: switch to gpiod_set_value_cansleep()

[Hugues Fruchet]

Switch gpiod_set_value to gpiod_set_value_cansleep to avoid
warnings when powering sensor.

Signed-off-by: Hugues Fruchet 
---
 drivers/media/i2c/ov5640.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index c89ed66..61071f5 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -1524,7 +1524,7 @@ static int ov5640_restore_mode(struct ov5640_dev *sensor)
 
 static void ov5640_power(struct ov5640_dev *sensor, bool enable)
 {
-   gpiod_set_value(sensor->pwdn_gpio, enable ? 0 : 1);
+   gpiod_set_value_cansleep(sensor->pwdn_gpio, enable ? 0 : 1);
 }
 
 static void ov5640_reset(struct ov5640_dev *sensor)
@@ -1532,7 +1532,7 @@ static void ov5640_reset(struct ov5640_dev *sensor)
if (!sensor->reset_gpio)
return;
 
-   gpiod_set_value(sensor->reset_gpio, 0);
+   gpiod_set_value_cansleep(sensor->reset_gpio, 0);
 
/* camera power cycle */
ov5640_power(sensor, false);
@@ -1540,10 +1540,10 @@ static void ov5640_reset(struct ov5640_dev *sensor)
ov5640_power(sensor, true);
usleep_range(5000, 1);
 
-   gpiod_set_value(sensor->reset_gpio, 1);
+   gpiod_set_value_cansleep(sensor->reset_gpio, 1);
usleep_range(1000, 2000);
 
-   gpiod_set_value(sensor->reset_gpio, 0);
+   gpiod_set_value_cansleep(sensor->reset_gpio, 0);
usleep_range(5000, 1);
 }
 
-- 
1.9.1

[PATCH v1 4/4] media: ov5640: add support of RGB565 and YUYV formats

[Hugues Fruchet]

Add RGB565 (LE & BE) and YUV422 YUYV format in addition
to existing YUV422 UYVY format.

Signed-off-by: Hugues Fruchet 
---
 drivers/media/i2c/ov5640.c | 79 +-
 1 file changed, 71 insertions(+), 8 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index fb519ad..086a451 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -77,8 +77,10 @@
 #define OV5640_REG_HZ5060_CTRL01   0x3c01
 #define OV5640_REG_SIGMADELTA_CTRL0C   0x3c0c
 #define OV5640_REG_FRAME_CTRL010x4202
+#define OV5640_REG_FORMAT_CONTROL000x4300
 #define OV5640_REG_MIPI_CTRL00 0x4800
 #define OV5640_REG_DEBUG_MODE  0x4814
+#define OV5640_REG_ISP_FORMAT_MUX_CTRL 0x501f
 #define OV5640_REG_PRE_ISP_TEST_SET1   0x503d
 #define OV5640_REG_SDE_CTRL0   0x5580
 #define OV5640_REG_SDE_CTRL1   0x5581
@@ -106,6 +108,18 @@ enum ov5640_frame_rate {
OV5640_NUM_FRAMERATES,
 };
 
+struct ov5640_pixfmt {
+   u32 code;
+   u32 colorspace;
+};
+
+static const struct ov5640_pixfmt ov5640_formats[] = {
+   { MEDIA_BUS_FMT_UYVY8_2X8, V4L2_COLORSPACE_SRGB, },
+   { MEDIA_BUS_FMT_YUYV8_2X8, V4L2_COLORSPACE_SRGB, },
+   { MEDIA_BUS_FMT_RGB565_2X8_LE, V4L2_COLORSPACE_SRGB, },
+   { MEDIA_BUS_FMT_RGB565_2X8_BE, V4L2_COLORSPACE_SRGB, },
+};
+
 /*
  * FIXME: remove this when a subdev API becomes available
  * to set the MIPI CSI-2 virtual channel.
@@ -1798,17 +1812,23 @@ static int ov5640_try_fmt_internal(struct v4l2_subdev 
*sd,
 {
struct ov5640_dev *sensor = to_ov5640_dev(sd);
const struct ov5640_mode_info *mode;
+   int i;
 
mode = ov5640_find_mode(sensor, fr, fmt->width, fmt->height, true);
if (!mode)
return -EINVAL;
-
fmt->width = mode->width;
fmt->height = mode->height;
-   fmt->code = sensor->fmt.code;
 
if (new_mode)
*new_mode = mode;
+
+   for (i = 0; i < ARRAY_SIZE(ov5640_formats); i++)
+   if (ov5640_formats[i].code == fmt->code)
+   break;
+   if (i >= ARRAY_SIZE(ov5640_formats))
+   fmt->code = ov5640_formats[0].code;
+
return 0;
 }
 
@@ -1851,6 +1871,49 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
return ret;
 }
 
+static int ov5640_set_framefmt(struct ov5640_dev *sensor,
+  struct v4l2_mbus_framefmt *format)
+{
+   int ret = 0;
+   bool is_rgb = false;
+   u8 val;
+
+   switch (format->code) {
+   case MEDIA_BUS_FMT_UYVY8_2X8:
+   /* YUV422, UYVY */
+   val = 0x3f;
+   break;
+   case MEDIA_BUS_FMT_YUYV8_2X8:
+   /* YUV422, YUYV */
+   val = 0x30;
+   break;
+   case MEDIA_BUS_FMT_RGB565_2X8_LE:
+   /* RGB565 {g[2:0],b[4:0]},{r[4:0],g[5:3]} */
+   val = 0x6F;
+   is_rgb = true;
+   break;
+   case MEDIA_BUS_FMT_RGB565_2X8_BE:
+   /* RGB565 {r[4:0],g[5:3]},{g[2:0],b[4:0]} */
+   val = 0x61;
+   is_rgb = true;
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   /* FORMAT CONTROL00: YUV and RGB formatting */
+   ret = ov5640_write_reg(sensor, OV5640_REG_FORMAT_CONTROL00, val);
+   if (ret)
+   return ret;
+
+   /* FORMAT MUX CONTROL: ISP YUV or RGB */
+   ret = ov5640_write_reg(sensor, OV5640_REG_ISP_FORMAT_MUX_CTRL,
+  is_rgb ? 0x01 : 0x00);
+   if (ret)
+   return ret;
+
+   return ret;
+}
 
 /*
  * Sensor Controls.
@@ -2236,15 +2299,12 @@ static int ov5640_enum_mbus_code(struct v4l2_subdev *sd,
  struct v4l2_subdev_pad_config *cfg,
  struct v4l2_subdev_mbus_code_enum *code)
 {
-   struct ov5640_dev *sensor = to_ov5640_dev(sd);
-
if (code->pad != 0)
return -EINVAL;
-   if (code->index != 0)
+   if (code->index >= ARRAY_SIZE(ov5640_formats))
return -EINVAL;
 
-   code->code = sensor->fmt.code;
-
+   code->code = ov5640_formats[code->index].code;
return 0;
 }
 
@@ -2254,12 +2314,15 @@ static int ov5640_s_stream(struct v4l2_subdev *sd, int 
enable)
int ret = 0;
 
mutex_lock(>lock);
-
if (sensor->streaming == !enable) {
if (enable && sensor->pending_mode_change) {
ret = ov5640_set_mode(sensor, sensor->current_mode);
if (ret)
goto out;
+
+   ret = ov5640_set_framefmt(sensor, >fmt);
+   if (ret)
+   goto out;
}
 
if (sensor->ep.bus_type == V4L2_MBUS_CSI2)
-- 
1.9.1

Re: [linux-sunxi] Cedrus driver

[Hans Verkuil]

On 16/11/17 14:17, Giulio Benetti wrote:
> Hi Hans,
> 
> Il 16/11/2017 14:12, Hans Verkuil ha scritto:
>> On 16/11/17 13:57, Giulio Benetti wrote:
>>> Il 16/11/2017 13:53, Maxime Ripard ha scritto:
 On Thu, Nov 16, 2017 at 01:30:52PM +0100, Giulio Benetti wrote:
>> On Thu, Nov 16, 2017 at 11:37:30AM +0100, Giulio Benetti wrote:
>>> Il 16/11/2017 11:31, Andreas Baierl ha scritto:
 Am 16.11.2017 um 11:13 schrieb Giulio Benetti:
> Hello,
>
 Hello,
> I'm wondering why cedrus
> https://github.com/FlorentRevest/linux-sunxi-cedrus has never been
> merged with linux-sunxi sunxi-next.
>
 Because it is not ready to be merged. It depends on the v4l2 request
 API, which was not merged and which is re-worked atm.
 Also, sunxi-cedrus itself is not in a finished state and is not as
 feature-complete to be merged. Anyway it might be something for
 staging... Has there been a [RFC] on the mailing list at all?
>>>
>>> Where can I find a list of TODOs to get it ready to be merged?
>>
>> Assuming that the request API is in, we'd need to:
>>  - Finish the MPEG4 support
>>  - Work on more useful codecs (H264 comes to my mind)
>>  - Implement the DRM planes support for the custom frame format
>>  - Implement the DRM planes support for scaling
>>  - Test it on more SoCs
>>
>> Or something along those lines.
>
> Lot of work to do

 Well... If it was fast and easy it would have been done already :)
>>>
>>> :))
>>>

> I see it seems to be dead, no commit in 1 year.

 Yes, because the author did this during an internship, which ended ...
 Afaik nobody picked up his work yet.
>>
>> That's not entirely true. Some work has been done by Thomas (in CC),
>> especially on the display engine side, but last time we talked his
>> work was not really upstreamable.
>>
>> We will also resume that effort starting next march.
>
> Is it possible a preview on a separate Reporitory to start working on now?
> Expecially to start porting everything done by FlorentRevest to mainline,
> admitted you've not already done.

 I'm not sure what you're asking for. Florent's work *was* on mainline.
>>>
>>> and then they took it off because it was unmantained?
>>> You've spoken about Thomas(in CC) not ready,
>>> maybe I could help on that if it's public to accelerate.
>>> If I'm able to of course, this is my primary concern.
>>>
>>> Otherwise, in which way can I help improving it to make it accept to 
>>> linux-sunxi?
>>> Starting from Florent's work and porting it to sunxi-next to begin?
>>> And after that adding all features you've listed?
>>> Tell me what I can do(I repeat, if I'm able to).
>>
>> The bottleneck is that the Request API is not mainlined. We restarted work
>> on it after a meeting a few weeks back where we all agreed on the roadmap
>> so hopefully it will go into mainline Q1 or Q2 next year.
>>
>> That said, you can use Florent's patch series for further development.
>> It should be relatively easy to convert it to the final version of the
>> Request API. Just note that the public API of the final Request API will
>> be somewhat different from the old version Florent's patch series is using.
> 
> So I'm going to try soon to :
> 1) adapt that patchset to sunxi-next
> 2) add A20 support
> 3) add A33 support
> 4) after mainlined APIs, merge
> 
> Alright?

Sounds reasonable.

Regards,

Hans

> 
> Regards
> 
>>
>> Regards,
>>
>> Hans
>>
>>>

 Maxime

>>>
>>> Thank you
>>>
>>
> 
> 

RE: [PATCH 1/2] dt-bindings: media: rcar_vin: add device tree support for r8a774[35]

[Fabrizio Castro]

Hello Geert,

thank you for your comment!

> > Add compatible strings for r8a7743 and r8a7745. No driver change
> > change is needed as "renesas,rcar-gen2-vin" will activate the right
>
> double "change"

ps, do you think a v2 is in order?

Thanks,
Fab



Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, 
Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered 
No. 04586709.

Re: [linux-sunxi] Cedrus driver

[Maxime Ripard]

On Thu, Nov 16, 2017 at 02:17:08PM +0100, Giulio Benetti wrote:
> Hi Hans,
> 
> Il 16/11/2017 14:12, Hans Verkuil ha scritto:
> > On 16/11/17 13:57, Giulio Benetti wrote:
> > > Il 16/11/2017 13:53, Maxime Ripard ha scritto:
> > > > On Thu, Nov 16, 2017 at 01:30:52PM +0100, Giulio Benetti wrote:
> > > > > > On Thu, Nov 16, 2017 at 11:37:30AM +0100, Giulio Benetti wrote:
> > > > > > > Il 16/11/2017 11:31, Andreas Baierl ha scritto:
> > > > > > > > Am 16.11.2017 um 11:13 schrieb Giulio Benetti:
> > > > > > > > > Hello,
> > > > > > > > > 
> > > > > > > > Hello,
> > > > > > > > > I'm wondering why cedrus
> > > > > > > > > https://github.com/FlorentRevest/linux-sunxi-cedrus has never 
> > > > > > > > > been
> > > > > > > > > merged with linux-sunxi sunxi-next.
> > > > > > > > > 
> > > > > > > > Because it is not ready to be merged. It depends on the v4l2 
> > > > > > > > request
> > > > > > > > API, which was not merged and which is re-worked atm.
> > > > > > > > Also, sunxi-cedrus itself is not in a finished state and is not 
> > > > > > > > as
> > > > > > > > feature-complete to be merged. Anyway it might be something for
> > > > > > > > staging... Has there been a [RFC] on the mailing list at all?
> > > > > > > 
> > > > > > > Where can I find a list of TODOs to get it ready to be merged?
> > > > > > 
> > > > > > Assuming that the request API is in, we'd need to:
> > > > > >  - Finish the MPEG4 support
> > > > > >  - Work on more useful codecs (H264 comes to my mind)
> > > > > >  - Implement the DRM planes support for the custom frame format
> > > > > >  - Implement the DRM planes support for scaling
> > > > > >  - Test it on more SoCs
> > > > > > 
> > > > > > Or something along those lines.
> > > > > 
> > > > > Lot of work to do
> > > > 
> > > > Well... If it was fast and easy it would have been done already :)
> > > 
> > > :))
> > > 
> > > > 
> > > > > > > > > I see it seems to be dead, no commit in 1 year.
> > > > > > > > 
> > > > > > > > Yes, because the author did this during an internship, which 
> > > > > > > > ended ...
> > > > > > > > Afaik nobody picked up his work yet.
> > > > > > 
> > > > > > That's not entirely true. Some work has been done by Thomas (in CC),
> > > > > > especially on the display engine side, but last time we talked his
> > > > > > work was not really upstreamable.
> > > > > > 
> > > > > > We will also resume that effort starting next march.
> > > > > 
> > > > > Is it possible a preview on a separate Reporitory to start working on 
> > > > > now?
> > > > > Expecially to start porting everything done by FlorentRevest to 
> > > > > mainline,
> > > > > admitted you've not already done.
> > > > 
> > > > I'm not sure what you're asking for. Florent's work *was* on mainline.
> > > 
> > > and then they took it off because it was unmantained?
> > > You've spoken about Thomas(in CC) not ready,
> > > maybe I could help on that if it's public to accelerate.
> > > If I'm able to of course, this is my primary concern.
> > > 
> > > Otherwise, in which way can I help improving it to make it accept to 
> > > linux-sunxi?
> > > Starting from Florent's work and porting it to sunxi-next to begin?
> > > And after that adding all features you've listed?
> > > Tell me what I can do(I repeat, if I'm able to).
> > 
> > The bottleneck is that the Request API is not mainlined. We restarted work
> > on it after a meeting a few weeks back where we all agreed on the roadmap
> > so hopefully it will go into mainline Q1 or Q2 next year.
> > 
> > That said, you can use Florent's patch series for further development.
> > It should be relatively easy to convert it to the final version of the
> > Request API. Just note that the public API of the final Request API will
> > be somewhat different from the old version Florent's patch series is using.
> 
> So I'm going to try soon to :
> 1) adapt that patchset to sunxi-next
> 2) add A20 support
> 3) add A33 support
> 4) after mainlined APIs, merge

That sounds good. Thomas already has the support for the A20, and as I
was saying, there is someone that is going to work full time on this
in a couple monthes on our side.

I'll set up a git repo on github so that we can collaborate until the
request API is ready.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH 1/2] dt-bindings: media: rcar_vin: add device tree support for r8a774[35]

[Geert Uytterhoeven]

On Thu, Nov 16, 2017 at 1:11 PM, Fabrizio Castro
 wrote:
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -14,7 +14,10 @@ channel which can be either RGB, YUYV or BT656.
> - "renesas,vin-r8a7790" for the R8A7790 device
> - "renesas,vin-r8a7779" for the R8A7779 device
> - "renesas,vin-r8a7778" for the R8A7778 device
> -   - "renesas,rcar-gen2-vin" for a generic R-Car Gen2 compatible device.
> +   - "renesas,vin-r8a7745" for the R8A7745 device
> +   - "renesas,vin-r8a7743" for the R8A7743 device

Please keep the list sorted by SoC part number.

> +   - "renesas,rcar-gen2-vin" for a generic R-Car Gen2 or RZ/G1 compatible
> + device.
> - "renesas,rcar-gen3-vin" for a generic R-Car Gen3 compatible device.
>
> When compatible with the generic version nodes must list the

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH 1/2] dt-bindings: media: rcar_vin: add device tree support for r8a774[35]

[Geert Uytterhoeven]

On Thu, Nov 16, 2017 at 1:11 PM, Fabrizio Castro
 wrote:
> Add compatible strings for r8a7743 and r8a7745. No driver change
> change is needed as "renesas,rcar-gen2-vin" will activate the right

double "change"

> code. However, it is good practice to document compatible strings
> for the specific SoC as this allows SoC specific changes to the
> driver if needed, in addition to document SoC support and therefore
> allow checkpatch.pl to validate compatible string values.
>
> Signed-off-by: Fabrizio Castro 
> Reviewed-by: Biju Das 

Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [linux-sunxi] Cedrus driver

[Giulio Benetti]

Hi Hans,

Il 16/11/2017 14:12, Hans Verkuil ha scritto:

On 16/11/17 13:57, Giulio Benetti wrote:

Il 16/11/2017 13:53, Maxime Ripard ha scritto:

On Thu, Nov 16, 2017 at 01:30:52PM +0100, Giulio Benetti wrote:

On Thu, Nov 16, 2017 at 11:37:30AM +0100, Giulio Benetti wrote:

Il 16/11/2017 11:31, Andreas Baierl ha scritto:

Am 16.11.2017 um 11:13 schrieb Giulio Benetti:

Hello,


Hello,

I'm wondering why cedrus
https://github.com/FlorentRevest/linux-sunxi-cedrus has never been
merged with linux-sunxi sunxi-next.


Because it is not ready to be merged. It depends on the v4l2 request
API, which was not merged and which is re-worked atm.
Also, sunxi-cedrus itself is not in a finished state and is not as
feature-complete to be merged. Anyway it might be something for
staging... Has there been a [RFC] on the mailing list at all?


Where can I find a list of TODOs to get it ready to be merged?


Assuming that the request API is in, we'd need to:
 - Finish the MPEG4 support
 - Work on more useful codecs (H264 comes to my mind)
 - Implement the DRM planes support for the custom frame format
 - Implement the DRM planes support for scaling
 - Test it on more SoCs

Or something along those lines.


Lot of work to do


Well... If it was fast and easy it would have been done already :)


:))




I see it seems to be dead, no commit in 1 year.


Yes, because the author did this during an internship, which ended ...
Afaik nobody picked up his work yet.


That's not entirely true. Some work has been done by Thomas (in CC),
especially on the display engine side, but last time we talked his
work was not really upstreamable.

We will also resume that effort starting next march.


Is it possible a preview on a separate Reporitory to start working on now?
Expecially to start porting everything done by FlorentRevest to mainline,
admitted you've not already done.


I'm not sure what you're asking for. Florent's work *was* on mainline.


and then they took it off because it was unmantained?
You've spoken about Thomas(in CC) not ready,
maybe I could help on that if it's public to accelerate.
If I'm able to of course, this is my primary concern.

Otherwise, in which way can I help improving it to make it accept to 
linux-sunxi?
Starting from Florent's work and porting it to sunxi-next to begin?
And after that adding all features you've listed?
Tell me what I can do(I repeat, if I'm able to).


The bottleneck is that the Request API is not mainlined. We restarted work
on it after a meeting a few weeks back where we all agreed on the roadmap
so hopefully it will go into mainline Q1 or Q2 next year.

That said, you can use Florent's patch series for further development.
It should be relatively easy to convert it to the final version of the
Request API. Just note that the public API of the final Request API will
be somewhat different from the old version Florent's patch series is using.


So I'm going to try soon to :
1) adapt that patchset to sunxi-next
2) add A20 support
3) add A33 support
4) after mainlined APIs, merge

Alright?

Regards



Regards,

Hans





Maxime



Thank you






--
Giulio Benetti
R Manager &
Advanced Research

MICRONOVA SRL
Sede: Via A. Niedda 3 - 35010 Vigonza (PD)
Tel. 049/8931563 - Fax 049/8931346
Cod.Fiscale - P.IVA 02663420285
Capitale Sociale € 26.000 i.v.
Iscritta al Reg. Imprese di Padova N. 02663420285
Numero R.E.A. 258642

Re: [linux-sunxi] Cedrus driver

[Hans Verkuil]

On 16/11/17 13:57, Giulio Benetti wrote:
> Il 16/11/2017 13:53, Maxime Ripard ha scritto:
>> On Thu, Nov 16, 2017 at 01:30:52PM +0100, Giulio Benetti wrote:
 On Thu, Nov 16, 2017 at 11:37:30AM +0100, Giulio Benetti wrote:
> Il 16/11/2017 11:31, Andreas Baierl ha scritto:
>> Am 16.11.2017 um 11:13 schrieb Giulio Benetti:
>>> Hello,
>>>
>> Hello,
>>> I'm wondering why cedrus
>>> https://github.com/FlorentRevest/linux-sunxi-cedrus has never been
>>> merged with linux-sunxi sunxi-next.
>>>
>> Because it is not ready to be merged. It depends on the v4l2 request
>> API, which was not merged and which is re-worked atm.
>> Also, sunxi-cedrus itself is not in a finished state and is not as
>> feature-complete to be merged. Anyway it might be something for
>> staging... Has there been a [RFC] on the mailing list at all?
>
> Where can I find a list of TODOs to get it ready to be merged?

 Assuming that the request API is in, we'd need to:
 - Finish the MPEG4 support
 - Work on more useful codecs (H264 comes to my mind)
 - Implement the DRM planes support for the custom frame format
 - Implement the DRM planes support for scaling
 - Test it on more SoCs

 Or something along those lines.
>>>
>>> Lot of work to do
>>
>> Well... If it was fast and easy it would have been done already :)
> 
> :))
> 
>>
>>> I see it seems to be dead, no commit in 1 year.
>>
>> Yes, because the author did this during an internship, which ended ...
>> Afaik nobody picked up his work yet.

 That's not entirely true. Some work has been done by Thomas (in CC),
 especially on the display engine side, but last time we talked his
 work was not really upstreamable.

 We will also resume that effort starting next march.
>>>
>>> Is it possible a preview on a separate Reporitory to start working on now?
>>> Expecially to start porting everything done by FlorentRevest to mainline,
>>> admitted you've not already done.
>>
>> I'm not sure what you're asking for. Florent's work *was* on mainline.
> 
> and then they took it off because it was unmantained?
> You've spoken about Thomas(in CC) not ready,
> maybe I could help on that if it's public to accelerate.
> If I'm able to of course, this is my primary concern.
> 
> Otherwise, in which way can I help improving it to make it accept to 
> linux-sunxi?
> Starting from Florent's work and porting it to sunxi-next to begin?
> And after that adding all features you've listed?
> Tell me what I can do(I repeat, if I'm able to).

The bottleneck is that the Request API is not mainlined. We restarted work
on it after a meeting a few weeks back where we all agreed on the roadmap
so hopefully it will go into mainline Q1 or Q2 next year.

That said, you can use Florent's patch series for further development.
It should be relatively easy to convert it to the final version of the
Request API. Just note that the public API of the final Request API will
be somewhat different from the old version Florent's patch series is using.

Regards,

Hans

> 
>>
>> Maxime
>>
> 
> Thank you
> 

Re: [linux-sunxi] Cedrus driver

[Giulio Benetti]

Il 16/11/2017 13:53, Maxime Ripard ha scritto:

On Thu, Nov 16, 2017 at 01:30:52PM +0100, Giulio Benetti wrote:

On Thu, Nov 16, 2017 at 11:37:30AM +0100, Giulio Benetti wrote:

Il 16/11/2017 11:31, Andreas Baierl ha scritto:

Am 16.11.2017 um 11:13 schrieb Giulio Benetti:

Hello,


Hello,

I'm wondering why cedrus
https://github.com/FlorentRevest/linux-sunxi-cedrus has never been
merged with linux-sunxi sunxi-next.


Because it is not ready to be merged. It depends on the v4l2 request
API, which was not merged and which is re-worked atm.
Also, sunxi-cedrus itself is not in a finished state and is not as
feature-complete to be merged. Anyway it might be something for
staging... Has there been a [RFC] on the mailing list at all?


Where can I find a list of TODOs to get it ready to be merged?


Assuming that the request API is in, we'd need to:
- Finish the MPEG4 support
- Work on more useful codecs (H264 comes to my mind)
- Implement the DRM planes support for the custom frame format
- Implement the DRM planes support for scaling
- Test it on more SoCs

Or something along those lines.


Lot of work to do


Well... If it was fast and easy it would have been done already :)


:))




I see it seems to be dead, no commit in 1 year.


Yes, because the author did this during an internship, which ended ...
Afaik nobody picked up his work yet.


That's not entirely true. Some work has been done by Thomas (in CC),
especially on the display engine side, but last time we talked his
work was not really upstreamable.

We will also resume that effort starting next march.


Is it possible a preview on a separate Reporitory to start working on now?
Expecially to start porting everything done by FlorentRevest to mainline,
admitted you've not already done.


I'm not sure what you're asking for. Florent's work *was* on mainline.


and then they took it off because it was unmantained?
You've spoken about Thomas(in CC) not ready,
maybe I could help on that if it's public to accelerate.
If I'm able to of course, this is my primary concern.

Otherwise, in which way can I help improving it to make it accept to 
linux-sunxi?

Starting from Florent's work and porting it to sunxi-next to begin?
And after that adding all features you've listed?
Tell me what I can do(I repeat, if I'm able to).



Maxime



Thank you

--
Giulio Benetti
R Manager &
Advanced Research

MICRONOVA SRL
Sede: Via A. Niedda 3 - 35010 Vigonza (PD)
Tel. 049/8931563 - Fax 049/8931346
Cod.Fiscale - P.IVA 02663420285
Capitale Sociale € 26.000 i.v.
Iscritta al Reg. Imprese di Padova N. 02663420285
Numero R.E.A. 258642

Re: [linux-sunxi] Cedrus driver

[Maxime Ripard]

On Thu, Nov 16, 2017 at 01:30:52PM +0100, Giulio Benetti wrote:
> > On Thu, Nov 16, 2017 at 11:37:30AM +0100, Giulio Benetti wrote:
> > > Il 16/11/2017 11:31, Andreas Baierl ha scritto:
> > > > Am 16.11.2017 um 11:13 schrieb Giulio Benetti:
> > > > > Hello,
> > > > > 
> > > > Hello,
> > > > > I'm wondering why cedrus
> > > > > https://github.com/FlorentRevest/linux-sunxi-cedrus has never been
> > > > > merged with linux-sunxi sunxi-next.
> > > > > 
> > > > Because it is not ready to be merged. It depends on the v4l2 request
> > > > API, which was not merged and which is re-worked atm.
> > > > Also, sunxi-cedrus itself is not in a finished state and is not as
> > > > feature-complete to be merged. Anyway it might be something for
> > > > staging... Has there been a [RFC] on the mailing list at all?
> > > 
> > > Where can I find a list of TODOs to get it ready to be merged?
> > 
> > Assuming that the request API is in, we'd need to:
> >- Finish the MPEG4 support
> >- Work on more useful codecs (H264 comes to my mind)
> >- Implement the DRM planes support for the custom frame format
> >- Implement the DRM planes support for scaling
> >- Test it on more SoCs
> > 
> > Or something along those lines.
> 
> Lot of work to do

Well... If it was fast and easy it would have been done already :)

> > > > > I see it seems to be dead, no commit in 1 year.
> > > > 
> > > > Yes, because the author did this during an internship, which ended ...
> > > > Afaik nobody picked up his work yet.
> > 
> > That's not entirely true. Some work has been done by Thomas (in CC),
> > especially on the display engine side, but last time we talked his
> > work was not really upstreamable.
> > 
> > We will also resume that effort starting next march.
> 
> Is it possible a preview on a separate Reporitory to start working on now?
> Expecially to start porting everything done by FlorentRevest to mainline,
> admitted you've not already done.

I'm not sure what you're asking for. Florent's work *was* on mainline.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [linux-sunxi] Cedrus driver

[Giulio Benetti]

Hi,

Il 16/11/2017 12:02, Maxime Ripard ha scritto:

Hi,

I'm not sure why there's so many recipients (Russell, Rob or Mark have
a limited interest in this I assume), and why you're also missing some
key ones (like the v4l2 list).


added in cc linux-media mailing list that appears to be the right one 
for v4l2 and removed Russell, Rob and Mark




On Thu, Nov 16, 2017 at 11:37:30AM +0100, Giulio Benetti wrote:

Il 16/11/2017 11:31, Andreas Baierl ha scritto:

Am 16.11.2017 um 11:13 schrieb Giulio Benetti:

Hello,


Hello,

I'm wondering why cedrus
https://github.com/FlorentRevest/linux-sunxi-cedrus has never been
merged with linux-sunxi sunxi-next.


Because it is not ready to be merged. It depends on the v4l2 request
API, which was not merged and which is re-worked atm.
Also, sunxi-cedrus itself is not in a finished state and is not as
feature-complete to be merged. Anyway it might be something for
staging... Has there been a [RFC] on the mailing list at all?


Where can I find a list of TODOs to get it ready to be merged?


Assuming that the request API is in, we'd need to:
   - Finish the MPEG4 support
   - Work on more useful codecs (H264 comes to my mind)
   - Implement the DRM planes support for the custom frame format
   - Implement the DRM planes support for scaling
   - Test it on more SoCs

Or something along those lines.


Lot of work to do




I see it seems to be dead, no commit in 1 year.


Yes, because the author did this during an internship, which ended ...
Afaik nobody picked up his work yet.


That's not entirely true. Some work has been done by Thomas (in CC),
especially on the display engine side, but last time we talked his
work was not really upstreamable.

We will also resume that effort starting next march.


Is it possible a preview on a separate Reporitory to start working on now?
Expecially to start porting everything done by FlorentRevest to mainline,
admitted you've not already done.




since we need video acceleration on A20 and A33.


ack.


By the way, when you answer to google group, is it right that all CC I
inserted are not inserted too?
Because this causes mess with mailing lists (seems to me).


Yes, that's one of the many brain-damaged thing happening on that
list...


Who can I ask to modify it?
Most of all, is it possible?



Maxime




--
Giulio Benetti
R Manager &
Advanced Research

MICRONOVA SRL
Sede: Via A. Niedda 3 - 35010 Vigonza (PD)
Tel. 049/8931563 - Fax 049/8931346
Cod.Fiscale - P.IVA 02663420285
Capitale Sociale € 26.000 i.v.
Iscritta al Reg. Imprese di Padova N. 02663420285
Numero R.E.A. 258642

Re: [PATCH/RFC 2/2] v4l: rcar-vin: Wait for device access to complete before unplugging

[Sakari Ailus]

On Thu, Nov 16, 2017 at 02:33:49AM +0200, Laurent Pinchart wrote:
> To avoid races between device access and unplug, call the
> video_device_unplug() function in the platform driver remove handler.
> This will unsure that all device access completes before the remove
> handler proceeds to free resources.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
> b/drivers/media/platform/rcar-vin/rcar-core.c
> index bd7976efa1fb..c5210f1d09ed 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -1273,6 +1273,7 @@ static int rcar_vin_remove(struct platform_device *pdev)
>  
>   pm_runtime_disable(>dev);
>  
> + video_device_unplug(>vdev);

Does this depend on another patch?

>  
>   if (!vin->info->use_mc) {
>   v4l2_async_notifier_unregister(>notifier);
> -- 
> Regards,
> 
> Laurent Pinchart
> 

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH/RFC 1/2] v4l: v4l2-dev: Add infrastructure to protect device unplug race

[Sakari Ailus]

Hi Laurent,

Thank you for the initiative to bring up and address the matter!

On Thu, Nov 16, 2017 at 02:33:48AM +0200, Laurent Pinchart wrote:
> Device unplug being asynchronous, it naturally races with operations
> performed by userspace through ioctls or other file operations on video
> device nodes.
> 
> This leads to potential access to freed memory or to other resources
> during device access if unplug occurs during device access. To solve
> this, we need to wait until all device access completes when unplugging
> the device, and block all further access when the device is being
> unplugged.
> 
> Three new functions are introduced. The video_device_enter() and
> video_device_exit() functions must be used to mark entry and exit from
> all code sections where the device can be accessed. The

I wonder if it'd help splitting this patch into two: one that introduces
the mechanism and the other that uses it. Up to you.

Nevertheless, it'd be better to have other system calls covered as well.

> video_device_unplug() function is then used in the unplug handler to
> mark the device as being unplugged and wait for all access to complete.
> 
> As an example mark the ioctl handler as a device access section. Other
> file operations need to be protected too, and blocking ioctls (such as
> VIDIOC_DQBUF) need to be handled as well.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/media/v4l2-core/v4l2-dev.c | 57 
> ++
>  include/media/v4l2-dev.h   | 47 +++
>  2 files changed, 104 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c 
> b/drivers/media/v4l2-core/v4l2-dev.c
> index c647ba648805..c73c6d49e7cf 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -156,6 +156,52 @@ void video_device_release_empty(struct video_device 
> *vdev)
>  }
>  EXPORT_SYMBOL(video_device_release_empty);
>  
> +int video_device_enter(struct video_device *vdev)
> +{
> + bool unplugged;
> +
> + spin_lock(>unplug_lock);
> + unplugged = vdev->unplugged;
> + if (!unplugged)
> + vdev->access_refcount++;
> + spin_unlock(>unplug_lock);
> +
> + return unplugged ? -ENODEV : 0;
> +}
> +EXPORT_SYMBOL_GPL(video_device_enter);
> +
> +void video_device_exit(struct video_device *vdev)
> +{
> + bool wake_up;
> +
> + spin_lock(>unplug_lock);
> + WARN_ON(--vdev->access_refcount < 0);
> + wake_up = vdev->access_refcount == 0;
> + spin_unlock(>unplug_lock);
> +
> + if (wake_up)
> + wake_up(>unplug_wait);
> +}
> +EXPORT_SYMBOL_GPL(video_device_exit);

Is there a need to export the two, i.e. wouldn't you only call them from
the framework, or the same module?

> +
> +void video_device_unplug(struct video_device *vdev)
> +{
> + bool unplug_blocked;
> +
> + spin_lock(>unplug_lock);
> + unplug_blocked = vdev->access_refcount > 0;
> + vdev->unplugged = true;

Shouldn't this be set to false in video_register_device()?

> + spin_unlock(>unplug_lock);
> +
> + if (!unplug_blocked)
> + return;

Not necessary, wait_event_timeout() handles this already.

> +
> + if (!wait_event_timeout(vdev->unplug_wait, !vdev->access_refcount,
> + msecs_to_jiffies(15)))
> + WARN(1, "Timeout waiting for device access to complete\n");

Why a timeout? Does this get somehow less problematic over time? :-)

> +}
> +EXPORT_SYMBOL_GPL(video_device_unplug);
> +
>  static inline void video_get(struct video_device *vdev)
>  {
>   get_device(>dev);
> @@ -351,6 +397,10 @@ static long v4l2_ioctl(struct file *filp, unsigned int 
> cmd, unsigned long arg)
>   struct video_device *vdev = video_devdata(filp);
>   int ret = -ENODEV;

You could leave ret unassigned here.

>  
> + ret = video_device_enter(vdev);
> + if (ret < 0)
> + return ret;
> +
>   if (vdev->fops->unlocked_ioctl) {
>   struct mutex *lock = v4l2_ioctl_get_lock(vdev, cmd);
>  
> @@ -358,11 +408,14 @@ static long v4l2_ioctl(struct file *filp, unsigned int 
> cmd, unsigned long arg)
>   return -ERESTARTSYS;
>   if (video_is_registered(vdev))
>   ret = vdev->fops->unlocked_ioctl(filp, cmd, arg);
> + else
> + ret = -ENODEV;
>   if (lock)
>   mutex_unlock(lock);
>   } else
>   ret = -ENOTTY;
>  
> + video_device_exit(vdev);
>   return ret;
>  }
>  
> @@ -841,6 +894,10 @@ int __video_register_device(struct video_device *vdev, 
> int type, int nr,
>   if (WARN_ON(!vdev->v4l2_dev))
>   return -EINVAL;
>  
> + /* unplug support */
> + spin_lock_init(>unplug_lock);
> + init_waitqueue_head(>unplug_wait);
> +
>   /* v4l2_fh support */
>   spin_lock_init(>fh_lock);
>   

[PATCH 1/2] dt-bindings: media: rcar_vin: add device tree support for r8a774[35]

[Fabrizio Castro]

Add compatible strings for r8a7743 and r8a7745. No driver change
change is needed as "renesas,rcar-gen2-vin" will activate the right
code. However, it is good practice to document compatible strings
for the specific SoC as this allows SoC specific changes to the
driver if needed, in addition to document SoC support and therefore
allow checkpatch.pl to validate compatible string values.

Signed-off-by: Fabrizio Castro 
Reviewed-by: Biju Das 
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt 
b/Documentation/devicetree/bindings/media/rcar_vin.txt
index 6e4ef8c..0042ef2 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -14,7 +14,10 @@ channel which can be either RGB, YUYV or BT656.
- "renesas,vin-r8a7790" for the R8A7790 device
- "renesas,vin-r8a7779" for the R8A7779 device
- "renesas,vin-r8a7778" for the R8A7778 device
-   - "renesas,rcar-gen2-vin" for a generic R-Car Gen2 compatible device.
+   - "renesas,vin-r8a7745" for the R8A7745 device
+   - "renesas,vin-r8a7743" for the R8A7743 device
+   - "renesas,rcar-gen2-vin" for a generic R-Car Gen2 or RZ/G1 compatible
+ device.
- "renesas,rcar-gen3-vin" for a generic R-Car Gen3 compatible device.
 
When compatible with the generic version nodes must list the
-- 
2.7.4

[PATCH 0/2] Add VIN support to r8a7743

[Fabrizio Castro]

Hello,

this series documents VIN related dt-bindings for r8a774[35], and adds VIN[012]
nodes to the r8a7743 SoC dtsi.

Best regards,

Fabrizio Castro (2):
  dt-bindings: media: rcar_vin: add device tree support for r8a774[35]
  ARM: dts: r8a7743: add VIN dt support

 .../devicetree/bindings/media/rcar_vin.txt |  5 ++-
 arch/arm/boot/dts/r8a7743.dtsi | 36 ++
 2 files changed, 40 insertions(+), 1 deletion(-)

-- 
2.7.4

Re: [PATCH 2/2] dvbv5-daemon: 0 is a valid fd

[Mauro Carvalho Chehab]

Em Thu, 16 Nov 2017 12:36:24 +0100
Rafaël Carré  escreveu:

> On 16/11/2017 12:25, Mauro Carvalho Chehab wrote:
> > Em Wed, 15 Nov 2017 12:33:36 +0100
> > Rafaël Carré  escreveu:
> >   
> >> ---
> >>  utils/dvb/dvbv5-daemon.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/utils/dvb/dvbv5-daemon.c b/utils/dvb/dvbv5-daemon.c
> >> index 58485ac6..711694e0 100644
> >> --- a/utils/dvb/dvbv5-daemon.c
> >> +++ b/utils/dvb/dvbv5-daemon.c
> >> @@ -570,7 +570,7 @@ void dvb_remote_log(void *priv, int level, const char 
> >> *fmt, ...)
> >>  
> >>va_end(ap);
> >>  
> >> -  if (fd > 0)
> >> +  if (fd >= 0)
> >>send_data(fd, "%i%s%i%s", 0, "log", level, buf);
> >>else
> >>local_log(level, buf);  
> 
> Signed-off-by: Rafaël Carré 
> 
> > 
> > Patch looks OK. Just need a description explaining why we
> > need to consider fd == 0 and a SOB.  
> 
> Sorry, I am not used to do sign-off, will try to remember.
> 
> fd == 0 can happen if the application closes stdin/out/err then opens a
> new fd.
> 
> Should I put this in the commit log?

For this one, no need. Next time, the best is to resend, as I usually
use patchwork also to pick the patches. Patchwork won't automatically
pick new patch descriptions.

> 
> > Thanks,
> > Mauro
> >   
> 



Thanks,
Mauro

Re: [PATCH v2] dvb_dev_get_fd(): return fd of local devices

[Rafaël Carré]

On 15/11/2017 15:25, Rafaël Carré wrote:
> This makes it possible to poll a local device.
> Getting the fd is preferrable to adding a dvb_dev_poll() function,
> because we can poll several fds together in an event-based program.
> 
> This is not implemented for remote devices, as polling a remote fd
> does not make sense.
> We could instead return the socket to know when to expect messages
> from the remote device, but the current implementation in
> dvb-dev-remote.c already runs a thread to receive remote messages
> as soon as possible.
> ---
> v2: get the private dvb_device_priv correctly
> 
>  lib/include/libdvbv5/dvb-dev.h | 12 
>  lib/libdvbv5/dvb-dev-local.c   |  6 ++
>  lib/libdvbv5/dvb-dev-priv.h|  1 +
>  lib/libdvbv5/dvb-dev.c | 11 +++
>  4 files changed, 30 insertions(+)
> 
> diff --git a/lib/include/libdvbv5/dvb-dev.h b/lib/include/libdvbv5/dvb-dev.h
> index 98bee5e7..55e0f065 100644
> --- a/lib/include/libdvbv5/dvb-dev.h
> +++ b/lib/include/libdvbv5/dvb-dev.h
> @@ -289,6 +289,18 @@ struct dvb_open_descriptor *dvb_dev_open(struct 
> dvb_device *dvb,
>   */
>  void dvb_dev_close(struct dvb_open_descriptor *open_dev);
>  
> +/**
> + * @brief returns fd from a local device
> + * This will not work for remote devices.
> + * @ingroup dvb_device
> + *
> + * @param open_dev   Points to the struct dvb_open_descriptor
> + *
> + * @return On success, returns the fd.
> + * Returns -1 on error.
> + */
> +int dvb_dev_get_fd(struct dvb_open_descriptor *open_dev);
> +
>  /**
>   * @brief read from a dvb demux or dvr file
>   * @ingroup dvb_device
> diff --git a/lib/libdvbv5/dvb-dev-local.c b/lib/libdvbv5/dvb-dev-local.c
> index b50b61b4..eb2f0775 100644
> --- a/lib/libdvbv5/dvb-dev-local.c
> +++ b/lib/libdvbv5/dvb-dev-local.c
> @@ -775,6 +775,11 @@ static void dvb_dev_local_free(struct dvb_device_priv 
> *dvb)
>   free(priv);
>  }
>  
> +static int dvb_local_get_fd(struct dvb_open_descriptor *open_dev)
> +{
> +return open_dev->fd;
> +}
> +
>  /* Initialize for local usage */
>  void dvb_dev_local_init(struct dvb_device_priv *dvb)
>  {
> @@ -788,6 +793,7 @@ void dvb_dev_local_init(struct dvb_device_priv *dvb)
>   ops->stop_monitor = dvb_local_stop_monitor;
>   ops->open = dvb_local_open;
>   ops->close = dvb_local_close;
> + ops->get_fd = dvb_local_get_fd;
>  
>   ops->dmx_stop = dvb_local_dmx_stop;
>   ops->set_bufsize = dvb_local_set_bufsize;
> diff --git a/lib/libdvbv5/dvb-dev-priv.h b/lib/libdvbv5/dvb-dev-priv.h
> index e05fcad2..2e69f766 100644
> --- a/lib/libdvbv5/dvb-dev-priv.h
> +++ b/lib/libdvbv5/dvb-dev-priv.h
> @@ -72,6 +72,7 @@ struct dvb_dev_ops {
>   int (*fe_get_stats)(struct dvb_v5_fe_parms *p);
>  
>   void (*free)(struct dvb_device_priv *dvb);
> + int (*get_fd)(struct dvb_open_descriptor *dvb);
>  };
>  
>  struct dvb_device_priv {
> diff --git a/lib/libdvbv5/dvb-dev.c b/lib/libdvbv5/dvb-dev.c
> index 7e2da1fb..e6a8fc76 100644
> --- a/lib/libdvbv5/dvb-dev.c
> +++ b/lib/libdvbv5/dvb-dev.c
> @@ -218,6 +218,17 @@ struct dvb_open_descriptor *dvb_dev_open(struct 
> dvb_device *d,
>   return ops->open(dvb, sysname, flags);
>  }
>  
> +int dvb_dev_get_fd(struct dvb_open_descriptor *open_dev)
> +{
> + struct dvb_device_priv *dvb = open_dev->dvb;
> + struct dvb_dev_ops *ops = >ops;
> +
> + if (!ops->get_fd)
> + return -1;
> +
> + return ops->get_fd(open_dev);
> +}
> +
>  void dvb_dev_close(struct dvb_open_descriptor *open_dev)
>  {
>   struct dvb_device_priv *dvb = open_dev->dvb;
> 

Signed-off-by: Rafaël Carré 

Re: [PATCH] media: dvb_frontend: Fix uninitialized error in dvb_frontend_handle_ioctl()

[Arnd Bergmann]

On Thu, Nov 16, 2017 at 11:51 AM, Geert Uytterhoeven
 wrote:
> With gcc-4.1.2:
>
> drivers/media/dvb-core/dvb_frontend.c: In function 
> ‘dvb_frontend_handle_ioctl’:
> drivers/media/dvb-core/dvb_frontend.c:2110: warning: ‘err’ may be used 
> uninitialized in this function
>
> Indeed, there are 13 cases where err is used initialized if one of the
> dvb_frontend_ops is not implemented.
>
> Preinitialize err to -EOPNOTSUPP like before to fix this.
>
> Fixes: d73dcf0cdb95a47f ("media: dvb_frontend: cleanup ioctl handling logic")
> Signed-off-by: Geert Uytterhoeven 

Good catch!

This one shows up on x86 allmdoconfig with gcc-4.5 or older but not gcc-4.6.

Acked-by: Arnd Bergmann 

Re: [PATCH 2/2] dvbv5-daemon: 0 is a valid fd

[Rafaël Carré]

On 16/11/2017 12:25, Mauro Carvalho Chehab wrote:
> Em Wed, 15 Nov 2017 12:33:36 +0100
> Rafaël Carré  escreveu:
> 
>> ---
>>  utils/dvb/dvbv5-daemon.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/utils/dvb/dvbv5-daemon.c b/utils/dvb/dvbv5-daemon.c
>> index 58485ac6..711694e0 100644
>> --- a/utils/dvb/dvbv5-daemon.c
>> +++ b/utils/dvb/dvbv5-daemon.c
>> @@ -570,7 +570,7 @@ void dvb_remote_log(void *priv, int level, const char 
>> *fmt, ...)
>>  
>>  va_end(ap);
>>  
>> -if (fd > 0)
>> +if (fd >= 0)
>>  send_data(fd, "%i%s%i%s", 0, "log", level, buf);
>>  else
>>  local_log(level, buf);

Signed-off-by: Rafaël Carré 

> 
> Patch looks OK. Just need a description explaining why we
> need to consider fd == 0 and a SOB.

Sorry, I am not used to do sign-off, will try to remember.

fd == 0 can happen if the application closes stdin/out/err then opens a
new fd.

Should I put this in the commit log?

> Thanks,
> Mauro
> 

Re: [PATCH] dvb_local_open(): strdup fname before calling dvb_fe_open_fname()

[Mauro Carvalho Chehab]

Em Tue, 14 Nov 2017 12:15:26 +0100
Rafaël Carré  escreveu:

> Issue spotted by valgrind:
> 
> ==5290== Invalid free() / delete / delete[] / realloc()
> ==5290==at 0x4C30D3B: free (in 
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==5290==by 0x4E54401: free_dvb_dev (dvb-dev.c:49)
> ==5290==by 0x4E5449A: dvb_dev_free_devices (dvb-dev.c:94)
> ==5290==by 0x4E547BA: dvb_dev_free (dvb-dev.c:121)
> ==5290==by 0x10881A: main (leak.c:26)
> ==5290==  Address 0x5e55910 is 0 bytes inside a block of size 28 free'd
> ==5290==at 0x4C30D3B: free (in 
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==5290==by 0x4E56504: dvb_v5_free (dvb-fe.c:85)
> ==5290==by 0x4E547B2: dvb_dev_free (dvb-dev.c:119)
> ==5290==by 0x10881A: main (leak.c:26)
> ==5290==  Block was alloc'd at
> ==5290==at 0x4C2FB0F: malloc (in 
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==5290==by 0x5119C39: strdup (strdup.c:42)
> ==5290==by 0x4E55B42: handle_device_change (dvb-dev-local.c:137)
> ==5290==by 0x4E561DA: dvb_local_find (dvb-dev-local.c:323)
> ==5290==by 0x10880E: main (leak.c:10)

SOB?

> 
> Signed-off-by: Rafaël Carré 
> ---
>  lib/libdvbv5/dvb-dev-local.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/libdvbv5/dvb-dev-local.c b/lib/libdvbv5/dvb-dev-local.c
> index 920e81fb..b50b61b4 100644
> --- a/lib/libdvbv5/dvb-dev-local.c
> +++ b/lib/libdvbv5/dvb-dev-local.c
> @@ -440,7 +440,7 @@ static struct dvb_open_descriptor
>*/
>   flags &= ~O_NONBLOCK;
>  
> - ret = dvb_fe_open_fname(parms, dev->path, flags);
> + ret = dvb_fe_open_fname(parms, strdup(dev->path), flags);
>   if (ret) {
>   free(open_dev);
>   return NULL;



Thanks,
Mauro

Re: [PATCH v2] dvb_dev_get_fd(): return fd of local devices

[Mauro Carvalho Chehab]

Em Wed, 15 Nov 2017 15:25:39 +0100
Rafaël Carré  escreveu:

> This makes it possible to poll a local device.
> Getting the fd is preferrable to adding a dvb_dev_poll() function,
> because we can poll several fds together in an event-based program.
> 
> This is not implemented for remote devices, as polling a remote fd
> does not make sense.
> We could instead return the socket to know when to expect messages
> from the remote device, but the current implementation in
> dvb-dev-remote.c already runs a thread to receive remote messages
> as soon as possible.

SOB?

> ---
> v2: get the private dvb_device_priv correctly
> 
>  lib/include/libdvbv5/dvb-dev.h | 12 
>  lib/libdvbv5/dvb-dev-local.c   |  6 ++
>  lib/libdvbv5/dvb-dev-priv.h|  1 +
>  lib/libdvbv5/dvb-dev.c | 11 +++
>  4 files changed, 30 insertions(+)
> 
> diff --git a/lib/include/libdvbv5/dvb-dev.h b/lib/include/libdvbv5/dvb-dev.h
> index 98bee5e7..55e0f065 100644
> --- a/lib/include/libdvbv5/dvb-dev.h
> +++ b/lib/include/libdvbv5/dvb-dev.h
> @@ -289,6 +289,18 @@ struct dvb_open_descriptor *dvb_dev_open(struct 
> dvb_device *dvb,
>   */
>  void dvb_dev_close(struct dvb_open_descriptor *open_dev);
>  
> +/**
> + * @brief returns fd from a local device
> + * This will not work for remote devices.
> + * @ingroup dvb_device
> + *
> + * @param open_dev   Points to the struct dvb_open_descriptor
> + *
> + * @return On success, returns the fd.
> + * Returns -1 on error.
> + */
> +int dvb_dev_get_fd(struct dvb_open_descriptor *open_dev);
> +
>  /**
>   * @brief read from a dvb demux or dvr file
>   * @ingroup dvb_device
> diff --git a/lib/libdvbv5/dvb-dev-local.c b/lib/libdvbv5/dvb-dev-local.c
> index b50b61b4..eb2f0775 100644
> --- a/lib/libdvbv5/dvb-dev-local.c
> +++ b/lib/libdvbv5/dvb-dev-local.c
> @@ -775,6 +775,11 @@ static void dvb_dev_local_free(struct dvb_device_priv 
> *dvb)
>   free(priv);
>  }
>  
> +static int dvb_local_get_fd(struct dvb_open_descriptor *open_dev)
> +{
> +return open_dev->fd;
> +}
> +
>  /* Initialize for local usage */
>  void dvb_dev_local_init(struct dvb_device_priv *dvb)
>  {
> @@ -788,6 +793,7 @@ void dvb_dev_local_init(struct dvb_device_priv *dvb)
>   ops->stop_monitor = dvb_local_stop_monitor;
>   ops->open = dvb_local_open;
>   ops->close = dvb_local_close;
> + ops->get_fd = dvb_local_get_fd;
>  
>   ops->dmx_stop = dvb_local_dmx_stop;
>   ops->set_bufsize = dvb_local_set_bufsize;
> diff --git a/lib/libdvbv5/dvb-dev-priv.h b/lib/libdvbv5/dvb-dev-priv.h
> index e05fcad2..2e69f766 100644
> --- a/lib/libdvbv5/dvb-dev-priv.h
> +++ b/lib/libdvbv5/dvb-dev-priv.h
> @@ -72,6 +72,7 @@ struct dvb_dev_ops {
>   int (*fe_get_stats)(struct dvb_v5_fe_parms *p);
>  
>   void (*free)(struct dvb_device_priv *dvb);
> + int (*get_fd)(struct dvb_open_descriptor *dvb);
>  };
>  
>  struct dvb_device_priv {
> diff --git a/lib/libdvbv5/dvb-dev.c b/lib/libdvbv5/dvb-dev.c
> index 7e2da1fb..e6a8fc76 100644
> --- a/lib/libdvbv5/dvb-dev.c
> +++ b/lib/libdvbv5/dvb-dev.c
> @@ -218,6 +218,17 @@ struct dvb_open_descriptor *dvb_dev_open(struct 
> dvb_device *d,
>   return ops->open(dvb, sysname, flags);
>  }
>  
> +int dvb_dev_get_fd(struct dvb_open_descriptor *open_dev)
> +{
> + struct dvb_device_priv *dvb = open_dev->dvb;
> + struct dvb_dev_ops *ops = >ops;
> +
> + if (!ops->get_fd)
> + return -1;
> +
> + return ops->get_fd(open_dev);
> +}
> +
>  void dvb_dev_close(struct dvb_open_descriptor *open_dev)
>  {
>   struct dvb_device_priv *dvb = open_dev->dvb;



Thanks,
Mauro

Re: [PATCH] dvb_dev_get_fd(): return fd of local devices

[Mauro Carvalho Chehab]

Em Wed, 15 Nov 2017 11:47:11 +0100
Rafaël Carré  escreveu:

> This makes it possible to poll a local device.
> Getting the fd is preferrable to adding a dvb_dev_poll() function,
> because we can poll several fds together in an event-based program.
> 
> This is not implemented for remote devices, as polling a remote fd
> does not make sense.
> We could instead return the socket to know when to expect messages
> from the remote device, but the current implementation in
> dvb-dev-remote.c already runs a thread to receive remote messages
> as soon as possible.

Patch looks OK.

Just a SOB (signed-off-by) is missing.

> ---
> 
> Note, after reading README, I did not bump the library version.
> 
> Comments welcome
> 
>  lib/include/libdvbv5/dvb-dev.h | 12 
>  lib/libdvbv5/dvb-dev-local.c   |  6 ++
>  lib/libdvbv5/dvb-dev-priv.h|  1 +
>  lib/libdvbv5/dvb-dev.c | 11 +++
>  4 files changed, 30 insertions(+)
> 
> diff --git a/lib/include/libdvbv5/dvb-dev.h b/lib/include/libdvbv5/dvb-dev.h
> index 98bee5e7..55e0f065 100644
> --- a/lib/include/libdvbv5/dvb-dev.h
> +++ b/lib/include/libdvbv5/dvb-dev.h
> @@ -289,6 +289,18 @@ struct dvb_open_descriptor *dvb_dev_open(struct 
> dvb_device *dvb,
>   */
>  void dvb_dev_close(struct dvb_open_descriptor *open_dev);
>  
> +/**
> + * @brief returns fd from a local device
> + * This will not work for remote devices.
> + * @ingroup dvb_device
> + *
> + * @param open_dev   Points to the struct dvb_open_descriptor
> + *
> + * @return On success, returns the fd.
> + * Returns -1 on error.
> + */
> +int dvb_dev_get_fd(struct dvb_open_descriptor *open_dev);
> +
>  /**
>   * @brief read from a dvb demux or dvr file
>   * @ingroup dvb_device
> diff --git a/lib/libdvbv5/dvb-dev-local.c b/lib/libdvbv5/dvb-dev-local.c
> index b50b61b4..eb2f0775 100644
> --- a/lib/libdvbv5/dvb-dev-local.c
> +++ b/lib/libdvbv5/dvb-dev-local.c
> @@ -775,6 +775,11 @@ static void dvb_dev_local_free(struct dvb_device_priv 
> *dvb)
>   free(priv);
>  }
>  
> +static int dvb_local_get_fd(struct dvb_open_descriptor *open_dev)
> +{
> +return open_dev->fd;
> +}
> +
>  /* Initialize for local usage */
>  void dvb_dev_local_init(struct dvb_device_priv *dvb)
>  {
> @@ -788,6 +793,7 @@ void dvb_dev_local_init(struct dvb_device_priv *dvb)
>   ops->stop_monitor = dvb_local_stop_monitor;
>   ops->open = dvb_local_open;
>   ops->close = dvb_local_close;
> + ops->get_fd = dvb_local_get_fd;
>  
>   ops->dmx_stop = dvb_local_dmx_stop;
>   ops->set_bufsize = dvb_local_set_bufsize;
> diff --git a/lib/libdvbv5/dvb-dev-priv.h b/lib/libdvbv5/dvb-dev-priv.h
> index e05fcad2..2e69f766 100644
> --- a/lib/libdvbv5/dvb-dev-priv.h
> +++ b/lib/libdvbv5/dvb-dev-priv.h
> @@ -72,6 +72,7 @@ struct dvb_dev_ops {
>   int (*fe_get_stats)(struct dvb_v5_fe_parms *p);
>  
>   void (*free)(struct dvb_device_priv *dvb);
> + int (*get_fd)(struct dvb_open_descriptor *dvb);
>  };
>  
>  struct dvb_device_priv {
> diff --git a/lib/libdvbv5/dvb-dev.c b/lib/libdvbv5/dvb-dev.c
> index 7e2da1fb..447c9fd5 100644
> --- a/lib/libdvbv5/dvb-dev.c
> +++ b/lib/libdvbv5/dvb-dev.c
> @@ -218,6 +218,17 @@ struct dvb_open_descriptor *dvb_dev_open(struct 
> dvb_device *d,
>   return ops->open(dvb, sysname, flags);
>  }
>  
> +int dvb_dev_get_fd(struct dvb_open_descriptor *open_dev)
> +{
> + struct dvb_device_priv *dvb = (void *)open_dev;
> + struct dvb_dev_ops *ops = >ops;
> +
> + if (!ops->get_fd)
> + return -1;
> +
> + return ops->get_fd(open_dev);
> +}
> +
>  void dvb_dev_close(struct dvb_open_descriptor *open_dev)
>  {
>   struct dvb_device_priv *dvb = open_dev->dvb;



Thanks,
Mauro

Re: [PATCH 2/2] dvbv5-daemon: 0 is a valid fd

[Mauro Carvalho Chehab]

Em Wed, 15 Nov 2017 12:33:36 +0100
Rafaël Carré  escreveu:

> ---
>  utils/dvb/dvbv5-daemon.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/utils/dvb/dvbv5-daemon.c b/utils/dvb/dvbv5-daemon.c
> index 58485ac6..711694e0 100644
> --- a/utils/dvb/dvbv5-daemon.c
> +++ b/utils/dvb/dvbv5-daemon.c
> @@ -570,7 +570,7 @@ void dvb_remote_log(void *priv, int level, const char 
> *fmt, ...)
>  
>   va_end(ap);
>  
> - if (fd > 0)
> + if (fd >= 0)
>   send_data(fd, "%i%s%i%s", 0, "log", level, buf);
>   else
>   local_log(level, buf);

Patch looks OK. Just need a description explaining why we
need to consider fd == 0 and a SOB.



Thanks,
Mauro

Re: [PATCH 1/2] dvb_logfunc: add a user private parameter

[Mauro Carvalho Chehab]

Em Wed, 15 Nov 2017 12:33:35 +0100
Rafaël Carré  escreveu:


At v4l-utils, we're using about the same submission rules as with the Linux
Kernel. So, I would be expecting a patch description and, most importantly,
a Signed-off-by.

> ---
>  lib/include/libdvbv5/dvb-dev.h |  3 ++-
>  lib/include/libdvbv5/dvb-fe.h  |  9 +++--
>  lib/include/libdvbv5/dvb-log.h | 33 +
>  lib/libdvbv5/dvb-dev.c |  3 ++-
>  lib/libdvbv5/dvb-fe.c  | 15 ---
>  lib/libdvbv5/dvb-log.c |  3 ++-
>  utils/dvb/dvb-fe-tool.c|  2 +-
>  utils/dvb/dvbv5-daemon.c   |  9 +
>  utils/dvb/dvbv5-scan.c |  2 +-
>  utils/dvb/dvbv5-zap.c  |  2 +-
>  10 files changed, 46 insertions(+), 35 deletions(-)
> 
> diff --git a/lib/include/libdvbv5/dvb-dev.h b/lib/include/libdvbv5/dvb-dev.h
> index 55e0f065..396fcd07 100644
> --- a/lib/include/libdvbv5/dvb-dev.h
> +++ b/lib/include/libdvbv5/dvb-dev.h
> @@ -243,6 +243,7 @@ void dvb_dev_stop_monitor(struct dvb_device *dvb);
>   * @param logfuncCallback function to be called when a log event
>   *   happens. Can either store the event into a file or
>   *   to print it at the TUI/GUI. Can be null.
> + * @param logpriv   Private data for log function
>   *
>   * @details Sets the function to report log errors and to set the verbosity
>   *   level of debug report messages. If not called, or if logfunc is
> @@ -252,7 +253,7 @@ void dvb_dev_stop_monitor(struct dvb_device *dvb);
>   */
>  void dvb_dev_set_log(struct dvb_device *dvb,
>unsigned verbose,
> -  dvb_logfunc logfunc);
> +  dvb_logfunc logfunc, void *logpriv);


That breaks the ABI. I guess the best here would be to create a
new function (dvb_dev_set_logpriv?), and keep this function name for
backward compatibility.

>  
>  /**
>   * @brief Opens a dvb device
> diff --git a/lib/include/libdvbv5/dvb-fe.h b/lib/include/libdvbv5/dvb-fe.h
> index 1d3565ec..107cb03d 100644
> --- a/lib/include/libdvbv5/dvb-fe.h
> +++ b/lib/include/libdvbv5/dvb-fe.h
> @@ -107,6 +107,7 @@
>   * @param freq_bpf   SCR/Unicable band-pass filter frequency to use, 
> in kHz
>   * @param verboseVerbosity level of the library (RW)
>   * @param dvb_logfuncFunction used to write log messages (RO)
> + * @param logprivPrivate data for logging function (RO)
>   * @param default_charsetName of the charset used by the DVB standard 
> (RW)
>   * @param output_charset Name of the charset to output (system specific) 
> (RW)
>   *
> @@ -140,7 +141,8 @@ struct dvb_v5_fe_parms {
>  
>   /* Function to write DVB logs */
>   unsignedverbose;
> - dvb_logfunc logfunc;
> + dvb_logfunc logfunc;
> + void*logpriv;

To avoid breaking API, please put this at dvb_v5_fe_parms_priv.

>  
>   /* Charsets to be used by the conversion utilities */
>   char*default_charset;
> @@ -176,6 +178,7 @@ struct dvb_v5_fe_parms *dvb_fe_dummy(void);
>   *   happens. Can either store the event into a file
>   *   or to print it at the TUI/GUI. If NULL, the
>   *   library will use its internal handler.
> + * @param logprivPrivate data for dvb_logfunc
>   * @param flags  Flags to be passed to open. Currently 
> only two
>   *   flags are supported: O_RDONLY or O_RDWR.
>   *   Using O_NONBLOCK may hit unexpected issues.
> @@ -195,6 +198,7 @@ struct dvb_v5_fe_parms *dvb_fe_open_flags(int adapter, 
> int frontend,
> unsigned verbose,
> unsigned use_legacy_call,
> dvb_logfunc logfunc,
> +   void *logpriv,
> int flags);
>  
>  /**
> @@ -231,6 +235,7 @@ struct dvb_v5_fe_parms *dvb_fe_open(int adapter, int 
> frontend,
>   * @param logfuncCallback function to be called when a log event
>   *   happens. Can either store the event into a file
>   *   or to print it at the TUI/GUI.
> + * @param logprivPrivate data for dvb_logfunc
>   *
>   * @details This function should be called before using any other function at
>   * the frontend library (or the other alternatives: dvb_fe_open() or
> @@ -240,7 +245,7 @@ struct dvb_v5_fe_parms *dvb_fe_open(int adapter, int 
> frontend,
>   */
>  struct dvb_v5_fe_parms *dvb_fe_open2(int adapter, int frontend,
>   unsigned verbose, unsigned use_legacy_call,
> - dvb_logfunc logfunc);
> +  

Re: [linux-sunxi] Cedrus driver

[Maxime Ripard]

Hi,

I'm not sure why there's so many recipients (Russell, Rob or Mark have
a limited interest in this I assume), and why you're also missing some
key ones (like the v4l2 list).

On Thu, Nov 16, 2017 at 11:37:30AM +0100, Giulio Benetti wrote:
> Il 16/11/2017 11:31, Andreas Baierl ha scritto:
> > Am 16.11.2017 um 11:13 schrieb Giulio Benetti:
> > > Hello,
> > > 
> > Hello,
> > > I'm wondering why cedrus
> > > https://github.com/FlorentRevest/linux-sunxi-cedrus has never been
> > > merged with linux-sunxi sunxi-next.
> > > 
> > Because it is not ready to be merged. It depends on the v4l2 request
> > API, which was not merged and which is re-worked atm.
> > Also, sunxi-cedrus itself is not in a finished state and is not as
> > feature-complete to be merged. Anyway it might be something for
> > staging... Has there been a [RFC] on the mailing list at all?
> 
> Where can I find a list of TODOs to get it ready to be merged?

Assuming that the request API is in, we'd need to:
  - Finish the MPEG4 support
  - Work on more useful codecs (H264 comes to my mind)
  - Implement the DRM planes support for the custom frame format
  - Implement the DRM planes support for scaling
  - Test it on more SoCs

Or something along those lines.

> > > I see it seems to be dead, no commit in 1 year.
> >
> > Yes, because the author did this during an internship, which ended ...
> > Afaik nobody picked up his work yet.

That's not entirely true. Some work has been done by Thomas (in CC),
especially on the display engine side, but last time we talked his
work was not really upstreamable.

We will also resume that effort starting next march.

> > > since we need video acceleration on A20 and A33.
> > > 
> > ack.
> 
> By the way, when you answer to google group, is it right that all CC I
> inserted are not inserted too?
> Because this causes mess with mailing lists (seems to me).

Yes, that's one of the many brain-damaged thing happening on that
list...

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

[PATCH] media: dvb_frontend: Fix uninitialized error in dvb_frontend_handle_ioctl()

[Geert Uytterhoeven]

With gcc-4.1.2:

drivers/media/dvb-core/dvb_frontend.c: In function 
‘dvb_frontend_handle_ioctl’:
drivers/media/dvb-core/dvb_frontend.c:2110: warning: ‘err’ may be used 
uninitialized in this function

Indeed, there are 13 cases where err is used initialized if one of the
dvb_frontend_ops is not implemented.

Preinitialize err to -EOPNOTSUPP like before to fix this.

Fixes: d73dcf0cdb95a47f ("media: dvb_frontend: cleanup ioctl handling logic")
Signed-off-by: Geert Uytterhoeven 
---
 drivers/media/dvb-core/dvb_frontend.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-core/dvb_frontend.c 
b/drivers/media/dvb-core/dvb_frontend.c
index 3ad83359098bde79..9eff8ce60d535379 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -2107,7 +2107,7 @@ static int dvb_frontend_handle_ioctl(struct file *file,
struct dvb_frontend *fe = dvbdev->priv;
struct dvb_frontend_private *fepriv = fe->frontend_priv;
struct dtv_frontend_properties *c = >dtv_property_cache;
-   int i, err;
+   int i, err = -ENOTSUPP;
 
dev_dbg(fe->dvb->device, "%s:\n", __func__);
 
-- 
2.7.4

Re: [PATCH] v4l: sh_mobile_ceu: Return buffers on streamoff()

[Sakari Ailus]

Hi Jacopo,

Thanks for the patch. I think you could add this to v2 of your related set;
that way it'll be easy to get the dependencies right. I don't think this is
urgent either...

On Wed, Nov 15, 2017 at 06:59:12PM +0100, Jacopo Mondi wrote:
> videobuf2 core reports an error when not all buffers have been returned
> to the framework:
> 
> drivers/media/v4l2-core/videobuf2-core.c:1651
> WARN_ON(atomic_read(>owned_by_drv_count))
> 
> Fix this returning all buffers currently in capture queue.
> 
> Signed-off-by: Jacopo Mondi 
> 
> ---
> 
> I know I'm working to get rid of this driver, but while I was trying to have
> it working again on mainline, I found this had to be fixed.
> 
> Thanks
>   j
> 
> ---
>  drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c 
> b/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c
> index 36762ec..9180a1d 100644
> --- a/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c
> +++ b/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c
> @@ -451,13 +451,18 @@ static void sh_mobile_ceu_stop_streaming(struct 
> vb2_queue *q)
>   struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
>   struct sh_mobile_ceu_dev *pcdev = ici->priv;
>   struct list_head *buf_head, *tmp;
> + struct vb2_v4l2_buffer *vbuf;
> 
>   spin_lock_irq(>lock);
> 
>   pcdev->active = NULL;
> 
> - list_for_each_safe(buf_head, tmp, >capture)
> + list_for_each_safe(buf_head, tmp, >capture) {
> + vbuf = _entry(buf_head, struct sh_mobile_ceu_buffer,
> +queue)->vb;
> + vb2_buffer_done(>vb2_buf, VB2_BUF_STATE_DONE);

This should be VB2_BUF_STATE_ERROR, as the hardware hasn't actually
processed them, right?

>   list_del_init(buf_head);
> + }
> 
>   spin_unlock_irq(>lock);
> 

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [RFCv1 PATCH 0/6] v4l2-ctrls: implement requests

[Hans Verkuil]

On 16/11/17 09:48, Alexandre Courbot wrote:
> Hi Hans,
> 
> On Wed, Nov 15, 2017 at 7:12 PM, Hans Verkuil  wrote:
>> Hi Alexandre,
>>
>> On 15/11/17 10:38, Alexandre Courbot wrote:
>>> Hi Hans!
>>>
>>> Thanks for the patchset! It looks quite good at first sight, a few comments 
>>> and
>>> questions follow though.
>>>
>>> On Monday, November 13, 2017 11:34:02 PM JST, Hans Verkuil wrote:
 From: Hans Verkuil 

 Hi Alexandre,

 This is a first implementation of the request API in the
 control framework. It is fairly simplistic at the moment in that
 it just clones all the control values (so no refcounting yet for
 values as Laurent proposed, I will work on that later). But this
 should not be a problem for codecs since there aren't all that many
 controls involved.
>>>
>>> Regarding value refcounting, I think we can probably do without it if we 
>>> parse
>>> the requests queue when looking values up. It may be more practical (having 
>>> a
>>> kref for each v4l2_ctrl_ref in a request sounds overkill to me), and maybe 
>>> also
>>> more predictible since we would have less chance of having dangling old 
>>> values.
>>>
 The API is as follows:

 struct v4l2_ctrl_handler *v4l2_ctrl_request_alloc(void);

 This allocates a struct v4l2_ctrl_handler that is empty (i.e. has
 no controls) but is refcounted and is marked as representing a
 request.

 int v4l2_ctrl_request_clone(struct v4l2_ctrl_handler *hdl,
 const struct v4l2_ctrl_handler *from,
 bool (*filter)(const struct v4l2_ctrl *ctrl));

 Delete any existing controls in handler 'hdl', then clone the values
 from an existing handler 'from' into 'hdl'. If 'from' == NULL, then
 this just clears the handler. 'from' can either be another request
 control handler or a regular control handler in which case the
 current values are cloned. If 'filter' != NULL then you can
 filter which controls you want to clone.
>>>
>>> One thing that seems to be missing is, what happens if you try to set a 
>>> control
>>> on an empty request? IIUC this would currently fail because find_ref() would
>>> not be able to find the control. The value ref should probably be created in
>>> that case so we can create requests with a handful of controls.
>>
>> Wasn't the intention that we never have an empty request but always clone?
>> I.e. in your code the _alloc call is always followed by a _clone call.
>>
>> The reason I have a separate _alloc function is that you use that when you
>> want to create a new control handler ('new request'). If the user wants to 
>> reuse an
>> existing request, then _clone can be called directly on the existing handler.
>>
>>> Also, if you clone a handler that is not a request, I understand that all
>>> controls will be deduplicated, creating a full-state copy? That could be 
>>> useful,
>>> but since this is the only way to make the current code work, I hope that 
>>> the
>>> current impossibility to set a control on an empty request is a bug (or 
>>> misunderstanding from my part).
>>
>> I think it is a misunderstanding. Seen from userspace you'll never have an 
>> empty
>> request.
> 
> It depends on what we want requests to be:
> 
> (1) A representation of what the complete state of the device should
> be when processing buffers, or
> 
> (2) A set of controls to be applied before processing buffers.
> 
> IIUC your patchset currently implements (1): as we clone a request (or
> the current state of the device), we create a new ref for every
> control that is not inherited. And when applying the request, we
> consider all these controls again.
> 
> There is the fact that on more complex pipelines, the number of
> controls may be big enough that we may want to limit the number we
> need to manage per request, but my main concern is that this will
> probably disallow some use-cases that we discussed in Prague.
> 
> For instance, let's say I create a request for a camera sensor by
> cloning the device's control_handler, and set a few controls like
> exposure that I want to be used for a give frame. But let's also say
> that I have another thread taking care of focus on the same device -
> this thread uses some other input to constantly adjust the focus by
> calling S_EXT_CTRLS directly (i.e. without a request). When my request
> gets processed, the focus will be reset to whatever it was when I
> cloned the request, which we want to avoid (if you remember, I
> interrupted Laurent to ask whether this is the behavior we wanted, and
> we all agreed it was).
> 
> Or maybe I am missing something in your implementation?

Such things are simply not yet implemented. This is just a first version
that should be good enough for codecs. Once we have something up and
running that can be used for some real-life testing, then I will work on
this some more.

> That does 

Re: [RFCv1 PATCH 0/6] v4l2-ctrls: implement requests

[Alexandre Courbot]

Hi Hans,

On Wed, Nov 15, 2017 at 7:12 PM, Hans Verkuil  wrote:
> Hi Alexandre,
>
> On 15/11/17 10:38, Alexandre Courbot wrote:
>> Hi Hans!
>>
>> Thanks for the patchset! It looks quite good at first sight, a few comments 
>> and
>> questions follow though.
>>
>> On Monday, November 13, 2017 11:34:02 PM JST, Hans Verkuil wrote:
>>> From: Hans Verkuil 
>>>
>>> Hi Alexandre,
>>>
>>> This is a first implementation of the request API in the
>>> control framework. It is fairly simplistic at the moment in that
>>> it just clones all the control values (so no refcounting yet for
>>> values as Laurent proposed, I will work on that later). But this
>>> should not be a problem for codecs since there aren't all that many
>>> controls involved.
>>
>> Regarding value refcounting, I think we can probably do without it if we 
>> parse
>> the requests queue when looking values up. It may be more practical (having a
>> kref for each v4l2_ctrl_ref in a request sounds overkill to me), and maybe 
>> also
>> more predictible since we would have less chance of having dangling old 
>> values.
>>
>>> The API is as follows:
>>>
>>> struct v4l2_ctrl_handler *v4l2_ctrl_request_alloc(void);
>>>
>>> This allocates a struct v4l2_ctrl_handler that is empty (i.e. has
>>> no controls) but is refcounted and is marked as representing a
>>> request.
>>>
>>> int v4l2_ctrl_request_clone(struct v4l2_ctrl_handler *hdl,
>>> const struct v4l2_ctrl_handler *from,
>>> bool (*filter)(const struct v4l2_ctrl *ctrl));
>>>
>>> Delete any existing controls in handler 'hdl', then clone the values
>>> from an existing handler 'from' into 'hdl'. If 'from' == NULL, then
>>> this just clears the handler. 'from' can either be another request
>>> control handler or a regular control handler in which case the
>>> current values are cloned. If 'filter' != NULL then you can
>>> filter which controls you want to clone.
>>
>> One thing that seems to be missing is, what happens if you try to set a 
>> control
>> on an empty request? IIUC this would currently fail because find_ref() would
>> not be able to find the control. The value ref should probably be created in
>> that case so we can create requests with a handful of controls.
>
> Wasn't the intention that we never have an empty request but always clone?
> I.e. in your code the _alloc call is always followed by a _clone call.
>
> The reason I have a separate _alloc function is that you use that when you
> want to create a new control handler ('new request'). If the user wants to 
> reuse an
> existing request, then _clone can be called directly on the existing handler.
>
>> Also, if you clone a handler that is not a request, I understand that all
>> controls will be deduplicated, creating a full-state copy? That could be 
>> useful,
>> but since this is the only way to make the current code work, I hope that the
>> current impossibility to set a control on an empty request is a bug (or 
>> misunderstanding from my part).
>
> I think it is a misunderstanding. Seen from userspace you'll never have an 
> empty
> request.

It depends on what we want requests to be:

(1) A representation of what the complete state of the device should
be when processing buffers, or

(2) A set of controls to be applied before processing buffers.

IIUC your patchset currently implements (1): as we clone a request (or
the current state of the device), we create a new ref for every
control that is not inherited. And when applying the request, we
consider all these controls again.

There is the fact that on more complex pipelines, the number of
controls may be big enough that we may want to limit the number we
need to manage per request, but my main concern is that this will
probably disallow some use-cases that we discussed in Prague.

For instance, let's say I create a request for a camera sensor by
cloning the device's control_handler, and set a few controls like
exposure that I want to be used for a give frame. But let's also say
that I have another thread taking care of focus on the same device -
this thread uses some other input to constantly adjust the focus by
calling S_EXT_CTRLS directly (i.e. without a request). When my request
gets processed, the focus will be reset to whatever it was when I
cloned the request, which we want to avoid (if you remember, I
interrupted Laurent to ask whether this is the behavior we wanted, and
we all agreed it was).

Or maybe I am missing something in your implementation?

That does not change the fact that I keep working on codecs using the
current model, but I think we will want to address this. The simplest
way to do this would be to only create refs for controls that we
explicitly change (or when cloning a request and not the
state_handler). It would also have the added benefit of reducing the
number of refs. :)

Cheers,
Alex.