cron job: media_tree daily build: ERRORS

[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:   Sat Jun 30 05:00:16 CEST 2018
media-tree git hash:3c4a737267e89aafa6308c6c456d2ebea3fcd085
media_build git hash:   26d102795c91f8593a4f74f96b955f9a8b81dbc3
v4l-utils git hash: 0898d1d4dbcfe2fc94f80de4f9de2b2994ccea99
gcc version:i686-linux-gcc (GCC) 8.1.0
sparse version: 0.5.2
smatch version: 0.5.1
host hardware:  x86_64
host os:4.16.0-1-amd64

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-arm64: OK
linux-git-i686: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
Check COMPILE_TEST: OK
linux-2.6.36.4-i686: ERRORS
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-i686: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-i686: ERRORS
linux-2.6.39.4-x86_64: ERRORS
linux-3.0.101-i686: ERRORS
linux-3.0.101-x86_64: ERRORS
linux-3.1.10-i686: ERRORS
linux-3.1.10-x86_64: ERRORS
linux-3.2.101-i686: ERRORS
linux-3.2.101-x86_64: ERRORS
linux-3.3.8-i686: ERRORS
linux-3.3.8-x86_64: ERRORS
linux-3.4.113-i686: ERRORS
linux-3.4.113-x86_64: ERRORS
linux-3.5.7-i686: ERRORS
linux-3.5.7-x86_64: ERRORS
linux-3.6.11-i686: ERRORS
linux-3.6.11-x86_64: ERRORS
linux-3.7.10-i686: ERRORS
linux-3.7.10-x86_64: ERRORS
linux-3.8.13-i686: ERRORS
linux-3.8.13-x86_64: ERRORS
linux-3.9.11-i686: ERRORS
linux-3.9.11-x86_64: ERRORS
linux-3.10.108-i686: ERRORS
linux-3.10.108-x86_64: ERRORS
linux-3.11.10-i686: ERRORS
linux-3.11.10-x86_64: ERRORS
linux-3.12.74-i686: ERRORS
linux-3.12.74-x86_64: ERRORS
linux-3.13.11-i686: ERRORS
linux-3.13.11-x86_64: ERRORS
linux-3.14.79-i686: ERRORS
linux-3.14.79-x86_64: ERRORS
linux-3.15.10-i686: ERRORS
linux-3.15.10-x86_64: ERRORS
linux-3.16.56-i686: ERRORS
linux-3.16.56-x86_64: ERRORS
linux-3.17.8-i686: ERRORS
linux-3.17.8-x86_64: ERRORS
linux-3.18.102-i686: ERRORS
linux-3.18.102-x86_64: ERRORS
linux-3.19.8-i686: ERRORS
linux-3.19.8-x86_64: ERRORS
linux-4.0.9-i686: ERRORS
linux-4.0.9-x86_64: ERRORS
linux-4.1.51-i686: ERRORS
linux-4.1.51-x86_64: ERRORS
linux-4.2.8-i686: ERRORS
linux-4.2.8-x86_64: ERRORS
linux-4.3.6-i686: ERRORS
linux-4.3.6-x86_64: ERRORS
linux-4.4.109-i686: ERRORS
linux-4.4.109-x86_64: ERRORS
linux-4.5.7-i686: ERRORS
linux-4.5.7-x86_64: ERRORS
linux-4.6.7-i686: ERRORS
linux-4.6.7-x86_64: ERRORS
linux-4.7.10-i686: ERRORS
linux-4.7.10-x86_64: ERRORS
linux-4.8.17-i686: ERRORS
linux-4.8.17-x86_64: ERRORS
linux-4.9.91-i686: ERRORS
linux-4.9.91-x86_64: ERRORS
linux-4.10.17-i686: ERRORS
linux-4.10.17-x86_64: ERRORS
linux-4.11.12-i686: ERRORS
linux-4.11.12-x86_64: ERRORS
linux-4.12.14-i686: ERRORS
linux-4.12.14-x86_64: ERRORS
linux-4.13.16-i686: ERRORS
linux-4.13.16-x86_64: ERRORS
linux-4.14.42-i686: ERRORS
linux-4.14.42-x86_64: ERRORS
linux-4.15.14-i686: ERRORS
linux-4.15.14-x86_64: ERRORS
linux-4.16.8-i686: ERRORS
linux-4.16.8-x86_64: ERRORS
linux-4.17.2-i686: ERRORS
linux-4.17.2-x86_64: ERRORS
linux-4.18-rc1-i686: ERRORS
linux-4.18-rc1-x86_64: ERRORS
apps: OK
spec-git: OK

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

[PATCH v4 0/2] Memory-to-memory media controller topology

[Ezequiel Garcia]

As discussed on IRC, memory-to-memory need to be modeled
properly in order to be supported by the media controller
framework, and thus to support the Request API.

First commit introduces a register/unregister API,
that creates/destroys all the entities and pads needed,
and links them.

The second commit uses this API to support the vim2m driver.

The series applies cleanly on v4.18-rc1.

v4:
  * Add unique entity names (Hans)
  * Add associated interface major and minor (Hans)
  * Remove unused MEM2MEM_ENT_TYPE_MAX  

v3:
  * Remove extra empty line.
  * Remove duplicated entity name set.
  * Reorder sink and source pads as suggested by Hans.
  * Drop unused media macros, we can add them as needed.

v2:
  * Fix compile error when MEDIA_CONTROLLER was not enabled.
  * Fix the 'proc' entity link, which was wrongly connecting
source to source and sink to sink.

Compliance output
=

v4l2-compliance -m 0 -v
v4l2-compliance SHA: 248491682a2919a1bd421f87b33c14125b9fc1f5, 64 bits

Compliance test for device /dev/media0:

Media Driver Info:
Driver name  : vim2m
Model: vim2m
Serial   : 
Bus info : 
Media version: 4.18.0
Hardware revision: 0x (0)
Driver version   : 4.18.0

Required ioctls:
test MEDIA_IOC_DEVICE_INFO: OK

Allow for multiple opens:
test second /dev/media0 open: OK
test MEDIA_IOC_DEVICE_INFO: OK
test for unlimited opens: OK

Media Controller ioctls:
Entity: 0x0001 (Name: 'vim2m-source', Function: V4L2 I/O)
Entity: 0x0003 (Name: 'vim2m-proc', Function: Video Scaler)
Entity: 0x0006 (Name: 'vim2m-sink', Function: V4L2 I/O)
Interface: 0x030c (Type: V4L Video, DevPath: /dev/video2)
Pad: 0x0102 (vim2m-source, Source)
Pad: 0x0104 (vim2m-proc, Sink)
Pad: 0x0105 (vim2m-proc, Source)
Pad: 0x0107 (vim2m-sink, Sink)
Link: 0x0208 (vim2m-source -> vim2m-proc)
Link: 0x020a (vim2m-proc -> vim2m-sink)
Link: 0x020d (vim2m-source to interface /dev/video2)
Link: 0x020e (vim2m-sink to interface /dev/video2)
test MEDIA_IOC_G_TOPOLOGY: OK
Entities: 3 Interfaces: 1 Pads: 4 Links: 4
Entity: 0x0001 (Name: 'vim2m-source', Type: V4L2 I/O 
DevPath: /dev/video2)
Entity: 0x0003 (Name: 'vim2m-proc', Type: Unknown legacy 
device node type (0001) DevPath: /dev/video2)
Entity: 0x0006 (Name: 'vim2m-sink', Type: V4L2 I/O DevPath: 
/dev/video2)
Entity Links: 0x0001 (Name: 'vim2m-source')
Entity Links: 0x0003 (Name: 'vim2m-proc')
Entity Links: 0x0006 (Name: 'vim2m-sink')
test MEDIA_IOC_ENUM_ENTITIES/LINKS: OK
test MEDIA_IOC_SETUP_LINK: OK


Compliance test for device /dev/video2:

Driver Info:
Driver name  : vim2m
Card type: vim2m
Bus info : platform:vim2m
Driver version   : 4.18.0
Capabilities : 0x84208000
Video Memory-to-Memory
Streaming
Extended Pix Format
Device Capabilities
Device Caps  : 0x04208000
Video Memory-to-Memory
Streaming
Extended Pix Format
Media Driver Info:
Driver name  : vim2m
Model: vim2m
Serial   : 
Bus info : 
Media version: 4.18.0
Hardware revision: 0x (0)
Driver version   : 4.18.0
Interface Info:
ID   : 0x030c
Type : V4L Video
Entity Info:
ID   : 0x0001 (1)
Name : vim2m-source
Function : V4L2 I/O
Pad 0x0102   : Source
  Link 0x0208: to remote pad 0x105 of entity 'vim2m-proc': 
Data, Enabled, Immutable

Required ioctls:
test MC information (see 'Media Driver Info' above): OK
test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
test second /dev/video2 open: OK
test VIDIOC_QUERYCAP: OK
test VIDIOC_G/S_PRIORITY: OK
test for unlimited opens: OK

Debug ioctls:
test VIDIOC_DBG_G/S_REGISTER: OK
test VIDIOC_LOG_STATUS: OK (Not Supported)

Input ioctls:
test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
test VIDIOC_ENUMAUDIO: OK (Not Supported)
test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 0 Audio Inputs: 0 Tuners: 0

Output ioctls:
  

[PATCH v4 2/2] vim2m: add media device

[Ezequiel Garcia]

From: Hans Verkuil 

Request API requires a media node. Add one to the vim2m driver so we can
use requests with it.

Signed-off-by: Hans Verkuil 
Signed-off-by: Ezequiel Garcia 
---
 drivers/media/platform/vim2m.c | 41 ++
 1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index 065483e62db4..da13a8927f3f 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -140,6 +140,9 @@ static struct vim2m_fmt *find_format(struct v4l2_format *f)
 struct vim2m_dev {
struct v4l2_device  v4l2_dev;
struct video_device vfd;
+#ifdef CONFIG_MEDIA_CONTROLLER
+   struct media_device mdev;
+#endif
 
atomic_tnum_inst;
struct mutexdev_mutex;
@@ -1016,7 +1019,7 @@ static int vim2m_probe(struct platform_device *pdev)
ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
if (ret) {
v4l2_err(>v4l2_dev, "Failed to register video device\n");
-   goto unreg_dev;
+   goto unreg_v4l2;
}
 
video_set_drvdata(vfd, dev);
@@ -1031,15 +1034,39 @@ static int vim2m_probe(struct platform_device *pdev)
if (IS_ERR(dev->m2m_dev)) {
v4l2_err(>v4l2_dev, "Failed to init mem2mem device\n");
ret = PTR_ERR(dev->m2m_dev);
-   goto err_m2m;
+   goto unreg_dev;
+   }
+
+#ifdef CONFIG_MEDIA_CONTROLLER
+   dev->mdev.dev = >dev;
+   strlcpy(dev->mdev.model, "vim2m", sizeof(dev->mdev.model));
+   media_device_init(>mdev);
+   dev->v4l2_dev.mdev = >mdev;
+
+   ret = v4l2_m2m_register_media_controller(dev->m2m_dev,
+   vfd, MEDIA_ENT_F_PROC_VIDEO_SCALER);
+   if (ret) {
+   v4l2_err(>v4l2_dev, "Failed to init mem2mem media 
controller\n");
+   goto unreg_m2m;
}
 
+   ret = media_device_register(>mdev);
+   if (ret) {
+   v4l2_err(>v4l2_dev, "Failed to register mem2mem media 
device\n");
+   goto unreg_m2m_mc;
+   }
+#endif
return 0;
 
-err_m2m:
+#ifdef CONFIG_MEDIA_CONTROLLER
+unreg_m2m_mc:
+   v4l2_m2m_unregister_media_controller(dev->m2m_dev);
+unreg_m2m:
v4l2_m2m_release(dev->m2m_dev);
-   video_unregister_device(>vfd);
+#endif
 unreg_dev:
+   video_unregister_device(>vfd);
+unreg_v4l2:
v4l2_device_unregister(>v4l2_dev);
 
return ret;
@@ -1050,6 +1077,12 @@ static int vim2m_remove(struct platform_device *pdev)
struct vim2m_dev *dev = platform_get_drvdata(pdev);
 
v4l2_info(>v4l2_dev, "Removing " MEM2MEM_NAME);
+
+#ifdef CONFIG_MEDIA_CONTROLLER
+   media_device_unregister(>mdev);
+   v4l2_m2m_unregister_media_controller(dev->m2m_dev);
+   media_device_cleanup(>mdev);
+#endif
v4l2_m2m_release(dev->m2m_dev);
del_timer_sync(>timer);
video_unregister_device(>vfd);
-- 
2.18.0.rc2

[PATCH v4 1/2] media: add helpers for memory-to-memory media controller

[Ezequiel Garcia]

A memory-to-memory pipeline device consists in three
entities: two DMA engine and one video processing entities.
The DMA engine entities are linked to a V4L interface.

This commit add a new v4l2_m2m_{un}register_media_controller
API to register this topology.

For instance, a typical mem2mem device topology would
look like this:

Device topology
- entity 1: source (1 pad, 1 link)
type Node subtype V4L flags 0
pad0: Source
-> "proc":1 [ENABLED,IMMUTABLE]

- entity 3: proc (2 pads, 2 links)
type Node subtype Unknown flags 0
pad0: Source
-> "sink":0 [ENABLED,IMMUTABLE]
pad1: Sink
<- "source":0 [ENABLED,IMMUTABLE]

- entity 6: sink (1 pad, 1 link)
type Node subtype V4L flags 0
pad0: Sink
<- "proc":0 [ENABLED,IMMUTABLE]

Suggested-by: Laurent Pinchart 
Signed-off-by: Ezequiel Garcia 
Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-dev.c |  13 +-
 drivers/media/v4l2-core/v4l2-mem2mem.c | 186 +
 include/media/v4l2-mem2mem.h   |  19 +++
 3 files changed, 213 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-dev.c 
b/drivers/media/v4l2-core/v4l2-dev.c
index 4ffd7d60a901..c1996d73ca74 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -202,7 +202,7 @@ static void v4l2_device_release(struct device *cd)
mutex_unlock(_lock);
 
 #if defined(CONFIG_MEDIA_CONTROLLER)
-   if (v4l2_dev->mdev) {
+   if (v4l2_dev->mdev && vdev->vfl_dir != VFL_DIR_M2M) {
/* Remove interfaces and interface links */
media_devnode_remove(vdev->intf_devnode);
if (vdev->entity.function != MEDIA_ENT_F_UNKNOWN)
@@ -733,19 +733,22 @@ static void determine_valid_ioctls(struct video_device 
*vdev)
BASE_VIDIOC_PRIVATE);
 }
 
-static int video_register_media_controller(struct video_device *vdev, int type)
+static int video_register_media_controller(struct video_device *vdev)
 {
 #if defined(CONFIG_MEDIA_CONTROLLER)
u32 intf_type;
int ret;
 
-   if (!vdev->v4l2_dev->mdev)
+   /* Memory-to-memory devices are more complex and use
+* their own function to register its mc entities.
+*/
+   if (!vdev->v4l2_dev->mdev || vdev->vfl_dir == VFL_DIR_M2M)
return 0;
 
vdev->entity.obj_type = MEDIA_ENTITY_TYPE_VIDEO_DEVICE;
vdev->entity.function = MEDIA_ENT_F_UNKNOWN;
 
-   switch (type) {
+   switch (vdev->vfl_type) {
case VFL_TYPE_GRABBER:
intf_type = MEDIA_INTF_T_V4L_VIDEO;
vdev->entity.function = MEDIA_ENT_F_IO_V4L;
@@ -993,7 +996,7 @@ int __video_register_device(struct video_device *vdev,
v4l2_device_get(vdev->v4l2_dev);
 
/* Part 5: Register the entity. */
-   ret = video_register_media_controller(vdev, type);
+   ret = video_register_media_controller(vdev);
 
/* Part 6: Activate this minor. The char device can now be used. */
set_bit(V4L2_FL_REGISTERED, >flags);
diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c 
b/drivers/media/v4l2-core/v4l2-mem2mem.c
index c4f963d96a79..60791f36af21 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -17,9 +17,11 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -50,6 +52,17 @@ module_param(debug, bool, 0644);
  * offsets but for different queues */
 #define DST_QUEUE_OFF_BASE (1 << 30)
 
+enum v4l2_m2m_entity_type {
+   MEM2MEM_ENT_TYPE_SOURCE,
+   MEM2MEM_ENT_TYPE_SINK,
+   MEM2MEM_ENT_TYPE_PROC
+};
+
+static const char * const m2m_entity_name[] = {
+   "source",
+   "sink",
+   "proc"
+};
 
 /**
  * struct v4l2_m2m_dev - per-device context
@@ -60,6 +73,15 @@ module_param(debug, bool, 0644);
  */
 struct v4l2_m2m_dev {
struct v4l2_m2m_ctx *curr_ctx;
+#ifdef CONFIG_MEDIA_CONTROLLER
+   struct media_entity *source;
+   struct media_padsource_pad;
+   struct media_entity sink;
+   struct media_padsink_pad;
+   struct media_entity proc;
+   struct media_padproc_pads[2];
+   struct media_intf_devnode *intf_devnode;
+#endif
 
struct list_headjob_queue;
spinlock_t  job_spinlock;
@@ -595,6 +617,170 @@ int v4l2_m2m_mmap(struct file *file, struct v4l2_m2m_ctx 
*m2m_ctx,
 }
 EXPORT_SYMBOL(v4l2_m2m_mmap);
 
+#if defined(CONFIG_MEDIA_CONTROLLER)
+void v4l2_m2m_unregister_media_controller(struct v4l2_m2m_dev *m2m_dev)
+{
+   media_remove_intf_links(_dev->intf_devnode->intf);
+   media_devnode_remove(m2m_dev->intf_devnode);
+
+   media_entity_remove_links(m2m_dev->source);
+   media_entity_remove_links(_dev->sink);
+   media_entity_remove_links(_dev->proc);
+   

Re: [PATCH v4 00/17] v4l2 core: push ioctl lock down to ioctl handler

[Ezequiel Garcia]

On Fri, 2018-06-15 at 16:07 -0300, Ezequiel Garcia wrote:
> Fourth spin of the series posted by Hans:
> 
> https://www.mail-archive.com/linux-media@vger.kernel.org/msg131363.ht
> ml
> 
> There aren't any changes from v3, aside from rebasing
> and re-ordering the patches as requested by Hans.
> 
> See v3 cover letter for more details.
> 
> Series was tested with tw686x, gspca sonixj and UVC devices.
> Build tested with the 0-day kbuild test robot.
> 
> 

AFAIK, nothing is preventing this series.

Hans, is there any other feedback?

Thanks,
Ezequiel

Re: [PATCH 1/2] v4l-helpers: Don't close the fd in {}_s_fd

[Ezequiel Garcia]

Hey Hans,

On Fri, 2018-06-29 at 09:03 +0200, Hans Verkuil wrote:
> On 06/28/2018 09:25 PM, Ezequiel Garcia wrote:
> > When creating a second node via copy or assignment:
> > 
> > node2 = node
> > 
> > The node being assigned to, i.e. node2, obtains the fd.
> > This causes a later call to node2.media_open to close()
> > the fd, thus unintendenly closing the original node fd,
> > via the call path (e.g. for media devices):
> > 
> >   node2.media_open
> >  v4l_media_open
> > v4l_media_s_fd
> > 
> > Similar call paths apply for other device types.
> > Fix this by removing the close in xxx_s_fd.
> 
> I fixed this in a different way by overloading the assignment
> operator
> and calling dup(fd). That solves this as well.
> 

This patch is also needed to prevent the compliance tool
from unintendenly closing a descriptor.

diff --git a/utils/common/v4l-helpers.h b/utils/common/v4l-helpers.h
index 27683a3d286d..45ed997379a1 100644
--- a/utils/common/v4l-helpers.h
+++ b/utils/common/v4l-helpers.h
@@ -118,7 +118,11 @@ static inline int v4l_wrap_open(struct v4l_fd *f,
const char *file, int oflag, .
 
 static inline int v4l_wrap_close(struct v4l_fd *f)
 {
-   return close(f->fd);
+   int ret;
+
+   ret = close(f->fd);
+   f->fd = -1;
+   return ret;
 }
 
 static inline ssize_t v4l_wrap_read(struct v4l_fd *f, void *buffer,
size_t n)

Regards,
Eze

Re: [PATCH 1/2] v4l-helpers: Don't close the fd in {}_s_fd

[Ezequiel Garcia]

On Fri, 2018-06-29 at 09:03 +0200, Hans Verkuil wrote:
> On 06/28/2018 09:25 PM, Ezequiel Garcia wrote:
> > When creating a second node via copy or assignment:
> > 
> > node2 = node
> > 
> > The node being assigned to, i.e. node2, obtains the fd.
> > This causes a later call to node2.media_open to close()
> > the fd, thus unintendenly closing the original node fd,
> > via the call path (e.g. for media devices):
> > 
> >   node2.media_open
> >  v4l_media_open
> > v4l_media_s_fd
> > 
> > Similar call paths apply for other device types.
> > Fix this by removing the close in xxx_s_fd.
> 
> I fixed this in a different way by overloading the assignment
> operator
> and calling dup(fd). That solves this as well.
> 

Yes, but I am now seeing another EBADF error in the compliance run.

close(3)= 0
openat(AT_FDCWD, "/dev/video2", O_RDWR) = 3
close(3)= 0
ioctl(3, VIDIOC_QUERYCAP, 0x7ffe54788794) = -1 EBADF
close(3)= -1 EBADF

Let me see if I can dig it.

Re: [PATCHv5 05/12] media: rename MEDIA_ENT_F_DTV_DECODER to MEDIA_ENT_F_DV_DECODER

[Ezequiel Garcia]

On 29 June 2018 at 08:43, Hans Verkuil  wrote:
> From: Hans Verkuil 
>
> The use of 'DTV' is very confusing since it normally refers to Digital
> TV e.g. DVB etc.
>
> Instead use 'DV' (Digital Video), which nicely corresponds to the
> DV Timings API used to configure such receivers and transmitters.
>
> We keep an alias to avoid breaking userspace applications.
>
> Signed-off-by: Hans Verkuil 
> ---
>  Documentation/media/uapi/mediactl/media-types.rst | 2 +-
>  drivers/media/i2c/adv7604.c   | 1 +
>  drivers/media/i2c/adv7842.c   | 1 +

It would be nice to mention in the commit log
that this patch also sets the function for these drivers.

Regards,
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar

[PATCH 2/2] media: i2c: ov5640: Remove start/stop sequence

[Jacopo Mondi]

From: Jacopo Mondi 

Now that data and clock lanes start in LP11 no need to issue a start/stop
sequence at power on time to 'coax the lanes in LP11 state'.

Signed-off-by: Jacopo Mondi 
---
 drivers/media/i2c/ov5640.c | 14 --
 1 file changed, 14 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 465acce..96d0203 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -1796,20 +1796,6 @@ static int ov5640_set_power(struct ov5640_dev *sensor, 
bool on)
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;
-   }
-
return 0;
}
 
-- 
2.7.4

[PATCH 0/2] media: i2c: ov5640: Re-work MIPI interface configuration

[Jacopo Mondi]

The sensor MIPI interface is initialized with settings that require a start/stop
sequence at power-up time in order to force lanes into LP11 state, as they are
initialized in LP00 when in 'sleep mode' which I assume to be the sensor
manual definition for the D-PHY stop mode.

The stream start/stop was performed by enabling disabling clock gating,
and had the side effect to change the lanes sleep mode configuration when
stream was stopped.

Clock gating/ungating:
-   ret = ov5640_mod_reg(sensor, OV5640_REG_MIPI_CTRL00, BIT(5),
-on ? 0 : BIT(5));
-   if (ret)

Set lanes in LP11 when in 'sleep mode':
-   ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT00,
-  on ? 0x00 : 0x70);

As some failing streaming start sequence have been observed on my testing
platform, the initialization sequence has been re-worked to the following one:

- 0x3019 = 0x70
  Configure data lanes and clock lane to LP11 when in 'sleep mode'
  (assuming this is D-PHY stop state)
- 0x4800 = 0x20
  Gate clock when non transmitting packets, MIPI bus in LP00 when non
  transmitting packets (assuming this is D-PHY LPDT)
- 0x300e = 0x58
  Select 2 lanes mode, power off TX and RX, disable MIPI interface

At stream on time:
- 0x300e = 0x4c: power TX up, enable MIPI interface

At stream off time
- 0x300e = 0x58: power TX down, disable MIPI interface.

Sam Bobrowicz has shared a patch to disable the single lanes at stream
off time and force them in LP00 state to prevent the sensor from
"transmit bad packets during configuration". Sam could you please test this
two patches and verify if you still have to manually disable the single lanes?

It would help validate the theory that 'sleep state' is actually 'stop state'
and having lanes in LP00 instead of LP11 prevents the receiver to detect when
to exit from high-speed mode.

Thanks
   j

Jacopo Mondi (2):
  media: i2c: ov5640: Re-work MIPI start sequence
  media: i2c: ov5640: Remove start/stop sequence

 drivers/media/i2c/ov5640.c | 40 ++--
 1 file changed, 18 insertions(+), 22 deletions(-)

--
2.7.4

[PATCH 1/2] media: i2c: ov5640: Re-work MIPI start sequence

[Jacopo Mondi]

From: Jacopo Mondi 

Change the MIPI CSI-2 interface startup sequence to the following:

Initialization:
0x3019 = 0x70 : Lane1, Lane2 and clock in LP11 when in 'sleep mode'
0x300e = 0x58 : 2 lanes mode, power down TX and RX, MIPI CSI-2 off
0x4800 = 0x20 : Gate clock when not transmitting, LP00 when not transmitting

Stream on:
0x300e = 0x4c : 2 lanes mode, power up TX and enable MIPI

Stream off:
0x300e = 0x58 : 2 lanes mode, power down TX and RX, MIPI CSI-2 off

Signed-off-by: Jacopo Mondi 
---
 drivers/media/i2c/ov5640.c | 26 ++
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 1ecbb7a..465acce 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -259,6 +259,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct 
v4l2_ctrl *ctrl)
 static const struct reg_value ov5640_init_setting_30fps_VGA[] = {
{0x3103, 0x11, 0, 0}, {0x3008, 0x82, 0, 5}, {0x3008, 0x42, 0, 0},
{0x3103, 0x03, 0, 0}, {0x3017, 0x00, 0, 0}, {0x3018, 0x00, 0, 0},
+   {0x3019, 0x70, 0, 0},
{0x3034, 0x18, 0, 0}, {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0},
{0x3037, 0x13, 0, 0}, {0x3630, 0x36, 0, 0},
{0x3631, 0x0e, 0, 0}, {0x3632, 0xe2, 0, 0}, {0x3633, 0x12, 0, 0},
@@ -286,10 +287,10 @@ static const struct reg_value 
ov5640_init_setting_30fps_VGA[] = {
{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x3000, 0x00, 0, 0},
{0x3002, 0x1c, 0, 0}, {0x3004, 0xff, 0, 0}, {0x3006, 0xc3, 0, 0},
-   {0x300e, 0x45, 0, 0}, {0x302e, 0x08, 0, 0}, {0x4300, 0x3f, 0, 0},
+   {0x300e, 0x58, 0, 0}, {0x302e, 0x08, 0, 0}, {0x4300, 0x3f, 0, 0},
{0x501f, 0x00, 0, 0}, {0x4713, 0x03, 0, 0}, {0x4407, 0x04, 0, 0},
{0x440e, 0x00, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-   {0x4837, 0x0a, 0, 0}, {0x4800, 0x04, 0, 0}, {0x3824, 0x02, 0, 0},
+   {0x4837, 0x0a, 0, 0}, {0x4800, 0x20, 0, 0}, {0x3824, 0x02, 0, 0},
{0x5000, 0xa7, 0, 0}, {0x5001, 0xa3, 0, 0}, {0x5180, 0xff, 0, 0},
{0x5181, 0xf2, 0, 0}, {0x5182, 0x00, 0, 0}, {0x5183, 0x14, 0, 0},
{0x5184, 0x25, 0, 0}, {0x5185, 0x24, 0, 0}, {0x5186, 0x09, 0, 0},
@@ -1102,12 +1103,21 @@ static int ov5640_set_stream_mipi(struct ov5640_dev 
*sensor, bool on)
 {
int ret;
 
-   ret = ov5640_mod_reg(sensor, OV5640_REG_MIPI_CTRL00, BIT(5),
-on ? 0 : BIT(5));
-   if (ret)
-   return ret;
-   ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT00,
-  on ? 0x00 : 0x70);
+   /*
+* powerup MIPI TX PHY & enable MIPI
+*
+* 0x4c: [7:5] 010 two lanes MIPI (FIXME: 'debug mode' in manual)
+*   [4]   0 power on MIPI TX
+*   [3]   1 power off MIPI RX
+*   [2]   1 enable MIPI
+*
+* 0x58: [7:5] 010 two lanes MIPI (FIXME: 'debug mode' in manual)
+*   [4]   1 power off MIPI TX
+*   [3]   1 power off MIPI RX
+*   [2]   0 disable MIPI
+*/
+   ret = ov5640_write_reg(sensor,
+  OV5640_REG_IO_MIPI_CTRL00, on ? 0x4c : 0x58);
if (ret)
return ret;
 
-- 
2.7.4

Re: [PATCH 05/22] [media] v4l2-rect.h: add position and equal helpers

[Sakari Ailus]

On Thu, Jun 28, 2018 at 06:20:37PM +0200, Marco Felsch wrote:
> Add two helper functions to check if two rectangles have the same
> position (top/left) and if two rectangles equals (same size and
> same position).
> 
> Signed-off-by: Marco Felsch 

Acked-by: Sakari Ailus 

-- 
Sakari Ailus
sakari.ai...@linux.intel.com

Re: [PATCH v3 03/12] media: ov5640: Remove the clocks registers initialization

[jacopo mondi]

Hi ov5640 people,
I'm happy to finally jump on the bandwagon...

I had a few days to test the sensor driver (on 2 platforms) and unfortunately
this series breaks MIPI capture for me as well..

On Fri, Jun 01, 2018 at 04:05:58PM -0700, Sam Bobrowicz wrote:
> >> On May 21, 2018, at 12:39 AM, Maxime Ripard  
> >> wrote:
> >>
> >>> On Fri, May 18, 2018 at 07:42:34PM -0700, Sam Bobrowicz wrote:
>  On Fri, May 18, 2018 at 3:35 AM, Daniel Mack  wrote:
>  On Thursday, May 17, 2018 10:53 AM, Maxime Ripard wrote:
> 
>  Part of the hardcoded initialization sequence is to set up the proper
>  clock
>  dividers. However, this is now done dynamically through proper code and 
>  as
>  such, the static one is now redundant.
> 
>  Let's remove it.
> 
>  Signed-off-by: Maxime Ripard 
>  ---
> >>>
> >>>
> >>> [...]
> >>>
>  @@ -625,8 +623,8 @@ static const struct reg_value
>  ov5640_setting_30fps_1080P_1920_1080[] = {
>    {0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
>    {0x4001, 0x02, 0, 0}, {0x4004, 0x06, 0, 0}, {0x4713, 0x03, 0, 0},
>    {0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
>  -   {0x3824, 0x02, 0, 0}, {0x5001, 0x83, 0, 0}, {0x3035, 0x11, 0, 0},
>  -   {0x3036, 0x54, 0, 0}, {0x3c07, 0x07, 0, 0}, {0x3c08, 0x00, 0, 0},
>  +   {0x3824, 0x02, 0, 0}, {0x5001, 0x83, 0, 0},
>  +   {0x3c07, 0x07, 0, 0}, {0x3c08, 0x00, 0, 0},
> >>>
> >>>
> >>> This is the mode that I'm testing with. Previously, the hard-coded 
> >>> registers
> >>> here were:
> >>>
> >>> OV5640_REG_SC_PLL_CTRL1 (0x3035) = 0x11
> >>> OV5640_REG_SC_PLL_CTRL2 (0x3036) = 0x54
> >>> OV5640_REG_SC_PLL_CTRL3 (0x3037) = 0x07
> >>>
> >>> Your new code that calculates the clock rates dynamically ends up with
> >>> different values however:
> >>>
> >>> OV5640_REG_SC_PLL_CTRL1 (0x3035) = 0x11
> >>> OV5640_REG_SC_PLL_CTRL2 (0x3036) = 0xa8
> >>> OV5640_REG_SC_PLL_CTRL3 (0x3037) = 0x03
> >>>
> >>> Interestingly, leaving the hard-coded values in the array *and* letting
> >>> ov5640_set_mipi_pclk() do its thing later still works. So again it seems
> >>> that writes to registers after 0x3035/0x3036/0x3037 seem to depend on the
> >>> values of these timing registers. You might need to leave these values as
> >>> dummies in the array. Confusing.
> >>>
> >>> Any idea?
> >>>
> >>>
> >>> Thanks,
> >>> Daniel
> >>
> >> This set of patches is also not working for my MIPI platform (mine has
> >> a 12 MHz external clock). I am pretty sure is isn't working because it
> >> does not include the following, which my tests have found to be
> >> necessary:
> >>
> >> 1) Setting pclk period reg in order to correct DPHY timing.
> >> 2) Disabling of MIPI lanes when streaming not enabled.
> >> 3) setting mipi_div to 1 when the scaler is disabled
> >> 4) Doubling ADC clock on faster resolutions.
> >
> > Yeah, I left them out because I didn't think this was relevant to this
> > patchset but should come as future improvements. However, given that
> > it works with the parallel bus, maybe the two first are needed when
> > adjusting the rate.
> >
> I agree that 1-4 are separate improvements to MIPI mode that may not
> affect all modules. They do break mine, but that has been true since
> the driver was released (mainly because of my 12 MHz clock and more
> stringent CSI RX requirements). So it makes sense for me to address
> them in a follow-up series. But I do think that we should get the
> clock generation a little closer to what I know works for MIPI so we
> don't break things for people that do have MIPI working.
>
> > The mipi divider however seems to be a bit more complicated than you
> > report here. It is indeed set to 1 when the scaler is enabled (all
> > resolutions > 1280 * 960), but it's also set to 4 in some cases
> > (640x480@30, 320x240@30, 176x144@30). I couldn't really find any
> > relationship between the resolution/framerate and whether to use a
> > divider of 2 or 4.
>
> I didn't notice the divide by 4, interesting. I have a theory
> though... it could be that the constraint of PCLK relative to SCLK is:
>
> SCLK*(cpp/scaler ratio)<=PCLK<= ?
> cpp=Components/pixel (1 for JPEG, 2 for YUV, e.g.)
>
> Since the scaler is in auto mode, the scaler ratio might automatically
> change depending on the resolution selected, something like (using int
> math):
>
> (hTotal/hActive) * (vTotal/vActive) = scaler ratio
>
> If SCLK is responsible for reading the data into the scaler, and PCLK
> is responsible for reading data out to the physical interface, this
> would make sense. It will require more experiments to verify any of
> this, though, and unfortunately I don't have a lot of time to put into
> this right now.
>
> On my platform I have also run into an upper bound for PCLK, where it
> seems that PCLK must be <= SCLK when the scaler is enabled. I think
> this may have to do with some of my platform's idiosyncrasies, so I'm
> not 

[GIT PULL for 4.19] ak7375 driver, imx274 improvements

[Sakari Ailus]

Hi Mauro,

Here's a new driver for the ak7375 VCM and improvements for imx274.

Please pull.


The following changes since commit 3c4a737267e89aafa6308c6c456d2ebea3fcd085:

  media: ov5640: fix frame interval enumeration (2018-06-28 09:24:38 -0400)

are available in the git repository at:

  ssh://linuxtv.org/git/sailus/media_tree.git for-4.19-2

for you to fetch changes up to d41713e25e7dd52d7e48228072438715660ebe6f:

  media: imx274: fix typo (2018-06-29 12:33:16 +0300)


Bingbu Cao (2):
  dt-bindings: Add bindings for AKM ak7375 voice coil lens
  media: ak7375: Add ak7375 lens voice coil driver

Luca Ceresoli (6):
  media: imx274: initialize format before v4l2 controls
  media: imx274: consolidate per-mode data in imx274_frmfmt
  media: imx274: get rid of mode_index
  media: imx274: actually use IMX274_DEFAULT_MODE
  media: imx274: simplify imx274_write_table()
  media: imx274: fix typo

 .../devicetree/bindings/media/i2c/ak7375.txt   |   8 +
 MAINTAINERS|   8 +
 drivers/media/i2c/Kconfig  |  10 +
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/ak7375.c | 292 +
 drivers/media/i2c/imx274.c | 194 ++
 6 files changed, 406 insertions(+), 107 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ak7375.txt
 create mode 100644 drivers/media/i2c/ak7375.c

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

Re: [PATCH v3 0/2] Memory-to-memory media controller topology

[Hans Verkuil]

On 06/29/18 09:06, Hans Verkuil wrote:
> On 06/28/2018 09:29 PM, Ezequiel Garcia wrote:



>> Compliance test for device /dev/video2:
>>
>> Driver Info:
>>  Driver name  : vim2m
>>  Card type: vim2m
>>  Bus info : platform:vim2m
>>  Driver version   : 4.18.0
>>  Capabilities : 0x84208000
>>  Video Memory-to-Memory
>>  Streaming
>>  Extended Pix Format
>>  Device Capabilities
>>  Device Caps  : 0x04208000
>>  Video Memory-to-Memory
>>  Streaming
>>  Extended Pix Format
>> Media Driver Info:
>>  Driver name  : vim2m
>>  Model: vim2m
>>  Serial   : 
>>  Bus info : 
>>  Media version: 4.18.0
>>  Hardware revision: 0x (0)
>>  Driver version   : 4.18.0
>> Interface Info:
>>  ID   : 0x030c
>>  Type : V4L Video
>> Entity Info:
>>  ID   : 0x0001 (1)
>>  Name : source
>>  Function : V4L2 I/O
>>  Pad 0x0102   : Source
>>Link 0x0208: to remote pad 0x105 of entity 'proc':
>> Data, Enabled, Immutable
> 
> Hmm, this doesn't show the sink entity associated with this interface.
> 
> It's a v4l2-compliance bug, but I need to think a bit more on how to
> fix this.

Some background: until now an interface was linked to one entity only.
But m2m devices now have two entities linked to the same interface.

So the v4l2-compliance code that outputs this text only sees one entity.
The code needs to be rewritten to be smarter about this.

Something for the (near) future.

Regards,

Hans

[PATCH 1/3] media: coda: jpeg: allow non-JPEG colorspace

[Philipp Zabel]

The hardware codec is not colorspace aware. We should trust userspace to
set the correct colorimetry information on the OUTPUT queue and mirror
the exact same setting on the CAPTURE queue.

There is no reason to restrict colorspace to JPEG for JPEG images, if
userspace injects the correct colorspace information into the JPEG
headers after encoding.

Fixes: b14ac545688d ("[media] coda: improve colorimetry handling")
Signed-off-by: Philipp Zabel 
---
 drivers/media/platform/coda/coda-common.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/media/platform/coda/coda-common.c 
b/drivers/media/platform/coda/coda-common.c
index b86d704ae10c..d951b81ac480 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -569,8 +569,6 @@ static int coda_try_fmt(struct coda_ctx *ctx, const struct 
coda_codec *codec,
f->fmt.pix.height * 2;
break;
case V4L2_PIX_FMT_JPEG:
-   f->fmt.pix.colorspace = V4L2_COLORSPACE_JPEG;
-   /* fallthrough */
case V4L2_PIX_FMT_H264:
case V4L2_PIX_FMT_MPEG4:
case V4L2_PIX_FMT_MPEG2:
-- 
2.17.1

[PATCH 2/3] media: coda: jpeg: only queue two buffers into the bitstream for JPEG on CODA7541

[Philipp Zabel]

Padding the bitstream buffer is not enough to reliably avoid prefetch
failures.  Picture runs with the next buffer's header already visible to
the CODA7541 succeed much more reliably, so always queue two JPEG frames
into the bitstream buffer.

Signed-off-by: Philipp Zabel 
---
 drivers/media/platform/coda/coda-bit.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/coda/coda-bit.c 
b/drivers/media/platform/coda/coda-bit.c
index 68ed2a564ad1..b4ff89200869 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -261,12 +261,13 @@ void coda_fill_bitstream(struct coda_ctx *ctx, struct 
list_head *buffer_list)
 
while (v4l2_m2m_num_src_bufs_ready(ctx->fh.m2m_ctx) > 0) {
/*
-* Only queue a single JPEG into the bitstream buffer, except
-* to increase payload over 512 bytes or if in hold state.
+* Only queue two JPEGs into the bitstream buffer to keep
+* latency low. We need at least one complete buffer and the
+* header of another buffer (for prescan) in the bitstream.
 */
if (ctx->codec->src_fourcc == V4L2_PIX_FMT_JPEG &&
-   (coda_get_bitstream_payload(ctx) >= 512) && !ctx->hold)
-   break;
+   ctx->num_metas > 1)
+   break;
 
src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
 
-- 
2.17.1

[PATCH] media: coda: mark CODA960 firmware version 2.1.9 as supported

[Philipp Zabel]

This patch adds the i.MX6 CODA960 firmware versions 2.1.9
(revision 32515) to the list of supported firmware versions.

Signed-off-by: Philipp Zabel 
---
 drivers/media/platform/coda/coda-bit.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/coda/coda-bit.c 
b/drivers/media/platform/coda/coda-bit.c
index 5d575da3efad..d9432e07aa48 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -722,6 +722,7 @@ static u32 coda_supported_firmwares[] = {
CODA_FIRMWARE_VERNUM(CODA_HX4, 1, 4, 50),
CODA_FIRMWARE_VERNUM(CODA_7541, 1, 4, 50),
CODA_FIRMWARE_VERNUM(CODA_960, 2, 1, 5),
+   CODA_FIRMWARE_VERNUM(CODA_960, 2, 1, 9),
CODA_FIRMWARE_VERNUM(CODA_960, 2, 3, 10),
CODA_FIRMWARE_VERNUM(CODA_960, 3, 1, 1),
 };
-- 
2.17.1

[PATCH 3/3] media: coda: jpeg: explicitly disable thumbnails in SEQ_INIT

[Philipp Zabel]

Explicitly clear DEC_SEQ_JPG_THUMB_EN during sequence initialization.
Not clearing the register does not cause problems, since the only other
codec (MPEG-4 decode) that writes to this register happens to always
write 0 as well.

Signed-off-by: Philipp Zabel 
---
 drivers/media/platform/coda/coda-bit.c  | 2 ++
 drivers/media/platform/coda/coda_regs.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/media/platform/coda/coda-bit.c 
b/drivers/media/platform/coda/coda-bit.c
index b4ff89200869..5d575da3efad 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -1695,6 +1695,8 @@ static int __coda_start_decoding(struct coda_ctx *ctx)
coda_write(dev, 512, CODA_CMD_DEC_SEQ_SPP_CHUNK_SIZE);
}
}
+   if (src_fourcc == V4L2_PIX_FMT_JPEG)
+   coda_write(dev, 0, CODA_CMD_DEC_SEQ_JPG_THUMB_EN);
if (dev->devtype->product != CODA_960)
coda_write(dev, 0, CODA_CMD_DEC_SEQ_SRC_SIZE);
 
diff --git a/drivers/media/platform/coda/coda_regs.h 
b/drivers/media/platform/coda/coda_regs.h
index 3b650b8aabe9..5e7b00a97671 100644
--- a/drivers/media/platform/coda/coda_regs.h
+++ b/drivers/media/platform/coda/coda_regs.h
@@ -157,6 +157,7 @@
 #define CODA_CMD_DEC_SEQ_START_BYTE0x190
 #define CODA_CMD_DEC_SEQ_PS_BB_START   0x194
 #define CODA_CMD_DEC_SEQ_PS_BB_SIZE0x198
+#define CODA_CMD_DEC_SEQ_JPG_THUMB_EN  0x19c
 #define CODA_CMD_DEC_SEQ_MP4_ASP_CLASS 0x19c
 #defineCODA_MP4_CLASS_MPEG40
 #define CODA_CMD_DEC_SEQ_X264_MV_EN0x19c
-- 
2.17.1

[PATCH] media: coda: clear hold flag on streamoff

[Philipp Zabel]

If new buffers are queued after streamoff, the flag will be cleared
anyway, so this is mostly for the purpose of correctness.

Signed-off-by: Philipp Zabel 
---
 drivers/media/platform/coda/coda-common.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/coda/coda-common.c 
b/drivers/media/platform/coda/coda-common.c
index b86d704ae10c..7911cd669fb0 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1634,6 +1634,7 @@ static void coda_stop_streaming(struct vb2_queue *q)
ctx->bitstream.vaddr, ctx->bitstream.size);
ctx->runcounter = 0;
ctx->aborting = 0;
+   ctx->hold = false;
}
 
if (!ctx->streamon_out && !ctx->streamon_cap)
-- 
2.17.1

Re: [PATCHv4 02/10] media-ioc-g-topology.rst: document new 'index' field

[Mauro Carvalho Chehab]

Em Fri, 29 Jun 2018 12:26:50 +0200
Hans Verkuil  escreveu:

> On 06/29/18 11:54, Mauro Carvalho Chehab wrote:
> > Em Thu, 28 Jun 2018 15:12:00 +0200
> > Hans Verkuil  escreveu:
> >   
> >> From: Hans Verkuil 
> >>
> >> Document the new struct media_v2_pad 'index' field.
> >>
> >> Signed-off-by: Hans Verkuil 
> >> Acked-by: Sakari Ailus 
> >> ---
> >>  .../media/uapi/mediactl/media-ioc-g-topology.rst  | 11 +--
> >>  1 file changed, 9 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst 
> >> b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
> >> index a3f259f83b25..24ab34b22df2 100644
> >> --- a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
> >> +++ b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
> >> @@ -176,7 +176,7 @@ desired arrays with the media graph elements.
> >>  *  -  struct media_v2_intf_devnode
> >> -  ``devnode``
> >> -  Used only for device node interfaces. See
> >> -:c:type:`media_v2_intf_devnode` for details..
> >> +:c:type:`media_v2_intf_devnode` for details.
> >>  
> >>  
> >>  .. tabularcolumns:: |p{1.6cm}|p{3.2cm}|p{12.7cm}|
> >> @@ -218,7 +218,14 @@ desired arrays with the media graph elements.
> >> -  Pad flags, see :ref:`media-pad-flag` for more details.
> >>  
> >>  *  -  __u32
> >> -   -  ``reserved``\ [5]
> >> +   -  ``index``
> >> +   -  0-based pad index. Only valid if 
> >> ``MEDIA_V2_PAD_HAS_INDEX(media_version)``
> >> +returns true. The ``media_version`` is defined in struct
> >> +:c:type:`media_device_info` and can be retrieved using
> >> +:ref:`MEDIA_IOC_DEVICE_INFO`.  
> > 
> > "0-based pad index"...
> > 
> > what you mean by that? If what you want to say is that it starts
> > with zero, I would use a clearer text, like:
> > 
> > "Pad index, starting from zero."  
> 
> This text is copied from media-ioc-enum-links.rst. I can rephrase it in both
> cases to this text. Although I don't think '0-based' is unclear, this even
> has its own wiki page: https://en.wikipedia.org/wiki/Zero-based_numbering

Well, it took me a while to understand what it was meant. As this is
a spec file, I prefer to use a more verbose word, in order to make
easier for people to understand what is written there.
> 
> > 
> > It is probably worth saying that the pad index could vary on newer
> > versions of the Kernel.  
> 
> I'll have to think how to phrase this.

Yeah, sure.

> 
> Regards,
> 
>   Hans
> 
> >   
> >> +
> >> +*  -  __u32
> >> +   -  ``reserved``\ [4]
> >> -  Reserved for future extensions. Drivers and applications must 
> >> set
> >>  this array to zero.
> >>
> > 
> > 
> > 
> > Thanks,
> > Mauro
> >   
> 



Thanks,
Mauro

[PATCHv5 04/12] media-ioc-g-topology.rst: document new 'flags' field

[Hans Verkuil]

From: Hans Verkuil 

Document the new struct media_v2_entity 'flags' field.

Signed-off-by: Hans Verkuil 
Acked-by: Sakari Ailus 
---
 .../media/uapi/mediactl/media-ioc-g-topology.rst   | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst 
b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
index bae2b4db89cc..e572dd0d806d 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
@@ -142,7 +142,15 @@ desired arrays with the media graph elements.
-  Entity main function, see :ref:`media-entity-functions` for details.
 
 *  -  __u32
-   -  ``reserved``\ [6]
+   -  ``flags``
+   -  Entity flags, see :ref:`media-entity-flag` for details.
+ Only valid if ``MEDIA_V2_ENTITY_HAS_FLAGS(media_version)``
+ returns true. The ``media_version`` is defined in struct
+ :c:type:`media_device_info` and can be retrieved using
+ :ref:`MEDIA_IOC_DEVICE_INFO`.
+
+*  -  __u32
+   -  ``reserved``\ [5]
-  Reserved for future extensions. Drivers and applications must set
  this array to zero.
 
-- 
2.17.0

[PATCHv5 03/12] media: add flags field to struct media_v2_entity

[Hans Verkuil]

From: Hans Verkuil 

The v2 entity structure never exposed the entity flags, which made it
impossible to detect connector or default entities.

It is really trivial to just expose this information, so implement this.

Signed-off-by: Hans Verkuil 
Acked-by: Sakari Ailus 
---
 drivers/media/media-device.c |  1 +
 include/uapi/linux/media.h   | 12 +++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 047d38372a27..14959b19a342 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -266,6 +266,7 @@ static long media_device_get_topology(struct media_device 
*mdev, void *arg)
memset(, 0, sizeof(kentity));
kentity.id = entity->graph_obj.id;
kentity.function = entity->function;
+   kentity.flags = entity->flags;
strlcpy(kentity.name, entity->name,
sizeof(kentity.name));
 
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index f6338bd57929..ebd2cda67833 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -280,11 +280,21 @@ struct media_links_enum {
  * MC next gen API definitions
  */
 
+/*
+ * Appeared in 4.19.0.
+ *
+ * The media_version argument comes from the media_version field in
+ * struct media_device_info.
+ */
+#define MEDIA_V2_ENTITY_HAS_FLAGS(media_version) \
+   ((media_version) >= ((4 << 16) | (19 << 8) | 0))
+
 struct media_v2_entity {
__u32 id;
char name[64];
__u32 function; /* Main function of the entity */
-   __u32 reserved[6];
+   __u32 flags;
+   __u32 reserved[5];
 } __attribute__ ((packed));
 
 /* Should match the specific fields at media_intf_devnode */
-- 
2.17.0

[PATCHv5 12/12] media-ioc-enum-entities.rst/-g-topology.rst: clarify ID/name usage

[Hans Verkuil]

From: Hans Verkuil 

Mention that IDs should not be hardcoded in applications and that the
entity name must be unique within the media topology.

Signed-off-by: Hans Verkuil 
---
 .../uapi/mediactl/media-ioc-enum-entities.rst |  9 +---
 .../uapi/mediactl/media-ioc-g-topology.rst| 22 ++-
 2 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/Documentation/media/uapi/mediactl/media-ioc-enum-entities.rst 
b/Documentation/media/uapi/mediactl/media-ioc-enum-entities.rst
index 961466ae821d..a4aa7deef690 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-enum-entities.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-enum-entities.rst
@@ -62,15 +62,18 @@ id's until they get an error.
-  ``id``
-
-
-   -  Entity id, set by the application. When the id is or'ed with
+   -  Entity id, set by the application. When the ID is or'ed with
  ``MEDIA_ENT_ID_FLAG_NEXT``, the driver clears the flag and returns
- the first entity with a larger id.
+ the first entity with a larger ID. Do not expect that the ID will
+ always be the same for each instance of the device. In other words,
+ do not hardcode entity IDs in an application.
 
 *  -  char
-  ``name``\ [32]
-
-
-   -  Entity name as an UTF-8 NULL-terminated string.
+   -  Entity name as an UTF-8 NULL-terminated string. This name must be 
unique
+  within the media topology.
 
 *  -  __u32
-  ``type``
diff --git a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst 
b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
index e572dd0d806d..3a5f165d9811 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
@@ -131,11 +131,14 @@ desired arrays with the media graph elements.
 
 *  -  __u32
-  ``id``
-   -  Unique ID for the entity.
+   -  Unique ID for the entity. Do not expect that the ID will
+ always be the same for each instance of the device. In other words,
+ do not hardcode entity IDs in an application.
 
 *  -  char
-  ``name``\ [64]
-   -  Entity name as an UTF-8 NULL-terminated string.
+   -  Entity name as an UTF-8 NULL-terminated string. This name must be 
unique
+  within the media topology.
 
 *  -  __u32
-  ``function``
@@ -166,7 +169,9 @@ desired arrays with the media graph elements.
 
 *  -  __u32
-  ``id``
-   -  Unique ID for the interface.
+   -  Unique ID for the interface. Do not expect that the ID will
+ always be the same for each instance of the device. In other words,
+ do not hardcode interface IDs in an application.
 
 *  -  __u32
-  ``intf_type``
@@ -215,7 +220,9 @@ desired arrays with the media graph elements.
 
 *  -  __u32
-  ``id``
-   -  Unique ID for the pad.
+   -  Unique ID for the pad. Do not expect that the ID will
+ always be the same for each instance of the device. In other words,
+ do not hardcode pad IDs in an application.
 
 *  -  __u32
-  ``entity_id``
@@ -231,7 +238,8 @@ desired arrays with the media graph elements.
  returns true. The ``media_version`` is defined in struct
  :c:type:`media_device_info` and can be retrieved using
  :ref:`MEDIA_IOC_DEVICE_INFO`. Pad indices are stable. If new pads are 
added
- for an entity in the future, then those will be added at the end.
+ for an entity in the future, then those will be added at the end of 
the
+ entity's pad array.
 
 *  -  __u32
-  ``reserved``\ [4]
@@ -250,7 +258,9 @@ desired arrays with the media graph elements.
 
 *  -  __u32
-  ``id``
-   -  Unique ID for the link.
+   -  Unique ID for the link. Do not expect that the ID will
+ always be the same for each instance of the device. In other words,
+ do not hardcode link IDs in an application.
 
 *  -  __u32
-  ``source_id``
-- 
2.17.0

[PATCHv5 10/12] media/i2c: add missing entity functions

[Hans Verkuil]

From: Hans Verkuil 

Several drivers in media/i2c do not set the entity function.
Correct this.

Signed-off-by: Hans Verkuil 
Acked-by: Sakari Ailus 
---
 drivers/media/i2c/et8ek8/et8ek8_driver.c | 1 +
 drivers/media/i2c/mt9m032.c  | 1 +
 drivers/media/i2c/mt9p031.c  | 1 +
 drivers/media/i2c/mt9t001.c  | 1 +
 drivers/media/i2c/mt9v032.c  | 1 +
 5 files changed, 5 insertions(+)

diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c 
b/drivers/media/i2c/et8ek8/et8ek8_driver.c
index e9eff9039ef5..37ef38947e01 100644
--- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
+++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
@@ -1446,6 +1446,7 @@ static int et8ek8_probe(struct i2c_client *client,
sensor->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
sensor->subdev.internal_ops = _internal_ops;
 
+   sensor->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
ret = media_entity_pads_init(>subdev.entity, 1, >pad);
if (ret < 0) {
diff --git a/drivers/media/i2c/mt9m032.c b/drivers/media/i2c/mt9m032.c
index 6a9e068462fd..b385f2b632ad 100644
--- a/drivers/media/i2c/mt9m032.c
+++ b/drivers/media/i2c/mt9m032.c
@@ -793,6 +793,7 @@ static int mt9m032_probe(struct i2c_client *client,
v4l2_ctrl_cluster(2, >hflip);
 
sensor->subdev.ctrl_handler = >ctrls;
+   sensor->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
ret = media_entity_pads_init(>subdev.entity, 1, >pad);
if (ret < 0)
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index 91d822fc4443..715be3632b01 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -,6 +,7 @@ static int mt9p031_probe(struct i2c_client *client,
v4l2_i2c_subdev_init(>subdev, client, _subdev_ops);
mt9p031->subdev.internal_ops = _subdev_internal_ops;
 
+   mt9p031->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
mt9p031->pad.flags = MEDIA_PAD_FL_SOURCE;
ret = media_entity_pads_init(>subdev.entity, 1, >pad);
if (ret < 0)
diff --git a/drivers/media/i2c/mt9t001.c b/drivers/media/i2c/mt9t001.c
index 9d981d9f5686..f683d2cb0486 100644
--- a/drivers/media/i2c/mt9t001.c
+++ b/drivers/media/i2c/mt9t001.c
@@ -943,6 +943,7 @@ static int mt9t001_probe(struct i2c_client *client,
mt9t001->subdev.internal_ops = _subdev_internal_ops;
mt9t001->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 
+   mt9t001->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
mt9t001->pad.flags = MEDIA_PAD_FL_SOURCE;
ret = media_entity_pads_init(>subdev.entity, 1, >pad);
 
diff --git a/drivers/media/i2c/mt9v032.c b/drivers/media/i2c/mt9v032.c
index 4de63b2df334..f74730d24d8f 100644
--- a/drivers/media/i2c/mt9v032.c
+++ b/drivers/media/i2c/mt9v032.c
@@ -1162,6 +1162,7 @@ static int mt9v032_probe(struct i2c_client *client,
mt9v032->subdev.internal_ops = _subdev_internal_ops;
mt9v032->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 
+   mt9v032->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
mt9v032->pad.flags = MEDIA_PAD_FL_SOURCE;
ret = media_entity_pads_init(>subdev.entity, 1, >pad);
if (ret < 0)
-- 
2.17.0

[PATCHv5 11/12] media-ioc-enum-links.rst: improve pad index description

[Hans Verkuil]

From: Hans Verkuil 

Make it clearer that the index starts at 0, and that it won't change
since future new pads will be added at the end.

Signed-off-by: Hans Verkuil 
---
 Documentation/media/uapi/mediactl/media-ioc-enum-links.rst | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/media/uapi/mediactl/media-ioc-enum-links.rst 
b/Documentation/media/uapi/mediactl/media-ioc-enum-links.rst
index 17abdeed1a9c..4cceeb8a6f73 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-enum-links.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-enum-links.rst
@@ -92,7 +92,9 @@ returned during the enumeration process.
 
 *  -  __u16
-  ``index``
-   -  0-based pad index.
+   -  Pad index, starts at 0. Pad indices are stable. If new pads are added
+ for an entity in the future, then those will be added at the end of 
the
+ entity's pad array.
 
 *  -  __u32
-  ``flags``
-- 
2.17.0

[PATCHv5 09/12] adv7180/tvp514x/tvp7002: fix entity function

[Hans Verkuil]

From: Hans Verkuil 

The entity function was ORed with the flags field instead of
assigned to the function field. Correct this.

Signed-off-by: Hans Verkuil 
Acked-by: Sakari Ailus 
---
 drivers/media/i2c/adv7180.c | 2 +-
 drivers/media/i2c/tvp514x.c | 2 +-
 drivers/media/i2c/tvp7002.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 25d24a3f10a7..a727d7f806a1 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -1335,7 +1335,7 @@ static int adv7180_probe(struct i2c_client *client,
goto err_unregister_vpp_client;
 
state->pad.flags = MEDIA_PAD_FL_SOURCE;
-   sd->entity.flags |= MEDIA_ENT_F_ATV_DECODER;
+   sd->entity.function = MEDIA_ENT_F_ATV_DECODER;
ret = media_entity_pads_init(>entity, 1, >pad);
if (ret)
goto err_free_ctrl;
diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
index 6a9890531d01..675b9ae212ab 100644
--- a/drivers/media/i2c/tvp514x.c
+++ b/drivers/media/i2c/tvp514x.c
@@ -1084,7 +1084,7 @@ tvp514x_probe(struct i2c_client *client, const struct 
i2c_device_id *id)
 #if defined(CONFIG_MEDIA_CONTROLLER)
decoder->pad.flags = MEDIA_PAD_FL_SOURCE;
decoder->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
-   decoder->sd.entity.flags |= MEDIA_ENT_F_ATV_DECODER;
+   decoder->sd.entity.function = MEDIA_ENT_F_ATV_DECODER;
 
ret = media_entity_pads_init(>sd.entity, 1, >pad);
if (ret < 0) {
diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c
index 4599b7e28a8d..4f5c627579c7 100644
--- a/drivers/media/i2c/tvp7002.c
+++ b/drivers/media/i2c/tvp7002.c
@@ -1010,7 +1010,7 @@ static int tvp7002_probe(struct i2c_client *c, const 
struct i2c_device_id *id)
 #if defined(CONFIG_MEDIA_CONTROLLER)
device->pad.flags = MEDIA_PAD_FL_SOURCE;
device->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
-   device->sd.entity.flags |= MEDIA_ENT_F_ATV_DECODER;
+   device->sd.entity.function = MEDIA_ENT_F_ATV_DECODER;
 
error = media_entity_pads_init(>sd.entity, 1, >pad);
if (error < 0)
-- 
2.17.0

[PATCHv5 00/12] media/mc: fix inconsistencies

[Hans Verkuil]

From: Hans Verkuil 

This series is v5 of my previous attempt:

https://www.mail-archive.com/linux-media@vger.kernel.org/msg133111.html

The goal is to fix the inconsistencies between the 'old' and 'new' 
MC API. I hate the terms 'old' and 'new', there is nothing wrong IMHO with 
using an 'old' API if it meets the needs of the application.

Making G_TOPOLOGY useful is urgently needed since I think that will be
very helpful for the work we want to do on library support for complex
cameras.

Changes since v4:

- Improve documentation of the new index field in patch 2.
- Added patch 11 to sync the index field documentation in 
media-ioc-enum-links.rst
  with the index field documentation from media-ioc-g-topology.rst.
- Added patch 12 that clarifies that you should not hardcode ID values
  in applications since they can change between instances of the device.
  Also document that the entity name is unique within the media topology.

Changes since v3:

- Renamed MEDIA_ENT_F_DTV_ENCODER to MEDIA_ENT_F_DV_ENCODER
- Added a new patch renaming MEDIA_ENT_F_DTV_DECODER to MEDIA_ENT_F_DV_DECODER.
- Added a new patch that reorders the function defines in media.h so that they
  are in increasing numerical order (the en/decoder functions where not in
  order).
- Added Sakari's Acks (except for the two new patches).

Regards,

Hans

Hans Verkuil (12):
  media: add 'index' to struct media_v2_pad
  media-ioc-g-topology.rst: document new 'index' field
  media: add flags field to struct media_v2_entity
  media-ioc-g-topology.rst: document new 'flags' field
  media: rename MEDIA_ENT_F_DTV_DECODER to MEDIA_ENT_F_DV_DECODER
  media.h: add MEDIA_ENT_F_DV_ENCODER
  media.h: reorder video en/decoder functions
  ad9389b/adv7511: set proper media entity function
  adv7180/tvp514x/tvp7002: fix entity function
  media/i2c: add missing entity functions
  media-ioc-enum-links.rst: improve pad index description
  media-ioc-enum-entities.rst/-g-topology.rst: clarify ID/name usage

 .../uapi/mediactl/media-ioc-enum-entities.rst |  9 ++--
 .../uapi/mediactl/media-ioc-enum-links.rst|  4 +-
 .../uapi/mediactl/media-ioc-g-topology.rst| 42 +++
 .../media/uapi/mediactl/media-types.rst   |  9 +++-
 drivers/media/i2c/ad9389b.c   |  1 +
 drivers/media/i2c/adv7180.c   |  2 +-
 drivers/media/i2c/adv7511.c   |  1 +
 drivers/media/i2c/adv7604.c   |  1 +
 drivers/media/i2c/adv7842.c   |  1 +
 drivers/media/i2c/et8ek8/et8ek8_driver.c  |  1 +
 drivers/media/i2c/mt9m032.c   |  1 +
 drivers/media/i2c/mt9p031.c   |  1 +
 drivers/media/i2c/mt9t001.c   |  1 +
 drivers/media/i2c/mt9v032.c   |  1 +
 drivers/media/i2c/tda1997x.c  |  2 +-
 drivers/media/i2c/tvp514x.c   |  2 +-
 drivers/media/i2c/tvp7002.c   |  2 +-
 drivers/media/media-device.c  |  2 +
 include/uapi/linux/media.h| 39 +
 19 files changed, 97 insertions(+), 25 deletions(-)

-- 
2.17.0

[PATCHv5 02/12] media-ioc-g-topology.rst: document new 'index' field

[Hans Verkuil]

From: Hans Verkuil 

Document the new struct media_v2_pad 'index' field.

Signed-off-by: Hans Verkuil 
Acked-by: Sakari Ailus 
---
 .../media/uapi/mediactl/media-ioc-g-topology.rst | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst 
b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
index a3f259f83b25..bae2b4db89cc 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
@@ -176,7 +176,7 @@ desired arrays with the media graph elements.
 *  -  struct media_v2_intf_devnode
-  ``devnode``
-  Used only for device node interfaces. See
- :c:type:`media_v2_intf_devnode` for details..
+ :c:type:`media_v2_intf_devnode` for details.
 
 
 .. tabularcolumns:: |p{1.6cm}|p{3.2cm}|p{12.7cm}|
@@ -218,7 +218,15 @@ desired arrays with the media graph elements.
-  Pad flags, see :ref:`media-pad-flag` for more details.
 
 *  -  __u32
-   -  ``reserved``\ [5]
+   -  ``index``
+   -  Pad index, starts at 0. Only valid if 
``MEDIA_V2_PAD_HAS_INDEX(media_version)``
+ returns true. The ``media_version`` is defined in struct
+ :c:type:`media_device_info` and can be retrieved using
+ :ref:`MEDIA_IOC_DEVICE_INFO`. Pad indices are stable. If new pads are 
added
+ for an entity in the future, then those will be added at the end.
+
+*  -  __u32
+   -  ``reserved``\ [4]
-  Reserved for future extensions. Drivers and applications must set
  this array to zero.
 
-- 
2.17.0

[PATCHv5 08/12] ad9389b/adv7511: set proper media entity function

[Hans Verkuil]

From: Hans Verkuil 

These two drivers both have function MEDIA_ENT_F_DV_ENCODER.

Signed-off-by: Hans Verkuil 
Acked-by: Sakari Ailus 
---
 drivers/media/i2c/ad9389b.c | 1 +
 drivers/media/i2c/adv7511.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/media/i2c/ad9389b.c b/drivers/media/i2c/ad9389b.c
index 91ff06088572..5b008b0002c0 100644
--- a/drivers/media/i2c/ad9389b.c
+++ b/drivers/media/i2c/ad9389b.c
@@ -1134,6 +1134,7 @@ static int ad9389b_probe(struct i2c_client *client, const 
struct i2c_device_id *
goto err_hdl;
}
state->pad.flags = MEDIA_PAD_FL_SINK;
+   sd->entity.function = MEDIA_ENT_F_DV_ENCODER;
err = media_entity_pads_init(>entity, 1, >pad);
if (err)
goto err_hdl;
diff --git a/drivers/media/i2c/adv7511.c b/drivers/media/i2c/adv7511.c
index 5731751d3f2a..55c2ea0720d9 100644
--- a/drivers/media/i2c/adv7511.c
+++ b/drivers/media/i2c/adv7511.c
@@ -1847,6 +1847,7 @@ static int adv7511_probe(struct i2c_client *client, const 
struct i2c_device_id *
goto err_hdl;
}
state->pad.flags = MEDIA_PAD_FL_SINK;
+   sd->entity.function = MEDIA_ENT_F_DV_ENCODER;
err = media_entity_pads_init(>entity, 1, >pad);
if (err)
goto err_hdl;
-- 
2.17.0

[PATCHv5 07/12] media.h: reorder video en/decoder functions

[Hans Verkuil]

From: Hans Verkuil 

Keep the function defines in numerical order: 0x6000 comes after
0x2000, so move it back.

Signed-off-by: Hans Verkuil 
---
 include/uapi/linux/media.h | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 6f594fa238c2..76d9bd64c116 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -89,13 +89,6 @@ struct media_device_info {
 #define MEDIA_ENT_F_FLASH  (MEDIA_ENT_F_OLD_SUBDEV_BASE + 
2)
 #define MEDIA_ENT_F_LENS   (MEDIA_ENT_F_OLD_SUBDEV_BASE + 
3)
 
-/*
- * Video decoder/encoder functions
- */
-#define MEDIA_ENT_F_ATV_DECODER
(MEDIA_ENT_F_OLD_SUBDEV_BASE + 4)
-#define MEDIA_ENT_F_DV_DECODER (MEDIA_ENT_F_BASE + 0x6001)
-#define MEDIA_ENT_F_DV_ENCODER (MEDIA_ENT_F_BASE + 0x6002)
-
 /*
  * Digital TV, analog TV, radio and/or software defined radio tuner functions.
  *
@@ -140,6 +133,13 @@ struct media_device_info {
 #define MEDIA_ENT_F_VID_MUX(MEDIA_ENT_F_BASE + 0x5001)
 #define MEDIA_ENT_F_VID_IF_BRIDGE  (MEDIA_ENT_F_BASE + 0x5002)
 
+/*
+ * Video decoder/encoder functions
+ */
+#define MEDIA_ENT_F_ATV_DECODER
(MEDIA_ENT_F_OLD_SUBDEV_BASE + 4)
+#define MEDIA_ENT_F_DV_DECODER (MEDIA_ENT_F_BASE + 0x6001)
+#define MEDIA_ENT_F_DV_ENCODER (MEDIA_ENT_F_BASE + 0x6002)
+
 /* Entity flags */
 #define MEDIA_ENT_FL_DEFAULT   (1 << 0)
 #define MEDIA_ENT_FL_CONNECTOR (1 << 1)
-- 
2.17.0

[PATCHv5 01/12] media: add 'index' to struct media_v2_pad

[Hans Verkuil]

From: Hans Verkuil 

The v2 pad structure never exposed the pad index, which made it impossible
to call the MEDIA_IOC_SETUP_LINK ioctl, which needs that information.

It is really trivial to just expose this information, so implement this.

Signed-off-by: Hans Verkuil 
Acked-by: Sakari Ailus 
---
 drivers/media/media-device.c |  1 +
 include/uapi/linux/media.h   | 12 +++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 47bb2254fbfd..047d38372a27 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -331,6 +331,7 @@ static long media_device_get_topology(struct media_device 
*mdev, void *arg)
kpad.id = pad->graph_obj.id;
kpad.entity_id = pad->entity->graph_obj.id;
kpad.flags = pad->flags;
+   kpad.index = pad->index;
 
if (copy_to_user(upad, , sizeof(kpad)))
ret = -EFAULT;
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 86c7dcc9cba3..f6338bd57929 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -305,11 +305,21 @@ struct media_v2_interface {
};
 } __attribute__ ((packed));
 
+/*
+ * Appeared in 4.19.0.
+ *
+ * The media_version argument comes from the media_version field in
+ * struct media_device_info.
+ */
+#define MEDIA_V2_PAD_HAS_INDEX(media_version) \
+   ((media_version) >= ((4 << 16) | (19 << 8) | 0))
+
 struct media_v2_pad {
__u32 id;
__u32 entity_id;
__u32 flags;
-   __u32 reserved[5];
+   __u32 index;
+   __u32 reserved[4];
 } __attribute__ ((packed));
 
 struct media_v2_link {
-- 
2.17.0

[PATCHv5 05/12] media: rename MEDIA_ENT_F_DTV_DECODER to MEDIA_ENT_F_DV_DECODER

[Hans Verkuil]

From: Hans Verkuil 

The use of 'DTV' is very confusing since it normally refers to Digital
TV e.g. DVB etc.

Instead use 'DV' (Digital Video), which nicely corresponds to the
DV Timings API used to configure such receivers and transmitters.

We keep an alias to avoid breaking userspace applications.

Signed-off-by: Hans Verkuil 
---
 Documentation/media/uapi/mediactl/media-types.rst | 2 +-
 drivers/media/i2c/adv7604.c   | 1 +
 drivers/media/i2c/adv7842.c   | 1 +
 drivers/media/i2c/tda1997x.c  | 2 +-
 include/uapi/linux/media.h| 4 +++-
 5 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/Documentation/media/uapi/mediactl/media-types.rst 
b/Documentation/media/uapi/mediactl/media-types.rst
index 96910cf2eaaa..c11b0c7e890b 100644
--- a/Documentation/media/uapi/mediactl/media-types.rst
+++ b/Documentation/media/uapi/mediactl/media-types.rst
@@ -200,7 +200,7 @@ Types and flags used to represent the media graph elements
  MIPI CSI-2, etc.), and outputs them on its source pad to an output
  video bus of another type (eDP, MIPI CSI-2, parallel, etc.).
 
-*  -  ``MEDIA_ENT_F_DTV_DECODER``
+*  -  ``MEDIA_ENT_F_DV_DECODER``
-  Digital video decoder. The basic function of the video decoder is
  to accept digital video from a wide variety of sources
  and output it in some digital video standard, with appropriate
diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 1a3b2c04d9f9..668be2bca57a 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -3499,6 +3499,7 @@ static int adv76xx_probe(struct i2c_client *client,
for (i = 0; i < state->source_pad; ++i)
state->pads[i].flags = MEDIA_PAD_FL_SINK;
state->pads[state->source_pad].flags = MEDIA_PAD_FL_SOURCE;
+   sd->entity.function = MEDIA_ENT_F_DV_DECODER;
 
err = media_entity_pads_init(>entity, state->source_pad + 1,
state->pads);
diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c
index fddac32e5051..99d781343fb1 100644
--- a/drivers/media/i2c/adv7842.c
+++ b/drivers/media/i2c/adv7842.c
@@ -3541,6 +3541,7 @@ static int adv7842_probe(struct i2c_client *client,
INIT_DELAYED_WORK(>delayed_work_enable_hotplug,
adv7842_delayed_work_enable_hotplug);
 
+   sd->entity.function = MEDIA_ENT_F_DV_DECODER;
state->pad.flags = MEDIA_PAD_FL_SOURCE;
err = media_entity_pads_init(>entity, 1, >pad);
if (err)
diff --git a/drivers/media/i2c/tda1997x.c b/drivers/media/i2c/tda1997x.c
index 039a92c3294a..d114ac5243ec 100644
--- a/drivers/media/i2c/tda1997x.c
+++ b/drivers/media/i2c/tda1997x.c
@@ -2570,7 +2570,7 @@ static int tda1997x_probe(struct i2c_client *client,
 id->name, i2c_adapter_id(client->adapter),
 client->addr);
sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
-   sd->entity.function = MEDIA_ENT_F_DTV_DECODER;
+   sd->entity.function = MEDIA_ENT_F_DV_DECODER;
sd->entity.ops = _media_ops;
 
/* set allowed mbus modes based on chip, bus-type, and bus-width */
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index ebd2cda67833..99f5e0978ebb 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -93,7 +93,7 @@ struct media_device_info {
  * Video decoder functions
  */
 #define MEDIA_ENT_F_ATV_DECODER
(MEDIA_ENT_F_OLD_SUBDEV_BASE + 4)
-#define MEDIA_ENT_F_DTV_DECODER(MEDIA_ENT_F_BASE + 
0x6001)
+#define MEDIA_ENT_F_DV_DECODER (MEDIA_ENT_F_BASE + 0x6001)
 
 /*
  * Digital TV, analog TV, radio and/or software defined radio tuner functions.
@@ -400,6 +400,8 @@ struct media_v2_topology {
 #define MEDIA_ENT_T_V4L2_SUBDEV_DECODERMEDIA_ENT_F_ATV_DECODER
 #define MEDIA_ENT_T_V4L2_SUBDEV_TUNER  MEDIA_ENT_F_TUNER
 
+#define MEDIA_ENT_F_DTV_DECODERMEDIA_ENT_F_DV_DECODER
+
 /*
  * There is still no ALSA support in the media controller. These
  * defines should not have been added and we leave them here only
-- 
2.17.0

[PATCHv5 06/12] media.h: add MEDIA_ENT_F_DV_ENCODER

[Hans Verkuil]

From: Hans Verkuil 

Add a new function for digital video encoders such as HDMI transmitters.

Signed-off-by: Hans Verkuil 
Acked-by: Sakari Ailus 
---
 Documentation/media/uapi/mediactl/media-types.rst | 7 +++
 include/uapi/linux/media.h| 3 ++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/media/uapi/mediactl/media-types.rst 
b/Documentation/media/uapi/mediactl/media-types.rst
index c11b0c7e890b..e90d4d0a7f8b 100644
--- a/Documentation/media/uapi/mediactl/media-types.rst
+++ b/Documentation/media/uapi/mediactl/media-types.rst
@@ -206,6 +206,13 @@ Types and flags used to represent the media graph elements
  and output it in some digital video standard, with appropriate
  timing signals.
 
+*  -  ``MEDIA_ENT_F_DV_ENCODER``
+   -  Digital video encoder. The basic function of the video encoder is
+ to accept digital video from some digital video standard with
+ appropriate timing signals (usually a parallel video bus with sync
+ signals) and output this to a digital video output connector such
+ as HDMI or DisplayPort.
+
 ..  tabularcolumns:: |p{5.5cm}|p{12.0cm}|
 
 .. _media-entity-flag:
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 99f5e0978ebb..6f594fa238c2 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -90,10 +90,11 @@ struct media_device_info {
 #define MEDIA_ENT_F_LENS   (MEDIA_ENT_F_OLD_SUBDEV_BASE + 
3)
 
 /*
- * Video decoder functions
+ * Video decoder/encoder functions
  */
 #define MEDIA_ENT_F_ATV_DECODER
(MEDIA_ENT_F_OLD_SUBDEV_BASE + 4)
 #define MEDIA_ENT_F_DV_DECODER (MEDIA_ENT_F_BASE + 0x6001)
+#define MEDIA_ENT_F_DV_ENCODER (MEDIA_ENT_F_BASE + 0x6002)
 
 /*
  * Digital TV, analog TV, radio and/or software defined radio tuner functions.
-- 
2.17.0

Re: Software-only image processing for Intel "complex" cameras

[Pavel Machek]

Hi!

> > > > On Nokia N900, I have similar problems as Intel IPU3 hardware.
> > > >
> > > > Meeting notes say that pure software implementation is not fast
> > > > enough, but that it may be useful for debugging. It would be also
> > > > useful for me on N900, and probably useful for processing "raw" images
> > > > from digital cameras.
> > > >
> > > > There is sensor part, and memory-to-memory part, right? What is
> > > > the format of data from the sensor part? What operations would be
> > > > expensive on the CPU? If we did everthing on the CPU, what would be
> > > > maximum resolution where we could still manage it in real time?
> > >
> > > We can still use the memory-to-memory part (IMGU), even without 3A. It
> > > would just do demosaicing at default parameters and give us a YUV
> > > (NV12) frame. We could use some software component to analyze the YUV
> > > output and adjust sensor parameters accordingly. Possibly the part we
> > > already have in libv4l2 could just work?
> >
> > As soon as you get YUV, yes, libv4l2 should be able to work with that.
> >
> > OTOH using the memory-to-memory part is going to be tricky.
> 
> Why? I don't see any reason why it would be tricky. You just feed the
> right CAPTURE node with YUV buffers and the right OUTPUT node with raw
> buffers and that should work.

I have seen insides of libv4l2. I believe unpacking by hand will be
simpler / less tricky than passing buffers around.

Of course, long term both will be needed.

> > What
> > format is the data before demosaicing? Something common like BGGR10?
> 
> It's documented in detail in V4L2 documentation:
> > https://www.kernel.org/doc/html/latest/media/uapi/v4l/pixfmt-srggb10-ipu3.html

Ah, thanks for pointer. That's not too bad.

> > > The expensive operation would be analyzing the frame itself. I suppose
> > > you need to build some histogram representing brightness and white
> > > balance of the frame and then infer necessary sensor adjustments from
> > > that.
> >
> > That does not really have to be expensive. You can sample ... say
> > 1 pixels from the image, and get good-enough data for 3A.
> 
> Yes, but you need to do it relatively frequently to react for scene
> changes and also even 1 pixels (depending on distribution
> ) might mean fetching 1*cacheline bytes of data.

Yeah, there are tradeoffs...

> > > > Would it be possible to get access to machine with IPU3, or would
> > > > there be someone willing to test libv4l2 patches?
> > >
> > > I should be able to help with some basic testing, preferably limited
> > > to command line tools (but I might be able to create a test
> > > environment for X11 tools if really necessary).
> >
> > Could you just compile libv4l2 with sdlcam demo on the target, and
> > then ssh -X there from some sort of reasonable system?
> 
> Yes, that should work I guess. That would be a Chrome OS device (which
> doesn't include X11), so that would mean compiling some X11 libs (and
> probably some more dependencies) as well, but that's not impossible.

I believe you want to put Debian chroot in there... or get machine
with IPU3 where you can install Debian directly.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCHv4 02/10] media-ioc-g-topology.rst: document new 'index' field

[Hans Verkuil]

On 06/29/18 11:54, Mauro Carvalho Chehab wrote:
> Em Thu, 28 Jun 2018 15:12:00 +0200
> Hans Verkuil  escreveu:
> 
>> From: Hans Verkuil 
>>
>> Document the new struct media_v2_pad 'index' field.
>>
>> Signed-off-by: Hans Verkuil 
>> Acked-by: Sakari Ailus 
>> ---
>>  .../media/uapi/mediactl/media-ioc-g-topology.rst  | 11 +--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst 
>> b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
>> index a3f259f83b25..24ab34b22df2 100644
>> --- a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
>> +++ b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
>> @@ -176,7 +176,7 @@ desired arrays with the media graph elements.
>>  *  -  struct media_v2_intf_devnode
>> -  ``devnode``
>> -  Used only for device node interfaces. See
>> -  :c:type:`media_v2_intf_devnode` for details..
>> +  :c:type:`media_v2_intf_devnode` for details.
>>  
>>  
>>  .. tabularcolumns:: |p{1.6cm}|p{3.2cm}|p{12.7cm}|
>> @@ -218,7 +218,14 @@ desired arrays with the media graph elements.
>> -  Pad flags, see :ref:`media-pad-flag` for more details.
>>  
>>  *  -  __u32
>> -   -  ``reserved``\ [5]
>> +   -  ``index``
>> +   -  0-based pad index. Only valid if 
>> ``MEDIA_V2_PAD_HAS_INDEX(media_version)``
>> +  returns true. The ``media_version`` is defined in struct
>> +  :c:type:`media_device_info` and can be retrieved using
>> +  :ref:`MEDIA_IOC_DEVICE_INFO`.
> 
> "0-based pad index"...
> 
> what you mean by that? If what you want to say is that it starts
> with zero, I would use a clearer text, like:
> 
> "Pad index, starting from zero."

This text is copied from media-ioc-enum-links.rst. I can rephrase it in both
cases to this text. Although I don't think '0-based' is unclear, this even
has its own wiki page: https://en.wikipedia.org/wiki/Zero-based_numbering

> 
> It is probably worth saying that the pad index could vary on newer
> versions of the Kernel.

I'll have to think how to phrase this.

Regards,

Hans

> 
>> +
>> +*  -  __u32
>> +   -  ``reserved``\ [4]
>> -  Reserved for future extensions. Drivers and applications must set
>>this array to zero.
>>  
> 
> 
> 
> Thanks,
> Mauro
> 

[PATCH] v4l2-ctrls.c: fix broken auto cluster handling

[Hans Verkuil]

When you switch from auto to manual mode for an auto-cluster (e.g.
autogain+gain controls), then the current HW value has to be copied
to the current control value. However, has_changed was never set to
true, so new_to_cur didn't actually copy this value.

Signed-off-by: Hans Verkuil 
Reported-by: Hugues FRUCHET 
---
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index d29e45516eb7..d1087573da34 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -3137,9 +3137,22 @@ static int try_or_set_cluster(struct v4l2_fh *fh, struct 
v4l2_ctrl *master,

/* If OK, then make the new values permanent. */
update_flag = is_cur_manual(master) != is_new_manual(master);
-   for (i = 0; i < master->ncontrols; i++)
+
+   for (i = 0; i < master->ncontrols; i++) {
+   /*
+* If we switch from auto to manual mode, and this cluster
+* contains volatile controls, then all non-master controls
+* have to be marked as changed. The 'new' value contains
+* the volatile value (obtained by update_from_auto_cluster),
+* which now has to become the current value.
+*/
+   if (i && update_flag && is_new_manual(master) &&
+   master->has_volatiles && master->cluster[i])
+   master->cluster[i]->has_changed = true;
+
new_to_cur(fh, master->cluster[i], ch_flags |
((update_flag && i > 0) ? V4L2_EVENT_CTRL_CH_FLAGS : 
0));
+   }
return 0;
 }

Re: [PATCHv4 00/10] media/mc: fix inconsistencies

[Mauro Carvalho Chehab]

Em Thu, 28 Jun 2018 15:11:58 +0200
Hans Verkuil  escreveu:

> From: Hans Verkuil 
> 
> This series is v4 of my previous attempt:
> 
> https://www.mail-archive.com/linux-media@vger.kernel.org/msg132942.html

Patches 1 and 3-10 looks OK to me.

Thanks,
Mauro

Re: [PATCHv4 02/10] media-ioc-g-topology.rst: document new 'index' field

[Mauro Carvalho Chehab]

Em Thu, 28 Jun 2018 15:12:00 +0200
Hans Verkuil  escreveu:

> From: Hans Verkuil 
> 
> Document the new struct media_v2_pad 'index' field.
> 
> Signed-off-by: Hans Verkuil 
> Acked-by: Sakari Ailus 
> ---
>  .../media/uapi/mediactl/media-ioc-g-topology.rst  | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst 
> b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
> index a3f259f83b25..24ab34b22df2 100644
> --- a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
> +++ b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
> @@ -176,7 +176,7 @@ desired arrays with the media graph elements.
>  *  -  struct media_v2_intf_devnode
> -  ``devnode``
> -  Used only for device node interfaces. See
> -   :c:type:`media_v2_intf_devnode` for details..
> +   :c:type:`media_v2_intf_devnode` for details.
>  
>  
>  .. tabularcolumns:: |p{1.6cm}|p{3.2cm}|p{12.7cm}|
> @@ -218,7 +218,14 @@ desired arrays with the media graph elements.
> -  Pad flags, see :ref:`media-pad-flag` for more details.
>  
>  *  -  __u32
> -   -  ``reserved``\ [5]
> +   -  ``index``
> +   -  0-based pad index. Only valid if 
> ``MEDIA_V2_PAD_HAS_INDEX(media_version)``
> +   returns true. The ``media_version`` is defined in struct
> +   :c:type:`media_device_info` and can be retrieved using
> +   :ref:`MEDIA_IOC_DEVICE_INFO`.

"0-based pad index"...

what you mean by that? If what you want to say is that it starts
with zero, I would use a clearer text, like:

"Pad index, starting from zero."

It is probably worth saying that the pad index could vary on newer
versions of the Kernel.

> +
> +*  -  __u32
> +   -  ``reserved``\ [4]
> -  Reserved for future extensions. Drivers and applications must set
> this array to zero.
>  



Thanks,
Mauro

[PATCH] vivid: fix gain when autogain is on

[Hans Verkuil]

In the vivid driver you want gain to continuous change while autogain
is on. However, dev->jiffies_vid_cap doesn't actually change. It probably
did in the past, but changes in the code caused this to be a fixed value
that is only set when you start streaming.

Replace it by jiffies, which is always changing.

Signed-off-by: Hans Verkuil 
---
diff --git a/drivers/media/platform/vivid/vivid-ctrls.c 
b/drivers/media/platform/vivid/vivid-ctrls.c
index 6b0bfa091592..6eb8ad7fb12c 100644
--- a/drivers/media/platform/vivid/vivid-ctrls.c
+++ b/drivers/media/platform/vivid/vivid-ctrls.c
@@ -295,7 +295,7 @@ static int vivid_user_vid_g_volatile_ctrl(struct v4l2_ctrl 
*ctrl)

switch (ctrl->id) {
case V4L2_CID_AUTOGAIN:
-   dev->gain->val = dev->jiffies_vid_cap & 0xff;
+   dev->gain->val = (jiffies / HZ) & 0xff;
break;
}
return 0;

Re: Software-only image processing for Intel "complex" cameras

[Tomasz Figa]

Hi Pavel,

On Fri, Jun 29, 2018 at 6:18 PM Pavel Machek  wrote:
>
> Hi!
>
> > > On Nokia N900, I have similar problems as Intel IPU3 hardware.
> > >
> > > Meeting notes say that pure software implementation is not fast
> > > enough, but that it may be useful for debugging. It would be also
> > > useful for me on N900, and probably useful for processing "raw" images
> > > from digital cameras.
> > >
> > > There is sensor part, and memory-to-memory part, right? What is
> > > the format of data from the sensor part? What operations would be
> > > expensive on the CPU? If we did everthing on the CPU, what would be
> > > maximum resolution where we could still manage it in real time?
> >
> > We can still use the memory-to-memory part (IMGU), even without 3A. It
> > would just do demosaicing at default parameters and give us a YUV
> > (NV12) frame. We could use some software component to analyze the YUV
> > output and adjust sensor parameters accordingly. Possibly the part we
> > already have in libv4l2 could just work?
>
> As soon as you get YUV, yes, libv4l2 should be able to work with that.
>
> OTOH using the memory-to-memory part is going to be tricky.

Why? I don't see any reason why it would be tricky. You just feed the
right CAPTURE node with YUV buffers and the right OUTPUT node with raw
buffers and that should work.

> What
> format is the data before demosaicing? Something common like BGGR10?

It's documented in detail in V4L2 documentation:
https://www.kernel.org/doc/html/latest/media/uapi/v4l/pixfmt-srggb10-ipu3.html

>
> > The expensive operation would be analyzing the frame itself. I suppose
> > you need to build some histogram representing brightness and white
> > balance of the frame and then infer necessary sensor adjustments from
> > that.
>
> That does not really have to be expensive. You can sample ... say
> 1 pixels from the image, and get good-enough data for 3A.

Yes, but you need to do it relatively frequently to react for scene
changes and also even 1 pixels (depending on distribution
) might mean fetching 1*cacheline bytes of data.

> > > Would it be possible to get access to machine with IPU3, or would
> > > there be someone willing to test libv4l2 patches?
> >
> > I should be able to help with some basic testing, preferably limited
> > to command line tools (but I might be able to create a test
> > environment for X11 tools if really necessary).
>
> Could you just compile libv4l2 with sdlcam demo on the target, and
> then ssh -X there from some sort of reasonable system?

Yes, that should work I guess. That would be a Chrome OS device (which
doesn't include X11), so that would mean compiling some X11 libs (and
probably some more dependencies) as well, but that's not impossible.

Best regards,
Tomasz

Re: Software-only image processing for Intel "complex" cameras

[Pavel Machek]

Hi!

> > On Nokia N900, I have similar problems as Intel IPU3 hardware.
> >
> > Meeting notes say that pure software implementation is not fast
> > enough, but that it may be useful for debugging. It would be also
> > useful for me on N900, and probably useful for processing "raw" images
> > from digital cameras.
> >
> > There is sensor part, and memory-to-memory part, right? What is
> > the format of data from the sensor part? What operations would be
> > expensive on the CPU? If we did everthing on the CPU, what would be
> > maximum resolution where we could still manage it in real time?
> 
> We can still use the memory-to-memory part (IMGU), even without 3A. It
> would just do demosaicing at default parameters and give us a YUV
> (NV12) frame. We could use some software component to analyze the YUV
> output and adjust sensor parameters accordingly. Possibly the part we
> already have in libv4l2 could just work?

As soon as you get YUV, yes, libv4l2 should be able to work with that.

OTOH using the memory-to-memory part is going to be tricky. What
format is the data before demosaicing? Something common like BGGR10?

> The expensive operation would be analyzing the frame itself. I suppose
> you need to build some histogram representing brightness and white
> balance of the frame and then infer necessary sensor adjustments from
> that.

That does not really have to be expensive. You can sample ... say
1 pixels from the image, and get good-enough data for 3A.

> > Would it be possible to get access to machine with IPU3, or would
> > there be someone willing to test libv4l2 patches?
> 
> I should be able to help with some basic testing, preferably limited
> to command line tools (but I might be able to create a test
> environment for X11 tools if really necessary).

Could you just compile libv4l2 with sdlcam demo on the target, and
then ssh -X there from some sort of reasonable system?

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

[PATCHv2] v4l2-mem2mem: set name and major/minor from video_device

[Hans Verkuil]

Codec devices have two m2m devices in the media topology: one
for decoding, one for encoding. Since the entity names were the
same for both, this was invalid. Also the major/minor numbers
were not set for the I/O entities.

Signed-off-by: Hans Verkuil 
---

Ezequiel, just fold this in your own patch. With this change in place everything
looks good to me.

PS: don't forget to remove the MEM2MEM_ENT_TYPE_MAX in the enum.

Regards,

Hans

---
 drivers/media/v4l2-core/v4l2-mem2mem.c | 27 +-
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c 
b/drivers/media/v4l2-core/v4l2-mem2mem.c
index 40d4d645e6a6..e8564c39f7d4 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -647,15 +647,20 @@ void v4l2_m2m_unregister_media_controller(struct 
v4l2_m2m_dev *m2m_dev)
media_device_unregister_entity(m2m_dev->source);
media_device_unregister_entity(_dev->sink);
media_device_unregister_entity(_dev->proc);
+   kfree(m2m_dev->source->name);
+   kfree(m2m_dev->sink.name);
+   kfree(m2m_dev->proc.name);
 }
 EXPORT_SYMBOL_GPL(v4l2_m2m_unregister_media_controller);

 static int v4l2_m2m_register_entity(struct media_device *mdev,
struct v4l2_m2m_dev *m2m_dev, enum v4l2_m2m_entity_type type,
-   int function)
+   struct video_device *vdev, int function)
 {
struct media_entity *entity;
struct media_pad *pads;
+   char *name;
+   unsigned int len;
int num_pads;
int ret;

@@ -671,6 +676,8 @@ static int v4l2_m2m_register_entity(struct media_device 
*mdev,
pads = _dev->sink_pad;
pads[0].flags = MEDIA_PAD_FL_SINK;
num_pads = 1;
+   entity->info.dev.major = m2m_dev->source->info.dev.major;
+   entity->info.dev.minor = m2m_dev->source->info.dev.minor;
break;
case MEM2MEM_ENT_TYPE_PROC:
entity = _dev->proc;
@@ -684,7 +691,14 @@ static int v4l2_m2m_register_entity(struct media_device 
*mdev,
}

entity->obj_type = MEDIA_ENTITY_TYPE_BASE;
-   entity->name = m2m_entity_name[type];
+   entity->info.dev.major = VIDEO_MAJOR;
+   entity->info.dev.minor = vdev->minor;
+   len = strlen(vdev->name) + 2 + strlen(m2m_entity_name[type]);
+   name = kmalloc(len, GFP_KERNEL);
+   if (!name)
+   return -ENOMEM;
+   snprintf(name, len, "%s-%s", vdev->name, m2m_entity_name[type]);
+   entity->name = name;
entity->function = function;

ret = media_entity_pads_init(entity, num_pads, pads);
@@ -715,15 +729,15 @@ int v4l2_m2m_register_media_controller(struct 
v4l2_m2m_dev *m2m_dev,
/* Create the three entities with their pads */
m2m_dev->source = >entity;
ret = v4l2_m2m_register_entity(mdev, m2m_dev,
-   MEM2MEM_ENT_TYPE_SOURCE, MEDIA_ENT_F_IO_V4L);
+   MEM2MEM_ENT_TYPE_SOURCE, vdev, MEDIA_ENT_F_IO_V4L);
if (ret)
return ret;
ret = v4l2_m2m_register_entity(mdev, m2m_dev,
-   MEM2MEM_ENT_TYPE_PROC, function);
+   MEM2MEM_ENT_TYPE_PROC, vdev, function);
if (ret)
goto err_rel_entity0;
ret = v4l2_m2m_register_entity(mdev, m2m_dev,
-   MEM2MEM_ENT_TYPE_SINK, MEDIA_ENT_F_IO_V4L);
+   MEM2MEM_ENT_TYPE_SINK, vdev, MEDIA_ENT_F_IO_V4L);
if (ret)
goto err_rel_entity1;

@@ -774,10 +788,13 @@ int v4l2_m2m_register_media_controller(struct 
v4l2_m2m_dev *m2m_dev,
media_entity_remove_links(m2m_dev->source);
 err_rel_entity2:
media_device_unregister_entity(_dev->proc);
+   kfree(m2m_dev->proc.name);
 err_rel_entity1:
media_device_unregister_entity(_dev->sink);
+   kfree(m2m_dev->sink.name);
 err_rel_entity0:
media_device_unregister_entity(m2m_dev->source);
+   kfree(m2m_dev->source->name);
return ret;
return 0;
 }
-- 
2.17.0

Re: [PATCH] v4l2-mem2mem: add name argument to v4l2_m2m_register_media_controller()

[Hans Verkuil]

On 06/29/2018 09:01 AM, Hans Verkuil wrote:
> Hi Ezequiel,
> 
> I added support for this to a codec and I discovered that we are missing a 
> 'name'
> argument to v4l2_m2m_register_media_controller(): a typical codec driver has 
> two
> m2m video nodes: one for encoding, one for decoding. That works fine, except 
> that
> the names of the source, sink and proc entities are the same for both encoder 
> and
> decoder node.
> 
> So add an extra name argument to help differentiate between the two.
> 
> Can you fold this in your v4l2-mem2mem patch?

Please ignore, I'll post a follow-up patch since this isn't sufficient.

Regards,

Hans

> 
> Thanks!
> 
>   Hans
> 
> Signed-off-by: Hans Verkuil 
> ---
> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c 
> b/drivers/media/v4l2-core/v4l2-mem2mem.c
> index 40d4d645e6a6..b3ecd5a41c48 100644
> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> @@ -76,10 +76,13 @@ struct v4l2_m2m_dev {
>  #ifdef CONFIG_MEDIA_CONTROLLER
>   struct media_entity *source;
>   struct media_padsource_pad;
> + char*source_name;
>   struct media_entity sink;
>   struct media_padsink_pad;
> + char*sink_name;
>   struct media_entity proc;
>   struct media_padproc_pads[2];
> + char*proc_name;
>   struct media_intf_devnode *intf_devnode;
>  #endif
> 
> @@ -647,33 +650,41 @@ void v4l2_m2m_unregister_media_controller(struct 
> v4l2_m2m_dev *m2m_dev)
>   media_device_unregister_entity(m2m_dev->source);
>   media_device_unregister_entity(_dev->sink);
>   media_device_unregister_entity(_dev->proc);
> + kfree(m2m_dev->source_name);
> + kfree(m2m_dev->sink_name);
> + kfree(m2m_dev->proc_name);
>  }
>  EXPORT_SYMBOL_GPL(v4l2_m2m_unregister_media_controller);
> 
>  static int v4l2_m2m_register_entity(struct media_device *mdev,
>   struct v4l2_m2m_dev *m2m_dev, enum v4l2_m2m_entity_type type,
> - int function)
> + const char *name, int function)
>  {
>   struct media_entity *entity;
>   struct media_pad *pads;
> + char **p_name;
> + unsigned int len;
>   int num_pads;
>   int ret;
> 
>   switch (type) {
>   case MEM2MEM_ENT_TYPE_SOURCE:
>   entity = m2m_dev->source;
> + p_name = _dev->source_name;
>   pads = _dev->source_pad;
>   pads[0].flags = MEDIA_PAD_FL_SOURCE;
>   num_pads = 1;
>   break;
>   case MEM2MEM_ENT_TYPE_SINK:
>   entity = _dev->sink;
> + p_name = _dev->sink_name;
>   pads = _dev->sink_pad;
>   pads[0].flags = MEDIA_PAD_FL_SINK;
>   num_pads = 1;
>   break;
>   case MEM2MEM_ENT_TYPE_PROC:
>   entity = _dev->proc;
> + p_name = _dev->proc_name;
>   pads = m2m_dev->proc_pads;
>   pads[0].flags = MEDIA_PAD_FL_SINK;
>   pads[1].flags = MEDIA_PAD_FL_SOURCE;
> @@ -683,8 +694,11 @@ static int v4l2_m2m_register_entity(struct media_device 
> *mdev,
>   return -EINVAL;
>   }
> 
> + len = strlen(name) + 2 + strlen(m2m_entity_name[type]);
>   entity->obj_type = MEDIA_ENTITY_TYPE_BASE;
> - entity->name = m2m_entity_name[type];
> + *p_name = kmalloc(len, GFP_KERNEL);
> + snprintf(*p_name, len, "%s-%s", name, m2m_entity_name[type]);
> + entity->name = *p_name;
>   entity->function = function;
> 
>   ret = media_entity_pads_init(entity, num_pads, pads);
> @@ -698,7 +712,8 @@ static int v4l2_m2m_register_entity(struct media_device 
> *mdev,
>  }
> 
>  int v4l2_m2m_register_media_controller(struct v4l2_m2m_dev *m2m_dev,
> - struct video_device *vdev, int function)
> + struct video_device *vdev, const char *name,
> + int function)
>  {
>   struct media_device *mdev = vdev->v4l2_dev->mdev;
>   struct media_link *link;
> @@ -715,15 +730,15 @@ int v4l2_m2m_register_media_controller(struct 
> v4l2_m2m_dev *m2m_dev,
>   /* Create the three entities with their pads */
>   m2m_dev->source = >entity;
>   ret = v4l2_m2m_register_entity(mdev, m2m_dev,
> - MEM2MEM_ENT_TYPE_SOURCE, MEDIA_ENT_F_IO_V4L);
> + MEM2MEM_ENT_TYPE_SOURCE, name, MEDIA_ENT_F_IO_V4L);
>   if (ret)
>   return ret;
>   ret = v4l2_m2m_register_entity(mdev, m2m_dev,
> - MEM2MEM_ENT_TYPE_PROC, function);
> + MEM2MEM_ENT_TYPE_PROC, name, function);
>   if (ret)
>   goto err_rel_entity0;
>   ret = v4l2_m2m_register_entity(mdev, m2m_dev,
> - MEM2MEM_ENT_TYPE_SINK, MEDIA_ENT_F_IO_V4L);
> + MEM2MEM_ENT_TYPE_SINK, name, MEDIA_ENT_F_IO_V4L);
>   if (ret)
>   goto err_rel_entity1;
> 
> diff --git 

Re: [PATCH v3 0/2] Memory-to-memory media controller topology

[Hans Verkuil]

On 06/28/2018 09:29 PM, Ezequiel Garcia wrote:
> Hi Hans,
> 
> On Thu, 2018-06-28 at 11:19 +0200, Hans Verkuil wrote:
>> On 06/27/18 22:35, Ezequiel Garcia wrote:
>>> As discussed on IRC, memory-to-memory need to be modeled
>>> properly in order to be supported by the media controller
>>> framework, and thus to support the Request API.
>>>
>>> First commit introduces a register/unregister API,
>>> that creates/destroys all the entities and pads needed,
>>> and links them.
>>>
>>> The second commit uses this API to support the vim2m driver.
>>>
>>> The series applies cleanly on v4.18-rc1.
>>>
>>> Topology (media-ctl -p output)
>>> ==
>>>
>>> media-ctl -p
>>> Media controller API version 4.17.0
>>>
>>> Media device information
>>> 
>>> driver  vim2m
>>> model   vim2m
>>> serial  
>>> bus info
>>> hw revision 0x0
>>> driver version  4.17.0
>>>
>>> Device topology
>>> - entity 1: source (1 pad, 1 link)
>>> type Node subtype V4L flags 0
>>> pad0: Source
>>> -> "proc":1 [ENABLED,IMMUTABLE]
>>>
>>> - entity 3: proc (2 pads, 2 links)
>>> type Node subtype Unknown flags 0
>>> pad0: Sink
>>> -> "sink":0 [ENABLED,IMMUTABLE]
>>> pad1: Source
>>> <- "source":0 [ENABLED,IMMUTABLE]
>>>
>>> - entity 6: sink (1 pad, 1 link)
>>> type Node subtype V4L flags 0
>>> pad0: Sink
>>> <- "proc":0 [ENABLED,IMMUTABLE]
>>>
>>> Compliance output
>>> =
>>>
>>> v4l2-compliance -m /dev/media0 -v 
>>> v4l2-compliance SHA: e2038ec6451293787b929338c2a671c732b8693d, 64
>>> bits
>>
>> This is an old version of v4l2-compliance. Can you update it to the
>> latest
>> version and run this again?
>>
> 
> With the two v4l-utils patches that I just sent:
> 
> https://patchwork.linuxtv.org/patch/50654/
> https://patchwork.linuxtv.org/patch/50655/
> 
> The compliance output looks OK, I think:
> 
> root@(none):/# v4l2-compliance -m 0 -v
> v4l2-compliance SHA: 248491682a2919a1bd421f87b33c14125b9fc1f5, 64 bits
> 
> Compliance test for device /dev/media0:
> 
> Media Driver Info:
>   Driver name  : vim2m
>   Model: vim2m
>   Serial   : 
>   Bus info : 
>   Media version: 4.18.0
>   Hardware revision: 0x (0)
>   Driver version   : 4.18.0
> 
> Required ioctls:
>   test MEDIA_IOC_DEVICE_INFO: OK
> 
> Allow for multiple opens:
>   test second /dev/media0 open: OK
>   test MEDIA_IOC_DEVICE_INFO: OK
>   test for unlimited opens: OK
> 
> Media Controller ioctls:
>   Entity: 0x0001 (Name: 'source', Function: V4L2 I/O)
>   Entity: 0x0003 (Name: 'proc', Function: Video
> Scaler)
>   Entity: 0x0006 (Name: 'sink', Function: V4L2 I/O)
>   Interface: 0x030c (Type: V4L Video, DevPath:
> /dev/video2)
>   Pad: 0x0102 (source, Source)
>   Pad: 0x0104 (proc, Sink)
>   Pad: 0x0105 (proc, Source)
>   Pad: 0x0107 (sink, Sink)
>   Link: 0x0208 (source -> proc)
>   Link: 0x020a (proc -> sink)
>   Link: 0x020d (source to interface /dev/video2)
>   Link: 0x020e (sink to interface /dev/video2)
>   test MEDIA_IOC_G_TOPOLOGY: OK
>   Entities: 3 Interfaces: 1 Pads: 4 Links: 4
>   Entity: 0x0001 (Name: 'source', Type: V4L2 I/O)
>   Entity: 0x0003 (Name: 'proc', Type: Unknown legacy
> device node type (0001))
>   Entity: 0x0006 (Name: 'sink', Type: V4L2 I/O)
>   Entity Links: 0x0001 (Name: 'source')
>   Entity Links: 0x0003 (Name: 'proc')
>   Entity Links: 0x0006 (Name: 'sink')
>   test MEDIA_IOC_ENUM_ENTITIES/LINKS: OK
>   test MEDIA_IOC_SETUP_LINK: OK
> 
> -
> ---
> Compliance test for device /dev/video2:
> 
> Driver Info:
>   Driver name  : vim2m
>   Card type: vim2m
>   Bus info : platform:vim2m
>   Driver version   : 4.18.0
>   Capabilities : 0x84208000
>   Video Memory-to-Memory
>   Streaming
>   Extended Pix Format
>   Device Capabilities
>   Device Caps  : 0x04208000
>   Video Memory-to-Memory
>   Streaming
>   Extended Pix Format
> Media Driver Info:
>   Driver name  : vim2m
>   Model: vim2m
>   Serial   : 
>   Bus info : 
>   Media version: 4.18.0
>   Hardware revision: 0x (0)
>   Driver version   : 4.18.0
> Interface Info:
>   ID   : 0x030c
>   Type : V4L Video
> Entity Info:
>   ID   : 0x0001 (1)
>   Name : source
>   Function 

Re: [PATCH 2/2] v4l-helpers: Fix EXPBUF queue type

[Hans Verkuil]

On 06/28/2018 09:25 PM, Ezequiel Garcia wrote:
> v4l_queue_export_bufs uses the v4l_fd type when calling
> EXPBUF ioctl. However, this doesn't work on mem2mem
> where there are one capture queue and one output queue
> associated to the device.
> 
> The current code calls v4l_queue_export_bufs with the
> wrong type, failing as:
> 
> fail: v4l2-test-buffers.cpp(544): q_.export_bufs(node)
> test VIDIOC_EXPBUF: FAIL
> 
> Fix this by using the queue type instead.

I changed this by requiring that the exp_type is provided by the caller.

Thanks for reporting this!

Regards,

Hans

> 
> Signed-off-by: Ezequiel Garcia 
> ---
>  utils/common/v4l-helpers.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/utils/common/v4l-helpers.h b/utils/common/v4l-helpers.h
> index 83d8d7d9c073..d6866f04e23a 100644
> --- a/utils/common/v4l-helpers.h
> +++ b/utils/common/v4l-helpers.h
> @@ -1633,7 +1633,7 @@ static inline int v4l_queue_export_bufs(struct v4l_fd 
> *f, struct v4l_queue *q,
>   unsigned b, p;
>   int ret = 0;
>  
> - expbuf.type = exp_type ? : f->type;
> + expbuf.type = exp_type ? : q->type;
>   expbuf.flags = O_RDWR;
>   memset(expbuf.reserved, 0, sizeof(expbuf.reserved));
>   for (b = 0; b < v4l_queue_g_buffers(q); b++) {
> 

Re: [PATCH 1/2] v4l-helpers: Don't close the fd in {}_s_fd

[Hans Verkuil]

On 06/28/2018 09:25 PM, Ezequiel Garcia wrote:
> When creating a second node via copy or assignment:
> 
> node2 = node
> 
> The node being assigned to, i.e. node2, obtains the fd.
> This causes a later call to node2.media_open to close()
> the fd, thus unintendenly closing the original node fd,
> via the call path (e.g. for media devices):
> 
>   node2.media_open
>  v4l_media_open
> v4l_media_s_fd
> 
> Similar call paths apply for other device types.
> Fix this by removing the close in xxx_s_fd.

I fixed this in a different way by overloading the assignment operator
and calling dup(fd). That solves this as well.

Regards,

Hans

> 
> Signed-off-by: Ezequiel Garcia 
> ---
>  utils/common/v4l-helpers.h | 9 -
>  1 file changed, 9 deletions(-)
> 
> diff --git a/utils/common/v4l-helpers.h b/utils/common/v4l-helpers.h
> index c37b72712126..83d8d7d9c073 100644
> --- a/utils/common/v4l-helpers.h
> +++ b/utils/common/v4l-helpers.h
> @@ -444,9 +444,6 @@ static inline int v4l_s_fd(struct v4l_fd *f, int fd, 
> const char *devname, bool d
>   struct v4l2_queryctrl qc;
>   struct v4l2_selection sel;
>  
> - if (f->fd >= 0)
> - f->close(f);
> -
>   f->fd = fd;
>   f->direct = direct;
>   if (fd < 0)
> @@ -492,9 +489,6 @@ static inline int v4l_open(struct v4l_fd *f, const char 
> *devname, bool non_block
>  
>  static inline int v4l_subdev_s_fd(struct v4l_fd *f, int fd, const char 
> *devname)
>  {
> - if (f->fd >= 0)
> - f->close(f);
> -
>   f->fd = fd;
>   f->direct = false;
>   if (fd < 0)
> @@ -525,9 +519,6 @@ static inline int v4l_subdev_open(struct v4l_fd *f, const 
> char *devname, bool no
>  
>  static inline int v4l_media_s_fd(struct v4l_fd *f, int fd, const char 
> *devname)
>  {
> - if (f->fd >= 0)
> - f->close(f);
> -
>   f->fd = fd;
>   f->direct = false;
>   if (fd < 0)
> 

[PATCH] v4l2-mem2mem: add name argument to v4l2_m2m_register_media_controller()

[Hans Verkuil]

Hi Ezequiel,

I added support for this to a codec and I discovered that we are missing a 
'name'
argument to v4l2_m2m_register_media_controller(): a typical codec driver has two
m2m video nodes: one for encoding, one for decoding. That works fine, except 
that
the names of the source, sink and proc entities are the same for both encoder 
and
decoder node.

So add an extra name argument to help differentiate between the two.

Can you fold this in your v4l2-mem2mem patch?

Thanks!

Hans

Signed-off-by: Hans Verkuil 
---
diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c 
b/drivers/media/v4l2-core/v4l2-mem2mem.c
index 40d4d645e6a6..b3ecd5a41c48 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -76,10 +76,13 @@ struct v4l2_m2m_dev {
 #ifdef CONFIG_MEDIA_CONTROLLER
struct media_entity *source;
struct media_padsource_pad;
+   char*source_name;
struct media_entity sink;
struct media_padsink_pad;
+   char*sink_name;
struct media_entity proc;
struct media_padproc_pads[2];
+   char*proc_name;
struct media_intf_devnode *intf_devnode;
 #endif

@@ -647,33 +650,41 @@ void v4l2_m2m_unregister_media_controller(struct 
v4l2_m2m_dev *m2m_dev)
media_device_unregister_entity(m2m_dev->source);
media_device_unregister_entity(_dev->sink);
media_device_unregister_entity(_dev->proc);
+   kfree(m2m_dev->source_name);
+   kfree(m2m_dev->sink_name);
+   kfree(m2m_dev->proc_name);
 }
 EXPORT_SYMBOL_GPL(v4l2_m2m_unregister_media_controller);

 static int v4l2_m2m_register_entity(struct media_device *mdev,
struct v4l2_m2m_dev *m2m_dev, enum v4l2_m2m_entity_type type,
-   int function)
+   const char *name, int function)
 {
struct media_entity *entity;
struct media_pad *pads;
+   char **p_name;
+   unsigned int len;
int num_pads;
int ret;

switch (type) {
case MEM2MEM_ENT_TYPE_SOURCE:
entity = m2m_dev->source;
+   p_name = _dev->source_name;
pads = _dev->source_pad;
pads[0].flags = MEDIA_PAD_FL_SOURCE;
num_pads = 1;
break;
case MEM2MEM_ENT_TYPE_SINK:
entity = _dev->sink;
+   p_name = _dev->sink_name;
pads = _dev->sink_pad;
pads[0].flags = MEDIA_PAD_FL_SINK;
num_pads = 1;
break;
case MEM2MEM_ENT_TYPE_PROC:
entity = _dev->proc;
+   p_name = _dev->proc_name;
pads = m2m_dev->proc_pads;
pads[0].flags = MEDIA_PAD_FL_SINK;
pads[1].flags = MEDIA_PAD_FL_SOURCE;
@@ -683,8 +694,11 @@ static int v4l2_m2m_register_entity(struct media_device 
*mdev,
return -EINVAL;
}

+   len = strlen(name) + 2 + strlen(m2m_entity_name[type]);
entity->obj_type = MEDIA_ENTITY_TYPE_BASE;
-   entity->name = m2m_entity_name[type];
+   *p_name = kmalloc(len, GFP_KERNEL);
+   snprintf(*p_name, len, "%s-%s", name, m2m_entity_name[type]);
+   entity->name = *p_name;
entity->function = function;

ret = media_entity_pads_init(entity, num_pads, pads);
@@ -698,7 +712,8 @@ static int v4l2_m2m_register_entity(struct media_device 
*mdev,
 }

 int v4l2_m2m_register_media_controller(struct v4l2_m2m_dev *m2m_dev,
-   struct video_device *vdev, int function)
+   struct video_device *vdev, const char *name,
+   int function)
 {
struct media_device *mdev = vdev->v4l2_dev->mdev;
struct media_link *link;
@@ -715,15 +730,15 @@ int v4l2_m2m_register_media_controller(struct 
v4l2_m2m_dev *m2m_dev,
/* Create the three entities with their pads */
m2m_dev->source = >entity;
ret = v4l2_m2m_register_entity(mdev, m2m_dev,
-   MEM2MEM_ENT_TYPE_SOURCE, MEDIA_ENT_F_IO_V4L);
+   MEM2MEM_ENT_TYPE_SOURCE, name, MEDIA_ENT_F_IO_V4L);
if (ret)
return ret;
ret = v4l2_m2m_register_entity(mdev, m2m_dev,
-   MEM2MEM_ENT_TYPE_PROC, function);
+   MEM2MEM_ENT_TYPE_PROC, name, function);
if (ret)
goto err_rel_entity0;
ret = v4l2_m2m_register_entity(mdev, m2m_dev,
-   MEM2MEM_ENT_TYPE_SINK, MEDIA_ENT_F_IO_V4L);
+   MEM2MEM_ENT_TYPE_SINK, name, MEDIA_ENT_F_IO_V4L);
if (ret)
goto err_rel_entity1;

diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
index 4c14818c270e..6d52e9c06440 100644
--- a/include/media/v4l2-mem2mem.h
+++ b/include/media/v4l2-mem2mem.h
@@ -332,7 +332,8 @@ struct v4l2_m2m_dev *v4l2_m2m_init(const struct 
v4l2_m2m_ops *m2m_ops);
 #if 

Re: Software-only image processing for Intel "complex" cameras

[Tomasz Figa]

Hi Pavel,

On Thu, Jun 21, 2018 at 5:38 AM Pavel Machek  wrote:
>
> Hi!
>
> On Nokia N900, I have similar problems as Intel IPU3 hardware.
>
> Meeting notes say that pure software implementation is not fast
> enough, but that it may be useful for debugging. It would be also
> useful for me on N900, and probably useful for processing "raw" images
> from digital cameras.
>
> There is sensor part, and memory-to-memory part, right? What is
> the format of data from the sensor part? What operations would be
> expensive on the CPU? If we did everthing on the CPU, what would be
> maximum resolution where we could still manage it in real time?

We can still use the memory-to-memory part (IMGU), even without 3A. It
would just do demosaicing at default parameters and give us a YUV
(NV12) frame. We could use some software component to analyze the YUV
output and adjust sensor parameters accordingly. Possibly the part we
already have in libv4l2 could just work?

The expensive operation would be analyzing the frame itself. I suppose
you need to build some histogram representing brightness and white
balance of the frame and then infer necessary sensor adjustments from
that.

>
> Would it be possible to get access to machine with IPU3, or would
> there be someone willing to test libv4l2 patches?

I should be able to help with some basic testing, preferably limited
to command line tools (but I might be able to create a test
environment for X11 tools if really necessary).

Best regards,
Tomasz