| Message ID | 20171214190835.7672-4-niklas.soderlund+renesas@ragnatech.se (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@ragnatech.se> > --- > 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 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. 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. > } > > 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 >
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@ragnatech.se> > > --- > > 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@linux.intel.com
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); } 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; }
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@ragnatech.se> --- drivers/media/platform/rcar-vin/rcar-dma.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)