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

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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;
> >
> >  	/*

-- 
Regards,

Laurent Pinchart