All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/12] media: allegro: Add support for firmware 2019.2
@ 2020-07-13 14:42 Michael Tretter
  2020-07-13 14:42 ` [PATCH v2 01/12] media: allegro: rework mbox handling Michael Tretter
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: Michael Tretter @ 2020-07-13 14:42 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab, kernel, Michael Tretter

This is v2 of the series to add support for the firmware version 2019.2 to the
allegro driver.

The changes compared to v1:

- Change the return type of allegro_mbox_notify() to void. The return
  value was neither checked, nor is there anything we can do, if an error
  occurs, thus there is no need to return an error.

- Add the string.h to allegro-mail.c, which is necessary for memcpy().

Motivation of the patch series in the cover letter of v1:

The updated firmware contains updates and bugfixes. For example, VBR encoding
works much more reliable than with the firmware 2018.2. Unfortunately, I was
not able to find an actual changelog that lists the firmware changes. Even
more unfortunately, the firmware ABI is not stable across firmware versions.

Therefore, this series changes how messages for the mailbox are created.
Previously, the driver defined a struct mcu_msg_* for each message that can be
written directly into the mailbox. This approach is incompatible with support
for more than one firmware version. Now the driver converts the struct
mcu_msg_* into the binary format that is expected by the firmware. The struct
mcu_msg_* are still the same to reduce changes to the code that is used to
prepare the messages.

This separation allows to change the struct mcu_msg_* more freely, because it
is not bound to the firmware ABI anymore and allows to get rid of ugly hacks
like using the message size to determine the number of elements in the
PUSH_BUFFER_INTERMEDIATE and PUSH_BUFFER_REFERENCE message.

Updated v4l2-compliance result:

v4l2-compliance SHA: d366a4e439599450ae1a82b23a37fc8e6b38d1ab, 64 bits, 64-bit time_t

Compliance test for allegro device /dev/video3:

Driver Info:
	Driver name      : allegro
	Card type        : Allegro DVT Video Encoder
	Bus info         : platform:a0009000.video-codec
	Driver version   : 5.8.0
	Capabilities     : 0x84208000
		Video Memory-to-Memory
		Streaming
		Extended Pix Format
		Device Capabilities
	Device Caps      : 0x04208000
		Video Memory-to-Memory
		Streaming
		Extended Pix Format
	Detected Stateful Encoder

Required ioctls:
	test VIDIOC_QUERYCAP: OK

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

	test invalid ioctls: OK
Debug ioctls:
	test VIDIOC_DBG_G/S_REGISTER: OK
	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: 16 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 (Not Supported)
	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)

Test input 0:

Streaming ioctls:
	test read/write: OK (Not Supported)
	test blocking wait: OK
	Video Capture: Captured 60 buffers
	test MMAP (select): OK
	Video Capture: Captured 60 buffers
	test MMAP (epoll): OK
	test USERPTR (select): OK (Not Supported)
	test DMABUF: Cannot test, specify --expbuf-device

Total for allegro device /dev/video3: 50, Succeeded: 50, Failed: 0, Warnings: 0

Michael

Changelog:

v1 -> v2:

- change allegro_mbox_notify() to void
- add missing string.h header

Michael Tretter (12):
  media: allegro: rework mbox handling
  media: allegro: rework read/write to mailbox
  media: allegro: add explicit mail encoding and decoding
  media: allegro: add field for number of buffers
  media: allegro: don't pack MCU messages
  media: allegro: support handling firmware dependent values
  media: allegro: encode bit fields separately
  media: allegro: add config blob for channel
  media: allegro: set num_ref_idx using response of configured channels
  media: allegro: drop length field from message header
  media: allegro: add a version field to mcu messages
  media: allegro: add support for allegro firmware 2019.2

 .../staging/media/allegro-dvt/allegro-core.c  | 383 +++++++------
 .../staging/media/allegro-dvt/allegro-mail.c  | 507 ++++++++++++++++++
 .../staging/media/allegro-dvt/allegro-mail.h  | 111 ++--
 3 files changed, 800 insertions(+), 201 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 01/12] media: allegro: rework mbox handling
  2020-07-13 14:42 [PATCH v2 00/12] media: allegro: Add support for firmware 2019.2 Michael Tretter
@ 2020-07-13 14:42 ` Michael Tretter
  2020-07-13 14:42 ` [PATCH v2 02/12] media: allegro: rework read/write to mailbox Michael Tretter
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Michael Tretter @ 2020-07-13 14:42 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab, kernel, Michael Tretter

Add a send/notify abstraction for the mailbox and separate the message
handling in the driver from the code to read and write message to the
mailbox.

This untangles how mails are written into the MCU's SRAM and signaled to
the MCU from the protocol between the driver and the firmware.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>

---
Changelog:

v1 -> v2:
- change allegro_mbox_notify() to void
---
 .../staging/media/allegro-dvt/allegro-core.c  | 145 +++++++++++-------
 1 file changed, 87 insertions(+), 58 deletions(-)

diff --git a/drivers/staging/media/allegro-dvt/allegro-core.c b/drivers/staging/media/allegro-dvt/allegro-core.c
index 3ed66aae741d..11333f900302 100644
--- a/drivers/staging/media/allegro-dvt/allegro-core.c
+++ b/drivers/staging/media/allegro-dvt/allegro-core.c
@@ -105,9 +105,11 @@ struct allegro_buffer {
 	struct list_head head;
 };
 
+struct allegro_dev;
 struct allegro_channel;
 
 struct allegro_mbox {
+	struct allegro_dev *dev;
 	unsigned int head;
 	unsigned int tail;
 	unsigned int data;
@@ -134,8 +136,8 @@ struct allegro_dev {
 	struct completion init_complete;
 
 	/* The mailbox interface */
-	struct allegro_mbox mbox_command;
-	struct allegro_mbox mbox_status;
+	struct allegro_mbox *mbox_command;
+	struct allegro_mbox *mbox_status;
 
 	/*
 	 * The downstream driver limits the users to 64 users, thus I can use
@@ -583,12 +585,20 @@ static void allegro_free_buffer(struct allegro_dev *dev,
  * 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)
+static void allegro_mcu_interrupt(struct allegro_dev *dev);
+static void allegro_handle_message(struct allegro_dev *dev,
+				   union mcu_msg_response *msg);
+
+static struct allegro_mbox *allegro_mbox_init(struct allegro_dev *dev,
+					      unsigned int base, size_t size)
 {
+	struct allegro_mbox *mbox;
+
+	mbox = devm_kmalloc(&dev->plat_dev->dev, sizeof(*mbox), GFP_KERNEL);
 	if (!mbox)
-		return -EINVAL;
+		return ERR_PTR(-ENOMEM);
+
+	mbox->dev = dev;
 
 	mbox->head = base;
 	mbox->tail = base + 0x4;
@@ -599,7 +609,7 @@ static int allegro_mbox_init(struct allegro_dev *dev,
 	regmap_write(dev->sram, mbox->head, 0);
 	regmap_write(dev->sram, mbox->tail, 0);
 
-	return 0;
+	return mbox;
 }
 
 static int allegro_mbox_write(struct allegro_dev *dev,
@@ -713,9 +723,50 @@ static ssize_t allegro_mbox_read(struct allegro_dev *dev,
 	return size;
 }
 
-static void allegro_mcu_interrupt(struct allegro_dev *dev)
+/**
+ * allegro_mbox_send() - Send a message via the mailbox
+ * @mbox: the mailbox which is used to send the message
+ * @msg: the message to send
+ */
+static int allegro_mbox_send(struct allegro_mbox *mbox, void *msg)
 {
-	regmap_write(dev->regmap, AL5_MCU_INTERRUPT, BIT(0));
+	struct allegro_dev *dev = mbox->dev;
+	struct mcu_msg_header *header = msg;
+	ssize_t size = sizeof(*header) + header->length;
+	int err;
+
+	err = allegro_mbox_write(dev, mbox, msg, size);
+	if (err)
+		goto out;
+
+	allegro_mcu_interrupt(dev);
+
+out:
+	return err;
+}
+
+/**
+ * allegro_mbox_notify() - Notify the mailbox about a new message
+ * @mbox: The allegro_mbox to notify
+ */
+static void allegro_mbox_notify(struct allegro_mbox *mbox)
+{
+	struct allegro_dev *dev = mbox->dev;
+	union mcu_msg_response *msg;
+	ssize_t size;
+
+	msg = kmalloc(sizeof(*msg), GFP_KERNEL);
+	if (!msg)
+		return;
+
+	size = allegro_mbox_read(dev, mbox, msg, sizeof(*msg));
+	if (size < 0)
+		goto out;
+
+	allegro_handle_message(dev, msg);
+
+out:
+	kfree(msg);
 }
 
 static void allegro_mcu_send_init(struct allegro_dev *dev,
@@ -736,8 +787,7 @@ static void allegro_mcu_send_init(struct allegro_dev *dev,
 	msg.l2_cache[1] = -1;
 	msg.l2_cache[2] = -1;
 
-	allegro_mbox_write(dev, &dev->mbox_command, &msg, sizeof(msg));
-	allegro_mcu_interrupt(dev);
+	allegro_mbox_send(dev->mbox_command, &msg);
 }
 
 static u32 v4l2_pixelformat_to_mcu_format(u32 pixelformat)
@@ -946,8 +996,7 @@ static int allegro_mcu_send_create_channel(struct allegro_dev *dev,
 
 	fill_create_channel_param(channel, &msg.param);
 
-	allegro_mbox_write(dev, &dev->mbox_command, &msg, sizeof(msg));
-	allegro_mcu_interrupt(dev);
+	allegro_mbox_send(dev->mbox_command, &msg);
 
 	return 0;
 }
@@ -964,8 +1013,7 @@ static int allegro_mcu_send_destroy_channel(struct allegro_dev *dev,
 
 	msg.channel_id = channel->mcu_channel_id;
 
-	allegro_mbox_write(dev, &dev->mbox_command, &msg, sizeof(msg));
-	allegro_mcu_interrupt(dev);
+	allegro_mbox_send(dev->mbox_command, &msg);
 
 	return 0;
 }
@@ -991,8 +1039,7 @@ static int allegro_mcu_send_put_stream_buffer(struct allegro_dev *dev,
 	/* copied to mcu_msg_encode_frame_response */
 	msg.stream_id = stream_id;
 
-	allegro_mbox_write(dev, &dev->mbox_command, &msg, sizeof(msg));
-	allegro_mcu_interrupt(dev);
+	allegro_mbox_send(dev->mbox_command, &msg);
 
 	return 0;
 }
@@ -1021,8 +1068,7 @@ static int allegro_mcu_send_encode_frame(struct allegro_dev *dev,
 	msg.ep2 = 0x0;
 	msg.ep2_v = to_mcu_addr(dev, msg.ep2);
 
-	allegro_mbox_write(dev, &dev->mbox_command, &msg, sizeof(msg));
-	allegro_mcu_interrupt(dev);
+	allegro_mbox_send(dev->mbox_command, &msg);
 
 	return 0;
 }
@@ -1084,12 +1130,8 @@ static int allegro_mcu_push_buffer_internal(struct allegro_channel *channel,
 		buffer++;
 	}
 
-	err = allegro_mbox_write(dev, &dev->mbox_command, msg, size);
-	if (err)
-		goto out;
-	allegro_mcu_interrupt(dev);
+	err = allegro_mbox_send(dev->mbox_command, msg);
 
-out:
 	kfree(msg);
 	return err;
 }
@@ -1681,51 +1723,28 @@ allegro_handle_encode_frame(struct allegro_dev *dev,
 	return 0;
 }
 
-static int allegro_receive_message(struct allegro_dev *dev)
+static void allegro_handle_message(struct allegro_dev *dev,
+				   union mcu_msg_response *msg)
 {
-	union mcu_msg_response *msg;
-	ssize_t size;
-	int err = 0;
-
-	msg = kmalloc(sizeof(*msg), GFP_KERNEL);
-	if (!msg)
-		return -ENOMEM;
-
-	size = allegro_mbox_read(dev, &dev->mbox_status, msg, sizeof(*msg));
-	if (size < sizeof(msg->header)) {
-		v4l2_err(&dev->v4l2_dev,
-			 "invalid mbox message (%zd): must be at least %zu\n",
-			 size, sizeof(msg->header));
-		err = -EINVAL;
-		goto out;
-	}
-
 	switch (msg->header.type) {
 	case MCU_MSG_TYPE_INIT:
-		err = allegro_handle_init(dev, &msg->init);
+		allegro_handle_init(dev, &msg->init);
 		break;
 	case MCU_MSG_TYPE_CREATE_CHANNEL:
-		err = allegro_handle_create_channel(dev, &msg->create_channel);
+		allegro_handle_create_channel(dev, &msg->create_channel);
 		break;
 	case MCU_MSG_TYPE_DESTROY_CHANNEL:
-		err = allegro_handle_destroy_channel(dev,
-						     &msg->destroy_channel);
+		allegro_handle_destroy_channel(dev, &msg->destroy_channel);
 		break;
 	case MCU_MSG_TYPE_ENCODE_FRAME:
-		err = allegro_handle_encode_frame(dev, &msg->encode_frame);
+		allegro_handle_encode_frame(dev, &msg->encode_frame);
 		break;
 	default:
 		v4l2_warn(&dev->v4l2_dev,
 			  "%s: unknown message %s\n",
 			  __func__, msg_type_name(msg->header.type));
-		err = -EINVAL;
 		break;
 	}
-
-out:
-	kfree(msg);
-
-	return err;
 }
 
 static irqreturn_t allegro_hardirq(int irq, void *data)
@@ -1746,7 +1765,7 @@ static irqreturn_t allegro_irq_thread(int irq, void *data)
 {
 	struct allegro_dev *dev = data;
 
-	allegro_receive_message(dev);
+	allegro_mbox_notify(dev->mbox_status);
 
 	return IRQ_HANDLED;
 }
@@ -1895,6 +1914,11 @@ static int allegro_mcu_reset(struct allegro_dev *dev)
 	return allegro_mcu_wait_for_sleep(dev);
 }
 
+static void allegro_mcu_interrupt(struct allegro_dev *dev)
+{
+	regmap_write(dev->regmap, AL5_MCU_INTERRUPT, BIT(0));
+}
+
 static void allegro_destroy_channel(struct allegro_channel *channel)
 {
 	struct allegro_dev *dev = channel->dev;
@@ -2887,10 +2911,15 @@ static int allegro_mcu_hw_init(struct allegro_dev *dev,
 {
 	int err;
 
-	allegro_mbox_init(dev, &dev->mbox_command,
-			  info->mailbox_cmd, info->mailbox_size);
-	allegro_mbox_init(dev, &dev->mbox_status,
-			  info->mailbox_status, info->mailbox_size);
+	dev->mbox_command = allegro_mbox_init(dev, info->mailbox_cmd,
+					      info->mailbox_size);
+	dev->mbox_status = allegro_mbox_init(dev, info->mailbox_status,
+					     info->mailbox_size);
+	if (!dev->mbox_command || !dev->mbox_status) {
+		v4l2_err(&dev->v4l2_dev,
+			 "failed to initialize mailboxes\n");
+		return -EIO;
+	}
 
 	allegro_mcu_enable_interrupts(dev);
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 02/12] media: allegro: rework read/write to mailbox
  2020-07-13 14:42 [PATCH v2 00/12] media: allegro: Add support for firmware 2019.2 Michael Tretter
  2020-07-13 14:42 ` [PATCH v2 01/12] media: allegro: rework mbox handling Michael Tretter
@ 2020-07-13 14:42 ` Michael Tretter
  2020-07-13 14:42 ` [PATCH v2 03/12] media: allegro: add explicit mail encoding and decoding Michael Tretter
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Michael Tretter @ 2020-07-13 14:42 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab, kernel, Michael Tretter

Rework the functions that read and write the SRAM that is used to
communicate with the MCU.

As the functions will not operate on structs but on prepared binary
buffers, make the buffer stride more explicit. Also, avoid any uses of
struct mcu_msg_header to analyze messages in memory, because the header
will be made independent of the binary representation in the mailbox.
Instead explicitly access the mail size field in the mailbox.

As at it, further reduce the dependency between the mailboxes and struct
allegro_dev.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 .../staging/media/allegro-dvt/allegro-core.c  | 110 ++++++------------
 1 file changed, 36 insertions(+), 74 deletions(-)

diff --git a/drivers/staging/media/allegro-dvt/allegro-core.c b/drivers/staging/media/allegro-dvt/allegro-core.c
index 11333f900302..dc2627df0eec 100644
--- a/drivers/staging/media/allegro-dvt/allegro-core.c
+++ b/drivers/staging/media/allegro-dvt/allegro-core.c
@@ -612,49 +612,34 @@ static struct allegro_mbox *allegro_mbox_init(struct allegro_dev *dev,
 	return mbox;
 }
 
-static int allegro_mbox_write(struct allegro_dev *dev,
-			      struct allegro_mbox *mbox, void *src, size_t size)
+static int allegro_mbox_write(struct allegro_mbox *mbox,
+			      const u32 *src, size_t size)
 {
-	struct mcu_msg_header *header = src;
+	struct regmap *sram = mbox->dev->sram;
 	unsigned int tail;
 	size_t size_no_wrap;
 	int err = 0;
+	int stride = regmap_get_reg_stride(sram);
 
 	if (!src)
 		return -EINVAL;
 
-	if (size > mbox->size) {
-		v4l2_err(&dev->v4l2_dev,
-			 "message (%zu bytes) too large for mailbox (%zu bytes)\n",
-			 size, mbox->size);
-		return -EINVAL;
-	}
-
-	if (header->length != size - sizeof(*header)) {
-		v4l2_err(&dev->v4l2_dev,
-			 "invalid message length: %u bytes (expected %zu bytes)\n",
-			 header->length, size - sizeof(*header));
+	if (size > mbox->size)
 		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);
+	regmap_read(sram, mbox->tail, &tail);
 	if (tail > mbox->size) {
-		v4l2_err(&dev->v4l2_dev,
-			 "invalid tail (0x%x): must be smaller than mailbox size (0x%zx)\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);
+	regmap_bulk_write(sram, mbox->data + tail,
+			  src, size_no_wrap / stride);
+	regmap_bulk_write(sram, mbox->data,
+			  src + (size_no_wrap / sizeof(*src)),
+			  (size - size_no_wrap) / stride);
+	regmap_write(sram, mbox->tail, (tail + size) % mbox->size);
 
 out:
 	mutex_unlock(&mbox->lock);
@@ -662,40 +647,32 @@ static int allegro_mbox_write(struct allegro_dev *dev,
 	return err;
 }
 
-static ssize_t allegro_mbox_read(struct allegro_dev *dev,
-				 struct allegro_mbox *mbox,
-				 void *dst, size_t nbyte)
+static ssize_t allegro_mbox_read(struct allegro_mbox *mbox,
+				 u32 *dst, size_t nbyte)
 {
-	struct mcu_msg_header *header;
+	struct {
+		u16 length;
+		u16 type;
+	} __attribute__ ((__packed__)) *header;
+	struct regmap *sram = mbox->dev->sram;
 	unsigned int head;
 	ssize_t size;
 	size_t body_no_wrap;
+	int stride = regmap_get_reg_stride(sram);
 
-	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%zx)\n",
-			 head, mbox->size);
+	regmap_read(sram, mbox->head, &head);
+	if (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;
+	regmap_bulk_read(sram, mbox->data + head,
+			 dst, sizeof(*header) / stride);
+	header = (void *)dst;
 	size = header->length + sizeof(*header);
-	if (size > mbox->size || size & 0x3) {
-		v4l2_err(&dev->v4l2_dev,
-			 "invalid message length: %zu bytes (maximum %zu bytes)\n",
-			 header->length + sizeof(*header), mbox->size);
+	if (size > mbox->size || size & 0x3)
 		return -EIO;
-	}
-	if (size > nbyte) {
-		v4l2_err(&dev->v4l2_dev,
-			 "destination buffer too small: %zu bytes (need %zu bytes)\n",
-			 nbyte, size);
+	if (size > nbyte)
 		return -EINVAL;
-	}
 
 	/*
 	 * The message might wrap within the mailbox. If the message does not
@@ -708,17 +685,14 @@ static ssize_t allegro_mbox_read(struct allegro_dev *dev,
 	 */
 	body_no_wrap = min((size_t)header->length,
 			   (size_t)(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_bulk_read(sram, mbox->data + head + sizeof(*header),
+			 dst + (sizeof(*header) / sizeof(*dst)),
+			 body_no_wrap / stride);
+	regmap_bulk_read(sram, mbox->data,
+			 dst + (sizeof(*header) + body_no_wrap) / sizeof(*dst),
+			 (header->length - body_no_wrap) / stride);
 
-	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);
+	regmap_write(sram, mbox->head, (head + size) % mbox->size);
 
 	return size;
 }
@@ -735,7 +709,7 @@ static int allegro_mbox_send(struct allegro_mbox *mbox, void *msg)
 	ssize_t size = sizeof(*header) + header->length;
 	int err;
 
-	err = allegro_mbox_write(dev, mbox, msg, size);
+	err = allegro_mbox_write(mbox, msg, size);
 	if (err)
 		goto out;
 
@@ -759,7 +733,7 @@ static void allegro_mbox_notify(struct allegro_mbox *mbox)
 	if (!msg)
 		return;
 
-	size = allegro_mbox_read(dev, mbox, msg, sizeof(*msg));
+	size = allegro_mbox_read(mbox, (u32 *)msg, sizeof(*msg));
 	if (size < 0)
 		goto out;
 
@@ -1604,12 +1578,6 @@ allegro_handle_create_channel(struct allegro_dev *dev,
 	struct allegro_channel *channel;
 	int err = 0;
 
-	if (msg->header.length != sizeof(*msg) - sizeof(msg->header))
-		v4l2_warn(&dev->v4l2_dev,
-			  "received message has %d bytes, but expected %zu\n",
-			  msg->header.length,
-			  sizeof(*msg) - sizeof(msg->header));
-
 	channel = allegro_find_channel_by_user_id(dev, msg->user_id);
 	if (IS_ERR(channel)) {
 		v4l2_warn(&dev->v4l2_dev,
@@ -1703,12 +1671,6 @@ allegro_handle_encode_frame(struct allegro_dev *dev,
 {
 	struct allegro_channel *channel;
 
-	if (msg->header.length != sizeof(*msg) - sizeof(msg->header))
-		v4l2_warn(&dev->v4l2_dev,
-			  "received message has %d bytes, but expected %zu\n",
-			  msg->header.length,
-			  sizeof(*msg) - sizeof(msg->header));
-
 	channel = allegro_find_channel_by_channel_id(dev, msg->channel_id);
 	if (IS_ERR(channel)) {
 		v4l2_err(&dev->v4l2_dev,
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 03/12] media: allegro: add explicit mail encoding and decoding
  2020-07-13 14:42 [PATCH v2 00/12] media: allegro: Add support for firmware 2019.2 Michael Tretter
  2020-07-13 14:42 ` [PATCH v2 01/12] media: allegro: rework mbox handling Michael Tretter
  2020-07-13 14:42 ` [PATCH v2 02/12] media: allegro: rework read/write to mailbox Michael Tretter
@ 2020-07-13 14:42 ` Michael Tretter
  2020-07-13 14:42 ` [PATCH v2 04/12] media: allegro: add field for number of buffers Michael Tretter
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Michael Tretter @ 2020-07-13 14:42 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab, kernel, Michael Tretter

The message format in the mailboxes differ between firmware versions.
Therefore, it is necessary to decouple the mailbox format of the driver
from the message format of the firmware. This allows to keep a
consistent message format in the driver while still supporting various
firmware versions.

Add an intermediate step to encode and decode message before writing the
mails to the mailboxes.

On the other hand, this allows to handle optional fields in the
messages, which is required for advanced features of the encoder and was
not possible until now.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 .../staging/media/allegro-dvt/allegro-core.c  |  28 +-
 .../staging/media/allegro-dvt/allegro-mail.c  | 364 ++++++++++++++++++
 .../staging/media/allegro-dvt/allegro-mail.h  |   3 +
 3 files changed, 391 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/allegro-dvt/allegro-core.c b/drivers/staging/media/allegro-dvt/allegro-core.c
index dc2627df0eec..6334b351ee3b 100644
--- a/drivers/staging/media/allegro-dvt/allegro-core.c
+++ b/drivers/staging/media/allegro-dvt/allegro-core.c
@@ -705,11 +705,20 @@ static ssize_t allegro_mbox_read(struct allegro_mbox *mbox,
 static int allegro_mbox_send(struct allegro_mbox *mbox, void *msg)
 {
 	struct allegro_dev *dev = mbox->dev;
-	struct mcu_msg_header *header = msg;
-	ssize_t size = sizeof(*header) + header->length;
+	ssize_t size;
 	int err;
+	u32 *tmp;
+
+	tmp = kzalloc(mbox->size, GFP_KERNEL);
+	if (!tmp) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	size = allegro_encode_mail(tmp, msg);
 
-	err = allegro_mbox_write(mbox, msg, size);
+	err = allegro_mbox_write(mbox, tmp, size);
+	kfree(tmp);
 	if (err)
 		goto out;
 
@@ -728,18 +737,29 @@ static void allegro_mbox_notify(struct allegro_mbox *mbox)
 	struct allegro_dev *dev = mbox->dev;
 	union mcu_msg_response *msg;
 	ssize_t size;
+	u32 *tmp;
+	int err;
 
 	msg = kmalloc(sizeof(*msg), GFP_KERNEL);
 	if (!msg)
 		return;
 
-	size = allegro_mbox_read(mbox, (u32 *)msg, sizeof(*msg));
+	tmp = kmalloc(mbox->size, GFP_KERNEL);
+	if (!tmp)
+		goto out;
+
+	size = allegro_mbox_read(mbox, tmp, mbox->size);
 	if (size < 0)
 		goto out;
 
+	err = allegro_decode_mail(msg, tmp);
+	if (err)
+		goto out;
+
 	allegro_handle_message(dev, msg);
 
 out:
+	kfree(tmp);
 	kfree(msg);
 }
 
diff --git a/drivers/staging/media/allegro-dvt/allegro-mail.c b/drivers/staging/media/allegro-dvt/allegro-mail.c
index df0d8d26a6fb..c78367d36e2e 100644
--- a/drivers/staging/media/allegro-dvt/allegro-mail.c
+++ b/drivers/staging/media/allegro-dvt/allegro-mail.c
@@ -6,7 +6,9 @@
  * Allegro VCU firmware.
  */
 
+#include <linux/bitfield.h>
 #include <linux/export.h>
+#include <linux/errno.h>
 
 #include "allegro-mail.h"
 
@@ -35,3 +37,365 @@ const char *msg_type_name(enum mcu_msg_type type)
 	}
 }
 EXPORT_SYMBOL(msg_type_name);
+
+static ssize_t
+allegro_enc_init(u32 *dst, struct mcu_msg_init_request *msg)
+{
+	unsigned int i = 0;
+
+	dst[i++] = msg->reserved0;
+	dst[i++] = msg->suballoc_dma;
+	dst[i++] = msg->suballoc_size;
+	dst[i++] = msg->l2_cache[0];
+	dst[i++] = msg->l2_cache[1];
+	dst[i++] = msg->l2_cache[2];
+
+	return i * sizeof(*dst);
+}
+
+static ssize_t
+allegro_encode_channel_config(u32 *dst, struct create_channel_param *param)
+{
+	unsigned int i = 0;
+
+	dst[i++] = FIELD_PREP(GENMASK(31, 16), param->height) |
+		   FIELD_PREP(GENMASK(15, 0), param->width);
+	dst[i++] = param->format;
+	dst[i++] = param->colorspace;
+	dst[i++] = param->src_mode;
+	dst[i++] = FIELD_PREP(GENMASK(31, 24), param->codec) |
+		   FIELD_PREP(GENMASK(23, 8), param->constraint_set_flags) |
+		   FIELD_PREP(GENMASK(7, 0), param->profile);
+	dst[i++] = FIELD_PREP(GENMASK(31, 16), param->tier) |
+		   FIELD_PREP(GENMASK(15, 0), param->level);
+	dst[i++] = param->sps_param;
+	dst[i++] = param->pps_param;
+	dst[i++] = param->enc_option;
+	dst[i++] = FIELD_PREP(GENMASK(15, 8), param->beta_offset) |
+		   FIELD_PREP(GENMASK(7, 0), param->tc_offset);
+	dst[i++] = param->unknown11;
+	dst[i++] = param->unknown12;
+	dst[i++] = FIELD_PREP(GENMASK(31, 16), param->prefetch_auto) |
+		   FIELD_PREP(GENMASK(15, 0), param->num_slices);
+	dst[i++] = param->prefetch_mem_offset;
+	dst[i++] = param->prefetch_mem_size;
+	dst[i++] = FIELD_PREP(GENMASK(31, 16), param->clip_vrt_range) |
+		   FIELD_PREP(GENMASK(15, 0), param->clip_hrz_range);
+	dst[i++] = FIELD_PREP(GENMASK(31, 16), param->me_range[1]) |
+		   FIELD_PREP(GENMASK(15, 0), param->me_range[0]);
+	dst[i++] = FIELD_PREP(GENMASK(31, 16), param->me_range[3]) |
+		   FIELD_PREP(GENMASK(15, 0), param->me_range[2]);
+	dst[i++] = FIELD_PREP(GENMASK(31, 24), param->min_tu_size) |
+		   FIELD_PREP(GENMASK(23, 16), param->max_tu_size) |
+		   FIELD_PREP(GENMASK(15, 8), param->min_cu_size) |
+		   FIELD_PREP(GENMASK(8, 0), param->max_cu_size);
+	dst[i++] = FIELD_PREP(GENMASK(15, 8), param->max_transfo_depth_intra) |
+		   FIELD_PREP(GENMASK(7, 0), param->max_transfo_depth_inter);
+	dst[i++] = param->entropy_mode;
+	dst[i++] = param->wp_mode;
+
+	dst[i++] = param->rate_control_mode;
+	dst[i++] = param->initial_rem_delay;
+	dst[i++] = param->cpb_size;
+	dst[i++] = FIELD_PREP(GENMASK(31, 16), param->clk_ratio) |
+		   FIELD_PREP(GENMASK(15, 0), param->framerate);
+	dst[i++] = param->target_bitrate;
+	dst[i++] = param->max_bitrate;
+	dst[i++] = FIELD_PREP(GENMASK(31, 16), param->min_qp) |
+		   FIELD_PREP(GENMASK(15, 0), param->initial_qp);
+	dst[i++] = FIELD_PREP(GENMASK(31, 16), param->ip_delta) |
+		   FIELD_PREP(GENMASK(15, 0), param->max_qp);
+	dst[i++] = FIELD_PREP(GENMASK(31, 16), param->golden_ref) |
+		   FIELD_PREP(GENMASK(15, 0), param->pb_delta);
+	dst[i++] = FIELD_PREP(GENMASK(31, 16), param->golden_ref_frequency) |
+		   FIELD_PREP(GENMASK(15, 0), param->golden_delta);
+	dst[i++] = param->rate_control_option;
+
+	dst[i++] = param->gop_ctrl_mode;
+	dst[i++] = param->freq_idr;
+	dst[i++] = param->freq_lt;
+	dst[i++] = param->gdr_mode;
+	dst[i++] = FIELD_PREP(GENMASK(31, 24), param->freq_golden_ref) |
+		   FIELD_PREP(GENMASK(23, 16), param->num_b) |
+		   FIELD_PREP(GENMASK(15, 0), param->gop_length);
+
+	dst[i++] = param->subframe_latency;
+	dst[i++] = param->lda_control_mode;
+	dst[i++] = param->unknown41;
+
+	return i * sizeof(*dst);
+}
+
+static ssize_t
+allegro_enc_create_channel(u32 *dst, struct mcu_msg_create_channel *msg)
+{
+	struct create_channel_param *param = &msg->param;
+	ssize_t size = 0;
+	unsigned int i = 0;
+
+	dst[i++] = msg->user_id;
+
+	size = allegro_encode_channel_config(&dst[i], param);
+	i += size / sizeof(*dst);
+
+	return i * sizeof(*dst);
+}
+
+static ssize_t
+allegro_enc_destroy_channel(u32 *dst, struct mcu_msg_destroy_channel *msg)
+{
+	unsigned int i = 0;
+
+	dst[i++] = msg->channel_id;
+
+	return i * sizeof(*dst);
+}
+
+static ssize_t
+allegro_enc_push_buffers(u32 *dst, struct mcu_msg_push_buffers_internal *msg)
+{
+	unsigned int i = 0;
+	struct mcu_msg_push_buffers_internal_buffer *buffer;
+	unsigned int num_buffers = (msg->header.length - 4) / sizeof(*buffer);
+	unsigned int j;
+
+	dst[i++] = msg->channel_id;
+
+	for (j = 0; j < num_buffers; j++) {
+		buffer = &msg->buffer[j];
+		dst[i++] = buffer->dma_addr;
+		dst[i++] = buffer->mcu_addr;
+		dst[i++] = buffer->size;
+	}
+
+	return i * sizeof(*dst);
+}
+
+static ssize_t
+allegro_enc_put_stream_buffer(u32 *dst,
+			      struct mcu_msg_put_stream_buffer *msg)
+{
+	unsigned int i = 0;
+
+	dst[i++] = msg->channel_id;
+	dst[i++] = msg->dma_addr;
+	dst[i++] = msg->mcu_addr;
+	dst[i++] = msg->size;
+	dst[i++] = msg->offset;
+	dst[i++] = lower_32_bits(msg->stream_id);
+	dst[i++] = upper_32_bits(msg->stream_id);
+
+	return i * sizeof(*dst);
+}
+
+static ssize_t
+allegro_enc_encode_frame(u32 *dst, struct mcu_msg_encode_frame *msg)
+{
+	unsigned int i = 0;
+
+	dst[i++] = msg->channel_id;
+
+	dst[i++] = msg->reserved;
+	dst[i++] = msg->encoding_options;
+	dst[i++] = FIELD_PREP(GENMASK(31, 16), msg->padding) |
+		   FIELD_PREP(GENMASK(15, 0), msg->pps_qp);
+	dst[i++] = lower_32_bits(msg->user_param);
+	dst[i++] = upper_32_bits(msg->user_param);
+	dst[i++] = lower_32_bits(msg->src_handle);
+	dst[i++] = upper_32_bits(msg->src_handle);
+	dst[i++] = msg->request_options;
+	dst[i++] = msg->src_y;
+	dst[i++] = msg->src_uv;
+	dst[i++] = msg->stride;
+	dst[i++] = msg->ep2;
+	dst[i++] = lower_32_bits(msg->ep2_v);
+	dst[i++] = upper_32_bits(msg->ep2_v);
+
+	return i * sizeof(*dst);
+}
+
+static ssize_t
+allegro_dec_init(struct mcu_msg_init_response *msg, u32 *src)
+{
+	unsigned int i = 0;
+
+	msg->header.type = FIELD_GET(GENMASK(31, 16), src[i]);
+	msg->header.length = FIELD_GET(GENMASK(15, 0), src[i++]);
+	msg->reserved0 = src[i++];
+
+	return i * sizeof(*src);
+}
+
+static ssize_t
+allegro_dec_create_channel(struct mcu_msg_create_channel_response *msg,
+			   u32 *src)
+{
+	unsigned int i = 0;
+
+	msg->header.type = FIELD_GET(GENMASK(31, 16), src[i]);
+	msg->header.length = FIELD_GET(GENMASK(15, 0), src[i++]);
+	msg->channel_id = src[i++];
+	msg->user_id = src[i++];
+	msg->options = src[i++];
+	msg->num_core = src[i++];
+	msg->pps_param = src[i++];
+	msg->int_buffers_count = src[i++];
+	msg->int_buffers_size = src[i++];
+	msg->rec_buffers_count = src[i++];
+	msg->rec_buffers_size = src[i++];
+	msg->reserved = src[i++];
+	msg->error_code = src[i++];
+
+	return i * sizeof(*src);
+}
+
+static ssize_t
+allegro_dec_destroy_channel(struct mcu_msg_destroy_channel_response *msg,
+			    u32 *src)
+{
+	unsigned int i = 0;
+
+	msg->header.type = FIELD_GET(GENMASK(31, 16), src[i]);
+	msg->header.length = FIELD_GET(GENMASK(15, 0), src[i++]);
+	msg->channel_id = src[i++];
+
+	return i * sizeof(*src);
+}
+
+static ssize_t
+allegro_dec_encode_frame(struct mcu_msg_encode_frame_response *msg, u32 *src)
+{
+	unsigned int i = 0;
+	unsigned int j;
+
+	msg->header.type = FIELD_GET(GENMASK(31, 16), src[i]);
+	msg->header.length = FIELD_GET(GENMASK(15, 0), src[i++]);
+	msg->channel_id = src[i++];
+
+	msg->stream_id = src[i++];
+	msg->stream_id |= (((u64)src[i++]) << 32);
+	msg->user_param = src[i++];
+	msg->user_param |= (((u64)src[i++]) << 32);
+	msg->src_handle = src[i++];
+	msg->src_handle |= (((u64)src[i++]) << 32);
+	msg->skip = FIELD_GET(GENMASK(31, 16), src[i]);
+	msg->is_ref = FIELD_GET(GENMASK(15, 0), src[i++]);
+	msg->initial_removal_delay = src[i++];
+	msg->dpb_output_delay = src[i++];
+	msg->size = src[i++];
+	msg->frame_tag_size = src[i++];
+	msg->stuffing = src[i++];
+	msg->filler = src[i++];
+	msg->num_column = FIELD_GET(GENMASK(31, 16), src[i]);
+	msg->num_row = FIELD_GET(GENMASK(15, 0), src[i++]);
+	msg->num_ref_idx_l1 = FIELD_GET(GENMASK(31, 24), src[i]);
+	msg->num_ref_idx_l0 = FIELD_GET(GENMASK(23, 16), src[i]);
+	msg->qp = FIELD_GET(GENMASK(15, 0), src[i++]);
+	msg->partition_table_offset = src[i++];
+	msg->partition_table_size = src[i++];
+	msg->sum_complex = src[i++];
+	for (j = 0; j < 4; j++)
+		msg->tile_width[j] = src[i++];
+	for (j = 0; j < 22; j++)
+		msg->tile_height[j] = src[i++];
+	msg->error_code = src[i++];
+	msg->slice_type = src[i++];
+	msg->pic_struct = src[i++];
+	msg->reserved = FIELD_GET(GENMASK(31, 24), src[i]);
+	msg->is_last_slice = FIELD_GET(GENMASK(23, 16), src[i]);
+	msg->is_first_slice = FIELD_GET(GENMASK(15, 8), src[i]);
+	msg->is_idr = FIELD_GET(GENMASK(7, 0), src[i++]);
+
+	msg->reserved1 = FIELD_GET(GENMASK(31, 16), src[i]);
+	msg->pps_qp = FIELD_GET(GENMASK(15, 0), src[i++]);
+
+	msg->reserved2 = src[i++];
+
+	return i * sizeof(*src);
+}
+
+/**
+ * allegro_encode_mail() - Encode allegro messages to firmware format
+ * @dst: Pointer to the memory that will be filled with data
+ * @msg: The allegro message that will be encoded
+ */
+ssize_t allegro_encode_mail(u32 *dst, void *msg)
+{
+	const struct mcu_msg_header *header = msg;
+	enum mcu_msg_type type = header->type;
+	ssize_t size;
+
+	if (!msg || !dst)
+		return -EINVAL;
+
+	switch (type) {
+	case MCU_MSG_TYPE_INIT:
+		size = allegro_enc_init(&dst[1], msg);
+		break;
+	case MCU_MSG_TYPE_CREATE_CHANNEL:
+		size = allegro_enc_create_channel(&dst[1], msg);
+		break;
+	case MCU_MSG_TYPE_DESTROY_CHANNEL:
+		size = allegro_enc_destroy_channel(&dst[1], msg);
+		break;
+	case MCU_MSG_TYPE_ENCODE_FRAME:
+		size = allegro_enc_encode_frame(&dst[1], msg);
+		break;
+	case MCU_MSG_TYPE_PUT_STREAM_BUFFER:
+		size = allegro_enc_put_stream_buffer(&dst[1], msg);
+		break;
+	case MCU_MSG_TYPE_PUSH_BUFFER_INTERMEDIATE:
+	case MCU_MSG_TYPE_PUSH_BUFFER_REFERENCE:
+		size = allegro_enc_push_buffers(&dst[1], msg);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/*
+	 * The encoded messages might have different length depending on
+	 * the firmware version or certain fields. Therefore, we have to
+	 * set the body length after encoding the message.
+	 */
+	dst[0] = FIELD_PREP(GENMASK(31, 16), header->type) |
+		 FIELD_PREP(GENMASK(15, 0), size);
+
+	return size + sizeof(*dst);
+}
+
+/**
+ * allegro_decode_mail() - Parse allegro messages from the firmware.
+ * @msg: The mcu_msg_response that will be filled with parsed values.
+ * @src: Pointer to the memory that will be parsed
+ *
+ * The message format in the mailbox depends on the firmware. Parse the
+ * different formats into a uniform message format that can be used without
+ * taking care of the firmware version.
+ */
+int allegro_decode_mail(void *msg, u32 *src)
+{
+	struct mcu_msg_header *header;
+
+	if (!src || !msg)
+		return -EINVAL;
+
+	header = (struct mcu_msg_header *)src;
+	switch (header->type) {
+	case MCU_MSG_TYPE_INIT:
+		allegro_dec_init(msg, src);
+		break;
+	case MCU_MSG_TYPE_CREATE_CHANNEL:
+		allegro_dec_create_channel(msg, src);
+		break;
+	case MCU_MSG_TYPE_DESTROY_CHANNEL:
+		allegro_dec_destroy_channel(msg, src);
+		break;
+	case MCU_MSG_TYPE_ENCODE_FRAME:
+		allegro_dec_encode_frame(msg, src);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
diff --git a/drivers/staging/media/allegro-dvt/allegro-mail.h b/drivers/staging/media/allegro-dvt/allegro-mail.h
index 17db665f8e1e..457caf50ebe6 100644
--- a/drivers/staging/media/allegro-dvt/allegro-mail.h
+++ b/drivers/staging/media/allegro-dvt/allegro-mail.h
@@ -264,4 +264,7 @@ union mcu_msg_response {
 	struct mcu_msg_encode_frame_response encode_frame;
 };
 
+int allegro_decode_mail(void *msg, u32 *src);
+ssize_t allegro_encode_mail(u32 *dst, void *msg);
+
 #endif
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 04/12] media: allegro: add field for number of buffers
  2020-07-13 14:42 [PATCH v2 00/12] media: allegro: Add support for firmware 2019.2 Michael Tretter
                   ` (2 preceding siblings ...)
  2020-07-13 14:42 ` [PATCH v2 03/12] media: allegro: add explicit mail encoding and decoding Michael Tretter
@ 2020-07-13 14:42 ` Michael Tretter
  2020-07-13 14:42 ` [PATCH v2 05/12] media: allegro: don't pack MCU messages Michael Tretter
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Michael Tretter @ 2020-07-13 14:42 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab, kernel, Michael Tretter

When pushing the buffers for the intermediate and reference frames to
the MCU, the driver relied on the message size to calculate the number
of buffers. As it is not necessary anymore to keep the messages binary
compatible to the firmware, we can just explicitly write the number of
buffers into the message.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/staging/media/allegro-dvt/allegro-core.c | 1 +
 drivers/staging/media/allegro-dvt/allegro-mail.c | 2 +-
 drivers/staging/media/allegro-dvt/allegro-mail.h | 1 +
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/allegro-dvt/allegro-core.c b/drivers/staging/media/allegro-dvt/allegro-core.c
index 6334b351ee3b..6745a5fa1167 100644
--- a/drivers/staging/media/allegro-dvt/allegro-core.c
+++ b/drivers/staging/media/allegro-dvt/allegro-core.c
@@ -1115,6 +1115,7 @@ static int allegro_mcu_push_buffer_internal(struct allegro_channel *channel,
 	msg->header.length = size - sizeof(msg->header);
 	msg->header.type = type;
 	msg->channel_id = channel->mcu_channel_id;
+	msg->num_buffers = num_buffers;
 
 	buffer = msg->buffer;
 	list_for_each_entry(al_buffer, list, head) {
diff --git a/drivers/staging/media/allegro-dvt/allegro-mail.c b/drivers/staging/media/allegro-dvt/allegro-mail.c
index c78367d36e2e..ba1c3bc587c6 100644
--- a/drivers/staging/media/allegro-dvt/allegro-mail.c
+++ b/drivers/staging/media/allegro-dvt/allegro-mail.c
@@ -156,7 +156,7 @@ allegro_enc_push_buffers(u32 *dst, struct mcu_msg_push_buffers_internal *msg)
 {
 	unsigned int i = 0;
 	struct mcu_msg_push_buffers_internal_buffer *buffer;
-	unsigned int num_buffers = (msg->header.length - 4) / sizeof(*buffer);
+	unsigned int num_buffers = msg->num_buffers;
 	unsigned int j;
 
 	dst[i++] = msg->channel_id;
diff --git a/drivers/staging/media/allegro-dvt/allegro-mail.h b/drivers/staging/media/allegro-dvt/allegro-mail.h
index 457caf50ebe6..14b54eb52488 100644
--- a/drivers/staging/media/allegro-dvt/allegro-mail.h
+++ b/drivers/staging/media/allegro-dvt/allegro-mail.h
@@ -169,6 +169,7 @@ struct mcu_msg_push_buffers_internal_buffer {
 struct mcu_msg_push_buffers_internal {
 	struct mcu_msg_header header;
 	u32 channel_id;
+	size_t num_buffers;
 	struct mcu_msg_push_buffers_internal_buffer buffer[];
 } __attribute__ ((__packed__));
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 05/12] media: allegro: don't pack MCU messages
  2020-07-13 14:42 [PATCH v2 00/12] media: allegro: Add support for firmware 2019.2 Michael Tretter
                   ` (3 preceding siblings ...)
  2020-07-13 14:42 ` [PATCH v2 04/12] media: allegro: add field for number of buffers Michael Tretter
@ 2020-07-13 14:42 ` Michael Tretter
  2020-07-13 14:42 ` [PATCH v2 06/12] media: allegro: support handling firmware dependent values Michael Tretter
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Michael Tretter @ 2020-07-13 14:42 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab, kernel, Michael Tretter

The messages are now explicitly converted from the struct to the binary
representation used by the firmware. Therefore, there is no need to keep
the structs packed anymore.

Drop the attribute and avoid confusion why the mails are packed.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 .../staging/media/allegro-dvt/allegro-mail.h  | 26 +++++++++----------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/media/allegro-dvt/allegro-mail.h b/drivers/staging/media/allegro-dvt/allegro-mail.h
index 14b54eb52488..ae81e94a2f2c 100644
--- a/drivers/staging/media/allegro-dvt/allegro-mail.h
+++ b/drivers/staging/media/allegro-dvt/allegro-mail.h
@@ -25,7 +25,7 @@ const char *msg_type_name(enum mcu_msg_type type);
 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;
@@ -33,12 +33,12 @@ struct mcu_msg_init_request {
 	u32 suballoc_dma;
 	u32 suballoc_size;
 	s32 l2_cache[3];
-} __attribute__ ((__packed__));
+};
 
 struct mcu_msg_init_response {
 	struct mcu_msg_header header;
 	u32 reserved0;
-} __attribute__ ((__packed__));
+};
 
 struct create_channel_param {
 	u16 width;
@@ -127,13 +127,13 @@ struct create_channel_param {
 	u32 subframe_latency;
 	u32 lda_control_mode;
 	u32 unknown41;
-} __attribute__ ((__packed__));
+};
 
 struct mcu_msg_create_channel {
 	struct mcu_msg_header header;
 	u32 user_id;
 	struct create_channel_param param;
-} __attribute__ ((__packed__));
+};
 
 struct mcu_msg_create_channel_response {
 	struct mcu_msg_header header;
@@ -148,30 +148,30 @@ struct mcu_msg_create_channel_response {
 	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;
 	size_t num_buffers;
 	struct mcu_msg_push_buffers_internal_buffer buffer[];
-} __attribute__ ((__packed__));
+};
 
 struct mcu_msg_put_stream_buffer {
 	struct mcu_msg_header header;
@@ -181,7 +181,7 @@ struct mcu_msg_put_stream_buffer {
 	u32 size;
 	u32 offset;
 	u64 stream_id;
-} __attribute__ ((__packed__));
+};
 
 struct mcu_msg_encode_frame {
 	struct mcu_msg_header header;
@@ -214,7 +214,7 @@ struct mcu_msg_encode_frame {
 	u32 stride;
 	u32 ep2;
 	u64 ep2_v;
-} __attribute__ ((__packed__));
+};
 
 struct mcu_msg_encode_frame_response {
 	struct mcu_msg_header header;
@@ -255,7 +255,7 @@ struct mcu_msg_encode_frame_response {
 	u16 pps_qp;
 	u16 reserved1;
 	u32 reserved2;
-} __attribute__ ((__packed__));
+};
 
 union mcu_msg_response {
 	struct mcu_msg_header header;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 06/12] media: allegro: support handling firmware dependent values
  2020-07-13 14:42 [PATCH v2 00/12] media: allegro: Add support for firmware 2019.2 Michael Tretter
                   ` (4 preceding siblings ...)
  2020-07-13 14:42 ` [PATCH v2 05/12] media: allegro: don't pack MCU messages Michael Tretter
@ 2020-07-13 14:42 ` Michael Tretter
  2020-07-13 14:42 ` [PATCH v2 07/12] media: allegro: encode bit fields separately Michael Tretter
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Michael Tretter @ 2020-07-13 14:42 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab, kernel, Michael Tretter

Like the message format, also the identifiers in the messages differ
between firmware versions. This especially affects the identifier for
the codec that shall be used. As the messages used by the driver are now
independent from the firmware, we can use the values defined by V4L2 as
identifiers in the messages.

Convert the V4L2 codec format to the respective firmware value when
encoding the messages to binary format instead beforehand.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/staging/media/allegro-dvt/allegro-core.c | 11 +----------
 drivers/staging/media/allegro-dvt/allegro-mail.c | 15 ++++++++++++++-
 drivers/staging/media/allegro-dvt/allegro-mail.h |  2 +-
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/media/allegro-dvt/allegro-core.c b/drivers/staging/media/allegro-dvt/allegro-core.c
index 6745a5fa1167..1bbb44140a6c 100644
--- a/drivers/staging/media/allegro-dvt/allegro-core.c
+++ b/drivers/staging/media/allegro-dvt/allegro-core.c
@@ -812,15 +812,6 @@ static u32 v4l2_colorspace_to_mcu_colorspace(enum v4l2_colorspace colorspace)
 	}
 }
 
-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) {
@@ -919,7 +910,7 @@ static int fill_create_channel_param(struct allegro_channel *channel,
 	param->src_mode = 0x0;
 	param->profile = v4l2_profile_to_mcu_profile(channel->profile);
 	param->constraint_set_flags = BIT(1);
-	param->codec = v4l2_pixelformat_to_mcu_codec(channel->codec);
+	param->codec = channel->codec;
 	param->level = v4l2_level_to_mcu_level(channel->level);
 	param->tier = 0;
 	param->sps_param = BIT(20) | 0x4a;
diff --git a/drivers/staging/media/allegro-dvt/allegro-mail.c b/drivers/staging/media/allegro-dvt/allegro-mail.c
index ba1c3bc587c6..ce183bd2495b 100644
--- a/drivers/staging/media/allegro-dvt/allegro-mail.c
+++ b/drivers/staging/media/allegro-dvt/allegro-mail.c
@@ -9,6 +9,7 @@
 #include <linux/bitfield.h>
 #include <linux/export.h>
 #include <linux/errno.h>
+#include <linux/videodev2.h>
 
 #include "allegro-mail.h"
 
@@ -53,17 +54,29 @@ allegro_enc_init(u32 *dst, struct mcu_msg_init_request *msg)
 	return i * sizeof(*dst);
 }
 
+static inline u32 settings_get_mcu_codec(struct create_channel_param *param)
+{
+	u32 pixelformat = param->codec;
+
+	switch (pixelformat) {
+	case V4L2_PIX_FMT_H264:
+	default:
+		return 1;
+	}
+}
+
 static ssize_t
 allegro_encode_channel_config(u32 *dst, struct create_channel_param *param)
 {
 	unsigned int i = 0;
+	unsigned int codec = settings_get_mcu_codec(param);
 
 	dst[i++] = FIELD_PREP(GENMASK(31, 16), param->height) |
 		   FIELD_PREP(GENMASK(15, 0), param->width);
 	dst[i++] = param->format;
 	dst[i++] = param->colorspace;
 	dst[i++] = param->src_mode;
-	dst[i++] = FIELD_PREP(GENMASK(31, 24), param->codec) |
+	dst[i++] = FIELD_PREP(GENMASK(31, 24), codec) |
 		   FIELD_PREP(GENMASK(23, 8), param->constraint_set_flags) |
 		   FIELD_PREP(GENMASK(7, 0), param->profile);
 	dst[i++] = FIELD_PREP(GENMASK(31, 16), param->tier) |
diff --git a/drivers/staging/media/allegro-dvt/allegro-mail.h b/drivers/staging/media/allegro-dvt/allegro-mail.h
index ae81e94a2f2c..a92a27e270b5 100644
--- a/drivers/staging/media/allegro-dvt/allegro-mail.h
+++ b/drivers/staging/media/allegro-dvt/allegro-mail.h
@@ -48,7 +48,7 @@ struct create_channel_param {
 	u32 src_mode;
 	u8 profile;
 	u16 constraint_set_flags;
-	s8 codec;
+	u32 codec;
 	u16 level;
 	u16 tier;
 	u32 sps_param;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 07/12] media: allegro: encode bit fields separately
  2020-07-13 14:42 [PATCH v2 00/12] media: allegro: Add support for firmware 2019.2 Michael Tretter
                   ` (5 preceding siblings ...)
  2020-07-13 14:42 ` [PATCH v2 06/12] media: allegro: support handling firmware dependent values Michael Tretter
@ 2020-07-13 14:42 ` Michael Tretter
  2020-07-13 14:42 ` [PATCH v2 08/12] media: allegro: add config blob for channel Michael Tretter
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Michael Tretter @ 2020-07-13 14:42 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab, kernel, Michael Tretter

Even bits in bitfields do not keep their position, but move around or
move to other bitfields. Therefore, the driver has to handle bitfields
differently depending on the firmware version.

Create separate fields for the options that have been in bitfields and
handle the bitfields when encoding the message.

As a side effect, this makes the messages a bit more readable.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 .../staging/media/allegro-dvt/allegro-core.c  | 15 ++++++---
 .../staging/media/allegro-dvt/allegro-mail.c  | 22 +++++++++++--
 .../staging/media/allegro-dvt/allegro-mail.h  | 32 +++++--------------
 3 files changed, 38 insertions(+), 31 deletions(-)

diff --git a/drivers/staging/media/allegro-dvt/allegro-core.c b/drivers/staging/media/allegro-dvt/allegro-core.c
index 1bbb44140a6c..e6d64151e4fe 100644
--- a/drivers/staging/media/allegro-dvt/allegro-core.c
+++ b/drivers/staging/media/allegro-dvt/allegro-core.c
@@ -913,10 +913,17 @@ static int fill_create_channel_param(struct allegro_channel *channel,
 	param->codec = channel->codec;
 	param->level = v4l2_level_to_mcu_level(channel->level);
 	param->tier = 0;
-	param->sps_param = BIT(20) | 0x4a;
-	param->pps_param = BIT(2);
-	param->enc_option = AL_OPT_RDO_COST_MODE | AL_OPT_LF_X_TILE |
-			    AL_OPT_LF_X_SLICE | AL_OPT_LF;
+
+	param->log2_max_poc = 10;
+	param->log2_max_frame_num = 4;
+	param->temporal_mvp_enable = 1;
+
+	param->dbf_ovr_en = 1;
+	param->rdo_cost_mode = 1;
+	param->lf = 1;
+	param->lf_x_tile = 1;
+	param->lf_x_slice = 1;
+
 	param->beta_offset = -1;
 	param->tc_offset = -1;
 	param->num_slices = 1;
diff --git a/drivers/staging/media/allegro-dvt/allegro-mail.c b/drivers/staging/media/allegro-dvt/allegro-mail.c
index ce183bd2495b..7121f128aff3 100644
--- a/drivers/staging/media/allegro-dvt/allegro-mail.c
+++ b/drivers/staging/media/allegro-dvt/allegro-mail.c
@@ -69,6 +69,7 @@ static ssize_t
 allegro_encode_channel_config(u32 *dst, struct create_channel_param *param)
 {
 	unsigned int i = 0;
+	u32 val;
 	unsigned int codec = settings_get_mcu_codec(param);
 
 	dst[i++] = FIELD_PREP(GENMASK(31, 16), param->height) |
@@ -81,9 +82,24 @@ allegro_encode_channel_config(u32 *dst, struct create_channel_param *param)
 		   FIELD_PREP(GENMASK(7, 0), param->profile);
 	dst[i++] = FIELD_PREP(GENMASK(31, 16), param->tier) |
 		   FIELD_PREP(GENMASK(15, 0), param->level);
-	dst[i++] = param->sps_param;
-	dst[i++] = param->pps_param;
-	dst[i++] = param->enc_option;
+
+	val = 0;
+	val |= param->temporal_mvp_enable ? BIT(20) : 0;
+	val |= FIELD_PREP(GENMASK(7, 4), param->log2_max_frame_num) |
+	       FIELD_PREP(GENMASK(3, 0), param->log2_max_poc);
+	dst[i++] = val;
+
+	val = 0;
+	val |= param->dbf_ovr_en ? BIT(2) : 0;
+	dst[i++] = val;
+
+	val = 0;
+	val |= param->lf ? BIT(2) : 0;
+	val |= param->lf_x_tile ? BIT(3) : 0;
+	val |= param->lf_x_slice ? BIT(4) : 0;
+	val |= param->rdo_cost_mode ? BIT(20) : 0;
+	dst[i++] = val;
+
 	dst[i++] = FIELD_PREP(GENMASK(15, 8), param->beta_offset) |
 		   FIELD_PREP(GENMASK(7, 0), param->tc_offset);
 	dst[i++] = param->unknown11;
diff --git a/drivers/staging/media/allegro-dvt/allegro-mail.h b/drivers/staging/media/allegro-dvt/allegro-mail.h
index a92a27e270b5..239fd8a20a69 100644
--- a/drivers/staging/media/allegro-dvt/allegro-mail.h
+++ b/drivers/staging/media/allegro-dvt/allegro-mail.h
@@ -51,30 +51,14 @@ struct create_channel_param {
 	u32 codec;
 	u16 level;
 	u16 tier;
-	u32 sps_param;
-	u32 pps_param;
-
-	u32 enc_option;
-#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)
-
+	u32 log2_max_poc;
+	u32 log2_max_frame_num;
+	u32 temporal_mvp_enable;
+	u32 dbf_ovr_en;
+	u32 rdo_cost_mode;
+	u32 lf;
+	u32 lf_x_tile;
+	u32 lf_x_slice;
 	s8 beta_offset;
 	s8 tc_offset;
 	u16 reserved10;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 08/12] media: allegro: add config blob for channel
  2020-07-13 14:42 [PATCH v2 00/12] media: allegro: Add support for firmware 2019.2 Michael Tretter
                   ` (6 preceding siblings ...)
  2020-07-13 14:42 ` [PATCH v2 07/12] media: allegro: encode bit fields separately Michael Tretter
@ 2020-07-13 14:42 ` Michael Tretter
  2020-07-13 14:42 ` [PATCH v2 09/12] media: allegro: set num_ref_idx using response of configured channels Michael Tretter
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Michael Tretter @ 2020-07-13 14:42 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab, kernel, Michael Tretter

Firmware versions >= 2019.2 do not configure the channel via the mailbox
interface anymore, but use a separate chunk of memory that is only
referenced by the message. As the configuration must be in a format that
is understood by the firmware and this format can change between
firmware versions, handle it similar to the message and treat is as a
blob.

In order to support both methods in the driver, always use a separate
blob for the channel configuration and copy that blob into the mailbox
if the firmware requires it and otherwise reference it.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>

---
Changelog:

v1 -> v2:
- add missing string.h
---
 .../staging/media/allegro-dvt/allegro-core.c  | 20 ++++++++++++++++++-
 .../staging/media/allegro-dvt/allegro-mail.c  | 18 +++++++++++------
 .../staging/media/allegro-dvt/allegro-mail.h  |  9 ++++++++-
 3 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/media/allegro-dvt/allegro-core.c b/drivers/staging/media/allegro-dvt/allegro-core.c
index e6d64151e4fe..51e85cad9941 100644
--- a/drivers/staging/media/allegro-dvt/allegro-core.c
+++ b/drivers/staging/media/allegro-dvt/allegro-core.c
@@ -206,6 +206,8 @@ struct allegro_channel {
 	unsigned int cpb_size;
 	unsigned int gop_size;
 
+	struct allegro_buffer config_blob;
+
 	struct v4l2_ctrl *mpeg_video_h264_profile;
 	struct v4l2_ctrl *mpeg_video_h264_level;
 	struct v4l2_ctrl *mpeg_video_h264_i_frame_qp;
@@ -978,6 +980,14 @@ static int allegro_mcu_send_create_channel(struct allegro_dev *dev,
 					   struct allegro_channel *channel)
 {
 	struct mcu_msg_create_channel msg;
+	struct allegro_buffer *blob = &channel->config_blob;
+	struct create_channel_param param;
+	size_t size;
+
+	memset(&param, 0, sizeof(param));
+	fill_create_channel_param(channel, &param);
+	allegro_alloc_buffer(dev, blob, sizeof(struct create_channel_param));
+	size = allegro_encode_config_blob(blob->vaddr, &param);
 
 	memset(&msg, 0, sizeof(msg));
 
@@ -986,7 +996,9 @@ static int allegro_mcu_send_create_channel(struct allegro_dev *dev,
 
 	msg.user_id = channel->user_id;
 
-	fill_create_channel_param(channel, &msg.param);
+	msg.blob = blob->vaddr;
+	msg.blob_size = size;
+	msg.blob_mcu_addr = to_mcu_addr(dev, blob->paddr);
 
 	allegro_mbox_send(dev->mbox_command, &msg);
 
@@ -1596,6 +1608,7 @@ allegro_handle_create_channel(struct allegro_dev *dev,
 {
 	struct allegro_channel *channel;
 	int err = 0;
+	struct create_channel_param param;
 
 	channel = allegro_find_channel_by_user_id(dev, msg->user_id);
 	if (IS_ERR(channel)) {
@@ -1621,6 +1634,11 @@ allegro_handle_create_channel(struct allegro_dev *dev,
 		 "user %d: channel has channel id %d\n",
 		 channel->user_id, channel->mcu_channel_id);
 
+	err = allegro_decode_config_blob(&param, msg, channel->config_blob.vaddr);
+	allegro_free_buffer(channel->dev, &channel->config_blob);
+	if (err)
+		goto out;
+
 	v4l2_dbg(1, debug, &dev->v4l2_dev,
 		 "channel %d: intermediate buffers: %d x %d bytes\n",
 		 channel->mcu_channel_id,
diff --git a/drivers/staging/media/allegro-dvt/allegro-mail.c b/drivers/staging/media/allegro-dvt/allegro-mail.c
index 7121f128aff3..c55b9339f9c6 100644
--- a/drivers/staging/media/allegro-dvt/allegro-mail.c
+++ b/drivers/staging/media/allegro-dvt/allegro-mail.c
@@ -9,6 +9,7 @@
 #include <linux/bitfield.h>
 #include <linux/export.h>
 #include <linux/errno.h>
+#include <linux/string.h>
 #include <linux/videodev2.h>
 
 #include "allegro-mail.h"
@@ -65,8 +66,8 @@ static inline u32 settings_get_mcu_codec(struct create_channel_param *param)
 	}
 }
 
-static ssize_t
-allegro_encode_channel_config(u32 *dst, struct create_channel_param *param)
+ssize_t
+allegro_encode_config_blob(u32 *dst, struct create_channel_param *param)
 {
 	unsigned int i = 0;
 	u32 val;
@@ -158,18 +159,23 @@ allegro_encode_channel_config(u32 *dst, struct create_channel_param *param)
 static ssize_t
 allegro_enc_create_channel(u32 *dst, struct mcu_msg_create_channel *msg)
 {
-	struct create_channel_param *param = &msg->param;
-	ssize_t size = 0;
 	unsigned int i = 0;
 
 	dst[i++] = msg->user_id;
 
-	size = allegro_encode_channel_config(&dst[i], param);
-	i += size / sizeof(*dst);
+	memcpy(&dst[i], msg->blob, msg->blob_size);
+	i += msg->blob_size / sizeof(*dst);
 
 	return i * sizeof(*dst);
 }
 
+ssize_t allegro_decode_config_blob(struct create_channel_param *param,
+				   struct mcu_msg_create_channel_response *msg,
+				   u32 *src)
+{
+	return 0;
+}
+
 static ssize_t
 allegro_enc_destroy_channel(u32 *dst, struct mcu_msg_destroy_channel *msg)
 {
diff --git a/drivers/staging/media/allegro-dvt/allegro-mail.h b/drivers/staging/media/allegro-dvt/allegro-mail.h
index 239fd8a20a69..07ed0a8d3de3 100644
--- a/drivers/staging/media/allegro-dvt/allegro-mail.h
+++ b/drivers/staging/media/allegro-dvt/allegro-mail.h
@@ -116,7 +116,9 @@ struct create_channel_param {
 struct mcu_msg_create_channel {
 	struct mcu_msg_header header;
 	u32 user_id;
-	struct create_channel_param param;
+	u32 *blob;
+	size_t blob_size;
+	u32 blob_mcu_addr;
 };
 
 struct mcu_msg_create_channel_response {
@@ -249,6 +251,11 @@ union mcu_msg_response {
 	struct mcu_msg_encode_frame_response encode_frame;
 };
 
+ssize_t allegro_encode_config_blob(u32 *dst, struct create_channel_param *param);
+ssize_t allegro_decode_config_blob(struct create_channel_param *param,
+				   struct mcu_msg_create_channel_response *msg,
+				   u32 *src);
+
 int allegro_decode_mail(void *msg, u32 *src);
 ssize_t allegro_encode_mail(u32 *dst, void *msg);
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 09/12] media: allegro: set num_ref_idx using response of configured channels
  2020-07-13 14:42 [PATCH v2 00/12] media: allegro: Add support for firmware 2019.2 Michael Tretter
                   ` (7 preceding siblings ...)
  2020-07-13 14:42 ` [PATCH v2 08/12] media: allegro: add config blob for channel Michael Tretter
@ 2020-07-13 14:42 ` Michael Tretter
  2020-07-13 14:42 ` [PATCH v2 10/12] media: allegro: drop length field from message header Michael Tretter
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Michael Tretter @ 2020-07-13 14:42 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab, kernel, Michael Tretter

The firmware decides how many reference frames shall be used und returns
this information via the config blob. In order to set the number of
reference frames in the PPS, the driver has to read these values from
the config blob after the channel has been created in the firmware.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/staging/media/allegro-dvt/allegro-core.c | 10 ++++++++--
 drivers/staging/media/allegro-dvt/allegro-mail.c |  6 +++++-
 drivers/staging/media/allegro-dvt/allegro-mail.h |  5 ++++-
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/allegro-dvt/allegro-core.c b/drivers/staging/media/allegro-dvt/allegro-core.c
index 51e85cad9941..07deb5ba13dc 100644
--- a/drivers/staging/media/allegro-dvt/allegro-core.c
+++ b/drivers/staging/media/allegro-dvt/allegro-core.c
@@ -208,6 +208,9 @@ struct allegro_channel {
 
 	struct allegro_buffer config_blob;
 
+	unsigned int num_ref_idx_l0;
+	unsigned int num_ref_idx_l1;
+
 	struct v4l2_ctrl *mpeg_video_h264_profile;
 	struct v4l2_ctrl *mpeg_video_h264_level;
 	struct v4l2_ctrl *mpeg_video_h264_i_frame_qp;
@@ -1336,8 +1339,8 @@ static ssize_t allegro_h264_write_pps(struct allegro_channel *channel,
 	pps->entropy_coding_mode_flag = 0;
 	pps->bottom_field_pic_order_in_frame_present_flag = 0;
 	pps->num_slice_groups_minus1 = 0;
-	pps->num_ref_idx_l0_default_active_minus1 = 2;
-	pps->num_ref_idx_l1_default_active_minus1 = 2;
+	pps->num_ref_idx_l0_default_active_minus1 = channel->num_ref_idx_l0 - 1;
+	pps->num_ref_idx_l1_default_active_minus1 = channel->num_ref_idx_l1 - 1;
 	pps->weighted_pred_flag = 0;
 	pps->weighted_bipred_idc = 0;
 	pps->pic_init_qp_minus26 = 0;
@@ -1639,6 +1642,9 @@ allegro_handle_create_channel(struct allegro_dev *dev,
 	if (err)
 		goto out;
 
+	channel->num_ref_idx_l0 = param.num_ref_idx_l0;
+	channel->num_ref_idx_l1 = param.num_ref_idx_l1;
+
 	v4l2_dbg(1, debug, &dev->v4l2_dev,
 		 "channel %d: intermediate buffers: %d x %d bytes\n",
 		 channel->mcu_channel_id,
diff --git a/drivers/staging/media/allegro-dvt/allegro-mail.c b/drivers/staging/media/allegro-dvt/allegro-mail.c
index c55b9339f9c6..bb15de080431 100644
--- a/drivers/staging/media/allegro-dvt/allegro-mail.c
+++ b/drivers/staging/media/allegro-dvt/allegro-mail.c
@@ -173,6 +173,9 @@ ssize_t allegro_decode_config_blob(struct create_channel_param *param,
 				   struct mcu_msg_create_channel_response *msg,
 				   u32 *src)
 {
+	param->num_ref_idx_l0 = msg->num_ref_idx_l0;
+	param->num_ref_idx_l1 = msg->num_ref_idx_l1;
+
 	return 0;
 }
 
@@ -273,7 +276,8 @@ allegro_dec_create_channel(struct mcu_msg_create_channel_response *msg,
 	msg->user_id = src[i++];
 	msg->options = src[i++];
 	msg->num_core = src[i++];
-	msg->pps_param = src[i++];
+	msg->num_ref_idx_l0 = FIELD_GET(GENMASK(7, 4), src[i]);
+	msg->num_ref_idx_l1 = FIELD_GET(GENMASK(11, 8), src[i++]);
 	msg->int_buffers_count = src[i++];
 	msg->int_buffers_size = src[i++];
 	msg->rec_buffers_count = src[i++];
diff --git a/drivers/staging/media/allegro-dvt/allegro-mail.h b/drivers/staging/media/allegro-dvt/allegro-mail.h
index 07ed0a8d3de3..a4d829f6f99d 100644
--- a/drivers/staging/media/allegro-dvt/allegro-mail.h
+++ b/drivers/staging/media/allegro-dvt/allegro-mail.h
@@ -55,6 +55,8 @@ struct create_channel_param {
 	u32 log2_max_frame_num;
 	u32 temporal_mvp_enable;
 	u32 dbf_ovr_en;
+	u32 num_ref_idx_l0;
+	u32 num_ref_idx_l1;
 	u32 rdo_cost_mode;
 	u32 lf;
 	u32 lf_x_tile;
@@ -127,7 +129,8 @@ struct mcu_msg_create_channel_response {
 	u32 user_id;
 	u32 options;
 	u32 num_core;
-	u32 pps_param;
+	u32 num_ref_idx_l0;
+	u32 num_ref_idx_l1;
 	u32 int_buffers_count;
 	u32 int_buffers_size;
 	u32 rec_buffers_count;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 10/12] media: allegro: drop length field from message header
  2020-07-13 14:42 [PATCH v2 00/12] media: allegro: Add support for firmware 2019.2 Michael Tretter
                   ` (8 preceding siblings ...)
  2020-07-13 14:42 ` [PATCH v2 09/12] media: allegro: set num_ref_idx using response of configured channels Michael Tretter
@ 2020-07-13 14:42 ` Michael Tretter
  2020-07-13 14:42 ` [PATCH v2 11/12] media: allegro: add a version field to mcu messages Michael Tretter
  2020-07-13 14:42 ` [PATCH v2 12/12] media: allegro: add support for allegro firmware 2019.2 Michael Tretter
  11 siblings, 0 replies; 13+ messages in thread
From: Michael Tretter @ 2020-07-13 14:42 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab, kernel, Michael Tretter

The length of the message will be determined when the message is
encoded.  Writing the size of the struct into the message in the driver
won't be the actual length of the message that is send to the firmware.
Therefore, drop the length field from the message.

Since the header is the same for all response messages, it does not make
sense to read the header in each decoding function, but we can simply
decode it once before dispatching to the respective functions.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/staging/media/allegro-dvt/allegro-core.c |  6 ------
 drivers/staging/media/allegro-dvt/allegro-mail.c | 13 ++++---------
 drivers/staging/media/allegro-dvt/allegro-mail.h |  3 +--
 3 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/media/allegro-dvt/allegro-core.c b/drivers/staging/media/allegro-dvt/allegro-core.c
index 07deb5ba13dc..85d2c45be9d2 100644
--- a/drivers/staging/media/allegro-dvt/allegro-core.c
+++ b/drivers/staging/media/allegro-dvt/allegro-core.c
@@ -776,7 +776,6 @@ static void allegro_mcu_send_init(struct allegro_dev *dev,
 	memset(&msg, 0, sizeof(msg));
 
 	msg.header.type = MCU_MSG_TYPE_INIT;
-	msg.header.length = sizeof(msg) - sizeof(msg.header);
 
 	msg.suballoc_dma = to_mcu_addr(dev, suballoc_dma);
 	msg.suballoc_size = to_mcu_size(dev, suballoc_size);
@@ -995,7 +994,6 @@ static int allegro_mcu_send_create_channel(struct allegro_dev *dev,
 	memset(&msg, 0, sizeof(msg));
 
 	msg.header.type = MCU_MSG_TYPE_CREATE_CHANNEL;
-	msg.header.length = sizeof(msg) - sizeof(msg.header);
 
 	msg.user_id = channel->user_id;
 
@@ -1016,7 +1014,6 @@ static int allegro_mcu_send_destroy_channel(struct allegro_dev *dev,
 	memset(&msg, 0, sizeof(msg));
 
 	msg.header.type = MCU_MSG_TYPE_DESTROY_CHANNEL;
-	msg.header.length = sizeof(msg) - sizeof(msg.header);
 
 	msg.channel_id = channel->mcu_channel_id;
 
@@ -1036,7 +1033,6 @@ static int allegro_mcu_send_put_stream_buffer(struct allegro_dev *dev,
 	memset(&msg, 0, sizeof(msg));
 
 	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 = to_codec_addr(dev, paddr);
@@ -1061,7 +1057,6 @@ static int allegro_mcu_send_encode_frame(struct allegro_dev *dev,
 	memset(&msg, 0, sizeof(msg));
 
 	msg.header.type = MCU_MSG_TYPE_ENCODE_FRAME;
-	msg.header.length = sizeof(msg) - sizeof(msg.header);
 
 	msg.channel_id = channel->mcu_channel_id;
 	msg.encoding_options = AL_OPT_FORCE_LOAD;
@@ -1125,7 +1120,6 @@ static int allegro_mcu_push_buffer_internal(struct allegro_channel *channel,
 	if (!msg)
 		return -ENOMEM;
 
-	msg->header.length = size - sizeof(msg->header);
 	msg->header.type = type;
 	msg->channel_id = channel->mcu_channel_id;
 	msg->num_buffers = num_buffers;
diff --git a/drivers/staging/media/allegro-dvt/allegro-mail.c b/drivers/staging/media/allegro-dvt/allegro-mail.c
index bb15de080431..da83aa85c873 100644
--- a/drivers/staging/media/allegro-dvt/allegro-mail.c
+++ b/drivers/staging/media/allegro-dvt/allegro-mail.c
@@ -257,8 +257,6 @@ allegro_dec_init(struct mcu_msg_init_response *msg, u32 *src)
 {
 	unsigned int i = 0;
 
-	msg->header.type = FIELD_GET(GENMASK(31, 16), src[i]);
-	msg->header.length = FIELD_GET(GENMASK(15, 0), src[i++]);
 	msg->reserved0 = src[i++];
 
 	return i * sizeof(*src);
@@ -270,8 +268,6 @@ allegro_dec_create_channel(struct mcu_msg_create_channel_response *msg,
 {
 	unsigned int i = 0;
 
-	msg->header.type = FIELD_GET(GENMASK(31, 16), src[i]);
-	msg->header.length = FIELD_GET(GENMASK(15, 0), src[i++]);
 	msg->channel_id = src[i++];
 	msg->user_id = src[i++];
 	msg->options = src[i++];
@@ -294,8 +290,6 @@ allegro_dec_destroy_channel(struct mcu_msg_destroy_channel_response *msg,
 {
 	unsigned int i = 0;
 
-	msg->header.type = FIELD_GET(GENMASK(31, 16), src[i]);
-	msg->header.length = FIELD_GET(GENMASK(15, 0), src[i++]);
 	msg->channel_id = src[i++];
 
 	return i * sizeof(*src);
@@ -307,8 +301,6 @@ allegro_dec_encode_frame(struct mcu_msg_encode_frame_response *msg, u32 *src)
 	unsigned int i = 0;
 	unsigned int j;
 
-	msg->header.type = FIELD_GET(GENMASK(31, 16), src[i]);
-	msg->header.length = FIELD_GET(GENMASK(15, 0), src[i++]);
 	msg->channel_id = src[i++];
 
 	msg->stream_id = src[i++];
@@ -418,7 +410,10 @@ int allegro_decode_mail(void *msg, u32 *src)
 	if (!src || !msg)
 		return -EINVAL;
 
-	header = (struct mcu_msg_header *)src;
+	header = msg;
+	header->type = FIELD_GET(GENMASK(31, 16), src[0]);
+
+	src++;
 	switch (header->type) {
 	case MCU_MSG_TYPE_INIT:
 		allegro_dec_init(msg, src);
diff --git a/drivers/staging/media/allegro-dvt/allegro-mail.h b/drivers/staging/media/allegro-dvt/allegro-mail.h
index a4d829f6f99d..397622973c19 100644
--- a/drivers/staging/media/allegro-dvt/allegro-mail.h
+++ b/drivers/staging/media/allegro-dvt/allegro-mail.h
@@ -23,8 +23,7 @@ enum mcu_msg_type {
 const char *msg_type_name(enum mcu_msg_type type);
 
 struct mcu_msg_header {
-	u16 length;		/* length of the body in bytes */
-	u16 type;
+	enum mcu_msg_type type;
 };
 
 struct mcu_msg_init_request {
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 11/12] media: allegro: add a version field to mcu messages
  2020-07-13 14:42 [PATCH v2 00/12] media: allegro: Add support for firmware 2019.2 Michael Tretter
                   ` (9 preceding siblings ...)
  2020-07-13 14:42 ` [PATCH v2 10/12] media: allegro: drop length field from message header Michael Tretter
@ 2020-07-13 14:42 ` Michael Tretter
  2020-07-13 14:42 ` [PATCH v2 12/12] media: allegro: add support for allegro firmware 2019.2 Michael Tretter
  11 siblings, 0 replies; 13+ messages in thread
From: Michael Tretter @ 2020-07-13 14:42 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab, kernel, Michael Tretter

In order to distinguish the message format that is expected by the
firmware, add a version field to the message header. This allows to
encode and decode the messages for the version of the firmware that was
loaded by the driver.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 .../staging/media/allegro-dvt/allegro-core.c  | 22 ++++++++++++++-----
 .../staging/media/allegro-dvt/allegro-mail.h  |  6 +++++
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/media/allegro-dvt/allegro-core.c b/drivers/staging/media/allegro-dvt/allegro-core.c
index 85d2c45be9d2..ada21ebf93bb 100644
--- a/drivers/staging/media/allegro-dvt/allegro-core.c
+++ b/drivers/staging/media/allegro-dvt/allegro-core.c
@@ -130,6 +130,7 @@ struct allegro_dev {
 	struct regmap *regmap;
 	struct regmap *sram;
 
+	const struct fw_info *fw_info;
 	struct allegro_buffer firmware;
 	struct allegro_buffer suballocator;
 
@@ -277,6 +278,7 @@ struct fw_info {
 	unsigned int mailbox_cmd;
 	unsigned int mailbox_status;
 	size_t mailbox_size;
+	enum mcu_msg_version mailbox_version;
 	size_t suballocator_size;
 };
 
@@ -288,6 +290,7 @@ static const struct fw_info supported_firmware[] = {
 		.mailbox_cmd = 0x7800,
 		.mailbox_status = 0x7c00,
 		.mailbox_size = 0x400 - 0x8,
+		.mailbox_version = MCU_MSG_VERSION_2018_2,
 		.suballocator_size = SZ_16M,
 	},
 };
@@ -749,6 +752,8 @@ static void allegro_mbox_notify(struct allegro_mbox *mbox)
 	if (!msg)
 		return;
 
+	msg->header.version = dev->fw_info->mailbox_version;
+
 	tmp = kmalloc(mbox->size, GFP_KERNEL);
 	if (!tmp)
 		goto out;
@@ -776,6 +781,7 @@ static void allegro_mcu_send_init(struct allegro_dev *dev,
 	memset(&msg, 0, sizeof(msg));
 
 	msg.header.type = MCU_MSG_TYPE_INIT;
+	msg.header.version = dev->fw_info->mailbox_version;
 
 	msg.suballoc_dma = to_mcu_addr(dev, suballoc_dma);
 	msg.suballoc_size = to_mcu_size(dev, suballoc_size);
@@ -989,11 +995,13 @@ static int allegro_mcu_send_create_channel(struct allegro_dev *dev,
 	memset(&param, 0, sizeof(param));
 	fill_create_channel_param(channel, &param);
 	allegro_alloc_buffer(dev, blob, sizeof(struct create_channel_param));
+	param.version = dev->fw_info->mailbox_version;
 	size = allegro_encode_config_blob(blob->vaddr, &param);
 
 	memset(&msg, 0, sizeof(msg));
 
 	msg.header.type = MCU_MSG_TYPE_CREATE_CHANNEL;
+	msg.header.version = dev->fw_info->mailbox_version;
 
 	msg.user_id = channel->user_id;
 
@@ -1014,6 +1022,7 @@ static int allegro_mcu_send_destroy_channel(struct allegro_dev *dev,
 	memset(&msg, 0, sizeof(msg));
 
 	msg.header.type = MCU_MSG_TYPE_DESTROY_CHANNEL;
+	msg.header.version = dev->fw_info->mailbox_version;
 
 	msg.channel_id = channel->mcu_channel_id;
 
@@ -1033,6 +1042,7 @@ static int allegro_mcu_send_put_stream_buffer(struct allegro_dev *dev,
 	memset(&msg, 0, sizeof(msg));
 
 	msg.header.type = MCU_MSG_TYPE_PUT_STREAM_BUFFER;
+	msg.header.version = dev->fw_info->mailbox_version;
 
 	msg.channel_id = channel->mcu_channel_id;
 	msg.dma_addr = to_codec_addr(dev, paddr);
@@ -1057,6 +1067,7 @@ static int allegro_mcu_send_encode_frame(struct allegro_dev *dev,
 	memset(&msg, 0, sizeof(msg));
 
 	msg.header.type = MCU_MSG_TYPE_ENCODE_FRAME;
+	msg.header.version = dev->fw_info->mailbox_version;
 
 	msg.channel_id = channel->mcu_channel_id;
 	msg.encoding_options = AL_OPT_FORCE_LOAD;
@@ -1121,6 +1132,8 @@ static int allegro_mcu_push_buffer_internal(struct allegro_channel *channel,
 		return -ENOMEM;
 
 	msg->header.type = type;
+	msg->header.version = dev->fw_info->mailbox_version;
+
 	msg->channel_id = channel->mcu_channel_id;
 	msg->num_buffers = num_buffers;
 
@@ -2988,7 +3001,6 @@ static void allegro_fw_callback(const struct firmware *fw, void *context)
 	const char *fw_codec_name = "al5e.fw";
 	const struct firmware *fw_codec;
 	int err;
-	const struct fw_info *info;
 
 	if (!fw)
 		return;
@@ -2999,14 +3011,14 @@ static void allegro_fw_callback(const struct firmware *fw, void *context)
 	if (err)
 		goto err_release_firmware;
 
-	info = allegro_get_firmware_info(dev, fw, fw_codec);
-	if (!info) {
+	dev->fw_info = allegro_get_firmware_info(dev, fw, fw_codec);
+	if (!dev->fw_info) {
 		v4l2_err(&dev->v4l2_dev, "firmware is not supported\n");
 		goto err_release_firmware_codec;
 	}
 
 	v4l2_info(&dev->v4l2_dev,
-		  "using mcu firmware version '%s'\n", info->version);
+		  "using mcu firmware version '%s'\n", dev->fw_info->version);
 
 	/* Ensure that the mcu is sleeping at the reset vector */
 	err = allegro_mcu_reset(dev);
@@ -3018,7 +3030,7 @@ static void allegro_fw_callback(const struct firmware *fw, void *context)
 	allegro_copy_firmware(dev, fw->data, fw->size);
 	allegro_copy_fw_codec(dev, fw_codec->data, fw_codec->size);
 
-	err = allegro_mcu_hw_init(dev, info);
+	err = allegro_mcu_hw_init(dev, dev->fw_info);
 	if (err) {
 		v4l2_err(&dev->v4l2_dev, "failed to initialize mcu\n");
 		goto err_free_fw_codec;
diff --git a/drivers/staging/media/allegro-dvt/allegro-mail.h b/drivers/staging/media/allegro-dvt/allegro-mail.h
index 397622973c19..c095dbfcf104 100644
--- a/drivers/staging/media/allegro-dvt/allegro-mail.h
+++ b/drivers/staging/media/allegro-dvt/allegro-mail.h
@@ -20,10 +20,15 @@ enum mcu_msg_type {
 	MCU_MSG_TYPE_PUSH_BUFFER_REFERENCE = 0x000f,
 };
 
+enum mcu_msg_version {
+	MCU_MSG_VERSION_2018_2,
+};
+
 const char *msg_type_name(enum mcu_msg_type type);
 
 struct mcu_msg_header {
 	enum mcu_msg_type type;
+	enum mcu_msg_version version;
 };
 
 struct mcu_msg_init_request {
@@ -40,6 +45,7 @@ struct mcu_msg_init_response {
 };
 
 struct create_channel_param {
+	enum mcu_msg_version version;
 	u16 width;
 	u16 height;
 	u32 format;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 12/12] media: allegro: add support for allegro firmware 2019.2
  2020-07-13 14:42 [PATCH v2 00/12] media: allegro: Add support for firmware 2019.2 Michael Tretter
                   ` (10 preceding siblings ...)
  2020-07-13 14:42 ` [PATCH v2 11/12] media: allegro: add a version field to mcu messages Michael Tretter
@ 2020-07-13 14:42 ` Michael Tretter
  11 siblings, 0 replies; 13+ messages in thread
From: Michael Tretter @ 2020-07-13 14:42 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab, kernel, Michael Tretter

Encode messages as necessary for the firmware 2019.2 and, thus, support
the more recent firmware version in the driver, too.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 .../staging/media/allegro-dvt/allegro-core.c  |  27 ++-
 .../staging/media/allegro-dvt/allegro-mail.c  | 163 +++++++++++++++---
 .../staging/media/allegro-dvt/allegro-mail.h  |  24 +++
 3 files changed, 186 insertions(+), 28 deletions(-)

diff --git a/drivers/staging/media/allegro-dvt/allegro-core.c b/drivers/staging/media/allegro-dvt/allegro-core.c
index ada21ebf93bb..61beae1fca36 100644
--- a/drivers/staging/media/allegro-dvt/allegro-core.c
+++ b/drivers/staging/media/allegro-dvt/allegro-core.c
@@ -292,6 +292,15 @@ static const struct fw_info supported_firmware[] = {
 		.mailbox_size = 0x400 - 0x8,
 		.mailbox_version = MCU_MSG_VERSION_2018_2,
 		.suballocator_size = SZ_16M,
+	}, {
+		.id = 14680,
+		.id_codec = 126572,
+		.version = "v2019.2",
+		.mailbox_cmd = 0x7000,
+		.mailbox_status = 0x7800,
+		.mailbox_size = 0x800 - 0x8,
+		.mailbox_version = MCU_MSG_VERSION_2019_2,
+		.suballocator_size = SZ_32M,
 	},
 };
 
@@ -930,10 +939,13 @@ static int fill_create_channel_param(struct allegro_channel *channel,
 
 	param->dbf_ovr_en = 1;
 	param->rdo_cost_mode = 1;
+	param->custom_lda = 1;
 	param->lf = 1;
 	param->lf_x_tile = 1;
 	param->lf_x_slice = 1;
 
+	param->src_bit_depth = 8;
+
 	param->beta_offset = -1;
 	param->tc_offset = -1;
 	param->num_slices = 1;
@@ -974,13 +986,26 @@ static int fill_create_channel_param(struct allegro_channel *channel,
 	param->golden_ref_frequency = 10;
 	param->rate_control_option = 0x00000000;
 
-	param->gop_ctrl_mode = 0x00000000;
+	param->num_pixel = channel->width + channel->height;
+	param->max_psnr = 4200;
+	param->max_pixel_value = 255;
+
+	param->gop_ctrl_mode = 0x00000002;
 	param->freq_idr = channel->gop_size;
 	param->freq_lt = 0;
 	param->gdr_mode = 0x00000000;
 	param->gop_length = channel->gop_size;
 	param->subframe_latency = 0x00000000;
 
+	param->lda_factors[0] = 51;
+	param->lda_factors[1] = 90;
+	param->lda_factors[2] = 151;
+	param->lda_factors[3] = 151;
+	param->lda_factors[4] = 151;
+	param->lda_factors[5] = 151;
+
+	param->max_num_merge_cand = 5;
+
 	return 0;
 }
 
diff --git a/drivers/staging/media/allegro-dvt/allegro-mail.c b/drivers/staging/media/allegro-dvt/allegro-mail.c
index da83aa85c873..4ac65de12463 100644
--- a/drivers/staging/media/allegro-dvt/allegro-mail.c
+++ b/drivers/staging/media/allegro-dvt/allegro-mail.c
@@ -44,6 +44,7 @@ static ssize_t
 allegro_enc_init(u32 *dst, struct mcu_msg_init_request *msg)
 {
 	unsigned int i = 0;
+	enum mcu_msg_version version = msg->header.version;
 
 	dst[i++] = msg->reserved0;
 	dst[i++] = msg->suballoc_dma;
@@ -51,33 +52,55 @@ allegro_enc_init(u32 *dst, struct mcu_msg_init_request *msg)
 	dst[i++] = msg->l2_cache[0];
 	dst[i++] = msg->l2_cache[1];
 	dst[i++] = msg->l2_cache[2];
+	if (version >= MCU_MSG_VERSION_2019_2) {
+		dst[i++] = -1;
+		dst[i++] = 0;
+	}
 
 	return i * sizeof(*dst);
 }
 
 static inline u32 settings_get_mcu_codec(struct create_channel_param *param)
 {
+	enum mcu_msg_version version = param->version;
 	u32 pixelformat = param->codec;
 
-	switch (pixelformat) {
-	case V4L2_PIX_FMT_H264:
-	default:
-		return 1;
+	if (version < MCU_MSG_VERSION_2019_2) {
+		switch (pixelformat) {
+		case V4L2_PIX_FMT_H264:
+		default:
+			return 1;
+		}
+	} else {
+		switch (pixelformat) {
+		case V4L2_PIX_FMT_H264:
+		default:
+			return 0;
+		}
 	}
 }
 
 ssize_t
 allegro_encode_config_blob(u32 *dst, struct create_channel_param *param)
 {
+	enum mcu_msg_version version = param->version;
 	unsigned int i = 0;
+	unsigned int j = 0;
 	u32 val;
 	unsigned int codec = settings_get_mcu_codec(param);
 
+	if (version >= MCU_MSG_VERSION_2019_2)
+		dst[i++] = param->layer_id;
 	dst[i++] = FIELD_PREP(GENMASK(31, 16), param->height) |
 		   FIELD_PREP(GENMASK(15, 0), param->width);
+	if (version >= MCU_MSG_VERSION_2019_2)
+		dst[i++] = param->videomode;
 	dst[i++] = param->format;
-	dst[i++] = param->colorspace;
+	if (version < MCU_MSG_VERSION_2019_2)
+		dst[i++] = param->colorspace;
 	dst[i++] = param->src_mode;
+	if (version >= MCU_MSG_VERSION_2019_2)
+		dst[i++] = param->src_bit_depth;
 	dst[i++] = FIELD_PREP(GENMASK(31, 24), codec) |
 		   FIELD_PREP(GENMASK(23, 8), param->constraint_set_flags) |
 		   FIELD_PREP(GENMASK(7, 0), param->profile);
@@ -94,19 +117,31 @@ allegro_encode_config_blob(u32 *dst, struct create_channel_param *param)
 	val |= param->dbf_ovr_en ? BIT(2) : 0;
 	dst[i++] = val;
 
-	val = 0;
-	val |= param->lf ? BIT(2) : 0;
-	val |= param->lf_x_tile ? BIT(3) : 0;
-	val |= param->lf_x_slice ? BIT(4) : 0;
-	val |= param->rdo_cost_mode ? BIT(20) : 0;
-	dst[i++] = val;
+	if (version >= MCU_MSG_VERSION_2019_2) {
+		val = 0;
+		val |= param->custom_lda ? BIT(2) : 0;
+		val |= param->rdo_cost_mode ? BIT(20) : 0;
+		dst[i++] = val;
+
+		val = 0;
+		val |= param->lf ? BIT(2) : 0;
+		val |= param->lf_x_tile ? BIT(3) : 0;
+		val |= param->lf_x_slice ? BIT(4) : 0;
+		dst[i++] = val;
+	} else {
+		val = 0;
+		dst[i++] = val;
+	}
 
 	dst[i++] = FIELD_PREP(GENMASK(15, 8), param->beta_offset) |
 		   FIELD_PREP(GENMASK(7, 0), param->tc_offset);
 	dst[i++] = param->unknown11;
 	dst[i++] = param->unknown12;
-	dst[i++] = FIELD_PREP(GENMASK(31, 16), param->prefetch_auto) |
-		   FIELD_PREP(GENMASK(15, 0), param->num_slices);
+	if (version >= MCU_MSG_VERSION_2019_2)
+		dst[i++] = param->num_slices;
+	else
+		dst[i++] = FIELD_PREP(GENMASK(31, 16), param->prefetch_auto) |
+			   FIELD_PREP(GENMASK(15, 0), param->num_slices);
 	dst[i++] = param->prefetch_mem_offset;
 	dst[i++] = param->prefetch_mem_size;
 	dst[i++] = FIELD_PREP(GENMASK(31, 16), param->clip_vrt_range) |
@@ -139,19 +174,51 @@ allegro_encode_config_blob(u32 *dst, struct create_channel_param *param)
 		   FIELD_PREP(GENMASK(15, 0), param->pb_delta);
 	dst[i++] = FIELD_PREP(GENMASK(31, 16), param->golden_ref_frequency) |
 		   FIELD_PREP(GENMASK(15, 0), param->golden_delta);
-	dst[i++] = param->rate_control_option;
+	if (version >= MCU_MSG_VERSION_2019_2)
+		dst[i++] = param->rate_control_option;
+	else
+		dst[i++] = 0;
+
+	if (version >= MCU_MSG_VERSION_2019_2) {
+		dst[i++] = param->num_pixel;
+		dst[i++] = FIELD_PREP(GENMASK(31, 16), param->max_pixel_value) |
+			FIELD_PREP(GENMASK(15, 0), param->max_psnr);
+		for (j = 0; j < 3; j++)
+			dst[i++] = param->maxpicturesize[j];
+	}
+
+	if (version >= MCU_MSG_VERSION_2019_2)
+		dst[i++] = param->gop_ctrl_mode;
+	else
+		dst[i++] = 0;
 
-	dst[i++] = param->gop_ctrl_mode;
+	if (version >= MCU_MSG_VERSION_2019_2)
+		dst[i++] = FIELD_PREP(GENMASK(31, 24), param->freq_golden_ref) |
+			   FIELD_PREP(GENMASK(23, 16), param->num_b) |
+			   FIELD_PREP(GENMASK(15, 0), param->gop_length);
 	dst[i++] = param->freq_idr;
+	if (version >= MCU_MSG_VERSION_2019_2)
+		dst[i++] = param->enable_lt;
 	dst[i++] = param->freq_lt;
 	dst[i++] = param->gdr_mode;
-	dst[i++] = FIELD_PREP(GENMASK(31, 24), param->freq_golden_ref) |
-		   FIELD_PREP(GENMASK(23, 16), param->num_b) |
-		   FIELD_PREP(GENMASK(15, 0), param->gop_length);
+	if (version < MCU_MSG_VERSION_2019_2)
+		dst[i++] = FIELD_PREP(GENMASK(31, 24), param->freq_golden_ref) |
+			   FIELD_PREP(GENMASK(23, 16), param->num_b) |
+			   FIELD_PREP(GENMASK(15, 0), param->gop_length);
+
+	if (version >= MCU_MSG_VERSION_2019_2)
+		dst[i++] = param->tmpdqp;
 
 	dst[i++] = param->subframe_latency;
 	dst[i++] = param->lda_control_mode;
-	dst[i++] = param->unknown41;
+	if (version < MCU_MSG_VERSION_2019_2)
+		dst[i++] = param->unknown41;
+
+	if (version >= MCU_MSG_VERSION_2019_2) {
+		for (j = 0; j < 6; j++)
+			dst[i++] = param->lda_factors[j];
+		dst[i++] = param->max_num_merge_cand;
+	}
 
 	return i * sizeof(*dst);
 }
@@ -159,12 +226,20 @@ allegro_encode_config_blob(u32 *dst, struct create_channel_param *param)
 static ssize_t
 allegro_enc_create_channel(u32 *dst, struct mcu_msg_create_channel *msg)
 {
+	enum mcu_msg_version version = msg->header.version;
 	unsigned int i = 0;
 
 	dst[i++] = msg->user_id;
 
-	memcpy(&dst[i], msg->blob, msg->blob_size);
-	i += msg->blob_size / sizeof(*dst);
+	if (version >= MCU_MSG_VERSION_2019_2) {
+		dst[i++] = msg->blob_mcu_addr;
+	} else {
+		memcpy(&dst[i], msg->blob, msg->blob_size);
+		i += msg->blob_size / sizeof(*dst);
+	}
+
+	if (version >= MCU_MSG_VERSION_2019_2)
+		dst[i++] = msg->ep1_addr;
 
 	return i * sizeof(*dst);
 }
@@ -173,8 +248,15 @@ ssize_t allegro_decode_config_blob(struct create_channel_param *param,
 				   struct mcu_msg_create_channel_response *msg,
 				   u32 *src)
 {
-	param->num_ref_idx_l0 = msg->num_ref_idx_l0;
-	param->num_ref_idx_l1 = msg->num_ref_idx_l1;
+	enum mcu_msg_version version = msg->header.version;
+
+	if (version >= MCU_MSG_VERSION_2019_2) {
+		param->num_ref_idx_l0 = FIELD_GET(GENMASK(7, 4), src[9]);
+		param->num_ref_idx_l1 = FIELD_GET(GENMASK(11, 8), src[9]);
+	} else {
+		param->num_ref_idx_l0 = msg->num_ref_idx_l0;
+		param->num_ref_idx_l1 = msg->num_ref_idx_l1;
+	}
 
 	return 0;
 }
@@ -229,6 +311,7 @@ allegro_enc_put_stream_buffer(u32 *dst,
 static ssize_t
 allegro_enc_encode_frame(u32 *dst, struct mcu_msg_encode_frame *msg)
 {
+	enum mcu_msg_version version = msg->header.version;
 	unsigned int i = 0;
 
 	dst[i++] = msg->channel_id;
@@ -237,6 +320,14 @@ allegro_enc_encode_frame(u32 *dst, struct mcu_msg_encode_frame *msg)
 	dst[i++] = msg->encoding_options;
 	dst[i++] = FIELD_PREP(GENMASK(31, 16), msg->padding) |
 		   FIELD_PREP(GENMASK(15, 0), msg->pps_qp);
+
+	if (version >= MCU_MSG_VERSION_2019_2) {
+		dst[i++] = 0;
+		dst[i++] = 0;
+		dst[i++] = 0;
+		dst[i++] = 0;
+	}
+
 	dst[i++] = lower_32_bits(msg->user_param);
 	dst[i++] = upper_32_bits(msg->user_param);
 	dst[i++] = lower_32_bits(msg->src_handle);
@@ -244,7 +335,11 @@ allegro_enc_encode_frame(u32 *dst, struct mcu_msg_encode_frame *msg)
 	dst[i++] = msg->request_options;
 	dst[i++] = msg->src_y;
 	dst[i++] = msg->src_uv;
+	if (version >= MCU_MSG_VERSION_2019_2)
+		dst[i++] = msg->is_10_bit;
 	dst[i++] = msg->stride;
+	if (version >= MCU_MSG_VERSION_2019_2)
+		dst[i++] = msg->format;
 	dst[i++] = msg->ep2;
 	dst[i++] = lower_32_bits(msg->ep2_v);
 	dst[i++] = upper_32_bits(msg->ep2_v);
@@ -266,14 +361,21 @@ static ssize_t
 allegro_dec_create_channel(struct mcu_msg_create_channel_response *msg,
 			   u32 *src)
 {
+	enum mcu_msg_version version = msg->header.version;
 	unsigned int i = 0;
 
 	msg->channel_id = src[i++];
 	msg->user_id = src[i++];
-	msg->options = src[i++];
-	msg->num_core = src[i++];
-	msg->num_ref_idx_l0 = FIELD_GET(GENMASK(7, 4), src[i]);
-	msg->num_ref_idx_l1 = FIELD_GET(GENMASK(11, 8), src[i++]);
+	/*
+	 * Version >= MCU_MSG_VERSION_2019_2 is handled in
+	 * allegro_decode_config_blob().
+	 */
+	if (version < MCU_MSG_VERSION_2019_2) {
+		msg->options = src[i++];
+		msg->num_core = src[i++];
+		msg->num_ref_idx_l0 = FIELD_GET(GENMASK(7, 4), src[i]);
+		msg->num_ref_idx_l1 = FIELD_GET(GENMASK(11, 8), src[i++]);
+	}
 	msg->int_buffers_count = src[i++];
 	msg->int_buffers_size = src[i++];
 	msg->rec_buffers_count = src[i++];
@@ -298,6 +400,7 @@ allegro_dec_destroy_channel(struct mcu_msg_destroy_channel_response *msg,
 static ssize_t
 allegro_dec_encode_frame(struct mcu_msg_encode_frame_response *msg, u32 *src)
 {
+	enum mcu_msg_version version = msg->header.version;
 	unsigned int i = 0;
 	unsigned int j;
 
@@ -341,6 +444,12 @@ allegro_dec_encode_frame(struct mcu_msg_encode_frame_response *msg, u32 *src)
 	msg->pps_qp = FIELD_GET(GENMASK(15, 0), src[i++]);
 
 	msg->reserved2 = src[i++];
+	if (version >= MCU_MSG_VERSION_2019_2) {
+		msg->reserved3 = src[i++];
+		msg->reserved4 = src[i++];
+		msg->reserved5 = src[i++];
+		msg->reserved6 = src[i++];
+	}
 
 	return i * sizeof(*src);
 }
diff --git a/drivers/staging/media/allegro-dvt/allegro-mail.h b/drivers/staging/media/allegro-dvt/allegro-mail.h
index c095dbfcf104..486ecb12b098 100644
--- a/drivers/staging/media/allegro-dvt/allegro-mail.h
+++ b/drivers/staging/media/allegro-dvt/allegro-mail.h
@@ -22,6 +22,7 @@ enum mcu_msg_type {
 
 enum mcu_msg_version {
 	MCU_MSG_VERSION_2018_2,
+	MCU_MSG_VERSION_2019_2,
 };
 
 const char *msg_type_name(enum mcu_msg_type type);
@@ -46,11 +47,14 @@ struct mcu_msg_init_response {
 
 struct create_channel_param {
 	enum mcu_msg_version version;
+	u32 layer_id;
 	u16 width;
 	u16 height;
+	u32 videomode;
 	u32 format;
 	u32 colorspace;
 	u32 src_mode;
+	u32 src_bit_depth;
 	u8 profile;
 	u16 constraint_set_flags;
 	u32 codec;
@@ -59,9 +63,11 @@ struct create_channel_param {
 	u32 log2_max_poc;
 	u32 log2_max_frame_num;
 	u32 temporal_mvp_enable;
+	u32 enable_reordering;
 	u32 dbf_ovr_en;
 	u32 num_ref_idx_l0;
 	u32 num_ref_idx_l1;
+	u32 custom_lda;
 	u32 rdo_cost_mode;
 	u32 lf;
 	u32 lf_x_tile;
@@ -105,6 +111,10 @@ struct create_channel_param {
 	u16 golden_delta;
 	u16 golden_ref_frequency;
 	u32 rate_control_option;
+	u32 num_pixel;
+	u16 max_psnr;
+	u16 max_pixel_value;
+	u32 maxpicturesize[3];
 
 	/* gop param */
 	u32 gop_ctrl_mode;
@@ -114,10 +124,16 @@ struct create_channel_param {
 	u16 gop_length;
 	u8 num_b;
 	u8 freq_golden_ref;
+	u32 enable_lt;
+	u32 tmpdqp;
 
 	u32 subframe_latency;
 	u32 lda_control_mode;
 	u32 unknown41;
+
+	u32 lda_factors[6];
+
+	u32 max_num_merge_cand;
 };
 
 struct mcu_msg_create_channel {
@@ -126,6 +142,7 @@ struct mcu_msg_create_channel {
 	u32 *blob;
 	size_t blob_size;
 	u32 blob_mcu_addr;
+	u32 ep1_addr;
 };
 
 struct mcu_msg_create_channel_response {
@@ -203,9 +220,12 @@ struct mcu_msg_encode_frame {
 	/* u32 scene_change_delay (optional) */
 	/* rate control param (optional) */
 	/* gop param (optional) */
+	/* dynamic resolution params (optional) */
 	u32 src_y;
 	u32 src_uv;
+	u32 is_10_bit;
 	u32 stride;
+	u32 format;
 	u32 ep2;
 	u64 ep2_v;
 };
@@ -249,6 +269,10 @@ struct mcu_msg_encode_frame_response {
 	u16 pps_qp;
 	u16 reserved1;
 	u32 reserved2;
+	u32 reserved3;
+	u32 reserved4;
+	u32 reserved5;
+	u32 reserved6;
 };
 
 union mcu_msg_response {
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-07-13 14:42 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-13 14:42 [PATCH v2 00/12] media: allegro: Add support for firmware 2019.2 Michael Tretter
2020-07-13 14:42 ` [PATCH v2 01/12] media: allegro: rework mbox handling Michael Tretter
2020-07-13 14:42 ` [PATCH v2 02/12] media: allegro: rework read/write to mailbox Michael Tretter
2020-07-13 14:42 ` [PATCH v2 03/12] media: allegro: add explicit mail encoding and decoding Michael Tretter
2020-07-13 14:42 ` [PATCH v2 04/12] media: allegro: add field for number of buffers Michael Tretter
2020-07-13 14:42 ` [PATCH v2 05/12] media: allegro: don't pack MCU messages Michael Tretter
2020-07-13 14:42 ` [PATCH v2 06/12] media: allegro: support handling firmware dependent values Michael Tretter
2020-07-13 14:42 ` [PATCH v2 07/12] media: allegro: encode bit fields separately Michael Tretter
2020-07-13 14:42 ` [PATCH v2 08/12] media: allegro: add config blob for channel Michael Tretter
2020-07-13 14:42 ` [PATCH v2 09/12] media: allegro: set num_ref_idx using response of configured channels Michael Tretter
2020-07-13 14:42 ` [PATCH v2 10/12] media: allegro: drop length field from message header Michael Tretter
2020-07-13 14:42 ` [PATCH v2 11/12] media: allegro: add a version field to mcu messages Michael Tretter
2020-07-13 14:42 ` [PATCH v2 12/12] media: allegro: add support for allegro firmware 2019.2 Michael Tretter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.