On Thu, May 02, 2024 at 08:52:34PM GMT, Laurent Pinchart wrote: > On Thu, May 02, 2024 at 08:51:16PM +0300, Laurent Pinchart wrote: > > Hi Jacopo, > > > > Thank you for the patch. > > > > On Tue, Apr 30, 2024 at 12:39:42PM +0200, Jacopo Mondi wrote: > > > Use the newly introduced routing table to configure on which MIPI > > > CSI-2 Virtual Channel to send the image stream on. > > > > The stream ID in the routing API isn't meant to be mapped directly to a > > virtual channel number. > Isn't that up to the device to decide what a stream number represent ? > Additionally, why do you need to make the virtual channel configurable, > instead of allocating them dynamically ? What do you mean with "allocating them dinamically" ? Anyway, I guess the main question is: - How to control VC selection if the stream number is not the right way to do that - Has VC selection any value at all > > > Sakari, your opinion would be appreciated. > > > > > Perform Virtual Channel selection at s_stream() time instead of > > > forcing it to 0 during the chip reset. > > > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx> > > > --- > > > drivers/media/i2c/adv748x/adv748x-core.c | 8 ++------ > > > drivers/media/i2c/adv748x/adv748x-csi2.c | 22 ++++++++++++++++++++-- > > > drivers/media/i2c/adv748x/adv748x.h | 1 - > > > 3 files changed, 22 insertions(+), 9 deletions(-) > > > > > > diff --git a/drivers/media/i2c/adv748x/adv748x-core.c b/drivers/media/i2c/adv748x/adv748x-core.c > > > index 3abc73ea8ccb..c9d917135709 100644 > > > --- a/drivers/media/i2c/adv748x/adv748x-core.c > > > +++ b/drivers/media/i2c/adv748x/adv748x-core.c > > > @@ -530,14 +530,10 @@ static int adv748x_reset(struct adv748x_state *state) > > > io_write(state, ADV748X_IO_PD, ADV748X_IO_PD_RX_EN); > > > > > > /* Conditionally enable TXa and TXb. */ > > > - if (is_tx_enabled(&state->txa)) { > > > + if (is_tx_enabled(&state->txa)) > > > regval |= ADV748X_IO_10_CSI4_EN; > > > - adv748x_csi2_set_virtual_channel(&state->txa, 0); > > > - } > > > - if (is_tx_enabled(&state->txb)) { > > > + if (is_tx_enabled(&state->txb)) > > > regval |= ADV748X_IO_10_CSI1_EN; > > > - adv748x_csi2_set_virtual_channel(&state->txb, 0); > > > - } > > > io_write(state, ADV748X_IO_10, regval); > > > > > > /* Use vid_std and v_freq as freerun resolution for CP */ > > > diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c > > > index 7fa72340e66e..a7bfed393ff0 100644 > > > --- a/drivers/media/i2c/adv748x/adv748x-csi2.c > > > +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c > > > @@ -14,7 +14,8 @@ > > > > > > #include "adv748x.h" > > > > > > -int adv748x_csi2_set_virtual_channel(struct adv748x_csi2 *tx, unsigned int vc) > > > +static int adv748x_csi2_set_virtual_channel(struct adv748x_csi2 *tx, > > > + unsigned int vc) > > > { > > > return tx_write(tx, ADV748X_CSI_VC_REF, vc << ADV748X_CSI_VC_REF_SHIFT); > > > } > > > @@ -175,13 +176,30 @@ static const struct v4l2_subdev_internal_ops adv748x_csi2_internal_ops = { > > > static int adv748x_csi2_s_stream(struct v4l2_subdev *sd, int enable) > > > { > > > struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd); > > > + struct v4l2_subdev_state *state; > > > struct v4l2_subdev *src; > > > + int ret; > > > > > > src = adv748x_get_remote_sd(&tx->pads[ADV748X_CSI2_SINK]); > > > if (!src) > > > return -EPIPE; > > > > > > - return v4l2_subdev_call(src, video, s_stream, enable); > > > + state = v4l2_subdev_lock_and_get_active_state(sd); > > > + > > > + if (enable) { > > > + /* A single route is available. */ > > > + struct v4l2_subdev_route *route = &state->routing.routes[0]; > > > + > > > + ret = adv748x_csi2_set_virtual_channel(tx, route->source_stream); > > > + if (ret) > > > + goto unlock; > > > + } > > > + > > > + ret = v4l2_subdev_call(src, video, s_stream, enable); > > > +unlock: > > > + v4l2_subdev_unlock_state(state); > > > + > > > + return ret; > > > } > > > > > > static const struct v4l2_subdev_video_ops adv748x_csi2_video_ops = { > > > diff --git a/drivers/media/i2c/adv748x/adv748x.h b/drivers/media/i2c/adv748x/adv748x.h > > > index be24bc57767c..95d04468af9d 100644 > > > --- a/drivers/media/i2c/adv748x/adv748x.h > > > +++ b/drivers/media/i2c/adv748x/adv748x.h > > > @@ -434,7 +434,6 @@ int adv748x_afe_s_input(struct adv748x_afe *afe, unsigned int input); > > > > > > int adv748x_csi2_init(struct adv748x_state *state, struct adv748x_csi2 *tx); > > > void adv748x_csi2_cleanup(struct adv748x_csi2 *tx); > > > -int adv748x_csi2_set_virtual_channel(struct adv748x_csi2 *tx, unsigned int vc); > > > int adv748x_csi2_set_pixelrate(struct v4l2_subdev *sd, s64 rate); > > > > > > int adv748x_hdmi_init(struct adv748x_hdmi *hdmi); > > > > -- > > Regards, > > > > Laurent Pinchart > > -- > Regards, > > Laurent Pinchart