diff mbox series

[v6,03/15] media: i2c: imx219: Report internal routes to userspace

Message ID 20240301213231.10340-4-laurent.pinchart@ideasonboard.com (mailing list archive)
State New, archived
Headers show
Series media: Add driver for the Raspberry Pi <5 CSI-2 receiver | expand

Commit Message

Laurent Pinchart March 1, 2024, 9:32 p.m. UTC
Usage of internal pads creates a route internal to the subdev, and the
V4L2 camera sensor API requires such routes to be reported to userspace.
Create the route in the .init_cfg() operation.

Internal routing support requires stream support, so set the
V4L2_SUBDEV_FL_HAS_STREAMS flag and switch formats and selection
rectangles access from pads to streams. As the route is immutable,
there's no need to implement the .set_routing() operation, and we can
hardcode the sink and source stream IDs to 0.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/imx219.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

Comments

Jacopo Mondi March 4, 2024, 9:30 a.m. UTC | #1
Hi Laurent

On Fri, Mar 01, 2024 at 11:32:18PM +0200, Laurent Pinchart wrote:
> Usage of internal pads creates a route internal to the subdev, and the
> V4L2 camera sensor API requires such routes to be reported to userspace.
> Create the route in the .init_cfg() operation.

It's now "init_state()"

>
> Internal routing support requires stream support, so set the
> V4L2_SUBDEV_FL_HAS_STREAMS flag and switch formats and selection
> rectangles access from pads to streams. As the route is immutable,
> there's no need to implement the .set_routing() operation, and we can
> hardcode the sink and source stream IDs to 0.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/i2c/imx219.c | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
> index 817bf192e4d9..52afb821f667 100644
> --- a/drivers/media/i2c/imx219.c
> +++ b/drivers/media/i2c/imx219.c
> @@ -381,7 +381,10 @@ static int imx219_set_ctrl(struct v4l2_ctrl *ctrl)
>  	int ret = 0;
>
>  	state = v4l2_subdev_get_locked_active_state(&imx219->sd);
> -	format = v4l2_subdev_state_get_format(state, IMX219_PAD_SOURCE);
> +
> +	format = v4l2_subdev_state_get_opposite_stream_format(state,
> +							      IMX219_PAD_IMAGE,
> +							      0);

Is this change necessary ?

Apart from this
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

>
>  	if (ctrl->id == V4L2_CID_VBLANK) {
>  		int exposure_max, exposure_def;
> @@ -993,15 +996,35 @@ static int imx219_get_selection(struct v4l2_subdev *sd,
>  static int imx219_init_state(struct v4l2_subdev *sd,
>  			     struct v4l2_subdev_state *state)
>  {
> +	struct v4l2_subdev_route routes[1] = {
> +		{
> +			.sink_pad = IMX219_PAD_IMAGE,
> +			.sink_stream = 0,
> +			.source_pad = IMX219_PAD_SOURCE,
> +			.source_stream = 0,
> +			.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> +		},
> +	};
> +	struct v4l2_subdev_krouting routing = {
> +		.len_routes = ARRAY_SIZE(routes),
> +		.num_routes = ARRAY_SIZE(routes),
> +		.routes = routes,
> +	};
>  	struct v4l2_subdev_format fmt = {
>  		.which = V4L2_SUBDEV_FORMAT_TRY,
>  		.pad = IMX219_PAD_SOURCE,
> +		.stream = 0,
>  		.format = {
>  			.code = MEDIA_BUS_FMT_SRGGB10_1X10,
>  			.width = supported_modes[0].width,
>  			.height = supported_modes[0].height,
>  		},
>  	};
> +	int ret;
> +
> +	ret = v4l2_subdev_set_routing(sd, state, &routing);
> +	if (ret)
> +		return ret;
>
>  	imx219_set_pad_format(sd, state, &fmt);
>
> @@ -1260,7 +1283,8 @@ static int imx219_probe(struct i2c_client *client)
>
>  	/* Initialize subdev */
>  	imx219->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> -			    V4L2_SUBDEV_FL_HAS_EVENTS;
> +			    V4L2_SUBDEV_FL_HAS_EVENTS |
> +			    V4L2_SUBDEV_FL_STREAMS;
>  	imx219->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
>
>  	/*
> --
> Regards,
>
> Laurent Pinchart
>
>
Laurent Pinchart March 4, 2024, 12:22 p.m. UTC | #2
Hi Jacopo,

On Mon, Mar 04, 2024 at 10:30:34AM +0100, Jacopo Mondi wrote:
> On Fri, Mar 01, 2024 at 11:32:18PM +0200, Laurent Pinchart wrote:
> > Usage of internal pads creates a route internal to the subdev, and the
> > V4L2 camera sensor API requires such routes to be reported to userspace.
> > Create the route in the .init_cfg() operation.
> 
> It's now "init_state()"

Oops. Will fix.

> > Internal routing support requires stream support, so set the
> > V4L2_SUBDEV_FL_HAS_STREAMS flag and switch formats and selection
> > rectangles access from pads to streams. As the route is immutable,
> > there's no need to implement the .set_routing() operation, and we can
> > hardcode the sink and source stream IDs to 0.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  drivers/media/i2c/imx219.c | 28 ++++++++++++++++++++++++++--
> >  1 file changed, 26 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
> > index 817bf192e4d9..52afb821f667 100644
> > --- a/drivers/media/i2c/imx219.c
> > +++ b/drivers/media/i2c/imx219.c
> > @@ -381,7 +381,10 @@ static int imx219_set_ctrl(struct v4l2_ctrl *ctrl)
> >  	int ret = 0;
> >
> >  	state = v4l2_subdev_get_locked_active_state(&imx219->sd);
> > -	format = v4l2_subdev_state_get_format(state, IMX219_PAD_SOURCE);
> > +
> > +	format = v4l2_subdev_state_get_opposite_stream_format(state,
> > +							      IMX219_PAD_IMAGE,
> > +							      0);
> 
> Is this change necessary ?

Now that we have routes, we need to get the format on the right stream.
There's only one at the moment, so there's no need to change it here.
I'll drop this change and update patch 05/15 accordingly.

> Apart from this
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> 
> >
> >  	if (ctrl->id == V4L2_CID_VBLANK) {
> >  		int exposure_max, exposure_def;
> > @@ -993,15 +996,35 @@ static int imx219_get_selection(struct v4l2_subdev *sd,
> >  static int imx219_init_state(struct v4l2_subdev *sd,
> >  			     struct v4l2_subdev_state *state)
> >  {
> > +	struct v4l2_subdev_route routes[1] = {
> > +		{
> > +			.sink_pad = IMX219_PAD_IMAGE,
> > +			.sink_stream = 0,
> > +			.source_pad = IMX219_PAD_SOURCE,
> > +			.source_stream = 0,
> > +			.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> > +		},
> > +	};
> > +	struct v4l2_subdev_krouting routing = {
> > +		.len_routes = ARRAY_SIZE(routes),
> > +		.num_routes = ARRAY_SIZE(routes),
> > +		.routes = routes,
> > +	};
> >  	struct v4l2_subdev_format fmt = {
> >  		.which = V4L2_SUBDEV_FORMAT_TRY,
> >  		.pad = IMX219_PAD_SOURCE,
> > +		.stream = 0,
> >  		.format = {
> >  			.code = MEDIA_BUS_FMT_SRGGB10_1X10,
> >  			.width = supported_modes[0].width,
> >  			.height = supported_modes[0].height,
> >  		},
> >  	};
> > +	int ret;
> > +
> > +	ret = v4l2_subdev_set_routing(sd, state, &routing);
> > +	if (ret)
> > +		return ret;
> >
> >  	imx219_set_pad_format(sd, state, &fmt);
> >
> > @@ -1260,7 +1283,8 @@ static int imx219_probe(struct i2c_client *client)
> >
> >  	/* Initialize subdev */
> >  	imx219->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> > -			    V4L2_SUBDEV_FL_HAS_EVENTS;
> > +			    V4L2_SUBDEV_FL_HAS_EVENTS |
> > +			    V4L2_SUBDEV_FL_STREAMS;
> >  	imx219->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> >
> >  	/*
diff mbox series

Patch

diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index 817bf192e4d9..52afb821f667 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -381,7 +381,10 @@  static int imx219_set_ctrl(struct v4l2_ctrl *ctrl)
 	int ret = 0;
 
 	state = v4l2_subdev_get_locked_active_state(&imx219->sd);
-	format = v4l2_subdev_state_get_format(state, IMX219_PAD_SOURCE);
+
+	format = v4l2_subdev_state_get_opposite_stream_format(state,
+							      IMX219_PAD_IMAGE,
+							      0);
 
 	if (ctrl->id == V4L2_CID_VBLANK) {
 		int exposure_max, exposure_def;
@@ -993,15 +996,35 @@  static int imx219_get_selection(struct v4l2_subdev *sd,
 static int imx219_init_state(struct v4l2_subdev *sd,
 			     struct v4l2_subdev_state *state)
 {
+	struct v4l2_subdev_route routes[1] = {
+		{
+			.sink_pad = IMX219_PAD_IMAGE,
+			.sink_stream = 0,
+			.source_pad = IMX219_PAD_SOURCE,
+			.source_stream = 0,
+			.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
+		},
+	};
+	struct v4l2_subdev_krouting routing = {
+		.len_routes = ARRAY_SIZE(routes),
+		.num_routes = ARRAY_SIZE(routes),
+		.routes = routes,
+	};
 	struct v4l2_subdev_format fmt = {
 		.which = V4L2_SUBDEV_FORMAT_TRY,
 		.pad = IMX219_PAD_SOURCE,
+		.stream = 0,
 		.format = {
 			.code = MEDIA_BUS_FMT_SRGGB10_1X10,
 			.width = supported_modes[0].width,
 			.height = supported_modes[0].height,
 		},
 	};
+	int ret;
+
+	ret = v4l2_subdev_set_routing(sd, state, &routing);
+	if (ret)
+		return ret;
 
 	imx219_set_pad_format(sd, state, &fmt);
 
@@ -1260,7 +1283,8 @@  static int imx219_probe(struct i2c_client *client)
 
 	/* Initialize subdev */
 	imx219->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
-			    V4L2_SUBDEV_FL_HAS_EVENTS;
+			    V4L2_SUBDEV_FL_HAS_EVENTS |
+			    V4L2_SUBDEV_FL_STREAMS;
 	imx219->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
 
 	/*