Linux-Media Archive on lore.kernel.org
 help / color / Atom feed
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>
Subject: Re: [PATCH v5 00/24] v4l: subdev internal routing
Date: Wed, 21 Apr 2021 15:57:07 +0300
Message-ID: <c3a6de40-e99a-7f4e-f36f-1b161d3be6df@ideasonboard.com> (raw)
In-Reply-To: <YHxtL6CM+AaV9+AG@pendragon.ideasonboard.com>

On 18/04/2021 20:32, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Nice to see a v5 of this plagued patch series :-)
> 
> On Thu, Apr 15, 2021 at 04:04:26PM +0300, Tomi Valkeinen wrote:
>> Hi,
>>
>> This is an RFC for subdev internal routing which is needed for
>> multiplexed streams support. I believe this is essentially a v5 of the
>> series, the v4 posted here:
>>
>> https://lore.kernel.org/linux-media/20190328200608.9463-1-jacopo+renesas@jmondi.org/
>>
>> Most of the patches are not changed (aside from fixing rebase issues
>> etc). The major changes in this version are:
>>
>> 1) Added 'which' field to the routing structs. It is currently not used,
>> as implementing it is not trivial. However, I think it's good to add it
>> to the uAPI now, and require the field to be set to
>> V4L2_SUBDEV_FORMAT_ACTIVE for now. See this RFC for an idea how this
>> could be implemented:
>>
>> https://lore.kernel.org/linux-media/20210409133659.389544-1-tomi.valkeinen@ideasonboard.com/
> 
> I've reviewed that, and I like it, but it's not straightforward to
> understand from that patch how you envision TRY to be implemented.

To be honest I don't have much at all experience with TRY. But, afaics, 
if we have means to store the TRY routes, and that is passed to the 
relevant ioctls in the subdev's, isn't that enough? It's up to the 
driver to implement the TRY functionality.

Although currently the S_ROUTING won't return anything to the userspace, 
it's supposed to either accept the routing table or return an error, 
whereas the S_FMT will do it's best to come up with a working setup and 
return it.

>> 2) No hardcoded maximum number of routes. Defining a maximum is not
>> possible, as there can be an arbitrary amount of routes per pad, and
>> there can be an arbitrary amount of pads per subdev. This series
>> allocates space for the routing table dynamically, which unfortunately
>> leads to not-just-a-few allocs and frees.
>>
>> 3) When searching for a format for a stream, the v4 looked for a
>> non-multiplexed pad only as far as the "other" side of the subdev. It
>> wouldn't work for a subdev which has multiplexed sink and source pads.
>> This series implements a "deep" get-format (v4l2_subdev_get_format_dir)
>> which follows a stream either towards the original source or the final
>> sink, while looking for a non-multiplexed pad with a format.
>>
>> Some thoughts:
>>
>> 1) Link validation and v4l2_subdev_get_format_dir need to look at the
>> routing, and this leads to multiple allocs to get a copy of the routing
>> table. There might be a possibility here to keep a table allocated and
>> re-use it in consecutive get_routing calls.
>>
>> Or even better, perhaps the kAPI could be changed so that allocs are not
>> needed. I thought about a kAPI where the subdev just returns a pointer
>> to its routing table, but then we hit the life-cycle problem: how to
>> ensure the table won't be freed or changed until the caller is done.
> 
> Storing the routing table in the v4l2_subdev_config (or
> v4l2_subdev_state) would be one way to do so, and I'd like to explore
> that direction. State lifetime is indeed an issue, and one simple option
> would be to just take the graph lock to modify the routing.
> 
>> 2) The routing uAPI is a bit vague. There is no way for the userspace to
>> figure out what kind of routing is allowed. Also, the existence of a
>> route in the routing table already indicates that the route is active,
>> but we also have V4L2_SUBDEV_ROUTE_FL_ACTIVE. I decided to keep
>> V4L2_SUBDEV_ROUTE_FL_ACTIVE for now, even if it doesn't really provide
>> any feature.
> 
> We can't report all possible routes if we take streams into account, but
> maybe we could report all possible routes between pads ? This could go
> through a separate ioctl.

That should be doable, but I wonder how much it helps. We should also 
somehow indicate if, say, routes from two source pads can go to the same 
sink pads, or can two streams from a single source pad go to separate 
sink pads, and so on. Is it better just to document what the driver 
supports for a specific hardware, than try to come up with a machine 
readable representation of the possible routings.

  Tomi

  reply index

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-15 13:04 Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 01/24] media: entity: Use pad as a starting point for graph walk Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 02/24] media: entity: Use pads instead of entities in the media graph walk stack Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 03/24] media: entity: Walk the graph based on pads Tomi Valkeinen
2021-04-18 17:47   ` Laurent Pinchart
2021-04-20 11:30     ` Sakari Ailus
2021-04-15 13:04 ` [PATCH v5 04/24] v4l: mc: Start walk from a specific pad in use count calculation Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 05/24] media: entity: Add iterator helper for entity pads Tomi Valkeinen
2021-04-18 17:52   ` Laurent Pinchart
2021-04-22 12:04     ` Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 06/24] media: entity: Move the pipeline from entity to pads Tomi Valkeinen
2021-04-18 18:00   ` Laurent Pinchart
2021-04-20 11:38     ` Sakari Ailus
2021-04-15 13:04 ` [PATCH v5 07/24] media: entity: Use pad as the starting point for a pipeline Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 08/24] media: entity: Add has_route entity operation Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 09/24] media: entity: Add media_entity_has_route() function Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 10/24] media: entity: Use routing information during graph traversal Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 11/24] media: entity: Skip link validation for pads to which there is no route to Tomi Valkeinen
2021-04-18 18:06   ` Laurent Pinchart
2021-04-20 11:41     ` Sakari Ailus
2021-04-23 12:37       ` Tomi Valkeinen
2021-04-29 12:06         ` Sakari Ailus
2021-04-29 14:10           ` Laurent Pinchart
2021-04-15 13:04 ` [PATCH v5 12/24] media: entity: Add an iterator helper for connected pads Tomi Valkeinen
2021-04-18 18:20   ` Laurent Pinchart
2021-04-20 11:48     ` Sakari Ailus
2021-04-29  1:33       ` Laurent Pinchart
2021-04-29 11:56         ` Sakari Ailus
2021-04-29 12:04           ` Tomi Valkeinen
2021-04-29 12:07             ` Sakari Ailus
2021-04-29 12:14               ` Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 13/24] media: entity: Add only connected pads to the pipeline Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 14/24] media: entity: Add debug information in graph walk route check Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 15/24] v4l: Add bus type to frame descriptors Tomi Valkeinen
2021-04-18 19:23   ` Laurent Pinchart
2021-04-20 11:50     ` Sakari Ailus
2021-04-22 12:30       ` Tomi Valkeinen
2021-04-29 11:58         ` Sakari Ailus
2021-04-29 14:09           ` Laurent Pinchart
2021-04-15 13:04 ` [PATCH v5 16/24] v4l: Add CSI-2 bus configuration " Tomi Valkeinen
2021-04-18 19:24   ` Laurent Pinchart
2021-04-20 16:32     ` Sakari Ailus
2021-04-15 13:04 ` [PATCH v5 17/24] v4l: Add stream to frame descriptor Tomi Valkeinen
2021-04-18 19:27   ` Laurent Pinchart
2021-04-22 12:47     ` Tomi Valkeinen
2021-04-22 16:18       ` Laurent Pinchart
2021-04-15 13:04 ` [PATCH v5 18/24] v4l: subdev: Add [GS]_ROUTING subdev ioctls and operations Tomi Valkeinen
2021-04-18 18:32   ` Laurent Pinchart
2021-04-22 11:16     ` Tomi Valkeinen
2021-04-22 16:20       ` Laurent Pinchart
2021-04-22 16:58         ` Tomi Valkeinen
2021-04-20 16:35   ` Sakari Ailus
2021-04-15 13:04 ` [PATCH v5 19/24] media: Documentation: Add GS_ROUTING documentation Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 20/24] v4l: mc: Add an S_ROUTING helper function for power state changes Tomi Valkeinen
2021-04-18 18:55   ` Laurent Pinchart
2021-04-20 16:41     ` Sakari Ailus
2021-04-15 13:04 ` [PATCH v5 21/24] v4l: subdev: routing kernel helper functions Tomi Valkeinen
2021-04-18 19:18   ` Laurent Pinchart
2021-04-15 13:04 ` [PATCH v5 22/24] v4l: subdev: add v4l2_subdev_get_format_dir() Tomi Valkeinen
2021-04-18 19:04   ` Laurent Pinchart
2021-04-21 13:04     ` Tomi Valkeinen
2021-04-29  1:43       ` Laurent Pinchart
2021-05-04  6:49         ` Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 23/24] v4l: subdev: Take routing information into account in link validation Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 24/24] v4l: subdev: increase V4L2_FRAME_DESC_ENTRY_MAX to 8 Tomi Valkeinen
2021-04-18 19:06   ` Laurent Pinchart
2021-04-16  8:38 ` [PATCH v5 00/24] v4l: subdev internal routing Niklas Söderlund
2021-04-16  8:47   ` Tomi Valkeinen
2021-04-16  8:56     ` Niklas Söderlund
2021-04-18 17:32 ` Laurent Pinchart
2021-04-21 12:57   ` Tomi Valkeinen [this message]
2021-04-29  1:27     ` Laurent Pinchart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c3a6de40-e99a-7f4e-f36f-1b161d3be6df@ideasonboard.com \
    --to=tomi.valkeinen@ideasonboard.com \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=jacopo+renesas@jmondi.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=niklas.soderlund+renesas@ragnatech.se \
    --cc=sakari.ailus@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org
	public-inbox-index linux-media

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git