* [PATCH v1 1/7] staging: media: starfive: Replaced current_fmt with get from sd_state
2024-03-06 9:33 [PATCH v1 0/7] Add ISP RAW for StarFive Changhuang Liang
@ 2024-03-06 9:33 ` Changhuang Liang
2024-03-06 14:23 ` Dan Carpenter
2024-03-06 9:33 ` [PATCH v1 2/7] staging: media: starfive: Add raw pad for ISP Changhuang Liang
` (5 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Changhuang Liang @ 2024-03-06 9:33 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Greg Kroah-Hartman
Cc: Hans Verkuil, Laurent Pinchart, Jack Zhu, Changhuang Liang,
linux-media, linux-kernel, linux-staging
current_fmt only can store one pad format, when setting other pad it
will be overwrote. Replaced it with get from sd_state directly.
Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
.../media/starfive/camss/stf-isp-hw-ops.c | 10 ++++---
.../staging/media/starfive/camss/stf-isp.c | 26 +++++++++++++++----
.../staging/media/starfive/camss/stf-isp.h | 3 ++-
3 files changed, 30 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/media/starfive/camss/stf-isp-hw-ops.c b/drivers/staging/media/starfive/camss/stf-isp-hw-ops.c
index c34631ff9422..b933d425cbd0 100644
--- a/drivers/staging/media/starfive/camss/stf-isp-hw-ops.c
+++ b/drivers/staging/media/starfive/camss/stf-isp-hw-ops.c
@@ -345,7 +345,6 @@ void stf_isp_init_cfg(struct stf_isp_dev *isp_dev)
static void stf_isp_config_crop(struct stfcamss *stfcamss,
struct v4l2_rect *crop)
{
- u32 bpp = stfcamss->isp_dev.current_fmt->bpp;
u32 val;
val = VSTART_CAP(crop->top) | HSTART_CAP(crop->left);
@@ -357,9 +356,14 @@ static void stf_isp_config_crop(struct stfcamss *stfcamss,
val = H_ACT_CAP(crop->height) | W_ACT_CAP(crop->width);
stf_isp_reg_write(stfcamss, ISP_REG_PIPELINE_XY_SIZE, val);
+}
+
+void stf_isp_config_yuv_out_stride(struct stf_isp_dev *isp_dev,
+ u32 width, u8 bpp)
+{
+ u32 val = ALIGN(width * bpp / 8, STFCAMSS_FRAME_WIDTH_ALIGN_8);
- val = ALIGN(crop->width * bpp / 8, STFCAMSS_FRAME_WIDTH_ALIGN_8);
- stf_isp_reg_write(stfcamss, ISP_REG_STRIDE, val);
+ stf_isp_reg_write(isp_dev->stfcamss, ISP_REG_STRIDE, val);
}
static void stf_isp_config_raw_fmt(struct stfcamss *stfcamss, u32 mcode)
diff --git a/drivers/staging/media/starfive/camss/stf-isp.c b/drivers/staging/media/starfive/camss/stf-isp.c
index d50616ef351e..a5573abe0d7b 100644
--- a/drivers/staging/media/starfive/camss/stf-isp.c
+++ b/drivers/staging/media/starfive/camss/stf-isp.c
@@ -46,6 +46,19 @@ stf_g_fmt_by_mcode(const struct stf_isp_format_table *fmt_table, u32 mcode)
return NULL;
}
+static int stf_isp_g_index_by_mcode(const struct stf_isp_format_table *fmt_table,
+ u32 mcode)
+{
+ int i;
+
+ for (i = 0; i < fmt_table->nfmts; i++) {
+ if (fmt_table->fmts[i].code == mcode)
+ return i;
+ }
+
+ return -EINVAL;
+}
+
int stf_isp_init(struct stfcamss *stfcamss)
{
struct stf_isp_dev *isp_dev = &stfcamss->isp_dev;
@@ -53,7 +66,6 @@ int stf_isp_init(struct stfcamss *stfcamss)
isp_dev->stfcamss = stfcamss;
isp_dev->formats = isp_formats_st7110;
isp_dev->nformats = ARRAY_SIZE(isp_formats_st7110);
- isp_dev->current_fmt = &isp_formats_source[0];
return 0;
}
@@ -61,18 +73,25 @@ int stf_isp_init(struct stfcamss *stfcamss)
static int isp_set_stream(struct v4l2_subdev *sd, int enable)
{
struct stf_isp_dev *isp_dev = v4l2_get_subdevdata(sd);
+ const struct stf_isp_format_table *fmt_t_src;
+ struct v4l2_mbus_framefmt *fmt, *fmt_src;
struct v4l2_subdev_state *sd_state;
- struct v4l2_mbus_framefmt *fmt;
struct v4l2_rect *crop;
+ int src;
sd_state = v4l2_subdev_lock_and_get_active_state(sd);
+ fmt_t_src = &isp_dev->formats[STF_ISP_PAD_SRC];
fmt = v4l2_subdev_state_get_format(sd_state, STF_ISP_PAD_SINK);
+ fmt_src = v4l2_subdev_state_get_format(sd_state, STF_ISP_PAD_SRC);
crop = v4l2_subdev_state_get_crop(sd_state, STF_ISP_PAD_SRC);
+ src = stf_isp_g_index_by_mcode(fmt_t_src, fmt_src->code);
if (enable) {
stf_isp_reset(isp_dev);
stf_isp_init_cfg(isp_dev);
stf_isp_settings(isp_dev, crop, fmt->code);
+ stf_isp_config_yuv_out_stride(isp_dev, crop->width,
+ fmt_t_src->fmts[src].bpp);
stf_isp_stream_set(isp_dev);
}
@@ -157,9 +176,6 @@ static int isp_set_format(struct v4l2_subdev *sd,
isp_try_format(isp_dev, state, fmt->pad, &fmt->format);
*format = fmt->format;
- isp_dev->current_fmt = stf_g_fmt_by_mcode(&isp_dev->formats[fmt->pad],
- fmt->format.code);
-
/* Propagate to in crop */
if (fmt->pad == STF_ISP_PAD_SINK) {
struct v4l2_subdev_selection sel = { 0 };
diff --git a/drivers/staging/media/starfive/camss/stf-isp.h b/drivers/staging/media/starfive/camss/stf-isp.h
index 955cbb048363..07d6c2758253 100644
--- a/drivers/staging/media/starfive/camss/stf-isp.h
+++ b/drivers/staging/media/starfive/camss/stf-isp.h
@@ -413,7 +413,6 @@ struct stf_isp_dev {
const struct stf_isp_format_table *formats;
unsigned int nformats;
struct v4l2_subdev *source_subdev;
- const struct stf_isp_format *current_fmt;
};
int stf_isp_reset(struct stf_isp_dev *isp_dev);
@@ -421,6 +420,8 @@ void stf_isp_init_cfg(struct stf_isp_dev *isp_dev);
void stf_isp_settings(struct stf_isp_dev *isp_dev,
struct v4l2_rect *crop, u32 mcode);
void stf_isp_stream_set(struct stf_isp_dev *isp_dev);
+void stf_isp_config_yuv_out_stride(struct stf_isp_dev *isp_dev,
+ u32 width, u8 bpp);
int stf_isp_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);
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v1 1/7] staging: media: starfive: Replaced current_fmt with get from sd_state
2024-03-06 9:33 ` [PATCH v1 1/7] staging: media: starfive: Replaced current_fmt with get from sd_state Changhuang Liang
@ 2024-03-06 14:23 ` Dan Carpenter
2024-03-07 1:58 ` 回复: " Changhuang Liang
0 siblings, 1 reply; 19+ messages in thread
From: Dan Carpenter @ 2024-03-06 14:23 UTC (permalink / raw)
To: Changhuang Liang
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, Hans Verkuil,
Laurent Pinchart, Jack Zhu, linux-media, linux-kernel,
linux-staging
On Wed, Mar 06, 2024 at 01:33:28AM -0800, Changhuang Liang wrote:
> current_fmt only can store one pad format, when setting other pad it
> will be overwrote. Replaced it with get from sd_state directly.
>
These commit descriptions are kind of hard to understand so I have
proposed new commit messages.
Subject: staging: media: starfive: Get rid of current_fmt
We want to support mutiple formats so saving one "current_fmt" doesn't
work. This was only used to set the ISP_REG_STRIDE so use the sd_state
directly for that and delete the ->current_fmt pointer. No functional
change.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 19+ messages in thread
* 回复: [PATCH v1 1/7] staging: media: starfive: Replaced current_fmt with get from sd_state
2024-03-06 14:23 ` Dan Carpenter
@ 2024-03-07 1:58 ` Changhuang Liang
0 siblings, 0 replies; 19+ messages in thread
From: Changhuang Liang @ 2024-03-07 1:58 UTC (permalink / raw)
To: Dan Carpenter
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, Hans Verkuil,
Laurent Pinchart, Jack Zhu, linux-media@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-staging@lists.linux.dev
Hi, Dan
Thanks for your comments.
> -----邮件原件-----
> 发件人: Dan Carpenter <dan.carpenter@linaro.org>
> 发送时间: 2024年3月6日 22:23
> 收件人: Changhuang Liang <changhuang.liang@starfivetech.com>
> 抄送: Mauro Carvalho Chehab <mchehab@kernel.org>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; Hans Verkuil <hverkuil-cisco@xs4all.nl>;
> Laurent Pinchart <laurent.pinchart@ideasonboard.com>; Jack Zhu
> <jack.zhu@starfivetech.com>; linux-media@vger.kernel.org;
> linux-kernel@vger.kernel.org; linux-staging@lists.linux.dev
> 主题: Re: [PATCH v1 1/7] staging: media: starfive: Replaced current_fmt with
> get from sd_state
>
> On Wed, Mar 06, 2024 at 01:33:28AM -0800, Changhuang Liang wrote:
> > current_fmt only can store one pad format, when setting other pad it
> > will be overwrote. Replaced it with get from sd_state directly.
> >
>
> These commit descriptions are kind of hard to understand so I have proposed
> new commit messages.
>
> Subject: staging: media: starfive: Get rid of current_fmt
>
> We want to support mutiple formats so saving one "current_fmt" doesn't
> work. This was only used to set the ISP_REG_STRIDE so use the sd_state
> directly for that and delete the ->current_fmt pointer. No functional change.
>
I will re-change the patch description and add more details to all the patches.
Thanks
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v1 2/7] staging: media: starfive: Add raw pad for ISP
2024-03-06 9:33 [PATCH v1 0/7] Add ISP RAW for StarFive Changhuang Liang
2024-03-06 9:33 ` [PATCH v1 1/7] staging: media: starfive: Replaced current_fmt with get from sd_state Changhuang Liang
@ 2024-03-06 9:33 ` Changhuang Liang
2024-03-06 14:23 ` Dan Carpenter
2024-03-06 9:33 ` [PATCH v1 3/7] staging: media: starfive: Sink rectangle set to ISP source pad Changhuang Liang
` (4 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Changhuang Liang @ 2024-03-06 9:33 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Greg Kroah-Hartman
Cc: Hans Verkuil, Laurent Pinchart, Jack Zhu, Changhuang Liang,
linux-media, linux-kernel, linux-staging
Add raw pad for ISP, it supported the conversion of RAW10 into RAW12.
Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
.../staging/media/starfive/camss/stf-isp.c | 26 ++++++++++++-------
.../staging/media/starfive/camss/stf-isp.h | 1 +
2 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/media/starfive/camss/stf-isp.c b/drivers/staging/media/starfive/camss/stf-isp.c
index a5573abe0d7b..6bab0ac23120 100644
--- a/drivers/staging/media/starfive/camss/stf-isp.c
+++ b/drivers/staging/media/starfive/camss/stf-isp.c
@@ -10,9 +10,6 @@
#include "stf-camss.h"
-#define SINK_FORMATS_INDEX 0
-#define SOURCE_FORMATS_INDEX 1
-
static int isp_set_selection(struct v4l2_subdev *sd,
struct v4l2_subdev_state *state,
struct v4l2_subdev_selection *sel);
@@ -28,9 +25,17 @@ static const struct stf_isp_format isp_formats_source[] = {
{ MEDIA_BUS_FMT_YUYV8_1_5X8, 8 },
};
+static const struct stf_isp_format isp_formats_source_raw[] = {
+ { MEDIA_BUS_FMT_SRGGB12_1X12, 12 },
+ { MEDIA_BUS_FMT_SGRBG12_1X12, 12 },
+ { MEDIA_BUS_FMT_SGBRG12_1X12, 12 },
+ { MEDIA_BUS_FMT_SBGGR12_1X12, 12 },
+};
+
static const struct stf_isp_format_table isp_formats_st7110[] = {
{ isp_formats_sink, ARRAY_SIZE(isp_formats_sink) },
{ isp_formats_source, ARRAY_SIZE(isp_formats_source) },
+ { isp_formats_source_raw, ARRAY_SIZE(isp_formats_source_raw) },
};
static const struct stf_isp_format *
@@ -113,10 +118,7 @@ static void isp_try_format(struct stf_isp_dev *isp_dev,
return;
}
- if (pad == STF_ISP_PAD_SINK)
- formats = &isp_dev->formats[SINK_FORMATS_INDEX];
- else if (pad == STF_ISP_PAD_SRC)
- formats = &isp_dev->formats[SOURCE_FORMATS_INDEX];
+ formats = &isp_dev->formats[pad];
fmt->width = clamp_t(u32, fmt->width, STFCAMSS_FRAME_MIN_WIDTH,
STFCAMSS_FRAME_MAX_WIDTH);
@@ -142,7 +144,7 @@ static int isp_enum_mbus_code(struct v4l2_subdev *sd,
if (code->index >= ARRAY_SIZE(isp_formats_sink))
return -EINVAL;
- formats = &isp_dev->formats[SINK_FORMATS_INDEX];
+ formats = &isp_dev->formats[code->pad];
code->code = formats->fmts[code->index].code;
} else {
struct v4l2_mbus_framefmt *sink_fmt;
@@ -281,8 +283,11 @@ static int isp_set_selection(struct v4l2_subdev *sd,
crop.target = V4L2_SEL_TGT_CROP;
crop.r = *rect;
isp_set_selection(sd, state, &crop);
+
+ crop.pad = STF_ISP_PAD_SRC_RAW;
+ isp_set_selection(sd, state, &crop);
} else if (sel->target == V4L2_SEL_TGT_CROP &&
- sel->pad == STF_ISP_PAD_SRC) {
+ (sel->pad == STF_ISP_PAD_SRC || sel->pad == STF_ISP_PAD_SRC_RAW)) {
struct v4l2_subdev_format fmt = { 0 };
rect = v4l2_subdev_state_get_crop(state, sel->pad);
@@ -294,7 +299,7 @@ static int isp_set_selection(struct v4l2_subdev *sd,
/* Reset source pad format width and height */
fmt.which = sel->which;
- fmt.pad = STF_ISP_PAD_SRC;
+ fmt.pad = sel->pad;
fmt.format.width = rect->width;
fmt.format.height = rect->height;
isp_set_format(sd, state, &fmt);
@@ -361,6 +366,7 @@ int stf_isp_register(struct stf_isp_dev *isp_dev, struct v4l2_device *v4l2_dev)
pads[STF_ISP_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
pads[STF_ISP_PAD_SRC].flags = MEDIA_PAD_FL_SOURCE;
+ pads[STF_ISP_PAD_SRC_RAW].flags = MEDIA_PAD_FL_SOURCE;
sd->entity.function = MEDIA_ENT_F_PROC_VIDEO_ISP;
sd->entity.ops = &isp_media_ops;
diff --git a/drivers/staging/media/starfive/camss/stf-isp.h b/drivers/staging/media/starfive/camss/stf-isp.h
index 07d6c2758253..4fc5cfac115c 100644
--- a/drivers/staging/media/starfive/camss/stf-isp.h
+++ b/drivers/staging/media/starfive/camss/stf-isp.h
@@ -393,6 +393,7 @@
enum stf_isp_pad_id {
STF_ISP_PAD_SINK = 0,
STF_ISP_PAD_SRC,
+ STF_ISP_PAD_SRC_RAW,
STF_ISP_PAD_MAX
};
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v1 2/7] staging: media: starfive: Add raw pad for ISP
2024-03-06 9:33 ` [PATCH v1 2/7] staging: media: starfive: Add raw pad for ISP Changhuang Liang
@ 2024-03-06 14:23 ` Dan Carpenter
2024-03-07 2:06 ` 回复: " Changhuang Liang
0 siblings, 1 reply; 19+ messages in thread
From: Dan Carpenter @ 2024-03-06 14:23 UTC (permalink / raw)
To: Changhuang Liang
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, Hans Verkuil,
Laurent Pinchart, Jack Zhu, linux-media, linux-kernel,
linux-staging
On Wed, Mar 06, 2024 at 01:33:29AM -0800, Changhuang Liang wrote:
> Add raw pad for ISP, it supported the conversion of RAW10 into RAW12.
>
To be honest, I don't understand what "it supported the conversion of
RAW10 into RAW12" means. I don't think that this patch has any impact
on user space but I'm not 100% sure.
A lot of this patch is just reformating stuff and it would be easier to
review if the reformating were separated into a separate patch.
patch 2: Clean pad selection in isp_try_format()
The code to select isp_dev->formats[] is overly complicated. We can
just use the "pad" as the index. This will making adding new pads
easier in future patches. No functional change.
patch 3: Add raw pad for ISP
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
> .../staging/media/starfive/camss/stf-isp.c | 26 ++++++++++++-------
> .../staging/media/starfive/camss/stf-isp.h | 1 +
> 2 files changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/media/starfive/camss/stf-isp.c b/drivers/staging/media/starfive/camss/stf-isp.c
> index a5573abe0d7b..6bab0ac23120 100644
> --- a/drivers/staging/media/starfive/camss/stf-isp.c
> +++ b/drivers/staging/media/starfive/camss/stf-isp.c
> @@ -10,9 +10,6 @@
>
> #include "stf-camss.h"
>
> -#define SINK_FORMATS_INDEX 0
> -#define SOURCE_FORMATS_INDEX 1
> -
This is cleanup. patch 2.
> static int isp_set_selection(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *state,
> struct v4l2_subdev_selection *sel);
> @@ -28,9 +25,17 @@ static const struct stf_isp_format isp_formats_source[] = {
> { MEDIA_BUS_FMT_YUYV8_1_5X8, 8 },
> };
>
> +static const struct stf_isp_format isp_formats_source_raw[] = {
> + { MEDIA_BUS_FMT_SRGGB12_1X12, 12 },
> + { MEDIA_BUS_FMT_SGRBG12_1X12, 12 },
> + { MEDIA_BUS_FMT_SGBRG12_1X12, 12 },
> + { MEDIA_BUS_FMT_SBGGR12_1X12, 12 },
> +};
> +
patch 3.
> static const struct stf_isp_format_table isp_formats_st7110[] = {
> { isp_formats_sink, ARRAY_SIZE(isp_formats_sink) },
> { isp_formats_source, ARRAY_SIZE(isp_formats_source) },
> + { isp_formats_source_raw, ARRAY_SIZE(isp_formats_source_raw) },
> };
patch 3.
>
> static const struct stf_isp_format *
> @@ -113,10 +118,7 @@ static void isp_try_format(struct stf_isp_dev *isp_dev,
> return;
> }
>
> - if (pad == STF_ISP_PAD_SINK)
> - formats = &isp_dev->formats[SINK_FORMATS_INDEX];
> - else if (pad == STF_ISP_PAD_SRC)
> - formats = &isp_dev->formats[SOURCE_FORMATS_INDEX];
> + formats = &isp_dev->formats[pad];
patch 2.
>
> fmt->width = clamp_t(u32, fmt->width, STFCAMSS_FRAME_MIN_WIDTH,
> STFCAMSS_FRAME_MAX_WIDTH);
> @@ -142,7 +144,7 @@ static int isp_enum_mbus_code(struct v4l2_subdev *sd,
> if (code->index >= ARRAY_SIZE(isp_formats_sink))
> return -EINVAL;
>
> - formats = &isp_dev->formats[SINK_FORMATS_INDEX];
> + formats = &isp_dev->formats[code->pad];
patch 2.
> code->code = formats->fmts[code->index].code;
> } else {
> struct v4l2_mbus_framefmt *sink_fmt;
> @@ -281,8 +283,11 @@ static int isp_set_selection(struct v4l2_subdev *sd,
> crop.target = V4L2_SEL_TGT_CROP;
> crop.r = *rect;
> isp_set_selection(sd, state, &crop);
> +
> + crop.pad = STF_ISP_PAD_SRC_RAW;
> + isp_set_selection(sd, state, &crop);
patch 3.
> } else if (sel->target == V4L2_SEL_TGT_CROP &&
> - sel->pad == STF_ISP_PAD_SRC) {
> + (sel->pad == STF_ISP_PAD_SRC || sel->pad == STF_ISP_PAD_SRC_RAW)) {
patch 3.
> struct v4l2_subdev_format fmt = { 0 };
>
> rect = v4l2_subdev_state_get_crop(state, sel->pad);
> @@ -294,7 +299,7 @@ static int isp_set_selection(struct v4l2_subdev *sd,
>
> /* Reset source pad format width and height */
> fmt.which = sel->which;
> - fmt.pad = STF_ISP_PAD_SRC;
> + fmt.pad = sel->pad;
patch 2.
> fmt.format.width = rect->width;
> fmt.format.height = rect->height;
> isp_set_format(sd, state, &fmt);
> @@ -361,6 +366,7 @@ int stf_isp_register(struct stf_isp_dev *isp_dev, struct v4l2_device *v4l2_dev)
>
> pads[STF_ISP_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
> pads[STF_ISP_PAD_SRC].flags = MEDIA_PAD_FL_SOURCE;
> + pads[STF_ISP_PAD_SRC_RAW].flags = MEDIA_PAD_FL_SOURCE;
patch 3.
>
> sd->entity.function = MEDIA_ENT_F_PROC_VIDEO_ISP;
> sd->entity.ops = &isp_media_ops;
> diff --git a/drivers/staging/media/starfive/camss/stf-isp.h b/drivers/staging/media/starfive/camss/stf-isp.h
> index 07d6c2758253..4fc5cfac115c 100644
> --- a/drivers/staging/media/starfive/camss/stf-isp.h
> +++ b/drivers/staging/media/starfive/camss/stf-isp.h
> @@ -393,6 +393,7 @@
> enum stf_isp_pad_id {
> STF_ISP_PAD_SINK = 0,
> STF_ISP_PAD_SRC,
> + STF_ISP_PAD_SRC_RAW,
patch 3.
> STF_ISP_PAD_MAX
> };
regards,
dan carpenter
^ permalink raw reply [flat|nested] 19+ messages in thread* 回复: [PATCH v1 2/7] staging: media: starfive: Add raw pad for ISP
2024-03-06 14:23 ` Dan Carpenter
@ 2024-03-07 2:06 ` Changhuang Liang
0 siblings, 0 replies; 19+ messages in thread
From: Changhuang Liang @ 2024-03-07 2:06 UTC (permalink / raw)
To: Dan Carpenter
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, Hans Verkuil,
Laurent Pinchart, Jack Zhu, linux-media@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-staging@lists.linux.dev
Hi Dan
> Re: [PATCH v1 2/7] staging: media: starfive: Add raw pad for ISP
>
> On Wed, Mar 06, 2024 at 01:33:29AM -0800, Changhuang Liang wrote:
> > Add raw pad for ISP, it supported the conversion of RAW10 into RAW12.
> >
>
> To be honest, I don't understand what "it supported the conversion of
> RAW10 into RAW12" means. I don't think that this patch has any impact on
> user space but I'm not 100% sure.
>
For StarFive ISP, the input format is RAW 10, which is converted to RAW 12 after ISP RAW pad output.
> A lot of this patch is just reformating stuff and it would be easier to review if
> the reformating were separated into a separate patch.
>
> patch 2: Clean pad selection in isp_try_format()
>
> The code to select isp_dev->formats[] is overly complicated. We can just use
> the "pad" as the index. This will making adding new pads easier in future
> patches. No functional change.
>
> patch 3: Add raw pad for ISP
>
Yes, this patch can also be split into two patches
regards,
Changhuang
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v1 3/7] staging: media: starfive: Sink rectangle set to ISP source pad
2024-03-06 9:33 [PATCH v1 0/7] Add ISP RAW for StarFive Changhuang Liang
2024-03-06 9:33 ` [PATCH v1 1/7] staging: media: starfive: Replaced current_fmt with get from sd_state Changhuang Liang
2024-03-06 9:33 ` [PATCH v1 2/7] staging: media: starfive: Add raw pad for ISP Changhuang Liang
@ 2024-03-06 9:33 ` Changhuang Liang
2024-03-06 14:23 ` Dan Carpenter
2024-03-06 9:33 ` [PATCH v1 4/7] staging: media: starfive: Add multi streams for ISP Changhuang Liang
` (3 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Changhuang Liang @ 2024-03-06 9:33 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Greg Kroah-Hartman
Cc: Hans Verkuil, Laurent Pinchart, Jack Zhu, Changhuang Liang,
linux-media, linux-kernel, linux-staging
Sink rectangle will be valid for all source pads.
Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
drivers/staging/media/starfive/camss/stf-isp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/starfive/camss/stf-isp.c b/drivers/staging/media/starfive/camss/stf-isp.c
index 6bab0ac23120..9b5745669fa6 100644
--- a/drivers/staging/media/starfive/camss/stf-isp.c
+++ b/drivers/staging/media/starfive/camss/stf-isp.c
@@ -88,7 +88,7 @@ static int isp_set_stream(struct v4l2_subdev *sd, int enable)
fmt_t_src = &isp_dev->formats[STF_ISP_PAD_SRC];
fmt = v4l2_subdev_state_get_format(sd_state, STF_ISP_PAD_SINK);
fmt_src = v4l2_subdev_state_get_format(sd_state, STF_ISP_PAD_SRC);
- crop = v4l2_subdev_state_get_crop(sd_state, STF_ISP_PAD_SRC);
+ crop = v4l2_subdev_state_get_crop(sd_state, STF_ISP_PAD_SINK);
src = stf_isp_g_index_by_mcode(fmt_t_src, fmt_src->code);
if (enable) {
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v1 3/7] staging: media: starfive: Sink rectangle set to ISP source pad
2024-03-06 9:33 ` [PATCH v1 3/7] staging: media: starfive: Sink rectangle set to ISP source pad Changhuang Liang
@ 2024-03-06 14:23 ` Dan Carpenter
0 siblings, 0 replies; 19+ messages in thread
From: Dan Carpenter @ 2024-03-06 14:23 UTC (permalink / raw)
To: Changhuang Liang
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, Hans Verkuil,
Laurent Pinchart, Jack Zhu, linux-media, linux-kernel,
linux-staging
On Wed, Mar 06, 2024 at 01:33:30AM -0800, Changhuang Liang wrote:
> Sink rectangle will be valid for all source pads.
>
This commit message is just really unclear. What does "Sink rectangle
set to ISP source pad" even mean? A better subject would probably be
"staging: media: starfive: Use PAD_SINK instead of PAD_SRC for crop"
But why are we making this change? It's not clear.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v1 4/7] staging: media: starfive: Add multi streams for ISP
2024-03-06 9:33 [PATCH v1 0/7] Add ISP RAW for StarFive Changhuang Liang
` (2 preceding siblings ...)
2024-03-06 9:33 ` [PATCH v1 3/7] staging: media: starfive: Sink rectangle set to ISP source pad Changhuang Liang
@ 2024-03-06 9:33 ` Changhuang Liang
2024-03-06 14:24 ` Dan Carpenter
2024-03-06 9:33 ` [PATCH v1 5/7] staging: media: starfive: Add ISP raw video device Changhuang Liang
` (2 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Changhuang Liang @ 2024-03-06 9:33 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Greg Kroah-Hartman
Cc: Hans Verkuil, Laurent Pinchart, Jack Zhu, Changhuang Liang,
linux-media, linux-kernel, linux-staging
ISP support multi streams output. Add stream_count to save the number
of stream on.
Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
.../staging/media/starfive/camss/stf-isp.c | 28 +++++++++++++------
.../staging/media/starfive/camss/stf-isp.h | 1 +
2 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/media/starfive/camss/stf-isp.c b/drivers/staging/media/starfive/camss/stf-isp.c
index 9b5745669fa6..1bfd336b14a1 100644
--- a/drivers/staging/media/starfive/camss/stf-isp.c
+++ b/drivers/staging/media/starfive/camss/stf-isp.c
@@ -92,15 +92,27 @@ static int isp_set_stream(struct v4l2_subdev *sd, int enable)
src = stf_isp_g_index_by_mcode(fmt_t_src, fmt_src->code);
if (enable) {
- stf_isp_reset(isp_dev);
- stf_isp_init_cfg(isp_dev);
- stf_isp_settings(isp_dev, crop, fmt->code);
- stf_isp_config_yuv_out_stride(isp_dev, crop->width,
- fmt_t_src->fmts[src].bpp);
- stf_isp_stream_set(isp_dev);
- }
+ if (!isp_dev->stream_count) {
+ stf_isp_reset(isp_dev);
+ stf_isp_init_cfg(isp_dev);
+ stf_isp_settings(isp_dev, crop, fmt->code);
+ stf_isp_config_yuv_out_stride(isp_dev, crop->width,
+ fmt_t_src->fmts[src].bpp);
+ stf_isp_stream_set(isp_dev);
+
+ v4l2_subdev_call(isp_dev->source_subdev, video,
+ s_stream, enable);
+ }
+
+ isp_dev->stream_count++;
+ } else {
+ isp_dev->stream_count--;
- v4l2_subdev_call(isp_dev->source_subdev, video, s_stream, enable);
+ if (!isp_dev->stream_count) {
+ v4l2_subdev_call(isp_dev->source_subdev, video,
+ s_stream, enable);
+ }
+ }
v4l2_subdev_unlock_state(sd_state);
return 0;
diff --git a/drivers/staging/media/starfive/camss/stf-isp.h b/drivers/staging/media/starfive/camss/stf-isp.h
index 4fc5cfac115c..30e4eb29f077 100644
--- a/drivers/staging/media/starfive/camss/stf-isp.h
+++ b/drivers/staging/media/starfive/camss/stf-isp.h
@@ -414,6 +414,7 @@ struct stf_isp_dev {
const struct stf_isp_format_table *formats;
unsigned int nformats;
struct v4l2_subdev *source_subdev;
+ unsigned int stream_count;
};
int stf_isp_reset(struct stf_isp_dev *isp_dev);
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v1 4/7] staging: media: starfive: Add multi streams for ISP
2024-03-06 9:33 ` [PATCH v1 4/7] staging: media: starfive: Add multi streams for ISP Changhuang Liang
@ 2024-03-06 14:24 ` Dan Carpenter
0 siblings, 0 replies; 19+ messages in thread
From: Dan Carpenter @ 2024-03-06 14:24 UTC (permalink / raw)
To: Changhuang Liang
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, Hans Verkuil,
Laurent Pinchart, Jack Zhu, linux-media, linux-kernel,
linux-staging
On Wed, Mar 06, 2024 at 01:33:31AM -0800, Changhuang Liang wrote:
> ISP support multi streams output. Add stream_count to save the number
> of stream on.
>
The ISP can support multiple output streams. Introduce stream_count to
store the number of streams. No functional change.
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
> .../staging/media/starfive/camss/stf-isp.c | 28 +++++++++++++------
> .../staging/media/starfive/camss/stf-isp.h | 1 +
> 2 files changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/media/starfive/camss/stf-isp.c b/drivers/staging/media/starfive/camss/stf-isp.c
> index 9b5745669fa6..1bfd336b14a1 100644
> --- a/drivers/staging/media/starfive/camss/stf-isp.c
> +++ b/drivers/staging/media/starfive/camss/stf-isp.c
> @@ -92,15 +92,27 @@ static int isp_set_stream(struct v4l2_subdev *sd, int enable)
> src = stf_isp_g_index_by_mcode(fmt_t_src, fmt_src->code);
>
> if (enable) {
> - stf_isp_reset(isp_dev);
> - stf_isp_init_cfg(isp_dev);
> - stf_isp_settings(isp_dev, crop, fmt->code);
> - stf_isp_config_yuv_out_stride(isp_dev, crop->width,
> - fmt_t_src->fmts[src].bpp);
> - stf_isp_stream_set(isp_dev);
> - }
> + if (!isp_dev->stream_count) {
> + stf_isp_reset(isp_dev);
> + stf_isp_init_cfg(isp_dev);
> + stf_isp_settings(isp_dev, crop, fmt->code);
> + stf_isp_config_yuv_out_stride(isp_dev, crop->width,
> + fmt_t_src->fmts[src].bpp);
> + stf_isp_stream_set(isp_dev);
> +
> + v4l2_subdev_call(isp_dev->source_subdev, video,
> + s_stream, enable);
s/enable/true/
> + }
> +
> + isp_dev->stream_count++;
> + } else {
> + isp_dev->stream_count--;
>
> - v4l2_subdev_call(isp_dev->source_subdev, video, s_stream, enable);
> + if (!isp_dev->stream_count) {
> + v4l2_subdev_call(isp_dev->source_subdev, video,
> + s_stream, enable);
s/enable/false/
> + }
> + }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v1 5/7] staging: media: starfive: Add ISP raw video device
2024-03-06 9:33 [PATCH v1 0/7] Add ISP RAW for StarFive Changhuang Liang
` (3 preceding siblings ...)
2024-03-06 9:33 ` [PATCH v1 4/7] staging: media: starfive: Add multi streams for ISP Changhuang Liang
@ 2024-03-06 9:33 ` Changhuang Liang
2024-03-06 14:26 ` Dan Carpenter
2024-03-06 9:33 ` [PATCH v1 6/7] staging: media: starfive: Add raw output stride configure Changhuang Liang
2024-03-06 9:33 ` [PATCH v1 7/7] admin-guide: media: Update documents for StarFive Camera Subsystem Changhuang Liang
6 siblings, 1 reply; 19+ messages in thread
From: Changhuang Liang @ 2024-03-06 9:33 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Greg Kroah-Hartman
Cc: Hans Verkuil, Laurent Pinchart, Jack Zhu, Changhuang Liang,
linux-media, linux-kernel, linux-staging
Add raw video device to capture raw data from ISP.
Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
.../staging/media/starfive/camss/stf-camss.c | 19 ++++++
.../media/starfive/camss/stf-capture.c | 58 ++++++++++++++++++-
.../staging/media/starfive/camss/stf-video.h | 1 +
3 files changed, 77 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/media/starfive/camss/stf-camss.c b/drivers/staging/media/starfive/camss/stf-camss.c
index 81fc39f20615..90ac8b67c76e 100644
--- a/drivers/staging/media/starfive/camss/stf-camss.c
+++ b/drivers/staging/media/starfive/camss/stf-camss.c
@@ -126,6 +126,7 @@ static int stfcamss_of_parse_ports(struct stfcamss *stfcamss)
static int stfcamss_register_devs(struct stfcamss *stfcamss)
{
struct stf_capture *cap_yuv = &stfcamss->captures[STF_CAPTURE_YUV];
+ struct stf_capture *cap_raw = &stfcamss->captures[STF_CAPTURE_RAW];
struct stf_isp_dev *isp_dev = &stfcamss->isp_dev;
int ret;
@@ -150,8 +151,18 @@ static int stfcamss_register_devs(struct stfcamss *stfcamss)
cap_yuv->video.source_subdev = &isp_dev->subdev;
+ ret = media_create_pad_link(&isp_dev->subdev.entity, STF_ISP_PAD_SRC_RAW,
+ &cap_raw->video.vdev.entity, 0, 0);
+ if (ret)
+ goto err_rm_links0;
+
+ cap_raw->video.source_subdev = &isp_dev->subdev;
+
return ret;
+err_rm_links0:
+ media_entity_remove_links(&isp_dev->subdev.entity);
+ media_entity_remove_links(&cap_yuv->video.vdev.entity);
err_cap_unregister:
stf_capture_unregister(stfcamss);
err_isp_unregister:
@@ -162,6 +173,14 @@ static int stfcamss_register_devs(struct stfcamss *stfcamss)
static void stfcamss_unregister_devs(struct stfcamss *stfcamss)
{
+ struct stf_capture *cap_yuv = &stfcamss->captures[STF_CAPTURE_YUV];
+ struct stf_capture *cap_raw = &stfcamss->captures[STF_CAPTURE_RAW];
+ struct stf_isp_dev *isp_dev = &stfcamss->isp_dev;
+
+ media_entity_remove_links(&isp_dev->subdev.entity);
+ media_entity_remove_links(&cap_raw->video.vdev.entity);
+ media_entity_remove_links(&cap_yuv->video.vdev.entity);
+
stf_isp_unregister(&stfcamss->isp_dev);
stf_capture_unregister(stfcamss);
}
diff --git a/drivers/staging/media/starfive/camss/stf-capture.c b/drivers/staging/media/starfive/camss/stf-capture.c
index 5c91126d5132..a5f10ec57782 100644
--- a/drivers/staging/media/starfive/camss/stf-capture.c
+++ b/drivers/staging/media/starfive/camss/stf-capture.c
@@ -12,6 +12,7 @@
static const char * const stf_cap_names[] = {
"capture_dump",
"capture_yuv",
+ "capture_raw",
};
static const struct stfcamss_format_info stf_wr_fmts[] = {
@@ -55,6 +56,37 @@ static const struct stfcamss_format_info stf_isp_fmts[] = {
},
};
+static const struct stfcamss_format_info stf_isp_raw_fmts[] = {
+ {
+ .code = MEDIA_BUS_FMT_SRGGB12_1X12,
+ .pixelformat = V4L2_PIX_FMT_SRGGB12,
+ .planes = 1,
+ .vsub = { 1 },
+ .bpp = 12,
+ },
+ {
+ .code = MEDIA_BUS_FMT_SGRBG12_1X12,
+ .pixelformat = V4L2_PIX_FMT_SGRBG12,
+ .planes = 1,
+ .vsub = { 1 },
+ .bpp = 12,
+ },
+ {
+ .code = MEDIA_BUS_FMT_SGBRG12_1X12,
+ .pixelformat = V4L2_PIX_FMT_SGBRG12,
+ .planes = 1,
+ .vsub = { 1 },
+ .bpp = 12,
+ },
+ {
+ .code = MEDIA_BUS_FMT_SBGGR12_1X12,
+ .pixelformat = V4L2_PIX_FMT_SBGGR12,
+ .planes = 1,
+ .vsub = { 1 },
+ .bpp = 12,
+ },
+};
+
static inline struct stf_capture *to_stf_capture(struct stfcamss_video *video)
{
return container_of(video, struct stf_capture, video);
@@ -73,6 +105,11 @@ static void stf_set_yuv_addr(struct stfcamss *stfcamss,
stf_isp_reg_write(stfcamss, ISP_REG_UV_PLANE_START_ADDR, uv_addr);
}
+static void stf_set_raw_addr(struct stfcamss *stfcamss, dma_addr_t raw_addr)
+{
+ stf_isp_reg_write(stfcamss, ISP_REG_DUMP_CFG_0, raw_addr);
+}
+
static void stf_init_addrs(struct stfcamss_video *video)
{
struct stf_capture *cap = to_stf_capture(video);
@@ -91,6 +128,8 @@ static void stf_init_addrs(struct stfcamss_video *video)
stf_set_dump_addr(video->stfcamss, addr0);
else if (cap->type == STF_CAPTURE_YUV)
stf_set_yuv_addr(video->stfcamss, addr0, addr1);
+ else
+ stf_set_raw_addr(video->stfcamss, addr0);
}
static struct stfcamss_buffer *stf_buf_get_pending(struct stf_v_buf *output)
@@ -250,7 +289,6 @@ static void stf_capture_init(struct stfcamss *stfcamss, struct stf_capture *cap)
cap->video.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
cap->video.stfcamss = stfcamss;
- cap->video.bpl_alignment = 16 * 8;
if (cap->type == STF_CAPTURE_DUMP) {
cap->video.formats = stf_wr_fmts;
@@ -260,6 +298,10 @@ static void stf_capture_init(struct stfcamss *stfcamss, struct stf_capture *cap)
cap->video.formats = stf_isp_fmts;
cap->video.nformats = ARRAY_SIZE(stf_isp_fmts);
cap->video.bpl_alignment = 1;
+ } else {
+ cap->video.formats = stf_isp_raw_fmts;
+ cap->video.nformats = ARRAY_SIZE(stf_isp_raw_fmts);
+ cap->video.bpl_alignment = STFCAMSS_FRAME_WIDTH_ALIGN_128;
}
}
@@ -441,6 +483,8 @@ static void stf_change_buffer(struct stf_v_buf *output)
stf_set_dump_addr(stfcamss, new_addr[0]);
else if (cap->type == STF_CAPTURE_YUV)
stf_set_yuv_addr(stfcamss, new_addr[0], new_addr[1]);
+ else
+ stf_set_raw_addr(stfcamss, new_addr[0]);
stf_buf_add_ready(output, ready_buf);
}
@@ -468,6 +512,7 @@ irqreturn_t stf_wr_irq_handler(int irq, void *priv)
irqreturn_t stf_isp_irq_handler(int irq, void *priv)
{
struct stfcamss *stfcamss = priv;
+ struct stf_capture *cap_raw = &stfcamss->captures[STF_CAPTURE_RAW];
struct stf_capture *cap = &stfcamss->captures[STF_CAPTURE_YUV];
u32 status;
@@ -476,6 +521,9 @@ irqreturn_t stf_isp_irq_handler(int irq, void *priv)
if (status & ISPC_ENUO)
stf_buf_done(&cap->buffers);
+ if (status & ISPC_CSI)
+ stf_buf_done(&cap_raw->buffers);
+
stf_isp_reg_write(stfcamss, ISP_REG_ISP_CTRL_0,
(status & ~ISPC_INT_ALL_MASK) |
ISPC_ISP | ISPC_CSI | ISPC_SC);
@@ -487,14 +535,20 @@ irqreturn_t stf_isp_irq_handler(int irq, void *priv)
irqreturn_t stf_line_irq_handler(int irq, void *priv)
{
struct stfcamss *stfcamss = priv;
+ struct stf_capture *cap_raw = &stfcamss->captures[STF_CAPTURE_RAW];
struct stf_capture *cap = &stfcamss->captures[STF_CAPTURE_YUV];
u32 status;
+ u32 value;
status = stf_isp_reg_read(stfcamss, ISP_REG_ISP_CTRL_0);
if (status & ISPC_LINE) {
if (atomic_dec_if_positive(&cap->buffers.frame_skip) < 0) {
if ((status & ISPC_ENUO))
stf_change_buffer(&cap->buffers);
+
+ value = stf_isp_reg_read(stfcamss, ISP_REG_CSI_MODULE_CFG);
+ if (value & CSI_DUMP_EN)
+ stf_change_buffer(&cap_raw->buffers);
}
stf_isp_reg_set_bit(stfcamss, ISP_REG_CSIINTS,
@@ -571,9 +625,11 @@ void stf_capture_unregister(struct stfcamss *stfcamss)
{
struct stf_capture *cap_dump = &stfcamss->captures[STF_CAPTURE_DUMP];
struct stf_capture *cap_yuv = &stfcamss->captures[STF_CAPTURE_YUV];
+ struct stf_capture *cap_raw = &stfcamss->captures[STF_CAPTURE_RAW];
stf_capture_unregister_one(cap_dump);
stf_capture_unregister_one(cap_yuv);
+ stf_capture_unregister_one(cap_raw);
}
int stf_capture_register(struct stfcamss *stfcamss,
diff --git a/drivers/staging/media/starfive/camss/stf-video.h b/drivers/staging/media/starfive/camss/stf-video.h
index 90c73c0dee89..cad861aae31c 100644
--- a/drivers/staging/media/starfive/camss/stf-video.h
+++ b/drivers/staging/media/starfive/camss/stf-video.h
@@ -37,6 +37,7 @@ enum stf_v_line_id {
enum stf_capture_type {
STF_CAPTURE_DUMP = 0,
STF_CAPTURE_YUV,
+ STF_CAPTURE_RAW,
STF_CAPTURE_NUM,
};
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v1 5/7] staging: media: starfive: Add ISP raw video device
2024-03-06 9:33 ` [PATCH v1 5/7] staging: media: starfive: Add ISP raw video device Changhuang Liang
@ 2024-03-06 14:26 ` Dan Carpenter
2024-03-07 2:13 ` 回复: " Changhuang Liang
0 siblings, 1 reply; 19+ messages in thread
From: Dan Carpenter @ 2024-03-06 14:26 UTC (permalink / raw)
To: Changhuang Liang
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, Hans Verkuil,
Laurent Pinchart, Jack Zhu, linux-media, linux-kernel,
linux-staging
I wasn't able to get this patch to apply. I tried applying the patch
mentioned in the cover letter first but it didn't help... It's not
your fault, but it made reviewing the rest hard so I might have made
some mistakes.
On Wed, Mar 06, 2024 at 01:33:32AM -0800, Changhuang Liang wrote:
> Add raw video device to capture raw data from ISP.
>
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
> .../staging/media/starfive/camss/stf-camss.c | 19 ++++++
> .../media/starfive/camss/stf-capture.c | 58 ++++++++++++++++++-
> .../staging/media/starfive/camss/stf-video.h | 1 +
> 3 files changed, 77 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/starfive/camss/stf-camss.c b/drivers/staging/media/starfive/camss/stf-camss.c
> index 81fc39f20615..90ac8b67c76e 100644
> --- a/drivers/staging/media/starfive/camss/stf-camss.c
> +++ b/drivers/staging/media/starfive/camss/stf-camss.c
> @@ -126,6 +126,7 @@ static int stfcamss_of_parse_ports(struct stfcamss *stfcamss)
> static int stfcamss_register_devs(struct stfcamss *stfcamss)
> {
> struct stf_capture *cap_yuv = &stfcamss->captures[STF_CAPTURE_YUV];
> + struct stf_capture *cap_raw = &stfcamss->captures[STF_CAPTURE_RAW];
> struct stf_isp_dev *isp_dev = &stfcamss->isp_dev;
> int ret;
>
> @@ -150,8 +151,18 @@ static int stfcamss_register_devs(struct stfcamss *stfcamss)
>
> cap_yuv->video.source_subdev = &isp_dev->subdev;
>
> + ret = media_create_pad_link(&isp_dev->subdev.entity, STF_ISP_PAD_SRC_RAW,
> + &cap_raw->video.vdev.entity, 0, 0);
> + if (ret)
> + goto err_rm_links0;
> +
> + cap_raw->video.source_subdev = &isp_dev->subdev;
> +
> return ret;
>
> +err_rm_links0:
> + media_entity_remove_links(&isp_dev->subdev.entity);
I don't think this line is correct. I think we only need to
remove &cap_yuv->video.vdev.entity.
> + media_entity_remove_links(&cap_yuv->video.vdev.entity);
> err_cap_unregister:
> stf_capture_unregister(stfcamss);
> err_isp_unregister:
> @@ -162,6 +173,14 @@ static int stfcamss_register_devs(struct stfcamss *stfcamss)
>
> static void stfcamss_unregister_devs(struct stfcamss *stfcamss)
> {
> + struct stf_capture *cap_yuv = &stfcamss->captures[STF_CAPTURE_YUV];
> + struct stf_capture *cap_raw = &stfcamss->captures[STF_CAPTURE_RAW];
> + struct stf_isp_dev *isp_dev = &stfcamss->isp_dev;
> +
> + media_entity_remove_links(&isp_dev->subdev.entity);
I think this line should be deleted.
> + media_entity_remove_links(&cap_raw->video.vdev.entity);
> + media_entity_remove_links(&cap_yuv->video.vdev.entity);
I think this "&cap_yuv" should be submitted by itself as a bugfix patch.
> +
> stf_isp_unregister(&stfcamss->isp_dev);
> stf_capture_unregister(stfcamss);
> }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 19+ messages in thread* 回复: [PATCH v1 5/7] staging: media: starfive: Add ISP raw video device
2024-03-06 14:26 ` Dan Carpenter
@ 2024-03-07 2:13 ` Changhuang Liang
2024-03-07 7:55 ` Dan Carpenter
0 siblings, 1 reply; 19+ messages in thread
From: Changhuang Liang @ 2024-03-07 2:13 UTC (permalink / raw)
To: Dan Carpenter
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, Hans Verkuil,
Laurent Pinchart, Jack Zhu, linux-media@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-staging@lists.linux.dev
Hi, Dan
[...]
> >
> > +err_rm_links0:
> > + media_entity_remove_links(&isp_dev->subdev.entity);
>
> I don't think this line is correct. I think we only need to remove
> &cap_yuv->video.vdev.entity.
>
The instance I refer to needs to clear both the source entity and the sink entity. See
https://elixir.bootlin.com/linux/v6.8-rc7/source/drivers/media/platform/verisilicon/hantro_drv.c#L855
> > + media_entity_remove_links(&cap_yuv->video.vdev.entity);
> > err_cap_unregister:
> > stf_capture_unregister(stfcamss);
> > err_isp_unregister:
> > @@ -162,6 +173,14 @@ static int stfcamss_register_devs(struct stfcamss
> > *stfcamss)
> >
> > static void stfcamss_unregister_devs(struct stfcamss *stfcamss) {
> > + struct stf_capture *cap_yuv =
> &stfcamss->captures[STF_CAPTURE_YUV];
> > + struct stf_capture *cap_raw =
> &stfcamss->captures[STF_CAPTURE_RAW];
> > + struct stf_isp_dev *isp_dev = &stfcamss->isp_dev;
> > +
> > + media_entity_remove_links(&isp_dev->subdev.entity);
>
> I think this line should be deleted.
>
> > + media_entity_remove_links(&cap_raw->video.vdev.entity);
> > + media_entity_remove_links(&cap_yuv->video.vdev.entity);
>
> I think this "&cap_yuv" should be submitted by itself as a bugfix patch.
>
Yes.
Regards
Changhuang
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: 回复: [PATCH v1 5/7] staging: media: starfive: Add ISP raw video device
2024-03-07 2:13 ` 回复: " Changhuang Liang
@ 2024-03-07 7:55 ` Dan Carpenter
0 siblings, 0 replies; 19+ messages in thread
From: Dan Carpenter @ 2024-03-07 7:55 UTC (permalink / raw)
To: Changhuang Liang
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, Hans Verkuil,
Laurent Pinchart, Jack Zhu, linux-media@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-staging@lists.linux.dev
On Thu, Mar 07, 2024 at 02:13:50AM +0000, Changhuang Liang wrote:
> Hi, Dan
>
> [...]
> > >
> > > +err_rm_links0:
> > > + media_entity_remove_links(&isp_dev->subdev.entity);
> >
> > I don't think this line is correct. I think we only need to remove
> > &cap_yuv->video.vdev.entity.
> >
>
> The instance I refer to needs to clear both the source entity and the sink entity. See
> https://elixir.bootlin.com/linux/v6.8-rc7/source/drivers/media/platform/verisilicon/hantro_drv.c#L855
>
Oh yeah. It's the same in v4l2_m2m_register_media_controller().
regards,
dan carpenter
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v1 6/7] staging: media: starfive: Add raw output stride configure
2024-03-06 9:33 [PATCH v1 0/7] Add ISP RAW for StarFive Changhuang Liang
` (4 preceding siblings ...)
2024-03-06 9:33 ` [PATCH v1 5/7] staging: media: starfive: Add ISP raw video device Changhuang Liang
@ 2024-03-06 9:33 ` Changhuang Liang
2024-03-06 14:26 ` Dan Carpenter
2024-03-06 9:33 ` [PATCH v1 7/7] admin-guide: media: Update documents for StarFive Camera Subsystem Changhuang Liang
6 siblings, 1 reply; 19+ messages in thread
From: Changhuang Liang @ 2024-03-06 9:33 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Greg Kroah-Hartman
Cc: Hans Verkuil, Laurent Pinchart, Jack Zhu, Changhuang Liang,
linux-media, linux-kernel, linux-staging
Add raw output stride to enable sent the aligned raw data to memory.
Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
.../staging/media/starfive/camss/stf-isp-hw-ops.c | 13 +++++++++----
drivers/staging/media/starfive/camss/stf-isp.c | 11 ++++++++---
drivers/staging/media/starfive/camss/stf-isp.h | 4 ++--
3 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/media/starfive/camss/stf-isp-hw-ops.c b/drivers/staging/media/starfive/camss/stf-isp-hw-ops.c
index b933d425cbd0..e7f60616bd7c 100644
--- a/drivers/staging/media/starfive/camss/stf-isp-hw-ops.c
+++ b/drivers/staging/media/starfive/camss/stf-isp-hw-ops.c
@@ -366,6 +366,15 @@ void stf_isp_config_yuv_out_stride(struct stf_isp_dev *isp_dev,
stf_isp_reg_write(isp_dev->stfcamss, ISP_REG_STRIDE, val);
}
+void stf_isp_config_raw_out_stride(struct stf_isp_dev *isp_dev,
+ u32 width, u8 bpp)
+{
+ u32 val = ALIGN(width * bpp / 8, STFCAMSS_FRAME_WIDTH_ALIGN_128);
+
+ stf_isp_reg_write(isp_dev->stfcamss, ISP_REG_DUMP_CFG_1,
+ DUMP_BURST_LEN(3) | val);
+}
+
static void stf_isp_config_raw_fmt(struct stfcamss *stfcamss, u32 mcode)
{
u32 val, val1;
@@ -417,10 +426,6 @@ void stf_isp_settings(struct stf_isp_dev *isp_dev,
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));
diff --git a/drivers/staging/media/starfive/camss/stf-isp.c b/drivers/staging/media/starfive/camss/stf-isp.c
index 1bfd336b14a1..845446ba279c 100644
--- a/drivers/staging/media/starfive/camss/stf-isp.c
+++ b/drivers/staging/media/starfive/camss/stf-isp.c
@@ -78,18 +78,21 @@ int stf_isp_init(struct stfcamss *stfcamss)
static int isp_set_stream(struct v4l2_subdev *sd, int enable)
{
struct stf_isp_dev *isp_dev = v4l2_get_subdevdata(sd);
- const struct stf_isp_format_table *fmt_t_src;
- struct v4l2_mbus_framefmt *fmt, *fmt_src;
+ const struct stf_isp_format_table *fmt_t_src, *fmt_t_raw;
+ struct v4l2_mbus_framefmt *fmt, *fmt_src, *fmt_raw;
struct v4l2_subdev_state *sd_state;
struct v4l2_rect *crop;
- int src;
+ int src, raw;
sd_state = v4l2_subdev_lock_and_get_active_state(sd);
fmt_t_src = &isp_dev->formats[STF_ISP_PAD_SRC];
+ fmt_t_raw = &isp_dev->formats[STF_ISP_PAD_SRC_RAW];
fmt = v4l2_subdev_state_get_format(sd_state, STF_ISP_PAD_SINK);
fmt_src = v4l2_subdev_state_get_format(sd_state, STF_ISP_PAD_SRC);
+ fmt_raw = v4l2_subdev_state_get_format(sd_state, STF_ISP_PAD_SRC_RAW);
crop = v4l2_subdev_state_get_crop(sd_state, STF_ISP_PAD_SINK);
src = stf_isp_g_index_by_mcode(fmt_t_src, fmt_src->code);
+ raw = stf_isp_g_index_by_mcode(fmt_t_raw, fmt_raw->code);
if (enable) {
if (!isp_dev->stream_count) {
@@ -98,6 +101,8 @@ static int isp_set_stream(struct v4l2_subdev *sd, int enable)
stf_isp_settings(isp_dev, crop, fmt->code);
stf_isp_config_yuv_out_stride(isp_dev, crop->width,
fmt_t_src->fmts[src].bpp);
+ stf_isp_config_raw_out_stride(isp_dev, crop->width,
+ fmt_t_raw->fmts[raw].bpp);
stf_isp_stream_set(isp_dev);
v4l2_subdev_call(isp_dev->source_subdev, video,
diff --git a/drivers/staging/media/starfive/camss/stf-isp.h b/drivers/staging/media/starfive/camss/stf-isp.h
index 30e4eb29f077..dcbc42f0dfe4 100644
--- a/drivers/staging/media/starfive/camss/stf-isp.h
+++ b/drivers/staging/media/starfive/camss/stf-isp.h
@@ -76,8 +76,6 @@
#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)
@@ -424,6 +422,8 @@ void stf_isp_settings(struct stf_isp_dev *isp_dev,
void stf_isp_stream_set(struct stf_isp_dev *isp_dev);
void stf_isp_config_yuv_out_stride(struct stf_isp_dev *isp_dev,
u32 width, u8 bpp);
+void stf_isp_config_raw_out_stride(struct stf_isp_dev *isp_dev,
+ u32 width, u8 bpp);
int stf_isp_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);
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v1 6/7] staging: media: starfive: Add raw output stride configure
2024-03-06 9:33 ` [PATCH v1 6/7] staging: media: starfive: Add raw output stride configure Changhuang Liang
@ 2024-03-06 14:26 ` Dan Carpenter
2024-03-07 2:34 ` 回复: " Changhuang Liang
0 siblings, 1 reply; 19+ messages in thread
From: Dan Carpenter @ 2024-03-06 14:26 UTC (permalink / raw)
To: Changhuang Liang
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, Hans Verkuil,
Laurent Pinchart, Jack Zhu, linux-media, linux-kernel,
linux-staging
On Wed, Mar 06, 2024 at 01:33:33AM -0800, Changhuang Liang wrote:
> Add raw output stride to enable sent the aligned raw data to memory.
>
Subject: staging: media: starfive: Fix raw output stride configuration
I think this is bugfix? What is the impact of this change to the user?
But I can't really understand the commit message and it's hard to review
the code without being able to apply it. I'm also new to this subsystem
so maybe someone else would have been able to understand it better...
Sorry...
regards,
dan carpenter
^ permalink raw reply [flat|nested] 19+ messages in thread
* 回复: [PATCH v1 6/7] staging: media: starfive: Add raw output stride configure
2024-03-06 14:26 ` Dan Carpenter
@ 2024-03-07 2:34 ` Changhuang Liang
0 siblings, 0 replies; 19+ messages in thread
From: Changhuang Liang @ 2024-03-07 2:34 UTC (permalink / raw)
To: Dan Carpenter
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, Hans Verkuil,
Laurent Pinchart, Jack Zhu, linux-media@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-staging@lists.linux.dev
Hi, Dan
> On Wed, Mar 06, 2024 at 01:33:33AM -0800, Changhuang Liang wrote:
> > Add raw output stride to enable sent the aligned raw data to memory.
> >
>
> Subject: staging: media: starfive: Fix raw output stride configuration
>
> I think this is bugfix? What is the impact of this change to the user?
>
> But I can't really understand the commit message and it's hard to review the
> code without being able to apply it. I'm also new to this subsystem so
> maybe someone else would have been able to understand it better...
> Sorry...
>
You can try to apply it on tag v6.8-rc7.
1.
Applying: staging: media: starfive: Renamed capture_raw to capture_dump
2.
Applying: staging: media: starfive: Replaced current_fmt with get from sd_state
Applying: staging: media: starfive: Add raw pad for ISP
Applying: staging: media: starfive: Sink rectangle set to ISP source pad
Applying: staging: media: starfive: Add multi streams for ISP
Applying: staging: media: starfive: Add ISP raw video device
Applying: staging: media: starfive: Add raw output stride configure
Applying: admin-guide: media: Update documents for StarFive Camera Subsystem
Regards,
Changhuang
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v1 7/7] admin-guide: media: Update documents for StarFive Camera Subsystem
2024-03-06 9:33 [PATCH v1 0/7] Add ISP RAW for StarFive Changhuang Liang
` (5 preceding siblings ...)
2024-03-06 9:33 ` [PATCH v1 6/7] staging: media: starfive: Add raw output stride configure Changhuang Liang
@ 2024-03-06 9:33 ` Changhuang Liang
6 siblings, 0 replies; 19+ messages in thread
From: Changhuang Liang @ 2024-03-06 9:33 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Greg Kroah-Hartman
Cc: Hans Verkuil, Laurent Pinchart, Jack Zhu, Changhuang Liang,
linux-media, linux-kernel, linux-staging
Add ISP capture_raw video device in documents. It support output raw
frames by ISP module.
Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
.../admin-guide/media/starfive_camss.rst | 11 ++++++----
.../media/starfive_camss_graph.dot | 20 ++++++++++---------
2 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/Documentation/admin-guide/media/starfive_camss.rst b/Documentation/admin-guide/media/starfive_camss.rst
index c224e6123042..b8d1ce878847 100644
--- a/Documentation/admin-guide/media/starfive_camss.rst
+++ b/Documentation/admin-guide/media/starfive_camss.rst
@@ -44,7 +44,7 @@ The Starfive Camera Subsystem hardware consists of::
- Parallel: The parallel interface, receiving data from a parallel sensor.
- ISP: The ISP, processing raw Bayer data from an image sensor and producing
- YUV frames.
+ YUV frames, and also support output RAW frames.
Topology
@@ -58,15 +58,18 @@ The media controller pipeline graph is as follows:
:alt: starfive_camss_graph.dot
:align: center
-The driver has 2 video devices:
+The driver has 3 video devices:
- capture_dump: The capture device, capturing image data directly from a sensor.
- capture_yuv: The capture device, capturing YUV frame data processed by the
- ISP module
+ ISP module.
+- capture_raw: The capture device, capturing RAW frame data processed by the
+ ISP module.
The driver has 3 subdevices:
-- stf_isp: is responsible for all the isp operations, outputs YUV frames.
+- stf_isp: is responsible for all the isp operations, outputs YUV frames and
+ RAW frames.
- cdns_csi2rx: a CSI-2 bridge supporting up to 4 CSI lanes in input, and 4
different pixel streams in output.
- imx219: an image sensor, image data is sent through MIPI CSI-2.
diff --git a/Documentation/admin-guide/media/starfive_camss_graph.dot b/Documentation/admin-guide/media/starfive_camss_graph.dot
index 5e8731e27701..dab3e3ccbe9a 100644
--- a/Documentation/admin-guide/media/starfive_camss_graph.dot
+++ b/Documentation/admin-guide/media/starfive_camss_graph.dot
@@ -1,12 +1,14 @@
digraph board {
rankdir=TB
- n00000001 [label="{{<port0> 0} | stf_isp\n/dev/v4l-subdev0 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
- n00000001:port1 -> n00000008 [style=dashed]
- n00000004 [label="capture_dump\n/dev/video0", shape=box, style=filled, fillcolor=yellow]
- n00000008 [label="capture_yuv\n/dev/video1", shape=box, style=filled, fillcolor=yellow]
- n0000000e [label="{{<port0> 0} | cdns_csi2rx.19800000.csi-bridge\n | {<port1> 1 | <port2> 2 | <port3> 3 | <port4> 4}}", shape=Mrecord, style=filled, fillcolor=green]
- n0000000e:port1 -> n00000001:port0 [style=dashed]
- n0000000e:port1 -> n00000004 [style=dashed]
- n00000018 [label="{{} | imx219 6-0010\n/dev/v4l-subdev1 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
- n00000018:port0 -> n0000000e:port0 [style=bold]
+ n00000001 [label="{{<port0> 0} | stf_isp\n/dev/v4l-subdev0 | {<port1> 1 | <port2> 2}}", shape=Mrecord, style=filled, fillcolor=green]
+ n00000001:port1 -> n00000009 [style=dashed]
+ n00000001:port2 -> n0000000d [style=dashed]
+ n00000005 [label="capture_dump\n/dev/video0", shape=box, style=filled, fillcolor=yellow]
+ n00000009 [label="capture_yuv\n/dev/video1", shape=box, style=filled, fillcolor=yellow]
+ n0000000d [label="capture_raw\n/dev/video2", shape=box, style=filled, fillcolor=yellow]
+ n00000015 [label="{{<port0> 0} | cdns_csi2rx.19800000.csi\n/dev/v4l-subdev1 | {<port1> 1 | <port2> 2 | <port3> 3 | <port4> 4}}", shape=Mrecord, style=filled, fillcolor=green]
+ n00000015:port1 -> n00000001:port0 [style=dashed]
+ n00000015:port1 -> n00000005 [style=dashed]
+ n0000001f [label="{{} | imx219 6-0010\n/dev/v4l-subdev2 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
+ n0000001f:port0 -> n00000015:port0 [style=bold]
}
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread