[PATCH v13 1/8] media: v4l2: Add ignore_cap_streaming flag

[PATCH v13 1/8] media: v4l2: Add ignore_cap_streaming flag

[Sebastian Fricke]

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

[PATCH v13 0/8] Wave5 codec driver

[Sebastian Fricke]

The Wave5 codec driver is a stateful encoder/decoder.
It is found on the J721S2 SoC, JH7100 SoC, ssd202d SoC. Etc.
But current test report is based on J721S2 SoC and pre-silicon FPGA.

The driver currently supports V4L2_PIX_FMT_HEVC, V4L2_PIX_FMT_H264
for both encoding and decoding.

This driver has so far been tested on J721S2 EVM board and pre-silicon
FPGA.

Testing on J721S2 EVM board shows it working fine both decoder and
encoder.
The driver is successfully working with gstreamer v4l2 good-plugin
without any modification.

We have based the tree on top of the latest
https://git.linuxtv.org/media_tree.git repository.

M2M framework modification:
===========================

(Patch 1 & 2)
Added a set of changes to enable ignoring the streaming state of the CAPTURE
queue while checking whether a new job can be inserted into the ready queue.
The use-case we encountered for this is a stateful decoder/encoder chip with a
tight connection to a firmware. On that firmware the bitstream is first
analyzed and the firmware produces output indicating the requirements for the
output of the decode (e.g. the buffers of the CAPTURE queue). We want to be
able to perform this action within the M2M job workflow in order to rely on the
concurrency safety provided by the M2M jobs.

v4l2-compliance results:
========================

v4l2-compliance 1.25.0-5100, 64 bits, 64-bit time_t
v4l2-compliance SHA: 8bf6cba8c0ef 2023-10-10 12:50:46

Buffer ioctls:
		fail: v4l2-test-buffers.cpp(702): doioctl(node, VIDIOC_CREATE_BUFS, &crbufs)
	test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL
	test VIDIOC_EXPBUF: OK
	test Requests: OK (Not Supported)

Total for wave5-dec device /dev/video0: 45, Succeeded: 44, Failed: 1, Warnings: 0

Total for wave5-enc device /dev/video1: 45, Succeeded: 45, Failed: 0, Warnings: 0

Fluster test results:
=====================

Running test suite JCT-VC-HEVC_V1 with decoder GStreamer-H.265-V4L2-Gst1.0
Using 1 parallel job(s)
Ran 133/147 tests successfully               in 182.673 secs

(1 test fails because of a missing frame and slight corruption
(DeltaQP_A_BRCM_4), 2 tests fail because of sizes which are incompatible with
the IP, 11 tests fail because of unsupported 10 bit format)

Running test suite JVT-AVC_V1 with decoder GStreamer-H.264-V4L2-Gst1.0
Using 1 parallel job(s)
Ran 78/135 tests successfully               in 140.128 secs

(57 fail because the hardware is unable to decode
MBAFF / FMO / Field / Extended profile streams.)

Encoder testing:
================

Among other tests the driver is able to produce valid output with the following test:
`gst-launch-1.0 videotestsrc num-buffers=300 ! v4l2h264enc ! h264parse ! qtmux ! filesink location=test.mp4`

Known missing features:
=======================

* 48 bit memory addressing
* Runtime suspend/resume support
* Encoder support for the YUV 422 format
* Support for the Background Detection encoder feature of the IP block
* Support for the Region Of Interest encoder feature of the IP block

Changes since v12:
==================

* Add patch "arm64: dts: ti: k3-j721s2-main: add wave5 video encoder/decoder node"
* Fix locking issues when mutex hw_lock is taken while holding state_spinlock

* For [PATCH v12 1/7] media: v4l2: Add ignore_streaming flag:
  - rename flag: ignore_streaming -> ignore_cap_streaming
  - move flag from v4l2_m2m_queue_ctx to v4l2_m2m_ctx

* For [PATCH v12 2/7] media: v4l2: Allow M2M job queuing w/o streaming
  CAP queue:
  - use renamed ignore_cap_streaming flag

* For [PATCH v12 4/7] media: chips-media: wave5: Add vpuapi layer
  - refactor: wave5_bit_issue_command() accepts additional parameters
  - refactor: use wave5_send_query() where possible, clean up return values
  - fix: initialize additional registers and fix typo
  - fix: do not warn when rotation isn't enabled
  - fix: prevent decoder close failure by returning 0 from reset_auxiliary_buffers

* For [PATCH v12 5/7] media: chips-media: wave5: Add the v4l2 layer
  - reduce interrupt handling to one function called by the irq handler thread
  - use dma_set_mask_and_coherent() and check if it returns an error
  - extend copyright: 2021 -> 2021-2023
  - remove redundant work (e.g. type checking) already handled by v4l2 core
  - queue_setup: remove src_buf_count and dst_buf_count and rely on v4l2 counts
  - create_bufs: return EOPNOTSUPP when ioctl is unsupported by a queue
  - start_streaming: return buffers properly on error
  - minor fixes for typos, adding comments and small refactoring improvements

* For [PATCH v12 6/7] dt-bindings: media: wave5: add yaml devicetree bindings
  - rename patch "media: dt-bindings: wave5: add Chips&Media 521c codec IP support"
  - remove clock-names; fix formatting

Changes since v11:
==================

* Major rework of the decoder
- Fix concurrency issues by moving the commands that invoke actions on the
firmware into the device_run function from M2M, effectively causing M2M to
take care of the concurrency via its job queue.
- In order to do that device_run needs to be able to run before both queues
  are ready, as a sequence needs to be initialized on the firmware to get
  the required buffer count & communicate the result back to userspace,
  thus we added a routine to allow jobs to be queued in M2M with only one
  of two queues being started. (See: "Add ignore_streaming flag to M2M"
  series)
- Add support for more output formats (YUV422P, YUV422M, NV16, NV16M, NV61,
NV61M)
- Add proper state switch function to verify state switches
- Simplify the queue_setup function and move the decoder opening to STREAMON
as request in the review of V11
- Enable handling dynamic resolution change and seeking
- Remove thumbnail mode

* Similar reworks on the encoder
- Move encoding into device_run and encoder opening + sequence intialization
to STREAMON, this change simplifies the encoder as it previously was able
to run multiple encode jobs simultaneously

* Remove unused configurations and support for untested hardware
- Remove the ability to configure the endianess of memory writes as only a
single hardware can be tested so far
- Remove the ability to configure the bitstream_mode, as the driver is
currently hardcoded for the INTERRUPT mode
- Remove support for all CODECS, that were not tested (everything besides
H264/HEVC encoder + decoder)
- The encoder currently contains a lot of configurable parameter, which are
hard-coded in the `wave5_set_enc_openparam` function, remove all parameters
which aren't currently specified in that configuration
- Remove unused rotation and mirroring options from the decoder

* Add FLUSH_FIRMWARE firmware command
* Refactoring
* Add wrappers for frequently used routines or to make the code more
descriptive
- Wrapper for firmware command calling `send_firmware_command`
- Wrappers to intialize the sequence and to set up the framebuffers in the
  decoder and encoder
- Using more general kernel functions and macros in various places
* Add Macros for constant values and bit shifts
* Fix typos and improve comments

* Fix bug with M2M instance stored in the driver instance, multiple
simultaneous instances would overwrite the M2M handler of each other. Use a
M2M handler per device instead to avoid overwriting.
* Adjust TRACE_PATH in the coda directory as highlighted in the review of V11
* Applied requested changes from review to the devicetree bindings file
(the bindings check didn't return any warnings or errors)

Changes since v10:
==================

* Remove structure member from the encoder and decoder output info
structs, that have assigned values from the registers but aren't used
in the driver, add comments to describe the register values in the
register definitions
* Fix issue with decoding videos with a dimension where the height is
not a multiple of 16 (270, 360, 540, 1024 etc.)
* Fix incorrect variable format identifiers in printks
* Use debug logs in loops to avoid flooding the message log
* Use the swap() function instead of manual swapping of two values
* Add extended controls for the encoder
* Fix control flow issue while handling bitstream buffers, where an
error while writing the source buffer into the hardware ring buffer
would result in skipping the problematic buffer, which in turn causes
a reordering of source buffers
* Use the rectangle format as described by the hardware, the hardware
uses for rectangles like the display rectangle 4 offsets (top, bottom,
left, right), which depict the offset from the respective edge. Use
this format instead of implicitly converting the bottom and right
attributes to width and height attributes.
* Return an error upon reading the sequence header while STREAMON
* Squash the VDI and the VPUAPI layer commits as they had circular
dependencies

changes since v9:
=================

* Move from staging to the media directory
* Move coda driver to sub-directory

* Fixes:
* Use platform_get_irq instead of platform_get_resource to fetch the IRQ

* General cleanups:
* Add missing error messages to error conditions
* Improve messages/variable names/comments, align parameter names across the driver
* Use macros instead of magic numbers in multiple occassions
* Reduce code duplication in multiple places
* Fix whitespace, newline and tab alignment issues
* Remove unused struct fields & commented out code
* Convert signed integers to unsigned if signed is not necessary
* Convert int/unsigned int to s32/u32, when the variable is assigned to the
return of a register read or provided as a parameter for a register write
(and vice versa)
* Fix incorrect bitwise operators where logical operators are appropriate
* Multiple smaller changes

* Generalization:
* Add new helper file providing generalized routines for vpu-dec & vpu-enc
* Generalize luma & chroma table size calculation and stride calculation

* Resource cleanup and error handling:
* Add error handling to all calls with ignored return codes
* Handle DMA resource cleanup properly
* Fix insufficient instance cleanup while opening dec/enc

Changes since v8:
=================

* add 'wave5' to DEV_NAME
* update to support Multi-stream
* update to support loop test/dynamic resolution change
* remove unnecessary memset, g_volatile, old version option

Changes since v7:
=================

* update v4l2-compliance test report
* fix build error on linux-kernel 5.18.0-rc4

Changes since v6:
=================

* update TODO file
* get sram info from device tree

Changes since v5:
=================

* support NV12/NV21 pixelformat for encoder and decoder
* handle adnormal exit and EOS

Changes since v4:
=================

* refactor functions in wave5-hw and fix bug reported by Daniel Palmer
* rename functions and variables to better names
* change variable types such as replacing s32 with u32 and int with bool
* as appropriate

Changes since v3:
=================

* Fixing all issues commented by Dan Carpenter
* Change file names to have wave5- prefix
* In wave5_vpu_probe, enable the clocks before reading registers, as
* commented from Daniel Palmer
* Add more to the TODO list,

Changes since v2:
=================

Main fixes includes:
* change the yaml and dirver code to support up to 4 clks (instead of
* one)
* fix Kconfig format
* remove unneeded cast,
* change var types
* change var names, func names
* checkpatch fixes

Changes since v1:
=================

Fix changes due to comments from Ezequiel and Dan Carpenter. Main fixes
inclueds:
* move all files to one dir 'wave5'
* replace private error codes with standard error codes
* fix extra spaces
* various checkpatch fixes
* replace private 'DPRINTK' macro with standard 'dev_err/dbg ..'
* fix error handling
* add more possible fixes to the TODO file

To: Mauro Carvalho Chehab <mchehab@kernel.org>
To: Hans Verkuil <hverkuil@xs4all.nl>
To: Philipp Zabel <p.zabel@pengutronix.de>
To: Shawn Guo <shawnguo@kernel.org>
To: Sascha Hauer <s.hauer@pengutronix.de>
To: Pengutronix Kernel Team <kernel@pengutronix.de>
To: Fabio Estevam <festevam@gmail.com>
To: NXP Linux Team <linux-imx@nxp.com>
To: Jackson Lee <jackson.lee@chipsnmedia.com>
To: Rob Herring <robh+dt@kernel.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
To: Conor Dooley <conor+dt@kernel.org>
Cc: linux-media@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: devicetree@vger.kernel.org
Cc: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Cc: Robert Beckett <bob.beckett@collabora.com>
Cc: Nas Chung <nas.chung@chipsnmedia.com>
CC: Tomasz Figa <tfiga@chromium.org>
CC: kernel@collabora.com
Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>

---
Darren Etheridge (1):
      arm64: dts: ti: k3-j721s2-main: add wave5 video encoder/decoder node

Nas Chung (1):
      media: chips-media: wave5: Add vpuapi layer

Robert Beckett (2):
      media: dt-bindings: wave5: add Chips&Media 521c codec IP support
      media: chips-media: wave5: Add wave5 driver to maintainers file

Sebastian Fricke (4):
      media: v4l2: Add ignore_cap_streaming flag
      media: v4l2: Allow M2M job queuing w/o streaming CAP queue
      media: platform: chips-media: Move Coda to separate folder
      media: chips-media: wave5: Add the v4l2 layer

 .../devicetree/bindings/media/cnm,wave5.yaml       |   60 +
 MAINTAINERS                                        |   10 +-
 arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi         |   14 +
 drivers/media/platform/chips-media/Kconfig         |   18 +-
 drivers/media/platform/chips-media/Makefile        |    6 +-
 drivers/media/platform/chips-media/coda/Kconfig    |   18 +
 drivers/media/platform/chips-media/coda/Makefile   |    6 +
 .../platform/chips-media/{ => coda}/coda-bit.c     |    0
 .../platform/chips-media/{ => coda}/coda-common.c  |    0
 .../platform/chips-media/{ => coda}/coda-gdi.c     |    0
 .../platform/chips-media/{ => coda}/coda-h264.c    |    0
 .../platform/chips-media/{ => coda}/coda-jpeg.c    |    0
 .../platform/chips-media/{ => coda}/coda-mpeg2.c   |    0
 .../platform/chips-media/{ => coda}/coda-mpeg4.c   |    0
 .../media/platform/chips-media/{ => coda}/coda.h   |    0
 .../platform/chips-media/{ => coda}/coda_regs.h    |    0
 .../platform/chips-media/{ => coda}/imx-vdoa.c     |    0
 .../platform/chips-media/{ => coda}/imx-vdoa.h     |    0
 .../media/platform/chips-media/{ => coda}/trace.h  |    2 +-
 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 +
 .../media/platform/chips-media/wave5/wave5-hw.c    | 2554 ++++++++++++++++++++
 .../platform/chips-media/wave5/wave5-regdefine.h   |  732 ++++++
 .../media/platform/chips-media/wave5/wave5-vdi.c   |  205 ++
 .../media/platform/chips-media/wave5/wave5-vdi.h   |   35 +
 .../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.c      |  960 ++++++++
 .../platform/chips-media/wave5/wave5-vpuapi.h      |  870 +++++++
 .../platform/chips-media/wave5/wave5-vpuconfig.h   |   77 +
 .../platform/chips-media/wave5/wave5-vpuerror.h    |  292 +++
 drivers/media/platform/chips-media/wave5/wave5.h   |  114 +
 drivers/media/v4l2-core/v4l2-mem2mem.c             |    9 +-
 include/media/v4l2-mem2mem.h                       |    7 +
 38 files changed, 10351 insertions(+), 25 deletions(-)
---
base-commit: 2c1bae27df787c9535e48cc27bbd11c3c3e0a235
change-id: 20230929-wave5_v13_media_master-e0b8d0044cd4

Best regards,
-- 
Sebastian Fricke <sebastian.fricke@collabora.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v13 2/8] media: v4l2: Allow M2M job queuing w/o streaming CAP queue

[Sebastian Fricke]

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

[PATCH v13 3/8] media: platform: chips-media: Move Coda to separate folder

[Sebastian Fricke]

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

[PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support

[Sebastian Fricke]

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

[PATCH v13 7/8] media: chips-media: wave5: Add wave5 driver to maintainers file

[Sebastian Fricke]

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

[PATCH v13 8/8] arm64: dts: ti: k3-j721s2-main: add wave5 video encoder/decoder node

[Sebastian Fricke]

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

Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support

[Krzysztof Kozlowski]

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

Re: [PATCH v13 5/8] media: chips-media: wave5: Add the v4l2 layer

[Hans Verkuil]

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

Re: [PATCH v13 5/8] media: chips-media: wave5: Add the v4l2 layer

[Sebastian Fricke]

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

Re: [PATCH v13 5/8] media: chips-media: wave5: Add the v4l2 layer

[Hans Verkuil]

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

Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support

[Rob Herring]

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

Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support

[Devarsh Thakkar]

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

Re: [PATCH v13 5/8] media: chips-media: wave5: Add the v4l2 layer

[Ivan Bornyakov]

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

Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support

[Sebastian Fricke]

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

Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support

[Sebastian Fricke]

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

Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support

[Krzysztof Kozlowski]

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

Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support

[Krzysztof Kozlowski]

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

Re: [PATCH v13 4/8] media: chips-media: wave5: Add vpuapi layer

[Christophe JAILLET]

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

Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support

[Sebastian Fricke]

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

Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support

[Krzysztof Kozlowski]

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

Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support

[Sebastian Fricke]

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

Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support

[Krzysztof Kozlowski]

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

Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support

[Sebastian Fricke]

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

Re: [PATCH v13 6/8] media: dt-bindings: wave5: add Chips&Media 521c codec IP support

[Krzysztof Kozlowski]

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

Re: [PATCH v13 5/8] media: chips-media: wave5: Add the v4l2 layer

[Deborah Brouwer]

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

Re: [PATCH v13 5/8] media: chips-media: wave5: Add the v4l2 layer

[Ivan Bornyakov]

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