* [PATCH v13 1/8] media: v4l2: Add ignore_cap_streaming flag
2023-10-12 11:00 [PATCH v13 0/8] Wave5 codec driver Sebastian Fricke
@ 2023-10-12 11:00 ` Sebastian Fricke
2023-10-12 11:01 ` [PATCH v13 2/8] media: v4l2: Allow M2M job queuing w/o streaming CAP queue Sebastian Fricke
` (6 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: Sebastian Fricke @ 2023-10-12 11:00 UTC (permalink / raw)
To: Krzysztof Kozlowski, NXP Linux Team, Conor Dooley,
Mauro Carvalho Chehab, Jackson Lee, Hans Verkuil, Sascha Hauer,
Rob Herring, Pengutronix Kernel Team, Shawn Guo, Philipp Zabel,
Nas Chung, Fabio Estevam
Cc: linux-media, Tomasz Figa, linux-kernel, Sebastian Fricke,
Nicolas Dufresne, kernel, Robert Beckett, devicetree,
linux-arm-kernel, Darren Etheridge
Add a new flag to 'struct v4l2_m2m_ctx' to toggle whether a CAPTURE queue
must be streaming in order to allow queuing OUTPUT jobs to the ready
queue. Currently, both queues (CAPTURE & OUTPUT) must be streaming in
order to add new jobs. This prevents firmware from analyzing the bitstream
header which arrives on the OUTPUT queue and performing an analysis of the
sequence to ensure that userspace prepares the CAPTURE queue correctly.
Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com>
---
include/media/v4l2-mem2mem.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
index d6c8eb2b5201..1288fe364fab 100644
--- a/include/media/v4l2-mem2mem.h
+++ b/include/media/v4l2-mem2mem.h
@@ -84,6 +84,12 @@ struct v4l2_m2m_queue_ctx {
* @last_src_buf: indicate the last source buffer for draining
* @next_buf_last: next capture queud buffer will be tagged as last
* @has_stopped: indicate the device has been stopped
+ * @ignore_cap_streaming: If true, job_ready can be called even if the CAPTURE
+ * queue is not streaming. This allows firmware to
+ * analyze the bitstream header which arrives on the
+ * OUTPUT queue. The driver must implement the job_ready
+ * callback correctly to make sure that the requirements
+ * for actual decoding are met.
* @m2m_dev: opaque pointer to the internal data to handle M2M context
* @cap_q_ctx: Capture (output to memory) queue context
* @out_q_ctx: Output (input from memory) queue context
@@ -106,6 +112,7 @@ struct v4l2_m2m_ctx {
struct vb2_v4l2_buffer *last_src_buf;
bool next_buf_last;
bool has_stopped;
+ bool ignore_cap_streaming;
/* internal use only */
struct v4l2_m2m_dev *m2m_dev;
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v13 2/8] media: v4l2: Allow M2M job queuing w/o streaming CAP queue
2023-10-12 11:00 [PATCH v13 0/8] Wave5 codec driver Sebastian Fricke
2023-10-12 11:00 ` [PATCH v13 1/8] media: v4l2: Add ignore_cap_streaming flag Sebastian Fricke
@ 2023-10-12 11:01 ` Sebastian Fricke
2023-10-12 11:01 ` [PATCH v13 3/8] media: platform: chips-media: Move Coda to separate folder Sebastian Fricke
` (5 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: Sebastian Fricke @ 2023-10-12 11:01 UTC (permalink / raw)
To: Krzysztof Kozlowski, NXP Linux Team, Conor Dooley,
Mauro Carvalho Chehab, Jackson Lee, Hans Verkuil, Sascha Hauer,
Rob Herring, Pengutronix Kernel Team, Shawn Guo, Philipp Zabel,
Nas Chung, Fabio Estevam
Cc: linux-media, Tomasz Figa, linux-kernel, Sebastian Fricke,
Nicolas Dufresne, kernel, Robert Beckett, devicetree,
linux-arm-kernel, Darren Etheridge
Allow decoder drivers to set the ignore_cap_streaming flag to allow
queuing jobs to the M2M ready queue and perform firmware sequence analysis
with just a streaming OUTPUT queue and available bitstream data.
Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com>
---
drivers/media/v4l2-core/v4l2-mem2mem.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
index 0cc30397fbad..9e983176542b 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -301,9 +301,12 @@ static void __v4l2_m2m_try_queue(struct v4l2_m2m_dev *m2m_dev,
dprintk("Trying to schedule a job for m2m_ctx: %p\n", m2m_ctx);
- if (!m2m_ctx->out_q_ctx.q.streaming
- || !m2m_ctx->cap_q_ctx.q.streaming) {
- dprintk("Streaming needs to be on for both queues\n");
+ if (!m2m_ctx->out_q_ctx.q.streaming ||
+ (!m2m_ctx->cap_q_ctx.q.streaming && !m2m_ctx->ignore_cap_streaming)) {
+ if (!m2m_ctx->ignore_cap_streaming)
+ dprintk("Streaming needs to be on for both queues\n");
+ else
+ dprintk("Streaming needs to be on for the OUTPUT queue\n");
return;
}
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v13 3/8] media: platform: chips-media: Move Coda to separate folder
2023-10-12 11:00 [PATCH v13 0/8] Wave5 codec driver Sebastian Fricke
2023-10-12 11:00 ` [PATCH v13 1/8] media: v4l2: Add ignore_cap_streaming flag Sebastian Fricke
2023-10-12 11:01 ` [PATCH v13 2/8] media: v4l2: Allow M2M job queuing w/o streaming CAP queue Sebastian Fricke
@ 2023-10-12 11:01 ` Sebastian Fricke
2023-10-12 11:01 ` [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support Sebastian Fricke
` (4 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: Sebastian Fricke @ 2023-10-12 11:01 UTC (permalink / raw)
To: Krzysztof Kozlowski, NXP Linux Team, Conor Dooley,
Mauro Carvalho Chehab, Jackson Lee, Hans Verkuil, Sascha Hauer,
Rob Herring, Pengutronix Kernel Team, Shawn Guo, Philipp Zabel,
Nas Chung, Fabio Estevam
Cc: linux-media, Tomasz Figa, linux-kernel, Sebastian Fricke,
Nicolas Dufresne, kernel, Robert Beckett, devicetree,
linux-arm-kernel, Darren Etheridge
Prepare the folder structure for a second Chips&Media driver.
Move the Coda driver to a sub-directory.
Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>
---
MAINTAINERS | 2 +-
drivers/media/platform/chips-media/Kconfig | 17 +----------------
drivers/media/platform/chips-media/Makefile | 5 +----
drivers/media/platform/chips-media/coda/Kconfig | 18 ++++++++++++++++++
drivers/media/platform/chips-media/coda/Makefile | 6 ++++++
.../media/platform/chips-media/{ => coda}/coda-bit.c | 0
.../platform/chips-media/{ => coda}/coda-common.c | 0
.../media/platform/chips-media/{ => coda}/coda-gdi.c | 0
.../media/platform/chips-media/{ => coda}/coda-h264.c | 0
.../media/platform/chips-media/{ => coda}/coda-jpeg.c | 0
.../media/platform/chips-media/{ => coda}/coda-mpeg2.c | 0
.../media/platform/chips-media/{ => coda}/coda-mpeg4.c | 0
drivers/media/platform/chips-media/{ => coda}/coda.h | 0
.../media/platform/chips-media/{ => coda}/coda_regs.h | 0
.../media/platform/chips-media/{ => coda}/imx-vdoa.c | 0
.../media/platform/chips-media/{ => coda}/imx-vdoa.h | 0
drivers/media/platform/chips-media/{ => coda}/trace.h | 2 +-
17 files changed, 28 insertions(+), 22 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 90f13281d297..063a11791bbf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5093,7 +5093,7 @@ M: Philipp Zabel <p.zabel@pengutronix.de>
L: linux-media@vger.kernel.org
S: Maintained
F: Documentation/devicetree/bindings/media/coda.yaml
-F: drivers/media/platform/chips-media/
+F: drivers/media/platform/chips-media/coda
CODE OF CONDUCT
M: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
diff --git a/drivers/media/platform/chips-media/Kconfig b/drivers/media/platform/chips-media/Kconfig
index 57f8f8a22df8..f87a0d693df7 100644
--- a/drivers/media/platform/chips-media/Kconfig
+++ b/drivers/media/platform/chips-media/Kconfig
@@ -2,19 +2,4 @@
comment "Chips&Media media platform drivers"
-config VIDEO_CODA
- tristate "Chips&Media Coda multi-standard codec IP"
- depends on V4L_MEM2MEM_DRIVERS
- depends on VIDEO_DEV && OF && (ARCH_MXC || COMPILE_TEST)
- select SRAM
- select VIDEOBUF2_DMA_CONTIG
- select VIDEOBUF2_VMALLOC
- select V4L2_JPEG_HELPER
- select V4L2_MEM2MEM_DEV
- select GENERIC_ALLOCATOR
- help
- Coda is a range of video codec IPs that supports
- H.264, MPEG-4, and other video formats.
-
-config VIDEO_IMX_VDOA
- def_tristate VIDEO_CODA if SOC_IMX6Q || COMPILE_TEST
+source "drivers/media/platform/chips-media/coda/Kconfig"
diff --git a/drivers/media/platform/chips-media/Makefile b/drivers/media/platform/chips-media/Makefile
index bbb16425a875..5ee693f651c1 100644
--- a/drivers/media/platform/chips-media/Makefile
+++ b/drivers/media/platform/chips-media/Makefile
@@ -1,6 +1,3 @@
# SPDX-License-Identifier: GPL-2.0-only
-coda-vpu-objs := coda-common.o coda-bit.o coda-gdi.o coda-h264.o coda-mpeg2.o coda-mpeg4.o coda-jpeg.o
-
-obj-$(CONFIG_VIDEO_CODA) += coda-vpu.o
-obj-$(CONFIG_VIDEO_IMX_VDOA) += imx-vdoa.o
+obj-y += coda/
diff --git a/drivers/media/platform/chips-media/coda/Kconfig b/drivers/media/platform/chips-media/coda/Kconfig
new file mode 100644
index 000000000000..cb7b66c71380
--- /dev/null
+++ b/drivers/media/platform/chips-media/coda/Kconfig
@@ -0,0 +1,18 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+config VIDEO_CODA
+ tristate "Chips&Media Coda multi-standard codec IP"
+ depends on V4L_MEM2MEM_DRIVERS
+ depends on VIDEO_DEV && OF && (ARCH_MXC || COMPILE_TEST)
+ select SRAM
+ select VIDEOBUF2_DMA_CONTIG
+ select VIDEOBUF2_VMALLOC
+ select V4L2_JPEG_HELPER
+ select V4L2_MEM2MEM_DEV
+ select GENERIC_ALLOCATOR
+ help
+ Coda is a range of video codec IPs that supports
+ H.264, MPEG-4, and other video formats.
+
+config VIDEO_IMX_VDOA
+ def_tristate VIDEO_CODA if SOC_IMX6Q || COMPILE_TEST
diff --git a/drivers/media/platform/chips-media/coda/Makefile b/drivers/media/platform/chips-media/coda/Makefile
new file mode 100644
index 000000000000..bbb16425a875
--- /dev/null
+++ b/drivers/media/platform/chips-media/coda/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+coda-vpu-objs := coda-common.o coda-bit.o coda-gdi.o coda-h264.o coda-mpeg2.o coda-mpeg4.o coda-jpeg.o
+
+obj-$(CONFIG_VIDEO_CODA) += coda-vpu.o
+obj-$(CONFIG_VIDEO_IMX_VDOA) += imx-vdoa.o
diff --git a/drivers/media/platform/chips-media/coda-bit.c b/drivers/media/platform/chips-media/coda/coda-bit.c
similarity index 100%
rename from drivers/media/platform/chips-media/coda-bit.c
rename to drivers/media/platform/chips-media/coda/coda-bit.c
diff --git a/drivers/media/platform/chips-media/coda-common.c b/drivers/media/platform/chips-media/coda/coda-common.c
similarity index 100%
rename from drivers/media/platform/chips-media/coda-common.c
rename to drivers/media/platform/chips-media/coda/coda-common.c
diff --git a/drivers/media/platform/chips-media/coda-gdi.c b/drivers/media/platform/chips-media/coda/coda-gdi.c
similarity index 100%
rename from drivers/media/platform/chips-media/coda-gdi.c
rename to drivers/media/platform/chips-media/coda/coda-gdi.c
diff --git a/drivers/media/platform/chips-media/coda-h264.c b/drivers/media/platform/chips-media/coda/coda-h264.c
similarity index 100%
rename from drivers/media/platform/chips-media/coda-h264.c
rename to drivers/media/platform/chips-media/coda/coda-h264.c
diff --git a/drivers/media/platform/chips-media/coda-jpeg.c b/drivers/media/platform/chips-media/coda/coda-jpeg.c
similarity index 100%
rename from drivers/media/platform/chips-media/coda-jpeg.c
rename to drivers/media/platform/chips-media/coda/coda-jpeg.c
diff --git a/drivers/media/platform/chips-media/coda-mpeg2.c b/drivers/media/platform/chips-media/coda/coda-mpeg2.c
similarity index 100%
rename from drivers/media/platform/chips-media/coda-mpeg2.c
rename to drivers/media/platform/chips-media/coda/coda-mpeg2.c
diff --git a/drivers/media/platform/chips-media/coda-mpeg4.c b/drivers/media/platform/chips-media/coda/coda-mpeg4.c
similarity index 100%
rename from drivers/media/platform/chips-media/coda-mpeg4.c
rename to drivers/media/platform/chips-media/coda/coda-mpeg4.c
diff --git a/drivers/media/platform/chips-media/coda.h b/drivers/media/platform/chips-media/coda/coda.h
similarity index 100%
rename from drivers/media/platform/chips-media/coda.h
rename to drivers/media/platform/chips-media/coda/coda.h
diff --git a/drivers/media/platform/chips-media/coda_regs.h b/drivers/media/platform/chips-media/coda/coda_regs.h
similarity index 100%
rename from drivers/media/platform/chips-media/coda_regs.h
rename to drivers/media/platform/chips-media/coda/coda_regs.h
diff --git a/drivers/media/platform/chips-media/imx-vdoa.c b/drivers/media/platform/chips-media/coda/imx-vdoa.c
similarity index 100%
rename from drivers/media/platform/chips-media/imx-vdoa.c
rename to drivers/media/platform/chips-media/coda/imx-vdoa.c
diff --git a/drivers/media/platform/chips-media/imx-vdoa.h b/drivers/media/platform/chips-media/coda/imx-vdoa.h
similarity index 100%
rename from drivers/media/platform/chips-media/imx-vdoa.h
rename to drivers/media/platform/chips-media/coda/imx-vdoa.h
diff --git a/drivers/media/platform/chips-media/trace.h b/drivers/media/platform/chips-media/coda/trace.h
similarity index 99%
rename from drivers/media/platform/chips-media/trace.h
rename to drivers/media/platform/chips-media/coda/trace.h
index 19f98e6dafb9..abc6a01a74e9 100644
--- a/drivers/media/platform/chips-media/trace.h
+++ b/drivers/media/platform/chips-media/coda/trace.h
@@ -167,7 +167,7 @@ DEFINE_EVENT(coda_buf_class, coda_jpeg_done,
#endif /* __CODA_TRACE_H__ */
#undef TRACE_INCLUDE_PATH
-#define TRACE_INCLUDE_PATH ../../drivers/media/platform/chips-media
+#define TRACE_INCLUDE_PATH ../../drivers/media/platform/chips-media/coda
#undef TRACE_INCLUDE_FILE
#define TRACE_INCLUDE_FILE trace
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support
2023-10-12 11:00 [PATCH v13 0/8] Wave5 codec driver Sebastian Fricke
` (2 preceding siblings ...)
2023-10-12 11:01 ` [PATCH v13 3/8] media: platform: chips-media: Move Coda to separate folder Sebastian Fricke
@ 2023-10-12 11:01 ` Sebastian Fricke
2023-10-12 13:24 ` Krzysztof Kozlowski
2023-10-17 13:39 ` Devarsh Thakkar
2023-10-12 11:01 ` [PATCH v13 7/8] media: chips-media: wave5: Add wave5 driver to maintainers file Sebastian Fricke
` (3 subsequent siblings)
7 siblings, 2 replies; 27+ messages in thread
From: Sebastian Fricke @ 2023-10-12 11:01 UTC (permalink / raw)
To: Krzysztof Kozlowski, NXP Linux Team, Conor Dooley,
Mauro Carvalho Chehab, Jackson Lee, Hans Verkuil, Sascha Hauer,
Rob Herring, Pengutronix Kernel Team, Shawn Guo, Philipp Zabel,
Nas Chung, Fabio Estevam
Cc: linux-media, Tomasz Figa, linux-kernel, Sebastian Fricke,
Nicolas Dufresne, kernel, Robert Beckett, devicetree,
linux-arm-kernel, Darren Etheridge
From: Robert Beckett <bob.beckett@collabora.com>
Add bindings for the chips&media wave5 codec driver
Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>
---
.../devicetree/bindings/media/cnm,wave5.yaml | 60 ++++++++++++++++++++++
1 file changed, 60 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/cnm,wave5.yaml b/Documentation/devicetree/bindings/media/cnm,wave5.yaml
new file mode 100644
index 000000000000..b31d34aec05b
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/cnm,wave5.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/cnm,wave5.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Chips&Media Wave 5 Series multi-standard codec IP
+
+maintainers:
+ - Nas Chung <nas.chung@chipsnmedia.com>
+ - Jackson Lee <jackson.lee@chipsnmedia.com>
+
+description:
+ The Chips&Media WAVE codec IP is a multi format video encoder/decoder
+
+properties:
+ compatible:
+ enum:
+ - cnm,cm521c-vpu
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: VCODEC clock
+
+ interrupts:
+ maxItems: 1
+
+ power-domains:
+ maxItems: 1
+
+ resets:
+ maxItems: 1
+
+ sram:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ The VPU uses the SRAM to store some of the reference data instead of
+ storing it on DMA memory. It is mainly used for the purpose of reducing
+ bandwidth.
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - interrupts
+
+additionalProperties: false
+
+examples:
+ - |
+ vpu: video-codec@12345678 {
+ compatible = "cnm,cm521c-vpu";
+ reg = <0x12345678 0x1000>;
+ clocks = <&clks 42>;
+ interrupts = <42>;
+ sram = <&sram>;
+ };
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support
2023-10-12 11:01 ` [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support Sebastian Fricke
@ 2023-10-12 13:24 ` Krzysztof Kozlowski
2023-10-16 13:47 ` Rob Herring
2023-10-17 13:39 ` Devarsh Thakkar
1 sibling, 1 reply; 27+ messages in thread
From: Krzysztof Kozlowski @ 2023-10-12 13:24 UTC (permalink / raw)
To: Sebastian Fricke, Krzysztof Kozlowski, NXP Linux Team,
Conor Dooley, Mauro Carvalho Chehab, Jackson Lee, Hans Verkuil,
Sascha Hauer, Rob Herring, Pengutronix Kernel Team, Shawn Guo,
Philipp Zabel, Nas Chung, Fabio Estevam
Cc: linux-media, Tomasz Figa, linux-kernel, Nicolas Dufresne, kernel,
Robert Beckett, devicetree, linux-arm-kernel, Darren Etheridge
On 12/10/2023 13:01, Sebastian Fricke wrote:
> From: Robert Beckett <bob.beckett@collabora.com>
>
> Add bindings for the chips&media wave5 codec driver
>
> Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>
> ---
> .../devicetree/bindings/media/cnm,wave5.yaml | 60 ++++++++++++++++++++++
> 1 file changed, 60 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/cnm,wave5.yaml b/Documentation/devicetree/bindings/media/cnm,wave5.yaml
> new file mode 100644
> index 000000000000..b31d34aec05b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/cnm,wave5.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/cnm,wave5.yaml#
Filename matching compatible, so: cnm,cm521c-vpu.yaml
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Chips&Media Wave 5 Series multi-standard codec IP
> +
> +maintainers:
> + - Nas Chung <nas.chung@chipsnmedia.com>
> + - Jackson Lee <jackson.lee@chipsnmedia.com>
> +
> +description:
> + The Chips&Media WAVE codec IP is a multi format video encoder/decoder
> +
> +properties:
> + compatible:
> + enum:
> + - cnm,cm521c-vpu
Can this device be anything else? Why VPU suffix?
> +
> + reg:
> + maxItems: 1
> +
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support
2023-10-12 13:24 ` Krzysztof Kozlowski
@ 2023-10-16 13:47 ` Rob Herring
2023-10-21 12:05 ` Sebastian Fricke
0 siblings, 1 reply; 27+ messages in thread
From: Rob Herring @ 2023-10-16 13:47 UTC (permalink / raw)
To: Krzysztof Kozlowski, Sebastian Fricke
Cc: Krzysztof Kozlowski, NXP Linux Team, Conor Dooley,
Mauro Carvalho Chehab, Jackson Lee, Hans Verkuil, Sascha Hauer,
Pengutronix Kernel Team, Shawn Guo, Philipp Zabel, Nas Chung,
Fabio Estevam, linux-media, Tomasz Figa, linux-kernel,
Nicolas Dufresne, kernel, Robert Beckett, devicetree,
linux-arm-kernel, Darren Etheridge
On Thu, Oct 12, 2023 at 03:24:12PM +0200, Krzysztof Kozlowski wrote:
> On 12/10/2023 13:01, Sebastian Fricke wrote:
> > From: Robert Beckett <bob.beckett@collabora.com>
> >
> > Add bindings for the chips&media wave5 codec driver
> >
> > Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
> > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> > Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>
> > ---
> > .../devicetree/bindings/media/cnm,wave5.yaml | 60 ++++++++++++++++++++++
> > 1 file changed, 60 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/media/cnm,wave5.yaml b/Documentation/devicetree/bindings/media/cnm,wave5.yaml
> > new file mode 100644
> > index 000000000000..b31d34aec05b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/cnm,wave5.yaml
> > @@ -0,0 +1,60 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/cnm,wave5.yaml#
>
> Filename matching compatible, so: cnm,cm521c-vpu.yaml
>
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Chips&Media Wave 5 Series multi-standard codec IP
> > +
> > +maintainers:
> > + - Nas Chung <nas.chung@chipsnmedia.com>
> > + - Jackson Lee <jackson.lee@chipsnmedia.com>
> > +
> > +description:
> > + The Chips&Media WAVE codec IP is a multi format video encoder/decoder
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - cnm,cm521c-vpu
>
> Can this device be anything else? Why VPU suffix?
It needs an SoC specific compatible (TI something...) as well (or
instead). Unless there's a public spec with details on how many
clocks, resets, interrupts, etc. there are.
Rob
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support
2023-10-16 13:47 ` Rob Herring
@ 2023-10-21 12:05 ` Sebastian Fricke
2023-10-22 16:01 ` Krzysztof Kozlowski
0 siblings, 1 reply; 27+ messages in thread
From: Sebastian Fricke @ 2023-10-21 12:05 UTC (permalink / raw)
To: Rob Herring
Cc: Krzysztof Kozlowski, Krzysztof Kozlowski, NXP Linux Team,
Conor Dooley, Mauro Carvalho Chehab, Jackson Lee, Hans Verkuil,
Sascha Hauer, Pengutronix Kernel Team, Shawn Guo, Philipp Zabel,
Nas Chung, Fabio Estevam, linux-media, Tomasz Figa, linux-kernel,
Nicolas Dufresne, kernel, Robert Beckett, devicetree,
linux-arm-kernel, Darren Etheridge
Hey Rob and Krzysztof,
On 16.10.2023 08:47, Rob Herring wrote:
>On Thu, Oct 12, 2023 at 03:24:12PM +0200, Krzysztof Kozlowski wrote:
>> On 12/10/2023 13:01, Sebastian Fricke wrote:
>> > From: Robert Beckett <bob.beckett@collabora.com>
>> >
>> > Add bindings for the chips&media wave5 codec driver
>> >
>> > Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
>> > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>> > Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>
>> > ---
>> > .../devicetree/bindings/media/cnm,wave5.yaml | 60 ++++++++++++++++++++++
>> > 1 file changed, 60 insertions(+)
>> >
>> > diff --git a/Documentation/devicetree/bindings/media/cnm,wave5.yaml b/Documentation/devicetree/bindings/media/cnm,wave5.yaml
>> > new file mode 100644
>> > index 000000000000..b31d34aec05b
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/media/cnm,wave5.yaml
>> > @@ -0,0 +1,60 @@
>> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> > +%YAML 1.2
>> > +---
>> > +$id: http://devicetree.org/schemas/media/cnm,wave5.yaml#
>>
>> Filename matching compatible, so: cnm,cm521c-vpu.yaml
With which compatible should the filename match? (see below)
And just to be sure, this means that I rename the file to:
`.../devicetree/bindings/media/cnm,wave521c.yaml`
>>
>> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> > +
>> > +title: Chips&Media Wave 5 Series multi-standard codec IP
>> > +
>> > +maintainers:
>> > + - Nas Chung <nas.chung@chipsnmedia.com>
>> > + - Jackson Lee <jackson.lee@chipsnmedia.com>
>> > +
>> > +description:
>> > + The Chips&Media WAVE codec IP is a multi format video encoder/decoder
>> > +
>> > +properties:
>> > + compatible:
>> > + enum:
>> > + - cnm,cm521c-vpu
>>
>> Can this device be anything else? Why VPU suffix?
>
>It needs an SoC specific compatible (TI something...) as well (or
>instead). Unless there's a public spec with details on how many
>clocks, resets, interrupts, etc. there are.
Okay so how about this, a bit similar to the Coda driver supplying both
a general option and a SoC specific version:
properties:
compatible:
enum:
- ti,k3-j721sX-wave521c
- cnm,wave521c
(ti,k3-j721sX-wave521c = manufacturer,SoC-codec)
(tested on j721s2 but should work on other variations as well)
Another alternative could be: ti,k3-wave521c (less specific on a single
SoC series but connected to a bigger range of devices)
>
>Rob
Regards,
Sebastian
>_______________________________________________
>Kernel mailing list -- kernel@mailman.collabora.com
>To unsubscribe send an email to kernel-leave@mailman.collabora.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support
2023-10-21 12:05 ` Sebastian Fricke
@ 2023-10-22 16:01 ` Krzysztof Kozlowski
2023-10-24 5:17 ` Sebastian Fricke
0 siblings, 1 reply; 27+ messages in thread
From: Krzysztof Kozlowski @ 2023-10-22 16:01 UTC (permalink / raw)
To: Sebastian Fricke, Rob Herring
Cc: Krzysztof Kozlowski, NXP Linux Team, Conor Dooley,
Mauro Carvalho Chehab, Jackson Lee, Hans Verkuil, Sascha Hauer,
Pengutronix Kernel Team, Shawn Guo, Philipp Zabel, Nas Chung,
Fabio Estevam, linux-media, Tomasz Figa, linux-kernel,
Nicolas Dufresne, kernel, Robert Beckett, devicetree,
linux-arm-kernel, Darren Etheridge
On 21/10/2023 14:05, Sebastian Fricke wrote:
> Hey Rob and Krzysztof,
>
> On 16.10.2023 08:47, Rob Herring wrote:
>> On Thu, Oct 12, 2023 at 03:24:12PM +0200, Krzysztof Kozlowski wrote:
>>> On 12/10/2023 13:01, Sebastian Fricke wrote:
>>>> From: Robert Beckett <bob.beckett@collabora.com>
>>>>
>>>> Add bindings for the chips&media wave5 codec driver
>>>>
>>>> Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
>>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>>>> Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>
>>>> ---
>>>> .../devicetree/bindings/media/cnm,wave5.yaml | 60 ++++++++++++++++++++++
>>>> 1 file changed, 60 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/media/cnm,wave5.yaml b/Documentation/devicetree/bindings/media/cnm,wave5.yaml
>>>> new file mode 100644
>>>> index 000000000000..b31d34aec05b
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/media/cnm,wave5.yaml
>>>> @@ -0,0 +1,60 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/media/cnm,wave5.yaml#
>>>
>>> Filename matching compatible, so: cnm,cm521c-vpu.yaml
>
> With which compatible should the filename match? (see below)
> And just to be sure, this means that I rename the file to:
> `.../devicetree/bindings/media/cnm,wave521c.yaml`
With the fallback compatible.
>
>>>
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Chips&Media Wave 5 Series multi-standard codec IP
>>>> +
>>>> +maintainers:
>>>> + - Nas Chung <nas.chung@chipsnmedia.com>
>>>> + - Jackson Lee <jackson.lee@chipsnmedia.com>
>>>> +
>>>> +description:
>>>> + The Chips&Media WAVE codec IP is a multi format video encoder/decoder
>>>> +
>>>> +properties:
>>>> + compatible:
>>>> + enum:
>>>> + - cnm,cm521c-vpu
>>>
>>> Can this device be anything else? Why VPU suffix?
>>
>> It needs an SoC specific compatible (TI something...) as well (or
>> instead). Unless there's a public spec with details on how many
>> clocks, resets, interrupts, etc. there are.
>
> Okay so how about this, a bit similar to the Coda driver supplying both
> a general option and a SoC specific version:
Can generic compatible be used alone in board designs? If it is licensed
block, then most likely you want a fallback.
>
> properties:
> compatible:
> enum:
> - ti,k3-j721sX-wave521c
> - cnm,wave521c
>
> (ti,k3-j721sX-wave521c = manufacturer,SoC-codec)
> (tested on j721s2 but should work on other variations as well)
>
> Another alternative could be: ti,k3-wave521c (less specific on a single
> SoC series but connected to a bigger range of devices)
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support
2023-10-22 16:01 ` Krzysztof Kozlowski
@ 2023-10-24 5:17 ` Sebastian Fricke
2023-10-24 7:24 ` Krzysztof Kozlowski
0 siblings, 1 reply; 27+ messages in thread
From: Sebastian Fricke @ 2023-10-24 5:17 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rob Herring, Krzysztof Kozlowski, NXP Linux Team, Conor Dooley,
Mauro Carvalho Chehab, Jackson Lee, Hans Verkuil, Sascha Hauer,
Pengutronix Kernel Team, Shawn Guo, Philipp Zabel, Nas Chung,
Fabio Estevam, linux-media, Tomasz Figa, linux-kernel,
Nicolas Dufresne, kernel, Robert Beckett, devicetree,
linux-arm-kernel, Darren Etheridge
Hey Krzysztof,
On 22.10.2023 18:01, Krzysztof Kozlowski wrote:
>On 21/10/2023 14:05, Sebastian Fricke wrote:
>> Hey Rob and Krzysztof,
>>
>> On 16.10.2023 08:47, Rob Herring wrote:
>>> On Thu, Oct 12, 2023 at 03:24:12PM +0200, Krzysztof Kozlowski wrote:
>>>> On 12/10/2023 13:01, Sebastian Fricke wrote:
>>>>> From: Robert Beckett <bob.beckett@collabora.com>
>>>>>
>>>>> Add bindings for the chips&media wave5 codec driver
>>>>>
>>>>> Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
>>>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>>>>> Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>
>>>>> ---
>>>>> .../devicetree/bindings/media/cnm,wave5.yaml | 60 ++++++++++++++++++++++
>>>>> 1 file changed, 60 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/media/cnm,wave5.yaml b/Documentation/devicetree/bindings/media/cnm,wave5.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..b31d34aec05b
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/media/cnm,wave5.yaml
>>>>> @@ -0,0 +1,60 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/media/cnm,wave5.yaml#
>>>>
>>>> Filename matching compatible, so: cnm,cm521c-vpu.yaml
>>
>> With which compatible should the filename match? (see below)
>> And just to be sure, this means that I rename the file to:
>> `.../devicetree/bindings/media/cnm,wave521c.yaml`
>
>With the fallback compatible.
>
>>
>>>>
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Chips&Media Wave 5 Series multi-standard codec IP
>>>>> +
>>>>> +maintainers:
>>>>> + - Nas Chung <nas.chung@chipsnmedia.com>
>>>>> + - Jackson Lee <jackson.lee@chipsnmedia.com>
>>>>> +
>>>>> +description:
>>>>> + The Chips&Media WAVE codec IP is a multi format video encoder/decoder
>>>>> +
>>>>> +properties:
>>>>> + compatible:
>>>>> + enum:
>>>>> + - cnm,cm521c-vpu
>>>>
>>>> Can this device be anything else? Why VPU suffix?
>>>
>>> It needs an SoC specific compatible (TI something...) as well (or
>>> instead). Unless there's a public spec with details on how many
>>> clocks, resets, interrupts, etc. there are.
>>
>> Okay so how about this, a bit similar to the Coda driver supplying both
>> a general option and a SoC specific version:
>
>Can generic compatible be used alone in board designs? If it is licensed
>block, then most likely you want a fallback.
Alright, so a fallback seems appropriate, how do you like this?
properties:
compatible:
items:
- enum:
- const: ti,k3-j721sX-wave521c
- const: cnm,wave521c
Providing a fallback and adding a enum which can be extended later on.
>
>>
>> properties:
>> compatible:
>> enum:
>> - ti,k3-j721sX-wave521c
>> - cnm,wave521c
>>
>> (ti,k3-j721sX-wave521c = manufacturer,SoC-codec)
>> (tested on j721s2 but should work on other variations as well)
>>
>> Another alternative could be: ti,k3-wave521c (less specific on a single
>> SoC series but connected to a bigger range of devices)
>
>Best regards,
>Krzysztof
Greetings,
Sebastian
>
>_______________________________________________
>Kernel mailing list -- kernel@mailman.collabora.com
>To unsubscribe send an email to kernel-leave@mailman.collabora.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support
2023-10-24 5:17 ` Sebastian Fricke
@ 2023-10-24 7:24 ` Krzysztof Kozlowski
2023-10-25 6:17 ` Sebastian Fricke
0 siblings, 1 reply; 27+ messages in thread
From: Krzysztof Kozlowski @ 2023-10-24 7:24 UTC (permalink / raw)
To: Sebastian Fricke
Cc: Rob Herring, Krzysztof Kozlowski, NXP Linux Team, Conor Dooley,
Mauro Carvalho Chehab, Jackson Lee, Hans Verkuil, Sascha Hauer,
Pengutronix Kernel Team, Shawn Guo, Philipp Zabel, Nas Chung,
Fabio Estevam, linux-media, Tomasz Figa, linux-kernel,
Nicolas Dufresne, kernel, Robert Beckett, devicetree,
linux-arm-kernel, Darren Etheridge
On 24/10/2023 07:17, Sebastian Fricke wrote:
>>>> It needs an SoC specific compatible (TI something...) as well (or
>>>> instead). Unless there's a public spec with details on how many
>>>> clocks, resets, interrupts, etc. there are.
>>>
>>> Okay so how about this, a bit similar to the Coda driver supplying both
>>> a general option and a SoC specific version:
>>
>> Can generic compatible be used alone in board designs? If it is licensed
>> block, then most likely you want a fallback.
>
> Alright, so a fallback seems appropriate, how do you like this?
>
> properties:
> compatible:
> items:
> - enum:
> - const: ti,k3-j721sX-wave521c
> - const: cnm,wave521c
>
> Providing a fallback and adding a enum which can be extended later on.
This looks almost good. I wonder what is "j721sX" - Google does not find
it. There is thouhg j721se.
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support
2023-10-24 7:24 ` Krzysztof Kozlowski
@ 2023-10-25 6:17 ` Sebastian Fricke
2023-10-25 7:04 ` Krzysztof Kozlowski
0 siblings, 1 reply; 27+ messages in thread
From: Sebastian Fricke @ 2023-10-25 6:17 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rob Herring, Krzysztof Kozlowski, NXP Linux Team, Conor Dooley,
Mauro Carvalho Chehab, Jackson Lee, Hans Verkuil, Sascha Hauer,
Pengutronix Kernel Team, Shawn Guo, Philipp Zabel, Nas Chung,
Fabio Estevam, linux-media, Tomasz Figa, linux-kernel,
Nicolas Dufresne, kernel, Robert Beckett, devicetree,
linux-arm-kernel, Darren Etheridge
Hey Krzysztof,
On 24.10.2023 09:24, Krzysztof Kozlowski wrote:
>On 24/10/2023 07:17, Sebastian Fricke wrote:
>
>>>>> It needs an SoC specific compatible (TI something...) as well (or
>>>>> instead). Unless there's a public spec with details on how many
>>>>> clocks, resets, interrupts, etc. there are.
>>>>
>>>> Okay so how about this, a bit similar to the Coda driver supplying both
>>>> a general option and a SoC specific version:
>>>
>>> Can generic compatible be used alone in board designs? If it is licensed
>>> block, then most likely you want a fallback.
>>
>> Alright, so a fallback seems appropriate, how do you like this?
>>
>> properties:
>> compatible:
>> items:
>> - enum:
>> - const: ti,k3-j721sX-wave521c
>> - const: cnm,wave521c
>>
>> Providing a fallback and adding a enum which can be extended later on.
>
>This looks almost good. I wonder what is "j721sX" - Google does not find
>it. There is thouhg j721se.
Well that was a misunderstanding from my side I thought that both j721se
and j721s2 have the Wave5 IP block and wanted to describe both with
j721sX. But as it turns out the IP block isn't present on j721se.
Additionally, I was only able to test the codec on j721s2 for now and so
I would opt for calling it: `ti,k3-j721s2-wave521c`
>
>Best regards,
>Krzysztof
Sincerely,
Sebastian
>
>_______________________________________________
>Kernel mailing list -- kernel@mailman.collabora.com
>To unsubscribe send an email to kernel-leave@mailman.collabora.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support
2023-10-25 6:17 ` Sebastian Fricke
@ 2023-10-25 7:04 ` Krzysztof Kozlowski
0 siblings, 0 replies; 27+ messages in thread
From: Krzysztof Kozlowski @ 2023-10-25 7:04 UTC (permalink / raw)
To: Sebastian Fricke
Cc: Rob Herring, Krzysztof Kozlowski, NXP Linux Team, Conor Dooley,
Mauro Carvalho Chehab, Jackson Lee, Hans Verkuil, Sascha Hauer,
Pengutronix Kernel Team, Shawn Guo, Philipp Zabel, Nas Chung,
Fabio Estevam, linux-media, Tomasz Figa, linux-kernel,
Nicolas Dufresne, kernel, Robert Beckett, devicetree,
linux-arm-kernel, Darren Etheridge
On 25/10/2023 08:17, Sebastian Fricke wrote:
> Hey Krzysztof,
>
> On 24.10.2023 09:24, Krzysztof Kozlowski wrote:
>> On 24/10/2023 07:17, Sebastian Fricke wrote:
>>
>>>>>> It needs an SoC specific compatible (TI something...) as well (or
>>>>>> instead). Unless there's a public spec with details on how many
>>>>>> clocks, resets, interrupts, etc. there are.
>>>>>
>>>>> Okay so how about this, a bit similar to the Coda driver supplying both
>>>>> a general option and a SoC specific version:
>>>>
>>>> Can generic compatible be used alone in board designs? If it is licensed
>>>> block, then most likely you want a fallback.
>>>
>>> Alright, so a fallback seems appropriate, how do you like this?
>>>
>>> properties:
>>> compatible:
>>> items:
>>> - enum:
>>> - const: ti,k3-j721sX-wave521c
>>> - const: cnm,wave521c
>>>
>>> Providing a fallback and adding a enum which can be extended later on.
>>
>> This looks almost good. I wonder what is "j721sX" - Google does not find
>> it. There is thouhg j721se.
>
> Well that was a misunderstanding from my side I thought that both j721se
> and j721s2 have the Wave5 IP block and wanted to describe both with
> j721sX. But as it turns out the IP block isn't present on j721se.
It does not matter. You must not have wildcards in compatibles.
> Additionally, I was only able to test the codec on j721s2 for now and so
> I would opt for calling it: `ti,k3-j721s2-wave521c`
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support
2023-10-12 11:01 ` [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support Sebastian Fricke
2023-10-12 13:24 ` Krzysztof Kozlowski
@ 2023-10-17 13:39 ` Devarsh Thakkar
2023-10-21 11:53 ` Sebastian Fricke
2023-10-22 16:12 ` Krzysztof Kozlowski
1 sibling, 2 replies; 27+ messages in thread
From: Devarsh Thakkar @ 2023-10-17 13:39 UTC (permalink / raw)
To: Sebastian Fricke, Krzysztof Kozlowski, NXP Linux Team,
Conor Dooley, Mauro Carvalho Chehab, Jackson Lee, Hans Verkuil,
Sascha Hauer, Rob Herring, Pengutronix Kernel Team, Shawn Guo,
Philipp Zabel, Nas Chung, Fabio Estevam
Cc: linux-media, Tomasz Figa, linux-kernel, Nicolas Dufresne, kernel,
Robert Beckett, devicetree, linux-arm-kernel, Darren Etheridge,
Bajjuri, Praneeth, Raghavendra, Vignesh, Bhatia, Aradhya,
Luthra, Jai, Bajjuri, Praneeth, Brnich, Brandon,
Pothukuchi, Vijay
Hi Sebastian, Krzysztof, Rob,
On 12/10/23 16:31, Sebastian Fricke wrote:
> From: Robert Beckett <bob.beckett@collabora.com>
>
> Add bindings for the chips&media wave5 codec driver
>
> Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>
> ---
> .../devicetree/bindings/media/cnm,wave5.yaml | 60 ++++++++++++++++++++++
> 1 file changed, 60 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/cnm,wave5.yaml b/Documentation/devicetree/bindings/media/cnm,wave5.yaml
> new file mode 100644
> index 000000000000..b31d34aec05b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/cnm,wave5.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/cnm,wave5.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Chips&Media Wave 5 Series multi-standard codec IP
> +
> +maintainers:
> + - Nas Chung <nas.chung@chipsnmedia.com>
> + - Jackson Lee <jackson.lee@chipsnmedia.com>
> +
> +description:
> + The Chips&Media WAVE codec IP is a multi format video encoder/decoder
> +
> +properties:
> + compatible:
> + enum:
> + - cnm,cm521c-vpu
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + items:
> + - description: VCODEC clock
> +
> + interrupts:
> + maxItems: 1
> +
> + power-domains:
> + maxItems: 1
> +
> + resets:
> + maxItems: 1
> +
> + sram:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + The VPU uses the SRAM to store some of the reference data instead of
> + storing it on DMA memory. It is mainly used for the purpose of reducing
> + bandwidth.
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - interrupts
> +
Is it possible to keep interrupts property as optional given HW can still work
without it if SW does polling of ISR using registers?
The reason to ask is in TI AM62A SoC (which also uses this codec) there is an
SoC errata of missing interrupt line to A53 and we are using SW based polling
locally to run the driver.
We were planning to upstream that SW based polling support patch in CnM driver
once this base initial driver patch series gets merged, but just wanted to
check if upfront it is possible to have interrupts property as optional so
that we don't have to change the binding doc again to make it optional later on.
Also note that the polling patch won't be specific to AM62A, other SoC's too
which use this wave5 hardware if they want can enable polling by choice (by
removing interrupt property)
Could you please share your opinion on this ?
Regards
Devarsh
> +additionalProperties: false
> +
> +examples:
> + - |
> + vpu: video-codec@12345678 {
> + compatible = "cnm,cm521c-vpu";
> + reg = <0x12345678 0x1000>;
> + clocks = <&clks 42>;
> + interrupts = <42>;
> + sram = <&sram>;
> + };
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support
2023-10-17 13:39 ` Devarsh Thakkar
@ 2023-10-21 11:53 ` Sebastian Fricke
2023-10-22 16:12 ` Krzysztof Kozlowski
1 sibling, 0 replies; 27+ messages in thread
From: Sebastian Fricke @ 2023-10-21 11:53 UTC (permalink / raw)
To: Devarsh Thakkar
Cc: Krzysztof Kozlowski, NXP Linux Team, Conor Dooley,
Mauro Carvalho Chehab, Jackson Lee, Hans Verkuil, Sascha Hauer,
Rob Herring, Pengutronix Kernel Team, Shawn Guo, Philipp Zabel,
Nas Chung, Fabio Estevam, linux-media, Tomasz Figa, linux-kernel,
Nicolas Dufresne, kernel, Robert Beckett, devicetree,
linux-arm-kernel, Darren Etheridge, Bajjuri, Praneeth,
Raghavendra, Vignesh, Bhatia, Aradhya, Luthra, Jai,
Brnich, Brandon, Pothukuchi, Vijay
Hello Krzysztof and Rob,
this question is quite important for our next version and for the
overall direction of the DT bindings, could you have a look at this?
Thank you and Regards,
Sebastian
On 17.10.2023 19:09, Devarsh Thakkar wrote:
>Hi Sebastian, Krzysztof, Rob,
>
>On 12/10/23 16:31, Sebastian Fricke wrote:
>> From: Robert Beckett <bob.beckett@collabora.com>
>>
>> Add bindings for the chips&media wave5 codec driver
>>
>> Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>> Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>
>> ---
>> .../devicetree/bindings/media/cnm,wave5.yaml | 60 ++++++++++++++++++++++
>> 1 file changed, 60 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/media/cnm,wave5.yaml b/Documentation/devicetree/bindings/media/cnm,wave5.yaml
>> new file mode 100644
>> index 000000000000..b31d34aec05b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/cnm,wave5.yaml
>> @@ -0,0 +1,60 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/media/cnm,wave5.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Chips&Media Wave 5 Series multi-standard codec IP
>> +
>> +maintainers:
>> + - Nas Chung <nas.chung@chipsnmedia.com>
>> + - Jackson Lee <jackson.lee@chipsnmedia.com>
>> +
>> +description:
>> + The Chips&Media WAVE codec IP is a multi format video encoder/decoder
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - cnm,cm521c-vpu
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + clocks:
>> + items:
>> + - description: VCODEC clock
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> + power-domains:
>> + maxItems: 1
>> +
>> + resets:
>> + maxItems: 1
>> +
>> + sram:
>> + $ref: /schemas/types.yaml#/definitions/phandle
>> + description:
>> + The VPU uses the SRAM to store some of the reference data instead of
>> + storing it on DMA memory. It is mainly used for the purpose of reducing
>> + bandwidth.
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - clocks
>> + - interrupts
>> +
>
>Is it possible to keep interrupts property as optional given HW can still work
>without it if SW does polling of ISR using registers?
>
>The reason to ask is in TI AM62A SoC (which also uses this codec) there is an
>SoC errata of missing interrupt line to A53 and we are using SW based polling
>locally to run the driver.
>
>We were planning to upstream that SW based polling support patch in CnM driver
>once this base initial driver patch series gets merged, but just wanted to
>check if upfront it is possible to have interrupts property as optional so
>that we don't have to change the binding doc again to make it optional later on.
>
>Also note that the polling patch won't be specific to AM62A, other SoC's too
>which use this wave5 hardware if they want can enable polling by choice (by
>removing interrupt property)
>
>Could you please share your opinion on this ?
>
>Regards
>Devarsh
>
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + vpu: video-codec@12345678 {
>> + compatible = "cnm,cm521c-vpu";
>> + reg = <0x12345678 0x1000>;
>> + clocks = <&clks 42>;
>> + interrupts = <42>;
>> + sram = <&sram>;
>> + };
>>
>_______________________________________________
>Kernel mailing list -- kernel@mailman.collabora.com
>To unsubscribe send an email to kernel-leave@mailman.collabora.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support
2023-10-17 13:39 ` Devarsh Thakkar
2023-10-21 11:53 ` Sebastian Fricke
@ 2023-10-22 16:12 ` Krzysztof Kozlowski
2023-10-26 16:33 ` Sebastian Fricke
1 sibling, 1 reply; 27+ messages in thread
From: Krzysztof Kozlowski @ 2023-10-22 16:12 UTC (permalink / raw)
To: Devarsh Thakkar, Sebastian Fricke, Krzysztof Kozlowski,
NXP Linux Team, Conor Dooley, Mauro Carvalho Chehab, Jackson Lee,
Hans Verkuil, Sascha Hauer, Rob Herring, Pengutronix Kernel Team,
Shawn Guo, Philipp Zabel, Nas Chung, Fabio Estevam
Cc: linux-media, Tomasz Figa, linux-kernel, Nicolas Dufresne, kernel,
Robert Beckett, devicetree, linux-arm-kernel, Darren Etheridge,
Bajjuri, Praneeth, Raghavendra, Vignesh, Bhatia, Aradhya,
Luthra, Jai, Brnich, Brandon, Pothukuchi, Vijay
On 17/10/2023 15:39, Devarsh Thakkar wrote:
>> +required:
>> + - compatible
>> + - reg
>> + - clocks
>> + - interrupts
>> +
>
> Is it possible to keep interrupts property as optional given HW can still work
> without it if SW does polling of ISR using registers?
>
> The reason to ask is in TI AM62A SoC (which also uses this codec) there is an
> SoC errata of missing interrupt line to A53 and we are using SW based polling
> locally to run the driver.
>
> We were planning to upstream that SW based polling support patch in CnM driver
> once this base initial driver patch series gets merged, but just wanted to
> check if upfront it is possible to have interrupts property as optional so
> that we don't have to change the binding doc again to make it optional later on.
>
> Also note that the polling patch won't be specific to AM62A, other SoC's too
> which use this wave5 hardware if they want can enable polling by choice (by
> removing interrupt property)
>
> Could you please share your opinion on this ?
You know, if you do not have interrupt line connected, how could it be
required, right? If the hardware does not require interrupt to be
connected then bindings should not require it.
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support
2023-10-22 16:12 ` Krzysztof Kozlowski
@ 2023-10-26 16:33 ` Sebastian Fricke
2023-10-27 7:07 ` Krzysztof Kozlowski
0 siblings, 1 reply; 27+ messages in thread
From: Sebastian Fricke @ 2023-10-26 16:33 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Devarsh Thakkar, Krzysztof Kozlowski, NXP Linux Team,
Conor Dooley, Mauro Carvalho Chehab, Jackson Lee, Hans Verkuil,
Sascha Hauer, Rob Herring, Pengutronix Kernel Team, Shawn Guo,
Philipp Zabel, Nas Chung, Fabio Estevam, linux-media, Tomasz Figa,
linux-kernel, Nicolas Dufresne, kernel, Robert Beckett,
devicetree, linux-arm-kernel, Darren Etheridge, Bajjuri, Praneeth,
Raghavendra, Vignesh, Bhatia, Aradhya, Luthra, Jai,
Brnich, Brandon, Pothukuchi, Vijay
Hey Krzysztof,
On 22.10.2023 18:12, Krzysztof Kozlowski wrote:
>On 17/10/2023 15:39, Devarsh Thakkar wrote:
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - clocks
>>> + - interrupts
>>> +
>>
>> Is it possible to keep interrupts property as optional given HW can still work
>> without it if SW does polling of ISR using registers?
>>
>> The reason to ask is in TI AM62A SoC (which also uses this codec) there is an
>> SoC errata of missing interrupt line to A53 and we are using SW based polling
>> locally to run the driver.
>>
>> We were planning to upstream that SW based polling support patch in CnM driver
>> once this base initial driver patch series gets merged, but just wanted to
>> check if upfront it is possible to have interrupts property as optional so
>> that we don't have to change the binding doc again to make it optional later on.
>>
>> Also note that the polling patch won't be specific to AM62A, other SoC's too
>> which use this wave5 hardware if they want can enable polling by choice (by
>> removing interrupt property)
>>
>> Could you please share your opinion on this ?
>
>You know, if you do not have interrupt line connected, how could it be
>required, right? If the hardware does not require interrupt to be
>connected then bindings should not require it.
Alright, so I will make the interrupt optional in the DT binding.
By simply removing it from this list:
required:
- compatible
- reg
- clocks
- interrupts
Is it possible to make it required later on for certain SoC by adding
something along the lines of:
allOf:
- if:
properties:
compatible:
contains:
enum:
- soc_compatible...
...
then:
properties:
interrupts: true
?
>
>Best regards,
>Krzysztof
Sincerely,
Sebastian
>
>_______________________________________________
>Kernel mailing list -- kernel@mailman.collabora.com
>To unsubscribe send an email to kernel-leave@mailman.collabora.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support
2023-10-26 16:33 ` Sebastian Fricke
@ 2023-10-27 7:07 ` Krzysztof Kozlowski
0 siblings, 0 replies; 27+ messages in thread
From: Krzysztof Kozlowski @ 2023-10-27 7:07 UTC (permalink / raw)
To: Sebastian Fricke
Cc: Devarsh Thakkar, Krzysztof Kozlowski, NXP Linux Team,
Conor Dooley, Mauro Carvalho Chehab, Jackson Lee, Hans Verkuil,
Sascha Hauer, Rob Herring, Pengutronix Kernel Team, Shawn Guo,
Philipp Zabel, Nas Chung, Fabio Estevam, linux-media, Tomasz Figa,
linux-kernel, Nicolas Dufresne, kernel, Robert Beckett,
devicetree, linux-arm-kernel, Darren Etheridge, Bajjuri, Praneeth,
Raghavendra, Vignesh, Bhatia, Aradhya, Luthra, Jai,
Brnich, Brandon, Pothukuchi, Vijay
On 26/10/2023 18:33, Sebastian Fricke wrote:
> required:
> - compatible
> - reg
> - clocks
> - interrupts
>
> Is it possible to make it required later on for certain SoC by adding
> something along the lines of:
>
> allOf:
> - if:
> properties:
> compatible:
> contains:
> enum:
> - soc_compatible...
> ...
> then:
> properties:
> interrupts: true
See example schema:
https://elixir.bootlin.com/linux/v5.19/source/Documentation/devicetree/bindings/example-schema.yaml#L212
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v13 7/8] media: chips-media: wave5: Add wave5 driver to maintainers file
2023-10-12 11:00 [PATCH v13 0/8] Wave5 codec driver Sebastian Fricke
` (3 preceding siblings ...)
2023-10-12 11:01 ` [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support Sebastian Fricke
@ 2023-10-12 11:01 ` Sebastian Fricke
2023-10-12 11:01 ` [PATCH v13 8/8] arm64: dts: ti: k3-j721s2-main: add wave5 video encoder/decoder node Sebastian Fricke
` (2 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: Sebastian Fricke @ 2023-10-12 11:01 UTC (permalink / raw)
To: Krzysztof Kozlowski, NXP Linux Team, Conor Dooley,
Mauro Carvalho Chehab, Jackson Lee, Hans Verkuil, Sascha Hauer,
Rob Herring, Pengutronix Kernel Team, Shawn Guo, Philipp Zabel,
Nas Chung, Fabio Estevam
Cc: linux-media, Tomasz Figa, linux-kernel, Sebastian Fricke,
Nicolas Dufresne, kernel, Robert Beckett, devicetree,
linux-arm-kernel, Darren Etheridge
From: Robert Beckett <bob.beckett@collabora.com>
Add the Chips&Media wave5 encoder/decoder driver to the maintainers file
Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>
---
MAINTAINERS | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 063a11791bbf..b6a592c14caa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23206,6 +23206,14 @@ F: include/linux/watchdog.h
F: include/trace/events/watchdog.h
F: include/uapi/linux/watchdog.h
+WAVE5 VPU CODEC DRIVER
+M: Nas Chung <nas.chung@chipsnmedia.com>
+M: Jackson Lee <jackson.lee@chipsnmedia.com>
+L: linux-media@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/media/cnm,wave5.yaml
+F: drivers/media/platform/chips-media/wave5/
+
WHISKEYCOVE PMIC GPIO DRIVER
M: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
L: linux-gpio@vger.kernel.org
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v13 8/8] arm64: dts: ti: k3-j721s2-main: add wave5 video encoder/decoder node
2023-10-12 11:00 [PATCH v13 0/8] Wave5 codec driver Sebastian Fricke
` (4 preceding siblings ...)
2023-10-12 11:01 ` [PATCH v13 7/8] media: chips-media: wave5: Add wave5 driver to maintainers file Sebastian Fricke
@ 2023-10-12 11:01 ` Sebastian Fricke
[not found] ` <20230929-wave5_v13_media_master-v13-5-5ac60ccbf2ce@collabora.com>
[not found] ` <20230929-wave5_v13_media_master-v13-4-5ac60ccbf2ce@collabora.com>
7 siblings, 0 replies; 27+ messages in thread
From: Sebastian Fricke @ 2023-10-12 11:01 UTC (permalink / raw)
To: Krzysztof Kozlowski, NXP Linux Team, Conor Dooley,
Mauro Carvalho Chehab, Jackson Lee, Hans Verkuil, Sascha Hauer,
Rob Herring, Pengutronix Kernel Team, Shawn Guo, Philipp Zabel,
Nas Chung, Fabio Estevam
Cc: linux-media, Tomasz Figa, linux-kernel, Sebastian Fricke,
Nicolas Dufresne, kernel, Robert Beckett, devicetree,
linux-arm-kernel, Darren Etheridge
From: Darren Etheridge <detheridge@ti.com>
Add the Chips and Media wave521cl video decoder/encoder node on J721S2.
This functional block also requires an SRAM buffer as a bandwidth saving
temporary store so we need to add a carve out of 126K for this as
specified in the documentation.
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
index 084f8f5b6699..7ae4c6436275 100644
--- a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
@@ -28,6 +28,10 @@ atf-sram@0 {
reg = <0x0 0x20000>;
};
+ vpu_sram: vpu-sram@20000 {
+ reg = <0x20000 0x1f800>;
+ };
+
tifs-sram@1f0000 {
reg = <0x1f0000 0x10000>;
};
@@ -716,6 +720,16 @@ main_i2c6: i2c@2060000 {
status = "disabled";
};
+ vpu: video-codec@4210000 {
+ compatible = "cnm,cm521c-vpu";
+ reg = <0x00 0x4210000 0x00 0x10000>;
+ interrupts = <GIC_SPI 182 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&k3_clks 179 2>;
+ clock-names = "vcodec";
+ power-domains = <&k3_pds 179 TI_SCI_PD_EXCLUSIVE>;
+ sram = <&vpu_sram>;
+ };
+
main_sdhci0: mmc@4f80000 {
compatible = "ti,j721e-sdhci-8bit";
reg = <0x00 0x04f80000 0x00 0x1000>,
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 27+ messages in thread[parent not found: <20230929-wave5_v13_media_master-v13-5-5ac60ccbf2ce@collabora.com>]
* Re: [PATCH v13 5/8] media: chips-media: wave5: Add the v4l2 layer
[not found] ` <20230929-wave5_v13_media_master-v13-5-5ac60ccbf2ce@collabora.com>
@ 2023-10-16 11:57 ` Hans Verkuil
2023-10-16 13:35 ` Sebastian Fricke
2023-10-17 22:13 ` Ivan Bornyakov
1 sibling, 1 reply; 27+ messages in thread
From: Hans Verkuil @ 2023-10-16 11:57 UTC (permalink / raw)
To: Sebastian Fricke, Krzysztof Kozlowski, NXP Linux Team,
Conor Dooley, Mauro Carvalho Chehab, Jackson Lee, Sascha Hauer,
Rob Herring, Pengutronix Kernel Team, Shawn Guo, Philipp Zabel,
Nas Chung, Fabio Estevam
Cc: linux-media, Tomasz Figa, linux-kernel, Nicolas Dufresne, kernel,
Robert Beckett, devicetree, linux-arm-kernel, Darren Etheridge
Hi Sebastian,
On 12/10/2023 13:01, Sebastian Fricke wrote:
> Add the decoder and encoder implementing the v4l2
> API. This patch also adds the Makefile and the VIDEO_WAVE_VPU config
>
> Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com>
> Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
> ---
> drivers/media/platform/chips-media/Kconfig | 1 +
> drivers/media/platform/chips-media/Makefile | 1 +
> drivers/media/platform/chips-media/wave5/Kconfig | 12 +
> drivers/media/platform/chips-media/wave5/Makefile | 10 +
> .../platform/chips-media/wave5/wave5-helper.c | 213 +++
> .../platform/chips-media/wave5/wave5-helper.h | 31 +
> .../platform/chips-media/wave5/wave5-vpu-dec.c | 1953 ++++++++++++++++++++
> .../platform/chips-media/wave5/wave5-vpu-enc.c | 1794 ++++++++++++++++++
> .../media/platform/chips-media/wave5/wave5-vpu.c | 291 +++
> .../media/platform/chips-media/wave5/wave5-vpu.h | 83 +
> .../platform/chips-media/wave5/wave5-vpuapi.h | 2 -
> 11 files changed, 4389 insertions(+), 2 deletions(-)
>
<snip>
> +static int wave5_vpu_dec_create_bufs(struct file *file, void *priv,
> + struct v4l2_create_buffers *create)
> +{
> + struct vpu_instance *inst = wave5_to_vpu_inst(priv);
> + struct v4l2_format *f = &create->format;
> +
> + /* Firmware does not support CREATE_BUFS for CAPTURE queues. */
> + if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
> + dev_dbg(inst->dev->dev,
> + "%s: VIDIOC_CREATE_BUFS not supported on CAPTURE queues.\n",
> + __func__);
> + return -EOPNOTSUPP;
> + }
> +
> + return v4l2_m2m_ioctl_create_bufs(file, priv, create);
> +}
Regarding the EOPNOTSUPP discussion: I discussed this some more with
Nicolas on irc, and we wonder if it isn't better to just drop create_bufs
support for the wave5 decoder altogether. Is there any point in supporting
it for OUTPUT but not CAPTURE?
<snip>
> +static const struct v4l2_ioctl_ops wave5_vpu_dec_ioctl_ops = {
> + .vidioc_querycap = wave5_vpu_dec_querycap,
> + .vidioc_enum_framesizes = wave5_vpu_dec_enum_framesizes,
> +
> + .vidioc_enum_fmt_vid_cap = wave5_vpu_dec_enum_fmt_cap,
> + .vidioc_s_fmt_vid_cap_mplane = wave5_vpu_dec_s_fmt_cap,
> + .vidioc_g_fmt_vid_cap_mplane = wave5_vpu_dec_g_fmt_cap,
> + .vidioc_try_fmt_vid_cap_mplane = wave5_vpu_dec_try_fmt_cap,
> +
> + .vidioc_enum_fmt_vid_out = wave5_vpu_dec_enum_fmt_out,
> + .vidioc_s_fmt_vid_out_mplane = wave5_vpu_dec_s_fmt_out,
> + .vidioc_g_fmt_vid_out_mplane = wave5_vpu_g_fmt_out,
> + .vidioc_try_fmt_vid_out_mplane = wave5_vpu_dec_try_fmt_out,
> +
> + .vidioc_g_selection = wave5_vpu_dec_g_selection,
> + .vidioc_s_selection = wave5_vpu_dec_s_selection,
> +
> + .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs,
> + .vidioc_querybuf = v4l2_m2m_ioctl_querybuf,
> + .vidioc_create_bufs = wave5_vpu_dec_create_bufs,
So this would just be dropped.
> + .vidioc_prepare_buf = v4l2_m2m_ioctl_prepare_buf,
> + .vidioc_qbuf = v4l2_m2m_ioctl_qbuf,
> + .vidioc_expbuf = v4l2_m2m_ioctl_expbuf,
> + .vidioc_dqbuf = v4l2_m2m_ioctl_dqbuf,
> + .vidioc_streamon = v4l2_m2m_ioctl_streamon,
> + .vidioc_streamoff = v4l2_m2m_ioctl_streamoff,
> +
> + .vidioc_try_decoder_cmd = v4l2_m2m_ioctl_try_decoder_cmd,
> + .vidioc_decoder_cmd = wave5_vpu_dec_decoder_cmd,
> +
> + .vidioc_subscribe_event = wave5_vpu_subscribe_event,
> + .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
> +};
This also means there is no need to document the new EOPNOTSUPP error
code in VIDIOC_CREATE_BUFS, or to modify v4l2-compliance.
You *do* need to add a comment somewhere explaining why you don't
support this ioctl. I think it would be best to do that right after
'.vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs,'.
Regards,
Hans
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v13 5/8] media: chips-media: wave5: Add the v4l2 layer
2023-10-16 11:57 ` [PATCH v13 5/8] media: chips-media: wave5: Add the v4l2 layer Hans Verkuil
@ 2023-10-16 13:35 ` Sebastian Fricke
2023-10-16 13:39 ` Hans Verkuil
0 siblings, 1 reply; 27+ messages in thread
From: Sebastian Fricke @ 2023-10-16 13:35 UTC (permalink / raw)
To: Hans Verkuil
Cc: Krzysztof Kozlowski, NXP Linux Team, Conor Dooley,
Mauro Carvalho Chehab, Jackson Lee, Sascha Hauer, Rob Herring,
Pengutronix Kernel Team, Shawn Guo, Philipp Zabel, Nas Chung,
Fabio Estevam, linux-media, Tomasz Figa, linux-kernel,
Nicolas Dufresne, kernel, Robert Beckett, devicetree,
linux-arm-kernel, Darren Etheridge
Hey Hans,
On 16.10.2023 13:57, Hans Verkuil wrote:
>Hi Sebastian,
>
>On 12/10/2023 13:01, Sebastian Fricke wrote:
>> Add the decoder and encoder implementing the v4l2
>> API. This patch also adds the Makefile and the VIDEO_WAVE_VPU config
>>
>> Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>
>> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>> Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com>
>> Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>> Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
>> ---
>> drivers/media/platform/chips-media/Kconfig | 1 +
>> drivers/media/platform/chips-media/Makefile | 1 +
>> drivers/media/platform/chips-media/wave5/Kconfig | 12 +
>> drivers/media/platform/chips-media/wave5/Makefile | 10 +
>> .../platform/chips-media/wave5/wave5-helper.c | 213 +++
>> .../platform/chips-media/wave5/wave5-helper.h | 31 +
>> .../platform/chips-media/wave5/wave5-vpu-dec.c | 1953 ++++++++++++++++++++
>> .../platform/chips-media/wave5/wave5-vpu-enc.c | 1794 ++++++++++++++++++
>> .../media/platform/chips-media/wave5/wave5-vpu.c | 291 +++
>> .../media/platform/chips-media/wave5/wave5-vpu.h | 83 +
>> .../platform/chips-media/wave5/wave5-vpuapi.h | 2 -
>> 11 files changed, 4389 insertions(+), 2 deletions(-)
>>
>
><snip>
>
>> +static int wave5_vpu_dec_create_bufs(struct file *file, void *priv,
>> + struct v4l2_create_buffers *create)
>> +{
>> + struct vpu_instance *inst = wave5_to_vpu_inst(priv);
>> + struct v4l2_format *f = &create->format;
>> +
>> + /* Firmware does not support CREATE_BUFS for CAPTURE queues. */
>> + if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
>> + dev_dbg(inst->dev->dev,
>> + "%s: VIDIOC_CREATE_BUFS not supported on CAPTURE queues.\n",
>> + __func__);
>> + return -EOPNOTSUPP;
>> + }
>> +
>> + return v4l2_m2m_ioctl_create_bufs(file, priv, create);
>> +}
>
>Regarding the EOPNOTSUPP discussion: I discussed this some more with
>Nicolas on irc, and we wonder if it isn't better to just drop create_bufs
>support for the wave5 decoder altogether. Is there any point in supporting
>it for OUTPUT but not CAPTURE?
>
><snip>
>
>> +static const struct v4l2_ioctl_ops wave5_vpu_dec_ioctl_ops = {
>> + .vidioc_querycap = wave5_vpu_dec_querycap,
>> + .vidioc_enum_framesizes = wave5_vpu_dec_enum_framesizes,
>> +
>> + .vidioc_enum_fmt_vid_cap = wave5_vpu_dec_enum_fmt_cap,
>> + .vidioc_s_fmt_vid_cap_mplane = wave5_vpu_dec_s_fmt_cap,
>> + .vidioc_g_fmt_vid_cap_mplane = wave5_vpu_dec_g_fmt_cap,
>> + .vidioc_try_fmt_vid_cap_mplane = wave5_vpu_dec_try_fmt_cap,
>> +
>> + .vidioc_enum_fmt_vid_out = wave5_vpu_dec_enum_fmt_out,
>> + .vidioc_s_fmt_vid_out_mplane = wave5_vpu_dec_s_fmt_out,
>> + .vidioc_g_fmt_vid_out_mplane = wave5_vpu_g_fmt_out,
>> + .vidioc_try_fmt_vid_out_mplane = wave5_vpu_dec_try_fmt_out,
>> +
>> + .vidioc_g_selection = wave5_vpu_dec_g_selection,
>> + .vidioc_s_selection = wave5_vpu_dec_s_selection,
>> +
>> + .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs,
>> + .vidioc_querybuf = v4l2_m2m_ioctl_querybuf,
>> + .vidioc_create_bufs = wave5_vpu_dec_create_bufs,
>
>So this would just be dropped.
>
>> + .vidioc_prepare_buf = v4l2_m2m_ioctl_prepare_buf,
>> + .vidioc_qbuf = v4l2_m2m_ioctl_qbuf,
>> + .vidioc_expbuf = v4l2_m2m_ioctl_expbuf,
>> + .vidioc_dqbuf = v4l2_m2m_ioctl_dqbuf,
>> + .vidioc_streamon = v4l2_m2m_ioctl_streamon,
>> + .vidioc_streamoff = v4l2_m2m_ioctl_streamoff,
>> +
>> + .vidioc_try_decoder_cmd = v4l2_m2m_ioctl_try_decoder_cmd,
>> + .vidioc_decoder_cmd = wave5_vpu_dec_decoder_cmd,
>> +
>> + .vidioc_subscribe_event = wave5_vpu_subscribe_event,
>> + .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
>> +};
>
>This also means there is no need to document the new EOPNOTSUPP error
>code in VIDIOC_CREATE_BUFS, or to modify v4l2-compliance.
>
>You *do* need to add a comment somewhere explaining why you don't
>support this ioctl. I think it would be best to do that right after
>'.vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs,'.
So, besides this issue would you judge the v4l2 layer of the driver to
be ready? Do you want a reviewed by tag for it or would you take it like
this as well?
>
>Regards,
>
> Hans
Sincerly,
Sebastian
>_______________________________________________
>Kernel mailing list -- kernel@mailman.collabora.com
>To unsubscribe send an email to kernel-leave@mailman.collabora.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v13 5/8] media: chips-media: wave5: Add the v4l2 layer
2023-10-16 13:35 ` Sebastian Fricke
@ 2023-10-16 13:39 ` Hans Verkuil
0 siblings, 0 replies; 27+ messages in thread
From: Hans Verkuil @ 2023-10-16 13:39 UTC (permalink / raw)
To: Sebastian Fricke
Cc: Krzysztof Kozlowski, NXP Linux Team, Conor Dooley,
Mauro Carvalho Chehab, Jackson Lee, Sascha Hauer, Rob Herring,
Pengutronix Kernel Team, Shawn Guo, Philipp Zabel, Nas Chung,
Fabio Estevam, linux-media, Tomasz Figa, linux-kernel,
Nicolas Dufresne, kernel, Robert Beckett, devicetree,
linux-arm-kernel, Darren Etheridge
On 16/10/2023 15:35, Sebastian Fricke wrote:
> Hey Hans,
>
> On 16.10.2023 13:57, Hans Verkuil wrote:
>> Hi Sebastian,
>>
>> On 12/10/2023 13:01, Sebastian Fricke wrote:
>>> Add the decoder and encoder implementing the v4l2
>>> API. This patch also adds the Makefile and the VIDEO_WAVE_VPU config
>>>
>>> Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>
>>> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>>> Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com>
>>> Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>>> Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
>>> ---
>>> drivers/media/platform/chips-media/Kconfig | 1 +
>>> drivers/media/platform/chips-media/Makefile | 1 +
>>> drivers/media/platform/chips-media/wave5/Kconfig | 12 +
>>> drivers/media/platform/chips-media/wave5/Makefile | 10 +
>>> .../platform/chips-media/wave5/wave5-helper.c | 213 +++
>>> .../platform/chips-media/wave5/wave5-helper.h | 31 +
>>> .../platform/chips-media/wave5/wave5-vpu-dec.c | 1953 ++++++++++++++++++++
>>> .../platform/chips-media/wave5/wave5-vpu-enc.c | 1794 ++++++++++++++++++
>>> .../media/platform/chips-media/wave5/wave5-vpu.c | 291 +++
>>> .../media/platform/chips-media/wave5/wave5-vpu.h | 83 +
>>> .../platform/chips-media/wave5/wave5-vpuapi.h | 2 -
>>> 11 files changed, 4389 insertions(+), 2 deletions(-)
>>>
>>
>> <snip>
>>
>>> +static int wave5_vpu_dec_create_bufs(struct file *file, void *priv,
>>> + struct v4l2_create_buffers *create)
>>> +{
>>> + struct vpu_instance *inst = wave5_to_vpu_inst(priv);
>>> + struct v4l2_format *f = &create->format;
>>> +
>>> + /* Firmware does not support CREATE_BUFS for CAPTURE queues. */
>>> + if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
>>> + dev_dbg(inst->dev->dev,
>>> + "%s: VIDIOC_CREATE_BUFS not supported on CAPTURE queues.\n",
>>> + __func__);
>>> + return -EOPNOTSUPP;
>>> + }
>>> +
>>> + return v4l2_m2m_ioctl_create_bufs(file, priv, create);
>>> +}
>>
>> Regarding the EOPNOTSUPP discussion: I discussed this some more with
>> Nicolas on irc, and we wonder if it isn't better to just drop create_bufs
>> support for the wave5 decoder altogether. Is there any point in supporting
>> it for OUTPUT but not CAPTURE?
>>
>> <snip>
>>
>>> +static const struct v4l2_ioctl_ops wave5_vpu_dec_ioctl_ops = {
>>> + .vidioc_querycap = wave5_vpu_dec_querycap,
>>> + .vidioc_enum_framesizes = wave5_vpu_dec_enum_framesizes,
>>> +
>>> + .vidioc_enum_fmt_vid_cap = wave5_vpu_dec_enum_fmt_cap,
>>> + .vidioc_s_fmt_vid_cap_mplane = wave5_vpu_dec_s_fmt_cap,
>>> + .vidioc_g_fmt_vid_cap_mplane = wave5_vpu_dec_g_fmt_cap,
>>> + .vidioc_try_fmt_vid_cap_mplane = wave5_vpu_dec_try_fmt_cap,
>>> +
>>> + .vidioc_enum_fmt_vid_out = wave5_vpu_dec_enum_fmt_out,
>>> + .vidioc_s_fmt_vid_out_mplane = wave5_vpu_dec_s_fmt_out,
>>> + .vidioc_g_fmt_vid_out_mplane = wave5_vpu_g_fmt_out,
>>> + .vidioc_try_fmt_vid_out_mplane = wave5_vpu_dec_try_fmt_out,
>>> +
>>> + .vidioc_g_selection = wave5_vpu_dec_g_selection,
>>> + .vidioc_s_selection = wave5_vpu_dec_s_selection,
>>> +
>>> + .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs,
>>> + .vidioc_querybuf = v4l2_m2m_ioctl_querybuf,
>>> + .vidioc_create_bufs = wave5_vpu_dec_create_bufs,
>>
>> So this would just be dropped.
>>
>>> + .vidioc_prepare_buf = v4l2_m2m_ioctl_prepare_buf,
>>> + .vidioc_qbuf = v4l2_m2m_ioctl_qbuf,
>>> + .vidioc_expbuf = v4l2_m2m_ioctl_expbuf,
>>> + .vidioc_dqbuf = v4l2_m2m_ioctl_dqbuf,
>>> + .vidioc_streamon = v4l2_m2m_ioctl_streamon,
>>> + .vidioc_streamoff = v4l2_m2m_ioctl_streamoff,
>>> +
>>> + .vidioc_try_decoder_cmd = v4l2_m2m_ioctl_try_decoder_cmd,
>>> + .vidioc_decoder_cmd = wave5_vpu_dec_decoder_cmd,
>>> +
>>> + .vidioc_subscribe_event = wave5_vpu_subscribe_event,
>>> + .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
>>> +};
>>
>> This also means there is no need to document the new EOPNOTSUPP error
>> code in VIDIOC_CREATE_BUFS, or to modify v4l2-compliance.
>>
>> You *do* need to add a comment somewhere explaining why you don't
>> support this ioctl. I think it would be best to do that right after
>> '.vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs,'.
>
> So, besides this issue would you judge the v4l2 layer of the driver to
> be ready? Do you want a reviewed by tag for it or would you take it like
> this as well?
No, it looks good. Please note though that patch 6/8 (dt-bindings) still
needs an Acked/Reviewed-by from the device tree maintainers.
There was a comment on it from Krzysztof.
Regards,
Hans
>
>>
>> Regards,
>>
>> Hans
>
> Sincerly,
> Sebastian
>> _______________________________________________
>> Kernel mailing list -- kernel@mailman.collabora.com
>> To unsubscribe send an email to kernel-leave@mailman.collabora.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v13 5/8] media: chips-media: wave5: Add the v4l2 layer
[not found] ` <20230929-wave5_v13_media_master-v13-5-5ac60ccbf2ce@collabora.com>
2023-10-16 11:57 ` [PATCH v13 5/8] media: chips-media: wave5: Add the v4l2 layer Hans Verkuil
@ 2023-10-17 22:13 ` Ivan Bornyakov
2023-11-02 17:07 ` Deborah Brouwer
1 sibling, 1 reply; 27+ messages in thread
From: Ivan Bornyakov @ 2023-10-17 22:13 UTC (permalink / raw)
To: Sebastian Fricke
Cc: Ivan Bornyakov, Krzysztof Kozlowski, NXP Linux Team, Conor Dooley,
Mauro Carvalho Chehab, Jackson Lee, Hans Verkuil, Sascha Hauer,
Rob Herring, Pengutronix Kernel Team, Shawn Guo, Philipp Zabel,
Nas Chung, Fabio Estevam, linux-media, Tomasz Figa, linux-kernel,
Nicolas Dufresne, kernel, Robert Beckett, devicetree,
linux-arm-kernel, Darren Etheridge
Hi!
On Thu, 12 Oct 2023 13:01:03 +0200, Sebastian Fricke wrote:
> Add the decoder and encoder implementing the v4l2
> API. This patch also adds the Makefile and the VIDEO_WAVE_VPU config
>
> Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com>
> Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
> ---
> drivers/media/platform/chips-media/Kconfig | 1 +
> drivers/media/platform/chips-media/Makefile | 1 +
> drivers/media/platform/chips-media/wave5/Kconfig | 12 +
> drivers/media/platform/chips-media/wave5/Makefile | 10 +
> .../platform/chips-media/wave5/wave5-helper.c | 213 +++
> .../platform/chips-media/wave5/wave5-helper.h | 31 +
> .../platform/chips-media/wave5/wave5-vpu-dec.c | 1953 ++++++++++++++++++++
> .../platform/chips-media/wave5/wave5-vpu-enc.c | 1794 ++++++++++++++++++
> .../media/platform/chips-media/wave5/wave5-vpu.c | 291 +++
> .../media/platform/chips-media/wave5/wave5-vpu.h | 83 +
> .../platform/chips-media/wave5/wave5-vpuapi.h | 2 -
> 11 files changed, 4389 insertions(+), 2 deletions(-)
[...]
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> new file mode 100644
> index 000000000000..74d1fae64fa4
> --- /dev/null
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
[...]
> +static int wave5_vpu_dec_queue_setup(struct vb2_queue *q, unsigned int *num_buffers,
> + unsigned int *num_planes, unsigned int sizes[],
> + struct device *alloc_devs[])
> +{
> + struct vpu_instance *inst = vb2_get_drv_priv(q);
> + struct v4l2_pix_format_mplane inst_format =
> + (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) ? inst->src_fmt : inst->dst_fmt;
> + unsigned int i;
> +
> + dev_dbg(inst->dev->dev, "%s: num_buffers: %u | num_planes: %u | type: %u\n", __func__,
> + *num_buffers, *num_planes, q->type);
> +
> + /* the CREATE_BUFS case */
> + if (*num_planes) {
> + if (inst_format.num_planes != *num_planes)
> + return -EINVAL;
> +
> + for (i = 0; i < *num_planes; i++) {
> + if (sizes[i] < inst_format.plane_fmt[i].sizeimage)
> + return -EINVAL;
> + }
> + /* the REQBUFS case */
> + } else {
> + *num_planes = inst_format.num_planes;
> +
> + if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
> + sizes[0] = inst_format.plane_fmt[0].sizeimage;
> + dev_dbg(inst->dev->dev, "%s: size[0]: %u\n", __func__, sizes[0]);
> + } else if (*num_planes == 1) {
I think, you should also set *num_buffers to be inst->fbc_buf_count for
V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, like this:
} else if (q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
if (*num_buffers < inst->fbc_buf_count)
*num_buffers = inst->fbc_buf_count;
switch (*num_planes) {
case 1:
...
case 2:
...
case 3:
...
}
}
The reason for that is if fbc_buf_count is greater than initial num_buffers,
wave5_vpu_dec_job_ready() wouldn't allow to invoke wave5_vpu_dec_device_run()
Here is a part of the log of described situation:
vdec 20410000.wave515: Switch state from NONE to OPEN.
[...]
vdec 20410000.wave515: wave5_vpu_dec_init_seq: init seq sent (queue 1 : 1)
vdec 20410000.wave515: wave5_vpu_dec_get_seq_info: init seq complete (queue 0 : 0)
[...]
vdec 20410000.wave515: handle_dynamic_resolution_change: width: 4112 height: 3008 profile: 1 | minbuffer: 6
^^^^^^^^ note that minbuffer is 6
vdec 20410000.wave515: Switch state from OPEN to INIT_SEQ.
[...]
vdec 20410000.wave515: decoder command: 1
[...]
vdec 20410000.wave515: wave5_vpu_dec_queue_setup: num_buffers: 4 | num_planes: 0 | type: 9
^^^^^^^^ note that num_buffers is 4
vdec 20410000.wave515: wave5_vpu_dec_queue_setup: size[0]: 18625536
vdec 20410000.wave515: CAPTURE queue must be streaming to queue jobs!
vdec 20410000.wave515: CAPTURE queue must be streaming to queue jobs!
vdec 20410000.wave515: CAPTURE queue must be streaming to queue jobs!
vdec 20410000.wave515: CAPTURE queue must be streaming to queue jobs!
vdec 20410000.wave515: wave5_vpu_dec_buf_queue: type: 9 index: 0 size: ([0]=18625536, [1]= 0, [2]= 0)
vdec 20410000.wave515: wave5_vpu_dec_buf_queue: type: 9 index: 1 size: ([0]=18625536, [1]= 0, [2]= 0)
vdec 20410000.wave515: wave5_vpu_dec_buf_queue: type: 9 index: 2 size: ([0]=18625536, [1]= 0, [2]= 0)
vdec 20410000.wave515: wave5_vpu_dec_buf_queue: type: 9 index: 3 size: ([0]=18625536, [1]= 0, [2]= 0)
vdec 20410000.wave515: wave5_vpu_dec_start_streaming: type: 9
vdec 20410000.wave515: No capture buffer ready to decode!
^^^^^^^^ here v4l2_m2m_num_dst_bufs_ready(m2m_ctx) < (inst->fbc_buf_count - 1), namely 4 < 6
vdec 20410000.wave515: wave5_vpu_dec_stop_streaming: type: 9
vdec 20410000.wave515: streamoff_capture: Setting display flag of buf index: 0, fail: -22
vdec 20410000.wave515: streamoff_capture: Setting display flag of buf index: 1, fail: -22
vdec 20410000.wave515: streamoff_capture: Setting display flag of buf index: 2, fail: -22
vdec 20410000.wave515: streamoff_capture: Setting display flag of buf index: 3, fail: -22
[...]
vdec 20410000.wave515: wave5_vpu_dec_close: dec_finish_seq complete
Altering num_buffers solves the issue for me.
> + if (inst->output_format == FORMAT_422)
> + sizes[0] = inst_format.width * inst_format.height * 2;
> + else
> + sizes[0] = inst_format.width * inst_format.height * 3 / 2;
> + dev_dbg(inst->dev->dev, "%s: size[0]: %u\n", __func__, sizes[0]);
> + } else if (*num_planes == 2) {
> + sizes[0] = inst_format.width * inst_format.height;
> + if (inst->output_format == FORMAT_422)
> + sizes[1] = inst_format.width * inst_format.height;
> + else
> + sizes[1] = inst_format.width * inst_format.height / 2;
> + dev_dbg(inst->dev->dev, "%s: size[0]: %u | size[1]: %u\n",
> + __func__, sizes[0], sizes[1]);
> + } else if (*num_planes == 3) {
> + sizes[0] = inst_format.width * inst_format.height;
> + if (inst->output_format == FORMAT_422) {
> + sizes[1] = inst_format.width * inst_format.height / 2;
> + sizes[2] = inst_format.width * inst_format.height / 2;
> + } else {
> + sizes[1] = inst_format.width * inst_format.height / 4;
> + sizes[2] = inst_format.width * inst_format.height / 4;
> + }
> + dev_dbg(inst->dev->dev, "%s: size[0]: %u | size[1]: %u | size[2]: %u\n",
> + __func__, sizes[0], sizes[1], sizes[2]);
> + }
> + }
> +
> + return 0;
> +}
BTW I'm currently tinkering with your patchset on another C&M IP and would be
gratefull if you Cc me in the future versions of the patchset, if any.
Thanks.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v13 5/8] media: chips-media: wave5: Add the v4l2 layer
2023-10-17 22:13 ` Ivan Bornyakov
@ 2023-11-02 17:07 ` Deborah Brouwer
2023-11-03 10:42 ` Ivan Bornyakov
0 siblings, 1 reply; 27+ messages in thread
From: Deborah Brouwer @ 2023-11-02 17:07 UTC (permalink / raw)
To: Ivan Bornyakov
Cc: Sebastian Fricke, Krzysztof Kozlowski, NXP Linux Team,
Conor Dooley, Mauro Carvalho Chehab, Jackson Lee, Hans Verkuil,
Sascha Hauer, Rob Herring, Pengutronix Kernel Team, Shawn Guo,
Philipp Zabel, Nas Chung, Fabio Estevam, linux-media, Tomasz Figa,
linux-kernel, Nicolas Dufresne, kernel, Robert Beckett,
devicetree, linux-arm-kernel, Darren Etheridge
On Wed, Oct 18, 2023 at 01:13:52AM +0300, Ivan Bornyakov wrote:
> Hi!
Hi Ivan,
>
> On Thu, 12 Oct 2023 13:01:03 +0200, Sebastian Fricke wrote:
> > Add the decoder and encoder implementing the v4l2
> > API. This patch also adds the Makefile and the VIDEO_WAVE_VPU config
> >
> > Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>
> > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com>
> > Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
> > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> > Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
> > ---
> > drivers/media/platform/chips-media/Kconfig | 1 +
> > drivers/media/platform/chips-media/Makefile | 1 +
> > drivers/media/platform/chips-media/wave5/Kconfig | 12 +
> > drivers/media/platform/chips-media/wave5/Makefile | 10 +
> > .../platform/chips-media/wave5/wave5-helper.c | 213 +++
> > .../platform/chips-media/wave5/wave5-helper.h | 31 +
> > .../platform/chips-media/wave5/wave5-vpu-dec.c | 1953 ++++++++++++++++++++
> > .../platform/chips-media/wave5/wave5-vpu-enc.c | 1794 ++++++++++++++++++
> > .../media/platform/chips-media/wave5/wave5-vpu.c | 291 +++
> > .../media/platform/chips-media/wave5/wave5-vpu.h | 83 +
> > .../platform/chips-media/wave5/wave5-vpuapi.h | 2 -
> > 11 files changed, 4389 insertions(+), 2 deletions(-)
>
> [...]
>
> > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> > new file mode 100644
> > index 000000000000..74d1fae64fa4
> > --- /dev/null
> > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
>
> [...]
>
> > +static int wave5_vpu_dec_queue_setup(struct vb2_queue *q, unsigned int *num_buffers,
> > + unsigned int *num_planes, unsigned int sizes[],
> > + struct device *alloc_devs[])
> > +{
> > + struct vpu_instance *inst = vb2_get_drv_priv(q);
> > + struct v4l2_pix_format_mplane inst_format =
> > + (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) ? inst->src_fmt : inst->dst_fmt;
> > + unsigned int i;
> > +
> > + dev_dbg(inst->dev->dev, "%s: num_buffers: %u | num_planes: %u | type: %u\n", __func__,
> > + *num_buffers, *num_planes, q->type);
> > +
> > + /* the CREATE_BUFS case */
> > + if (*num_planes) {
> > + if (inst_format.num_planes != *num_planes)
> > + return -EINVAL;
> > +
> > + for (i = 0; i < *num_planes; i++) {
> > + if (sizes[i] < inst_format.plane_fmt[i].sizeimage)
> > + return -EINVAL;
> > + }
> > + /* the REQBUFS case */
> > + } else {
> > + *num_planes = inst_format.num_planes;
> > +
> > + if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
> > + sizes[0] = inst_format.plane_fmt[0].sizeimage;
> > + dev_dbg(inst->dev->dev, "%s: size[0]: %u\n", __func__, sizes[0]);
> > + } else if (*num_planes == 1) {
>
> I think, you should also set *num_buffers to be inst->fbc_buf_count for
> V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, like this:
>
> } else if (q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
> if (*num_buffers < inst->fbc_buf_count)
> *num_buffers = inst->fbc_buf_count;
>
> switch (*num_planes) {
> case 1:
> ...
> case 2:
> ...
> case 3:
> ...
> }
> }
I was able to reproduce this issue by requesting a small number of buffers
on the CAPTURE queue that was less than inst→fbc_buf_count. When this happens,
wave5_vpu_dec_job_ready() fails here:
(v4l2_m2m_num_dst_bufs_ready(m2m_ctx) < (inst->fbc_buf_count - 1)
I also tested your suggestion to set the *num_buffers to inst→fbc_buf_count
in queue_setup() and it seems to be working well, thanks for this.
I've been working on ways to improve testing for stateful decoders so
I'm curious how you found this issue?
With fluster tests that we use, gstreamer seems to be calculating correct number of
CAPTURE buffers to request, so we wouldn't see this.
>
> The reason for that is if fbc_buf_count is greater than initial num_buffers,
> wave5_vpu_dec_job_ready() wouldn't allow to invoke wave5_vpu_dec_device_run()
>
> Here is a part of the log of described situation:
>
> vdec 20410000.wave515: Switch state from NONE to OPEN.
> [...]
> vdec 20410000.wave515: wave5_vpu_dec_init_seq: init seq sent (queue 1 : 1)
> vdec 20410000.wave515: wave5_vpu_dec_get_seq_info: init seq complete (queue 0 : 0)
> [...]
> vdec 20410000.wave515: handle_dynamic_resolution_change: width: 4112 height: 3008 profile: 1 | minbuffer: 6
> ^^^^^^^^ note that minbuffer is 6
>
> vdec 20410000.wave515: Switch state from OPEN to INIT_SEQ.
> [...]
> vdec 20410000.wave515: decoder command: 1
> [...]
> vdec 20410000.wave515: wave5_vpu_dec_queue_setup: num_buffers: 4 | num_planes: 0 | type: 9
> ^^^^^^^^ note that num_buffers is 4
>
> vdec 20410000.wave515: wave5_vpu_dec_queue_setup: size[0]: 18625536
> vdec 20410000.wave515: CAPTURE queue must be streaming to queue jobs!
> vdec 20410000.wave515: CAPTURE queue must be streaming to queue jobs!
> vdec 20410000.wave515: CAPTURE queue must be streaming to queue jobs!
> vdec 20410000.wave515: CAPTURE queue must be streaming to queue jobs!
> vdec 20410000.wave515: wave5_vpu_dec_buf_queue: type: 9 index: 0 size: ([0]=18625536, [1]= 0, [2]= 0)
> vdec 20410000.wave515: wave5_vpu_dec_buf_queue: type: 9 index: 1 size: ([0]=18625536, [1]= 0, [2]= 0)
> vdec 20410000.wave515: wave5_vpu_dec_buf_queue: type: 9 index: 2 size: ([0]=18625536, [1]= 0, [2]= 0)
> vdec 20410000.wave515: wave5_vpu_dec_buf_queue: type: 9 index: 3 size: ([0]=18625536, [1]= 0, [2]= 0)
> vdec 20410000.wave515: wave5_vpu_dec_start_streaming: type: 9
> vdec 20410000.wave515: No capture buffer ready to decode!
> ^^^^^^^^ here v4l2_m2m_num_dst_bufs_ready(m2m_ctx) < (inst->fbc_buf_count - 1), namely 4 < 6
>
> vdec 20410000.wave515: wave5_vpu_dec_stop_streaming: type: 9
> vdec 20410000.wave515: streamoff_capture: Setting display flag of buf index: 0, fail: -22
> vdec 20410000.wave515: streamoff_capture: Setting display flag of buf index: 1, fail: -22
> vdec 20410000.wave515: streamoff_capture: Setting display flag of buf index: 2, fail: -22
> vdec 20410000.wave515: streamoff_capture: Setting display flag of buf index: 3, fail: -22
> [...]
> vdec 20410000.wave515: wave5_vpu_dec_close: dec_finish_seq complete
>
> Altering num_buffers solves the issue for me.
>
> > + if (inst->output_format == FORMAT_422)
> > + sizes[0] = inst_format.width * inst_format.height * 2;
> > + else
> > + sizes[0] = inst_format.width * inst_format.height * 3 / 2;
> > + dev_dbg(inst->dev->dev, "%s: size[0]: %u\n", __func__, sizes[0]);
> > + } else if (*num_planes == 2) {
> > + sizes[0] = inst_format.width * inst_format.height;
> > + if (inst->output_format == FORMAT_422)
> > + sizes[1] = inst_format.width * inst_format.height;
> > + else
> > + sizes[1] = inst_format.width * inst_format.height / 2;
> > + dev_dbg(inst->dev->dev, "%s: size[0]: %u | size[1]: %u\n",
> > + __func__, sizes[0], sizes[1]);
> > + } else if (*num_planes == 3) {
> > + sizes[0] = inst_format.width * inst_format.height;
> > + if (inst->output_format == FORMAT_422) {
> > + sizes[1] = inst_format.width * inst_format.height / 2;
> > + sizes[2] = inst_format.width * inst_format.height / 2;
> > + } else {
> > + sizes[1] = inst_format.width * inst_format.height / 4;
> > + sizes[2] = inst_format.width * inst_format.height / 4;
> > + }
> > + dev_dbg(inst->dev->dev, "%s: size[0]: %u | size[1]: %u | size[2]: %u\n",
> > + __func__, sizes[0], sizes[1], sizes[2]);
> > + }
> > + }
> > +
> > + return 0;
> > +}
>
> BTW I'm currently tinkering with your patchset on another C&M IP and would be
> gratefull if you Cc me in the future versions of the patchset, if any.
Yes, sorry for missing you on v13, thanks for taking the time to review.
Deborah
>
> Thanks.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v13 5/8] media: chips-media: wave5: Add the v4l2 layer
2023-11-02 17:07 ` Deborah Brouwer
@ 2023-11-03 10:42 ` Ivan Bornyakov
0 siblings, 0 replies; 27+ messages in thread
From: Ivan Bornyakov @ 2023-11-03 10:42 UTC (permalink / raw)
To: Deborah Brouwer
Cc: Sebastian Fricke, Krzysztof Kozlowski, NXP Linux Team,
Conor Dooley, Mauro Carvalho Chehab, Jackson Lee, Hans Verkuil,
Sascha Hauer, Rob Herring, Pengutronix Kernel Team, Shawn Guo,
Philipp Zabel, Nas Chung, Fabio Estevam, linux-media, Tomasz Figa,
linux-kernel, Nicolas Dufresne, kernel, Robert Beckett,
devicetree, linux-arm-kernel, Darren Etheridge
Hi, Deborah
On Thu, Nov 02, 2023 at 10:07:59AM -0700, Deborah Brouwer wrote:
> On Wed, Oct 18, 2023 at 01:13:52AM +0300, Ivan Bornyakov wrote:
> > Hi!
>
> Hi Ivan,
>
> >
> > On Thu, 12 Oct 2023 13:01:03 +0200, Sebastian Fricke wrote:
> > > Add the decoder and encoder implementing the v4l2
> > > API. This patch also adds the Makefile and the VIDEO_WAVE_VPU config
> > >
> > > Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>
> > > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > > Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com>
> > > Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
> > > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> > > Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
> > > ---
> > > drivers/media/platform/chips-media/Kconfig | 1 +
> > > drivers/media/platform/chips-media/Makefile | 1 +
> > > drivers/media/platform/chips-media/wave5/Kconfig | 12 +
> > > drivers/media/platform/chips-media/wave5/Makefile | 10 +
> > > .../platform/chips-media/wave5/wave5-helper.c | 213 +++
> > > .../platform/chips-media/wave5/wave5-helper.h | 31 +
> > > .../platform/chips-media/wave5/wave5-vpu-dec.c | 1953 ++++++++++++++++++++
> > > .../platform/chips-media/wave5/wave5-vpu-enc.c | 1794 ++++++++++++++++++
> > > .../media/platform/chips-media/wave5/wave5-vpu.c | 291 +++
> > > .../media/platform/chips-media/wave5/wave5-vpu.h | 83 +
> > > .../platform/chips-media/wave5/wave5-vpuapi.h | 2 -
> > > 11 files changed, 4389 insertions(+), 2 deletions(-)
> >
> > [...]
> >
> > > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> > > new file mode 100644
> > > index 000000000000..74d1fae64fa4
> > > --- /dev/null
> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> >
> > [...]
> >
> > > +static int wave5_vpu_dec_queue_setup(struct vb2_queue *q, unsigned int *num_buffers,
> > > + unsigned int *num_planes, unsigned int sizes[],
> > > + struct device *alloc_devs[])
> > > +{
> > > + struct vpu_instance *inst = vb2_get_drv_priv(q);
> > > + struct v4l2_pix_format_mplane inst_format =
> > > + (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) ? inst->src_fmt : inst->dst_fmt;
> > > + unsigned int i;
> > > +
> > > + dev_dbg(inst->dev->dev, "%s: num_buffers: %u | num_planes: %u | type: %u\n", __func__,
> > > + *num_buffers, *num_planes, q->type);
> > > +
> > > + /* the CREATE_BUFS case */
> > > + if (*num_planes) {
> > > + if (inst_format.num_planes != *num_planes)
> > > + return -EINVAL;
> > > +
> > > + for (i = 0; i < *num_planes; i++) {
> > > + if (sizes[i] < inst_format.plane_fmt[i].sizeimage)
> > > + return -EINVAL;
> > > + }
> > > + /* the REQBUFS case */
> > > + } else {
> > > + *num_planes = inst_format.num_planes;
> > > +
> > > + if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
> > > + sizes[0] = inst_format.plane_fmt[0].sizeimage;
> > > + dev_dbg(inst->dev->dev, "%s: size[0]: %u\n", __func__, sizes[0]);
> > > + } else if (*num_planes == 1) {
> >
> > I think, you should also set *num_buffers to be inst->fbc_buf_count for
> > V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, like this:
> >
> > } else if (q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
> > if (*num_buffers < inst->fbc_buf_count)
> > *num_buffers = inst->fbc_buf_count;
> >
> > switch (*num_planes) {
> > case 1:
> > ...
> > case 2:
> > ...
> > case 3:
> > ...
> > }
> > }
>
> I was able to reproduce this issue by requesting a small number of buffers
> on the CAPTURE queue that was less than inst→fbc_buf_count. When this happens,
> wave5_vpu_dec_job_ready() fails here:
> (v4l2_m2m_num_dst_bufs_ready(m2m_ctx) < (inst->fbc_buf_count - 1)
>
> I also tested your suggestion to set the *num_buffers to inst→fbc_buf_count
> in queue_setup() and it seems to be working well, thanks for this.
>
> I've been working on ways to improve testing for stateful decoders so
> I'm curious how you found this issue?
>
> With fluster tests that we use, gstreamer seems to be calculating correct number of
> CAPTURE buffers to request, so we wouldn't see this.
>
I use v4l2-ctl and ffmpeg for testing the decoder.
The first time I've encountered the discussed issue was when v4l2-ctl was
able to decode one file but not the other with bigger resolution.
For v4l2-ctl command line will look like this:
v4l2-ctl --stream-from src-file.h265 --stream-to out-file.yuv --stream-mmap --stream-out-mmap
For ffmpeg command line will look like this:
ffmpeg -c:v hevc_v4l2m2m -i src-file.h265 -fps_mode passthrough out-file.yuv
Also for ffmpeg there are options -num_output_buffers and -num_capture_buffers
to set initial numbers of buffers in OUTPUT and CAPTURE queues, by
default they are set to 20 or so.
> >
> > The reason for that is if fbc_buf_count is greater than initial num_buffers,
> > wave5_vpu_dec_job_ready() wouldn't allow to invoke wave5_vpu_dec_device_run()
> >
> > Here is a part of the log of described situation:
> >
> > vdec 20410000.wave515: Switch state from NONE to OPEN.
> > [...]
> > vdec 20410000.wave515: wave5_vpu_dec_init_seq: init seq sent (queue 1 : 1)
> > vdec 20410000.wave515: wave5_vpu_dec_get_seq_info: init seq complete (queue 0 : 0)
> > [...]
> > vdec 20410000.wave515: handle_dynamic_resolution_change: width: 4112 height: 3008 profile: 1 | minbuffer: 6
> > ^^^^^^^^ note that minbuffer is 6
> >
> > vdec 20410000.wave515: Switch state from OPEN to INIT_SEQ.
> > [...]
> > vdec 20410000.wave515: decoder command: 1
> > [...]
> > vdec 20410000.wave515: wave5_vpu_dec_queue_setup: num_buffers: 4 | num_planes: 0 | type: 9
> > ^^^^^^^^ note that num_buffers is 4
> >
> > vdec 20410000.wave515: wave5_vpu_dec_queue_setup: size[0]: 18625536
> > vdec 20410000.wave515: CAPTURE queue must be streaming to queue jobs!
> > vdec 20410000.wave515: CAPTURE queue must be streaming to queue jobs!
> > vdec 20410000.wave515: CAPTURE queue must be streaming to queue jobs!
> > vdec 20410000.wave515: CAPTURE queue must be streaming to queue jobs!
> > vdec 20410000.wave515: wave5_vpu_dec_buf_queue: type: 9 index: 0 size: ([0]=18625536, [1]= 0, [2]= 0)
> > vdec 20410000.wave515: wave5_vpu_dec_buf_queue: type: 9 index: 1 size: ([0]=18625536, [1]= 0, [2]= 0)
> > vdec 20410000.wave515: wave5_vpu_dec_buf_queue: type: 9 index: 2 size: ([0]=18625536, [1]= 0, [2]= 0)
> > vdec 20410000.wave515: wave5_vpu_dec_buf_queue: type: 9 index: 3 size: ([0]=18625536, [1]= 0, [2]= 0)
> > vdec 20410000.wave515: wave5_vpu_dec_start_streaming: type: 9
> > vdec 20410000.wave515: No capture buffer ready to decode!
> > ^^^^^^^^ here v4l2_m2m_num_dst_bufs_ready(m2m_ctx) < (inst->fbc_buf_count - 1), namely 4 < 6
> >
> > vdec 20410000.wave515: wave5_vpu_dec_stop_streaming: type: 9
> > vdec 20410000.wave515: streamoff_capture: Setting display flag of buf index: 0, fail: -22
> > vdec 20410000.wave515: streamoff_capture: Setting display flag of buf index: 1, fail: -22
> > vdec 20410000.wave515: streamoff_capture: Setting display flag of buf index: 2, fail: -22
> > vdec 20410000.wave515: streamoff_capture: Setting display flag of buf index: 3, fail: -22
> > [...]
> > vdec 20410000.wave515: wave5_vpu_dec_close: dec_finish_seq complete
> >
> > Altering num_buffers solves the issue for me.
> >
> > > + if (inst->output_format == FORMAT_422)
> > > + sizes[0] = inst_format.width * inst_format.height * 2;
> > > + else
> > > + sizes[0] = inst_format.width * inst_format.height * 3 / 2;
> > > + dev_dbg(inst->dev->dev, "%s: size[0]: %u\n", __func__, sizes[0]);
> > > + } else if (*num_planes == 2) {
> > > + sizes[0] = inst_format.width * inst_format.height;
> > > + if (inst->output_format == FORMAT_422)
> > > + sizes[1] = inst_format.width * inst_format.height;
> > > + else
> > > + sizes[1] = inst_format.width * inst_format.height / 2;
> > > + dev_dbg(inst->dev->dev, "%s: size[0]: %u | size[1]: %u\n",
> > > + __func__, sizes[0], sizes[1]);
> > > + } else if (*num_planes == 3) {
> > > + sizes[0] = inst_format.width * inst_format.height;
> > > + if (inst->output_format == FORMAT_422) {
> > > + sizes[1] = inst_format.width * inst_format.height / 2;
> > > + sizes[2] = inst_format.width * inst_format.height / 2;
> > > + } else {
> > > + sizes[1] = inst_format.width * inst_format.height / 4;
> > > + sizes[2] = inst_format.width * inst_format.height / 4;
> > > + }
> > > + dev_dbg(inst->dev->dev, "%s: size[0]: %u | size[1]: %u | size[2]: %u\n",
> > > + __func__, sizes[0], sizes[1], sizes[2]);
> > > + }
> > > + }
> > > +
> > > + return 0;
> > > +}
> >
> > BTW I'm currently tinkering with your patchset on another C&M IP and would be
> > gratefull if you Cc me in the future versions of the patchset, if any.
>
> Yes, sorry for missing you on v13, thanks for taking the time to review.
>
> Deborah
>
> >
> > Thanks.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20230929-wave5_v13_media_master-v13-4-5ac60ccbf2ce@collabora.com>]
* Re: [PATCH v13 4/8] media: chips-media: wave5: Add vpuapi layer
[not found] ` <20230929-wave5_v13_media_master-v13-4-5ac60ccbf2ce@collabora.com>
@ 2023-10-22 16:27 ` Christophe JAILLET
0 siblings, 0 replies; 27+ messages in thread
From: Christophe JAILLET @ 2023-10-22 16:27 UTC (permalink / raw)
To: Sebastian Fricke, Krzysztof Kozlowski, NXP Linux Team,
Conor Dooley, Mauro Carvalho Chehab, Jackson Lee, Hans Verkuil,
Sascha Hauer, Rob Herring, Pengutronix Kernel Team, Shawn Guo,
Philipp Zabel, Nas Chung, Fabio Estevam
Cc: linux-media, Tomasz Figa, linux-kernel, Nicolas Dufresne, kernel,
Robert Beckett, devicetree, linux-arm-kernel, Darren Etheridge
Le 12/10/2023 à 13:01, Sebastian Fricke a écrit :
> From: Nas Chung <nas.chung@chipsnmedia.com>
>
> Add the vpuapi layer of the wave5 codec driver.
> This layer is used to configure the hardware according
> to the parameters.
>
> Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
> Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
> ---
...
> +int wave5_vpu_dec_clr_disp_flag(struct vpu_instance *inst, int index)
> +{
> + struct dec_info *p_dec_info = &inst->codec_info->dec_info;
> + int ret = 0;
Nit: No need to init.
> + struct vpu_device *vpu_dev = inst->dev;
> +
> + if (index >= p_dec_info->num_of_display_fbs)
> + return -EINVAL;
> +
> + ret = mutex_lock_interruptible(&vpu_dev->hw_lock);
> + if (ret)
> + return ret;
> + ret = wave5_dec_clr_disp_flag(inst, index);
> + mutex_unlock(&vpu_dev->hw_lock);
> +
> + return ret;
> +}
...
> +int wave5_vpu_dec_give_command(struct vpu_instance *inst, enum codec_command cmd, void *parameter)
> +{
> + struct dec_info *p_dec_info = &inst->codec_info->dec_info;
> + int ret = 0;
> +
> + switch (cmd) {
> + case DEC_GET_QUEUE_STATUS: {
> + struct queue_status_info *queue_info = parameter;
> +
> + queue_info->instance_queue_count = p_dec_info->instance_queue_count;
> + queue_info->report_queue_count = p_dec_info->report_queue_count;
> + break;
> + }
> + case DEC_RESET_FRAMEBUF_INFO: {
> + int i;
> +
> + for (i = 0; i < MAX_REG_FRAME; i++) {
> + ret = wave5_vpu_dec_reset_framebuffer(inst, i);
> + if (ret)
> + break;
> + }
> +
> + for (i = 0; i < MAX_REG_FRAME; i++) {
> + ret = reset_auxiliary_buffers(inst, i);
> + if (ret)
> + break;
> + }
> +
> + wave5_vdi_free_dma_memory(inst->dev, &p_dec_info->vb_task);
> + break;
> + }
> + case DEC_GET_SEQ_INFO: {
> + struct dec_initial_info *seq_info = parameter;
> +
> + *seq_info = p_dec_info->initial_info;
> + break;
> + }
> +
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
return ret;
?
CJ
> +}
...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread