Re: [PATCH v7 5/6] media: starfive: camss: Add ISP driver
From: Laurent Pinchart
Date: Thu Jul 27 2023 - 16:41:09 EST
Hi Jack,
Thank you for the patch.
On Mon, Jun 19, 2023 at 07:28:37PM +0800, Jack Zhu wrote:
> Add ISP driver for StarFive Camera Subsystem.
>
> Signed-off-by: Jack Zhu <jack.zhu@xxxxxxxxxxxxxxxx>
> ---
> .../media/platform/starfive/camss/Makefile | 2 +
> .../media/platform/starfive/camss/stf_camss.c | 76 ++-
> .../media/platform/starfive/camss/stf_camss.h | 3 +
> .../media/platform/starfive/camss/stf_isp.c | 519 ++++++++++++++++++
> .../media/platform/starfive/camss/stf_isp.h | 479 ++++++++++++++++
> .../platform/starfive/camss/stf_isp_hw_ops.c | 468 ++++++++++++++++
> 6 files changed, 1544 insertions(+), 3 deletions(-)
> create mode 100644 drivers/media/platform/starfive/camss/stf_isp.c
> create mode 100644 drivers/media/platform/starfive/camss/stf_isp.h
> create mode 100644 drivers/media/platform/starfive/camss/stf_isp_hw_ops.c
>
> diff --git a/drivers/media/platform/starfive/camss/Makefile b/drivers/media/platform/starfive/camss/Makefile
> index eb457917a914..cdf57e8c9546 100644
> --- a/drivers/media/platform/starfive/camss/Makefile
> +++ b/drivers/media/platform/starfive/camss/Makefile
> @@ -5,6 +5,8 @@
>
> starfive-camss-objs += \
> stf_camss.o \
> + stf_isp.o \
> + stf_isp_hw_ops.o \
> stf_video.o
>
> obj-$(CONFIG_VIDEO_STARFIVE_CAMSS) += starfive-camss.o
> diff --git a/drivers/media/platform/starfive/camss/stf_camss.c b/drivers/media/platform/starfive/camss/stf_camss.c
> index dc2b5dba7bd4..6f56b45f57db 100644
> --- a/drivers/media/platform/starfive/camss/stf_camss.c
> +++ b/drivers/media/platform/starfive/camss/stf_camss.c
> @@ -115,12 +115,65 @@ static int stfcamss_of_parse_ports(struct stfcamss *stfcamss)
> return ret;
> }
>
> +/*
> + * stfcamss_init_subdevices - Initialize subdev structures and resources
> + * @stfcamss: STFCAMSS device
> + *
> + * Return 0 on success or a negative error code on failure
> + */
> +static int stfcamss_init_subdevices(struct stfcamss *stfcamss)
> +{
> + int ret;
> +
> + ret = stf_isp_subdev_init(stfcamss);
> + if (ret < 0) {
> + dev_err(stfcamss->dev, "Failed to init isp subdev: %d\n", ret);
> + return ret;
> + }
> +
> + return ret;
> +}
> +
> +static int stfcamss_register_subdevices(struct stfcamss *stfcamss)
> +{
> + int ret;
> + struct stf_isp_dev *isp_dev = &stfcamss->isp_dev;
> +
> + ret = stf_isp_register(isp_dev, &stfcamss->v4l2_dev);
> + if (ret < 0) {
> + dev_err(stfcamss->dev,
> + "Failed to register stf isp%d entity: %d\n", 0, ret);
> + return ret;
> + }
> +
> + return ret;
> +}
> +
> +static void stfcamss_unregister_subdevices(struct stfcamss *stfcamss)
> +{
> + stf_isp_unregister(&stfcamss->isp_dev);
> +}
> +
> static int stfcamss_subdev_notifier_bound(struct v4l2_async_notifier *async,
> struct v4l2_subdev *subdev,
> struct v4l2_async_subdev *asd)
> {
> + struct stfcamss *stfcamss =
> + container_of(async, struct stfcamss, notifier);
> + struct stfcamss_async_subdev *csd =
> + container_of(asd, struct stfcamss_async_subdev, asd);
> + enum stf_port_num port = csd->port;
> + struct stf_isp_dev *isp_dev = &stfcamss->isp_dev;
> struct media_pad *pad[STF_PADS_NUM];
> - unsigned int i, pad_num = 0;
> + unsigned int i, pad_num;
> +
> + if (port == STF_PORT_CSI2RX) {
> + pad[0] = &isp_dev->pads[STF_PAD_SINK];
> + pad_num = 1;
> + } else if (port == STF_PORT_DVP) {
> + dev_err(stfcamss->dev, "Not support DVP sensor\n");
> + return -EPERM;
> + }
>
> for (i = 0; i < pad_num; ++i) {
> int ret;
> @@ -223,12 +276,18 @@ static int stfcamss_probe(struct platform_device *pdev)
> goto err_cleanup_notifier;
> }
>
> + ret = stfcamss_init_subdevices(stfcamss);
> + if (ret < 0) {
> + dev_err(dev, "Failed to init subdevice: %d\n", ret);
> + goto err_cleanup_notifier;
> + }
> +
> stfcamss_mc_init(pdev, stfcamss);
>
> ret = v4l2_device_register(stfcamss->dev, &stfcamss->v4l2_dev);
> if (ret < 0) {
> dev_err(dev, "Failed to register V4L2 device: %d\n", ret);
> - goto err_cleanup_notifier;
> + goto err_cleanup_media_device;
> }
>
> ret = media_device_register(&stfcamss->media_dev);
> @@ -237,6 +296,12 @@ static int stfcamss_probe(struct platform_device *pdev)
> goto err_unregister_device;
> }
>
> + ret = stfcamss_register_subdevices(stfcamss);
> + if (ret < 0) {
> + dev_err(dev, "Failed to register subdevice: %d\n", ret);
> + goto err_unregister_media_dev;
> + }
> +
> pm_runtime_enable(dev);
>
> stfcamss->notifier.ops = &stfcamss_subdev_notifier_ops;
> @@ -244,15 +309,19 @@ static int stfcamss_probe(struct platform_device *pdev)
> if (ret) {
> dev_err(dev, "Failed to register async subdev nodes: %d\n",
> ret);
> - goto err_unregister_media_dev;
> + goto err_unregister_subdevs;
> }
>
> return 0;
>
> +err_unregister_subdevs:
> + stfcamss_unregister_subdevices(stfcamss);
> err_unregister_media_dev:
> media_device_unregister(&stfcamss->media_dev);
> err_unregister_device:
> v4l2_device_unregister(&stfcamss->v4l2_dev);
> +err_cleanup_media_device:
> + media_device_cleanup(&stfcamss->media_dev);
> err_cleanup_notifier:
> v4l2_async_nf_cleanup(&stfcamss->notifier);
> return ret;
> @@ -268,6 +337,7 @@ static int stfcamss_remove(struct platform_device *pdev)
> {
> struct stfcamss *stfcamss = platform_get_drvdata(pdev);
>
> + stfcamss_unregister_subdevices(stfcamss);
> v4l2_device_unregister(&stfcamss->v4l2_dev);
> media_device_cleanup(&stfcamss->media_dev);
> pm_runtime_disable(&pdev->dev);
> diff --git a/drivers/media/platform/starfive/camss/stf_camss.h b/drivers/media/platform/starfive/camss/stf_camss.h
> index 15c4f34b9054..9482081288fa 100644
> --- a/drivers/media/platform/starfive/camss/stf_camss.h
> +++ b/drivers/media/platform/starfive/camss/stf_camss.h
> @@ -18,6 +18,8 @@
> #include <media/v4l2-async.h>
> #include <media/v4l2-device.h>
>
> +#include "stf_isp.h"
> +
> #define STF_DVP_NAME "stf_dvp"
> #define STF_CSI_NAME "cdns_csi2rx"
> #define STF_ISP_NAME "stf_isp"
> @@ -65,6 +67,7 @@ struct stfcamss {
> struct media_device media_dev;
> struct media_pipeline pipe;
> struct device *dev;
> + struct stf_isp_dev isp_dev;
> struct v4l2_async_notifier notifier;
> void __iomem *syscon_base;
> void __iomem *isp_base;
> diff --git a/drivers/media/platform/starfive/camss/stf_isp.c b/drivers/media/platform/starfive/camss/stf_isp.c
> new file mode 100644
> index 000000000000..933a583b398c
> --- /dev/null
> +++ b/drivers/media/platform/starfive/camss/stf_isp.c
> @@ -0,0 +1,519 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * stf_isp.c
> + *
> + * StarFive Camera Subsystem - ISP Module
> + *
> + * Copyright (C) 2021-2023 StarFive Technology Co., Ltd.
> + */
> +#include <linux/firmware.h>
This doesn't seem needed.
> +#include <media/v4l2-event.h>
> +
> +#include "stf_camss.h"
> +
> +#define SINK_FORMATS_INDEX 0
> +#define UO_FORMATS_INDEX 1
What does "UO" stand for ?
> +
> +static int isp_set_selection(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + struct v4l2_subdev_selection *sel);
> +
> +static const struct isp_format isp_formats_sink[] = {
> + { MEDIA_BUS_FMT_SRGGB10_1X10, 10 },
> + { MEDIA_BUS_FMT_SGRBG10_1X10, 10 },
> + { MEDIA_BUS_FMT_SGBRG10_1X10, 10 },
> + { MEDIA_BUS_FMT_SBGGR10_1X10, 10 },
> +};
> +
> +static const struct isp_format isp_formats_uo[] = {
> + { MEDIA_BUS_FMT_Y12_1X12, 8 },
Y12 is a greyscale format, I don't think that's what you need here.
> +};
> +
> +static const struct isp_format_table isp_formats_st7110[] = {
> + { isp_formats_sink, ARRAY_SIZE(isp_formats_sink) },
> + { isp_formats_uo, ARRAY_SIZE(isp_formats_uo) },
> +};
> +
> +int stf_isp_subdev_init(struct stfcamss *stfcamss)
This function doesn't initialize the subdev, I'd call it stf_isp_init().
I would also create a stf_isp_cleanup() function to be consistent, and
move the mutex_destroy() call there.
> +{
> + struct stf_isp_dev *isp_dev = &stfcamss->isp_dev;
> +
> + isp_dev->stfcamss = stfcamss;
> + isp_dev->formats = isp_formats_st7110;
> + isp_dev->nformats = ARRAY_SIZE(isp_formats_st7110);
> +
> + mutex_init(&isp_dev->stream_lock);
> + return 0;
> +}
> +
> +static struct v4l2_mbus_framefmt *
> +__isp_get_format(struct stf_isp_dev *isp_dev,
> + struct v4l2_subdev_state *state,
> + unsigned int pad,
> + enum v4l2_subdev_format_whence which)
> +{
> + if (which == V4L2_SUBDEV_FORMAT_TRY)
> + return v4l2_subdev_get_try_format(&isp_dev->subdev, state, pad);
> +
> + return &isp_dev->fmt[pad];
Use the subdev active state API to store the active format on subdev
pads, it will simplify the driver. See commit a2514b9a634a ("media: i2c:
imx290: Use V4L2 subdev active state") for an example of a subdev driver
being converted to that API, it should help understanding how to use it.
You will be able to drop the stf_isp_dev fmt and rect fields. For the
bpp value stored in the rect structure, replace it with a const struct
isp_format pointer in stf_isp_dev that you set in .set_fmt() (as well as
initialization time) to the active format information.
> +}
> +
> +static int isp_set_stream(struct v4l2_subdev *sd, int enable)
> +{
> + struct stf_isp_dev *isp_dev = v4l2_get_subdevdata(sd);
> + int ret = 0;
> + struct v4l2_mbus_framefmt *fmt;
> + struct v4l2_event src_ch = { 0 };
> +
> + fmt = __isp_get_format(isp_dev, NULL, STF_ISP_PAD_SINK,
> + V4L2_SUBDEV_FORMAT_ACTIVE);
> + mutex_lock(&isp_dev->stream_lock);
> + if (enable) {
> + if (isp_dev->stream_count == 0) {
> + stf_isp_clk_enable(isp_dev);
> + stf_isp_reset(isp_dev);
> + stf_isp_init_cfg(isp_dev);
> + stf_isp_settings(isp_dev, isp_dev->rect, fmt->code);
> + stf_isp_stream_set(isp_dev);
> + }
> + isp_dev->stream_count++;
The subdev .s_stream() operation isn't supposed to be called multiple
times. If you need to count how many video nodes have started streaming,
that should be handled in the vb2 queue .start_streaming() and
.stop_streaming() operations.
> + } else {
> + if (isp_dev->stream_count == 0)
> + goto exit;
> +
> + if (isp_dev->stream_count == 1)
> + stf_isp_clk_disable(isp_dev);
> +
> + isp_dev->stream_count--;
> + }
> + src_ch.type = V4L2_EVENT_SOURCE_CHANGE,
; instead of , at the end of the line. Same for the next line.
> + src_ch.u.src_change.changes = isp_dev->stream_count,
> +
> + v4l2_subdev_notify_event(sd, &src_ch);
This doesn't seem right. V4L2_EVENT_SOURCE_CHANGE is meant to report
changes to the source format. I would drop support for events
completely from this patch, it doesn't seem to be needed.
> +exit:
> + mutex_unlock(&isp_dev->stream_lock);
> +
> + return ret;
ret is never set to a value other than 9, return 0 and drop the
variable.
> +}
> +
> +static void isp_try_format(struct stf_isp_dev *isp_dev,
> + struct v4l2_subdev_state *state,
> + unsigned int pad,
> + struct v4l2_mbus_framefmt *fmt,
> + enum v4l2_subdev_format_whence which)
> +{
> + const struct isp_format_table *formats;
> + struct stf_isp_crop *rect;
> + unsigned int i;
> +
> + if (pad == STF_ISP_PAD_SINK) {
> + /* Set format on sink pad */
> + formats = &isp_dev->formats[SINK_FORMATS_INDEX];
> + rect = &isp_dev->rect[SINK_FORMATS_INDEX];
> + } else if (pad == STF_ISP_PAD_SRC) {
> + formats = &isp_dev->formats[UO_FORMATS_INDEX];
> + rect = &isp_dev->rect[UO_FORMATS_INDEX];
> + }
> +
> + fmt->width = clamp_t(u32, fmt->width, STFCAMSS_FRAME_MIN_WIDTH,
> + STFCAMSS_FRAME_MAX_WIDTH);
> + fmt->height = clamp_t(u32, fmt->height, STFCAMSS_FRAME_MIN_HEIGHT,
> + STFCAMSS_FRAME_MAX_HEIGHT);
> + fmt->height &= ~0x1;
> + fmt->field = V4L2_FIELD_NONE;
> + fmt->colorspace = V4L2_COLORSPACE_SRGB;
> + fmt->flags = 0;
> +
> + for (i = 0; i < formats->nfmts; i++) {
> + if (fmt->code == formats->fmts[i].code)
> + break;
> + }
> +
> + if (i >= formats->nfmts) {
> + fmt->code = formats->fmts[0].code;
> + rect->bpp = formats->fmts[0].bpp;
> + } else {
> + rect->bpp = formats->fmts[i].bpp;
> + }
> +}
> +
> +static int isp_enum_mbus_code(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + struct v4l2_subdev_mbus_code_enum *code)
> +{
> + struct stf_isp_dev *isp_dev = v4l2_get_subdevdata(sd);
> + const struct isp_format_table *formats;
> +
> + if (code->index >= isp_dev->nformats)
That doesn't seem right, isp_dev->nformats isn't the number of formats
supported on the ISP, but the number of format types (sink and uo).
> + return -EINVAL;
Add a blank line here, it makes code easier to read.
> + if (code->pad == STF_ISP_PAD_SINK) {
> + formats = &isp_dev->formats[SINK_FORMATS_INDEX];
> + code->code = formats->fmts[code->index].code;
> + } else {
> + struct v4l2_mbus_framefmt *sink_fmt;
> +
> + sink_fmt = __isp_get_format(isp_dev, state, STF_ISP_PAD_SINK,
> + code->which);
> +
> + code->code = sink_fmt->code;
This doesn't seem right, you need to enumerate the ISP output media bus
codes here, and the function returns the sink media bus code instead.
> + if (!code->code)
> + return -EINVAL;
> + }
> + code->flags = 0;
> +
> + return 0;
> +}
> +
> +static int isp_enum_frame_size(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + struct v4l2_subdev_frame_size_enum *fse)
> +{
> + struct stf_isp_dev *isp_dev = 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;
> + isp_try_format(isp_dev, 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;
> + isp_try_format(isp_dev, state, fse->pad, &format, fse->which);
> + fse->max_width = format.width;
> + fse->max_height = format.height;
This seems unnecessarily complicated, you can just set the min and max
width and height to STFCAMSS_FRAME_MIN_WIDTH, STFCAMSS_FRAME_MAX_WIDTH,
STFCAMSS_FRAME_MIN_HEIGHT and STFCAMSS_FRAME_MAX_HEIGHT here.
> +
> + return 0;
> +}
> +
> +static int isp_get_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct stf_isp_dev *isp_dev = v4l2_get_subdevdata(sd);
> + struct v4l2_mbus_framefmt *format;
> +
> + format = __isp_get_format(isp_dev, state, fmt->pad, fmt->which);
> + if (!format)
> + return -EINVAL;
> +
> + fmt->format = *format;
> +
> + return 0;
> +}
> +
> +static int isp_set_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct stf_isp_dev *isp_dev = v4l2_get_subdevdata(sd);
> + struct v4l2_mbus_framefmt *format;
> +
> + format = __isp_get_format(isp_dev, state, fmt->pad, fmt->which);
> + if (!format)
> + return -EINVAL;
> +
> + mutex_lock(&isp_dev->stream_lock);
> +
> + isp_try_format(isp_dev, state, fmt->pad, &fmt->format, fmt->which);
> + *format = fmt->format;
> +
> + mutex_unlock(&isp_dev->stream_lock);
> +
> + /* Propagate to in crop */
> + if (fmt->pad == STF_ISP_PAD_SINK) {
> + struct v4l2_subdev_selection sel = { 0 };
> + int ret;
> +
> + /* Reset sink pad compose selection */
> + sel.which = fmt->which;
> + sel.pad = STF_ISP_PAD_SINK;
> + sel.target = V4L2_SEL_TGT_CROP;
> + sel.r.width = fmt->format.width;
> + sel.r.height = fmt->format.height;
> + ret = isp_set_selection(sd, state, &sel);
> + if (ret < 0)
> + return ret;
You're setting the which, pad and target values manually, so they're
guaranteed to be correct. isp_set_selection() will thus never return an
error. You can drop the error check.
> + }
> +
> + return 0;
> +}
> +
> +static struct v4l2_rect *
> +__isp_get_crop(struct stf_isp_dev *isp_dev,
> + struct v4l2_subdev_state *state,
> + unsigned int pad,
> + enum v4l2_subdev_format_whence which)
> +{
> + if (which == V4L2_SUBDEV_FORMAT_TRY)
> + return v4l2_subdev_get_try_crop(&isp_dev->subdev, state,
> + STF_ISP_PAD_SINK);
> +
> + return &isp_dev->rect[pad].rect;
> +}
> +
> +static void isp_try_crop(struct stf_isp_dev *isp_dev,
> + struct v4l2_subdev_state *state,
> + struct v4l2_rect *rect,
> + enum v4l2_subdev_format_whence which)
> +{
> + struct v4l2_mbus_framefmt *fmt;
> +
> + fmt = __isp_get_format(isp_dev, state, STF_ISP_PAD_SINK, which);
> +
> + if (rect->width > fmt->width)
> + rect->width = fmt->width;
> +
> + if (rect->width + rect->left > fmt->width)
> + rect->left = fmt->width - rect->width;
> +
> + if (rect->height > fmt->height)
> + rect->height = fmt->height;
> +
> + if (rect->height + rect->top > fmt->height)
> + rect->top = fmt->height - rect->height;
> +
> + if (rect->width < STFCAMSS_FRAME_MIN_WIDTH) {
> + rect->left = 0;
> + rect->width = STFCAMSS_FRAME_MAX_WIDTH;
> + }
> +
> + if (rect->height < STFCAMSS_FRAME_MIN_HEIGHT) {
> + rect->top = 0;
> + rect->height = STFCAMSS_FRAME_MAX_HEIGHT;
> + }
> + rect->height &= ~0x1;
The v4l2_rect_set_min_size() and v4l2_rect_map_inside() functions can
simplify this code.
> +}
> +
> +static int isp_get_selection(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + struct v4l2_subdev_selection *sel)
> +{
> + struct stf_isp_dev *isp_dev = v4l2_get_subdevdata(sd);
> + struct v4l2_subdev_format fmt = { 0 };
> + struct v4l2_rect *rect;
> + int ret;
> +
> + switch (sel->target) {
> + case V4L2_SEL_TGT_CROP_BOUNDS:
> + if (sel->pad == STF_ISP_PAD_SINK) {
> + fmt.pad = sel->pad;
> + fmt.which = sel->which;
> + ret = isp_get_format(sd, state, &fmt);
> + if (ret < 0)
> + return ret;
> +
> + sel->r.left = 0;
> + sel->r.top = 0;
> + sel->r.width = fmt.format.width;
> + sel->r.height = fmt.format.height;
> + } else if (sel->pad == STF_ISP_PAD_SRC) {
> + rect = __isp_get_crop(isp_dev, state,
> + sel->pad, sel->which);
> + sel->r = *rect;
> + }
> + break;
> +
> + case V4L2_SEL_TGT_CROP:
> + rect = __isp_get_crop(isp_dev, state, sel->pad, sel->which);
> + if (!rect)
> + return -EINVAL;
> +
> + sel->r = *rect;
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int isp_set_selection(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + struct v4l2_subdev_selection *sel)
> +{
> + struct stf_isp_dev *isp_dev = v4l2_get_subdevdata(sd);
> + struct v4l2_rect *rect;
> + int ret = 0;
> +
if (sel->target != V4L2_SEL_TGT_CROP)
return -EINVAL;
> + if (sel->target == V4L2_SEL_TGT_CROP &&
> + sel->pad == STF_ISP_PAD_SINK) {
> + struct v4l2_subdev_selection crop = { 0 };
> +
> + rect = __isp_get_crop(isp_dev, state, sel->pad, sel->which);
> + if (!rect)
> + return -EINVAL;
> +
> + mutex_lock(&isp_dev->stream_lock);
> + isp_try_crop(isp_dev, state, &sel->r, sel->which);
> + *rect = sel->r;
> + mutex_unlock(&isp_dev->stream_lock);
> +
> + /* Reset source crop selection */
> + crop.which = sel->which;
> + crop.pad = STF_ISP_PAD_SRC;
> + crop.target = V4L2_SEL_TGT_CROP;
> + crop.r = *rect;
> + ret = isp_set_selection(sd, state, &crop);
> + } else if (sel->target == V4L2_SEL_TGT_CROP &&
> + sel->pad == STF_ISP_PAD_SRC) {
> + struct v4l2_subdev_format fmt = { 0 };
> +
> + rect = __isp_get_crop(isp_dev, state, sel->pad, sel->which);
> + if (!rect)
> + return -EINVAL;
> +
> + mutex_lock(&isp_dev->stream_lock);
> + isp_try_crop(isp_dev, state, &sel->r, sel->which);
> + *rect = sel->r;
> + mutex_unlock(&isp_dev->stream_lock);
> +
> + /* Reset source pad format width and height */
> + fmt.which = sel->which;
> + fmt.pad = STF_ISP_PAD_SRC;
> + fmt.format.width = rect->width;
> + fmt.format.height = rect->height;
> + ret = isp_set_format(sd, state, &fmt);
> + if (ret < 0)
> + return ret;
You can drop the error check here too.
> + }
> +
> + dev_dbg(isp_dev->stfcamss->dev, "pad: %d sel(%d,%d)/%dx%d\n",
> + sel->pad, sel->r.left, sel->r.top, sel->r.width, sel->r.height);
> +
> + return 0;
> +}
> +
> +static int isp_init_formats(struct v4l2_subdev *sd,
> + struct v4l2_subdev_fh *fh)
> +{
> + struct v4l2_subdev_format format = {
> + .pad = STF_ISP_PAD_SINK,
> + .which =
> + fh ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE,
> + .format = {
> + .code = MEDIA_BUS_FMT_RGB565_2X8_LE,
Pick a format that the driver supports.
> + .width = 1920,
> + .height = 1080
> + }
> + };
> +
> + return isp_set_format(sd, fh ? fh->state : NULL, &format);
> +}
> +
> +static int isp_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;
Is this check really needed ?
> +}
> +
> +static int stf_isp_subscribe_event(struct v4l2_subdev *sd,
> + struct v4l2_fh *fh,
> + struct v4l2_event_subscription *sub)
> +{
> + switch (sub->type) {
> + case V4L2_EVENT_SOURCE_CHANGE:
> + return v4l2_src_change_event_subdev_subscribe(sd, fh, sub);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static const struct v4l2_subdev_core_ops isp_core_ops = {
> + .subscribe_event = stf_isp_subscribe_event,
> + .unsubscribe_event = v4l2_event_subdev_unsubscribe,
> +};
Drop events support.
> +
> +static const struct v4l2_subdev_video_ops isp_video_ops = {
> + .s_stream = isp_set_stream,
> +};
> +
> +static const struct v4l2_subdev_pad_ops isp_pad_ops = {
> + .enum_mbus_code = isp_enum_mbus_code,
> + .enum_frame_size = isp_enum_frame_size,
> + .get_fmt = isp_get_format,
> + .set_fmt = isp_set_format,
> + .get_selection = isp_get_selection,
> + .set_selection = isp_set_selection,
> +};
> +
> +static const struct v4l2_subdev_ops isp_v4l2_ops = {
> + .core = &isp_core_ops,
> + .video = &isp_video_ops,
> + .pad = &isp_pad_ops,
> +};
> +
> +static const struct v4l2_subdev_internal_ops isp_v4l2_internal_ops = {
> + .open = isp_init_formats,
> +};
Replace this with the .init_cfg() pad operation.
> +
> +static const struct media_entity_operations isp_media_ops = {
> + .link_setup = isp_link_setup,
> + .link_validate = v4l2_subdev_link_validate,
> +};
> +
> +int stf_isp_register(struct stf_isp_dev *isp_dev, struct v4l2_device *v4l2_dev)
> +{
> + struct v4l2_subdev *sd = &isp_dev->subdev;
> + struct media_pad *pads = isp_dev->pads;
> + int ret;
> +
> + v4l2_subdev_init(sd, &isp_v4l2_ops);
> + sd->internal_ops = &isp_v4l2_internal_ops;
> + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
Drop events here too.
> + snprintf(sd->name, ARRAY_SIZE(sd->name), "%s%d", STF_ISP_NAME, 0);
> + v4l2_set_subdevdata(sd, isp_dev);
> +
> + ret = isp_init_formats(sd, NULL);
> + if (ret < 0) {
> + dev_err(isp_dev->stfcamss->dev, "Failed to init format: %d\n",
> + ret);
> + return ret;
> + }
> +
> + pads[STF_ISP_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
> + pads[STF_ISP_PAD_SRC].flags = MEDIA_PAD_FL_SOURCE;
> +
> + sd->entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
MEDIA_ENT_F_PROC_VIDEO_ISP seems a better match.
> + sd->entity.ops = &isp_media_ops;
> + ret = media_entity_pads_init(&sd->entity, STF_ISP_PAD_MAX, pads);
> + if (ret < 0) {
> + dev_err(isp_dev->stfcamss->dev,
> + "Failed to init media entity: %d\n", ret);
> + return ret;
> + }
> +
> + ret = v4l2_device_register_subdev(v4l2_dev, sd);
> + if (ret < 0) {
> + dev_err(isp_dev->stfcamss->dev,
> + "Failed to register subdev: %d\n", ret);
> + goto err_sreg;
> + }
> +
> + return 0;
> +
> +err_sreg:
> + media_entity_cleanup(&sd->entity);
> + return ret;
> +}
> +
> +int stf_isp_unregister(struct stf_isp_dev *isp_dev)
> +{
> + v4l2_device_unregister_subdev(&isp_dev->subdev);
> + media_entity_cleanup(&isp_dev->subdev.entity);
> + mutex_destroy(&isp_dev->stream_lock);
> + return 0;
> +}
> diff --git a/drivers/media/platform/starfive/camss/stf_isp.h b/drivers/media/platform/starfive/camss/stf_isp.h
> new file mode 100644
> index 000000000000..1e5c98482350
> --- /dev/null
> +++ b/drivers/media/platform/starfive/camss/stf_isp.h
> @@ -0,0 +1,479 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * stf_isp.h
> + *
> + * StarFive Camera Subsystem - ISP Module
> + *
> + * Copyright (C) 2021-2023 StarFive Technology Co., Ltd.
> + */
> +
> +#ifndef STF_ISP_H
> +#define STF_ISP_H
> +
> +#include <media/v4l2-subdev.h>
> +
> +#include "stf_video.h"
> +
> +#define ISP_RAW_DATA_BITS 12
> +#define SCALER_RATIO_MAX 1
> +#define STF_ISP_REG_OFFSET_MAX 0x0fff
> +#define STF_ISP_REG_DELAY_MAX 100
> +
> +/* isp registers */
> +#define ISP_REG_CSI_INPUT_EN_AND_STATUS 0x000
> +#define CSI_SCD_ERR BIT(6)
> +#define CSI_ITU656_ERR BIT(4)
> +#define CSI_ITU656_F BIT(3)
> +#define CSI_SCD_DONE BIT(2)
> +#define CSI_BUSY_S BIT(1)
> +#define CSI_EN_S BIT(0)
Could you align the macros values consistently in the whole file ? It
would be easier to read. For the lines above, that would be
#define ISP_RAW_DATA_BITS 12
#define SCALER_RATIO_MAX 1
#define STF_ISP_REG_OFFSET_MAX 0x0fff
#define STF_ISP_REG_DELAY_MAX 100
/* isp registers */
#define ISP_REG_CSI_INPUT_EN_AND_STATUS 0x000
#define CSI_SCD_ERR BIT(6)
#define CSI_ITU656_ERR BIT(4)
#define CSI_ITU656_F BIT(3)
#define CSI_SCD_DONE BIT(2)
#define CSI_BUSY_S BIT(1)
#define CSI_EN_S BIT(0)
(possibly with additional indentation if some of the lines below require
that)
You could also move the register definitions to a stf_isp_regs.h header
if desired, as they're only needed by stf_isp_hw_ops.c, not by the other
files that include stf_isp.h. up to you.
> +
> +#define ISP_REG_CSIINTS 0x008
> +#define CSI_INTS(n) ((n) << 16)
> +#define CSI_SHA_M(n) ((n) << 0)
> +#define CSI_INTS_MASK GENMASK(17, 16)
> +
> +#define ISP_REG_CSI_MODULE_CFG 0x010
> +#define CSI_DUMP_EN BIT(19)
> +#define CSI_VS_EN BIT(18)
> +#define CSI_SC_EN BIT(17)
> +#define CSI_OBA_EN BIT(16)
> +#define CSI_AWB_EN BIT(7)
> +#define CSI_LCCF_EN BIT(6)
> +#define CSI_OECFHM_EN BIT(5)
> +#define CSI_OECF_EN BIT(4)
> +#define CSI_LCBQ_EN BIT(3)
> +#define CSI_OBC_EN BIT(2)
> +#define CSI_DEC_EN BIT(1)
> +#define CSI_DC_EN BIT(0)
> +
> +#define ISP_REG_SENSOR 0x014
> +#define DVP_SYNC_POL(n) ((n) << 2)
> +#define ITU656_EN(n) ((n) << 1)
> +#define IMAGER_SEL(n) ((n) << 0)
> +
> +#define ISP_REG_RAW_FORMAT_CFG 0x018
> +#define SMY13(n) ((n) << 14)
> +#define SMY12(n) ((n) << 12)
> +#define SMY11(n) ((n) << 10)
> +#define SMY10(n) ((n) << 8)
> +#define SMY3(n) ((n) << 6)
> +#define SMY2(n) ((n) << 4)
> +#define SMY1(n) ((n) << 2)
> +#define SMY0(n) ((n) << 0)
> +
> +#define ISP_REG_PIC_CAPTURE_START_CFG 0x01c
> +#define VSTART_CAP(n) ((n) << 16)
> +#define HSTART_CAP(n) ((n) << 0)
> +
> +#define ISP_REG_PIC_CAPTURE_END_CFG 0x020
> +#define VEND_CAP(n) ((n) << 16)
> +#define HEND_CAP(n) ((n) << 0)
> +
> +#define ISP_REG_DUMP_CFG_0 0x024
> +#define ISP_REG_DUMP_CFG_1 0x028
> +#define DUMP_ID(n) ((n) << 24)
> +#define DUMP_SHT(n) ((n) << 20)
> +#define DUMP_BURST_LEN(n) ((n) << 16)
> +#define DUMP_SD(n) ((n) << 0)
> +#define DUMP_BURST_LEN_MASK GENMASK(17, 16)
> +#define DUMP_SD_MASK GENMASK(15, 0)
> +
> +#define ISP_REG_DEC_CFG 0x030
> +#define DEC_V_KEEP(n) ((n) << 24)
> +#define DEC_V_PERIOD(n) ((n) << 16)
> +#define DEC_H_KEEP(n) ((n) << 8)
> +#define DEC_H_PERIOD(n) ((n) << 0)
> +
> +#define ISP_REG_OBC_CFG 0x034
> +#define OBC_W_H(y) ((y) << 4)
> +#define OBC_W_W(x) ((x) << 0)
> +
> +#define ISP_REG_DC_CFG_1 0x044
> +#define DC_AXI_ID(n) ((n) << 0)
> +
> +#define ISP_REG_LCCF_CFG_0 0x050
> +#define Y_DISTANCE(y) ((y) << 16)
> +#define X_DISTANCE(x) ((x) << 0)
> +
> +#define ISP_REG_LCCF_CFG_1 0x058
> +#define LCCF_MAX_DIS(n) ((n) << 0)
> +
> +#define ISP_REG_LCBQ_CFG_0 0x074
> +#define H_LCBQ(y) ((y) << 12)
> +#define W_LCBQ(x) ((x) << 8)
> +
> +#define ISP_REG_LCBQ_CFG_1 0x07c
> +#define Y_COOR(y) ((y) << 16)
> +#define X_COOR(x) ((x) << 0)
> +
> +#define ISP_REG_LCCF_CFG_2 0x0e0
> +#define ISP_REG_LCCF_CFG_3 0x0e4
> +#define ISP_REG_LCCF_CFG_4 0x0e8
> +#define ISP_REG_LCCF_CFG_5 0x0ec
> +#define LCCF_F2_PAR(n) ((n) << 16)
> +#define LCCF_F1_PAR(n) ((n) << 0)
> +
> +#define ISP_REG_OECF_X0_CFG0 0x100
> +#define ISP_REG_OECF_X0_CFG1 0x104
> +#define ISP_REG_OECF_X0_CFG2 0x108
> +#define ISP_REG_OECF_X0_CFG3 0x10c
> +#define ISP_REG_OECF_X0_CFG4 0x110
> +#define ISP_REG_OECF_X0_CFG5 0x114
> +#define ISP_REG_OECF_X0_CFG6 0x118
> +#define ISP_REG_OECF_X0_CFG7 0x11c
> +
> +#define ISP_REG_OECF_Y3_CFG0 0x1e0
> +#define ISP_REG_OECF_Y3_CFG1 0x1e4
> +#define ISP_REG_OECF_Y3_CFG2 0x1e8
> +#define ISP_REG_OECF_Y3_CFG3 0x1ec
> +#define ISP_REG_OECF_Y3_CFG4 0x1f0
> +#define ISP_REG_OECF_Y3_CFG5 0x1f4
> +#define ISP_REG_OECF_Y3_CFG6 0x1f8
> +#define ISP_REG_OECF_Y3_CFG7 0x1fc
> +
> +#define ISP_REG_OECF_S0_CFG0 0x200
> +#define ISP_REG_OECF_S3_CFG7 0x27c
> +#define OCEF_PAR_H(n) ((n) << 16)
> +#define OCEF_PAR_L(n) ((n) << 0)
> +
> +#define ISP_REG_AWB_X0_CFG_0 0x280
> +#define ISP_REG_AWB_X0_CFG_1 0x284
> +#define ISP_REG_AWB_X1_CFG_0 0x288
> +#define ISP_REG_AWB_X1_CFG_1 0x28c
> +#define ISP_REG_AWB_X2_CFG_0 0x290
> +#define ISP_REG_AWB_X2_CFG_1 0x294
> +#define ISP_REG_AWB_X3_CFG_0 0x298
> +#define ISP_REG_AWB_X3_CFG_1 0x29c
> +#define AWB_X_SYMBOL_H(n) ((n) << 16)
> +#define AWB_X_SYMBOL_L(n) ((n) << 0)
> +
> +#define ISP_REG_AWB_Y0_CFG_0 0x2a0
> +#define ISP_REG_AWB_Y0_CFG_1 0x2a4
> +#define ISP_REG_AWB_Y1_CFG_0 0x2a8
> +#define ISP_REG_AWB_Y1_CFG_1 0x2ac
> +#define ISP_REG_AWB_Y2_CFG_0 0x2b0
> +#define ISP_REG_AWB_Y2_CFG_1 0x2b4
> +#define ISP_REG_AWB_Y3_CFG_0 0x2b8
> +#define ISP_REG_AWB_Y3_CFG_1 0x2bc
> +#define AWB_Y_SYMBOL_H(n) ((n) << 16)
> +#define AWB_Y_SYMBOL_L(n) ((n) << 0)
> +
> +#define ISP_REG_AWB_S0_CFG_0 0x2c0
> +#define ISP_REG_AWB_S0_CFG_1 0x2c4
> +#define ISP_REG_AWB_S1_CFG_0 0x2c8
> +#define ISP_REG_AWB_S1_CFG_1 0x2cc
> +#define ISP_REG_AWB_S2_CFG_0 0x2d0
> +#define ISP_REG_AWB_S2_CFG_1 0x2d4
> +#define ISP_REG_AWB_S3_CFG_0 0x2d8
> +#define ISP_REG_AWB_S3_CFG_1 0x2dc
> +#define AWB_S_SYMBOL_H(n) ((n) << 16)
> +#define AWB_S_SYMBOL_L(n) ((n) << 0)
> +
> +#define ISP_REG_OBCG_CFG_0 0x2e0
> +#define ISP_REG_OBCG_CFG_1 0x2e4
> +#define ISP_REG_OBCG_CFG_2 0x2e8
> +#define ISP_REG_OBCG_CFG_3 0x2ec
> +#define ISP_REG_OBCO_CFG_0 0x2f0
> +#define ISP_REG_OBCO_CFG_1 0x2f4
> +#define ISP_REG_OBCO_CFG_2 0x2f8
> +#define ISP_REG_OBCO_CFG_3 0x2fc
> +#define GAIN_D_POINT(x) ((x) << 24)
> +#define GAIN_C_POINT(x) ((x) << 16)
> +#define GAIN_B_POINT(x) ((x) << 8)
> +#define GAIN_A_POINT(x) ((x) << 0)
> +#define OFFSET_D_POINT(x) ((x) << 24)
> +#define OFFSET_C_POINT(x) ((x) << 16)
> +#define OFFSET_B_POINT(x) ((x) << 8)
> +#define OFFSET_A_POINT(x) ((x) << 0)
> +
> +#define ISP_REG_ISP_CTRL_0 0xa00
> +#define ISPC_SCFEINT BIT(27)
> +#define ISPC_VSFWINT BIT(26)
> +#define ISPC_VSINT BIT(25)
> +#define ISPC_INTS BIT(24)
> +#define ISPC_ENUO BIT(20)
> +#define ISPC_ENLS BIT(17)
> +#define ISPC_ENSS1 BIT(12)
> +#define ISPC_ENSS0 BIT(11)
> +#define ISPC_RST BIT(1)
> +#define ISPC_EN BIT(0)
> +#define ISPC_RST_MASK BIT(1)
> +
> +#define ISP_REG_ISP_CTRL_1 0xa08
> +#define CTRL_SAT(n) ((n) << 28)
> +#define CTRL_DBC BIT(22)
> +#define CTRL_CTC BIT(21)
> +#define CTRL_YHIST BIT(20)
> +#define CTRL_YCURVE BIT(19)
> +#define CTRL_CTM BIT(18)
> +#define CTRL_BIYUV BIT(17)
> +#define CTRL_SCE BIT(8)
> +#define CTRL_EE BIT(7)
> +#define CTRL_CCE BIT(5)
> +#define CTRL_RGE BIT(4)
> +#define CTRL_CME BIT(3)
> +#define CTRL_AE BIT(2)
> +#define CTRL_CE BIT(1)
> +#define CTRL_SAT_MASK GENMASK(31, 28)
> +
> +#define ISP_REG_PIPELINE_XY_SIZE 0xa0c
> +#define H_ACT_CAP(n) ((n) << 16)
> +#define W_ACT_CAP(n) ((n) << 0)
> +
> +#define ISP_REG_ICTC 0xa10
> +#define GF_MODE(n) ((n) << 30)
> +#define MAXGT(n) ((n) << 16)
> +#define MINGT(n) ((n) << 0)
> +
> +#define ISP_REG_IDBC 0xa14
> +#define BADGT(n) ((n) << 16)
> +#define BADXT(n) ((n) << 0)
> +
> +#define ISP_REG_ICFAM 0xa1c
> +#define CROSS_COV(n) ((n) << 4)
> +#define HV_W(n) ((n) << 0)
> +
> +#define ISP_REG_CS_GAIN 0xa30
> +#define CMAD(n) ((n) << 16)
> +#define CMAB(n) ((n) << 0)
> +
> +#define ISP_REG_CS_THRESHOLD 0xa34
> +#define CMD(n) ((n) << 16)
> +#define CMB(n) ((n) << 0)
> +
> +#define ISP_REG_CS_OFFSET 0xa38
> +#define VOFF(n) ((n) << 16)
> +#define UOFF(n) ((n) << 0)
> +
> +#define ISP_REG_CS_HUE_F 0xa3c
> +#define SIN(n) ((n) << 16)
> +#define COS(n) ((n) << 0)
> +
> +#define ISP_REG_CS_SCALE 0xa40
> +#define CMSF(n) ((n) << 0)
> +
> +#define ISP_REG_IESHD 0xa50
> +#define SHAD_UP_M BIT(1)
> +#define SHAD_UP_EN BIT(0)
> +
> +#define ISP_REG_YADJ0 0xa54
> +#define YOIR(n) ((n) << 16)
> +#define YIMIN(n) ((n) << 0)
> +
> +#define ISP_REG_YADJ1 0xa58
> +#define YOMAX(n) ((n) << 16)
> +#define YOMIN(n) ((n) << 0)
> +
> +#define ISP_REG_Y_PLANE_START_ADDR 0xa80
> +#define ISP_REG_UV_PLANE_START_ADDR 0xa84
> +
> +#define ISP_REG_STRIDE 0xa88
> +#define IMG_STR(n) ((n) << 0)
For registers that contain a single field at offset 0, you don't need to
define a macro, you can pass the value directly to the register write
function (IMG_STR is actually not used :-)). Same above and/or below
where applicable.
> +
> +#define ISP_REG_ITIIWSR 0xb20
> +#define ITI_HSIZE(n) ((n) << 16)
> +#define ITI_WSIZE(n) ((n) << 0)
> +
> +#define ISP_REG_ITIDWLSR 0xb24
> +#define ITI_WSTRIDE(n) ((n) << 0)
> +
> +#define ISP_REG_ITIPDFR 0xb38
> +#define ITI_PACKAGE_FMT(n) ((n) << 0)
> +
> +#define ISP_REG_ITIDRLSR 0xb3C
> +#define ITI_STRIDE_L(n) ((n) << 0)
> +
> +#define ISP_REG_DNYUV_YSWR0 0xc00
> +#define ISP_REG_DNYUV_YSWR1 0xc04
> +#define ISP_REG_DNYUV_CSWR0 0xc08
> +#define ISP_REG_DNYUV_CSWR1 0xc0c
> +#define YUVSW5(n) ((n) << 20)
> +#define YUVSW4(n) ((n) << 16)
> +#define YUVSW3(n) ((n) << 12)
> +#define YUVSW2(n) ((n) << 8)
> +#define YUVSW1(n) ((n) << 4)
> +#define YUVSW0(n) ((n) << 0)
> +
> +#define ISP_REG_DNYUV_YDR0 0xc10
> +#define ISP_REG_DNYUV_YDR1 0xc14
> +#define ISP_REG_DNYUV_YDR2 0xc18
> +#define ISP_REG_DNYUV_CDR0 0xc1c
> +#define ISP_REG_DNYUV_CDR1 0xc20
> +#define ISP_REG_DNYUV_CDR2 0xc24
> +#define CURVE_D_H(n) ((n) << 16)
> +#define CURVE_D_L(n) ((n) << 0)
> +
> +#define ISP_REG_ICAMD_0 0xc40
> +#define DNRM_F(n) ((n) << 16)
> +#define ISP_REG_ICAMD_12 0xc70
> +#define ISP_REG_ICAMD_20 0xc90
> +#define ISP_REG_ICAMD_24 0xca0
> +#define ISP_REG_ICAMD_25 0xca4
> +#define CCM_M_DAT(n) ((n) << 0)
> +
> +#define ISP_REG_GAMMA_VAL0 0xe00
> +#define ISP_REG_GAMMA_VAL1 0xe04
> +#define ISP_REG_GAMMA_VAL2 0xe08
> +#define ISP_REG_GAMMA_VAL3 0xe0c
> +#define ISP_REG_GAMMA_VAL4 0xe10
> +#define ISP_REG_GAMMA_VAL5 0xe14
> +#define ISP_REG_GAMMA_VAL6 0xe18
> +#define ISP_REG_GAMMA_VAL7 0xe1c
> +#define ISP_REG_GAMMA_VAL8 0xe20
> +#define ISP_REG_GAMMA_VAL9 0xe24
> +#define ISP_REG_GAMMA_VAL10 0xe28
> +#define ISP_REG_GAMMA_VAL11 0xe2c
> +#define ISP_REG_GAMMA_VAL12 0xe30
> +#define ISP_REG_GAMMA_VAL13 0xe34
> +#define ISP_REG_GAMMA_VAL14 0xe38
> +#define GAMMA_S_VAL(n) ((n) << 16)
> +#define GAMMA_VAL(n) ((n) << 0)
> +
> +#define ISP_REG_R2Y_0 0xe40
> +#define ISP_REG_R2Y_1 0xe44
> +#define ISP_REG_R2Y_2 0xe48
> +#define ISP_REG_R2Y_3 0xe4c
> +#define ISP_REG_R2Y_4 0xe50
> +#define ISP_REG_R2Y_5 0xe54
> +#define ISP_REG_R2Y_6 0xe58
> +#define ISP_REG_R2Y_7 0xe5c
> +#define ISP_REG_R2Y_8 0xe60
> +#define CSC_M(n) ((n) << 0)
> +
> +#define ISP_REG_SHARPEN0 0xe80
> +#define ISP_REG_SHARPEN1 0xe84
> +#define ISP_REG_SHARPEN2 0xe88
> +#define ISP_REG_SHARPEN3 0xe8c
> +#define ISP_REG_SHARPEN4 0xe90
> +#define ISP_REG_SHARPEN5 0xe94
> +#define ISP_REG_SHARPEN6 0xe98
> +#define ISP_REG_SHARPEN7 0xe9c
> +#define ISP_REG_SHARPEN8 0xea0
> +#define ISP_REG_SHARPEN9 0xea4
> +#define ISP_REG_SHARPEN10 0xea8
> +#define ISP_REG_SHARPEN11 0xeac
> +#define ISP_REG_SHARPEN12 0xeb0
> +#define ISP_REG_SHARPEN13 0xeb4
> +#define ISP_REG_SHARPEN14 0xeb8
> +#define S_DELTA(n) ((n) << 16)
> +#define S_WEIGHT(n) ((n) << 8)
> +
> +#define ISP_REG_SHARPEN_FS0 0xebc
> +#define ISP_REG_SHARPEN_FS1 0xec0
> +#define ISP_REG_SHARPEN_FS2 0xec4
> +#define ISP_REG_SHARPEN_FS3 0xec8
> +#define ISP_REG_SHARPEN_FS4 0xecc
> +#define ISP_REG_SHARPEN_FS5 0xed0
> +#define S_FACTOR(n) ((n) << 24)
> +#define S_SLOPE(n) ((n) << 0)
> +
> +#define ISP_REG_SHARPEN_WN 0xed4
> +#define PDIRF(n) ((n) << 28)
> +#define NDIRF(n) ((n) << 24)
> +#define WSUM(n) ((n) << 0)
> +
> +#define ISP_REG_IUVS1 0xed8
> +#define UVDIFF2(n) ((n) << 16)
> +#define UVDIFF1(n) ((n) << 0)
> +
> +#define ISP_REG_IUVS2 0xedc
> +#define UVF(n) ((n) << 24)
> +#define UVSLOPE(n) ((n) << 0)
> +
> +#define ISP_REG_IUVCKS1 0xee0
> +#define UVCKDIFF2(n) ((n) << 16)
> +#define UVCKDIFF1(n) ((n) << 0)
> +
> +#define ISP_REG_IUVCKS2 0xee4
> +#define UVCKSLOPE(n) ((n) << 0)
> +
> +#define ISP_REG_ISHRPET 0xee8
> +#define TH(n) ((n) << 8)
> +#define EN(n) ((n) << 0)
> +
> +#define ISP_REG_YCURVE_0 0xf00
> +#define ISP_REG_YCURVE_63 0xffc
> +#define L_PARAM(n) ((n) << 0)
> +
> +#define IMAGE_MAX_WIDTH 1920
> +#define IMAGE_MAX_HEIGH 1080
> +
> +/* The output line of ISP */
What is an ISP "line" ?
> +enum isp_line_id {
> + STF_ISP_LINE_INVALID = -1,
> + STF_ISP_LINE_SRC = 1,
> + STF_ISP_LINE_MAX = STF_ISP_LINE_SRC
> +};
> +
> +/* pad id for media framework */
> +enum isp_pad_id {
> + STF_ISP_PAD_SINK = 0,
> + STF_ISP_PAD_SRC,
> + STF_ISP_PAD_MAX
> +};
> +
> +enum {
> + EN_INT_NONE = 0,
> + EN_INT_ISP_DONE = (0x1 << 24),
> + EN_INT_CSI_DONE = (0x1 << 25),
> + EN_INT_SC_DONE = (0x1 << 26),
> + EN_INT_LINE_INT = (0x1 << 27),
> + EN_INT_ALL = (0xF << 24),
Please use lowercase for hex constants.
But I'm wondering if this is really needed, it seems to duplicate the
ISPC_* bits defined above. I would use those directly.
> +};
> +
> +enum {
> + INTERFACE_DVP = 0,
> + INTERFACE_CSI,
> +};
This isn't used.
> +
> +struct isp_format {
> + u32 code;
> + u8 bpp;
> +};
> +
> +struct isp_format_table {
> + const struct isp_format *fmts;
> + int nfmts;
> +};
> +
> +struct regval_t {
> + u32 addr;
> + u32 val;
> + u32 delay_ms;
> +};
> +
> +struct reg_table {
> + const struct regval_t *regval;
> + int regval_num;
> +};
Those two structures are not used.
> +
> +struct stf_isp_crop {
> + struct v4l2_rect rect;
> + u32 bpp;
> +};
> +
> +struct stf_isp_dev {
> + struct stfcamss *stfcamss;
> + struct v4l2_subdev subdev;
> + struct media_pad pads[STF_ISP_PAD_MAX];
> + struct v4l2_mbus_framefmt fmt[STF_ISP_PAD_MAX];
> + struct stf_isp_crop rect[STF_ISP_PAD_MAX];
> + const struct isp_format_table *formats;
> + unsigned int nformats;
> + struct mutex stream_lock; /* serialize stream control */
You should explicitly list the fields protected by the mutex.
> + int stream_count;
> +};
> +
> +int stf_isp_clk_enable(struct stf_isp_dev *isp_dev);
> +int stf_isp_clk_disable(struct stf_isp_dev *isp_dev);
> +int stf_isp_reset(struct stf_isp_dev *isp_dev);
> +void stf_isp_init_cfg(struct stf_isp_dev *isp_dev);
> +void stf_isp_settings(struct stf_isp_dev *isp_dev,
> + struct stf_isp_crop *crop_array, u32 mcode);
> +void stf_isp_stream_set(struct stf_isp_dev *isp_dev);
> +int stf_isp_subdev_init(struct stfcamss *stfcamss);
> +int stf_isp_register(struct stf_isp_dev *isp_dev, struct v4l2_device *v4l2_dev);
> +int stf_isp_unregister(struct stf_isp_dev *isp_dev);
> +
> +#endif /* STF_ISP_H */
> diff --git a/drivers/media/platform/starfive/camss/stf_isp_hw_ops.c b/drivers/media/platform/starfive/camss/stf_isp_hw_ops.c
> new file mode 100644
> index 000000000000..2088b6bd0d61
> --- /dev/null
> +++ b/drivers/media/platform/starfive/camss/stf_isp_hw_ops.c
> @@ -0,0 +1,468 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * stf_isp_hw_ops.c
> + *
> + * Register interface file for StarFive ISP driver
> + *
> + * Copyright (C) 2021-2023 StarFive Technology Co., Ltd.
> + *
> + */
> +
> +#include "stf_camss.h"
> +
> +static void stf_isp_config_obc(struct stfcamss *stfcamss)
> +{
> + u32 reg_val, reg_add;
> +
> + stf_isp_reg_write(stfcamss, ISP_REG_OBC_CFG, OBC_W_H(11) | OBC_W_W(11));
> +
> + reg_val = GAIN_D_POINT(0x40) | GAIN_C_POINT(0x40) |
> + GAIN_B_POINT(0x40) | GAIN_A_POINT(0x40);
> + for (reg_add = ISP_REG_OBCG_CFG_0; reg_add <= ISP_REG_OBCG_CFG_3;) {
> + stf_isp_reg_write(stfcamss, reg_add, reg_val);
> + reg_add += 4;
> + }
> +
> + reg_val = OFFSET_D_POINT(0) | OFFSET_C_POINT(0) |
> + OFFSET_B_POINT(0) | OFFSET_A_POINT(0);
> + for (reg_add = ISP_REG_OBCO_CFG_0; reg_add <= ISP_REG_OBCO_CFG_3;) {
> + stf_isp_reg_write(stfcamss, reg_add, reg_val);
> + reg_add += 4;
> + }
> +}
> +
> +static void stf_isp_config_oecf(struct stfcamss *stfcamss)
> +{
> + u32 reg_add, par_val;
> + u16 par_h, par_l;
> +
> + par_h = 0x10; par_l = 0;
> + par_val = OCEF_PAR_H(par_h) | OCEF_PAR_L(par_l);
> + for (reg_add = ISP_REG_OECF_X0_CFG0; reg_add <= ISP_REG_OECF_Y3_CFG0;) {
> + stf_isp_reg_write(stfcamss, reg_add, par_val);
> + reg_add += 0x20;
> + }
> +
> + par_h = 0x40; par_l = 0x20;
> + par_val = OCEF_PAR_H(par_h) | OCEF_PAR_L(par_l);
> + for (reg_add = ISP_REG_OECF_X0_CFG1; reg_add <= ISP_REG_OECF_Y3_CFG1;) {
> + stf_isp_reg_write(stfcamss, reg_add, par_val);
> + reg_add += 0x20;
> + }
> +
> + par_h = 0x80; par_l = 0x60;
> + par_val = OCEF_PAR_H(par_h) | OCEF_PAR_L(par_l);
> + for (reg_add = ISP_REG_OECF_X0_CFG2; reg_add <= ISP_REG_OECF_Y3_CFG2;) {
> + stf_isp_reg_write(stfcamss, reg_add, par_val);
> + reg_add += 0x20;
> + }
> +
> + par_h = 0xc0; par_l = 0xa0;
> + par_val = OCEF_PAR_H(par_h) | OCEF_PAR_L(par_l);
> + for (reg_add = ISP_REG_OECF_X0_CFG3; reg_add <= ISP_REG_OECF_Y3_CFG3;) {
> + stf_isp_reg_write(stfcamss, reg_add, par_val);
> + reg_add += 0x20;
> + }
> +
> + par_h = 0x100; par_l = 0xe0;
> + par_val = OCEF_PAR_H(par_h) | OCEF_PAR_L(par_l);
> + for (reg_add = ISP_REG_OECF_X0_CFG4; reg_add <= ISP_REG_OECF_Y3_CFG4;) {
> + stf_isp_reg_write(stfcamss, reg_add, par_val);
> + reg_add += 0x20;
> + }
> +
> + par_h = 0x200; par_l = 0x180;
> + par_val = OCEF_PAR_H(par_h) | OCEF_PAR_L(par_l);
> + for (reg_add = ISP_REG_OECF_X0_CFG5; reg_add <= ISP_REG_OECF_Y3_CFG5;) {
> + stf_isp_reg_write(stfcamss, reg_add, par_val);
> + reg_add += 0x20;
> + }
> +
> + par_h = 0x300; par_l = 0x280;
> + par_val = OCEF_PAR_H(par_h) | OCEF_PAR_L(par_l);
> + for (reg_add = ISP_REG_OECF_X0_CFG6; reg_add <= ISP_REG_OECF_Y3_CFG6;) {
> + stf_isp_reg_write(stfcamss, reg_add, par_val);
> + reg_add += 0x20;
> + }
> +
> + par_h = 0x3fe; par_l = 0x380;
> + par_val = OCEF_PAR_H(par_h) | OCEF_PAR_L(par_l);
> + for (reg_add = ISP_REG_OECF_X0_CFG7; reg_add <= ISP_REG_OECF_Y3_CFG7;) {
> + stf_isp_reg_write(stfcamss, reg_add, par_val);
> + reg_add += 0x20;
> + }
> +
> + par_h = 0x80; par_l = 0x80;
> + par_val = OCEF_PAR_H(par_h) | OCEF_PAR_L(par_l);
> + for (reg_add = ISP_REG_OECF_S0_CFG0; reg_add <= ISP_REG_OECF_S3_CFG7;) {
> + stf_isp_reg_write(stfcamss, reg_add, par_val);
> + reg_add += 4;
> + }
> +}
> +
> +static void stf_isp_config_lccf(struct stfcamss *stfcamss)
> +{
> + u32 reg_add;
> +
> + stf_isp_reg_write(stfcamss, ISP_REG_LCCF_CFG_0,
> + Y_DISTANCE(0x21C) | X_DISTANCE(0x3C0));
> + stf_isp_reg_write(stfcamss, ISP_REG_LCCF_CFG_1, LCCF_MAX_DIS(0xb));
> +
> + for (reg_add = ISP_REG_LCCF_CFG_2; reg_add <= ISP_REG_LCCF_CFG_5;) {
> + stf_isp_reg_write(stfcamss, reg_add,
> + LCCF_F2_PAR(0x0) | LCCF_F1_PAR(0x0));
> + reg_add += 4;
> + }
> +}
> +
> +static void stf_isp_config_awb(struct stfcamss *stfcamss)
> +{
> + u32 reg_val, reg_add;
> + u16 symbol_h, symbol_l;
> +
> + symbol_h = 0x0; symbol_l = 0x0;
> + reg_val = AWB_X_SYMBOL_H(symbol_h) | AWB_X_SYMBOL_L(symbol_l);
> +
> + for (reg_add = ISP_REG_AWB_X0_CFG_0; reg_add <= ISP_REG_AWB_X3_CFG_1;) {
> + stf_isp_reg_write(stfcamss, reg_add, reg_val);
> + reg_add += 4;
> + }
> +
> + symbol_h = 0x0, symbol_l = 0x0;
> + reg_val = AWB_Y_SYMBOL_H(symbol_h) | AWB_Y_SYMBOL_L(symbol_l);
> +
> + for (reg_add = ISP_REG_AWB_Y0_CFG_0; reg_add <= ISP_REG_AWB_Y3_CFG_1;) {
> + stf_isp_reg_write(stfcamss, reg_add, reg_val);
> + reg_add += 4;
> + }
> +
> + symbol_h = 0x80, symbol_l = 0x80;
> + reg_val = AWB_S_SYMBOL_H(symbol_h) | AWB_S_SYMBOL_L(symbol_l);
> +
> + for (reg_add = ISP_REG_AWB_S0_CFG_0; reg_add <= ISP_REG_AWB_S3_CFG_1;) {
> + stf_isp_reg_write(stfcamss, reg_add, reg_val);
> + reg_add += 4;
> + }
> +}
> +
> +static void stf_isp_config_grgb(struct stfcamss *stfcamss)
> +{
> + stf_isp_reg_write(stfcamss, ISP_REG_ICTC,
> + GF_MODE(1) | MAXGT(0x140) | MINGT(0x40));
> + stf_isp_reg_write(stfcamss, ISP_REG_IDBC, BADGT(0x200) | BADXT(0x200));
> +}
> +
> +static void stf_isp_config_cfa(struct stfcamss *stfcamss)
> +{
> + stf_isp_reg_write(stfcamss, ISP_REG_RAW_FORMAT_CFG,
> + SMY13(0) | SMY12(1) | SMY11(0) | SMY10(1) | SMY3(2) |
> + SMY2(3) | SMY1(2) | SMY0(3));
> + stf_isp_reg_write(stfcamss, ISP_REG_ICFAM, CROSS_COV(3) | HV_W(2));
> +}
> +
> +static void stf_isp_config_ccm(struct stfcamss *stfcamss)
> +{
> + u32 reg_add;
> +
> + stf_isp_reg_write(stfcamss, ISP_REG_ICAMD_0, DNRM_F(6) | CCM_M_DAT(0));
> +
> + for (reg_add = ISP_REG_ICAMD_12; reg_add <= ISP_REG_ICAMD_20;) {
> + stf_isp_reg_write(stfcamss, reg_add, CCM_M_DAT(0x80));
> + reg_add += 0x10;
> + }
> +
> + stf_isp_reg_write(stfcamss, ISP_REG_ICAMD_24, CCM_M_DAT(0x700));
> + stf_isp_reg_write(stfcamss, ISP_REG_ICAMD_25, CCM_M_DAT(0x200));
> +}
> +
> +static void stf_isp_config_gamma(struct stfcamss *stfcamss)
> +{
> + u32 reg_val, reg_add;
> + u16 gamma_slope_v, gamma_v;
> +
> + gamma_slope_v = 0x2400; gamma_v = 0x0;
> + reg_val = GAMMA_S_VAL(gamma_slope_v) | GAMMA_VAL(gamma_v);
> + stf_isp_reg_write(stfcamss, ISP_REG_GAMMA_VAL0, reg_val);
> +
> + gamma_slope_v = 0x800; gamma_v = 0x20;
> + for (reg_add = ISP_REG_GAMMA_VAL1; reg_add <= ISP_REG_GAMMA_VAL7;) {
> + reg_val = GAMMA_S_VAL(gamma_slope_v) | GAMMA_VAL(gamma_v);
> + stf_isp_reg_write(stfcamss, reg_add, reg_val);
> + reg_add += 4;
> + gamma_v += 0x20;
> + }
> +
> + gamma_v = 0x100;
> + for (reg_add = ISP_REG_GAMMA_VAL8; reg_add <= ISP_REG_GAMMA_VAL13;) {
> + reg_val = GAMMA_S_VAL(gamma_slope_v) | GAMMA_VAL(gamma_v);
> + stf_isp_reg_write(stfcamss, reg_add, reg_val);
> + reg_add += 4;
> + gamma_v += 0x80;
> + }
> +
> + gamma_v = 0x3fe;
> + reg_val = GAMMA_S_VAL(gamma_slope_v) | GAMMA_VAL(gamma_v);
> + stf_isp_reg_write(stfcamss, ISP_REG_GAMMA_VAL14, reg_val);
> +}
> +
> +static void stf_isp_config_r2y(struct stfcamss *stfcamss)
> +{
> + stf_isp_reg_write(stfcamss, ISP_REG_R2Y_0, CSC_M(0x4C));
> + stf_isp_reg_write(stfcamss, ISP_REG_R2Y_1, CSC_M(0x97));
> + stf_isp_reg_write(stfcamss, ISP_REG_R2Y_2, CSC_M(0x1d));
> + stf_isp_reg_write(stfcamss, ISP_REG_R2Y_3, CSC_M(0x1d5));
> + stf_isp_reg_write(stfcamss, ISP_REG_R2Y_4, CSC_M(0x1ac));
> + stf_isp_reg_write(stfcamss, ISP_REG_R2Y_5, CSC_M(0x80));
> + stf_isp_reg_write(stfcamss, ISP_REG_R2Y_6, CSC_M(0x80));
> + stf_isp_reg_write(stfcamss, ISP_REG_R2Y_7, CSC_M(0x194));
> + stf_isp_reg_write(stfcamss, ISP_REG_R2Y_8, CSC_M(0x1ec));
> +}
> +
> +static void stf_isp_config_y_curve(struct stfcamss *stfcamss)
> +{
> + u32 reg_add;
> + u16 y_curve;
> +
> + y_curve = 0x0;
> + for (reg_add = ISP_REG_YCURVE_0; reg_add <= ISP_REG_YCURVE_63;) {
> + stf_isp_reg_write(stfcamss, reg_add, y_curve);
> + reg_add += 4;
> + y_curve += 0x10;
> + }
> +}
> +
> +static void stf_isp_config_sharpen(struct stfcamss *sc)
> +{
> + u32 reg_add;
> +
> + stf_isp_reg_write(sc, ISP_REG_SHARPEN0, S_DELTA(0x7) | S_WEIGHT(0xf));
> + stf_isp_reg_write(sc, ISP_REG_SHARPEN1, S_DELTA(0x18) | S_WEIGHT(0xf));
> + stf_isp_reg_write(sc, ISP_REG_SHARPEN2, S_DELTA(0x80) | S_WEIGHT(0xf));
> + stf_isp_reg_write(sc, ISP_REG_SHARPEN3, S_DELTA(0x100) | S_WEIGHT(0xf));
> + stf_isp_reg_write(sc, ISP_REG_SHARPEN4, S_DELTA(0x10) | S_WEIGHT(0xf));
> + stf_isp_reg_write(sc, ISP_REG_SHARPEN5, S_DELTA(0x60) | S_WEIGHT(0xf));
> + stf_isp_reg_write(sc, ISP_REG_SHARPEN6, S_DELTA(0x100) | S_WEIGHT(0xf));
> + stf_isp_reg_write(sc, ISP_REG_SHARPEN7, S_DELTA(0x190) | S_WEIGHT(0xf));
> + stf_isp_reg_write(sc, ISP_REG_SHARPEN8, S_DELTA(0x0) | S_WEIGHT(0xf));
> +
> + for (reg_add = ISP_REG_SHARPEN9; reg_add <= ISP_REG_SHARPEN14;) {
> + stf_isp_reg_write(sc, reg_add, S_WEIGHT(0xf));
> + reg_add += 4;
> + }
> +
> + for (reg_add = ISP_REG_SHARPEN_FS0; reg_add <= ISP_REG_SHARPEN_FS5;) {
> + stf_isp_reg_write(sc, reg_add, S_FACTOR(0x10) | S_SLOPE(0x0));
> + reg_add += 4;
> + }
> +
> + stf_isp_reg_write(sc, ISP_REG_SHARPEN_WN,
> + PDIRF(0x8) | NDIRF(0x8) | WSUM(0xd7c));
> + stf_isp_reg_write(sc, ISP_REG_IUVS1, UVDIFF2(0xC0) | UVDIFF1(0x40));
> + stf_isp_reg_write(sc, ISP_REG_IUVS2, UVF(0xff) | UVSLOPE(0x0));
> + stf_isp_reg_write(sc, ISP_REG_IUVCKS1,
> + UVCKDIFF2(0xa0) | UVCKDIFF1(0x40));
> +}
> +
> +static void stf_isp_config_dnyuv(struct stfcamss *stfcamss)
> +{
> + u32 reg_val;
> +
> + reg_val = YUVSW5(7) | YUVSW4(7) | YUVSW3(7) | YUVSW2(7) |
> + YUVSW1(7) | YUVSW0(7);
> + stf_isp_reg_write(stfcamss, ISP_REG_DNYUV_YSWR0, reg_val);
> + stf_isp_reg_write(stfcamss, ISP_REG_DNYUV_CSWR0, reg_val);
> +
> + reg_val = YUVSW3(7) | YUVSW2(7) | YUVSW1(7) | YUVSW0(7);
> + stf_isp_reg_write(stfcamss, ISP_REG_DNYUV_YSWR1, reg_val);
> + stf_isp_reg_write(stfcamss, ISP_REG_DNYUV_CSWR1, reg_val);
> +
> + reg_val = CURVE_D_H(0x60) | CURVE_D_L(0x40);
> + stf_isp_reg_write(stfcamss, ISP_REG_DNYUV_YDR0, reg_val);
> + stf_isp_reg_write(stfcamss, ISP_REG_DNYUV_CDR0, reg_val);
> +
> + reg_val = CURVE_D_H(0xd8) | CURVE_D_L(0x90);
> + stf_isp_reg_write(stfcamss, ISP_REG_DNYUV_YDR1, reg_val);
> + stf_isp_reg_write(stfcamss, ISP_REG_DNYUV_CDR1, reg_val);
> +
> + reg_val = CURVE_D_H(0x1e6) | CURVE_D_L(0x144);
> + stf_isp_reg_write(stfcamss, ISP_REG_DNYUV_YDR2, reg_val);
> + stf_isp_reg_write(stfcamss, ISP_REG_DNYUV_CDR2, reg_val);
> +}
> +
> +static void stf_isp_config_sat(struct stfcamss *stfcamss)
> +{
> + stf_isp_reg_write(stfcamss, ISP_REG_CS_GAIN, CMAD(0x0) | CMAB(0x100));
> + stf_isp_reg_write(stfcamss, ISP_REG_CS_THRESHOLD, CMD(0x1f) | CMB(0x1));
> + stf_isp_reg_write(stfcamss, ISP_REG_CS_OFFSET, VOFF(0x0) | UOFF(0x0));
> + stf_isp_reg_write(stfcamss, ISP_REG_CS_HUE_F, SIN(0x0) | COS(0x100));
> + stf_isp_reg_write(stfcamss, ISP_REG_CS_SCALE, CMSF(0x8));
> + stf_isp_reg_write(stfcamss, ISP_REG_YADJ0, YOIR(0x401) | YIMIN(0x1));
> + stf_isp_reg_write(stfcamss, ISP_REG_YADJ1, YOMAX(0x3ff) | YOMIN(0x1));
> +}
> +
> +int stf_isp_clk_enable(struct stf_isp_dev *isp_dev)
> +{
> + struct stfcamss *stfcamss = isp_dev->stfcamss;
> +
> + clk_prepare_enable(stfcamss->sys_clk[STF_CLK_WRAPPER_CLK_C].clk);
> + reset_control_deassert(stfcamss->sys_rst[STF_RST_WRAPPER_C].rstc);
> + reset_control_deassert(stfcamss->sys_rst[STF_RST_WRAPPER_P].rstc);
Missing error handling.
Is there a specific hardware requirement to split the clock enabling and
reset deassertion in two, with some of them handled in the runtime PM
resume handler and the rest here ? If not, moving all clock and reset
control to the runtime PM handlers would simplify the code.
> +
> + return 0;
> +}
> +
> +int stf_isp_clk_disable(struct stf_isp_dev *isp_dev)
> +{
> + struct stfcamss *stfcamss = isp_dev->stfcamss;
> +
> + reset_control_assert(stfcamss->sys_rst[STF_RST_WRAPPER_C].rstc);
> + reset_control_assert(stfcamss->sys_rst[STF_RST_WRAPPER_P].rstc);
> + clk_disable_unprepare(stfcamss->sys_clk[STF_CLK_WRAPPER_CLK_C].clk);
> +
> + return 0;
> +}
> +
> +int stf_isp_reset(struct stf_isp_dev *isp_dev)
> +{
> + stf_isp_reg_set_bit(isp_dev->stfcamss, ISP_REG_ISP_CTRL_0,
> + ISPC_RST_MASK, ISPC_RST);
> + stf_isp_reg_set_bit(isp_dev->stfcamss, ISP_REG_ISP_CTRL_0,
> + ISPC_RST_MASK, 0);
> +
> + return 0;
> +}
> +
> +void stf_isp_init_cfg(struct stf_isp_dev *isp_dev)
> +{
> + stf_isp_reg_write(isp_dev->stfcamss, ISP_REG_DC_CFG_1, DC_AXI_ID(0x0));
> + stf_isp_reg_write(isp_dev->stfcamss, ISP_REG_DEC_CFG,
> + DEC_V_KEEP(0x0) |
> + DEC_V_PERIOD(0x0) |
> + DEC_H_KEEP(0x0) |
> + DEC_H_PERIOD(0x0));
> +
> + stf_isp_config_obc(isp_dev->stfcamss);
> + stf_isp_config_oecf(isp_dev->stfcamss);
> + stf_isp_config_lccf(isp_dev->stfcamss);
> + stf_isp_config_awb(isp_dev->stfcamss);
> + stf_isp_config_grgb(isp_dev->stfcamss);
> + stf_isp_config_cfa(isp_dev->stfcamss);
> + stf_isp_config_ccm(isp_dev->stfcamss);
> + stf_isp_config_gamma(isp_dev->stfcamss);
> + stf_isp_config_r2y(isp_dev->stfcamss);
> + stf_isp_config_y_curve(isp_dev->stfcamss);
> + stf_isp_config_sharpen(isp_dev->stfcamss);
> + stf_isp_config_dnyuv(isp_dev->stfcamss);
> + stf_isp_config_sat(isp_dev->stfcamss);
All these parameters are hardcoded, why are they not exposed to
userspace ?
> +
> + stf_isp_reg_write(isp_dev->stfcamss, ISP_REG_CSI_MODULE_CFG,
> + CSI_DUMP_EN | CSI_SC_EN | CSI_AWB_EN |
> + CSI_LCCF_EN | CSI_OECF_EN | CSI_OBC_EN | CSI_DEC_EN);
> + stf_isp_reg_write(isp_dev->stfcamss, ISP_REG_ISP_CTRL_1,
> + CTRL_SAT(1) | CTRL_DBC | CTRL_CTC | CTRL_YHIST |
> + CTRL_YCURVE | CTRL_BIYUV | CTRL_SCE | CTRL_EE |
> + CTRL_CCE | CTRL_RGE | CTRL_CME | CTRL_AE | CTRL_CE);
> +}
> +
> +static void stf_isp_config_crop(struct stfcamss *stfcamss,
> + struct stf_isp_crop *crop)
> +{
> + struct v4l2_rect *rect = &crop[STF_ISP_PAD_SRC].rect;
> + u32 bpp = crop[STF_ISP_PAD_SRC].bpp;
> + u32 val;
> +
> + val = VSTART_CAP(rect->top) | HSTART_CAP(rect->left);
> + stf_isp_reg_write(stfcamss, ISP_REG_PIC_CAPTURE_START_CFG, val);
> +
> + val = VEND_CAP(rect->height + rect->top - 1) |
> + HEND_CAP(rect->width + rect->left - 1);
> + stf_isp_reg_write(stfcamss, ISP_REG_PIC_CAPTURE_END_CFG, val);
> +
> + val = H_ACT_CAP(rect->height) | W_ACT_CAP(rect->width);
> + stf_isp_reg_write(stfcamss, ISP_REG_PIPELINE_XY_SIZE, val);
> +
> + val = ALIGN(rect->width * bpp / 8, STFCAMSS_FRAME_WIDTH_ALIGN_8);
> + stf_isp_reg_write(stfcamss, ISP_REG_STRIDE, val);
> +}
> +
> +static void stf_isp_config_raw_fmt(struct stfcamss *stfcamss, u32 mcode)
> +{
> + u32 val, val1;
> +
> + switch (mcode) {
> + case MEDIA_BUS_FMT_SRGGB10_1X10:
> + case MEDIA_BUS_FMT_SRGGB8_1X8:
> + /* 3 2 3 2 1 0 1 0 B Gb B Gb Gr R Gr R */
> + val = SMY13(3) | SMY12(2) | SMY11(3) | SMY10(2) |
> + SMY3(1) | SMY2(0) | SMY1(1) | SMY0(0);
> + val1 = CTRL_SAT(0x0);
> + break;
> + case MEDIA_BUS_FMT_SGRBG10_1X10:
> + case MEDIA_BUS_FMT_SGRBG8_1X8:
> + /* 2 3 2 3 0 1 0 1, Gb B Gb B R Gr R Gr */
> + val = SMY13(2) | SMY12(3) | SMY11(2) | SMY10(3) |
> + SMY3(0) | SMY2(1) | SMY1(0) | SMY0(1);
> + val1 = CTRL_SAT(0x2);
> + break;
> + case MEDIA_BUS_FMT_SGBRG10_1X10:
> + case MEDIA_BUS_FMT_SGBRG8_1X8:
> + /* 1 0 1 0 3 2 3 2, Gr R Gr R B Gb B Gb */
> + val = SMY13(1) | SMY12(0) | SMY11(1) | SMY10(0) |
> + SMY3(3) | SMY2(2) | SMY1(3) | SMY0(2);
> + val1 = CTRL_SAT(0x3);
> + break;
> + case MEDIA_BUS_FMT_SBGGR10_1X10:
> + case MEDIA_BUS_FMT_SBGGR8_1X8:
> + /* 0 1 0 1 2 3 2 3 R Gr R Gr Gb B Gb B */
> + val = SMY13(0) | SMY12(1) | SMY11(0) | SMY10(1) |
> + SMY3(2) | SMY2(3) | SMY1(2) | SMY0(3);
> + val1 = CTRL_SAT(0x1);
> + break;
> + default:
> + val = SMY13(0) | SMY12(1) | SMY11(0) | SMY10(1) |
> + SMY3(2) | SMY2(3) | SMY1(2) | SMY0(3);
> + val1 = CTRL_SAT(0x1);
> + break;
> + }
> + stf_isp_reg_write(stfcamss, ISP_REG_RAW_FORMAT_CFG, val);
> + stf_isp_reg_set_bit(stfcamss, ISP_REG_ISP_CTRL_1, CTRL_SAT_MASK, val1);
> +}
> +
> +void stf_isp_settings(struct stf_isp_dev *isp_dev,
> + struct stf_isp_crop *crop, u32 mcode)
> +{
> + struct stfcamss *stfcamss = isp_dev->stfcamss;
> +
> + stf_isp_config_crop(stfcamss, crop);
> + stf_isp_config_raw_fmt(stfcamss, mcode);
> +
> + stf_isp_reg_set_bit(stfcamss, ISP_REG_DUMP_CFG_1,
> + DUMP_BURST_LEN_MASK | DUMP_SD_MASK,
> + DUMP_BURST_LEN(3));
> +
> + stf_isp_reg_write(stfcamss, ISP_REG_ITIIWSR,
> + ITI_HSIZE(IMAGE_MAX_HEIGH) |
> + ITI_WSIZE(IMAGE_MAX_WIDTH));
> + stf_isp_reg_write(stfcamss, ISP_REG_ITIDWLSR, ITI_WSTRIDE(0x960));
> + stf_isp_reg_write(stfcamss, ISP_REG_ITIDRLSR, ITI_STRIDE_L(0x960));
> + stf_isp_reg_write(stfcamss, ISP_REG_SENSOR, 0x1);
Should 0x1 be IMAGER_SEL(1) or are those unrelated ?
> +}
> +
> +void stf_isp_stream_set(struct stf_isp_dev *isp_dev)
> +{
> + struct stfcamss *stfcamss = isp_dev->stfcamss;
> +
> + stf_isp_reg_write_delay(stfcamss, ISP_REG_ISP_CTRL_0,
> + ISPC_ENUO | ISPC_ENLS | ISPC_RST, 10);
> + stf_isp_reg_write_delay(stfcamss, ISP_REG_ISP_CTRL_0,
> + ISPC_ENUO | ISPC_ENLS, 10);
> + stf_isp_reg_write(stfcamss, ISP_REG_IESHD, SHAD_UP_M);
> + stf_isp_reg_write_delay(stfcamss, ISP_REG_ISP_CTRL_0,
> + ISPC_ENUO | ISPC_ENLS | ISPC_EN, 10);
> + stf_isp_reg_write_delay(stfcamss, ISP_REG_CSIINTS,
> + CSI_INTS(1) | CSI_SHA_M(4), 10);
> + stf_isp_reg_write_delay(stfcamss, ISP_REG_CSIINTS,
> + CSI_INTS(2) | CSI_SHA_M(4), 10);
> + stf_isp_reg_write_delay(stfcamss, ISP_REG_CSI_INPUT_EN_AND_STATUS,
> + CSI_EN_S, 10);
> +}
--
Regards,
Laurent Pinchart