[3/4] media: i2c: st-vgxy61: Switch to {enable,disable}_streams
Commit Message
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
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 = {
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.
On 4/3/24 14:38, Sakari Ailus wrote:
> 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.
>
Perfect. Thank you for the clarification.
@@ -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 = {