* [PATCH v1 0/3] media: rkisp1: Convert to V4L2 subdev active state
@ 2023-01-27 0:31 ` Laurent Pinchart
0 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2023-01-27 0:31 UTC (permalink / raw)
To: linux-media; +Cc: Dafna Hirschfeld, Paul Elder, Heiko Stuebner, linux-rockchip
Hello,
This small patch series converts the three subdevs of the rkisp1 (CSI,
ISP and resizer) to use the V4L2 subdev active state. This simplifies
the implementation of the subdevs, as well as the locking scheme. There
isn't much else to add here, please see individual patches for details.
I have successfully tested this series on an i.MX8MP (Debix) and an
RK3399 (Rock Pi 4).
Laurent Pinchart (3):
media: rkisp1: resizer: Use V4L2 subdev active state
media: rkisp1: isp: Use V4L2 subdev active state
media: rkisp1: csi: Use V4L2 subdev active state
.../platform/rockchip/rkisp1/rkisp1-common.h | 18 --
.../platform/rockchip/rkisp1/rkisp1-csi.c | 107 +++----
.../platform/rockchip/rkisp1/rkisp1-isp.c | 261 +++++++-----------
.../platform/rockchip/rkisp1/rkisp1-resizer.c | 184 +++++-------
4 files changed, 201 insertions(+), 369 deletions(-)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v1 0/3] media: rkisp1: Convert to V4L2 subdev active state
@ 2023-01-27 0:31 ` Laurent Pinchart
0 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2023-01-27 0:31 UTC (permalink / raw)
To: linux-media; +Cc: Dafna Hirschfeld, Paul Elder, Heiko Stuebner, linux-rockchip
Hello,
This small patch series converts the three subdevs of the rkisp1 (CSI,
ISP and resizer) to use the V4L2 subdev active state. This simplifies
the implementation of the subdevs, as well as the locking scheme. There
isn't much else to add here, please see individual patches for details.
I have successfully tested this series on an i.MX8MP (Debix) and an
RK3399 (Rock Pi 4).
Laurent Pinchart (3):
media: rkisp1: resizer: Use V4L2 subdev active state
media: rkisp1: isp: Use V4L2 subdev active state
media: rkisp1: csi: Use V4L2 subdev active state
.../platform/rockchip/rkisp1/rkisp1-common.h | 18 --
.../platform/rockchip/rkisp1/rkisp1-csi.c | 107 +++----
.../platform/rockchip/rkisp1/rkisp1-isp.c | 261 +++++++-----------
.../platform/rockchip/rkisp1/rkisp1-resizer.c | 184 +++++-------
4 files changed, 201 insertions(+), 369 deletions(-)
--
Regards,
Laurent Pinchart
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v1 1/3] media: rkisp1: resizer: Use V4L2 subdev active state
2023-01-27 0:31 ` Laurent Pinchart
@ 2023-01-27 0:31 ` Laurent Pinchart
-1 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2023-01-27 0:31 UTC (permalink / raw)
To: linux-media; +Cc: Dafna Hirschfeld, Paul Elder, Heiko Stuebner, linux-rockchip
Use the V4L2 subdev active state API to store the active format and crop
rectangle. This simplifies the driver not only by dropping the state
stored in the rkisp1_resizer structure, but also by replacing the
ops_lock with the state lock.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
.../platform/rockchip/rkisp1/rkisp1-common.h | 6 -
.../platform/rockchip/rkisp1/rkisp1-resizer.c | 184 +++++++-----------
2 files changed, 66 insertions(+), 124 deletions(-)
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
index a1293c45aae1..e9626e59e1c8 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
@@ -390,10 +390,7 @@ struct rkisp1_params {
* @id: id of the resizer, one of RKISP1_SELFPATH, RKISP1_MAINPATH
* @rkisp1: pointer to the rkisp1 device
* @pads: media pads
- * @pad_cfg: configurations for the pads
* @config: the set of registers to configure the resizer
- * @pixel_enc: pixel encoding of the resizer
- * @ops_lock: a lock for the subdev ops
*/
struct rkisp1_resizer {
struct v4l2_subdev sd;
@@ -401,10 +398,7 @@ struct rkisp1_resizer {
enum rkisp1_stream_id id;
struct rkisp1_device *rkisp1;
struct media_pad pads[RKISP1_RSZ_PAD_MAX];
- struct v4l2_subdev_pad_config pad_cfg[RKISP1_RSZ_PAD_MAX];
const struct rkisp1_rsz_config *config;
- enum v4l2_pixel_encoding pixel_enc;
- struct mutex ops_lock; /* serialize the subdevice ops */
};
/*
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
index f76afd8112b2..62728ee6d058 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
@@ -117,34 +117,6 @@ static inline void rkisp1_rsz_write(struct rkisp1_resizer *rsz, u32 offset,
rkisp1_write(rsz->rkisp1, rsz->regs_base + offset, value);
}
-static struct v4l2_mbus_framefmt *
-rkisp1_rsz_get_pad_fmt(struct rkisp1_resizer *rsz,
- struct v4l2_subdev_state *sd_state,
- unsigned int pad, u32 which)
-{
- struct v4l2_subdev_state state = {
- .pads = rsz->pad_cfg
- };
- if (which == V4L2_SUBDEV_FORMAT_TRY)
- return v4l2_subdev_get_try_format(&rsz->sd, sd_state, pad);
- else
- return v4l2_subdev_get_try_format(&rsz->sd, &state, pad);
-}
-
-static struct v4l2_rect *
-rkisp1_rsz_get_pad_crop(struct rkisp1_resizer *rsz,
- struct v4l2_subdev_state *sd_state,
- unsigned int pad, u32 which)
-{
- struct v4l2_subdev_state state = {
- .pads = rsz->pad_cfg
- };
- if (which == V4L2_SUBDEV_FORMAT_TRY)
- return v4l2_subdev_get_try_crop(&rsz->sd, sd_state, pad);
- else
- return v4l2_subdev_get_try_crop(&rsz->sd, &state, pad);
-}
-
/* ----------------------------------------------------------------------------
* Dual crop hw configs
*/
@@ -165,17 +137,18 @@ static void rkisp1_dcrop_disable(struct rkisp1_resizer *rsz,
}
/* configure dual-crop unit */
-static void rkisp1_dcrop_config(struct rkisp1_resizer *rsz)
+static void rkisp1_dcrop_config(struct rkisp1_resizer *rsz,
+ struct v4l2_subdev_state *sd_state)
{
struct rkisp1_device *rkisp1 = rsz->rkisp1;
struct v4l2_mbus_framefmt *sink_fmt;
struct v4l2_rect *sink_crop;
u32 dc_ctrl;
- sink_crop = rkisp1_rsz_get_pad_crop(rsz, NULL, RKISP1_RSZ_PAD_SINK,
- V4L2_SUBDEV_FORMAT_ACTIVE);
- sink_fmt = rkisp1_rsz_get_pad_fmt(rsz, NULL, RKISP1_RSZ_PAD_SINK,
- V4L2_SUBDEV_FORMAT_ACTIVE);
+ sink_crop = v4l2_subdev_get_pad_crop(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SINK);
+ sink_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SINK);
if (sink_crop->width == sink_fmt->width &&
sink_crop->height == sink_fmt->height &&
@@ -296,6 +269,7 @@ static void rkisp1_rsz_config_regs(struct rkisp1_resizer *rsz,
}
static void rkisp1_rsz_config(struct rkisp1_resizer *rsz,
+ struct v4l2_subdev_state *sd_state,
enum rkisp1_shadow_regs_when when)
{
const struct rkisp1_rsz_yuv_mbus_info *sink_yuv_info, *src_yuv_info;
@@ -303,20 +277,21 @@ static void rkisp1_rsz_config(struct rkisp1_resizer *rsz,
struct v4l2_mbus_framefmt *src_fmt, *sink_fmt;
struct v4l2_rect *sink_crop;
- sink_crop = rkisp1_rsz_get_pad_crop(rsz, NULL, RKISP1_RSZ_PAD_SINK,
- V4L2_SUBDEV_FORMAT_ACTIVE);
- src_fmt = rkisp1_rsz_get_pad_fmt(rsz, NULL, RKISP1_RSZ_PAD_SRC,
- V4L2_SUBDEV_FORMAT_ACTIVE);
- src_yuv_info = rkisp1_rsz_get_yuv_mbus_info(src_fmt->code);
- sink_fmt = rkisp1_rsz_get_pad_fmt(rsz, NULL, RKISP1_RSZ_PAD_SINK,
- V4L2_SUBDEV_FORMAT_ACTIVE);
+ sink_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SINK);
+ sink_crop = v4l2_subdev_get_pad_crop(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SINK);
+ src_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SRC);
+
sink_yuv_info = rkisp1_rsz_get_yuv_mbus_info(sink_fmt->code);
+ src_yuv_info = rkisp1_rsz_get_yuv_mbus_info(src_fmt->code);
/*
* The resizer only works on yuv formats,
* so return if it is bayer format.
*/
- if (rsz->pixel_enc == V4L2_PIXEL_ENC_BAYER) {
+ if (!sink_yuv_info) {
rkisp1_rsz_disable(rsz, when);
return;
}
@@ -405,7 +380,7 @@ static int rkisp1_rsz_init_config(struct v4l2_subdev *sd,
struct v4l2_mbus_framefmt *sink_fmt, *src_fmt;
struct v4l2_rect *sink_crop;
- sink_fmt = v4l2_subdev_get_try_format(sd, sd_state,
+ sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
RKISP1_RSZ_PAD_SRC);
sink_fmt->width = RKISP1_DEFAULT_WIDTH;
sink_fmt->height = RKISP1_DEFAULT_HEIGHT;
@@ -423,7 +398,7 @@ static int rkisp1_rsz_init_config(struct v4l2_subdev *sd,
sink_crop->left = 0;
sink_crop->top = 0;
- src_fmt = v4l2_subdev_get_try_format(sd, sd_state,
+ src_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
RKISP1_RSZ_PAD_SINK);
*src_fmt = *sink_fmt;
@@ -434,16 +409,16 @@ static int rkisp1_rsz_init_config(struct v4l2_subdev *sd,
static void rkisp1_rsz_set_src_fmt(struct rkisp1_resizer *rsz,
struct v4l2_subdev_state *sd_state,
- struct v4l2_mbus_framefmt *format,
- unsigned int which)
+ struct v4l2_mbus_framefmt *format)
{
const struct rkisp1_mbus_info *sink_mbus_info;
struct v4l2_mbus_framefmt *src_fmt, *sink_fmt;
- sink_fmt = rkisp1_rsz_get_pad_fmt(rsz, sd_state, RKISP1_RSZ_PAD_SINK,
- which);
- src_fmt = rkisp1_rsz_get_pad_fmt(rsz, sd_state, RKISP1_RSZ_PAD_SRC,
- which);
+ sink_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SINK);
+ src_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SRC);
+
sink_mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code);
/* for YUV formats, userspace can change the mbus code on the src pad if it is supported */
@@ -463,18 +438,16 @@ static void rkisp1_rsz_set_src_fmt(struct rkisp1_resizer *rsz,
static void rkisp1_rsz_set_sink_crop(struct rkisp1_resizer *rsz,
struct v4l2_subdev_state *sd_state,
- struct v4l2_rect *r,
- unsigned int which)
+ struct v4l2_rect *r)
{
const struct rkisp1_mbus_info *mbus_info;
struct v4l2_mbus_framefmt *sink_fmt;
struct v4l2_rect *sink_crop;
- sink_fmt = rkisp1_rsz_get_pad_fmt(rsz, sd_state, RKISP1_RSZ_PAD_SINK,
- which);
- sink_crop = rkisp1_rsz_get_pad_crop(rsz, sd_state,
- RKISP1_RSZ_PAD_SINK,
- which);
+ sink_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SINK);
+ sink_crop = v4l2_subdev_get_pad_crop(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SINK);
/* Not crop for MP bayer raw data */
mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code);
@@ -501,21 +474,20 @@ static void rkisp1_rsz_set_sink_crop(struct rkisp1_resizer *rsz,
static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz,
struct v4l2_subdev_state *sd_state,
- struct v4l2_mbus_framefmt *format,
- unsigned int which)
+ struct v4l2_mbus_framefmt *format)
{
const struct rkisp1_mbus_info *mbus_info;
struct v4l2_mbus_framefmt *sink_fmt, *src_fmt;
struct v4l2_rect *sink_crop;
bool is_yuv;
- sink_fmt = rkisp1_rsz_get_pad_fmt(rsz, sd_state, RKISP1_RSZ_PAD_SINK,
- which);
- src_fmt = rkisp1_rsz_get_pad_fmt(rsz, sd_state, RKISP1_RSZ_PAD_SRC,
- which);
- sink_crop = rkisp1_rsz_get_pad_crop(rsz, sd_state,
- RKISP1_RSZ_PAD_SINK,
- which);
+ sink_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SINK);
+ src_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SRC);
+ sink_crop = v4l2_subdev_get_pad_crop(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SINK);
+
if (rsz->id == RKISP1_SELFPATH)
sink_fmt->code = MEDIA_BUS_FMT_YUYV8_2X8;
else
@@ -526,8 +498,6 @@ static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz,
sink_fmt->code = RKISP1_DEF_FMT;
mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code);
}
- if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
- rsz->pixel_enc = mbus_info->pixel_enc;
sink_fmt->width = clamp_t(u32, format->width,
RKISP1_ISP_MIN_WIDTH,
@@ -576,21 +546,7 @@ static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz,
src_fmt->quantization = sink_fmt->quantization;
/* Update sink crop */
- rkisp1_rsz_set_sink_crop(rsz, sd_state, sink_crop, which);
-}
-
-static int rkisp1_rsz_get_fmt(struct v4l2_subdev *sd,
- struct v4l2_subdev_state *sd_state,
- struct v4l2_subdev_format *fmt)
-{
- struct rkisp1_resizer *rsz =
- container_of(sd, struct rkisp1_resizer, sd);
-
- mutex_lock(&rsz->ops_lock);
- fmt->format = *rkisp1_rsz_get_pad_fmt(rsz, sd_state, fmt->pad,
- fmt->which);
- mutex_unlock(&rsz->ops_lock);
- return 0;
+ rkisp1_rsz_set_sink_crop(rsz, sd_state, sink_crop);
}
static int rkisp1_rsz_set_fmt(struct v4l2_subdev *sd,
@@ -600,15 +556,11 @@ static int rkisp1_rsz_set_fmt(struct v4l2_subdev *sd,
struct rkisp1_resizer *rsz =
container_of(sd, struct rkisp1_resizer, sd);
- mutex_lock(&rsz->ops_lock);
if (fmt->pad == RKISP1_RSZ_PAD_SINK)
- rkisp1_rsz_set_sink_fmt(rsz, sd_state, &fmt->format,
- fmt->which);
+ rkisp1_rsz_set_sink_fmt(rsz, sd_state, &fmt->format);
else
- rkisp1_rsz_set_src_fmt(rsz, sd_state, &fmt->format,
- fmt->which);
+ rkisp1_rsz_set_src_fmt(rsz, sd_state, &fmt->format);
- mutex_unlock(&rsz->ops_lock);
return 0;
}
@@ -616,35 +568,32 @@ static int rkisp1_rsz_get_selection(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state,
struct v4l2_subdev_selection *sel)
{
- struct rkisp1_resizer *rsz =
- container_of(sd, struct rkisp1_resizer, sd);
struct v4l2_mbus_framefmt *mf_sink;
int ret = 0;
if (sel->pad == RKISP1_RSZ_PAD_SRC)
return -EINVAL;
- mutex_lock(&rsz->ops_lock);
switch (sel->target) {
case V4L2_SEL_TGT_CROP_BOUNDS:
- mf_sink = rkisp1_rsz_get_pad_fmt(rsz, sd_state,
- RKISP1_RSZ_PAD_SINK,
- sel->which);
+ mf_sink = v4l2_subdev_get_pad_format(sd, sd_state,
+ RKISP1_RSZ_PAD_SINK);
sel->r.height = mf_sink->height;
sel->r.width = mf_sink->width;
sel->r.left = 0;
sel->r.top = 0;
break;
+
case V4L2_SEL_TGT_CROP:
- sel->r = *rkisp1_rsz_get_pad_crop(rsz, sd_state,
- RKISP1_RSZ_PAD_SINK,
- sel->which);
+ sel->r = *v4l2_subdev_get_pad_crop(sd, sd_state,
+ RKISP1_RSZ_PAD_SINK);
break;
+
default:
ret = -EINVAL;
+ break;
}
- mutex_unlock(&rsz->ops_lock);
return ret;
}
@@ -661,9 +610,7 @@ static int rkisp1_rsz_set_selection(struct v4l2_subdev *sd,
dev_dbg(rsz->rkisp1->dev, "%s: pad: %d sel(%d,%d)/%dx%d\n", __func__,
sel->pad, sel->r.left, sel->r.top, sel->r.width, sel->r.height);
- mutex_lock(&rsz->ops_lock);
- rkisp1_rsz_set_sink_crop(rsz, sd_state, &sel->r, sel->which);
- mutex_unlock(&rsz->ops_lock);
+ rkisp1_rsz_set_sink_crop(rsz, sd_state, &sel->r);
return 0;
}
@@ -677,7 +624,7 @@ static const struct v4l2_subdev_pad_ops rkisp1_rsz_pad_ops = {
.get_selection = rkisp1_rsz_get_selection,
.set_selection = rkisp1_rsz_set_selection,
.init_cfg = rkisp1_rsz_init_config,
- .get_fmt = rkisp1_rsz_get_fmt,
+ .get_fmt = v4l2_subdev_get_fmt,
.set_fmt = rkisp1_rsz_set_fmt,
.link_validate = v4l2_subdev_link_validate_default,
};
@@ -693,6 +640,7 @@ static int rkisp1_rsz_s_stream(struct v4l2_subdev *sd, int enable)
struct rkisp1_device *rkisp1 = rsz->rkisp1;
struct rkisp1_capture *other = &rkisp1->capture_devs[rsz->id ^ 1];
enum rkisp1_shadow_regs_when when = RKISP1_SHADOW_REGS_SYNC;
+ struct v4l2_subdev_state *sd_state;
if (!enable) {
rkisp1_dcrop_disable(rsz, RKISP1_SHADOW_REGS_ASYNC);
@@ -703,11 +651,13 @@ static int rkisp1_rsz_s_stream(struct v4l2_subdev *sd, int enable)
if (other->is_streaming)
when = RKISP1_SHADOW_REGS_ASYNC;
- mutex_lock(&rsz->ops_lock);
- rkisp1_rsz_config(rsz, when);
- rkisp1_dcrop_config(rsz);
+ sd_state = v4l2_subdev_lock_and_get_active_state(sd);
+
+ rkisp1_rsz_config(rsz, sd_state, when);
+ rkisp1_dcrop_config(rsz, sd_state);
+
+ v4l2_subdev_unlock_state(sd_state);
- mutex_unlock(&rsz->ops_lock);
return 0;
}
@@ -726,15 +676,12 @@ static void rkisp1_rsz_unregister(struct rkisp1_resizer *rsz)
return;
v4l2_device_unregister_subdev(&rsz->sd);
+ v4l2_subdev_cleanup(&rsz->sd);
media_entity_cleanup(&rsz->sd.entity);
- mutex_destroy(&rsz->ops_lock);
}
static int rkisp1_rsz_register(struct rkisp1_resizer *rsz)
{
- struct v4l2_subdev_state state = {
- .pads = rsz->pad_cfg
- };
static const char * const dev_names[] = {
RKISP1_RSZ_MP_DEV_NAME,
RKISP1_RSZ_SP_DEV_NAME
@@ -763,25 +710,26 @@ static int rkisp1_rsz_register(struct rkisp1_resizer *rsz)
pads[RKISP1_RSZ_PAD_SRC].flags = MEDIA_PAD_FL_SOURCE |
MEDIA_PAD_FL_MUST_CONNECT;
- rsz->pixel_enc = RKISP1_DEF_PIXEL_ENC;
-
- mutex_init(&rsz->ops_lock);
ret = media_entity_pads_init(&sd->entity, RKISP1_RSZ_PAD_MAX, pads);
if (ret)
- goto error;
+ goto err_entity_cleanup;
+
+ ret = v4l2_subdev_init_finalize(sd);
+ if (ret)
+ goto err_entity_cleanup;
ret = v4l2_device_register_subdev(&rsz->rkisp1->v4l2_dev, sd);
if (ret) {
dev_err(sd->dev, "Failed to register resizer subdev\n");
- goto error;
+ goto err_subdev_cleanup;
}
- rkisp1_rsz_init_config(sd, &state);
return 0;
-error:
+err_subdev_cleanup:
+ v4l2_subdev_cleanup(sd);
+err_entity_cleanup:
media_entity_cleanup(&sd->entity);
- mutex_destroy(&rsz->ops_lock);
return ret;
}
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v1 1/3] media: rkisp1: resizer: Use V4L2 subdev active state
@ 2023-01-27 0:31 ` Laurent Pinchart
0 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2023-01-27 0:31 UTC (permalink / raw)
To: linux-media; +Cc: Dafna Hirschfeld, Paul Elder, Heiko Stuebner, linux-rockchip
Use the V4L2 subdev active state API to store the active format and crop
rectangle. This simplifies the driver not only by dropping the state
stored in the rkisp1_resizer structure, but also by replacing the
ops_lock with the state lock.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
.../platform/rockchip/rkisp1/rkisp1-common.h | 6 -
.../platform/rockchip/rkisp1/rkisp1-resizer.c | 184 +++++++-----------
2 files changed, 66 insertions(+), 124 deletions(-)
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
index a1293c45aae1..e9626e59e1c8 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
@@ -390,10 +390,7 @@ struct rkisp1_params {
* @id: id of the resizer, one of RKISP1_SELFPATH, RKISP1_MAINPATH
* @rkisp1: pointer to the rkisp1 device
* @pads: media pads
- * @pad_cfg: configurations for the pads
* @config: the set of registers to configure the resizer
- * @pixel_enc: pixel encoding of the resizer
- * @ops_lock: a lock for the subdev ops
*/
struct rkisp1_resizer {
struct v4l2_subdev sd;
@@ -401,10 +398,7 @@ struct rkisp1_resizer {
enum rkisp1_stream_id id;
struct rkisp1_device *rkisp1;
struct media_pad pads[RKISP1_RSZ_PAD_MAX];
- struct v4l2_subdev_pad_config pad_cfg[RKISP1_RSZ_PAD_MAX];
const struct rkisp1_rsz_config *config;
- enum v4l2_pixel_encoding pixel_enc;
- struct mutex ops_lock; /* serialize the subdevice ops */
};
/*
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
index f76afd8112b2..62728ee6d058 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
@@ -117,34 +117,6 @@ static inline void rkisp1_rsz_write(struct rkisp1_resizer *rsz, u32 offset,
rkisp1_write(rsz->rkisp1, rsz->regs_base + offset, value);
}
-static struct v4l2_mbus_framefmt *
-rkisp1_rsz_get_pad_fmt(struct rkisp1_resizer *rsz,
- struct v4l2_subdev_state *sd_state,
- unsigned int pad, u32 which)
-{
- struct v4l2_subdev_state state = {
- .pads = rsz->pad_cfg
- };
- if (which == V4L2_SUBDEV_FORMAT_TRY)
- return v4l2_subdev_get_try_format(&rsz->sd, sd_state, pad);
- else
- return v4l2_subdev_get_try_format(&rsz->sd, &state, pad);
-}
-
-static struct v4l2_rect *
-rkisp1_rsz_get_pad_crop(struct rkisp1_resizer *rsz,
- struct v4l2_subdev_state *sd_state,
- unsigned int pad, u32 which)
-{
- struct v4l2_subdev_state state = {
- .pads = rsz->pad_cfg
- };
- if (which == V4L2_SUBDEV_FORMAT_TRY)
- return v4l2_subdev_get_try_crop(&rsz->sd, sd_state, pad);
- else
- return v4l2_subdev_get_try_crop(&rsz->sd, &state, pad);
-}
-
/* ----------------------------------------------------------------------------
* Dual crop hw configs
*/
@@ -165,17 +137,18 @@ static void rkisp1_dcrop_disable(struct rkisp1_resizer *rsz,
}
/* configure dual-crop unit */
-static void rkisp1_dcrop_config(struct rkisp1_resizer *rsz)
+static void rkisp1_dcrop_config(struct rkisp1_resizer *rsz,
+ struct v4l2_subdev_state *sd_state)
{
struct rkisp1_device *rkisp1 = rsz->rkisp1;
struct v4l2_mbus_framefmt *sink_fmt;
struct v4l2_rect *sink_crop;
u32 dc_ctrl;
- sink_crop = rkisp1_rsz_get_pad_crop(rsz, NULL, RKISP1_RSZ_PAD_SINK,
- V4L2_SUBDEV_FORMAT_ACTIVE);
- sink_fmt = rkisp1_rsz_get_pad_fmt(rsz, NULL, RKISP1_RSZ_PAD_SINK,
- V4L2_SUBDEV_FORMAT_ACTIVE);
+ sink_crop = v4l2_subdev_get_pad_crop(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SINK);
+ sink_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SINK);
if (sink_crop->width == sink_fmt->width &&
sink_crop->height == sink_fmt->height &&
@@ -296,6 +269,7 @@ static void rkisp1_rsz_config_regs(struct rkisp1_resizer *rsz,
}
static void rkisp1_rsz_config(struct rkisp1_resizer *rsz,
+ struct v4l2_subdev_state *sd_state,
enum rkisp1_shadow_regs_when when)
{
const struct rkisp1_rsz_yuv_mbus_info *sink_yuv_info, *src_yuv_info;
@@ -303,20 +277,21 @@ static void rkisp1_rsz_config(struct rkisp1_resizer *rsz,
struct v4l2_mbus_framefmt *src_fmt, *sink_fmt;
struct v4l2_rect *sink_crop;
- sink_crop = rkisp1_rsz_get_pad_crop(rsz, NULL, RKISP1_RSZ_PAD_SINK,
- V4L2_SUBDEV_FORMAT_ACTIVE);
- src_fmt = rkisp1_rsz_get_pad_fmt(rsz, NULL, RKISP1_RSZ_PAD_SRC,
- V4L2_SUBDEV_FORMAT_ACTIVE);
- src_yuv_info = rkisp1_rsz_get_yuv_mbus_info(src_fmt->code);
- sink_fmt = rkisp1_rsz_get_pad_fmt(rsz, NULL, RKISP1_RSZ_PAD_SINK,
- V4L2_SUBDEV_FORMAT_ACTIVE);
+ sink_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SINK);
+ sink_crop = v4l2_subdev_get_pad_crop(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SINK);
+ src_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SRC);
+
sink_yuv_info = rkisp1_rsz_get_yuv_mbus_info(sink_fmt->code);
+ src_yuv_info = rkisp1_rsz_get_yuv_mbus_info(src_fmt->code);
/*
* The resizer only works on yuv formats,
* so return if it is bayer format.
*/
- if (rsz->pixel_enc == V4L2_PIXEL_ENC_BAYER) {
+ if (!sink_yuv_info) {
rkisp1_rsz_disable(rsz, when);
return;
}
@@ -405,7 +380,7 @@ static int rkisp1_rsz_init_config(struct v4l2_subdev *sd,
struct v4l2_mbus_framefmt *sink_fmt, *src_fmt;
struct v4l2_rect *sink_crop;
- sink_fmt = v4l2_subdev_get_try_format(sd, sd_state,
+ sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
RKISP1_RSZ_PAD_SRC);
sink_fmt->width = RKISP1_DEFAULT_WIDTH;
sink_fmt->height = RKISP1_DEFAULT_HEIGHT;
@@ -423,7 +398,7 @@ static int rkisp1_rsz_init_config(struct v4l2_subdev *sd,
sink_crop->left = 0;
sink_crop->top = 0;
- src_fmt = v4l2_subdev_get_try_format(sd, sd_state,
+ src_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
RKISP1_RSZ_PAD_SINK);
*src_fmt = *sink_fmt;
@@ -434,16 +409,16 @@ static int rkisp1_rsz_init_config(struct v4l2_subdev *sd,
static void rkisp1_rsz_set_src_fmt(struct rkisp1_resizer *rsz,
struct v4l2_subdev_state *sd_state,
- struct v4l2_mbus_framefmt *format,
- unsigned int which)
+ struct v4l2_mbus_framefmt *format)
{
const struct rkisp1_mbus_info *sink_mbus_info;
struct v4l2_mbus_framefmt *src_fmt, *sink_fmt;
- sink_fmt = rkisp1_rsz_get_pad_fmt(rsz, sd_state, RKISP1_RSZ_PAD_SINK,
- which);
- src_fmt = rkisp1_rsz_get_pad_fmt(rsz, sd_state, RKISP1_RSZ_PAD_SRC,
- which);
+ sink_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SINK);
+ src_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SRC);
+
sink_mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code);
/* for YUV formats, userspace can change the mbus code on the src pad if it is supported */
@@ -463,18 +438,16 @@ static void rkisp1_rsz_set_src_fmt(struct rkisp1_resizer *rsz,
static void rkisp1_rsz_set_sink_crop(struct rkisp1_resizer *rsz,
struct v4l2_subdev_state *sd_state,
- struct v4l2_rect *r,
- unsigned int which)
+ struct v4l2_rect *r)
{
const struct rkisp1_mbus_info *mbus_info;
struct v4l2_mbus_framefmt *sink_fmt;
struct v4l2_rect *sink_crop;
- sink_fmt = rkisp1_rsz_get_pad_fmt(rsz, sd_state, RKISP1_RSZ_PAD_SINK,
- which);
- sink_crop = rkisp1_rsz_get_pad_crop(rsz, sd_state,
- RKISP1_RSZ_PAD_SINK,
- which);
+ sink_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SINK);
+ sink_crop = v4l2_subdev_get_pad_crop(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SINK);
/* Not crop for MP bayer raw data */
mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code);
@@ -501,21 +474,20 @@ static void rkisp1_rsz_set_sink_crop(struct rkisp1_resizer *rsz,
static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz,
struct v4l2_subdev_state *sd_state,
- struct v4l2_mbus_framefmt *format,
- unsigned int which)
+ struct v4l2_mbus_framefmt *format)
{
const struct rkisp1_mbus_info *mbus_info;
struct v4l2_mbus_framefmt *sink_fmt, *src_fmt;
struct v4l2_rect *sink_crop;
bool is_yuv;
- sink_fmt = rkisp1_rsz_get_pad_fmt(rsz, sd_state, RKISP1_RSZ_PAD_SINK,
- which);
- src_fmt = rkisp1_rsz_get_pad_fmt(rsz, sd_state, RKISP1_RSZ_PAD_SRC,
- which);
- sink_crop = rkisp1_rsz_get_pad_crop(rsz, sd_state,
- RKISP1_RSZ_PAD_SINK,
- which);
+ sink_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SINK);
+ src_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SRC);
+ sink_crop = v4l2_subdev_get_pad_crop(&rsz->sd, sd_state,
+ RKISP1_RSZ_PAD_SINK);
+
if (rsz->id == RKISP1_SELFPATH)
sink_fmt->code = MEDIA_BUS_FMT_YUYV8_2X8;
else
@@ -526,8 +498,6 @@ static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz,
sink_fmt->code = RKISP1_DEF_FMT;
mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code);
}
- if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
- rsz->pixel_enc = mbus_info->pixel_enc;
sink_fmt->width = clamp_t(u32, format->width,
RKISP1_ISP_MIN_WIDTH,
@@ -576,21 +546,7 @@ static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz,
src_fmt->quantization = sink_fmt->quantization;
/* Update sink crop */
- rkisp1_rsz_set_sink_crop(rsz, sd_state, sink_crop, which);
-}
-
-static int rkisp1_rsz_get_fmt(struct v4l2_subdev *sd,
- struct v4l2_subdev_state *sd_state,
- struct v4l2_subdev_format *fmt)
-{
- struct rkisp1_resizer *rsz =
- container_of(sd, struct rkisp1_resizer, sd);
-
- mutex_lock(&rsz->ops_lock);
- fmt->format = *rkisp1_rsz_get_pad_fmt(rsz, sd_state, fmt->pad,
- fmt->which);
- mutex_unlock(&rsz->ops_lock);
- return 0;
+ rkisp1_rsz_set_sink_crop(rsz, sd_state, sink_crop);
}
static int rkisp1_rsz_set_fmt(struct v4l2_subdev *sd,
@@ -600,15 +556,11 @@ static int rkisp1_rsz_set_fmt(struct v4l2_subdev *sd,
struct rkisp1_resizer *rsz =
container_of(sd, struct rkisp1_resizer, sd);
- mutex_lock(&rsz->ops_lock);
if (fmt->pad == RKISP1_RSZ_PAD_SINK)
- rkisp1_rsz_set_sink_fmt(rsz, sd_state, &fmt->format,
- fmt->which);
+ rkisp1_rsz_set_sink_fmt(rsz, sd_state, &fmt->format);
else
- rkisp1_rsz_set_src_fmt(rsz, sd_state, &fmt->format,
- fmt->which);
+ rkisp1_rsz_set_src_fmt(rsz, sd_state, &fmt->format);
- mutex_unlock(&rsz->ops_lock);
return 0;
}
@@ -616,35 +568,32 @@ static int rkisp1_rsz_get_selection(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state,
struct v4l2_subdev_selection *sel)
{
- struct rkisp1_resizer *rsz =
- container_of(sd, struct rkisp1_resizer, sd);
struct v4l2_mbus_framefmt *mf_sink;
int ret = 0;
if (sel->pad == RKISP1_RSZ_PAD_SRC)
return -EINVAL;
- mutex_lock(&rsz->ops_lock);
switch (sel->target) {
case V4L2_SEL_TGT_CROP_BOUNDS:
- mf_sink = rkisp1_rsz_get_pad_fmt(rsz, sd_state,
- RKISP1_RSZ_PAD_SINK,
- sel->which);
+ mf_sink = v4l2_subdev_get_pad_format(sd, sd_state,
+ RKISP1_RSZ_PAD_SINK);
sel->r.height = mf_sink->height;
sel->r.width = mf_sink->width;
sel->r.left = 0;
sel->r.top = 0;
break;
+
case V4L2_SEL_TGT_CROP:
- sel->r = *rkisp1_rsz_get_pad_crop(rsz, sd_state,
- RKISP1_RSZ_PAD_SINK,
- sel->which);
+ sel->r = *v4l2_subdev_get_pad_crop(sd, sd_state,
+ RKISP1_RSZ_PAD_SINK);
break;
+
default:
ret = -EINVAL;
+ break;
}
- mutex_unlock(&rsz->ops_lock);
return ret;
}
@@ -661,9 +610,7 @@ static int rkisp1_rsz_set_selection(struct v4l2_subdev *sd,
dev_dbg(rsz->rkisp1->dev, "%s: pad: %d sel(%d,%d)/%dx%d\n", __func__,
sel->pad, sel->r.left, sel->r.top, sel->r.width, sel->r.height);
- mutex_lock(&rsz->ops_lock);
- rkisp1_rsz_set_sink_crop(rsz, sd_state, &sel->r, sel->which);
- mutex_unlock(&rsz->ops_lock);
+ rkisp1_rsz_set_sink_crop(rsz, sd_state, &sel->r);
return 0;
}
@@ -677,7 +624,7 @@ static const struct v4l2_subdev_pad_ops rkisp1_rsz_pad_ops = {
.get_selection = rkisp1_rsz_get_selection,
.set_selection = rkisp1_rsz_set_selection,
.init_cfg = rkisp1_rsz_init_config,
- .get_fmt = rkisp1_rsz_get_fmt,
+ .get_fmt = v4l2_subdev_get_fmt,
.set_fmt = rkisp1_rsz_set_fmt,
.link_validate = v4l2_subdev_link_validate_default,
};
@@ -693,6 +640,7 @@ static int rkisp1_rsz_s_stream(struct v4l2_subdev *sd, int enable)
struct rkisp1_device *rkisp1 = rsz->rkisp1;
struct rkisp1_capture *other = &rkisp1->capture_devs[rsz->id ^ 1];
enum rkisp1_shadow_regs_when when = RKISP1_SHADOW_REGS_SYNC;
+ struct v4l2_subdev_state *sd_state;
if (!enable) {
rkisp1_dcrop_disable(rsz, RKISP1_SHADOW_REGS_ASYNC);
@@ -703,11 +651,13 @@ static int rkisp1_rsz_s_stream(struct v4l2_subdev *sd, int enable)
if (other->is_streaming)
when = RKISP1_SHADOW_REGS_ASYNC;
- mutex_lock(&rsz->ops_lock);
- rkisp1_rsz_config(rsz, when);
- rkisp1_dcrop_config(rsz);
+ sd_state = v4l2_subdev_lock_and_get_active_state(sd);
+
+ rkisp1_rsz_config(rsz, sd_state, when);
+ rkisp1_dcrop_config(rsz, sd_state);
+
+ v4l2_subdev_unlock_state(sd_state);
- mutex_unlock(&rsz->ops_lock);
return 0;
}
@@ -726,15 +676,12 @@ static void rkisp1_rsz_unregister(struct rkisp1_resizer *rsz)
return;
v4l2_device_unregister_subdev(&rsz->sd);
+ v4l2_subdev_cleanup(&rsz->sd);
media_entity_cleanup(&rsz->sd.entity);
- mutex_destroy(&rsz->ops_lock);
}
static int rkisp1_rsz_register(struct rkisp1_resizer *rsz)
{
- struct v4l2_subdev_state state = {
- .pads = rsz->pad_cfg
- };
static const char * const dev_names[] = {
RKISP1_RSZ_MP_DEV_NAME,
RKISP1_RSZ_SP_DEV_NAME
@@ -763,25 +710,26 @@ static int rkisp1_rsz_register(struct rkisp1_resizer *rsz)
pads[RKISP1_RSZ_PAD_SRC].flags = MEDIA_PAD_FL_SOURCE |
MEDIA_PAD_FL_MUST_CONNECT;
- rsz->pixel_enc = RKISP1_DEF_PIXEL_ENC;
-
- mutex_init(&rsz->ops_lock);
ret = media_entity_pads_init(&sd->entity, RKISP1_RSZ_PAD_MAX, pads);
if (ret)
- goto error;
+ goto err_entity_cleanup;
+
+ ret = v4l2_subdev_init_finalize(sd);
+ if (ret)
+ goto err_entity_cleanup;
ret = v4l2_device_register_subdev(&rsz->rkisp1->v4l2_dev, sd);
if (ret) {
dev_err(sd->dev, "Failed to register resizer subdev\n");
- goto error;
+ goto err_subdev_cleanup;
}
- rkisp1_rsz_init_config(sd, &state);
return 0;
-error:
+err_subdev_cleanup:
+ v4l2_subdev_cleanup(sd);
+err_entity_cleanup:
media_entity_cleanup(&sd->entity);
- mutex_destroy(&rsz->ops_lock);
return ret;
}
--
Regards,
Laurent Pinchart
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v1 2/3] media: rkisp1: isp: Use V4L2 subdev active state
2023-01-27 0:31 ` Laurent Pinchart
@ 2023-01-27 0:31 ` Laurent Pinchart
-1 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2023-01-27 0:31 UTC (permalink / raw)
To: linux-media; +Cc: Dafna Hirschfeld, Paul Elder, Heiko Stuebner, linux-rockchip
Use the V4L2 subdev active state API to store the active format and crop
rectangle. This simplifies the driver not only by dropping the state
stored in the rkisp1_isp structure, but also by replacing the ops_lock
with the state lock.
The rkisp1_isp.sink_fmt field needs to be kept, as it is accessed from
the stats interrupt handler. To simplifye the rkisp1_isp_set_sink_fmt()
implementation, the field is now set when starting the ISP, instead of
when setting the format.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
.../platform/rockchip/rkisp1/rkisp1-common.h | 6 -
.../platform/rockchip/rkisp1/rkisp1-isp.c | 261 +++++++-----------
2 files changed, 102 insertions(+), 165 deletions(-)
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
index e9626e59e1c8..d43ff1b549d9 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
@@ -190,20 +190,14 @@ struct rkisp1_csi {
* @sd: v4l2_subdev variable
* @rkisp1: pointer to rkisp1_device
* @pads: media pads
- * @pad_cfg: pads configurations
* @sink_fmt: input format
- * @src_fmt: output format
- * @ops_lock: ops serialization
* @frame_sequence: used to synchronize frame_id between video devices.
*/
struct rkisp1_isp {
struct v4l2_subdev sd;
struct rkisp1_device *rkisp1;
struct media_pad pads[RKISP1_ISP_PAD_MAX];
- struct v4l2_subdev_pad_config pad_cfg[RKISP1_ISP_PAD_MAX];
const struct rkisp1_mbus_info *sink_fmt;
- const struct rkisp1_mbus_info *src_fmt;
- struct mutex ops_lock; /* serialize the subdevice ops */
__u32 frame_sequence;
};
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
index 585cf3f53469..4d4f7b59f848 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
@@ -53,40 +53,6 @@
* +---------------------------------------------------------+
*/
-/* ----------------------------------------------------------------------------
- * Helpers
- */
-
-static struct v4l2_mbus_framefmt *
-rkisp1_isp_get_pad_fmt(struct rkisp1_isp *isp,
- struct v4l2_subdev_state *sd_state,
- unsigned int pad, u32 which)
-{
- struct v4l2_subdev_state state = {
- .pads = isp->pad_cfg
- };
-
- if (which == V4L2_SUBDEV_FORMAT_TRY)
- return v4l2_subdev_get_try_format(&isp->sd, sd_state, pad);
- else
- return v4l2_subdev_get_try_format(&isp->sd, &state, pad);
-}
-
-static struct v4l2_rect *
-rkisp1_isp_get_pad_crop(struct rkisp1_isp *isp,
- struct v4l2_subdev_state *sd_state,
- unsigned int pad, u32 which)
-{
- struct v4l2_subdev_state state = {
- .pads = isp->pad_cfg
- };
-
- if (which == V4L2_SUBDEV_FORMAT_TRY)
- return v4l2_subdev_get_try_crop(&isp->sd, sd_state, pad);
- else
- return v4l2_subdev_get_try_crop(&isp->sd, &state, pad);
-}
-
/* ----------------------------------------------------------------------------
* Camera Interface registers configurations
*/
@@ -96,12 +62,12 @@ rkisp1_isp_get_pad_crop(struct rkisp1_isp *isp,
* This should only be called when configuring CIF
* or at the frame end interrupt
*/
-static void rkisp1_config_ism(struct rkisp1_isp *isp)
+static void rkisp1_config_ism(struct rkisp1_isp *isp,
+ struct v4l2_subdev_state *sd_state)
{
const struct v4l2_rect *src_crop =
- rkisp1_isp_get_pad_crop(isp, NULL,
- RKISP1_ISP_PAD_SOURCE_VIDEO,
- V4L2_SUBDEV_FORMAT_ACTIVE);
+ v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SOURCE_VIDEO);
struct rkisp1_device *rkisp1 = isp->rkisp1;
u32 val;
@@ -125,21 +91,26 @@ static void rkisp1_config_ism(struct rkisp1_isp *isp)
* configure ISP blocks with input format, size......
*/
static int rkisp1_config_isp(struct rkisp1_isp *isp,
+ struct v4l2_subdev_state *sd_state,
enum v4l2_mbus_type mbus_type, u32 mbus_flags)
{
struct rkisp1_device *rkisp1 = isp->rkisp1;
u32 isp_ctrl = 0, irq_mask = 0, acq_mult = 0, acq_prop = 0;
- const struct rkisp1_mbus_info *sink_fmt = isp->sink_fmt;
- const struct rkisp1_mbus_info *src_fmt = isp->src_fmt;
+ const struct rkisp1_mbus_info *sink_fmt;
+ const struct rkisp1_mbus_info *src_fmt;
+ const struct v4l2_mbus_framefmt *src_frm;
const struct v4l2_mbus_framefmt *sink_frm;
const struct v4l2_rect *sink_crop;
- sink_frm = rkisp1_isp_get_pad_fmt(isp, NULL,
- RKISP1_ISP_PAD_SINK_VIDEO,
- V4L2_SUBDEV_FORMAT_ACTIVE);
- sink_crop = rkisp1_isp_get_pad_crop(isp, NULL,
- RKISP1_ISP_PAD_SINK_VIDEO,
- V4L2_SUBDEV_FORMAT_ACTIVE);
+ sink_frm = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SINK_VIDEO);
+ sink_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SINK_VIDEO);
+ src_frm = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SOURCE_VIDEO);
+
+ sink_fmt = rkisp1_mbus_info_get_by_code(sink_frm->code);
+ src_fmt = rkisp1_mbus_info_get_by_code(src_frm->code);
if (sink_fmt->pixel_enc == V4L2_PIXEL_ENC_BAYER) {
acq_mult = 1;
@@ -230,14 +201,15 @@ static int rkisp1_config_isp(struct rkisp1_isp *isp,
} else {
struct v4l2_mbus_framefmt *src_frm;
- src_frm = rkisp1_isp_get_pad_fmt(isp, NULL,
- RKISP1_ISP_PAD_SOURCE_VIDEO,
- V4L2_SUBDEV_FORMAT_ACTIVE);
+ src_frm = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SOURCE_VIDEO);
rkisp1_params_pre_configure(&rkisp1->params, sink_fmt->bayer_pat,
src_frm->quantization,
src_frm->ycbcr_enc);
}
+ isp->sink_fmt = sink_fmt;
+
return 0;
}
@@ -258,16 +230,17 @@ static void rkisp1_config_path(struct rkisp1_isp *isp,
/* Hardware configure Entry */
static int rkisp1_config_cif(struct rkisp1_isp *isp,
+ struct v4l2_subdev_state *sd_state,
enum v4l2_mbus_type mbus_type, u32 mbus_flags)
{
int ret;
- ret = rkisp1_config_isp(isp, mbus_type, mbus_flags);
+ ret = rkisp1_config_isp(isp, sd_state, mbus_type, mbus_flags);
if (ret)
return ret;
rkisp1_config_path(isp, mbus_type);
- rkisp1_config_ism(isp);
+ rkisp1_config_ism(isp, sd_state);
return 0;
}
@@ -328,9 +301,12 @@ static void rkisp1_config_clk(struct rkisp1_isp *isp)
}
}
-static void rkisp1_isp_start(struct rkisp1_isp *isp)
+static void rkisp1_isp_start(struct rkisp1_isp *isp,
+ struct v4l2_subdev_state *sd_state)
{
struct rkisp1_device *rkisp1 = isp->rkisp1;
+ const struct v4l2_mbus_framefmt *src_fmt;
+ const struct rkisp1_mbus_info *src_info;
u32 val;
rkisp1_config_clk(isp);
@@ -342,7 +318,11 @@ static void rkisp1_isp_start(struct rkisp1_isp *isp)
RKISP1_CIF_ISP_CTRL_ISP_INFORM_ENABLE;
rkisp1_write(rkisp1, RKISP1_CIF_ISP_CTRL, val);
- if (isp->src_fmt->pixel_enc != V4L2_PIXEL_ENC_BAYER)
+ src_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SOURCE_VIDEO);
+ src_info = rkisp1_mbus_info_get_by_code(src_fmt->code);
+
+ if (src_info->pixel_enc != V4L2_PIXEL_ENC_BAYER)
rkisp1_params_post_configure(&rkisp1->params);
}
@@ -436,7 +416,7 @@ static int rkisp1_isp_init_config(struct v4l2_subdev *sd,
struct v4l2_rect *sink_crop, *src_crop;
/* Video. */
- sink_fmt = v4l2_subdev_get_try_format(sd, sd_state,
+ sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
RKISP1_ISP_PAD_SINK_VIDEO);
sink_fmt->width = RKISP1_DEFAULT_WIDTH;
sink_fmt->height = RKISP1_DEFAULT_HEIGHT;
@@ -447,14 +427,14 @@ static int rkisp1_isp_init_config(struct v4l2_subdev *sd,
sink_fmt->ycbcr_enc = V4L2_YCBCR_ENC_601;
sink_fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
- sink_crop = v4l2_subdev_get_try_crop(sd, sd_state,
+ sink_crop = v4l2_subdev_get_pad_crop(sd, sd_state,
RKISP1_ISP_PAD_SINK_VIDEO);
sink_crop->width = RKISP1_DEFAULT_WIDTH;
sink_crop->height = RKISP1_DEFAULT_HEIGHT;
sink_crop->left = 0;
sink_crop->top = 0;
- src_fmt = v4l2_subdev_get_try_format(sd, sd_state,
+ src_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
RKISP1_ISP_PAD_SOURCE_VIDEO);
*src_fmt = *sink_fmt;
src_fmt->code = RKISP1_DEF_SRC_PAD_FMT;
@@ -463,14 +443,14 @@ static int rkisp1_isp_init_config(struct v4l2_subdev *sd,
src_fmt->ycbcr_enc = V4L2_YCBCR_ENC_601;
src_fmt->quantization = V4L2_QUANTIZATION_LIM_RANGE;
- src_crop = v4l2_subdev_get_try_crop(sd, sd_state,
+ src_crop = v4l2_subdev_get_pad_crop(sd, sd_state,
RKISP1_ISP_PAD_SOURCE_VIDEO);
*src_crop = *sink_crop;
/* Parameters and statistics. */
- sink_fmt = v4l2_subdev_get_try_format(sd, sd_state,
+ sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
RKISP1_ISP_PAD_SINK_PARAMS);
- src_fmt = v4l2_subdev_get_try_format(sd, sd_state,
+ src_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
RKISP1_ISP_PAD_SOURCE_STATS);
sink_fmt->width = 0;
sink_fmt->height = 0;
@@ -483,8 +463,7 @@ static int rkisp1_isp_init_config(struct v4l2_subdev *sd,
static void rkisp1_isp_set_src_fmt(struct rkisp1_isp *isp,
struct v4l2_subdev_state *sd_state,
- struct v4l2_mbus_framefmt *format,
- unsigned int which)
+ struct v4l2_mbus_framefmt *format)
{
const struct rkisp1_mbus_info *sink_info;
const struct rkisp1_mbus_info *src_info;
@@ -493,12 +472,12 @@ static void rkisp1_isp_set_src_fmt(struct rkisp1_isp *isp,
const struct v4l2_rect *src_crop;
bool set_csc;
- sink_fmt = rkisp1_isp_get_pad_fmt(isp, sd_state,
- RKISP1_ISP_PAD_SINK_VIDEO, which);
- src_fmt = rkisp1_isp_get_pad_fmt(isp, sd_state,
- RKISP1_ISP_PAD_SOURCE_VIDEO, which);
- src_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
- RKISP1_ISP_PAD_SOURCE_VIDEO, which);
+ sink_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SINK_VIDEO);
+ src_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SOURCE_VIDEO);
+ src_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SOURCE_VIDEO);
/*
* Media bus code. The ISP can operate in pass-through mode (Bayer in,
@@ -581,26 +560,20 @@ static void rkisp1_isp_set_src_fmt(struct rkisp1_isp *isp,
*/
if (set_csc)
format->flags |= V4L2_MBUS_FRAMEFMT_SET_CSC;
-
- /* Store the source format info when setting the active format. */
- if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
- isp->src_fmt = src_info;
}
static void rkisp1_isp_set_src_crop(struct rkisp1_isp *isp,
struct v4l2_subdev_state *sd_state,
- struct v4l2_rect *r, unsigned int which)
+ struct v4l2_rect *r)
{
struct v4l2_mbus_framefmt *src_fmt;
const struct v4l2_rect *sink_crop;
struct v4l2_rect *src_crop;
- src_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
- RKISP1_ISP_PAD_SOURCE_VIDEO,
- which);
- sink_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
- RKISP1_ISP_PAD_SINK_VIDEO,
- which);
+ src_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SOURCE_VIDEO);
+ sink_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SINK_VIDEO);
src_crop->left = ALIGN(r->left, 2);
src_crop->width = ALIGN(r->width, 2);
@@ -611,24 +584,22 @@ static void rkisp1_isp_set_src_crop(struct rkisp1_isp *isp,
*r = *src_crop;
/* Propagate to out format */
- src_fmt = rkisp1_isp_get_pad_fmt(isp, sd_state,
- RKISP1_ISP_PAD_SOURCE_VIDEO, which);
- rkisp1_isp_set_src_fmt(isp, sd_state, src_fmt, which);
+ src_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SOURCE_VIDEO);
+ rkisp1_isp_set_src_fmt(isp, sd_state, src_fmt);
}
static void rkisp1_isp_set_sink_crop(struct rkisp1_isp *isp,
struct v4l2_subdev_state *sd_state,
- struct v4l2_rect *r, unsigned int which)
+ struct v4l2_rect *r)
{
struct v4l2_rect *sink_crop, *src_crop;
const struct v4l2_mbus_framefmt *sink_fmt;
- sink_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
- RKISP1_ISP_PAD_SINK_VIDEO,
- which);
- sink_fmt = rkisp1_isp_get_pad_fmt(isp, sd_state,
- RKISP1_ISP_PAD_SINK_VIDEO,
- which);
+ sink_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SINK_VIDEO);
+ sink_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SINK_VIDEO);
sink_crop->left = ALIGN(r->left, 2);
sink_crop->width = ALIGN(r->width, 2);
@@ -639,32 +610,28 @@ static void rkisp1_isp_set_sink_crop(struct rkisp1_isp *isp,
*r = *sink_crop;
/* Propagate to out crop */
- src_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
- RKISP1_ISP_PAD_SOURCE_VIDEO, which);
- rkisp1_isp_set_src_crop(isp, sd_state, src_crop, which);
+ src_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SOURCE_VIDEO);
+ rkisp1_isp_set_src_crop(isp, sd_state, src_crop);
}
static void rkisp1_isp_set_sink_fmt(struct rkisp1_isp *isp,
struct v4l2_subdev_state *sd_state,
- struct v4l2_mbus_framefmt *format,
- unsigned int which)
+ struct v4l2_mbus_framefmt *format)
{
const struct rkisp1_mbus_info *mbus_info;
struct v4l2_mbus_framefmt *sink_fmt;
struct v4l2_rect *sink_crop;
bool is_yuv;
- sink_fmt = rkisp1_isp_get_pad_fmt(isp, sd_state,
- RKISP1_ISP_PAD_SINK_VIDEO,
- which);
+ sink_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SINK_VIDEO);
sink_fmt->code = format->code;
mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code);
if (!mbus_info || !(mbus_info->direction & RKISP1_ISP_SD_SINK)) {
sink_fmt->code = RKISP1_DEF_SINK_PAD_FMT;
mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code);
}
- if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
- isp->sink_fmt = mbus_info;
sink_fmt->width = clamp_t(u32, format->width,
RKISP1_ISP_MIN_WIDTH,
@@ -706,23 +673,9 @@ static void rkisp1_isp_set_sink_fmt(struct rkisp1_isp *isp,
*format = *sink_fmt;
/* Propagate to in crop */
- sink_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
- RKISP1_ISP_PAD_SINK_VIDEO,
- which);
- rkisp1_isp_set_sink_crop(isp, sd_state, sink_crop, which);
-}
-
-static int rkisp1_isp_get_fmt(struct v4l2_subdev *sd,
- struct v4l2_subdev_state *sd_state,
- struct v4l2_subdev_format *fmt)
-{
- struct rkisp1_isp *isp = to_rkisp1_isp(sd);
-
- mutex_lock(&isp->ops_lock);
- fmt->format = *rkisp1_isp_get_pad_fmt(isp, sd_state, fmt->pad,
- fmt->which);
- mutex_unlock(&isp->ops_lock);
- return 0;
+ sink_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SINK_VIDEO);
+ rkisp1_isp_set_sink_crop(isp, sd_state, sink_crop);
}
static int rkisp1_isp_set_fmt(struct v4l2_subdev *sd,
@@ -731,18 +684,13 @@ static int rkisp1_isp_set_fmt(struct v4l2_subdev *sd,
{
struct rkisp1_isp *isp = to_rkisp1_isp(sd);
- mutex_lock(&isp->ops_lock);
if (fmt->pad == RKISP1_ISP_PAD_SINK_VIDEO)
- rkisp1_isp_set_sink_fmt(isp, sd_state, &fmt->format,
- fmt->which);
+ rkisp1_isp_set_sink_fmt(isp, sd_state, &fmt->format);
else if (fmt->pad == RKISP1_ISP_PAD_SOURCE_VIDEO)
- rkisp1_isp_set_src_fmt(isp, sd_state, &fmt->format,
- fmt->which);
+ rkisp1_isp_set_src_fmt(isp, sd_state, &fmt->format);
else
- fmt->format = *rkisp1_isp_get_pad_fmt(isp, sd_state, fmt->pad,
- fmt->which);
+ fmt->format = *v4l2_subdev_get_pad_format(sd, sd_state, fmt->pad);
- mutex_unlock(&isp->ops_lock);
return 0;
}
@@ -750,39 +698,37 @@ static int rkisp1_isp_get_selection(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state,
struct v4l2_subdev_selection *sel)
{
- struct rkisp1_isp *isp = to_rkisp1_isp(sd);
int ret = 0;
if (sel->pad != RKISP1_ISP_PAD_SOURCE_VIDEO &&
sel->pad != RKISP1_ISP_PAD_SINK_VIDEO)
return -EINVAL;
- mutex_lock(&isp->ops_lock);
switch (sel->target) {
case V4L2_SEL_TGT_CROP_BOUNDS:
if (sel->pad == RKISP1_ISP_PAD_SINK_VIDEO) {
struct v4l2_mbus_framefmt *fmt;
- fmt = rkisp1_isp_get_pad_fmt(isp, sd_state, sel->pad,
- sel->which);
+ fmt = v4l2_subdev_get_pad_format(sd, sd_state, sel->pad);
sel->r.height = fmt->height;
sel->r.width = fmt->width;
sel->r.left = 0;
sel->r.top = 0;
} else {
- sel->r = *rkisp1_isp_get_pad_crop(isp, sd_state,
- RKISP1_ISP_PAD_SINK_VIDEO,
- sel->which);
+ sel->r = *v4l2_subdev_get_pad_crop(sd, sd_state,
+ RKISP1_ISP_PAD_SINK_VIDEO);
}
break;
+
case V4L2_SEL_TGT_CROP:
- sel->r = *rkisp1_isp_get_pad_crop(isp, sd_state, sel->pad,
- sel->which);
+ sel->r = *v4l2_subdev_get_pad_crop(sd, sd_state, sel->pad);
break;
+
default:
ret = -EINVAL;
+ break;
}
- mutex_unlock(&isp->ops_lock);
+
return ret;
}
@@ -798,15 +744,14 @@ static int rkisp1_isp_set_selection(struct v4l2_subdev *sd,
dev_dbg(isp->rkisp1->dev, "%s: pad: %d sel(%d,%d)/%dx%d\n", __func__,
sel->pad, sel->r.left, sel->r.top, sel->r.width, sel->r.height);
- mutex_lock(&isp->ops_lock);
+
if (sel->pad == RKISP1_ISP_PAD_SINK_VIDEO)
- rkisp1_isp_set_sink_crop(isp, sd_state, &sel->r, sel->which);
+ rkisp1_isp_set_sink_crop(isp, sd_state, &sel->r);
else if (sel->pad == RKISP1_ISP_PAD_SOURCE_VIDEO)
- rkisp1_isp_set_src_crop(isp, sd_state, &sel->r, sel->which);
+ rkisp1_isp_set_src_crop(isp, sd_state, &sel->r);
else
ret = -EINVAL;
- mutex_unlock(&isp->ops_lock);
return ret;
}
@@ -824,7 +769,7 @@ static const struct v4l2_subdev_pad_ops rkisp1_isp_pad_ops = {
.get_selection = rkisp1_isp_get_selection,
.set_selection = rkisp1_isp_set_selection,
.init_cfg = rkisp1_isp_init_config,
- .get_fmt = rkisp1_isp_get_fmt,
+ .get_fmt = v4l2_subdev_get_fmt,
.set_fmt = rkisp1_isp_set_fmt,
.link_validate = v4l2_subdev_link_validate_default,
};
@@ -837,6 +782,7 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
{
struct rkisp1_isp *isp = to_rkisp1_isp(sd);
struct rkisp1_device *rkisp1 = isp->rkisp1;
+ struct v4l2_subdev_state *sd_state;
struct media_pad *source_pad;
struct media_pad *sink_pad;
enum v4l2_mbus_type mbus_type;
@@ -877,21 +823,23 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
}
isp->frame_sequence = -1;
- mutex_lock(&isp->ops_lock);
- ret = rkisp1_config_cif(isp, mbus_type, mbus_flags);
+
+ sd_state = v4l2_subdev_lock_and_get_active_state(sd);
+
+ ret = rkisp1_config_cif(isp, sd_state, mbus_type, mbus_flags);
if (ret)
- goto mutex_unlock;
+ goto out_unlock;
- rkisp1_isp_start(isp);
+ rkisp1_isp_start(isp, sd_state);
ret = v4l2_subdev_call(rkisp1->source, video, s_stream, true);
if (ret) {
rkisp1_isp_stop(isp);
- goto mutex_unlock;
+ goto out_unlock;
}
-mutex_unlock:
- mutex_unlock(&isp->ops_lock);
+out_unlock:
+ v4l2_subdev_unlock_state(sd_state);
return ret;
}
@@ -929,9 +877,6 @@ static const struct v4l2_subdev_ops rkisp1_isp_ops = {
int rkisp1_isp_register(struct rkisp1_device *rkisp1)
{
- struct v4l2_subdev_state state = {
- .pads = rkisp1->isp.pad_cfg
- };
struct rkisp1_isp *isp = &rkisp1->isp;
struct media_pad *pads = isp->pads;
struct v4l2_subdev *sd = &isp->sd;
@@ -952,27 +897,26 @@ int rkisp1_isp_register(struct rkisp1_device *rkisp1)
pads[RKISP1_ISP_PAD_SOURCE_VIDEO].flags = MEDIA_PAD_FL_SOURCE;
pads[RKISP1_ISP_PAD_SOURCE_STATS].flags = MEDIA_PAD_FL_SOURCE;
- isp->sink_fmt = rkisp1_mbus_info_get_by_code(RKISP1_DEF_SINK_PAD_FMT);
- isp->src_fmt = rkisp1_mbus_info_get_by_code(RKISP1_DEF_SRC_PAD_FMT);
-
- mutex_init(&isp->ops_lock);
ret = media_entity_pads_init(&sd->entity, RKISP1_ISP_PAD_MAX, pads);
if (ret)
- goto error;
+ goto err_entity_cleanup;
+
+ ret = v4l2_subdev_init_finalize(sd);
+ if (ret)
+ goto err_subdev_cleanup;
ret = v4l2_device_register_subdev(&rkisp1->v4l2_dev, sd);
if (ret) {
dev_err(rkisp1->dev, "Failed to register isp subdev\n");
- goto error;
+ goto err_subdev_cleanup;
}
- rkisp1_isp_init_config(sd, &state);
-
return 0;
-error:
+err_subdev_cleanup:
+ v4l2_subdev_cleanup(sd);
+err_entity_cleanup:
media_entity_cleanup(&sd->entity);
- mutex_destroy(&isp->ops_lock);
isp->sd.v4l2_dev = NULL;
return ret;
}
@@ -986,7 +930,6 @@ void rkisp1_isp_unregister(struct rkisp1_device *rkisp1)
v4l2_device_unregister_subdev(&isp->sd);
media_entity_cleanup(&isp->sd.entity);
- mutex_destroy(&isp->ops_lock);
}
/* ----------------------------------------------------------------------------
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v1 2/3] media: rkisp1: isp: Use V4L2 subdev active state
@ 2023-01-27 0:31 ` Laurent Pinchart
0 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2023-01-27 0:31 UTC (permalink / raw)
To: linux-media; +Cc: Dafna Hirschfeld, Paul Elder, Heiko Stuebner, linux-rockchip
Use the V4L2 subdev active state API to store the active format and crop
rectangle. This simplifies the driver not only by dropping the state
stored in the rkisp1_isp structure, but also by replacing the ops_lock
with the state lock.
The rkisp1_isp.sink_fmt field needs to be kept, as it is accessed from
the stats interrupt handler. To simplifye the rkisp1_isp_set_sink_fmt()
implementation, the field is now set when starting the ISP, instead of
when setting the format.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
.../platform/rockchip/rkisp1/rkisp1-common.h | 6 -
.../platform/rockchip/rkisp1/rkisp1-isp.c | 261 +++++++-----------
2 files changed, 102 insertions(+), 165 deletions(-)
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
index e9626e59e1c8..d43ff1b549d9 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
@@ -190,20 +190,14 @@ struct rkisp1_csi {
* @sd: v4l2_subdev variable
* @rkisp1: pointer to rkisp1_device
* @pads: media pads
- * @pad_cfg: pads configurations
* @sink_fmt: input format
- * @src_fmt: output format
- * @ops_lock: ops serialization
* @frame_sequence: used to synchronize frame_id between video devices.
*/
struct rkisp1_isp {
struct v4l2_subdev sd;
struct rkisp1_device *rkisp1;
struct media_pad pads[RKISP1_ISP_PAD_MAX];
- struct v4l2_subdev_pad_config pad_cfg[RKISP1_ISP_PAD_MAX];
const struct rkisp1_mbus_info *sink_fmt;
- const struct rkisp1_mbus_info *src_fmt;
- struct mutex ops_lock; /* serialize the subdevice ops */
__u32 frame_sequence;
};
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
index 585cf3f53469..4d4f7b59f848 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
@@ -53,40 +53,6 @@
* +---------------------------------------------------------+
*/
-/* ----------------------------------------------------------------------------
- * Helpers
- */
-
-static struct v4l2_mbus_framefmt *
-rkisp1_isp_get_pad_fmt(struct rkisp1_isp *isp,
- struct v4l2_subdev_state *sd_state,
- unsigned int pad, u32 which)
-{
- struct v4l2_subdev_state state = {
- .pads = isp->pad_cfg
- };
-
- if (which == V4L2_SUBDEV_FORMAT_TRY)
- return v4l2_subdev_get_try_format(&isp->sd, sd_state, pad);
- else
- return v4l2_subdev_get_try_format(&isp->sd, &state, pad);
-}
-
-static struct v4l2_rect *
-rkisp1_isp_get_pad_crop(struct rkisp1_isp *isp,
- struct v4l2_subdev_state *sd_state,
- unsigned int pad, u32 which)
-{
- struct v4l2_subdev_state state = {
- .pads = isp->pad_cfg
- };
-
- if (which == V4L2_SUBDEV_FORMAT_TRY)
- return v4l2_subdev_get_try_crop(&isp->sd, sd_state, pad);
- else
- return v4l2_subdev_get_try_crop(&isp->sd, &state, pad);
-}
-
/* ----------------------------------------------------------------------------
* Camera Interface registers configurations
*/
@@ -96,12 +62,12 @@ rkisp1_isp_get_pad_crop(struct rkisp1_isp *isp,
* This should only be called when configuring CIF
* or at the frame end interrupt
*/
-static void rkisp1_config_ism(struct rkisp1_isp *isp)
+static void rkisp1_config_ism(struct rkisp1_isp *isp,
+ struct v4l2_subdev_state *sd_state)
{
const struct v4l2_rect *src_crop =
- rkisp1_isp_get_pad_crop(isp, NULL,
- RKISP1_ISP_PAD_SOURCE_VIDEO,
- V4L2_SUBDEV_FORMAT_ACTIVE);
+ v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SOURCE_VIDEO);
struct rkisp1_device *rkisp1 = isp->rkisp1;
u32 val;
@@ -125,21 +91,26 @@ static void rkisp1_config_ism(struct rkisp1_isp *isp)
* configure ISP blocks with input format, size......
*/
static int rkisp1_config_isp(struct rkisp1_isp *isp,
+ struct v4l2_subdev_state *sd_state,
enum v4l2_mbus_type mbus_type, u32 mbus_flags)
{
struct rkisp1_device *rkisp1 = isp->rkisp1;
u32 isp_ctrl = 0, irq_mask = 0, acq_mult = 0, acq_prop = 0;
- const struct rkisp1_mbus_info *sink_fmt = isp->sink_fmt;
- const struct rkisp1_mbus_info *src_fmt = isp->src_fmt;
+ const struct rkisp1_mbus_info *sink_fmt;
+ const struct rkisp1_mbus_info *src_fmt;
+ const struct v4l2_mbus_framefmt *src_frm;
const struct v4l2_mbus_framefmt *sink_frm;
const struct v4l2_rect *sink_crop;
- sink_frm = rkisp1_isp_get_pad_fmt(isp, NULL,
- RKISP1_ISP_PAD_SINK_VIDEO,
- V4L2_SUBDEV_FORMAT_ACTIVE);
- sink_crop = rkisp1_isp_get_pad_crop(isp, NULL,
- RKISP1_ISP_PAD_SINK_VIDEO,
- V4L2_SUBDEV_FORMAT_ACTIVE);
+ sink_frm = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SINK_VIDEO);
+ sink_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SINK_VIDEO);
+ src_frm = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SOURCE_VIDEO);
+
+ sink_fmt = rkisp1_mbus_info_get_by_code(sink_frm->code);
+ src_fmt = rkisp1_mbus_info_get_by_code(src_frm->code);
if (sink_fmt->pixel_enc == V4L2_PIXEL_ENC_BAYER) {
acq_mult = 1;
@@ -230,14 +201,15 @@ static int rkisp1_config_isp(struct rkisp1_isp *isp,
} else {
struct v4l2_mbus_framefmt *src_frm;
- src_frm = rkisp1_isp_get_pad_fmt(isp, NULL,
- RKISP1_ISP_PAD_SOURCE_VIDEO,
- V4L2_SUBDEV_FORMAT_ACTIVE);
+ src_frm = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SOURCE_VIDEO);
rkisp1_params_pre_configure(&rkisp1->params, sink_fmt->bayer_pat,
src_frm->quantization,
src_frm->ycbcr_enc);
}
+ isp->sink_fmt = sink_fmt;
+
return 0;
}
@@ -258,16 +230,17 @@ static void rkisp1_config_path(struct rkisp1_isp *isp,
/* Hardware configure Entry */
static int rkisp1_config_cif(struct rkisp1_isp *isp,
+ struct v4l2_subdev_state *sd_state,
enum v4l2_mbus_type mbus_type, u32 mbus_flags)
{
int ret;
- ret = rkisp1_config_isp(isp, mbus_type, mbus_flags);
+ ret = rkisp1_config_isp(isp, sd_state, mbus_type, mbus_flags);
if (ret)
return ret;
rkisp1_config_path(isp, mbus_type);
- rkisp1_config_ism(isp);
+ rkisp1_config_ism(isp, sd_state);
return 0;
}
@@ -328,9 +301,12 @@ static void rkisp1_config_clk(struct rkisp1_isp *isp)
}
}
-static void rkisp1_isp_start(struct rkisp1_isp *isp)
+static void rkisp1_isp_start(struct rkisp1_isp *isp,
+ struct v4l2_subdev_state *sd_state)
{
struct rkisp1_device *rkisp1 = isp->rkisp1;
+ const struct v4l2_mbus_framefmt *src_fmt;
+ const struct rkisp1_mbus_info *src_info;
u32 val;
rkisp1_config_clk(isp);
@@ -342,7 +318,11 @@ static void rkisp1_isp_start(struct rkisp1_isp *isp)
RKISP1_CIF_ISP_CTRL_ISP_INFORM_ENABLE;
rkisp1_write(rkisp1, RKISP1_CIF_ISP_CTRL, val);
- if (isp->src_fmt->pixel_enc != V4L2_PIXEL_ENC_BAYER)
+ src_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SOURCE_VIDEO);
+ src_info = rkisp1_mbus_info_get_by_code(src_fmt->code);
+
+ if (src_info->pixel_enc != V4L2_PIXEL_ENC_BAYER)
rkisp1_params_post_configure(&rkisp1->params);
}
@@ -436,7 +416,7 @@ static int rkisp1_isp_init_config(struct v4l2_subdev *sd,
struct v4l2_rect *sink_crop, *src_crop;
/* Video. */
- sink_fmt = v4l2_subdev_get_try_format(sd, sd_state,
+ sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
RKISP1_ISP_PAD_SINK_VIDEO);
sink_fmt->width = RKISP1_DEFAULT_WIDTH;
sink_fmt->height = RKISP1_DEFAULT_HEIGHT;
@@ -447,14 +427,14 @@ static int rkisp1_isp_init_config(struct v4l2_subdev *sd,
sink_fmt->ycbcr_enc = V4L2_YCBCR_ENC_601;
sink_fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
- sink_crop = v4l2_subdev_get_try_crop(sd, sd_state,
+ sink_crop = v4l2_subdev_get_pad_crop(sd, sd_state,
RKISP1_ISP_PAD_SINK_VIDEO);
sink_crop->width = RKISP1_DEFAULT_WIDTH;
sink_crop->height = RKISP1_DEFAULT_HEIGHT;
sink_crop->left = 0;
sink_crop->top = 0;
- src_fmt = v4l2_subdev_get_try_format(sd, sd_state,
+ src_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
RKISP1_ISP_PAD_SOURCE_VIDEO);
*src_fmt = *sink_fmt;
src_fmt->code = RKISP1_DEF_SRC_PAD_FMT;
@@ -463,14 +443,14 @@ static int rkisp1_isp_init_config(struct v4l2_subdev *sd,
src_fmt->ycbcr_enc = V4L2_YCBCR_ENC_601;
src_fmt->quantization = V4L2_QUANTIZATION_LIM_RANGE;
- src_crop = v4l2_subdev_get_try_crop(sd, sd_state,
+ src_crop = v4l2_subdev_get_pad_crop(sd, sd_state,
RKISP1_ISP_PAD_SOURCE_VIDEO);
*src_crop = *sink_crop;
/* Parameters and statistics. */
- sink_fmt = v4l2_subdev_get_try_format(sd, sd_state,
+ sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
RKISP1_ISP_PAD_SINK_PARAMS);
- src_fmt = v4l2_subdev_get_try_format(sd, sd_state,
+ src_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
RKISP1_ISP_PAD_SOURCE_STATS);
sink_fmt->width = 0;
sink_fmt->height = 0;
@@ -483,8 +463,7 @@ static int rkisp1_isp_init_config(struct v4l2_subdev *sd,
static void rkisp1_isp_set_src_fmt(struct rkisp1_isp *isp,
struct v4l2_subdev_state *sd_state,
- struct v4l2_mbus_framefmt *format,
- unsigned int which)
+ struct v4l2_mbus_framefmt *format)
{
const struct rkisp1_mbus_info *sink_info;
const struct rkisp1_mbus_info *src_info;
@@ -493,12 +472,12 @@ static void rkisp1_isp_set_src_fmt(struct rkisp1_isp *isp,
const struct v4l2_rect *src_crop;
bool set_csc;
- sink_fmt = rkisp1_isp_get_pad_fmt(isp, sd_state,
- RKISP1_ISP_PAD_SINK_VIDEO, which);
- src_fmt = rkisp1_isp_get_pad_fmt(isp, sd_state,
- RKISP1_ISP_PAD_SOURCE_VIDEO, which);
- src_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
- RKISP1_ISP_PAD_SOURCE_VIDEO, which);
+ sink_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SINK_VIDEO);
+ src_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SOURCE_VIDEO);
+ src_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SOURCE_VIDEO);
/*
* Media bus code. The ISP can operate in pass-through mode (Bayer in,
@@ -581,26 +560,20 @@ static void rkisp1_isp_set_src_fmt(struct rkisp1_isp *isp,
*/
if (set_csc)
format->flags |= V4L2_MBUS_FRAMEFMT_SET_CSC;
-
- /* Store the source format info when setting the active format. */
- if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
- isp->src_fmt = src_info;
}
static void rkisp1_isp_set_src_crop(struct rkisp1_isp *isp,
struct v4l2_subdev_state *sd_state,
- struct v4l2_rect *r, unsigned int which)
+ struct v4l2_rect *r)
{
struct v4l2_mbus_framefmt *src_fmt;
const struct v4l2_rect *sink_crop;
struct v4l2_rect *src_crop;
- src_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
- RKISP1_ISP_PAD_SOURCE_VIDEO,
- which);
- sink_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
- RKISP1_ISP_PAD_SINK_VIDEO,
- which);
+ src_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SOURCE_VIDEO);
+ sink_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SINK_VIDEO);
src_crop->left = ALIGN(r->left, 2);
src_crop->width = ALIGN(r->width, 2);
@@ -611,24 +584,22 @@ static void rkisp1_isp_set_src_crop(struct rkisp1_isp *isp,
*r = *src_crop;
/* Propagate to out format */
- src_fmt = rkisp1_isp_get_pad_fmt(isp, sd_state,
- RKISP1_ISP_PAD_SOURCE_VIDEO, which);
- rkisp1_isp_set_src_fmt(isp, sd_state, src_fmt, which);
+ src_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SOURCE_VIDEO);
+ rkisp1_isp_set_src_fmt(isp, sd_state, src_fmt);
}
static void rkisp1_isp_set_sink_crop(struct rkisp1_isp *isp,
struct v4l2_subdev_state *sd_state,
- struct v4l2_rect *r, unsigned int which)
+ struct v4l2_rect *r)
{
struct v4l2_rect *sink_crop, *src_crop;
const struct v4l2_mbus_framefmt *sink_fmt;
- sink_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
- RKISP1_ISP_PAD_SINK_VIDEO,
- which);
- sink_fmt = rkisp1_isp_get_pad_fmt(isp, sd_state,
- RKISP1_ISP_PAD_SINK_VIDEO,
- which);
+ sink_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SINK_VIDEO);
+ sink_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SINK_VIDEO);
sink_crop->left = ALIGN(r->left, 2);
sink_crop->width = ALIGN(r->width, 2);
@@ -639,32 +610,28 @@ static void rkisp1_isp_set_sink_crop(struct rkisp1_isp *isp,
*r = *sink_crop;
/* Propagate to out crop */
- src_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
- RKISP1_ISP_PAD_SOURCE_VIDEO, which);
- rkisp1_isp_set_src_crop(isp, sd_state, src_crop, which);
+ src_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SOURCE_VIDEO);
+ rkisp1_isp_set_src_crop(isp, sd_state, src_crop);
}
static void rkisp1_isp_set_sink_fmt(struct rkisp1_isp *isp,
struct v4l2_subdev_state *sd_state,
- struct v4l2_mbus_framefmt *format,
- unsigned int which)
+ struct v4l2_mbus_framefmt *format)
{
const struct rkisp1_mbus_info *mbus_info;
struct v4l2_mbus_framefmt *sink_fmt;
struct v4l2_rect *sink_crop;
bool is_yuv;
- sink_fmt = rkisp1_isp_get_pad_fmt(isp, sd_state,
- RKISP1_ISP_PAD_SINK_VIDEO,
- which);
+ sink_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SINK_VIDEO);
sink_fmt->code = format->code;
mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code);
if (!mbus_info || !(mbus_info->direction & RKISP1_ISP_SD_SINK)) {
sink_fmt->code = RKISP1_DEF_SINK_PAD_FMT;
mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code);
}
- if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
- isp->sink_fmt = mbus_info;
sink_fmt->width = clamp_t(u32, format->width,
RKISP1_ISP_MIN_WIDTH,
@@ -706,23 +673,9 @@ static void rkisp1_isp_set_sink_fmt(struct rkisp1_isp *isp,
*format = *sink_fmt;
/* Propagate to in crop */
- sink_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
- RKISP1_ISP_PAD_SINK_VIDEO,
- which);
- rkisp1_isp_set_sink_crop(isp, sd_state, sink_crop, which);
-}
-
-static int rkisp1_isp_get_fmt(struct v4l2_subdev *sd,
- struct v4l2_subdev_state *sd_state,
- struct v4l2_subdev_format *fmt)
-{
- struct rkisp1_isp *isp = to_rkisp1_isp(sd);
-
- mutex_lock(&isp->ops_lock);
- fmt->format = *rkisp1_isp_get_pad_fmt(isp, sd_state, fmt->pad,
- fmt->which);
- mutex_unlock(&isp->ops_lock);
- return 0;
+ sink_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
+ RKISP1_ISP_PAD_SINK_VIDEO);
+ rkisp1_isp_set_sink_crop(isp, sd_state, sink_crop);
}
static int rkisp1_isp_set_fmt(struct v4l2_subdev *sd,
@@ -731,18 +684,13 @@ static int rkisp1_isp_set_fmt(struct v4l2_subdev *sd,
{
struct rkisp1_isp *isp = to_rkisp1_isp(sd);
- mutex_lock(&isp->ops_lock);
if (fmt->pad == RKISP1_ISP_PAD_SINK_VIDEO)
- rkisp1_isp_set_sink_fmt(isp, sd_state, &fmt->format,
- fmt->which);
+ rkisp1_isp_set_sink_fmt(isp, sd_state, &fmt->format);
else if (fmt->pad == RKISP1_ISP_PAD_SOURCE_VIDEO)
- rkisp1_isp_set_src_fmt(isp, sd_state, &fmt->format,
- fmt->which);
+ rkisp1_isp_set_src_fmt(isp, sd_state, &fmt->format);
else
- fmt->format = *rkisp1_isp_get_pad_fmt(isp, sd_state, fmt->pad,
- fmt->which);
+ fmt->format = *v4l2_subdev_get_pad_format(sd, sd_state, fmt->pad);
- mutex_unlock(&isp->ops_lock);
return 0;
}
@@ -750,39 +698,37 @@ static int rkisp1_isp_get_selection(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state,
struct v4l2_subdev_selection *sel)
{
- struct rkisp1_isp *isp = to_rkisp1_isp(sd);
int ret = 0;
if (sel->pad != RKISP1_ISP_PAD_SOURCE_VIDEO &&
sel->pad != RKISP1_ISP_PAD_SINK_VIDEO)
return -EINVAL;
- mutex_lock(&isp->ops_lock);
switch (sel->target) {
case V4L2_SEL_TGT_CROP_BOUNDS:
if (sel->pad == RKISP1_ISP_PAD_SINK_VIDEO) {
struct v4l2_mbus_framefmt *fmt;
- fmt = rkisp1_isp_get_pad_fmt(isp, sd_state, sel->pad,
- sel->which);
+ fmt = v4l2_subdev_get_pad_format(sd, sd_state, sel->pad);
sel->r.height = fmt->height;
sel->r.width = fmt->width;
sel->r.left = 0;
sel->r.top = 0;
} else {
- sel->r = *rkisp1_isp_get_pad_crop(isp, sd_state,
- RKISP1_ISP_PAD_SINK_VIDEO,
- sel->which);
+ sel->r = *v4l2_subdev_get_pad_crop(sd, sd_state,
+ RKISP1_ISP_PAD_SINK_VIDEO);
}
break;
+
case V4L2_SEL_TGT_CROP:
- sel->r = *rkisp1_isp_get_pad_crop(isp, sd_state, sel->pad,
- sel->which);
+ sel->r = *v4l2_subdev_get_pad_crop(sd, sd_state, sel->pad);
break;
+
default:
ret = -EINVAL;
+ break;
}
- mutex_unlock(&isp->ops_lock);
+
return ret;
}
@@ -798,15 +744,14 @@ static int rkisp1_isp_set_selection(struct v4l2_subdev *sd,
dev_dbg(isp->rkisp1->dev, "%s: pad: %d sel(%d,%d)/%dx%d\n", __func__,
sel->pad, sel->r.left, sel->r.top, sel->r.width, sel->r.height);
- mutex_lock(&isp->ops_lock);
+
if (sel->pad == RKISP1_ISP_PAD_SINK_VIDEO)
- rkisp1_isp_set_sink_crop(isp, sd_state, &sel->r, sel->which);
+ rkisp1_isp_set_sink_crop(isp, sd_state, &sel->r);
else if (sel->pad == RKISP1_ISP_PAD_SOURCE_VIDEO)
- rkisp1_isp_set_src_crop(isp, sd_state, &sel->r, sel->which);
+ rkisp1_isp_set_src_crop(isp, sd_state, &sel->r);
else
ret = -EINVAL;
- mutex_unlock(&isp->ops_lock);
return ret;
}
@@ -824,7 +769,7 @@ static const struct v4l2_subdev_pad_ops rkisp1_isp_pad_ops = {
.get_selection = rkisp1_isp_get_selection,
.set_selection = rkisp1_isp_set_selection,
.init_cfg = rkisp1_isp_init_config,
- .get_fmt = rkisp1_isp_get_fmt,
+ .get_fmt = v4l2_subdev_get_fmt,
.set_fmt = rkisp1_isp_set_fmt,
.link_validate = v4l2_subdev_link_validate_default,
};
@@ -837,6 +782,7 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
{
struct rkisp1_isp *isp = to_rkisp1_isp(sd);
struct rkisp1_device *rkisp1 = isp->rkisp1;
+ struct v4l2_subdev_state *sd_state;
struct media_pad *source_pad;
struct media_pad *sink_pad;
enum v4l2_mbus_type mbus_type;
@@ -877,21 +823,23 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
}
isp->frame_sequence = -1;
- mutex_lock(&isp->ops_lock);
- ret = rkisp1_config_cif(isp, mbus_type, mbus_flags);
+
+ sd_state = v4l2_subdev_lock_and_get_active_state(sd);
+
+ ret = rkisp1_config_cif(isp, sd_state, mbus_type, mbus_flags);
if (ret)
- goto mutex_unlock;
+ goto out_unlock;
- rkisp1_isp_start(isp);
+ rkisp1_isp_start(isp, sd_state);
ret = v4l2_subdev_call(rkisp1->source, video, s_stream, true);
if (ret) {
rkisp1_isp_stop(isp);
- goto mutex_unlock;
+ goto out_unlock;
}
-mutex_unlock:
- mutex_unlock(&isp->ops_lock);
+out_unlock:
+ v4l2_subdev_unlock_state(sd_state);
return ret;
}
@@ -929,9 +877,6 @@ static const struct v4l2_subdev_ops rkisp1_isp_ops = {
int rkisp1_isp_register(struct rkisp1_device *rkisp1)
{
- struct v4l2_subdev_state state = {
- .pads = rkisp1->isp.pad_cfg
- };
struct rkisp1_isp *isp = &rkisp1->isp;
struct media_pad *pads = isp->pads;
struct v4l2_subdev *sd = &isp->sd;
@@ -952,27 +897,26 @@ int rkisp1_isp_register(struct rkisp1_device *rkisp1)
pads[RKISP1_ISP_PAD_SOURCE_VIDEO].flags = MEDIA_PAD_FL_SOURCE;
pads[RKISP1_ISP_PAD_SOURCE_STATS].flags = MEDIA_PAD_FL_SOURCE;
- isp->sink_fmt = rkisp1_mbus_info_get_by_code(RKISP1_DEF_SINK_PAD_FMT);
- isp->src_fmt = rkisp1_mbus_info_get_by_code(RKISP1_DEF_SRC_PAD_FMT);
-
- mutex_init(&isp->ops_lock);
ret = media_entity_pads_init(&sd->entity, RKISP1_ISP_PAD_MAX, pads);
if (ret)
- goto error;
+ goto err_entity_cleanup;
+
+ ret = v4l2_subdev_init_finalize(sd);
+ if (ret)
+ goto err_subdev_cleanup;
ret = v4l2_device_register_subdev(&rkisp1->v4l2_dev, sd);
if (ret) {
dev_err(rkisp1->dev, "Failed to register isp subdev\n");
- goto error;
+ goto err_subdev_cleanup;
}
- rkisp1_isp_init_config(sd, &state);
-
return 0;
-error:
+err_subdev_cleanup:
+ v4l2_subdev_cleanup(sd);
+err_entity_cleanup:
media_entity_cleanup(&sd->entity);
- mutex_destroy(&isp->ops_lock);
isp->sd.v4l2_dev = NULL;
return ret;
}
@@ -986,7 +930,6 @@ void rkisp1_isp_unregister(struct rkisp1_device *rkisp1)
v4l2_device_unregister_subdev(&isp->sd);
media_entity_cleanup(&isp->sd.entity);
- mutex_destroy(&isp->ops_lock);
}
/* ----------------------------------------------------------------------------
--
Regards,
Laurent Pinchart
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v1 3/3] media: rkisp1: csi: Use V4L2 subdev active state
2023-01-27 0:31 ` Laurent Pinchart
@ 2023-01-27 0:31 ` Laurent Pinchart
-1 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2023-01-27 0:31 UTC (permalink / raw)
To: linux-media; +Cc: Dafna Hirschfeld, Paul Elder, Heiko Stuebner, linux-rockchip
Use the V4L2 subdev active state API to store the active format and crop
rectangle. This simplifies the driver not only by dropping the state
stored in the rkisp1_csi structure, but also by replacing the ops_lock
with the state lock.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
.../platform/rockchip/rkisp1/rkisp1-common.h | 6 -
.../platform/rockchip/rkisp1/rkisp1-csi.c | 107 ++++++------------
2 files changed, 33 insertions(+), 80 deletions(-)
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
index d43ff1b549d9..dbd8725f2ec9 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
@@ -167,9 +167,6 @@ struct rkisp1_sensor_async {
* @is_dphy_errctrl_disabled: if dphy errctrl is disabled (avoid endless interrupt)
* @sd: v4l2_subdev variable
* @pads: media pads
- * @pad_cfg: configurations for the pads
- * @sink_fmt: input format
- * @lock: protects pad_cfg and sink_fmt
* @source: source in-use, set when starting streaming
*/
struct rkisp1_csi {
@@ -178,9 +175,6 @@ struct rkisp1_csi {
bool is_dphy_errctrl_disabled;
struct v4l2_subdev sd;
struct media_pad pads[RKISP1_CSI_PAD_NUM];
- struct v4l2_subdev_pad_config pad_cfg[RKISP1_CSI_PAD_NUM];
- const struct rkisp1_mbus_info *sink_fmt;
- struct mutex lock;
struct v4l2_subdev *source;
};
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c
index d7acc94e10f8..d8d50270f6db 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c
@@ -30,23 +30,6 @@ static inline struct rkisp1_csi *to_rkisp1_csi(struct v4l2_subdev *sd)
return container_of(sd, struct rkisp1_csi, sd);
}
-static struct v4l2_mbus_framefmt *
-rkisp1_csi_get_pad_fmt(struct rkisp1_csi *csi,
- struct v4l2_subdev_state *sd_state,
- unsigned int pad, u32 which)
-{
- struct v4l2_subdev_state state = {
- .pads = csi->pad_cfg
- };
-
- lockdep_assert_held(&csi->lock);
-
- if (which == V4L2_SUBDEV_FORMAT_TRY)
- return v4l2_subdev_get_try_format(&csi->sd, sd_state, pad);
- else
- return v4l2_subdev_get_try_format(&csi->sd, &state, pad);
-}
-
int rkisp1_csi_link_sensor(struct rkisp1_device *rkisp1, struct v4l2_subdev *sd,
struct rkisp1_sensor_async *s_asd,
unsigned int source_pad)
@@ -76,7 +59,8 @@ int rkisp1_csi_link_sensor(struct rkisp1_device *rkisp1, struct v4l2_subdev *sd,
}
static int rkisp1_csi_config(struct rkisp1_csi *csi,
- const struct rkisp1_sensor_async *sensor)
+ const struct rkisp1_sensor_async *sensor,
+ const struct rkisp1_mbus_info *format)
{
struct rkisp1_device *rkisp1 = csi->rkisp1;
unsigned int lanes = sensor->lanes;
@@ -98,7 +82,7 @@ static int rkisp1_csi_config(struct rkisp1_csi *csi,
/* Configure Data Type and Virtual Channel */
rkisp1_write(rkisp1, RKISP1_CIF_MIPI_IMG_DATA_SEL,
- RKISP1_CIF_MIPI_DATA_SEL_DT(csi->sink_fmt->mipi_dt) |
+ RKISP1_CIF_MIPI_DATA_SEL_DT(format->mipi_dt) |
RKISP1_CIF_MIPI_DATA_SEL_VC(0));
/* Clear MIPI interrupts */
@@ -151,7 +135,8 @@ static void rkisp1_csi_disable(struct rkisp1_csi *csi)
}
static int rkisp1_csi_start(struct rkisp1_csi *csi,
- const struct rkisp1_sensor_async *sensor)
+ const struct rkisp1_sensor_async *sensor,
+ const struct rkisp1_mbus_info *format)
{
struct rkisp1_device *rkisp1 = csi->rkisp1;
union phy_configure_opts opts;
@@ -159,7 +144,7 @@ static int rkisp1_csi_start(struct rkisp1_csi *csi,
s64 pixel_clock;
int ret;
- ret = rkisp1_csi_config(csi, sensor);
+ ret = rkisp1_csi_config(csi, sensor, format);
if (ret)
return ret;
@@ -169,7 +154,7 @@ static int rkisp1_csi_start(struct rkisp1_csi *csi,
return -EINVAL;
}
- phy_mipi_dphy_get_default_config(pixel_clock, csi->sink_fmt->bus_width,
+ phy_mipi_dphy_get_default_config(pixel_clock, format->bus_width,
sensor->lanes, cfg);
phy_set_mode(csi->dphy, PHY_MODE_MIPI_DPHY);
phy_configure(csi->dphy, &opts);
@@ -248,7 +233,6 @@ static int rkisp1_csi_enum_mbus_code(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state,
struct v4l2_subdev_mbus_code_enum *code)
{
- struct rkisp1_csi *csi = to_rkisp1_csi(sd);
unsigned int i;
int pos = 0;
@@ -258,15 +242,10 @@ static int rkisp1_csi_enum_mbus_code(struct v4l2_subdev *sd,
if (code->index)
return -EINVAL;
- mutex_lock(&csi->lock);
-
- sink_fmt = rkisp1_csi_get_pad_fmt(csi, sd_state,
- RKISP1_CSI_PAD_SINK,
- code->which);
+ sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
+ RKISP1_CSI_PAD_SINK);
code->code = sink_fmt->code;
- mutex_unlock(&csi->lock);
-
return 0;
}
@@ -296,9 +275,9 @@ static int rkisp1_csi_init_config(struct v4l2_subdev *sd,
{
struct v4l2_mbus_framefmt *sink_fmt, *src_fmt;
- sink_fmt = v4l2_subdev_get_try_format(sd, sd_state,
+ sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
RKISP1_CSI_PAD_SINK);
- src_fmt = v4l2_subdev_get_try_format(sd, sd_state,
+ src_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
RKISP1_CSI_PAD_SRC);
sink_fmt->width = RKISP1_DEFAULT_WIDTH;
@@ -311,36 +290,18 @@ static int rkisp1_csi_init_config(struct v4l2_subdev *sd,
return 0;
}
-static int rkisp1_csi_get_fmt(struct v4l2_subdev *sd,
- struct v4l2_subdev_state *sd_state,
- struct v4l2_subdev_format *fmt)
-{
- struct rkisp1_csi *csi = to_rkisp1_csi(sd);
-
- mutex_lock(&csi->lock);
- fmt->format = *rkisp1_csi_get_pad_fmt(csi, sd_state, fmt->pad,
- fmt->which);
- mutex_unlock(&csi->lock);
-
- return 0;
-}
-
static int rkisp1_csi_set_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state,
struct v4l2_subdev_format *fmt)
{
- struct rkisp1_csi *csi = to_rkisp1_csi(sd);
const struct rkisp1_mbus_info *mbus_info;
struct v4l2_mbus_framefmt *sink_fmt, *src_fmt;
/* The format on the source pad always matches the sink pad. */
if (fmt->pad == RKISP1_CSI_PAD_SRC)
- return rkisp1_csi_get_fmt(sd, sd_state, fmt);
+ return v4l2_subdev_get_fmt(sd, sd_state, fmt);
- mutex_lock(&csi->lock);
-
- sink_fmt = rkisp1_csi_get_pad_fmt(csi, sd_state, RKISP1_CSI_PAD_SINK,
- fmt->which);
+ sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state, RKISP1_CSI_PAD_SINK);
sink_fmt->code = fmt->format.code;
@@ -359,16 +320,10 @@ static int rkisp1_csi_set_fmt(struct v4l2_subdev *sd,
fmt->format = *sink_fmt;
- if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE)
- csi->sink_fmt = mbus_info;
-
/* Propagate the format to the source pad. */
- src_fmt = rkisp1_csi_get_pad_fmt(csi, sd_state, RKISP1_CSI_PAD_SRC,
- fmt->which);
+ src_fmt = v4l2_subdev_get_pad_format(sd, sd_state, RKISP1_CSI_PAD_SRC);
*src_fmt = *sink_fmt;
- mutex_unlock(&csi->lock);
-
return 0;
}
@@ -380,7 +335,10 @@ static int rkisp1_csi_s_stream(struct v4l2_subdev *sd, int enable)
{
struct rkisp1_csi *csi = to_rkisp1_csi(sd);
struct rkisp1_device *rkisp1 = csi->rkisp1;
+ const struct v4l2_mbus_framefmt *sink_fmt;
+ const struct rkisp1_mbus_info *format;
struct rkisp1_sensor_async *source_asd;
+ struct v4l2_subdev_state *sd_state;
struct media_pad *source_pad;
struct v4l2_subdev *source;
int ret;
@@ -410,9 +368,12 @@ static int rkisp1_csi_s_stream(struct v4l2_subdev *sd, int enable)
if (source_asd->mbus_type != V4L2_MBUS_CSI2_DPHY)
return -EINVAL;
- mutex_lock(&csi->lock);
- ret = rkisp1_csi_start(csi, source_asd);
- mutex_unlock(&csi->lock);
+ sd_state = v4l2_subdev_lock_and_get_active_state(sd);
+ sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state, RKISP1_CSI_PAD_SINK);
+ format = rkisp1_mbus_info_get_by_code(sink_fmt->code);
+ v4l2_subdev_unlock_state(sd_state);
+
+ ret = rkisp1_csi_start(csi, source_asd, format);
if (ret)
return ret;
@@ -442,7 +403,7 @@ static const struct v4l2_subdev_video_ops rkisp1_csi_video_ops = {
static const struct v4l2_subdev_pad_ops rkisp1_csi_pad_ops = {
.enum_mbus_code = rkisp1_csi_enum_mbus_code,
.init_cfg = rkisp1_csi_init_config,
- .get_fmt = rkisp1_csi_get_fmt,
+ .get_fmt = v4l2_subdev_get_fmt,
.set_fmt = rkisp1_csi_set_fmt,
};
@@ -454,13 +415,11 @@ static const struct v4l2_subdev_ops rkisp1_csi_ops = {
int rkisp1_csi_register(struct rkisp1_device *rkisp1)
{
struct rkisp1_csi *csi = &rkisp1->csi;
- struct v4l2_subdev_state state = {};
struct media_pad *pads;
struct v4l2_subdev *sd;
int ret;
csi->rkisp1 = rkisp1;
- mutex_init(&csi->lock);
sd = &csi->sd;
v4l2_subdev_init(sd, &rkisp1_csi_ops);
@@ -476,26 +435,26 @@ int rkisp1_csi_register(struct rkisp1_device *rkisp1)
pads[RKISP1_CSI_PAD_SRC].flags = MEDIA_PAD_FL_SOURCE |
MEDIA_PAD_FL_MUST_CONNECT;
- csi->sink_fmt = rkisp1_mbus_info_get_by_code(RKISP1_CSI_DEF_FMT);
-
ret = media_entity_pads_init(&sd->entity, RKISP1_CSI_PAD_NUM, pads);
if (ret)
- goto error;
+ goto err_entity_cleanup;
- state.pads = csi->pad_cfg;
- rkisp1_csi_init_config(sd, &state);
+ ret = v4l2_subdev_init_finalize(sd);
+ if (ret)
+ goto err_entity_cleanup;
ret = v4l2_device_register_subdev(&csi->rkisp1->v4l2_dev, sd);
if (ret) {
dev_err(sd->dev, "Failed to register csi receiver subdev\n");
- goto error;
+ goto err_subdev_cleanup;
}
return 0;
-error:
+err_subdev_cleanup:
+ v4l2_subdev_cleanup(sd);
+err_entity_cleanup:
media_entity_cleanup(&sd->entity);
- mutex_destroy(&csi->lock);
csi->rkisp1 = NULL;
return ret;
}
@@ -508,8 +467,8 @@ void rkisp1_csi_unregister(struct rkisp1_device *rkisp1)
return;
v4l2_device_unregister_subdev(&csi->sd);
+ v4l2_subdev_cleanup(&csi->sd);
media_entity_cleanup(&csi->sd.entity);
- mutex_destroy(&csi->lock);
}
int rkisp1_csi_init(struct rkisp1_device *rkisp1)
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v1 3/3] media: rkisp1: csi: Use V4L2 subdev active state
@ 2023-01-27 0:31 ` Laurent Pinchart
0 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2023-01-27 0:31 UTC (permalink / raw)
To: linux-media; +Cc: Dafna Hirschfeld, Paul Elder, Heiko Stuebner, linux-rockchip
Use the V4L2 subdev active state API to store the active format and crop
rectangle. This simplifies the driver not only by dropping the state
stored in the rkisp1_csi structure, but also by replacing the ops_lock
with the state lock.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
.../platform/rockchip/rkisp1/rkisp1-common.h | 6 -
.../platform/rockchip/rkisp1/rkisp1-csi.c | 107 ++++++------------
2 files changed, 33 insertions(+), 80 deletions(-)
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
index d43ff1b549d9..dbd8725f2ec9 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
@@ -167,9 +167,6 @@ struct rkisp1_sensor_async {
* @is_dphy_errctrl_disabled: if dphy errctrl is disabled (avoid endless interrupt)
* @sd: v4l2_subdev variable
* @pads: media pads
- * @pad_cfg: configurations for the pads
- * @sink_fmt: input format
- * @lock: protects pad_cfg and sink_fmt
* @source: source in-use, set when starting streaming
*/
struct rkisp1_csi {
@@ -178,9 +175,6 @@ struct rkisp1_csi {
bool is_dphy_errctrl_disabled;
struct v4l2_subdev sd;
struct media_pad pads[RKISP1_CSI_PAD_NUM];
- struct v4l2_subdev_pad_config pad_cfg[RKISP1_CSI_PAD_NUM];
- const struct rkisp1_mbus_info *sink_fmt;
- struct mutex lock;
struct v4l2_subdev *source;
};
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c
index d7acc94e10f8..d8d50270f6db 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c
@@ -30,23 +30,6 @@ static inline struct rkisp1_csi *to_rkisp1_csi(struct v4l2_subdev *sd)
return container_of(sd, struct rkisp1_csi, sd);
}
-static struct v4l2_mbus_framefmt *
-rkisp1_csi_get_pad_fmt(struct rkisp1_csi *csi,
- struct v4l2_subdev_state *sd_state,
- unsigned int pad, u32 which)
-{
- struct v4l2_subdev_state state = {
- .pads = csi->pad_cfg
- };
-
- lockdep_assert_held(&csi->lock);
-
- if (which == V4L2_SUBDEV_FORMAT_TRY)
- return v4l2_subdev_get_try_format(&csi->sd, sd_state, pad);
- else
- return v4l2_subdev_get_try_format(&csi->sd, &state, pad);
-}
-
int rkisp1_csi_link_sensor(struct rkisp1_device *rkisp1, struct v4l2_subdev *sd,
struct rkisp1_sensor_async *s_asd,
unsigned int source_pad)
@@ -76,7 +59,8 @@ int rkisp1_csi_link_sensor(struct rkisp1_device *rkisp1, struct v4l2_subdev *sd,
}
static int rkisp1_csi_config(struct rkisp1_csi *csi,
- const struct rkisp1_sensor_async *sensor)
+ const struct rkisp1_sensor_async *sensor,
+ const struct rkisp1_mbus_info *format)
{
struct rkisp1_device *rkisp1 = csi->rkisp1;
unsigned int lanes = sensor->lanes;
@@ -98,7 +82,7 @@ static int rkisp1_csi_config(struct rkisp1_csi *csi,
/* Configure Data Type and Virtual Channel */
rkisp1_write(rkisp1, RKISP1_CIF_MIPI_IMG_DATA_SEL,
- RKISP1_CIF_MIPI_DATA_SEL_DT(csi->sink_fmt->mipi_dt) |
+ RKISP1_CIF_MIPI_DATA_SEL_DT(format->mipi_dt) |
RKISP1_CIF_MIPI_DATA_SEL_VC(0));
/* Clear MIPI interrupts */
@@ -151,7 +135,8 @@ static void rkisp1_csi_disable(struct rkisp1_csi *csi)
}
static int rkisp1_csi_start(struct rkisp1_csi *csi,
- const struct rkisp1_sensor_async *sensor)
+ const struct rkisp1_sensor_async *sensor,
+ const struct rkisp1_mbus_info *format)
{
struct rkisp1_device *rkisp1 = csi->rkisp1;
union phy_configure_opts opts;
@@ -159,7 +144,7 @@ static int rkisp1_csi_start(struct rkisp1_csi *csi,
s64 pixel_clock;
int ret;
- ret = rkisp1_csi_config(csi, sensor);
+ ret = rkisp1_csi_config(csi, sensor, format);
if (ret)
return ret;
@@ -169,7 +154,7 @@ static int rkisp1_csi_start(struct rkisp1_csi *csi,
return -EINVAL;
}
- phy_mipi_dphy_get_default_config(pixel_clock, csi->sink_fmt->bus_width,
+ phy_mipi_dphy_get_default_config(pixel_clock, format->bus_width,
sensor->lanes, cfg);
phy_set_mode(csi->dphy, PHY_MODE_MIPI_DPHY);
phy_configure(csi->dphy, &opts);
@@ -248,7 +233,6 @@ static int rkisp1_csi_enum_mbus_code(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state,
struct v4l2_subdev_mbus_code_enum *code)
{
- struct rkisp1_csi *csi = to_rkisp1_csi(sd);
unsigned int i;
int pos = 0;
@@ -258,15 +242,10 @@ static int rkisp1_csi_enum_mbus_code(struct v4l2_subdev *sd,
if (code->index)
return -EINVAL;
- mutex_lock(&csi->lock);
-
- sink_fmt = rkisp1_csi_get_pad_fmt(csi, sd_state,
- RKISP1_CSI_PAD_SINK,
- code->which);
+ sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
+ RKISP1_CSI_PAD_SINK);
code->code = sink_fmt->code;
- mutex_unlock(&csi->lock);
-
return 0;
}
@@ -296,9 +275,9 @@ static int rkisp1_csi_init_config(struct v4l2_subdev *sd,
{
struct v4l2_mbus_framefmt *sink_fmt, *src_fmt;
- sink_fmt = v4l2_subdev_get_try_format(sd, sd_state,
+ sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
RKISP1_CSI_PAD_SINK);
- src_fmt = v4l2_subdev_get_try_format(sd, sd_state,
+ src_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
RKISP1_CSI_PAD_SRC);
sink_fmt->width = RKISP1_DEFAULT_WIDTH;
@@ -311,36 +290,18 @@ static int rkisp1_csi_init_config(struct v4l2_subdev *sd,
return 0;
}
-static int rkisp1_csi_get_fmt(struct v4l2_subdev *sd,
- struct v4l2_subdev_state *sd_state,
- struct v4l2_subdev_format *fmt)
-{
- struct rkisp1_csi *csi = to_rkisp1_csi(sd);
-
- mutex_lock(&csi->lock);
- fmt->format = *rkisp1_csi_get_pad_fmt(csi, sd_state, fmt->pad,
- fmt->which);
- mutex_unlock(&csi->lock);
-
- return 0;
-}
-
static int rkisp1_csi_set_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state,
struct v4l2_subdev_format *fmt)
{
- struct rkisp1_csi *csi = to_rkisp1_csi(sd);
const struct rkisp1_mbus_info *mbus_info;
struct v4l2_mbus_framefmt *sink_fmt, *src_fmt;
/* The format on the source pad always matches the sink pad. */
if (fmt->pad == RKISP1_CSI_PAD_SRC)
- return rkisp1_csi_get_fmt(sd, sd_state, fmt);
+ return v4l2_subdev_get_fmt(sd, sd_state, fmt);
- mutex_lock(&csi->lock);
-
- sink_fmt = rkisp1_csi_get_pad_fmt(csi, sd_state, RKISP1_CSI_PAD_SINK,
- fmt->which);
+ sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state, RKISP1_CSI_PAD_SINK);
sink_fmt->code = fmt->format.code;
@@ -359,16 +320,10 @@ static int rkisp1_csi_set_fmt(struct v4l2_subdev *sd,
fmt->format = *sink_fmt;
- if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE)
- csi->sink_fmt = mbus_info;
-
/* Propagate the format to the source pad. */
- src_fmt = rkisp1_csi_get_pad_fmt(csi, sd_state, RKISP1_CSI_PAD_SRC,
- fmt->which);
+ src_fmt = v4l2_subdev_get_pad_format(sd, sd_state, RKISP1_CSI_PAD_SRC);
*src_fmt = *sink_fmt;
- mutex_unlock(&csi->lock);
-
return 0;
}
@@ -380,7 +335,10 @@ static int rkisp1_csi_s_stream(struct v4l2_subdev *sd, int enable)
{
struct rkisp1_csi *csi = to_rkisp1_csi(sd);
struct rkisp1_device *rkisp1 = csi->rkisp1;
+ const struct v4l2_mbus_framefmt *sink_fmt;
+ const struct rkisp1_mbus_info *format;
struct rkisp1_sensor_async *source_asd;
+ struct v4l2_subdev_state *sd_state;
struct media_pad *source_pad;
struct v4l2_subdev *source;
int ret;
@@ -410,9 +368,12 @@ static int rkisp1_csi_s_stream(struct v4l2_subdev *sd, int enable)
if (source_asd->mbus_type != V4L2_MBUS_CSI2_DPHY)
return -EINVAL;
- mutex_lock(&csi->lock);
- ret = rkisp1_csi_start(csi, source_asd);
- mutex_unlock(&csi->lock);
+ sd_state = v4l2_subdev_lock_and_get_active_state(sd);
+ sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state, RKISP1_CSI_PAD_SINK);
+ format = rkisp1_mbus_info_get_by_code(sink_fmt->code);
+ v4l2_subdev_unlock_state(sd_state);
+
+ ret = rkisp1_csi_start(csi, source_asd, format);
if (ret)
return ret;
@@ -442,7 +403,7 @@ static const struct v4l2_subdev_video_ops rkisp1_csi_video_ops = {
static const struct v4l2_subdev_pad_ops rkisp1_csi_pad_ops = {
.enum_mbus_code = rkisp1_csi_enum_mbus_code,
.init_cfg = rkisp1_csi_init_config,
- .get_fmt = rkisp1_csi_get_fmt,
+ .get_fmt = v4l2_subdev_get_fmt,
.set_fmt = rkisp1_csi_set_fmt,
};
@@ -454,13 +415,11 @@ static const struct v4l2_subdev_ops rkisp1_csi_ops = {
int rkisp1_csi_register(struct rkisp1_device *rkisp1)
{
struct rkisp1_csi *csi = &rkisp1->csi;
- struct v4l2_subdev_state state = {};
struct media_pad *pads;
struct v4l2_subdev *sd;
int ret;
csi->rkisp1 = rkisp1;
- mutex_init(&csi->lock);
sd = &csi->sd;
v4l2_subdev_init(sd, &rkisp1_csi_ops);
@@ -476,26 +435,26 @@ int rkisp1_csi_register(struct rkisp1_device *rkisp1)
pads[RKISP1_CSI_PAD_SRC].flags = MEDIA_PAD_FL_SOURCE |
MEDIA_PAD_FL_MUST_CONNECT;
- csi->sink_fmt = rkisp1_mbus_info_get_by_code(RKISP1_CSI_DEF_FMT);
-
ret = media_entity_pads_init(&sd->entity, RKISP1_CSI_PAD_NUM, pads);
if (ret)
- goto error;
+ goto err_entity_cleanup;
- state.pads = csi->pad_cfg;
- rkisp1_csi_init_config(sd, &state);
+ ret = v4l2_subdev_init_finalize(sd);
+ if (ret)
+ goto err_entity_cleanup;
ret = v4l2_device_register_subdev(&csi->rkisp1->v4l2_dev, sd);
if (ret) {
dev_err(sd->dev, "Failed to register csi receiver subdev\n");
- goto error;
+ goto err_subdev_cleanup;
}
return 0;
-error:
+err_subdev_cleanup:
+ v4l2_subdev_cleanup(sd);
+err_entity_cleanup:
media_entity_cleanup(&sd->entity);
- mutex_destroy(&csi->lock);
csi->rkisp1 = NULL;
return ret;
}
@@ -508,8 +467,8 @@ void rkisp1_csi_unregister(struct rkisp1_device *rkisp1)
return;
v4l2_device_unregister_subdev(&csi->sd);
+ v4l2_subdev_cleanup(&csi->sd);
media_entity_cleanup(&csi->sd.entity);
- mutex_destroy(&csi->lock);
}
int rkisp1_csi_init(struct rkisp1_device *rkisp1)
--
Regards,
Laurent Pinchart
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v1 0/3] media: rkisp1: Convert to V4L2 subdev active state
2023-01-27 0:31 ` Laurent Pinchart
@ 2023-02-15 23:53 ` Laurent Pinchart
-1 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2023-02-15 23:53 UTC (permalink / raw)
To: Dafna Hirschfeld; +Cc: linux-media, Paul Elder, Heiko Stuebner, linux-rockchip
Hi Dafna,
Would you have time to review this series at some point ? There's still
plenty of time before v6.4, but I'd like to make sure not to miss that
version.
On Fri, Jan 27, 2023 at 02:31:21AM +0200, Laurent Pinchart wrote:
> Hello,
>
> This small patch series converts the three subdevs of the rkisp1 (CSI,
> ISP and resizer) to use the V4L2 subdev active state. This simplifies
> the implementation of the subdevs, as well as the locking scheme. There
> isn't much else to add here, please see individual patches for details.
>
> I have successfully tested this series on an i.MX8MP (Debix) and an
> RK3399 (Rock Pi 4).
>
> Laurent Pinchart (3):
> media: rkisp1: resizer: Use V4L2 subdev active state
> media: rkisp1: isp: Use V4L2 subdev active state
> media: rkisp1: csi: Use V4L2 subdev active state
>
> .../platform/rockchip/rkisp1/rkisp1-common.h | 18 --
> .../platform/rockchip/rkisp1/rkisp1-csi.c | 107 +++----
> .../platform/rockchip/rkisp1/rkisp1-isp.c | 261 +++++++-----------
> .../platform/rockchip/rkisp1/rkisp1-resizer.c | 184 +++++-------
> 4 files changed, 201 insertions(+), 369 deletions(-)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 0/3] media: rkisp1: Convert to V4L2 subdev active state
@ 2023-02-15 23:53 ` Laurent Pinchart
0 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2023-02-15 23:53 UTC (permalink / raw)
To: Dafna Hirschfeld; +Cc: linux-media, Paul Elder, Heiko Stuebner, linux-rockchip
Hi Dafna,
Would you have time to review this series at some point ? There's still
plenty of time before v6.4, but I'd like to make sure not to miss that
version.
On Fri, Jan 27, 2023 at 02:31:21AM +0200, Laurent Pinchart wrote:
> Hello,
>
> This small patch series converts the three subdevs of the rkisp1 (CSI,
> ISP and resizer) to use the V4L2 subdev active state. This simplifies
> the implementation of the subdevs, as well as the locking scheme. There
> isn't much else to add here, please see individual patches for details.
>
> I have successfully tested this series on an i.MX8MP (Debix) and an
> RK3399 (Rock Pi 4).
>
> Laurent Pinchart (3):
> media: rkisp1: resizer: Use V4L2 subdev active state
> media: rkisp1: isp: Use V4L2 subdev active state
> media: rkisp1: csi: Use V4L2 subdev active state
>
> .../platform/rockchip/rkisp1/rkisp1-common.h | 18 --
> .../platform/rockchip/rkisp1/rkisp1-csi.c | 107 +++----
> .../platform/rockchip/rkisp1/rkisp1-isp.c | 261 +++++++-----------
> .../platform/rockchip/rkisp1/rkisp1-resizer.c | 184 +++++-------
> 4 files changed, 201 insertions(+), 369 deletions(-)
--
Regards,
Laurent Pinchart
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 2/3] media: rkisp1: isp: Use V4L2 subdev active state
2023-01-27 0:31 ` Laurent Pinchart
@ 2023-04-27 3:51 ` Paul Elder
-1 siblings, 0 replies; 16+ messages in thread
From: Paul Elder @ 2023-04-27 3:51 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Dafna Hirschfeld, Heiko Stuebner, linux-rockchip
On Fri, Jan 27, 2023 at 02:31:23AM +0200, Laurent Pinchart wrote:
> Use the V4L2 subdev active state API to store the active format and crop
> rectangle. This simplifies the driver not only by dropping the state
> stored in the rkisp1_isp structure, but also by replacing the ops_lock
> with the state lock.
>
> The rkisp1_isp.sink_fmt field needs to be kept, as it is accessed from
> the stats interrupt handler. To simplifye the rkisp1_isp_set_sink_fmt()
s/simplifye/simplify/
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> implementation, the field is now set when starting the ISP, instead of
> when setting the format.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> .../platform/rockchip/rkisp1/rkisp1-common.h | 6 -
> .../platform/rockchip/rkisp1/rkisp1-isp.c | 261 +++++++-----------
> 2 files changed, 102 insertions(+), 165 deletions(-)
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> index e9626e59e1c8..d43ff1b549d9 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> @@ -190,20 +190,14 @@ struct rkisp1_csi {
> * @sd: v4l2_subdev variable
> * @rkisp1: pointer to rkisp1_device
> * @pads: media pads
> - * @pad_cfg: pads configurations
> * @sink_fmt: input format
> - * @src_fmt: output format
> - * @ops_lock: ops serialization
> * @frame_sequence: used to synchronize frame_id between video devices.
> */
> struct rkisp1_isp {
> struct v4l2_subdev sd;
> struct rkisp1_device *rkisp1;
> struct media_pad pads[RKISP1_ISP_PAD_MAX];
> - struct v4l2_subdev_pad_config pad_cfg[RKISP1_ISP_PAD_MAX];
> const struct rkisp1_mbus_info *sink_fmt;
> - const struct rkisp1_mbus_info *src_fmt;
> - struct mutex ops_lock; /* serialize the subdevice ops */
> __u32 frame_sequence;
> };
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> index 585cf3f53469..4d4f7b59f848 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> @@ -53,40 +53,6 @@
> * +---------------------------------------------------------+
> */
>
> -/* ----------------------------------------------------------------------------
> - * Helpers
> - */
> -
> -static struct v4l2_mbus_framefmt *
> -rkisp1_isp_get_pad_fmt(struct rkisp1_isp *isp,
> - struct v4l2_subdev_state *sd_state,
> - unsigned int pad, u32 which)
> -{
> - struct v4l2_subdev_state state = {
> - .pads = isp->pad_cfg
> - };
> -
> - if (which == V4L2_SUBDEV_FORMAT_TRY)
> - return v4l2_subdev_get_try_format(&isp->sd, sd_state, pad);
> - else
> - return v4l2_subdev_get_try_format(&isp->sd, &state, pad);
> -}
> -
> -static struct v4l2_rect *
> -rkisp1_isp_get_pad_crop(struct rkisp1_isp *isp,
> - struct v4l2_subdev_state *sd_state,
> - unsigned int pad, u32 which)
> -{
> - struct v4l2_subdev_state state = {
> - .pads = isp->pad_cfg
> - };
> -
> - if (which == V4L2_SUBDEV_FORMAT_TRY)
> - return v4l2_subdev_get_try_crop(&isp->sd, sd_state, pad);
> - else
> - return v4l2_subdev_get_try_crop(&isp->sd, &state, pad);
> -}
> -
> /* ----------------------------------------------------------------------------
> * Camera Interface registers configurations
> */
> @@ -96,12 +62,12 @@ rkisp1_isp_get_pad_crop(struct rkisp1_isp *isp,
> * This should only be called when configuring CIF
> * or at the frame end interrupt
> */
> -static void rkisp1_config_ism(struct rkisp1_isp *isp)
> +static void rkisp1_config_ism(struct rkisp1_isp *isp,
> + struct v4l2_subdev_state *sd_state)
> {
> const struct v4l2_rect *src_crop =
> - rkisp1_isp_get_pad_crop(isp, NULL,
> - RKISP1_ISP_PAD_SOURCE_VIDEO,
> - V4L2_SUBDEV_FORMAT_ACTIVE);
> + v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SOURCE_VIDEO);
> struct rkisp1_device *rkisp1 = isp->rkisp1;
> u32 val;
>
> @@ -125,21 +91,26 @@ static void rkisp1_config_ism(struct rkisp1_isp *isp)
> * configure ISP blocks with input format, size......
> */
> static int rkisp1_config_isp(struct rkisp1_isp *isp,
> + struct v4l2_subdev_state *sd_state,
> enum v4l2_mbus_type mbus_type, u32 mbus_flags)
> {
> struct rkisp1_device *rkisp1 = isp->rkisp1;
> u32 isp_ctrl = 0, irq_mask = 0, acq_mult = 0, acq_prop = 0;
> - const struct rkisp1_mbus_info *sink_fmt = isp->sink_fmt;
> - const struct rkisp1_mbus_info *src_fmt = isp->src_fmt;
> + const struct rkisp1_mbus_info *sink_fmt;
> + const struct rkisp1_mbus_info *src_fmt;
> + const struct v4l2_mbus_framefmt *src_frm;
> const struct v4l2_mbus_framefmt *sink_frm;
> const struct v4l2_rect *sink_crop;
>
> - sink_frm = rkisp1_isp_get_pad_fmt(isp, NULL,
> - RKISP1_ISP_PAD_SINK_VIDEO,
> - V4L2_SUBDEV_FORMAT_ACTIVE);
> - sink_crop = rkisp1_isp_get_pad_crop(isp, NULL,
> - RKISP1_ISP_PAD_SINK_VIDEO,
> - V4L2_SUBDEV_FORMAT_ACTIVE);
> + sink_frm = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SINK_VIDEO);
> + sink_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SINK_VIDEO);
> + src_frm = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SOURCE_VIDEO);
> +
> + sink_fmt = rkisp1_mbus_info_get_by_code(sink_frm->code);
> + src_fmt = rkisp1_mbus_info_get_by_code(src_frm->code);
>
> if (sink_fmt->pixel_enc == V4L2_PIXEL_ENC_BAYER) {
> acq_mult = 1;
> @@ -230,14 +201,15 @@ static int rkisp1_config_isp(struct rkisp1_isp *isp,
> } else {
> struct v4l2_mbus_framefmt *src_frm;
>
> - src_frm = rkisp1_isp_get_pad_fmt(isp, NULL,
> - RKISP1_ISP_PAD_SOURCE_VIDEO,
> - V4L2_SUBDEV_FORMAT_ACTIVE);
> + src_frm = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SOURCE_VIDEO);
> rkisp1_params_pre_configure(&rkisp1->params, sink_fmt->bayer_pat,
> src_frm->quantization,
> src_frm->ycbcr_enc);
> }
>
> + isp->sink_fmt = sink_fmt;
> +
> return 0;
> }
>
> @@ -258,16 +230,17 @@ static void rkisp1_config_path(struct rkisp1_isp *isp,
>
> /* Hardware configure Entry */
> static int rkisp1_config_cif(struct rkisp1_isp *isp,
> + struct v4l2_subdev_state *sd_state,
> enum v4l2_mbus_type mbus_type, u32 mbus_flags)
> {
> int ret;
>
> - ret = rkisp1_config_isp(isp, mbus_type, mbus_flags);
> + ret = rkisp1_config_isp(isp, sd_state, mbus_type, mbus_flags);
> if (ret)
> return ret;
>
> rkisp1_config_path(isp, mbus_type);
> - rkisp1_config_ism(isp);
> + rkisp1_config_ism(isp, sd_state);
>
> return 0;
> }
> @@ -328,9 +301,12 @@ static void rkisp1_config_clk(struct rkisp1_isp *isp)
> }
> }
>
> -static void rkisp1_isp_start(struct rkisp1_isp *isp)
> +static void rkisp1_isp_start(struct rkisp1_isp *isp,
> + struct v4l2_subdev_state *sd_state)
> {
> struct rkisp1_device *rkisp1 = isp->rkisp1;
> + const struct v4l2_mbus_framefmt *src_fmt;
> + const struct rkisp1_mbus_info *src_info;
> u32 val;
>
> rkisp1_config_clk(isp);
> @@ -342,7 +318,11 @@ static void rkisp1_isp_start(struct rkisp1_isp *isp)
> RKISP1_CIF_ISP_CTRL_ISP_INFORM_ENABLE;
> rkisp1_write(rkisp1, RKISP1_CIF_ISP_CTRL, val);
>
> - if (isp->src_fmt->pixel_enc != V4L2_PIXEL_ENC_BAYER)
> + src_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SOURCE_VIDEO);
> + src_info = rkisp1_mbus_info_get_by_code(src_fmt->code);
> +
> + if (src_info->pixel_enc != V4L2_PIXEL_ENC_BAYER)
> rkisp1_params_post_configure(&rkisp1->params);
> }
>
> @@ -436,7 +416,7 @@ static int rkisp1_isp_init_config(struct v4l2_subdev *sd,
> struct v4l2_rect *sink_crop, *src_crop;
>
> /* Video. */
> - sink_fmt = v4l2_subdev_get_try_format(sd, sd_state,
> + sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
> RKISP1_ISP_PAD_SINK_VIDEO);
> sink_fmt->width = RKISP1_DEFAULT_WIDTH;
> sink_fmt->height = RKISP1_DEFAULT_HEIGHT;
> @@ -447,14 +427,14 @@ static int rkisp1_isp_init_config(struct v4l2_subdev *sd,
> sink_fmt->ycbcr_enc = V4L2_YCBCR_ENC_601;
> sink_fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
>
> - sink_crop = v4l2_subdev_get_try_crop(sd, sd_state,
> + sink_crop = v4l2_subdev_get_pad_crop(sd, sd_state,
> RKISP1_ISP_PAD_SINK_VIDEO);
> sink_crop->width = RKISP1_DEFAULT_WIDTH;
> sink_crop->height = RKISP1_DEFAULT_HEIGHT;
> sink_crop->left = 0;
> sink_crop->top = 0;
>
> - src_fmt = v4l2_subdev_get_try_format(sd, sd_state,
> + src_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
> RKISP1_ISP_PAD_SOURCE_VIDEO);
> *src_fmt = *sink_fmt;
> src_fmt->code = RKISP1_DEF_SRC_PAD_FMT;
> @@ -463,14 +443,14 @@ static int rkisp1_isp_init_config(struct v4l2_subdev *sd,
> src_fmt->ycbcr_enc = V4L2_YCBCR_ENC_601;
> src_fmt->quantization = V4L2_QUANTIZATION_LIM_RANGE;
>
> - src_crop = v4l2_subdev_get_try_crop(sd, sd_state,
> + src_crop = v4l2_subdev_get_pad_crop(sd, sd_state,
> RKISP1_ISP_PAD_SOURCE_VIDEO);
> *src_crop = *sink_crop;
>
> /* Parameters and statistics. */
> - sink_fmt = v4l2_subdev_get_try_format(sd, sd_state,
> + sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
> RKISP1_ISP_PAD_SINK_PARAMS);
> - src_fmt = v4l2_subdev_get_try_format(sd, sd_state,
> + src_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
> RKISP1_ISP_PAD_SOURCE_STATS);
> sink_fmt->width = 0;
> sink_fmt->height = 0;
> @@ -483,8 +463,7 @@ static int rkisp1_isp_init_config(struct v4l2_subdev *sd,
>
> static void rkisp1_isp_set_src_fmt(struct rkisp1_isp *isp,
> struct v4l2_subdev_state *sd_state,
> - struct v4l2_mbus_framefmt *format,
> - unsigned int which)
> + struct v4l2_mbus_framefmt *format)
> {
> const struct rkisp1_mbus_info *sink_info;
> const struct rkisp1_mbus_info *src_info;
> @@ -493,12 +472,12 @@ static void rkisp1_isp_set_src_fmt(struct rkisp1_isp *isp,
> const struct v4l2_rect *src_crop;
> bool set_csc;
>
> - sink_fmt = rkisp1_isp_get_pad_fmt(isp, sd_state,
> - RKISP1_ISP_PAD_SINK_VIDEO, which);
> - src_fmt = rkisp1_isp_get_pad_fmt(isp, sd_state,
> - RKISP1_ISP_PAD_SOURCE_VIDEO, which);
> - src_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
> - RKISP1_ISP_PAD_SOURCE_VIDEO, which);
> + sink_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SINK_VIDEO);
> + src_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SOURCE_VIDEO);
> + src_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SOURCE_VIDEO);
>
> /*
> * Media bus code. The ISP can operate in pass-through mode (Bayer in,
> @@ -581,26 +560,20 @@ static void rkisp1_isp_set_src_fmt(struct rkisp1_isp *isp,
> */
> if (set_csc)
> format->flags |= V4L2_MBUS_FRAMEFMT_SET_CSC;
> -
> - /* Store the source format info when setting the active format. */
> - if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
> - isp->src_fmt = src_info;
> }
>
> static void rkisp1_isp_set_src_crop(struct rkisp1_isp *isp,
> struct v4l2_subdev_state *sd_state,
> - struct v4l2_rect *r, unsigned int which)
> + struct v4l2_rect *r)
> {
> struct v4l2_mbus_framefmt *src_fmt;
> const struct v4l2_rect *sink_crop;
> struct v4l2_rect *src_crop;
>
> - src_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
> - RKISP1_ISP_PAD_SOURCE_VIDEO,
> - which);
> - sink_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
> - RKISP1_ISP_PAD_SINK_VIDEO,
> - which);
> + src_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SOURCE_VIDEO);
> + sink_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SINK_VIDEO);
>
> src_crop->left = ALIGN(r->left, 2);
> src_crop->width = ALIGN(r->width, 2);
> @@ -611,24 +584,22 @@ static void rkisp1_isp_set_src_crop(struct rkisp1_isp *isp,
> *r = *src_crop;
>
> /* Propagate to out format */
> - src_fmt = rkisp1_isp_get_pad_fmt(isp, sd_state,
> - RKISP1_ISP_PAD_SOURCE_VIDEO, which);
> - rkisp1_isp_set_src_fmt(isp, sd_state, src_fmt, which);
> + src_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SOURCE_VIDEO);
> + rkisp1_isp_set_src_fmt(isp, sd_state, src_fmt);
> }
>
> static void rkisp1_isp_set_sink_crop(struct rkisp1_isp *isp,
> struct v4l2_subdev_state *sd_state,
> - struct v4l2_rect *r, unsigned int which)
> + struct v4l2_rect *r)
> {
> struct v4l2_rect *sink_crop, *src_crop;
> const struct v4l2_mbus_framefmt *sink_fmt;
>
> - sink_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
> - RKISP1_ISP_PAD_SINK_VIDEO,
> - which);
> - sink_fmt = rkisp1_isp_get_pad_fmt(isp, sd_state,
> - RKISP1_ISP_PAD_SINK_VIDEO,
> - which);
> + sink_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SINK_VIDEO);
> + sink_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SINK_VIDEO);
>
> sink_crop->left = ALIGN(r->left, 2);
> sink_crop->width = ALIGN(r->width, 2);
> @@ -639,32 +610,28 @@ static void rkisp1_isp_set_sink_crop(struct rkisp1_isp *isp,
> *r = *sink_crop;
>
> /* Propagate to out crop */
> - src_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
> - RKISP1_ISP_PAD_SOURCE_VIDEO, which);
> - rkisp1_isp_set_src_crop(isp, sd_state, src_crop, which);
> + src_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SOURCE_VIDEO);
> + rkisp1_isp_set_src_crop(isp, sd_state, src_crop);
> }
>
> static void rkisp1_isp_set_sink_fmt(struct rkisp1_isp *isp,
> struct v4l2_subdev_state *sd_state,
> - struct v4l2_mbus_framefmt *format,
> - unsigned int which)
> + struct v4l2_mbus_framefmt *format)
> {
> const struct rkisp1_mbus_info *mbus_info;
> struct v4l2_mbus_framefmt *sink_fmt;
> struct v4l2_rect *sink_crop;
> bool is_yuv;
>
> - sink_fmt = rkisp1_isp_get_pad_fmt(isp, sd_state,
> - RKISP1_ISP_PAD_SINK_VIDEO,
> - which);
> + sink_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SINK_VIDEO);
> sink_fmt->code = format->code;
> mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code);
> if (!mbus_info || !(mbus_info->direction & RKISP1_ISP_SD_SINK)) {
> sink_fmt->code = RKISP1_DEF_SINK_PAD_FMT;
> mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code);
> }
> - if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
> - isp->sink_fmt = mbus_info;
>
> sink_fmt->width = clamp_t(u32, format->width,
> RKISP1_ISP_MIN_WIDTH,
> @@ -706,23 +673,9 @@ static void rkisp1_isp_set_sink_fmt(struct rkisp1_isp *isp,
> *format = *sink_fmt;
>
> /* Propagate to in crop */
> - sink_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
> - RKISP1_ISP_PAD_SINK_VIDEO,
> - which);
> - rkisp1_isp_set_sink_crop(isp, sd_state, sink_crop, which);
> -}
> -
> -static int rkisp1_isp_get_fmt(struct v4l2_subdev *sd,
> - struct v4l2_subdev_state *sd_state,
> - struct v4l2_subdev_format *fmt)
> -{
> - struct rkisp1_isp *isp = to_rkisp1_isp(sd);
> -
> - mutex_lock(&isp->ops_lock);
> - fmt->format = *rkisp1_isp_get_pad_fmt(isp, sd_state, fmt->pad,
> - fmt->which);
> - mutex_unlock(&isp->ops_lock);
> - return 0;
> + sink_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SINK_VIDEO);
> + rkisp1_isp_set_sink_crop(isp, sd_state, sink_crop);
> }
>
> static int rkisp1_isp_set_fmt(struct v4l2_subdev *sd,
> @@ -731,18 +684,13 @@ static int rkisp1_isp_set_fmt(struct v4l2_subdev *sd,
> {
> struct rkisp1_isp *isp = to_rkisp1_isp(sd);
>
> - mutex_lock(&isp->ops_lock);
> if (fmt->pad == RKISP1_ISP_PAD_SINK_VIDEO)
> - rkisp1_isp_set_sink_fmt(isp, sd_state, &fmt->format,
> - fmt->which);
> + rkisp1_isp_set_sink_fmt(isp, sd_state, &fmt->format);
> else if (fmt->pad == RKISP1_ISP_PAD_SOURCE_VIDEO)
> - rkisp1_isp_set_src_fmt(isp, sd_state, &fmt->format,
> - fmt->which);
> + rkisp1_isp_set_src_fmt(isp, sd_state, &fmt->format);
> else
> - fmt->format = *rkisp1_isp_get_pad_fmt(isp, sd_state, fmt->pad,
> - fmt->which);
> + fmt->format = *v4l2_subdev_get_pad_format(sd, sd_state, fmt->pad);
>
> - mutex_unlock(&isp->ops_lock);
> return 0;
> }
>
> @@ -750,39 +698,37 @@ static int rkisp1_isp_get_selection(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *sd_state,
> struct v4l2_subdev_selection *sel)
> {
> - struct rkisp1_isp *isp = to_rkisp1_isp(sd);
> int ret = 0;
>
> if (sel->pad != RKISP1_ISP_PAD_SOURCE_VIDEO &&
> sel->pad != RKISP1_ISP_PAD_SINK_VIDEO)
> return -EINVAL;
>
> - mutex_lock(&isp->ops_lock);
> switch (sel->target) {
> case V4L2_SEL_TGT_CROP_BOUNDS:
> if (sel->pad == RKISP1_ISP_PAD_SINK_VIDEO) {
> struct v4l2_mbus_framefmt *fmt;
>
> - fmt = rkisp1_isp_get_pad_fmt(isp, sd_state, sel->pad,
> - sel->which);
> + fmt = v4l2_subdev_get_pad_format(sd, sd_state, sel->pad);
> sel->r.height = fmt->height;
> sel->r.width = fmt->width;
> sel->r.left = 0;
> sel->r.top = 0;
> } else {
> - sel->r = *rkisp1_isp_get_pad_crop(isp, sd_state,
> - RKISP1_ISP_PAD_SINK_VIDEO,
> - sel->which);
> + sel->r = *v4l2_subdev_get_pad_crop(sd, sd_state,
> + RKISP1_ISP_PAD_SINK_VIDEO);
> }
> break;
> +
> case V4L2_SEL_TGT_CROP:
> - sel->r = *rkisp1_isp_get_pad_crop(isp, sd_state, sel->pad,
> - sel->which);
> + sel->r = *v4l2_subdev_get_pad_crop(sd, sd_state, sel->pad);
> break;
> +
> default:
> ret = -EINVAL;
> + break;
> }
> - mutex_unlock(&isp->ops_lock);
> +
> return ret;
> }
>
> @@ -798,15 +744,14 @@ static int rkisp1_isp_set_selection(struct v4l2_subdev *sd,
>
> dev_dbg(isp->rkisp1->dev, "%s: pad: %d sel(%d,%d)/%dx%d\n", __func__,
> sel->pad, sel->r.left, sel->r.top, sel->r.width, sel->r.height);
> - mutex_lock(&isp->ops_lock);
> +
> if (sel->pad == RKISP1_ISP_PAD_SINK_VIDEO)
> - rkisp1_isp_set_sink_crop(isp, sd_state, &sel->r, sel->which);
> + rkisp1_isp_set_sink_crop(isp, sd_state, &sel->r);
> else if (sel->pad == RKISP1_ISP_PAD_SOURCE_VIDEO)
> - rkisp1_isp_set_src_crop(isp, sd_state, &sel->r, sel->which);
> + rkisp1_isp_set_src_crop(isp, sd_state, &sel->r);
> else
> ret = -EINVAL;
>
> - mutex_unlock(&isp->ops_lock);
> return ret;
> }
>
> @@ -824,7 +769,7 @@ static const struct v4l2_subdev_pad_ops rkisp1_isp_pad_ops = {
> .get_selection = rkisp1_isp_get_selection,
> .set_selection = rkisp1_isp_set_selection,
> .init_cfg = rkisp1_isp_init_config,
> - .get_fmt = rkisp1_isp_get_fmt,
> + .get_fmt = v4l2_subdev_get_fmt,
> .set_fmt = rkisp1_isp_set_fmt,
> .link_validate = v4l2_subdev_link_validate_default,
> };
> @@ -837,6 +782,7 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
> {
> struct rkisp1_isp *isp = to_rkisp1_isp(sd);
> struct rkisp1_device *rkisp1 = isp->rkisp1;
> + struct v4l2_subdev_state *sd_state;
> struct media_pad *source_pad;
> struct media_pad *sink_pad;
> enum v4l2_mbus_type mbus_type;
> @@ -877,21 +823,23 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
> }
>
> isp->frame_sequence = -1;
> - mutex_lock(&isp->ops_lock);
> - ret = rkisp1_config_cif(isp, mbus_type, mbus_flags);
> +
> + sd_state = v4l2_subdev_lock_and_get_active_state(sd);
> +
> + ret = rkisp1_config_cif(isp, sd_state, mbus_type, mbus_flags);
> if (ret)
> - goto mutex_unlock;
> + goto out_unlock;
>
> - rkisp1_isp_start(isp);
> + rkisp1_isp_start(isp, sd_state);
>
> ret = v4l2_subdev_call(rkisp1->source, video, s_stream, true);
> if (ret) {
> rkisp1_isp_stop(isp);
> - goto mutex_unlock;
> + goto out_unlock;
> }
>
> -mutex_unlock:
> - mutex_unlock(&isp->ops_lock);
> +out_unlock:
> + v4l2_subdev_unlock_state(sd_state);
> return ret;
> }
>
> @@ -929,9 +877,6 @@ static const struct v4l2_subdev_ops rkisp1_isp_ops = {
>
> int rkisp1_isp_register(struct rkisp1_device *rkisp1)
> {
> - struct v4l2_subdev_state state = {
> - .pads = rkisp1->isp.pad_cfg
> - };
> struct rkisp1_isp *isp = &rkisp1->isp;
> struct media_pad *pads = isp->pads;
> struct v4l2_subdev *sd = &isp->sd;
> @@ -952,27 +897,26 @@ int rkisp1_isp_register(struct rkisp1_device *rkisp1)
> pads[RKISP1_ISP_PAD_SOURCE_VIDEO].flags = MEDIA_PAD_FL_SOURCE;
> pads[RKISP1_ISP_PAD_SOURCE_STATS].flags = MEDIA_PAD_FL_SOURCE;
>
> - isp->sink_fmt = rkisp1_mbus_info_get_by_code(RKISP1_DEF_SINK_PAD_FMT);
> - isp->src_fmt = rkisp1_mbus_info_get_by_code(RKISP1_DEF_SRC_PAD_FMT);
> -
> - mutex_init(&isp->ops_lock);
> ret = media_entity_pads_init(&sd->entity, RKISP1_ISP_PAD_MAX, pads);
> if (ret)
> - goto error;
> + goto err_entity_cleanup;
> +
> + ret = v4l2_subdev_init_finalize(sd);
> + if (ret)
> + goto err_subdev_cleanup;
>
> ret = v4l2_device_register_subdev(&rkisp1->v4l2_dev, sd);
> if (ret) {
> dev_err(rkisp1->dev, "Failed to register isp subdev\n");
> - goto error;
> + goto err_subdev_cleanup;
> }
>
> - rkisp1_isp_init_config(sd, &state);
> -
> return 0;
>
> -error:
> +err_subdev_cleanup:
> + v4l2_subdev_cleanup(sd);
> +err_entity_cleanup:
> media_entity_cleanup(&sd->entity);
> - mutex_destroy(&isp->ops_lock);
> isp->sd.v4l2_dev = NULL;
> return ret;
> }
> @@ -986,7 +930,6 @@ void rkisp1_isp_unregister(struct rkisp1_device *rkisp1)
>
> v4l2_device_unregister_subdev(&isp->sd);
> media_entity_cleanup(&isp->sd.entity);
> - mutex_destroy(&isp->ops_lock);
> }
>
> /* ----------------------------------------------------------------------------
> --
> Regards,
>
> Laurent Pinchart
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 2/3] media: rkisp1: isp: Use V4L2 subdev active state
@ 2023-04-27 3:51 ` Paul Elder
0 siblings, 0 replies; 16+ messages in thread
From: Paul Elder @ 2023-04-27 3:51 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Dafna Hirschfeld, Heiko Stuebner, linux-rockchip
On Fri, Jan 27, 2023 at 02:31:23AM +0200, Laurent Pinchart wrote:
> Use the V4L2 subdev active state API to store the active format and crop
> rectangle. This simplifies the driver not only by dropping the state
> stored in the rkisp1_isp structure, but also by replacing the ops_lock
> with the state lock.
>
> The rkisp1_isp.sink_fmt field needs to be kept, as it is accessed from
> the stats interrupt handler. To simplifye the rkisp1_isp_set_sink_fmt()
s/simplifye/simplify/
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> implementation, the field is now set when starting the ISP, instead of
> when setting the format.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> .../platform/rockchip/rkisp1/rkisp1-common.h | 6 -
> .../platform/rockchip/rkisp1/rkisp1-isp.c | 261 +++++++-----------
> 2 files changed, 102 insertions(+), 165 deletions(-)
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> index e9626e59e1c8..d43ff1b549d9 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> @@ -190,20 +190,14 @@ struct rkisp1_csi {
> * @sd: v4l2_subdev variable
> * @rkisp1: pointer to rkisp1_device
> * @pads: media pads
> - * @pad_cfg: pads configurations
> * @sink_fmt: input format
> - * @src_fmt: output format
> - * @ops_lock: ops serialization
> * @frame_sequence: used to synchronize frame_id between video devices.
> */
> struct rkisp1_isp {
> struct v4l2_subdev sd;
> struct rkisp1_device *rkisp1;
> struct media_pad pads[RKISP1_ISP_PAD_MAX];
> - struct v4l2_subdev_pad_config pad_cfg[RKISP1_ISP_PAD_MAX];
> const struct rkisp1_mbus_info *sink_fmt;
> - const struct rkisp1_mbus_info *src_fmt;
> - struct mutex ops_lock; /* serialize the subdevice ops */
> __u32 frame_sequence;
> };
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> index 585cf3f53469..4d4f7b59f848 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> @@ -53,40 +53,6 @@
> * +---------------------------------------------------------+
> */
>
> -/* ----------------------------------------------------------------------------
> - * Helpers
> - */
> -
> -static struct v4l2_mbus_framefmt *
> -rkisp1_isp_get_pad_fmt(struct rkisp1_isp *isp,
> - struct v4l2_subdev_state *sd_state,
> - unsigned int pad, u32 which)
> -{
> - struct v4l2_subdev_state state = {
> - .pads = isp->pad_cfg
> - };
> -
> - if (which == V4L2_SUBDEV_FORMAT_TRY)
> - return v4l2_subdev_get_try_format(&isp->sd, sd_state, pad);
> - else
> - return v4l2_subdev_get_try_format(&isp->sd, &state, pad);
> -}
> -
> -static struct v4l2_rect *
> -rkisp1_isp_get_pad_crop(struct rkisp1_isp *isp,
> - struct v4l2_subdev_state *sd_state,
> - unsigned int pad, u32 which)
> -{
> - struct v4l2_subdev_state state = {
> - .pads = isp->pad_cfg
> - };
> -
> - if (which == V4L2_SUBDEV_FORMAT_TRY)
> - return v4l2_subdev_get_try_crop(&isp->sd, sd_state, pad);
> - else
> - return v4l2_subdev_get_try_crop(&isp->sd, &state, pad);
> -}
> -
> /* ----------------------------------------------------------------------------
> * Camera Interface registers configurations
> */
> @@ -96,12 +62,12 @@ rkisp1_isp_get_pad_crop(struct rkisp1_isp *isp,
> * This should only be called when configuring CIF
> * or at the frame end interrupt
> */
> -static void rkisp1_config_ism(struct rkisp1_isp *isp)
> +static void rkisp1_config_ism(struct rkisp1_isp *isp,
> + struct v4l2_subdev_state *sd_state)
> {
> const struct v4l2_rect *src_crop =
> - rkisp1_isp_get_pad_crop(isp, NULL,
> - RKISP1_ISP_PAD_SOURCE_VIDEO,
> - V4L2_SUBDEV_FORMAT_ACTIVE);
> + v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SOURCE_VIDEO);
> struct rkisp1_device *rkisp1 = isp->rkisp1;
> u32 val;
>
> @@ -125,21 +91,26 @@ static void rkisp1_config_ism(struct rkisp1_isp *isp)
> * configure ISP blocks with input format, size......
> */
> static int rkisp1_config_isp(struct rkisp1_isp *isp,
> + struct v4l2_subdev_state *sd_state,
> enum v4l2_mbus_type mbus_type, u32 mbus_flags)
> {
> struct rkisp1_device *rkisp1 = isp->rkisp1;
> u32 isp_ctrl = 0, irq_mask = 0, acq_mult = 0, acq_prop = 0;
> - const struct rkisp1_mbus_info *sink_fmt = isp->sink_fmt;
> - const struct rkisp1_mbus_info *src_fmt = isp->src_fmt;
> + const struct rkisp1_mbus_info *sink_fmt;
> + const struct rkisp1_mbus_info *src_fmt;
> + const struct v4l2_mbus_framefmt *src_frm;
> const struct v4l2_mbus_framefmt *sink_frm;
> const struct v4l2_rect *sink_crop;
>
> - sink_frm = rkisp1_isp_get_pad_fmt(isp, NULL,
> - RKISP1_ISP_PAD_SINK_VIDEO,
> - V4L2_SUBDEV_FORMAT_ACTIVE);
> - sink_crop = rkisp1_isp_get_pad_crop(isp, NULL,
> - RKISP1_ISP_PAD_SINK_VIDEO,
> - V4L2_SUBDEV_FORMAT_ACTIVE);
> + sink_frm = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SINK_VIDEO);
> + sink_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SINK_VIDEO);
> + src_frm = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SOURCE_VIDEO);
> +
> + sink_fmt = rkisp1_mbus_info_get_by_code(sink_frm->code);
> + src_fmt = rkisp1_mbus_info_get_by_code(src_frm->code);
>
> if (sink_fmt->pixel_enc == V4L2_PIXEL_ENC_BAYER) {
> acq_mult = 1;
> @@ -230,14 +201,15 @@ static int rkisp1_config_isp(struct rkisp1_isp *isp,
> } else {
> struct v4l2_mbus_framefmt *src_frm;
>
> - src_frm = rkisp1_isp_get_pad_fmt(isp, NULL,
> - RKISP1_ISP_PAD_SOURCE_VIDEO,
> - V4L2_SUBDEV_FORMAT_ACTIVE);
> + src_frm = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SOURCE_VIDEO);
> rkisp1_params_pre_configure(&rkisp1->params, sink_fmt->bayer_pat,
> src_frm->quantization,
> src_frm->ycbcr_enc);
> }
>
> + isp->sink_fmt = sink_fmt;
> +
> return 0;
> }
>
> @@ -258,16 +230,17 @@ static void rkisp1_config_path(struct rkisp1_isp *isp,
>
> /* Hardware configure Entry */
> static int rkisp1_config_cif(struct rkisp1_isp *isp,
> + struct v4l2_subdev_state *sd_state,
> enum v4l2_mbus_type mbus_type, u32 mbus_flags)
> {
> int ret;
>
> - ret = rkisp1_config_isp(isp, mbus_type, mbus_flags);
> + ret = rkisp1_config_isp(isp, sd_state, mbus_type, mbus_flags);
> if (ret)
> return ret;
>
> rkisp1_config_path(isp, mbus_type);
> - rkisp1_config_ism(isp);
> + rkisp1_config_ism(isp, sd_state);
>
> return 0;
> }
> @@ -328,9 +301,12 @@ static void rkisp1_config_clk(struct rkisp1_isp *isp)
> }
> }
>
> -static void rkisp1_isp_start(struct rkisp1_isp *isp)
> +static void rkisp1_isp_start(struct rkisp1_isp *isp,
> + struct v4l2_subdev_state *sd_state)
> {
> struct rkisp1_device *rkisp1 = isp->rkisp1;
> + const struct v4l2_mbus_framefmt *src_fmt;
> + const struct rkisp1_mbus_info *src_info;
> u32 val;
>
> rkisp1_config_clk(isp);
> @@ -342,7 +318,11 @@ static void rkisp1_isp_start(struct rkisp1_isp *isp)
> RKISP1_CIF_ISP_CTRL_ISP_INFORM_ENABLE;
> rkisp1_write(rkisp1, RKISP1_CIF_ISP_CTRL, val);
>
> - if (isp->src_fmt->pixel_enc != V4L2_PIXEL_ENC_BAYER)
> + src_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SOURCE_VIDEO);
> + src_info = rkisp1_mbus_info_get_by_code(src_fmt->code);
> +
> + if (src_info->pixel_enc != V4L2_PIXEL_ENC_BAYER)
> rkisp1_params_post_configure(&rkisp1->params);
> }
>
> @@ -436,7 +416,7 @@ static int rkisp1_isp_init_config(struct v4l2_subdev *sd,
> struct v4l2_rect *sink_crop, *src_crop;
>
> /* Video. */
> - sink_fmt = v4l2_subdev_get_try_format(sd, sd_state,
> + sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
> RKISP1_ISP_PAD_SINK_VIDEO);
> sink_fmt->width = RKISP1_DEFAULT_WIDTH;
> sink_fmt->height = RKISP1_DEFAULT_HEIGHT;
> @@ -447,14 +427,14 @@ static int rkisp1_isp_init_config(struct v4l2_subdev *sd,
> sink_fmt->ycbcr_enc = V4L2_YCBCR_ENC_601;
> sink_fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
>
> - sink_crop = v4l2_subdev_get_try_crop(sd, sd_state,
> + sink_crop = v4l2_subdev_get_pad_crop(sd, sd_state,
> RKISP1_ISP_PAD_SINK_VIDEO);
> sink_crop->width = RKISP1_DEFAULT_WIDTH;
> sink_crop->height = RKISP1_DEFAULT_HEIGHT;
> sink_crop->left = 0;
> sink_crop->top = 0;
>
> - src_fmt = v4l2_subdev_get_try_format(sd, sd_state,
> + src_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
> RKISP1_ISP_PAD_SOURCE_VIDEO);
> *src_fmt = *sink_fmt;
> src_fmt->code = RKISP1_DEF_SRC_PAD_FMT;
> @@ -463,14 +443,14 @@ static int rkisp1_isp_init_config(struct v4l2_subdev *sd,
> src_fmt->ycbcr_enc = V4L2_YCBCR_ENC_601;
> src_fmt->quantization = V4L2_QUANTIZATION_LIM_RANGE;
>
> - src_crop = v4l2_subdev_get_try_crop(sd, sd_state,
> + src_crop = v4l2_subdev_get_pad_crop(sd, sd_state,
> RKISP1_ISP_PAD_SOURCE_VIDEO);
> *src_crop = *sink_crop;
>
> /* Parameters and statistics. */
> - sink_fmt = v4l2_subdev_get_try_format(sd, sd_state,
> + sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
> RKISP1_ISP_PAD_SINK_PARAMS);
> - src_fmt = v4l2_subdev_get_try_format(sd, sd_state,
> + src_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
> RKISP1_ISP_PAD_SOURCE_STATS);
> sink_fmt->width = 0;
> sink_fmt->height = 0;
> @@ -483,8 +463,7 @@ static int rkisp1_isp_init_config(struct v4l2_subdev *sd,
>
> static void rkisp1_isp_set_src_fmt(struct rkisp1_isp *isp,
> struct v4l2_subdev_state *sd_state,
> - struct v4l2_mbus_framefmt *format,
> - unsigned int which)
> + struct v4l2_mbus_framefmt *format)
> {
> const struct rkisp1_mbus_info *sink_info;
> const struct rkisp1_mbus_info *src_info;
> @@ -493,12 +472,12 @@ static void rkisp1_isp_set_src_fmt(struct rkisp1_isp *isp,
> const struct v4l2_rect *src_crop;
> bool set_csc;
>
> - sink_fmt = rkisp1_isp_get_pad_fmt(isp, sd_state,
> - RKISP1_ISP_PAD_SINK_VIDEO, which);
> - src_fmt = rkisp1_isp_get_pad_fmt(isp, sd_state,
> - RKISP1_ISP_PAD_SOURCE_VIDEO, which);
> - src_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
> - RKISP1_ISP_PAD_SOURCE_VIDEO, which);
> + sink_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SINK_VIDEO);
> + src_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SOURCE_VIDEO);
> + src_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SOURCE_VIDEO);
>
> /*
> * Media bus code. The ISP can operate in pass-through mode (Bayer in,
> @@ -581,26 +560,20 @@ static void rkisp1_isp_set_src_fmt(struct rkisp1_isp *isp,
> */
> if (set_csc)
> format->flags |= V4L2_MBUS_FRAMEFMT_SET_CSC;
> -
> - /* Store the source format info when setting the active format. */
> - if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
> - isp->src_fmt = src_info;
> }
>
> static void rkisp1_isp_set_src_crop(struct rkisp1_isp *isp,
> struct v4l2_subdev_state *sd_state,
> - struct v4l2_rect *r, unsigned int which)
> + struct v4l2_rect *r)
> {
> struct v4l2_mbus_framefmt *src_fmt;
> const struct v4l2_rect *sink_crop;
> struct v4l2_rect *src_crop;
>
> - src_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
> - RKISP1_ISP_PAD_SOURCE_VIDEO,
> - which);
> - sink_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
> - RKISP1_ISP_PAD_SINK_VIDEO,
> - which);
> + src_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SOURCE_VIDEO);
> + sink_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SINK_VIDEO);
>
> src_crop->left = ALIGN(r->left, 2);
> src_crop->width = ALIGN(r->width, 2);
> @@ -611,24 +584,22 @@ static void rkisp1_isp_set_src_crop(struct rkisp1_isp *isp,
> *r = *src_crop;
>
> /* Propagate to out format */
> - src_fmt = rkisp1_isp_get_pad_fmt(isp, sd_state,
> - RKISP1_ISP_PAD_SOURCE_VIDEO, which);
> - rkisp1_isp_set_src_fmt(isp, sd_state, src_fmt, which);
> + src_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SOURCE_VIDEO);
> + rkisp1_isp_set_src_fmt(isp, sd_state, src_fmt);
> }
>
> static void rkisp1_isp_set_sink_crop(struct rkisp1_isp *isp,
> struct v4l2_subdev_state *sd_state,
> - struct v4l2_rect *r, unsigned int which)
> + struct v4l2_rect *r)
> {
> struct v4l2_rect *sink_crop, *src_crop;
> const struct v4l2_mbus_framefmt *sink_fmt;
>
> - sink_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
> - RKISP1_ISP_PAD_SINK_VIDEO,
> - which);
> - sink_fmt = rkisp1_isp_get_pad_fmt(isp, sd_state,
> - RKISP1_ISP_PAD_SINK_VIDEO,
> - which);
> + sink_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SINK_VIDEO);
> + sink_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SINK_VIDEO);
>
> sink_crop->left = ALIGN(r->left, 2);
> sink_crop->width = ALIGN(r->width, 2);
> @@ -639,32 +610,28 @@ static void rkisp1_isp_set_sink_crop(struct rkisp1_isp *isp,
> *r = *sink_crop;
>
> /* Propagate to out crop */
> - src_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
> - RKISP1_ISP_PAD_SOURCE_VIDEO, which);
> - rkisp1_isp_set_src_crop(isp, sd_state, src_crop, which);
> + src_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SOURCE_VIDEO);
> + rkisp1_isp_set_src_crop(isp, sd_state, src_crop);
> }
>
> static void rkisp1_isp_set_sink_fmt(struct rkisp1_isp *isp,
> struct v4l2_subdev_state *sd_state,
> - struct v4l2_mbus_framefmt *format,
> - unsigned int which)
> + struct v4l2_mbus_framefmt *format)
> {
> const struct rkisp1_mbus_info *mbus_info;
> struct v4l2_mbus_framefmt *sink_fmt;
> struct v4l2_rect *sink_crop;
> bool is_yuv;
>
> - sink_fmt = rkisp1_isp_get_pad_fmt(isp, sd_state,
> - RKISP1_ISP_PAD_SINK_VIDEO,
> - which);
> + sink_fmt = v4l2_subdev_get_pad_format(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SINK_VIDEO);
> sink_fmt->code = format->code;
> mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code);
> if (!mbus_info || !(mbus_info->direction & RKISP1_ISP_SD_SINK)) {
> sink_fmt->code = RKISP1_DEF_SINK_PAD_FMT;
> mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code);
> }
> - if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
> - isp->sink_fmt = mbus_info;
>
> sink_fmt->width = clamp_t(u32, format->width,
> RKISP1_ISP_MIN_WIDTH,
> @@ -706,23 +673,9 @@ static void rkisp1_isp_set_sink_fmt(struct rkisp1_isp *isp,
> *format = *sink_fmt;
>
> /* Propagate to in crop */
> - sink_crop = rkisp1_isp_get_pad_crop(isp, sd_state,
> - RKISP1_ISP_PAD_SINK_VIDEO,
> - which);
> - rkisp1_isp_set_sink_crop(isp, sd_state, sink_crop, which);
> -}
> -
> -static int rkisp1_isp_get_fmt(struct v4l2_subdev *sd,
> - struct v4l2_subdev_state *sd_state,
> - struct v4l2_subdev_format *fmt)
> -{
> - struct rkisp1_isp *isp = to_rkisp1_isp(sd);
> -
> - mutex_lock(&isp->ops_lock);
> - fmt->format = *rkisp1_isp_get_pad_fmt(isp, sd_state, fmt->pad,
> - fmt->which);
> - mutex_unlock(&isp->ops_lock);
> - return 0;
> + sink_crop = v4l2_subdev_get_pad_crop(&isp->sd, sd_state,
> + RKISP1_ISP_PAD_SINK_VIDEO);
> + rkisp1_isp_set_sink_crop(isp, sd_state, sink_crop);
> }
>
> static int rkisp1_isp_set_fmt(struct v4l2_subdev *sd,
> @@ -731,18 +684,13 @@ static int rkisp1_isp_set_fmt(struct v4l2_subdev *sd,
> {
> struct rkisp1_isp *isp = to_rkisp1_isp(sd);
>
> - mutex_lock(&isp->ops_lock);
> if (fmt->pad == RKISP1_ISP_PAD_SINK_VIDEO)
> - rkisp1_isp_set_sink_fmt(isp, sd_state, &fmt->format,
> - fmt->which);
> + rkisp1_isp_set_sink_fmt(isp, sd_state, &fmt->format);
> else if (fmt->pad == RKISP1_ISP_PAD_SOURCE_VIDEO)
> - rkisp1_isp_set_src_fmt(isp, sd_state, &fmt->format,
> - fmt->which);
> + rkisp1_isp_set_src_fmt(isp, sd_state, &fmt->format);
> else
> - fmt->format = *rkisp1_isp_get_pad_fmt(isp, sd_state, fmt->pad,
> - fmt->which);
> + fmt->format = *v4l2_subdev_get_pad_format(sd, sd_state, fmt->pad);
>
> - mutex_unlock(&isp->ops_lock);
> return 0;
> }
>
> @@ -750,39 +698,37 @@ static int rkisp1_isp_get_selection(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *sd_state,
> struct v4l2_subdev_selection *sel)
> {
> - struct rkisp1_isp *isp = to_rkisp1_isp(sd);
> int ret = 0;
>
> if (sel->pad != RKISP1_ISP_PAD_SOURCE_VIDEO &&
> sel->pad != RKISP1_ISP_PAD_SINK_VIDEO)
> return -EINVAL;
>
> - mutex_lock(&isp->ops_lock);
> switch (sel->target) {
> case V4L2_SEL_TGT_CROP_BOUNDS:
> if (sel->pad == RKISP1_ISP_PAD_SINK_VIDEO) {
> struct v4l2_mbus_framefmt *fmt;
>
> - fmt = rkisp1_isp_get_pad_fmt(isp, sd_state, sel->pad,
> - sel->which);
> + fmt = v4l2_subdev_get_pad_format(sd, sd_state, sel->pad);
> sel->r.height = fmt->height;
> sel->r.width = fmt->width;
> sel->r.left = 0;
> sel->r.top = 0;
> } else {
> - sel->r = *rkisp1_isp_get_pad_crop(isp, sd_state,
> - RKISP1_ISP_PAD_SINK_VIDEO,
> - sel->which);
> + sel->r = *v4l2_subdev_get_pad_crop(sd, sd_state,
> + RKISP1_ISP_PAD_SINK_VIDEO);
> }
> break;
> +
> case V4L2_SEL_TGT_CROP:
> - sel->r = *rkisp1_isp_get_pad_crop(isp, sd_state, sel->pad,
> - sel->which);
> + sel->r = *v4l2_subdev_get_pad_crop(sd, sd_state, sel->pad);
> break;
> +
> default:
> ret = -EINVAL;
> + break;
> }
> - mutex_unlock(&isp->ops_lock);
> +
> return ret;
> }
>
> @@ -798,15 +744,14 @@ static int rkisp1_isp_set_selection(struct v4l2_subdev *sd,
>
> dev_dbg(isp->rkisp1->dev, "%s: pad: %d sel(%d,%d)/%dx%d\n", __func__,
> sel->pad, sel->r.left, sel->r.top, sel->r.width, sel->r.height);
> - mutex_lock(&isp->ops_lock);
> +
> if (sel->pad == RKISP1_ISP_PAD_SINK_VIDEO)
> - rkisp1_isp_set_sink_crop(isp, sd_state, &sel->r, sel->which);
> + rkisp1_isp_set_sink_crop(isp, sd_state, &sel->r);
> else if (sel->pad == RKISP1_ISP_PAD_SOURCE_VIDEO)
> - rkisp1_isp_set_src_crop(isp, sd_state, &sel->r, sel->which);
> + rkisp1_isp_set_src_crop(isp, sd_state, &sel->r);
> else
> ret = -EINVAL;
>
> - mutex_unlock(&isp->ops_lock);
> return ret;
> }
>
> @@ -824,7 +769,7 @@ static const struct v4l2_subdev_pad_ops rkisp1_isp_pad_ops = {
> .get_selection = rkisp1_isp_get_selection,
> .set_selection = rkisp1_isp_set_selection,
> .init_cfg = rkisp1_isp_init_config,
> - .get_fmt = rkisp1_isp_get_fmt,
> + .get_fmt = v4l2_subdev_get_fmt,
> .set_fmt = rkisp1_isp_set_fmt,
> .link_validate = v4l2_subdev_link_validate_default,
> };
> @@ -837,6 +782,7 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
> {
> struct rkisp1_isp *isp = to_rkisp1_isp(sd);
> struct rkisp1_device *rkisp1 = isp->rkisp1;
> + struct v4l2_subdev_state *sd_state;
> struct media_pad *source_pad;
> struct media_pad *sink_pad;
> enum v4l2_mbus_type mbus_type;
> @@ -877,21 +823,23 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
> }
>
> isp->frame_sequence = -1;
> - mutex_lock(&isp->ops_lock);
> - ret = rkisp1_config_cif(isp, mbus_type, mbus_flags);
> +
> + sd_state = v4l2_subdev_lock_and_get_active_state(sd);
> +
> + ret = rkisp1_config_cif(isp, sd_state, mbus_type, mbus_flags);
> if (ret)
> - goto mutex_unlock;
> + goto out_unlock;
>
> - rkisp1_isp_start(isp);
> + rkisp1_isp_start(isp, sd_state);
>
> ret = v4l2_subdev_call(rkisp1->source, video, s_stream, true);
> if (ret) {
> rkisp1_isp_stop(isp);
> - goto mutex_unlock;
> + goto out_unlock;
> }
>
> -mutex_unlock:
> - mutex_unlock(&isp->ops_lock);
> +out_unlock:
> + v4l2_subdev_unlock_state(sd_state);
> return ret;
> }
>
> @@ -929,9 +877,6 @@ static const struct v4l2_subdev_ops rkisp1_isp_ops = {
>
> int rkisp1_isp_register(struct rkisp1_device *rkisp1)
> {
> - struct v4l2_subdev_state state = {
> - .pads = rkisp1->isp.pad_cfg
> - };
> struct rkisp1_isp *isp = &rkisp1->isp;
> struct media_pad *pads = isp->pads;
> struct v4l2_subdev *sd = &isp->sd;
> @@ -952,27 +897,26 @@ int rkisp1_isp_register(struct rkisp1_device *rkisp1)
> pads[RKISP1_ISP_PAD_SOURCE_VIDEO].flags = MEDIA_PAD_FL_SOURCE;
> pads[RKISP1_ISP_PAD_SOURCE_STATS].flags = MEDIA_PAD_FL_SOURCE;
>
> - isp->sink_fmt = rkisp1_mbus_info_get_by_code(RKISP1_DEF_SINK_PAD_FMT);
> - isp->src_fmt = rkisp1_mbus_info_get_by_code(RKISP1_DEF_SRC_PAD_FMT);
> -
> - mutex_init(&isp->ops_lock);
> ret = media_entity_pads_init(&sd->entity, RKISP1_ISP_PAD_MAX, pads);
> if (ret)
> - goto error;
> + goto err_entity_cleanup;
> +
> + ret = v4l2_subdev_init_finalize(sd);
> + if (ret)
> + goto err_subdev_cleanup;
>
> ret = v4l2_device_register_subdev(&rkisp1->v4l2_dev, sd);
> if (ret) {
> dev_err(rkisp1->dev, "Failed to register isp subdev\n");
> - goto error;
> + goto err_subdev_cleanup;
> }
>
> - rkisp1_isp_init_config(sd, &state);
> -
> return 0;
>
> -error:
> +err_subdev_cleanup:
> + v4l2_subdev_cleanup(sd);
> +err_entity_cleanup:
> media_entity_cleanup(&sd->entity);
> - mutex_destroy(&isp->ops_lock);
> isp->sd.v4l2_dev = NULL;
> return ret;
> }
> @@ -986,7 +930,6 @@ void rkisp1_isp_unregister(struct rkisp1_device *rkisp1)
>
> v4l2_device_unregister_subdev(&isp->sd);
> media_entity_cleanup(&isp->sd.entity);
> - mutex_destroy(&isp->ops_lock);
> }
>
> /* ----------------------------------------------------------------------------
> --
> Regards,
>
> Laurent Pinchart
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 1/3] media: rkisp1: resizer: Use V4L2 subdev active state
2023-01-27 0:31 ` Laurent Pinchart
@ 2023-04-27 3:52 ` Paul Elder
-1 siblings, 0 replies; 16+ messages in thread
From: Paul Elder @ 2023-04-27 3:52 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Dafna Hirschfeld, Heiko Stuebner, linux-rockchip
On Fri, Jan 27, 2023 at 02:31:22AM +0200, Laurent Pinchart wrote:
> Use the V4L2 subdev active state API to store the active format and crop
> rectangle. This simplifies the driver not only by dropping the state
> stored in the rkisp1_resizer structure, but also by replacing the
> ops_lock with the state lock.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
> .../platform/rockchip/rkisp1/rkisp1-common.h | 6 -
> .../platform/rockchip/rkisp1/rkisp1-resizer.c | 184 +++++++-----------
> 2 files changed, 66 insertions(+), 124 deletions(-)
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> index a1293c45aae1..e9626e59e1c8 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> @@ -390,10 +390,7 @@ struct rkisp1_params {
> * @id: id of the resizer, one of RKISP1_SELFPATH, RKISP1_MAINPATH
> * @rkisp1: pointer to the rkisp1 device
> * @pads: media pads
> - * @pad_cfg: configurations for the pads
> * @config: the set of registers to configure the resizer
> - * @pixel_enc: pixel encoding of the resizer
> - * @ops_lock: a lock for the subdev ops
> */
> struct rkisp1_resizer {
> struct v4l2_subdev sd;
> @@ -401,10 +398,7 @@ struct rkisp1_resizer {
> enum rkisp1_stream_id id;
> struct rkisp1_device *rkisp1;
> struct media_pad pads[RKISP1_RSZ_PAD_MAX];
> - struct v4l2_subdev_pad_config pad_cfg[RKISP1_RSZ_PAD_MAX];
> const struct rkisp1_rsz_config *config;
> - enum v4l2_pixel_encoding pixel_enc;
> - struct mutex ops_lock; /* serialize the subdevice ops */
> };
>
> /*
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
> index f76afd8112b2..62728ee6d058 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
> @@ -117,34 +117,6 @@ static inline void rkisp1_rsz_write(struct rkisp1_resizer *rsz, u32 offset,
> rkisp1_write(rsz->rkisp1, rsz->regs_base + offset, value);
> }
>
> -static struct v4l2_mbus_framefmt *
> -rkisp1_rsz_get_pad_fmt(struct rkisp1_resizer *rsz,
> - struct v4l2_subdev_state *sd_state,
> - unsigned int pad, u32 which)
> -{
> - struct v4l2_subdev_state state = {
> - .pads = rsz->pad_cfg
> - };
> - if (which == V4L2_SUBDEV_FORMAT_TRY)
> - return v4l2_subdev_get_try_format(&rsz->sd, sd_state, pad);
> - else
> - return v4l2_subdev_get_try_format(&rsz->sd, &state, pad);
> -}
> -
> -static struct v4l2_rect *
> -rkisp1_rsz_get_pad_crop(struct rkisp1_resizer *rsz,
> - struct v4l2_subdev_state *sd_state,
> - unsigned int pad, u32 which)
> -{
> - struct v4l2_subdev_state state = {
> - .pads = rsz->pad_cfg
> - };
> - if (which == V4L2_SUBDEV_FORMAT_TRY)
> - return v4l2_subdev_get_try_crop(&rsz->sd, sd_state, pad);
> - else
> - return v4l2_subdev_get_try_crop(&rsz->sd, &state, pad);
> -}
> -
> /* ----------------------------------------------------------------------------
> * Dual crop hw configs
> */
> @@ -165,17 +137,18 @@ static void rkisp1_dcrop_disable(struct rkisp1_resizer *rsz,
> }
>
> /* configure dual-crop unit */
> -static void rkisp1_dcrop_config(struct rkisp1_resizer *rsz)
> +static void rkisp1_dcrop_config(struct rkisp1_resizer *rsz,
> + struct v4l2_subdev_state *sd_state)
> {
> struct rkisp1_device *rkisp1 = rsz->rkisp1;
> struct v4l2_mbus_framefmt *sink_fmt;
> struct v4l2_rect *sink_crop;
> u32 dc_ctrl;
>
> - sink_crop = rkisp1_rsz_get_pad_crop(rsz, NULL, RKISP1_RSZ_PAD_SINK,
> - V4L2_SUBDEV_FORMAT_ACTIVE);
> - sink_fmt = rkisp1_rsz_get_pad_fmt(rsz, NULL, RKISP1_RSZ_PAD_SINK,
> - V4L2_SUBDEV_FORMAT_ACTIVE);
> + sink_crop = v4l2_subdev_get_pad_crop(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SINK);
> + sink_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SINK);
>
> if (sink_crop->width == sink_fmt->width &&
> sink_crop->height == sink_fmt->height &&
> @@ -296,6 +269,7 @@ static void rkisp1_rsz_config_regs(struct rkisp1_resizer *rsz,
> }
>
> static void rkisp1_rsz_config(struct rkisp1_resizer *rsz,
> + struct v4l2_subdev_state *sd_state,
> enum rkisp1_shadow_regs_when when)
> {
> const struct rkisp1_rsz_yuv_mbus_info *sink_yuv_info, *src_yuv_info;
> @@ -303,20 +277,21 @@ static void rkisp1_rsz_config(struct rkisp1_resizer *rsz,
> struct v4l2_mbus_framefmt *src_fmt, *sink_fmt;
> struct v4l2_rect *sink_crop;
>
> - sink_crop = rkisp1_rsz_get_pad_crop(rsz, NULL, RKISP1_RSZ_PAD_SINK,
> - V4L2_SUBDEV_FORMAT_ACTIVE);
> - src_fmt = rkisp1_rsz_get_pad_fmt(rsz, NULL, RKISP1_RSZ_PAD_SRC,
> - V4L2_SUBDEV_FORMAT_ACTIVE);
> - src_yuv_info = rkisp1_rsz_get_yuv_mbus_info(src_fmt->code);
> - sink_fmt = rkisp1_rsz_get_pad_fmt(rsz, NULL, RKISP1_RSZ_PAD_SINK,
> - V4L2_SUBDEV_FORMAT_ACTIVE);
> + sink_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SINK);
> + sink_crop = v4l2_subdev_get_pad_crop(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SINK);
> + src_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SRC);
> +
> sink_yuv_info = rkisp1_rsz_get_yuv_mbus_info(sink_fmt->code);
> + src_yuv_info = rkisp1_rsz_get_yuv_mbus_info(src_fmt->code);
>
> /*
> * The resizer only works on yuv formats,
> * so return if it is bayer format.
> */
> - if (rsz->pixel_enc == V4L2_PIXEL_ENC_BAYER) {
> + if (!sink_yuv_info) {
> rkisp1_rsz_disable(rsz, when);
> return;
> }
> @@ -405,7 +380,7 @@ static int rkisp1_rsz_init_config(struct v4l2_subdev *sd,
> struct v4l2_mbus_framefmt *sink_fmt, *src_fmt;
> struct v4l2_rect *sink_crop;
>
> - sink_fmt = v4l2_subdev_get_try_format(sd, sd_state,
> + sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
> RKISP1_RSZ_PAD_SRC);
> sink_fmt->width = RKISP1_DEFAULT_WIDTH;
> sink_fmt->height = RKISP1_DEFAULT_HEIGHT;
> @@ -423,7 +398,7 @@ static int rkisp1_rsz_init_config(struct v4l2_subdev *sd,
> sink_crop->left = 0;
> sink_crop->top = 0;
>
> - src_fmt = v4l2_subdev_get_try_format(sd, sd_state,
> + src_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
> RKISP1_RSZ_PAD_SINK);
> *src_fmt = *sink_fmt;
>
> @@ -434,16 +409,16 @@ static int rkisp1_rsz_init_config(struct v4l2_subdev *sd,
>
> static void rkisp1_rsz_set_src_fmt(struct rkisp1_resizer *rsz,
> struct v4l2_subdev_state *sd_state,
> - struct v4l2_mbus_framefmt *format,
> - unsigned int which)
> + struct v4l2_mbus_framefmt *format)
> {
> const struct rkisp1_mbus_info *sink_mbus_info;
> struct v4l2_mbus_framefmt *src_fmt, *sink_fmt;
>
> - sink_fmt = rkisp1_rsz_get_pad_fmt(rsz, sd_state, RKISP1_RSZ_PAD_SINK,
> - which);
> - src_fmt = rkisp1_rsz_get_pad_fmt(rsz, sd_state, RKISP1_RSZ_PAD_SRC,
> - which);
> + sink_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SINK);
> + src_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SRC);
> +
> sink_mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code);
>
> /* for YUV formats, userspace can change the mbus code on the src pad if it is supported */
> @@ -463,18 +438,16 @@ static void rkisp1_rsz_set_src_fmt(struct rkisp1_resizer *rsz,
>
> static void rkisp1_rsz_set_sink_crop(struct rkisp1_resizer *rsz,
> struct v4l2_subdev_state *sd_state,
> - struct v4l2_rect *r,
> - unsigned int which)
> + struct v4l2_rect *r)
> {
> const struct rkisp1_mbus_info *mbus_info;
> struct v4l2_mbus_framefmt *sink_fmt;
> struct v4l2_rect *sink_crop;
>
> - sink_fmt = rkisp1_rsz_get_pad_fmt(rsz, sd_state, RKISP1_RSZ_PAD_SINK,
> - which);
> - sink_crop = rkisp1_rsz_get_pad_crop(rsz, sd_state,
> - RKISP1_RSZ_PAD_SINK,
> - which);
> + sink_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SINK);
> + sink_crop = v4l2_subdev_get_pad_crop(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SINK);
>
> /* Not crop for MP bayer raw data */
> mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code);
> @@ -501,21 +474,20 @@ static void rkisp1_rsz_set_sink_crop(struct rkisp1_resizer *rsz,
>
> static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz,
> struct v4l2_subdev_state *sd_state,
> - struct v4l2_mbus_framefmt *format,
> - unsigned int which)
> + struct v4l2_mbus_framefmt *format)
> {
> const struct rkisp1_mbus_info *mbus_info;
> struct v4l2_mbus_framefmt *sink_fmt, *src_fmt;
> struct v4l2_rect *sink_crop;
> bool is_yuv;
>
> - sink_fmt = rkisp1_rsz_get_pad_fmt(rsz, sd_state, RKISP1_RSZ_PAD_SINK,
> - which);
> - src_fmt = rkisp1_rsz_get_pad_fmt(rsz, sd_state, RKISP1_RSZ_PAD_SRC,
> - which);
> - sink_crop = rkisp1_rsz_get_pad_crop(rsz, sd_state,
> - RKISP1_RSZ_PAD_SINK,
> - which);
> + sink_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SINK);
> + src_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SRC);
> + sink_crop = v4l2_subdev_get_pad_crop(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SINK);
> +
> if (rsz->id == RKISP1_SELFPATH)
> sink_fmt->code = MEDIA_BUS_FMT_YUYV8_2X8;
> else
> @@ -526,8 +498,6 @@ static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz,
> sink_fmt->code = RKISP1_DEF_FMT;
> mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code);
> }
> - if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
> - rsz->pixel_enc = mbus_info->pixel_enc;
>
> sink_fmt->width = clamp_t(u32, format->width,
> RKISP1_ISP_MIN_WIDTH,
> @@ -576,21 +546,7 @@ static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz,
> src_fmt->quantization = sink_fmt->quantization;
>
> /* Update sink crop */
> - rkisp1_rsz_set_sink_crop(rsz, sd_state, sink_crop, which);
> -}
> -
> -static int rkisp1_rsz_get_fmt(struct v4l2_subdev *sd,
> - struct v4l2_subdev_state *sd_state,
> - struct v4l2_subdev_format *fmt)
> -{
> - struct rkisp1_resizer *rsz =
> - container_of(sd, struct rkisp1_resizer, sd);
> -
> - mutex_lock(&rsz->ops_lock);
> - fmt->format = *rkisp1_rsz_get_pad_fmt(rsz, sd_state, fmt->pad,
> - fmt->which);
> - mutex_unlock(&rsz->ops_lock);
> - return 0;
> + rkisp1_rsz_set_sink_crop(rsz, sd_state, sink_crop);
> }
>
> static int rkisp1_rsz_set_fmt(struct v4l2_subdev *sd,
> @@ -600,15 +556,11 @@ static int rkisp1_rsz_set_fmt(struct v4l2_subdev *sd,
> struct rkisp1_resizer *rsz =
> container_of(sd, struct rkisp1_resizer, sd);
>
> - mutex_lock(&rsz->ops_lock);
> if (fmt->pad == RKISP1_RSZ_PAD_SINK)
> - rkisp1_rsz_set_sink_fmt(rsz, sd_state, &fmt->format,
> - fmt->which);
> + rkisp1_rsz_set_sink_fmt(rsz, sd_state, &fmt->format);
> else
> - rkisp1_rsz_set_src_fmt(rsz, sd_state, &fmt->format,
> - fmt->which);
> + rkisp1_rsz_set_src_fmt(rsz, sd_state, &fmt->format);
>
> - mutex_unlock(&rsz->ops_lock);
> return 0;
> }
>
> @@ -616,35 +568,32 @@ static int rkisp1_rsz_get_selection(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *sd_state,
> struct v4l2_subdev_selection *sel)
> {
> - struct rkisp1_resizer *rsz =
> - container_of(sd, struct rkisp1_resizer, sd);
> struct v4l2_mbus_framefmt *mf_sink;
> int ret = 0;
>
> if (sel->pad == RKISP1_RSZ_PAD_SRC)
> return -EINVAL;
>
> - mutex_lock(&rsz->ops_lock);
> switch (sel->target) {
> case V4L2_SEL_TGT_CROP_BOUNDS:
> - mf_sink = rkisp1_rsz_get_pad_fmt(rsz, sd_state,
> - RKISP1_RSZ_PAD_SINK,
> - sel->which);
> + mf_sink = v4l2_subdev_get_pad_format(sd, sd_state,
> + RKISP1_RSZ_PAD_SINK);
> sel->r.height = mf_sink->height;
> sel->r.width = mf_sink->width;
> sel->r.left = 0;
> sel->r.top = 0;
> break;
> +
> case V4L2_SEL_TGT_CROP:
> - sel->r = *rkisp1_rsz_get_pad_crop(rsz, sd_state,
> - RKISP1_RSZ_PAD_SINK,
> - sel->which);
> + sel->r = *v4l2_subdev_get_pad_crop(sd, sd_state,
> + RKISP1_RSZ_PAD_SINK);
> break;
> +
> default:
> ret = -EINVAL;
> + break;
> }
>
> - mutex_unlock(&rsz->ops_lock);
> return ret;
> }
>
> @@ -661,9 +610,7 @@ static int rkisp1_rsz_set_selection(struct v4l2_subdev *sd,
> dev_dbg(rsz->rkisp1->dev, "%s: pad: %d sel(%d,%d)/%dx%d\n", __func__,
> sel->pad, sel->r.left, sel->r.top, sel->r.width, sel->r.height);
>
> - mutex_lock(&rsz->ops_lock);
> - rkisp1_rsz_set_sink_crop(rsz, sd_state, &sel->r, sel->which);
> - mutex_unlock(&rsz->ops_lock);
> + rkisp1_rsz_set_sink_crop(rsz, sd_state, &sel->r);
>
> return 0;
> }
> @@ -677,7 +624,7 @@ static const struct v4l2_subdev_pad_ops rkisp1_rsz_pad_ops = {
> .get_selection = rkisp1_rsz_get_selection,
> .set_selection = rkisp1_rsz_set_selection,
> .init_cfg = rkisp1_rsz_init_config,
> - .get_fmt = rkisp1_rsz_get_fmt,
> + .get_fmt = v4l2_subdev_get_fmt,
> .set_fmt = rkisp1_rsz_set_fmt,
> .link_validate = v4l2_subdev_link_validate_default,
> };
> @@ -693,6 +640,7 @@ static int rkisp1_rsz_s_stream(struct v4l2_subdev *sd, int enable)
> struct rkisp1_device *rkisp1 = rsz->rkisp1;
> struct rkisp1_capture *other = &rkisp1->capture_devs[rsz->id ^ 1];
> enum rkisp1_shadow_regs_when when = RKISP1_SHADOW_REGS_SYNC;
> + struct v4l2_subdev_state *sd_state;
>
> if (!enable) {
> rkisp1_dcrop_disable(rsz, RKISP1_SHADOW_REGS_ASYNC);
> @@ -703,11 +651,13 @@ static int rkisp1_rsz_s_stream(struct v4l2_subdev *sd, int enable)
> if (other->is_streaming)
> when = RKISP1_SHADOW_REGS_ASYNC;
>
> - mutex_lock(&rsz->ops_lock);
> - rkisp1_rsz_config(rsz, when);
> - rkisp1_dcrop_config(rsz);
> + sd_state = v4l2_subdev_lock_and_get_active_state(sd);
> +
> + rkisp1_rsz_config(rsz, sd_state, when);
> + rkisp1_dcrop_config(rsz, sd_state);
> +
> + v4l2_subdev_unlock_state(sd_state);
>
> - mutex_unlock(&rsz->ops_lock);
> return 0;
> }
>
> @@ -726,15 +676,12 @@ static void rkisp1_rsz_unregister(struct rkisp1_resizer *rsz)
> return;
>
> v4l2_device_unregister_subdev(&rsz->sd);
> + v4l2_subdev_cleanup(&rsz->sd);
> media_entity_cleanup(&rsz->sd.entity);
> - mutex_destroy(&rsz->ops_lock);
> }
>
> static int rkisp1_rsz_register(struct rkisp1_resizer *rsz)
> {
> - struct v4l2_subdev_state state = {
> - .pads = rsz->pad_cfg
> - };
> static const char * const dev_names[] = {
> RKISP1_RSZ_MP_DEV_NAME,
> RKISP1_RSZ_SP_DEV_NAME
> @@ -763,25 +710,26 @@ static int rkisp1_rsz_register(struct rkisp1_resizer *rsz)
> pads[RKISP1_RSZ_PAD_SRC].flags = MEDIA_PAD_FL_SOURCE |
> MEDIA_PAD_FL_MUST_CONNECT;
>
> - rsz->pixel_enc = RKISP1_DEF_PIXEL_ENC;
> -
> - mutex_init(&rsz->ops_lock);
> ret = media_entity_pads_init(&sd->entity, RKISP1_RSZ_PAD_MAX, pads);
> if (ret)
> - goto error;
> + goto err_entity_cleanup;
> +
> + ret = v4l2_subdev_init_finalize(sd);
> + if (ret)
> + goto err_entity_cleanup;
>
> ret = v4l2_device_register_subdev(&rsz->rkisp1->v4l2_dev, sd);
> if (ret) {
> dev_err(sd->dev, "Failed to register resizer subdev\n");
> - goto error;
> + goto err_subdev_cleanup;
> }
>
> - rkisp1_rsz_init_config(sd, &state);
> return 0;
>
> -error:
> +err_subdev_cleanup:
> + v4l2_subdev_cleanup(sd);
> +err_entity_cleanup:
> media_entity_cleanup(&sd->entity);
> - mutex_destroy(&rsz->ops_lock);
> return ret;
> }
>
> --
> Regards,
>
> Laurent Pinchart
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 1/3] media: rkisp1: resizer: Use V4L2 subdev active state
@ 2023-04-27 3:52 ` Paul Elder
0 siblings, 0 replies; 16+ messages in thread
From: Paul Elder @ 2023-04-27 3:52 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Dafna Hirschfeld, Heiko Stuebner, linux-rockchip
On Fri, Jan 27, 2023 at 02:31:22AM +0200, Laurent Pinchart wrote:
> Use the V4L2 subdev active state API to store the active format and crop
> rectangle. This simplifies the driver not only by dropping the state
> stored in the rkisp1_resizer structure, but also by replacing the
> ops_lock with the state lock.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
> .../platform/rockchip/rkisp1/rkisp1-common.h | 6 -
> .../platform/rockchip/rkisp1/rkisp1-resizer.c | 184 +++++++-----------
> 2 files changed, 66 insertions(+), 124 deletions(-)
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> index a1293c45aae1..e9626e59e1c8 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> @@ -390,10 +390,7 @@ struct rkisp1_params {
> * @id: id of the resizer, one of RKISP1_SELFPATH, RKISP1_MAINPATH
> * @rkisp1: pointer to the rkisp1 device
> * @pads: media pads
> - * @pad_cfg: configurations for the pads
> * @config: the set of registers to configure the resizer
> - * @pixel_enc: pixel encoding of the resizer
> - * @ops_lock: a lock for the subdev ops
> */
> struct rkisp1_resizer {
> struct v4l2_subdev sd;
> @@ -401,10 +398,7 @@ struct rkisp1_resizer {
> enum rkisp1_stream_id id;
> struct rkisp1_device *rkisp1;
> struct media_pad pads[RKISP1_RSZ_PAD_MAX];
> - struct v4l2_subdev_pad_config pad_cfg[RKISP1_RSZ_PAD_MAX];
> const struct rkisp1_rsz_config *config;
> - enum v4l2_pixel_encoding pixel_enc;
> - struct mutex ops_lock; /* serialize the subdevice ops */
> };
>
> /*
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
> index f76afd8112b2..62728ee6d058 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
> @@ -117,34 +117,6 @@ static inline void rkisp1_rsz_write(struct rkisp1_resizer *rsz, u32 offset,
> rkisp1_write(rsz->rkisp1, rsz->regs_base + offset, value);
> }
>
> -static struct v4l2_mbus_framefmt *
> -rkisp1_rsz_get_pad_fmt(struct rkisp1_resizer *rsz,
> - struct v4l2_subdev_state *sd_state,
> - unsigned int pad, u32 which)
> -{
> - struct v4l2_subdev_state state = {
> - .pads = rsz->pad_cfg
> - };
> - if (which == V4L2_SUBDEV_FORMAT_TRY)
> - return v4l2_subdev_get_try_format(&rsz->sd, sd_state, pad);
> - else
> - return v4l2_subdev_get_try_format(&rsz->sd, &state, pad);
> -}
> -
> -static struct v4l2_rect *
> -rkisp1_rsz_get_pad_crop(struct rkisp1_resizer *rsz,
> - struct v4l2_subdev_state *sd_state,
> - unsigned int pad, u32 which)
> -{
> - struct v4l2_subdev_state state = {
> - .pads = rsz->pad_cfg
> - };
> - if (which == V4L2_SUBDEV_FORMAT_TRY)
> - return v4l2_subdev_get_try_crop(&rsz->sd, sd_state, pad);
> - else
> - return v4l2_subdev_get_try_crop(&rsz->sd, &state, pad);
> -}
> -
> /* ----------------------------------------------------------------------------
> * Dual crop hw configs
> */
> @@ -165,17 +137,18 @@ static void rkisp1_dcrop_disable(struct rkisp1_resizer *rsz,
> }
>
> /* configure dual-crop unit */
> -static void rkisp1_dcrop_config(struct rkisp1_resizer *rsz)
> +static void rkisp1_dcrop_config(struct rkisp1_resizer *rsz,
> + struct v4l2_subdev_state *sd_state)
> {
> struct rkisp1_device *rkisp1 = rsz->rkisp1;
> struct v4l2_mbus_framefmt *sink_fmt;
> struct v4l2_rect *sink_crop;
> u32 dc_ctrl;
>
> - sink_crop = rkisp1_rsz_get_pad_crop(rsz, NULL, RKISP1_RSZ_PAD_SINK,
> - V4L2_SUBDEV_FORMAT_ACTIVE);
> - sink_fmt = rkisp1_rsz_get_pad_fmt(rsz, NULL, RKISP1_RSZ_PAD_SINK,
> - V4L2_SUBDEV_FORMAT_ACTIVE);
> + sink_crop = v4l2_subdev_get_pad_crop(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SINK);
> + sink_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SINK);
>
> if (sink_crop->width == sink_fmt->width &&
> sink_crop->height == sink_fmt->height &&
> @@ -296,6 +269,7 @@ static void rkisp1_rsz_config_regs(struct rkisp1_resizer *rsz,
> }
>
> static void rkisp1_rsz_config(struct rkisp1_resizer *rsz,
> + struct v4l2_subdev_state *sd_state,
> enum rkisp1_shadow_regs_when when)
> {
> const struct rkisp1_rsz_yuv_mbus_info *sink_yuv_info, *src_yuv_info;
> @@ -303,20 +277,21 @@ static void rkisp1_rsz_config(struct rkisp1_resizer *rsz,
> struct v4l2_mbus_framefmt *src_fmt, *sink_fmt;
> struct v4l2_rect *sink_crop;
>
> - sink_crop = rkisp1_rsz_get_pad_crop(rsz, NULL, RKISP1_RSZ_PAD_SINK,
> - V4L2_SUBDEV_FORMAT_ACTIVE);
> - src_fmt = rkisp1_rsz_get_pad_fmt(rsz, NULL, RKISP1_RSZ_PAD_SRC,
> - V4L2_SUBDEV_FORMAT_ACTIVE);
> - src_yuv_info = rkisp1_rsz_get_yuv_mbus_info(src_fmt->code);
> - sink_fmt = rkisp1_rsz_get_pad_fmt(rsz, NULL, RKISP1_RSZ_PAD_SINK,
> - V4L2_SUBDEV_FORMAT_ACTIVE);
> + sink_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SINK);
> + sink_crop = v4l2_subdev_get_pad_crop(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SINK);
> + src_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SRC);
> +
> sink_yuv_info = rkisp1_rsz_get_yuv_mbus_info(sink_fmt->code);
> + src_yuv_info = rkisp1_rsz_get_yuv_mbus_info(src_fmt->code);
>
> /*
> * The resizer only works on yuv formats,
> * so return if it is bayer format.
> */
> - if (rsz->pixel_enc == V4L2_PIXEL_ENC_BAYER) {
> + if (!sink_yuv_info) {
> rkisp1_rsz_disable(rsz, when);
> return;
> }
> @@ -405,7 +380,7 @@ static int rkisp1_rsz_init_config(struct v4l2_subdev *sd,
> struct v4l2_mbus_framefmt *sink_fmt, *src_fmt;
> struct v4l2_rect *sink_crop;
>
> - sink_fmt = v4l2_subdev_get_try_format(sd, sd_state,
> + sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
> RKISP1_RSZ_PAD_SRC);
> sink_fmt->width = RKISP1_DEFAULT_WIDTH;
> sink_fmt->height = RKISP1_DEFAULT_HEIGHT;
> @@ -423,7 +398,7 @@ static int rkisp1_rsz_init_config(struct v4l2_subdev *sd,
> sink_crop->left = 0;
> sink_crop->top = 0;
>
> - src_fmt = v4l2_subdev_get_try_format(sd, sd_state,
> + src_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
> RKISP1_RSZ_PAD_SINK);
> *src_fmt = *sink_fmt;
>
> @@ -434,16 +409,16 @@ static int rkisp1_rsz_init_config(struct v4l2_subdev *sd,
>
> static void rkisp1_rsz_set_src_fmt(struct rkisp1_resizer *rsz,
> struct v4l2_subdev_state *sd_state,
> - struct v4l2_mbus_framefmt *format,
> - unsigned int which)
> + struct v4l2_mbus_framefmt *format)
> {
> const struct rkisp1_mbus_info *sink_mbus_info;
> struct v4l2_mbus_framefmt *src_fmt, *sink_fmt;
>
> - sink_fmt = rkisp1_rsz_get_pad_fmt(rsz, sd_state, RKISP1_RSZ_PAD_SINK,
> - which);
> - src_fmt = rkisp1_rsz_get_pad_fmt(rsz, sd_state, RKISP1_RSZ_PAD_SRC,
> - which);
> + sink_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SINK);
> + src_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SRC);
> +
> sink_mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code);
>
> /* for YUV formats, userspace can change the mbus code on the src pad if it is supported */
> @@ -463,18 +438,16 @@ static void rkisp1_rsz_set_src_fmt(struct rkisp1_resizer *rsz,
>
> static void rkisp1_rsz_set_sink_crop(struct rkisp1_resizer *rsz,
> struct v4l2_subdev_state *sd_state,
> - struct v4l2_rect *r,
> - unsigned int which)
> + struct v4l2_rect *r)
> {
> const struct rkisp1_mbus_info *mbus_info;
> struct v4l2_mbus_framefmt *sink_fmt;
> struct v4l2_rect *sink_crop;
>
> - sink_fmt = rkisp1_rsz_get_pad_fmt(rsz, sd_state, RKISP1_RSZ_PAD_SINK,
> - which);
> - sink_crop = rkisp1_rsz_get_pad_crop(rsz, sd_state,
> - RKISP1_RSZ_PAD_SINK,
> - which);
> + sink_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SINK);
> + sink_crop = v4l2_subdev_get_pad_crop(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SINK);
>
> /* Not crop for MP bayer raw data */
> mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code);
> @@ -501,21 +474,20 @@ static void rkisp1_rsz_set_sink_crop(struct rkisp1_resizer *rsz,
>
> static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz,
> struct v4l2_subdev_state *sd_state,
> - struct v4l2_mbus_framefmt *format,
> - unsigned int which)
> + struct v4l2_mbus_framefmt *format)
> {
> const struct rkisp1_mbus_info *mbus_info;
> struct v4l2_mbus_framefmt *sink_fmt, *src_fmt;
> struct v4l2_rect *sink_crop;
> bool is_yuv;
>
> - sink_fmt = rkisp1_rsz_get_pad_fmt(rsz, sd_state, RKISP1_RSZ_PAD_SINK,
> - which);
> - src_fmt = rkisp1_rsz_get_pad_fmt(rsz, sd_state, RKISP1_RSZ_PAD_SRC,
> - which);
> - sink_crop = rkisp1_rsz_get_pad_crop(rsz, sd_state,
> - RKISP1_RSZ_PAD_SINK,
> - which);
> + sink_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SINK);
> + src_fmt = v4l2_subdev_get_pad_format(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SRC);
> + sink_crop = v4l2_subdev_get_pad_crop(&rsz->sd, sd_state,
> + RKISP1_RSZ_PAD_SINK);
> +
> if (rsz->id == RKISP1_SELFPATH)
> sink_fmt->code = MEDIA_BUS_FMT_YUYV8_2X8;
> else
> @@ -526,8 +498,6 @@ static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz,
> sink_fmt->code = RKISP1_DEF_FMT;
> mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code);
> }
> - if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
> - rsz->pixel_enc = mbus_info->pixel_enc;
>
> sink_fmt->width = clamp_t(u32, format->width,
> RKISP1_ISP_MIN_WIDTH,
> @@ -576,21 +546,7 @@ static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz,
> src_fmt->quantization = sink_fmt->quantization;
>
> /* Update sink crop */
> - rkisp1_rsz_set_sink_crop(rsz, sd_state, sink_crop, which);
> -}
> -
> -static int rkisp1_rsz_get_fmt(struct v4l2_subdev *sd,
> - struct v4l2_subdev_state *sd_state,
> - struct v4l2_subdev_format *fmt)
> -{
> - struct rkisp1_resizer *rsz =
> - container_of(sd, struct rkisp1_resizer, sd);
> -
> - mutex_lock(&rsz->ops_lock);
> - fmt->format = *rkisp1_rsz_get_pad_fmt(rsz, sd_state, fmt->pad,
> - fmt->which);
> - mutex_unlock(&rsz->ops_lock);
> - return 0;
> + rkisp1_rsz_set_sink_crop(rsz, sd_state, sink_crop);
> }
>
> static int rkisp1_rsz_set_fmt(struct v4l2_subdev *sd,
> @@ -600,15 +556,11 @@ static int rkisp1_rsz_set_fmt(struct v4l2_subdev *sd,
> struct rkisp1_resizer *rsz =
> container_of(sd, struct rkisp1_resizer, sd);
>
> - mutex_lock(&rsz->ops_lock);
> if (fmt->pad == RKISP1_RSZ_PAD_SINK)
> - rkisp1_rsz_set_sink_fmt(rsz, sd_state, &fmt->format,
> - fmt->which);
> + rkisp1_rsz_set_sink_fmt(rsz, sd_state, &fmt->format);
> else
> - rkisp1_rsz_set_src_fmt(rsz, sd_state, &fmt->format,
> - fmt->which);
> + rkisp1_rsz_set_src_fmt(rsz, sd_state, &fmt->format);
>
> - mutex_unlock(&rsz->ops_lock);
> return 0;
> }
>
> @@ -616,35 +568,32 @@ static int rkisp1_rsz_get_selection(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *sd_state,
> struct v4l2_subdev_selection *sel)
> {
> - struct rkisp1_resizer *rsz =
> - container_of(sd, struct rkisp1_resizer, sd);
> struct v4l2_mbus_framefmt *mf_sink;
> int ret = 0;
>
> if (sel->pad == RKISP1_RSZ_PAD_SRC)
> return -EINVAL;
>
> - mutex_lock(&rsz->ops_lock);
> switch (sel->target) {
> case V4L2_SEL_TGT_CROP_BOUNDS:
> - mf_sink = rkisp1_rsz_get_pad_fmt(rsz, sd_state,
> - RKISP1_RSZ_PAD_SINK,
> - sel->which);
> + mf_sink = v4l2_subdev_get_pad_format(sd, sd_state,
> + RKISP1_RSZ_PAD_SINK);
> sel->r.height = mf_sink->height;
> sel->r.width = mf_sink->width;
> sel->r.left = 0;
> sel->r.top = 0;
> break;
> +
> case V4L2_SEL_TGT_CROP:
> - sel->r = *rkisp1_rsz_get_pad_crop(rsz, sd_state,
> - RKISP1_RSZ_PAD_SINK,
> - sel->which);
> + sel->r = *v4l2_subdev_get_pad_crop(sd, sd_state,
> + RKISP1_RSZ_PAD_SINK);
> break;
> +
> default:
> ret = -EINVAL;
> + break;
> }
>
> - mutex_unlock(&rsz->ops_lock);
> return ret;
> }
>
> @@ -661,9 +610,7 @@ static int rkisp1_rsz_set_selection(struct v4l2_subdev *sd,
> dev_dbg(rsz->rkisp1->dev, "%s: pad: %d sel(%d,%d)/%dx%d\n", __func__,
> sel->pad, sel->r.left, sel->r.top, sel->r.width, sel->r.height);
>
> - mutex_lock(&rsz->ops_lock);
> - rkisp1_rsz_set_sink_crop(rsz, sd_state, &sel->r, sel->which);
> - mutex_unlock(&rsz->ops_lock);
> + rkisp1_rsz_set_sink_crop(rsz, sd_state, &sel->r);
>
> return 0;
> }
> @@ -677,7 +624,7 @@ static const struct v4l2_subdev_pad_ops rkisp1_rsz_pad_ops = {
> .get_selection = rkisp1_rsz_get_selection,
> .set_selection = rkisp1_rsz_set_selection,
> .init_cfg = rkisp1_rsz_init_config,
> - .get_fmt = rkisp1_rsz_get_fmt,
> + .get_fmt = v4l2_subdev_get_fmt,
> .set_fmt = rkisp1_rsz_set_fmt,
> .link_validate = v4l2_subdev_link_validate_default,
> };
> @@ -693,6 +640,7 @@ static int rkisp1_rsz_s_stream(struct v4l2_subdev *sd, int enable)
> struct rkisp1_device *rkisp1 = rsz->rkisp1;
> struct rkisp1_capture *other = &rkisp1->capture_devs[rsz->id ^ 1];
> enum rkisp1_shadow_regs_when when = RKISP1_SHADOW_REGS_SYNC;
> + struct v4l2_subdev_state *sd_state;
>
> if (!enable) {
> rkisp1_dcrop_disable(rsz, RKISP1_SHADOW_REGS_ASYNC);
> @@ -703,11 +651,13 @@ static int rkisp1_rsz_s_stream(struct v4l2_subdev *sd, int enable)
> if (other->is_streaming)
> when = RKISP1_SHADOW_REGS_ASYNC;
>
> - mutex_lock(&rsz->ops_lock);
> - rkisp1_rsz_config(rsz, when);
> - rkisp1_dcrop_config(rsz);
> + sd_state = v4l2_subdev_lock_and_get_active_state(sd);
> +
> + rkisp1_rsz_config(rsz, sd_state, when);
> + rkisp1_dcrop_config(rsz, sd_state);
> +
> + v4l2_subdev_unlock_state(sd_state);
>
> - mutex_unlock(&rsz->ops_lock);
> return 0;
> }
>
> @@ -726,15 +676,12 @@ static void rkisp1_rsz_unregister(struct rkisp1_resizer *rsz)
> return;
>
> v4l2_device_unregister_subdev(&rsz->sd);
> + v4l2_subdev_cleanup(&rsz->sd);
> media_entity_cleanup(&rsz->sd.entity);
> - mutex_destroy(&rsz->ops_lock);
> }
>
> static int rkisp1_rsz_register(struct rkisp1_resizer *rsz)
> {
> - struct v4l2_subdev_state state = {
> - .pads = rsz->pad_cfg
> - };
> static const char * const dev_names[] = {
> RKISP1_RSZ_MP_DEV_NAME,
> RKISP1_RSZ_SP_DEV_NAME
> @@ -763,25 +710,26 @@ static int rkisp1_rsz_register(struct rkisp1_resizer *rsz)
> pads[RKISP1_RSZ_PAD_SRC].flags = MEDIA_PAD_FL_SOURCE |
> MEDIA_PAD_FL_MUST_CONNECT;
>
> - rsz->pixel_enc = RKISP1_DEF_PIXEL_ENC;
> -
> - mutex_init(&rsz->ops_lock);
> ret = media_entity_pads_init(&sd->entity, RKISP1_RSZ_PAD_MAX, pads);
> if (ret)
> - goto error;
> + goto err_entity_cleanup;
> +
> + ret = v4l2_subdev_init_finalize(sd);
> + if (ret)
> + goto err_entity_cleanup;
>
> ret = v4l2_device_register_subdev(&rsz->rkisp1->v4l2_dev, sd);
> if (ret) {
> dev_err(sd->dev, "Failed to register resizer subdev\n");
> - goto error;
> + goto err_subdev_cleanup;
> }
>
> - rkisp1_rsz_init_config(sd, &state);
> return 0;
>
> -error:
> +err_subdev_cleanup:
> + v4l2_subdev_cleanup(sd);
> +err_entity_cleanup:
> media_entity_cleanup(&sd->entity);
> - mutex_destroy(&rsz->ops_lock);
> return ret;
> }
>
> --
> Regards,
>
> Laurent Pinchart
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 3/3] media: rkisp1: csi: Use V4L2 subdev active state
2023-01-27 0:31 ` Laurent Pinchart
@ 2023-04-27 3:52 ` Paul Elder
-1 siblings, 0 replies; 16+ messages in thread
From: Paul Elder @ 2023-04-27 3:52 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Dafna Hirschfeld, Heiko Stuebner, linux-rockchip
On Fri, Jan 27, 2023 at 02:31:24AM +0200, Laurent Pinchart wrote:
> Use the V4L2 subdev active state API to store the active format and crop
> rectangle. This simplifies the driver not only by dropping the state
> stored in the rkisp1_csi structure, but also by replacing the ops_lock
> with the state lock.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
> .../platform/rockchip/rkisp1/rkisp1-common.h | 6 -
> .../platform/rockchip/rkisp1/rkisp1-csi.c | 107 ++++++------------
> 2 files changed, 33 insertions(+), 80 deletions(-)
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> index d43ff1b549d9..dbd8725f2ec9 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> @@ -167,9 +167,6 @@ struct rkisp1_sensor_async {
> * @is_dphy_errctrl_disabled: if dphy errctrl is disabled (avoid endless interrupt)
> * @sd: v4l2_subdev variable
> * @pads: media pads
> - * @pad_cfg: configurations for the pads
> - * @sink_fmt: input format
> - * @lock: protects pad_cfg and sink_fmt
> * @source: source in-use, set when starting streaming
> */
> struct rkisp1_csi {
> @@ -178,9 +175,6 @@ struct rkisp1_csi {
> bool is_dphy_errctrl_disabled;
> struct v4l2_subdev sd;
> struct media_pad pads[RKISP1_CSI_PAD_NUM];
> - struct v4l2_subdev_pad_config pad_cfg[RKISP1_CSI_PAD_NUM];
> - const struct rkisp1_mbus_info *sink_fmt;
> - struct mutex lock;
> struct v4l2_subdev *source;
> };
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c
> index d7acc94e10f8..d8d50270f6db 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c
> @@ -30,23 +30,6 @@ static inline struct rkisp1_csi *to_rkisp1_csi(struct v4l2_subdev *sd)
> return container_of(sd, struct rkisp1_csi, sd);
> }
>
> -static struct v4l2_mbus_framefmt *
> -rkisp1_csi_get_pad_fmt(struct rkisp1_csi *csi,
> - struct v4l2_subdev_state *sd_state,
> - unsigned int pad, u32 which)
> -{
> - struct v4l2_subdev_state state = {
> - .pads = csi->pad_cfg
> - };
> -
> - lockdep_assert_held(&csi->lock);
> -
> - if (which == V4L2_SUBDEV_FORMAT_TRY)
> - return v4l2_subdev_get_try_format(&csi->sd, sd_state, pad);
> - else
> - return v4l2_subdev_get_try_format(&csi->sd, &state, pad);
> -}
> -
> int rkisp1_csi_link_sensor(struct rkisp1_device *rkisp1, struct v4l2_subdev *sd,
> struct rkisp1_sensor_async *s_asd,
> unsigned int source_pad)
> @@ -76,7 +59,8 @@ int rkisp1_csi_link_sensor(struct rkisp1_device *rkisp1, struct v4l2_subdev *sd,
> }
>
> static int rkisp1_csi_config(struct rkisp1_csi *csi,
> - const struct rkisp1_sensor_async *sensor)
> + const struct rkisp1_sensor_async *sensor,
> + const struct rkisp1_mbus_info *format)
> {
> struct rkisp1_device *rkisp1 = csi->rkisp1;
> unsigned int lanes = sensor->lanes;
> @@ -98,7 +82,7 @@ static int rkisp1_csi_config(struct rkisp1_csi *csi,
>
> /* Configure Data Type and Virtual Channel */
> rkisp1_write(rkisp1, RKISP1_CIF_MIPI_IMG_DATA_SEL,
> - RKISP1_CIF_MIPI_DATA_SEL_DT(csi->sink_fmt->mipi_dt) |
> + RKISP1_CIF_MIPI_DATA_SEL_DT(format->mipi_dt) |
> RKISP1_CIF_MIPI_DATA_SEL_VC(0));
>
> /* Clear MIPI interrupts */
> @@ -151,7 +135,8 @@ static void rkisp1_csi_disable(struct rkisp1_csi *csi)
> }
>
> static int rkisp1_csi_start(struct rkisp1_csi *csi,
> - const struct rkisp1_sensor_async *sensor)
> + const struct rkisp1_sensor_async *sensor,
> + const struct rkisp1_mbus_info *format)
> {
> struct rkisp1_device *rkisp1 = csi->rkisp1;
> union phy_configure_opts opts;
> @@ -159,7 +144,7 @@ static int rkisp1_csi_start(struct rkisp1_csi *csi,
> s64 pixel_clock;
> int ret;
>
> - ret = rkisp1_csi_config(csi, sensor);
> + ret = rkisp1_csi_config(csi, sensor, format);
> if (ret)
> return ret;
>
> @@ -169,7 +154,7 @@ static int rkisp1_csi_start(struct rkisp1_csi *csi,
> return -EINVAL;
> }
>
> - phy_mipi_dphy_get_default_config(pixel_clock, csi->sink_fmt->bus_width,
> + phy_mipi_dphy_get_default_config(pixel_clock, format->bus_width,
> sensor->lanes, cfg);
> phy_set_mode(csi->dphy, PHY_MODE_MIPI_DPHY);
> phy_configure(csi->dphy, &opts);
> @@ -248,7 +233,6 @@ static int rkisp1_csi_enum_mbus_code(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *sd_state,
> struct v4l2_subdev_mbus_code_enum *code)
> {
> - struct rkisp1_csi *csi = to_rkisp1_csi(sd);
> unsigned int i;
> int pos = 0;
>
> @@ -258,15 +242,10 @@ static int rkisp1_csi_enum_mbus_code(struct v4l2_subdev *sd,
> if (code->index)
> return -EINVAL;
>
> - mutex_lock(&csi->lock);
> -
> - sink_fmt = rkisp1_csi_get_pad_fmt(csi, sd_state,
> - RKISP1_CSI_PAD_SINK,
> - code->which);
> + sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
> + RKISP1_CSI_PAD_SINK);
> code->code = sink_fmt->code;
>
> - mutex_unlock(&csi->lock);
> -
> return 0;
> }
>
> @@ -296,9 +275,9 @@ static int rkisp1_csi_init_config(struct v4l2_subdev *sd,
> {
> struct v4l2_mbus_framefmt *sink_fmt, *src_fmt;
>
> - sink_fmt = v4l2_subdev_get_try_format(sd, sd_state,
> + sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
> RKISP1_CSI_PAD_SINK);
> - src_fmt = v4l2_subdev_get_try_format(sd, sd_state,
> + src_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
> RKISP1_CSI_PAD_SRC);
>
> sink_fmt->width = RKISP1_DEFAULT_WIDTH;
> @@ -311,36 +290,18 @@ static int rkisp1_csi_init_config(struct v4l2_subdev *sd,
> return 0;
> }
>
> -static int rkisp1_csi_get_fmt(struct v4l2_subdev *sd,
> - struct v4l2_subdev_state *sd_state,
> - struct v4l2_subdev_format *fmt)
> -{
> - struct rkisp1_csi *csi = to_rkisp1_csi(sd);
> -
> - mutex_lock(&csi->lock);
> - fmt->format = *rkisp1_csi_get_pad_fmt(csi, sd_state, fmt->pad,
> - fmt->which);
> - mutex_unlock(&csi->lock);
> -
> - return 0;
> -}
> -
> static int rkisp1_csi_set_fmt(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *sd_state,
> struct v4l2_subdev_format *fmt)
> {
> - struct rkisp1_csi *csi = to_rkisp1_csi(sd);
> const struct rkisp1_mbus_info *mbus_info;
> struct v4l2_mbus_framefmt *sink_fmt, *src_fmt;
>
> /* The format on the source pad always matches the sink pad. */
> if (fmt->pad == RKISP1_CSI_PAD_SRC)
> - return rkisp1_csi_get_fmt(sd, sd_state, fmt);
> + return v4l2_subdev_get_fmt(sd, sd_state, fmt);
>
> - mutex_lock(&csi->lock);
> -
> - sink_fmt = rkisp1_csi_get_pad_fmt(csi, sd_state, RKISP1_CSI_PAD_SINK,
> - fmt->which);
> + sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state, RKISP1_CSI_PAD_SINK);
>
> sink_fmt->code = fmt->format.code;
>
> @@ -359,16 +320,10 @@ static int rkisp1_csi_set_fmt(struct v4l2_subdev *sd,
>
> fmt->format = *sink_fmt;
>
> - if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE)
> - csi->sink_fmt = mbus_info;
> -
> /* Propagate the format to the source pad. */
> - src_fmt = rkisp1_csi_get_pad_fmt(csi, sd_state, RKISP1_CSI_PAD_SRC,
> - fmt->which);
> + src_fmt = v4l2_subdev_get_pad_format(sd, sd_state, RKISP1_CSI_PAD_SRC);
> *src_fmt = *sink_fmt;
>
> - mutex_unlock(&csi->lock);
> -
> return 0;
> }
>
> @@ -380,7 +335,10 @@ static int rkisp1_csi_s_stream(struct v4l2_subdev *sd, int enable)
> {
> struct rkisp1_csi *csi = to_rkisp1_csi(sd);
> struct rkisp1_device *rkisp1 = csi->rkisp1;
> + const struct v4l2_mbus_framefmt *sink_fmt;
> + const struct rkisp1_mbus_info *format;
> struct rkisp1_sensor_async *source_asd;
> + struct v4l2_subdev_state *sd_state;
> struct media_pad *source_pad;
> struct v4l2_subdev *source;
> int ret;
> @@ -410,9 +368,12 @@ static int rkisp1_csi_s_stream(struct v4l2_subdev *sd, int enable)
> if (source_asd->mbus_type != V4L2_MBUS_CSI2_DPHY)
> return -EINVAL;
>
> - mutex_lock(&csi->lock);
> - ret = rkisp1_csi_start(csi, source_asd);
> - mutex_unlock(&csi->lock);
> + sd_state = v4l2_subdev_lock_and_get_active_state(sd);
> + sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state, RKISP1_CSI_PAD_SINK);
> + format = rkisp1_mbus_info_get_by_code(sink_fmt->code);
> + v4l2_subdev_unlock_state(sd_state);
> +
> + ret = rkisp1_csi_start(csi, source_asd, format);
> if (ret)
> return ret;
>
> @@ -442,7 +403,7 @@ static const struct v4l2_subdev_video_ops rkisp1_csi_video_ops = {
> static const struct v4l2_subdev_pad_ops rkisp1_csi_pad_ops = {
> .enum_mbus_code = rkisp1_csi_enum_mbus_code,
> .init_cfg = rkisp1_csi_init_config,
> - .get_fmt = rkisp1_csi_get_fmt,
> + .get_fmt = v4l2_subdev_get_fmt,
> .set_fmt = rkisp1_csi_set_fmt,
> };
>
> @@ -454,13 +415,11 @@ static const struct v4l2_subdev_ops rkisp1_csi_ops = {
> int rkisp1_csi_register(struct rkisp1_device *rkisp1)
> {
> struct rkisp1_csi *csi = &rkisp1->csi;
> - struct v4l2_subdev_state state = {};
> struct media_pad *pads;
> struct v4l2_subdev *sd;
> int ret;
>
> csi->rkisp1 = rkisp1;
> - mutex_init(&csi->lock);
>
> sd = &csi->sd;
> v4l2_subdev_init(sd, &rkisp1_csi_ops);
> @@ -476,26 +435,26 @@ int rkisp1_csi_register(struct rkisp1_device *rkisp1)
> pads[RKISP1_CSI_PAD_SRC].flags = MEDIA_PAD_FL_SOURCE |
> MEDIA_PAD_FL_MUST_CONNECT;
>
> - csi->sink_fmt = rkisp1_mbus_info_get_by_code(RKISP1_CSI_DEF_FMT);
> -
> ret = media_entity_pads_init(&sd->entity, RKISP1_CSI_PAD_NUM, pads);
> if (ret)
> - goto error;
> + goto err_entity_cleanup;
>
> - state.pads = csi->pad_cfg;
> - rkisp1_csi_init_config(sd, &state);
> + ret = v4l2_subdev_init_finalize(sd);
> + if (ret)
> + goto err_entity_cleanup;
>
> ret = v4l2_device_register_subdev(&csi->rkisp1->v4l2_dev, sd);
> if (ret) {
> dev_err(sd->dev, "Failed to register csi receiver subdev\n");
> - goto error;
> + goto err_subdev_cleanup;
> }
>
> return 0;
>
> -error:
> +err_subdev_cleanup:
> + v4l2_subdev_cleanup(sd);
> +err_entity_cleanup:
> media_entity_cleanup(&sd->entity);
> - mutex_destroy(&csi->lock);
> csi->rkisp1 = NULL;
> return ret;
> }
> @@ -508,8 +467,8 @@ void rkisp1_csi_unregister(struct rkisp1_device *rkisp1)
> return;
>
> v4l2_device_unregister_subdev(&csi->sd);
> + v4l2_subdev_cleanup(&csi->sd);
> media_entity_cleanup(&csi->sd.entity);
> - mutex_destroy(&csi->lock);
> }
>
> int rkisp1_csi_init(struct rkisp1_device *rkisp1)
> --
> Regards,
>
> Laurent Pinchart
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 3/3] media: rkisp1: csi: Use V4L2 subdev active state
@ 2023-04-27 3:52 ` Paul Elder
0 siblings, 0 replies; 16+ messages in thread
From: Paul Elder @ 2023-04-27 3:52 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Dafna Hirschfeld, Heiko Stuebner, linux-rockchip
On Fri, Jan 27, 2023 at 02:31:24AM +0200, Laurent Pinchart wrote:
> Use the V4L2 subdev active state API to store the active format and crop
> rectangle. This simplifies the driver not only by dropping the state
> stored in the rkisp1_csi structure, but also by replacing the ops_lock
> with the state lock.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
> .../platform/rockchip/rkisp1/rkisp1-common.h | 6 -
> .../platform/rockchip/rkisp1/rkisp1-csi.c | 107 ++++++------------
> 2 files changed, 33 insertions(+), 80 deletions(-)
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> index d43ff1b549d9..dbd8725f2ec9 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> @@ -167,9 +167,6 @@ struct rkisp1_sensor_async {
> * @is_dphy_errctrl_disabled: if dphy errctrl is disabled (avoid endless interrupt)
> * @sd: v4l2_subdev variable
> * @pads: media pads
> - * @pad_cfg: configurations for the pads
> - * @sink_fmt: input format
> - * @lock: protects pad_cfg and sink_fmt
> * @source: source in-use, set when starting streaming
> */
> struct rkisp1_csi {
> @@ -178,9 +175,6 @@ struct rkisp1_csi {
> bool is_dphy_errctrl_disabled;
> struct v4l2_subdev sd;
> struct media_pad pads[RKISP1_CSI_PAD_NUM];
> - struct v4l2_subdev_pad_config pad_cfg[RKISP1_CSI_PAD_NUM];
> - const struct rkisp1_mbus_info *sink_fmt;
> - struct mutex lock;
> struct v4l2_subdev *source;
> };
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c
> index d7acc94e10f8..d8d50270f6db 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c
> @@ -30,23 +30,6 @@ static inline struct rkisp1_csi *to_rkisp1_csi(struct v4l2_subdev *sd)
> return container_of(sd, struct rkisp1_csi, sd);
> }
>
> -static struct v4l2_mbus_framefmt *
> -rkisp1_csi_get_pad_fmt(struct rkisp1_csi *csi,
> - struct v4l2_subdev_state *sd_state,
> - unsigned int pad, u32 which)
> -{
> - struct v4l2_subdev_state state = {
> - .pads = csi->pad_cfg
> - };
> -
> - lockdep_assert_held(&csi->lock);
> -
> - if (which == V4L2_SUBDEV_FORMAT_TRY)
> - return v4l2_subdev_get_try_format(&csi->sd, sd_state, pad);
> - else
> - return v4l2_subdev_get_try_format(&csi->sd, &state, pad);
> -}
> -
> int rkisp1_csi_link_sensor(struct rkisp1_device *rkisp1, struct v4l2_subdev *sd,
> struct rkisp1_sensor_async *s_asd,
> unsigned int source_pad)
> @@ -76,7 +59,8 @@ int rkisp1_csi_link_sensor(struct rkisp1_device *rkisp1, struct v4l2_subdev *sd,
> }
>
> static int rkisp1_csi_config(struct rkisp1_csi *csi,
> - const struct rkisp1_sensor_async *sensor)
> + const struct rkisp1_sensor_async *sensor,
> + const struct rkisp1_mbus_info *format)
> {
> struct rkisp1_device *rkisp1 = csi->rkisp1;
> unsigned int lanes = sensor->lanes;
> @@ -98,7 +82,7 @@ static int rkisp1_csi_config(struct rkisp1_csi *csi,
>
> /* Configure Data Type and Virtual Channel */
> rkisp1_write(rkisp1, RKISP1_CIF_MIPI_IMG_DATA_SEL,
> - RKISP1_CIF_MIPI_DATA_SEL_DT(csi->sink_fmt->mipi_dt) |
> + RKISP1_CIF_MIPI_DATA_SEL_DT(format->mipi_dt) |
> RKISP1_CIF_MIPI_DATA_SEL_VC(0));
>
> /* Clear MIPI interrupts */
> @@ -151,7 +135,8 @@ static void rkisp1_csi_disable(struct rkisp1_csi *csi)
> }
>
> static int rkisp1_csi_start(struct rkisp1_csi *csi,
> - const struct rkisp1_sensor_async *sensor)
> + const struct rkisp1_sensor_async *sensor,
> + const struct rkisp1_mbus_info *format)
> {
> struct rkisp1_device *rkisp1 = csi->rkisp1;
> union phy_configure_opts opts;
> @@ -159,7 +144,7 @@ static int rkisp1_csi_start(struct rkisp1_csi *csi,
> s64 pixel_clock;
> int ret;
>
> - ret = rkisp1_csi_config(csi, sensor);
> + ret = rkisp1_csi_config(csi, sensor, format);
> if (ret)
> return ret;
>
> @@ -169,7 +154,7 @@ static int rkisp1_csi_start(struct rkisp1_csi *csi,
> return -EINVAL;
> }
>
> - phy_mipi_dphy_get_default_config(pixel_clock, csi->sink_fmt->bus_width,
> + phy_mipi_dphy_get_default_config(pixel_clock, format->bus_width,
> sensor->lanes, cfg);
> phy_set_mode(csi->dphy, PHY_MODE_MIPI_DPHY);
> phy_configure(csi->dphy, &opts);
> @@ -248,7 +233,6 @@ static int rkisp1_csi_enum_mbus_code(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *sd_state,
> struct v4l2_subdev_mbus_code_enum *code)
> {
> - struct rkisp1_csi *csi = to_rkisp1_csi(sd);
> unsigned int i;
> int pos = 0;
>
> @@ -258,15 +242,10 @@ static int rkisp1_csi_enum_mbus_code(struct v4l2_subdev *sd,
> if (code->index)
> return -EINVAL;
>
> - mutex_lock(&csi->lock);
> -
> - sink_fmt = rkisp1_csi_get_pad_fmt(csi, sd_state,
> - RKISP1_CSI_PAD_SINK,
> - code->which);
> + sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
> + RKISP1_CSI_PAD_SINK);
> code->code = sink_fmt->code;
>
> - mutex_unlock(&csi->lock);
> -
> return 0;
> }
>
> @@ -296,9 +275,9 @@ static int rkisp1_csi_init_config(struct v4l2_subdev *sd,
> {
> struct v4l2_mbus_framefmt *sink_fmt, *src_fmt;
>
> - sink_fmt = v4l2_subdev_get_try_format(sd, sd_state,
> + sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
> RKISP1_CSI_PAD_SINK);
> - src_fmt = v4l2_subdev_get_try_format(sd, sd_state,
> + src_fmt = v4l2_subdev_get_pad_format(sd, sd_state,
> RKISP1_CSI_PAD_SRC);
>
> sink_fmt->width = RKISP1_DEFAULT_WIDTH;
> @@ -311,36 +290,18 @@ static int rkisp1_csi_init_config(struct v4l2_subdev *sd,
> return 0;
> }
>
> -static int rkisp1_csi_get_fmt(struct v4l2_subdev *sd,
> - struct v4l2_subdev_state *sd_state,
> - struct v4l2_subdev_format *fmt)
> -{
> - struct rkisp1_csi *csi = to_rkisp1_csi(sd);
> -
> - mutex_lock(&csi->lock);
> - fmt->format = *rkisp1_csi_get_pad_fmt(csi, sd_state, fmt->pad,
> - fmt->which);
> - mutex_unlock(&csi->lock);
> -
> - return 0;
> -}
> -
> static int rkisp1_csi_set_fmt(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *sd_state,
> struct v4l2_subdev_format *fmt)
> {
> - struct rkisp1_csi *csi = to_rkisp1_csi(sd);
> const struct rkisp1_mbus_info *mbus_info;
> struct v4l2_mbus_framefmt *sink_fmt, *src_fmt;
>
> /* The format on the source pad always matches the sink pad. */
> if (fmt->pad == RKISP1_CSI_PAD_SRC)
> - return rkisp1_csi_get_fmt(sd, sd_state, fmt);
> + return v4l2_subdev_get_fmt(sd, sd_state, fmt);
>
> - mutex_lock(&csi->lock);
> -
> - sink_fmt = rkisp1_csi_get_pad_fmt(csi, sd_state, RKISP1_CSI_PAD_SINK,
> - fmt->which);
> + sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state, RKISP1_CSI_PAD_SINK);
>
> sink_fmt->code = fmt->format.code;
>
> @@ -359,16 +320,10 @@ static int rkisp1_csi_set_fmt(struct v4l2_subdev *sd,
>
> fmt->format = *sink_fmt;
>
> - if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE)
> - csi->sink_fmt = mbus_info;
> -
> /* Propagate the format to the source pad. */
> - src_fmt = rkisp1_csi_get_pad_fmt(csi, sd_state, RKISP1_CSI_PAD_SRC,
> - fmt->which);
> + src_fmt = v4l2_subdev_get_pad_format(sd, sd_state, RKISP1_CSI_PAD_SRC);
> *src_fmt = *sink_fmt;
>
> - mutex_unlock(&csi->lock);
> -
> return 0;
> }
>
> @@ -380,7 +335,10 @@ static int rkisp1_csi_s_stream(struct v4l2_subdev *sd, int enable)
> {
> struct rkisp1_csi *csi = to_rkisp1_csi(sd);
> struct rkisp1_device *rkisp1 = csi->rkisp1;
> + const struct v4l2_mbus_framefmt *sink_fmt;
> + const struct rkisp1_mbus_info *format;
> struct rkisp1_sensor_async *source_asd;
> + struct v4l2_subdev_state *sd_state;
> struct media_pad *source_pad;
> struct v4l2_subdev *source;
> int ret;
> @@ -410,9 +368,12 @@ static int rkisp1_csi_s_stream(struct v4l2_subdev *sd, int enable)
> if (source_asd->mbus_type != V4L2_MBUS_CSI2_DPHY)
> return -EINVAL;
>
> - mutex_lock(&csi->lock);
> - ret = rkisp1_csi_start(csi, source_asd);
> - mutex_unlock(&csi->lock);
> + sd_state = v4l2_subdev_lock_and_get_active_state(sd);
> + sink_fmt = v4l2_subdev_get_pad_format(sd, sd_state, RKISP1_CSI_PAD_SINK);
> + format = rkisp1_mbus_info_get_by_code(sink_fmt->code);
> + v4l2_subdev_unlock_state(sd_state);
> +
> + ret = rkisp1_csi_start(csi, source_asd, format);
> if (ret)
> return ret;
>
> @@ -442,7 +403,7 @@ static const struct v4l2_subdev_video_ops rkisp1_csi_video_ops = {
> static const struct v4l2_subdev_pad_ops rkisp1_csi_pad_ops = {
> .enum_mbus_code = rkisp1_csi_enum_mbus_code,
> .init_cfg = rkisp1_csi_init_config,
> - .get_fmt = rkisp1_csi_get_fmt,
> + .get_fmt = v4l2_subdev_get_fmt,
> .set_fmt = rkisp1_csi_set_fmt,
> };
>
> @@ -454,13 +415,11 @@ static const struct v4l2_subdev_ops rkisp1_csi_ops = {
> int rkisp1_csi_register(struct rkisp1_device *rkisp1)
> {
> struct rkisp1_csi *csi = &rkisp1->csi;
> - struct v4l2_subdev_state state = {};
> struct media_pad *pads;
> struct v4l2_subdev *sd;
> int ret;
>
> csi->rkisp1 = rkisp1;
> - mutex_init(&csi->lock);
>
> sd = &csi->sd;
> v4l2_subdev_init(sd, &rkisp1_csi_ops);
> @@ -476,26 +435,26 @@ int rkisp1_csi_register(struct rkisp1_device *rkisp1)
> pads[RKISP1_CSI_PAD_SRC].flags = MEDIA_PAD_FL_SOURCE |
> MEDIA_PAD_FL_MUST_CONNECT;
>
> - csi->sink_fmt = rkisp1_mbus_info_get_by_code(RKISP1_CSI_DEF_FMT);
> -
> ret = media_entity_pads_init(&sd->entity, RKISP1_CSI_PAD_NUM, pads);
> if (ret)
> - goto error;
> + goto err_entity_cleanup;
>
> - state.pads = csi->pad_cfg;
> - rkisp1_csi_init_config(sd, &state);
> + ret = v4l2_subdev_init_finalize(sd);
> + if (ret)
> + goto err_entity_cleanup;
>
> ret = v4l2_device_register_subdev(&csi->rkisp1->v4l2_dev, sd);
> if (ret) {
> dev_err(sd->dev, "Failed to register csi receiver subdev\n");
> - goto error;
> + goto err_subdev_cleanup;
> }
>
> return 0;
>
> -error:
> +err_subdev_cleanup:
> + v4l2_subdev_cleanup(sd);
> +err_entity_cleanup:
> media_entity_cleanup(&sd->entity);
> - mutex_destroy(&csi->lock);
> csi->rkisp1 = NULL;
> return ret;
> }
> @@ -508,8 +467,8 @@ void rkisp1_csi_unregister(struct rkisp1_device *rkisp1)
> return;
>
> v4l2_device_unregister_subdev(&csi->sd);
> + v4l2_subdev_cleanup(&csi->sd);
> media_entity_cleanup(&csi->sd.entity);
> - mutex_destroy(&csi->lock);
> }
>
> int rkisp1_csi_init(struct rkisp1_device *rkisp1)
> --
> Regards,
>
> Laurent Pinchart
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2023-04-27 3:53 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-27 0:31 [PATCH v1 0/3] media: rkisp1: Convert to V4L2 subdev active state Laurent Pinchart
2023-01-27 0:31 ` Laurent Pinchart
2023-01-27 0:31 ` [PATCH v1 1/3] media: rkisp1: resizer: Use " Laurent Pinchart
2023-01-27 0:31 ` Laurent Pinchart
2023-04-27 3:52 ` Paul Elder
2023-04-27 3:52 ` Paul Elder
2023-01-27 0:31 ` [PATCH v1 2/3] media: rkisp1: isp: " Laurent Pinchart
2023-01-27 0:31 ` Laurent Pinchart
2023-04-27 3:51 ` Paul Elder
2023-04-27 3:51 ` Paul Elder
2023-01-27 0:31 ` [PATCH v1 3/3] media: rkisp1: csi: " Laurent Pinchart
2023-01-27 0:31 ` Laurent Pinchart
2023-04-27 3:52 ` Paul Elder
2023-04-27 3:52 ` Paul Elder
2023-02-15 23:53 ` [PATCH v1 0/3] media: rkisp1: Convert to " Laurent Pinchart
2023-02-15 23:53 ` Laurent Pinchart
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.