Re: [PATCH v8 00/36] v4l: subdev internal routing and streams

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi all,

On 30/08/2021 14:00, Tomi Valkeinen wrote:
Hi,

This is v8 of the multiplexed streams series. v7 can be found from:

https://lore.kernel.org/linux-media/20210524104408.599645-1-tomi.valkeinen@xxxxxxxxxxxxxxxx/

The main change in this version is the implementation and use of
centralized active state for subdevs.

I have pushed my work branch to:

git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git multistream/work-v8

which contains the patches in this series, along with subdev drivers
using multiplexed streams.

Both this series and the branch above are based on top of today's
git://linuxtv.org/media_tree.git master.

The documentation still needs improving, but I hope the docs in this
series, and the drivers in the work branch, are enough to give the
reviewers enough information to do a review.

As can be guessed from the work branch, I have been testing this series
with TI's FPDLink setup. I have also done a "backwards compatibility"
test by dropping all multiplexed streams patches from the CAL driver
(the CSI-2 RX on the TI SoC), and using the FPDLink drivers with
single-stream configuration.

We've had good discussions with Jacopo about this series.

I chose the approaches in this series based on what I think the API should be, even if the API has behaved differently before. And I think I'm also leaning forward a bit, in the sense that the full benefit of the API can only be had after more changes to the core and subdev drivers (changes which may or may not happen).

If I understood Jacopo correctly, his comments were essentially that my approach is different than the current one, and as the current drivers anyway do things the old way, this is very confusing. Basically I create two different kinds of subdev drivers: the old and new ones, which manage state differently.

I want to summarize two particular topics:

1) Active state & subdev ops

In upstream we have v4l2_subdev_state which contains only the pad_config array. This state is "try" state, it's allocated per file-handle, and passed to the subdev drivers when executing subdev ioctls in try-mode (which == V4L2_SUBDEV_FORMAT_TRY). This try-state is sometimes also passed to the subdev drivers when executing in active-mode (V4L2_SUBDEV_FORMAT_ACTIVE), but the drivers are supposed to ignore it.

There is also an active-state, but it's driver-specific and driver-internal. The drivers check the 'which' value, and either use the passed try-state, or the internal state.

What I did in this series aims to have both try- and active-states in v4l2 core, and passing the correct state to subdevs so that they don't (necessarily) need any internal state. There are some issues with it, which have been discussed, but I believe those issues can be fixed.

The subdev drivers need to be written to use this new active-state, so it doesn't affect the current drivers.

The question is, do we want to go that way? We could as well keep the current behavior of subdev drivers only getting the try-state as a parameter, and the drivers digging out the active state manually. This active state could either be internal to the driver, or it could be in the base struct v4l2_subdev (see also topic 2).

2) Shared subdev active-state

The try-state is specific to a file-handle, and afaics have no real race-issues as it's not really shared. Although I guess in theory an application could call subdev ioctls from multiple threads using the same fd.

In upstream the subdev drivers' internal state is managed fully by the subdev drivers. The drivers are expected to handle necessary locking in their subdev ops and interrupt handlers. If, say, v4l2 core needs to get a format from the subdev, it calls a subdev op to get it.

In my series I aimed to a shared active-state. The state is located in a known place, struct v4l2_subdev, and can be accessed without the subdev driver's help. This requires locking, which I have implemented.

At the moment the only real benefit with this is reading the routing table while doing pipeline validation: Instead of having to dynamically allocate memory and call the subdev op to create a copy of the routing table (for each subdev, possibly multiple times), the validator can just lock the state, and use it. And, in fact, there is no get_routing subdev op at all.

But this means that the subdev drivers that support this new active-state have to handle locking for the active state, and the "mindset" is different than previously.

So the question is, do we want to go that way? We could as well mandate that the active-state can only be accessed via subdev's ops (and add the get-routing, of course), and the subdev manages the locking internally.

 Tomi



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux