[v8,33/38] media: ov2740: Switch to {enable,disable}_streams

Message ID 20240313072516.241106-34-sakari.ailus@linux.intel.com (mailing list archive)
State New
Delegated to: Sakari Ailus
Headers
Series Generic line based metadata support, internal pads |

Commit Message

Sakari Ailus March 13, 2024, 7:25 a.m. UTC
  Switch from s_stream to enable_streams and disable_streams callbacks.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov2740.c | 72 +++++++++++++++++---------------------
 1 file changed, 32 insertions(+), 40 deletions(-)
  

Comments

Julien Massot March 15, 2024, 4:13 p.m. UTC | #1
On 3/13/24 08:25, Sakari Ailus wrote:
> Switch from s_stream to enable_streams and disable_streams callbacks.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Julien Massot <julien.massot@collabora.com>
> ---
>   drivers/media/i2c/ov2740.c | 72 +++++++++++++++++---------------------
>   1 file changed, 32 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
> index 552935ccb4a9..44c6724a102c 100644
> --- a/drivers/media/i2c/ov2740.c
> +++ b/drivers/media/i2c/ov2740.c
> @@ -919,16 +919,23 @@ static int ov2740_load_otp_data(struct nvm_data *nvm)
>   	return ret;
>   }
>   
> -static int ov2740_start_streaming(struct ov2740 *ov2740)
> +static int ov2740_enable_streams(struct v4l2_subdev *sd,
> +				 struct v4l2_subdev_state *state, u32 pad,
> +				 u64 streams_mask)
>   {
> -	struct i2c_client *client = v4l2_get_subdevdata(&ov2740->sd);
> +	struct i2c_client *client = v4l2_get_subdevdata(sd);
> +	struct ov2740 *ov2740 = to_ov2740(sd);
>   	const struct ov2740_reg_list *reg_list;
>   	int link_freq_index;
>   	int ret;
>   
> +	ret = pm_runtime_resume_and_get(&client->dev);
> +	if (ret < 0)
> +		return ret;
> +
>   	ret = ov2740_identify_module(ov2740);
>   	if (ret)
> -		return ret;
> +		goto out_pm_put;
>   
>   	if (ov2740->nvm)
>   		ov2740_load_otp_data(ov2740->nvm);
> @@ -937,7 +944,7 @@ static int ov2740_start_streaming(struct ov2740 *ov2740)
>   	ret = ov2740_write_reg(ov2740, 0x0103, 1, 0x01);
>   	if (ret) {
>   		dev_err(&client->dev, "failed to reset\n");
> -		return ret;
> +		goto out_pm_put;
>   	}
>   
>   	usleep_range(10000, 15000);
> @@ -947,64 +954,47 @@ static int ov2740_start_streaming(struct ov2740 *ov2740)
>   	ret = ov2740_write_reg_list(ov2740, reg_list);
>   	if (ret) {
>   		dev_err(&client->dev, "failed to set plls\n");
> -		return ret;
> +		goto out_pm_put;
>   	}
>   
>   	reg_list = &ov2740->cur_mode->reg_list;
>   	ret = ov2740_write_reg_list(ov2740, reg_list);
>   	if (ret) {
>   		dev_err(&client->dev, "failed to set mode\n");
> -		return ret;
> +		goto out_pm_put;
>   	}
>   
>   	ret = __v4l2_ctrl_handler_setup(ov2740->sd.ctrl_handler);
>   	if (ret)
> -		return ret;
> +		goto out_pm_put;
>   
>   	ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
>   			       OV2740_MODE_STREAMING);
> -	if (ret)
> +	if (ret) {
>   		dev_err(&client->dev, "failed to start streaming\n");
> +		goto out_pm_put;
> +	}
>   
> -	return ret;
> -}
> +	return 0;
>   
> -static void ov2740_stop_streaming(struct ov2740 *ov2740)
> -{
> -	struct i2c_client *client = v4l2_get_subdevdata(&ov2740->sd);
> +out_pm_put:
> +	pm_runtime_put(&client->dev);
>   
> -	if (ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
> -			     OV2740_MODE_STANDBY))
> -		dev_err(&client->dev, "failed to stop streaming\n");
> +	return ret;
>   }
>   
> -static int ov2740_set_stream(struct v4l2_subdev *sd, int enable)
> +static int ov2740_disable_streams(struct v4l2_subdev *sd,
> +				  struct v4l2_subdev_state *state, u32 pad,
> +				  u64 streams_mask)
>   {
> -	struct ov2740 *ov2740 = to_ov2740(sd);
>   	struct i2c_client *client = v4l2_get_subdevdata(sd);
> -	struct v4l2_subdev_state *sd_state;
> -	int ret = 0;
> -
> -	sd_state = v4l2_subdev_lock_and_get_active_state(&ov2740->sd);
> -
> -	if (enable) {
> -		ret = pm_runtime_resume_and_get(&client->dev);
> -		if (ret < 0)
> -			goto out_unlock;
> +	struct ov2740 *ov2740 = to_ov2740(sd);
> +	int ret;
>   
> -		ret = ov2740_start_streaming(ov2740);
> -		if (ret) {
> -			enable = 0;
> -			ov2740_stop_streaming(ov2740);
> -			pm_runtime_put(&client->dev);
> -		}
> -	} else {
> -		ov2740_stop_streaming(ov2740);
> -		pm_runtime_put(&client->dev);
> -	}
> +	ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
> +			       OV2740_MODE_STANDBY);
>   
> -out_unlock:
> -	v4l2_subdev_unlock_state(sd_state);
> +	pm_runtime_put(&client->dev);
>   
>   	return ret;
>   }
> @@ -1089,7 +1079,7 @@ static int ov2740_init_state(struct v4l2_subdev *sd,
>   }
>   
>   static const struct v4l2_subdev_video_ops ov2740_video_ops = {
> -	.s_stream = ov2740_set_stream,
> +	.s_stream = v4l2_subdev_s_stream_helper,
>   };
>   
>   static const struct v4l2_subdev_pad_ops ov2740_pad_ops = {
> @@ -1097,6 +1087,8 @@ static const struct v4l2_subdev_pad_ops ov2740_pad_ops = {
>   	.set_fmt = ov2740_set_format,
>   	.enum_mbus_code = ov2740_enum_mbus_code,
>   	.enum_frame_size = ov2740_enum_frame_size,
> +	.enable_streams = ov2740_enable_streams,
> +	.disable_streams = ov2740_disable_streams,
>   };
>   
>   static const struct v4l2_subdev_ops ov2740_subdev_ops = {
  
Laurent Pinchart March 21, 2024, 4:56 p.m. UTC | #2
Hi Sakari,

Thank you for the patch.

On Wed, Mar 13, 2024 at 09:25:11AM +0200, Sakari Ailus wrote:
> Switch from s_stream to enable_streams and disable_streams callbacks.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
>  drivers/media/i2c/ov2740.c | 72 +++++++++++++++++---------------------
>  1 file changed, 32 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
> index 552935ccb4a9..44c6724a102c 100644
> --- a/drivers/media/i2c/ov2740.c
> +++ b/drivers/media/i2c/ov2740.c
> @@ -919,16 +919,23 @@ static int ov2740_load_otp_data(struct nvm_data *nvm)
>  	return ret;
>  }
>  
> -static int ov2740_start_streaming(struct ov2740 *ov2740)
> +static int ov2740_enable_streams(struct v4l2_subdev *sd,
> +				 struct v4l2_subdev_state *state, u32 pad,
> +				 u64 streams_mask)
>  {
> -	struct i2c_client *client = v4l2_get_subdevdata(&ov2740->sd);
> +	struct i2c_client *client = v4l2_get_subdevdata(sd);
> +	struct ov2740 *ov2740 = to_ov2740(sd);
>  	const struct ov2740_reg_list *reg_list;
>  	int link_freq_index;
>  	int ret;
>  
> +	ret = pm_runtime_resume_and_get(&client->dev);
> +	if (ret < 0)
> +		return ret;
> +
>  	ret = ov2740_identify_module(ov2740);
>  	if (ret)
> -		return ret;
> +		goto out_pm_put;
>  
>  	if (ov2740->nvm)
>  		ov2740_load_otp_data(ov2740->nvm);
> @@ -937,7 +944,7 @@ static int ov2740_start_streaming(struct ov2740 *ov2740)
>  	ret = ov2740_write_reg(ov2740, 0x0103, 1, 0x01);
>  	if (ret) {
>  		dev_err(&client->dev, "failed to reset\n");
> -		return ret;
> +		goto out_pm_put;
>  	}
>  
>  	usleep_range(10000, 15000);
> @@ -947,64 +954,47 @@ static int ov2740_start_streaming(struct ov2740 *ov2740)
>  	ret = ov2740_write_reg_list(ov2740, reg_list);
>  	if (ret) {
>  		dev_err(&client->dev, "failed to set plls\n");
> -		return ret;
> +		goto out_pm_put;
>  	}
>  
>  	reg_list = &ov2740->cur_mode->reg_list;
>  	ret = ov2740_write_reg_list(ov2740, reg_list);
>  	if (ret) {
>  		dev_err(&client->dev, "failed to set mode\n");
> -		return ret;
> +		goto out_pm_put;
>  	}
>  
>  	ret = __v4l2_ctrl_handler_setup(ov2740->sd.ctrl_handler);
>  	if (ret)
> -		return ret;
> +		goto out_pm_put;
>  
>  	ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
>  			       OV2740_MODE_STREAMING);
> -	if (ret)
> +	if (ret) {
>  		dev_err(&client->dev, "failed to start streaming\n");
> +		goto out_pm_put;
> +	}
>  
> -	return ret;
> -}
> +	return 0;
>  
> -static void ov2740_stop_streaming(struct ov2740 *ov2740)
> -{
> -	struct i2c_client *client = v4l2_get_subdevdata(&ov2740->sd);
> +out_pm_put:
> +	pm_runtime_put(&client->dev);
>  
> -	if (ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
> -			     OV2740_MODE_STANDBY))
> -		dev_err(&client->dev, "failed to stop streaming\n");
> +	return ret;
>  }
>  
> -static int ov2740_set_stream(struct v4l2_subdev *sd, int enable)
> +static int ov2740_disable_streams(struct v4l2_subdev *sd,
> +				  struct v4l2_subdev_state *state, u32 pad,
> +				  u64 streams_mask)
>  {
> -	struct ov2740 *ov2740 = to_ov2740(sd);
>  	struct i2c_client *client = v4l2_get_subdevdata(sd);
> -	struct v4l2_subdev_state *sd_state;
> -	int ret = 0;
> -
> -	sd_state = v4l2_subdev_lock_and_get_active_state(&ov2740->sd);
> -
> -	if (enable) {
> -		ret = pm_runtime_resume_and_get(&client->dev);
> -		if (ret < 0)
> -			goto out_unlock;
> +	struct ov2740 *ov2740 = to_ov2740(sd);
> +	int ret;
>  
> -		ret = ov2740_start_streaming(ov2740);
> -		if (ret) {
> -			enable = 0;
> -			ov2740_stop_streaming(ov2740);
> -			pm_runtime_put(&client->dev);
> -		}
> -	} else {
> -		ov2740_stop_streaming(ov2740);
> -		pm_runtime_put(&client->dev);
> -	}
> +	ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
> +			       OV2740_MODE_STANDBY);
>  
> -out_unlock:
> -	v4l2_subdev_unlock_state(sd_state);
> +	pm_runtime_put(&client->dev);
>  
>  	return ret;
>  }
> @@ -1089,7 +1079,7 @@ static int ov2740_init_state(struct v4l2_subdev *sd,
>  }
>  
>  static const struct v4l2_subdev_video_ops ov2740_video_ops = {
> -	.s_stream = ov2740_set_stream,
> +	.s_stream = v4l2_subdev_s_stream_helper,
>  };
>  
>  static const struct v4l2_subdev_pad_ops ov2740_pad_ops = {
> @@ -1097,6 +1087,8 @@ static const struct v4l2_subdev_pad_ops ov2740_pad_ops = {
>  	.set_fmt = ov2740_set_format,
>  	.enum_mbus_code = ov2740_enum_mbus_code,
>  	.enum_frame_size = ov2740_enum_frame_size,
> +	.enable_streams = ov2740_enable_streams,
> +	.disable_streams = ov2740_disable_streams,
>  };
>  
>  static const struct v4l2_subdev_ops ov2740_subdev_ops = {
  

Patch

diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
index 552935ccb4a9..44c6724a102c 100644
--- a/drivers/media/i2c/ov2740.c
+++ b/drivers/media/i2c/ov2740.c
@@ -919,16 +919,23 @@  static int ov2740_load_otp_data(struct nvm_data *nvm)
 	return ret;
 }
 
-static int ov2740_start_streaming(struct ov2740 *ov2740)
+static int ov2740_enable_streams(struct v4l2_subdev *sd,
+				 struct v4l2_subdev_state *state, u32 pad,
+				 u64 streams_mask)
 {
-	struct i2c_client *client = v4l2_get_subdevdata(&ov2740->sd);
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct ov2740 *ov2740 = to_ov2740(sd);
 	const struct ov2740_reg_list *reg_list;
 	int link_freq_index;
 	int ret;
 
+	ret = pm_runtime_resume_and_get(&client->dev);
+	if (ret < 0)
+		return ret;
+
 	ret = ov2740_identify_module(ov2740);
 	if (ret)
-		return ret;
+		goto out_pm_put;
 
 	if (ov2740->nvm)
 		ov2740_load_otp_data(ov2740->nvm);
@@ -937,7 +944,7 @@  static int ov2740_start_streaming(struct ov2740 *ov2740)
 	ret = ov2740_write_reg(ov2740, 0x0103, 1, 0x01);
 	if (ret) {
 		dev_err(&client->dev, "failed to reset\n");
-		return ret;
+		goto out_pm_put;
 	}
 
 	usleep_range(10000, 15000);
@@ -947,64 +954,47 @@  static int ov2740_start_streaming(struct ov2740 *ov2740)
 	ret = ov2740_write_reg_list(ov2740, reg_list);
 	if (ret) {
 		dev_err(&client->dev, "failed to set plls\n");
-		return ret;
+		goto out_pm_put;
 	}
 
 	reg_list = &ov2740->cur_mode->reg_list;
 	ret = ov2740_write_reg_list(ov2740, reg_list);
 	if (ret) {
 		dev_err(&client->dev, "failed to set mode\n");
-		return ret;
+		goto out_pm_put;
 	}
 
 	ret = __v4l2_ctrl_handler_setup(ov2740->sd.ctrl_handler);
 	if (ret)
-		return ret;
+		goto out_pm_put;
 
 	ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
 			       OV2740_MODE_STREAMING);
-	if (ret)
+	if (ret) {
 		dev_err(&client->dev, "failed to start streaming\n");
+		goto out_pm_put;
+	}
 
-	return ret;
-}
+	return 0;
 
-static void ov2740_stop_streaming(struct ov2740 *ov2740)
-{
-	struct i2c_client *client = v4l2_get_subdevdata(&ov2740->sd);
+out_pm_put:
+	pm_runtime_put(&client->dev);
 
-	if (ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
-			     OV2740_MODE_STANDBY))
-		dev_err(&client->dev, "failed to stop streaming\n");
+	return ret;
 }
 
-static int ov2740_set_stream(struct v4l2_subdev *sd, int enable)
+static int ov2740_disable_streams(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_state *state, u32 pad,
+				  u64 streams_mask)
 {
-	struct ov2740 *ov2740 = to_ov2740(sd);
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	struct v4l2_subdev_state *sd_state;
-	int ret = 0;
-
-	sd_state = v4l2_subdev_lock_and_get_active_state(&ov2740->sd);
-
-	if (enable) {
-		ret = pm_runtime_resume_and_get(&client->dev);
-		if (ret < 0)
-			goto out_unlock;
+	struct ov2740 *ov2740 = to_ov2740(sd);
+	int ret;
 
-		ret = ov2740_start_streaming(ov2740);
-		if (ret) {
-			enable = 0;
-			ov2740_stop_streaming(ov2740);
-			pm_runtime_put(&client->dev);
-		}
-	} else {
-		ov2740_stop_streaming(ov2740);
-		pm_runtime_put(&client->dev);
-	}
+	ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
+			       OV2740_MODE_STANDBY);
 
-out_unlock:
-	v4l2_subdev_unlock_state(sd_state);
+	pm_runtime_put(&client->dev);
 
 	return ret;
 }
@@ -1089,7 +1079,7 @@  static int ov2740_init_state(struct v4l2_subdev *sd,
 }
 
 static const struct v4l2_subdev_video_ops ov2740_video_ops = {
-	.s_stream = ov2740_set_stream,
+	.s_stream = v4l2_subdev_s_stream_helper,
 };
 
 static const struct v4l2_subdev_pad_ops ov2740_pad_ops = {
@@ -1097,6 +1087,8 @@  static const struct v4l2_subdev_pad_ops ov2740_pad_ops = {
 	.set_fmt = ov2740_set_format,
 	.enum_mbus_code = ov2740_enum_mbus_code,
 	.enum_frame_size = ov2740_enum_frame_size,
+	.enable_streams = ov2740_enable_streams,
+	.disable_streams = ov2740_disable_streams,
 };
 
 static const struct v4l2_subdev_ops ov2740_subdev_ops = {