Re: [PATCH v7 20/27] v4l: subdev: add V4L2_SUBDEV_ROUTE_FL_SOURCE

From: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: linux-media@vger.kernel.org, sakari.ailus@linux.intel.com,
	Jacopo Mondi <jacopo+renesas@jmondi.org>,
	niklas.soderlund+renesas@ragnatech.se,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Hans Verkuil <hverkuil-cisco@xs4all.nl>,
	Pratyush Yadav <p.yadav@ti.com>,
	Lokesh Vutla <lokeshvutla@ti.com>
Subject: Re: [PATCH v7 20/27] v4l: subdev: add V4L2_SUBDEV_ROUTE_FL_SOURCE
Date: Fri, 2 Jul 2021 10:49:32 +0300	[thread overview]
Message-ID: <385db9a9-d2b9-64f6-6ba2-fe9ffa2d83f7@ideasonboard.com> (raw)
In-Reply-To: <YLv+1yqN90T2Rxhd@pendragon.ideasonboard.com>

Hi,

On 06/06/2021 01:46, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Sun, Jun 06, 2021 at 01:44:54AM +0300, Laurent Pinchart wrote:
>> On Mon, May 24, 2021 at 01:44:01PM +0300, Tomi Valkeinen wrote:
>>> Add route flag to indicate that the route is a source route. This means
>>> that the route does not lead anywhere, and the sink_pad and sink_stream
>>> should not be used.
>>
>> I don't like this much. It's not a route if it doesn't lead anywhere, so
>> this flag seems like a hack. If we need a way to discover streams on a
>> source, I'd rather have an explicit operation to do so. Can't the get
>> frame descriptor operation be used for this ?

If the "route" word is the problem, perhaps we can rename it to... 
"streams"?

If the sensor has one source pad with one or two streams, the userspace 
needs to be able to 1) configure if we have one or two streams 2) 
configure the stream IDs 3) configure the stream format. This is exactly 
the same as when configuring routes for a bridge, the only difference is 
that the sensor's streams come from inside the sensor, not from an 
external source.

Somewhat related, I was wondering if the whole routing table should be 
dropped. Now that the stream endpoints each have a configuration, 
similar to pads in current drivers, perhaps the routing information 
could be just part of the stream endpoints configuration.

So, stream 0 in pad 0 could have a config field that has the destination 
pad+stream tuple. The destination should, of course, have a matching but 
inverse destination configuration. The pad+stream tuples would be 
validated in the normal subdev validation step.

I haven't looked at this closely, so I'm not sure if this would help or not.

>> If the need is to find out that we're reaching the end of a pipeline
>> while going through links and routes, I'd rather have a pad flag to
>> indicate that the pad is an endpoint.

A pad could have both source streams and "routed" streams.

> Also, I can't see where this is being used, so maybe we can just drop it
> :-)

True, I seem to have dropped the use when I dropped my metadata hacks 
from the test sensor driver. But I think the flag, or something similar, 
is needed if this information is in the routing data (or whatever we 
rename it to).

  Tomi