lkml.org 
[lkml]   [2024]   [Apr]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v3 9/9] media: subdev: Support single-stream case in v4l2_subdev_enable/disable_streams()
    From
    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
    >


    \
     
     \ /
      Last update: 2024-05-27 16:35    [W:3.983 / U:0.392 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site