[1/3] media: rcar-vin: Refactor link notify
Commit Message
The code have grown organically and a lot of checks are preformed for
the CSI-2 use-case even if the link notify is for a subdevice connected
to the parallel interface.
Before reworking the CSI-2 routing logic split the CSI-2 and parallel
link notify code in two separate blocks to make it clearer. There is no
functional change.
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
drivers/media/platform/rcar-vin/rcar-core.c | 87 +++++++++++----------
1 file changed, 45 insertions(+), 42 deletions(-)
Comments
Hi Niklas,
On Wed, Oct 20, 2021 at 10:02:23PM +0200, Niklas Söderlund wrote:
> The code have grown organically and a lot of checks are preformed for
code -> has
preformed -> performed
> the CSI-2 use-case even if the link notify is for a subdevice connected
> to the parallel interface.
>
> Before reworking the CSI-2 routing logic split the CSI-2 and parallel
> link notify code in two separate blocks to make it clearer. There is no
> functional change.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> drivers/media/platform/rcar-vin/rcar-core.c | 87 +++++++++++----------
> 1 file changed, 45 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index 1d92cc8ede8f8a3e..bd960c348ba5228c 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -795,12 +795,10 @@ static int rvin_csi2_link_notify(struct media_link *link, u32 flags,
> {
> struct rvin_group *group = container_of(link->graph_obj.mdev,
> struct rvin_group, mdev);
> - unsigned int master_id, channel, mask_new, i;
> - unsigned int mask = ~0;
> struct media_entity *entity;
> struct video_device *vdev;
> - struct media_pad *csi_pad;
> - struct rvin_dev *vin = NULL;
> + struct rvin_dev *vin;
> + unsigned int i;
> int csi_id, ret;
>
> ret = v4l2_pipeline_link_notify(link, flags, notification);
> @@ -826,33 +824,9 @@ static int rvin_csi2_link_notify(struct media_link *link, u32 flags,
> /* Find the master VIN that controls the routes. */
> vdev = media_entity_to_video_device(link->sink->entity);
> vin = container_of(vdev, struct rvin_dev, vdev);
> - master_id = rvin_group_id_to_master(vin->id);
>
> - if (WARN_ON(!group->vin[master_id])) {
> - ret = -ENODEV;
> - goto out;
> - }
>
> - /* Build a mask for already enabled links. */
> - for (i = master_id; i < master_id + 4; i++) {
> - if (!group->vin[i])
> - continue;
> -
> - /* Get remote CSI-2, if any. */
> - csi_pad = media_entity_remote_pad(
> - &group->vin[i]->vdev.entity.pads[0]);
> - if (!csi_pad)
> - continue;
> -
> - csi_id = rvin_group_entity_to_remote_id(group, csi_pad->entity);
> - channel = rvin_group_csi_pad_to_channel(csi_pad->index);
> -
> - mask &= rvin_csi2_get_mask(group->vin[i], csi_id, channel);
> - }
> -
There are now two empty lines here
> - /* Add the new link to the existing mask and check if it works. */
> csi_id = rvin_group_entity_to_remote_id(group, link->source->entity);
> -
> if (csi_id == -ENODEV) {
> struct v4l2_subdev *sd;
>
> @@ -877,25 +851,54 @@ static int rvin_csi2_link_notify(struct media_link *link, u32 flags,
> vin_err(vin, "Subdevice %s not registered to any VIN\n",
> link->source->entity->name);
> ret = -ENODEV;
> - goto out;
> - }
> + } else {
I wonder if the group mutex should be locked in the else branch only
and simplify the whole thing as:
if (csi_id == -ENODEV) {
/* subdev is parallel. */
for () {
return 0;
}
return -ENODEV;
}
/* subdev is CSI-2 */
mutex_lock(group->mutex);
...
mutex_unlock(group->mutex);
Even if I see above:
/*
* Don't allow link changes if any stream in the graph is active as
* modifying the CHSEL register fields can disrupt running streams.
*/
media_device_for_each_entity(entity, &group->mdev) {
struct media_pad *iter;
media_entity_for_each_pad(entity, iter) {
if (iter->stream_count)
return -EBUSY;
}
}
Being run out of the critical session, but that was already the case.
> + unsigned int master_id, channel, mask_new;
> + unsigned int mask = ~0;
> + struct media_pad *csi_pad;
>
> - channel = rvin_group_csi_pad_to_channel(link->source->index);
> - mask_new = mask & rvin_csi2_get_mask(vin, csi_id, channel);
> - vin_dbg(vin, "Try link change mask: 0x%x new: 0x%x\n", mask, mask_new);
> + master_id = rvin_group_id_to_master(vin->id);
>
> - if (!mask_new) {
> - ret = -EMLINK;
> - goto out;
> - }
> + if (WARN_ON(!group->vin[master_id])) {
> + ret = -ENODEV;
> + goto out;
> + }
> +
> + /* Build a mask for already enabled links. */
> + for (i = master_id; i < master_id + 4; i++) {
> + if (!group->vin[i])
> + continue;
> +
> + /* Get remote CSI-2, if any. */
> + csi_pad = media_entity_remote_pad(
> + &group->vin[i]->vdev.entity.pads[0]);
> + if (!csi_pad)
> + continue;
>
> - /* New valid CHSEL found, set the new value. */
> - ret = rvin_set_channel_routing(group->vin[master_id], __ffs(mask_new));
> - if (ret)
> - goto out;
> + csi_id = rvin_group_entity_to_remote_id(group,
> + csi_pad->entity);
> + channel = rvin_group_csi_pad_to_channel(csi_pad->index);
>
> - vin->is_csi = true;
> + mask &= rvin_csi2_get_mask(group->vin[i], csi_id, channel);
> + }
>
> + channel = rvin_group_csi_pad_to_channel(link->source->index);
> + mask_new = mask & rvin_csi2_get_mask(vin, csi_id, channel);
> + vin_dbg(vin, "Try link change mask: 0x%x new: 0x%x\n", mask,
> + mask_new);
> +
> + if (!mask_new) {
> + ret = -EMLINK;
> + goto out;
> + }
> +
> + /* New valid CHSEL found, set the new value. */
> + ret = rvin_set_channel_routing(group->vin[master_id],
> + __ffs(mask_new));
> + if (ret)
> + goto out;
> +
> + vin->is_csi = true;
> + }
> out:
> mutex_unlock(&group->lock);
>
> --
> 2.33.1
>
On Thu, Nov 04, 2021 at 05:36:54PM +0100, Jacopo Mondi wrote:
> On Wed, Oct 20, 2021 at 10:02:23PM +0200, Niklas Söderlund wrote:
> > The code have grown organically and a lot of checks are preformed for
>
> code -> has
> preformed -> performed
>
> > the CSI-2 use-case even if the link notify is for a subdevice connected
> > to the parallel interface.
> >
> > Before reworking the CSI-2 routing logic split the CSI-2 and parallel
> > link notify code in two separate blocks to make it clearer. There is no
> > functional change.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> > drivers/media/platform/rcar-vin/rcar-core.c | 87 +++++++++++----------
> > 1 file changed, 45 insertions(+), 42 deletions(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> > index 1d92cc8ede8f8a3e..bd960c348ba5228c 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-core.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> > @@ -795,12 +795,10 @@ static int rvin_csi2_link_notify(struct media_link *link, u32 flags,
> > {
> > struct rvin_group *group = container_of(link->graph_obj.mdev,
> > struct rvin_group, mdev);
> > - unsigned int master_id, channel, mask_new, i;
> > - unsigned int mask = ~0;
> > struct media_entity *entity;
> > struct video_device *vdev;
> > - struct media_pad *csi_pad;
> > - struct rvin_dev *vin = NULL;
> > + struct rvin_dev *vin;
> > + unsigned int i;
> > int csi_id, ret;
> >
> > ret = v4l2_pipeline_link_notify(link, flags, notification);
> > @@ -826,33 +824,9 @@ static int rvin_csi2_link_notify(struct media_link *link, u32 flags,
> > /* Find the master VIN that controls the routes. */
> > vdev = media_entity_to_video_device(link->sink->entity);
> > vin = container_of(vdev, struct rvin_dev, vdev);
> > - master_id = rvin_group_id_to_master(vin->id);
> >
> > - if (WARN_ON(!group->vin[master_id])) {
> > - ret = -ENODEV;
> > - goto out;
> > - }
> >
> > - /* Build a mask for already enabled links. */
> > - for (i = master_id; i < master_id + 4; i++) {
> > - if (!group->vin[i])
> > - continue;
> > -
> > - /* Get remote CSI-2, if any. */
> > - csi_pad = media_entity_remote_pad(
> > - &group->vin[i]->vdev.entity.pads[0]);
> > - if (!csi_pad)
> > - continue;
> > -
> > - csi_id = rvin_group_entity_to_remote_id(group, csi_pad->entity);
> > - channel = rvin_group_csi_pad_to_channel(csi_pad->index);
> > -
> > - mask &= rvin_csi2_get_mask(group->vin[i], csi_id, channel);
> > - }
> > -
>
> There are now two empty lines here
>
> > - /* Add the new link to the existing mask and check if it works. */
> > csi_id = rvin_group_entity_to_remote_id(group, link->source->entity);
> > -
> > if (csi_id == -ENODEV) {
> > struct v4l2_subdev *sd;
> >
> > @@ -877,25 +851,54 @@ static int rvin_csi2_link_notify(struct media_link *link, u32 flags,
> > vin_err(vin, "Subdevice %s not registered to any VIN\n",
> > link->source->entity->name);
> > ret = -ENODEV;
> > - goto out;
> > - }
> > + } else {
>
> I wonder if the group mutex should be locked in the else branch only
> and simplify the whole thing as:
Isn't the lock required to call rvin_group_entity_to_remote_id() ? I
think the two lines before:
vdev = media_entity_to_video_device(link->sink->entity);
vin = container_of(vdev, struct rvin_dev, vdev);
could however be moved before mutex_lock().
> if (csi_id == -ENODEV) {
> /* subdev is parallel. */
> for () {
Unless I'm mistaken, this needs locking in order to access group->vin[]
safely.
> return 0;
> }
>
> return -ENODEV;
> }
>
> /* subdev is CSI-2 */
> mutex_lock(group->mutex);
>
> ...
>
> mutex_unlock(group->mutex);
>
> Even if I see above:
>
> /*
> * Don't allow link changes if any stream in the graph is active as
> * modifying the CHSEL register fields can disrupt running streams.
> */
> media_device_for_each_entity(entity, &group->mdev) {
> struct media_pad *iter;
>
> media_entity_for_each_pad(entity, iter) {
> if (iter->stream_count)
> return -EBUSY;
> }
> }
>
> Being run out of the critical session, but that was already the case.
>
>
>
> > + unsigned int master_id, channel, mask_new;
> > + unsigned int mask = ~0;
> > + struct media_pad *csi_pad;
> >
> > - channel = rvin_group_csi_pad_to_channel(link->source->index);
> > - mask_new = mask & rvin_csi2_get_mask(vin, csi_id, channel);
> > - vin_dbg(vin, "Try link change mask: 0x%x new: 0x%x\n", mask, mask_new);
> > + master_id = rvin_group_id_to_master(vin->id);
> >
> > - if (!mask_new) {
> > - ret = -EMLINK;
> > - goto out;
> > - }
> > + if (WARN_ON(!group->vin[master_id])) {
> > + ret = -ENODEV;
> > + goto out;
> > + }
> > +
> > + /* Build a mask for already enabled links. */
> > + for (i = master_id; i < master_id + 4; i++) {
> > + if (!group->vin[i])
> > + continue;
> > +
> > + /* Get remote CSI-2, if any. */
> > + csi_pad = media_entity_remote_pad(
> > + &group->vin[i]->vdev.entity.pads[0]);
> > + if (!csi_pad)
> > + continue;
> >
> > - /* New valid CHSEL found, set the new value. */
> > - ret = rvin_set_channel_routing(group->vin[master_id], __ffs(mask_new));
> > - if (ret)
> > - goto out;
> > + csi_id = rvin_group_entity_to_remote_id(group,
> > + csi_pad->entity);
> > + channel = rvin_group_csi_pad_to_channel(csi_pad->index);
> >
> > - vin->is_csi = true;
> > + mask &= rvin_csi2_get_mask(group->vin[i], csi_id, channel);
> > + }
> >
> > + channel = rvin_group_csi_pad_to_channel(link->source->index);
> > + mask_new = mask & rvin_csi2_get_mask(vin, csi_id, channel);
> > + vin_dbg(vin, "Try link change mask: 0x%x new: 0x%x\n", mask,
> > + mask_new);
> > +
> > + if (!mask_new) {
> > + ret = -EMLINK;
> > + goto out;
> > + }
> > +
> > + /* New valid CHSEL found, set the new value. */
> > + ret = rvin_set_channel_routing(group->vin[master_id],
> > + __ffs(mask_new));
> > + if (ret)
> > + goto out;
> > +
> > + vin->is_csi = true;
I'm tempted to factor code out to two functions for the parallel and
CSI-2 case, but it could be done later too.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > + }
> > out:
> > mutex_unlock(&group->lock);
> >
Hi Jacopo and Laurent,
Thanks for your feedback!
On 2021-11-10 19:48:05 +0200, Laurent Pinchart wrote:
> On Thu, Nov 04, 2021 at 05:36:54PM +0100, Jacopo Mondi wrote:
> > On Wed, Oct 20, 2021 at 10:02:23PM +0200, Niklas Söderlund wrote:
> > > The code have grown organically and a lot of checks are preformed for
> >
> > code -> has
> > preformed -> performed
> >
> > > the CSI-2 use-case even if the link notify is for a subdevice connected
> > > to the parallel interface.
> > >
> > > Before reworking the CSI-2 routing logic split the CSI-2 and parallel
> > > link notify code in two separate blocks to make it clearer. There is no
> > > functional change.
> > >
> > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > > ---
> > > drivers/media/platform/rcar-vin/rcar-core.c | 87 +++++++++++----------
> > > 1 file changed, 45 insertions(+), 42 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> > > index 1d92cc8ede8f8a3e..bd960c348ba5228c 100644
> > > --- a/drivers/media/platform/rcar-vin/rcar-core.c
> > > +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> > > @@ -795,12 +795,10 @@ static int rvin_csi2_link_notify(struct media_link *link, u32 flags,
> > > {
> > > struct rvin_group *group = container_of(link->graph_obj.mdev,
> > > struct rvin_group, mdev);
> > > - unsigned int master_id, channel, mask_new, i;
> > > - unsigned int mask = ~0;
> > > struct media_entity *entity;
> > > struct video_device *vdev;
> > > - struct media_pad *csi_pad;
> > > - struct rvin_dev *vin = NULL;
> > > + struct rvin_dev *vin;
> > > + unsigned int i;
> > > int csi_id, ret;
> > >
> > > ret = v4l2_pipeline_link_notify(link, flags, notification);
> > > @@ -826,33 +824,9 @@ static int rvin_csi2_link_notify(struct media_link *link, u32 flags,
> > > /* Find the master VIN that controls the routes. */
> > > vdev = media_entity_to_video_device(link->sink->entity);
> > > vin = container_of(vdev, struct rvin_dev, vdev);
> > > - master_id = rvin_group_id_to_master(vin->id);
> > >
> > > - if (WARN_ON(!group->vin[master_id])) {
> > > - ret = -ENODEV;
> > > - goto out;
> > > - }
> > >
> > > - /* Build a mask for already enabled links. */
> > > - for (i = master_id; i < master_id + 4; i++) {
> > > - if (!group->vin[i])
> > > - continue;
> > > -
> > > - /* Get remote CSI-2, if any. */
> > > - csi_pad = media_entity_remote_pad(
> > > - &group->vin[i]->vdev.entity.pads[0]);
> > > - if (!csi_pad)
> > > - continue;
> > > -
> > > - csi_id = rvin_group_entity_to_remote_id(group, csi_pad->entity);
> > > - channel = rvin_group_csi_pad_to_channel(csi_pad->index);
> > > -
> > > - mask &= rvin_csi2_get_mask(group->vin[i], csi_id, channel);
> > > - }
> > > -
> >
> > There are now two empty lines here
> >
> > > - /* Add the new link to the existing mask and check if it works. */
> > > csi_id = rvin_group_entity_to_remote_id(group, link->source->entity);
> > > -
> > > if (csi_id == -ENODEV) {
> > > struct v4l2_subdev *sd;
> > >
> > > @@ -877,25 +851,54 @@ static int rvin_csi2_link_notify(struct media_link *link, u32 flags,
> > > vin_err(vin, "Subdevice %s not registered to any VIN\n",
> > > link->source->entity->name);
> > > ret = -ENODEV;
> > > - goto out;
> > > - }
> > > + } else {
> >
> > I wonder if the group mutex should be locked in the else branch only
> > and simplify the whole thing as:
>
> Isn't the lock required to call rvin_group_entity_to_remote_id() ? I
> think the two lines before:
>
> vdev = media_entity_to_video_device(link->sink->entity);
> vin = container_of(vdev, struct rvin_dev, vdev);
>
> could however be moved before mutex_lock().
>
> > if (csi_id == -ENODEV) {
> > /* subdev is parallel. */
> > for () {
>
> Unless I'm mistaken, this needs locking in order to access group->vin[]
> safely.
As Laurent points out it is needed to hold the lock both to call
rvin_group_entity_to_remote_id() and to access group->vin[i].
I have taken the suggestion to move the lookup of vdev and vin outside
the mutex.
>
> > return 0;
> > }
> >
> > return -ENODEV;
> > }
> >
> > /* subdev is CSI-2 */
> > mutex_lock(group->mutex);
> >
> > ...
> >
> > mutex_unlock(group->mutex);
> >
> > Even if I see above:
> >
> > /*
> > * Don't allow link changes if any stream in the graph is active as
> > * modifying the CHSEL register fields can disrupt running streams.
> > */
> > media_device_for_each_entity(entity, &group->mdev) {
> > struct media_pad *iter;
> >
> > media_entity_for_each_pad(entity, iter) {
> > if (iter->stream_count)
> > return -EBUSY;
> > }
> > }
> >
> > Being run out of the critical session, but that was already the case.
> >
> >
> >
> > > + unsigned int master_id, channel, mask_new;
> > > + unsigned int mask = ~0;
> > > + struct media_pad *csi_pad;
> > >
> > > - channel = rvin_group_csi_pad_to_channel(link->source->index);
> > > - mask_new = mask & rvin_csi2_get_mask(vin, csi_id, channel);
> > > - vin_dbg(vin, "Try link change mask: 0x%x new: 0x%x\n", mask, mask_new);
> > > + master_id = rvin_group_id_to_master(vin->id);
> > >
> > > - if (!mask_new) {
> > > - ret = -EMLINK;
> > > - goto out;
> > > - }
> > > + if (WARN_ON(!group->vin[master_id])) {
> > > + ret = -ENODEV;
> > > + goto out;
> > > + }
> > > +
> > > + /* Build a mask for already enabled links. */
> > > + for (i = master_id; i < master_id + 4; i++) {
> > > + if (!group->vin[i])
> > > + continue;
> > > +
> > > + /* Get remote CSI-2, if any. */
> > > + csi_pad = media_entity_remote_pad(
> > > + &group->vin[i]->vdev.entity.pads[0]);
> > > + if (!csi_pad)
> > > + continue;
> > >
> > > - /* New valid CHSEL found, set the new value. */
> > > - ret = rvin_set_channel_routing(group->vin[master_id], __ffs(mask_new));
> > > - if (ret)
> > > - goto out;
> > > + csi_id = rvin_group_entity_to_remote_id(group,
> > > + csi_pad->entity);
> > > + channel = rvin_group_csi_pad_to_channel(csi_pad->index);
> > >
> > > - vin->is_csi = true;
> > > + mask &= rvin_csi2_get_mask(group->vin[i], csi_id, channel);
> > > + }
> > >
> > > + channel = rvin_group_csi_pad_to_channel(link->source->index);
> > > + mask_new = mask & rvin_csi2_get_mask(vin, csi_id, channel);
> > > + vin_dbg(vin, "Try link change mask: 0x%x new: 0x%x\n", mask,
> > > + mask_new);
> > > +
> > > + if (!mask_new) {
> > > + ret = -EMLINK;
> > > + goto out;
> > > + }
> > > +
> > > + /* New valid CHSEL found, set the new value. */
> > > + ret = rvin_set_channel_routing(group->vin[master_id],
> > > + __ffs(mask_new));
> > > + if (ret)
> > > + goto out;
> > > +
> > > + vin->is_csi = true;
>
> I'm tempted to factor code out to two functions for the parallel and
> CSI-2 case, but it could be done later too.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> > > + }
> > > out:
> > > mutex_unlock(&group->lock);
> > >
>
> --
> Regards,
>
> Laurent Pinchart
@@ -795,12 +795,10 @@ static int rvin_csi2_link_notify(struct media_link *link, u32 flags,
{
struct rvin_group *group = container_of(link->graph_obj.mdev,
struct rvin_group, mdev);
- unsigned int master_id, channel, mask_new, i;
- unsigned int mask = ~0;
struct media_entity *entity;
struct video_device *vdev;
- struct media_pad *csi_pad;
- struct rvin_dev *vin = NULL;
+ struct rvin_dev *vin;
+ unsigned int i;
int csi_id, ret;
ret = v4l2_pipeline_link_notify(link, flags, notification);
@@ -826,33 +824,9 @@ static int rvin_csi2_link_notify(struct media_link *link, u32 flags,
/* Find the master VIN that controls the routes. */
vdev = media_entity_to_video_device(link->sink->entity);
vin = container_of(vdev, struct rvin_dev, vdev);
- master_id = rvin_group_id_to_master(vin->id);
- if (WARN_ON(!group->vin[master_id])) {
- ret = -ENODEV;
- goto out;
- }
- /* Build a mask for already enabled links. */
- for (i = master_id; i < master_id + 4; i++) {
- if (!group->vin[i])
- continue;
-
- /* Get remote CSI-2, if any. */
- csi_pad = media_entity_remote_pad(
- &group->vin[i]->vdev.entity.pads[0]);
- if (!csi_pad)
- continue;
-
- csi_id = rvin_group_entity_to_remote_id(group, csi_pad->entity);
- channel = rvin_group_csi_pad_to_channel(csi_pad->index);
-
- mask &= rvin_csi2_get_mask(group->vin[i], csi_id, channel);
- }
-
- /* Add the new link to the existing mask and check if it works. */
csi_id = rvin_group_entity_to_remote_id(group, link->source->entity);
-
if (csi_id == -ENODEV) {
struct v4l2_subdev *sd;
@@ -877,25 +851,54 @@ static int rvin_csi2_link_notify(struct media_link *link, u32 flags,
vin_err(vin, "Subdevice %s not registered to any VIN\n",
link->source->entity->name);
ret = -ENODEV;
- goto out;
- }
+ } else {
+ unsigned int master_id, channel, mask_new;
+ unsigned int mask = ~0;
+ struct media_pad *csi_pad;
- channel = rvin_group_csi_pad_to_channel(link->source->index);
- mask_new = mask & rvin_csi2_get_mask(vin, csi_id, channel);
- vin_dbg(vin, "Try link change mask: 0x%x new: 0x%x\n", mask, mask_new);
+ master_id = rvin_group_id_to_master(vin->id);
- if (!mask_new) {
- ret = -EMLINK;
- goto out;
- }
+ if (WARN_ON(!group->vin[master_id])) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ /* Build a mask for already enabled links. */
+ for (i = master_id; i < master_id + 4; i++) {
+ if (!group->vin[i])
+ continue;
+
+ /* Get remote CSI-2, if any. */
+ csi_pad = media_entity_remote_pad(
+ &group->vin[i]->vdev.entity.pads[0]);
+ if (!csi_pad)
+ continue;
- /* New valid CHSEL found, set the new value. */
- ret = rvin_set_channel_routing(group->vin[master_id], __ffs(mask_new));
- if (ret)
- goto out;
+ csi_id = rvin_group_entity_to_remote_id(group,
+ csi_pad->entity);
+ channel = rvin_group_csi_pad_to_channel(csi_pad->index);
- vin->is_csi = true;
+ mask &= rvin_csi2_get_mask(group->vin[i], csi_id, channel);
+ }
+ channel = rvin_group_csi_pad_to_channel(link->source->index);
+ mask_new = mask & rvin_csi2_get_mask(vin, csi_id, channel);
+ vin_dbg(vin, "Try link change mask: 0x%x new: 0x%x\n", mask,
+ mask_new);
+
+ if (!mask_new) {
+ ret = -EMLINK;
+ goto out;
+ }
+
+ /* New valid CHSEL found, set the new value. */
+ ret = rvin_set_channel_routing(group->vin[master_id],
+ __ffs(mask_new));
+ if (ret)
+ goto out;
+
+ vin->is_csi = true;
+ }
out:
mutex_unlock(&group->lock);