diff mbox series

[3/4] media: i2c: st-vgxy61: Switch to {enable,disable}_streams

Message ID 20240315085158.1213159-4-julien.massot@collabora.com
State New
Headers show
Series [1/4] media: i2c: st-vgxy61: Use sub-device active state | expand

Commit Message

Julien Massot March 15, 2024, 8:51 a.m. UTC
Switch from s_stream to enable_streams and disable_streams callbacks.

Signed-off-by: Julien Massot <julien.massot@collabora.com>
---
 drivers/media/i2c/st-vgxy61.c | 37 +++++++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 10 deletions(-)

Comments

Benjamin Mugnier April 3, 2024, 12:26 p.m. UTC | #1
Hi Julien,

On 3/15/24 09:51, Julien Massot wrote:
> Switch from s_stream to enable_streams and disable_streams callbacks.
> 
> Signed-off-by: Julien Massot <julien.massot@collabora.com>
> ---
>  drivers/media/i2c/st-vgxy61.c | 37 +++++++++++++++++++++++++----------
>  1 file changed, 27 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/i2c/st-vgxy61.c b/drivers/media/i2c/st-vgxy61.c
> index e8302456a8d9..754853378ee6 100644
> --- a/drivers/media/i2c/st-vgxy61.c
> +++ b/drivers/media/i2c/st-vgxy61.c
> @@ -420,7 +420,7 @@ struct vgxy61_dev {
>  	struct v4l2_ctrl *vblank_ctrl;
>  	struct v4l2_ctrl *vflip_ctrl;
>  	struct v4l2_ctrl *hflip_ctrl;
> -	bool streaming;
> +	u8 streaming;
>  	struct v4l2_mbus_framefmt fmt;
>  	const struct vgxy61_mode_info *sensor_modes;
>  	unsigned int sensor_modes_nb;
> @@ -1188,20 +1188,35 @@ static int vgxy61_stream_disable(struct vgxy61_dev *sensor)
>  	return ret;
>  }
>  
> -static int vgxy61_s_stream(struct v4l2_subdev *sd, int enable)
> +static int vgxy61_enable_streams(struct v4l2_subdev *sd,
> +				 struct v4l2_subdev_state *state, u32 pad,
> +				 u64 streams_mask)
>  {

Should we also check that 'pad == 0' ? Or is it always so ?

>  	struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
> -	struct v4l2_subdev_state *sd_state;
>  	int ret = 0;
>  
> -	sd_state = v4l2_subdev_lock_and_get_active_state(&sensor->sd);
> -
> -	ret = enable ? vgxy61_stream_enable(sensor) :
> -	      vgxy61_stream_disable(sensor);
> +	if (!sensor->streaming)
> +		ret = vgxy61_stream_enable(sensor);
>  	if (!ret)
> -		sensor->streaming = enable;
> +		sensor->streaming |= streams_mask;
>  
> -	v4l2_subdev_unlock_state(sd_state);
> +	return ret;
> +}
> +
> +static int vgxy61_disable_streams(struct v4l2_subdev *sd,
> +				  struct v4l2_subdev_state *state, u32 pad,
> +				  u64 streams_mask)
> +{

Ditto.

> +	struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
> +	int ret;
> +
> +	sensor->streaming &= ~streams_mask;
> +	if (sensor->streaming)
> +		return 0;
> +
> +	ret = vgxy61_stream_disable(sensor);
> +	if (!ret)
> +		sensor->streaming = false;
>  
>  	return ret;
>  }
> @@ -1496,7 +1511,7 @@ static const struct v4l2_subdev_core_ops vgxy61_core_ops = {
>  };
>  
>  static const struct v4l2_subdev_video_ops vgxy61_video_ops = {
> -	.s_stream = vgxy61_s_stream,
> +	.s_stream = v4l2_subdev_s_stream_helper,
>  };
>  
>  static const struct v4l2_subdev_pad_ops vgxy61_pad_ops = {
> @@ -1506,6 +1521,8 @@ static const struct v4l2_subdev_pad_ops vgxy61_pad_ops = {
>  	.get_frame_desc = vgxy61_get_frame_desc,
>  	.get_selection = vgxy61_get_selection,
>  	.enum_frame_size = vgxy61_enum_frame_size,
> +	.enable_streams = vgxy61_enable_streams,
> +	.disable_streams = vgxy61_disable_streams,
>  };
>  
>  static const struct v4l2_subdev_ops vgxy61_subdev_ops = {
Sakari Ailus April 3, 2024, 12:38 p.m. UTC | #2
On Wed, Apr 03, 2024 at 02:26:32PM +0200, Benjamin Mugnier wrote:
> > @@ -1188,20 +1188,35 @@ static int vgxy61_stream_disable(struct vgxy61_dev *sensor)
> >  	return ret;
> >  }
> >  
> > -static int vgxy61_s_stream(struct v4l2_subdev *sd, int enable)
> > +static int vgxy61_enable_streams(struct v4l2_subdev *sd,
> > +				 struct v4l2_subdev_state *state, u32 pad,
> > +				 u64 streams_mask)
> >  {
> 
> Should we also check that 'pad == 0' ? Or is it always so ?

If the number of pads in the sub-device is 1, then the pad is always 0
here: this is checked in the framework.
diff mbox series

Patch

diff --git a/drivers/media/i2c/st-vgxy61.c b/drivers/media/i2c/st-vgxy61.c
index e8302456a8d9..754853378ee6 100644
--- a/drivers/media/i2c/st-vgxy61.c
+++ b/drivers/media/i2c/st-vgxy61.c
@@ -420,7 +420,7 @@  struct vgxy61_dev {
 	struct v4l2_ctrl *vblank_ctrl;
 	struct v4l2_ctrl *vflip_ctrl;
 	struct v4l2_ctrl *hflip_ctrl;
-	bool streaming;
+	u8 streaming;
 	struct v4l2_mbus_framefmt fmt;
 	const struct vgxy61_mode_info *sensor_modes;
 	unsigned int sensor_modes_nb;
@@ -1188,20 +1188,35 @@  static int vgxy61_stream_disable(struct vgxy61_dev *sensor)
 	return ret;
 }
 
-static int vgxy61_s_stream(struct v4l2_subdev *sd, int enable)
+static int vgxy61_enable_streams(struct v4l2_subdev *sd,
+				 struct v4l2_subdev_state *state, u32 pad,
+				 u64 streams_mask)
 {
 	struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
-	struct v4l2_subdev_state *sd_state;
 	int ret = 0;
 
-	sd_state = v4l2_subdev_lock_and_get_active_state(&sensor->sd);
-
-	ret = enable ? vgxy61_stream_enable(sensor) :
-	      vgxy61_stream_disable(sensor);
+	if (!sensor->streaming)
+		ret = vgxy61_stream_enable(sensor);
 	if (!ret)
-		sensor->streaming = enable;
+		sensor->streaming |= streams_mask;
 
-	v4l2_subdev_unlock_state(sd_state);
+	return ret;
+}
+
+static int vgxy61_disable_streams(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_state *state, u32 pad,
+				  u64 streams_mask)
+{
+	struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
+	int ret;
+
+	sensor->streaming &= ~streams_mask;
+	if (sensor->streaming)
+		return 0;
+
+	ret = vgxy61_stream_disable(sensor);
+	if (!ret)
+		sensor->streaming = false;
 
 	return ret;
 }
@@ -1496,7 +1511,7 @@  static const struct v4l2_subdev_core_ops vgxy61_core_ops = {
 };
 
 static const struct v4l2_subdev_video_ops vgxy61_video_ops = {
-	.s_stream = vgxy61_s_stream,
+	.s_stream = v4l2_subdev_s_stream_helper,
 };
 
 static const struct v4l2_subdev_pad_ops vgxy61_pad_ops = {
@@ -1506,6 +1521,8 @@  static const struct v4l2_subdev_pad_ops vgxy61_pad_ops = {
 	.get_frame_desc = vgxy61_get_frame_desc,
 	.get_selection = vgxy61_get_selection,
 	.enum_frame_size = vgxy61_enum_frame_size,
+	.enable_streams = vgxy61_enable_streams,
+	.disable_streams = vgxy61_disable_streams,
 };
 
 static const struct v4l2_subdev_ops vgxy61_subdev_ops = {