[PATCH 0/4] media: staging: rkisp1: add serialization to the isp and resizer ops

linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH 0/4] media: staging: rkisp1: add serialization to the isp and resizer ops
@ 2020-02-07  8:59 Dafna Hirschfeld
  2020-02-07  8:59 ` [PATCH 1/4] media: staging: rkisp1: change function to return void instead of int Dafna Hirschfeld
                   ` (3 more replies)
  0 siblings, 4 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

Subdevices' ops callbacks can be called either through uAPI with
ioctl or through kAPI with the v4l2_subdev_call macro. Therefore
the lock of the subdevice node is not set and instead the driver
itself is responsible for serializing the ops.
This patchset adds serialization to the isp and resizer subdevices.
A mutex field 'ops_lock' is added to the inner struct of each of them.

The ops_lock is then used in the following operations:

set_fmt, get_fmt, set_selection, get_selection, s_stream

Serialization for enum_mbus_code is not needed since this operation
is independent of the specific configuration.

Patches summary:

patch 1 Changes two functions that return int to return void
        and removes a redundant check of error since the functions don't fail.

patch 2 moves the check that the bus type is DPHY before initializing registers
        in the s_stream callback of the isp.

patch 3 adds serialization for the isp subdevice.

patch 4 adds serialization for the resizer subdevice.

Dafna Hirschfeld (4):
  media: staging: rkisp1: change function to return void instead of int
  media: staging: rkisp1: isp: check for dphy bus before initializations
    in s_stream
  media: staging: rkisp1: add serialization to the isp subdev ops
  media: staging: rkisp1: add serialization to the resizer subdev ops

 drivers/staging/media/rkisp1/rkisp1-common.h  |  3 ++
 drivers/staging/media/rkisp1/rkisp1-isp.c     | 51 ++++++++++---------
 drivers/staging/media/rkisp1/rkisp1-resizer.c | 16 +++++-
 3 files changed, 45 insertions(+), 25 deletions(-)

-- 
2.17.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [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

end of thread, other threads:[~2020-02-14  9:46 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 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
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 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
2020-02-07 13:56   ` Helen Koike

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).