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
