linux-kernel - Re: [PATCH] media: v4l2-subdev: Support enable/disable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0ad9841d-bb51-4512-9388-f9ce36372677@ideasonboard.com>
Date: Mon, 25 Mar 2024 15:43:01 +0200
From: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
To: Sakari Ailus <sakari.ailus@...ux.intel.com>,
 Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>,
 Hans Verkuil <hverkuil@...all.nl>, Umang Jain <umang.jain@...asonboard.com>,
 linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] media: v4l2-subdev: Support enable/disable_streams for
 single-pad subdevs

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@...asonboard.com>
>>> ---
>>> 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.

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?

>>> 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.

  Tomi