Messages in this thread | | | Date | Thu, 11 Apr 2024 14:53:55 +0300 | Subject | Re: [PATCH v3 9/9] media: subdev: Support single-stream case in v4l2_subdev_enable/disable_streams() | From | Tomi Valkeinen <> |
| |
On 11/04/2024 14:48, Umang Jain wrote: > Hi Tomi, > > On 11/04/24 4:37 pm, Tomi Valkeinen wrote: >> On 11/04/2024 14:02, Umang Jain wrote: >>> Hi Tomi, >>> >>> On 10/04/24 6:05 pm, Tomi Valkeinen wrote: >>>> At the moment the v4l2_subdev_enable/disable_streams() functions call >>>> fallback helpers to handle the case where the subdev only implements >>>> .s_stream(), and the main function handles the case where the subdev >>>> implements streams (V4L2_SUBDEV_FL_STREAMS, which implies >>>> .enable/disable_streams()). >>>> >>>> What is missing is support for subdevs which do not implement streams >>>> support, but do implement .enable/disable_streams(). Example cases of >>>> these subdevices are single-stream cameras, where using >>>> .enable/disable_streams() is not required but helps us remove the users >>>> of the legacy .s_stream(), and subdevices with multiple source pads >>>> (but >>>> single stream per pad), where .enable/disable_streams() allows the >>>> subdevice to control the enable/disable state per pad. >>>> >>>> The two single-streams cases (.s_stream() and >>>> .enable/disable_streams()) >>>> are very similar, and with small changes we can change the >>>> v4l2_subdev_enable/disable_streams() functions to support all three >>>> cases, without needing separate fallback functions. >>>> >>>> A few potentially problematic details, though: >>> >>> Does this mean the patch needs to be worked upon more ? >> >> I don't see the two issues below as blockers. >> >>> I quickly tested the series by applying it locally with my use case >>> of IMX283 .enable/disable streams and s_stream as the helper function >>> and it seems I am still seeing the same behaviour as before (i.e. not >>> being streamed) and have to carry the workaround as mentioned in [1] >>> **NOTE** >> >> Ok... Then something bugs here, as it is supposed to fix the problem. >> Can you trace the code a bit to see where it goes wrong? >> >> The execution should go to the "if (!(sd->flags & >> V4L2_SUBDEV_FL_STREAMS))" blocks in v4l2_subdev_collect_streams() and >> v4l2_subdev_set_streams_enabled(), > > The execution is not reaching in v4l2_subdev_collect streams() even, it > returns at > > if (!streams_mask) > return 0; > > in v4l2_subdev_enable_streams() > > Refer to : https://paste.debian.net/1313760/ > > My tree is based on v6.8 currently, but the series applies cleanly, so I > have not introduced any rebase artifacts. If you think, v6.8 might be > causing issues, I'll then try to test on RPi 5 with the latest media > tree perhaps.
So who is calling the v4l2_subdev_enable_streams? I presume it comes from v4l2_subdev_s_stream_helper(), in other words the sink side in your pipeline is using legacy s_stream?
Indeed, that helper still needs work. It needs to detect if there's no routing, and use the implicit stream 0. I missed that one.
Tomi
| |