RE: [PATCH v3] media: imx208: Add imx208 camera sensor driver
Hi Tomasz,
Please see my comments.
-Original Message-
>Hi Ping-chung,
>On Wed, Aug 1, 2018 at 11:31 AM Ping-chung Chen
>wrote:
>
> From: "Chen, Ping-chung"
>
> Add a V4L2 sub-device driver for the Sony IMX208 image sensor.
> This is a camera sensor using the I2C bus for control and the
> CSI-2 bus for data.
>
> Signed-off-by: Ping-Chung Chen
> ---
> since v1:
> -- Update the function media_entity_pads_init for upstreaming.
> -- Change the structure name mutex as imx208_mx.
> -- Refine the control flow of test pattern function.
> -- vflip/hflip control support (will impact the output bayer order)
> -- support 4 bayer orders output (via change v/hflip)
> - SRGGB10(default), SGRBG10, SGBRG10, SBGGR10
> -- Simplify error handling in the set_stream function.
> since v2:
> -- Refine coding style.
> -- Fix the if statement to use pm_runtime_get_if_in_use().
> -- Print more error log during error handling.
> -- Remove mutex_destroy() from imx208_free_controls().
> -- Add more comments.
>Thanks for addressing the comments. There are still few remaining, though.
>Please see inline.
[snip]
> +enum {
> + IMX208_LINK_FREQ_384MHZ_INDEX,
> + IMX208_LINK_FREQ_96MHZ_INDEX,
> +};
> +
> +/*
> + * pixel_rate = link_freq * data-rate * nr_of_lanes / bits_per_sample
> + * data rate => double data rate; number of lanes => 2; bits per
> +pixel => 10 */ static u64 link_freq_to_pixel_rate(u64 f) {
> + f *= IMX208_DATA_RATE_DOUBLE * IMX208_NUM_OF_LANES;
> + do_div(f, IMX208_PIXEL_BITS);
> +
> + return f;
> +}
> +
> +/* Menu items for LINK_FREQ V4L2 control */ static const s64
> +link_freq_menu_items[] = {
> + IMX208_LINK_FREQ_384MHZ,
> + IMX208_LINK_FREQ_96MHZ,
>I asked for having explicit indices using IMX208_LINK_FREQ_*_INDEX enum used
>here too.
Sorry I forgot this one, fixed.
> +};
> +
> +/* Link frequency configs */
> +static const struct imx208_link_freq_config link_freq_configs[] = {
> + [IMX208_LINK_FREQ_384MHZ_INDEX] = {
> + .pixels_per_line = IMX208_PPL_384MHZ,
> + .reg_list = {
> + .num_of_regs = ARRAY_SIZE(pll_ctrl_reg),
> + .regs = pll_ctrl_reg,
> + }
> + },
> + [IMX208_LINK_FREQ_96MHZ_INDEX] = {
> + .pixels_per_line = IMX208_PPL_96MHZ,
> + .reg_list = {
> + .num_of_regs = ARRAY_SIZE(pll_ctrl_reg),
> + .regs = pll_ctrl_reg,
> + }
> + },
> +};
> +
> +/* Mode configs */
> +static const struct imx208_mode supported_modes[] = {
> + [IMX208_LINK_FREQ_384MHZ_INDEX] = {
>This is not the right index for this array (even if numerically the same. This
>array is just a list of available modes (resolutions) and there is no other
>data structure referring to particular entries of it, so it doesn't need
>explicit indexing.
Done.
> + .width = 1936,
> + .height = 1096,
> + .vts_def = IMX208_VTS_60FPS,
> + .vts_min = IMX208_VTS_60FPS_MIN,
> + .reg_list = {
> + .num_of_regs = ARRAY_SIZE(mode_1936x1096_60fps_regs),
> + .regs = mode_1936x1096_60fps_regs,
> + },
> + .link_freq_index = IMX208_LINK_FREQ_384MHZ_INDEX,
> + },
> + [IMX208_LINK_FREQ_96MHZ_INDEX] = {
>Ditto.
Best regards,
Tomasz
Re: [PATCH v3] media: imx208: Add imx208 camera sensor driver
Hi Ping-chung,
On Wed, Aug 1, 2018 at 11:31 AM Ping-chung Chen
wrote:
>
> From: "Chen, Ping-chung"
>
> Add a V4L2 sub-device driver for the Sony IMX208 image sensor.
> This is a camera sensor using the I2C bus for control and the
> CSI-2 bus for data.
>
> Signed-off-by: Ping-Chung Chen
> ---
> since v1:
> -- Update the function media_entity_pads_init for upstreaming.
> -- Change the structure name mutex as imx208_mx.
> -- Refine the control flow of test pattern function.
> -- vflip/hflip control support (will impact the output bayer order)
> -- support 4 bayer orders output (via change v/hflip)
> - SRGGB10(default), SGRBG10, SGBRG10, SBGGR10
> -- Simplify error handling in the set_stream function.
> since v2:
> -- Refine coding style.
> -- Fix the if statement to use pm_runtime_get_if_in_use().
> -- Print more error log during error handling.
> -- Remove mutex_destroy() from imx208_free_controls().
> -- Add more comments.
Thanks for addressing the comments. There are still few remaining,
though. Please see inline.
[snip]
> +enum {
> + IMX208_LINK_FREQ_384MHZ_INDEX,
> + IMX208_LINK_FREQ_96MHZ_INDEX,
> +};
> +
> +/*
> + * pixel_rate = link_freq * data-rate * nr_of_lanes / bits_per_sample
> + * data rate => double data rate; number of lanes => 2; bits per pixel => 10
> + */
> +static u64 link_freq_to_pixel_rate(u64 f)
> +{
> + f *= IMX208_DATA_RATE_DOUBLE * IMX208_NUM_OF_LANES;
> + do_div(f, IMX208_PIXEL_BITS);
> +
> + return f;
> +}
> +
> +/* Menu items for LINK_FREQ V4L2 control */
> +static const s64 link_freq_menu_items[] = {
> + IMX208_LINK_FREQ_384MHZ,
> + IMX208_LINK_FREQ_96MHZ,
I asked for having explicit indices using IMX208_LINK_FREQ_*_INDEX
enum used here too.
> +};
> +
> +/* Link frequency configs */
> +static const struct imx208_link_freq_config link_freq_configs[] = {
> + [IMX208_LINK_FREQ_384MHZ_INDEX] = {
> + .pixels_per_line = IMX208_PPL_384MHZ,
> + .reg_list = {
> + .num_of_regs = ARRAY_SIZE(pll_ctrl_reg),
> + .regs = pll_ctrl_reg,
> + }
> + },
> + [IMX208_LINK_FREQ_96MHZ_INDEX] = {
> + .pixels_per_line = IMX208_PPL_96MHZ,
> + .reg_list = {
> + .num_of_regs = ARRAY_SIZE(pll_ctrl_reg),
> + .regs = pll_ctrl_reg,
> + }
> + },
> +};
> +
> +/* Mode configs */
> +static const struct imx208_mode supported_modes[] = {
> + [IMX208_LINK_FREQ_384MHZ_INDEX] = {
This is not the right index for this array (even if numerically the
same. This array is just a list of available modes (resolutions) and
there is no other data structure referring to particular entries of
it, so it doesn't need explicit indexing.
> + .width = 1936,
> + .height = 1096,
> + .vts_def = IMX208_VTS_60FPS,
> + .vts_min = IMX208_VTS_60FPS_MIN,
> + .reg_list = {
> + .num_of_regs = ARRAY_SIZE(mode_1936x1096_60fps_regs),
> + .regs = mode_1936x1096_60fps_regs,
> + },
> + .link_freq_index = IMX208_LINK_FREQ_384MHZ_INDEX,
> + },
> + [IMX208_LINK_FREQ_96MHZ_INDEX] = {
Ditto.
Best regards,
Tomasz
cron job: media_tree daily build: WARNINGS
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: Thu Aug 2 05:00:10 CEST 2018 media-tree git hash:1d06352e18ef502e30837cedfe618298816fb48c media_build git hash: e75fbc93fc427f769c3ce5a020bf204e02a45852 v4l-utils git hash: 5583f43ef1a4814c2bd3c43cb06461b7f532b141 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-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: WARNINGS linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: WARNINGS 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.102-i686: OK linux-3.2.102-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.57-i686: OK linux-3.16.57-x86_64: OK linux-3.17.8-i686: OK linux-3.17.8-x86_64: OK linux-3.18.115-i686: OK linux-3.18.115-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.52-i686: OK linux-4.1.52-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.140-i686: OK linux-4.4.140-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.112-i686: OK linux-4.9.112-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.55-i686: OK linux-4.14.55-x86_64: OK linux-4.15.18-i686: OK linux-4.15.18-x86_64: OK linux-4.16.18-i686: OK linux-4.16.18-x86_64: OK linux-4.17.6-i686: OK linux-4.17.6-x86_64: OK linux-4.18-rc4-i686: OK linux-4.18-rc4-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Spende von $ 4,800,000.00!
Hallo, Sie haben eine Spende von $ 4,800,000.00, ich habe die America Lotterie im Wert von $ 40 Millionen gewonnen und ich gebe einen Teil davon an fünf glückliche Menschen und Wohltätigkeitseinrichtungen zum Gedenken an meine verstorbene Frau, die an Krebs starb. Spendenreferenznummer: BBIB / AVL017 / 28392 Kontaktieren Sie mich für weitere Informationen: tomcrist...@gmail.com Prost Tom Crist
[PATCH v3 4/4] selftests: media_tests: Add a memory-to-memory concurrent stress test
Add a test for the memory-to-memory framework, to exercise the
scheduling of concurrent jobs, using multiple contexts.
This test needs to be run using the vim2m virtual driver,
and so needs no hardware.
While here, rework the media_tests suite in order to make it
useful for automatic tools. Those tests that need human intervention
are now separated from those that can run automatically, needing
only virtual drivers to work.
Signed-off-by: Ezequiel Garcia
---
.../testing/selftests/media_tests/.gitignore | 1 +
tools/testing/selftests/media_tests/Makefile | 5 +-
.../selftests/media_tests/m2m_job_test.c | 287 ++
.../selftests/media_tests/m2m_job_test.sh | 32 ++
4 files changed, 324 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/media_tests/m2m_job_test.c
create mode 100755 tools/testing/selftests/media_tests/m2m_job_test.sh
diff --git a/tools/testing/selftests/media_tests/.gitignore
b/tools/testing/selftests/media_tests/.gitignore
index 8745eba39012..71c6508348ce 100644
--- a/tools/testing/selftests/media_tests/.gitignore
+++ b/tools/testing/selftests/media_tests/.gitignore
@@ -1,3 +1,4 @@
media_device_test
media_device_open
video_device_test
+m2m_job_test
diff --git a/tools/testing/selftests/media_tests/Makefile
b/tools/testing/selftests/media_tests/Makefile
index 60826d7d37d4..d25d4c3eb7d2 100644
--- a/tools/testing/selftests/media_tests/Makefile
+++ b/tools/testing/selftests/media_tests/Makefile
@@ -1,6 +1,9 @@
# SPDX-License-Identifier: GPL-2.0
#
CFLAGS += -I../ -I../../../../usr/include/
-TEST_GEN_PROGS := media_device_test media_device_open video_device_test
+TEST_GEN_PROGS_EXTENDED := media_device_test media_device_open
video_device_test m2m_job_test
+TEST_PROGS := m2m_job_test.sh
include ../lib.mk
+
+LDLIBS += -lpthread
diff --git a/tools/testing/selftests/media_tests/m2m_job_test.c
b/tools/testing/selftests/media_tests/m2m_job_test.c
new file mode 100644
index ..5800269567e6
--- /dev/null
+++ b/tools/testing/selftests/media_tests/m2m_job_test.c
@@ -0,0 +1,287 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Copyright (c) Collabora, Ltd.
+
+/*
+ * This file adds a test for the memory-to-memory
+ * framework.
+ *
+ * This test opens a user specified video device and then
+ * queues concurrent jobs. The jobs are totally dummy,
+ * its purpose is only to verify that each of the queued
+ * jobs is run, and is run only once.
+ *
+ * The vim2m driver is needed in order to run the test.
+ *
+ * Usage:
+ * ./m2m-job-test -d /dev/videoX
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include "../kselftest.h"
+
+#define V4L2_CID_TRANS_TIME_MSEC(V4L2_CID_USER_BASE + 0x1000)
+#define V4L2_CID_TRANS_NUM_BUFS (V4L2_CID_USER_BASE + 0x1001)
+
+#define MAX_TRANS_TIME_MSEC 500
+#define MAX_COUNT 50
+#define MAX_BUFFERS 5
+#define W 10
+#define H 10
+
+#ifndef DEBUG
+#define dprintf(fmt, arg...) \
+ do {\
+ } while (0)
+#else
+#define dprintf(fmt, arg...) printf(fmt, ## arg)
+#endif
+
+static char video_device[256];
+static int thread_count;
+
+struct context {
+ int vfd;
+ unsigned int width;
+ unsigned int height;
+ int buffers;
+};
+
+static int req_src_buf(struct context *ctx, int buffers)
+{
+ struct v4l2_requestbuffers reqbuf;
+ struct v4l2_buffer buf;
+ int i, ret;
+
+ memset(, 0, sizeof(reqbuf));
+ memset(, 0, sizeof(buf));
+
+ reqbuf.count= buffers;
+ reqbuf.type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
+ reqbuf.memory = V4L2_MEMORY_MMAP;
+ ret = ioctl(ctx->vfd, VIDIOC_REQBUFS, );
+ if (ret)
+ return ret;
+
+ for (i = 0; i < buffers; i++) {
+ buf.type= V4L2_BUF_TYPE_VIDEO_OUTPUT;
+ buf.memory = V4L2_MEMORY_MMAP;
+ buf.index = i;
+ ret = ioctl(ctx->vfd, VIDIOC_QUERYBUF, );
+ if (ret)
+ return ret;
+ buf.bytesused = W*H*2;
+ ret = ioctl(ctx->vfd, VIDIOC_QBUF, );
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int req_dst_buf(struct context *ctx, int buffers)
+{
+ struct v4l2_requestbuffers reqbuf;
+ struct v4l2_buffer buf;
+ int i, ret;
+
+ memset(, 0, sizeof(reqbuf));
+ memset(, 0, sizeof(buf));
+
+ reqbuf.count= buffers;
+ reqbuf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+ reqbuf.memory = V4L2_MEMORY_MMAP;
+
+ ret = ioctl(ctx->vfd, VIDIOC_REQBUFS, );
+ if (ret)
+ return ret;
+
+ for (i = 0; i < buffers; i++) {
+ buf.type= V4L2_BUF_TYPE_VIDEO_CAPTURE;
+ buf.memory = V4L2_MEMORY_MMAP;
+ buf.index
[PATCH v3 2/4] v4l2-mem2mem: Simplify exiting the function in __v4l2_m2m_try_schedule
From: Sakari Ailus
The __v4l2_m2m_try_schedule function acquires and releases multiple
spinlocks. Simplify unlocking the job lock by adding labels to unlock
the lock and exit the function.
Signed-off-by: Sakari Ailus
Signed-off-by: Ezequiel Garcia
---
drivers/media/v4l2-core/v4l2-mem2mem.c | 29 --
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c
b/drivers/media/v4l2-core/v4l2-mem2mem.c
index efae845435c9..da82d151dd20 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -279,51 +279,48 @@ static void __v4l2_m2m_try_queue(struct v4l2_m2m_dev
*m2m_dev,
/* If the context is aborted then don't schedule it */
if (m2m_ctx->job_flags & TRANS_ABORT) {
- spin_unlock_irqrestore(_dev->job_spinlock, flags_job);
dprintk("Aborted context\n");
- return;
+ goto job_unlock;
}
if (m2m_ctx->job_flags & TRANS_QUEUED) {
- spin_unlock_irqrestore(_dev->job_spinlock, flags_job);
dprintk("On job queue already\n");
- return;
+ goto job_unlock;
}
spin_lock_irqsave(_ctx->out_q_ctx.rdy_spinlock, flags_out);
if (list_empty(_ctx->out_q_ctx.rdy_queue)
&& !m2m_ctx->out_q_ctx.buffered) {
- spin_unlock_irqrestore(_ctx->out_q_ctx.rdy_spinlock,
- flags_out);
- spin_unlock_irqrestore(_dev->job_spinlock, flags_job);
dprintk("No input buffers available\n");
- return;
+ goto out_unlock;
}
spin_lock_irqsave(_ctx->cap_q_ctx.rdy_spinlock, flags_cap);
if (list_empty(_ctx->cap_q_ctx.rdy_queue)
&& !m2m_ctx->cap_q_ctx.buffered) {
- spin_unlock_irqrestore(_ctx->cap_q_ctx.rdy_spinlock,
- flags_cap);
- spin_unlock_irqrestore(_ctx->out_q_ctx.rdy_spinlock,
- flags_out);
- spin_unlock_irqrestore(_dev->job_spinlock, flags_job);
dprintk("No output buffers available\n");
- return;
+ goto cap_unlock;
}
spin_unlock_irqrestore(_ctx->cap_q_ctx.rdy_spinlock, flags_cap);
spin_unlock_irqrestore(_ctx->out_q_ctx.rdy_spinlock, flags_out);
if (m2m_dev->m2m_ops->job_ready
&& (!m2m_dev->m2m_ops->job_ready(m2m_ctx->priv))) {
- spin_unlock_irqrestore(_dev->job_spinlock, flags_job);
dprintk("Driver not ready\n");
- return;
+ goto job_unlock;
}
list_add_tail(_ctx->queue, _dev->job_queue);
m2m_ctx->job_flags |= TRANS_QUEUED;
spin_unlock_irqrestore(_dev->job_spinlock, flags_job);
+ return;
+
+cap_unlock:
+ spin_unlock_irqrestore(_ctx->cap_q_ctx.rdy_spinlock, flags_cap);
+out_unlock:
+ spin_unlock_irqrestore(_ctx->out_q_ctx.rdy_spinlock, flags_out);
+job_unlock:
+ spin_unlock_irqrestore(_dev->job_spinlock, flags_job);
}
/**
--
2.18.0
[PATCH v3 1/4] v4l2-mem2mem: Avoid v4l2_m2m_prepare_buf from scheduling a job
There is no need for v4l2_m2m_prepare_buf to try to schedule a job,
as it only prepares a buffer, but does not queue or changes the
state of the queue.
Remove the call to v4l2_m2m_try_schedule from v4l2_m2m_prepare_buf.
Signed-off-by: Ezequiel Garcia
---
drivers/media/v4l2-core/v4l2-mem2mem.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c
b/drivers/media/v4l2-core/v4l2-mem2mem.c
index 0a93c5b173c2..efae845435c9 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -481,14 +481,9 @@ int v4l2_m2m_prepare_buf(struct file *file, struct
v4l2_m2m_ctx *m2m_ctx,
struct v4l2_buffer *buf)
{
struct vb2_queue *vq;
- int ret;
vq = v4l2_m2m_get_vq(m2m_ctx, buf->type);
- ret = vb2_prepare_buf(vq, buf);
- if (!ret)
- v4l2_m2m_try_schedule(m2m_ctx);
-
- return ret;
+ return vb2_prepare_buf(vq, buf);
}
EXPORT_SYMBOL_GPL(v4l2_m2m_prepare_buf);
--
2.18.0
[PATCH v3 0/4] Make sure .device_run is always called in non-atomic context
This series goal is to avoid drivers from having ad-hoc code to call .device_run in non-atomic context. Currently, .device_run can be called via v4l2_m2m_job_finish(), potentially running in interrupt context. This series will be useful for the upcoming Request API, where drivers typically require .device_run to be called in non-atomic context for v4l2_ctrl_request_setup() calls. The solution is to add a per-device worker that is scheduled by v4l2_m2m_job_finish, which replaces drivers having a threaded interrupt or similar. This change allows v4l2_m2m_job_finish() to be called in interrupt context, separating .device_run and v4l2_m2m_job_finish() contexts. It's worth mentioning that v4l2_m2m_cancel_job() doesn't need to flush or cancel the new worker, because the job_spinlock synchronizes both and also because the core prevents simultaneous jobs. Either v4l2_m2m_cancel_job() will wait for the worker, or the worker will be unable to run a new job. Testing --- In order to test the change, and make sure no regressions are introduced, a kselftest test is added to stress the mem2mem framework. Note that this series rework the kselftest media_tests target. Those tests that need hardware and human intervention are now marked as _EXTENDED, and a frontend script is added to run those tests that can run without hardware or human intervention. This will allow the media_tests target to be included in automatic regression testing setups. Hopefully, we will be able to introduce more and more automatic regression tests. Currently, our self-test run looks like: $ make TARGETS=media_tests kselftest make[1]: Entering directory '/home/zeta/repos/builds/virtme-x86_64' make[3]: warning: jobserver unavailable: using -j1. Add '+' to parent make rule. make[3]: Nothing to be done for 'all'. make[3]: warning: jobserver unavailable: using -j1. Add '+' to parent make rule. TAP version 13 selftests: media_tests: m2m_job_test.sh --- running media tests --- media_device : no video4linux drivers loaded, vim2m is needed not ok 1..1 selftests: media_tests: m2m_job_test.sh [SKIP] make[1]: Leaving directory '/home/zeta/repos/builds/virtme-x86_64' Ezequiel Garcia (3): v4l2-mem2mem: Avoid v4l2_m2m_prepare_buf from scheduling a job v4l2-mem2mem: Avoid calling .device_run in v4l2_m2m_job_finish selftests: media_tests: Add a memory-to-memory concurrent stress test Sakari Ailus (1): v4l2-mem2mem: Simplify exiting the function in __v4l2_m2m_try_schedule drivers/media/v4l2-core/v4l2-mem2mem.c| 72 +++-- .../testing/selftests/media_tests/.gitignore | 1 + tools/testing/selftests/media_tests/Makefile | 5 +- .../selftests/media_tests/m2m_job_test.c | 287 ++ .../selftests/media_tests/m2m_job_test.sh | 32 ++ 5 files changed, 371 insertions(+), 26 deletions(-) create mode 100644 tools/testing/selftests/media_tests/m2m_job_test.c create mode 100755 tools/testing/selftests/media_tests/m2m_job_test.sh -- 2.18.0
[PATCH v3 3/4] v4l2-mem2mem: Avoid calling .device_run in v4l2_m2m_job_finish
v4l2_m2m_job_finish() is typically called in interrupt context.
Some implementation of .device_run might sleep, and so it's
desirable to avoid calling it directly from
v4l2_m2m_job_finish(), thus avoiding .device_run from running
in interrupt context.
Implement a deferred context that calls v4l2_m2m_try_run,
and gets scheduled by v4l2_m2m_job_finish().
Signed-off-by: Ezequiel Garcia
---
drivers/media/v4l2-core/v4l2-mem2mem.c | 36 +++---
1 file changed, 33 insertions(+), 3 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c
b/drivers/media/v4l2-core/v4l2-mem2mem.c
index da82d151dd20..0bf4deefa899 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -69,6 +69,7 @@ static const char * const m2m_entity_name[] = {
* @curr_ctx: currently running instance
* @job_queue: instances queued to run
* @job_spinlock: protects job_queue
+ * @job_work: worker to run queued jobs.
* @m2m_ops: driver callbacks
*/
struct v4l2_m2m_dev {
@@ -85,6 +86,7 @@ struct v4l2_m2m_dev {
struct list_headjob_queue;
spinlock_t job_spinlock;
+ struct work_struct job_work;
const struct v4l2_m2m_ops *m2m_ops;
};
@@ -224,10 +226,11 @@ EXPORT_SYMBOL(v4l2_m2m_get_curr_priv);
/**
* v4l2_m2m_try_run() - select next job to perform and run it if possible
* @m2m_dev: per-device context
+ * @try_lock: indicates if the queue lock should be taken
*
* Get next transaction (if present) from the waiting jobs list and run it.
*/
-static void v4l2_m2m_try_run(struct v4l2_m2m_dev *m2m_dev)
+static void v4l2_m2m_try_run(struct v4l2_m2m_dev *m2m_dev, bool try_lock)
{
unsigned long flags;
@@ -250,7 +253,20 @@ static void v4l2_m2m_try_run(struct v4l2_m2m_dev *m2m_dev)
spin_unlock_irqrestore(_dev->job_spinlock, flags);
dprintk("Running job on m2m_ctx: %p\n", m2m_dev->curr_ctx);
+
+ /*
+* A m2m context lock is taken only after a m2m context
+* is picked from the queue and marked as running.
+* The lock is only needed if v4l2_m2m_try_run is called
+* from the async worker.
+*/
+ if (try_lock && m2m_dev->curr_ctx->q_lock)
+ mutex_lock(m2m_dev->curr_ctx->q_lock);
+
m2m_dev->m2m_ops->device_run(m2m_dev->curr_ctx->priv);
+
+ if (try_lock && m2m_dev->curr_ctx->q_lock)
+ mutex_unlock(m2m_dev->curr_ctx->q_lock);
}
/*
@@ -340,10 +356,22 @@ void v4l2_m2m_try_schedule(struct v4l2_m2m_ctx *m2m_ctx)
struct v4l2_m2m_dev *m2m_dev = m2m_ctx->m2m_dev;
__v4l2_m2m_try_queue(m2m_dev, m2m_ctx);
- v4l2_m2m_try_run(m2m_dev);
+ v4l2_m2m_try_run(m2m_dev, false);
}
EXPORT_SYMBOL_GPL(v4l2_m2m_try_schedule);
+/**
+ * v4l2_m2m_device_run_work() - run pending jobs for the context
+ * @work: Work structure used for scheduling the execution of this function.
+ */
+static void v4l2_m2m_device_run_work(struct work_struct *work)
+{
+ struct v4l2_m2m_dev *m2m_dev =
+ container_of(work, struct v4l2_m2m_dev, job_work);
+
+ v4l2_m2m_try_run(m2m_dev, true);
+}
+
/**
* v4l2_m2m_cancel_job() - cancel pending jobs for the context
* @m2m_ctx: m2m context with jobs to be canceled
@@ -403,7 +431,8 @@ void v4l2_m2m_job_finish(struct v4l2_m2m_dev *m2m_dev,
/* This instance might have more buffers ready, but since we do not
* allow more than one job on the job_queue per instance, each has
* to be scheduled separately after the previous one finishes. */
- v4l2_m2m_try_schedule(m2m_ctx);
+ __v4l2_m2m_try_queue(m2m_dev, m2m_ctx);
+ schedule_work(_dev->job_work);
}
EXPORT_SYMBOL(v4l2_m2m_job_finish);
@@ -837,6 +866,7 @@ struct v4l2_m2m_dev *v4l2_m2m_init(const struct
v4l2_m2m_ops *m2m_ops)
m2m_dev->m2m_ops = m2m_ops;
INIT_LIST_HEAD(_dev->job_queue);
spin_lock_init(_dev->job_spinlock);
+ INIT_WORK(_dev->job_work, v4l2_m2m_device_run_work);
return m2m_dev;
}
--
2.18.0
[PATCH 1/2] media: v4l2-common: v4l2_spi_subdev_init : generate unique name
While v4l2_i2c_subdev_init does give a unique name to the subdev, matching the one appearing in dmesg for messages generated by dev_info and friends (e.g. imx185 30-0010), v4l2_spi_subdev_init does a poor job, copying only the driver name, but not the dev_name(), yielding e.g. "imx185", but missing the "spi1.1" part, and not generating a unique name. Fix that. Signed-off-by: Philippe De Muyter --- drivers/media/v4l2-core/v4l2-common.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c index b518b92..5471c6d 100644 --- a/drivers/media/v4l2-core/v4l2-common.c +++ b/drivers/media/v4l2-core/v4l2-common.c @@ -255,7 +255,9 @@ void v4l2_spi_subdev_init(struct v4l2_subdev *sd, struct spi_device *spi, v4l2_set_subdevdata(sd, spi); spi_set_drvdata(spi, sd); /* initialize name */ - strlcpy(sd->name, spi->dev.driver->name, sizeof(sd->name)); + snprintf(sd->name, sizeof(sd->name), "%s %s", + spi->dev.driver->name, dev_name(>dev)); + } EXPORT_SYMBOL_GPL(v4l2_spi_subdev_init); -- 1.8.4
[PATCH 2/2] media: v4l2-common: simplify v4l2_i2c_subdev_init name generation
When v4l2_i2c_subdev_init is called, dev_name(>dev) has already been set. Use it to generate subdev's name instead of recreating it with "%d-%04x". This improves the similarity in subdev's name creation between v4l2_i2c_subdev_init and v4l2_spi_subdev_init. Signed-off-by: Philippe De Muyter --- drivers/media/v4l2-core/v4l2-common.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c index 5471c6d..b062111 100644 --- a/drivers/media/v4l2-core/v4l2-common.c +++ b/drivers/media/v4l2-core/v4l2-common.c @@ -121,9 +121,8 @@ void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client *client, v4l2_set_subdevdata(sd, client); i2c_set_clientdata(client, sd); /* initialize name */ - snprintf(sd->name, sizeof(sd->name), "%s %d-%04x", - client->dev.driver->name, i2c_adapter_id(client->adapter), - client->addr); + snprintf(sd->name, sizeof(sd->name), "%s %s", + client->dev.driver->name, dev_name(>dev)); } EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_init); -- 1.8.4
[PATCH 3/3] media: add Rockchip VPU driver
Add a mem2mem driver for the VPU available on Rockchip SoCs.
Currently only JPEG encoding is supported, for RK3399 and RK3288
platforms.
Signed-off-by: Ezequiel Garcia
---
drivers/media/platform/Kconfig| 12 +
drivers/media/platform/Makefile | 1 +
drivers/media/platform/rockchip/vpu/Makefile | 8 +
.../platform/rockchip/vpu/rk3288_vpu_hw.c | 127 +++
.../rockchip/vpu/rk3288_vpu_hw_jpege.c| 156
.../platform/rockchip/vpu/rk3288_vpu_regs.h | 442 ++
.../platform/rockchip/vpu/rk3399_vpu_hw.c | 127 +++
.../rockchip/vpu/rk3399_vpu_hw_jpege.c| 165
.../platform/rockchip/vpu/rk3399_vpu_regs.h | 601 ++
.../platform/rockchip/vpu/rockchip_vpu.h | 270 +++
.../platform/rockchip/vpu/rockchip_vpu_drv.c | 416 ++
.../platform/rockchip/vpu/rockchip_vpu_enc.c | 763 ++
.../platform/rockchip/vpu/rockchip_vpu_enc.h | 25 +
.../platform/rockchip/vpu/rockchip_vpu_hw.h | 67 ++
14 files changed, 3180 insertions(+)
create mode 100644 drivers/media/platform/rockchip/vpu/Makefile
create mode 100644 drivers/media/platform/rockchip/vpu/rk3288_vpu_hw.c
create mode 100644 drivers/media/platform/rockchip/vpu/rk3288_vpu_hw_jpege.c
create mode 100644 drivers/media/platform/rockchip/vpu/rk3288_vpu_regs.h
create mode 100644 drivers/media/platform/rockchip/vpu/rk3399_vpu_hw.c
create mode 100644 drivers/media/platform/rockchip/vpu/rk3399_vpu_hw_jpege.c
create mode 100644 drivers/media/platform/rockchip/vpu/rk3399_vpu_regs.h
create mode 100644 drivers/media/platform/rockchip/vpu/rockchip_vpu.h
create mode 100644 drivers/media/platform/rockchip/vpu/rockchip_vpu_drv.c
create mode 100644 drivers/media/platform/rockchip/vpu/rockchip_vpu_enc.c
create mode 100644 drivers/media/platform/rockchip/vpu/rockchip_vpu_enc.h
create mode 100644 drivers/media/platform/rockchip/vpu/rockchip_vpu_hw.h
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 2728376b04b5..7244a2360732 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -448,6 +448,18 @@ config VIDEO_ROCKCHIP_RGA
To compile this driver as a module choose m here.
+config VIDEO_ROCKCHIP_VPU
+ tristate "Rockchip VPU driver"
+ depends on VIDEO_DEV && VIDEO_V4L2
+ depends on ARCH_ROCKCHIP || COMPILE_TEST
+ select VIDEOBUF2_DMA_CONTIG
+ select V4L2_MEM2MEM_DEV
+ default n
+ ---help---
+ Support for the VPU video codec found on Rockchip SoC.
+ To compile this driver as a module, choose M here: the module
+ will be called rockchip-vpu.
+
config VIDEO_TI_VPE
tristate "TI VPE (Video Processing Engine) driver"
depends on VIDEO_DEV && VIDEO_V4L2
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 04bc1502a30e..83378180d8ac 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -66,6 +66,7 @@ obj-$(CONFIG_VIDEO_RENESAS_JPU) += rcar_jpu.o
obj-$(CONFIG_VIDEO_RENESAS_VSP1) += vsp1/
obj-$(CONFIG_VIDEO_ROCKCHIP_RGA) += rockchip/rga/
+obj-$(CONFIG_VIDEO_ROCKCHIP_VPU)+= rockchip/vpu/
obj-y += omap/
diff --git a/drivers/media/platform/rockchip/vpu/Makefile
b/drivers/media/platform/rockchip/vpu/Makefile
new file mode 100644
index ..cab0123c49d4
--- /dev/null
+++ b/drivers/media/platform/rockchip/vpu/Makefile
@@ -0,0 +1,8 @@
+obj-$(CONFIG_VIDEO_ROCKCHIP_VPU) += rockchip-vpu.o
+
+rockchip-vpu-y += rockchip_vpu_drv.o \
+ rockchip_vpu_enc.o \
+ rk3288_vpu_hw.o \
+ rk3288_vpu_hw_jpege.o \
+ rk3399_vpu_hw.o \
+ rk3399_vpu_hw_jpege.o
diff --git a/drivers/media/platform/rockchip/vpu/rk3288_vpu_hw.c
b/drivers/media/platform/rockchip/vpu/rk3288_vpu_hw.c
new file mode 100644
index ..0caff8ecf343
--- /dev/null
+++ b/drivers/media/platform/rockchip/vpu/rk3288_vpu_hw.c
@@ -0,0 +1,127 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Rockchip VPU codec driver
+ *
+ * Copyright (C) 2018 Rockchip Electronics Co., Ltd.
+ * Jeffy Chen
+ */
+
+#include
+
+#include "rockchip_vpu.h"
+#include "rk3288_vpu_regs.h"
+
+#define RK3288_ACLK_MAX_FREQ (400 * 1000 * 1000)
+
+/*
+ * Supported formats.
+ */
+
+static const struct rockchip_vpu_fmt rk3288_vpu_enc_fmts[] = {
+ /* Source formats. */
+ {
+ .name = "4:2:0 3 planes Y/Cb/Cr",
+ .fourcc = V4L2_PIX_FMT_YUV420M,
+ .codec_mode = RK_VPU_CODEC_NONE,
+ .num_planes = 3,
+ .depth = { 8, 4, 4 },
+ .enc_fmt = RK3288_VPU_ENC_FMT_YUV420P,
+ },
+ {
+ .name = "4:2:0 2 plane Y/CbCr",
+ .fourcc = V4L2_PIX_FMT_NV12M,
+ .codec_mode = RK_VPU_CODEC_NONE,
+ .num_planes = 2,
+ .depth = { 8, 8 },
+ .enc_fmt =
[PATCH 2/3] media: Add controls for jpeg quantization tables
From: Shunqian Zheng
Add V4L2_CID_JPEG_LUMA/CHROMA_QUANTIZATION controls to allow userspace
configure the JPEG quantization tables.
Signed-off-by: Shunqian Zheng
---
drivers/media/v4l2-core/v4l2-ctrls.c | 4
include/uapi/linux/v4l2-controls.h | 3 +++
2 files changed, 7 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
b/drivers/media/v4l2-core/v4l2-ctrls.c
index d29e45516eb7..b06adba2c486 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -980,6 +980,8 @@ const char *v4l2_ctrl_get_name(u32 id)
case V4L2_CID_JPEG_RESTART_INTERVAL:return "Restart Interval";
case V4L2_CID_JPEG_COMPRESSION_QUALITY: return "Compression Quality";
case V4L2_CID_JPEG_ACTIVE_MARKER: return "Active Markers";
+ case V4L2_CID_JPEG_LUMA_QUANTIZATION: return "Luminance Quantization
Matrix";
+ case V4L2_CID_JPEG_CHROMA_QUANTIZATION: return "Chrominance
Quantization Matrix";
/* Image source controls */
/* Keep the order of the 'case's the same as in v4l2-controls.h! */
@@ -1263,6 +1265,8 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum
v4l2_ctrl_type *type,
*flags |= V4L2_CTRL_FLAG_READ_ONLY;
break;
case V4L2_CID_DETECT_MD_REGION_GRID:
+ case V4L2_CID_JPEG_LUMA_QUANTIZATION:
+ case V4L2_CID_JPEG_CHROMA_QUANTIZATION:
*type = V4L2_CTRL_TYPE_U8;
break;
case V4L2_CID_DETECT_MD_THRESHOLD_GRID:
diff --git a/include/uapi/linux/v4l2-controls.h
b/include/uapi/linux/v4l2-controls.h
index 8d473c979b61..b731fd7bc899 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -971,6 +971,9 @@ enum v4l2_jpeg_chroma_subsampling {
#defineV4L2_JPEG_ACTIVE_MARKER_DQT (1 << 17)
#defineV4L2_JPEG_ACTIVE_MARKER_DHT (1 << 18)
+#define V4L2_CID_JPEG_LUMA_QUANTIZATION
(V4L2_CID_JPEG_CLASS_BASE + 5)
+#define V4L2_CID_JPEG_CHROMA_QUANTIZATION (V4L2_CID_JPEG_CLASS_BASE + 6)
+
/* Image source controls */
#define V4L2_CID_IMAGE_SOURCE_CLASS_BASE (V4L2_CTRL_CLASS_IMAGE_SOURCE |
0x900)
--
2.18.0.rc2
[PATCH v2 0/3] Add Rockchip VPU JPEG encoder
This series adds support for JPEG encoding via the VPU block
present in Rockchip platforms. Currently, support for RK3288
and RK3399 is included.
The hardware produces a Raw JPEG format (i.e. works as a
JPEG accelerator). It requires quantization tables provided
by the application, and uses standard huffman tables,
as recommended by the JPEG specification.
In order to support this, the series introduces a new pixel format,
and a new pair of controls, V4L2_CID_JPEG_{LUMA,CHROMA}_QUANTIZATION
allowing userspace to specify the quantization tables.
Userspace is then responsible to add the required headers
and tables to the produced raw payload, to produce a JPEG image.
Compliance
==
# v4l2-compliance -d 0 -s
Ezequiel Garcia (1):
media: add Rockchip VPU driver
Shunqian Zheng (2):
media: Add JPEG_RAW format
media: Add controls for jpeg quantization tables
.../media/uapi/v4l/pixfmt-compressed.rst | 5 +
drivers/media/platform/Kconfig| 12 +
drivers/media/platform/Makefile | 1 +
drivers/media/platform/rockchip/vpu/Makefile | 8 +
.../platform/rockchip/vpu/rk3288_vpu_hw.c | 127 +++
.../rockchip/vpu/rk3288_vpu_hw_jpege.c| 156
.../platform/rockchip/vpu/rk3288_vpu_regs.h | 442 ++
.../platform/rockchip/vpu/rk3399_vpu_hw.c | 127 +++
.../rockchip/vpu/rk3399_vpu_hw_jpege.c| 165
.../platform/rockchip/vpu/rk3399_vpu_regs.h | 601 ++
.../platform/rockchip/vpu/rockchip_vpu.h | 270 +++
.../platform/rockchip/vpu/rockchip_vpu_drv.c | 416 ++
.../platform/rockchip/vpu/rockchip_vpu_enc.c | 763 ++
.../platform/rockchip/vpu/rockchip_vpu_enc.h | 25 +
.../platform/rockchip/vpu/rockchip_vpu_hw.h | 67 ++
drivers/media/v4l2-core/v4l2-ctrls.c | 4 +
drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
include/uapi/linux/v4l2-controls.h| 3 +
include/uapi/linux/videodev2.h| 1 +
19 files changed, 3194 insertions(+)
create mode 100644 drivers/media/platform/rockchip/vpu/Makefile
create mode 100644 drivers/media/platform/rockchip/vpu/rk3288_vpu_hw.c
create mode 100644 drivers/media/platform/rockchip/vpu/rk3288_vpu_hw_jpege.c
create mode 100644 drivers/media/platform/rockchip/vpu/rk3288_vpu_regs.h
create mode 100644 drivers/media/platform/rockchip/vpu/rk3399_vpu_hw.c
create mode 100644 drivers/media/platform/rockchip/vpu/rk3399_vpu_hw_jpege.c
create mode 100644 drivers/media/platform/rockchip/vpu/rk3399_vpu_regs.h
create mode 100644 drivers/media/platform/rockchip/vpu/rockchip_vpu.h
create mode 100644 drivers/media/platform/rockchip/vpu/rockchip_vpu_drv.c
create mode 100644 drivers/media/platform/rockchip/vpu/rockchip_vpu_enc.c
create mode 100644 drivers/media/platform/rockchip/vpu/rockchip_vpu_enc.h
create mode 100644 drivers/media/platform/rockchip/vpu/rockchip_vpu_hw.h
--
2.18.0.rc2
[PATCH 1/3] media: Add JPEG_RAW format
From: Shunqian Zheng
Add V4L2_PIX_FMT_JPEG_RAW format that does not contain
JPEG header in the output frame.
Signed-off-by: Shunqian Zheng
---
Documentation/media/uapi/v4l/pixfmt-compressed.rst | 5 +
drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
include/uapi/linux/videodev2.h | 1 +
3 files changed, 7 insertions(+)
diff --git a/Documentation/media/uapi/v4l/pixfmt-compressed.rst
b/Documentation/media/uapi/v4l/pixfmt-compressed.rst
index abec03937bb3..ebfc3cb7399c 100644
--- a/Documentation/media/uapi/v4l/pixfmt-compressed.rst
+++ b/Documentation/media/uapi/v4l/pixfmt-compressed.rst
@@ -23,6 +23,11 @@ Compressed Formats
- 'JPEG'
- TBD. See also :ref:`VIDIOC_G_JPEGCOMP `,
:ref:`VIDIOC_S_JPEGCOMP `.
+* .. _V4L2-PIX-FMT-JPEG-RAW:
+
+ - ``V4L2_PIX_FMT_JPEG_RAW``
+ - 'Raw JPEG'
+ - JPEG without any headers.
* .. _V4L2-PIX-FMT-MPEG:
- ``V4L2_PIX_FMT_MPEG``
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
b/drivers/media/v4l2-core/v4l2-ioctl.c
index dd210067151f..9f0c76ec7c2c 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1259,6 +1259,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
/* Max description length mask: descr =
"0123456789012345678901234567890" */
case V4L2_PIX_FMT_MJPEG:descr = "Motion-JPEG"; break;
case V4L2_PIX_FMT_JPEG: descr = "JFIF JPEG"; break;
+ case V4L2_PIX_FMT_JPEG_RAW: descr = "Raw JPEG"; break;
case V4L2_PIX_FMT_DV: descr = "1394"; break;
case V4L2_PIX_FMT_MPEG: descr = "MPEG-1/2/4"; break;
case V4L2_PIX_FMT_H264: descr = "H.264"; break;
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 600877be5c22..934e91af1b40 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -621,6 +621,7 @@ struct v4l2_pix_format {
/* compressed formats */
#define V4L2_PIX_FMT_MJPEGv4l2_fourcc('M', 'J', 'P', 'G') /* Motion-JPEG
*/
#define V4L2_PIX_FMT_JPEG v4l2_fourcc('J', 'P', 'E', 'G') /* JFIF JPEG
*/
+#define V4L2_PIX_FMT_JPEG_RAW v4l2_fourcc('J', 'P', 'G', 'R') /* JFIF JPEG RAW
without headers */
#define V4L2_PIX_FMT_DV v4l2_fourcc('d', 'v', 's', 'd') /* 1394
*/
#define V4L2_PIX_FMT_MPEG v4l2_fourcc('M', 'P', 'E', 'G') /* MPEG-1/2/4
Multiplexed */
#define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /* H264 with
start codes */
--
2.18.0.rc2
Re: [RFC 4/4] dt-bindings: media: add Amlogic Meson Video Decoder Bindings
Hi Maxime,
many thanks for your work!
On Wed, Aug 1, 2018 at 9:34 PM Maxime Jourdan wrote:
>
> Add documentation for the meson vdec dts node.
>
> Signed-off-by: Maxime Jourdan
> ---
> .../bindings/media/amlogic,meson-vdec.txt | 60 +++
> 1 file changed, 60 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/media/amlogic,meson-vdec.txt
>
> diff --git a/Documentation/devicetree/bindings/media/amlogic,meson-vdec.txt
> b/Documentation/devicetree/bindings/media/amlogic,meson-vdec.txt
> new file mode 100644
> index ..120b135e6bb5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/amlogic,meson-vdec.txt
> @@ -0,0 +1,60 @@
> +Amlogic Meson Video Decoder
> +
> +
> +The VDEC IP is composed of the following blocks :
> +
> +- ESPARSER is a bitstream parser that outputs to a VIFIFO. Further VDEC
> blocks
> +then feed from this VIFIFO.
> +- VDEC_1 can decode MPEG-1, MPEG-2, MPEG-4 part 2, H.263, H.264.
> +- VDEC_2 is used as a helper for corner cases like H.264 4K on older SoCs.
> +It is not handled by this driver.
is it currently not handled or will it never be?
> +- VDEC_HCODEC is the H.264 encoding block. It is not handled by this driver.
> +- VDEC_HEVC can decode HEVC and VP9.
> +
> +Device Tree Bindings:
> +-
> +
> +VDEC: Video Decoder
> +--
> +
> +Required properties:
> +- compatible: value should be different for each SoC family as :
> + - GXBB (S905) : "amlogic,meson-gxbb-vdec"
> + - GXL (S905X, S905D) : "amlogic,meson-gxl-vdec"
> + - GXM (S912) : "amlogic,meson-gxm-vdec"
> +- reg: base address and size of he following memory-mapped regions :
> + - dos
> + - esparser
> + - dmc
> +- reg-names: should contain the names of the previous memory regions
any reason why you are not using the DMC syscon (as added in your
patch "dt-bindings: soc: amlogic: add meson-canvas documentation")
instead of mapping the DMC region again?
> +- interrupts: should contain the vdec and esparser IRQs.
are these two IRQs the "currently supported" ones or are there more
for the whole IP block (but just not implemented yet)?
> +- clocks: should contain the following clocks :
> + - dos_parser
> + - dos
> + - vdec_1
> + - vdec_hevc
> +- clock-names: should contain the names of the previous clocks
> +- resets: should contain the parser reset.
> +- reset-names: should be "esparser".
> +
> +Example:
> +
> +vdec: video-decoder@0xd005 {
> + compatible = "amlogic,meson-gxbb-vdec";
> + reg = <0x0 0xc882 0x0 0x1
> + 0x0 0xc110a580 0x0 0xe4
> + 0x0 0xc8838000 0x0 0x60>;
AFAIK the "correct" format is (just like you've done for the clocks below):
reg = <0x0 0xc882 0x0 0x1>,
<0x0 0xc110a580 0x0 0xe4>,
<0x0 0xc8838000 0x0 0x60>;
> + reg-names = "dos", "esparser", "dmc";
> +
> + interrupts = + GIC_SPI 32 IRQ_TYPE_EDGE_RISING>;
AFAIK the "correct" format is (just like you've done for the clocks below):
interrupts = ,
;
> + interrupt-names = "vdec", "esparser";
> +
> + amlogic,ao-sysctrl = <_AO>;
this is not documented above - is it needed?
Regards
Martin
[0] http://lists.infradead.org/pipermail/linux-amlogic/2018-August/008034.html
[RFC 0/4] media: meson: add video decoder driver
This is a Request for Comments for the amlogic (meson) video decoder driver. It is written around the V4L2 M2M framework without using the Request API as there are a hardware bitstream parser and firmwares. It features decoding for: - MPEG 1/2/4, H.263, H.264, MJPEG, HEVC 8-bit (partial) Even though they are supported in hardware, it doesn't leverage support for: - HEVC 10-bit, VP9, VC1 (all those are in TODOs) The output is multiplanar NV12 (V4L2_PIX_FMT_NV12M). Supported SoCs are: GXBB (S905), GXL (S905X/W/D), GXM (S912) It was tested primarily with FFmpeg, GStreamer and kodi. The file hierarchy can be boiled down to: | codec_h264.c | codec_mjpeg.c | codec_mpeg4.c | vdec_1.c -->| codec_mpeg12.c vdec.c -->| vdec_hevc.c -->| codec_hevc.c | esparser.c The V4L2 code is handled mostly in vdec.c. Each VDEC and CODEC unit is accessed via ops structs to facilitate the code. The arrangement between vdecs and codecs can be seen in vdec_platform.c This file also declares things like pixfmts, min/max buffers and firmware paths for each SoC. Specific questions about the code: - While I do use the platform's general clks and resets tied to the vdec in a nice way (dts + clock/reset controller with clk/reset frameworks), there are some subclocks and resets that I use in the driver by writing directly to registers. e.g: - writel_relaxed((1<<7) | (1<<6), core->dos_base + DOS_SW_RESET0); - writel_relaxed(0x3ff, core->dos_base + DOS_GCLK_EN0); and a few other instances where that happens. Is it okay to not create specific controllers for those ? The main issue is the lack of documentation so I don't know which resets/clocks are impacted by those writes. The only thing I'm certain of is that they only apply to the vdec/esparser. - I tend to call vdec_* functions from the codec handlers. For instance, codec_h264 will call vdec_dst_buf_done_idx to DONE a capture buffer. vdec_dst_buf_done_idx is as such a public symbol. Should I use an ops struct for those instead, so that the codec handlers don't depend directly on the vdec general code ? - Naming: my public symbols either start with vdec_* or esparser_* Should I change that to something meson/amlogic specific ? - I have a _lot_ of writel_relaxed calls. Can I leave them be or is there a nicer way to do it ? - Since the decoder is single instance, I only allow one _open at a time. However the v4l2 compliance suite complains about this. How should I safely make it single instance ? Not allowing multiple start_streaming ? - I am getting these 2 fails, but unsure what they are about: Buffer ioctls: fail: ../../../v4l-utils-1.12.3/utils/v4l2-compliance/v4l2-test-buffers.cpp(428): node->node2 == NULL test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL fail: ../../../v4l-utils-1.12.3/utils/v4l2-compliance/v4l2-test-buffers.cpp(571): q.has_expbuf(node) test VIDIOC_EXPBUF: FAIL And of course, I will gladly accept any kind of other feedback you would have. Thanks! Maxime Jourdan (4): media: meson: add v4l2 m2m video decoder driver ARM64: dts: meson-gx: add vdec entry ARM64: dts: meson: add vdec entries dt-bindings: media: add Amlogic Meson Video Decoder Bindings .../bindings/media/amlogic,meson-vdec.txt | 60 + arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 14 + arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi |8 + arch/arm64/boot/dts/amlogic/meson-gxl.dtsi|8 + arch/arm64/boot/dts/amlogic/meson-gxm.dtsi|4 + drivers/media/platform/Kconfig| 10 + drivers/media/platform/meson/Makefile |1 + drivers/media/platform/meson/vdec/Makefile|7 + drivers/media/platform/meson/vdec/canvas.c| 69 + drivers/media/platform/meson/vdec/canvas.h| 42 + .../media/platform/meson/vdec/codec_h264.c| 376 + .../media/platform/meson/vdec/codec_h264.h| 13 + .../media/platform/meson/vdec/codec_helpers.c | 45 + .../media/platform/meson/vdec/codec_helpers.h |8 + .../media/platform/meson/vdec/codec_hevc.c| 1383 + .../media/platform/meson/vdec/codec_hevc.h| 13 + .../media/platform/meson/vdec/codec_mjpeg.c | 203 +++ .../media/platform/meson/vdec/codec_mjpeg.h | 13 + .../media/platform/meson/vdec/codec_mpeg12.c | 183 +++ .../media/platform/meson/vdec/codec_mpeg12.h | 13 + .../media/platform/meson/vdec/codec_mpeg4.c | 213 +++ .../media/platform/meson/vdec/codec_mpeg4.h | 13 + drivers/media/platform/meson/vdec/esparser.c | 320 drivers/media/platform/meson/vdec/esparser.h | 16 + drivers/media/platform/meson/vdec/hevc_regs.h | 742 + drivers/media/platform/meson/vdec/vdec.c | 1009 drivers/media/platform/meson/vdec/vdec.h | 152 ++ drivers/media/platform/meson/vdec/vdec_1.c| 266 drivers/media/platform/meson/vdec/vdec_1.h| 13 +
[RFC 2/4] ARM64: dts: meson-gx: add vdec entry
Add the video decoder dts entry
Signed-off-by: Maxime Jourdan
---
arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 14 ++
1 file changed, 14 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index b8dc4dbb391b..248052737aa7 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -483,6 +483,20 @@
};
};
+ vdec: video-decoder@0xd005 {
+ compatible = "amlogic,meson-gx-vdec";
+ reg = <0x0 0xc882 0x0 0x1
+ 0x0 0xc110a580 0x0 0xe4
+ 0x0 0xc8838000 0x0 0x60>;
+ reg-names = "dos", "esparser", "dmc";
+
+ interrupts = ;
+ interrupt-names = "vdec", "esparser";
+
+ amlogic,ao-sysctrl = <_AO>;
+ };
+
vpu: vpu@d010 {
compatible = "amlogic,meson-gx-vpu";
reg = <0x0 0xd010 0x0 0x10>,
--
2.17.1
[RFC 3/4] ARM64: dts: meson: add vdec entries
This enables the video decoder for gxbb, gxl and gxm chips
Signed-off-by: Maxime Jourdan
---
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 8
arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 8
arch/arm64/boot/dts/amlogic/meson-gxm.dtsi | 4
3 files changed, 20 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index 98cbba6809ca..0ea4b5684a11 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -774,3 +774,11 @@
compatible = "amlogic,meson-gxbb-vpu", "amlogic,meson-gx-vpu";
power-domains = <_vpu>;
};
+
+ {
+ compatible = "amlogic,meson-gxbb-vdec";
+ clocks = < CLKID_DOS_PARSER>, < CLKID_DOS>, <
CLKID_VDEC_1>, < CLKID_VDEC_HEVC>;
+ clock-names = "dos_parser", "dos", "vdec_1", "vdec_hevc";
+ resets = < RESET_PARSER>;
+ reset-names = "esparser";
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
index c87a80e9bcc6..20045a9c26fa 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
@@ -775,3 +775,11 @@
compatible = "amlogic,meson-gxl-vpu", "amlogic,meson-gx-vpu";
power-domains = <_vpu>;
};
+
+ {
+ compatible = "amlogic,meson-gxl-vdec";
+ clocks = < CLKID_DOS_PARSER>, < CLKID_DOS>, <
CLKID_VDEC_1>, < CLKID_VDEC_HEVC>;
+ clock-names = "dos_parser", "dos", "vdec_1", "vdec_hevc";
+ resets = < RESET_PARSER>;
+ reset-names = "esparser";
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
b/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
index 247888d68a3a..4ce7b12014bb 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
@@ -117,3 +117,7 @@
{
phys = <_phy>, <_phy0>, <_phy1>, <_phy2>;
};
+
+ {
+ compatible = "amlogic,meson-gxm-vdec";
+};
--
2.17.1
[RFC 4/4] dt-bindings: media: add Amlogic Meson Video Decoder Bindings
Add documentation for the meson vdec dts node.
Signed-off-by: Maxime Jourdan
---
.../bindings/media/amlogic,meson-vdec.txt | 60 +++
1 file changed, 60 insertions(+)
create mode 100644
Documentation/devicetree/bindings/media/amlogic,meson-vdec.txt
diff --git a/Documentation/devicetree/bindings/media/amlogic,meson-vdec.txt
b/Documentation/devicetree/bindings/media/amlogic,meson-vdec.txt
new file mode 100644
index ..120b135e6bb5
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/amlogic,meson-vdec.txt
@@ -0,0 +1,60 @@
+Amlogic Meson Video Decoder
+
+
+The VDEC IP is composed of the following blocks :
+
+- ESPARSER is a bitstream parser that outputs to a VIFIFO. Further VDEC blocks
+then feed from this VIFIFO.
+- VDEC_1 can decode MPEG-1, MPEG-2, MPEG-4 part 2, H.263, H.264.
+- VDEC_2 is used as a helper for corner cases like H.264 4K on older SoCs.
+It is not handled by this driver.
+- VDEC_HCODEC is the H.264 encoding block. It is not handled by this driver.
+- VDEC_HEVC can decode HEVC and VP9.
+
+Device Tree Bindings:
+-
+
+VDEC: Video Decoder
+--
+
+Required properties:
+- compatible: value should be different for each SoC family as :
+ - GXBB (S905) : "amlogic,meson-gxbb-vdec"
+ - GXL (S905X, S905D) : "amlogic,meson-gxl-vdec"
+ - GXM (S912) : "amlogic,meson-gxm-vdec"
+- reg: base address and size of he following memory-mapped regions :
+ - dos
+ - esparser
+ - dmc
+- reg-names: should contain the names of the previous memory regions
+- interrupts: should contain the vdec and esparser IRQs.
+- clocks: should contain the following clocks :
+ - dos_parser
+ - dos
+ - vdec_1
+ - vdec_hevc
+- clock-names: should contain the names of the previous clocks
+- resets: should contain the parser reset.
+- reset-names: should be "esparser".
+
+Example:
+
+vdec: video-decoder@0xd005 {
+ compatible = "amlogic,meson-gxbb-vdec";
+ reg = <0x0 0xc882 0x0 0x1
+ 0x0 0xc110a580 0x0 0xe4
+ 0x0 0xc8838000 0x0 0x60>;
+ reg-names = "dos", "esparser", "dmc";
+
+ interrupts = ;
+ interrupt-names = "vdec", "esparser";
+
+ amlogic,ao-sysctrl = <_AO>;
+
+ clocks = < CLKID_DOS_PARSER>, < CLKID_DOS>, <
CLKID_VDEC_1>, < CLKID_VDEC_HEVC>;
+ clock-names = "dos_parser", "dos", "vdec_1", "vdec_hevc";
+
+ resets = < RESET_PARSER>;
+ reset-names = "esparser";
+};
--
2.17.1
[PATCH v3 00/14] imx-media: Fixes for interlaced capture
A set of patches that fixes some bugs with capturing from an interlaced source, and incompatibilites between IDMAC interlace interweaving and 4:2:0 data write reduction. History: v3: - add support for/fix interweaved scan with YUV planar output. - fix bug in 4:2:0 U/V offset macros. - add patch that generalizes behavior of field swap in ipu_csi_init_interface(). - add support for interweaved scan with field order swap. Suggested by Philipp Zabel. - in v2, inteweave scan was determined using field types of CSI (and PRPENCVF) at the sink and source pads. In v3, this has been moved one hop downstream: interweave is now determined using field type at source pad, and field type selected at capture interface. Suggested by Philipp. - make sure to double CSI crop target height when input field type in alternate. - more updates to media driver doc to reflect above. v2: - update media driver doc. - enable idmac interweave only if input field is sequential/alternate, and output field is 'interlaced*'. - move field try logic out of *try_fmt and into separate function. - fix bug with resetting crop/compose rectangles. - add a patch that fixes a field order bug in VDIC indirect mode. - remove alternate field type from V4L2_FIELD_IS_SEQUENTIAL() macro Suggested-by: Nicolas Dufresne . - add macro V4L2_FIELD_IS_INTERLACED(). Philipp Zabel (1): gpu: ipu-v3: Allow negative offsets for interlaced scanning Steve Longerbeam (13): media: videodev2.h: Add more field helper macros gpu: ipu-csi: Check for field type alternate gpu: ipu-csi: Swap fields according to input/output field types gpu: ipu-v3: Fix U/V offset macros for planar 4:2:0 gpu: ipu-v3: Add planar support to interlaced scan media: imx: Fix field negotiation media: imx-csi: Double crop height for alternate fields at sink media: imx: interweave and odd-chroma-row skip are incompatible media: imx-csi: Allow skipping odd chroma rows for YVU420 media: imx: vdic: rely on VDIC for correct field order media: imx-csi: Move crop/compose reset after filling default mbus fields media: imx: Allow interweave with top/bottom lines swapped media: imx.rst: Update doc to reflect fixes to interlaced capture Documentation/media/v4l-drivers/imx.rst | 93 ++- drivers/gpu/ipu-v3/ipu-cpmem.c| 45 ++- drivers/gpu/ipu-v3/ipu-csi.c | 136 ++--- drivers/staging/media/imx/imx-ic-prpencvf.c | 48 ++-- drivers/staging/media/imx/imx-media-capture.c | 14 +++ drivers/staging/media/imx/imx-media-csi.c | 166 ++ drivers/staging/media/imx/imx-media-vdic.c| 12 +- include/uapi/linux/videodev2.h| 7 ++ include/video/imx-ipu-v3.h| 6 +- 9 files changed, 377 insertions(+), 150 deletions(-) -- 2.7.4
Re: [PATCH 18/22] partial revert of "[media] tvp5150: add HW input connectors support"
Hi Marco, On 08/01/2018 05:49 PM, Marco Felsch wrote: > Hi Javier, > > On 18-07-31 15:30, Marco Felsch wrote: >> Hi Javier, >> >> On 18-07-31 14:52, Javier Martinez Canillas wrote: >>> Hi Marco, >>> >>> On 07/31/2018 02:36 PM, Marco Felsch wrote: >>> >>> [snip] >>> > > Yes, another thing that patch 19/22 should take into account is DTs that > don't have input connectors defined. So probably TVP5150_PORT_YOUT should > be 0 instead of TVP5150_PORT_NUM - 1 as is the case in the current patch. > > In other words, it should work both when input connectors are defined in > the DT and when these are not defined and only an output port is defined. Yes, it would be a approach to map the output port dynamicaly to the highest port number. I tried to keep things easy by a static mapping. Maybe a follow up patch can change this behaviour. Anyway, input connectors aren't required. There must be at least one port child node with a correct port-number in the DT. >>> >>> Yes, that was my point. But your patch uses the port child reg property as >>> the index for the struct device_node *endpoints[TVP5150_PORT_NUM] array. >>> >>> If there's only one port child (for the output) then the DT binding says >>> that the reg property isn't required, so this will be 0 and your patch will >>> wrongly map it to TVP5150_PORT_AIP1A. That's why I said that the output port >>> should be the first one in your enum tvp5150_ports and not the last one. >> >> Yes, now I got you. I implemted this in such a way in my first apporach. >> But at the moment I don't know why I changed this. Maybe to keep the >> decoder->input number in sync with the em28xx devices, which will set the >> port by the s_routing() callback. >> >> Let me check this. > > I checked it again. Your're right, it should be doable but IMHO it isn't > the right solution. I checked some drivers which use of_graph and all of > them put the output at the end. So the tvp5150 will be the only one > which maps the out put to the first pad and it isn't intuitive. > Ah, I didn't notice that the tvp5150 was the exception. I just mentioned due DT maintainers always ask for driver changes to be DTB backward compatible. > I discused it with a colleague. We think a better solution would be to fix > the v4l2-core parser code to allow a independent dt-port<->pad mapping. > Since now the pad's correspond to the port number. This mapping should > be done by a driver callback, so each driver can do it's own custom > mapping. > That sounds good to me. > Regards, > Marco > Best regards, -- Javier Martinez Canillas Software Engineer - Desktop Hardware Enablement Red Hat
[PATCH 13/13] media: v4l2-mc: get rid of global pad indexes
Now that all drivers are using pad signal types, we can get
rid of the global static definition, as routes are stablished
using the pad signal type.
The tuner and IF-PLL pads are now used only by the tuner core,
so move the definitions to be there.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/v4l2-core/tuner-core.c | 13 +
include/media/v4l2-mc.h | 76
2 files changed, 13 insertions(+), 76 deletions(-)
diff --git a/drivers/media/v4l2-core/tuner-core.c
b/drivers/media/v4l2-core/tuner-core.c
index d4c32ccd0930..e35438ca0b50 100644
--- a/drivers/media/v4l2-core/tuner-core.c
+++ b/drivers/media/v4l2-core/tuner-core.c
@@ -97,6 +97,19 @@ static const struct v4l2_subdev_ops tuner_ops;
* Internal struct used inside the driver
*/
+enum tuner_pad_index {
+ TUNER_PAD_RF_INPUT,
+ TUNER_PAD_OUTPUT,
+ TUNER_PAD_AUD_OUT,
+ TUNER_NUM_PADS
+};
+
+enum if_vid_dec_pad_index {
+ IF_VID_DEC_PAD_IF_INPUT,
+ IF_VID_DEC_PAD_OUT,
+ IF_VID_DEC_PAD_NUM_PADS
+};
+
struct tuner {
/* device */
struct dvb_frontend fe;
diff --git a/include/media/v4l2-mc.h b/include/media/v4l2-mc.h
index 7c9c781b16a9..bf5043c1ab6b 100644
--- a/include/media/v4l2-mc.h
+++ b/include/media/v4l2-mc.h
@@ -23,82 +23,6 @@
#include
#include
-/**
- * enum tuner_pad_index - tuner pad index for MEDIA_ENT_F_TUNER
- *
- * @TUNER_PAD_RF_INPUT:Radiofrequency (RF) sink pad, usually linked to
a
- * RF connector entity.
- * @TUNER_PAD_OUTPUT: Tuner video output source pad. Contains the video
- * chrominance and luminance or the hole bandwidth
- * of the signal converted to an Intermediate Frequency
- * (IF) or to baseband (on zero-IF tuners).
- * @TUNER_PAD_AUD_OUT: Tuner audio output source pad. Tuners used to decode
- * analog TV signals have an extra pad for audio output.
- * Old tuners use an analog stage with a saw filter for
- * the audio IF frequency. The output of the pad is, in
- * this case, the audio IF, with should be decoded either
- * by the bridge chipset (that's the case of cx2388x
- * chipsets) or may require an external IF sound
- * processor, like msp34xx. On modern silicon tuners,
- * the audio IF decoder is usually incorporated at the
- * tuner. On such case, the output of this pad is an
- * audio sampled data.
- * @TUNER_NUM_PADS:Number of pads of the tuner.
- */
-enum tuner_pad_index {
- TUNER_PAD_RF_INPUT,
- TUNER_PAD_OUTPUT,
- TUNER_PAD_AUD_OUT,
- TUNER_NUM_PADS
-};
-
-/**
- * enum if_vid_dec_pad_index - video IF-PLL pad index for
- *MEDIA_ENT_F_IF_VID_DECODER
- *
- * @IF_VID_DEC_PAD_IF_INPUT: video Intermediate Frequency (IF) sink pad
- * @IF_VID_DEC_PAD_OUT:IF-PLL video output source pad.
Contains the
- * video chrominance and luminance IF signals.
- * @IF_VID_DEC_PAD_NUM_PADS: Number of pads of the video IF-PLL.
- */
-enum if_vid_dec_pad_index {
- IF_VID_DEC_PAD_IF_INPUT,
- IF_VID_DEC_PAD_OUT,
- IF_VID_DEC_PAD_NUM_PADS
-};
-
-/**
- * enum if_aud_dec_pad_index - audio/sound IF-PLL pad index for
- *MEDIA_ENT_F_IF_AUD_DECODER
- *
- * @IF_AUD_DEC_PAD_IF_INPUT: audio Intermediate Frequency (IF) sink pad
- * @IF_AUD_DEC_PAD_OUT:IF-PLL audio output source pad.
Contains the
- * audio sampled stream data, usually connected
- * to the bridge bus via an Inter-IC Sound (I2S)
- * bus.
- * @IF_AUD_DEC_PAD_NUM_PADS: Number of pads of the audio IF-PLL.
- */
-enum if_aud_dec_pad_index {
- IF_AUD_DEC_PAD_IF_INPUT,
- IF_AUD_DEC_PAD_OUT,
- IF_AUD_DEC_PAD_NUM_PADS
-};
-
-/**
- * enum demod_pad_index - analog TV pad index for MEDIA_ENT_F_ATV_DECODER
- *
- * @DEMOD_PAD_IF_INPUT:IF input sink pad.
- * @DEMOD_PAD_VID_OUT: Video output source pad.
- * @DEMOD_PAD_AUDIO_OUT: Audio output source pad.
- * @DEMOD_NUM_PADS:Maximum number of output pads.
- */
-enum demod_pad_index {
- DEMOD_PAD_IF_INPUT,
- DEMOD_PAD_VID_OUT,
- DEMOD_PAD_AUDIO_OUT,
- DEMOD_NUM_PADS
-};
-
/* We don't need to include pci.h or usb.h here */
struct pci_dev;
struct usb_device;
--
2.17.1
[PATCH 10/13] media: si2157: declare its own pads
As we don't need anymore to share pad numbers with similar
drivers, use its own pad definition instead of a global
model.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/tuners/si2157.c | 14 +++---
drivers/media/tuners/si2157_priv.h | 9 -
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index d222912662d7..dc21a86c0175 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -468,14 +468,14 @@ static int si2157_probe(struct i2c_client *client,
dev->ent.name = KBUILD_MODNAME;
dev->ent.function = MEDIA_ENT_F_TUNER;
- dev->pad[TUNER_PAD_RF_INPUT].flags = MEDIA_PAD_FL_SINK;
- dev->pad[TUNER_PAD_RF_INPUT].sig_type = PAD_SIGNAL_ANALOG;
- dev->pad[TUNER_PAD_OUTPUT].flags = MEDIA_PAD_FL_SOURCE;
- dev->pad[TUNER_PAD_OUTPUT].sig_type = PAD_SIGNAL_TV_CARRIERS;
- dev->pad[TUNER_PAD_AUD_OUT].flags = MEDIA_PAD_FL_SOURCE;
- dev->pad[TUNER_PAD_AUD_OUT].sig_type = PAD_SIGNAL_AUDIO;
+ dev->pad[SI2157_PAD_RF_INPUT].flags = MEDIA_PAD_FL_SINK;
+ dev->pad[SI2157_PAD_RF_INPUT].sig_type = PAD_SIGNAL_ANALOG;
+ dev->pad[SI2157_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
+ dev->pad[SI2157_PAD_VID_OUT].sig_type = PAD_SIGNAL_TV_CARRIERS;
+ dev->pad[SI2157_PAD_AUD_OUT].flags = MEDIA_PAD_FL_SOURCE;
+ dev->pad[SI2157_PAD_AUD_OUT].sig_type = PAD_SIGNAL_AUDIO;
- ret = media_entity_pads_init(>ent, TUNER_NUM_PADS,
+ ret = media_entity_pads_init(>ent, SI2157_NUM_PADS,
>pad[0]);
if (ret)
diff --git a/drivers/media/tuners/si2157_priv.h
b/drivers/media/tuners/si2157_priv.h
index e6436f74abaa..129a35e4e11b 100644
--- a/drivers/media/tuners/si2157_priv.h
+++ b/drivers/media/tuners/si2157_priv.h
@@ -21,6 +21,13 @@
#include
#include "si2157.h"
+enum si2157_pads {
+ SI2157_PAD_RF_INPUT,
+ SI2157_PAD_VID_OUT,
+ SI2157_PAD_AUD_OUT,
+ SI2157_NUM_PADS
+};
+
/* state struct */
struct si2157_dev {
struct mutex i2c_mutex;
@@ -35,7 +42,7 @@ struct si2157_dev {
#if defined(CONFIG_MEDIA_CONTROLLER)
struct media_device *mdev;
struct media_entity ent;
- struct media_padpad[TUNER_NUM_PADS];
+ struct media_padpad[SI2157_NUM_PADS];
#endif
};
--
2.17.1
[PATCH 01/13] media: v4l2: remove VBI output pad
The signal there is the same as the video output (well,
except for sliced VBI, but let's simplify the model and ignore
it, at least for now - as it is routed together with raw
VBI).
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/dvb-frontends/au8522_decoder.c | 1 -
drivers/media/i2c/saa7115.c | 1 -
drivers/media/i2c/tvp5150.c | 1 -
drivers/media/pci/saa7134/saa7134-core.c | 1 -
drivers/media/v4l2-core/v4l2-mc.c| 2 +-
include/media/v4l2-mc.h | 2 --
6 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/media/dvb-frontends/au8522_decoder.c
b/drivers/media/dvb-frontends/au8522_decoder.c
index f285096a48f0..198dd2b6f326 100644
--- a/drivers/media/dvb-frontends/au8522_decoder.c
+++ b/drivers/media/dvb-frontends/au8522_decoder.c
@@ -720,7 +720,6 @@ static int au8522_probe(struct i2c_client *client,
state->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
state->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
- state->pads[DEMOD_PAD_VBI_OUT].flags = MEDIA_PAD_FL_SOURCE;
state->pads[DEMOD_PAD_AUDIO_OUT].flags = MEDIA_PAD_FL_SOURCE;
sd->entity.function = MEDIA_ENT_F_ATV_DECODER;
diff --git a/drivers/media/i2c/saa7115.c b/drivers/media/i2c/saa7115.c
index b07114b5efb2..4c72db58dfd2 100644
--- a/drivers/media/i2c/saa7115.c
+++ b/drivers/media/i2c/saa7115.c
@@ -1836,7 +1836,6 @@ static int saa711x_probe(struct i2c_client *client,
#if defined(CONFIG_MEDIA_CONTROLLER)
state->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
state->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
- state->pads[DEMOD_PAD_VBI_OUT].flags = MEDIA_PAD_FL_SOURCE;
sd->entity.function = MEDIA_ENT_F_ATV_DECODER;
diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 76e6bed5a1da..66235e10acdd 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -1501,7 +1501,6 @@ static int tvp5150_probe(struct i2c_client *c,
#if defined(CONFIG_MEDIA_CONTROLLER)
core->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
core->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
- core->pads[DEMOD_PAD_VBI_OUT].flags = MEDIA_PAD_FL_SOURCE;
sd->entity.function = MEDIA_ENT_F_ATV_DECODER;
diff --git a/drivers/media/pci/saa7134/saa7134-core.c
b/drivers/media/pci/saa7134/saa7134-core.c
index 9e76de2411ae..267d143c3a48 100644
--- a/drivers/media/pci/saa7134/saa7134-core.c
+++ b/drivers/media/pci/saa7134/saa7134-core.c
@@ -847,7 +847,6 @@ static void saa7134_create_entities(struct saa7134_dev *dev)
dev->demod.name = "saa713x";
dev->demod_pad[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
dev->demod_pad[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
- dev->demod_pad[DEMOD_PAD_VBI_OUT].flags = MEDIA_PAD_FL_SOURCE;
dev->demod.function = MEDIA_ENT_F_ATV_DECODER;
ret = media_entity_pads_init(>demod, DEMOD_NUM_PADS,
diff --git a/drivers/media/v4l2-core/v4l2-mc.c
b/drivers/media/v4l2-core/v4l2-mc.c
index 0fc185a2ce90..982bab3530f6 100644
--- a/drivers/media/v4l2-core/v4l2-mc.c
+++ b/drivers/media/v4l2-core/v4l2-mc.c
@@ -147,7 +147,7 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
}
if (io_vbi) {
- ret = media_create_pad_link(decoder, DEMOD_PAD_VBI_OUT,
+ ret = media_create_pad_link(decoder, DEMOD_PAD_VID_OUT,
io_vbi, 0,
MEDIA_LNK_FL_ENABLED);
if (ret)
diff --git a/include/media/v4l2-mc.h b/include/media/v4l2-mc.h
index 2634d9dc9916..7c9c781b16a9 100644
--- a/include/media/v4l2-mc.h
+++ b/include/media/v4l2-mc.h
@@ -89,14 +89,12 @@ enum if_aud_dec_pad_index {
*
* @DEMOD_PAD_IF_INPUT:IF input sink pad.
* @DEMOD_PAD_VID_OUT: Video output source pad.
- * @DEMOD_PAD_VBI_OUT: Vertical Blank Interface (VBI) output source pad.
* @DEMOD_PAD_AUDIO_OUT: Audio output source pad.
* @DEMOD_NUM_PADS:Maximum number of output pads.
*/
enum demod_pad_index {
DEMOD_PAD_IF_INPUT,
DEMOD_PAD_VID_OUT,
- DEMOD_PAD_VBI_OUT,
DEMOD_PAD_AUDIO_OUT,
DEMOD_NUM_PADS
};
--
2.17.1
[PATCH 11/13] media: saa7134: declare its own pads
As we don't need anymore to share pad numbers with similar
drivers, use its own pad definition instead of a global
model.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/pci/saa7134/saa7134-core.c | 10 +-
drivers/media/pci/saa7134/saa7134.h | 8 +++-
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/media/pci/saa7134/saa7134-core.c
b/drivers/media/pci/saa7134/saa7134-core.c
index c4e2df197bf9..8984b1bf57a5 100644
--- a/drivers/media/pci/saa7134/saa7134-core.c
+++ b/drivers/media/pci/saa7134/saa7134-core.c
@@ -845,13 +845,13 @@ static void saa7134_create_entities(struct saa7134_dev
*dev)
*/
if (!decoder) {
dev->demod.name = "saa713x";
- dev->demod_pad[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
- dev->demod_pad[DEMOD_PAD_IF_INPUT].sig_type = PAD_SIGNAL_ANALOG;
- dev->demod_pad[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
- dev->demod_pad[DEMOD_PAD_VID_OUT].sig_type = PAD_SIGNAL_DV;
+ dev->demod_pad[SAA7134_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
+ dev->demod_pad[SAA7134_PAD_IF_INPUT].sig_type =
PAD_SIGNAL_ANALOG;
+ dev->demod_pad[SAA7134_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
+ dev->demod_pad[SAA7134_PAD_VID_OUT].sig_type = PAD_SIGNAL_DV;
dev->demod.function = MEDIA_ENT_F_ATV_DECODER;
- ret = media_entity_pads_init(>demod, DEMOD_NUM_PADS,
+ ret = media_entity_pads_init(>demod, SAA7134_NUM_PADS,
dev->demod_pad);
if (ret < 0)
pr_err("failed to initialize demod pad!\n");
diff --git a/drivers/media/pci/saa7134/saa7134.h
b/drivers/media/pci/saa7134/saa7134.h
index d99e937a98c1..ac05a38aa728 100644
--- a/drivers/media/pci/saa7134/saa7134.h
+++ b/drivers/media/pci/saa7134/saa7134.h
@@ -547,6 +547,12 @@ struct saa7134_mpeg_ops {
unsigned long status);
};
+enum saa7134_pads {
+ SAA7134_PAD_IF_INPUT,
+ SAA7134_PAD_VID_OUT,
+ SAA7134_NUM_PADS
+};
+
/* global device status */
struct saa7134_dev {
struct list_head devlist;
@@ -674,7 +680,7 @@ struct saa7134_dev {
struct media_pad input_pad[SAA7134_INPUT_MAX + 1];
struct media_entity demod;
- struct media_pad demod_pad[DEMOD_NUM_PADS];
+ struct media_pad demod_pad[SAA7134_NUM_PADS];
struct media_pad video_pad, vbi_pad;
struct media_entity *decoder;
--
2.17.1
[PATCH 05/13] media: au0828: use signals instead of hardcoding a pad number
When creating the audio link, use pad signals, instead of
hardcoding using the pad index number.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/usb/au0828/au0828-core.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/media/usb/au0828/au0828-core.c
b/drivers/media/usb/au0828/au0828-core.c
index cd363a2100d4..4729b2a2f21c 100644
--- a/drivers/media/usb/au0828/au0828-core.c
+++ b/drivers/media/usb/au0828/au0828-core.c
@@ -266,11 +266,13 @@ static void au0828_media_graph_notify(struct media_entity
*new,
create_link:
if (decoder && mixer) {
- ret = media_create_pad_link(decoder,
- DEMOD_PAD_AUDIO_OUT,
- mixer, 0,
- MEDIA_LNK_FL_ENABLED);
- if (ret)
+ ret = media_get_pad_index(decoder, false,
+ PAD_SIGNAL_AUDIO);
+ if (ret >= 0)
+ ret = media_create_pad_link(decoder, ret,
+ mixer, 0,
+ MEDIA_LNK_FL_ENABLED);
+ if (ret < 0)
dev_err(>usbdev->dev,
"Mixer Pad Link Create Error: %d\n", ret);
}
--
2.17.1
[PATCH 00/13] Better handle pads for tuning/decoder part of the devices
At PC consumer devices, it is very common that the bridge same driver to be attached to different types of tuners and demods. We need a way for the Kernel to properly identify what kind of signal is provided by each PAD, in order to properly setup the pipelines. The previous approach were to hardcode a fixed number of PADs for all elements of the same type. This is not good, as different devices may actually have a different number of pads. It was acceptable in the past, as there were a promisse of adding "soon" a properties API that would allow to identify the type for each PADs, but this was never merged (or even a patchset got submitted). So, replace this approach by another one: add a "taint" mark to pads that contain different types of signals. I tried to minimize the number of signals, in order to make it simpler to convert from the past way. For now, it is tested only with a simple grabber device. I intend to do more tests before merging it, but it would be interesting to have this merged for Kernel 4.19, as we'll now be exposing the pad index via the MC API version 2. Mauro Carvalho Chehab (13): media: v4l2: remove VBI output pad media: v4l2: taint pads with the signal types for consumer devices v4l2-mc: switch it to use the new approach to setup pipelines media: dvb: use signals to discover pads media: au0828: use signals instead of hardcoding a pad number media: au8522: declare its own pads media: msp3400: declare its own pads media: saa7115: declare its own pads media: tvp5150: declare its own pads media: si2157: declare its own pads media: saa7134: declare its own pads media: mxl111sf: declare its own pads media: v4l2-mc: get rid of global pad indexes drivers/media/dvb-core/dvbdev.c | 19 +++-- drivers/media/dvb-frontends/au8522_decoder.c | 10 ++- drivers/media/dvb-frontends/au8522_priv.h| 9 ++- drivers/media/i2c/msp3400-driver.c | 6 +- drivers/media/i2c/msp3400-driver.h | 8 +- drivers/media/i2c/saa7115.c | 18 +++-- drivers/media/i2c/tvp5150.c | 21 -- drivers/media/media-entity.c | 26 +++ drivers/media/pci/saa7134/saa7134-core.c | 9 ++- drivers/media/pci/saa7134/saa7134.h | 8 +- drivers/media/tuners/si2157.c| 11 ++- drivers/media/tuners/si2157_priv.h | 9 ++- drivers/media/usb/au0828/au0828-core.c | 12 +-- drivers/media/usb/dvb-usb-v2/mxl111sf.c | 8 +- drivers/media/usb/dvb-usb-v2/mxl111sf.h | 8 +- drivers/media/v4l2-core/tuner-core.c | 18 + drivers/media/v4l2-core/v4l2-mc.c| 73 +- include/media/media-entity.h | 54 ++ include/media/v4l2-mc.h | 78 19 files changed, 266 insertions(+), 139 deletions(-) -- 2.17.1
[PATCH 07/13] media: msp3400: declare its own pads
As we don't need anymore to share pad numbers with similar
drivers, use its own pad definition instead of a global
model.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/i2c/msp3400-driver.c | 8
drivers/media/i2c/msp3400-driver.h | 8 +++-
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/media/i2c/msp3400-driver.c
b/drivers/media/i2c/msp3400-driver.c
index 3b9c729fbd52..ef70fe0c77a1 100644
--- a/drivers/media/i2c/msp3400-driver.c
+++ b/drivers/media/i2c/msp3400-driver.c
@@ -703,10 +703,10 @@ static int msp_probe(struct i2c_client *client, const
struct i2c_device_id *id)
v4l2_i2c_subdev_init(sd, client, _ops);
#if defined(CONFIG_MEDIA_CONTROLLER)
- state->pads[IF_AUD_DEC_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
- state->pads[IF_AUD_DEC_PAD_IF_INPUT].sig_type = PAD_SIGNAL_AUDIO;
- state->pads[IF_AUD_DEC_PAD_OUT].flags = MEDIA_PAD_FL_SOURCE;
- state->pads[IF_AUD_DEC_PAD_OUT].sig_type = PAD_SIGNAL_AUDIO;
+ state->pads[MSP3400_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
+ state->pads[MSP3400_PAD_IF_INPUT].sig_type = PAD_SIGNAL_AUDIO;
+ state->pads[MSP3400_PAD_OUT].flags = MEDIA_PAD_FL_SOURCE;
+ state->pads[MSP3400_PAD_OUT].sig_type = PAD_SIGNAL_AUDIO;
sd->entity.function = MEDIA_ENT_F_IF_AUD_DECODER;
diff --git a/drivers/media/i2c/msp3400-driver.h
b/drivers/media/i2c/msp3400-driver.h
index b6c7698bce5a..2bb9d5ff1bbd 100644
--- a/drivers/media/i2c/msp3400-driver.h
+++ b/drivers/media/i2c/msp3400-driver.h
@@ -52,6 +52,12 @@ extern int msp_standard;
extern bool msp_dolby;
extern int msp_stereo_thresh;
+enum msp3400_pads {
+ MSP3400_PAD_IF_INPUT,
+ MSP3400_PAD_OUT,
+ MSP3400_NUM_PADS
+};
+
struct msp_state {
struct v4l2_subdev sd;
struct v4l2_ctrl_handler hdl;
@@ -106,7 +112,7 @@ struct msp_state {
unsigned int watch_stereo:1;
#if IS_ENABLED(CONFIG_MEDIA_CONTROLLER)
- struct media_pad pads[IF_AUD_DEC_PAD_NUM_PADS];
+ struct media_pad pads[MSP3400_NUM_PADS];
#endif
};
--
2.17.1
[PATCH 09/13] media: tvp5150: declare its own pads
As we don't need anymore to share pad numbers with similar
drivers, use its own pad definition instead of a global
model.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/i2c/tvp5150.c | 22 ++
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 5037a03b5442..a9f7c70ca25c 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -38,10 +38,16 @@ MODULE_PARM_DESC(debug, "Debug level (0-2)");
#define dprintk0(__dev, __arg...) dev_dbg_lvl(__dev, 0, 0, __arg)
+enum tvp5150_pads {
+ TVP5150_PAD_IF_INPUT,
+ TVP5150_PAD_VID_OUT,
+ TVP5150_NUM_PADS
+};
+
struct tvp5150 {
struct v4l2_subdev sd;
#ifdef CONFIG_MEDIA_CONTROLLER
- struct media_pad pads[DEMOD_NUM_PADS];
+ struct media_pad pads[TVP5150_NUM_PADS];
struct media_entity input_ent[TVP5150_INPUT_NUM];
struct media_pad input_pad[TVP5150_INPUT_NUM];
#endif
@@ -866,7 +872,7 @@ static int tvp5150_fill_fmt(struct v4l2_subdev *sd,
struct v4l2_mbus_framefmt *f;
struct tvp5150 *decoder = to_tvp5150(sd);
- if (!format || (format->pad != DEMOD_PAD_VID_OUT))
+ if (!format || (format->pad != TVP5150_PAD_VID_OUT))
return -EINVAL;
f = >format;
@@ -1217,7 +1223,7 @@ static int tvp5150_registered(struct v4l2_subdev *sd)
return ret;
ret = media_create_pad_link(input, 0, >entity,
- DEMOD_PAD_IF_INPUT, 0);
+ TVP5150_PAD_IF_INPUT, 0);
if (ret < 0) {
media_device_unregister_entity(input);
return ret;
@@ -1499,14 +1505,14 @@ static int tvp5150_probe(struct i2c_client *c,
sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
#if defined(CONFIG_MEDIA_CONTROLLER)
- core->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
- core->pads[DEMOD_PAD_IF_INPUT].sig_type = PAD_SIGNAL_ANALOG;
- core->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
- core->pads[DEMOD_PAD_VID_OUT].sig_type = PAD_SIGNAL_DV;
+ core->pads[TVP5150_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
+ core->pads[TVP5150_PAD_IF_INPUT].sig_type = PAD_SIGNAL_ANALOG;
+ core->pads[TVP5150_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
+ core->pads[TVP5150_PAD_VID_OUT].sig_type = PAD_SIGNAL_DV;
sd->entity.function = MEDIA_ENT_F_ATV_DECODER;
- res = media_entity_pads_init(>entity, DEMOD_NUM_PADS, core->pads);
+ res = media_entity_pads_init(>entity, TVP5150_NUM_PADS, core->pads);
if (res < 0)
return res;
--
2.17.1
[PATCH 02/13] media: v4l2: taint pads with the signal types for consumer devices
Consumer devices are provided with a wide diferent range of types
supported by the same driver, allowing different configutations.
In order to make easier to setup media controller links, "taint"
pads with the signal type it carries.
While here, get rid of DEMOD_PAD_VBI_OUT, as the signal it carries
is actually the same as the normal video output.
The difference happens at the video/VBI interface:
- for VBI, only the hidden lines are streamed;
- for video, the stream is usually cropped to hide the
vbi lines.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/dvb-frontends/au8522_decoder.c | 3 ++
drivers/media/i2c/msp3400-driver.c | 2 ++
drivers/media/i2c/saa7115.c | 2 ++
drivers/media/i2c/tvp5150.c | 2 ++
drivers/media/pci/saa7134/saa7134-core.c | 2 ++
drivers/media/tuners/si2157.c| 3 ++
drivers/media/usb/dvb-usb-v2/mxl111sf.c | 2 ++
drivers/media/v4l2-core/tuner-core.c | 5 +++
include/media/media-entity.h | 35
9 files changed, 56 insertions(+)
diff --git a/drivers/media/dvb-frontends/au8522_decoder.c
b/drivers/media/dvb-frontends/au8522_decoder.c
index 198dd2b6f326..583fdaa7339f 100644
--- a/drivers/media/dvb-frontends/au8522_decoder.c
+++ b/drivers/media/dvb-frontends/au8522_decoder.c
@@ -719,8 +719,11 @@ static int au8522_probe(struct i2c_client *client,
#if defined(CONFIG_MEDIA_CONTROLLER)
state->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
+ state->pads[DEMOD_PAD_IF_INPUT].sig_type = PAD_SIGNAL_ANALOG;
state->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
+ state->pads[DEMOD_PAD_VID_OUT].sig_type = PAD_SIGNAL_DV;
state->pads[DEMOD_PAD_AUDIO_OUT].flags = MEDIA_PAD_FL_SOURCE;
+ state->pads[DEMOD_PAD_AUDIO_OUT].sig_type = PAD_SIGNAL_AUDIO;
sd->entity.function = MEDIA_ENT_F_ATV_DECODER;
ret = media_entity_pads_init(>entity, ARRAY_SIZE(state->pads),
diff --git a/drivers/media/i2c/msp3400-driver.c
b/drivers/media/i2c/msp3400-driver.c
index 3db966db83eb..3b9c729fbd52 100644
--- a/drivers/media/i2c/msp3400-driver.c
+++ b/drivers/media/i2c/msp3400-driver.c
@@ -704,7 +704,9 @@ static int msp_probe(struct i2c_client *client, const
struct i2c_device_id *id)
#if defined(CONFIG_MEDIA_CONTROLLER)
state->pads[IF_AUD_DEC_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
+ state->pads[IF_AUD_DEC_PAD_IF_INPUT].sig_type = PAD_SIGNAL_AUDIO;
state->pads[IF_AUD_DEC_PAD_OUT].flags = MEDIA_PAD_FL_SOURCE;
+ state->pads[IF_AUD_DEC_PAD_OUT].sig_type = PAD_SIGNAL_AUDIO;
sd->entity.function = MEDIA_ENT_F_IF_AUD_DECODER;
diff --git a/drivers/media/i2c/saa7115.c b/drivers/media/i2c/saa7115.c
index 4c72db58dfd2..8798a06c212f 100644
--- a/drivers/media/i2c/saa7115.c
+++ b/drivers/media/i2c/saa7115.c
@@ -1835,7 +1835,9 @@ static int saa711x_probe(struct i2c_client *client,
#if defined(CONFIG_MEDIA_CONTROLLER)
state->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
+ state->pads[DEMOD_PAD_IF_INPUT].sig_type = PAD_SIGNAL_ANALOG;
state->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
+ state->pads[DEMOD_PAD_VID_OUT].sig_type = PAD_SIGNAL_DV;
sd->entity.function = MEDIA_ENT_F_ATV_DECODER;
diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 66235e10acdd..5037a03b5442 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -1500,7 +1500,9 @@ static int tvp5150_probe(struct i2c_client *c,
#if defined(CONFIG_MEDIA_CONTROLLER)
core->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
+ core->pads[DEMOD_PAD_IF_INPUT].sig_type = PAD_SIGNAL_ANALOG;
core->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
+ core->pads[DEMOD_PAD_VID_OUT].sig_type = PAD_SIGNAL_DV;
sd->entity.function = MEDIA_ENT_F_ATV_DECODER;
diff --git a/drivers/media/pci/saa7134/saa7134-core.c
b/drivers/media/pci/saa7134/saa7134-core.c
index 267d143c3a48..c4e2df197bf9 100644
--- a/drivers/media/pci/saa7134/saa7134-core.c
+++ b/drivers/media/pci/saa7134/saa7134-core.c
@@ -846,7 +846,9 @@ static void saa7134_create_entities(struct saa7134_dev *dev)
if (!decoder) {
dev->demod.name = "saa713x";
dev->demod_pad[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
+ dev->demod_pad[DEMOD_PAD_IF_INPUT].sig_type = PAD_SIGNAL_ANALOG;
dev->demod_pad[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
+ dev->demod_pad[DEMOD_PAD_VID_OUT].sig_type = PAD_SIGNAL_DV;
dev->demod.function = MEDIA_ENT_F_ATV_DECODER;
ret = media_entity_pads_init(>demod, DEMOD_NUM_PADS,
diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 9e34d31d724d..d222912662d7 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -469,8 +469,11 @@
[PATCH 06/13] media: au8522: declare its own pads
As we don't need anymore to share pad numbers with similar
drivers, use its own pad definition instead of a global
model.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/dvb-frontends/au8522_decoder.c | 12 ++--
drivers/media/dvb-frontends/au8522_priv.h| 9 -
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/media/dvb-frontends/au8522_decoder.c
b/drivers/media/dvb-frontends/au8522_decoder.c
index 583fdaa7339f..b2dd20ffd002 100644
--- a/drivers/media/dvb-frontends/au8522_decoder.c
+++ b/drivers/media/dvb-frontends/au8522_decoder.c
@@ -718,12 +718,12 @@ static int au8522_probe(struct i2c_client *client,
v4l2_i2c_subdev_init(sd, client, _ops);
#if defined(CONFIG_MEDIA_CONTROLLER)
- state->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
- state->pads[DEMOD_PAD_IF_INPUT].sig_type = PAD_SIGNAL_ANALOG;
- state->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
- state->pads[DEMOD_PAD_VID_OUT].sig_type = PAD_SIGNAL_DV;
- state->pads[DEMOD_PAD_AUDIO_OUT].flags = MEDIA_PAD_FL_SOURCE;
- state->pads[DEMOD_PAD_AUDIO_OUT].sig_type = PAD_SIGNAL_AUDIO;
+ state->pads[AU8522_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
+ state->pads[AU8522_PAD_IF_INPUT].sig_type = PAD_SIGNAL_ANALOG;
+ state->pads[AU8522_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
+ state->pads[AU8522_PAD_VID_OUT].sig_type = PAD_SIGNAL_DV;
+ state->pads[AU8522_PAD_AUDIO_OUT].flags = MEDIA_PAD_FL_SOURCE;
+ state->pads[AU8522_PAD_AUDIO_OUT].sig_type = PAD_SIGNAL_AUDIO;
sd->entity.function = MEDIA_ENT_F_ATV_DECODER;
ret = media_entity_pads_init(>entity, ARRAY_SIZE(state->pads),
diff --git a/drivers/media/dvb-frontends/au8522_priv.h
b/drivers/media/dvb-frontends/au8522_priv.h
index 2043c1744753..68299d2705f7 100644
--- a/drivers/media/dvb-frontends/au8522_priv.h
+++ b/drivers/media/dvb-frontends/au8522_priv.h
@@ -40,6 +40,13 @@
#define AU8522_DIGITAL_MODE 1
#define AU8522_SUSPEND_MODE 2
+enum au8522_pads {
+ AU8522_PAD_IF_INPUT,
+ AU8522_PAD_VID_OUT,
+ AU8522_PAD_AUDIO_OUT,
+ AU8522_NUM_PADS
+};
+
struct au8522_state {
struct i2c_client *c;
struct i2c_adapter *i2c;
@@ -71,7 +78,7 @@ struct au8522_state {
struct v4l2_ctrl_handler hdl;
#ifdef CONFIG_MEDIA_CONTROLLER
- struct media_pad pads[DEMOD_NUM_PADS];
+ struct media_pad pads[AU8522_NUM_PADS];
#endif
};
--
2.17.1
[PATCH 04/13] media: dvb: use signals to discover pads
On tuner pads, multiple signals are present. Be sure to get
the right PAD by using them.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/dvb-core/dvbdev.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index 3c8778570331..50f174248eef 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -621,7 +621,7 @@ int dvb_create_media_graph(struct dvb_adapter *adap,
unsigned demux_pad = 0;
unsigned dvr_pad = 0;
unsigned ntuner = 0, ndemod = 0;
- int ret;
+ int ret, pad_source, pad_sink;
static const char *connector_name = "Television";
if (!mdev)
@@ -681,7 +681,7 @@ int dvb_create_media_graph(struct dvb_adapter *adap,
if (ret)
return ret;
- if (!ntuner)
+ if (!ntuner) {
ret = media_create_pad_links(mdev,
MEDIA_ENT_F_CONN_RF,
conn, 0,
@@ -689,22 +689,31 @@ int dvb_create_media_graph(struct dvb_adapter *adap,
demod, 0,
MEDIA_LNK_FL_ENABLED,
false);
- else
+ } else {
+ pad_sink = media_get_pad_index(tuner, true,
+ PAD_SIGNAL_ANALOG);
+ if (pad_sink < 0)
+ return -EINVAL;
ret = media_create_pad_links(mdev,
MEDIA_ENT_F_CONN_RF,
conn, 0,
MEDIA_ENT_F_TUNER,
-tuner, TUNER_PAD_RF_INPUT,
+tuner, pad_sink,
MEDIA_LNK_FL_ENABLED,
false);
+ }
if (ret)
return ret;
}
if (ntuner && ndemod) {
+ pad_source = media_get_pad_index(tuner, true,
+PAD_SIGNAL_ANALOG);
+ if (pad_source)
+ return -EINVAL;
ret = media_create_pad_links(mdev,
MEDIA_ENT_F_TUNER,
-tuner, TUNER_PAD_OUTPUT,
+tuner, pad_source,
MEDIA_ENT_F_DTV_DEMOD,
demod, 0, MEDIA_LNK_FL_ENABLED,
false);
--
2.17.1
[PATCH 03/13] v4l2-mc: switch it to use the new approach to setup pipelines
Instead of relying on a static map for pids, use the new sig_type
"taint" type to setup the pipelines with the same tipe between
different entities.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/media-entity.c | 26 +++
drivers/media/v4l2-core/v4l2-mc.c | 73 ---
include/media/media-entity.h | 19
3 files changed, 101 insertions(+), 17 deletions(-)
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 3498551e618e..0b1cb3559140 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -662,6 +662,32 @@ static void __media_entity_remove_link(struct media_entity
*entity,
kfree(link);
}
+int media_get_pad_index(struct media_entity *entity, bool is_sink,
+ enum media_pad_signal_type sig_type)
+{
+ int i;
+ bool pad_is_sink;
+
+ if (!entity)
+ return -EINVAL;
+
+ for (i = 0; i < entity->num_pads; i++) {
+ if (entity->pads[i].flags == MEDIA_PAD_FL_SINK)
+ pad_is_sink = true;
+ else if (entity->pads[i].flags == MEDIA_PAD_FL_SOURCE)
+ pad_is_sink = false;
+ else
+ continue; /* This is an error! */
+
+ if (pad_is_sink != is_sink)
+ continue;
+ if (entity->pads[i].sig_type == sig_type)
+ return i;
+ }
+ return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(media_get_pad_index);
+
int
media_create_pad_link(struct media_entity *source, u16 source_pad,
struct media_entity *sink, u16 sink_pad, u32 flags)
diff --git a/drivers/media/v4l2-core/v4l2-mc.c
b/drivers/media/v4l2-core/v4l2-mc.c
index 982bab3530f6..1925e1a3b861 100644
--- a/drivers/media/v4l2-core/v4l2-mc.c
+++ b/drivers/media/v4l2-core/v4l2-mc.c
@@ -28,7 +28,7 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
struct media_entity *io_v4l = NULL, *io_vbi = NULL, *io_swradio = NULL;
bool is_webcam = false;
u32 flags;
- int ret;
+ int ret, pad_sink, pad_source;
if (!mdev)
return 0;
@@ -97,29 +97,52 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
/* Link the tuner and IF video output pads */
if (tuner) {
if (if_vid) {
- ret = media_create_pad_link(tuner, TUNER_PAD_OUTPUT,
- if_vid,
- IF_VID_DEC_PAD_IF_INPUT,
+ pad_source = media_get_pad_index(tuner, false,
+PAD_SIGNAL_ANALOG);
+ pad_sink = media_get_pad_index(if_vid, true,
+ PAD_SIGNAL_ANALOG);
+ if (pad_source < 0 || pad_sink < 0)
+ return -EINVAL;
+ ret = media_create_pad_link(tuner, pad_source,
+ if_vid, pad_sink,
MEDIA_LNK_FL_ENABLED);
if (ret)
return ret;
- ret = media_create_pad_link(if_vid, IF_VID_DEC_PAD_OUT,
- decoder, DEMOD_PAD_IF_INPUT,
+
+ pad_source = media_get_pad_index(if_vid, false,
+PAD_SIGNAL_DV);
+ pad_sink = media_get_pad_index(decoder, true,
+ PAD_SIGNAL_DV);
+ if (pad_source < 0 || pad_sink < 0)
+ return -EINVAL;
+ ret = media_create_pad_link(if_vid, pad_source,
+ decoder, pad_sink,
MEDIA_LNK_FL_ENABLED);
if (ret)
return ret;
} else {
- ret = media_create_pad_link(tuner, TUNER_PAD_OUTPUT,
- decoder, DEMOD_PAD_IF_INPUT,
+ pad_source = media_get_pad_index(tuner, false,
+PAD_SIGNAL_ANALOG);
+ pad_sink = media_get_pad_index(decoder, true,
+ PAD_SIGNAL_ANALOG);
+ if (pad_source < 0 || pad_sink < 0)
+ return -EINVAL;
+ ret = media_create_pad_link(tuner, pad_source,
+ decoder, pad_sink,
MEDIA_LNK_FL_ENABLED);
if (ret)
[PATCH 12/13] media: mxl111sf: declare its own pads
As we don't need anymore to share pad numbers with similar
drivers, use its own pad definition instead of a global
model.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/usb/dvb-usb-v2/mxl111sf.c | 10 +-
drivers/media/usb/dvb-usb-v2/mxl111sf.h | 8 +++-
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/media/usb/dvb-usb-v2/mxl111sf.c
b/drivers/media/usb/dvb-usb-v2/mxl111sf.c
index 51a1b26199e6..7454672fbec9 100644
--- a/drivers/media/usb/dvb-usb-v2/mxl111sf.c
+++ b/drivers/media/usb/dvb-usb-v2/mxl111sf.c
@@ -892,13 +892,13 @@ static int mxl111sf_attach_tuner(struct dvb_usb_adapter
*adap)
#ifdef CONFIG_MEDIA_CONTROLLER_DVB
state->tuner.function = MEDIA_ENT_F_TUNER;
state->tuner.name = "mxl111sf tuner";
- state->tuner_pads[TUNER_PAD_RF_INPUT].flags = MEDIA_PAD_FL_SINK;
- state->tuner_pads[TUNER_PAD_RF_INPUT].sig_type = PAD_SIGNAL_ANALOG;
- state->tuner_pads[TUNER_PAD_OUTPUT].flags = MEDIA_PAD_FL_SOURCE;
- state->tuner_pads[TUNER_PAD_OUTPUT].sig_type = PAD_SIGNAL_TV_CARRIERS;
+ state->tuner_pads[MXL111SF_PAD_RF_INPUT].flags = MEDIA_PAD_FL_SINK;
+ state->tuner_pads[MXL111SF_PAD_RF_INPUT].sig_type = PAD_SIGNAL_ANALOG;
+ state->tuner_pads[MXL111SF_PAD_OUTPUT].flags = MEDIA_PAD_FL_SOURCE;
+ state->tuner_pads[MXL111SF_PAD_OUTPUT].sig_type =
PAD_SIGNAL_TV_CARRIERS;
ret = media_entity_pads_init(>tuner,
-TUNER_NUM_PADS, state->tuner_pads);
+MXL111SF_NUM_PADS, state->tuner_pads);
if (ret)
return ret;
diff --git a/drivers/media/usb/dvb-usb-v2/mxl111sf.h
b/drivers/media/usb/dvb-usb-v2/mxl111sf.h
index 22253d4908eb..ed98654ba7fd 100644
--- a/drivers/media/usb/dvb-usb-v2/mxl111sf.h
+++ b/drivers/media/usb/dvb-usb-v2/mxl111sf.h
@@ -52,6 +52,12 @@ struct mxl111sf_adap_state {
int (*fe_sleep)(struct dvb_frontend *);
};
+enum mxl111sf_pads {
+ MXL111SF_PAD_RF_INPUT,
+ MXL111SF_PAD_OUTPUT,
+ MXL111SF_NUM_PADS
+};
+
struct mxl111sf_state {
struct dvb_usb_device *d;
@@ -94,7 +100,7 @@ struct mxl111sf_state {
struct mutex msg_lock;
#ifdef CONFIG_MEDIA_CONTROLLER_DVB
struct media_entity tuner;
- struct media_pad tuner_pads[2];
+ struct media_pad tuner_pads[MXL111SF_NUM_PADS];
#endif
};
--
2.17.1
[PATCH 08/13] media: saa7115: declare its own pads
As we don't need anymore to share pad numbers with similar
drivers, use its own pad definition instead of a global
model.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/i2c/saa7115.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/media/i2c/saa7115.c b/drivers/media/i2c/saa7115.c
index 8798a06c212f..09bedbc71567 100644
--- a/drivers/media/i2c/saa7115.c
+++ b/drivers/media/i2c/saa7115.c
@@ -59,10 +59,17 @@ enum saa711x_model {
SAA7118,
};
+
+enum saa711x_pads {
+ SAA711X_PAD_IF_INPUT,
+ SAA711X_PAD_VID_OUT,
+ SAA711X_NUM_PADS
+};
+
struct saa711x_state {
struct v4l2_subdev sd;
#ifdef CONFIG_MEDIA_CONTROLLER
- struct media_pad pads[DEMOD_NUM_PADS];
+ struct media_pad pads[SAA711X_NUM_PADS];
#endif
struct v4l2_ctrl_handler hdl;
@@ -1834,14 +1841,14 @@ static int saa711x_probe(struct i2c_client *client,
v4l2_i2c_subdev_init(sd, client, _ops);
#if defined(CONFIG_MEDIA_CONTROLLER)
- state->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
- state->pads[DEMOD_PAD_IF_INPUT].sig_type = PAD_SIGNAL_ANALOG;
- state->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
- state->pads[DEMOD_PAD_VID_OUT].sig_type = PAD_SIGNAL_DV;
+ state->pads[SAA711X_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
+ state->pads[SAA711X_PAD_IF_INPUT].sig_type = PAD_SIGNAL_ANALOG;
+ state->pads[SAA711X_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
+ state->pads[SAA711X_PAD_VID_OUT].sig_type = PAD_SIGNAL_DV;
sd->entity.function = MEDIA_ENT_F_ATV_DECODER;
- ret = media_entity_pads_init(>entity, DEMOD_NUM_PADS, state->pads);
+ ret = media_entity_pads_init(>entity, SAA711X_NUM_PADS,
state->pads);
if (ret < 0)
return ret;
#endif
--
2.17.1
Re: [PATCH 16/22] [media] tvp5150: add querystd
Em Wed, 1 Aug 2018 16:49:26 +0200
Marco Felsch escreveu:
> Hi Mauro,
>
> On 18-08-01 11:22, Mauro Carvalho Chehab wrote:
> > Em Wed, 1 Aug 2018 15:21:25 +0200
> > Marco Felsch escreveu:
> >
> > > Hi Mauro,
> > >
> > > On 18-07-30 15:09, Mauro Carvalho Chehab wrote:
> > > > Em Thu, 28 Jun 2018 18:20:48 +0200
> > > > Marco Felsch escreveu:
> > > >
> > > > > From: Philipp Zabel
> > > > >
> > > > > Add the querystd video_op and make it return V4L2_STD_UNKNOWN while
> > > > > the
> > > > > TVP5150 is not locked to a signal.
> > > > >
> > > > > Signed-off-by: Philipp Zabel
> > > > > Signed-off-by: Marco Felsch
> > > > > ---
> > > > > drivers/media/i2c/tvp5150.c | 10 ++
> > > > > 1 file changed, 10 insertions(+)
> > > > >
> > > > > diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> > > > > index 99d887936ea0..1990aaa17749 100644
> > > > > --- a/drivers/media/i2c/tvp5150.c
> > > > > +++ b/drivers/media/i2c/tvp5150.c
> > > > > @@ -796,6 +796,15 @@ static v4l2_std_id tvp5150_read_std(struct
> > > > > v4l2_subdev *sd)
> > > > > }
> > > > > }
> > > > >
> > > > > +static int tvp5150_querystd(struct v4l2_subdev *sd, v4l2_std_id
> > > > > *std_id)
> > > > > +{
> > > > > + struct tvp5150 *decoder = to_tvp5150(sd);
> > > > > +
> > > > > + *std_id = decoder->lock ? tvp5150_read_std(sd) :
> > > > > V4L2_STD_UNKNOWN;
> > > >
> > > > This patch requires rework. What happens when a device doesn't have
> > > > IRQ enabled? Perhaps it should, instead, read some register in order
> > > > to check for the locking status, as this would work on both cases.
> > >
> > > If IRQ isn't enabled, decoder->lock is set to always true during
> > > probe(). So this case should be fine.
> >
> > Not sure if tvp5150_read_std() will do the right thing. If it does,
> > the above could simply be:
> > std_id = tvp5150_read_std(sd);
> >
> > But, as there are 3 variants of this chipset, it sounds safer to check
> > if the device is locked before calling tvp5150_read_std().
>
> Yes, I'm with you.
>
> >
> > IMHO, the best would be to have a patch like the one below.
> >
> > Regards,
> > Mauro
> >
> > [PATCH] media: tvp5150: implement decoder lock when irq is not used
> >
> > When irq is used, the lock is set via IRQ code. When it isn't,
> > the driver just assumes it is always locked. Instead, read the
> > lock status from the status register.
>
> Yes, that is a better solution.
>
> >
> > Compile-tested only.
> >
> > Signed-off-by: Mauro Carvalho Chehab
> >
> > diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> > index 75e5ffc6573d..e07020d4053d 100644
> > --- a/drivers/media/i2c/tvp5150.c
> > +++ b/drivers/media/i2c/tvp5150.c
> > @@ -811,11 +811,24 @@ static v4l2_std_id tvp5150_read_std(struct
> > v4l2_subdev *sd)
> > }
> > }
> >
> > +static int query_lock(struct v4l2_subdev *sd)
> > +{
> > + struct tvp5150 *decoder = to_tvp5150(sd);
> > + int status;
> > +
> > + if (decoder->irq)
> > + return decoder->lock;
> > +
> > + regmap_read(map, TVP5150_INT_STATUS_REG_A, );
> > +
> > + return (status & 0x06) == 0x06;
>
> Typo? It should be 0x80, as described in the datasheet (SLES209E) or
> just use the TVP5150_INT_A_LOCK_STATUS define. This avoid datasheet
> cross check during reading.
Yes, it is a typo, but at the other line... I meant to use the register
0x88, e. g.:
regmap_read(decoder->regmap, TVP5150_STATUS_REG_1, );
I ran some tests here: the int status reg is not updated.
Also, after thinking a little bit, I opted to not use the query_lock()
at s_stream. It makes no sense there without adding a status polling
logic. I also opted to remove initializing decoder->lock to true, as
this is very counter-intuitive. Instead, I'm adding a test at s_stream
if decoder->irq is set. This makes easier to understand the code.
Btw, on my tests here, I noticed a problem with S-Video... at least with
AV-350 grabber, composite is only working when S-Video is connected.
This bug also happens before your patchset, so this is not a regression
caused by your patches.
Anyway, patch enclosed. I added it together with my patch series at:
https://git.linuxtv.org/mchehab/experimental.git/log/?h=tvp5150-2
I'll keep doing more tests here.
>
> > +}
> > +
> > static int tvp5150_querystd(struct v4l2_subdev *sd, v4l2_std_id *std_id)
> > {
> > struct tvp5150 *decoder = to_tvp5150(sd);
> >
> > - *std_id = decoder->lock ? tvp5150_read_std(sd) : V4L2_STD_UNKNOWN;
> > + *std_id = query_lock(sd) ? tvp5150_read_std(sd) : V4L2_STD_UNKNOWN;
> >
> > return 0;
> > }
> > @@ -1247,7 +1260,7 @@ static int tvp5150_s_stream(struct v4l2_subdev *sd,
> > int enable)
> > tvp5150_enable(sd);
> >
> > /* Enable outputs if decoder is locked */
> > - val = decoder->lock ? decoder->oe : 0;
> > + val = query_lock(sd) ? decoder->oe : 0;
> > int_val =
Re: [PATCH 18/22] partial revert of "[media] tvp5150: add HW input connectors support"
Hi Javier, On 18-07-31 15:30, Marco Felsch wrote: > Hi Javier, > > On 18-07-31 14:52, Javier Martinez Canillas wrote: > > Hi Marco, > > > > On 07/31/2018 02:36 PM, Marco Felsch wrote: > > > > [snip] > > > > >> > > >> Yes, another thing that patch 19/22 should take into account is DTs that > > >> don't have input connectors defined. So probably TVP5150_PORT_YOUT should > > >> be 0 instead of TVP5150_PORT_NUM - 1 as is the case in the current patch. > > >> > > >> In other words, it should work both when input connectors are defined in > > >> the DT and when these are not defined and only an output port is defined. > > > > > > Yes, it would be a approach to map the output port dynamicaly to the > > > highest port number. I tried to keep things easy by a static mapping. > > > Maybe a follow up patch can change this behaviour. > > > > > > Anyway, input connectors aren't required. There must be at least one > > > port child node with a correct port-number in the DT. > > > > > > > Yes, that was my point. But your patch uses the port child reg property as > > the index for the struct device_node *endpoints[TVP5150_PORT_NUM] array. > > > > If there's only one port child (for the output) then the DT binding says > > that the reg property isn't required, so this will be 0 and your patch will > > wrongly map it to TVP5150_PORT_AIP1A. That's why I said that the output port > > should be the first one in your enum tvp5150_ports and not the last one. > > Yes, now I got you. I implemted this in such a way in my first apporach. > But at the moment I don't know why I changed this. Maybe to keep the > decoder->input number in sync with the em28xx devices, which will set the > port by the s_routing() callback. > > Let me check this. I checked it again. Your're right, it should be doable but IMHO it isn't the right solution. I checked some drivers which use of_graph and all of them put the output at the end. So the tvp5150 will be the only one which maps the out put to the first pad and it isn't intuitive. I discused it with a colleague. We think a better solution would be to fix the v4l2-core parser code to allow a independent dt-port<->pad mapping. Since now the pad's correspond to the port number. This mapping should be done by a driver callback, so each driver can do it's own custom mapping. Regards, Marco > > Best Regards, > Marco > > > > Regards, > > > Marco > > > > > > > Best regards, >
Re: [PATCH 16/22] [media] tvp5150: add querystd
Hi Mauro,
On 18-08-01 11:22, Mauro Carvalho Chehab wrote:
> Em Wed, 1 Aug 2018 15:21:25 +0200
> Marco Felsch escreveu:
>
> > Hi Mauro,
> >
> > On 18-07-30 15:09, Mauro Carvalho Chehab wrote:
> > > Em Thu, 28 Jun 2018 18:20:48 +0200
> > > Marco Felsch escreveu:
> > >
> > > > From: Philipp Zabel
> > > >
> > > > Add the querystd video_op and make it return V4L2_STD_UNKNOWN while the
> > > > TVP5150 is not locked to a signal.
> > > >
> > > > Signed-off-by: Philipp Zabel
> > > > Signed-off-by: Marco Felsch
> > > > ---
> > > > drivers/media/i2c/tvp5150.c | 10 ++
> > > > 1 file changed, 10 insertions(+)
> > > >
> > > > diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> > > > index 99d887936ea0..1990aaa17749 100644
> > > > --- a/drivers/media/i2c/tvp5150.c
> > > > +++ b/drivers/media/i2c/tvp5150.c
> > > > @@ -796,6 +796,15 @@ static v4l2_std_id tvp5150_read_std(struct
> > > > v4l2_subdev *sd)
> > > > }
> > > > }
> > > >
> > > > +static int tvp5150_querystd(struct v4l2_subdev *sd, v4l2_std_id
> > > > *std_id)
> > > > +{
> > > > + struct tvp5150 *decoder = to_tvp5150(sd);
> > > > +
> > > > + *std_id = decoder->lock ? tvp5150_read_std(sd) :
> > > > V4L2_STD_UNKNOWN;
> > >
> > > This patch requires rework. What happens when a device doesn't have
> > > IRQ enabled? Perhaps it should, instead, read some register in order
> > > to check for the locking status, as this would work on both cases.
> >
> > If IRQ isn't enabled, decoder->lock is set to always true during
> > probe(). So this case should be fine.
>
> Not sure if tvp5150_read_std() will do the right thing. If it does,
> the above could simply be:
> std_id = tvp5150_read_std(sd);
>
> But, as there are 3 variants of this chipset, it sounds safer to check
> if the device is locked before calling tvp5150_read_std().
Yes, I'm with you.
>
> IMHO, the best would be to have a patch like the one below.
>
> Regards,
> Mauro
>
> [PATCH] media: tvp5150: implement decoder lock when irq is not used
>
> When irq is used, the lock is set via IRQ code. When it isn't,
> the driver just assumes it is always locked. Instead, read the
> lock status from the status register.
Yes, that is a better solution.
>
> Compile-tested only.
>
> Signed-off-by: Mauro Carvalho Chehab
>
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index 75e5ffc6573d..e07020d4053d 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -811,11 +811,24 @@ static v4l2_std_id tvp5150_read_std(struct v4l2_subdev
> *sd)
> }
> }
>
> +static int query_lock(struct v4l2_subdev *sd)
> +{
> + struct tvp5150 *decoder = to_tvp5150(sd);
> + int status;
> +
> + if (decoder->irq)
> + return decoder->lock;
> +
> + regmap_read(map, TVP5150_INT_STATUS_REG_A, );
> +
> + return (status & 0x06) == 0x06;
Typo? It should be 0x80, as described in the datasheet (SLES209E) or
just use the TVP5150_INT_A_LOCK_STATUS define. This avoid datasheet
cross check during reading.
> +}
> +
> static int tvp5150_querystd(struct v4l2_subdev *sd, v4l2_std_id *std_id)
> {
> struct tvp5150 *decoder = to_tvp5150(sd);
>
> - *std_id = decoder->lock ? tvp5150_read_std(sd) : V4L2_STD_UNKNOWN;
> + *std_id = query_lock(sd) ? tvp5150_read_std(sd) : V4L2_STD_UNKNOWN;
>
> return 0;
> }
> @@ -1247,7 +1260,7 @@ static int tvp5150_s_stream(struct v4l2_subdev *sd, int
> enable)
> tvp5150_enable(sd);
>
> /* Enable outputs if decoder is locked */
> - val = decoder->lock ? decoder->oe : 0;
> + val = query_lock(sd) ? decoder->oe : 0;
> int_val = TVP5150_INT_A_LOCK;
> v4l2_subdev_notify_event(>sd, _ev_fmt);
> }
> @@ -1816,8 +1829,6 @@ static int tvp5150_probe(struct i2c_client *c,
> IRQF_ONESHOT, "tvp5150", core);
> if (res)
> return res;
> - } else {
> - core->lock = true;
> }
>
> res = v4l2_async_register_subdev(sd);
>
>
>
Re: [PATCH] media: coda: don't overwrite h.264 profile_idc on decoder instance
On Wed, 2018-08-01 at 16:18 +0200, Lucas Stach wrote: > On a decoder instance, after the profile has been parsed from the stream > __v4l2_ctrl_s_ctrl() is called to notify userspace about changes in the > read-only profile control. This ends up calling back into the CODA driver > where a mssing check on the s_ctrl caused the profile information that has > just been parsed from the stream to be overwritten with the default > baseline profile. > > Later on the driver fails to enable frame reordering, based on the wrong > profile information. > > Fixes: 347de126d1da (media: coda: add read-only h.264 decoder > profile/level controls) > Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel > --- > drivers/media/platform/coda/coda-common.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/coda/coda-common.c > b/drivers/media/platform/coda/coda-common.c > index c7631e117dd3..1ae15d4ec5ed 100644 > --- a/drivers/media/platform/coda/coda-common.c > +++ b/drivers/media/platform/coda/coda-common.c > @@ -1719,7 +1719,8 @@ static int coda_s_ctrl(struct v4l2_ctrl *ctrl) > break; > case V4L2_CID_MPEG_VIDEO_H264_PROFILE: > /* TODO: switch between baseline and constrained baseline */ > - ctx->params.h264_profile_idc = 66; > + if (ctx->inst_type == CODA_INST_ENCODER) > + ctx->params.h264_profile_idc = 66; > break; > case V4L2_CID_MPEG_VIDEO_H264_LEVEL: > /* nothing to do, this is set by the encoder */ regards Philipp
Re: [PATCH 16/22] [media] tvp5150: add querystd
Em Wed, 1 Aug 2018 15:21:25 +0200
Marco Felsch escreveu:
> Hi Mauro,
>
> On 18-07-30 15:09, Mauro Carvalho Chehab wrote:
> > Em Thu, 28 Jun 2018 18:20:48 +0200
> > Marco Felsch escreveu:
> >
> > > From: Philipp Zabel
> > >
> > > Add the querystd video_op and make it return V4L2_STD_UNKNOWN while the
> > > TVP5150 is not locked to a signal.
> > >
> > > Signed-off-by: Philipp Zabel
> > > Signed-off-by: Marco Felsch
> > > ---
> > > drivers/media/i2c/tvp5150.c | 10 ++
> > > 1 file changed, 10 insertions(+)
> > >
> > > diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> > > index 99d887936ea0..1990aaa17749 100644
> > > --- a/drivers/media/i2c/tvp5150.c
> > > +++ b/drivers/media/i2c/tvp5150.c
> > > @@ -796,6 +796,15 @@ static v4l2_std_id tvp5150_read_std(struct
> > > v4l2_subdev *sd)
> > > }
> > > }
> > >
> > > +static int tvp5150_querystd(struct v4l2_subdev *sd, v4l2_std_id *std_id)
> > > +{
> > > + struct tvp5150 *decoder = to_tvp5150(sd);
> > > +
> > > + *std_id = decoder->lock ? tvp5150_read_std(sd) : V4L2_STD_UNKNOWN;
> >
> > This patch requires rework. What happens when a device doesn't have
> > IRQ enabled? Perhaps it should, instead, read some register in order
> > to check for the locking status, as this would work on both cases.
>
> If IRQ isn't enabled, decoder->lock is set to always true during
> probe(). So this case should be fine.
Not sure if tvp5150_read_std() will do the right thing. If it does,
the above could simply be:
std_id = tvp5150_read_std(sd);
But, as there are 3 variants of this chipset, it sounds safer to check
if the device is locked before calling tvp5150_read_std().
IMHO, the best would be to have a patch like the one below.
Regards,
Mauro
[PATCH] media: tvp5150: implement decoder lock when irq is not used
When irq is used, the lock is set via IRQ code. When it isn't,
the driver just assumes it is always locked. Instead, read the
lock status from the status register.
Compile-tested only.
Signed-off-by: Mauro Carvalho Chehab
diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 75e5ffc6573d..e07020d4053d 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -811,11 +811,24 @@ static v4l2_std_id tvp5150_read_std(struct v4l2_subdev
*sd)
}
}
+static int query_lock(struct v4l2_subdev *sd)
+{
+ struct tvp5150 *decoder = to_tvp5150(sd);
+ int status;
+
+ if (decoder->irq)
+ return decoder->lock;
+
+ regmap_read(map, TVP5150_INT_STATUS_REG_A, );
+
+ return (status & 0x06) == 0x06;
+}
+
static int tvp5150_querystd(struct v4l2_subdev *sd, v4l2_std_id *std_id)
{
struct tvp5150 *decoder = to_tvp5150(sd);
- *std_id = decoder->lock ? tvp5150_read_std(sd) : V4L2_STD_UNKNOWN;
+ *std_id = query_lock(sd) ? tvp5150_read_std(sd) : V4L2_STD_UNKNOWN;
return 0;
}
@@ -1247,7 +1260,7 @@ static int tvp5150_s_stream(struct v4l2_subdev *sd, int
enable)
tvp5150_enable(sd);
/* Enable outputs if decoder is locked */
- val = decoder->lock ? decoder->oe : 0;
+ val = query_lock(sd) ? decoder->oe : 0;
int_val = TVP5150_INT_A_LOCK;
v4l2_subdev_notify_event(>sd, _ev_fmt);
}
@@ -1816,8 +1829,6 @@ static int tvp5150_probe(struct i2c_client *c,
IRQF_ONESHOT, "tvp5150", core);
if (res)
return res;
- } else {
- core->lock = true;
}
res = v4l2_async_register_subdev(sd);
[PATCH] media: coda: don't overwrite h.264 profile_idc on decoder instance
On a decoder instance, after the profile has been parsed from the stream __v4l2_ctrl_s_ctrl() is called to notify userspace about changes in the read-only profile control. This ends up calling back into the CODA driver where a mssing check on the s_ctrl caused the profile information that has just been parsed from the stream to be overwritten with the default baseline profile. Later on the driver fails to enable frame reordering, based on the wrong profile information. Fixes: 347de126d1da (media: coda: add read-only h.264 decoder profile/level controls) Signed-off-by: Lucas Stach --- drivers/media/platform/coda/coda-common.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c index c7631e117dd3..1ae15d4ec5ed 100644 --- a/drivers/media/platform/coda/coda-common.c +++ b/drivers/media/platform/coda/coda-common.c @@ -1719,7 +1719,8 @@ static int coda_s_ctrl(struct v4l2_ctrl *ctrl) break; case V4L2_CID_MPEG_VIDEO_H264_PROFILE: /* TODO: switch between baseline and constrained baseline */ - ctx->params.h264_profile_idc = 66; + if (ctx->inst_type == CODA_INST_ENCODER) + ctx->params.h264_profile_idc = 66; break; case V4L2_CID_MPEG_VIDEO_H264_LEVEL: /* nothing to do, this is set by the encoder */ -- 2.18.0
Re: [PATCH 18/22] partial revert of "[media] tvp5150: add HW input connectors support"
Em Wed, 1 Aug 2018 14:10:47 +0200
Marco Felsch escreveu:
> Hi Mauro,
>
> On 18-07-31 16:56, Mauro Carvalho Chehab wrote:
> > Em Tue, 31 Jul 2018 15:30:56 +0200
> > Marco Felsch escreveu:
> >
> > > Hi Javier,
> > >
> > > On 18-07-31 14:52, Javier Martinez Canillas wrote:
> > > > Hi Marco,
> > > >
> > > > On 07/31/2018 02:36 PM, Marco Felsch wrote:
> > > >
> > > > [snip]
> > > >
> > > > >>
> > > > >> Yes, another thing that patch 19/22 should take into account is DTs
> > > > >> that
> > > > >> don't have input connectors defined. So probably TVP5150_PORT_YOUT
> > > > >> should
> > > > >> be 0 instead of TVP5150_PORT_NUM - 1 as is the case in the current
> > > > >> patch.
> > > > >>
> > > > >> In other words, it should work both when input connectors are
> > > > >> defined in
> > > > >> the DT and when these are not defined and only an output port is
> > > > >> defined.
> > > > >
> > > > > Yes, it would be a approach to map the output port dynamicaly to the
> > > > > highest port number. I tried to keep things easy by a static mapping.
> > > > > Maybe a follow up patch can change this behaviour.
> > > > >
> > > > > Anyway, input connectors aren't required. There must be at least one
> > > > > port child node with a correct port-number in the DT.
> > > > >
> > > >
> > > > Yes, that was my point. But your patch uses the port child reg property
> > > > as
> > > > the index for the struct device_node *endpoints[TVP5150_PORT_NUM] array.
> > > >
> > > > If there's only one port child (for the output) then the DT binding says
> > > > that the reg property isn't required, so this will be 0 and your patch
> > > > will
> > > > wrongly map it to TVP5150_PORT_AIP1A. That's why I said that the output
> > > > port
> > > > should be the first one in your enum tvp5150_ports and not the last
> > > > one.
> > >
> > > Yes, now I got you. I implemted this in such a way in my first apporach.
> > > But at the moment I don't know why I changed this. Maybe to keep the
> > > decoder->input number in sync with the em28xx devices, which will set the
> > > port by the s_routing() callback.
> > >
> > > Let me check this.
>
> I will prepare a follow up patch wich fix this behaviour, if possible.
>
> >
> > Anyway, with the patchset I sent (with one fix), it will do the right
> > thing with regards to the pad output:
> > https://git.linuxtv.org/mchehab/experimental.git/log/?h=tvp5150
>
> Thanks for your work :)
> I have just one question. Is it correct to set the .sig_type only for the
> tvp5150 'main' entity or should it be set for the dynamical connector
> entities too?
It should be needed only for entities with multiple pads, when
the pad index is not enough to uniquely identify what's there at
the pad. I don't think this applies to typical connectors
(although it might make sense on HDMI, where it may contain
different signals besides video, like CEC and ethernet).
>
> >
> > $ mc_nextgen_test -D
> > digraph board {
> > rankdir=TB
> > colorscheme=x11
> > labelloc="t"
> > label="Grabster AV 350
> > driver:em28xx, bus: usb-:00:14.0-2
> > "
> > intf_devnode_7 [label="intf_devnode_7\nvideo\n/dev/video0", shape=box,
> > style=filled, fillcolor=yellow]
> > intf_devnode_10 [label="intf_devnode_10\nvbi\n/dev/vbi0", shape=box,
> > style=filled, fillcolor=yellow]
> > entity_1 [label="{{ 0 | 1 | 2} | entity_1\nATV
> > decoder\ntvp5150 0-005c | { 3}}", shape=Mrecord, style=filled,
> > fillcolor=lightblue]
> > entity_6 [label="{{ 0} | entity_6\nV4L I/O\n2-2:1.0 video}",
> > shape=Mrecord, style=filled, fillcolor=aquamarine]
> > entity_9 [label="{{ 0} | entity_9\nVBI I/O\n2-2:1.0 vbi}",
> > shape=Mrecord, style=filled, fillcolor=aquamarine]
> > entity_14 [label="{entity_14\nunknown entity type\nComposite |
> > { 0}}", shape=Mrecord, style=filled, fillcolor=cadetblue]
> > entity_16 [label="{entity_16\nunknown entity type\nS-Video | {
> > 0}}", shape=Mrecord, style=filled, fillcolor=cadetblue]
> > intf_devnode_7 -> entity_6 [dir="none" color="orange"]
> > intf_devnode_10 -> entity_9 [dir="none" color="orange"]
> > entity_1:pad_5 -> entity_6:pad_12 [color=blue]
> > entity_1:pad_5 -> entity_9:pad_13 [color=blue]
> > entity_14:pad_15 -> entity_1:pad_2 [color=blue]
> > entity_16:pad_17 -> entity_1:pad_2 [color=blue style="dashed"]
> > }
> >
> > It won't do the right thing with regards to the input, though, as
> > the code at v4l2-mc.c expects just one input. So, both composite and
> > S-Video connectors (created outside tvp5150, based on the input entries
> > at em28xx cards table) are linked to pad 0.
>
> Should we add comment for this behaviour in v4l2-mc.c? Since the
> MEDIA_ENT_F_CONN_RF case updates the pad number.
I don't think so... The stuff at v4l2-mc are there to help setting the
pipelines for devnode-based devices that also exposes their internal
wiring via MC. For those, it is up to the Kernel to
Re: [PATCH 16/22] [media] tvp5150: add querystd
Hi Mauro,
On 18-07-30 15:09, Mauro Carvalho Chehab wrote:
> Em Thu, 28 Jun 2018 18:20:48 +0200
> Marco Felsch escreveu:
>
> > From: Philipp Zabel
> >
> > Add the querystd video_op and make it return V4L2_STD_UNKNOWN while the
> > TVP5150 is not locked to a signal.
> >
> > Signed-off-by: Philipp Zabel
> > Signed-off-by: Marco Felsch
> > ---
> > drivers/media/i2c/tvp5150.c | 10 ++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> > index 99d887936ea0..1990aaa17749 100644
> > --- a/drivers/media/i2c/tvp5150.c
> > +++ b/drivers/media/i2c/tvp5150.c
> > @@ -796,6 +796,15 @@ static v4l2_std_id tvp5150_read_std(struct v4l2_subdev
> > *sd)
> > }
> > }
> >
> > +static int tvp5150_querystd(struct v4l2_subdev *sd, v4l2_std_id *std_id)
> > +{
> > + struct tvp5150 *decoder = to_tvp5150(sd);
> > +
> > + *std_id = decoder->lock ? tvp5150_read_std(sd) : V4L2_STD_UNKNOWN;
>
> This patch requires rework. What happens when a device doesn't have
> IRQ enabled? Perhaps it should, instead, read some register in order
> to check for the locking status, as this would work on both cases.
If IRQ isn't enabled, decoder->lock is set to always true during
probe(). So this case should be fine.
Regards,
Marco
> > +
> > + return 0;
> > +}
> > +
> > static const struct v4l2_event tvp5150_ev_fmt = {
> > .type = V4L2_EVENT_SOURCE_CHANGE,
> > .u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION,
> > @@ -1408,6 +1417,7 @@ static const struct v4l2_subdev_tuner_ops
> > tvp5150_tuner_ops = {
> >
> > static const struct v4l2_subdev_video_ops tvp5150_video_ops = {
> > .s_std = tvp5150_s_std,
> > + .querystd = tvp5150_querystd,
> > .s_stream = tvp5150_s_stream,
> > .s_routing = tvp5150_s_routing,
> > .g_mbus_config = tvp5150_g_mbus_config,
>
>
>
> Thanks,
> Mauro
>
Re: [PATCH 18/22] partial revert of "[media] tvp5150: add HW input connectors support"
Hi Mauro,
On 18-07-31 16:56, Mauro Carvalho Chehab wrote:
> Em Tue, 31 Jul 2018 15:30:56 +0200
> Marco Felsch escreveu:
>
> > Hi Javier,
> >
> > On 18-07-31 14:52, Javier Martinez Canillas wrote:
> > > Hi Marco,
> > >
> > > On 07/31/2018 02:36 PM, Marco Felsch wrote:
> > >
> > > [snip]
> > >
> > > >>
> > > >> Yes, another thing that patch 19/22 should take into account is DTs
> > > >> that
> > > >> don't have input connectors defined. So probably TVP5150_PORT_YOUT
> > > >> should
> > > >> be 0 instead of TVP5150_PORT_NUM - 1 as is the case in the current
> > > >> patch.
> > > >>
> > > >> In other words, it should work both when input connectors are defined
> > > >> in
> > > >> the DT and when these are not defined and only an output port is
> > > >> defined.
> > > >
> > > > Yes, it would be a approach to map the output port dynamicaly to the
> > > > highest port number. I tried to keep things easy by a static mapping.
> > > > Maybe a follow up patch can change this behaviour.
> > > >
> > > > Anyway, input connectors aren't required. There must be at least one
> > > > port child node with a correct port-number in the DT.
> > > >
> > >
> > > Yes, that was my point. But your patch uses the port child reg property as
> > > the index for the struct device_node *endpoints[TVP5150_PORT_NUM] array.
> > >
> > > If there's only one port child (for the output) then the DT binding says
> > > that the reg property isn't required, so this will be 0 and your patch
> > > will
> > > wrongly map it to TVP5150_PORT_AIP1A. That's why I said that the output
> > > port
> > > should be the first one in your enum tvp5150_ports and not the last one.
> >
> > Yes, now I got you. I implemted this in such a way in my first apporach.
> > But at the moment I don't know why I changed this. Maybe to keep the
> > decoder->input number in sync with the em28xx devices, which will set the
> > port by the s_routing() callback.
> >
> > Let me check this.
I will prepare a follow up patch wich fix this behaviour, if possible.
>
> Anyway, with the patchset I sent (with one fix), it will do the right
> thing with regards to the pad output:
> https://git.linuxtv.org/mchehab/experimental.git/log/?h=tvp5150
Thanks for your work :)
I have just one question. Is it correct to set the .sig_type only for the
tvp5150 'main' entity or should it be set for the dynamical connector
entities too?
>
> $ mc_nextgen_test -D
> digraph board {
> rankdir=TB
> colorscheme=x11
> labelloc="t"
> label="Grabster AV 350
> driver:em28xx, bus: usb-:00:14.0-2
> "
> intf_devnode_7 [label="intf_devnode_7\nvideo\n/dev/video0", shape=box,
> style=filled, fillcolor=yellow]
> intf_devnode_10 [label="intf_devnode_10\nvbi\n/dev/vbi0", shape=box,
> style=filled, fillcolor=yellow]
> entity_1 [label="{{ 0 | 1 | 2} | entity_1\nATV
> decoder\ntvp5150 0-005c | { 3}}", shape=Mrecord, style=filled,
> fillcolor=lightblue]
> entity_6 [label="{{ 0} | entity_6\nV4L I/O\n2-2:1.0 video}",
> shape=Mrecord, style=filled, fillcolor=aquamarine]
> entity_9 [label="{{ 0} | entity_9\nVBI I/O\n2-2:1.0 vbi}",
> shape=Mrecord, style=filled, fillcolor=aquamarine]
> entity_14 [label="{entity_14\nunknown entity type\nComposite |
> { 0}}", shape=Mrecord, style=filled, fillcolor=cadetblue]
> entity_16 [label="{entity_16\nunknown entity type\nS-Video | {
> 0}}", shape=Mrecord, style=filled, fillcolor=cadetblue]
> intf_devnode_7 -> entity_6 [dir="none" color="orange"]
> intf_devnode_10 -> entity_9 [dir="none" color="orange"]
> entity_1:pad_5 -> entity_6:pad_12 [color=blue]
> entity_1:pad_5 -> entity_9:pad_13 [color=blue]
> entity_14:pad_15 -> entity_1:pad_2 [color=blue]
> entity_16:pad_17 -> entity_1:pad_2 [color=blue style="dashed"]
> }
>
> It won't do the right thing with regards to the input, though, as
> the code at v4l2-mc.c expects just one input. So, both composite and
> S-Video connectors (created outside tvp5150, based on the input entries
> at em28xx cards table) are linked to pad 0.
Should we add comment for this behaviour in v4l2-mc.c? Since the
MEDIA_ENT_F_CONN_RF case updates the pad number.
>
> Thanks,
> Mauro
>
Regards,
Marco
Re: [PATCH 05/21] dt-bindings: media: Specify bus type for MIPI D-PHY, others, explicitly
Hi Rob, Thanks for the review. On Tue, Jul 31, 2018 at 03:32:10PM -0600, Rob Herring wrote: > On Mon, Jul 23, 2018 at 04:46:50PM +0300, Sakari Ailus wrote: > > Allow specifying the bus type explicitly for MIPI D-PHY, parallel and > > Bt.656 busses. This is useful for devices that can make use of different > > bus types. There are CSI-2 transmitters and receivers but the PHY > > selection needs to be made between C-PHY and D-PHY; many devices also > > support parallel and Bt.656 interfaces but the means to pass that > > information to software wasn't there. > > > > Autodetection (value 0) is removed as an option as the property could be > > simply omitted in that case. > > Presumably there are users, so you can't remove it. But documenting > behavior when absent would be good. Well, it's effectively the same as having no such property at all: the type is not specified. Generally there are two possibilities: the hardware supports just a single bus or it supports more than one. If there's just one, the type can be known by the driver. In that case there's no use for autodetection. The second case is a bit more complicated: the bus type detection is solely based on properties available in the endpoint, and I think that may have been feasible approach when there were just parallel and Bt.656 busses that were supported, but with the additional busses, the V4L2 fwnode framework may no longer guess the bus in any meaningful way from the available properties. I'd think the only known-good option here is to specify the type explicitly in that case: there's no room for guessing. (This patchset makes it possible for drivers to explicitly define the bus type, but the autodetection support is maintained for backwards compatibility.) One of the existing issues is that there are combined parallel/Bt.656 receivers that need to know the type of the bus. This is based on the existence parallel interface only properties: if any of these exist, then the interface is parallel, otherwise it is Bt.656. The DT bindings for the same devices also define the defaults for the parallel interface. This leaves the end result ambiguous: is it the parallel interface with the default configuration or is it Bt.656? There will likely be similar issues for CSI-2 D-PHY and CSI-2 C-PHY. The question there would be: is this CSI-2 C-PHY or CSI-2 D-PHY with default clock lane configuration? In either case the autodetection option for the bus type provides no useful information. If it exists in DT source, that's fine, there's just no use for it. Let me know if you still think it should be maintained in binding documentation. > > > > > Signed-off-by: Sakari Ailus > > --- > > Documentation/devicetree/bindings/media/video-interfaces.txt | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt > > b/Documentation/devicetree/bindings/media/video-interfaces.txt > > index baf9d9756b3c..f884ada0bffc 100644 > > --- a/Documentation/devicetree/bindings/media/video-interfaces.txt > > +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt > > @@ -100,10 +100,12 @@ Optional endpoint properties > >slave device (data source) by the master device (data sink). In the > > master > >mode the data source device is also the source of the synchronization > > signals. > > - bus-type: data bus type. Possible values are: > > - 0 - autodetect based on other properties (MIPI CSI-2 D-PHY, parallel or > > Bt656) > >1 - MIPI CSI-2 C-PHY > >2 - MIPI CSI1 > >3 - CCP2 > > + 4 - MIPI CSI-2 D-PHY > > + 5 - Parallel > > Is that really specific enough to be useful? Yes; see above. > > > + 6 - Bt.656 > > - bus-width: number of data lines actively used, valid for the parallel > > busses. > > - data-shift: on the parallel data busses, if bus-width is used to specify > > the > >number of data lines, data-shift can be used to specify which data lines > > are -- Kind regards, Sakari Ailus sakari.ai...@linux.intel.com
[GIT PULL FOR v4.18 or v4.19] Qualcomm Camera Subsystem driver - 8x96 support
Hi Mauro,
This pull request adds camera support for Qualcomm's 8x96.
Since 4.18 is delayed by another week (see lwn.net) I am hoping this can still
be applied for 4.18. If not, then it can go to 4.19.
Regards,
Hans
The following changes since commit 1d06352e18ef502e30837cedfe618298816fb48c:
media: tvp5150: add g_std callback (2018-07-30 20:04:33 -0400)
are available in the git repository at:
git://linuxtv.org/hverkuil/media_tree.git camss
for you to fetch changes up to d0151162ad654aa2d8be51f6cf08f108f976:
media: camss: csid: Add support for events triggered by user controls
(2018-08-01 12:30:01 +0200)
Sakari Ailus (1):
doc-rst: Add packed Bayer raw14 pixel formats
Todor Tomov (33):
media: v4l: Add new 2X8 10-bit grayscale media bus code
media: v4l: Add new 10-bit packed grayscale format
media: Rename CAMSS driver path
media: camss: Use SPDX license headers
media: camss: Fix OF node usage
media: camss: csiphy: Ensure clock mux config is done before the rest
media: dt-bindings: media: qcom, camss: Unify the clock names
media: camss: Unify the clock names
media: camss: csiphy: Update settle count calculation
media: camss: csid: Configure data type and decode format properly
media: camss: vfe: Fix to_vfe() macro member name
media: camss: vfe: Get line pointer as container of video_out
media: camss: vfe: Do not disable CAMIF when clearing its status
media: dt-bindings: media: qcom,camss: Fix whitespaces
media: dt-bindings: media: qcom,camss: Add 8996 bindings
media: camss: Add 8x96 resources
media: camss: Add basic runtime PM support
media: camss: csiphy: Split to hardware dependent and independent parts
media: camss: csiphy: Unify lane handling
media: camss: csiphy: Add support for 8x96
media: camss: csid: Add support for 8x96
media: camss: ispif: Add support for 8x96
media: camss: vfe: Split to hardware dependent and independent parts
media: camss: vfe: Add support for 8x96
media: camss: Format configuration per hardware version
media: camss: vfe: Different format support on source pad
media: camss: vfe: Add support for UYVY output from VFE on 8x96
media: camss: csid: Different format support on source pad
media: camss: csid: MIPI10 to Plain16 format conversion
media: camss: Add support for RAW MIPI14 on 8x96
media: camss: Add support for 10-bit grayscale formats
media: doc: media/v4l-drivers: Update Qualcomm CAMSS driver document for
8x96
media: camss: csid: Add support for events triggered by user controls
Documentation/devicetree/bindings/media/qcom,camss.txt | 128 ++--
Documentation/media/uapi/v4l/pixfmt-rgb.rst |1 +
Documentation/media/uapi/v4l/pixfmt-srggb14p.rst | 127
Documentation/media/uapi/v4l/pixfmt-y10p.rst | 33 +
Documentation/media/uapi/v4l/subdev-formats.rst | 72 ++
Documentation/media/uapi/v4l/yuv-formats.rst |1 +
Documentation/media/v4l-drivers/qcom_camss.rst | 93 ++-
Documentation/media/v4l-drivers/qcom_camss_8x96_graph.dot| 104 +++
MAINTAINERS |2 +-
drivers/media/platform/Kconfig |2 +-
drivers/media/platform/Makefile |2 +-
drivers/media/platform/qcom/camss-8x16/camss-vfe.h | 123
drivers/media/platform/qcom/{camss-8x16 => camss}/Makefile |4 +
drivers/media/platform/qcom/{camss-8x16 => camss}/camss-csid.c | 471
++---
drivers/media/platform/qcom/{camss-8x16 => camss}/camss-csid.h | 17 +-
drivers/media/platform/qcom/camss/camss-csiphy-2ph-1-0.c | 176 +
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c | 256 +++
drivers/media/platform/qcom/{camss-8x16 => camss}/camss-csiphy.c | 363
--
drivers/media/platform/qcom/{camss-8x16 => camss}/camss-csiphy.h | 37 +-
drivers/media/platform/qcom/{camss-8x16 => camss}/camss-ispif.c | 264 ++-
drivers/media/platform/qcom/{camss-8x16 => camss}/camss-ispif.h | 23 +-
drivers/media/platform/qcom/camss/camss-vfe-4-1.c| 1018
+++
drivers/media/platform/qcom/camss/camss-vfe-4-7.c| 1140
++
drivers/media/platform/qcom/{camss-8x16 => camss}/camss-vfe.c| 1569
+++---
drivers/media/platform/qcom/camss/camss-vfe.h| 186 +
drivers/media/platform/qcom/{camss-8x16 => camss}/camss-video.c | 133 +++-
drivers/media/platform/qcom/{camss-8x16 => camss}/camss-video.h | 12 +-
drivers/media/platform/qcom/{camss-8x16 =>
[PATCH -next] media: tvp5150: make function tvp5150_volatile_reg() static
Fixes the following sparse warning:
drivers/media/i2c/tvp5150.c:1457:6: warning:
symbol 'tvp5150_volatile_reg' was not declared. Should it be static?
Signed-off-by: Wei Yongjun
---
drivers/media/i2c/tvp5150.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 7b0d42b..ad3b728 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -1454,7 +1454,7 @@ static int tvp5150_registered(struct v4l2_subdev *sd)
},
};
-bool tvp5150_volatile_reg(struct device *dev, unsigned int reg)
+static bool tvp5150_volatile_reg(struct device *dev, unsigned int reg)
{
switch (reg) {
case TVP5150_VERT_LN_COUNT_MSB:
BUG ? v4l2_spi_subdev_init does not produce unique names
Hello v4l2 gurus, Sorry for the people who already have read my previous mail. I changed the subject to make it more sexy :) Documentation/media/kapi/v4l2-subdev.rst states : "Afterwards you need to initialize :c:type:`sd `->name with a unique name and set the module owner. This is done for you if you use the i2c helper functions" I try to write a v4l2 spi driver and use hence v4l2_spi_subdev_init, not v4l2_i2c_subdev_init. In v4l2_i2c_subdev_init, subdev name is initialised by snprintf(sd->name, sizeof(sd->name), "%s %d-%04x", client->dev.driver->name, i2c_adapter_id(client->adapter), client->addr); In v4l2_spi_subdev_init, subdev name is initialised by strlcpy(sd->name, spi->dev.driver->name, sizeof(sd->name)); This does not give similar results :( with i2c, subdev name is set as "xxx %d-%04x", giving a unique name to the subdev. with spi, subdev name is set as "xxx", giving the same name to all similar subdevs on the same host Is that intentional or an oversight, and if so, how should that be fixed ? Best regards Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
