Re: [PATCH v2 2/3] [media] allegro: add Allegro DVT video IP core driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Michael,

Review comments follow:

On 01/18/19 14:37, Michael Tretter wrote:
> Add a V4L2 mem-to-mem driver for Allegro DVT video IP cores as found in
> the EV family of the Xilinx ZynqMP SoC. The Zynq UltraScale+ Device
> Technical Reference Manual uses the term VCU (Video Codec Unit) for the
> encoder, decoder and system integration block.
> 
> This driver takes care of interacting with the MicroBlaze MCU that
> controls the actual IP cores. The IP cores and MCU are integrated in the
> FPGA. The xlnx_vcu driver is responsible for configuring the clocks and
> providing information about the codec configuration.
> 
> The driver currently only supports the H.264 video encoder.
> 
> Signed-off-by: Michael Tretter <m.tretter@xxxxxxxxxxxxxx>
> ---
> Changes since v1:
> - clean up debug log levels
> - fix unused variable in allegro_mbox_init
> - fix uninitialized variable in allegro_mbox_write
> - fix global module parameters
> - fix Kconfig dependencies
> - return h264 as default codec for mcu
> - implement device reset as documented
> - document why irq does not wait for clear
> - rename ENCODE_ONE_FRM to ENCODE_FRAME
> - allow error codes for mcu_channel_id
> - move control handler to channel
> - add fw version check
> - add support for colorspaces
> - enable configuration of H.264 levels
> - enable configuration of frame size
> - enable configuration of bit rate and CPB size
> - enable configuration of GOP size
> - rework response handling
> 
> ---
>  MAINTAINERS                                   |    6 +
>  drivers/staging/media/Kconfig                 |    2 +
>  drivers/staging/media/Makefile                |    1 +
>  drivers/staging/media/allegro-dvt/Kconfig     |   16 +
>  drivers/staging/media/allegro-dvt/Makefile    |    4 +
>  .../staging/media/allegro-dvt/allegro-core.c  | 2648 +++++++++++++++++
>  6 files changed, 2677 insertions(+)
>  create mode 100644 drivers/staging/media/allegro-dvt/Kconfig
>  create mode 100644 drivers/staging/media/allegro-dvt/Makefile
>  create mode 100644 drivers/staging/media/allegro-dvt/allegro-core.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 17ad1d7b5510..6ccb298cd8a4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -666,6 +666,12 @@ S:	Maintained
>  F:	Documentation/i2c/busses/i2c-ali1563
>  F:	drivers/i2c/busses/i2c-ali1563.c
>  
> +ALLEGRO DVT VIDEO IP CORE DRIVER
> +M:	Michael Tretter <m.tretter@xxxxxxxxxxxxxx>
> +L:	linux-media@xxxxxxxxxxxxxxx
> +S:	Maintained
> +F:	drivers/staging/media/allegro-dvt/
> +
>  ALLWINNER SECURITY SYSTEM
>  M:	Corentin Labbe <clabbe.montjoie@xxxxxxxxx>
>  L:	linux-crypto@xxxxxxxxxxxxxxx
> diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig
> index 19cadd17e542..0756c8669314 100644
> --- a/drivers/staging/media/Kconfig
> +++ b/drivers/staging/media/Kconfig
> @@ -19,6 +19,8 @@ menuconfig STAGING_MEDIA
>  if STAGING_MEDIA && MEDIA_SUPPORT
>  
>  # Please keep them in alphabetic order
> +source "drivers/staging/media/allegro-dvt/Kconfig"
> +
>  source "drivers/staging/media/bcm2048/Kconfig"
>  
>  source "drivers/staging/media/davinci_vpfe/Kconfig"
> diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile
> index edde1960b030..6578dfcbccb6 100644
> --- a/drivers/staging/media/Makefile
> +++ b/drivers/staging/media/Makefile
> @@ -1,4 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_VIDEO_ALLEGRO_DVT)	+= allegro-dvt/
>  obj-$(CONFIG_I2C_BCM2048)	+= bcm2048/
>  obj-$(CONFIG_VIDEO_IMX_MEDIA)	+= imx/
>  obj-$(CONFIG_SOC_CAMERA_IMX074)	+= imx074/
> diff --git a/drivers/staging/media/allegro-dvt/Kconfig b/drivers/staging/media/allegro-dvt/Kconfig
> new file mode 100644
> index 000000000000..96921e506c5f
> --- /dev/null
> +++ b/drivers/staging/media/allegro-dvt/Kconfig
> @@ -0,0 +1,16 @@
> +config VIDEO_ALLEGRO_DVT
> +	tristate "Allegro DVT Video IP Cores"
> +	depends on V4L_MEM2MEM_DRIVERS
> +	depends on ARCH_ZYNQMP || COMPILE_TEST
> +	depends on VIDEO_DEV && VIDEO_V4L2
> +	select REGMAP
> +	select REGMAP_MMIO
> +	select V4L2_MEM2MEM_DEV
> +	default n
> +	help
> +	  Support for the encoder and decoder video IP cores by Allegro DVT.
> +	  These cores are found for example on the Xilinx ZynqMP SoC in the EV
> +	  family and are called VCU in the reference manual.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called allegro.
> diff --git a/drivers/staging/media/allegro-dvt/Makefile b/drivers/staging/media/allegro-dvt/Makefile
> new file mode 100644
> index 000000000000..bc30addee47f
> --- /dev/null
> +++ b/drivers/staging/media/allegro-dvt/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0
> +allegro-objs := allegro-core.o
> +
> +obj-$(CONFIG_VIDEO_ALLEGRO_DVT) += allegro.o
> diff --git a/drivers/staging/media/allegro-dvt/allegro-core.c b/drivers/staging/media/allegro-dvt/allegro-core.c
> new file mode 100644
> index 000000000000..db48747b345c
> --- /dev/null
> +++ b/drivers/staging/media/allegro-dvt/allegro-core.c
> @@ -0,0 +1,2648 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 Pengutronix, Michael Tretter <kernel@xxxxxxxxxxxxxx>
> + *
> + * Allegro DVT encoder and decoder driver
> + */
> +
> +#include <linux/firmware.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/log2.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/sizes.h>
> +#include <linux/slab.h>
> +#include <linux/videodev2.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-event.h>
> +#include <media/v4l2-ioctl.h>
> +#include <media/v4l2-mem2mem.h>
> +#include <media/videobuf2-dma-contig.h>
> +#include <media/videobuf2-v4l2.h>
> +
> +/*
> + * PG252 June 6, 2018 (H.264/H.265 Video Codec Unit v1.1) Chapter 3
> + *
> + * Configurable resolution
> + * - picture width and height multiple of 8
> + * - minimum size: 128 × 64
> + * - maximum width or height: 16384
> + * - maximum size: 33.5 megapixel
> + *
> + * I cannot trust the 33.5 megapixel, yet, and I do not understand the memory
> + * requirements at higher resolutions. Therefore, I limit the resolution to
> + * full HD, for now.
> + */
> +#define ALLEGRO_WIDTH_MIN 128
> +#define ALLEGRO_WIDTH_DEFAULT 1920
> +#define ALLEGRO_WIDTH_MAX 1920
> +#define ALLEGRO_HEIGHT_MIN 64
> +#define ALLEGRO_HEIGHT_DEFAULT 1080
> +#define ALLEGRO_HEIGHT_MAX 1080
> +
> +#define ALLEGRO_GOP_SIZE_DEFAULT 25
> +#define ALLEGRO_GOP_SIZE_MAX 1000
> +
> +/*
> + * TODO The mailbox addresses and size are only valid for the 2018.2 release
> + * of the firmware. Firmware release 2018.3 uses double the size and changes
> + * the base addresses accordingly.
> + */
> +#define MAILBOX_CMD                     0x7800
> +#define MAILBOX_STATUS                  0x7c00
> +#define MAILBOX_SIZE                    (0x400 - 0x8)
> +
> +/*
> + * MCU Control Registers
> + *
> + * The Zynq UltraScale+ Devices Register Reference documents the registers
> + * with an offset of 0x9000, which equals the size of the SRAM and one page
> + * gap. The driver handles SRAM and registers separately and, therefore, is
> + * oblivious of the offset.
> + */
> +#define AL5_MCU_RESET                   0x0000
> +#define AL5_MCU_RESET_SOFT              BIT(0)
> +#define AL5_MCU_RESET_REGS              BIT(1)
> +#define AL5_MCU_RESET_MODE              0x0004
> +#define AL5_MCU_RESET_MODE_SLEEP        BIT(0)
> +#define AL5_MCU_RESET_MODE_HALT         BIT(1)
> +#define AL5_MCU_STA                     0x0008
> +#define AL5_MCU_STA_SLEEP               BIT(0)
> +#define AL5_MCU_WAKEUP                  0x000c
> +
> +#define AL5_ICACHE_ADDR_OFFSET_MSB      0x0010
> +#define AL5_ICACHE_ADDR_OFFSET_LSB      0x0014
> +#define AL5_DCACHE_ADDR_OFFSET_MSB      0x0018
> +#define AL5_DCACHE_ADDR_OFFSET_LSB      0x001c
> +
> +#define AL5_MCU_INTERRUPT               0x0100
> +#define AL5_ITC_CPU_IRQ_MSK             0x0104
> +#define AL5_ITC_CPU_IRQ_CLR             0x0108
> +#define AL5_ITC_CPU_IRQ_STA             0x010C
> +#define AL5_ITC_CPU_IRQ_STA_TRIGGERED	BIT(0)
> +
> +#define AXI_ADDR_OFFSET_IP              0x0208
> +
> +/*
> + * TODO It seems that the MCU accesses the system memory with a 2G offset
> + * compared to CPU physical addresses.
> + */
> +#define MCU_CACHE_OFFSET SZ_2G
> +
> +/*
> + * TODO The hardware encoder does not write SPS/PPS NAL units, but relies on
> + * the driver to add these. Thus, we need to save some space in the beginning
> + * of the stream buffers for these NAL units.
> + *
> + * The encoder will add the offset before writing the stream data.
> + * Unfortunately, the data does not start exactly at that offset, but a bit
> + * later and we need to add a filler nal.
> + */
> +#define ENCODER_STREAM_OFFSET SZ_64
> +
> +/*
> + * TODO The downstream driver version 2018.02 uses 16M for the suballocator.
> + * I don't know what this memory is used for and how much is actually
> + * required. Firmware version 2018.02 already uses 32M.
> + */
> +#define MCU_SUBALLOCATOR_SIZE SZ_16M
> +
> +/*
> + * Copied from the downstream driver.
> + */
> +#define AL_OPT_WPP			BIT(0)
> +#define AL_OPT_TILE			BIT(1)
> +#define AL_OPT_LF			BIT(2)
> +#define AL_OPT_LF_X_SLICE		BIT(3)
> +#define AL_OPT_LF_X_TILE		BIT(4)
> +#define AL_OPT_SCL_LST			BIT(5)
> +#define AL_OPT_CONST_INTRA_PRED		BIT(6)
> +#define AL_OPT_QP_TAB_RELATIVE		BIT(7)
> +#define AL_OPT_FIX_PREDICTOR		BIT(8)
> +#define AL_OPT_CUSTOM_LDA		BIT(9)
> +#define AL_OPT_ENABLE_AUTO_QP		BIT(10)
> +#define AL_OPT_ADAPT_AUTO_QP		BIT(11)
> +#define AL_OPT_TRANSFO_SKIP		BIT(13)
> +#define AL_OPT_FORCE_REC		BIT(15)
> +#define AL_OPT_FORCE_MV_OUT		BIT(16)
> +#define AL_OPT_FORCE_MV_CLIP		BIT(17)
> +#define AL_OPT_LOWLAT_SYNC		BIT(18)
> +#define AL_OPT_LOWLAT_INT		BIT(19)
> +#define AL_OPT_RDO_COST_MODE		BIT(20)
> +
> +/*
> + * Slice types reported in the mcu_msg_encode_frame_response message.
> + */
> +#define AL_ENC_SLICE_TYPE_B             0
> +#define AL_ENC_SLICE_TYPE_P             1
> +#define AL_ENC_SLICE_TYPE_I             2
> +
> +#define SIZE_MACROBLOCK 16
> +
> +static int debug = 0;
> +module_param(debug, int, 0644);
> +MODULE_PARM_DESC(debug, "Debug level (0-2)");
> +
> +static bool poison_capture_buffers = false;
> +module_param(poison_capture_buffers, bool, 0644);
> +MODULE_PARM_DESC(poison_capture_buffers,
> +		 "Poison capture buffer before writing stream data");

Why is this needed? Is it just for debugging?

At the very least I expect some comments describing the purpose of
this parameter.

> +
> +struct allegro_buffer {
> +	void *vaddr;
> +	dma_addr_t paddr;
> +	size_t size;
> +	struct list_head head;
> +};
> +
> +struct allegro_channel;
> +
> +struct allegro_mbox {
> +	unsigned int head;
> +	unsigned int tail;
> +	unsigned int data;
> +	size_t size;
> +	/* protect mailbox from simultaneous accesses */
> +	struct mutex lock;
> +};
> +
> +struct allegro_dev {
> +	struct v4l2_device v4l2_dev;
> +	struct video_device video_dev;
> +	struct v4l2_m2m_dev *m2m_dev;
> +	struct platform_device *plat_dev;
> +
> +	/* mutex protecting vb2_queue structure */
> +	struct mutex lock;
> +
> +	struct regmap *regmap;
> +	struct regmap *sram;
> +
> +	struct allegro_buffer firmware;
> +	struct allegro_buffer suballocator;
> +
> +	struct completion init_complete;
> +
> +	/* The mailbox interface */
> +	struct allegro_mbox mbox_command;
> +	struct allegro_mbox mbox_status;
> +
> +	/*
> +	 * The downstream driver limits the users to 64 users, thus I can use
> +	 * a bitfield for the user_ids that are in use. See also user_id in
> +	 * struct allegro_channel.
> +	 */
> +	unsigned long channel_user_ids;
> +	struct list_head channels;
> +};
> +
> +static struct regmap_config allegro_regmap_config = {
> +	.name = "regmap",
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +	.max_register = 0xfff,
> +	.cache_type = REGCACHE_NONE,
> +};
> +
> +static struct regmap_config allegro_sram_config = {
> +	.name = "sram",
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +	.max_register = 0x7fff,
> +	.cache_type = REGCACHE_NONE,
> +};
> +
> +#define fh_to_channel(__fh) container_of(__fh, struct allegro_channel, fh)
> +
> +struct allegro_channel {
> +	struct allegro_dev *dev;
> +	struct v4l2_fh fh;
> +	struct v4l2_ctrl_handler ctrl_handler;
> +
> +	unsigned int width;
> +	unsigned int height;
> +	unsigned int stride;
> +
> +	enum v4l2_colorspace colorspace;
> +	enum v4l2_ycbcr_encoding ycbcr_enc;
> +	enum v4l2_quantization quantization;
> +	enum v4l2_xfer_func xfer_func;
> +
> +	u32 pixelformat;
> +	unsigned int sizeimage_raw;
> +
> +	u32 codec;
> +	enum v4l2_mpeg_video_h264_profile profile;
> +	enum v4l2_mpeg_video_h264_level level;
> +	unsigned int sizeimage_encoded;
> +
> +	enum v4l2_mpeg_video_bitrate_mode bitrate_mode;
> +	unsigned int bitrate;
> +	unsigned int bitrate_peak;
> +	unsigned int cpb_size;
> +	unsigned int gop_size;
> +
> +	/* user_id is used to identify the channel during CREATE_CHANNEL */
> +	/* not sure, what to set here and if this is actually required */
> +	int user_id;
> +	/* channel_id is set by the mcu and used by all later commands */
> +	int mcu_channel_id;
> +
> +	struct list_head buffers_reference;
> +	struct list_head buffers_intermediate;
> +
> +	struct list_head list;
> +	struct completion completion;
> +
> +	bool created;
> +	bool stop;
> +};
> +
> +struct fw_info {
> +	unsigned int id;
> +	char *version;
> +};
> +
> +static const struct fw_info supported_firmware[] = {
> +	{
> +		.id = 18296,
> +		.version = "v2018.2",
> +	},
> +};
> +
> +enum mcu_msg_type {
> +	MCU_MSG_TYPE_INIT = 0x0000,
> +	MCU_MSG_TYPE_CREATE_CHANNEL = 0x0005,
> +	MCU_MSG_TYPE_DESTROY_CHANNEL = 0x0006,
> +	MCU_MSG_TYPE_ENCODE_FRAME = 0x0007,
> +	MCU_MSG_TYPE_PUT_STREAM_BUFFER = 0x0012,
> +	MCU_MSG_TYPE_PUSH_BUFFER_INTERMEDIATE = 0x000e,
> +	MCU_MSG_TYPE_PUSH_BUFFER_REFERENCE = 0x000f,
> +};
> +
> +static const char *msg_type_name(enum mcu_msg_type type)
> +{
> +	static char buf[9];
> +
> +	switch (type) {
> +	case MCU_MSG_TYPE_INIT:
> +		return "INIT";
> +	case MCU_MSG_TYPE_CREATE_CHANNEL:
> +		return "CREATE_CHANNEL";
> +	case MCU_MSG_TYPE_DESTROY_CHANNEL:
> +		return "DESTROY_CHANNEL";
> +	case MCU_MSG_TYPE_ENCODE_FRAME:
> +		return "ENCODE_FRAME";
> +	case MCU_MSG_TYPE_PUT_STREAM_BUFFER:
> +		return "PUT_STREAM_BUFFER";
> +	case MCU_MSG_TYPE_PUSH_BUFFER_INTERMEDIATE:
> +		return "PUSH_BUFFER_INTERMEDIATE";
> +	case MCU_MSG_TYPE_PUSH_BUFFER_REFERENCE:
> +		return "PUSH_BUFFER_REFERENCE";
> +	default:
> +		snprintf(buf, sizeof(buf), "(0x%04x)", type);
> +		return buf;
> +	}
> +}
> +
> +struct mcu_msg_header {
> +	u16 length;		/* length of the body in bytes */
> +	u16 type;
> +} __attribute__ ((__packed__));
> +
> +struct mcu_msg_init_request {
> +	struct mcu_msg_header header;
> +	u32 reserved0;		/* maybe a unused channel id */
> +	u32 suballoc_dma;
> +	u32 suballoc_size;
> +	s32 l2_cache[3];
> +} __attribute__ ((__packed__));
> +
> +struct mcu_msg_init_reply {
> +	struct mcu_msg_header header;
> +	u32 reserved0;
> +} __attribute__ ((__packed__));
> +
> +struct mcu_msg_create_channel {
> +	struct mcu_msg_header header;
> +	u32 user_id;
> +	u16 width;
> +	u16 height;
> +	u32 format;
> +	u32 colorspace;
> +	u32 src_mode;
> +	u8 profile;
> +	u16 constraint_set_flags;
> +	s8 codec;
> +	u16 level;
> +	u16 tier;
> +	u32 sps_param;
> +	u32 pps_param;
> +	u32 enc_option;
> +	s8 beta_offset;
> +	s8 tc_offset;
> +	u16 reserved10;
> +	u32 unknown11;
> +	u32 unknown12;
> +	u16 num_slices;
> +	u16 prefetch_auto;
> +	u32 prefetch_mem_offset;
> +	u32 prefetch_mem_size;
> +	u16 clip_hrz_range;
> +	u16 clip_vrt_range;
> +	u16 me_range[4];
> +	u8 max_cu_size;
> +	u8 min_cu_size;
> +	u8 max_tu_size;
> +	u8 min_tu_size;
> +	u8 max_transfo_depth_inter;
> +	u8 max_transfo_depth_intra;
> +	u16 reserved20;
> +	u32 entropy_mode;
> +	u32 wp_mode;
> +	/* rate control param */
> +	u32 rate_control_mode;
> +	u32 initial_rem_delay;
> +	u32 cpb_size;
> +	u16 framerate;
> +	u16 clk_ratio;
> +	u32 target_bitrate;
> +	u32 max_bitrate;
> +	u16 initial_qp;
> +	u16 min_qp;
> +	u16 max_qp;
> +	s16 ip_delta;
> +	s16 pb_delta;
> +	u16 golden_ref;
> +	u16 golden_delta;
> +	u16 golden_ref_frequency;
> +	u32 rate_control_option;
> +	/* gop param */
> +	u32 gop_ctrl_mode;
> +	u32 freq_ird;
> +	u32 freq_lt;
> +	u32 gdr_mode;
> +	u32 gop_length;
> +	u32 unknown39;
> +	u32 subframe_latency;
> +	u32 lda_control_mode;
> +} __attribute__ ((__packed__));
> +
> +struct mcu_msg_create_channel_response {
> +	struct mcu_msg_header header;
> +	u32 channel_id;
> +	u32 user_id;
> +	u32 options;
> +	u32 num_core;
> +	u32 pps_param;
> +	u32 int_buffers_count;
> +	u32 int_buffers_size;
> +	u32 rec_buffers_count;
> +	u32 rec_buffers_size;
> +	u32 reserved;
> +	u32 error_code;
> +} __attribute__ ((__packed__));
> +
> +struct mcu_msg_destroy_channel {
> +	struct mcu_msg_header header;
> +	u32 channel_id;
> +} __attribute__ ((__packed__));
> +
> +struct mcu_msg_destroy_channel_response {
> +	struct mcu_msg_header header;
> +	u32 channel_id;
> +} __attribute__ ((__packed__));
> +
> +struct mcu_msg_push_buffers_internal_buffer {
> +	u32 dma_addr;
> +	u32 mcu_addr;
> +	u32 size;
> +} __attribute__ ((__packed__));
> +
> +struct mcu_msg_push_buffers_internal {
> +	struct mcu_msg_header header;
> +	u32 channel_id;
> +	/* variable length array of mcu_msg_put_internal_buffers_buffer */
> +} __attribute__ ((__packed__));
> +
> +struct mcu_msg_put_stream_buffer {
> +	struct mcu_msg_header header;
> +	u32 channel_id;
> +	u32 dma_addr;
> +	u32 mcu_addr;
> +	u32 size;
> +	u32 offset;
> +	u64 stream_id;
> +} __attribute__ ((__packed__));
> +
> +struct mcu_msg_encode_frame {
> +	struct mcu_msg_header header;
> +	u32 channel_id;
> +	u32 reserved;
> +	u32 encoding_options;
> +	s16 pps_qp;
> +	u16 padding;
> +	u64 user_param;
> +	u64 src_handle;
> +	u32 request_options;
> +	/* u32 scene_change_delay (optional) */
> +	/* rate control param (optional) */
> +	/* gop param (optional) */
> +	u32 src_y;
> +	u32 src_uv;
> +	u32 stride;
> +	u32 ep2;
> +	u64 ep2_v;
> +} __attribute__ ((__packed__));
> +
> +struct mcu_msg_encode_frame_response {
> +	struct mcu_msg_header header;
> +	u32 channel_id;
> +	u64 stream_id;		/* see mcu_msg_put_stream_buffer */
> +	u64 user_param;		/* see mcu_msg_encode_frame */
> +	u64 src_handle;		/* see mcu_msg_encode_frame */
> +	u16 skip;
> +	u16 is_ref;
> +	u32 initial_removal_delay;
> +	u32 dpb_output_delay;
> +	u32 size;
> +	u32 frame_tag_size;
> +	s32 stuffing;
> +	s32 filler;
> +	u16 num_column;
> +	u16 num_row;
> +	u16 qp;
> +	u8 num_ref_idx_l0;
> +	u8 num_ref_idx_l1;
> +	u32 partition_table_offset;
> +	s32 partition_table_size;
> +	u32 sum_complex;
> +	s32 tile_width[4];	/* TODO AL_ENC_NUM_CORES = 4 */
> +	s32 tile_height[22];	/* TODO AL_MAX_ROWS_TILE = 22 */
> +	u32 error_code;
> +	u32 slice_type;
> +	u32 pic_struct;
> +	u8 is_idr;
> +	u8 is_first_slice;
> +	u8 is_last_slice;
> +	u8 reserved;
> +	u16 pps_qp;
> +	u16 reserved1;
> +	u32 reserved2;
> +} __attribute__ ((__packed__));
> +
> +/* Helper functions for channel and user operations */
> +
> +static unsigned long allegro_next_user_id(struct allegro_dev *dev)
> +{
> +	if (dev->channel_user_ids == ~0UL)
> +		return -EBUSY;
> +
> +	return ffz(dev->channel_user_ids);
> +}
> +
> +static struct allegro_channel *
> +allegro_find_channel_by_user_id(struct allegro_dev *dev,
> +				unsigned int user_id)
> +{
> +	struct allegro_channel *channel;
> +
> +	list_for_each_entry(channel, &dev->channels, list) {
> +		if (channel->user_id == user_id)
> +			return channel;
> +	}
> +
> +	return ERR_PTR(-EINVAL);
> +}
> +
> +static struct allegro_channel *
> +allegro_find_channel_by_channel_id(struct allegro_dev *dev,
> +				   unsigned int channel_id)
> +{
> +	struct allegro_channel *channel;
> +
> +	list_for_each_entry(channel, &dev->channels, list) {
> +		if (channel->mcu_channel_id == channel_id)
> +			return channel;
> +	}
> +
> +	return ERR_PTR(-EINVAL);
> +}
> +
> +static unsigned int estimate_stream_size(unsigned int width, unsigned int height)
> +{
> +	int num_blocks = DIV_ROUND_UP(width, 64) * DIV_ROUND_UP(height, 64);
> +	int pcm_size = SZ_1K + SZ_512;
> +	int offset = ENCODER_STREAM_OFFSET;
> +	int partition_table = SZ_256;
> +
> +	return round_up(offset + num_blocks * pcm_size + partition_table, 32);
> +}
> +
> +static enum v4l2_mpeg_video_h264_level
> +select_minimum_h264_level(unsigned int width, unsigned int height)
> +{
> +	unsigned int pic_width_in_mb = DIV_ROUND_UP(width, SIZE_MACROBLOCK);
> +	unsigned int frame_height_in_mb = DIV_ROUND_UP(height, SIZE_MACROBLOCK);
> +	unsigned int frame_size_in_mb = pic_width_in_mb * frame_height_in_mb;
> +	enum v4l2_mpeg_video_h264_level level = V4L2_MPEG_VIDEO_H264_LEVEL_4_0;
> +
> +	/*
> +	 * TODO This a very rudimentary implementation of the level limits
> +	 * specified in Annex A.3.1, which for now only respects the frame
> +	 * size.
> +	 */
> +
> +	if (frame_size_in_mb <= 99)
> +		level = V4L2_MPEG_VIDEO_H264_LEVEL_1_0;
> +	else if (frame_size_in_mb <= 396)
> +		level = V4L2_MPEG_VIDEO_H264_LEVEL_1_1;
> +	else if (frame_size_in_mb <= 792)
> +		level = V4L2_MPEG_VIDEO_H264_LEVEL_2_1;
> +	else if (frame_size_in_mb <= 1620)
> +		level = V4L2_MPEG_VIDEO_H264_LEVEL_2_2;
> +	else if (frame_size_in_mb <= 3600)
> +		level = V4L2_MPEG_VIDEO_H264_LEVEL_3_1;
> +	else if (frame_size_in_mb <= 5120)
> +		level = V4L2_MPEG_VIDEO_H264_LEVEL_3_2;
> +	else if (frame_size_in_mb <= 8192)
> +		level = V4L2_MPEG_VIDEO_H264_LEVEL_4_0;
> +	else if (frame_size_in_mb <= 8704)
> +		level = V4L2_MPEG_VIDEO_H264_LEVEL_4_2;
> +	else if (frame_size_in_mb <= 22080)
> +		level = V4L2_MPEG_VIDEO_H264_LEVEL_5_0;
> +	else
> +		level = V4L2_MPEG_VIDEO_H264_LEVEL_5_1;
> +
> +	/*
> +	 * The encoder supports maximum 4k resolution, which equals a frame
> +	 * size of 32400 macro blocks and is supported by Level 5.1 (36864
> +	 * macro blocks. Therefore, defaulting to Level 5.1 isn't that bad.
> +	 */
> +
> +	return level;
> +}
> +
> +static unsigned int maximum_bitrate(enum v4l2_mpeg_video_h264_level level)
> +{
> +	switch (level) {
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_1_0:
> +		return 64000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_1B:
> +		return 128000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_1_1:
> +		return 192000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_1_2:
> +		return 384000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_1_3:
> +		return 768000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_2_0:
> +		return 2000000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_2_1:
> +		return 4000000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_2_2:
> +		return 4000000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_3_0:
> +		return 10000000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_3_1:
> +		return 14000000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_3_2:
> +		return 20000000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_4_0:
> +		return 20000000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_4_1:
> +		return 50000000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_4_2:
> +		return 50000000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_5_0:
> +		return 135000000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_5_1:
> +	default:
> +		return 240000000;
> +	}
> +}
> +
> +static unsigned int maximum_cpb_size(enum v4l2_mpeg_video_h264_level level)
> +{
> +	switch (level) {
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_1_0:
> +		return 175;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_1B:
> +		return 350;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_1_1:
> +		return 500;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_1_2:
> +		return 1000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_1_3:
> +		return 2000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_2_0:
> +		return 2000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_2_1:
> +		return 4000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_2_2:
> +		return 4000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_3_0:
> +		return 10000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_3_1:
> +		return 14000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_3_2:
> +		return 20000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_4_0:
> +		return 25000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_4_1:
> +		return 62500;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_4_2:
> +		return 62500;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_5_0:
> +		return 135000;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_5_1:
> +	default:
> +		return 240000;
> +	}
> +}
> +
> +static const struct fw_info *allegro_get_firmware_info(struct allegro_dev *dev,
> +						       const struct firmware *fw)
> +{
> +	int i;
> +	unsigned int id = fw->size;
> +
> +	for (i = 0; i < ARRAY_SIZE(supported_firmware); i++)
> +		if (supported_firmware[i].id == id)
> +			return &supported_firmware[i];
> +
> +	return NULL;
> +}
> +
> +/*
> + * Buffers that are used internally by the MCU.
> + */
> +
> +int allegro_alloc_buffer(struct allegro_dev *dev,
> +			 struct allegro_buffer *buffer, size_t size)
> +{
> +	buffer->vaddr = dma_alloc_coherent(&dev->plat_dev->dev, size,
> +					   &buffer->paddr, GFP_KERNEL);
> +	if (!buffer->vaddr)
> +		return -ENOMEM;
> +	buffer->size = size;
> +
> +	return 0;
> +}
> +
> +void allegro_free_buffer(struct allegro_dev *dev, struct allegro_buffer *buffer)
> +{
> +	if (buffer->vaddr) {
> +		dma_free_coherent(&dev->plat_dev->dev, buffer->size,
> +				  buffer->vaddr, buffer->paddr);
> +		buffer->vaddr = NULL;
> +		buffer->size = 0;
> +	}
> +}
> +
> +/*
> + * Mailbox interface to send messages to the MCU.
> + */
> +
> +static int allegro_mbox_init(struct allegro_dev *dev,
> +			     struct allegro_mbox *mbox,
> +			     unsigned int base, size_t size)
> +{
> +	if (!mbox)
> +		return -EINVAL;
> +
> +	mbox->head = base;
> +	mbox->tail = base + 0x4;
> +	mbox->data = base + 0x8;
> +	mbox->size = size;
> +	mutex_init(&mbox->lock);
> +
> +	regmap_write(dev->sram, mbox->head, 0);
> +	regmap_write(dev->sram, mbox->tail, 0);
> +
> +	return 0;
> +}
> +
> +static int allegro_mbox_write(struct allegro_dev *dev,
> +			      struct allegro_mbox *mbox, void *src, size_t size)
> +{
> +	struct mcu_msg_header *header = src;
> +	unsigned int tail;
> +	size_t size_no_wrap;
> +	int err = 0;
> +
> +	if (!src)
> +		return -EINVAL;
> +
> +	if (size > mbox->size) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "message (%lu bytes) to large for mailbox (%lu bytes)\n",
> +			 size, mbox->size);
> +		return -EINVAL;
> +	}
> +
> +	if (header->length != size - sizeof(*header)) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "invalid message length: %u bytes (expected %lu bytes)\n",
> +			 header->length, size - sizeof(*header));
> +		return -EINVAL;
> +	}
> +
> +	v4l2_dbg(2, debug, &dev->v4l2_dev,
> +		"write command message: type %s, body length %d\n",
> +		msg_type_name(header->type), header->length);
> +
> +	mutex_lock(&mbox->lock);
> +	regmap_read(dev->sram, mbox->tail, &tail);
> +	if (tail > mbox->size) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "invalid tail (0x%x): must be smaller than mailbox size (0x%lx)\n",
> +			 tail, mbox->size);
> +		err = -EIO;
> +		goto out;
> +	}
> +	size_no_wrap = min(size, mbox->size - (size_t)tail);
> +	regmap_bulk_write(dev->sram, mbox->data + tail, src, size_no_wrap / 4);
> +	regmap_bulk_write(dev->sram, mbox->data,
> +			  src + size_no_wrap, (size - size_no_wrap) / 4);
> +	regmap_write(dev->sram, mbox->tail, (tail + size) % mbox->size);
> +
> +out:
> +	mutex_unlock(&mbox->lock);
> +
> +	return err;
> +}
> +
> +static ssize_t allegro_mbox_read(struct allegro_dev *dev,
> +				 struct allegro_mbox *mbox,
> +				 void *dst, size_t nbyte)
> +{
> +	struct mcu_msg_header *header;
> +	unsigned int head;
> +	ssize_t size;
> +	size_t body_no_wrap;
> +
> +	regmap_read(dev->sram, mbox->head, &head);
> +	if (head > mbox->size) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "invalid head (0x%x): must be smaller than mailbox size (0x%lx)\n",
> +			 head, mbox->size);
> +		return -EIO;
> +	}
> +
> +	/* Assume that the header does not wrap. */
> +	regmap_bulk_read(dev->sram, mbox->data + head,
> +			 dst, sizeof(*header) / 4);
> +	header = dst;
> +	size = header->length + sizeof(*header);
> +	if (size > mbox->size || size & 0x3) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "invalid message length: %lu bytes (maximum %lu bytes)\n",
> +			 header->length + sizeof(*header), mbox->size);
> +		return -EIO;
> +	}
> +	if (size > nbyte) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "destination buffer too small: %lu bytes (need %lu bytes)\n",
> +			 nbyte, size);
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * The message might wrap within the mailbox. If the message does not
> +	 * wrap, the first read will read the entire message, otherwise the
> +	 * first read will read message until the end of the mailbox and the
> +	 * second read will read the remaining bytes from the beginning of the
> +	 * mailbox.
> +	 *
> +	 * Skip the header, as was already read to get the size of the body.
> +	 */
> +	body_no_wrap = min((size_t)header->length,
> +			   (mbox->size - (head + sizeof(*header))));
> +	regmap_bulk_read(dev->sram, mbox->data + head + sizeof(*header),
> +			 dst + sizeof(*header), body_no_wrap / 4);
> +	regmap_bulk_read(dev->sram, mbox->data,
> +			 dst + sizeof(*header) + body_no_wrap,
> +			 (header->length - body_no_wrap) / 4);
> +
> +	regmap_write(dev->sram, mbox->head, (head + size) % mbox->size);
> +
> +	v4l2_dbg(2, debug, &dev->v4l2_dev,
> +		"read status message: type %s, body length %d\n",
> +		msg_type_name(header->type), header->length);
> +
> +	return size;
> +}
> +
> +static void allegro_mcu_interrupt(struct allegro_dev *dev)
> +{
> +	regmap_write(dev->regmap, AL5_MCU_INTERRUPT, BIT(0));
> +}
> +
> +static void allegro_mcu_send_init(struct allegro_dev *dev,
> +				  dma_addr_t suballoc_dma, size_t suballoc_size)
> +{
> +	struct mcu_msg_init_request msg;
> +
> +	msg.header.type = MCU_MSG_TYPE_INIT;
> +	msg.header.length = sizeof(msg) - sizeof(msg.header);
> +	msg.reserved0 = 0;
> +	msg.suballoc_dma = lower_32_bits(suballoc_dma) | MCU_CACHE_OFFSET;
> +	msg.suballoc_size = suballoc_size;
> +
> +	/* TODO Add L2 cache support. */
> +	msg.l2_cache[0] = -1;
> +	msg.l2_cache[1] = -1;
> +	msg.l2_cache[2] = -1;
> +
> +	allegro_mbox_write(dev, &dev->mbox_command, &msg, sizeof(msg));
> +	allegro_mcu_interrupt(dev);
> +}
> +
> +static u32 v4l2_pixelformat_to_mcu_format(u32 pixelformat)
> +{
> +	switch (pixelformat) {
> +	case V4L2_PIX_FMT_NV12:
> +		/* AL_420_8BITS: 0x100 -> NV12, 0x88 -> 8 bit */
> +		return 0x100 | 0x88;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static u32 v4l2_colorspace_to_mcu_colorspace(enum v4l2_colorspace colorspace)
> +{
> +	switch (colorspace) {
> +	case V4L2_COLORSPACE_REC709:
> +		return 2;
> +	case V4L2_COLORSPACE_SMPTE170M:
> +		return 3;
> +	case V4L2_COLORSPACE_SMPTE240M:
> +		return 4;
> +	case V4L2_COLORSPACE_SRGB:
> +		return 7;
> +	default:
> +		/* UNKNOWN */
> +		return 0;
> +	}
> +}
> +
> +static s8 v4l2_pixelformat_to_mcu_codec(u32 pixelformat)
> +{
> +	switch (pixelformat) {
> +	case V4L2_PIX_FMT_H264:
> +	default:
> +		return 1;
> +	}
> +}
> +
> +static u8 v4l2_profile_to_mcu_profile(enum v4l2_mpeg_video_h264_profile profile)
> +{
> +	switch (profile) {
> +	case V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE:
> +	default:
> +		return 66;
> +	}
> +}
> +
> +static u16 v4l2_level_to_mcu_level(enum v4l2_mpeg_video_h264_level level)
> +{
> +	switch (level) {
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_1_0:
> +		return 10;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_1_1:
> +		return 11;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_1_2:
> +		return 12;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_1_3:
> +		return 13;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_2_0:
> +		return 20;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_2_1:
> +		return 21;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_2_2:
> +		return 22;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_3_0:
> +		return 30;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_3_1:
> +		return 31;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_3_2:
> +		return 32;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_4_0:
> +		return 40;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_4_1:
> +		return 41;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_4_2:
> +		return 42;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_5_0:
> +		return 50;
> +	case V4L2_MPEG_VIDEO_H264_LEVEL_5_1:
> +	default:
> +		return 51;
> +	}
> +}
> +
> +static u32 v4l2_bitrate_mode_to_mcu_mode(enum v4l2_mpeg_video_bitrate_mode mode)
> +{
> +	switch (mode) {
> +	case V4L2_MPEG_VIDEO_BITRATE_MODE_VBR:
> +		return 2;
> +	case V4L2_MPEG_VIDEO_BITRATE_MODE_CBR:
> +	default:
> +		return 1;
> +	}
> +}
> +
> +static void allegro_mcu_send_create_channel(struct allegro_dev *dev,
> +					    struct allegro_channel *channel)
> +{
> +	struct mcu_msg_create_channel msg;
> +
> +	msg.header.type = MCU_MSG_TYPE_CREATE_CHANNEL;
> +	msg.header.length = sizeof(msg) - sizeof(msg.header);
> +
> +	msg.user_id = channel->user_id;
> +	msg.width = channel->width;
> +	msg.height = channel->height;
> +	msg.format = v4l2_pixelformat_to_mcu_format(channel->pixelformat);
> +	msg.colorspace = v4l2_colorspace_to_mcu_colorspace(channel->colorspace);
> +	msg.src_mode = 0x0;
> +	msg.profile = v4l2_profile_to_mcu_profile(channel->profile);
> +	msg.constraint_set_flags = BIT(1);
> +	msg.codec = v4l2_pixelformat_to_mcu_codec(channel->codec);
> +	msg.level = v4l2_level_to_mcu_level(channel->level);
> +	msg.tier = 0;
> +	msg.sps_param = BIT(20) | 0x4a;
> +	msg.pps_param = BIT(2);
> +	msg.enc_option = AL_OPT_RDO_COST_MODE | AL_OPT_LF_X_TILE |
> +			 AL_OPT_LF_X_SLICE | AL_OPT_LF;
> +	msg.beta_offset = -1;
> +	msg.tc_offset = -1;
> +	msg.reserved10 = 0x0000;
> +	msg.unknown11 = 0x00000000;
> +	msg.unknown12 = 0x00000000;
> +	msg.num_slices = 1;
> +	msg.prefetch_auto = 0x0000;	/* false */
> +	msg.prefetch_mem_offset = 0x00000000;
> +	msg.prefetch_mem_size = 0x00000000;
> +	msg.clip_vrt_range = 0x0000;
> +	msg.clip_hrz_range = 0x0000;
> +	msg.me_range[0] = 8;
> +	msg.me_range[1] = 8;
> +	msg.me_range[2] = 16;
> +	msg.me_range[3] = 16;
> +	msg.max_cu_size = ilog2(SIZE_MACROBLOCK);
> +	msg.min_cu_size = ilog2(8);
> +	msg.max_tu_size = 2;
> +	msg.min_tu_size = 2;
> +	msg.max_transfo_depth_intra = 1;
> +	msg.max_transfo_depth_inter = 1;
> +	msg.reserved20 = 0x0000;
> +	msg.entropy_mode = 0x00000000;
> +	msg.wp_mode = 0x00000000;
> +	msg.rate_control_mode = v4l2_bitrate_mode_to_mcu_mode(channel->bitrate_mode);
> +	/* Shall be ]0;cpb_size in  90 kHz units]. Use maximum value for now. */
> +	msg.initial_rem_delay = ((channel->cpb_size * 1000) / channel->bitrate_peak) * 90000;
> +	/* Encoder expects cpb_size in units of a 90 kHz clock. */
> +	msg.cpb_size = ((channel->cpb_size * 1000) / channel->bitrate_peak) * 90000;
> +	msg.framerate = 25;
> +	msg.clk_ratio = 1000;
> +	msg.target_bitrate = channel->bitrate;
> +	msg.max_bitrate = channel->bitrate_peak;
> +	msg.initial_qp = 25;
> +	msg.min_qp = 10;
> +	msg.max_qp = 51;
> +	msg.ip_delta = -1;
> +	msg.pb_delta = -1;
> +	msg.golden_ref = 0;
> +	msg.golden_delta = 2;
> +	msg.golden_ref_frequency = 10;
> +	msg.rate_control_option = 0x00000000;
> +	msg.gop_ctrl_mode = 0x00000000;
> +	msg.freq_ird = 0x7fffffff;	/* MAX_INT??? */
> +	msg.freq_lt = 0;
> +	msg.gdr_mode = 0x00000000;
> +	msg.gop_length = channel->gop_size;
> +	msg.unknown39 = 0;
> +	msg.subframe_latency = 0x00000000;	/* ??? */
> +	msg.lda_control_mode = 0x700d0000;	/* ??? */
> +
> +	allegro_mbox_write(dev, &dev->mbox_command, &msg, sizeof(msg));
> +	allegro_mcu_interrupt(dev);
> +}
> +
> +static int allegro_mcu_send_destroy_channel(struct allegro_dev *dev,
> +					    struct allegro_channel *channel)
> +{
> +	struct mcu_msg_destroy_channel msg;
> +
> +	msg.header.type = MCU_MSG_TYPE_DESTROY_CHANNEL;
> +	msg.header.length = sizeof(msg) - sizeof(msg.header);
> +
> +	msg.channel_id = channel->mcu_channel_id;
> +
> +	allegro_mbox_write(dev, &dev->mbox_command, &msg, sizeof(msg));
> +	allegro_mcu_interrupt(dev);
> +
> +	return 0;
> +}
> +
> +static int allegro_mcu_send_put_stream_buffer(struct allegro_dev *dev,
> +					      struct allegro_channel *channel,
> +					      struct allegro_buffer buffer)
> +{
> +	struct mcu_msg_put_stream_buffer msg;
> +	/*
> +	 * TODO It seems that stream_id is not used, but allows to identify
> +	 * the used stream buffer from the ENCODE_FRAME response.
> +	 */
> +	int stream_id = 0;
> +
> +	msg.header.type = MCU_MSG_TYPE_PUT_STREAM_BUFFER;
> +	msg.header.length = sizeof(msg) - sizeof(msg.header);
> +
> +	msg.channel_id = channel->mcu_channel_id;
> +	msg.dma_addr = buffer.paddr;
> +	msg.mcu_addr = buffer.paddr | MCU_CACHE_OFFSET;
> +	msg.size = buffer.size;
> +	msg.offset = ENCODER_STREAM_OFFSET;
> +	msg.stream_id = stream_id;
> +
> +	allegro_mbox_write(dev, &dev->mbox_command, &msg, sizeof(msg));
> +	allegro_mcu_interrupt(dev);
> +
> +	return 0;
> +}
> +
> +static void allegro_mcu_send_encode_frame(struct allegro_dev *dev,
> +					  struct allegro_channel *channel,
> +					  dma_addr_t src_y, dma_addr_t src_uv)
> +{
> +	struct mcu_msg_encode_frame msg;
> +
> +	msg.header.type = MCU_MSG_TYPE_ENCODE_FRAME;
> +	/*
> +	 * TODO Calculating the length of the message by using the struct size
> +	 * breaks once the optional parameters are used.
> +	 */
> +	msg.header.length = sizeof(msg) - sizeof(msg.header);
> +
> +	msg.channel_id = channel->mcu_channel_id;
> +	msg.encoding_options = 0x2;	/* AL_OPT_FORCE_LOAD */
> +	msg.pps_qp = 26;	/* The SliceQP are relative to 26 */
> +	msg.padding = 0x0;
> +	/*
> +	 * TODO It seems that user_param and src_handle are not used by the
> +	 * mcu, but allow to relate the response message to the request.
> +	 */
> +	msg.user_param = 0;
> +	msg.src_handle = 0;
> +	msg.request_options = 0x0;	/* AL_NO_OPT */
> +
> +	msg.src_y = src_y;
> +	msg.src_uv = src_uv;
> +	msg.stride = channel->stride;
> +	msg.ep2 = 0x0;
> +	msg.ep2_v = msg.ep2 | MCU_CACHE_OFFSET;
> +
> +	allegro_mbox_write(dev, &dev->mbox_command, &msg, sizeof(msg));
> +	allegro_mcu_interrupt(dev);
> +}
> +
> +static int allegro_mcu_wait_for_init_timeout(struct allegro_dev *dev,
> +					     unsigned long timeout_ms)
> +{
> +	unsigned long tmo;
> +
> +	tmo = wait_for_completion_timeout(&dev->init_complete,
> +					  msecs_to_jiffies(timeout_ms));
> +	if (tmo == 0)
> +		return -ETIMEDOUT;
> +
> +	reinit_completion(&dev->init_complete);
> +	return 0;
> +}
> +
> +static int allegro_mcu_push_buffer_internal(struct allegro_channel *channel,
> +					    enum mcu_msg_type type)
> +{
> +	struct allegro_dev *dev = channel->dev;
> +	struct mcu_msg_push_buffers_internal *msg;
> +	struct mcu_msg_push_buffers_internal_buffer *buffer;
> +	unsigned int num_buffers = 0;
> +	unsigned int size;
> +	struct allegro_buffer *al_buffer;
> +	struct list_head *list;
> +	int err;
> +
> +	if (channel->created)
> +		return -EINVAL;
> +
> +	switch (type) {
> +	case MCU_MSG_TYPE_PUSH_BUFFER_REFERENCE:
> +		list = &channel->buffers_reference;
> +		break;
> +	case MCU_MSG_TYPE_PUSH_BUFFER_INTERMEDIATE:
> +		list = &channel->buffers_intermediate;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	list_for_each_entry(al_buffer, list, head) {
> +		num_buffers++;
> +	}
> +
> +	size = sizeof(*msg) + num_buffers * sizeof(*buffer);
> +
> +	msg = kmalloc(size, GFP_KERNEL);
> +	if (!msg)
> +		return -ENOMEM;
> +
> +	msg->header.length = size - sizeof(msg->header);
> +	msg->header.type = type;
> +	msg->channel_id = channel->mcu_channel_id;
> +
> +	buffer =
> +	    (struct mcu_msg_push_buffers_internal_buffer *)((char *)msg +
> +							    sizeof(*msg));
> +	list_for_each_entry(al_buffer, list, head) {
> +		buffer->dma_addr = lower_32_bits(al_buffer->paddr);
> +		buffer->mcu_addr =
> +		    lower_32_bits(al_buffer->paddr) | MCU_CACHE_OFFSET;
> +		buffer->size = al_buffer->size;
> +		buffer++;
> +	}
> +
> +	err = allegro_mbox_write(dev, &dev->mbox_command, msg, size);
> +	if (err)
> +		goto out;
> +	allegro_mcu_interrupt(dev);
> +
> +out:
> +	kfree(msg);
> +	return err;
> +}
> +
> +static int allegro_mcu_push_buffer_intermediate(struct allegro_channel *channel)
> +{
> +	return allegro_mcu_push_buffer_internal(channel,
> +						MCU_MSG_TYPE_PUSH_BUFFER_INTERMEDIATE);
> +}
> +
> +static int allegro_mcu_push_buffer_reference(struct allegro_channel *channel)
> +{
> +	return allegro_mcu_push_buffer_internal(channel,
> +						MCU_MSG_TYPE_PUSH_BUFFER_REFERENCE);
> +}
> +
> +static int allocate_buffers_internal(struct allegro_channel *channel,
> +				     struct list_head *list,
> +				     size_t n, size_t size)
> +{
> +	struct allegro_dev *dev = channel->dev;
> +	unsigned int i;
> +	int err;
> +	struct allegro_buffer *buffer, *tmp;
> +
> +	for (i = 0; i < n; i++) {
> +		buffer = kmalloc(sizeof(*buffer), GFP_KERNEL);
> +		if (!buffer) {
> +			err = -ENOMEM;
> +			goto err;
> +		}
> +		INIT_LIST_HEAD(&buffer->head);
> +
> +		err = allegro_alloc_buffer(dev, buffer, size);
> +		if (err)
> +			goto err;
> +		list_add(&buffer->head, list);
> +	}
> +
> +	return 0;
> +
> +err:
> +	list_for_each_entry_safe(buffer, tmp, list, head) {
> +		list_del(&buffer->head);
> +		allegro_free_buffer(dev, buffer);
> +		kfree(buffer);
> +	}
> +	return err;
> +}
> +
> +static void destroy_buffers_internal(struct allegro_channel *channel,
> +				     struct list_head *list)
> +{
> +	struct allegro_dev *dev = channel->dev;
> +	struct allegro_buffer *buffer, *tmp;
> +
> +	list_for_each_entry_safe(buffer, tmp, list, head) {
> +		list_del(&buffer->head);
> +		allegro_free_buffer(dev, buffer);
> +		kfree(buffer);
> +	}
> +}
> +
> +static void destroy_reference_buffers(struct allegro_channel *channel)
> +{
> +	return destroy_buffers_internal(channel, &channel->buffers_reference);
> +}
> +
> +static void destroy_intermediate_buffers(struct allegro_channel *channel)
> +{
> +	return destroy_buffers_internal(channel,
> +					&channel->buffers_intermediate);
> +}
> +
> +static int allocate_intermediate_buffers(struct allegro_channel *channel,
> +					 size_t n, size_t size)
> +{
> +	return allocate_buffers_internal(channel,
> +					 &channel->buffers_intermediate,
> +					 n, size);
> +}
> +
> +static int allocate_reference_buffers(struct allegro_channel *channel,
> +				      size_t n, size_t size)
> +{
> +	return allocate_buffers_internal(channel,
> +					 &channel->buffers_reference,
> +					 n, PAGE_ALIGN(size));
> +}
> +
> +static void allegro_finish_frame(struct allegro_channel *channel,
> +				 struct mcu_msg_encode_frame_response *msg)
> +{
> +	struct allegro_dev *dev = channel->dev;
> +	struct vb2_v4l2_buffer *src_buf;
> +	struct vb2_v4l2_buffer *dst_buf;
> +	struct {
> +		u32 offset;
> +		u32 size;
> +	} *partition;
> +	enum vb2_buffer_state state = VB2_BUF_STATE_ERROR;
> +
> +	src_buf = v4l2_m2m_src_buf_remove(channel->fh.m2m_ctx);
> +	v4l2_m2m_buf_done(src_buf, VB2_BUF_STATE_DONE);
> +
> +	dst_buf = v4l2_m2m_dst_buf_remove(channel->fh.m2m_ctx);
> +
> +	if (msg->error_code) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "channel %d: error while encoding frame: %x\n",
> +			 channel->mcu_channel_id, msg->error_code);
> +		goto err;
> +	}
> +
> +	if (msg->partition_table_size != 1) {
> +		v4l2_warn(&dev->v4l2_dev,
> +			  "channel %d: only handling first partition table entry (%d entries)\n",
> +			  channel->mcu_channel_id, msg->partition_table_size);
> +	}
> +
> +	if (msg->partition_table_offset +
> +	    msg->partition_table_size * sizeof(*partition) >
> +	    vb2_plane_size(&dst_buf->vb2_buf, 0)) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "channel %d: partition table outside of dst_buf\n",
> +			 channel->mcu_channel_id);
> +		goto err;
> +	}
> +
> +	partition =
> +	    vb2_plane_vaddr(&dst_buf->vb2_buf, 0) + msg->partition_table_offset;
> +	if (partition->offset + partition->size >
> +	    vb2_plane_size(&dst_buf->vb2_buf, 0)) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "channel %d: encoded frame is outside of dst_buf (offset 0x%x, size 0x%x)\n",
> +			 channel->mcu_channel_id, partition->offset,
> +			 partition->size);
> +		goto err;
> +	}
> +
> +	v4l2_dbg(2, debug, &dev->v4l2_dev,
> +		"channel %d: encoded frame of size %d is at offset 0x%x\n",
> +		channel->mcu_channel_id, partition->size, partition->offset);
> +
> +	/*
> +	 * TODO The partition->offset differs from the ENCODER_STREAM_OFFSET.
> +	 * Does the encoder add any data before its configured offset that we
> +	 * need to handle?
> +	 */
> +
> +	vb2_set_plane_payload(&dst_buf->vb2_buf, 0,
> +			      partition->offset + partition->size);
> +
> +	state = VB2_BUF_STATE_DONE;
> +
> +	v4l2_dbg(1, debug, &dev->v4l2_dev,
> +		 "channel %d: encoded frame (%s%s, %d bytes)\n",
> +		 channel->mcu_channel_id,
> +		 msg->is_idr ? "IDR, " : "",
> +		 msg->slice_type == AL_ENC_SLICE_TYPE_I ? "I slice" :
> +		 msg->slice_type == AL_ENC_SLICE_TYPE_P ? "P slice" : "unknown",
> +		 partition->size);
> +
> +err:
> +	if (channel->stop) {
> +		const struct v4l2_event eos_event = {
> +			.type = V4L2_EVENT_EOS
> +		};
> +		dst_buf->flags |= V4L2_BUF_FLAG_LAST;
> +		v4l2_event_queue_fh(&channel->fh, &eos_event);
> +	}
> +
> +	v4l2_m2m_buf_done(dst_buf, state);
> +	v4l2_m2m_job_finish(dev->m2m_dev, channel->fh.m2m_ctx);
> +}
> +
> +static int allegro_handle_init(struct allegro_dev *dev,
> +			       struct mcu_msg_init_reply *msg)
> +{
> +	complete(&dev->init_complete);
> +
> +	return 0;
> +}
> +
> +static int
> +allegro_handle_create_channel(struct allegro_dev *dev,
> +			      struct mcu_msg_create_channel_response *msg)
> +{
> +	struct allegro_channel *channel;
> +	int err = 0;
> +
> +	channel = allegro_find_channel_by_user_id(dev, msg->user_id);
> +	if (IS_ERR(channel)) {
> +		v4l2_warn(&dev->v4l2_dev,
> +			  "received %s for unknown user %d\n",
> +			  msg_type_name(msg->header.type),
> +			  msg->user_id);
> +		return -EINVAL;
> +	}
> +
> +	if (msg->error_code) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "user %d: failed to create channel: error %d",
> +			 channel->user_id, msg->error_code);
> +		return -EINVAL;
> +	}
> +
> +	channel->mcu_channel_id = msg->channel_id;
> +	v4l2_dbg(1, debug, &dev->v4l2_dev,
> +		 "user %d: channel has channel id %d\n",
> +		 channel->user_id, channel->mcu_channel_id);
> +
> +	v4l2_dbg(1, debug, &dev->v4l2_dev,
> +		 "channel %d: intermediate buffers: %d x %d bytes\n",
> +		 channel->mcu_channel_id, msg->int_buffers_count,
> +		 msg->int_buffers_size);
> +	err = allocate_intermediate_buffers(channel, msg->int_buffers_count,
> +					    msg->int_buffers_size);
> +	if (err) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "channel %d: failed to allocate intermediate buffers",
> +			 channel->mcu_channel_id);
> +		goto out;
> +	}
> +	allegro_mcu_push_buffer_intermediate(channel);
> +	if (err) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "channel %d: failed to push reference buffers",
> +			 channel->mcu_channel_id);
> +		goto out;
> +	}
> +
> +	v4l2_dbg(1, debug, &dev->v4l2_dev,
> +		 "channel %d: reference buffers: %d x %d bytes\n",
> +		 channel->mcu_channel_id, msg->rec_buffers_count,
> +		 msg->rec_buffers_size);
> +	err = allocate_reference_buffers(channel, msg->rec_buffers_count,
> +					 msg->rec_buffers_size);
> +	if (err) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "channel %d: failed to allocate reference buffers",
> +			 channel->mcu_channel_id);
> +		goto out;
> +	}
> +	err = allegro_mcu_push_buffer_reference(channel);
> +	if (err) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "channel %d: failed to push reference buffers",
> +			 channel->mcu_channel_id);
> +		goto out;
> +	}
> +
> +	channel->created = true;
> +
> +	/*
> +	 * FIXME Need to send CHANNEL_DESTROY if we fail to allocate the
> +	 * intermediate or reference buffers?
> +	 */
> +out:
> +	complete(&channel->completion);
> +	return err;
> +}
> +
> +static int
> +allegro_handle_destroy_channel(struct allegro_dev *dev,
> +		struct mcu_msg_destroy_channel_response *msg)
> +{
> +	struct allegro_channel *channel;
> +
> +	channel = allegro_find_channel_by_channel_id(dev, msg->channel_id);
> +	if (IS_ERR(channel)) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "received %s for unknown channel %d\n",
> +			 msg_type_name(msg->header.type),
> +			 msg->channel_id);
> +		return -EINVAL;
> +	}
> +
> +	v4l2_dbg(2, debug, &dev->v4l2_dev,
> +			"user %d: vcu destroyed channel %d\n",
> +			channel->user_id, channel->mcu_channel_id);
> +	complete(&channel->completion);
> +
> +	return 0;
> +}
> +
> +static int
> +allegro_handle_encode_frame(struct allegro_dev *dev,
> +			    struct mcu_msg_encode_frame_response *msg)
> +{
> +	struct allegro_channel *channel;
> +
> +	channel = allegro_find_channel_by_channel_id(dev, msg->channel_id);
> +	if (IS_ERR(channel)) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "received %s for unknown channel %d\n",
> +			 msg_type_name(msg->header.type),
> +			 msg->channel_id);
> +		return -EINVAL;
> +	}
> +
> +	/* TODO Not sure if I really want to call this from here */
> +	allegro_finish_frame(channel, msg);
> +
> +	return 0;
> +}
> +
> +static int allegro_receive_message(struct allegro_dev *dev)
> +{
> +	struct mcu_msg_header *header;
> +	ssize_t size;
> +	int err = 0;
> +
> +	/*
> +	 * FIXME header is struct mcu_msg_header, but we are allocating memory
> +	 * for a struct mcu_msg_encode_one_frm_response, because we only need
> +	 * to parse the header before we can determine the type and size of
> +	 * the actual message and struct mcu_msg_encode_one_frm_response is
> +	 * currently the largest message.
> +	 */
> +	header = kmalloc(sizeof(struct mcu_msg_encode_frame_response),
> +			 GFP_KERNEL);
> +	if (!header)
> +		return -ENOMEM;
> +
> +	size = allegro_mbox_read(dev, &dev->mbox_status, header,
> +				 sizeof(struct mcu_msg_encode_frame_response));
> +	if (size < sizeof(*header)) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "invalid mbox message (%ld): must be at least %lu\n",
> +			 size, sizeof(*header));
> +		err = -EINVAL;
> +		goto out;
> +	}
> +
> +	switch(header->type) {
> +	case MCU_MSG_TYPE_INIT:
> +		err = allegro_handle_init(dev,
> +				(struct mcu_msg_init_reply *) header);
> +		break;
> +	case MCU_MSG_TYPE_CREATE_CHANNEL:
> +		err = allegro_handle_create_channel(dev,
> +				(struct mcu_msg_create_channel_response *)header);
> +		break;
> +	case MCU_MSG_TYPE_DESTROY_CHANNEL:
> +		err = allegro_handle_destroy_channel(dev,
> +				(struct mcu_msg_destroy_channel_response *)header);
> +		break;
> +	case MCU_MSG_TYPE_ENCODE_FRAME:
> +		err = allegro_handle_encode_frame(dev,
> +				(struct mcu_msg_encode_frame_response *)header);
> +		break;
> +	default:
> +		v4l2_warn(&dev->v4l2_dev,
> +			  "%s: unknown message %s\n",
> +			  __func__, msg_type_name(header->type));
> +		err = -EINVAL;
> +		break;
> +	}
> +
> +out:
> +	kfree(header);
> +
> +	return err;
> +}
> +
> +irqreturn_t allegro_hardirq(int irq, void *data)
> +{
> +	struct allegro_dev *dev = data;
> +	unsigned int status;
> +
> +	regmap_read(dev->regmap, AL5_ITC_CPU_IRQ_STA, &status);
> +	if (!(status & AL5_ITC_CPU_IRQ_STA_TRIGGERED))
> +		return IRQ_NONE;
> +
> +	regmap_write(dev->regmap, AL5_ITC_CPU_IRQ_CLR, status);
> +
> +	/*
> +	 * The downstream driver suggests to wait until the clear has
> +	 * propagated through the hardware, i.e.,
> +	 *
> +	 * 	!(read(AL5_ITC_CPU_IRQ_STA) & AL5_ITC_CPU_IRQ_STA_TRIGGERED)
> +	 *
> +	 * I assume that waiting for the interrupt to be cleared is not
> +	 * required and just continue execution.
> +	 */
> +
> +	return IRQ_WAKE_THREAD;
> +}
> +
> +irqreturn_t allegro_irq_thread(int irq, void *data)
> +{
> +	struct allegro_dev *dev = data;
> +
> +	allegro_receive_message(dev);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void allegro_copy_firmware(struct allegro_dev *dev,
> +				  const u8 * const buf, size_t size)
> +{
> +	int err = 0;
> +
> +	v4l2_dbg(1, debug, &dev->v4l2_dev,
> +		"copy mcu firmware (%lu B) to SRAM\n", size);
> +	err = regmap_bulk_write(dev->sram, 0x0, buf, size / 4);
> +	if (err)
> +		v4l2_err(&dev->v4l2_dev,
> +			 "failed to copy firmware: %d\n", err);
> +}
> +
> +static void allegro_copy_fw_codec(struct allegro_dev *dev,
> +				  const u8 * const buf, size_t size)
> +{
> +	int err;
> +	dma_addr_t icache_offset, dcache_offset;
> +
> +	/*
> +	 * The downstream allocates 600 KB for the codec firmware to have some
> +	 * extra space for "possible extensions." My tests were fine with
> +	 * allocating just enough memory for the actual firmware, but I am not
> +	 * sure that the firmware really does not use the remaining space.
> +	 */
> +	err = allegro_alloc_buffer(dev, &dev->firmware, size);
> +	if (err) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "failed to allocate %lu bytes for firmware\n", size);
> +		return;
> +	}
> +
> +	v4l2_dbg(1, debug, &dev->v4l2_dev,
> +		"copy codec firmware (%ld B) to phys 0x%llx",
> +		size, dev->firmware.paddr);
> +	memcpy(dev->firmware.vaddr, buf, size);
> +
> +	regmap_write(dev->regmap, AXI_ADDR_OFFSET_IP,
> +		     upper_32_bits(dev->firmware.paddr));
> +
> +	/*
> +	 * TODO I am not sure what the icache and dcache offsets are doing.
> +	 * Copy behavior from downstream driver for now.
> +	 */
> +	icache_offset = dev->firmware.paddr - MCU_CACHE_OFFSET;
> +	v4l2_dbg(2, debug, &dev->v4l2_dev,
> +		"icache_offset: msb = 0x%x, lsb = 0x%x\n",
> +		upper_32_bits(icache_offset), lower_32_bits(icache_offset));
> +	regmap_write(dev->regmap, AL5_ICACHE_ADDR_OFFSET_MSB,
> +		     upper_32_bits(icache_offset));
> +	regmap_write(dev->regmap, AL5_ICACHE_ADDR_OFFSET_LSB,
> +		     lower_32_bits(icache_offset));
> +
> +	dcache_offset =
> +	    (dev->firmware.paddr & 0xffffffff00000000) - MCU_CACHE_OFFSET;
> +	v4l2_dbg(2, debug, &dev->v4l2_dev,
> +		 "dcache_offset: msb = 0x%x, lsb = 0x%x\n",
> +		 upper_32_bits(dcache_offset), lower_32_bits(dcache_offset));
> +	regmap_write(dev->regmap, AL5_DCACHE_ADDR_OFFSET_MSB,
> +		     upper_32_bits(dcache_offset));
> +	regmap_write(dev->regmap, AL5_DCACHE_ADDR_OFFSET_LSB,
> +		     lower_32_bits(dcache_offset));
> +}
> +
> +/*
> + * Control functions for the MCU
> + */
> +
> +static void allegro_mcu_enable_interrupts(struct allegro_dev *dev)
> +{
> +	regmap_write(dev->regmap, AL5_ITC_CPU_IRQ_MSK, BIT(0));
> +}
> +
> +static int allegro_mcu_wait_for_sleep(struct allegro_dev *dev)
> +{
> +	unsigned long timeout;
> +	unsigned int status;
> +
> +	timeout = jiffies + msecs_to_jiffies(100);
> +	while (regmap_read(dev->regmap, AL5_MCU_STA, &status) == 0 &&
> +	       status != AL5_MCU_STA_SLEEP) {
> +		if (time_after(jiffies, timeout))
> +			return -ETIMEDOUT;
> +		cpu_relax();
> +	}
> +
> +	return 0;
> +}
> +
> +static int allegro_mcu_start(struct allegro_dev *dev)
> +{
> +	unsigned long timeout;
> +	unsigned int status;
> +	int err;
> +
> +	err = regmap_write(dev->regmap, AL5_MCU_WAKEUP, BIT(0));
> +	if (err)
> +		return err;
> +
> +	timeout = jiffies + msecs_to_jiffies(100);
> +	while (regmap_read(dev->regmap, AL5_MCU_STA, &status) == 0 &&
> +	       status == AL5_MCU_STA_SLEEP) {
> +		if (time_after(jiffies, timeout))
> +			return -ETIMEDOUT;
> +		cpu_relax();
> +	}
> +
> +	err = regmap_write(dev->regmap, AL5_MCU_WAKEUP, 0);
> +	if (err)
> +		return err;
> +
> +	return 0;
> +}
> +
> +static int allegro_mcu_reset(struct allegro_dev *dev)
> +{
> +	int err;
> +
> +	err = regmap_write(dev->regmap,
> +			   AL5_MCU_RESET_MODE, AL5_MCU_RESET_MODE_SLEEP);
> +	if (err < 0)
> +		return err;
> +
> +	err = regmap_write(dev->regmap, AL5_MCU_RESET, AL5_MCU_RESET_SOFT);
> +	if (err < 0)
> +		return err;
> +
> +	return allegro_mcu_wait_for_sleep(dev);
> +}
> +
> +/*
> + * Create the MCU channel
> + *
> + * After the channel has been created, the picture size, format, colorspace
> + * and framerate are fixed. Also the codec, profile, bitrate, etc. cannot be
> + * changed anymore.
> + *
> + * The channel can be created only once. The MCU will accept source buffers
> + * and stream buffers only after a channel has been created.
> + */
> +static int allegro_create_channel(struct allegro_channel *channel)
> +{
> +	struct allegro_dev *dev = channel->dev;
> +	unsigned long timeout;
> +	int err;
> +	enum v4l2_mpeg_video_h264_level min_level;
> +
> +	if (channel->created) {
> +		v4l2_warn(&dev->v4l2_dev,
> +			  "channel already has been created\n");
> +		return 0;
> +	}
> +
> +	channel->user_id = allegro_next_user_id(dev);
> +	if (channel->user_id < 0) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "no free channels available\n");
> +		return -EBUSY;
> +	}
> +	set_bit(channel->user_id, &dev->channel_user_ids);
> +
> +	v4l2_dbg(1, debug, &dev->v4l2_dev,
> +		 "user %d: creating channel (%4.4s, %dx%d@%d) \n",
> +		 channel->user_id,
> +		 (char *)&channel->codec, channel->width, channel->height, 25);
> +
> +	min_level = select_minimum_h264_level(channel->width, channel->height);
> +	if (channel->level < min_level) {
> +		v4l2_warn(&dev->v4l2_dev,
> +			  "user %d: selected Level %s too low: increasing to Level %s\n",
> +			  channel->user_id,
> +			  v4l2_ctrl_get_menu(V4L2_CID_MPEG_VIDEO_H264_LEVEL)[channel->level],
> +			  v4l2_ctrl_get_menu(V4L2_CID_MPEG_VIDEO_H264_LEVEL)[min_level]);
> +		channel->level = min_level;
> +	}
> +
> +	reinit_completion(&channel->completion);
> +	allegro_mcu_send_create_channel(dev, channel);
> +	timeout = wait_for_completion_timeout(&channel->completion,
> +					      msecs_to_jiffies(5000));
> +	if (timeout == 0) {
> +		err = -ETIMEDOUT;
> +		goto err;
> +	}
> +	if (!channel->created) {
> +		/* FIXME Return a proper error code */
> +		err = -1;
> +		goto err;
> +	}
> +
> +	v4l2_dbg(1, debug, &dev->v4l2_dev,
> +		 "channel %d: accepting buffers\n",
> +		 channel->mcu_channel_id);
> +
> +	return 0;
> +
> +err:
> +	clear_bit(channel->user_id, &dev->channel_user_ids);
> +	channel->user_id = -1;
> +	return err;
> +}
> +
> +static void allegro_destroy_channel(struct allegro_channel *channel)
> +{
> +	struct allegro_dev *dev = channel->dev;
> +	unsigned long timeout;
> +
> +	if (!channel->created) {
> +		v4l2_warn(&dev->v4l2_dev,
> +			  "cannot destroy channel: has not been created\n");
> +		return;
> +	}
> +
> +	reinit_completion(&channel->completion);
> +	allegro_mcu_send_destroy_channel(dev, channel);
> +	timeout = wait_for_completion_timeout(&channel->completion,
> +					      msecs_to_jiffies(5000));
> +	if (timeout == 0)
> +		v4l2_warn(&dev->v4l2_dev,
> +			  "timeout while destroying channel %d\n",
> +			  channel->mcu_channel_id);
> +
> +	channel->mcu_channel_id = -1;
> +	channel->created = false;
> +	clear_bit(channel->user_id, &dev->channel_user_ids);
> +	channel->user_id = -1;
> +
> +	destroy_intermediate_buffers(channel);
> +	destroy_reference_buffers(channel);
> +}
> +
> +static void allegro_set_default_params(struct allegro_channel *channel)
> +{
> +	channel->width = ALLEGRO_WIDTH_DEFAULT;
> +	channel->height = ALLEGRO_HEIGHT_DEFAULT;
> +	channel->stride = round_up(channel->width, 32);
> +
> +	channel->colorspace = V4L2_COLORSPACE_REC709;
> +	channel->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> +	channel->quantization = V4L2_QUANTIZATION_DEFAULT;
> +	channel->xfer_func = V4L2_XFER_FUNC_DEFAULT;
> +
> +	channel->pixelformat = V4L2_PIX_FMT_NV12;
> +	channel->sizeimage_raw = channel->stride * channel->height * 3 / 2;
> +
> +	channel->codec = V4L2_PIX_FMT_H264;
> +	channel->profile = V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE;
> +	channel->level =
> +		select_minimum_h264_level(channel->width, channel->height);
> +	channel->sizeimage_encoded =
> +		estimate_stream_size(channel->width, channel->height);
> +
> +	channel->bitrate_mode = V4L2_MPEG_VIDEO_BITRATE_MODE_CBR;
> +	channel->bitrate = maximum_bitrate(channel->level);
> +	channel->bitrate_peak = maximum_bitrate(channel->level);
> +	channel->cpb_size = maximum_cpb_size(channel->level);
> +	channel->gop_size = ALLEGRO_GOP_SIZE_DEFAULT;
> +}
> +
> +static int allegro_queue_setup(struct vb2_queue *vq,
> +			       unsigned int *nbuffers, unsigned int *nplanes,
> +			       unsigned int sizes[],
> +			       struct device *alloc_devs[])
> +{
> +	struct allegro_channel *channel = vb2_get_drv_priv(vq);
> +	struct allegro_dev *dev = channel->dev;
> +
> +	v4l2_dbg(2, debug, &dev->v4l2_dev,
> +		 "%s: queue setup[%s]: nplanes = %d\n",
> +		 vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT ? "output" : "capture",
> +		 *nplanes == 0 ? "REQBUFS" : "CREATE_BUFS", *nplanes);
> +
> +	if (*nplanes != 0) {
> +		if (vq->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> +			if (*nplanes != 1 ||
> +			    sizes[0] < channel->sizeimage_encoded)
> +				return -EINVAL;

Question relating to calculating sizeimage_encoded: is that guaranteed to be
the largest buffer size that is needed to compress a frame? What if it is
not large enough after all? Does the encoder protect against that?

I have a patch pending that allows an encoder to spread the compressed
output over multiple buffers:

https://patchwork.linuxtv.org/patch/53536/

I wonder if this encoder would be able to use it.

> +
> +		} else {
> +			if (*nplanes != 1 || sizes[0] < channel->sizeimage_raw)
> +				return -EINVAL;
> +		}

I would do the shared 'if (*nplanes != 1)' check first.

It makes the code look a bit cleaner.

> +		return 0;
> +	}
> +
> +	if (vq->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> +		*nplanes = 1;
> +		sizes[0] = channel->sizeimage_encoded;
> +	} else {
> +		*nplanes = 1;
> +		sizes[0] = channel->sizeimage_raw;
> +	}

Here too *nplanes = 1; can be moved before the 'if'.

> +
> +	return 0;
> +}
> +
> +static void allegro_buf_queue(struct vb2_buffer *vb)
> +{
> +	struct allegro_channel *channel = vb2_get_drv_priv(vb->vb2_queue);
> +	struct allegro_dev *dev = channel->dev;
> +	struct vb2_queue *vq = vb->vb2_queue;
> +	struct allegro_buffer al_buf;
> +
> +	if (vq->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> +		al_buf.paddr = vb2_dma_contig_plane_dma_addr(vb, 0);
> +		al_buf.size = vb2_plane_size(vb, 0);
> +
> +		if (poison_capture_buffers)
> +			memset(vb2_plane_vaddr(vb, 0), 0x1a, vb2_plane_size(vb, 0));
> +
> +		v4l2_dbg(1, debug, &dev->v4l2_dev,
> +			 "channel %d: queuing stream buffer: paddr: 0x%08llx, %ld bytes\n",
> +			 channel->mcu_channel_id, al_buf.paddr, al_buf.size);
> +		allegro_mcu_send_put_stream_buffer(dev, channel, al_buf);
> +	} else {
> +		v4l2_dbg(1, debug, &dev->v4l2_dev,
> +			 "channel %d: queuing source buffer\n",
> +			 channel->mcu_channel_id);
> +	}
> +
> +	v4l2_m2m_buf_queue(channel->fh.m2m_ctx, to_vb2_v4l2_buffer(vb));
> +}
> +
> +static int allegro_start_streaming(struct vb2_queue *q, unsigned int count)
> +{
> +	struct allegro_channel *channel = vb2_get_drv_priv(q);
> +	struct allegro_dev *dev = channel->dev;
> +
> +	v4l2_dbg(2, debug, &dev->v4l2_dev,
> +		 "%s: start streaming\n",
> +		 q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT ? "output" : "capture");
> +
> +	return 0;
> +}
> +
> +static void allegro_stop_streaming(struct vb2_queue *q)
> +{
> +	struct allegro_channel *channel = vb2_get_drv_priv(q);
> +	struct allegro_dev *dev = channel->dev;
> +	struct vb2_v4l2_buffer *buffer;
> +
> +	v4l2_dbg(2, debug, &dev->v4l2_dev,
> +		 "%s: stop streaming\n",
> +		 q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT ? "output" : "capture");
> +
> +	if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
> +		while ((buffer = v4l2_m2m_src_buf_remove(channel->fh.m2m_ctx)))
> +			v4l2_m2m_buf_done(buffer, VB2_BUF_STATE_ERROR);
> +	} else if (q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> +		/*
> +		 * FIXME The channel is created during queue_setup().
> +		 * Therefore, destroying it in stop_streaming might result in
> +		 * a start_streaming() without a channel, which is really bad.
> +		 */
> +		allegro_destroy_channel(channel);
> +		while ((buffer = v4l2_m2m_dst_buf_remove(channel->fh.m2m_ctx)))
> +			v4l2_m2m_buf_done(buffer, VB2_BUF_STATE_ERROR);
> +	}
> +}
> +
> +static const struct vb2_ops allegro_queue_ops = {
> +	.queue_setup = allegro_queue_setup,
> +	.buf_queue = allegro_buf_queue,
> +	.start_streaming = allegro_start_streaming,
> +	.stop_streaming = allegro_stop_streaming,

You are missing these:

        .wait_prepare    = vb2_ops_wait_prepare,
        .wait_finish     = vb2_ops_wait_finish,

It's the reason for 'test blocking wait: FAIL' in the compliance test.

> +};
> +
> +static int allegro_queue_init(void *priv,
> +			      struct vb2_queue *src_vq,
> +			      struct vb2_queue *dst_vq)
> +{
> +	int err;
> +	struct allegro_channel *channel = priv;
> +
> +	src_vq->dev = &channel->dev->plat_dev->dev;
> +	src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
> +	src_vq->io_modes = VB2_DMABUF | VB2_MMAP;
> +	src_vq->mem_ops = &vb2_dma_contig_memops;
> +	src_vq->drv_priv = channel;
> +	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +	src_vq->ops = &allegro_queue_ops;
> +	src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> +	err = vb2_queue_init(src_vq);
> +	if (err)
> +		return err;
> +
> +	dst_vq->dev = &channel->dev->plat_dev->dev;
> +	dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> +	dst_vq->io_modes = VB2_DMABUF | VB2_MMAP;
> +	dst_vq->mem_ops = &vb2_dma_contig_memops;
> +	dst_vq->drv_priv = channel;
> +	dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +	dst_vq->ops = &allegro_queue_ops;
> +	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> +	err = vb2_queue_init(dst_vq);
> +	if (err)
> +		return err;
> +
> +	return 0;
> +}
> +
> +static int allegro_s_ctrl(struct v4l2_ctrl *ctrl)
> +{
> +	struct allegro_channel *channel = container_of(ctrl->handler,
> +						       struct allegro_channel,
> +						       ctrl_handler);
> +	struct allegro_dev *dev = channel->dev;
> +
> +	v4l2_dbg(1, debug, &dev->v4l2_dev,
> +		 "s_ctrl: id = %d, val = %d\n", ctrl->id, ctrl->val);
> +
> +	switch (ctrl->id) {
> +	case V4L2_CID_MPEG_VIDEO_H264_LEVEL:
> +		channel->level = ctrl->val;
> +		break;
> +	case V4L2_CID_MPEG_VIDEO_BITRATE_MODE:
> +		channel->bitrate_mode = ctrl->val;
> +		break;
> +	case V4L2_CID_MPEG_VIDEO_BITRATE:
> +		channel->bitrate = ctrl->val;
> +		break;
> +	case V4L2_CID_MPEG_VIDEO_BITRATE_PEAK:
> +		channel->bitrate_peak = ctrl->val;
> +		break;
> +	case V4L2_CID_MPEG_VIDEO_H264_CPB_SIZE:
> +		channel->cpb_size = ctrl->val;
> +		break;
> +	case V4L2_CID_MPEG_VIDEO_GOP_SIZE:
> +		channel->gop_size = ctrl->val;
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_ctrl_ops allegro_ctrl_ops = {
> +	.s_ctrl = allegro_s_ctrl,
> +};
> +
> +static int allegro_open(struct file *file)
> +{
> +	struct video_device *vdev = video_devdata(file);
> +	struct allegro_dev *dev = video_get_drvdata(vdev);
> +	struct allegro_channel *channel = NULL;
> +	u64 mask;
> +
> +	channel = kzalloc(sizeof(*channel), GFP_KERNEL);
> +	if (!channel)
> +		return -ENOMEM;
> +
> +	v4l2_fh_init(&channel->fh, vdev);
> +	file->private_data = &channel->fh;
> +	v4l2_fh_add(&channel->fh);
> +
> +	init_completion(&channel->completion);
> +
> +	channel->dev = dev;
> +
> +	allegro_set_default_params(channel);
> +
> +	v4l2_ctrl_handler_init(&channel->ctrl_handler, 0);
> +	v4l2_ctrl_new_std_menu(&channel->ctrl_handler, &allegro_ctrl_ops,
> +			       V4L2_CID_MPEG_VIDEO_H264_PROFILE,
> +			       V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE, 0x0,
> +			       V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE);
> +	mask = 1 << V4L2_MPEG_VIDEO_H264_LEVEL_1B;
> +	v4l2_ctrl_new_std_menu(&channel->ctrl_handler, &allegro_ctrl_ops,
> +			       V4L2_CID_MPEG_VIDEO_H264_LEVEL,
> +			       V4L2_MPEG_VIDEO_H264_LEVEL_5_1, mask,
> +			       V4L2_MPEG_VIDEO_H264_LEVEL_5_1);
> +	v4l2_ctrl_new_std_menu(&channel->ctrl_handler, &allegro_ctrl_ops,
> +			       V4L2_CID_MPEG_VIDEO_BITRATE_MODE,
> +			       V4L2_MPEG_VIDEO_BITRATE_MODE_CBR, 0,
> +			       channel->bitrate_mode);
> +	v4l2_ctrl_new_std(&channel->ctrl_handler, &allegro_ctrl_ops,
> +			  V4L2_CID_MPEG_VIDEO_BITRATE,
> +			  0, maximum_bitrate(V4L2_MPEG_VIDEO_H264_LEVEL_5_1),
> +			  1, channel->bitrate);
> +	v4l2_ctrl_new_std(&channel->ctrl_handler, &allegro_ctrl_ops,
> +			  V4L2_CID_MPEG_VIDEO_BITRATE_PEAK,
> +			  0, maximum_bitrate(V4L2_MPEG_VIDEO_H264_LEVEL_5_1),
> +			  1, channel->bitrate_peak);
> +	v4l2_ctrl_new_std(&channel->ctrl_handler, &allegro_ctrl_ops,
> +			  V4L2_CID_MPEG_VIDEO_H264_CPB_SIZE,
> +			  0, maximum_cpb_size(V4L2_MPEG_VIDEO_H264_LEVEL_5_1),
> +			  1, channel->cpb_size);
> +	v4l2_ctrl_new_std(&channel->ctrl_handler, &allegro_ctrl_ops,
> +			  V4L2_CID_MPEG_VIDEO_GOP_SIZE,
> +			  0, ALLEGRO_GOP_SIZE_MAX,
> +			  1, channel->gop_size);
> +	channel->fh.ctrl_handler = &channel->ctrl_handler;
> +
> +	channel->mcu_channel_id = -1;
> +	channel->user_id = -1;
> +	channel->created = false;
> +	channel->stop = false;
> +
> +	INIT_LIST_HEAD(&channel->buffers_reference);
> +	INIT_LIST_HEAD(&channel->buffers_intermediate);
> +
> +	channel->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->m2m_dev, channel,
> +						allegro_queue_init);
> +
> +	list_add(&channel->list, &dev->channels);
> +
> +	return 0;
> +}
> +
> +static int allegro_release(struct file *file)
> +{
> +	struct allegro_channel *channel = fh_to_channel(file->private_data);
> +
> +	list_del(&channel->list);
> +
> +	v4l2_m2m_ctx_release(channel->fh.m2m_ctx);
> +
> +	v4l2_ctrl_handler_free(&channel->ctrl_handler);
> +
> +	v4l2_fh_del(&channel->fh);
> +	v4l2_fh_exit(&channel->fh);
> +
> +	kfree(channel);
> +
> +	return 0;
> +}
> +
> +static int allegro_querycap(struct file *file, void *fh,
> +			    struct v4l2_capability *cap)
> +{
> +	struct video_device *vdev = video_devdata(file);
> +	struct allegro_dev *dev = video_get_drvdata(vdev);
> +
> +	strscpy(cap->driver, KBUILD_MODNAME, sizeof(cap->driver));
> +	strscpy(cap->card, "Allegro DVT Video Encoder", sizeof(cap->card));
> +	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> +		 dev_name(&dev->plat_dev->dev));
> +
> +	cap->device_caps = V4L2_CAP_VIDEO_M2M | V4L2_CAP_STREAMING;
> +	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;

Don't set this here. Instead set the device_caps field of struct video_device.

The V4L2 core will then fill in device_caps and capabilities for you
based on that information.

> +
> +	return 0;
> +}
> +
> +static int allegro_enum_fmt_vid(struct file *file, void *fh,
> +				struct v4l2_fmtdesc *f)
> +{
> +	unsigned int num_formats = 1;
> +
> +	switch (f->type) {
> +	case V4L2_BUF_TYPE_VIDEO_OUTPUT:
> +		f->pixelformat = V4L2_PIX_FMT_NV12;
> +		break;
> +	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
> +		f->pixelformat = V4L2_PIX_FMT_H264;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	if (f->index >= num_formats)

Just write 'if (f->index)' and drop num_formats.

> +		return -EINVAL;
> +	return 0;
> +}
> +
> +static int allegro_g_fmt_vid_cap(struct file *file, void *fh,
> +				 struct v4l2_format *f)
> +{
> +	struct allegro_channel *channel = fh_to_channel(fh);
> +
> +	if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +		return -EINVAL;

No need for this check, this function is only called for this buffer type
anyway.

> +
> +	f->fmt.pix.field = V4L2_FIELD_NONE;
> +	f->fmt.pix.width = channel->width;
> +	f->fmt.pix.height = channel->height;
> +
> +	f->fmt.pix.colorspace = channel->colorspace;
> +	f->fmt.pix.ycbcr_enc = channel->ycbcr_enc;
> +	f->fmt.pix.quantization = channel->quantization;
> +	f->fmt.pix.xfer_func = channel->xfer_func;
> +
> +	f->fmt.pix.pixelformat = channel->codec;
> +	f->fmt.pix.flags = V4L2_FMT_FLAG_COMPRESSED;

V4L2_FMT_FLAG_COMPRESSED is used with ENUM_FMT, not with G_FMT.
You don't need to set it in allegro_enum_fmt_vid() either since the
V4L2 core will do that for you. So just drop this line.

> +	f->fmt.pix.bytesperline = channel->width;
> +	f->fmt.pix.sizeimage = channel->sizeimage_encoded;
> +
> +	return 0;
> +}
> +
> +static int allegro_try_fmt_vid_cap(struct file *file, void *fh,
> +				   struct v4l2_format *f)
> +{
> +	f->fmt.pix.field = V4L2_FIELD_NONE;
> +
> +	f->fmt.pix.width = clamp_t(__u32, f->fmt.pix.width,
> +				   ALLEGRO_WIDTH_MIN, ALLEGRO_WIDTH_MAX);
> +	f->fmt.pix.height = clamp_t(__u32, f->fmt.pix.height,
> +				    ALLEGRO_HEIGHT_MIN, ALLEGRO_HEIGHT_MAX);
> +
> +	f->fmt.pix.pixelformat = V4L2_PIX_FMT_H264;
> +	f->fmt.pix.flags = V4L2_FMT_FLAG_COMPRESSED;

Same here, drop this line.

> +	f->fmt.pix.bytesperline = f->fmt.pix.width;
> +	f->fmt.pix.sizeimage =
> +		estimate_stream_size(f->fmt.pix.width, f->fmt.pix.height);
> +
> +	return 0;
> +}
> +
> +static int allegro_g_fmt_vid_out(struct file *file, void *fh,
> +				 struct v4l2_format *f)
> +{
> +	struct allegro_channel *channel = fh_to_channel(fh);
> +
> +	if (f->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
> +		return -EINVAL;

Drop this check.

> +
> +	f->fmt.pix.field = V4L2_FIELD_NONE;
> +
> +	f->fmt.pix.width = channel->width;
> +	f->fmt.pix.height = channel->height;
> +
> +	f->fmt.pix.colorspace = channel->colorspace;
> +	f->fmt.pix.ycbcr_enc = channel->ycbcr_enc;
> +	f->fmt.pix.quantization = channel->quantization;
> +	f->fmt.pix.xfer_func = channel->xfer_func;
> +
> +	f->fmt.pix.pixelformat = channel->pixelformat;
> +	f->fmt.pix.bytesperline = channel->stride;
> +	f->fmt.pix.sizeimage = channel->sizeimage_raw;
> +
> +	return 0;
> +}
> +
> +static int allegro_try_fmt_vid_out(struct file *file, void *fh,
> +				   struct v4l2_format *f)
> +{
> +	f->fmt.pix.field = V4L2_FIELD_NONE;
> +
> +	f->fmt.pix.width = clamp_t(__u32, f->fmt.pix.width,
> +				   ALLEGRO_WIDTH_MIN, ALLEGRO_WIDTH_MAX);
> +	f->fmt.pix.height = clamp_t(__u32, f->fmt.pix.height,
> +				    ALLEGRO_HEIGHT_MIN, ALLEGRO_HEIGHT_MAX);
> +
> +	f->fmt.pix.pixelformat = V4L2_PIX_FMT_NV12;
> +	f->fmt.pix.bytesperline = round_up(f->fmt.pix.width, 32);
> +	f->fmt.pix.sizeimage =
> +		f->fmt.pix.bytesperline * f->fmt.pix.height * 3 / 2;
> +
> +	return 0;
> +}
> +
> +static int allegro_s_fmt_vid_out(struct file *file, void *fh,
> +				 struct v4l2_format *f)
> +{
> +	struct allegro_channel *channel = fh_to_channel(fh);
> +	int err;
> +
> +	err = allegro_try_fmt_vid_out(file, fh, f);
> +	if (err)
> +		return err;
> +
> +	channel->width = f->fmt.pix.width;
> +	channel->height = f->fmt.pix.height;
> +	channel->stride = f->fmt.pix.bytesperline;
> +	channel->sizeimage_raw = f->fmt.pix.sizeimage;
> +
> +	channel->colorspace = f->fmt.pix.colorspace;
> +	channel->ycbcr_enc = f->fmt.pix.ycbcr_enc;
> +	channel->quantization = f->fmt.pix.quantization;
> +	channel->xfer_func = f->fmt.pix.xfer_func;
> +
> +	channel->level =
> +		select_minimum_h264_level(channel->width, channel->height);
> +	channel->sizeimage_encoded =
> +		estimate_stream_size(channel->width, channel->height);
> +
> +	return 0;
> +}
> +
> +static int allegro_try_encoder_cmd(struct file *file, void *fh,
> +				   struct v4l2_encoder_cmd *cmd)
> +{
> +	if (cmd->cmd != V4L2_ENC_CMD_STOP)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int allegro_encoder_cmd(struct file *file, void *fh,
> +			       struct v4l2_encoder_cmd *cmd)
> +{
> +	struct allegro_channel *channel = fh_to_channel(fh);
> +	struct allegro_dev *dev = channel->dev;
> +	int err;
> +
> +	err = allegro_try_encoder_cmd(file, fh, cmd);
> +	if (err)
> +		return err;
> +
> +	v4l2_dbg(2, debug, &dev->v4l2_dev,
> +		 "channel %d: stop encoder\n", channel->mcu_channel_id);
> +
> +	channel->stop = true;
> +
> +	return 0;
> +}
> +
> +static int allegro_ioctl_streamon(struct file *file, void *priv,
> +				  enum v4l2_buf_type type)
> +{
> +	struct v4l2_fh *fh = file->private_data;
> +	struct allegro_channel *channel = fh_to_channel(fh);
> +	struct allegro_dev *dev = channel->dev;
> +	int err;
> +
> +	/*
> +	 * The channel should be created as late as possible, but has to be
> +	 * created before we queue stream buffers on capture. Therefore,
> +	 * start_streaming() is already too late.
> +	 */
> +	if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> +		err = allegro_create_channel(channel);
> +		if (err) {
> +			v4l2_err(&dev->v4l2_dev, "failed to create channel");
> +			return err;
> +		}
> +	}
> +
> +	return v4l2_m2m_streamon(file, fh->m2m_ctx, type);
> +}
> +
> +static const struct v4l2_ioctl_ops allegro_ioctl_ops = {
> +	.vidioc_querycap = allegro_querycap,
> +	.vidioc_enum_fmt_vid_cap = allegro_enum_fmt_vid,
> +	.vidioc_enum_fmt_vid_out = allegro_enum_fmt_vid,
> +	.vidioc_g_fmt_vid_cap = allegro_g_fmt_vid_cap,
> +	.vidioc_try_fmt_vid_cap = allegro_try_fmt_vid_cap,
> +	.vidioc_s_fmt_vid_cap = allegro_try_fmt_vid_cap,
> +	.vidioc_g_fmt_vid_out = allegro_g_fmt_vid_out,
> +	.vidioc_try_fmt_vid_out = allegro_try_fmt_vid_out,
> +	.vidioc_s_fmt_vid_out = allegro_s_fmt_vid_out,
> +
> +	.vidioc_create_bufs = v4l2_m2m_ioctl_create_bufs,
> +	.vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs,
> +
> +	.vidioc_expbuf = v4l2_m2m_ioctl_expbuf,
> +	.vidioc_querybuf = v4l2_m2m_ioctl_querybuf,
> +	.vidioc_qbuf = v4l2_m2m_ioctl_qbuf,
> +	.vidioc_dqbuf = v4l2_m2m_ioctl_dqbuf,

Add:

        .vidioc_prepare_buf     = v4l2_m2m_ioctl_prepare_buf,

> +
> +	.vidioc_streamon = allegro_ioctl_streamon,
> +	.vidioc_streamoff = v4l2_m2m_ioctl_streamoff,
> +
> +	.vidioc_try_encoder_cmd = allegro_try_encoder_cmd,
> +	.vidioc_encoder_cmd = allegro_encoder_cmd,
> +
> +	.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
> +	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
> +};
> +
> +static const struct v4l2_file_operations allegro_fops = {
> +	.owner = THIS_MODULE,
> +	.open = allegro_open,
> +	.release = allegro_release,
> +	.poll = v4l2_m2m_fop_poll,
> +	.unlocked_ioctl = video_ioctl2,
> +	.mmap = v4l2_m2m_fop_mmap,
> +};
> +
> +static int allegro_register_device(struct allegro_dev *dev)
> +{
> +	struct video_device *video_dev = &dev->video_dev;
> +
> +	strscpy(video_dev->name, "allegro", sizeof(video_dev->name));
> +	video_dev->fops = &allegro_fops;
> +	video_dev->ioctl_ops = &allegro_ioctl_ops;
> +	video_dev->release = video_device_release_empty;
> +	video_dev->lock = &dev->lock;
> +	video_dev->v4l2_dev = &dev->v4l2_dev;
> +	video_dev->vfl_dir = VFL_DIR_M2M;
> +	video_set_drvdata(video_dev, dev);
> +
> +	v4l2_disable_ioctl(video_dev, VIDIOC_CROPCAP);
> +	v4l2_disable_ioctl(video_dev, VIDIOC_G_CROP);
> +	v4l2_disable_ioctl(video_dev, VIDIOC_S_CROP);

Huh? Why do you need this?

> +
> +	return video_register_device(video_dev, VFL_TYPE_GRABBER, 0);
> +}
> +
> +static void allegro_device_run(void *priv)
> +{
> +	struct allegro_channel *channel = priv;
> +	struct allegro_dev *dev = channel->dev;
> +	struct vb2_v4l2_buffer *src_buf;
> +	dma_addr_t src_y;
> +	dma_addr_t src_uv;
> +
> +	src_buf = v4l2_m2m_next_src_buf(channel->fh.m2m_ctx);
> +
> +	src_y = vb2_dma_contig_plane_dma_addr(&src_buf->vb2_buf, 0);
> +	src_uv = src_y + (channel->stride * channel->height);
> +
> +	allegro_mcu_send_encode_frame(dev, channel, src_y, src_uv);
> +}
> +
> +static int allegro_job_ready(void *priv)
> +{
> +	struct allegro_channel *channel = priv;
> +	struct allegro_dev *dev = channel->dev;
> +
> +	if (v4l2_m2m_num_src_bufs_ready(channel->fh.m2m_ctx) == 0) {
> +		v4l2_dbg(2, debug, &dev->v4l2_dev,
> +			 "channel %d: no frame buffers\n",
> +			 channel->mcu_channel_id);
> +		return false;
> +	}
> +
> +	if (v4l2_m2m_num_dst_bufs_ready(channel->fh.m2m_ctx) == 0) {
> +		v4l2_dbg(2, debug, &dev->v4l2_dev,
> +			 "channel %d: no stream buffers\n",
> +			 channel->mcu_channel_id);
> +		return false;
> +	}
> +
> +	return true;
> +}

This is the default check, i.e. job_ready isn't even called unless there
is at least one capture and at least one output buffer ready, so as far as
I can see you can just drop job_ready.

> +
> +static void allegro_job_abort(void *priv)
> +{
> +	struct allegro_channel *channel = priv;
> +	struct allegro_dev *dev = channel->dev;
> +
> +	v4l2_dbg(1, debug, &dev->v4l2_dev, "%s\n", __func__);
> +}

Same for job_abort, just drop it.

> +
> +static const struct v4l2_m2m_ops allegro_m2m_ops = {
> +	.device_run = allegro_device_run,
> +	.job_ready = allegro_job_ready,
> +	.job_abort = allegro_job_abort,
> +};
> +
> +static void allegro_fw_callback(const struct firmware *fw, void *context)
> +{
> +	struct allegro_dev *dev = context;
> +	const char *fw_codec_name = "al5e.fw";
> +	const struct firmware *fw_codec;
> +	int err;
> +	const struct fw_info *info;
> +
> +	info = allegro_get_firmware_info(dev, fw);
> +	if (!info) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "mcu firmware %s is not supported\n", fw_codec_name);
> +		release_firmware(fw);
> +		return;
> +	}
> +	v4l2_dbg(1, debug, &dev->v4l2_dev,
> +		 "detected mcu firmware version '%s'\n", info->version);
> +
> +	allegro_mcu_reset(dev);
> +
> +	allegro_copy_firmware(dev, fw->data, fw->size);
> +	release_firmware(fw);
> +
> +	v4l2_dbg(1, debug, &dev->v4l2_dev,
> +		 "requesting firmware '%s'\n", fw_codec_name);
> +	err = request_firmware(&fw_codec, fw_codec_name, &dev->plat_dev->dev);
> +	if (err) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "cannot find codec firmware '%s'\n", fw_codec_name);
> +		release_firmware(fw_codec);
> +		return;
> +	}
> +	allegro_copy_fw_codec(dev, fw_codec->data, fw_codec->size);
> +	release_firmware(fw_codec);
> +
> +	allegro_mbox_init(dev, &dev->mbox_command, MAILBOX_CMD, MAILBOX_SIZE);
> +	allegro_mbox_init(dev, &dev->mbox_status, MAILBOX_STATUS, MAILBOX_SIZE);
> +
> +	allegro_mcu_enable_interrupts(dev);
> +	allegro_mcu_start(dev);
> +
> +	/* The MCU sends an INIT message once it has started. */
> +	err = allegro_mcu_wait_for_init_timeout(dev, 5000);
> +	if (err == -ETIMEDOUT) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "no INIT from MCU after start\n");
> +		return;
> +	}
> +
> +	err =
> +	    allegro_alloc_buffer(dev, &dev->suballocator,
> +				 MCU_SUBALLOCATOR_SIZE);
> +	if (err) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "failed to allocate %u bytes for suballocator\n",
> +			 MCU_SUBALLOCATOR_SIZE);
> +		return;
> +	}
> +
> +	allegro_mcu_send_init(dev, dev->suballocator.paddr,
> +			      dev->suballocator.size);
> +	err = allegro_mcu_wait_for_init_timeout(dev, 5000);
> +	if (err == -ETIMEDOUT) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "failed to configure sub-allocator\n");
> +		return;
> +	}
> +
> +	dev->m2m_dev = v4l2_m2m_init(&allegro_m2m_ops);
> +	if (IS_ERR(dev->m2m_dev)) {
> +		v4l2_err(&dev->v4l2_dev, "failed to init mem2mem device\n");
> +		return;
> +	}
> +
> +	err = allegro_register_device(dev);
> +	if (err) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "Failed to register devices\n");
> +		return;
> +	}
> +
> +	v4l2_dbg(1, debug, &dev->v4l2_dev,
> +		 "allegro codec registered as /dev/video%d\n",
> +		 dev->video_dev.num);
> +}
> +
> +static int allegro_firmware_request(struct allegro_dev *dev)

Please add a _nowait suffix to this function name. It should be
explicit that this function doesn't wait for the FW load to
finish.

> +{
> +	const char *fw = "al5e_b.fw";
> +
> +	v4l2_dbg(1, debug, &dev->v4l2_dev,
> +		 "requesting firmware '%s'\n", fw);
> +	return request_firmware_nowait(THIS_MODULE, true, fw,
> +				       &dev->plat_dev->dev, GFP_KERNEL, dev,
> +				       allegro_fw_callback);
> +}
> +
> +static int allegro_probe(struct platform_device *pdev)
> +{
> +	struct allegro_dev *dev;
> +	struct resource *res, *sram_res;
> +	int ret;
> +	int irq;
> +	void __iomem *regs, *sram_regs;
> +
> +	dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
> +	if (!dev)
> +		return -ENOMEM;
> +	dev->plat_dev = pdev;
> +	init_completion(&dev->init_complete);
> +	INIT_LIST_HEAD(&dev->channels);
> +
> +	mutex_init(&dev->lock);
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> +	if (!res) {
> +		dev_err(&pdev->dev,
> +			"regs resource missing from device tree\n");
> +		return -EINVAL;
> +	}
> +	regs = devm_ioremap_nocache(&pdev->dev, res->start, resource_size(res));
> +	if (IS_ERR(regs)) {
> +		dev_err(&pdev->dev, "failed to map registers\n");
> +		return PTR_ERR(regs);
> +	}
> +	dev->regmap = devm_regmap_init_mmio(&pdev->dev, regs,
> +					    &allegro_regmap_config);
> +	if (IS_ERR(dev->regmap)) {
> +		dev_err(&pdev->dev, "failed to init regmap\n");
> +		return PTR_ERR(dev->regmap);
> +	}
> +
> +	sram_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "sram");
> +	if (!sram_res) {
> +		dev_err(&pdev->dev,
> +			"sram resource missing from device tree\n");
> +		return -EINVAL;
> +	}
> +	sram_regs = devm_ioremap_nocache(&pdev->dev,
> +					 sram_res->start,
> +					 resource_size(sram_res));
> +	if (IS_ERR(sram_regs)) {
> +		dev_err(&pdev->dev, "failed to map sram\n");
> +		return PTR_ERR(sram_regs);
> +	}
> +	dev->sram = devm_regmap_init_mmio(&pdev->dev, sram_regs,
> +					  &allegro_sram_config);
> +	if (IS_ERR(dev->sram)) {
> +		dev_err(&pdev->dev, "failed to init sram\n");
> +		return PTR_ERR(dev->sram);
> +	}
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0) {
> +		dev_err(&pdev->dev, "failed to get irq resource\n");
> +		return irq;
> +	}
> +	ret = devm_request_threaded_irq(&pdev->dev, irq,
> +					allegro_hardirq,
> +					allegro_irq_thread,
> +					IRQF_SHARED, dev_name(&pdev->dev), dev);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to request irq: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
> +	if (ret)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, dev);
> +
> +	ret = allegro_firmware_request(dev);
> +	if (ret < 0) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "failed to request firmware: %d\n", ret);
> +		return ret;
> +	}

Hmm, what happens if the FW load fails? Can you safely remove this
driver? Just want to make sure you tested this.

> +
> +	return 0;
> +}
> +
> +static int allegro_remove(struct platform_device *pdev)
> +{
> +	struct allegro_dev *dev = platform_get_drvdata(pdev);
> +
> +	allegro_free_buffer(dev, &dev->firmware);
> +	allegro_free_buffer(dev, &dev->suballocator);
> +
> +	video_unregister_device(&dev->video_dev);
> +
> +	v4l2_m2m_release(dev->m2m_dev);
> +
> +	v4l2_device_unregister(&dev->v4l2_dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id allegro_dt_ids[] = {
> +	{ .compatible = "allegro,al5e-1.1" },
> +	{ /* sentinel */ }
> +};
> +
> +MODULE_DEVICE_TABLE(of, allegro_dt_ids);
> +
> +static struct platform_driver allegro_driver = {
> +	.probe = allegro_probe,
> +	.remove = allegro_remove,
> +	.driver = {
> +		.name = "allegro",
> +		.of_match_table = of_match_ptr(allegro_dt_ids),
> +	},
> +};
> +
> +module_platform_driver(allegro_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Michael Tretter <kernel@xxxxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("Allegro DVT encoder and decoder driver");
> 

Regards,

	Hans




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux