| 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
| ||
|
Message-ID: <87a31d4e-b604-4b9a-92e9-75fd2748489a@linaro.org>
Date: Tue, 13 Jan 2026 18:27:57 +0200
From: Vladimir Zapolskiy <vladimir.zapolskiy@...aro.org>
To: Wenmeng Liu <wenmeng.liu@....qualcomm.com>, Robert Foss
<rfoss@...nel.org>, Todor Tomov <todor.too@...il.com>,
Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
Mauro Carvalho Chehab <mchehab@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-media@...r.kernel.org,
linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH v8 1/3] media: qcom: camss: Add common TPG support
Hello Wenmeng.
On 1/13/26 11:03, Wenmeng Liu wrote:
> Introduce a new common Test Pattern Generator (TPG) implementation for
> Qualcomm CAMSS. This module provides a generic interface for pattern
> generation that can be reused by multiple platforms.
>
> Unlike CSID-integrated TPG, this TPG acts as a standalone block
> that emulates both CSIPHY and sensor behavior, enabling flexible test
> patterns without external hardware.
>
> Signed-off-by: Wenmeng Liu <wenmeng.liu@....qualcomm.com>
> ---
> drivers/media/platform/qcom/camss/Makefile | 1 +
> drivers/media/platform/qcom/camss/camss-tpg.c | 710 ++++++++++++++++++++++++++
> drivers/media/platform/qcom/camss/camss-tpg.h | 127 +++++
> drivers/media/platform/qcom/camss/camss.h | 5 +
> 4 files changed, 843 insertions(+)
>
> diff --git a/drivers/media/platform/qcom/camss/Makefile b/drivers/media/platform/qcom/camss/Makefile
> index 5e349b4915130c71dbff90e73102e46dfede1520..d355e67c25700ac061b878543c32ed8defc03ad0 100644
> --- a/drivers/media/platform/qcom/camss/Makefile
> +++ b/drivers/media/platform/qcom/camss/Makefile
> @@ -27,5 +27,6 @@ qcom-camss-objs += \
> camss-vfe.o \
> camss-video.o \
> camss-format.o \
> + camss-tpg.o \
While you're here, please sort and keep the lines in alphabetical order.
>
> obj-$(CONFIG_VIDEO_QCOM_CAMSS) += qcom-camss.o
> diff --git a/drivers/media/platform/qcom/camss/camss-tpg.c b/drivers/media/platform/qcom/camss/camss-tpg.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..f4c015aafa202e5b64fafa3c543128fda6440b11
> --- /dev/null
> +++ b/drivers/media/platform/qcom/camss/camss-tpg.c
> @@ -0,0 +1,710 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + *
> + * Qualcomm MSM Camera Subsystem - TPG Module
> + *
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <media/media-entity.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-subdev.h>
> +
> +#include "camss-tpg.h"
> +#include "camss.h"
> +
> +const char * const testgen_payload_modes[] = {
> + "Disabled",
> + "Incrementing",
> + "Alternating 0x55/0xAA",
> + "Reserved",
> + "Reserved",
> + "Pseudo-random Data",
> + "User Specified",
> + "Reserved",
> + "Reserved",
> + "Color bars",
> + "Reserved"
It makes little sense to mention the unsupported values, and then
introduce enum tpg_testgen_mode to list the supported ones.
> +};
Are these test pattern modes specific to TPG Gen1 only?
CSID TPG uses a different 'csid_testgen_modes' list, and if the list above is
TPG Gen1 specific, it would make sense to place it right in camss-tpg-gen1.c
> +
> +static const struct tpg_format_info formats_gen1[] = {
> + {
> + MEDIA_BUS_FMT_SBGGR8_1X8,
> + DATA_TYPE_RAW_8BIT,
Please replace it with MIPI_CSI2_DT_RAW8
> + ENCODE_FORMAT_UNCOMPRESSED_8_BIT,
> + 8,
> + },
> + {
> + MEDIA_BUS_FMT_SGBRG8_1X8,
> + DATA_TYPE_RAW_8BIT,
> + ENCODE_FORMAT_UNCOMPRESSED_8_BIT,
> + 8,
> + },
> + {
> + MEDIA_BUS_FMT_SGRBG8_1X8,
> + DATA_TYPE_RAW_8BIT,
> + ENCODE_FORMAT_UNCOMPRESSED_8_BIT,
> + 8,
> + },
> + {
> + MEDIA_BUS_FMT_SRGGB8_1X8,
> + DATA_TYPE_RAW_8BIT,
> + ENCODE_FORMAT_UNCOMPRESSED_8_BIT,
> + 8,
> + },
> + {
> + MEDIA_BUS_FMT_SBGGR10_1X10,
> + DATA_TYPE_RAW_10BIT,
MIPI_CSI2_DT_RAW10
> + ENCODE_FORMAT_UNCOMPRESSED_10_BIT,
> + 10,
> + },
> + {
> + MEDIA_BUS_FMT_SGBRG10_1X10,
> + DATA_TYPE_RAW_10BIT,
> + ENCODE_FORMAT_UNCOMPRESSED_10_BIT,
> + 10,
> + },
> + {
> + MEDIA_BUS_FMT_SGRBG10_1X10,
> + DATA_TYPE_RAW_10BIT,
> + ENCODE_FORMAT_UNCOMPRESSED_10_BIT,
> + 10,
> + },
> + {
> + MEDIA_BUS_FMT_SRGGB10_1X10,
> + DATA_TYPE_RAW_10BIT,
> + ENCODE_FORMAT_UNCOMPRESSED_10_BIT,
> + 10,
> + },
> + {
> + MEDIA_BUS_FMT_SBGGR12_1X12,
> + DATA_TYPE_RAW_12BIT,
MIPI_CSI2_DT_RAW12
> + ENCODE_FORMAT_UNCOMPRESSED_12_BIT,
> + 12,
> + },
> + {
> + MEDIA_BUS_FMT_SGBRG12_1X12,
> + DATA_TYPE_RAW_12BIT,
> + ENCODE_FORMAT_UNCOMPRESSED_12_BIT,
> + 12,
> + },
> + {
> + MEDIA_BUS_FMT_SGRBG12_1X12,
> + DATA_TYPE_RAW_12BIT,
> + ENCODE_FORMAT_UNCOMPRESSED_12_BIT,
> + 12,
> + },
> + {
> + MEDIA_BUS_FMT_SRGGB12_1X12,
> + DATA_TYPE_RAW_12BIT,
> + ENCODE_FORMAT_UNCOMPRESSED_12_BIT,
> + 12,
> + },
> + {
> + MEDIA_BUS_FMT_Y8_1X8,
> + DATA_TYPE_RAW_8BIT,
> + ENCODE_FORMAT_UNCOMPRESSED_8_BIT,
> + 8,
> + },
> + {
> + MEDIA_BUS_FMT_Y10_1X10,
> + DATA_TYPE_RAW_10BIT,
> + ENCODE_FORMAT_UNCOMPRESSED_10_BIT,
> + 10,
> + },
> +};
> +
> +const struct tpg_formats tpg_formats_gen1 = {
> + .nformats = ARRAY_SIZE(formats_gen1),
> + .formats = formats_gen1
> +};
> +
> +const struct tpg_format_info *tpg_get_fmt_entry(struct tpg_device *tpg,
> + const struct tpg_format_info *formats,
> + unsigned int nformats,
> + u32 code)
> +{
> + struct device *dev = tpg->camss->dev;
> + size_t i;
unsigned int i, size_t is very unexpected to get here.
> +
> + for (i = 0; i < nformats; i++)
> + if (code == formats[i].code)
> + return &formats[i];
> +
> + dev_warn(dev, "Unknown pixel format code=0x%08x\n", code);
Please remove dev_warn() completely, it opens a way to flood the kernel log.
> +
> + return ERR_PTR(-EINVAL);
> +}
> +
> +/*
> + * tpg_set_clock_rates - set clock rates on tpg module
> + * @tpg: tpg device
> + */
> +static int tpg_set_clock_rates(struct tpg_device *tpg)
> +{
> + struct device *dev = tpg->camss->dev;
> + int ret;
> + int i;
> +
> + for (i = 0; i < tpg->nclocks; i++) {
> + struct camss_clock *clock = &tpg->clock[i];
> + long round_rate;
> +
> + if (clock->freq) {
> + round_rate = clk_round_rate(clock->clk, clock->freq[0]);
> + if (round_rate < 0) {
> + dev_err(dev, "clk round rate failed: %ld\n",
> + round_rate);
> + return -EINVAL;
> + }
> +
> + ret = clk_set_rate(clock->clk, round_rate);
> + if (ret < 0) {
> + dev_err(dev, "clk set rate failed: %d\n", ret);
> + return ret;
> + }
> + }
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * tpg_set_power - Power on/off tpg module
> + * @sd: tpg V4L2 subdevice
> + * @on: Requested power state
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +static int tpg_set_power(struct v4l2_subdev *sd, int on)
> +{
> + struct tpg_device *tpg = v4l2_get_subdevdata(sd);
> + struct device *dev = tpg->camss->dev;
> +
> + if (on) {
> + int ret;
> +
> + ret = pm_runtime_resume_and_get(dev);
> + if (ret < 0)
> + return ret;
> +
> + ret = tpg_set_clock_rates(tpg);
> + if (ret < 0) {
> + pm_runtime_put_sync(dev);
> + return ret;
> + }
> +
> + ret = camss_enable_clocks(tpg->nclocks, tpg->clock, dev);
> + if (ret < 0) {
> + pm_runtime_put_sync(dev);
> + return ret;
> + }
> +
> + tpg->res->hw_ops->reset(tpg);
> +
> + tpg->res->hw_ops->hw_version(tpg);
> + } else {
> + camss_disable_clocks(tpg->nclocks, tpg->clock);
> +
> + pm_runtime_put_sync(dev);
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * tpg_set_stream - Enable/disable streaming on tpg module
> + * @sd: tpg V4L2 subdevice
> + * @enable: Requested streaming state
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +static int tpg_set_stream(struct v4l2_subdev *sd, int enable)
> +{
> + struct tpg_device *tpg = v4l2_get_subdevdata(sd);
> + int ret = 0;
> +
> + if (enable) {
> + ret = v4l2_ctrl_handler_setup(&tpg->ctrls);
> + if (ret < 0) {
> + dev_err(tpg->camss->dev,
> + "could not sync v4l2 controls: %d\n", ret);
> + return ret;
> + }
> + }
> +
> + ret = tpg->res->hw_ops->configure_stream(tpg, enable);
> +
> + return ret;
> +}
> +
> +/*
> + * __tpg_get_format - Get pointer to format structure
> + * @tpg: tpg device
> + * @cfg: V4L2 subdev pad configuration
There is no such function argument. There are much more errors in
the doxygen descriptions of functions, please remove all these
doxygen comments, they do not bring anything valuable here.
> + * @pad: pad from which format is requested
> + * @which: TRY or ACTIVE format
> + *
> + * Return pointer to TRY or ACTIVE format structure
> + */
> +static struct v4l2_mbus_framefmt *
> +__tpg_get_format(struct tpg_device *tpg,
> + struct v4l2_subdev_state *sd_state,
> + unsigned int pad,
> + enum v4l2_subdev_format_whence which)
> +{
> + if (which == V4L2_SUBDEV_FORMAT_TRY)
> + return v4l2_subdev_state_get_format(sd_state,
> + pad);
> +
> + return &tpg->fmt[pad];
> +}
> +
> +/*
> + * tpg_try_format - Handle try format by pad subdev method
> + * @tpg: tpg device
> + * @cfg: V4L2 subdev pad configuration
No such argument.
> + * @pad: pad on which format is requested
> + * @fmt: pointer to v4l2 format structure
> + * @which: wanted subdev format
> + */
> +static void tpg_try_format(struct tpg_device *tpg,
> + struct v4l2_subdev_state *sd_state,
> + unsigned int pad,
> + struct v4l2_mbus_framefmt *fmt,
> + enum v4l2_subdev_format_whence which)
> +{
> + unsigned int i;
> +
> + switch (pad) {
> + case MSM_TPG_PAD_SINK:
> + for (i = 0; i < tpg->res->formats->nformats; i++)
> + if (tpg->res->formats->formats[i].code == fmt->code)
> + break;
> +
> + /* If not found, use SBGGR8 as default */
> + if (i >= tpg->res->formats->nformats)
> + fmt->code = MEDIA_BUS_FMT_SBGGR8_1X8;
> +
> + fmt->width = clamp_t(u32, fmt->width, 1, 8191);
> + fmt->height = clamp_t(u32, fmt->height, 1, 8191);
> +
> + fmt->field = V4L2_FIELD_NONE;
> + fmt->colorspace = V4L2_COLORSPACE_SRGB;
> +
> + break;
> + case MSM_TPG_PAD_SRC:
> + *fmt = *__tpg_get_format(tpg, sd_state,
> + MSM_TPG_PAD_SINK,
> + which);
> +
> + break;
> + }
> +}
> +
> +/*
> + * tpg_enum_mbus_code - Handle format enumeration
> + * @sd: tpg V4L2 subdevice
> + * @cfg: V4L2 subdev pad configuration
No such argument.
> + * @code: pointer to v4l2_subdev_mbus_code_enum structure
> + * return -EINVAL or zero on success
> + */
> +static int tpg_enum_mbus_code(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_mbus_code_enum *code)
> +{
> + struct tpg_device *tpg = v4l2_get_subdevdata(sd);
> + struct v4l2_mbus_framefmt *format;
> +
> + if (code->pad == MSM_TPG_PAD_SINK) {
> + if (code->index >= tpg->res->formats->nformats)
> + return -EINVAL;
> +
> + code->code = tpg->res->formats->formats[code->index].code;
> + } else {
> + if (code->index > 0)
> + return -EINVAL;
> +
> + format = __tpg_get_format(tpg, sd_state,
> + MSM_TPG_PAD_SINK,
> + code->which);
> +
> + code->code = format->code;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * tpg_enum_frame_size - Handle frame size enumeration
> + * @sd: tpg V4L2 subdevice
> + * @cfg: V4L2 subdev pad configuration
> + * @fse: pointer to v4l2_subdev_frame_size_enum structure
> + * return -EINVAL or zero on success
> + */
> +static int tpg_enum_frame_size(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_frame_size_enum *fse)
> +{
> + struct tpg_device *tpg = v4l2_get_subdevdata(sd);
> + struct v4l2_mbus_framefmt format;
> +
> + if (fse->index != 0)
> + return -EINVAL;
> +
> + format.code = fse->code;
> + format.width = 1;
> + format.height = 1;
> + tpg_try_format(tpg, sd_state, fse->pad, &format, fse->which);
> + fse->min_width = format.width;
> + fse->min_height = format.height;
> +
> + if (format.code != fse->code)
> + return -EINVAL;
> +
> + format.code = fse->code;
> + format.width = -1;
> + format.height = -1;
format.width and format.height are rewritten in the tpg_try_format(),
so it makes no sense to assign them.
The problem is that for whatever reason you can tpg_try_format() twice
in a raw, it looks wrong, and I'm certain you can modify the functions
so that only one call would be needed.
> + tpg_try_format(tpg, sd_state, fse->pad, &format, fse->which);
> + fse->max_width = format.width;
> + fse->max_height = format.height;
> +
> + return 0;
> +}
> +
> +/*
> + * tpg_get_format - Handle get format by pads subdev method
> + * @sd: tpg V4L2 subdevice
> + * @cfg: V4L2 subdev pad configuration
> + * @fmt: pointer to v4l2 subdev format structure
> + *
> + * Return -EINVAL or zero on success
> + */
> +static int tpg_get_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct tpg_device *tpg = v4l2_get_subdevdata(sd);
> + struct v4l2_mbus_framefmt *format;
> +
> + format = __tpg_get_format(tpg, sd_state, fmt->pad, fmt->which);
> + if (!format)
> + return -EINVAL;
> +
> + fmt->format = *format;
> +
> + return 0;
> +}
> +
> +/*
> + * tpg_set_format - Handle set format by pads subdev method
> + * @sd: tpg V4L2 subdevice
> + * @cfg: V4L2 subdev pad configuration
> + * @fmt: pointer to v4l2 subdev format structure
> + *
> + * Return -EINVAL or zero on success
> + */
> +static int tpg_set_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct tpg_device *tpg = v4l2_get_subdevdata(sd);
> + struct v4l2_mbus_framefmt *format;
> +
> + format = __tpg_get_format(tpg, sd_state, fmt->pad, fmt->which);
> + if (!format)
> + return -EINVAL;
> +
> + tpg_try_format(tpg, sd_state, fmt->pad, &fmt->format,
> + fmt->which);
> + *format = fmt->format;
> +
> + if (fmt->pad == MSM_TPG_PAD_SINK) {
> + format = __tpg_get_format(tpg, sd_state,
> + MSM_TPG_PAD_SRC,
> + fmt->which);
> +
> + *format = fmt->format;
> + tpg_try_format(tpg, sd_state, MSM_TPG_PAD_SRC,
> + format,
> + fmt->which);
> + }
> + return 0;
> +}
> +
> +/*
> + * tpg_init_formats - Initialize formats on all pads
> + * @sd: tpg V4L2 subdevice
> + * @fh: V4L2 subdev file handle
> + *
> + * Initialize all pad formats with default values.
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +static int tpg_init_formats(struct v4l2_subdev *sd,
> + struct v4l2_subdev_fh *fh)
> +{
> + struct v4l2_subdev_format format = {
> + .pad = MSM_TPG_PAD_SINK,
> + .which = fh ? V4L2_SUBDEV_FORMAT_TRY :
> + V4L2_SUBDEV_FORMAT_ACTIVE,
> + .format = {
> + .code = MEDIA_BUS_FMT_SBGGR8_1X8,
> + .width = 1920,
> + .height = 1080
> + }
> + };
> +
> + return tpg_set_format(sd, fh ? fh->state : NULL, &format);
> +}
> +
> +/*
> + * tpg_set_test_pattern - Set test generator's pattern mode
> + * @tpg: TPG device
> + * @value: desired test pattern mode
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +static int tpg_set_test_pattern(struct tpg_device *tpg, s32 value)
> +{
> + return tpg->res->hw_ops->configure_testgen_pattern(tpg, value);
> +}
> +
> +/*
> + * tpg_s_ctrl - Handle set control subdev method
> + * @ctrl: pointer to v4l2 control structure
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +static int tpg_s_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct tpg_device *tpg = container_of(ctrl->handler,
> + struct tpg_device, ctrls);
> + int ret = -EINVAL;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_TEST_PATTERN:
> + ret = tpg_set_test_pattern(tpg, ctrl->val);
> + break;
> + }
> +
> + return ret;
> +}
> +
> +static const struct v4l2_ctrl_ops tpg_ctrl_ops = {
> + .s_ctrl = tpg_s_ctrl,
> +};
> +
> +/*
> + * msm_tpg_subdev_init - Initialize tpg device structure and resources
> + * @tpg: tpg device
And no 'camss' argument. The whole function description comment is quite
useless, it can be just removed with no losses.
> + * @res: tpg module resources table
> + * @id: tpg module id
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +int msm_tpg_subdev_init(struct camss *camss,
> + struct tpg_device *tpg,
> + const struct camss_subdev_resources *res, u8 id)
> +{
> + struct platform_device *pdev;
> + struct device *dev;
> + int i, j;
> +
> + dev = camss->dev;
> + pdev = to_platform_device(dev);
> +
> + tpg->camss = camss;
> + tpg->id = id;
> + tpg->res = &res->tpg;
> + tpg->res->hw_ops->subdev_init(tpg);
> +
> + tpg->base = devm_platform_ioremap_resource_byname(pdev, res->reg[0]);
> + if (IS_ERR(tpg->base))
> + return PTR_ERR(tpg->base);
> +
> + tpg->nclocks = 0;
> + while (res->clock[tpg->nclocks])
> + tpg->nclocks++;
> +
> + if (tpg->nclocks) {
if (!tpg->nclocks)
return 0;
> + tpg->clock = devm_kcalloc(dev,
> + tpg->nclocks, sizeof(*tpg->clock),
> + GFP_KERNEL);
> + if (!tpg->clock)
> + return -ENOMEM;
> +
> + for (i = 0; i < tpg->nclocks; i++) {
> + struct camss_clock *clock = &tpg->clock[i];
> +
> + clock->clk = devm_clk_get(dev, res->clock[i]);
> + if (IS_ERR(clock->clk))
> + return PTR_ERR(clock->clk);
> +
> + clock->name = res->clock[i];
> +
> + clock->nfreqs = 0;
> + while (res->clock_rate[i][clock->nfreqs])
> + clock->nfreqs++;
> +
> + if (!clock->nfreqs) {
> + clock->freq = NULL;
> + continue;
> + }
> +
> + clock->freq = devm_kcalloc(dev,
> + clock->nfreqs,
> + sizeof(*clock->freq),
> + GFP_KERNEL);
> + if (!clock->freq)
> + return -ENOMEM;
> +
> + for (j = 0; j < clock->nfreqs; j++)
> + clock->freq[j] = res->clock_rate[i][j];
> + }
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * tpg_link_setup - Setup tpg connections
> + * @entity: Pointer to media entity structure
> + * @local: Pointer to local pad
> + * @remote: Pointer to remote pad
> + * @flags: Link flags
> + *
> + * Return 0 on success
> + */
> +static int tpg_link_setup(struct media_entity *entity,
> + const struct media_pad *local,
> + const struct media_pad *remote, u32 flags)
> +{
> + if (flags & MEDIA_LNK_FL_ENABLED)
> + if (media_pad_remote_pad_first(local))
> + return -EBUSY;
> +
> + return 0;
> +}
> +
> +static const struct v4l2_subdev_core_ops tpg_core_ops = {
> + .s_power = tpg_set_power,
> +};
> +
> +static const struct v4l2_subdev_video_ops tpg_video_ops = {
> + .s_stream = tpg_set_stream,
> +};
> +
> +static const struct v4l2_subdev_pad_ops tpg_pad_ops = {
> + .enum_mbus_code = tpg_enum_mbus_code,
> + .enum_frame_size = tpg_enum_frame_size,
> + .get_fmt = tpg_get_format,
> + .set_fmt = tpg_set_format,
> +};
> +
> +static const struct v4l2_subdev_ops tpg_v4l2_ops = {
> + .core = &tpg_core_ops,
> + .video = &tpg_video_ops,
> + .pad = &tpg_pad_ops,
> +};
> +
> +static const struct v4l2_subdev_internal_ops tpg_v4l2_internal_ops = {
> + .open = tpg_init_formats,
> +};
> +
> +static const struct media_entity_operations tpg_media_ops = {
> + .link_setup = tpg_link_setup,
> + .link_validate = v4l2_subdev_link_validate,
> +};
> +
> +/*
> + * msm_tpg_register_entity - Register subdev node for tpg module
> + * @tpg: tpg device
> + * @v4l2_dev: V4L2 device
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +int msm_tpg_register_entity(struct tpg_device *tpg,
> + struct v4l2_device *v4l2_dev)
> +{
> + struct v4l2_subdev *sd = &tpg->subdev;
> + struct media_pad *pads = tpg->pads;
> + struct device *dev = tpg->camss->dev;
> + int ret;
> +
> + v4l2_subdev_init(sd, &tpg_v4l2_ops);
> + sd->internal_ops = &tpg_v4l2_internal_ops;
> + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> + V4L2_SUBDEV_FL_HAS_EVENTS;
> + snprintf(sd->name, ARRAY_SIZE(sd->name), "%s%d",
> + MSM_TPG_NAME, tpg->id);
> + sd->grp_id = TPG_GUP_ID;
> + v4l2_set_subdevdata(sd, tpg);
> +
> + ret = v4l2_ctrl_handler_init(&tpg->ctrls, 1);
> + if (ret < 0) {
> + dev_err(dev, "Failed to init ctrl handler: %d\n", ret);
> + return ret;
> + }
> +
> + tpg->testgen_mode = v4l2_ctrl_new_std_menu_items(&tpg->ctrls,
> + &tpg_ctrl_ops, V4L2_CID_TEST_PATTERN,
> + tpg->testgen.nmodes, 0, 0,
> + tpg->testgen.modes);
> +
Please remove a blank line above.
> + if (tpg->ctrls.error) {
> + dev_err(dev, "Failed to init ctrl: %d\n", tpg->ctrls.error);
> + ret = tpg->ctrls.error;
> + goto free_ctrl;
> + }
> +
> + tpg->subdev.ctrl_handler = &tpg->ctrls;
> +
> + ret = tpg_init_formats(sd, NULL);
> + if (ret < 0) {
> + dev_err(dev, "Failed to init format: %d\n", ret);
> + goto free_ctrl;
> + }
> +
> + pads[MSM_TPG_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
> + pads[MSM_TPG_PAD_SRC].flags = MEDIA_PAD_FL_SOURCE;
Why do you need two pads for TPG?
> +
> + sd->entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
TPG is not a video pixel encoding converter device.
> + sd->entity.ops = &tpg_media_ops;
> + ret = media_entity_pads_init(&sd->entity, MSM_TPG_PADS_NUM, pads);
> + if (ret < 0) {
> + dev_err(dev, "Failed to init media entity: %d\n", ret);
> + goto free_ctrl;
> + }
> +
> + ret = v4l2_device_register_subdev(v4l2_dev, sd);
> + if (ret < 0) {
> + dev_err(dev, "Failed to register subdev: %d\n", ret);
> + media_entity_cleanup(&sd->entity);
> + goto free_ctrl;
> + }
> +
> + return 0;
> +
> +free_ctrl:
> + v4l2_ctrl_handler_free(&tpg->ctrls);
> +
> + return ret;
> +}
> +
> +/*
> + * msm_tpg_unregister_entity - Unregister tpg module subdev node
> + * @tpg: tpg device
> + */
> +void msm_tpg_unregister_entity(struct tpg_device *tpg)
> +{
> + v4l2_device_unregister_subdev(&tpg->subdev);
> + media_entity_cleanup(&tpg->subdev.entity);
> + v4l2_ctrl_handler_free(&tpg->ctrls);
> +}
> diff --git a/drivers/media/platform/qcom/camss/camss-tpg.h b/drivers/media/platform/qcom/camss/camss-tpg.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..1a16addac19418f2f11d0b8abb1c865c99888bde
> --- /dev/null
> +++ b/drivers/media/platform/qcom/camss/camss-tpg.h
> @@ -0,0 +1,127 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * camss-tpg.h
> + *
> + * Qualcomm MSM Camera Subsystem - TPG Module
> + *
> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +#ifndef QC_MSM_CAMSS_TPG_H
> +#define QC_MSM_CAMSS_TPG_H
> +
> +#include <linux/clk.h>
> +#include <media/media-entity.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-mediabus.h>
> +#include <media/v4l2-subdev.h>
> +
> +#define MSM_TPG_PAD_SINK 0
> +#define MSM_TPG_PAD_SRC 1
> +#define MSM_TPG_PADS_NUM 2
> +
> +#define DATA_TYPE_RAW_8BIT 0x2a
> +#define DATA_TYPE_RAW_10BIT 0x2b
> +#define DATA_TYPE_RAW_12BIT 0x2c
Remove all above to get the macro values from include/media/mipi-csi2.h
> +
> +#define ENCODE_FORMAT_UNCOMPRESSED_8_BIT 0x1
> +#define ENCODE_FORMAT_UNCOMPRESSED_10_BIT 0x2
> +#define ENCODE_FORMAT_UNCOMPRESSED_12_BIT 0x3
> +#define ENCODE_FORMAT_UNCOMPRESSED_14_BIT 0x4
> +#define ENCODE_FORMAT_UNCOMPRESSED_16_BIT 0x5
> +#define ENCODE_FORMAT_UNCOMPRESSED_20_BIT 0x6
> +#define ENCODE_FORMAT_UNCOMPRESSED_24_BIT 0x7
> +
> +#define TPG_GUP_ID 0
> +#define MSM_TPG_NAME "msm_tpg"
Remove the macro, it's used only once in the code and more usecases are
not expected happen.
> +
> +enum tpg_testgen_mode {
> + TPG_PAYLOAD_MODE_DISABLED = 0,
> + TPG_PAYLOAD_MODE_INCREMENTING = 1,
> + TPG_PAYLOAD_MODE_ALTERNATING_55_AA = 2,
> + TPG_PAYLOAD_MODE_RANDOM = 5,
> + TPG_PAYLOAD_MODE_USER_SPECIFIED = 6,
> + TPG_PAYLOAD_MODE_COLOR_BARS = 9,
> + TPG_PAYLOAD_MODE_NUM_SUPPORTED_GEN1 = 9,
> +};
> +
> +struct tpg_testgen_config {
> + enum tpg_testgen_mode mode;
> + const char * const*modes;
> + u8 nmodes;
> +};
> +
> +struct tpg_format_info {
> + u32 code;
> + u8 data_type;
> + u8 encode_format;
> + u8 bpp;
> +};
> +
> +struct tpg_formats {
> + unsigned int nformats;
> + const struct tpg_format_info *formats;
> +};
> +
> +struct tpg_device;
> +
> +struct tpg_hw_ops {
> + int (*configure_stream)(struct tpg_device *tpg, u8 enable);
> +
> + int (*configure_testgen_pattern)(struct tpg_device *tpg, s32 val);
> +
> + u32 (*hw_version)(struct tpg_device *tpg);
> +
> + int (*reset)(struct tpg_device *tpg);
> +
> + void (*subdev_init)(struct tpg_device *tpg);
> +};
> +
> +struct tpg_subdev_resources {
> + u8 lane_cnt;
> + u8 vc_cnt;
> + const struct tpg_formats *formats;
> + const struct tpg_hw_ops *hw_ops;
> +};
> +
> +struct tpg_device {
> + struct camss *camss;
> + u8 id;
> + struct v4l2_subdev subdev;
> + struct media_pad pads[MSM_TPG_PADS_NUM];
> + void __iomem *base;
> + struct camss_clock *clock;
> + int nclocks;
> + struct tpg_testgen_config testgen;
> + struct v4l2_mbus_framefmt fmt[MSM_TPG_PADS_NUM];
> + struct v4l2_ctrl_handler ctrls;
> + struct v4l2_ctrl *testgen_mode;
> + const struct tpg_subdev_resources *res;
> + const struct tpg_format *formats;
> + unsigned int nformats;
> + u32 hw_version;
> +};
> +
> +struct camss_subdev_resources;
> +
> +const struct tpg_format_info *tpg_get_fmt_entry(struct tpg_device *tpg,
> + const struct tpg_format_info *formats,
> + unsigned int nformats,
> + u32 code);
> +
> +int msm_tpg_subdev_init(struct camss *camss,
> + struct tpg_device *tpg,
> + const struct camss_subdev_resources *res, u8 id);
> +
> +int msm_tpg_register_entity(struct tpg_device *tpg,
> + struct v4l2_device *v4l2_dev);
> +
> +void msm_tpg_unregister_entity(struct tpg_device *tpg);
> +
> +extern const char * const testgen_payload_modes[];
> +
> +extern const struct tpg_formats tpg_formats_gen1;
> +
> +extern const struct tpg_hw_ops tpg_ops_gen1;
> +
> +#endif /* QC_MSM_CAMSS_TPG_H */
> diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
> index 9d9a62640e25dce0e8d45af9df01bbfd64b9bb4b..a892a87bed8bde8919200d6eac2b7a5338763c0e 100644
> --- a/drivers/media/platform/qcom/camss/camss.h
> +++ b/drivers/media/platform/qcom/camss/camss.h
> @@ -21,6 +21,7 @@
> #include "camss-csid.h"
> #include "camss-csiphy.h"
> #include "camss-ispif.h"
> +#include "camss-tpg.h"
> #include "camss-vfe.h"
> #include "camss-format.h"
>
> @@ -52,6 +53,7 @@ struct camss_subdev_resources {
> char *interrupt[CAMSS_RES_MAX];
> union {
> struct csiphy_subdev_resources csiphy;
> + struct tpg_subdev_resources tpg;
> struct csid_subdev_resources csid;
> struct vfe_subdev_resources vfe;
> };
> @@ -104,6 +106,7 @@ struct camss_resources {
> enum camss_version version;
> const char *pd_name;
> const struct camss_subdev_resources *csiphy_res;
> + const struct camss_subdev_resources *tpg_res;
> const struct camss_subdev_resources *csid_res;
> const struct camss_subdev_resources *ispif_res;
> const struct camss_subdev_resources *vfe_res;
> @@ -111,6 +114,7 @@ struct camss_resources {
> const struct resources_icc *icc_res;
> const unsigned int icc_path_num;
> const unsigned int csiphy_num;
> + const unsigned int tpg_num;
> const unsigned int csid_num;
> const unsigned int vfe_num;
> };
> @@ -121,6 +125,7 @@ struct camss {
> struct media_device media_dev;
> struct device *dev;
> struct csiphy_device *csiphy;
> + struct tpg_device *tpg;
> struct csid_device *csid;
> struct ispif_device *ispif;
> struct vfe_device *vfe;
>
--
Best wishes,
Vladimir
Powered by blists - more mailing lists