Hej Sakari,
Tack för dina kommentarer.
On 2017-12-15 14:07:39 +0200, Sakari Ailus wrote:
> Hej,
>
> On Thu, Dec 14, 2017 at 08:08:23PM +0100, Niklas Söderlund wrote:
> > To work with multiplexed streams the pad and stream aware s_stream
> > operation needs to be used.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> > ---
> > drivers/media/platform/rcar-vin/rcar-dma.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> > index cf30e5fceb1d493a..8435491535060eae 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> > @@ -1180,7 +1180,7 @@ static int rvin_set_stream(struct rvin_dev *vin, int on)
> >
> > if (!on) {
> > media_pipeline_stop(vin->vdev.entity.pads);
> > - return v4l2_subdev_call(sd, video, s_stream, 0);
> > + return v4l2_subdev_call(sd, pad, s_stream, pad->index, 0, 0);
>
> Have you thought of adding a wrapper for the s_stream callback?
I toyed with the idea, then I reached the same conclusion you do bellow.
>
> I think you should either change all s_stream callbacks from video to pad,
> or add a wrapper which then calls the video op instead of the pad op if the
> pad op does not exist. Otherwise we again have two non-interoperable
> classes of drivers for no good reason.
Agreed.
>
> Thinking about it, I'm not all that certain changing all instances would be
> that much work in the end; it should be done anyway. Devices that have a
> single stream (i.e. everything right now) just don't care about the pad
> number.
I tried to cover this in the cover-letter, I believe the correct
approach is probably to move s_stream() from video ops to pad ops in the
long run. I did post a similar patch for this a while ago [1] but I fear
it's outdated by now. Before I refreshed that particular patch I was
interested on the feedback on this from this series as I don't want to
send out a patch touching so many drivers without at least some
discussion :-)
I would be find with both the helper and a full conversion approach. Or
to do it in stages, add a helper now and slowly convert all drivers and
then removing the helper. What would be your preferred way of dealing
with this?
1. http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2016-June/091250.html
>
> > }
> >
> > fmt.pad = pad->index;
> > @@ -1239,12 +1239,14 @@ static int rvin_set_stream(struct rvin_dev *vin, int on)
> > if (media_pipeline_start(vin->vdev.entity.pads, pipe))
> > return -EPIPE;
> >
> > - ret = v4l2_subdev_call(sd, video, s_stream, 1);
> > + ret = v4l2_subdev_call(sd, pad, s_stream, pad->index, 0, 1);
> > if (ret == -ENOIOCTLCMD)
> > ret = 0;
> > if (ret)
> > media_pipeline_stop(vin->vdev.entity.pads);
> >
> > + vin_dbg(vin, "pad: %u stream: 0 enable: %d\n", pad->index, on);
> > +
> > return ret;
> > }
> >
> > --
> > 2.15.1
> >
>
> --
> Sakari Ailus
> sakari.ailus@xxxxxxxxxxxxxxx
--
Regards,
Niklas Söderlund
