| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 12 Jun 2024 10:06:35 +0200
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Julien Stephan <jstephan@...libre.com>
Cc: Louis Kuo <louis.kuo@...iatek.com>, Phi-bang Nguyen
<pnguyen@...libre.com>, Florian Sylvestre <fsylvestre@...libre.com>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Andy Hsieh <andy.hsieh@...iatek.com>, Conor Dooley <conor+dt@...nel.org>,
devicetree@...r.kernel.org,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-mediatek@...ts.infradead.org, linux-media@...r.kernel.org,
Matthias Brugger <matthias.bgg@...il.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Paul Elder <paul.elder@...asonboard.com>, Rob Herring <robh+dt@...nel.org>
Subject: Re: [PATCH v4 3/5] media: platform: mediatek: isp_30: add mediatek
ISP3.0 sensor interface
Il 10/06/24 16:39, Julien Stephan ha scritto:
> Le jeu. 11 janv. 2024 à 13:04, AngeloGioacchino Del Regno
> <angelogioacchino.delregno@...labora.com> a écrit :
>>
>> Il 10/01/24 15:14, Julien Stephan ha scritto:
>>> From: Louis Kuo <louis.kuo@...iatek.com>
>>>
>>> This will add the mediatek ISP3.0 seninf (sensor interface) driver found
>>> on several Mediatek SoCs such as the mt8365.
>>>
>>> Then seninf module has 4 physical CSI-2 inputs. Depending on the soc they
>>> may not be all connected.
>>>
>>> Signed-off-by: Louis Kuo <louis.kuo@...iatek.com>
>>> Signed-off-by: Phi-bang Nguyen <pnguyen@...libre.com>
>>> Signed-off-by: Florian Sylvestre <fsylvestre@...libre.com>
>>> Co-developed-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>
>>> Co-developed-by: Julien Stephan <jstephan@...libre.com>
>>> Signed-off-by: Julien Stephan <jstephan@...libre.com>
>>> ---
>>> MAINTAINERS | 1 +
>>> drivers/media/platform/mediatek/Kconfig | 1 +
>>> drivers/media/platform/mediatek/Makefile | 1 +
>>> drivers/media/platform/mediatek/isp/Kconfig | 2 +
>>> drivers/media/platform/mediatek/isp/Makefile | 3 +
>>> .../platform/mediatek/isp/isp_30/Kconfig | 16 +
>>> .../platform/mediatek/isp/isp_30/Makefile | 3 +
>>> .../mediatek/isp/isp_30/seninf/Makefile | 5 +
>>> .../mediatek/isp/isp_30/seninf/mtk_seninf.c | 1488 +++++++++++++++++
>>> .../isp/isp_30/seninf/mtk_seninf_reg.h | 112 ++
>>> 10 files changed, 1632 insertions(+)
>>> create mode 100644 drivers/media/platform/mediatek/isp/Kconfig
>>> create mode 100644 drivers/media/platform/mediatek/isp/Makefile
>>> create mode 100644 drivers/media/platform/mediatek/isp/isp_30/Kconfig
>>> create mode 100644 drivers/media/platform/mediatek/isp/isp_30/Makefile
>>> create mode 100644 drivers/media/platform/mediatek/isp/isp_30/seninf/Makefile
>>> create mode 100644 drivers/media/platform/mediatek/isp/isp_30/seninf/mtk_seninf.c
>>> create mode 100644 drivers/media/platform/mediatek/isp/isp_30/seninf/mtk_seninf_reg.h
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 3ea2158864e1..52d200d5e36c 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -13618,6 +13618,7 @@ M: Andy Hsieh <andy.hsieh@...iatek.com>
>>> 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/seninf/*
>>>
>>> MEDIATEK SMI DRIVER
>>> M: Yong Wu <yong.wu@...iatek.com>
>>> diff --git a/drivers/media/platform/mediatek/Kconfig b/drivers/media/platform/mediatek/Kconfig
>>> index 84104e2cd024..4e0a5a43f35e 100644
>>> --- a/drivers/media/platform/mediatek/Kconfig
>>> +++ b/drivers/media/platform/mediatek/Kconfig
>>> @@ -7,3 +7,4 @@ source "drivers/media/platform/mediatek/mdp/Kconfig"
>>> source "drivers/media/platform/mediatek/vcodec/Kconfig"
>>> source "drivers/media/platform/mediatek/vpu/Kconfig"
>>> source "drivers/media/platform/mediatek/mdp3/Kconfig"
>>> +source "drivers/media/platform/mediatek/isp/Kconfig"
>>> diff --git a/drivers/media/platform/mediatek/Makefile b/drivers/media/platform/mediatek/Makefile
>>> index 38e6ba917fe5..695f05f525a6 100644
>>> --- a/drivers/media/platform/mediatek/Makefile
>>> +++ b/drivers/media/platform/mediatek/Makefile
>>> @@ -4,3 +4,4 @@ obj-y += mdp/
>>> obj-y += vcodec/
>>> obj-y += vpu/
>>> obj-y += mdp3/
>>> +obj-y += isp/
>>> diff --git a/drivers/media/platform/mediatek/isp/Kconfig b/drivers/media/platform/mediatek/isp/Kconfig
>>> new file mode 100644
>>> index 000000000000..708b9a6660d2
>>> --- /dev/null
>>> +++ b/drivers/media/platform/mediatek/isp/Kconfig
>>> @@ -0,0 +1,2 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only
>>> +source "drivers/media/platform/mediatek/isp/isp_30/Kconfig"
>>> diff --git a/drivers/media/platform/mediatek/isp/Makefile b/drivers/media/platform/mediatek/isp/Makefile
>>> new file mode 100644
>>> index 000000000000..a81ab33d0dd3
>>> --- /dev/null
>>> +++ b/drivers/media/platform/mediatek/isp/Makefile
>>> @@ -0,0 +1,3 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only
>>> +
>>> +obj-y += isp_30/
>>> diff --git a/drivers/media/platform/mediatek/isp/isp_30/Kconfig b/drivers/media/platform/mediatek/isp/isp_30/Kconfig
>>> new file mode 100644
>>> index 000000000000..9791312589fb
>>> --- /dev/null
>>> +++ b/drivers/media/platform/mediatek/isp/isp_30/Kconfig
>>> @@ -0,0 +1,16 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only
>>> +config MTK_SENINF30
>>> + tristate "MediaTek ISP3.0 SENINF driver"
>>> + depends on VIDEO_V4L2_SUBDEV_API
>>> + depends on MEDIA_CAMERA_SUPPORT
>>> + depends on ARCH_MEDIATEK || COMPILE_TEST
>>> + depends on OF
>>> + select V4L2_FWNODE
>>> + default n
>>> + help
>>> + This driver provides a MIPI CSI-2 receiver interface to connect
>>> + an external camera module with MediaTek ISP3.0. It is able to handle
>>> + multiple cameras at the same time.
>>> +
>>> + To compile this driver as a module, choose M here: the
>>> + module will be called mtk-seninf.
>>> diff --git a/drivers/media/platform/mediatek/isp/isp_30/Makefile b/drivers/media/platform/mediatek/isp/isp_30/Makefile
>>> new file mode 100644
>>> index 000000000000..ac3142de4739
>>> --- /dev/null
>>> +++ b/drivers/media/platform/mediatek/isp/isp_30/Makefile
>>> @@ -0,0 +1,3 @@
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +
>>> +obj-$(CONFIG_MTK_SENINF30) += seninf/
>>> diff --git a/drivers/media/platform/mediatek/isp/isp_30/seninf/Makefile b/drivers/media/platform/mediatek/isp/isp_30/seninf/Makefile
>>> new file mode 100644
>>> index 000000000000..f28480d6d6c3
>>> --- /dev/null
>>> +++ b/drivers/media/platform/mediatek/isp/isp_30/seninf/Makefile
>>> @@ -0,0 +1,5 @@
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +
>>> +mtk-seninf-objs += mtk_seninf.o
>>> +
>>> +obj-$(CONFIG_MTK_SENINF30) += mtk-seninf.o
>>> diff --git a/drivers/media/platform/mediatek/isp/isp_30/seninf/mtk_seninf.c b/drivers/media/platform/mediatek/isp/isp_30/seninf/mtk_seninf.c
>>> new file mode 100644
>>> index 000000000000..67b2c697d9ca
>>> --- /dev/null
>>> +++ b/drivers/media/platform/mediatek/isp/isp_30/seninf/mtk_seninf.c
>>> @@ -0,0 +1,1488 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (c) 2022 MediaTek Inc.
>>> + */
>>> +#include <linux/bitfield.h>
>>> +#include <linux/clk.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of_graph.h>
>>> +#include <linux/of_platform.h>
>>> +#include <linux/phy/phy.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/pm_runtime.h>
>>> +#include <linux/videodev2.h>
>>> +#include <media/media-device.h>
>>> +#include <media/media-entity.h>
>>> +#include <media/v4l2-async.h>
>>> +#include <media/v4l2-common.h>
>>> +#include <media/v4l2-ctrls.h>
>>> +#include <media/v4l2-dev.h>
>>> +#include <media/v4l2-device.h>
>>> +#include <media/v4l2-event.h>
>>> +#include <media/v4l2-fwnode.h>
>>> +#include <media/v4l2-mc.h>
>>> +#include <media/v4l2-subdev.h>
>>> +
>>> +#include "mtk_seninf_reg.h"
>>> +
>>> +#define SENINF_TIMESTAMP_STEP 0x67
>>> +#define SENINF_SETTLE_DELAY 0x15
>>> +#define SENINF_HS_TRAIL_PARAMETER 0x8
>>> +
>>> +#define SENINF_MAX_NUM_INPUTS 4
>>> +#define SENINF_MAX_NUM_OUTPUTS 6
>>> +#define SENINF_MAX_NUM_MUXES 6
>>> +#define SENINF_MAX_NUM_PADS (SENINF_MAX_NUM_INPUTS + \
>>> + SENINF_MAX_NUM_OUTPUTS)
>>> +
>>> +#define SENINF_DEFAULT_BUS_FMT MEDIA_BUS_FMT_SGRBG10_1X10
>>> +#define SENINF_DEFAULT_WIDTH 1920
>>> +#define SENINF_DEFAULT_HEIGHT 1080
>>> +
>>> +#define SENINF_PAD_10BIT 0
>>> +
>>> +#define SENINF_TEST_MODEL 0
>>> +#define SENINF_NORMAL_MODEL 1
>>> +#define SENINF_ALL_ERR_IRQ_EN 0x7f
>>> +#define SENINF_IRQ_CLR_SEL 0x80000000
>>> +
>>> +#define SENINF_MIPI_SENSOR 0x8
>>> +
>>> +#define MTK_CSI_MAX_LANES 4
>>> +
>>> +/* Port number in the device tree. */
>>> +enum mtk_seninf_port {
>>> + CSI_PORT_0 = 0, /* 4D1C or 2D1C */
>>> + CSI_PORT_1, /* 4D1C */
>>> + CSI_PORT_2, /* 4D1C */
>>> + CSI_PORT_0B, /* 2D1C */
>>> +};
>>> +
>>> +enum mtk_seninf_id {
>>> + SENINF_1 = 0,
>>> + SENINF_2 = 1,
>>> + SENINF_3 = 2,
>>> + SENINF_5 = 4,
>>> +};
>>> +
>>> +static const u32 port_to_seninf_id[] = {
>>> + [CSI_PORT_0] = SENINF_1,
>>> + [CSI_PORT_1] = SENINF_3,
>>> + [CSI_PORT_2] = SENINF_5,
>>> + [CSI_PORT_0B] = SENINF_2,
>>> +};
>>> +
>>> +enum mtk_seninf_phy_mode {
>>> + SENINF_PHY_MODE_NONE,
>>> + SENINF_PHY_MODE_4D1C,
>>> + SENINF_PHY_MODE_2D1C,
>>> +};
>>> +
>>> +enum mtk_seninf_format_flag {
>>> + MTK_SENINF_FORMAT_BAYER = BIT(0),
>>> + MTK_SENINF_FORMAT_DPCM = BIT(1),
>>> + MTK_SENINF_FORMAT_JPEG = BIT(2),
>>> + MTK_SENINF_FORMAT_INPUT_ONLY = BIT(3),
>>> +};
>>> +
>>> +/**
>>> + * struct mtk_seninf_conf - Model-specific SENINF parameters
>>> + * @model: Model description
>>> + * @nb_inputs: Number of SENINF inputs
>>> + * @nb_muxes: Number of SENINF MUX (FIFO) instances
>>> + * @nb_outputs: Number of outputs (to CAM and CAMSV instances)
>>> + */
>>> +struct mtk_seninf_conf {
>>> + const char *model;
>>> + u8 nb_inputs;
>>> + u8 nb_muxes;
>>> + u8 nb_outputs;
>>> +};
>>> +
>>> +/**
>>> + * struct mtk_seninf_format_info - Information about media bus formats
>>> + * @code: V4L2 media bus code
>>> + * @flags: Flags describing the format, as a combination of MTK_SENINF_FORMAT_*
>>> + */
>>> +struct mtk_seninf_format_info {
>>> + u32 code;
>>> + u32 flags;
>>> +};
>>> +
>>> +/**
>>> + * struct mtk_seninf_input - SENINF input block
>>> + * @pad: DT port and media entity pad number
>>> + * @seninf_id: SENINF hardware instance ID
>>> + * @base: Memory mapped I/O based address
>>> + * @seninf: Back pointer to the mtk_seninf
>>> + * @phy: PHY connected to the input
>>> + * @phy_mode: PHY operation mode (NONE when the input is not connected)
>>> + * @bus: CSI-2 bus configuration from DT
>>> + * @source_sd: Source subdev connected to the input
>>> + */
>>> +struct mtk_seninf_input {
>>> + enum mtk_seninf_port pad;
>>> + enum mtk_seninf_id seninf_id;
>>> + void __iomem *base;
>>> + struct mtk_seninf *seninf;
>>> +
>>> + struct phy *phy;
>>> + enum mtk_seninf_phy_mode phy_mode;
>>> +
>>> + struct v4l2_mbus_config_mipi_csi2 bus;
>>> +
>>> + struct v4l2_subdev *source_sd;
>>> +};
>>> +
>>> +/**
>>> + * struct mtk_seninf_mux - SENINF MUX channel
>>> + * @pad: DT port and media entity pad number
>>> + * @mux_id: MUX hardware instance ID
>>> + * @base: Memory mapped I/O based address
>>> + * @seninf: Back pointer to the mtk_seninf
>>> + */
>>> +struct mtk_seninf_mux {
>>> + unsigned int pad;
>>> + unsigned int mux_id;
>>> + void __iomem *base;
>>> + struct mtk_seninf *seninf;
>>> +};
>>> +
>>> +/**
>>> + * struct mtk_seninf - Top-level SENINF device
>>> + * @dev: The (platform) device
>>> + * @phy: PHYs at the SENINF inputs
>>> + * @num_clks: Number of clocks in the clks array
>>> + * @clks: Clocks
>>> + * @base: Memory mapped I/O base address
>>> + * @media_dev: Media controller device
>>> + * @v4l2_dev: V4L2 device
>>> + * @subdev: V4L2 subdevice
>>> + * @pads: Media entity pads
>>> + * @notifier: V4L2 async notifier for source subdevs
>>> + * @ctrl_handler: V4L2 controls handler
>>> + * @source_format: Active format on the source pad
>>> + * @inputs: Array of SENINF inputs
>>> + * @muxes: Array of MUXes
>>> + * @conf: Model-specific SENINF parameters
>>> + * @is_testmode: Whether or not the test pattern generator is enabled
>>> + */
>>> +struct mtk_seninf {
>>> + struct device *dev;
>>> + struct phy *phy[5];
>>> + unsigned int num_clks;
>>> + struct clk_bulk_data *clks;
>>> + void __iomem *base;
>>> +
>>> + struct media_device media_dev;
>>> + struct v4l2_device v4l2_dev;
>>> + struct v4l2_subdev subdev;
>>> + struct media_pad pads[SENINF_MAX_NUM_PADS];
>>> + struct v4l2_async_notifier notifier;
>>> + struct v4l2_ctrl_handler ctrl_handler;
>>> +
>>> + struct mtk_seninf_input inputs[SENINF_MAX_NUM_INPUTS];
>>> + struct mtk_seninf_mux muxes[SENINF_MAX_NUM_MUXES];
>>> +
>>> + const struct mtk_seninf_conf *conf;
>>> +
>>> + bool is_testmode;
>>> +};
>>> +
>>> +inline struct mtk_seninf *sd_to_mtk_seninf(struct v4l2_subdev *sd)
>>> +{
>>> + return container_of(sd, struct mtk_seninf, subdev);
>>> +}
>>> +
>>> +static inline bool mtk_seninf_pad_is_sink(struct mtk_seninf *priv,
>>> + unsigned int pad)
>>> +{
>>> + return pad < priv->conf->nb_inputs;
>>> +}
>>> +
>>> +static inline bool mtk_seninf_pad_is_source(struct mtk_seninf *priv,
>>> + unsigned int pad)
>>> +{
>>> + return !mtk_seninf_pad_is_sink(priv, pad);
>>> +}
>>> +
>>> +/* -----------------------------------------------------------------------------
>>> + * Formats
>>> + */
>>> +
>>> +static const struct mtk_seninf_format_info mtk_seninf_formats[] = {
>>> + {
>>> + .code = MEDIA_BUS_FMT_SBGGR8_1X8,
>>> + .flags = MTK_SENINF_FORMAT_BAYER,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_SGBRG8_1X8,
>>> + .flags = MTK_SENINF_FORMAT_BAYER,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_SGRBG8_1X8,
>>> + .flags = MTK_SENINF_FORMAT_BAYER,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_SRGGB8_1X8,
>>> + .flags = MTK_SENINF_FORMAT_BAYER,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_SGRBG10_1X10,
>>> + .flags = MTK_SENINF_FORMAT_BAYER,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_SRGGB10_1X10,
>>> + .flags = MTK_SENINF_FORMAT_BAYER,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_SBGGR10_1X10,
>>> + .flags = MTK_SENINF_FORMAT_BAYER,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_SGBRG10_1X10,
>>> + .flags = MTK_SENINF_FORMAT_BAYER,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_SBGGR12_1X12,
>>> + .flags = MTK_SENINF_FORMAT_BAYER,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_SGBRG12_1X12,
>>> + .flags = MTK_SENINF_FORMAT_BAYER,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_SGRBG12_1X12,
>>> + .flags = MTK_SENINF_FORMAT_BAYER,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_SRGGB12_1X12,
>>> + .flags = MTK_SENINF_FORMAT_BAYER,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_SBGGR14_1X14,
>>> + .flags = MTK_SENINF_FORMAT_BAYER,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_SGBRG14_1X14,
>>> + .flags = MTK_SENINF_FORMAT_BAYER,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_SGRBG14_1X14,
>>> + .flags = MTK_SENINF_FORMAT_BAYER,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_SRGGB14_1X14,
>>> + .flags = MTK_SENINF_FORMAT_BAYER,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_SBGGR16_1X16,
>>> + .flags = MTK_SENINF_FORMAT_BAYER,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_SGBRG16_1X16,
>>> + .flags = MTK_SENINF_FORMAT_BAYER,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_SGRBG16_1X16,
>>> + .flags = MTK_SENINF_FORMAT_BAYER,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_SRGGB16_1X16,
>>> + .flags = MTK_SENINF_FORMAT_BAYER,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_UYVY8_1X16,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_VYUY8_1X16,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_YUYV8_1X16,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_YVYU8_1X16,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_JPEG_1X8,
>>> + .flags = MTK_SENINF_FORMAT_JPEG,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_S5C_UYVY_JPEG_1X8,
>>> + .flags = MTK_SENINF_FORMAT_JPEG,
>>> + },
>>> + /* Keep the input-only formats last. */
>>
>> Your comment doesn't make me understand why input-only formats shall be
>> placed last - and makes me think that having two arrays (one for both
>> and one for input only) would be easier and less error prone, other than
>> making you able to drop the MTK_SENINF_FORMAT_INPUT_ONLY flag entirely.
>>
>>> + {
>>> + .code = MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8,
>>> + .flags = MTK_SENINF_FORMAT_DPCM | MTK_SENINF_FORMAT_INPUT_ONLY,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_SRGGB10_DPCM8_1X8,
>>> + .flags = MTK_SENINF_FORMAT_DPCM | MTK_SENINF_FORMAT_INPUT_ONLY,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_SBGGR10_DPCM8_1X8,
>>> + .flags = MTK_SENINF_FORMAT_DPCM | MTK_SENINF_FORMAT_INPUT_ONLY,
>>> + }, {
>>> + .code = MEDIA_BUS_FMT_SGBRG10_DPCM8_1X8,
>>> + .flags = MTK_SENINF_FORMAT_DPCM | MTK_SENINF_FORMAT_INPUT_ONLY,
>>> + }
>>> +};
>>> +
>>> +static const struct mtk_seninf_format_info *mtk_seninf_format_info(u32 code)
>>> +{
>>> + unsigned int i;
>>> +
>>> + for (i = 0; i < ARRAY_SIZE(mtk_seninf_formats); ++i) {
>>> + if (mtk_seninf_formats[i].code == code)
>>> + return &mtk_seninf_formats[i];
>>> + }
>>> +
>>> + return NULL;
>>> +}
>>> +
>>
>> ..snip..
>>
>>> +
>>> +static void mtk_seninf_input_setup_csi2(struct mtk_seninf_input *input,
>>> + struct v4l2_subdev_state *state)
>>> +{
>>> + const struct mtk_seninf_format_info *fmtinfo;
>>> + const struct v4l2_mbus_framefmt *format;
>>> + unsigned int num_data_lanes = input->bus.num_data_lanes;
>>> + unsigned int val = 0;
>>> +
>>> + format = v4l2_subdev_state_get_stream_format(state, input->pad, 0);
>>> + fmtinfo = mtk_seninf_format_info(format->code);
>>> +
>>> + /* Configure timestamp */
>>> + writel(SENINF_TIMESTAMP_STEP, input->base + SENINF_TG1_TM_STP);
>>> +
>>> + /* HQ */
>>> + writel(0x0, input->base + SENINF_TG1_PH_CNT);
>>
>> Zero means:
>> - Sensor master clock: ISP_CLK
>> - Sensor clock polarity: Rising edge
>> - Sensor reset deasserted
>> - Sensor powered up
>> - Pixel clock inversion disabled
>> - Sensor master clock polarity disabled
>> - Phase counter disabled
>>
>>> + writel(0x10001, input->base + SENINF_TG1_SEN_CK);
>>
>> Unroll this one... this is the TG1 sensor clock divider.
>>
>> CLKFL GENMASK(5, 0)
>> CLKRS GENMASK(13, 8)
>> CLKCNT GENMASK(21,16)
>>
>> Like this, I don't get what you're trying to set, because you're using a fixed
>> sensor clock rate, meaning that only a handful of camera sensors will be usable.
>>
>> Is this 8Mhz? 16? 24? what? :-)
>>
>> Two hints:
>> - sensor_clk = clk_get_rate(isp_clk) / (tg1_sen_ck_clkcnt + 1);
>> - int mtk_seninf_set_sensor_clk(u8 rate_mhz);
>>
>> Please :-)
>
> Hi Angelo,
>
> I think I get your point about not hardcoding the sensor rate, but I
> am not sure how to use
> a mtk_seninf_set_sensor_clk(u8 rate_mhz); function.
>
> Where would it be called? How is it exposed to the user?
>
As for where: setup, streaming start, resolution change (which may be covered
by streaming start anyway, as a change should be calling stop->start anyway).
And for the how is it exposed to the user - well, depends what you mean for user,
but it's all standard V4L2 API :-)
Last but not least, I can give you another hint....
struct media_entity *entity = (something_here);
struct media_pad *mpad;
struct v4l2_subdev *cam_subdev;
struct v4l2_ctrl *ctl;
s64 link_frequency, pixel_clock;
if (entity->pads[0].flags & MEDIA_PAD_FL_SINK)
return -E_NOT_A_CAMERA_SENSOR_WE_IGNORE_THIS_ONE;
pad = media_pad_remote_pad_first(&entity->pads[0]);
if (!pad)
return -ENOENT;
if (!is_media_entity_v4l2_subdev(pad->entity))
return -ENOENT;
if (pad->entity->function != MEDIA_ENT_F_CAM_SENSOR)
return -ENOENT;
cam_subdev = media_entity_to_v4l2_subdev(pad->entity);
ctl = v4l2_ctrl_find(subdev->ctrl_handler, V4L2_CID_PIXEL_RATE);
/* multiplier is usually bits per pixel, divider is usually num of lanes */
link_frequency = v4l2_get_link_freq(cam_subdev->ctrl_handler, multiplier, divider);
pixel_clock = v4l2_ctrl_g_ctrl_int64(ctl);
....now you know what the sensor wants, set the seninf sensor clock accordingly.
Cheers
Angelo
> Cheers
> Julien
>
>>
>>> +
>>> + /* First Enable Sensor interface and select pad (0x1a04_0200) */
>>> + mtk_seninf_input_update(input, SENINF_CTRL, SENINF_EN, 1);
>>> + mtk_seninf_input_update(input, SENINF_CTRL, PAD2CAM_DATA_SEL, SENINF_PAD_10BIT);
>>> + mtk_seninf_input_update(input, SENINF_CTRL, SENINF_SRC_SEL, 0);
>>> + mtk_seninf_input_update(input, SENINF_CTRL_EXT, SENINF_CSI2_IP_EN, 1);
>>> + mtk_seninf_input_update(input, SENINF_CTRL_EXT, SENINF_NCSI2_IP_EN, 0);
>>> +
>>> + /* DPCM Enable */
>>> + if (fmtinfo->flags & MTK_SENINF_FORMAT_DPCM)
>>> + val = SENINF_CSI2_DPCM_DI_2A_DPCM_EN;
>>> + else
>>> + val = SENINF_CSI2_DPCM_DI_30_DPCM_EN;
>>> + writel(val, input->base + SENINF_CSI2_DPCM);
>>> +
>>> + /* Settle delay */
>>> + mtk_seninf_input_update(input, SENINF_CSI2_LNRD_TIMING,
>>> + DATA_SETTLE_PARAMETER, SENINF_SETTLE_DELAY);
>>> +
>>> + /* HQ */
>>> + writel(0x10, input->base + SENINF_CSI2_LNRC_FSM);
>>
>> As far as I know, SENINF_CSI2_LNRC_FSM is a read-only register: this write will do
>> exactly nothing...
>>
>>> +
>>> + /* CSI2 control */
>>> + val = readl(input->base + SENINF_CSI2_CTL)
>>> + | (FIELD_PREP(SENINF_CSI2_CTL_ED_SEL, DATA_HEADER_ORDER_DI_WCL_WCH)
>>> + | SENINF_CSI2_CTL_CLOCK_LANE_EN | (BIT(num_data_lanes) - 1));
>>> + writel(val, input->base + SENINF_CSI2_CTL);
>>> +
>>> + mtk_seninf_input_update(input, SENINF_CSI2_RESYNC_MERGE_CTL,
>>> + BYPASS_LANE_RESYNC, 0);
>>
>> 93 columns: fits in one line (not only this one!).
>>
>>> + mtk_seninf_input_update(input, SENINF_CSI2_RESYNC_MERGE_CTL, CDPHY_SEL, 0);
>>> + mtk_seninf_input_update(input, SENINF_CSI2_RESYNC_MERGE_CTL,
>>> + CPHY_LANE_RESYNC_CNT, 3);
>>> + mtk_seninf_input_update(input, SENINF_CSI2_MODE, CSR_CSI2_MODE, 0);
>>> + mtk_seninf_input_update(input, SENINF_CSI2_MODE, CSR_CSI2_HEADER_LEN, 0);
>>> + mtk_seninf_input_update(input, SENINF_CSI2_DPHY_SYNC, SYNC_SEQ_MASK_0, 0xff00);
>>> + mtk_seninf_input_update(input, SENINF_CSI2_DPHY_SYNC, SYNC_SEQ_PAT_0, 0x001d);
>>> +
>>> + mtk_seninf_input_update(input, SENINF_CSI2_CTL, CLOCK_HS_OPTION, 0);
>>> + mtk_seninf_input_update(input, SENINF_CSI2_CTL, HSRX_DET_EN, 0);
>>> + mtk_seninf_input_update(input, SENINF_CSI2_CTL, HS_TRAIL_EN, 1);
>>> + mtk_seninf_input_update(input, SENINF_CSI2_HS_TRAIL, HS_TRAIL_PARAMETER,
>>> + SENINF_HS_TRAIL_PARAMETER);
>>> +
>>> + /* Set debug port to output packet number */
>>> + mtk_seninf_input_update(input, SENINF_CSI2_DGB_SEL, DEBUG_EN, 1);
>>> + mtk_seninf_input_update(input, SENINF_CSI2_DGB_SEL, DEBUG_SEL, 0x1a);
>>> +
>>> + /* HQ */
>>> + writel(0xfffffffe, input->base + SENINF_CSI2_SPARE0);
>>
>> I have no idea what this SPARE0 does, but I think that this is something that you
>> want to get from platform_data, as I guess this would be different on various SoCs?
>>
>>> +
>>> + /* Enable CSI2 IRQ mask */
>>> + /* Turn on all interrupt */
>>> + writel(0xffffffff, input->base + SENINF_CSI2_INT_EN);
>>> + /* Write clear CSI2 IRQ */
>>> + writel(0xffffffff, input->base + SENINF_CSI2_INT_STATUS);
>>> + /* Enable CSI2 Extend IRQ mask */
>>
>> You missed:
>> writel(0xffffffff, input->base + SENINF_CSI2_INT_EN_EXT);
>>
>> P.S.: #define SENINF_CSI2_INT_EN_EXT 0x0b10
>>
>>
>>> + /* Turn on all interrupt */
>>
>> /* Reset the CSI2 to commit changes */ <-- makes more sense, doesn't it?
>>
>>> + mtk_seninf_input_update(input, SENINF_CTRL, CSI2_SW_RST, 1);
>>> + udelay(1);
>>> + mtk_seninf_input_update(input, SENINF_CTRL, CSI2_SW_RST, 0);
>>> +}
>>> +
>>> +static void mtk_seninf_mux_setup(struct mtk_seninf_mux *mux,
>>> + struct mtk_seninf_input *input,
>>> + struct v4l2_subdev_state *state)
>>> +{
>>> + const struct mtk_seninf_format_info *fmtinfo;
>>> + const struct v4l2_mbus_framefmt *format;
>>> + unsigned int pix_sel_ext;
>>> + unsigned int pix_sel;
>>> + unsigned int hs_pol = 0;
>>> + unsigned int vs_pol = 0;
>>> + unsigned int val;
>>> + u32 rst_mask;
>>> +
>>> + format = v4l2_subdev_state_get_stream_format(state, input->pad, 0);
>>> + fmtinfo = mtk_seninf_format_info(format->code);
>>> +
>>> + /* Enable mux */
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, SENINF_MUX_EN, 1);
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, SENINF_SRC_SEL, SENINF_MIPI_SENSOR);
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL_EXT, SENINF_SRC_SEL_EXT, SENINF_NORMAL_MODEL);
>>> +
>>> + pix_sel_ext = 0;
>>> + pix_sel = 1;
>>
>>
>> pixels_per_cycle = 1;
>> bus_width = pixels_per_cycle >> 1;
>>
>> because: 0 == 1pix/cyc, 1 == 2pix/cyc, 2 == 4pix/cyc, 3 == 8pix... etc
>> ...but the width of this register depends on the SoC, so you also want to set
>> constraints to the bus width on a per-soc basis (platform data again, or at
>> least leave a comment here).
>>
>> mtk_seninf_mux_update(.... PIX_SEL_EXT, bus_width);
>> mtk_seninf_mux_update(.... PIX_SEL, bus_width);
>>
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL_EXT, SENINF_PIX_SEL_EXT, pix_sel_ext);
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, SENINF_PIX_SEL, pix_sel);
>>> +
>>> + if (fmtinfo->flags & MTK_SENINF_FORMAT_JPEG) {
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, FIFO_FULL_WR_EN, 0);
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, FIFO_FLUSH_EN,
>>> + FIFO_FLUSH_EN_JPEG_2_PIXEL_MODE);
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, FIFO_PUSH_EN,
>>> + FIFO_PUSH_EN_JPEG_2_PIXEL_MODE);
>>> + } else {
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, FIFO_FULL_WR_EN, 2);
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, FIFO_FLUSH_EN,
>>> + FIFO_FLUSH_EN_NORMAL_MODE);
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, FIFO_PUSH_EN,
>>> + FIFO_PUSH_EN_NORMAL_MODE);
>>> + }
>>> +
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, SENINF_HSYNC_POL, hs_pol);
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, SENINF_VSYNC_POL, vs_pol);
>>> +
>>> + val = mtk_seninf_mux_read(mux, SENINF_MUX_CTRL);
>>> + rst_mask = SENINF_MUX_CTRL_SENINF_IRQ_SW_RST | SENINF_MUX_CTRL_SENINF_MUX_SW_RST;
>>> +
>>> + mtk_seninf_mux_write(mux, SENINF_MUX_CTRL, val | rst_mask);
>>
>> Are you sure that you don't need any wait between assertion and deassertion of RST?
>> Looks strange, but I don't really know then.
>>
>>> + mtk_seninf_mux_write(mux, SENINF_MUX_CTRL, val & ~rst_mask);
>>> +
>>> + /* HQ */
>>> + mtk_seninf_mux_write(mux, SENINF_MUX_SPARE, 0xc2000);
>>
>> val = SENINF_FIFO_FULL_SEL;
>>
>> /* SPARE field meaning is unknown */
>> val |= 0xc0000;
>>
>> mtk_seninf_mux_write(mux, SENINF_MUX_SPARE, val);
>>
>>> +}
>>> +
>>> +static void mtk_seninf_top_mux_setup(struct mtk_seninf *priv,
>>> + enum mtk_seninf_id seninf_id,
>>> + struct mtk_seninf_mux *mux)
>>> +{
>>> + unsigned int val;
>>> +
>>> + /*
>>> + * Use the top mux (from SENINF input to MUX) to configure routing, and
>>> + * hardcode a 1:1 mapping from the MUX instances to the SENINF outputs.
>>> + */
>>> + val = readl(priv->base + SENINF_TOP_MUX_CTRL)
>>> + & ~(0xf << (mux->mux_id * 4));
>>> + val |= (seninf_id & 0xf) << (mux->mux_id * 4);
>>> + writel(val, priv->base + SENINF_TOP_MUX_CTRL);
>>> +
>>> + writel(0x76541010, priv->base + SENINF_TOP_CAM_MUX_CTRL);
>>
>> Each four bits of TOP_CAM_MUX_CTRL selects between seninf1 to seninf8 muxes, and
>> TOP_MUX_CTRL is laid out in the very same way.
>>
>> This means that if you're calculating a value for TOP_MUX_CTRL, you can do exactly
>> the same for TOP_CAM_MUX_CTRL.
>>
>>> +}
>>> +
>>> +static void seninf_enable_test_pattern(struct mtk_seninf *priv,
>>> + struct v4l2_subdev_state *state)
>>> +{
>>> + struct mtk_seninf_input *input = &priv->inputs[CSI_PORT_0];
>>> + struct mtk_seninf_mux *mux = &priv->muxes[0];
>>> + const struct mtk_seninf_format_info *fmtinfo;
>>> + const struct v4l2_mbus_framefmt *format;
>>> + unsigned int val;
>>> + unsigned int pix_sel_ext;
>>> + unsigned int pix_sel;
>>> + unsigned int hs_pol = 0;
>>> + unsigned int vs_pol = 0;
>>> + unsigned int seninf = 0;
>>> + unsigned int tm_size = 0;
>>> + unsigned int mux_id = mux->mux_id;
>>> +
>>> + format = v4l2_subdev_state_get_stream_format(state, priv->conf->nb_inputs, 0);
>>> + fmtinfo = mtk_seninf_format_info(format->code);
>>> +
>>> + mtk_seninf_update(priv, SENINF_TOP_CTRL, MUX_LP_MODE, 0);
>>> +
>>> + mtk_seninf_update(priv, SENINF_TOP_CTRL, SENINF_PCLK_EN, 1);
>>> + mtk_seninf_update(priv, SENINF_TOP_CTRL, SENINF2_PCLK_EN, 1);
>>> +
>>> + mtk_seninf_input_update(input, SENINF_CTRL, SENINF_EN, 1);
>>> + mtk_seninf_input_update(input, SENINF_CTRL, SENINF_SRC_SEL, 1);
>>> + mtk_seninf_input_update(input, SENINF_CTRL_EXT,
>>> + SENINF_TESTMDL_IP_EN, 1);
>>> +
>>> + mtk_seninf_input_update(input, SENINF_TG1_TM_CTL, TM_EN, 1);
>>> + mtk_seninf_input_update(input, SENINF_TG1_TM_CTL, TM_PAT, 0xc);
>>> + mtk_seninf_input_update(input, SENINF_TG1_TM_CTL, TM_VSYNC, 4);
>>> + mtk_seninf_input_update(input, SENINF_TG1_TM_CTL, TM_DUMMYPXL, 0x28);
>>> +
>>> + if (fmtinfo->flags & MTK_SENINF_FORMAT_BAYER)
>>> + mtk_seninf_input_update(input, SENINF_TG1_TM_CTL, TM_FMT, 0x0);
>>> + else
>>> + mtk_seninf_input_update(input, SENINF_TG1_TM_CTL, TM_FMT, 0x1);
>>> +
>>> + tm_size = FIELD_PREP(SENINF_TG1_TM_SIZE_TM_LINE, format->height + 8);
>>> + switch (format->code) {
>>> + case MEDIA_BUS_FMT_UYVY8_1X16:
>>> + case MEDIA_BUS_FMT_VYUY8_1X16:
>>> + case MEDIA_BUS_FMT_YUYV8_1X16:
>>> + case MEDIA_BUS_FMT_YVYU8_1X16:
>>> + tm_size |= FIELD_PREP(SENINF_TG1_TM_SIZE_TM_PXL, format->width * 2);
>>> + break;
>>> + default:
>>> + tm_size |= FIELD_PREP(SENINF_TG1_TM_SIZE_TM_PXL, format->width);
>>> + break;
>>> + }
>>> + writel(tm_size, input->base + SENINF_TG1_TM_SIZE);
>>> +
>>> + writel(TEST_MODEL_CLK_DIVIDED_CNT, input->base + SENINF_TG1_TM_CLK);
>>> + writel(TIME_STAMP_DIVIDER, input->base + SENINF_TG1_TM_STP);
>>> +
>>> + /* Set top mux */
>>> + val = (readl(priv->base + SENINF_TOP_MUX_CTRL) & (~(0xf << (mux_id * 4)))) |
>>> + ((seninf & 0xf) << (mux_id * 4));
>>> + writel(val, priv->base + SENINF_TOP_MUX_CTRL);
>>
>> This is duplicated, and it is the same that you have in mtk_seninf_top_mux_setup()
>>
>>> +
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, SENINF_MUX_EN, 1);
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL_EXT,
>>> + SENINF_SRC_SEL_EXT, SENINF_TEST_MODEL);
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, SENINF_SRC_SEL, 1);
>>> +
>>> + pix_sel_ext = 0;
>>> + pix_sel = 1;
>>> +
>>
>> This is in mtk_seninf_mux_setup(), but if you apply my suggestion, it won't be in
>> there anymore, so you'll call a function here to set the right value :-)
>>
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL_EXT,
>>> + SENINF_PIX_SEL_EXT, pix_sel_ext);
>>> +
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, SENINF_PIX_SEL, pix_sel);
>>> +
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, FIFO_PUSH_EN, 0x1f);
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, FIFO_FLUSH_EN, 0x1b);
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, FIFO_FULL_WR_EN, 2);
>>> +
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, SENINF_HSYNC_POL, hs_pol);
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, SENINF_VSYNC_POL, vs_pol);
>>> + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, SENINF_HSYNC_MASK, 1);
>>> +
>>> + mtk_seninf_mux_write(mux, SENINF_MUX_INTEN,
>>> + SENINF_IRQ_CLR_SEL | SENINF_ALL_ERR_IRQ_EN);
>>> +
>>> + mtk_seninf_mux_write(mux, SENINF_MUX_CTRL,
>>> + mtk_seninf_mux_read(mux, SENINF_MUX_CTRL) |
>>> + SENINF_MUX_CTRL_SENINF_IRQ_SW_RST |
>>> + SENINF_MUX_CTRL_SENINF_MUX_SW_RST);
>>> + udelay(1);
>>> + mtk_seninf_mux_write(mux, SENINF_MUX_CTRL,
>>> + mtk_seninf_mux_read(mux, SENINF_MUX_CTRL) &
>>> + ~(SENINF_MUX_CTRL_SENINF_IRQ_SW_RST |
>>> + SENINF_MUX_CTRL_SENINF_MUX_SW_RST));
>>> +
>>> + //check this
>>> + writel(0x76540010, priv->base + SENINF_TOP_CAM_MUX_CTRL);
>>> +
>>> + dev_dbg(priv->dev, "%s: OK\n", __func__);
>>> +}
>>> +
>>
>> Cheers,
>> Angelo
>>
Powered by blists - more mailing lists