[v3,26/31] adv748x: csi2: add internal routing configuration

Commit Message

Jacopo Mondi March 5, 2019, 6:51 p.m. UTC

From: Niklas Söderlund <[e-mail]>

Add support to get and set the internal routing between the adv748x
CSI-2 transmitters sink pad and its multiplexed source pad. This routing
includes which stream of the multiplexed pad to use, allowing the user
to select which CSI-2 virtual channel to use when transmitting the
stream.

Signed-off-by: Niklas Söderlund <[e-mail]>
Reviewed-by: Jacopo Mondi <[e-mail]>
---
 drivers/media/i2c/adv748x/adv748x-csi2.c | 65 ++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

Comments

Luca Ceresoli March 14, 2019, 2:45 p.m. UTC | #1

Hi,

begging your pardon for the noob question below...

On 05/03/19 19:51, Jacopo Mondi wrote:
> From: Niklas Söderlund <[e-mail]>
> 
> Add support to get and set the internal routing between the adv748x
> CSI-2 transmitters sink pad and its multiplexed source pad. This routing
> includes which stream of the multiplexed pad to use, allowing the user
> to select which CSI-2 virtual channel to use when transmitting the
> stream.
> 
> Signed-off-by: Niklas Söderlund <[e-mail]>
> Reviewed-by: Jacopo Mondi <[e-mail]>
> ---
>  drivers/media/i2c/adv748x/adv748x-csi2.c | 65 ++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c
> index d8f7cbee86e7..13454af72c6e 100644
> --- a/drivers/media/i2c/adv748x/adv748x-csi2.c
> +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
> @@ -14,6 +14,8 @@
>  
>  #include "adv748x.h"
>  
> +#define ADV748X_CSI2_ROUTES_MAX 4
> +
>  struct adv748x_csi2_format {
>  	unsigned int code;
>  	unsigned int datatype;
> @@ -253,10 +255,73 @@ static int adv748x_csi2_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
>  	return 0;
>  }
>  
> +static int adv748x_csi2_get_routing(struct v4l2_subdev *sd,
> +				    struct v4l2_subdev_krouting *routing)
> +{
> +	struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
> +	struct v4l2_subdev_route *r = routing->routes;
> +	unsigned int vc;
> +
> +	if (routing->num_routes < ADV748X_CSI2_ROUTES_MAX) {
> +		routing->num_routes = ADV748X_CSI2_ROUTES_MAX;
> +		return -ENOSPC;
> +	}
> +
> +	routing->num_routes = ADV748X_CSI2_ROUTES_MAX;
> +
> +	for (vc = 0; vc < ADV748X_CSI2_ROUTES_MAX; vc++) {
> +		r->sink_pad = ADV748X_CSI2_SINK;
> +		r->sink_stream = 0;
> +		r->source_pad = ADV748X_CSI2_SOURCE;
> +		r->source_stream = vc;
> +		r->flags = vc == tx->vc ? V4L2_SUBDEV_ROUTE_FL_ACTIVE : 0;
> +		r++;Begging your pardon for the noob question...

> +	}
> +
> +	return 0;
> +}
> +
> +static int adv748x_csi2_set_routing(struct v4l2_subdev *sd,
> +				    struct v4l2_subdev_krouting *routing)
> +{
> +	struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
> +	struct v4l2_subdev_route *r = routing->routes;
> +	unsigned int i;
> +	int vc = -1;
> +
> +	if (routing->num_routes > ADV748X_CSI2_ROUTES_MAX)
> +		return -ENOSPC;
> +
> +	for (i = 0; i < routing->num_routes; i++) {
> +		if (r->sink_pad != ADV748X_CSI2_SINK ||
> +		    r->sink_stream != 0 ||
> +		    r->source_pad != ADV748X_CSI2_SOURCE ||
> +		    r->source_stream >= ADV748X_CSI2_ROUTES_MAX)
> +			return -EINVAL;
> +
> +		if (r->flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE) {
> +			if (vc != -1)
> +				return -EMLINK;
> +
> +			vc = r->source_stream;
> +		}
> +		r++;
> +	}
> +
> +	if (vc != -1)
> +		tx->vc = vc;
> +
> +	adv748x_csi2_set_virtual_channel(tx, tx->vc);
> +
> +	return 0;
> +}

Not specific to this patch but rather to the set_routing idea as a
whole: can the set_routing ioctl be called while the stream is running?

If it cannot, I find it a limiting factor for nowadays use cases. I also
didn't find where the ioctl is rejected.

If it can, then shouldn't this function call s_stream(stop) through the
sink pad whose route becomes disabled, and a s_stream(start) through the
one that gets enabled?

Thanks,

Jacopo Mondi March 15, 2019, 9:45 a.m. UTC | #2

Hi Luca,

On Thu, Mar 14, 2019 at 03:45:27PM +0100, Luca Ceresoli wrote:
> Hi,
>
> begging your pardon for the noob question below...
>

Let a noob help another noob then

> On 05/03/19 19:51, Jacopo Mondi wrote:
> > From: Niklas Söderlund <[e-mail]>
> >
> > Add support to get and set the internal routing between the adv748x
> > CSI-2 transmitters sink pad and its multiplexed source pad. This routing
> > includes which stream of the multiplexed pad to use, allowing the user
> > to select which CSI-2 virtual channel to use when transmitting the
> > stream.
> >
> > Signed-off-by: Niklas Söderlund <[e-mail]>
> > Reviewed-by: Jacopo Mondi <[e-mail]>
> > ---
> >  drivers/media/i2c/adv748x/adv748x-csi2.c | 65 ++++++++++++++++++++++++
> >  1 file changed, 65 insertions(+)
> >
> > diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c
> > index d8f7cbee86e7..13454af72c6e 100644
> > --- a/drivers/media/i2c/adv748x/adv748x-csi2.c
> > +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
> > @@ -14,6 +14,8 @@
> >
> >  #include "adv748x.h"
> >
> > +#define ADV748X_CSI2_ROUTES_MAX 4
> > +
> >  struct adv748x_csi2_format {
> >  	unsigned int code;
> >  	unsigned int datatype;
> > @@ -253,10 +255,73 @@ static int adv748x_csi2_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> >  	return 0;
> >  }
> >
> > +static int adv748x_csi2_get_routing(struct v4l2_subdev *sd,
> > +				    struct v4l2_subdev_krouting *routing)
> > +{
> > +	struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
> > +	struct v4l2_subdev_route *r = routing->routes;
> > +	unsigned int vc;
> > +
> > +	if (routing->num_routes < ADV748X_CSI2_ROUTES_MAX) {
> > +		routing->num_routes = ADV748X_CSI2_ROUTES_MAX;
> > +		return -ENOSPC;
> > +	}
> > +
> > +	routing->num_routes = ADV748X_CSI2_ROUTES_MAX;
> > +
> > +	for (vc = 0; vc < ADV748X_CSI2_ROUTES_MAX; vc++) {
> > +		r->sink_pad = ADV748X_CSI2_SINK;
> > +		r->sink_stream = 0;
> > +		r->source_pad = ADV748X_CSI2_SOURCE;
> > +		r->source_stream = vc;
> > +		r->flags = vc == tx->vc ? V4L2_SUBDEV_ROUTE_FL_ACTIVE : 0;
> > +		r++;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int adv748x_csi2_set_routing(struct v4l2_subdev *sd,
> > +				    struct v4l2_subdev_krouting *routing)
> > +{
> > +	struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
> > +	struct v4l2_subdev_route *r = routing->routes;
> > +	unsigned int i;
> > +	int vc = -1;
> > +
> > +	if (routing->num_routes > ADV748X_CSI2_ROUTES_MAX)
> > +		return -ENOSPC;
> > +
> > +	for (i = 0; i < routing->num_routes; i++) {
> > +		if (r->sink_pad != ADV748X_CSI2_SINK ||
> > +		    r->sink_stream != 0 ||
> > +		    r->source_pad != ADV748X_CSI2_SOURCE ||
> > +		    r->source_stream >= ADV748X_CSI2_ROUTES_MAX)
> > +			return -EINVAL;
> > +
> > +		if (r->flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE) {
> > +			if (vc != -1)
> > +				return -EMLINK;
> > +
> > +			vc = r->source_stream;
> > +		}
> > +		r++;
> > +	}
> > +
> > +	if (vc != -1)
> > +		tx->vc = vc;
> > +
> > +	adv748x_csi2_set_virtual_channel(tx, tx->vc);
> > +
> > +	return 0;
> > +}
>
> Not specific to this patch but rather to the set_routing idea as a
> whole: can the set_routing ioctl be called while the stream is running?
>
> If it cannot, I find it a limiting factor for nowadays use cases. I also
> didn't find where the ioctl is rejected.
>

The framework does not make assumptions about that at the moment.

> If it can, then shouldn't this function call s_stream(stop) through the
> sink pad whose route becomes disabled, and a s_stream(start) through the
> one that gets enabled?
>

If I got this right, you're here rightfully pointing out that changing
the routing between pads in an entity migh impact the pipeline as a
whole, and this would require, to activate/deactivate devices that
where not part of the pipeline.

This is probably the wrong patch to use an example, as this one is for
a multiplexed interface, where there is no need to go through an
s_stream() for the two CSI-2 endpoints, but as you pointed out in our
brief offline chat, the AFE->TX routing example for this very device
is a good one: if we change the analogue source that is internally
routed to the CSI-2 output of the adv748x, do we need to s_stream(1)
the now routed entity and s_stream(0) on the not not-anymore-routed
one?

My gut feeling is that this is up to userspace, as it should know
what are the requirements of the devices in the system, but this mean
going through an s_stream(0)/s_stream(1) sequence on the video device,
and that would interrupt the streaming for sure.

At the same time, I don't feel too much at ease with the idea of
s_routing calling s_stream on the entity' remote subdevices, as this
would skip the link format validation that media_pipeline_start()
performs.

So yeah, I understand your point, but I don't have a real answer,
maybe someone else does and want to share his mind?

Thanks
   j

> Thanks,
> --
> Luca

Sakari Ailus March 15, 2019, 10:06 a.m. UTC | #3

Hi Luca, Jacopo,

On Fri, Mar 15, 2019 at 10:45:38AM +0100, Jacopo Mondi wrote:
> Hi Luca,
> 
> On Thu, Mar 14, 2019 at 03:45:27PM +0100, Luca Ceresoli wrote:
> > Hi,
> >
> > begging your pardon for the noob question below...
> >
> 
> Let a noob help another noob then
> 
> > On 05/03/19 19:51, Jacopo Mondi wrote:
> > > From: Niklas Söderlund <[e-mail]>
> > >
> > > Add support to get and set the internal routing between the adv748x
> > > CSI-2 transmitters sink pad and its multiplexed source pad. This routing
> > > includes which stream of the multiplexed pad to use, allowing the user
> > > to select which CSI-2 virtual channel to use when transmitting the
> > > stream.
> > >
> > > Signed-off-by: Niklas Söderlund <[e-mail]>
> > > Reviewed-by: Jacopo Mondi <[e-mail]>
> > > ---
> > >  drivers/media/i2c/adv748x/adv748x-csi2.c | 65 ++++++++++++++++++++++++
> > >  1 file changed, 65 insertions(+)
> > >
> > > diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c
> > > index d8f7cbee86e7..13454af72c6e 100644
> > > --- a/drivers/media/i2c/adv748x/adv748x-csi2.c
> > > +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
> > > @@ -14,6 +14,8 @@
> > >
> > >  #include "adv748x.h"
> > >
> > > +#define ADV748X_CSI2_ROUTES_MAX 4
> > > +
> > >  struct adv748x_csi2_format {
> > >  	unsigned int code;
> > >  	unsigned int datatype;
> > > @@ -253,10 +255,73 @@ static int adv748x_csi2_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> > >  	return 0;
> > >  }
> > >
> > > +static int adv748x_csi2_get_routing(struct v4l2_subdev *sd,
> > > +				    struct v4l2_subdev_krouting *routing)
> > > +{
> > > +	struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
> > > +	struct v4l2_subdev_route *r = routing->routes;
> > > +	unsigned int vc;
> > > +
> > > +	if (routing->num_routes < ADV748X_CSI2_ROUTES_MAX) {
> > > +		routing->num_routes = ADV748X_CSI2_ROUTES_MAX;
> > > +		return -ENOSPC;
> > > +	}
> > > +
> > > +	routing->num_routes = ADV748X_CSI2_ROUTES_MAX;
> > > +
> > > +	for (vc = 0; vc < ADV748X_CSI2_ROUTES_MAX; vc++) {
> > > +		r->sink_pad = ADV748X_CSI2_SINK;
> > > +		r->sink_stream = 0;
> > > +		r->source_pad = ADV748X_CSI2_SOURCE;
> > > +		r->source_stream = vc;
> > > +		r->flags = vc == tx->vc ? V4L2_SUBDEV_ROUTE_FL_ACTIVE : 0;
> > > +		r++;
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int adv748x_csi2_set_routing(struct v4l2_subdev *sd,
> > > +				    struct v4l2_subdev_krouting *routing)
> > > +{
> > > +	struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
> > > +	struct v4l2_subdev_route *r = routing->routes;
> > > +	unsigned int i;
> > > +	int vc = -1;
> > > +
> > > +	if (routing->num_routes > ADV748X_CSI2_ROUTES_MAX)
> > > +		return -ENOSPC;
> > > +
> > > +	for (i = 0; i < routing->num_routes; i++) {
> > > +		if (r->sink_pad != ADV748X_CSI2_SINK ||
> > > +		    r->sink_stream != 0 ||
> > > +		    r->source_pad != ADV748X_CSI2_SOURCE ||
> > > +		    r->source_stream >= ADV748X_CSI2_ROUTES_MAX)
> > > +			return -EINVAL;
> > > +
> > > +		if (r->flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE) {
> > > +			if (vc != -1)
> > > +				return -EMLINK;
> > > +
> > > +			vc = r->source_stream;
> > > +		}
> > > +		r++;
> > > +	}
> > > +
> > > +	if (vc != -1)
> > > +		tx->vc = vc;
> > > +
> > > +	adv748x_csi2_set_virtual_channel(tx, tx->vc);
> > > +
> > > +	return 0;
> > > +}
> >
> > Not specific to this patch but rather to the set_routing idea as a
> > whole: can the set_routing ioctl be called while the stream is running?
> >
> > If it cannot, I find it a limiting factor for nowadays use cases. I also
> > didn't find where the ioctl is rejected.
> >
> 
> The framework does not make assumptions about that at the moment.
> 
> > If it can, then shouldn't this function call s_stream(stop) through the
> > sink pad whose route becomes disabled, and a s_stream(start) through the
> > one that gets enabled?
> >
> 
> If I got this right, you're here rightfully pointing out that changing
> the routing between pads in an entity migh impact the pipeline as a
> whole, and this would require, to activate/deactivate devices that
> where not part of the pipeline.

I'd say that ultimately this depends on the devices themselves, whether
they support this or not. In practice I don't think we have any such cases
at the moment, but it's possible in principle. Changes on the framework may
well be needed but likely the biggest complications will still be in
drivers supporting that.

The media links have a dynamic flag for this purpose but I don't think it's
ever been used.

> 
> This is probably the wrong patch to use an example, as this one is for
> a multiplexed interface, where there is no need to go through an
> s_stream() for the two CSI-2 endpoints, but as you pointed out in our
> brief offline chat, the AFE->TX routing example for this very device
> is a good one: if we change the analogue source that is internally
> routed to the CSI-2 output of the adv748x, do we need to s_stream(1)
> the now routed entity and s_stream(0) on the not not-anymore-routed
> one?
> 
> My gut feeling is that this is up to userspace, as it should know
> what are the requirements of the devices in the system, but this mean
> going through an s_stream(0)/s_stream(1) sequence on the video device,
> and that would interrupt the streaming for sure.
> 
> At the same time, I don't feel too much at ease with the idea of
> s_routing calling s_stream on the entity' remote subdevices, as this
> would skip the link format validation that media_pipeline_start()
> performs.

The link validation must be done in this case as well, it may not be
simply skipped.

> 
> So yeah, I understand your point, but I don't have a real answer,
> maybe someone else does and want to share his mind?

Luca Ceresoli March 16, 2019, 10:23 a.m. UTC | #4

Hi Jacopo, Sakari,

On 15/03/19 11:06, Sakari Ailus wrote:
> Hi Luca, Jacopo,
> 
> On Fri, Mar 15, 2019 at 10:45:38AM +0100, Jacopo Mondi wrote:
>> Hi Luca,
>>
>> On Thu, Mar 14, 2019 at 03:45:27PM +0100, Luca Ceresoli wrote:
>>> Hi,
>>>
>>> begging your pardon for the noob question below...
>>>
>>
>> Let a noob help another noob then
>>
>>> On 05/03/19 19:51, Jacopo Mondi wrote:
>>>> From: Niklas Söderlund <[e-mail]>
>>>>
>>>> Add support to get and set the internal routing between the adv748x
>>>> CSI-2 transmitters sink pad and its multiplexed source pad. This routing
>>>> includes which stream of the multiplexed pad to use, allowing the user
>>>> to select which CSI-2 virtual channel to use when transmitting the
>>>> stream.
>>>>
>>>> Signed-off-by: Niklas Söderlund <[e-mail]>
>>>> Reviewed-by: Jacopo Mondi <[e-mail]>
>>>> ---
>>>>  drivers/media/i2c/adv748x/adv748x-csi2.c | 65 ++++++++++++++++++++++++
>>>>  1 file changed, 65 insertions(+)
>>>>
>>>> diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c
>>>> index d8f7cbee86e7..13454af72c6e 100644
>>>> --- a/drivers/media/i2c/adv748x/adv748x-csi2.c
>>>> +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
>>>> @@ -14,6 +14,8 @@
>>>>
>>>>  #include "adv748x.h"
>>>>
>>>> +#define ADV748X_CSI2_ROUTES_MAX 4
>>>> +
>>>>  struct adv748x_csi2_format {
>>>>  	unsigned int code;
>>>>  	unsigned int datatype;
>>>> @@ -253,10 +255,73 @@ static int adv748x_csi2_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
>>>>  	return 0;
>>>>  }
>>>>
>>>> +static int adv748x_csi2_get_routing(struct v4l2_subdev *sd,
>>>> +				    struct v4l2_subdev_krouting *routing)
>>>> +{
>>>> +	struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
>>>> +	struct v4l2_subdev_route *r = routing->routes;
>>>> +	unsigned int vc;
>>>> +
>>>> +	if (routing->num_routes < ADV748X_CSI2_ROUTES_MAX) {
>>>> +		routing->num_routes = ADV748X_CSI2_ROUTES_MAX;
>>>> +		return -ENOSPC;
>>>> +	}
>>>> +
>>>> +	routing->num_routes = ADV748X_CSI2_ROUTES_MAX;
>>>> +
>>>> +	for (vc = 0; vc < ADV748X_CSI2_ROUTES_MAX; vc++) {
>>>> +		r->sink_pad = ADV748X_CSI2_SINK;
>>>> +		r->sink_stream = 0;
>>>> +		r->source_pad = ADV748X_CSI2_SOURCE;
>>>> +		r->source_stream = vc;
>>>> +		r->flags = vc == tx->vc ? V4L2_SUBDEV_ROUTE_FL_ACTIVE : 0;
>>>> +		r++;
>>>> +	}
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int adv748x_csi2_set_routing(struct v4l2_subdev *sd,
>>>> +				    struct v4l2_subdev_krouting *routing)
>>>> +{
>>>> +	struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
>>>> +	struct v4l2_subdev_route *r = routing->routes;
>>>> +	unsigned int i;
>>>> +	int vc = -1;
>>>> +
>>>> +	if (routing->num_routes > ADV748X_CSI2_ROUTES_MAX)
>>>> +		return -ENOSPC;
>>>> +
>>>> +	for (i = 0; i < routing->num_routes; i++) {
>>>> +		if (r->sink_pad != ADV748X_CSI2_SINK ||
>>>> +		    r->sink_stream != 0 ||
>>>> +		    r->source_pad != ADV748X_CSI2_SOURCE ||
>>>> +		    r->source_stream >= ADV748X_CSI2_ROUTES_MAX)
>>>> +			return -EINVAL;
>>>> +
>>>> +		if (r->flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE) {
>>>> +			if (vc != -1)
>>>> +				return -EMLINK;
>>>> +
>>>> +			vc = r->source_stream;
>>>> +		}
>>>> +		r++;
>>>> +	}
>>>> +
>>>> +	if (vc != -1)
>>>> +		tx->vc = vc;
>>>> +
>>>> +	adv748x_csi2_set_virtual_channel(tx, tx->vc);
>>>> +
>>>> +	return 0;
>>>> +}
>>>
>>> Not specific to this patch but rather to the set_routing idea as a
>>> whole: can the set_routing ioctl be called while the stream is running?
>>>
>>> If it cannot, I find it a limiting factor for nowadays use cases. I also
>>> didn't find where the ioctl is rejected.
>>>
>>
>> The framework does not make assumptions about that at the moment.
>>
>>> If it can, then shouldn't this function call s_stream(stop) through the
>>> sink pad whose route becomes disabled, and a s_stream(start) through the
>>> one that gets enabled?
>>>
>>
>> If I got this right, you're here rightfully pointing out that changing
>> the routing between pads in an entity migh impact the pipeline as a
>> whole, and this would require, to activate/deactivate devices that
>> where not part of the pipeline.
> 
> I'd say that ultimately this depends on the devices themselves, whether
> they support this or not. In practice I don't think we have any such cases
> at the moment, but it's possible in principle. Changes on the framework may
> well be needed but likely the biggest complications will still be in
> drivers supporting that.

I understand V4L2 currently does not support changing a pipeline that is
running. However there are many use cases that would require it.

Most of the use cases that come to my mind involve a multiplexer with
multiple inputs, one of which can be selected to be forwarded. In those
cases s_routing deselects an input and selects another one. How the can
we handle such cases without sending a s_stream on the two upstreams?
Having all possible inputs always running is not a real solution.

> The media links have a dynamic flag for this purpose but I don't think it's
> ever been used.
> 
>>
>> This is probably the wrong patch to use an example, as this one is for
>> a multiplexed interface, where there is no need to go through an
>> s_stream() for the two CSI-2 endpoints, but as you pointed out in our
>> brief offline chat, the AFE->TX routing example for this very device
>> is a good one: if we change the analogue source that is internally
>> routed to the CSI-2 output of the adv748x, do we need to s_stream(1)
>> the now routed entity and s_stream(0) on the not not-anymore-routed
>> one?
>>
>> My gut feeling is that this is up to userspace, as it should know
>> what are the requirements of the devices in the system, but this mean
>> going through an s_stream(0)/s_stream(1) sequence on the video device,
>> and that would interrupt the streaming for sure.
>>
>> At the same time, I don't feel too much at ease with the idea of
>> s_routing calling s_stream on the entity' remote subdevices, as this
>> would skip the link format validation that media_pipeline_start()
>> performs.
> 
> The link validation must be done in this case as well, it may not be
> simply skipped.

Agreed.

The routing VS pipeline validation point is a very important one. The
current proposed workflow is:

 1. the pipeline is validated as a whole, having knowledge of all the
    entities
 2. streaming is started
 3. s_routing is called on an entity (not on the pipeline!)

Now the s_routing function in the entity driver is not in a good
position to validate the candidate future pipeline as a whole.

Naively I'd say there are two possible solutions:

 1. the s_routing reaches the pipeline first, then the new pipeline is
    computed and verified, and if verification succeeds it is applied
 2. a partial pipeline verification mechanism is added, so the entity
    receiving a s_routing request to e.g. change the sink pad can invoke
    a verification on the part of pipeline that is about to be
    activated, and if verification succeeds it is applied

Somehow I suspect neither is trivial...

Jacopo Mondi March 20, 2019, 5:14 p.m. UTC | #5

Hi Luca,
   thanks for the input

On Sat, Mar 16, 2019 at 11:23:42AM +0100, Luca Ceresoli wrote:
> Hi Jacopo, Sakari,
>
> On 15/03/19 11:06, Sakari Ailus wrote:
> > Hi Luca, Jacopo,
> >
> > On Fri, Mar 15, 2019 at 10:45:38AM +0100, Jacopo Mondi wrote:
> >> Hi Luca,
> >>
> >> On Thu, Mar 14, 2019 at 03:45:27PM +0100, Luca Ceresoli wrote:
> >>> Hi,
> >>>
> >>> begging your pardon for the noob question below...
> >>>
> >>
> >> Let a noob help another noob then
> >>
> >>> On 05/03/19 19:51, Jacopo Mondi wrote:
> >>>> From: Niklas Söderlund <[e-mail]>
> >>>>
> >>>> Add support to get and set the internal routing between the adv748x
> >>>> CSI-2 transmitters sink pad and its multiplexed source pad. This routing
> >>>> includes which stream of the multiplexed pad to use, allowing the user
> >>>> to select which CSI-2 virtual channel to use when transmitting the
> >>>> stream.
> >>>>
> >>>> Signed-off-by: Niklas Söderlund <[e-mail]>
> >>>> Reviewed-by: Jacopo Mondi <[e-mail]>
> >>>> ---
> >>>>  drivers/media/i2c/adv748x/adv748x-csi2.c | 65 ++++++++++++++++++++++++
> >>>>  1 file changed, 65 insertions(+)
> >>>>
> >>>> diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c
> >>>> index d8f7cbee86e7..13454af72c6e 100644
> >>>> --- a/drivers/media/i2c/adv748x/adv748x-csi2.c
> >>>> +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
> >>>> @@ -14,6 +14,8 @@
> >>>>
> >>>>  #include "adv748x.h"
> >>>>
> >>>> +#define ADV748X_CSI2_ROUTES_MAX 4
> >>>> +
> >>>>  struct adv748x_csi2_format {
> >>>>  	unsigned int code;
> >>>>  	unsigned int datatype;
> >>>> @@ -253,10 +255,73 @@ static int adv748x_csi2_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> >>>>  	return 0;
> >>>>  }
> >>>>
> >>>> +static int adv748x_csi2_get_routing(struct v4l2_subdev *sd,
> >>>> +				    struct v4l2_subdev_krouting *routing)
> >>>> +{
> >>>> +	struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
> >>>> +	struct v4l2_subdev_route *r = routing->routes;
> >>>> +	unsigned int vc;
> >>>> +
> >>>> +	if (routing->num_routes < ADV748X_CSI2_ROUTES_MAX) {
> >>>> +		routing->num_routes = ADV748X_CSI2_ROUTES_MAX;
> >>>> +		return -ENOSPC;
> >>>> +	}
> >>>> +
> >>>> +	routing->num_routes = ADV748X_CSI2_ROUTES_MAX;
> >>>> +
> >>>> +	for (vc = 0; vc < ADV748X_CSI2_ROUTES_MAX; vc++) {
> >>>> +		r->sink_pad = ADV748X_CSI2_SINK;
> >>>> +		r->sink_stream = 0;
> >>>> +		r->source_pad = ADV748X_CSI2_SOURCE;
> >>>> +		r->source_stream = vc;
> >>>> +		r->flags = vc == tx->vc ? V4L2_SUBDEV_ROUTE_FL_ACTIVE : 0;
> >>>> +		r++;
> >>>> +	}
> >>>> +
> >>>> +	return 0;
> >>>> +}
> >>>> +
> >>>> +static int adv748x_csi2_set_routing(struct v4l2_subdev *sd,
> >>>> +				    struct v4l2_subdev_krouting *routing)
> >>>> +{
> >>>> +	struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
> >>>> +	struct v4l2_subdev_route *r = routing->routes;
> >>>> +	unsigned int i;
> >>>> +	int vc = -1;
> >>>> +
> >>>> +	if (routing->num_routes > ADV748X_CSI2_ROUTES_MAX)
> >>>> +		return -ENOSPC;
> >>>> +
> >>>> +	for (i = 0; i < routing->num_routes; i++) {
> >>>> +		if (r->sink_pad != ADV748X_CSI2_SINK ||
> >>>> +		    r->sink_stream != 0 ||
> >>>> +		    r->source_pad != ADV748X_CSI2_SOURCE ||
> >>>> +		    r->source_stream >= ADV748X_CSI2_ROUTES_MAX)
> >>>> +			return -EINVAL;
> >>>> +
> >>>> +		if (r->flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE) {
> >>>> +			if (vc != -1)
> >>>> +				return -EMLINK;
> >>>> +
> >>>> +			vc = r->source_stream;
> >>>> +		}
> >>>> +		r++;
> >>>> +	}
> >>>> +
> >>>> +	if (vc != -1)
> >>>> +		tx->vc = vc;
> >>>> +
> >>>> +	adv748x_csi2_set_virtual_channel(tx, tx->vc);
> >>>> +
> >>>> +	return 0;
> >>>> +}
> >>>
> >>> Not specific to this patch but rather to the set_routing idea as a
> >>> whole: can the set_routing ioctl be called while the stream is running?
> >>>
> >>> If it cannot, I find it a limiting factor for nowadays use cases. I also
> >>> didn't find where the ioctl is rejected.
> >>>
> >>
> >> The framework does not make assumptions about that at the moment.
> >>
> >>> If it can, then shouldn't this function call s_stream(stop) through the
> >>> sink pad whose route becomes disabled, and a s_stream(start) through the
> >>> one that gets enabled?
> >>>
> >>
> >> If I got this right, you're here rightfully pointing out that changing
> >> the routing between pads in an entity migh impact the pipeline as a
> >> whole, and this would require, to activate/deactivate devices that
> >> where not part of the pipeline.
> >
> > I'd say that ultimately this depends on the devices themselves, whether
> > they support this or not. In practice I don't think we have any such cases
> > at the moment, but it's possible in principle. Changes on the framework may
> > well be needed but likely the biggest complications will still be in
> > drivers supporting that.
>
> I understand V4L2 currently does not support changing a pipeline that is
> running. However there are many use cases that would require it.
>
> Most of the use cases that come to my mind involve a multiplexer with
> multiple inputs, one of which can be selected to be forwarded. In those
> cases s_routing deselects an input and selects another one. How the can
> we handle such cases without sending a s_stream on the two upstreams?
> Having all possible inputs always running is not a real solution.

This seems a very specific use case, I might surely be wrong, but if
such a muxing device allows you to switch from one source to another
without interruption, its driver should also take care of several
other things, such as controlling power and format validations. I
don't think that's something that should be considered at framework
level, but again without a real use case I can't tell for real.

>
> > The media links have a dynamic flag for this purpose but I don't think it's
> > ever been used.
> >
> >>
> >> This is probably the wrong patch to use an example, as this one is for
> >> a multiplexed interface, where there is no need to go through an
> >> s_stream() for the two CSI-2 endpoints, but as you pointed out in our
> >> brief offline chat, the AFE->TX routing example for this very device
> >> is a good one: if we change the analogue source that is internally
> >> routed to the CSI-2 output of the adv748x, do we need to s_stream(1)
> >> the now routed entity and s_stream(0) on the not not-anymore-routed
> >> one?
> >>
> >> My gut feeling is that this is up to userspace, as it should know
> >> what are the requirements of the devices in the system, but this mean
> >> going through an s_stream(0)/s_stream(1) sequence on the video device,
> >> and that would interrupt the streaming for sure.
> >>
> >> At the same time, I don't feel too much at ease with the idea of
> >> s_routing calling s_stream on the entity' remote subdevices, as this
> >> would skip the link format validation that media_pipeline_start()
> >> performs.
> >
> > The link validation must be done in this case as well, it may not be
> > simply skipped.
>
> Agreed.
>
> The routing VS pipeline validation point is a very important one. The
> current proposed workflow is:
>
>  1. the pipeline is validated as a whole, having knowledge of all the
>     entities

let me specify this to avoid confusions:
     "all the entities -with an active route in the pipeline-"

>  2. streaming is started
>  3. s_routing is called on an entity (not on the pipeline!)
>
> Now the s_routing function in the entity driver is not in a good
> position to validate the candidate future pipeline as a whole.
>
> Naively I'd say there are two possible solutions:
>
>  1. the s_routing reaches the pipeline first, then the new pipeline is
>     computed and verified, and if verification succeeds it is applied
>  2. a partial pipeline verification mechanism is added, so the entity
>     receiving a s_routing request to e.g. change the sink pad can invoke
>     a verification on the part of pipeline that is about to be
>     activated, and if verification succeeds it is applied
>
> Somehow I suspect neither is trivial...

I would say it is not, but if you have such a device that does not
require going through a s_stream(0)/s_stream(1) cycle and all the
associated re-negotiation and validations, it seems to me nothing
prevents you from handling this in the driver implementation. Maybe it
won't look that great, but this seems to be quite a custom design that
requires all input sources to be linked to your sink pads, their
format validated all at the same time, power, stream activation and
internal mux configuration controlled by s_routing. Am I wrong or
nothing in this series would prevent your from doing this?

tl;dr: I would not make this something the framework should be
concerned about, as there's nothing preventing you from
implementing support for such a use case. But again, without a real
example we can only guess, and I might be overlooking the issue or
missing some relevant detail for sure.

Thanks
   j

>
> --
> Luca
>
>

Luca Ceresoli March 22, 2019, 4:52 p.m. UTC | #6

Hi,

thanks for the follow-up.

On 20/03/19 18:14, Jacopo Mondi wrote:
>>>> This is probably the wrong patch to use an example, as this one is for
>>>> a multiplexed interface, where there is no need to go through an
>>>> s_stream() for the two CSI-2 endpoints, but as you pointed out in our
>>>> brief offline chat, the AFE->TX routing example for this very device
>>>> is a good one: if we change the analogue source that is internally
>>>> routed to the CSI-2 output of the adv748x, do we need to s_stream(1)
>>>> the now routed entity and s_stream(0) on the not not-anymore-routed
>>>> one?
>>>>
>>>> My gut feeling is that this is up to userspace, as it should know
>>>> what are the requirements of the devices in the system, but this mean
>>>> going through an s_stream(0)/s_stream(1) sequence on the video device,
>>>> and that would interrupt the streaming for sure.
>>>>
>>>> At the same time, I don't feel too much at ease with the idea of
>>>> s_routing calling s_stream on the entity' remote subdevices, as this
>>>> would skip the link format validation that media_pipeline_start()
>>>> performs.
>>>
>>> The link validation must be done in this case as well, it may not be
>>> simply skipped.
>>
>> Agreed.
>>
>> The routing VS pipeline validation point is a very important one. The
>> current proposed workflow is:
>>
>>  1. the pipeline is validated as a whole, having knowledge of all the
>>     entities
> 
> let me specify this to avoid confusions:
>      "all the entities -with an active route in the pipeline-"
> 
>>  2. streaming is started
>>  3. s_routing is called on an entity (not on the pipeline!)
>>
>> Now the s_routing function in the entity driver is not in a good
>> position to validate the candidate future pipeline as a whole.
>>
>> Naively I'd say there are two possible solutions:
>>
>>  1. the s_routing reaches the pipeline first, then the new pipeline is
>>     computed and verified, and if verification succeeds it is applied
>>  2. a partial pipeline verification mechanism is added, so the entity
>>     receiving a s_routing request to e.g. change the sink pad can invoke
>>     a verification on the part of pipeline that is about to be
>>     activated, and if verification succeeds it is applied
>>
>> Somehow I suspect neither is trivial...
> 
> I would say it is not, but if you have such a device that does not
> require going through a s_stream(0)/s_stream(1) cycle and all the
> associated re-negotiation and validations, it seems to me nothing
> prevents you from handling this in the driver implementation. Maybe it
> won't look that great, but this seems to be quite a custom design that
> requires all input sources to be linked to your sink pads, their
> format validated all at the same time, power, stream activation and
> internal mux configuration controlled by s_routing. Am I wrong or
> nothing in this series would prevent your from doing this?

You're right, nothing prevents me from doing a custom hack for my custom
design. It's what I'm doing right now. My concern is whether the
framework will evolve to allow modifying a running pipeline without
custom hacks.

> tl;dr: I would not make this something the framework should be
> concerned about, as there's nothing preventing you from
> implementing support for such a use case. But again, without a real
> example we can only guess, and I might be overlooking the issue or
> missing some relevant detail for sure.

I'm a bit surprised in observing that my use case looks so strange,
perhaps yours is so different that we don't quite understand each other.
So below is an example of what I have in mind. Can you explain your use
case too?


Here's a use case. Consider a product that takes 3 camera inputs,
selects one of them and produces a continuous video stream with the
camera image and an OSD on top of it. The selected camera can be changed
at any time (e.g. upon user selection).

                  OSD FB ---.
                            |
            .--------.      V
Camera 0 -->|        |   .-----.
Camera 1 -->|  MUX   |-->| OSD |--> DMA --> /dev/video0
Camera 2 -->|        |   `-----'
            `--------'

A prerequisite is obviously that each piece of hardware and software
involved is able to cope with a sudden stream change. Perhaps not that
common, but no rocket science.

It looks to me that each of these pieces can be modeled as an entity and
the s_routing API is a perfect fit for the mux block. Am I wrong?

Thanks,

Jacopo Mondi March 28, 2019, 3:08 p.m. UTC | #7

Hi Luca,

On Fri, Mar 22, 2019 at 05:52:15PM +0100, Luca Ceresoli wrote:
> Hi,
>
> thanks for the follow-up.
>
> On 20/03/19 18:14, Jacopo Mondi wrote:
> >>>> This is probably the wrong patch to use an example, as this one is for
> >>>> a multiplexed interface, where there is no need to go through an
> >>>> s_stream() for the two CSI-2 endpoints, but as you pointed out in our
> >>>> brief offline chat, the AFE->TX routing example for this very device
> >>>> is a good one: if we change the analogue source that is internally
> >>>> routed to the CSI-2 output of the adv748x, do we need to s_stream(1)
> >>>> the now routed entity and s_stream(0) on the not not-anymore-routed
> >>>> one?
> >>>>
> >>>> My gut feeling is that this is up to userspace, as it should know
> >>>> what are the requirements of the devices in the system, but this mean
> >>>> going through an s_stream(0)/s_stream(1) sequence on the video device,
> >>>> and that would interrupt the streaming for sure.
> >>>>
> >>>> At the same time, I don't feel too much at ease with the idea of
> >>>> s_routing calling s_stream on the entity' remote subdevices, as this
> >>>> would skip the link format validation that media_pipeline_start()
> >>>> performs.
> >>>
> >>> The link validation must be done in this case as well, it may not be
> >>> simply skipped.
> >>
> >> Agreed.
> >>
> >> The routing VS pipeline validation point is a very important one. The
> >> current proposed workflow is:
> >>
> >>  1. the pipeline is validated as a whole, having knowledge of all the
> >>     entities
> >
> > let me specify this to avoid confusions:
> >      "all the entities -with an active route in the pipeline-"
> >
> >>  2. streaming is started
> >>  3. s_routing is called on an entity (not on the pipeline!)
> >>
> >> Now the s_routing function in the entity driver is not in a good
> >> position to validate the candidate future pipeline as a whole.
> >>
> >> Naively I'd say there are two possible solutions:
> >>
> >>  1. the s_routing reaches the pipeline first, then the new pipeline is
> >>     computed and verified, and if verification succeeds it is applied
> >>  2. a partial pipeline verification mechanism is added, so the entity
> >>     receiving a s_routing request to e.g. change the sink pad can invoke
> >>     a verification on the part of pipeline that is about to be
> >>     activated, and if verification succeeds it is applied
> >>
> >> Somehow I suspect neither is trivial...
> >
> > I would say it is not, but if you have such a device that does not
> > require going through a s_stream(0)/s_stream(1) cycle and all the
> > associated re-negotiation and validations, it seems to me nothing
> > prevents you from handling this in the driver implementation. Maybe it
> > won't look that great, but this seems to be quite a custom design that
> > requires all input sources to be linked to your sink pads, their
> > format validated all at the same time, power, stream activation and
> > internal mux configuration controlled by s_routing. Am I wrong or
> > nothing in this series would prevent your from doing this?
>
> You're right, nothing prevents me from doing a custom hack for my custom
> design. It's what I'm doing right now. My concern is whether the
> framework will evolve to allow modifying a running pipeline without
> custom hacks.
>
> > tl;dr: I would not make this something the framework should be
> > concerned about, as there's nothing preventing you from
> > implementing support for such a use case. But again, without a real
> > example we can only guess, and I might be overlooking the issue or
> > missing some relevant detail for sure.
>
> I'm a bit surprised in observing that my use case looks so strange,
> perhaps yours is so different that we don't quite understand each other.
> So below is an example of what I have in mind. Can you explain your use
> case too?

I'm mostly interested in this series as it allows me to add data lane
negotiation at run time. In my case, there are no stream continuity
constraints, but I get your point here.
>
>
> Here's a use case. Consider a product that takes 3 camera inputs,
> selects one of them and produces a continuous video stream with the
> camera image and an OSD on top of it. The selected camera can be changed
> at any time (e.g. upon user selection).
>
>                   OSD FB ---.
>                             |
>             .--------.      V
> Camera 0 -->|        |   .-----.
> Camera 1 -->|  MUX   |-->| OSD |--> DMA --> /dev/video0
> Camera 2 -->|        |   `-----'
>             `--------'
>
> A prerequisite is obviously that each piece of hardware and software
> involved is able to cope with a sudden stream change. Perhaps not that
> common, but no rocket science.
>
> It looks to me that each of these pieces can be modeled as an entity and
> the s_routing API is a perfect fit for the mux block. Am I wrong?
>

Personally, I would say that your muxer driver, if able to switch
source without requiring a pipeline restart, should handle it
internally. There are specific details that the mux should be able to
handle, and we're still pretty vague on the details. As a start, which
is the bus type your sources connects to the muxer with? Parallel?
CSI-2? How is the video stream multiplexed? with CSI-2 VC? Do you need
to control your source power during inactive period? I'm sure there
are more questions... I agree a partial pipeline restart might be
something to consider, but at the same time I think this is not
something strictly required to get this series merged, isn't it?

Thanks
  j


> Thanks,
> --
> Luca

Luca Ceresoli March 28, 2019, 3:17 p.m. UTC | #8

Hi Jacopo,

On 28/03/19 16:08, Jacopo Mondi wrote:
> Hi Luca,
> 
> On Fri, Mar 22, 2019 at 05:52:15PM +0100, Luca Ceresoli wrote:
>> Hi,
>>
>> thanks for the follow-up.
>>
>> On 20/03/19 18:14, Jacopo Mondi wrote:
>>>>>> This is probably the wrong patch to use an example, as this one is for
>>>>>> a multiplexed interface, where there is no need to go through an
>>>>>> s_stream() for the two CSI-2 endpoints, but as you pointed out in our
>>>>>> brief offline chat, the AFE->TX routing example for this very device
>>>>>> is a good one: if we change the analogue source that is internally
>>>>>> routed to the CSI-2 output of the adv748x, do we need to s_stream(1)
>>>>>> the now routed entity and s_stream(0) on the not not-anymore-routed
>>>>>> one?
>>>>>>
>>>>>> My gut feeling is that this is up to userspace, as it should know
>>>>>> what are the requirements of the devices in the system, but this mean
>>>>>> going through an s_stream(0)/s_stream(1) sequence on the video device,
>>>>>> and that would interrupt the streaming for sure.
>>>>>>
>>>>>> At the same time, I don't feel too much at ease with the idea of
>>>>>> s_routing calling s_stream on the entity' remote subdevices, as this
>>>>>> would skip the link format validation that media_pipeline_start()
>>>>>> performs.
>>>>>
>>>>> The link validation must be done in this case as well, it may not be
>>>>> simply skipped.
>>>>
>>>> Agreed.
>>>>
>>>> The routing VS pipeline validation point is a very important one. The
>>>> current proposed workflow is:
>>>>
>>>>  1. the pipeline is validated as a whole, having knowledge of all the
>>>>     entities
>>>
>>> let me specify this to avoid confusions:
>>>      "all the entities -with an active route in the pipeline-"
>>>
>>>>  2. streaming is started
>>>>  3. s_routing is called on an entity (not on the pipeline!)
>>>>
>>>> Now the s_routing function in the entity driver is not in a good
>>>> position to validate the candidate future pipeline as a whole.
>>>>
>>>> Naively I'd say there are two possible solutions:
>>>>
>>>>  1. the s_routing reaches the pipeline first, then the new pipeline is
>>>>     computed and verified, and if verification succeeds it is applied
>>>>  2. a partial pipeline verification mechanism is added, so the entity
>>>>     receiving a s_routing request to e.g. change the sink pad can invoke
>>>>     a verification on the part of pipeline that is about to be
>>>>     activated, and if verification succeeds it is applied
>>>>
>>>> Somehow I suspect neither is trivial...
>>>
>>> I would say it is not, but if you have such a device that does not
>>> require going through a s_stream(0)/s_stream(1) cycle and all the
>>> associated re-negotiation and validations, it seems to me nothing
>>> prevents you from handling this in the driver implementation. Maybe it
>>> won't look that great, but this seems to be quite a custom design that
>>> requires all input sources to be linked to your sink pads, their
>>> format validated all at the same time, power, stream activation and
>>> internal mux configuration controlled by s_routing. Am I wrong or
>>> nothing in this series would prevent your from doing this?
>>
>> You're right, nothing prevents me from doing a custom hack for my custom
>> design. It's what I'm doing right now. My concern is whether the
>> framework will evolve to allow modifying a running pipeline without
>> custom hacks.
>>
>>> tl;dr: I would not make this something the framework should be
>>> concerned about, as there's nothing preventing you from
>>> implementing support for such a use case. But again, without a real
>>> example we can only guess, and I might be overlooking the issue or
>>> missing some relevant detail for sure.
>>
>> I'm a bit surprised in observing that my use case looks so strange,
>> perhaps yours is so different that we don't quite understand each other.
>> So below is an example of what I have in mind. Can you explain your use
>> case too?
> 
> I'm mostly interested in this series as it allows me to add data lane
> negotiation at run time. In my case, there are no stream continuity
> constraints, but I get your point here.
>>
>>
>> Here's a use case. Consider a product that takes 3 camera inputs,
>> selects one of them and produces a continuous video stream with the
>> camera image and an OSD on top of it. The selected camera can be changed
>> at any time (e.g. upon user selection).
>>
>>                   OSD FB ---.
>>                             |
>>             .--------.      V
>> Camera 0 -->|        |   .-----.
>> Camera 1 -->|  MUX   |-->| OSD |--> DMA --> /dev/video0
>> Camera 2 -->|        |   `-----'
>>             `--------'
>>
>> A prerequisite is obviously that each piece of hardware and software
>> involved is able to cope with a sudden stream change. Perhaps not that
>> common, but no rocket science.
>>
>> It looks to me that each of these pieces can be modeled as an entity and
>> the s_routing API is a perfect fit for the mux block. Am I wrong?
>>
> 
> Personally, I would say that your muxer driver, if able to switch
> source without requiring a pipeline restart, should handle it
> internally. There are specific details that the mux should be able to
> handle, and we're still pretty vague on the details. As a start, which
> is the bus type your sources connects to the muxer with? Parallel?
> CSI-2? How is the video stream multiplexed? with CSI-2 VC? Do you need
> to control your source power during inactive period?

Not completely defined, but it could be either parallel or MIPI CSI-2,
and each input could be connected either directly from the sensor or
through a ser/des. Controlling power would be important as well.

> I'm sure there
> are more questions... I agree a partial pipeline restart might be
> something to consider, but at the same time I think this is not
> something strictly required to get this series merged, isn't it?

Right. As far as I can understand this series is already OK if you need
to change routing while the stream is not running.

Patch

diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c
index d8f7cbee86e7..13454af72c6e 100644
--- a/drivers/media/i2c/adv748x/adv748x-csi2.c
+++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
@@ -14,6 +14,8 @@ 
 
 #include "adv748x.h"
 
+#define ADV748X_CSI2_ROUTES_MAX 4
+
 struct adv748x_csi2_format {
 	unsigned int code;
 	unsigned int datatype;
@@ -253,10 +255,73 @@  static int adv748x_csi2_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
 	return 0;
 }
 
+static int adv748x_csi2_get_routing(struct v4l2_subdev *sd,
+				    struct v4l2_subdev_krouting *routing)
+{
+	struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
+	struct v4l2_subdev_route *r = routing->routes;
+	unsigned int vc;
+
+	if (routing->num_routes < ADV748X_CSI2_ROUTES_MAX) {
+		routing->num_routes = ADV748X_CSI2_ROUTES_MAX;
+		return -ENOSPC;
+	}
+
+	routing->num_routes = ADV748X_CSI2_ROUTES_MAX;
+
+	for (vc = 0; vc < ADV748X_CSI2_ROUTES_MAX; vc++) {
+		r->sink_pad = ADV748X_CSI2_SINK;
+		r->sink_stream = 0;
+		r->source_pad = ADV748X_CSI2_SOURCE;
+		r->source_stream = vc;
+		r->flags = vc == tx->vc ? V4L2_SUBDEV_ROUTE_FL_ACTIVE : 0;
+		r++;
+	}
+
+	return 0;
+}
+
+static int adv748x_csi2_set_routing(struct v4l2_subdev *sd,
+				    struct v4l2_subdev_krouting *routing)
+{
+	struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
+	struct v4l2_subdev_route *r = routing->routes;
+	unsigned int i;
+	int vc = -1;
+
+	if (routing->num_routes > ADV748X_CSI2_ROUTES_MAX)
+		return -ENOSPC;
+
+	for (i = 0; i < routing->num_routes; i++) {
+		if (r->sink_pad != ADV748X_CSI2_SINK ||
+		    r->sink_stream != 0 ||
+		    r->source_pad != ADV748X_CSI2_SOURCE ||
+		    r->source_stream >= ADV748X_CSI2_ROUTES_MAX)
+			return -EINVAL;
+
+		if (r->flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE) {
+			if (vc != -1)
+				return -EMLINK;
+
+			vc = r->source_stream;
+		}
+		r++;
+	}
+
+	if (vc != -1)
+		tx->vc = vc;
+
+	adv748x_csi2_set_virtual_channel(tx, tx->vc);
+
+	return 0;
+}
+
 static const struct v4l2_subdev_pad_ops adv748x_csi2_pad_ops = {
 	.get_fmt = adv748x_csi2_get_format,
 	.set_fmt = adv748x_csi2_set_format,
 	.get_frame_desc = adv748x_csi2_get_frame_desc,
+	.get_routing = adv748x_csi2_get_routing,
+	.set_routing = adv748x_csi2_set_routing,
 };
 
 /* -----------------------------------------------------------------------------