Re: [RFC PATCH v2] media: rcar-vin: Allow independent VIN link enablement

[Steve Longerbeam]

On 1/9/19 5:19 PM, Steve Longerbeam wrote:
> On 1/9/19 2:40 PM, Niklas SÃderlund wrote:
> > Hi Steve,
> >
> > Thanks for your patch, I think it looks good.
>
> Thanks for the Ack! I'm not real familiar with the RFC patch process. 
> Should this be submitted again with RFC stripped from the subject line?

I've been told there isn't really a process for RFC patch submission. I 
will re-submit and strip off the RFC and add your Reviewed-by.

Steve

> > On 2019-01-06 13:20:18 -0800, Steve Longerbeam wrote:
> > > There is a block of code in rvin_group_link_notify() that loops through
> > > all entities in the media graph, and prevents enabling a link to a VIN
> > > node if any entity is in use. This prevents enabling a VIN link even if
> > > there is an in-use entity somewhere in the graph that is independent of
> > > the link's pipeline.
> > >
> > > For example, the code will prevent enabling a link from the first
> > > rcar-csi2 receiver to a VIN node even if there is an enabled link
> > > somewhere far upstream on the second independent rcar-csi2 receiver
> > > pipeline.
> > >
> > > If this code is meant to prevent modifying a link if any entity in the
> > > graph is actively involved in streaming (because modifying the CHSEL
> > > register fields can disrupt any/all running streams), then the entities
> > > stream counts should be checked rather than the use counts.
> > >
> > > (There is already such a check in __media_entity_setup_link() that 
> > > verifies
> > > the stream_count of the link's source and sink entities are both zero,
> > > but that is insufficient, since there should be no running streams in
> > > the entire graph).
> > >
> > > Modify the media_device_for_each_entity() loop to check the entity
> > > stream_count instead of the use_count, and elaborate on the comment.
> > > VIN node links can now be enabled even if there are other independent
> > > in-use entities that are not streaming.
> > >
> > > Fixes: c0cc5aef31 ("media: rcar-vin: add link notify for Gen3")
> > >
> > > Signed-off-by: Steve Longerbeam <slongerbeam@xxxxxxxxx>
> > Reviewed-by: Niklas SÃderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> >
> > > ---
> > > Changes in v2:
> > > - bring back the media_device_for_each_entity() loop but check the
> > > ÂÂ stream_count not the use_count.
> > > ---
> > > Â drivers/media/platform/rcar-vin/rcar-core.c | 8 ++++++--
> > > Â 1 file changed, 6 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
> > > b/drivers/media/platform/rcar-vin/rcar-core.c
> > > index f0719ce24b97..6dd6b11c1b2b 100644
> > > --- a/drivers/media/platform/rcar-vin/rcar-core.c
> > > +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> > > @@ -131,9 +131,13 @@ static int rvin_group_link_notify(struct 
> > > media_link *link, u32 flags,
> > > !is_media_entity_v4l2_video_device(link->sink->entity))
> > > ÂÂÂÂÂÂÂÂÂ return 0;
> > > Â -ÂÂÂ /* If any entity is in use don't allow link changes. */
> > > +ÂÂÂ /*
> > > +ÂÂÂÂ * Don't allow link changes if any entity in the graph is
> > > +ÂÂÂÂ * streaming, because modifying the CHSEL register fields
> > > +ÂÂÂÂ * can disrupt running streams.
> > > +ÂÂÂÂ */
> > > ÂÂÂÂÂ media_device_for_each_entity(entity, &group->mdev)
> > > -ÂÂÂÂÂÂÂ if (entity->use_count)
> > > +ÂÂÂÂÂÂÂ if (entity->stream_count)
> > > ÂÂÂÂÂÂÂÂÂÂÂÂÂ return -EBUSY;
> > > Â ÂÂÂÂÂ mutex_lock(&group->lock);
> > > --
> > > 2.17.1