Re: [PATCH v2 14/34] media: iris: implement iris v4l2 file ops

From: Bryan O'Donoghue
Date: Tue Dec 19 2023 - 06:57:41 EST


On 18/12/2023 11:32, Dikshita Agarwal wrote:
From: Vikash Garodia <quic_vgarodia@xxxxxxxxxxx>

Implements iris v4l2 file ops - open, close and poll.

Open:
Configure the vb2 queue and v4l2 file handler. Initialize
a video instance and add the instance to core instance list.
Open a session in video firmware via HFI_CMD_OPEN.

Close:
De-initialize the instance and remove it from core
instance list. Close the session in firmware via HFI_CMD_CLOSE.

Signed-off-by: Vikash Garodia <quic_vgarodia@xxxxxxxxxxx>
Signed-off-by: Dikshita Agarwal <quic_dikshita@xxxxxxxxxxx>
---
drivers/media/platform/qcom/vcodec/iris/Makefile | 2 +
.../media/platform/qcom/vcodec/iris/hfi_defines.h | 2 +
.../media/platform/qcom/vcodec/iris/iris_common.h | 26 ++
.../media/platform/qcom/vcodec/iris/iris_core.c | 42 ++-
.../media/platform/qcom/vcodec/iris/iris_core.h | 8 +
.../media/platform/qcom/vcodec/iris/iris_helpers.c | 62 +++++
.../media/platform/qcom/vcodec/iris/iris_helpers.h | 2 +
drivers/media/platform/qcom/vcodec/iris/iris_hfi.c | 115 +++++++-
drivers/media/platform/qcom/vcodec/iris/iris_hfi.h | 3 +
.../platform/qcom/vcodec/iris/iris_hfi_packet.c | 28 ++
.../platform/qcom/vcodec/iris/iris_hfi_packet.h | 7 +
.../platform/qcom/vcodec/iris/iris_instance.h | 48 ++++
.../media/platform/qcom/vcodec/iris/iris_probe.c | 10 +
.../media/platform/qcom/vcodec/iris/iris_vdec.c | 47 ++++
.../media/platform/qcom/vcodec/iris/iris_vdec.h | 14 +
.../media/platform/qcom/vcodec/iris/iris_vidc.c | 298 +++++++++++++++++++++
.../media/platform/qcom/vcodec/iris/iris_vidc.h | 15 ++
17 files changed, 719 insertions(+), 10 deletions(-)
create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_common.h
create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_instance.h
create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_vdec.c
create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_vdec.h
create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_vidc.c
create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_vidc.h

diff --git a/drivers/media/platform/qcom/vcodec/iris/Makefile b/drivers/media/platform/qcom/vcodec/iris/Makefile
index c50e3241..3c076d0 100644
--- a/drivers/media/platform/qcom/vcodec/iris/Makefile
+++ b/drivers/media/platform/qcom/vcodec/iris/Makefile
@@ -3,6 +3,8 @@ iris-objs += ../hfi_queue.o ../firmware.o
iris-objs += iris_probe.o \
iris_state.o \
iris_core.o \
+ iris_vidc.o \
+ iris_vdec.o \
iris_state.o \
iris_helpers.o \
iris_hfi.o \
diff --git a/drivers/media/platform/qcom/vcodec/iris/hfi_defines.h b/drivers/media/platform/qcom/vcodec/iris/hfi_defines.h
index fb383b2..423ba1a 100644
--- a/drivers/media/platform/qcom/vcodec/iris/hfi_defines.h
+++ b/drivers/media/platform/qcom/vcodec/iris/hfi_defines.h
@@ -9,6 +9,8 @@
#define HFI_VIDEO_ARCH_LX 0x1
#define HFI_CMD_INIT 0x01000001
+#define HFI_CMD_OPEN 0x01000003
+#define HFI_CMD_CLOSE 0x01000004
#define HFI_PROP_IMAGE_VERSION 0x03000001
diff --git a/drivers/media/platform/qcom/vcodec/iris/iris_common.h b/drivers/media/platform/qcom/vcodec/iris/iris_common.h
new file mode 100644
index 0000000..3e4dd71
--- /dev/null
+++ b/drivers/media/platform/qcom/vcodec/iris/iris_common.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+#ifndef _IRIS_COMMON_H_
+#define _IRIS_COMMON_H_
+
+#include <media/v4l2-device.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-ioctl.h>
+#include <media/videobuf2-v4l2.h>
+
+#define INPUT_MPLANE V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE
+#define OUTPUT_MPLANE V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE
+#define DEFAULT_WIDTH 320
+#define DEFAULT_HEIGHT 240
+#define DEFAULT_BUF_SIZE 115200
+
+enum signal_session_response {
+ SIGNAL_CMD_STOP_INPUT = 0,
+ SIGNAL_CMD_STOP_OUTPUT,
+ SIGNAL_CMD_CLOSE,
+ MAX_SIGNAL,
+};
+
+#endif
diff --git a/drivers/media/platform/qcom/vcodec/iris/iris_core.c b/drivers/media/platform/qcom/vcodec/iris/iris_core.c
index ba8960d..174fad9 100644
--- a/drivers/media/platform/qcom/vcodec/iris/iris_core.c
+++ b/drivers/media/platform/qcom/vcodec/iris/iris_core.c
@@ -3,12 +3,14 @@
* Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved.
*/
+#include <linux/delay.h>
+
#include "iris_core.h"
#include "iris_helpers.h"
#include "iris_hfi.h"
#include "iris_state.h"
-static int iris_core_deinit_locked(struct iris_core *core)
+int iris_core_deinit_locked(struct iris_core *core)
{
int ret;
@@ -68,3 +70,41 @@ int iris_core_init(struct iris_core *core)
return ret;
}
+
+int iris_core_init_wait(struct iris_core *core)
+{
+ const int interval = 10;
+ int max_tries, count = 0, ret = 0;
+
+ mutex_lock(&core->lock);
+ if (!core_in_valid_state(core)) {
+ ret = -EINVAL;
+ goto unlock;
+ }
+
+ if (core->state == IRIS_CORE_INIT)
+ goto unlock;
+
+ max_tries = core->cap[HW_RESPONSE_TIMEOUT].value / interval;
+ while (count < max_tries) {
+ if (core->state != IRIS_CORE_INIT_WAIT)
+ break;
+ msleep(interval);
+ count++;
+ }

Is the intention here really to poll and sleep for 10 milliseconds and if so why ?

Shouldn't there be some sort of bootup IRQ or event that we can trap ?

That's how it works in the venus driver right ? Just kick a workqueue when the IRQ fires and have the iris_core_init_wait() do a wait_for_completion_timeout().


+int iris_hfi_session_open(struct iris_inst *inst)
+{
+ struct iris_core *core;
+ int ret;
+
+ inst->packet_size = 4096;
+ inst->packet = kzalloc(inst->packet_size, GFP_KERNEL);
+ if (!inst->packet)
+ return -ENOMEM;

Who is responsible for cleaning up this allocation ?

+
+fail_free_packet:
+ kfree(inst->packet);

freed here.

+int iris_hfi_session_close(struct iris_inst *inst)
+{
}

but not here ?

Still finding this alot of code to go through.

Please try to break this up into smaller series.

---
bod