Hi Tomasz, On Tue, May 26, 2020 at 06:11:00PM +0200, Tomasz Figa wrote: > On Fri, May 22, 2020 at 11:06 AM Helen Koike <helen.koike@xxxxxxxxxxxxx> wrote: > > On 5/22/20 4:55 AM, Dafna Hirschfeld wrote: > > > Hi, > > > This is v4 of the patchset that was sent by Helen Koike. > > > > > > Media drivers need to iterate through the pipeline and call .s_stream() > > > callbacks in the subdevices. > > > > > > Instead of repeating code, add helpers for this. > > > > > > These helpers will go walk through the pipeline only visiting entities > > > that participates in the stream, i.e. it follows links from sink to source > > > (and not the opposite). > > > For example, in a topology like this https://bit.ly/3b2MxjI > > > calling v4l2_pipeline_stream_enable() from rkisp1_mainpath won't call > > > .s_stream(true) for rkisp1_resizer_selfpath. > > > > > > stream_count variable was added in v4l2_subdevice to handle nested calls > > > to the helpers. > > > This is useful when the driver allows streaming from more then one > > > capture device sharing subdevices. > > > > If I understand correctly, this isn't true anymore right? Nested calls aren't > > possible anymore since this version doesn't contain stream_count in struct v4l2_subdevice. > > > > Documentation of v4l2_pipeline_stream_*() should also be updated. > > > > Just to be clear, without the nested call, vimc will require to add its own > > counters, patch https://patchwork.kernel.org/patch/10948833/ will be > > required again to allow multi streaming. > > > > Also, patch "media: staging: rkisp1: cap: use v4l2_pipeline_stream_{enable,disable}" > > is cleaner in the previous version (with stream_count in struct v4l2_subdevice). > > > > All drivers that allows multi streaming will need to implement some special handling. > > > > Adding stream_count in struct v4l2_subdevice still seems cleaner to me. I'd like to hear > > what others think. > > I certainly would see this reference counting done in generic code, > because requiring every driver to do it simply adds to the endless > amount of boiler plate that V4L2 currently requires from the drivers. > :( > > I wonder if it wouldn't be possible to redesign the framework so that > .s_stream() at the subdev level is only called when it's expected to > either start or stop this particular subdev and driver's > implementation can simply execute the requested action. I'd very much like that. Note that I think a few drivers abuse the on parameter to the function to pass other values than 0 or 1. We'd have to fix those first (or maybe it has been done already, it's been a long time since I last checked). -- Regards, Laurent Pinchart