cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Tue Jul 3 05:00:13 CEST 2018 media-tree git hash:3c4a737267e89aafa6308c6c456d2ebea3fcd085 media_build git hash: 90c56d1e6345747b6af929867f5b7c16017dcf02 v4l-utils git hash: 47593771ad61f52d3670ef35373f24f85d5da267 edid-decode git hash: ab18befbcacd6cd4dff63faa82e32700369d6f25 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-2-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: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-i686: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-i686: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-i686: OK linux-2.6.39.4-x86_64: OK linux-3.0.101-i686: OK linux-3.0.101-x86_64: OK linux-3.1.10-i686: OK linux-3.1.10-x86_64: OK linux-3.2.101-i686: OK linux-3.2.101-x86_64: OK linux-3.3.8-i686: OK linux-3.3.8-x86_64: OK linux-3.4.113-i686: OK linux-3.4.113-x86_64: OK linux-3.5.7-i686: OK linux-3.5.7-x86_64: OK linux-3.6.11-i686: OK linux-3.6.11-x86_64: OK linux-3.7.10-i686: OK linux-3.7.10-x86_64: OK linux-3.8.13-i686: OK linux-3.8.13-x86_64: OK linux-3.9.11-i686: OK linux-3.9.11-x86_64: OK linux-3.10.108-i686: OK linux-3.10.108-x86_64: OK linux-3.11.10-i686: OK linux-3.11.10-x86_64: OK linux-3.12.74-i686: OK linux-3.12.74-x86_64: OK linux-3.13.11-i686: OK linux-3.13.11-x86_64: OK linux-3.14.79-i686: OK linux-3.14.79-x86_64: OK linux-3.15.10-i686: OK linux-3.15.10-x86_64: OK linux-3.16.56-i686: OK linux-3.16.56-x86_64: OK linux-3.17.8-i686: ERRORS linux-3.17.8-x86_64: OK linux-3.18.102-i686: ERRORS linux-3.18.102-x86_64: OK linux-3.19.8-i686: OK linux-3.19.8-x86_64: OK linux-4.0.9-i686: OK linux-4.0.9-x86_64: OK linux-4.1.51-i686: OK linux-4.1.51-x86_64: OK linux-4.2.8-i686: OK linux-4.2.8-x86_64: OK linux-4.3.6-i686: OK linux-4.3.6-x86_64: OK linux-4.4.109-i686: OK linux-4.4.109-x86_64: OK linux-4.5.7-i686: OK linux-4.5.7-x86_64: OK linux-4.6.7-i686: OK linux-4.6.7-x86_64: OK linux-4.7.10-i686: OK linux-4.7.10-x86_64: OK linux-4.8.17-i686: OK linux-4.8.17-x86_64: OK linux-4.9.91-i686: OK linux-4.9.91-x86_64: OK linux-4.10.17-i686: OK linux-4.10.17-x86_64: OK linux-4.11.12-i686: OK linux-4.11.12-x86_64: OK linux-4.12.14-i686: OK linux-4.12.14-x86_64: OK linux-4.13.16-i686: OK linux-4.13.16-x86_64: OK linux-4.14.42-i686: OK linux-4.14.42-x86_64: OK linux-4.15.14-i686: OK linux-4.15.14-x86_64: OK linux-4.16.8-i686: OK linux-4.16.8-x86_64: OK linux-4.17.2-i686: OK linux-4.17.2-x86_64: OK linux-4.18-rc1-i686: OK linux-4.18-rc1-x86_64: OK apps: OK spec-git: OK Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: [PATCH v6 04/17] omap3isp: Add vb2_queue lock
Hi Ezequiel,
(CC'ing Sakari)
Thank you for the patch.
On Friday, 22 June 2018 06:53:58 EEST Ezequiel Garcia wrote:
> vb2_queue locks is now mandatory. Add it, remove driver ad-hoc locks,
> and implement wait_{prepare, finish}.
>
> Also, remove stream_lock mutex. Since the ioctls operations
> are now protected by the queue mutex, the stream_lock mutex is
> not needed.
>
> Signed-off-by: Ezequiel Garcia
> ---
> drivers/media/platform/omap3isp/ispvideo.c | 65 --
> drivers/media/platform/omap3isp/ispvideo.h | 1 -
> 2 files changed, 11 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/media/platform/omap3isp/ispvideo.c
> b/drivers/media/platform/omap3isp/ispvideo.c index
> 9d228eac24ea..f835aeb9ddc5 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.c
> +++ b/drivers/media/platform/omap3isp/ispvideo.c
> @@ -496,6 +496,8 @@ static const struct vb2_ops isp_video_queue_ops = {
> .buf_prepare = isp_video_buffer_prepare,
> .buf_queue = isp_video_buffer_queue,
> .start_streaming = isp_video_start_streaming,
> + .wait_prepare = vb2_ops_wait_prepare,
> + .wait_finish = vb2_ops_wait_finish,
> };
>
> /*
> @@ -628,11 +630,8 @@ void omap3isp_video_resume(struct isp_video *video, int
> continuous) {
> struct isp_buffer *buf = NULL;
>
> - if (continuous && video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> - mutex_lock(>queue_lock);
> + if (continuous && video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> vb2_discard_done(video->queue);
> - mutex_unlock(>queue_lock);
> - }
Why can locking be removed here ? As far as I know the lock isn't taken by the
caller. It would help to explain the rationale in the commit message.
> if (!list_empty(>dmaqueue)) {
> buf = list_first_entry(>dmaqueue,
> @@ -909,13 +908,8 @@ isp_video_reqbufs(struct file *file, void *fh, struct
> v4l2_requestbuffers *rb) {
> struct isp_video_fh *vfh = to_isp_video_fh(fh);
> struct isp_video *video = video_drvdata(file);
The video variable isn't used anymore. Didn't gcc warn you ?
> - int ret;
> -
> - mutex_lock(>queue_lock);
> - ret = vb2_reqbufs(>queue, rb);
> - mutex_unlock(>queue_lock);
>
> - return ret;
> + return vb2_reqbufs(>queue, rb);
> }
>
> static int
> @@ -923,13 +917,8 @@ isp_video_querybuf(struct file *file, void *fh, struct
> v4l2_buffer *b) {
> struct isp_video_fh *vfh = to_isp_video_fh(fh);
> struct isp_video *video = video_drvdata(file);
Same here and in other functions below.
> - int ret;
>
> - mutex_lock(>queue_lock);
> - ret = vb2_querybuf(>queue, b);
> - mutex_unlock(>queue_lock);
> -
> - return ret;
> + return vb2_querybuf(>queue, b);
> }
>
> static int
> @@ -937,13 +926,8 @@ isp_video_qbuf(struct file *file, void *fh, struct
> v4l2_buffer *b) {
> struct isp_video_fh *vfh = to_isp_video_fh(fh);
> struct isp_video *video = video_drvdata(file);
> - int ret;
>
> - mutex_lock(>queue_lock);
> - ret = vb2_qbuf(>queue, b);
> - mutex_unlock(>queue_lock);
> -
> - return ret;
> + return vb2_qbuf(>queue, b);
> }
>
> static int
> @@ -951,13 +935,8 @@ isp_video_dqbuf(struct file *file, void *fh, struct
> v4l2_buffer *b) {
> struct isp_video_fh *vfh = to_isp_video_fh(fh);
> struct isp_video *video = video_drvdata(file);
> - int ret;
>
> - mutex_lock(>queue_lock);
> - ret = vb2_dqbuf(>queue, b, file->f_flags & O_NONBLOCK);
> - mutex_unlock(>queue_lock);
> -
> - return ret;
> + return vb2_dqbuf(>queue, b, file->f_flags & O_NONBLOCK);
> }
>
> static int isp_video_check_external_subdevs(struct isp_video *video,
> @@ -1096,8 +1075,6 @@ isp_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type) if (type != video->type)
> return -EINVAL;
>
> - mutex_lock(>stream_lock);
> -
> /* Start streaming on the pipeline. No link touching an entity in the
>* pipeline can be activated or deactivated once streaming is started.
>*/
> @@ -1106,7 +1083,7 @@ isp_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type)
>
> ret = media_entity_enum_init(>ent_enum, >isp->media_dev);
> if (ret)
> - goto err_enum_init;
> + return ret;
>
> /* TODO: Implement PM QoS */
> pipe->l3_ick = clk_get_rate(video->isp->clock[ISP_CLK_L3_ICK]);
> @@ -1158,14 +1135,10 @@ isp_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type) atomic_set(>frame_number, -1);
> pipe->field = vfh->format.fmt.pix.field;
>
> - mutex_lock(>queue_lock);
> ret = vb2_streamon(>queue, type);
> - mutex_unlock(>queue_lock);
> if (ret < 0)
> goto err_check_format;
>
> - mutex_unlock(>stream_lock);
> -
> return 0;
>
> err_check_format:
> @@ -1184,9 +1157,6 @@ isp_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type)
>
[GIT PULL FOR v4.19] Various fixes/improvements
Fix a long-standing bug in v4l2-ctrls.c, mark all interface links as IMMUTABLE and add helpers to configure an MC for mem-to-mem devices and use that in vim2m. Note regarding the v4l2-ctrls.c fix: I've decided not to backport this. It's a dark corner of the spec that few people use and that AFAICT never worked properly. Hugues Fruchet was the first to complain about this in many years, so this is clearly not a high prio thing. If I change my mind, then this should be backported all the way to 3.11. But before that it was also wrong, except this patch no longer applies. Regards, Hans 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: git://linuxtv.org/hverkuil/media_tree.git for-v4.19g for you to fetch changes up to dd8320d3a8af219ce084e7342df3e9a0cecc289b: vim2m: add media device (2018-07-02 18:00:04 +0200) Ezequiel Garcia (1): media: add helpers for memory-to-memory media controller Hans Verkuil (3): v4l2-ctrls.c: fix broken auto cluster handling media: mark entity-intf links as IMMUTABLE vim2m: add media device drivers/media/dvb-core/dvbdev.c| 18 -- drivers/media/platform/vim2m.c | 41 -- drivers/media/v4l2-core/v4l2-ctrls.c | 15 - drivers/media/v4l2-core/v4l2-dev.c | 16 -- drivers/media/v4l2-core/v4l2-device.c | 3 +- drivers/media/v4l2-core/v4l2-mem2mem.c | 190 include/media/v4l2-mem2mem.h | 19 +++ 7 files changed, 284 insertions(+), 18 deletions(-)
[PATCH v5 0/2] Memory-to-memory media controller topology
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-rc3. v5: * Don't set major:minor to proc entity (Hans). v4: * Add unique entity names (Hans) * Add associated interface major and minor (Hans) * Remove unused MEM2MEM_ENT_TYPE_MAX (Hans) v3: * Remove extra empty line. * Remove duplicated entity name set. * Reorder sink and source pads (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 = 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: '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)) 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
[PATCH v5 1/2] media: add helpers for memory-to-memory media controller
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 | 188 +
include/media/v4l2-mem2mem.h | 19 +++
3 files changed, 215 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..47d4b36978a1 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,172 @@ 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);
+
[PATCH v5 2/2] vim2m: add media device
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
Re: [PATCHv15 07/35] v4l2-dev: lock req_queue_mutex
On 02/07/18 15:06, Tomasz Figa wrote:
> Hi Hans,
>
> On Mon, Jun 4, 2018 at 8:48 PM Hans Verkuil wrote:
> [snip]
>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
>> b/drivers/media/v4l2-core/v4l2-ioctl.c
>> index 965fd301f617..27a893aa0664 100644
>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>> @@ -2714,6 +2714,7 @@ static long __video_do_ioctl(struct file *file,
>> unsigned int cmd, void *arg)
>> {
>> struct video_device *vfd = video_devdata(file);
>> + struct mutex *req_queue_lock = NULL;
>> struct mutex *lock; /* ioctl serialization mutex */
>> const struct v4l2_ioctl_ops *ops = vfd->ioctl_ops;
>> bool write_only = false;
>> @@ -2733,10 +2734,27 @@ static long __video_do_ioctl(struct file *file,
>> if (test_bit(V4L2_FL_USES_V4L2_FH, >flags))
>> vfh = file->private_data;
>>
>> + /*
>> +* We need to serialize streamon/off with queueing new requests.
>> +* These ioctls may trigger the cancellation of a streaming
>> +* operation, and that should not be mixed with queueing a new
>> +* request at the same time.
>> +*/
>> + if (v4l2_device_supports_requests(vfd->v4l2_dev) &&
>> + (cmd == VIDIOC_STREAMON || cmd == VIDIOC_STREAMOFF)) {
>
> Are STREAMON and STREAMOFF the only ioctls we need to acquire
> req_queue_lock for? How about a race with, for example, S_CTRL(control
> X, request_fd = -1) and req_validate() on a request that depends on
> the value of control X? Maybe we should just lock here for any ioctl?
Definitely not, that would seriously impact performance since this is such
a high-level lock.
It is indeed possible to set controls directly even if the same control is
also used in a request. It is intentional that you can still set controls
directly: for some controls (e.g. POWERLINE frequency) it makes no sense to
use them in a request and you just want to be able to set them directly. It
is also very useful for debugging. So even though it can potentially mess
things up, it is practical to allow this.
Drivers need to be able to handle such situations anyway since setting
controls from a request can fail regardless due to HW errors of some kind,
so a control might not have the value that you expected.
And worse case there will only be a wrong control value until a request is
applied containing a new value of this control.
It might be that in the future we need to restrict this for selected controls
in some manner, but for now I think this is fine.
Note that this is different for buffers: once you've switched to using requests
to queue buffers, you can no longer queue a buffer directly. Mixing that is
theoretically possible, but it is very confusing.
>
>> + req_queue_lock = >v4l2_dev->mdev->req_queue_mutex;
>> +
>> + if (req_queue_lock &&
>> mutex_lock_interruptible(req_queue_lock))
>
> I believe it isn't possible for req_queue_lock to be NULL here.
True, I can remove that test.
Regards,
Hans
>
>> + return -ERESTARTSYS;
>
> I guess it isn't really possible for mutex_lock_interruptible() to
> return anything non-zero other than this, but I'd still return what it
> returns here just in case.
Re: [PATCHv15 07/35] v4l2-dev: lock req_queue_mutex
Hi Hans,
On Mon, Jun 4, 2018 at 8:48 PM Hans Verkuil wrote:
[snip]
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 965fd301f617..27a893aa0664 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -2714,6 +2714,7 @@ static long __video_do_ioctl(struct file *file,
> unsigned int cmd, void *arg)
> {
> struct video_device *vfd = video_devdata(file);
> + struct mutex *req_queue_lock = NULL;
> struct mutex *lock; /* ioctl serialization mutex */
> const struct v4l2_ioctl_ops *ops = vfd->ioctl_ops;
> bool write_only = false;
> @@ -2733,10 +2734,27 @@ static long __video_do_ioctl(struct file *file,
> if (test_bit(V4L2_FL_USES_V4L2_FH, >flags))
> vfh = file->private_data;
>
> + /*
> +* We need to serialize streamon/off with queueing new requests.
> +* These ioctls may trigger the cancellation of a streaming
> +* operation, and that should not be mixed with queueing a new
> +* request at the same time.
> +*/
> + if (v4l2_device_supports_requests(vfd->v4l2_dev) &&
> + (cmd == VIDIOC_STREAMON || cmd == VIDIOC_STREAMOFF)) {
Are STREAMON and STREAMOFF the only ioctls we need to acquire
req_queue_lock for? How about a race with, for example, S_CTRL(control
X, request_fd = -1) and req_validate() on a request that depends on
the value of control X? Maybe we should just lock here for any ioctl?
> + req_queue_lock = >v4l2_dev->mdev->req_queue_mutex;
> +
> + if (req_queue_lock &&
> mutex_lock_interruptible(req_queue_lock))
I believe it isn't possible for req_queue_lock to be NULL here.
> + return -ERESTARTSYS;
I guess it isn't really possible for mutex_lock_interruptible() to
return anything non-zero other than this, but I'd still return what it
returns here just in case.
Best regards,
Tomasz
Re: [PATCHv15 06/35] v4l2-device.h: add v4l2_device_supports_requests() helper
On Mon, Jun 04, 2018 at 01:46:19PM +0200, Hans Verkuil wrote: > From: Hans Verkuil > > Add a simple helper function that tests if the driver supports > the request API. > > Signed-off-by: Hans Verkuil Acked-by: Sakari Ailus -- Sakari Ailus e-mail: sakari.ai...@iki.fi
Re: [PATCH 1/2] media: i2c: ov5640: Re-work MIPI start sequence
On Fri, Jun 29, 2018 at 06:42:39PM +0200, Jacopo Mondi wrote:
> 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},
I'd really prefer to remove parts of that array, instead of adding
more to that unmaintainable blob.
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
Re: [PATCH] media: mark entity-intf links as IMMUTABLE
On Mon, Jul 02, 2018 at 02:43:02PM +0200, Hans Verkuil wrote: > Currently links between entities and an interface are just marked as > ENABLED. But (at least today) these links cannot be disabled by userspace > or the driver, so they should also be marked as IMMUTABLE. > > It might become possible that drivers can disable such links (if for some > reason the device node cannot be used), so we might need to add a new link > flag at some point to mark interface links that can be changed by the driver > but not by userspace. > > Signed-off-by: Hans Verkuil Acked-by: Sakari Ailus -- Sakari Ailus e-mail: sakari.ai...@iki.fi
[PATCH] media: mark entity-intf links as IMMUTABLE
Currently links between entities and an interface are just marked as
ENABLED. But (at least today) these links cannot be disabled by userspace
or the driver, so they should also be marked as IMMUTABLE.
It might become possible that drivers can disable such links (if for some
reason the device node cannot be used), so we might need to add a new link
flag at some point to mark interface links that can be changed by the driver
but not by userspace.
Signed-off-by: Hans Verkuil
---
diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index 64d6793674b9..3c8778570331 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -440,8 +440,10 @@ static int dvb_register_media_device(struct dvb_device
*dvbdev,
if (!dvbdev->entity)
return 0;
- link = media_create_intf_link(dvbdev->entity,
>intf_devnode->intf,
- MEDIA_LNK_FL_ENABLED);
+ link = media_create_intf_link(dvbdev->entity,
+ >intf_devnode->intf,
+ MEDIA_LNK_FL_ENABLED |
+ MEDIA_LNK_FL_IMMUTABLE);
if (!link)
return -ENOMEM;
#endif
@@ -599,7 +601,8 @@ static int dvb_create_io_intf_links(struct dvb_adapter
*adap,
if (strncmp(entity->name, name, strlen(name)))
continue;
link = media_create_intf_link(entity, intf,
- MEDIA_LNK_FL_ENABLED);
+ MEDIA_LNK_FL_ENABLED |
+ MEDIA_LNK_FL_IMMUTABLE);
if (!link)
return -ENOMEM;
}
@@ -754,14 +757,16 @@ int dvb_create_media_graph(struct dvb_adapter *adap,
media_device_for_each_intf(intf, mdev) {
if (intf->type == MEDIA_INTF_T_DVB_CA && ca) {
link = media_create_intf_link(ca, intf,
- MEDIA_LNK_FL_ENABLED);
+ MEDIA_LNK_FL_ENABLED |
+ MEDIA_LNK_FL_IMMUTABLE);
if (!link)
return -ENOMEM;
}
if (intf->type == MEDIA_INTF_T_DVB_FE && tuner) {
link = media_create_intf_link(tuner, intf,
- MEDIA_LNK_FL_ENABLED);
+ MEDIA_LNK_FL_ENABLED |
+ MEDIA_LNK_FL_IMMUTABLE);
if (!link)
return -ENOMEM;
}
@@ -773,7 +778,8 @@ int dvb_create_media_graph(struct dvb_adapter *adap,
*/
if (intf->type == MEDIA_INTF_T_DVB_DVR && demux) {
link = media_create_intf_link(demux, intf,
- MEDIA_LNK_FL_ENABLED);
+ MEDIA_LNK_FL_ENABLED |
+ MEDIA_LNK_FL_IMMUTABLE);
if (!link)
return -ENOMEM;
}
diff --git a/drivers/media/v4l2-core/v4l2-dev.c
b/drivers/media/v4l2-core/v4l2-dev.c
index 4ffd7d60a901..5f43f63fa700 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -808,7 +808,8 @@ static int video_register_media_controller(struct
video_device *vdev, int type)
link = media_create_intf_link(>entity,
>intf_devnode->intf,
- MEDIA_LNK_FL_ENABLED);
+ MEDIA_LNK_FL_ENABLED |
+ MEDIA_LNK_FL_IMMUTABLE);
if (!link) {
media_devnode_remove(vdev->intf_devnode);
media_device_unregister_entity(>entity);
diff --git a/drivers/media/v4l2-core/v4l2-device.c
b/drivers/media/v4l2-core/v4l2-device.c
index 937c6de85606..3940e55c72f1 100644
--- a/drivers/media/v4l2-core/v4l2-device.c
+++ b/drivers/media/v4l2-core/v4l2-device.c
@@ -267,7 +267,8 @@ int v4l2_device_register_subdev_nodes(struct v4l2_device
*v4l2_dev)
link = media_create_intf_link(>entity,
>intf_devnode->intf,
- MEDIA_LNK_FL_ENABLED);
+ MEDIA_LNK_FL_ENABLED |
+ MEDIA_LNK_FL_IMMUTABLE);
if (!link) {
Re: [PATCHv15 05/35] media-request: add media_request_object_find
On 02/07/18 14:33, Tomasz Figa wrote: > Hi Hans, > On Mon, Jun 4, 2018 at 8:48 PM Hans Verkuil wrote: >> >> From: Hans Verkuil >> >> Add media_request_object_find to find a request object inside a >> request based on ops and/or priv values. > > Current code seems to always find based on both ops and priv values. Outdated commit log. I'll change it. Regards, Hans
Re: [PATCHv15 05/35] media-request: add media_request_object_find
Hi Hans, On Mon, Jun 4, 2018 at 8:48 PM Hans Verkuil wrote: > > From: Hans Verkuil > > Add media_request_object_find to find a request object inside a > request based on ops and/or priv values. Current code seems to always find based on both ops and priv values. Best regards, Tomasz
Re: [RFC] Make entity to interface links immutable
Em Mon, 2 Jul 2018 12:38:23 +0200 Hans Verkuil escreveu: > On 02/07/18 11:41, Mauro Carvalho Chehab wrote: > > Em Mon, 2 Jul 2018 10:18:37 +0200 > > Hans Verkuil escreveu: > > > >> While working on v4l2-compliance I noticed that entity to interface links > >> have just the MEDIA_LNK_FL_ENABLED flag set. > >> > >> Shouldn't we also set the MEDIA_LNK_FL_IMMUTABLE? After all, you cannot > >> change > >> an entity-interface link. It feels inconsistent not to have this flag. > > > > It could make sense for non-hybrid devices, but this may not be true > > for hybrid ones. See below. > > > >> I also propose that media_create_intf_link() drops the last flags argument: > >> it can set the link flags directly since they are always the same anyway. > > > > When we came with this design, the idea is that an interface can be > > disabled/enabled at runtime, if the entity it links can't be used, > > because the hardware is busy doing something else. > > > > That could happen with hybrid devices, where the analog part could > > be consuming resources that would be needed for the digital part. > > > > Disabling the link at runtime has an advantage that it makes easier > > to check - as open() syscalls could just use it to return -EBUSY, > > instead of doing a complete graph analysis. Also, applications can > > test it directly, in order to have a hint if a device is ready for > > usage. > > > > That was one of the approaches we considered at the design, but I > > don't remember if Shuah's patch series actually used it or not, > > as I don't look at her pending patches for a long time. I suspect > > she took a different approach. > > > > Anyway, before touching it, I'd like to see her patches merged, > > and do some tests with real case scenarios before changing it. > > Hmm, this also highlights another deficiency in the spec. Currently > the description of IMMUTABLE says: > > "The link enabled state can’t be modified at runtime. An immutable link > is always enabled." > > But in the plan above the link can change, but only the driver can > enable/disable the link. It makes no sense AFAICS for userspace to > enable/disable an interface link. > > If I am right that it makes no sense for userspace to disable an interface > link, then we should update the spec to clarify that this is not allowed > (in fact, it is not possible today anyway). And we should also clarify > that the driver can disable an interface link and what that means. > > If userspace CAN in some circumstances disable an interface link, then > we need to add something like a READ_ONLY flag to signal whether or not > userspace can change an interface link. If it is READ_ONLY, then only the > driver can change that. Yes, this makes sense. > > That said, given that today there are no drivers that disable an interface > link, I still think that we should mark them all as IMMUTABLE. That flag > can be removed when we actually let drivers change this. Ok, let's do this, but without touching at the media_create_intf_link(), as we need first to apply Shuah's patch, and see how we'll handle, before start stripping code that will probably be needed. > > It would be consistent with the current usage of interface links. > > Regards, > > Hans Thanks, Mauro
Re: [PATCHv15 02/35] media-request: implement media requests
On 02/07/18 12:56, Tomasz Figa wrote:
> Hi Hans,
>
> On Mon, Jun 4, 2018 at 8:47 PM Hans Verkuil wrote:
> [snip]
>> +static void media_request_object_release(struct kref *kref)
>> +{
>> + struct media_request_object *obj =
>> + container_of(kref, struct media_request_object, kref);
>> + struct media_request *req = obj->req;
>> +
>> + if (req)
>> + media_request_object_unbind(obj);
>
> Is it possible and fine to have a request bound here?
> media_request_clean() seems to explicitly unbind before releasing and
> this function would be only called if last reference is put, so maybe
> we should actually WARN_ON(req)?
I agree. It used to be that req could be non-NULL for valid reasons in
previous versions of the series (at least, I think so), but this is no
longer the case in the current code. Adding a WARN_ON makes a lot of
sense.
>
>> + obj->ops->release(obj);
>> +}
> [snip]
>> @@ -87,7 +104,12 @@ struct media_device_ops {
>> * @enable_source: Enable Source Handler function pointer
>> * @disable_source: Disable Source Handler function pointer
>> *
>> + * @req_queue_mutex: Serialise validating and queueing requests in order to
>> + * guarantee exclusive access to the state of the device on
>> + * the tip of the request queue.
>> * @ops: Operation handler callbacks
>> + * @req_queue_mutex: Serialise the MEDIA_REQUEST_IOC_QUEUE ioctl w.r.t.
>> + * other operations that stop or start streaming.
>
> Merge conflict? req_queue_mutex is documented twice.
Thanks, I'll remove that.
Regards,
Hans
>
> Best regards,
> Tomasz
>
Re: [PATCHv15 02/35] media-request: implement media requests
On Mon, Jul 2, 2018 at 6:11 PM Hans Verkuil wrote:
>
> On 02/07/18 10:58, Tomasz Figa wrote:
> > Hi Hans,
> >
> > On Fri, Jun 15, 2018 at 4:09 PM Hans Verkuil wrote:
> >>
> >> On 04/06/18 13:46, Hans Verkuil wrote:
> >>> From: Hans Verkuil
> >>>
> >>> Add initial media request support:
> >>>
> >>> 1) Add MEDIA_IOC_REQUEST_ALLOC ioctl support to media-device.c
> >>> 2) Add struct media_request to store request objects.
> >>> 3) Add struct media_request_object to represent a request object.
> >>> 4) Add MEDIA_REQUEST_IOC_QUEUE/REINIT ioctl support.
> >>>
> >>> Basic lifecycle: the application allocates a request, adds
> >>> objects to it, queues the request, polls until it is completed
> >>> and can then read the final values of the objects at the time
> >>> of completion. When it closes the file descriptor the request
> >>> memory will be freed (actually, when the last user of that request
> >>> releases the request).
> >>>
> >>> Drivers will bind an object to a request (the 'adds objects to it'
> >>> phase), when MEDIA_REQUEST_IOC_QUEUE is called the request is
> >>> validated (req_validate op), then queued (the req_queue op).
> >>>
> >>> When done with an object it can either be unbound from the request
> >>> (e.g. when the driver has finished with a vb2 buffer) or marked as
> >>> completed (e.g. for controls associated with a buffer). When all
> >>> objects in the request are completed (or unbound), then the request
> >>> fd will signal an exception (poll).
> >>>
> >>> Signed-off-by: Hans Verkuil
> >>> Co-developed-by: Sakari Ailus
> >>> Signed-off-by: Sakari Ailus
> >>> Co-developed-by: Laurent Pinchart
> >>> Co-developed-by: Alexandre Courbot
> >>> ---
> >>> drivers/media/Makefile| 3 +-
> >>> drivers/media/media-device.c | 14 ++
> >>> drivers/media/media-request.c | 421 ++
> >>> include/media/media-device.h | 24 ++
> >>> include/media/media-request.h | 326 ++
> >>> 5 files changed, 787 insertions(+), 1 deletion(-)
> >>> create mode 100644 drivers/media/media-request.c
> >>> create mode 100644 include/media/media-request.h
> >>>
> >>
> >>
> >>
> >>> diff --git a/include/media/media-request.h b/include/media/media-request.h
> >>> new file mode 100644
> >>> index ..8acd2627835c
> >>> --- /dev/null
> >>> +++ b/include/media/media-request.h
> >>> @@ -0,0 +1,326 @@
> >>
> >>
> >>
> >>> +/**
> >>> + * struct media_request - Media device request
> >>> + * @mdev: Media device this request belongs to
> >>> + * @kref: Reference count
> >>> + * @debug_str: Prefix for debug messages (process name:fd)
> >>> + * @state: The state of the request
> >>> + * @updating_count: count the number of request updates that are in
> >>> progress
> >>> + * @objects: List of @struct media_request_object request objects
> >>> + * @num_incomplete_objects: The number of incomplete objects in the
> >>> request
> >>> + * @poll_wait: Wait queue for poll
> >>> + * @lock: Serializes access to this struct
> >>> + */
> >>> +struct media_request {
> >>> + struct media_device *mdev;
> >>> + struct kref kref;
> >>> + char debug_str[TASK_COMM_LEN + 11];
> >>> + enum media_request_state state;
> >>> + refcount_t updating_count;
> >>
> >> This will be replaced by unsigned int: it turns out that if
> >> CONFIG_REFCOUNT_FULL is set,
> >> the refcount implementation will complain when you increase the refcount
> >> from 0 to 1
> >> since it expects that refcount_t it used as it is in kref. Since
> >> updating_count is
> >> protected by the spinlock anyway there is no need to use refcount_t, a
> >> simple
> >> unsigned int works just as well.
> >
> > It seems to be read in media_request_clean(), which can be called by
> > media_request_release() or media_request_ioctl_reinit() and neither
> > acquires the spinlock for the time of the call.
>
> The request object is in state CLEANING at that time, and that guarantees
> that nobody else will make changes to the request.
Makes sense. Thanks for clarification.
Best regards,
Tomasz
Re: [PATCHv15 02/35] media-request: implement media requests
Hi Hans,
On Mon, Jun 4, 2018 at 8:47 PM Hans Verkuil wrote:
[snip]
> +static void media_request_object_release(struct kref *kref)
> +{
> + struct media_request_object *obj =
> + container_of(kref, struct media_request_object, kref);
> + struct media_request *req = obj->req;
> +
> + if (req)
> + media_request_object_unbind(obj);
Is it possible and fine to have a request bound here?
media_request_clean() seems to explicitly unbind before releasing and
this function would be only called if last reference is put, so maybe
we should actually WARN_ON(req)?
> + obj->ops->release(obj);
> +}
[snip]
> @@ -87,7 +104,12 @@ struct media_device_ops {
> * @enable_source: Enable Source Handler function pointer
> * @disable_source: Disable Source Handler function pointer
> *
> + * @req_queue_mutex: Serialise validating and queueing requests in order to
> + * guarantee exclusive access to the state of the device on
> + * the tip of the request queue.
> * @ops: Operation handler callbacks
> + * @req_queue_mutex: Serialise the MEDIA_REQUEST_IOC_QUEUE ioctl w.r.t.
> + * other operations that stop or start streaming.
Merge conflict? req_queue_mutex is documented twice.
Best regards,
Tomasz
Re: [RFC] Make entity to interface links immutable
On 02/07/18 11:41, Mauro Carvalho Chehab wrote: > Em Mon, 2 Jul 2018 10:18:37 +0200 > Hans Verkuil escreveu: > >> While working on v4l2-compliance I noticed that entity to interface links >> have just the MEDIA_LNK_FL_ENABLED flag set. >> >> Shouldn't we also set the MEDIA_LNK_FL_IMMUTABLE? After all, you cannot >> change >> an entity-interface link. It feels inconsistent not to have this flag. > > It could make sense for non-hybrid devices, but this may not be true > for hybrid ones. See below. > >> I also propose that media_create_intf_link() drops the last flags argument: >> it can set the link flags directly since they are always the same anyway. > > When we came with this design, the idea is that an interface can be > disabled/enabled at runtime, if the entity it links can't be used, > because the hardware is busy doing something else. > > That could happen with hybrid devices, where the analog part could > be consuming resources that would be needed for the digital part. > > Disabling the link at runtime has an advantage that it makes easier > to check - as open() syscalls could just use it to return -EBUSY, > instead of doing a complete graph analysis. Also, applications can > test it directly, in order to have a hint if a device is ready for > usage. > > That was one of the approaches we considered at the design, but I > don't remember if Shuah's patch series actually used it or not, > as I don't look at her pending patches for a long time. I suspect > she took a different approach. > > Anyway, before touching it, I'd like to see her patches merged, > and do some tests with real case scenarios before changing it. Hmm, this also highlights another deficiency in the spec. Currently the description of IMMUTABLE says: "The link enabled state can’t be modified at runtime. An immutable link is always enabled." But in the plan above the link can change, but only the driver can enable/disable the link. It makes no sense AFAICS for userspace to enable/disable an interface link. If I am right that it makes no sense for userspace to disable an interface link, then we should update the spec to clarify that this is not allowed (in fact, it is not possible today anyway). And we should also clarify that the driver can disable an interface link and what that means. If userspace CAN in some circumstances disable an interface link, then we need to add something like a READ_ONLY flag to signal whether or not userspace can change an interface link. If it is READ_ONLY, then only the driver can change that. That said, given that today there are no drivers that disable an interface link, I still think that we should mark them all as IMMUTABLE. That flag can be removed when we actually let drivers change this. It would be consistent with the current usage of interface links. Regards, Hans
Re: [PATCH] v4l2-ctrls.c: fix broken auto cluster handling
Many thanks Hans,
This fix my issue with ov5640,
Best regards,
Hugues.
On 06/29/2018 12:12 PM, Hans Verkuil wrote:
> 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
Tested-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: [RFC] Make entity to interface links immutable
Em Mon, 2 Jul 2018 10:18:37 +0200 Hans Verkuil escreveu: > While working on v4l2-compliance I noticed that entity to interface links > have just the MEDIA_LNK_FL_ENABLED flag set. > > Shouldn't we also set the MEDIA_LNK_FL_IMMUTABLE? After all, you cannot change > an entity-interface link. It feels inconsistent not to have this flag. It could make sense for non-hybrid devices, but this may not be true for hybrid ones. See below. > I also propose that media_create_intf_link() drops the last flags argument: > it can set the link flags directly since they are always the same anyway. When we came with this design, the idea is that an interface can be disabled/enabled at runtime, if the entity it links can't be used, because the hardware is busy doing something else. That could happen with hybrid devices, where the analog part could be consuming resources that would be needed for the digital part. Disabling the link at runtime has an advantage that it makes easier to check - as open() syscalls could just use it to return -EBUSY, instead of doing a complete graph analysis. Also, applications can test it directly, in order to have a hint if a device is ready for usage. That was one of the approaches we considered at the design, but I don't remember if Shuah's patch series actually used it or not, as I don't look at her pending patches for a long time. I suspect she took a different approach. Anyway, before touching it, I'd like to see her patches merged, and do some tests with real case scenarios before changing it. Thanks, Mauro
[GIT PULL FOR v4.19] Various fixes
Hi Mauro, The usual 'various fixes'. A bunch of coda fixes, and I cherry-picked from Ezequiel's https://www.spinics.net/lists/linux-media/msg136223.html patch series. Regards, Hans 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: git://linuxtv.org/hverkuil/media_tree.git for-v4.19f for you to fetch changes up to bf40e3b08f4b7f5f49ffe28f94a2a505017df5c2: media: fsl-viu: fix error handling in viu_of_probe() (2018-07-02 10:56:01 +0200) Alexey Khoroshilov (1): media: fsl-viu: fix error handling in viu_of_probe() Ezequiel Garcia (8): sta2x11: Add video_device and vb2_queue locks mtk-mdp: Add locks for capture and output vb2_queues s5p-g2d: Implement wait_prepare and wait_finish staging: bcm2835-camera: Provide lock for vb2_queue davinci_vpfe: Add video_device and vb2_queue locks mx_emmaprp: Implement wait_prepare and wait_finish m2m-deinterlace: Implement wait_prepare and wait_finish stk1160: Set the vb2_queue lock before calling vb2_queue_init Hans Verkuil (2): v4l2-ioctl.c: use correct vb2_queue lock for m2m devices vivid: fix gain when autogain is on Philipp Zabel (8): media: coda: fix encoder source stride media: coda: add read-only h.264 decoder profile/level controls media: coda: fix reorder detection for unknown levels media: coda: clear hold flag on streamoff media: coda: jpeg: allow non-JPEG colorspace media: coda: jpeg: only queue two buffers into the bitstream for JPEG on CODA7541 media: coda: jpeg: explicitly disable thumbnails in SEQ_INIT media: coda: mark CODA960 firmware version 2.1.9 as supported Steve Longerbeam (1): media: v4l2-ctrls: Fix CID base conflict between MAX217X and IMX drivers/media/pci/sta2x11/sta2x11_vip.c | 6 +++ drivers/media/platform/coda/coda-bit.c| 38 + drivers/media/platform/coda/coda-common.c | 118 +++-- drivers/media/platform/coda/coda.h| 2 + drivers/media/platform/coda/coda_regs.h | 1 + drivers/media/platform/fsl-viu.c | 38 +++-- drivers/media/platform/m2m-deinterlace.c | 4 ++ drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c | 20 ++- drivers/media/platform/mx2_emmaprp.c | 4 ++ drivers/media/platform/s5p-g2d/g2d.c | 2 + drivers/media/platform/vivid/vivid-ctrls.c| 2 +- drivers/media/usb/stk1160/stk1160-v4l.c | 2 +- drivers/media/v4l2-core/v4l2-ioctl.c | 56 ++- drivers/staging/media/davinci_vpfe/vpfe_video.c | 6 ++- drivers/staging/media/davinci_vpfe/vpfe_video.h | 2 +- drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 24 ++--- include/uapi/linux/v4l2-controls.h| 2 +- 17 files changed, 242 insertions(+), 85 deletions(-)
Re: [PATCHv15 02/35] media-request: implement media requests
On 02/07/18 10:58, Tomasz Figa wrote:
> Hi Hans,
>
> On Fri, Jun 15, 2018 at 4:09 PM Hans Verkuil wrote:
>>
>> On 04/06/18 13:46, Hans Verkuil wrote:
>>> From: Hans Verkuil
>>>
>>> Add initial media request support:
>>>
>>> 1) Add MEDIA_IOC_REQUEST_ALLOC ioctl support to media-device.c
>>> 2) Add struct media_request to store request objects.
>>> 3) Add struct media_request_object to represent a request object.
>>> 4) Add MEDIA_REQUEST_IOC_QUEUE/REINIT ioctl support.
>>>
>>> Basic lifecycle: the application allocates a request, adds
>>> objects to it, queues the request, polls until it is completed
>>> and can then read the final values of the objects at the time
>>> of completion. When it closes the file descriptor the request
>>> memory will be freed (actually, when the last user of that request
>>> releases the request).
>>>
>>> Drivers will bind an object to a request (the 'adds objects to it'
>>> phase), when MEDIA_REQUEST_IOC_QUEUE is called the request is
>>> validated (req_validate op), then queued (the req_queue op).
>>>
>>> When done with an object it can either be unbound from the request
>>> (e.g. when the driver has finished with a vb2 buffer) or marked as
>>> completed (e.g. for controls associated with a buffer). When all
>>> objects in the request are completed (or unbound), then the request
>>> fd will signal an exception (poll).
>>>
>>> Signed-off-by: Hans Verkuil
>>> Co-developed-by: Sakari Ailus
>>> Signed-off-by: Sakari Ailus
>>> Co-developed-by: Laurent Pinchart
>>> Co-developed-by: Alexandre Courbot
>>> ---
>>> drivers/media/Makefile| 3 +-
>>> drivers/media/media-device.c | 14 ++
>>> drivers/media/media-request.c | 421 ++
>>> include/media/media-device.h | 24 ++
>>> include/media/media-request.h | 326 ++
>>> 5 files changed, 787 insertions(+), 1 deletion(-)
>>> create mode 100644 drivers/media/media-request.c
>>> create mode 100644 include/media/media-request.h
>>>
>>
>>
>>
>>> diff --git a/include/media/media-request.h b/include/media/media-request.h
>>> new file mode 100644
>>> index ..8acd2627835c
>>> --- /dev/null
>>> +++ b/include/media/media-request.h
>>> @@ -0,0 +1,326 @@
>>
>>
>>
>>> +/**
>>> + * struct media_request - Media device request
>>> + * @mdev: Media device this request belongs to
>>> + * @kref: Reference count
>>> + * @debug_str: Prefix for debug messages (process name:fd)
>>> + * @state: The state of the request
>>> + * @updating_count: count the number of request updates that are in
>>> progress
>>> + * @objects: List of @struct media_request_object request objects
>>> + * @num_incomplete_objects: The number of incomplete objects in the request
>>> + * @poll_wait: Wait queue for poll
>>> + * @lock: Serializes access to this struct
>>> + */
>>> +struct media_request {
>>> + struct media_device *mdev;
>>> + struct kref kref;
>>> + char debug_str[TASK_COMM_LEN + 11];
>>> + enum media_request_state state;
>>> + refcount_t updating_count;
>>
>> This will be replaced by unsigned int: it turns out that if
>> CONFIG_REFCOUNT_FULL is set,
>> the refcount implementation will complain when you increase the refcount
>> from 0 to 1
>> since it expects that refcount_t it used as it is in kref. Since
>> updating_count is
>> protected by the spinlock anyway there is no need to use refcount_t, a simple
>> unsigned int works just as well.
>
> It seems to be read in media_request_clean(), which can be called by
> media_request_release() or media_request_ioctl_reinit() and neither
> acquires the spinlock for the time of the call.
The request object is in state CLEANING at that time, and that guarantees
that nobody else will make changes to the request.
Regards,
Hans
>
> Agreed, though, that refcount_t doesn't really match what we need
> updating_count here for.
>
> Best regards,
> Tomasz
>
Re: [PATCHv15 02/35] media-request: implement media requests
Hi Hans,
On Fri, Jun 15, 2018 at 4:09 PM Hans Verkuil wrote:
>
> On 04/06/18 13:46, Hans Verkuil wrote:
> > From: Hans Verkuil
> >
> > Add initial media request support:
> >
> > 1) Add MEDIA_IOC_REQUEST_ALLOC ioctl support to media-device.c
> > 2) Add struct media_request to store request objects.
> > 3) Add struct media_request_object to represent a request object.
> > 4) Add MEDIA_REQUEST_IOC_QUEUE/REINIT ioctl support.
> >
> > Basic lifecycle: the application allocates a request, adds
> > objects to it, queues the request, polls until it is completed
> > and can then read the final values of the objects at the time
> > of completion. When it closes the file descriptor the request
> > memory will be freed (actually, when the last user of that request
> > releases the request).
> >
> > Drivers will bind an object to a request (the 'adds objects to it'
> > phase), when MEDIA_REQUEST_IOC_QUEUE is called the request is
> > validated (req_validate op), then queued (the req_queue op).
> >
> > When done with an object it can either be unbound from the request
> > (e.g. when the driver has finished with a vb2 buffer) or marked as
> > completed (e.g. for controls associated with a buffer). When all
> > objects in the request are completed (or unbound), then the request
> > fd will signal an exception (poll).
> >
> > Signed-off-by: Hans Verkuil
> > Co-developed-by: Sakari Ailus
> > Signed-off-by: Sakari Ailus
> > Co-developed-by: Laurent Pinchart
> > Co-developed-by: Alexandre Courbot
> > ---
> > drivers/media/Makefile| 3 +-
> > drivers/media/media-device.c | 14 ++
> > drivers/media/media-request.c | 421 ++
> > include/media/media-device.h | 24 ++
> > include/media/media-request.h | 326 ++
> > 5 files changed, 787 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/media/media-request.c
> > create mode 100644 include/media/media-request.h
> >
>
>
>
> > diff --git a/include/media/media-request.h b/include/media/media-request.h
> > new file mode 100644
> > index ..8acd2627835c
> > --- /dev/null
> > +++ b/include/media/media-request.h
> > @@ -0,0 +1,326 @@
>
>
>
> > +/**
> > + * struct media_request - Media device request
> > + * @mdev: Media device this request belongs to
> > + * @kref: Reference count
> > + * @debug_str: Prefix for debug messages (process name:fd)
> > + * @state: The state of the request
> > + * @updating_count: count the number of request updates that are in
> > progress
> > + * @objects: List of @struct media_request_object request objects
> > + * @num_incomplete_objects: The number of incomplete objects in the request
> > + * @poll_wait: Wait queue for poll
> > + * @lock: Serializes access to this struct
> > + */
> > +struct media_request {
> > + struct media_device *mdev;
> > + struct kref kref;
> > + char debug_str[TASK_COMM_LEN + 11];
> > + enum media_request_state state;
> > + refcount_t updating_count;
>
> This will be replaced by unsigned int: it turns out that if
> CONFIG_REFCOUNT_FULL is set,
> the refcount implementation will complain when you increase the refcount from
> 0 to 1
> since it expects that refcount_t it used as it is in kref. Since
> updating_count is
> protected by the spinlock anyway there is no need to use refcount_t, a simple
> unsigned int works just as well.
It seems to be read in media_request_clean(), which can be called by
media_request_release() or media_request_ioctl_reinit() and neither
acquires the spinlock for the time of the call.
Agreed, though, that refcount_t doesn't really match what we need
updating_count here for.
Best regards,
Tomasz
Re: [PATCH] media: coda: add SPS fixup code for frame sizes that are not multiples of 16
On 28/06/18 18:47, Philipp Zabel wrote:
> The CODA firmware does not set the VUI frame cropping fields to properly
> describe coded h.264 streams with frame sizes that are not a multiple of
> the macroblock size.
> This adds RBSP parsing code and a SPS fixup routine to manually replace
> the cropping information in the headers produced by the firmware with
> the correct values.
>
> Signed-off-by: Philipp Zabel
> ---
> drivers/media/platform/coda/coda-bit.c | 11 +
> drivers/media/platform/coda/coda-h264.c | 302
> drivers/media/platform/coda/coda.h | 2 +
> 3 files changed, 315 insertions(+)
>
> diff --git a/drivers/media/platform/coda/coda-bit.c
> b/drivers/media/platform/coda/coda-bit.c
> index 94ba3cc3bf14..d6380a10fa2c 100644
> --- a/drivers/media/platform/coda/coda-bit.c
> +++ b/drivers/media/platform/coda/coda-bit.c
> @@ -1197,6 +1197,17 @@ static int coda_start_encoding(struct coda_ctx *ctx)
> if (ret < 0)
> goto out;
>
> + if ((q_data_src->rect.width % 16) ||
> + (q_data_src->rect.height % 16)) {
> + ret = coda_sps_fixup(ctx, q_data_src->rect.width,
> + q_data_src->rect.height,
> + >vpu_header[0][0],
> + >vpu_header_size[0],
> + sizeof(ctx->vpu_header[0]));
You need to add a comment here why this is needed.
> + if (ret < 0)
> + goto out;
> + }
> +
> /*
>* Get PPS in the first frame and copy it to an
>* intermediate buffer.
> diff --git a/drivers/media/platform/coda/coda-h264.c
> b/drivers/media/platform/coda/coda-h264.c
> index 0e27412e01f5..8ec5a5d076c0 100644
> --- a/drivers/media/platform/coda/coda-h264.c
> +++ b/drivers/media/platform/coda/coda-h264.c
> @@ -111,3 +111,305 @@ int coda_h264_level(int level_idc)
> default: return -EINVAL;
> }
> }
> +
> +struct rbsp {
> + char *buf;
> + int size;
> + int pos;
> +};
> +
> +static inline int rbsp_read_bit(struct rbsp *rbsp)
> +{
> + int shift = 7 - (rbsp->pos % 8);
> + int ofs = rbsp->pos++ / 8;
> +
> + if (ofs >= rbsp->size)
> + return -EINVAL;
> +
> + return (rbsp->buf[ofs] >> shift) & 1;
> +}
> +
> +static inline int rbsp_write_bit(struct rbsp *rbsp, int bit)
> +{
> + int shift = 7 - (rbsp->pos % 8);
> + int ofs = rbsp->pos++ / 8;
> +
> + if (ofs >= rbsp->size)
> + return -EINVAL;
> +
> + rbsp->buf[ofs] &= ~(1 << shift);
> + rbsp->buf[ofs] |= bit << shift;
> +
> + return 0;
> +}
> +
> +static inline int rbsp_read_bits(struct rbsp *rbsp, int num, int *val)
> +{
> + int i, ret;
> + int tmp = 0;
> +
> + if (num > 32)
> + return -EINVAL;
> +
> + for (i = 0; i < num; i++) {
> + ret = rbsp_read_bit(rbsp);
> + if (ret < 0)
> + return ret;
> + tmp |= ret << (num - i - 1);
> + }
> +
> + if (val)
> + *val = tmp;
> +
> + return 0;
> +}
> +
> +static int rbsp_write_bits(struct rbsp *rbsp, int num, int value)
> +{
> + int ret;
> +
> + while (num--) {
> + ret = rbsp_write_bit(rbsp, (value >> num) & 1);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int rbsp_read_uev(struct rbsp *rbsp, unsigned int *val)
> +{
> + int leading_zero_bits = 0;
> + unsigned int tmp = 0;
> + int ret;
> +
> + while ((ret = rbsp_read_bit(rbsp)) == 0)
> + leading_zero_bits++;
> + if (ret < 0)
> + return ret;
> +
> + if (leading_zero_bits > 0) {
> + ret = rbsp_read_bits(rbsp, leading_zero_bits, );
> + if (ret)
> + return ret;
> + }
> +
> + if (val)
> + *val = (1 << leading_zero_bits) - 1 + tmp;
> +
> + return 0;
> +}
> +
> +static int rbsp_write_uev(struct rbsp *rbsp, unsigned int value)
> +{
> + int i;
> + int ret;
> + int tmp = value + 1;
> + int leading_zero_bits = fls(tmp) - 1;
> +
> + for (i = 0; i < leading_zero_bits; i++) {
> + ret = rbsp_write_bit(rbsp, 0);
> + if (ret)
> + return ret;
> + }
> +
> + return rbsp_write_bits(rbsp, leading_zero_bits + 1, tmp);
> +}
> +
> +static int rbsp_read_sev(struct rbsp *rbsp, int *val)
> +{
> + unsigned int tmp;
> + int ret;
> +
> + ret = rbsp_read_uev(rbsp, );
> + if (ret)
> + return ret;
> +
> + if (val) {
> + if (tmp & 1)
> + *val = (tmp + 1) / 2;
> + else
> + *val = -(tmp / 2);
> + }
> +
> + return 0;
> +}
> +
> +int coda_sps_fixup(struct coda_ctx *ctx, int width, int height, char *buf,
Re: [PATCH v4 03/17] omap4iss: Add vb2_queue lock
On 15/06/18 21:07, Ezequiel Garcia wrote:
> vb2_queue lock is now mandatory. Add it, remove driver ad-hoc
> locks, and implement wait_{prepare, finish}.
I don't see any wait_prepare/finish implementation?!
Regards,
Hans
>
> Signed-off-by: Ezequiel Garcia
> ---
> drivers/staging/media/omap4iss/iss_video.c | 13 ++---
> 1 file changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/media/omap4iss/iss_video.c
> b/drivers/staging/media/omap4iss/iss_video.c
> index a3a83424a926..d919bae83828 100644
> --- a/drivers/staging/media/omap4iss/iss_video.c
> +++ b/drivers/staging/media/omap4iss/iss_video.c
> @@ -873,8 +873,6 @@ iss_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type)
> if (type != video->type)
> return -EINVAL;
>
> - mutex_lock(>stream_lock);
> -
> /*
>* Start streaming on the pipeline. No link touching an entity in the
>* pipeline can be activated or deactivated once streaming is started.
> @@ -978,8 +976,6 @@ iss_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type)
>
> media_graph_walk_cleanup();
>
> - mutex_unlock(>stream_lock);
> -
> return 0;
>
> err_omap4iss_set_stream:
> @@ -996,8 +992,6 @@ iss_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type)
> err_graph_walk_init:
> media_entity_enum_cleanup(>ent_enum);
>
> - mutex_unlock(>stream_lock);
> -
> return ret;
> }
>
> @@ -1013,10 +1007,8 @@ iss_video_streamoff(struct file *file, void *fh, enum
> v4l2_buf_type type)
> if (type != video->type)
> return -EINVAL;
>
> - mutex_lock(>stream_lock);
> -
> if (!vb2_is_streaming(>queue))
> - goto done;
> + return 0;
>
> /* Update the pipeline state. */
> if (video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> @@ -1041,8 +1033,6 @@ iss_video_streamoff(struct file *file, void *fh, enum
> v4l2_buf_type type)
> video->iss->pdata->set_constraints(video->iss, false);
> media_pipeline_stop(>video.entity);
>
> -done:
> - mutex_unlock(>stream_lock);
> return 0;
> }
>
> @@ -1137,6 +1127,7 @@ static int iss_video_open(struct file *file)
> q->buf_struct_size = sizeof(struct iss_buffer);
> q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> q->dev = video->iss->dev;
> + q->lock = >stream_lock;
>
> ret = vb2_queue_init(q);
> if (ret) {
>
[RFC] Make entity to interface links immutable
While working on v4l2-compliance I noticed that entity to interface links have just the MEDIA_LNK_FL_ENABLED flag set. Shouldn't we also set the MEDIA_LNK_FL_IMMUTABLE? After all, you cannot change an entity-interface link. It feels inconsistent not to have this flag. I also propose that media_create_intf_link() drops the last flags argument: it can set the link flags directly since they are always the same anyway. Regards, Hans
Re: [PATCH v6 12/12] intel-ipu3: Add imgu top level pci device driver
Hi Yong,
On Fri, Mar 30, 2018 at 11:15 AM Yong Zhi wrote:
> +/*
> + * Queue as many buffers to CSS as possible. If all buffers don't fit into
> + * CSS buffer queues, they remain unqueued and will be queued later.
> + */
> +int imgu_queue_buffers(struct imgu_device *imgu, bool initial)
> +{
> + unsigned int node;
> + int r = 0;
> + struct imgu_buffer *ibuf;
> +
> + if (!ipu3_css_is_streaming(>css))
> + return 0;
> +
> + mutex_lock(>lock);
> +
> + /* Buffer set is queued to FW only when input buffer is ready */
> + if (!imgu_queue_getbuf(imgu, IMGU_NODE_IN)) {
> + mutex_unlock(>lock);
> + return 0;
> + }
> + for (node = IMGU_NODE_IN + 1; 1; node = (node + 1) % IMGU_NODE_NUM) {
Shouldn't we make (node != IMGU_NODE_IN || imgu_queue_getbuf(imgu,
IMGU_NODE_IN)) the condition here, rather than 1?
This would also let us remove the explicit call to imgu_queue_getbuf()
above the loop.
> + if (node == IMGU_NODE_VF &&
> + (imgu->css.pipe_id == IPU3_CSS_PIPE_ID_CAPTURE ||
> +!imgu->nodes[IMGU_NODE_VF].enabled)) {
> + continue;
> + } else if (node == IMGU_NODE_PV &&
> + (imgu->css.pipe_id == IPU3_CSS_PIPE_ID_VIDEO ||
> + !imgu->nodes[IMGU_NODE_PV].enabled)) {
> + continue;
> + } else if (imgu->queue_enabled[node]) {
> + struct ipu3_css_buffer *buf =
> + imgu_queue_getbuf(imgu, node);
> + int dummy;
> +
> + if (!buf)
> + break;
> +
> + r = ipu3_css_buf_queue(>css, buf);
> + if (r)
> + break;
> + dummy = imgu_dummybufs_check(imgu, buf);
> + if (!dummy)
> + ibuf = container_of(buf, struct imgu_buffer,
> + css_buf);
> + dev_dbg(>pci_dev->dev,
> + "queue %s %s buffer %d to css da: 0x%08x\n",
> + dummy ? "dummy" : "user",
> + imgu_node_map[node].name,
> + dummy ? 0 : ibuf->vid_buf.vbb.vb2_buf.index,
> + (u32)buf->daddr);
> + }
> + if (node == IMGU_NODE_IN &&
> + !imgu_queue_getbuf(imgu, IMGU_NODE_IN))
> + break;
My suggestion to the for loop condition is based on this.
> + }
> + mutex_unlock(>lock);
> +
> + if (r && r != -EBUSY)
> + goto failed;
> +
> + return 0;
> +
> +failed:
> + /*
> +* On error, mark all buffers as failed which are not
> +* yet queued to CSS
> +*/
> + dev_err(>pci_dev->dev,
> + "failed to queue buffer to CSS on queue %i (%d)\n",
> + node, r);
> +
> + if (initial)
> + /* If we were called from streamon(), no need to finish bufs
> */
> + return r;
> +
> + for (node = 0; node < IMGU_NODE_NUM; node++) {
> + struct imgu_buffer *buf, *buf0;
> +
> + if (!imgu->queue_enabled[node])
> + continue; /* Skip disabled queues */
> +
> + mutex_lock(>lock);
> + list_for_each_entry_safe(buf, buf0,
> >nodes[node].buffers,
> +vid_buf.list) {
> + if (ipu3_css_buf_state(>css_buf) ==
> + IPU3_CSS_BUFFER_QUEUED)
> + continue; /* Was already queued, skip */
> +
> + ipu3_v4l2_buffer_done(>vid_buf.vbb.vb2_buf,
> + VB2_BUF_STATE_ERROR);
> + }
> + mutex_unlock(>lock);
> + }
> +
> + return r;
> +}
[snip]
> +static irqreturn_t imgu_isr_threaded(int irq, void *imgu_ptr)
> +{
> + struct imgu_device *imgu = imgu_ptr;
> +
> + /* Dequeue / queue buffers */
> + do {
> + u64 ns = ktime_get_ns();
> + struct ipu3_css_buffer *b;
> + struct imgu_buffer *buf;
> + unsigned int node;
> + bool dummy;
> +
> + do {
> + mutex_lock(>lock);
> + b = ipu3_css_buf_dequeue(>css);
> + mutex_unlock(>lock);
> + } while (PTR_ERR(b) == -EAGAIN);
> +
> + if (IS_ERR_OR_NULL(b)) {
> + if (!b || PTR_ERR(b) == -EBUSY) /* All done */
> + break;
> + dev_err(>pci_dev->dev,
> +
Re: [PATCH v3 03/13] media: v4l2: async: Add v4l2_async_notifier_add_subdev
On Tue, Jun 26, 2018 at 01:47:45PM -0700, Steve Longerbeam wrote:
>
>
> On 06/26/2018 12:12 AM, Sakari Ailus wrote:
> > On Wed, May 09, 2018 at 04:06:32PM -0700, Steve Longerbeam wrote:
> > >
> > > On 05/08/2018 03:12 AM, Sakari Ailus wrote:
> > > > On Fri, Apr 20, 2018 at 10:12:33AM -0700, Steve Longerbeam wrote:
> > > > > Hi Sakari,
> > > > >
> > > > >
> > > > > On 04/20/2018 05:24 AM, Sakari Ailus wrote:
> > > > > > Hi Steve,
> > > > > >
> > > > > > Thanks for the patchset.
> > > > > >
> > > > > > On Tue, Mar 20, 2018 at 05:37:19PM -0700, Steve Longerbeam wrote:
> > > > > > > v4l2_async_notifier_add_subdev() adds an asd to the notifier. It
> > > > > > > checks
> > > > > > > that the asd's match_type is valid and that no other equivalent
> > > > > > > asd's
> > > > > > > have already been added to this notifier's asd list, or to other
> > > > > > > registered notifier's waiting or done lists, and increments
> > > > > > > num_subdevs.
> > > > > > >
> > > > > > > v4l2_async_notifier_add_subdev() does not make use of the
> > > > > > > notifier subdevs
> > > > > > > array, otherwise it would have to re-allocate the array every
> > > > > > > time the
> > > > > > > function was called. In place of the subdevs array, the function
> > > > > > > adds
> > > > > > > the asd to a new master asd_list. The function will return error
> > > > > > > with a
> > > > > > > WARN() if it is ever called with the subdevs array allocated.
> > > > > > >
> > > > > > > In v4l2_async_notifier_has_async_subdev(),
> > > > > > > __v4l2_async_notifier_register(),
> > > > > > > and v4l2_async_notifier_cleanup(), alternatively operate on the
> > > > > > > subdevs
> > > > > > > array or a non-empty notifier->asd_list.
> > > > > > I do agree with the approach, but this patch leaves the remaining
> > > > > > users of
> > > > > > the subdevs array in the notifier intact. Could you rework them to
> > > > > > use the
> > > > > > v4l2_async_notifier_add_subdev() as well?
> > > > > >
> > > > > > There seem to be just a few of them --- exynos4-is and rcar_drif.
> > > > > I count more than a few :)
> > > > >
> > > > > % cd drivers/media && grep -l -r --include "*.[ch]"
> > > > > 'notifier[\.\-]>*subdevs[ ]*='
> > > > > v4l2-core/v4l2-async.c
> > > > > platform/pxa_camera.c
> > > > > platform/ti-vpe/cal.c
> > > > > platform/exynos4-is/media-dev.c
> > > > > platform/qcom/camss-8x16/camss.c
> > > > > platform/soc_camera/soc_camera.c
> > > > > platform/atmel/atmel-isi.c
> > > > > platform/atmel/atmel-isc.c
> > > > > platform/stm32/stm32-dcmi.c
> > > > > platform/davinci/vpif_capture.c
> > > > > platform/davinci/vpif_display.c
> > > > > platform/renesas-ceu.c
> > > > > platform/am437x/am437x-vpfe.c
> > > > > platform/xilinx/xilinx-vipp.c
> > > > > platform/rcar_drif.c
> > > > >
> > > > >
> > > > > So not including v4l2-core, the list is:
> > > > >
> > > > > pxa_camera
> > > > > ti-vpe
> > > > > exynos4-is
> > > > > qcom
> > > > > soc_camera
> > > > > atmel
> > > > > stm32
> > > > > davinci
> > > > > renesas-ceu
> > > > > am437x
> > > > > xilinx
> > > > > rcar_drif
> > > > >
> > > > >
> > > > > Given such a large list of the users of the notifier->subdevs array,
> > > > > I think this should be done is two steps: submit this patchset first,
> > > > > that keeps the backward compatibility, and then a subsequent patchset
> > > > > that converts all drivers to use v4l2_async_notifier_add_subdev(), at
> > > > > which point the subdevs array can be removed from struct
> > > > > v4l2_async_notifier.
> > > > >
> > > > > Or, do you still think this should be done all at once? I could add a
> > > > > large patch to this patchset that does the conversion and removes
> > > > > the subdevs array.
> > > > Sorry for the delay. I grepped for this, too, but I guess I ended up
> > > > using
> > > > an expression that missed most of the users. :-(
> > > >
> > > > I think it'd be very good to perform that conversion --- the code in the
> > > > framework would be quite a bit cleaner and easier to maintain whereas
> > > > the
> > > > per-driver conversions seem pretty simple, such as this on in
> > > > drivers/media/platform/atmel/atmel-isi.c :
> > > >
> > > > /* Register the subdevices notifier. */
> > > > subdevs = devm_kzalloc(isi->dev, sizeof(*subdevs), GFP_KERNEL);
> > > > if (!subdevs) {
> > > > of_node_put(isi->entity.node);
> > > > return -ENOMEM;
> > > > }
> > > >
> > > > subdevs[0] = >entity.asd;
> > > >
> > > > isi->notifier.subdevs = subdevs;
> > > > isi->notifier.num_subdevs = 1;
> > > > isi->notifier.ops = _graph_notify_ops;
> > >
> > > Yes, the conversions are fairly straightforward. I've completed that work,
> > > but it was a very manual conversion, every platform is different and
> > > needed
> > > careful study.
> > >
> > > Although I was careful about getting the error-out paths correct, there
> > > could
> > > be
Re: [PATCH v6 11/12] intel-ipu3: Add v4l2 driver based on media framework
Hi Yong,
On Fri, Mar 30, 2018 at 11:15 AM Yong Zhi wrote:
[snip]
> +static int ipu3_vidioc_enum_input(struct file *file, void *fh,
> + struct v4l2_input *input)
> +{
> + if (input->index > 0)
> + return -EINVAL;
> + strlcpy(input->name, "camera", sizeof(input->name));
> + input->type = V4L2_INPUT_TYPE_CAMERA;
> +
> + return 0;
> +}
> +
> +static int ipu3_vidioc_g_input(struct file *file, void *fh, unsigned int
> *input)
> +{
> + *input = 0;
> +
> + return 0;
> +}
> +
> +static int ipu3_vidioc_s_input(struct file *file, void *fh, unsigned int
> input)
> +{
> + return input == 0 ? 0 : -EINVAL;
> +}
> +
> +static int ipu3_vidioc_enum_output(struct file *file, void *fh,
> + struct v4l2_output *output)
> +{
> + if (output->index > 0)
> + return -EINVAL;
> + strlcpy(output->name, "camera", sizeof(output->name));
> + output->type = V4L2_INPUT_TYPE_CAMERA;
> +
> + return 0;
> +}
> +
> +static int ipu3_vidioc_g_output(struct file *file, void *fh,
> + unsigned int *output)
> +{
> + *output = 0;
> +
> + return 0;
> +}
> +
> +static int ipu3_vidioc_s_output(struct file *file, void *fh,
> + unsigned int output)
> +{
> + return output == 0 ? 0 : -EINVAL;
> +}
Do we really need to implement the 6 functions above? They don't seem
to be doing anything useful.
[snip]
> +int ipu3_v4l2_register(struct imgu_device *imgu)
> +{
> + struct v4l2_mbus_framefmt def_bus_fmt = { 0 };
> + struct v4l2_pix_format_mplane def_pix_fmt = { 0 };
> +
> + int i, r;
> +
> + /* Initialize miscellaneous variables */
> + imgu->streaming = false;
> +
> + /* Set up media device */
> + imgu->media_dev.dev = >pci_dev->dev;
> + strlcpy(imgu->media_dev.model, IMGU_NAME,
> + sizeof(imgu->media_dev.model));
> + snprintf(imgu->media_dev.bus_info, sizeof(imgu->media_dev.bus_info),
> +"%s", dev_name(>pci_dev->dev));
> + imgu->media_dev.hw_revision = 0;
> + media_device_init(>media_dev);
> + r = media_device_register(>media_dev);
> + if (r) {
> + dev_err(>pci_dev->dev,
> + "failed to register media device (%d)\n", r);
> + return r;
> + }
Shouldn't we register the media device at the end, after all video
nodes are registered below? Otherwise, since media_device_register()
exposes the media node to userspace, we risk a race, when userspace
opens the media device before all the entities are created and linked.
[snip]
> +int ipu3_v4l2_unregister(struct imgu_device *imgu)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < IMGU_NODE_NUM; i++) {
> + video_unregister_device(>nodes[i].vdev);
> + media_entity_cleanup(>nodes[i].vdev.entity);
> + mutex_destroy(>nodes[i].lock);
> + }
> +
> + v4l2_device_unregister_subdev(>subdev);
> + media_entity_cleanup(>subdev.entity);
> + kfree(imgu->subdev_pads);
> + v4l2_device_unregister(>v4l2_dev);
> + media_device_unregister(>media_dev);
Should unregister media device at the beginning, so that it cannot be
used when we continue to clean up the entities.
> + media_device_cleanup(>media_dev);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(ipu3_v4l2_unregister);
Best regards,
Tomasz
Re: [PATCH v3 05/13] media: v4l2-fwnode: Add a convenience function for registering subdevs with notifiers
Hi Steve,
On Tue, May 08, 2018 at 08:55:04PM -0700, Steve Longerbeam wrote:
>
>
> On 05/08/2018 03:28 AM, Sakari Ailus wrote:
> > Hi Steve,
> >
> > Again, sorry about the delay. This thread got buried in my inbox. :-(
> > Please see my reply below.
> >
> > On Mon, Apr 23, 2018 at 11:00:22AM -0700, Steve Longerbeam wrote:
> > >
> > > On 04/23/2018 12:14 AM, Sakari Ailus wrote:
> > > > Hi Steve,
> > > >
> > > > On Tue, Mar 20, 2018 at 05:37:21PM -0700, Steve Longerbeam wrote:
> > > > > Adds v4l2_async_register_fwnode_subdev(), which is a convenience
> > > > > function
> > > > > for parsing a sub-device's fwnode port endpoints for connected remote
> > > > > sub-devices, registering a sub-device notifier, and then registering
> > > > > the sub-device itself.
> > > > >
> > > > > Signed-off-by: Steve Longerbeam
> > > > > ---
> > > > > Changes since v2:
> > > > > - fix error-out path in v4l2_async_register_fwnode_subdev() that
> > > > > forgot
> > > > > to put device.
> > > > > Changes since v1:
> > > > > - add #include to v4l2-fwnode.h for
> > > > > 'struct v4l2_subdev' declaration.
> > > > > ---
> > > > >drivers/media/v4l2-core/v4l2-fwnode.c | 101
> > > > > ++
> > > > >include/media/v4l2-fwnode.h | 43 +++
> > > > >2 files changed, 144 insertions(+)
> > > > >
> > > > > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
> > > > > b/drivers/media/v4l2-core/v4l2-fwnode.c
> > > > > index 99198b9..d42024d 100644
> > > > > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > > > > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > > > > @@ -880,6 +880,107 @@ int
> > > > > v4l2_async_register_subdev_sensor_common(struct v4l2_subdev *sd)
> > > > >}
> > > > >EXPORT_SYMBOL_GPL(v4l2_async_register_subdev_sensor_common);
> > > > > +int v4l2_async_register_fwnode_subdev(
> > > > > + struct v4l2_subdev *sd, size_t asd_struct_size,
> > > > > + unsigned int *ports, unsigned int num_ports,
> > > > > + int (*parse_endpoint)(struct device *dev,
> > > > > + struct v4l2_fwnode_endpoint *vep,
> > > > > + struct v4l2_async_subdev *asd))
> > > > > +{
> > > > > + struct v4l2_async_notifier *notifier;
> > > > > + struct device *dev = sd->dev;
> > > > > + struct fwnode_handle *fwnode;
> > > > > + unsigned int subdev_port;
> > > > > + bool is_port;
> > > > > + int ret;
> > > > > +
> > > > > + if (WARN_ON(!dev))
> > > > > + return -ENODEV;
> > > > > +
> > > > > + fwnode = dev_fwnode(dev);
> > > > > + if (!fwnode_device_is_available(fwnode))
> > > > > + return -ENODEV;
> > > > > +
> > > > > + is_port = (is_of_node(fwnode) &&
> > > > > +of_node_cmp(to_of_node(fwnode)->name, "port") == 0);
> > > > What's the intent of this and the code below? You may not parse the
> > > > graph
> > > > data structure here, it should be done in the actual firmware
> > > > implementation instead.
> > > The i.MX6 CSI sub-device registers itself from a port fwnode, so
> > > the intent of the is_port code below is to support the i.MX6 CSI.
> > >
> > > I can remove the is_port checks, but it means
> > > v4l2_async_register_fwnode_subdev() won't be usable by the CSI
> > > sub-device.
> > This won't scale.
>
> The vast majority of sub-devices register themselves as
> port parent nodes. So for now at least, I think
> v4l2_async_register_fwnode_subdev() could be useful to many
> platforms.
It's because the graph bindings define that the port nodes are sub-nodes of
a device node (see Documentation/devicetree/bindings/graph.txt). It's not
exactly the same as doing this in DT but still the kernel implementation
pretty much assumes the same.
>
> > Instead, I think we'd need to separate registering
> > sub-devices (through async sub-devices) and binding them with the driver
> > that registered the notifier. Or at least change how that process works: a
> > single sub-device can well be bound to multiple notifiers,
>
> Ok, that is certainly not the case now, a sub-device can only
> be bound to a single notifier.
>
> > or multiple
> > times to the same notifier while it may be registered only once.
>
> Anyway, this is a future generalization if I understand you
> correctly. Not something to deal with here.
Indeed; just FYI.
>
> >
> > > > Also, sub-devices generally do not match ports.
> > > Yes that's generally true, sub-devices generally match to port parent
> > > nodes. But I do know of one other sub-device that buck that trend.
> > > The ADV748x CSI-2 output sub-devices match against endpoint nodes.
> > Endpoints, yes, but not ports.
>
> Well, the imx CSI registers from a port node.
>
> >
> > > >How sub-devices generally
> > > > correspond to fwnodes is up to the device.
> > > What do you think of adding a v4l2_async_register_port_fwnode_subdev(),
> > > and a v4l2_async_register_endpoint_fwnode_subdev() to support such
Re: [PATCH] media: v4l2-ctrls: Fix CID base conflict between MAX217X and IMX
On Wed, Jun 27, 2018 at 11:39:43AM -0700, Steve Longerbeam wrote:
> When the imx-media driver was initially merged, there was a conflict
> with 8d67ae25 ("media: v4l2-ctrls: Reserve controls for MAX217X") which
> was not fixed up correctly, resulting in V4L2_CID_USER_MAX217X_BASE and
> V4L2_CID_USER_IMX_BASE taking on the same value. Fix by assigning imx
> CID base the next available range at 0x10b0.
>
> Signed-off-by: Steve Longerbeam
Acked-by: Sakari Ailus
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
Re: [PATCH v6 06/12] intel-ipu3: css: Add support for firmware management
Hi Yong,
Continuing my review. Sorry for the delay.
On Fri, Mar 30, 2018 at 11:15 AM Yong Zhi wrote:
>
> Introduce functions to load and install ImgU FW blobs.
>
> Signed-off-by: Yong Zhi
> ---
> drivers/media/pci/intel/ipu3/ipu3-abi.h| 1888
>
> drivers/media/pci/intel/ipu3/ipu3-css-fw.c | 261
> drivers/media/pci/intel/ipu3/ipu3-css-fw.h | 198 +++
> 3 files changed, 2347 insertions(+)
> create mode 100644 drivers/media/pci/intel/ipu3/ipu3-abi.h
> create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-fw.c
> create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-fw.h
>
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-abi.h
> b/drivers/media/pci/intel/ipu3/ipu3-abi.h
> new file mode 100644
> index ..24102647a89e
> --- /dev/null
> +++ b/drivers/media/pci/intel/ipu3/ipu3-abi.h
[snip]
> +/* SYSTEM_REQ_0_5_0_IMGHMMADR */
> +#define IMGU_REG_SYSTEM_REQ0x18
> +#define IMGU_SYSTEM_REQ_FREQ_MASK 0x3f
> +#define IMGU_SYSTEM_REQ_FREQ_DIVIDER 25
> +#define IMGU_REG_INT_STATUS0x30
> +#define IMGU_REG_INT_ENABLE0x34
> +#define IMGU_REG_INT_CSS_IRQ (1 << 31)
BIT(31)
[snip]
> + IMGU_ABI_FRAME_FORMAT_CSI_MIPI_LEGACY_YUV420_8, /* Legacy YUV420.
> +* UY odd line;
> +* VY even line
> +*/
> + IMGU_ABI_FRAME_FORMAT_CSI_MIPI_YUV420_10,/* 10 bit per Y/U/V. Y odd
> + * line; UYVY interleaved
> + * even line
> + */
> + IMGU_ABI_FRAME_FORMAT_YCgCo444_16, /* Internal format for ISP2.7,
Macros and enums should be uppercase.
[snip]
> +struct imgu_abi_shd_intra_frame_operations_data {
> + struct imgu_abi_acc_operation
> + operation_list[IMGU_ABI_SHD_MAX_OPERATIONS] IPU3_ALIGN;
> + struct imgu_abi_acc_process_lines_cmd_data
> + process_lines_data[IMGU_ABI_SHD_MAX_PROCESS_LINES] IPU3_ALIGN;
> + struct imgu_abi_shd_transfer_luts_set_data
> + transfer_data[IMGU_ABI_SHD_MAX_TRANSFERS] IPU3_ALIGN;
> +} __packed;
> +
> +struct imgu_abi_shd_config {
> + struct ipu3_uapi_shd_config_static shd IMGU_ABI_PAD;
> + struct imgu_abi_shd_intra_frame_operations_data shd_ops IMGU_ABI_PAD;
> + struct ipu3_uapi_shd_lut shd_lut IMGU_ABI_PAD;
Definitions of both IPU3_ALIGN and IMGU_ABI_PAD seem to be equivalent.
Could we remove one and use the other everywhere?
[snip]
> +struct imgu_abi_osys_scaler_params {
> + __u32 inp_buf_y_st_addr;
> + __u32 inp_buf_y_line_stride;
> + __u32 inp_buf_y_buffer_stride;
> + __u32 inp_buf_u_st_addr;
> + __u32 inp_buf_v_st_addr;
> + __u32 inp_buf_uv_line_stride;
> + __u32 inp_buf_uv_buffer_stride;
> + __u32 inp_buf_chunk_width;
> + __u32 inp_buf_nr_buffers;
> + /* Output buffers */
> + __u32 out_buf_y_st_addr;
> + __u32 out_buf_y_line_stride;
> + __u32 out_buf_y_buffer_stride;
> + __u32 out_buf_u_st_addr;
> + __u32 out_buf_v_st_addr;
> + __u32 out_buf_uv_line_stride;
> + __u32 out_buf_uv_buffer_stride;
> + __u32 out_buf_nr_buffers;
> + /* Intermediate buffers */
> + __u32 int_buf_y_st_addr;
> + __u32 int_buf_y_line_stride;
> + __u32 int_buf_u_st_addr;
> + __u32 int_buf_v_st_addr;
> + __u32 int_buf_uv_line_stride;
> + __u32 int_buf_height;
> + __u32 int_buf_chunk_width;
> + __u32 int_buf_chunk_height;
> + /* Context buffers */
> + __u32 ctx_buf_hor_y_st_addr;
> + __u32 ctx_buf_hor_u_st_addr;
> + __u32 ctx_buf_hor_v_st_addr;
> + __u32 ctx_buf_ver_y_st_addr;
> + __u32 ctx_buf_ver_u_st_addr;
> + __u32 ctx_buf_ver_v_st_addr;
> + /* Addresses for release-input and process-output tokens */
> + __u32 release_inp_buf_addr;
> + __u32 release_inp_buf_en;
> + __u32 release_out_buf_en;
> + __u32 process_out_buf_addr;
> + /* Settings dimensions, padding, cropping */
> + __u32 input_image_y_width;
> + __u32 input_image_y_height;
> + __u32 input_image_y_start_column;
> + __u32 input_image_uv_start_column;
> + __u32 input_image_y_left_pad;
> + __u32 input_image_uv_left_pad;
> + __u32 input_image_y_right_pad;
> + __u32 input_image_uv_right_pad;
> + __u32 input_image_y_top_pad;
> + __u32 input_image_uv_top_pad;
> + __u32 input_image_y_bottom_pad;
> + __u32 input_image_uv_bottom_pad;
> + __u32 processing_mode;
> +#define IMGU_ABI_OSYS_PROCMODE_BYPASS 0
> +#define IMGU_ABI_OSYS_PROCMODE_UPSCALE 1
> +#define IMGU_ABI_OSYS_PROCMODE_DOWNSCALE 2
