* [PATCH 1/4] media: staging: rkisp1: change function to return void instead of int
2020-02-07 8:59 [PATCH 0/4] media: staging: rkisp1: add serialization to the isp and resizer ops Dafna Hirschfeld
@ 2020-02-07 8:59 ` Dafna Hirschfeld
2020-02-07 12:57 ` Helen Koike
2020-02-07 8:59 ` [PATCH 2/4] media: staging: rkisp1: isp: check for dphy bus before initializations in s_stream Dafna Hirschfeld
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Dafna Hirschfeld @ 2020-02-07 8:59 UTC (permalink / raw)
To: linux-media
Cc: dafna.hirschfeld, helen.koike, ezequiel, hverkuil, kernel,
dafna3, sakari.ailus, linux-rockchip, mchehab
There are functions that return int but actually return always 0.
Change them to return void and then there is no need to check
for error return value.
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
drivers/staging/media/rkisp1/rkisp1-isp.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/media/rkisp1/rkisp1-isp.c b/drivers/staging/media/rkisp1/rkisp1-isp.c
index 2b0513e826fe..56bd95d01f65 100644
--- a/drivers/staging/media/rkisp1/rkisp1-isp.c
+++ b/drivers/staging/media/rkisp1/rkisp1-isp.c
@@ -504,7 +504,7 @@ static int rkisp1_config_cif(struct rkisp1_device *rkisp1)
return 0;
}
-static int rkisp1_isp_stop(struct rkisp1_device *rkisp1)
+static void rkisp1_isp_stop(struct rkisp1_device *rkisp1)
{
u32 val;
@@ -540,8 +540,6 @@ static int rkisp1_isp_stop(struct rkisp1_device *rkisp1)
RKISP1_CIF_IRCL_MIPI_SW_RST | RKISP1_CIF_IRCL_ISP_SW_RST,
RKISP1_CIF_IRCL);
rkisp1_write(rkisp1, 0x0, RKISP1_CIF_IRCL);
-
- return 0;
}
static void rkisp1_config_clk(struct rkisp1_device *rkisp1)
@@ -555,7 +553,7 @@ static void rkisp1_config_clk(struct rkisp1_device *rkisp1)
rkisp1_write(rkisp1, val, RKISP1_CIF_ICCL);
}
-static int rkisp1_isp_start(struct rkisp1_device *rkisp1)
+static void rkisp1_isp_start(struct rkisp1_device *rkisp1)
{
struct rkisp1_sensor_async *sensor = rkisp1->active_sensor;
u32 val;
@@ -580,8 +578,6 @@ static int rkisp1_isp_start(struct rkisp1_device *rkisp1)
* the MIPI interface and before starting the sensor output.
*/
usleep_range(1000, 1200);
-
- return 0;
}
/* ----------------------------------------------------------------------------
@@ -956,9 +952,7 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
int ret = 0;
if (!enable) {
- ret = rkisp1_isp_stop(rkisp1);
- if (ret)
- return ret;
+ rkisp1_isp_stop(rkisp1);
rkisp1_mipi_csi2_stop(rkisp1->active_sensor);
return 0;
}
@@ -981,9 +975,7 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
if (ret)
return ret;
- ret = rkisp1_isp_start(rkisp1);
- if (ret)
- rkisp1_mipi_csi2_stop(rkisp1->active_sensor);
+ rkisp1_isp_start(rkisp1);
return ret;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] media: staging: rkisp1: change function to return void instead of int
2020-02-07 8:59 ` [PATCH 1/4] media: staging: rkisp1: change function to return void instead of int Dafna Hirschfeld
@ 2020-02-07 12:57 ` Helen Koike
0 siblings, 0 replies; 10+ messages in thread
From: Helen Koike @ 2020-02-07 12:57 UTC (permalink / raw)
To: Dafna Hirschfeld, linux-media
Cc: ezequiel, hverkuil, kernel, dafna3, sakari.ailus, linux-rockchip,
mchehab
On 2/7/20 6:59 AM, Dafna Hirschfeld wrote:
> There are functions that return int but actually return always 0.
> Change them to return void and then there is no need to check
> for error return value.
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
Thanks
Helen
> ---
> drivers/staging/media/rkisp1/rkisp1-isp.c | 16 ++++------------
> 1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/staging/media/rkisp1/rkisp1-isp.c b/drivers/staging/media/rkisp1/rkisp1-isp.c
> index 2b0513e826fe..56bd95d01f65 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-isp.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-isp.c
> @@ -504,7 +504,7 @@ static int rkisp1_config_cif(struct rkisp1_device *rkisp1)
> return 0;
> }
>
> -static int rkisp1_isp_stop(struct rkisp1_device *rkisp1)
> +static void rkisp1_isp_stop(struct rkisp1_device *rkisp1)
> {
> u32 val;
>
> @@ -540,8 +540,6 @@ static int rkisp1_isp_stop(struct rkisp1_device *rkisp1)
> RKISP1_CIF_IRCL_MIPI_SW_RST | RKISP1_CIF_IRCL_ISP_SW_RST,
> RKISP1_CIF_IRCL);
> rkisp1_write(rkisp1, 0x0, RKISP1_CIF_IRCL);
> -
> - return 0;
> }
>
> static void rkisp1_config_clk(struct rkisp1_device *rkisp1)
> @@ -555,7 +553,7 @@ static void rkisp1_config_clk(struct rkisp1_device *rkisp1)
> rkisp1_write(rkisp1, val, RKISP1_CIF_ICCL);
> }
>
> -static int rkisp1_isp_start(struct rkisp1_device *rkisp1)
> +static void rkisp1_isp_start(struct rkisp1_device *rkisp1)
> {
> struct rkisp1_sensor_async *sensor = rkisp1->active_sensor;
> u32 val;
> @@ -580,8 +578,6 @@ static int rkisp1_isp_start(struct rkisp1_device *rkisp1)
> * the MIPI interface and before starting the sensor output.
> */
> usleep_range(1000, 1200);
> -
> - return 0;
> }
>
> /* ----------------------------------------------------------------------------
> @@ -956,9 +952,7 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
> int ret = 0;
>
> if (!enable) {
> - ret = rkisp1_isp_stop(rkisp1);
> - if (ret)
> - return ret;
> + rkisp1_isp_stop(rkisp1);
> rkisp1_mipi_csi2_stop(rkisp1->active_sensor);
> return 0;
> }
> @@ -981,9 +975,7 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
> if (ret)
> return ret;
>
> - ret = rkisp1_isp_start(rkisp1);
> - if (ret)
> - rkisp1_mipi_csi2_stop(rkisp1->active_sensor);
> + rkisp1_isp_start(rkisp1);
>
> return ret;
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/4] media: staging: rkisp1: isp: check for dphy bus before initializations in s_stream
2020-02-07 8:59 [PATCH 0/4] media: staging: rkisp1: add serialization to the isp and resizer ops Dafna Hirschfeld
2020-02-07 8:59 ` [PATCH 1/4] media: staging: rkisp1: change function to return void instead of int Dafna Hirschfeld
@ 2020-02-07 8:59 ` Dafna Hirschfeld
2020-02-07 12:57 ` Helen Koike
2020-02-07 8:59 ` [PATCH 3/4] media: staging: rkisp1: add serialization to the isp subdev ops Dafna Hirschfeld
2020-02-07 8:59 ` [PATCH 4/4] media: staging: rkisp1: add serialization to the resizer " Dafna Hirschfeld
3 siblings, 1 reply; 10+ messages in thread
From: Dafna Hirschfeld @ 2020-02-07 8:59 UTC (permalink / raw)
To: linux-media
Cc: dafna.hirschfeld, helen.koike, ezequiel, hverkuil, kernel,
dafna3, sakari.ailus, linux-rockchip, mchehab
In rkisp1_isp_s_stream it is better to return error in case the
bus type is not dphy before initializing the registers.
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
drivers/staging/media/rkisp1/rkisp1-isp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/media/rkisp1/rkisp1-isp.c b/drivers/staging/media/rkisp1/rkisp1-isp.c
index 56bd95d01f65..c98e3c16f520 100644
--- a/drivers/staging/media/rkisp1/rkisp1-isp.c
+++ b/drivers/staging/media/rkisp1/rkisp1-isp.c
@@ -963,14 +963,14 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
rkisp1->active_sensor = container_of(sensor_sd->asd,
struct rkisp1_sensor_async, asd);
+ if (rkisp1->active_sensor->mbus.type != V4L2_MBUS_CSI2_DPHY)
+ return -EINVAL;
+
atomic_set(&rkisp1->isp.frame_sequence, -1);
ret = rkisp1_config_cif(rkisp1);
if (ret)
return ret;
- if (rkisp1->active_sensor->mbus.type != V4L2_MBUS_CSI2_DPHY)
- return -EINVAL;
-
ret = rkisp1_mipi_csi2_start(&rkisp1->isp, rkisp1->active_sensor);
if (ret)
return ret;
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/4] media: staging: rkisp1: isp: check for dphy bus before initializations in s_stream
2020-02-07 8:59 ` [PATCH 2/4] media: staging: rkisp1: isp: check for dphy bus before initializations in s_stream Dafna Hirschfeld
@ 2020-02-07 12:57 ` Helen Koike
0 siblings, 0 replies; 10+ messages in thread
From: Helen Koike @ 2020-02-07 12:57 UTC (permalink / raw)
To: Dafna Hirschfeld, linux-media
Cc: ezequiel, hverkuil, kernel, dafna3, sakari.ailus, linux-rockchip,
mchehab
On 2/7/20 6:59 AM, Dafna Hirschfeld wrote:
> In rkisp1_isp_s_stream it is better to return error in case the
> bus type is not dphy before initializing the registers.
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
Thanks
Helen
> ---
> drivers/staging/media/rkisp1/rkisp1-isp.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/media/rkisp1/rkisp1-isp.c b/drivers/staging/media/rkisp1/rkisp1-isp.c
> index 56bd95d01f65..c98e3c16f520 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-isp.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-isp.c
> @@ -963,14 +963,14 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
> rkisp1->active_sensor = container_of(sensor_sd->asd,
> struct rkisp1_sensor_async, asd);
>
> + if (rkisp1->active_sensor->mbus.type != V4L2_MBUS_CSI2_DPHY)
> + return -EINVAL;
> +
> atomic_set(&rkisp1->isp.frame_sequence, -1);
> ret = rkisp1_config_cif(rkisp1);
> if (ret)
> return ret;
>
> - if (rkisp1->active_sensor->mbus.type != V4L2_MBUS_CSI2_DPHY)
> - return -EINVAL;
> -
> ret = rkisp1_mipi_csi2_start(&rkisp1->isp, rkisp1->active_sensor);
> if (ret)
> return ret;
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/4] media: staging: rkisp1: add serialization to the isp subdev ops
2020-02-07 8:59 [PATCH 0/4] media: staging: rkisp1: add serialization to the isp and resizer ops Dafna Hirschfeld
2020-02-07 8:59 ` [PATCH 1/4] media: staging: rkisp1: change function to return void instead of int Dafna Hirschfeld
2020-02-07 8:59 ` [PATCH 2/4] media: staging: rkisp1: isp: check for dphy bus before initializations in s_stream Dafna Hirschfeld
@ 2020-02-07 8:59 ` Dafna Hirschfeld
2020-02-07 13:54 ` Helen Koike
2020-02-14 9:46 ` Sakari Ailus
2020-02-07 8:59 ` [PATCH 4/4] media: staging: rkisp1: add serialization to the resizer " Dafna Hirschfeld
3 siblings, 2 replies; 10+ messages in thread
From: Dafna Hirschfeld @ 2020-02-07 8:59 UTC (permalink / raw)
To: linux-media
Cc: dafna.hirschfeld, helen.koike, ezequiel, hverkuil, kernel,
dafna3, sakari.ailus, linux-rockchip, mchehab
For subdevices drivers, the drivers themself are responsible
for serializing their operations.
This patch adds serialization to the isp subdevice.
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
drivers/staging/media/rkisp1/rkisp1-common.h | 2 ++
drivers/staging/media/rkisp1/rkisp1-isp.c | 29 ++++++++++++++------
2 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/media/rkisp1/rkisp1-common.h b/drivers/staging/media/rkisp1/rkisp1-common.h
index 4e773d611d1b..7c668ac4bdd5 100644
--- a/drivers/staging/media/rkisp1/rkisp1-common.h
+++ b/drivers/staging/media/rkisp1/rkisp1-common.h
@@ -96,6 +96,7 @@ struct rkisp1_sensor_async {
* @sink_crop: crop for sink pad
* @src_fmt: output format
* @src_crop: output size
+ * @ops_lock: ops serialization
*
* @is_dphy_errctrl_disabled : if dphy errctrl is disabled (avoid endless interrupt)
* @frame_sequence: used to synchronize frame_id between video devices.
@@ -107,6 +108,7 @@ struct rkisp1_isp {
struct v4l2_subdev_pad_config pad_cfg[RKISP1_ISP_PAD_MAX];
const struct rkisp1_isp_mbus_info *sink_fmt;
const struct rkisp1_isp_mbus_info *src_fmt;
+ struct mutex ops_lock;
bool is_dphy_errctrl_disabled;
atomic_t frame_sequence;
};
diff --git a/drivers/staging/media/rkisp1/rkisp1-isp.c b/drivers/staging/media/rkisp1/rkisp1-isp.c
index c98e3c16f520..aa7a842f97f8 100644
--- a/drivers/staging/media/rkisp1/rkisp1-isp.c
+++ b/drivers/staging/media/rkisp1/rkisp1-isp.c
@@ -791,7 +791,9 @@ static int rkisp1_isp_get_fmt(struct v4l2_subdev *sd,
{
struct rkisp1_isp *isp = container_of(sd, struct rkisp1_isp, sd);
+ mutex_lock(&isp->ops_lock);
fmt->format = *rkisp1_isp_get_pad_fmt(isp, cfg, fmt->pad, fmt->which);
+ mutex_unlock(&isp->ops_lock);
return 0;
}
@@ -801,6 +803,7 @@ static int rkisp1_isp_set_fmt(struct v4l2_subdev *sd,
{
struct rkisp1_isp *isp = container_of(sd, struct rkisp1_isp, sd);
+ mutex_lock(&isp->ops_lock);
if (fmt->pad == RKISP1_ISP_PAD_SINK_VIDEO)
rkisp1_isp_set_sink_fmt(isp, cfg, &fmt->format, fmt->which);
else if (fmt->pad == RKISP1_ISP_PAD_SOURCE_VIDEO)
@@ -809,6 +812,7 @@ static int rkisp1_isp_set_fmt(struct v4l2_subdev *sd,
fmt->format = *rkisp1_isp_get_pad_fmt(isp, cfg, fmt->pad,
fmt->which);
+ mutex_unlock(&isp->ops_lock);
return 0;
}
@@ -817,11 +821,13 @@ static int rkisp1_isp_get_selection(struct v4l2_subdev *sd,
struct v4l2_subdev_selection *sel)
{
struct rkisp1_isp *isp = container_of(sd, struct 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) {
@@ -844,10 +850,10 @@ static int rkisp1_isp_get_selection(struct v4l2_subdev *sd,
sel->which);
break;
default:
- return -EINVAL;
+ ret = -EINVAL;
}
-
- return 0;
+ mutex_unlock(&isp->ops_lock);
+ return ret;
}
static int rkisp1_isp_set_selection(struct v4l2_subdev *sd,
@@ -857,21 +863,23 @@ static int rkisp1_isp_set_selection(struct v4l2_subdev *sd,
struct rkisp1_device *rkisp1 =
container_of(sd->v4l2_dev, struct rkisp1_device, v4l2_dev);
struct rkisp1_isp *isp = container_of(sd, struct rkisp1_isp, sd);
+ int ret = 0;
if (sel->target != V4L2_SEL_TGT_CROP)
return -EINVAL;
dev_dbg(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, cfg, &sel->r, sel->which);
else if (sel->pad == RKISP1_ISP_PAD_SOURCE_VIDEO)
rkisp1_isp_set_src_crop(isp, cfg, &sel->r, sel->which);
else
- return -EINVAL;
+ ret = -EINVAL;
- return 0;
+ mutex_unlock(&isp->ops_lock);
+ return ret;
}
static int rkisp1_subdev_link_validate(struct media_link *link)
@@ -948,6 +956,7 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
{
struct rkisp1_device *rkisp1 =
container_of(sd->v4l2_dev, struct rkisp1_device, v4l2_dev);
+ struct rkisp1_isp *isp = &rkisp1->isp;
struct v4l2_subdev *sensor_sd;
int ret = 0;
@@ -967,16 +976,19 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
return -EINVAL;
atomic_set(&rkisp1->isp.frame_sequence, -1);
+ mutex_lock(&isp->ops_lock);
ret = rkisp1_config_cif(rkisp1);
if (ret)
- return ret;
+ goto mutex_unlock;
ret = rkisp1_mipi_csi2_start(&rkisp1->isp, rkisp1->active_sensor);
if (ret)
- return ret;
+ goto mutex_unlock;
rkisp1_isp_start(rkisp1);
+mutex_unlock:
+ mutex_unlock(&isp->ops_lock);
return ret;
}
@@ -1036,6 +1048,7 @@ int rkisp1_isp_register(struct rkisp1_device *rkisp1,
isp->sink_fmt = rkisp1_isp_mbus_info_get(RKISP1_DEF_SINK_PAD_FMT);
isp->src_fmt = rkisp1_isp_mbus_info_get(RKISP1_DEF_SRC_PAD_FMT);
+ mutex_init(&isp->ops_lock);
ret = media_entity_pads_init(&sd->entity, RKISP1_ISP_PAD_MAX, pads);
if (ret)
return ret;
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] media: staging: rkisp1: add serialization to the isp subdev ops
2020-02-07 8:59 ` [PATCH 3/4] media: staging: rkisp1: add serialization to the isp subdev ops Dafna Hirschfeld
@ 2020-02-07 13:54 ` Helen Koike
2020-02-14 9:46 ` Sakari Ailus
1 sibling, 0 replies; 10+ messages in thread
From: Helen Koike @ 2020-02-07 13:54 UTC (permalink / raw)
To: Dafna Hirschfeld, linux-media
Cc: ezequiel, hverkuil, kernel, dafna3, sakari.ailus, linux-rockchip,
mchehab
On 2/7/20 6:59 AM, Dafna Hirschfeld wrote:
> For subdevices drivers, the drivers themself are responsible
> for serializing their operations.
> This patch adds serialization to the isp subdevice.
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
Sakari, if you could take a look at this too it would be great.
Thanks,
Helen
> ---
> drivers/staging/media/rkisp1/rkisp1-common.h | 2 ++
> drivers/staging/media/rkisp1/rkisp1-isp.c | 29 ++++++++++++++------
> 2 files changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/media/rkisp1/rkisp1-common.h b/drivers/staging/media/rkisp1/rkisp1-common.h
> index 4e773d611d1b..7c668ac4bdd5 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-common.h
> +++ b/drivers/staging/media/rkisp1/rkisp1-common.h
> @@ -96,6 +96,7 @@ struct rkisp1_sensor_async {
> * @sink_crop: crop for sink pad
> * @src_fmt: output format
> * @src_crop: output size
> + * @ops_lock: ops serialization
> *
> * @is_dphy_errctrl_disabled : if dphy errctrl is disabled (avoid endless interrupt)
> * @frame_sequence: used to synchronize frame_id between video devices.
> @@ -107,6 +108,7 @@ struct rkisp1_isp {
> struct v4l2_subdev_pad_config pad_cfg[RKISP1_ISP_PAD_MAX];
> const struct rkisp1_isp_mbus_info *sink_fmt;
> const struct rkisp1_isp_mbus_info *src_fmt;
> + struct mutex ops_lock;
> bool is_dphy_errctrl_disabled;
> atomic_t frame_sequence;
> };
> diff --git a/drivers/staging/media/rkisp1/rkisp1-isp.c b/drivers/staging/media/rkisp1/rkisp1-isp.c
> index c98e3c16f520..aa7a842f97f8 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-isp.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-isp.c
> @@ -791,7 +791,9 @@ static int rkisp1_isp_get_fmt(struct v4l2_subdev *sd,
> {
> struct rkisp1_isp *isp = container_of(sd, struct rkisp1_isp, sd);
>
> + mutex_lock(&isp->ops_lock);
> fmt->format = *rkisp1_isp_get_pad_fmt(isp, cfg, fmt->pad, fmt->which);
> + mutex_unlock(&isp->ops_lock);
> return 0;
> }
>
> @@ -801,6 +803,7 @@ static int rkisp1_isp_set_fmt(struct v4l2_subdev *sd,
> {
> struct rkisp1_isp *isp = container_of(sd, struct rkisp1_isp, sd);
>
> + mutex_lock(&isp->ops_lock);
> if (fmt->pad == RKISP1_ISP_PAD_SINK_VIDEO)
> rkisp1_isp_set_sink_fmt(isp, cfg, &fmt->format, fmt->which);
> else if (fmt->pad == RKISP1_ISP_PAD_SOURCE_VIDEO)
> @@ -809,6 +812,7 @@ static int rkisp1_isp_set_fmt(struct v4l2_subdev *sd,
> fmt->format = *rkisp1_isp_get_pad_fmt(isp, cfg, fmt->pad,
> fmt->which);
>
> + mutex_unlock(&isp->ops_lock);
> return 0;
> }
>
> @@ -817,11 +821,13 @@ static int rkisp1_isp_get_selection(struct v4l2_subdev *sd,
> struct v4l2_subdev_selection *sel)
> {
> struct rkisp1_isp *isp = container_of(sd, struct 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) {
> @@ -844,10 +850,10 @@ static int rkisp1_isp_get_selection(struct v4l2_subdev *sd,
> sel->which);
> break;
> default:
> - return -EINVAL;
> + ret = -EINVAL;
> }
> -
> - return 0;
> + mutex_unlock(&isp->ops_lock);
> + return ret;
> }
>
> static int rkisp1_isp_set_selection(struct v4l2_subdev *sd,
> @@ -857,21 +863,23 @@ static int rkisp1_isp_set_selection(struct v4l2_subdev *sd,
> struct rkisp1_device *rkisp1 =
> container_of(sd->v4l2_dev, struct rkisp1_device, v4l2_dev);
> struct rkisp1_isp *isp = container_of(sd, struct rkisp1_isp, sd);
> + int ret = 0;
>
> if (sel->target != V4L2_SEL_TGT_CROP)
> return -EINVAL;
>
> dev_dbg(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, cfg, &sel->r, sel->which);
> else if (sel->pad == RKISP1_ISP_PAD_SOURCE_VIDEO)
> rkisp1_isp_set_src_crop(isp, cfg, &sel->r, sel->which);
> else
> - return -EINVAL;
> + ret = -EINVAL;
>
> - return 0;
> + mutex_unlock(&isp->ops_lock);
> + return ret;
> }
>
> static int rkisp1_subdev_link_validate(struct media_link *link)
> @@ -948,6 +956,7 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
> {
> struct rkisp1_device *rkisp1 =
> container_of(sd->v4l2_dev, struct rkisp1_device, v4l2_dev);
> + struct rkisp1_isp *isp = &rkisp1->isp;
> struct v4l2_subdev *sensor_sd;
> int ret = 0;
>
> @@ -967,16 +976,19 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
> return -EINVAL;
>
> atomic_set(&rkisp1->isp.frame_sequence, -1);
> + mutex_lock(&isp->ops_lock);
> ret = rkisp1_config_cif(rkisp1);
> if (ret)
> - return ret;
> + goto mutex_unlock;
>
> ret = rkisp1_mipi_csi2_start(&rkisp1->isp, rkisp1->active_sensor);
> if (ret)
> - return ret;
> + goto mutex_unlock;
>
> rkisp1_isp_start(rkisp1);
>
> +mutex_unlock:
> + mutex_unlock(&isp->ops_lock);
> return ret;
> }
>
> @@ -1036,6 +1048,7 @@ int rkisp1_isp_register(struct rkisp1_device *rkisp1,
> isp->sink_fmt = rkisp1_isp_mbus_info_get(RKISP1_DEF_SINK_PAD_FMT);
> isp->src_fmt = rkisp1_isp_mbus_info_get(RKISP1_DEF_SRC_PAD_FMT);
>
> + mutex_init(&isp->ops_lock);
> ret = media_entity_pads_init(&sd->entity, RKISP1_ISP_PAD_MAX, pads);
> if (ret)
> return ret;
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] media: staging: rkisp1: add serialization to the isp subdev ops
2020-02-07 8:59 ` [PATCH 3/4] media: staging: rkisp1: add serialization to the isp subdev ops Dafna Hirschfeld
2020-02-07 13:54 ` Helen Koike
@ 2020-02-14 9:46 ` Sakari Ailus
1 sibling, 0 replies; 10+ messages in thread
From: Sakari Ailus @ 2020-02-14 9:46 UTC (permalink / raw)
To: Dafna Hirschfeld
Cc: linux-media, helen.koike, ezequiel, hverkuil, kernel, dafna3,
linux-rockchip, mchehab
Hi Dafna,
Thanks for the patchset.
On Fri, Feb 07, 2020 at 09:59:50AM +0100, Dafna Hirschfeld wrote:
> For subdevices drivers, the drivers themself are responsible
> for serializing their operations.
> This patch adds serialization to the isp subdevice.
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
> drivers/staging/media/rkisp1/rkisp1-common.h | 2 ++
> drivers/staging/media/rkisp1/rkisp1-isp.c | 29 ++++++++++++++------
> 2 files changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/media/rkisp1/rkisp1-common.h b/drivers/staging/media/rkisp1/rkisp1-common.h
> index 4e773d611d1b..7c668ac4bdd5 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-common.h
> +++ b/drivers/staging/media/rkisp1/rkisp1-common.h
> @@ -96,6 +96,7 @@ struct rkisp1_sensor_async {
> * @sink_crop: crop for sink pad
> * @src_fmt: output format
> * @src_crop: output size
> + * @ops_lock: ops serialization
> *
> * @is_dphy_errctrl_disabled : if dphy errctrl is disabled (avoid endless interrupt)
> * @frame_sequence: used to synchronize frame_id between video devices.
> @@ -107,6 +108,7 @@ struct rkisp1_isp {
> struct v4l2_subdev_pad_config pad_cfg[RKISP1_ISP_PAD_MAX];
> const struct rkisp1_isp_mbus_info *sink_fmt;
> const struct rkisp1_isp_mbus_info *src_fmt;
> + struct mutex ops_lock;
> bool is_dphy_errctrl_disabled;
> atomic_t frame_sequence;
> };
> diff --git a/drivers/staging/media/rkisp1/rkisp1-isp.c b/drivers/staging/media/rkisp1/rkisp1-isp.c
> index c98e3c16f520..aa7a842f97f8 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-isp.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-isp.c
> @@ -791,7 +791,9 @@ static int rkisp1_isp_get_fmt(struct v4l2_subdev *sd,
> {
> struct rkisp1_isp *isp = container_of(sd, struct rkisp1_isp, sd);
>
> + mutex_lock(&isp->ops_lock);
> fmt->format = *rkisp1_isp_get_pad_fmt(isp, cfg, fmt->pad, fmt->which);
> + mutex_unlock(&isp->ops_lock);
> return 0;
> }
>
> @@ -801,6 +803,7 @@ static int rkisp1_isp_set_fmt(struct v4l2_subdev *sd,
> {
> struct rkisp1_isp *isp = container_of(sd, struct rkisp1_isp, sd);
>
> + mutex_lock(&isp->ops_lock);
> if (fmt->pad == RKISP1_ISP_PAD_SINK_VIDEO)
> rkisp1_isp_set_sink_fmt(isp, cfg, &fmt->format, fmt->which);
> else if (fmt->pad == RKISP1_ISP_PAD_SOURCE_VIDEO)
> @@ -809,6 +812,7 @@ static int rkisp1_isp_set_fmt(struct v4l2_subdev *sd,
> fmt->format = *rkisp1_isp_get_pad_fmt(isp, cfg, fmt->pad,
> fmt->which);
>
> + mutex_unlock(&isp->ops_lock);
> return 0;
> }
>
> @@ -817,11 +821,13 @@ static int rkisp1_isp_get_selection(struct v4l2_subdev *sd,
> struct v4l2_subdev_selection *sel)
> {
> struct rkisp1_isp *isp = container_of(sd, struct 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) {
> @@ -844,10 +850,10 @@ static int rkisp1_isp_get_selection(struct v4l2_subdev *sd,
> sel->which);
> break;
> default:
> - return -EINVAL;
> + ret = -EINVAL;
> }
> -
> - return 0;
> + mutex_unlock(&isp->ops_lock);
> + return ret;
> }
>
> static int rkisp1_isp_set_selection(struct v4l2_subdev *sd,
> @@ -857,21 +863,23 @@ static int rkisp1_isp_set_selection(struct v4l2_subdev *sd,
> struct rkisp1_device *rkisp1 =
> container_of(sd->v4l2_dev, struct rkisp1_device, v4l2_dev);
> struct rkisp1_isp *isp = container_of(sd, struct rkisp1_isp, sd);
> + int ret = 0;
>
> if (sel->target != V4L2_SEL_TGT_CROP)
> return -EINVAL;
>
> dev_dbg(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, cfg, &sel->r, sel->which);
> else if (sel->pad == RKISP1_ISP_PAD_SOURCE_VIDEO)
> rkisp1_isp_set_src_crop(isp, cfg, &sel->r, sel->which);
> else
> - return -EINVAL;
> + ret = -EINVAL;
>
> - return 0;
> + mutex_unlock(&isp->ops_lock);
> + return ret;
> }
>
> static int rkisp1_subdev_link_validate(struct media_link *link)
> @@ -948,6 +956,7 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
> {
> struct rkisp1_device *rkisp1 =
> container_of(sd->v4l2_dev, struct rkisp1_device, v4l2_dev);
> + struct rkisp1_isp *isp = &rkisp1->isp;
> struct v4l2_subdev *sensor_sd;
> int ret = 0;
>
> @@ -967,16 +976,19 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
> return -EINVAL;
>
> atomic_set(&rkisp1->isp.frame_sequence, -1);
> + mutex_lock(&isp->ops_lock);
> ret = rkisp1_config_cif(rkisp1);
> if (ret)
> - return ret;
> + goto mutex_unlock;
>
> ret = rkisp1_mipi_csi2_start(&rkisp1->isp, rkisp1->active_sensor);
> if (ret)
> - return ret;
> + goto mutex_unlock;
>
> rkisp1_isp_start(rkisp1);
>
> +mutex_unlock:
> + mutex_unlock(&isp->ops_lock);
> return ret;
> }
>
> @@ -1036,6 +1048,7 @@ int rkisp1_isp_register(struct rkisp1_device *rkisp1,
> isp->sink_fmt = rkisp1_isp_mbus_info_get(RKISP1_DEF_SINK_PAD_FMT);
> isp->src_fmt = rkisp1_isp_mbus_info_get(RKISP1_DEF_SRC_PAD_FMT);
>
> + mutex_init(&isp->ops_lock);
> ret = media_entity_pads_init(&sd->entity, RKISP1_ISP_PAD_MAX, pads);
> if (ret)
> return ret;
Could you add mutex_destroy() on the mutex in rkisp1_isp_unregister(),
please? Similar comment on the 4th patch. This could be a follow-up patch,
too.
For the set,
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/4] media: staging: rkisp1: add serialization to the resizer subdev ops
2020-02-07 8:59 [PATCH 0/4] media: staging: rkisp1: add serialization to the isp and resizer ops Dafna Hirschfeld
` (2 preceding siblings ...)
2020-02-07 8:59 ` [PATCH 3/4] media: staging: rkisp1: add serialization to the isp subdev ops Dafna Hirschfeld
@ 2020-02-07 8:59 ` Dafna Hirschfeld
2020-02-07 13:56 ` Helen Koike
3 siblings, 1 reply; 10+ messages in thread
From: Dafna Hirschfeld @ 2020-02-07 8:59 UTC (permalink / raw)
To: linux-media
Cc: dafna.hirschfeld, helen.koike, ezequiel, hverkuil, kernel,
dafna3, sakari.ailus, linux-rockchip, mchehab
For subdevices drivers, the drivers themself are responsible
for serializing their operations.
This patch adds serialization to the resizer subdevice.
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
drivers/staging/media/rkisp1/rkisp1-common.h | 1 +
drivers/staging/media/rkisp1/rkisp1-resizer.c | 16 ++++++++++++++--
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/rkisp1/rkisp1-common.h b/drivers/staging/media/rkisp1/rkisp1-common.h
index 7c668ac4bdd5..18507f5b6f3c 100644
--- a/drivers/staging/media/rkisp1/rkisp1-common.h
+++ b/drivers/staging/media/rkisp1/rkisp1-common.h
@@ -226,6 +226,7 @@ struct rkisp1_resizer {
struct v4l2_subdev_pad_config pad_cfg[RKISP1_ISP_PAD_MAX];
const struct rkisp1_rsz_config *config;
enum rkisp1_fmt_pix_type fmt_type;
+ struct mutex ops_lock;
};
struct rkisp1_debug {
diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c
index 9de6744f0900..87799fbf0363 100644
--- a/drivers/staging/media/rkisp1/rkisp1-resizer.c
+++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c
@@ -562,7 +562,9 @@ static int rkisp1_rsz_get_fmt(struct v4l2_subdev *sd,
struct rkisp1_resizer *rsz =
container_of(sd, struct rkisp1_resizer, sd);
+ mutex_lock(&rsz->ops_lock);
fmt->format = *rkisp1_rsz_get_pad_fmt(rsz, cfg, fmt->pad, fmt->which);
+ mutex_unlock(&rsz->ops_lock);
return 0;
}
@@ -573,11 +575,13 @@ 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, cfg, &fmt->format, fmt->which);
else
rkisp1_rsz_set_src_fmt(rsz, cfg, &fmt->format, fmt->which);
+ mutex_unlock(&rsz->ops_lock);
return 0;
}
@@ -588,10 +592,12 @@ static int rkisp1_rsz_get_selection(struct v4l2_subdev *sd,
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, cfg, RKISP1_RSZ_PAD_SINK,
@@ -606,10 +612,11 @@ static int rkisp1_rsz_get_selection(struct v4l2_subdev *sd,
sel->which);
break;
default:
- return -EINVAL;
+ ret = -EINVAL;
}
- return 0;
+ mutex_unlock(&rsz->ops_lock);
+ return ret;
}
static int rkisp1_rsz_set_selection(struct v4l2_subdev *sd,
@@ -625,7 +632,9 @@ static int rkisp1_rsz_set_selection(struct v4l2_subdev *sd,
dev_dbg(sd->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, cfg, &sel->r, sel->which);
+ mutex_unlock(&rsz->ops_lock);
return 0;
}
@@ -665,9 +674,11 @@ 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);
+ mutex_unlock(&rsz->ops_lock);
return 0;
}
@@ -713,6 +724,7 @@ static int rkisp1_rsz_register(struct rkisp1_resizer *rsz)
rsz->fmt_type = RKISP1_DEF_FMT_TYPE;
+ mutex_init(&rsz->ops_lock);
ret = media_entity_pads_init(&sd->entity, 2, pads);
if (ret)
return ret;
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] media: staging: rkisp1: add serialization to the resizer subdev ops
2020-02-07 8:59 ` [PATCH 4/4] media: staging: rkisp1: add serialization to the resizer " Dafna Hirschfeld
@ 2020-02-07 13:56 ` Helen Koike
0 siblings, 0 replies; 10+ messages in thread
From: Helen Koike @ 2020-02-07 13:56 UTC (permalink / raw)
To: Dafna Hirschfeld, linux-media
Cc: ezequiel, hverkuil, kernel, dafna3, sakari.ailus, linux-rockchip,
mchehab
On 2/7/20 6:59 AM, Dafna Hirschfeld wrote:
> For subdevices drivers, the drivers themself are responsible
> for serializing their operations.
> This patch adds serialization to the resizer subdevice.
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
Sakari, if you could take a look at this too it would be great.
Dafna, could you also send a patch removing the item in the TODO list
regarding ioctls serialization?
Thanks,
Helen
> ---
> drivers/staging/media/rkisp1/rkisp1-common.h | 1 +
> drivers/staging/media/rkisp1/rkisp1-resizer.c | 16 ++++++++++++++--
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/media/rkisp1/rkisp1-common.h b/drivers/staging/media/rkisp1/rkisp1-common.h
> index 7c668ac4bdd5..18507f5b6f3c 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-common.h
> +++ b/drivers/staging/media/rkisp1/rkisp1-common.h
> @@ -226,6 +226,7 @@ struct rkisp1_resizer {
> struct v4l2_subdev_pad_config pad_cfg[RKISP1_ISP_PAD_MAX];
> const struct rkisp1_rsz_config *config;
> enum rkisp1_fmt_pix_type fmt_type;
> + struct mutex ops_lock;
> };
>
> struct rkisp1_debug {
> diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c
> index 9de6744f0900..87799fbf0363 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-resizer.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c
> @@ -562,7 +562,9 @@ static int rkisp1_rsz_get_fmt(struct v4l2_subdev *sd,
> struct rkisp1_resizer *rsz =
> container_of(sd, struct rkisp1_resizer, sd);
>
> + mutex_lock(&rsz->ops_lock);
> fmt->format = *rkisp1_rsz_get_pad_fmt(rsz, cfg, fmt->pad, fmt->which);
> + mutex_unlock(&rsz->ops_lock);
> return 0;
> }
>
> @@ -573,11 +575,13 @@ 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, cfg, &fmt->format, fmt->which);
> else
> rkisp1_rsz_set_src_fmt(rsz, cfg, &fmt->format, fmt->which);
>
> + mutex_unlock(&rsz->ops_lock);
> return 0;
> }
>
> @@ -588,10 +592,12 @@ static int rkisp1_rsz_get_selection(struct v4l2_subdev *sd,
> 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, cfg, RKISP1_RSZ_PAD_SINK,
> @@ -606,10 +612,11 @@ static int rkisp1_rsz_get_selection(struct v4l2_subdev *sd,
> sel->which);
> break;
> default:
> - return -EINVAL;
> + ret = -EINVAL;
> }
>
> - return 0;
> + mutex_unlock(&rsz->ops_lock);
> + return ret;
> }
>
> static int rkisp1_rsz_set_selection(struct v4l2_subdev *sd,
> @@ -625,7 +632,9 @@ static int rkisp1_rsz_set_selection(struct v4l2_subdev *sd,
> dev_dbg(sd->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, cfg, &sel->r, sel->which);
> + mutex_unlock(&rsz->ops_lock);
>
> return 0;
> }
> @@ -665,9 +674,11 @@ 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);
>
> + mutex_unlock(&rsz->ops_lock);
> return 0;
> }
>
> @@ -713,6 +724,7 @@ static int rkisp1_rsz_register(struct rkisp1_resizer *rsz)
>
> rsz->fmt_type = RKISP1_DEF_FMT_TYPE;
>
> + mutex_init(&rsz->ops_lock);
> ret = media_entity_pads_init(&sd->entity, 2, pads);
> if (ret)
> return ret;
>
^ permalink raw reply [flat|nested] 10+ messages in thread