Re: [PATCH v6 4/5] media: platform: mediatek: isp_30: add mediatek ISP3.0 camsv
From: Laurent Pinchart
Date: Tue Jul 30 2024 - 10:09:54 EST
Hi Julien,
Thank you for the patch.
On Mon, Jul 29, 2024 at 04:48:03PM +0200, Julien Stephan wrote:
> From: Phi-bang Nguyen <pnguyen@xxxxxxxxxxxx>
>
> This driver provides a path to bypass the SoC ISP so that image data
> coming from the SENINF can go directly into memory without any image
> processing. This allows the use of an external ISP.
>
> Signed-off-by: Phi-bang Nguyen <pnguyen@xxxxxxxxxxxx>
> Signed-off-by: Florian Sylvestre <fsylvestre@xxxxxxxxxxxx>
> [Paul Elder fix irq locking]
> Signed-off-by: Paul Elder <paul.elder@xxxxxxxxxxxxxxxx>
> Co-developed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Co-developed-by: Julien Stephan <jstephan@xxxxxxxxxxxx>
> Signed-off-by: Julien Stephan <jstephan@xxxxxxxxxxxx>
> ---
> MAINTAINERS | 1 +
> drivers/media/platform/mediatek/isp/isp_30/Kconfig | 19 +
> .../media/platform/mediatek/isp/isp_30/Makefile | 1 +
> .../platform/mediatek/isp/isp_30/camsv/Makefile | 7 +
> .../platform/mediatek/isp/isp_30/camsv/mtk_camsv.c | 327 +++++++++
> .../platform/mediatek/isp/isp_30/camsv/mtk_camsv.h | 192 ++++++
> .../mediatek/isp/isp_30/camsv/mtk_camsv30_hw.c | 413 ++++++++++++
> .../mediatek/isp/isp_30/camsv/mtk_camsv30_regs.h | 60 ++
> .../mediatek/isp/isp_30/camsv/mtk_camsv_video.c | 742 +++++++++++++++++++++
> 9 files changed, 1762 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5e697e288671..9b866731bf38 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14165,6 +14165,7 @@ M: Andy Hsieh <andy.hsieh@xxxxxxxxxxxx>
> S: Supported
> F: Documentation/devicetree/bindings/media/mediatek,mt8365-camsv.yaml
> F: Documentation/devicetree/bindings/media/mediatek,mt8365-seninf.yaml
> +F: drivers/media/platform/mediatek/isp/isp_30/camsv/*
> F: drivers/media/platform/mediatek/isp/isp_30/seninf/*
>
> MEDIATEK SMI DRIVER
> diff --git a/drivers/media/platform/mediatek/isp/isp_30/Kconfig b/drivers/media/platform/mediatek/isp/isp_30/Kconfig
> index 9791312589fb..5293a061ae0b 100644
> --- a/drivers/media/platform/mediatek/isp/isp_30/Kconfig
> +++ b/drivers/media/platform/mediatek/isp/isp_30/Kconfig
> @@ -14,3 +14,22 @@ config MTK_SENINF30
>
> To compile this driver as a module, choose M here: the
> module will be called mtk-seninf.
> +
> +config MTK_CAMSV30
Alphabetical order please.
> + tristate "MediaTek ISP3.0 CAMSV driver"
> + depends on VIDEO_V4L2_SUBDEV_API
You can replace this with
select MEDIA_CONTROLLER
select VIDEO_V4L2_SUBDEV_API
> + depends on ARCH_MEDIATEK || COMPILE_TEST
> + depends on OF
> + depends on PM
> + select VIDEOBUF2_VMALLOC
> + select VIDEOBUF2_DMA_CONTIG
> + select MTK_SENINF30
> + select PHY_MTK_MIPI_CSI_0_5
Alphabetical order please.
> + default n
> + help
> + This driver provides a path to bypass the SoC ISP so that
> + image data come from the SENINF can go directly into memory
> + without any image processing.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called mtk-camsv30.
> diff --git a/drivers/media/platform/mediatek/isp/isp_30/Makefile b/drivers/media/platform/mediatek/isp/isp_30/Makefile
> index ac3142de4739..a76f440c5358 100644
> --- a/drivers/media/platform/mediatek/isp/isp_30/Makefile
> +++ b/drivers/media/platform/mediatek/isp/isp_30/Makefile
> @@ -1,3 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0
>
> obj-$(CONFIG_MTK_SENINF30) += seninf/
> +obj-$(CONFIG_MTK_CAMSV30) += camsv/
Alphabetical order too.
> diff --git a/drivers/media/platform/mediatek/isp/isp_30/camsv/Makefile b/drivers/media/platform/mediatek/isp/isp_30/camsv/Makefile
> new file mode 100644
> index 000000000000..fffbc6e7cb78
> --- /dev/null
> +++ b/drivers/media/platform/mediatek/isp/isp_30/camsv/Makefile
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +mtk-camsv30-objs += mtk_camsv.o
> +mtk-camsv30-objs += mtk_camsv30_hw.o
> +mtk-camsv30-objs += mtk_camsv_video.o
> +
> +obj-$(CONFIG_MTK_CAMSV30) += mtk-camsv30.o
> diff --git a/drivers/media/platform/mediatek/isp/isp_30/camsv/mtk_camsv.c b/drivers/media/platform/mediatek/isp/isp_30/camsv/mtk_camsv.c
> new file mode 100644
> index 000000000000..9dd3c6a0e09b
> --- /dev/null
> +++ b/drivers/media/platform/mediatek/isp/isp_30/camsv/mtk_camsv.c
> @@ -0,0 +1,327 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2020 BayLibre
> + * Copyright (c) 2022 MediaTek Inc.
> + */
> +
> +#include <media/v4l2-async.h>
> +#include <media/v4l2-device.h>
> +
> +#include "mtk_camsv.h"
> +
> +static inline struct mtk_cam_dev *to_mtk_cam_dev(struct v4l2_subdev *sd)
> +{
> + return container_of(sd, struct mtk_cam_dev, subdev);
> +}
> +
> +static const u32 mtk_cam_mbus_formats[] = {
> + MEDIA_BUS_FMT_SBGGR8_1X8,
> + MEDIA_BUS_FMT_SGBRG8_1X8,
> + MEDIA_BUS_FMT_SGRBG8_1X8,
> + MEDIA_BUS_FMT_SRGGB8_1X8,
> + MEDIA_BUS_FMT_SBGGR10_1X10,
> + MEDIA_BUS_FMT_SGBRG10_1X10,
> + MEDIA_BUS_FMT_SGRBG10_1X10,
> + MEDIA_BUS_FMT_SRGGB10_1X10,
> + MEDIA_BUS_FMT_SBGGR12_1X12,
> + MEDIA_BUS_FMT_SGBRG12_1X12,
> + MEDIA_BUS_FMT_SGRBG12_1X12,
> + MEDIA_BUS_FMT_SRGGB12_1X12,
> + MEDIA_BUS_FMT_UYVY8_1X16,
> + MEDIA_BUS_FMT_VYUY8_1X16,
> + MEDIA_BUS_FMT_YUYV8_1X16,
> + MEDIA_BUS_FMT_YVYU8_1X16,
> +};
> +
> +/* -----------------------------------------------------------------------------
> + * V4L2 Subdev Operations
> + */
> +
> +static int mtk_cam_cio_stream_on(struct mtk_cam_dev *cam)
> +{
> + struct device *dev = cam->dev;
> + struct v4l2_subdev *seninf;
> + int ret;
> +
> + if (!cam->seninf) {
> + cam->seninf = media_pad_remote_pad_first(&cam->subdev_pads[MTK_CAM_CIO_PAD_SENINF]);
> + if (!cam->seninf) {
> + dev_err(dev, "%s: No SENINF connected\n", __func__);
> + return -ENOLINK;
> + }
> + }
> +
> + seninf = media_entity_to_v4l2_subdev(cam->seninf->entity);
> +
> + /* Seninf must stream on first */
> + ret = v4l2_subdev_call(seninf, pad, enable_streams, NULL, cam->seninf->index, 0);
Use v4l2_subdev_enable_streams() and v4l2_subdev_disable_streams().
> + if (ret) {
> + dev_err(dev, "failed to stream on %s:%d\n",
> + seninf->entity.name, ret);
> + return ret;
> + }
> +
> + cam->streaming = true;
> +
> + return 0;
> +}
> +
> +static int mtk_cam_cio_stream_off(struct mtk_cam_dev *cam)
> +{
> + int ret;
> +
> + if (cam->seninf) {
> + struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(cam->seninf->entity);
> +
> + ret = v4l2_subdev_call(sd, pad, disable_streams, NULL,
> + cam->seninf->index, 0);
> + if (ret) {
> + dev_err(cam->dev, "failed to stream off %s:%d\n",
> + sd->entity.name, ret);
> + return ret;
> + }
> + }
> +
> + cam->streaming = false;
> +
> + return 0;
> +}
> +
> +static int mtk_cam_sd_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> + struct mtk_cam_dev *cam = to_mtk_cam_dev(sd);
> +
> + if (enable) {
> + /* Align vb2_core_streamon design */
> + if (cam->streaming) {
> + dev_warn(cam->dev, "already streaming on\n");
Can this happen ?
> + return 0;
> + }
> + return mtk_cam_cio_stream_on(cam);
> + }
> +
> + if (!cam->streaming) {
> + dev_warn(cam->dev, "already streaming off\n");
And this.
> + return 0;
> + }
> +
> + return mtk_cam_cio_stream_off(cam);
> +}
> +
> +static struct v4l2_mbus_framefmt *
> +mtk_cam_get_pad_format(struct mtk_cam_dev *cam,
> + struct v4l2_subdev_state *sd_state,
> + unsigned int pad, u32 which)
> +{
> + switch (which) {
> + case V4L2_SUBDEV_FORMAT_TRY:
> + return v4l2_subdev_state_get_format(sd_state, pad);
> + case V4L2_SUBDEV_FORMAT_ACTIVE:
> + return &cam->formats[pad];
> + default:
> + return NULL;
> + }
> +}
> +
> +static int mtk_cam_init_state(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state)
> +{
> + static const struct v4l2_mbus_framefmt def_format = {
> + .code = MEDIA_BUS_FMT_SGRBG10_1X10,
> + .width = IMG_MAX_WIDTH,
> + .height = IMG_MAX_HEIGHT,
> + .field = V4L2_FIELD_NONE,
> + .colorspace = V4L2_COLORSPACE_SRGB,
> + .xfer_func = V4L2_XFER_FUNC_DEFAULT,
> + .ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT,
> + .quantization = V4L2_QUANTIZATION_DEFAULT,
> + };
> + struct mtk_cam_dev *cam = to_mtk_cam_dev(sd);
> + u32 which = sd_state ? V4L2_SUBDEV_FORMAT_TRY
> + : V4L2_SUBDEV_FORMAT_ACTIVE;
> + struct v4l2_mbus_framefmt *format;
> + unsigned int i;
> +
> + for (i = 0; i < sd->entity.num_pads; i++) {
> + format = mtk_cam_get_pad_format(cam, sd_state, i, which);
> + *format = def_format;
> + }
> +
> + return 0;
> +}
> +
> +static int mtk_cam_enum_mbus_code(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_mbus_code_enum *code)
> +{
> + if (code->index >= ARRAY_SIZE(mtk_cam_mbus_formats))
> + return -EINVAL;
> +
> + code->code = mtk_cam_mbus_formats[code->index];
> +
> + return 0;
> +}
> +
> +static int mtk_cam_get_fmt(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct mtk_cam_dev *cam = to_mtk_cam_dev(sd);
> +
> + fmt->format = *mtk_cam_get_pad_format(cam, sd_state, fmt->pad,
> + fmt->which);
> +
> + return 0;
> +}
> +
> +static int mtk_cam_set_fmt(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct mtk_cam_dev *cam = to_mtk_cam_dev(sd);
> + struct v4l2_mbus_framefmt *format;
> + unsigned int i;
> +
> + /*
> + * We only support pass-through mode, the format on source pads can't
> + * be modified.
> + */
> + if (fmt->pad != MTK_CAM_CIO_PAD_SENINF)
> + return mtk_cam_get_fmt(sd, sd_state, fmt);
> +
> + for (i = 0; i < ARRAY_SIZE(mtk_cam_mbus_formats); ++i) {
> + if (mtk_cam_mbus_formats[i] == fmt->format.code)
> + break;
> + }
> +
> + if (i == ARRAY_SIZE(mtk_cam_mbus_formats))
> + fmt->format.code = mtk_cam_mbus_formats[0];
> +
> + format = mtk_cam_get_pad_format(cam, sd_state, fmt->pad, fmt->which);
> + format->width = fmt->format.width;
> + format->height = fmt->format.height;
> + format->code = fmt->format.code;
> +
> + fmt->format = *format;
> +
> + /* Propagate the format to the source pad. */
> + format = mtk_cam_get_pad_format(cam, sd_state, MTK_CAM_CIO_PAD_VIDEO,
> + fmt->which);
> + format->width = fmt->format.width;
> + format->height = fmt->format.height;
> + format->code = fmt->format.code;
> +
> + return 0;
> +}
> +
> +static int mtk_cam_subdev_registered(struct v4l2_subdev *sd)
> +{
> + struct mtk_cam_dev *cam = to_mtk_cam_dev(sd);
> +
> + /* Create the video device and link. */
> + return mtk_cam_video_register(cam);
> +}
> +
> +static const struct v4l2_subdev_video_ops mtk_cam_subdev_video_ops = {
> + .s_stream = mtk_cam_sd_s_stream,
> +};
> +
> +static const struct v4l2_subdev_pad_ops mtk_cam_subdev_pad_ops = {
> + .enum_mbus_code = mtk_cam_enum_mbus_code,
> + .set_fmt = mtk_cam_set_fmt,
> + .get_fmt = mtk_cam_get_fmt,
> + .link_validate = v4l2_subdev_link_validate_default,
> +};
> +
> +static const struct v4l2_subdev_ops mtk_cam_subdev_ops = {
> + .video = &mtk_cam_subdev_video_ops,
> + .pad = &mtk_cam_subdev_pad_ops,
> +};
> +
> +static const struct v4l2_subdev_internal_ops mtk_cam_internal_ops = {
> + .init_state = mtk_cam_init_state,
> + .registered = mtk_cam_subdev_registered,
> +};
> +
> +/* -----------------------------------------------------------------------------
> + * Media Entity Operations
> + */
> +
> +static const struct media_entity_operations mtk_cam_media_entity_ops = {
> + .link_validate = v4l2_subdev_link_validate,
> + .get_fwnode_pad = v4l2_subdev_get_fwnode_pad_1_to_1,
> +};
> +
> +/* -----------------------------------------------------------------------------
> + * Init & Cleanup
> + */
> +
> +static int mtk_cam_v4l2_register(struct mtk_cam_dev *cam)
> +{
> + struct device *dev = cam->dev;
> + int ret;
> +
> + cam->subdev_pads[MTK_CAM_CIO_PAD_SENINF].flags = MEDIA_PAD_FL_SINK;
> + cam->subdev_pads[MTK_CAM_CIO_PAD_VIDEO].flags = MEDIA_PAD_FL_SOURCE;
> +
> + /* Initialize subdev pads */
> + ret = media_entity_pads_init(&cam->subdev.entity,
> + ARRAY_SIZE(cam->subdev_pads),
> + cam->subdev_pads);
> + if (ret) {
> + dev_err(dev, "failed to initialize media pads:%d\n", ret);
> + return ret;
> + }
> +
> + /* Initialize subdev */
> + v4l2_subdev_init(&cam->subdev, &mtk_cam_subdev_ops);
> +
> + cam->subdev.dev = dev;
> + cam->subdev.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
> + cam->subdev.entity.ops = &mtk_cam_media_entity_ops;
> + cam->subdev.internal_ops = &mtk_cam_internal_ops;
> + cam->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> + strscpy(cam->subdev.name, dev_name(dev), sizeof(cam->subdev.name));
> + v4l2_set_subdevdata(&cam->subdev, cam);
> +
> + mtk_cam_init_state(&cam->subdev, NULL);
Please use v4l2_subdev_init_finalize() instead of doing this, and switch
to storing the active state in the active state managed by the
v4l2_subdev core, like you've done for seninf. Please also implement the
.enable_streams() and .disable_streams() operations instead of
.s_stream() if possible.
> +
> + ret = v4l2_async_register_subdev(&cam->subdev);
> + if (ret) {
> + dev_err(dev, "failed to initialize subdev:%d\n", ret);
> + media_entity_cleanup(&cam->subdev.entity);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void mtk_cam_v4l2_unregister(struct mtk_cam_dev *cam)
> +{
> + mtk_cam_video_unregister(&cam->vdev);
> +
> + media_entity_cleanup(&cam->subdev.entity);
> + v4l2_async_unregister_subdev(&cam->subdev);
> +}
> +
> +int mtk_cam_dev_init(struct mtk_cam_dev *cam_dev)
> +{
> + int ret;
> +
> + mutex_init(&cam_dev->op_lock);
> +
> + /* v4l2 sub-device registration */
> + ret = mtk_cam_v4l2_register(cam_dev);
> + if (ret) {
> + mutex_destroy(&cam_dev->op_lock);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +void mtk_cam_dev_cleanup(struct mtk_cam_dev *cam)
> +{
> + mtk_cam_v4l2_unregister(cam);
> + mutex_destroy(&cam->op_lock);
> +}
> diff --git a/drivers/media/platform/mediatek/isp/isp_30/camsv/mtk_camsv.h b/drivers/media/platform/mediatek/isp/isp_30/camsv/mtk_camsv.h
> new file mode 100644
> index 000000000000..9de53230bf2d
> --- /dev/null
> +++ b/drivers/media/platform/mediatek/isp/isp_30/camsv/mtk_camsv.h
> @@ -0,0 +1,192 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2020 BayLibre
> + * Copyright (c) 2022 MediaTek Inc.
> + */
> +
> +#ifndef __MTK_CAMSV_H__
> +#define __MTK_CAMSV_H__
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/kernel.h>
> +#include <linux/of_graph.h>
This can be dropped.
> +#include <linux/pm_runtime.h>
> +#include <linux/videodev2.h>
> +#include <media/media-entity.h>
> +#include <media/v4l2-subdev.h>
> +#include <media/videobuf2-core.h>
> +#include <media/videobuf2-dma-contig.h>
> +#include <media/videobuf2-v4l2.h>
> +#include <soc/mediatek/smi.h>
> +
> +#define IMG_MAX_WIDTH 5376U
> +#define IMG_MAX_HEIGHT 4032U
> +#define IMG_MIN_WIDTH 80U
> +#define IMG_MIN_HEIGHT 60U
> +
> +#define MTK_CAM_CIO_PAD_SENINF 0U
> +#define MTK_CAM_CIO_PAD_VIDEO 1U
> +#define MTK_CAM_CIO_NUM_PADS 2U
> +
> +struct mtk_cam_format_info {
> + u32 code;
> + u32 fourcc;
> + u8 bpp;
> +};
> +
> +struct mtk_cam_dev_buffer {
> + struct vb2_v4l2_buffer v4l2_buf;
> + struct list_head list;
> + dma_addr_t daddr;
> + void *vaddr;
> +};
> +
> +struct mtk_cam_sparams {
> + u32 w_factor;
> + u32 module_en_pak;
> + u32 fmt_sel;
> + u32 pak;
> + u32 imgo_stride;
> +};
> +
> +/**
> + * struct mtk_cam_vdev_desc - MTK camera device descriptor
> + * @name: name of the node
> + * @cap: supported V4L2 capabilities
> + * @buf_type: supported V4L2 buffer type
> + * @link_flags: default media link flags
> + * @def_width: the default format width
> + * @def_height: the default format height
> + * @num_fmts: the number of supported node formats
> + * @ioctl_ops: mapped to v4l2_ioctl_ops
> + * @fmts: supported format
> + * @frmsizes: supported V4L2 frame size number
> + */
> +struct mtk_cam_vdev_desc {
> + const char *name;
> + u32 cap;
> + u32 buf_type;
> + u32 link_flags;
> + u32 def_width;
> + u32 def_height;
> + u8 num_fmts;
> + const struct v4l2_ioctl_ops *ioctl_ops;
> + const u32 *fmts;
> + const struct v4l2_frmsizeenum *frmsizes;
> +};
> +
> +/**
> + * struct mtk_cam_video_device - MediaTek video device structure
> + * @desc: The node description of video device
> + * @vdev_pad: The media pad graph object of video device
> + * @vdev: The video device instance
> + * @vbq: A videobuf queue of video device
> + * @vdev_lock: Serializes vb2 queue and video device operations
> + * @format: The V4L2 format of video device
> + * @fmtinfo: Information about the current format
> + */
> +struct mtk_cam_video_device {
> + const struct mtk_cam_vdev_desc *desc;
> +
> + struct media_pad vdev_pad;
> + struct video_device vdev;
> + struct vb2_queue vbq;
> +
> + /* Serializes vb2 queue and video device operations */
> + struct mutex vdev_lock;
> +
> + struct v4l2_pix_format_mplane format;
> + const struct mtk_cam_format_info *fmtinfo;
> +};
> +
> +/**
> + * struct mtk_cam_dev - MediaTek camera device structure.
> + * @dev: Pointer to device.
> + * @regs: Base address of CAMSV.
> + * @regs_img0: Base address of CAMSV IMG0.
> + * @regs_tg: Base address of CAMSV TG.
> + * @num_clks: Number of clocks.
> + * @clks: The clocks.
> + * @irq: Irq fired when buffer is ready.
> + * @conf: soc specific driver data.
> + * @pipeline: Media pipeline information.
> + * @subdev: The V4L2 sub-device instance.
> + * @subdev_pads: Media pads of this sub-device.
> + * @formats: Media bus format for all pads.
> + * @vdev: The video device node.
> + * @seninf: Pointer to the seninf pad.
> + * @streaming: Indicate the overall streaming status is on or off.
> + * @stream_count: Number of streaming video nodes.
> + * @sequence: Buffer sequence number.
> + * @op_lock: Serializes driver's VB2 callback operations.
Name it vb2_lock to make it clearer. <reads the rest of the driver>
Except that's not what the lock is used for... You seem to use this to
protect against .s_stream() being called concurrently from different
camsv instances, is that right ? If so this won't help, as different
instances will have different locks. Locking needs to be revisited.
> + * @buf_list_lock: Protects the buffer list.
> + * @buf_list: List head for the buffer list.
> + * @hw_functions: Hardware specific functions.
> + * @dummy: Dummy buffer used when user buffer is not available.
> + * @dummy_size : Size of the dummy buffer.
> + * @is_dummy_used: True if dummy buffer is currently used.
> + */
> +struct mtk_cam_dev {
> + struct device *dev;
> + void __iomem *regs;
> + void __iomem *regs_img0;
> + void __iomem *regs_tg;
> +
> + unsigned int num_clks;
> + struct clk_bulk_data *clks;
> + unsigned int irq;
> + const struct mtk_cam_conf *conf;
> +
> + struct media_pipeline pipeline;
> + struct v4l2_subdev subdev;
> + struct media_pad subdev_pads[MTK_CAM_CIO_NUM_PADS];
> + struct v4l2_mbus_framefmt formats[MTK_CAM_CIO_NUM_PADS];
> + struct mtk_cam_video_device vdev;
> + struct media_pad *seninf;
> + unsigned int streaming;
> + unsigned int stream_count;
> + unsigned int sequence;
> +
> + struct mutex op_lock;
> + spinlock_t buf_list_lock;
> +
> + struct list_head buf_list;
> +
> + const struct mtk_cam_hw_functions *hw_functions;
> +
> + struct mtk_cam_dev_buffer dummy;
> + unsigned int dummy_size;
> + bool is_dummy_used;
> +};
> +
> +/**
> + * struct mtk_cam_conf - MediaTek camera configuration structure
> + * @tg_sen_mode: TG sensor mode
> + * @module_en: module enable
> + * @imgo_con: dma control register
> + * @imgo_con2: dma control register 2
> + */
> +struct mtk_cam_conf {
> + u32 tg_sen_mode;
> + u32 module_en;
> + u32 imgo_con;
> + u32 imgo_con2;
> +};
> +
> +struct mtk_cam_hw_functions {
> + void (*mtk_cam_setup)(struct mtk_cam_dev *cam_dev, u32 width,
> + u32 height, u32 bpl, u32 mbus_fmt);
> + void (*mtk_cam_update_buffers_add)(struct mtk_cam_dev *cam_dev,
> + struct mtk_cam_dev_buffer *buf);
> + void (*mtk_cam_cmos_vf_hw_enable)(struct mtk_cam_dev *cam_dev);
> + void (*mtk_cam_cmos_vf_hw_disable)(struct mtk_cam_dev *cam_dev);
> +};
> +
> +int mtk_cam_dev_init(struct mtk_cam_dev *cam_dev);
> +void mtk_cam_dev_cleanup(struct mtk_cam_dev *cam_dev);
> +int mtk_cam_video_register(struct mtk_cam_dev *cam_dev);
> +void mtk_cam_video_unregister(struct mtk_cam_video_device *vdev);
> +
> +#endif /* __MTK_CAMSV_H__ */
> diff --git a/drivers/media/platform/mediatek/isp/isp_30/camsv/mtk_camsv30_hw.c b/drivers/media/platform/mediatek/isp/isp_30/camsv/mtk_camsv30_hw.c
> new file mode 100644
> index 000000000000..6ce256743f56
> --- /dev/null
> +++ b/drivers/media/platform/mediatek/isp/isp_30/camsv/mtk_camsv30_hw.c
> @@ -0,0 +1,413 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2020 BayLibre
> + * Copyright (c) 2022 MediaTek Inc.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/interrupt.h>
> +#include <linux/iommu.h>
> +#include <linux/iopoll.h>
> +#include <linux/ktime.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +
> +#include "mtk_camsv.h"
> +#include "mtk_camsv30_regs.h"
> +
> +#define MTK_CAMSV30_AUTOSUSPEND_DELAY_MS 100
> +
> +static const struct mtk_cam_conf camsv30_conf = {
> + .tg_sen_mode = 0x00010002U, /* TIME_STP_EN = 1. DBL_DATA_BUS = 1 */
> + .module_en = 0x40000001U, /* enable double buffer and TG */
> + .imgo_con = 0x80000080U, /* DMA FIFO depth and burst */
> + .imgo_con2 = 0x00020002U, /* DMA priority */
> +};
> +
> +static void fmt_to_sparams(u32 mbus_fmt, struct mtk_cam_sparams *sparams)
> +{
> + switch (mbus_fmt) {
> + /*
> + * SBGGR values coming from isp5.0 configuration.
> + * not tested on isp2.0
> + */
> + case MEDIA_BUS_FMT_SBGGR12_1X12:
> + case MEDIA_BUS_FMT_SGBRG12_1X12:
> + case MEDIA_BUS_FMT_SGRBG12_1X12:
> + case MEDIA_BUS_FMT_SRGGB12_1X12:
> + sparams->w_factor = 1;
> + sparams->module_en_pak = 0x4;
> + sparams->fmt_sel = 0x2;
> + sparams->pak = 0x5;
> + sparams->imgo_stride = 0x000B0000;
Lowercase hex constants.
Please define macros for all the magic values, here and through the
driver.
> + break;
> + case MEDIA_BUS_FMT_SBGGR10_1X10:
> + case MEDIA_BUS_FMT_SGBRG10_1X10:
> + case MEDIA_BUS_FMT_SGRBG10_1X10:
> + case MEDIA_BUS_FMT_SRGGB10_1X10:
> + sparams->w_factor = 1;
> + sparams->module_en_pak = 0x4;
> + sparams->fmt_sel = 0x1;
> + sparams->pak = 0x6;
> + sparams->imgo_stride = 0x000B0000;
> + break;
> + case MEDIA_BUS_FMT_SBGGR8_1X8:
> + case MEDIA_BUS_FMT_SGBRG8_1X8:
> + case MEDIA_BUS_FMT_SGRBG8_1X8:
> + case MEDIA_BUS_FMT_SRGGB8_1X8:
> + sparams->w_factor = 1;
> + sparams->module_en_pak = 0x4;
> + sparams->fmt_sel = 0x0;
> + sparams->pak = 0x7;
> + sparams->imgo_stride = 0x000B0000;
> + break;
> + case MEDIA_BUS_FMT_UYVY8_1X16:
> + case MEDIA_BUS_FMT_VYUY8_1X16:
> + case MEDIA_BUS_FMT_YUYV8_1X16:
> + case MEDIA_BUS_FMT_YVYU8_1X16:
> + sparams->w_factor = 2;
> + sparams->module_en_pak = 0x8;
> + sparams->fmt_sel = 0x1000003;
> + sparams->pak = 0x0;
> + sparams->imgo_stride = 0x00090000;
> + break;
> + default:
> + break;
> + }
> +}
> +
> +static void mtk_camsv30_update_buffers_add(struct mtk_cam_dev *cam_dev,
> + struct mtk_cam_dev_buffer *buf)
> +{
> + writel(buf->daddr, cam_dev->regs_img0 + CAMSV_IMGO_SV_BASE_ADDR);
> +
> + writel(0x1U, cam_dev->regs + CAMSV_IMGO_FBC);
> +}
> +
> +static void mtk_camsv30_cmos_vf_hw_enable(struct mtk_cam_dev *cam_dev)
> +{
> + u32 clk_en = CAMSV_TG_DP_CLK_EN | CAMSV_DMA_DP_CLK_EN | CAMSV_PAK_DP_CLK_EN;
> +
> + writel(clk_en, cam_dev->regs + CAMSV_CLK_EN);
> + writel(readl(cam_dev->regs_tg + CAMSV_TG_VF_CON) | CAMSV_TG_VF_CON_VFDATA_EN,
> + cam_dev->regs_tg + CAMSV_TG_VF_CON);
> +}
> +
> +static void mtk_camsv30_cmos_vf_hw_disable(struct mtk_cam_dev *cam_dev)
> +{
> + writel(readl(cam_dev->regs_tg + CAMSV_TG_SEN_MODE) & ~CAMSV_TG_SEN_MODE_CMOS_EN,
> + cam_dev->regs_tg + CAMSV_TG_SEN_MODE);
> + writel(readl(cam_dev->regs_tg + CAMSV_TG_VF_CON) & ~CAMSV_TG_VF_CON_VFDATA_EN,
> + cam_dev->regs_tg + CAMSV_TG_VF_CON);
> +}
> +
> +static void mtk_camsv30_setup(struct mtk_cam_dev *cam_dev, u32 w, u32 h,
> + u32 bpl, u32 mbus_fmt)
> +{
> + const struct mtk_cam_conf *conf = cam_dev->conf;
> + u32 int_en = INT_ST_MASK_CAMSV;
> + u32 tmp;
> + struct mtk_cam_sparams sparams;
> +
> + fmt_to_sparams(mbus_fmt, &sparams);
> +
> + if (pm_runtime_resume_and_get(cam_dev->dev) < 0) {
> + dev_err(cam_dev->dev, "failed to get pm_runtime\n");
> + return;
> + }
> +
> + writel(conf->tg_sen_mode, cam_dev->regs_tg + CAMSV_TG_SEN_MODE);
> +
> + writel((w * sparams.w_factor) << 16U, cam_dev->regs_tg + CAMSV_TG_SEN_GRAB_PXL);
> +
> + writel(h << 16U, cam_dev->regs_tg + CAMSV_TG_SEN_GRAB_LIN);
> +
> + /* YUV_U2S_DIS: disable YUV sensor unsigned to signed */
> + writel(0x1000U, cam_dev->regs_tg + CAMSV_TG_PATH_CFG);
> +
> + /* Reset cam */
> + writel(CAMSV_SW_RST, cam_dev->regs + CAMSV_SW_CTL);
> + writel(0x0U, cam_dev->regs + CAMSV_SW_CTL);
> + writel(CAMSV_IMGO_RST_TRIG, cam_dev->regs + CAMSV_SW_CTL);
> +
> + readl_poll_timeout_atomic(cam_dev->regs + CAMSV_SW_CTL, tmp,
> + (tmp == (CAMSV_IMGO_RST_TRIG | CAMSV_IMGO_RST_ST)), 10, 200);
> +
> + writel(0x0U, cam_dev->regs + CAMSV_SW_CTL);
> +
> + writel(int_en, cam_dev->regs + CAMSV_INT_EN);
> +
> + writel(conf->module_en | sparams.module_en_pak,
> + cam_dev->regs + CAMSV_MODULE_EN);
> + writel(sparams.fmt_sel, cam_dev->regs + CAMSV_FMT_SEL);
> + writel(sparams.pak, cam_dev->regs + CAMSV_PAK);
> +
> + writel(bpl - 1U, cam_dev->regs_img0 + CAMSV_IMGO_SV_XSIZE);
> + writel(h - 1U, cam_dev->regs_img0 + CAMSV_IMGO_SV_YSIZE);
> +
> + writel(sparams.imgo_stride | bpl, cam_dev->regs_img0 + CAMSV_IMGO_SV_STRIDE);
> +
> + writel(conf->imgo_con, cam_dev->regs_img0 + CAMSV_IMGO_SV_CON);
> + writel(conf->imgo_con2, cam_dev->regs_img0 + CAMSV_IMGO_SV_CON2);
> +
> + /* CMOS_EN first */
> + writel(readl(cam_dev->regs_tg + CAMSV_TG_SEN_MODE) | CAMSV_TG_SEN_MODE_CMOS_EN,
> + cam_dev->regs_tg + CAMSV_TG_SEN_MODE);
> +
> + /* finally, CAMSV_MODULE_EN : IMGO_EN */
> + writel(readl(cam_dev->regs + CAMSV_MODULE_EN) | CAMSV_MODULE_EN_IMGO_EN,
> + cam_dev->regs + CAMSV_MODULE_EN);
> +
> + pm_runtime_put_autosuspend(cam_dev->dev);
> +}
> +
> +static irqreturn_t isp_irq_camsv30(int irq, void *data)
> +{
> + struct mtk_cam_dev *cam_dev = (struct mtk_cam_dev *)data;
> + struct mtk_cam_dev_buffer *buf;
> + unsigned int irq_status;
> +
> + spin_lock(&cam_dev->buf_list_lock);
> +
> + irq_status = readl(cam_dev->regs + CAMSV_INT_STATUS);
> +
> + if (irq_status & INT_ST_MASK_CAMSV_ERR)
> + dev_err(cam_dev->dev, "irq error 0x%lx\n", irq_status & INT_ST_MASK_CAMSV_ERR);
> +
> + /* De-queue frame */
> + if (irq_status & CAMSV_IRQ_PASS1_DON) {
> + cam_dev->sequence++;
> +
> + if (!cam_dev->is_dummy_used) {
> + buf = list_first_entry_or_null(&cam_dev->buf_list,
> + struct mtk_cam_dev_buffer,
> + list);
> + if (buf) {
> + buf->v4l2_buf.sequence = cam_dev->sequence;
> + buf->v4l2_buf.vb2_buf.timestamp = ktime_get_ns();
> + vb2_buffer_done(&buf->v4l2_buf.vb2_buf,
> + VB2_BUF_STATE_DONE);
> + list_del(&buf->list);
> + }
> + }
> +
> + if (list_empty(&cam_dev->buf_list)) {
> + mtk_camsv30_update_buffers_add(cam_dev, &cam_dev->dummy);
> + cam_dev->is_dummy_used = true;
> + } else {
> + buf = list_first_entry_or_null(&cam_dev->buf_list,
> + struct mtk_cam_dev_buffer,
> + list);
> + mtk_camsv30_update_buffers_add(cam_dev, buf);
> + cam_dev->is_dummy_used = false;
> + }
> + }
> +
> + spin_unlock(&cam_dev->buf_list_lock);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int mtk_camsv30_runtime_suspend(struct device *dev)
> +{
> + struct mtk_cam_dev *cam_dev = dev_get_drvdata(dev);
> + struct vb2_queue *vbq = &cam_dev->vdev.vbq;
> +
> + if (vb2_is_streaming(vbq)) {
> + mutex_lock(&cam_dev->op_lock);
> + v4l2_subdev_call(&cam_dev->subdev, video, s_stream, 0);
Use the v4l2_subdev_disable_streams() helper.
> + mutex_unlock(&cam_dev->op_lock);
> + }
> +
> + clk_bulk_disable_unprepare(cam_dev->num_clks, cam_dev->clks);
> +
> + return 0;
> +}
> +
> +static int mtk_camsv30_runtime_resume(struct device *dev)
> +{
> + struct mtk_cam_dev *cam_dev = dev_get_drvdata(dev);
> + struct mtk_cam_video_device *vdev = &cam_dev->vdev;
> + const struct v4l2_pix_format_mplane *fmt = &vdev->format;
> + struct vb2_queue *vbq = &vdev->vbq;
> + struct mtk_cam_dev_buffer *buf, *buf_prev;
> + int ret;
> + unsigned long flags = 0;
> +
> + ret = clk_bulk_prepare_enable(cam_dev->num_clks, cam_dev->clks);
> + if (ret) {
> + dev_err(dev, "failed to enable clock:%d\n", ret);
> + return ret;
> + }
> +
> + if (vb2_is_streaming(vbq)) {
> +
> + mtk_camsv30_setup(cam_dev, fmt->width, fmt->height,
> + fmt->plane_fmt[0].bytesperline, vdev->fmtinfo->code);
> +
> + spin_lock_irqsave(&cam_dev->buf_list_lock, flags);
> + buf = list_first_entry_or_null(&cam_dev->buf_list,
> + struct mtk_cam_dev_buffer,
> + list);
> + if (buf) {
> + mtk_camsv30_update_buffers_add(cam_dev, buf);
> + cam_dev->is_dummy_used = false;
> + } else {
> + mtk_camsv30_update_buffers_add(cam_dev, &cam_dev->dummy);
> + cam_dev->is_dummy_used = true;
> + }
> +
> + mtk_camsv30_cmos_vf_hw_enable(cam_dev);
> +
> + spin_unlock_irqrestore(&cam_dev->buf_list_lock, flags);
> +
> + /* Stream on the sub-device */
> + mutex_lock(&cam_dev->op_lock);
> + ret = v4l2_subdev_call(&cam_dev->subdev, video, s_stream, 1);
And v4l2_subdev_enable_streams() here. Same in
mtk_cam_vb2_start_streaming() and mtk_cam_vb2_stop_streaming()
> +
> + if (ret) {
> + cam_dev->stream_count--;
> + if (cam_dev->stream_count == 0)
> + media_pipeline_stop(vdev->vdev.entity.pads);
> + }
> + mutex_unlock(&cam_dev->op_lock);
> +
> + if (ret)
> + goto fail_no_stream;
> + }
> +
> + return 0;
> +
> +fail_no_stream:
> + spin_lock_irqsave(&cam_dev->buf_list_lock, flags);
> + list_for_each_entry_safe(buf, buf_prev, &cam_dev->buf_list, list) {
> + buf->daddr = 0ULL;
> + list_del(&buf->list);
> + vb2_buffer_done(&buf->v4l2_buf.vb2_buf, VB2_BUF_STATE_ERROR);
> + }
> + spin_unlock_irqrestore(&cam_dev->buf_list_lock, flags);
> + return ret;
> +}
> +
> +static const struct mtk_cam_hw_functions mtk_camsv30_hw_functions = {
> + .mtk_cam_setup = mtk_camsv30_setup,
> + .mtk_cam_update_buffers_add = mtk_camsv30_update_buffers_add,
> + .mtk_cam_cmos_vf_hw_enable = mtk_camsv30_cmos_vf_hw_enable,
> + .mtk_cam_cmos_vf_hw_disable = mtk_camsv30_cmos_vf_hw_disable,
> +};
> +
> +static int mtk_camsv30_probe(struct platform_device *pdev)
> +{
> + static const char * const clk_names[] = { "cam", "camtg", "camsv"};
> +
> + struct mtk_cam_dev *cam_dev;
> + struct device *dev = &pdev->dev;
> + int ret;
> + int i;
i is never negative, make it an unsigned int.
> +
> + if (!iommu_present(&platform_bus_type))
> + return -EPROBE_DEFER;
> +
> + cam_dev = devm_kzalloc(dev, sizeof(*cam_dev), GFP_KERNEL);
> + if (!cam_dev)
> + return -ENOMEM;
> +
> + cam_dev->conf = of_device_get_match_data(dev);
device_get_match_data()
and replace
#include <linux/of_platform.h>
above with linux/property.h.
> + if (!cam_dev->conf)
> + return -ENODEV;
> +
> + cam_dev->dev = dev;
> + dev_set_drvdata(dev, cam_dev);
> +
> + cam_dev->regs = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(cam_dev->regs))
> + return dev_err_probe(dev, PTR_ERR(cam_dev->regs),
> + "failed to map register base\n");
Wrong alignment
return dev_err_probe(dev, PTR_ERR(cam_dev->regs),
"failed to map register base\n");
Same in other places.
> +
> +
> + cam_dev->regs_img0 = devm_platform_ioremap_resource(pdev, 1);
> +
Drop the blank line.
> + if (IS_ERR(cam_dev->regs_img0))
> + return dev_err_probe(dev, PTR_ERR(cam_dev->regs_img0),
> + "failed to map img0 register base\n");
> +
> +
> + cam_dev->regs_tg = devm_platform_ioremap_resource(pdev, 2);
> + if (IS_ERR(cam_dev->regs_tg))
> + return dev_err_probe(dev, PTR_ERR(cam_dev->regs_tg),
> + "failed to map TG register base\n");
> +
> +
> + cam_dev->num_clks = ARRAY_SIZE(clk_names);
> + cam_dev->clks = devm_kcalloc(dev, cam_dev->num_clks,
> + sizeof(*cam_dev->clks), GFP_KERNEL);
> + if (!cam_dev->clks)
> + return -ENOMEM;
> +
> + for (i = 0; i < cam_dev->num_clks; ++i)
> + cam_dev->clks[i].id = clk_names[i];
> +
> + ret = devm_clk_bulk_get(dev, cam_dev->num_clks, cam_dev->clks);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to get clocks: %i\n", ret);
> +
> +
Extra blank line.
> + cam_dev->irq = platform_get_irq(pdev, 0);
> + ret = devm_request_irq(dev, cam_dev->irq, isp_irq_camsv30, 0, dev_name(dev), cam_dev);
Line wrap.
> + if (ret != 0)
> + return dev_err_probe(dev, -ENODEV, "failed to request irq=%d\n",
> + cam_dev->irq);
> +
> + cam_dev->hw_functions = &mtk_camsv30_hw_functions;
> +
> + spin_lock_init(&cam_dev->buf_list_lock);
> +
> + /* initialise runtime power management */
> + pm_runtime_set_autosuspend_delay(dev, MTK_CAMSV30_AUTOSUSPEND_DELAY_MS);
> + pm_runtime_use_autosuspend(dev);
> + pm_runtime_set_suspended(dev);
> + devm_pm_runtime_enable(dev);
> +
> + /* Initialize the v4l2 common part */
> + return mtk_cam_dev_init(cam_dev);
> +}
> +
> +static void mtk_camsv30_remove(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct mtk_cam_dev *cam_dev = dev_get_drvdata(dev);
> +
> + mtk_cam_dev_cleanup(cam_dev);
> + pm_runtime_put_autosuspend(dev);
> +}
> +
> +static const struct dev_pm_ops mtk_camsv30_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> + SET_RUNTIME_PM_OPS(mtk_camsv30_runtime_suspend,
> + mtk_camsv30_runtime_resume, NULL)
> +};
> +
> +static const struct of_device_id mtk_camsv30_of_ids[] = {
> + { .compatible = "mediatek,mt8365-camsv", .data = &camsv30_conf },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, mtk_camsv30_of_ids);
> +
> +static struct platform_driver mtk_camsv30_driver = {
> + .probe = mtk_camsv30_probe,
> + .remove_new = mtk_camsv30_remove,
You can now use .remove() again.
> + .driver = {
> + .name = "mtk-camsv-isp30",
> + .of_match_table = mtk_camsv30_of_ids,
> + .pm = &mtk_camsv30_pm_ops,
> + }
> +};
> +
> +module_platform_driver(mtk_camsv30_driver);
> +
> +MODULE_DESCRIPTION("MediaTek CAMSV ISP3.0 driver");
> +MODULE_AUTHOR("Florian Sylvestre <fsylvestre@xxxxxxxxxxxx>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/media/platform/mediatek/isp/isp_30/camsv/mtk_camsv30_regs.h b/drivers/media/platform/mediatek/isp/isp_30/camsv/mtk_camsv30_regs.h
> new file mode 100644
> index 000000000000..6d30087270cc
> --- /dev/null
> +++ b/drivers/media/platform/mediatek/isp/isp_30/camsv/mtk_camsv30_regs.h
> @@ -0,0 +1,60 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2022 MediaTek Inc.
> + */
> +
> +#ifndef __MTK_CAMSV30_REGS_H__
> +#define __MTK_CAMSV30_REGS_H__
> +
#include <linux/bit.h>
> +/* CAMSV */
> +#define CAMSV_MODULE_EN 0x0000
> +#define CAMSV_MODULE_EN_IMGO_EN BIT(4)
> +#define CAMSV_FMT_SEL 0x0004
> +#define CAMSV_INT_EN 0x0008
> +#define CAMSV_INT_STATUS 0x000c
> +#define CAMSV_SW_CTL 0x0010
> +#define CAMSV_IMGO_FBC 0x001C
> +#define CAMSV_CLK_EN 0x0020
> +#define CAMSV_PAK 0x003c
> +
> +/* CAMSV_TG */
> +#define CAMSV_TG_SEN_MODE 0x0010
> +#define CAMSV_TG_VF_CON 0x0014
> +#define CAMSV_TG_SEN_GRAB_PXL 0x0018
> +#define CAMSV_TG_SEN_GRAB_LIN 0x001c
> +#define CAMSV_TG_PATH_CFG 0x0020
> +
> +/* CAMSV_IMG0 */
> +#define CAMSV_IMGO_SV_BASE_ADDR 0x0000
> +#define CAMSV_IMGO_SV_XSIZE 0x0008
> +#define CAMSV_IMGO_SV_YSIZE 0x000c
> +#define CAMSV_IMGO_SV_STRIDE 0x0010
> +#define CAMSV_IMGO_SV_CON 0x0014
> +#define CAMSV_IMGO_SV_CON2 0x0018
> +
> +#define CAMSV_TG_SEN_MODE_CMOS_EN BIT(0)
> +#define CAMSV_TG_VF_CON_VFDATA_EN BIT(0)
> +
> +/* CAMSV_CLK_EN bits */
> +#define CAMSV_TG_DP_CLK_EN BIT(0)
> +#define CAMSV_PAK_DP_CLK_EN BIT(2)
> +#define CAMSV_DMA_DP_CLK_EN BIT(15)
> +
> +/* CAMSV_SW_CTL bits */
> +#define CAMSV_IMGO_RST_TRIG BIT(0)
> +#define CAMSV_IMGO_RST_ST BIT(1)
> +#define CAMSV_SW_RST BIT(2)
> +
> +/* IRQ BITS */
> +#define CAMSV_IRQ_TG_ERR BIT(4)
> +#define CAMSV_IRQ_TG_GBERR BIT(5)
> +#define CAMSV_IRQ_PASS1_DON BIT(10)
> +#define CAMSV_IRQ_IMGO_ERR BIT(16)
> +
> +#define INT_ST_MASK_CAMSV \
> + (CAMSV_IRQ_PASS1_DON)
> +
> +#define INT_ST_MASK_CAMSV_ERR \
> + (CAMSV_IRQ_TG_ERR | CAMSV_IRQ_TG_GBERR | CAMSV_IRQ_IMGO_ERR)
> +
> +#endif /* __MTK_CAMSV30_REGS_H__ */
> diff --git a/drivers/media/platform/mediatek/isp/isp_30/camsv/mtk_camsv_video.c b/drivers/media/platform/mediatek/isp/isp_30/camsv/mtk_camsv_video.c
> new file mode 100644
> index 000000000000..967bac446fb3
> --- /dev/null
> +++ b/drivers/media/platform/mediatek/isp/isp_30/camsv/mtk_camsv_video.c
> @@ -0,0 +1,742 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * mtk_camsv_video.c - V4L2 video node support
> + *
> + * Copyright (c) 2020 BayLibre
> + * Copyright (c) 2022 MediaTek Inc.
> + */
> +
> +#include <linux/version.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-event.h>
> +#include <media/v4l2-ioctl.h>
> +#include <media/v4l2-mediabus.h>
> +
> +#include "mtk_camsv.h"
> +
> +static inline struct mtk_cam_video_device *
> +file_to_mtk_cam_video_device(struct file *__file)
> +{
> + return container_of(video_devdata(__file),
> + struct mtk_cam_video_device, vdev);
> +}
> +
> +static inline struct mtk_cam_video_device *
> +vb2_queue_to_mtk_cam_video_device(struct vb2_queue *vq)
> +{
> + return container_of(vq, struct mtk_cam_video_device, vbq);
> +}
> +
> +static inline struct mtk_cam_dev_buffer *
> +to_mtk_cam_dev_buffer(struct vb2_buffer *buf)
> +{
> + return container_of(buf, struct mtk_cam_dev_buffer, v4l2_buf.vb2_buf);
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * Format Information
> + */
> +
> +static const struct mtk_cam_format_info mtk_cam_format_info[] = {
> + {
> + .fourcc = V4L2_PIX_FMT_SBGGR8,
> + .code = MEDIA_BUS_FMT_SBGGR8_1X8,
> + .bpp = 8,
> + }, {
> + .fourcc = V4L2_PIX_FMT_SGBRG8,
> + .code = MEDIA_BUS_FMT_SGBRG8_1X8,
> + .bpp = 8,
> + }, {
> + .fourcc = V4L2_PIX_FMT_SGRBG8,
> + .code = MEDIA_BUS_FMT_SGRBG8_1X8,
> + .bpp = 8,
> + }, {
> + .fourcc = V4L2_PIX_FMT_SRGGB8,
> + .code = MEDIA_BUS_FMT_SRGGB8_1X8,
> + .bpp = 8,
> + }, {
> + .fourcc = V4L2_PIX_FMT_YUYV,
> + .code = MEDIA_BUS_FMT_YUYV8_1X16,
> + .bpp = 16,
> + }, {
> + .fourcc = V4L2_PIX_FMT_YVYU,
> + .code = MEDIA_BUS_FMT_YVYU8_1X16,
> + .bpp = 16,
> + }, {
> + .fourcc = V4L2_PIX_FMT_UYVY,
> + .code = MEDIA_BUS_FMT_UYVY8_1X16,
> + .bpp = 16,
> + }, {
> + .fourcc = V4L2_PIX_FMT_VYUY,
> + .code = MEDIA_BUS_FMT_VYUY8_1X16,
> + .bpp = 16,
> + },
> +};
> +
> +static const struct mtk_cam_format_info *
> +mtk_cam_format_info_by_fourcc(u32 fourcc)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(mtk_cam_format_info); ++i) {
> + const struct mtk_cam_format_info *info =
> + &mtk_cam_format_info[i];
> +
> + if (info->fourcc == fourcc)
> + return info;
> + }
> +
> + return NULL;
> +}
> +
> +static const struct mtk_cam_format_info *
> +mtk_cam_format_info_by_code(u32 code)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(mtk_cam_format_info); ++i) {
> + const struct mtk_cam_format_info *info =
> + &mtk_cam_format_info[i];
> +
> + if (info->code == code)
> + return info;
> + }
> +
> + return NULL;
> +}
> +
> +static bool mtk_cam_dev_find_fmt(const struct mtk_cam_vdev_desc *desc,
> + u32 format)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < desc->num_fmts; i++) {
> + if (desc->fmts[i] == format)
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static void calc_bpl_size_pix_mp(const struct mtk_cam_format_info *fmtinfo,
> + struct v4l2_pix_format_mplane *pix_mp)
> +{
> + unsigned int bpl;
> + unsigned int i;
> +
> + bpl = ALIGN(DIV_ROUND_UP(pix_mp->width * fmtinfo->bpp, 8), 2);
> +
> + for (i = 0; i < pix_mp->num_planes; ++i) {
> + pix_mp->plane_fmt[i].bytesperline = bpl;
> + pix_mp->plane_fmt[i].sizeimage = bpl * pix_mp->height;
Can we make at least the stride configurable by the application ?
> + }
> +}
> +
> +static void mtk_cam_dev_load_default_fmt(struct mtk_cam_dev *cam)
> +{
> + struct mtk_cam_video_device *vdev = &cam->vdev;
> + struct v4l2_pix_format_mplane *fmt = &vdev->format;
> +
> + fmt->num_planes = 1;
> + fmt->pixelformat = vdev->desc->fmts[0];
> + fmt->width = vdev->desc->def_width;
> + fmt->height = vdev->desc->def_height;
> +
> + fmt->colorspace = V4L2_COLORSPACE_SRGB;
> + fmt->field = V4L2_FIELD_NONE;
> + fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> + fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
> + fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
> +
> + vdev->fmtinfo = mtk_cam_format_info_by_fourcc(fmt->pixelformat);
> +
> + calc_bpl_size_pix_mp(vdev->fmtinfo, fmt);
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * VB2 Queue Operations
> + */
> +
> +static int mtk_cam_vb2_queue_setup(struct vb2_queue *vq,
> + unsigned int *num_buffers,
> + unsigned int *num_planes,
> + unsigned int sizes[],
> + struct device *alloc_devs[])
> +{
> + struct mtk_cam_video_device *vdev =
> + vb2_queue_to_mtk_cam_video_device(vq);
> + const struct v4l2_pix_format_mplane *fmt = &vdev->format;
> + unsigned int size;
> + unsigned int np_conf;
> + unsigned int i;
> +
> + size = fmt->plane_fmt[0].sizeimage;
> + /* Add for q.create_bufs with fmt.g_sizeimage(p) / 2 test */
What's that ?
> +
> + np_conf = 1;
Weird name.
> +
> + if (*num_planes == 0) {
> + *num_planes = np_conf;
> + for (i = 0; i < *num_planes; ++i)
> + sizes[i] = size;
> + } else if (*num_planes != np_conf || sizes[0] < size) {
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int mtk_cam_vb2_buf_prepare(struct vb2_buffer *vb)
> +{
> + struct mtk_cam_video_device *vdev =
> + vb2_queue_to_mtk_cam_video_device(vb->vb2_queue);
> + struct mtk_cam_dev *cam = vb2_get_drv_priv(vb->vb2_queue);
> + struct mtk_cam_dev_buffer *buf = to_mtk_cam_dev_buffer(vb);
> + const struct v4l2_pix_format_mplane *fmt = &vdev->format;
> + u32 size;
> + int i;
> +
> + for (i = 0; i < vb->num_planes; i++) {
> + size = fmt->plane_fmt[i].sizeimage;
> + if (vb2_plane_size(vb, i) < size) {
> + dev_err(cam->dev, "plane size is too small:%lu<%u\n",
> + vb2_plane_size(vb, i), size);
> + return -EINVAL;
> + }
> + }
> +
> + buf->v4l2_buf.field = V4L2_FIELD_NONE;
> +
> + for (i = 0; i < vb->num_planes; i++) {
> + size = fmt->plane_fmt[i].sizeimage;
> + vb2_set_plane_payload(vb, i, size);
> + }
> +
> + if (!buf->daddr)
> + buf->daddr = vb2_dma_contig_plane_dma_addr(vb, 0);
> +
> + return 0;
> +}
> +
> +static void mtk_cam_vb2_buf_queue(struct vb2_buffer *vb)
> +{
> + struct mtk_cam_dev *cam = vb2_get_drv_priv(vb->vb2_queue);
> + struct mtk_cam_dev_buffer *buf = to_mtk_cam_dev_buffer(vb);
> + unsigned long flags;
> +
> + /* added the buffer into the tracking list */
s/added/Add/
> + spin_lock_irqsave(&cam->buf_list_lock, flags);
> + list_add_tail(&buf->list, &cam->buf_list);
> + spin_unlock_irqrestore(&cam->buf_list_lock, flags);
> +}
> +
> +static void mtk_cam_vb2_return_all_buffers(struct mtk_cam_dev *cam,
> + enum vb2_buffer_state state)
> +{
> + struct mtk_cam_dev_buffer *buf, *buf_prev;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&cam->buf_list_lock, flags);
> + list_for_each_entry_safe(buf, buf_prev, &cam->buf_list, list) {
> + buf->daddr = 0ULL;
> + list_del(&buf->list);
> + vb2_buffer_done(&buf->v4l2_buf.vb2_buf, state);
> + }
> + spin_unlock_irqrestore(&cam->buf_list_lock, flags);
> +}
> +
> +static void mtk_cam_cmos_vf_enable(struct mtk_cam_dev *cam_dev,
> + bool enable, bool pak_en)
> +{
> + struct device *dev = cam_dev->dev;
> +
> + if (pm_runtime_get_sync(dev) < 0) {
> + dev_err(dev, "failed to get pm_runtime\n");
> + goto out;
pm_runtime_resume_and_get(), so you can return here.
Actually, do you need to get runtime PM here ?
> + }
> +
> + if (enable)
> + cam_dev->hw_functions->mtk_cam_cmos_vf_hw_enable(cam_dev);
> + else
> + cam_dev->hw_functions->mtk_cam_cmos_vf_hw_disable(cam_dev);
> +
> +out:
> + pm_runtime_put_autosuspend(dev);
> +}
> +
> +static int mtk_cam_verify_format(struct mtk_cam_dev *cam)
> +{
> + struct mtk_cam_video_device *vdev = &cam->vdev;
> + struct v4l2_subdev_format fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + .pad = MTK_CAM_CIO_PAD_VIDEO,
> + };
> + int ret;
> +
> + ret = v4l2_subdev_call(&cam->subdev, pad, get_fmt, NULL, &fmt);
> + if (ret < 0)
> + return ret == -ENOIOCTLCMD ? -EINVAL : ret;
> +
> + if (vdev->fmtinfo->code != fmt.format.code ||
> + vdev->format.height != fmt.format.height ||
> + vdev->format.width != fmt.format.width)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int mtk_cam_vb2_start_streaming(struct vb2_queue *vq,
> + unsigned int count)
> +{
> + struct mtk_cam_dev *cam = vb2_get_drv_priv(vq);
> + struct mtk_cam_dev_buffer *buf;
> + struct mtk_cam_video_device *vdev =
> + vb2_queue_to_mtk_cam_video_device(vq);
> + struct device *dev = cam->dev;
> + const struct v4l2_pix_format_mplane *fmt = &vdev->format;
> + int ret;
> + unsigned long flags;
> +
> + if (pm_runtime_get_sync(dev) < 0) {
pm_runtime_resume_and_get(), so you can drop the
pm_runtime_put_autosuspend() in the error path.
> + dev_err(dev, "failed to get pm_runtime\n");
> + pm_runtime_put_autosuspend(dev);
> + return -1;
> + }
> +
> + (*cam->hw_functions->mtk_cam_setup)(cam, fmt->width, fmt->height,
> + fmt->plane_fmt[0].bytesperline, vdev->fmtinfo->code);
> +
> +
> + /* Enable CMOS and VF */
> + mtk_cam_cmos_vf_enable(cam, true, true);
> +
> + mutex_lock(&cam->op_lock);
> +
> + ret = mtk_cam_verify_format(cam);
> + if (ret < 0)
> + goto fail_unlock;
> +
> + /* Start streaming of the whole pipeline now*/
> + if (!cam->pipeline.start_count) {
> + ret = media_pipeline_start(vdev->vdev.entity.pads,
> + &cam->pipeline);
> + if (ret) {
> + dev_err(dev, "failed to start pipeline:%d\n", ret);
> + goto fail_unlock;
> + }
> + }
> +
> + /* Media links are fixed after media_pipeline_start */
> + cam->stream_count++;
> +
> + cam->sequence = (unsigned int)-1;
> +
> + /* Stream on the sub-device */
> + ret = v4l2_subdev_call(&cam->subdev, video, s_stream, 1);
> + if (ret)
> + goto fail_no_stream;
> +
> + mutex_unlock(&cam->op_lock);
> +
> + /* Create dummy buffer */
> + cam->dummy_size = fmt->plane_fmt[0].sizeimage;
> +
> + cam->dummy.vaddr = dma_alloc_coherent(cam->dev, cam->dummy_size,
> + &cam->dummy.daddr, GFP_KERNEL);
There's an ongoing discussion on the need to allocate a large dummy
buffer in the previous version of this patch. It's still applicable
here.
> + if (!cam->dummy.vaddr) {
> + ret = -ENOMEM;
> + goto fail_no_buffer;
> + }
> +
> + /* update first buffer address */
> +
> + /* added the buffer into the tracking list */
> + spin_lock_irqsave(&cam->buf_list_lock, flags);
> + if (list_empty(&cam->buf_list)) {
> + (*cam->hw_functions->mtk_cam_update_buffers_add)(cam, &cam->dummy);
> + cam->is_dummy_used = true;
> + } else {
> + buf = list_first_entry_or_null(&cam->buf_list,
> + struct mtk_cam_dev_buffer,
> + list);
> + (*cam->hw_functions->mtk_cam_update_buffers_add)(cam, buf);
> + cam->is_dummy_used = false;
> + }
> + spin_unlock_irqrestore(&cam->buf_list_lock, flags);
> +
> + return 0;
> +
> +fail_no_buffer:
> + mutex_lock(&cam->op_lock);
> + v4l2_subdev_call(&cam->subdev, video, s_stream, 0);
> +fail_no_stream:
> + cam->stream_count--;
> + if (cam->stream_count == 0)
> + media_pipeline_stop(vdev->vdev.entity.pads);
> +fail_unlock:
> + mutex_unlock(&cam->op_lock);
> + mtk_cam_vb2_return_all_buffers(cam, VB2_BUF_STATE_QUEUED);
> +
> + return ret;
> +}
> +
> +static void mtk_cam_vb2_stop_streaming(struct vb2_queue *vq)
> +{
> + struct mtk_cam_dev *cam = vb2_get_drv_priv(vq);
> + struct mtk_cam_video_device *vdev =
> + vb2_queue_to_mtk_cam_video_device(vq);
> +
> + /* Disable CMOS and VF */
> + mtk_cam_cmos_vf_enable(cam, false, false);
> +
> + mutex_lock(&cam->op_lock);
> +
> + v4l2_subdev_call(&cam->subdev, video, s_stream, 0);
> +
> + mtk_cam_vb2_return_all_buffers(cam, VB2_BUF_STATE_ERROR);
> + cam->stream_count--;
> + if (cam->stream_count) {
> + mutex_unlock(&cam->op_lock);
> + return;
> + }
> +
> + /* Destroy dummy buffer */
> + if (cam->dummy.vaddr) {
> + dma_free_coherent(cam->dev, cam->dummy_size, cam->dummy.vaddr,
> + cam->dummy.daddr);
> + memset(&cam->dummy, 0, sizeof(cam->dummy));
> + cam->dummy_size = 0;
> + cam->is_dummy_used = false;
> + }
> +
> + mutex_unlock(&cam->op_lock);
> +
> + media_pipeline_stop(vdev->vdev.entity.pads);
> +}
> +
> +static const struct vb2_ops mtk_cam_vb2_ops = {
> + .queue_setup = mtk_cam_vb2_queue_setup,
> + .buf_prepare = mtk_cam_vb2_buf_prepare,
> + .buf_queue = mtk_cam_vb2_buf_queue,
> + .start_streaming = mtk_cam_vb2_start_streaming,
> + .stop_streaming = mtk_cam_vb2_stop_streaming,
> + .wait_prepare = vb2_ops_wait_prepare,
> + .wait_finish = vb2_ops_wait_finish,
> +};
> +
> +/* -----------------------------------------------------------------------------
> + * V4L2 Video IOCTLs
> + */
> +
> +static int mtk_cam_vidioc_querycap(struct file *file, void *fh,
> + struct v4l2_capability *cap)
> +{
> + struct mtk_cam_dev *cam = video_drvdata(file);
> +
> + strscpy(cap->driver, dev_driver_string(cam->dev), sizeof(cap->driver));
> + strscpy(cap->card, dev_driver_string(cam->dev), sizeof(cap->card));
> +
> + return 0;
> +}
> +
> +static int mtk_cam_vidioc_enum_fmt(struct file *file, void *fh,
> + struct v4l2_fmtdesc *f)
> +{
> + struct mtk_cam_video_device *vdev = file_to_mtk_cam_video_device(file);
> + const struct mtk_cam_format_info *fmtinfo;
> + unsigned int i;
> +
> + /* If mbus_code is not set enumerate all supported formats. */
> + if (!f->mbus_code) {
> + if (f->index >= vdev->desc->num_fmts)
> + return -EINVAL;
> +
> + /* f->description is filled in v4l_fill_fmtdesc function */
> + f->pixelformat = vdev->desc->fmts[f->index];
> + f->flags = 0;
> +
> + return 0;
> + }
> +
> + /*
> + * Otherwise only enumerate supported pixel formats corresponding to
> + * that bus code.
> + */
> + if (f->index)
> + return -EINVAL;
> +
> + fmtinfo = mtk_cam_format_info_by_code(f->mbus_code);
> + if (!fmtinfo)
> + return -EINVAL;
> +
> + for (i = 0; i < vdev->desc->num_fmts; ++i) {
> + if (vdev->desc->fmts[i] == fmtinfo->fourcc) {
> + f->pixelformat = fmtinfo->fourcc;
> + f->flags = 0;
> + return 0;
> + }
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int mtk_cam_vidioc_g_fmt(struct file *file, void *fh,
> + struct v4l2_format *f)
> +{
> + struct mtk_cam_video_device *vdev = file_to_mtk_cam_video_device(file);
> +
> + f->fmt.pix_mp = vdev->format;
> +
> + return 0;
> +}
> +
> +static int mtk_cam_vidioc_try_fmt(struct file *file, void *fh,
> + struct v4l2_format *f)
> +{
> + struct mtk_cam_video_device *vdev = file_to_mtk_cam_video_device(file);
> + struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> + const struct mtk_cam_format_info *fmtinfo;
> +
> + /* Validate pixelformat */
> + if (!mtk_cam_dev_find_fmt(vdev->desc, pix_mp->pixelformat))
> + pix_mp->pixelformat = vdev->desc->fmts[0];
> +
> + pix_mp->width = clamp_val(pix_mp->width, IMG_MIN_WIDTH, IMG_MAX_WIDTH);
> + pix_mp->height = clamp_val(pix_mp->height, IMG_MIN_HEIGHT,
> + IMG_MAX_HEIGHT);
> +
> + pix_mp->num_planes = 1;
> +
> + fmtinfo = mtk_cam_format_info_by_fourcc(pix_mp->pixelformat);
> + calc_bpl_size_pix_mp(fmtinfo, pix_mp);
> +
> + /* Constant format fields */
> + pix_mp->colorspace = V4L2_COLORSPACE_SRGB;
> + pix_mp->field = V4L2_FIELD_NONE;
> + pix_mp->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> + pix_mp->quantization = V4L2_QUANTIZATION_DEFAULT;
> + pix_mp->xfer_func = V4L2_XFER_FUNC_DEFAULT;
> +
> + return 0;
> +}
> +
> +static int mtk_cam_vidioc_s_fmt(struct file *file, void *fh,
> + struct v4l2_format *f)
> +{
> + struct mtk_cam_dev *cam = video_drvdata(file);
> + struct mtk_cam_video_device *vdev = file_to_mtk_cam_video_device(file);
> + int ret;
> +
> + if (vb2_is_busy(vdev->vdev.queue)) {
> + dev_dbg(cam->dev, "%s: queue is busy\n", __func__);
I think you can drop this message.
> + return -EBUSY;
> + }
> +
> + ret = mtk_cam_vidioc_try_fmt(file, fh, f);
> + if (ret)
> + return ret;
> +
> + /* Configure to video device */
> + vdev->format = f->fmt.pix_mp;
> + vdev->fmtinfo =
> + mtk_cam_format_info_by_fourcc(f->fmt.pix_mp.pixelformat);
> +
> + return 0;
> +}
> +
> +static int mtk_cam_vidioc_enum_framesizes(struct file *file, void *priv,
> + struct v4l2_frmsizeenum *sizes)
> +{
> + struct mtk_cam_video_device *vdev = file_to_mtk_cam_video_device(file);
> +
> + if (sizes->index)
> + return -EINVAL;
> +
> + if (!mtk_cam_dev_find_fmt(vdev->desc, sizes->pixel_format))
> + return -EINVAL;
> +
> + sizes->type = vdev->desc->frmsizes->type;
> + memcpy(&sizes->stepwise, &vdev->desc->frmsizes->stepwise,
> + sizeof(sizes->stepwise));
> +
> + return 0;
> +}
> +
> +static const struct v4l2_ioctl_ops mtk_cam_v4l2_vcap_ioctl_ops = {
> + .vidioc_querycap = mtk_cam_vidioc_querycap,
> + .vidioc_enum_framesizes = mtk_cam_vidioc_enum_framesizes,
> + .vidioc_enum_fmt_vid_cap = mtk_cam_vidioc_enum_fmt,
> + .vidioc_g_fmt_vid_cap_mplane = mtk_cam_vidioc_g_fmt,
> + .vidioc_s_fmt_vid_cap_mplane = mtk_cam_vidioc_s_fmt,
> + .vidioc_try_fmt_vid_cap_mplane = mtk_cam_vidioc_try_fmt,
> + .vidioc_reqbufs = vb2_ioctl_reqbufs,
> + .vidioc_create_bufs = vb2_ioctl_create_bufs,
> + .vidioc_prepare_buf = vb2_ioctl_prepare_buf,
> + .vidioc_querybuf = vb2_ioctl_querybuf,
> + .vidioc_qbuf = vb2_ioctl_qbuf,
> + .vidioc_dqbuf = vb2_ioctl_dqbuf,
> + .vidioc_streamon = vb2_ioctl_streamon,
> + .vidioc_streamoff = vb2_ioctl_streamoff,
> + .vidioc_expbuf = vb2_ioctl_expbuf,
> + .vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
> + .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
> +};
> +
> +static const struct v4l2_file_operations mtk_cam_v4l2_fops = {
> + .unlocked_ioctl = video_ioctl2,
> + .open = v4l2_fh_open,
> + .release = vb2_fop_release,
> + .poll = vb2_fop_poll,
> + .mmap = vb2_fop_mmap,
> +#ifdef CONFIG_COMPAT
> + .compat_ioctl32 = v4l2_compat_ioctl32,
> +#endif
> +};
> +
> +/* -----------------------------------------------------------------------------
> + * Init & Cleanup
> + */
> +
> +static const u32 stream_out_fmts[] = {
> + /* The 1st entry is the default image format */
> + V4L2_PIX_FMT_SBGGR8,
> + V4L2_PIX_FMT_SGBRG8,
> + V4L2_PIX_FMT_SGRBG8,
> + V4L2_PIX_FMT_SRGGB8,
> + V4L2_PIX_FMT_UYVY,
> + V4L2_PIX_FMT_VYUY,
> + V4L2_PIX_FMT_YUYV,
> + V4L2_PIX_FMT_YVYU,
> +};
> +
> +static const struct mtk_cam_vdev_desc video_stream = {
> + .name = "video stream",
> + .cap = V4L2_CAP_VIDEO_CAPTURE_MPLANE,
> + .buf_type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
> + .link_flags = MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED,
> + .fmts = stream_out_fmts,
> + .num_fmts = ARRAY_SIZE(stream_out_fmts),
> + .def_width = IMG_MAX_WIDTH,
> + .def_height = IMG_MAX_HEIGHT,
> + .ioctl_ops = &mtk_cam_v4l2_vcap_ioctl_ops,
> + .frmsizes =
> + &(struct v4l2_frmsizeenum){
> + .index = 0,
> + .type = V4L2_FRMSIZE_TYPE_CONTINUOUS,
> + .stepwise = {
> + .max_width = IMG_MAX_WIDTH,
> + .min_width = IMG_MIN_WIDTH,
> + .max_height = IMG_MAX_HEIGHT,
> + .min_height = IMG_MIN_HEIGHT,
> + .step_height = 1,
> + .step_width = 1,
> + },
> + },
> +};
> +
> +int mtk_cam_video_register(struct mtk_cam_dev *cam)
> +{
> + struct device *dev = cam->dev;
> + struct mtk_cam_video_device *cam_vdev = &cam->vdev;
> + struct video_device *vdev = &cam_vdev->vdev;
> + struct vb2_queue *vbq = &cam_vdev->vbq;
> + int ret;
> +
> + vb2_dma_contig_set_max_seg_size(cam->dev, DMA_BIT_MASK(32));
> +
> + cam_vdev->desc = &video_stream;
> +
> + /* Initialize mtk_cam_video_device */
> + mtk_cam_dev_load_default_fmt(cam);
> +
> + cam_vdev->vdev_pad.flags = MEDIA_PAD_FL_SOURCE;
> +
> + /* Initialize media entities */
> + ret = media_entity_pads_init(&vdev->entity, 1, &cam_vdev->vdev_pad);
> + if (ret) {
> + dev_err(dev, "failed to initialize media pad:%d\n", ret);
> + return ret;
> + }
> + cam_vdev->vdev_pad.flags = MEDIA_PAD_FL_SINK;
> +
> + vbq->type = cam_vdev->desc->buf_type;
> + vbq->io_modes = VB2_MMAP | VB2_DMABUF;
> + vbq->dev = dev;
> + vbq->ops = &mtk_cam_vb2_ops;
> + vbq->mem_ops = &vb2_dma_contig_memops;
> + vbq->buf_struct_size = sizeof(struct mtk_cam_dev_buffer);
> + /*
> + * TODO: The hardware supports SOF interrupts, switch to a SOF
> + * timestamp source would give better accuracy, but first requires
> + * extending the V4L2 API to support it.
> + */
> + vbq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC
> + | V4L2_BUF_FLAG_TSTAMP_SRC_EOF;
> +
> + /* No minimum buffers limitation */
> + vbq->min_queued_buffers = 0;
> + vbq->drv_priv = cam;
> +
> + vbq->lock = &cam_vdev->vdev_lock;
> + ret = vb2_queue_init(vbq);
> + if (ret) {
> + dev_err(dev, "failed to init. vb2 queue:%d\n", ret);
> + goto fail_media_clean;
> + }
> +
> + /* Initialize vdev */
> + snprintf(vdev->name, sizeof(vdev->name), "%s %s",
> + dev_name(dev), cam_vdev->desc->name);
> +
> + /* Set cap/type/ioctl_ops of the video device */
> + vdev->device_caps = cam_vdev->desc->cap | V4L2_CAP_STREAMING
> + | V4L2_CAP_IO_MC;
> + vdev->ioctl_ops = cam_vdev->desc->ioctl_ops;
Drop ioctl_ops from mtk_cam_vdev_desc and hardcode it here. Same for
name, cap, buf_type and link_flags. If we ever merge support for another
hardware variant in this driver that requires a different configuration,
we'll address it then. I'm actually tempted to drop mtk_cam_vdev_desc
completely.
> + vdev->fops = &mtk_cam_v4l2_fops;
> + vdev->release = video_device_release_empty;
> + vdev->lock = &cam_vdev->vdev_lock;
> + vdev->v4l2_dev = cam->subdev.v4l2_dev;
> + vdev->queue = &cam_vdev->vbq;
> + vdev->vfl_dir = VFL_DIR_RX;
> + vdev->entity.function = MEDIA_ENT_F_IO_V4L;
> + video_set_drvdata(vdev, cam);
> +
> + /* Initialize miscellaneous variables */
> + mutex_init(&cam_vdev->vdev_lock);
> + INIT_LIST_HEAD(&cam->buf_list);
> +
> + ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1);
> + if (ret) {
> + dev_err(dev, "failed to register vde:%d\n", ret);
> + goto fail_vb2_rel;
> + }
> +
> + /* Create link between the video pad and the subdev pad. */
> + ret = media_create_pad_link(&cam->subdev.entity,
> + MTK_CAM_CIO_PAD_VIDEO,
> + &vdev->entity, 0, cam_vdev->desc->link_flags);
> +
Drop the blank line.
> + if (ret)
> + goto fail_vdev_ureg;
> +
> + return 0;
> +
> +fail_vdev_ureg:
> + video_unregister_device(vdev);
> +fail_vb2_rel:
> + mutex_destroy(&cam_vdev->vdev_lock);
> + vb2_queue_release(vbq);
> +fail_media_clean:
> + media_entity_cleanup(&vdev->entity);
> +
> + return ret;
> +}
> +
> +void mtk_cam_video_unregister(struct mtk_cam_video_device *vdev)
> +{
> + video_unregister_device(&vdev->vdev);
> + vb2_queue_release(&vdev->vbq);
> + media_entity_cleanup(&vdev->vdev.entity);
> + mutex_destroy(&vdev->vdev_lock);
> + vb2_dma_contig_clear_max_seg_size(&vdev->vdev.dev);
> +}
--
Regards,
Laurent Pinchart