On 25/03/2024 19:52, Sakari Ailus wrote:
Moi,
On Mon, Mar 25, 2024 at 03:43:01PM +0200, Tomi Valkeinen wrote:
On 25/03/2024 15:02, Sakari Ailus wrote:
Moi,
Thanks for the patch.
On Mon, Mar 25, 2024 at 02:50:55PM +0200, Laurent Pinchart wrote:
Hi Tomi,
On Mon, Mar 25, 2024 at 02:43:23PM +0200, Tomi Valkeinen wrote:
Currently a subdevice with a single pad, e.g. a sensor subdevice, must
use the v4l2_subdev_video_ops.s_stream op, instead of
v4l2_subdev_pad_ops.enable/disable_streams. This is because the
enable/disable_streams machinery requires a routing table which a subdev
cannot have with a single pad.
Implement enable/disable_streams support for these single-pad subdevices
by assuming an implicit stream 0 when the subdevice has only one pad.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
---
Even though I did send this patch, I'm not sure if this is necessary.
s_stream works fine for the subdevs with a single pad. With the upcoming
internal pads, adding an internal pad to the subdev will create a
routing table, and enable/disable_streams would get "fixed" that way.
I'd like to get rid of a redundant way to control streaming.
We can't get rid of it anyway, can we? We're not going to convert old
drivers to streams.
I'd expect to do that but it'd take a long time. That being said, I think
we need to consider devices without pads (VCMs) so it may well be this
would remain after all.
For new drivers, yes, we shouldn't use s_stream. But is the answer for new
sensor drivers this patch, or requiring an internal pad?
For new drivers I'd like to see an internal pad in fact.
{enable,disable}_streams is still internal to the kernel.
So, you think this patch should be dropped?
So perhaps the question is, do we want to support single-pad subdevs in
the future, in which case something like this patch is necessary, or
will all modern source subdev drivers have internal pads, in which
case this is not needed...
I think the latter would be best. I however can't guarantee we won't
have valid use cases for (enable|disable)_streams on single-pad subdevs
though, so you patch could still be interesting.
Instead of the number of pads, could we use instead the
V4L2_SUBDEV_FL_STREAMS flag or whether g_routing op is supported to
determine the need for this?
Maybe, but are they better? Do you see some issue with checking for the
number of pads? I considered a few options, but then thought that the most
safest test for this case is 1) one pad 2) enable/disable_streams
implemented.
I think I'd actually prefer {enable,disable}_streams in fact.
Hmm, sorry, now I'm confused =). What do you mean with that?
Tomi
