Messages in this thread | | | Date | Thu, 11 Apr 2024 17:26:58 +0530 | Subject | Re: [PATCH v3 9/9] media: subdev: Support single-stream case in v4l2_subdev_enable/disable_streams() | From | Umang Jain <> |
| |
Hi Tomi
On 11/04/24 5:23 pm, Tomi Valkeinen wrote: > 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?
Yes it comes from the helper function
static const struct v4l2_subdev_video_ops imx283_video_ops = { .s_stream = v4l2_subdev_s_stream_helper, };
> > 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.
Yes, no routing in the driver. > > Tomi >
| |