* [PATCH v1 1/3] media: rkisp1: resizer: Use V4L2 subdev active state
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-04-27 3:52 ` Paul Elder
2023-01-27 0:31 ` [PATCH v1 2/3] media: rkisp1: isp: " Laurent Pinchart
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ 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] 8+ messages in thread* Re: [PATCH v1 1/3] media: rkisp1: resizer: Use V4L2 subdev active state
2023-01-27 0:31 ` [PATCH v1 1/3] media: rkisp1: resizer: Use " Laurent Pinchart
@ 2023-04-27 3:52 ` Paul Elder
0 siblings, 0 replies; 8+ 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] 8+ messages in thread
* [PATCH v1 2/3] media: rkisp1: isp: Use V4L2 subdev active state
2023-01-27 0:31 [PATCH v1 0/3] media: rkisp1: Convert to V4L2 subdev active state 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:51 ` Paul Elder
2023-01-27 0:31 ` [PATCH v1 3/3] media: rkisp1: csi: " Laurent Pinchart
2023-02-15 23:53 ` [PATCH v1 0/3] media: rkisp1: Convert to " Laurent Pinchart
3 siblings, 1 reply; 8+ 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] 8+ messages in thread* Re: [PATCH v1 2/3] media: rkisp1: isp: Use V4L2 subdev active state
2023-01-27 0:31 ` [PATCH v1 2/3] media: rkisp1: isp: " Laurent Pinchart
@ 2023-04-27 3:51 ` Paul Elder
0 siblings, 0 replies; 8+ 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] 8+ messages in thread
* [PATCH v1 3/3] media: rkisp1: csi: Use V4L2 subdev active state
2023-01-27 0:31 [PATCH v1 0/3] media: rkisp1: Convert to V4L2 subdev active state Laurent Pinchart
2023-01-27 0:31 ` [PATCH v1 1/3] media: rkisp1: resizer: Use " Laurent Pinchart
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:52 ` Paul Elder
2023-02-15 23:53 ` [PATCH v1 0/3] media: rkisp1: Convert to " Laurent Pinchart
3 siblings, 1 reply; 8+ 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] 8+ messages in thread* Re: [PATCH v1 3/3] media: rkisp1: csi: Use V4L2 subdev active state
2023-01-27 0:31 ` [PATCH v1 3/3] media: rkisp1: csi: " Laurent Pinchart
@ 2023-04-27 3:52 ` Paul Elder
0 siblings, 0 replies; 8+ 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] 8+ messages in thread
* Re: [PATCH v1 0/3] media: rkisp1: Convert to V4L2 subdev active state
2023-01-27 0:31 [PATCH v1 0/3] media: rkisp1: Convert to V4L2 subdev active state Laurent Pinchart
` (2 preceding siblings ...)
2023-01-27 0:31 ` [PATCH v1 3/3] media: rkisp1: csi: " Laurent Pinchart
@ 2023-02-15 23:53 ` Laurent Pinchart
3 siblings, 0 replies; 8+ 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] 8+ messages in thread