[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6b2ce264-3cc6-4c00-85c8-c885b2707787@linaro.org>
Date: Tue, 19 Dec 2023 11:57:03 +0000
From: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
To: Dikshita Agarwal <quic_dikshita@...cinc.com>,
linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
stanimir.k.varbanov@...il.com, quic_vgarodia@...cinc.com, agross@...nel.org,
andersson@...nel.org, konrad.dybcio@...aro.org, mchehab@...nel.org
Cc: linux-arm-msm@...r.kernel.org, quic_abhinavk@...cinc.com
Subject: Re: [PATCH v2 14/34] media: iris: implement iris v4l2 file ops
On 18/12/2023 11:32, Dikshita Agarwal wrote:
> From: Vikash Garodia <quic_vgarodia@...cinc.com>
>
> 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@...cinc.com>
> Signed-off-by: Dikshita Agarwal <quic_dikshita@...cinc.com>
> ---
> 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
Powered by blists - more mailing lists