Re: [PATCH 3/4] media: subdev: Improve v4l2_subdev_enable/disable_streams_fallback

From: Tomi Valkeinen
Date: Fri Apr 05 2024 - 05:13:18 EST


On 04/04/2024 17:25, Laurent Pinchart wrote:
On Thu, Apr 04, 2024 at 04:47:41PM +0300, Tomi Valkeinen wrote:
On 04/04/2024 16:06, Laurent Pinchart wrote:
On Thu, Apr 04, 2024 at 03:38:45PM +0300, Tomi Valkeinen wrote:
On 04/04/2024 15:18, Sakari Ailus wrote:
On Thu, Apr 04, 2024 at 01:50:02PM +0300, Tomi Valkeinen wrote:
v4l2_subdev_enable/disable_streams_fallback() supports falling back to
.s_stream() for subdevs with a single source pad. It also tracks the
enabled streams for that one pad in the sd->enabled_streams field.

Tracking the enabled streams with sd->enabled_streams does not make
sense, as with .s_stream() there can only be a single stream per pad.
Thus, as the v4l2_subdev_enable/disable_streams_fallback() only supports
a single source pad, all we really need is a boolean which tells whether
streaming has been enabled on this pad or not.

However, as we only need a true/false state for a pad (instead of
tracking which streams have been enabled for a pad), we can easily
extend the fallback mechanism to support multiple source pads as we only
need to keep track of which pads have been enabled.

Change the sd->enabled_streams field to sd->enabled_pads, which is a
64-bit bitmask tracking the enabled source pads. With this change we can
remove the restriction that
v4l2_subdev_enable/disable_streams_fallback() only supports a single
soruce pad.

s/soruce/source/


Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
---
drivers/media/v4l2-core/v4l2-subdev.c | 68 ++++++++++++++++++++---------------
include/media/v4l2-subdev.h | 9 +++--
2 files changed, 44 insertions(+), 33 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 3b3310bce5d4..91298bb84e6b 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -2090,37 +2090,43 @@ static int v4l2_subdev_enable_streams_fallback(struct v4l2_subdev *sd, u32 pad,
u64 streams_mask)
{
struct device *dev = sd->entity.graph_obj.mdev->dev;
- unsigned int i;
int ret;
/*
* The subdev doesn't implement pad-based stream enable, fall back
- * on the .s_stream() operation. This can only be done for subdevs that
- * have a single source pad, as sd->enabled_streams is global to the
- * subdev.
+ * on the .s_stream() operation.

While at it, s/on/to/

Actually, now that we support multiple pads here... Should the comment
and the if below, checking whether the pad is a source pad, be removed?
Is there any reason to require a source pad here (but not in the
non-fallback case)?

Mostly the lack of test platforms where we handle stream start/stop from
source to sink, calling the operations on sink pads. I'm sure there will
be unforeseen issues :-)

Have we tested that for the full streams version?

In the v2 I'll send shortly I have extended this test to cover also the full streams version. We can discuss there if this test is ok, or should it be dropped or only limited to the fallback case.

Tomi