mbox series

[v9,0/6] staging: media: wave5: add wave5 codec driver

Message ID 20220628110821.716-1-nas.chung@chipsnmedia.com (mailing list archive)
Headers show
Series staging: media: wave5: add wave5 codec driver | expand

Message

Nas Chung June 28, 2022, 11:08 a.m. UTC 
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.

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.

Testing on FPGA also shows it working fine, though the FPGA uses polled
interrupts and copied buffers between the host and it's on board RAM.

# v4l2-compliance -d0
v4l2-compliance 1.23.0-4923, 64 bits, 64-bit time_t
v4l2-compliance SHA: 163144712a46 2022-04-25 05:31:44

Compliance test for wave5-dec device /dev/video0:

Driver Info:
	Driver name      : wave5-dec
	Card type        : wave5-dec
	Bus info         : platform:wave5-dec
	Driver version   : 5.18.0
	Capabilities     : 0x84204000
		Video Memory-to-Memory Multiplanar
		Streaming
		Extended Pix Format
		Device Capabilities
	Device Caps      : 0x04204000
		Video Memory-to-Memory Multiplanar
		Streaming
		Extended Pix Format
	Detected Stateful Decoder

Required ioctls:
	test VIDIOC_QUERYCAP: OK
	test invalid ioctls: OK

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

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

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

Output ioctls:
	test VIDIOC_G/S_MODULATOR: OK (Not Supported)
	test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
	test VIDIOC_ENUMAUDOUT: OK (Not Supported)
	test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
	test VIDIOC_G/S_AUDOUT: OK (Not Supported)
	Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
	test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
	test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
	test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
	test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls:
	test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
	test VIDIOC_QUERYCTRL: OK
	test VIDIOC_G/S_CTRL: OK
	test VIDIOC_G/S/TRY_EXT_CTRLS: OK
	test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
	test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
	Standard Controls: 2 Private Controls: 1

Format ioctls:
	test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
	test VIDIOC_G/S_PARM: OK (Not Supported)
	test VIDIOC_G_FBUF: OK (Not Supported)
	test VIDIOC_G_FMT: OK
	test VIDIOC_TRY_FMT: OK
	test VIDIOC_S_FMT: OK
	test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
	test Cropping: OK (Not Supported)
	test Composing: OK
	test Scaling: OK (Not Supported)

Codec ioctls:
	test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
	test VIDIOC_G_ENC_INDEX: OK (Not Supported)
	test VIDIOC_(TRY_)DECODER_CMD: OK

Buffer ioctls:
	test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
	test VIDIOC_EXPBUF: OK
	test Requests: OK (Not Supported)

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

# v4l2-compliance -d1
v4l2-compliance 1.23.0-4923, 64 bits, 64-bit time_t
v4l2-compliance SHA: 163144712a46 2022-04-25 05:31:44

Compliance test for wave5-enc device /dev/video1:

Driver Info:
	Driver name      : wave5-enc
	Card type        : wave5-enc
	Bus info         : platform:wave5-enc
	Driver version   : 5.18.0
	Capabilities     : 0x84204000
		Video Memory-to-Memory Multiplanar
		Streaming
		Extended Pix Format
		Device Capabilities
	Device Caps      : 0x04204000
		Video Memory-to-Memory Multiplanar
		Streaming
		Extended Pix Format
	Detected Stateful Encoder

Required ioctls:
	test VIDIOC_QUERYCAP: OK
	test invalid ioctls: OK

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

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

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

Output ioctls:
	test VIDIOC_G/S_MODULATOR: OK (Not Supported)
	test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
	test VIDIOC_ENUMAUDOUT: OK (Not Supported)
	test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
	test VIDIOC_G/S_AUDOUT: OK (Not Supported)
	Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
	test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
	test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
	test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
	test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls:
	test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
	test VIDIOC_QUERYCTRL: OK
	test VIDIOC_G/S_CTRL: OK
	test VIDIOC_G/S/TRY_EXT_CTRLS: OK
	test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
	test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
	Standard Controls: 15 Private Controls: 0

Format ioctls:
	test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
	test VIDIOC_G/S_PARM: OK
	test VIDIOC_G_FBUF: OK (Not Supported)
	test VIDIOC_G_FMT: OK
	test VIDIOC_TRY_FMT: OK
	test VIDIOC_S_FMT: OK
	test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
	test Cropping: OK
	test Composing: OK (Not Supported)
	test Scaling: OK (Not Supported)

Codec ioctls:
	test VIDIOC_(TRY_)ENCODER_CMD: OK
	test VIDIOC_G_ENC_INDEX: OK (Not Supported)
	test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls:
	test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
	test VIDIOC_EXPBUF: OK
	test Requests: OK (Not Supported)

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

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 chanes 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

Dafna Hirschfeld (1):
  staging: media: wave5: Add the vdi layer

Nas Chung (3):
  staging: media: wave5: Add vpuapi layer
  staging: media: wave5: Add the v4l2 layer
  staging: media: wave5: Add TODO file

Robert Beckett (2):
  dt-bindings: media: staging: wave5: add yaml devicetree bindings
  media: wave5: Add wave5 driver to maintainers file

 .../bindings/staging/media/cnm,wave.yaml      |   73 +
 MAINTAINERS                                   |    9 +
 drivers/staging/media/Kconfig                 |    2 +
 drivers/staging/media/Makefile                |    1 +
 drivers/staging/media/wave5/Kconfig           |   12 +
 drivers/staging/media/wave5/Makefile          |   10 +
 drivers/staging/media/wave5/TODO              |   36 +
 drivers/staging/media/wave5/wave5-hw.c        | 3442 +++++++++++++++++
 drivers/staging/media/wave5/wave5-regdefine.h |  655 ++++
 drivers/staging/media/wave5/wave5-vdi.c       |  260 ++
 drivers/staging/media/wave5/wave5-vdi.h       |   81 +
 drivers/staging/media/wave5/wave5-vpu-dec.c   | 1445 +++++++
 drivers/staging/media/wave5/wave5-vpu-enc.c   | 1565 ++++++++
 drivers/staging/media/wave5/wave5-vpu.c       |  369 ++
 drivers/staging/media/wave5/wave5-vpu.h       |   73 +
 drivers/staging/media/wave5/wave5-vpuapi.c    | 1101 ++++++
 drivers/staging/media/wave5/wave5-vpuapi.h    | 1150 ++++++
 drivers/staging/media/wave5/wave5-vpuconfig.h |   91 +
 drivers/staging/media/wave5/wave5-vpuerror.h  |  455 +++
 drivers/staging/media/wave5/wave5.h           |   86 +
 20 files changed, 10916 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/staging/media/cnm,wave.yaml
 create mode 100644 drivers/staging/media/wave5/Kconfig
 create mode 100644 drivers/staging/media/wave5/Makefile
 create mode 100644 drivers/staging/media/wave5/TODO
 create mode 100644 drivers/staging/media/wave5/wave5-hw.c
 create mode 100644 drivers/staging/media/wave5/wave5-regdefine.h
 create mode 100644 drivers/staging/media/wave5/wave5-vdi.c
 create mode 100644 drivers/staging/media/wave5/wave5-vdi.h
 create mode 100644 drivers/staging/media/wave5/wave5-vpu-dec.c
 create mode 100644 drivers/staging/media/wave5/wave5-vpu-enc.c
 create mode 100644 drivers/staging/media/wave5/wave5-vpu.c
 create mode 100644 drivers/staging/media/wave5/wave5-vpu.h
 create mode 100644 drivers/staging/media/wave5/wave5-vpuapi.c
 create mode 100644 drivers/staging/media/wave5/wave5-vpuapi.h
 create mode 100644 drivers/staging/media/wave5/wave5-vpuconfig.h
 create mode 100644 drivers/staging/media/wave5/wave5-vpuerror.h
 create mode 100644 drivers/staging/media/wave5/wave5.h

Comments

Hans Verkuil June 29, 2022, 3:36 p.m. UTC | #1 
Hi Nas,

On 28/06/2022 13:08, Nas Chung wrote:
> 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.
> 
> 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.
> 
> Testing on FPGA also shows it working fine, though the FPGA uses polled
> interrupts and copied buffers between the host and it's on board RAM.

I wanted to merge this series, but I got several compile/smatch
errors/warnings:

There are several compile errors under i686 (i.e. a 32 bit architecture):

linux-git-i686: WARNINGS

/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-dec.c: In function 'wave5_vpu_open_dec':
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-dec.c:1358:16: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
 1358 |         return ret;
      |                ^~~
In file included from /home/hans/work/build/media-git/include/linux/device.h:15,
                 from /home/hans/work/build/media-git/include/linux/platform_device.h:13,
                 from /home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu.c:9:
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu.c: In function 'wave5_vpu_probe':
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu.c:232:29: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 3 has type 'dma_addr_t' {aka
'unsigned int'} [-Wformat=]
  232 |         dev_err(&pdev->dev, "sram daddr: 0x%llx, size: 0x%lx\n",
      |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/hans/work/build/media-git/include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
  110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
      |                              ^~~
/home/hans/work/build/media-git/include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
  144 |         dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
      |                                                        ^~~~~~~
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu.c:232:9: note: in expansion of macro 'dev_err'
  232 |         dev_err(&pdev->dev, "sram daddr: 0x%llx, size: 0x%lx\n",
      |         ^~~~~~~
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu.c:232:47: note: format string is defined here
  232 |         dev_err(&pdev->dev, "sram daddr: 0x%llx, size: 0x%lx\n",
      |                                            ~~~^
      |                                               |
      |                                               long long unsigned int
      |                                            %x
In file included from /home/hans/work/build/media-git/include/linux/device.h:15,
                 from /home/hans/work/build/media-git/include/linux/platform_device.h:13,
                 from /home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu.c:9:
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu.c:232:29: warning: format '%lx' expects argument of type 'long unsigned int', but argument 4 has type 'size_t' {aka 'unsigned
int'} [-Wformat=]
  232 |         dev_err(&pdev->dev, "sram daddr: 0x%llx, size: 0x%lx\n",
      |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/hans/work/build/media-git/include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
  110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
      |                              ^~~
/home/hans/work/build/media-git/include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
  144 |         dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
      |                                                        ^~~~~~~
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu.c:232:9: note: in expansion of macro 'dev_err'
  232 |         dev_err(&pdev->dev, "sram daddr: 0x%llx, size: 0x%lx\n",
      |         ^~~~~~~
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu.c:232:60: note: format string is defined here
  232 |         dev_err(&pdev->dev, "sram daddr: 0x%llx, size: 0x%lx\n",
      |                                                          ~~^
      |                                                            |
      |                                                            long unsigned int
      |                                                          %x
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-enc.c: In function 'wave5_vpu_open_enc':
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-enc.c:1479:16: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
 1479 |         return ret;
      |                ^~~

linux-git-x86_64: WARNINGS

/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-dec.c: In function 'wave5_vpu_open_dec':
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-dec.c:1358:16: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
 1358 |         return ret;
      |                ^~~
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-enc.c: In function 'wave5_vpu_open_enc':
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-enc.c:1479:16: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
 1479 |         return ret;
      |                ^~~

And smatch gives this:

/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-dec.c:1143:21: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-dec.c:1358:16: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-enc.c:1247:21: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-enc.c:1479:16: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpuapi.c: /home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpuapi.c:93 wave5_check_dec_open_param() warn: maybe use
&& instead of &
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpuapi.c: /home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpuapi.c:505 wave5_vpu_dec_get_output_info() warn:
inconsistent indenting
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-dec.c: /home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-dec.c:1357 wave5_vpu_open_dec() warn: '&inst->list'
not removed from list
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-dec.c: /home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-dec.c:1358 wave5_vpu_open_dec() error: uninitialized
symbol 'ret'.
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-enc.c: /home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-enc.c:1478 wave5_vpu_open_enc() warn: '&inst->list'
not removed from list
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-enc.c: /home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-enc.c:1479 wave5_vpu_open_enc() error: uninitialized
symbol 'ret'.

Also two kerneldoc issues:

kerneldoc: WARNINGS
drivers/staging/media/wave5/wave5-vpuapi.h:144: warning: Cannot understand  * \brief parameters of DEC_SET_SEQ_CHANGE_MASK
drivers/staging/media/wave5/wave5-vdi.h:68: warning: Cannot understand  * @brief make clock stable before changing clock frequency

And finally: quite a lot of the new sources have an empty line at the end. Please remove that.

It's all small stuff, but I'd appreciate it if you can fix it and post a v7.

Regards,

	Hans
Dan Carpenter June 30, 2022, 11:22 a.m. UTC | #2 
On Wed, Jun 29, 2022 at 05:36:31PM +0200, Hans Verkuil wrote:
> drivers/staging/media/wave5/wave5-vpuapi.c:93 wave5_check_dec_open_param() warn: maybe use
> && instead of &

The warning is not clear but the bug looks real.

	if (!(BIT(param->bitstream_mode) & p_attr->support_bitstream_mode))
		return -EINVAL;

If this is really what you intended to write then it would be more
readable as:

	if (!p_attr->support_bitstream_mode ||
	    param->bitstream_mode != BS_MODE_INTERRUPT)
		return -EINVAL;

> drivers/staging/media/wave5/wave5-vpuapi.c:505 wave5_vpu_dec_get_output_info() warn:
> inconsistent indenting

Self explanatory.

> drivers/staging/media/wave5/wave5-vpu-dec.c:1357 wave5_vpu_open_dec() warn: '&inst->list'
> not removed from list

Use after free.  Probably the fix is to not call
list_add_tail(&inst->list, &dev->instances) until everything succeeds.

The others are hopefully self explanatory.

regards,
dan carpenter