#linux-media 2021-06-16,Wed

andrey-konovalov has quit IRC (Ping timeout: 480 seconds)
miqztee has quit IRC (Remote host closed the connection)
jernej_ has joined #linux-media
jernej has quit IRC (Read error: Connection reset by peer)

[08:57]

andrey-konovalov has joined #linux-media

[09:06]

.... (idle for 15mn)

andrey-konovalov has quit IRC (Ping timeout: 480 seconds)

[09:21]

andrey-konovalov has joined #linux-media

[09:30]

jernej_ has quit IRC ()
lucaceresoli has joined #linux-media

[09:36]

hverkuil

jmondi: I'm ready in 15 minutes.

[09:44]

jmondi

whenever :)

[09:47]

hverkuil

jmondi: ping

[09:57]

jmondi

hverkuil: pong

[09:58]

hverkuil

sorry for the delay, something unexpected came up this morning throwing my schedule in disarray :-)

[09:59]

jmondi

no worries, thank you for your time
I've sent you a small series to comment on
I'm not happy with it, but it shows where I would like to go

[09:59]

hverkuil

Anyway, I've been reading up on this (including your series), but I think it is doable, but in a different way than your series.

[10:00]

***

jernej has joined #linux-media

[10:01]

jmondi

happy to receive suggestions :)

[10:01]

hverkuil

The key issue here is that some of the initialization of the subdev has to be done at a specific place in the notify_bound() of the parent of the subdev. Hence the use of the init op.
But that is too specific, you want a subdev driver to work for both parent devices that need this init step, but also those that do not.
The key functions that are involved are v4l2_async_match_notify() in v4l2-async.c which calls v4l2_device_register_subdev() and v4l2_async_notifier_call_bound().
And v4l2_device_register_subdev() will call the registered() op of the subdev if defined.
The problem is that the registered() op is called before the v4l2_async_notifier_call_bound().

[10:02]

jmondi

executing bound() before registered() would probably be enough, yes

[10:06]

hverkuil

What you want is that when the registered() op is called in the subdev, it can tell if a second init step is needed or not. If not, then it can do the whole initialization sequence. If a second init is needed, then it just does part of it and wait for a post_register callback to be called from the parent driver.

[10:06]

jmondi

where do you envision post_register() to be called ?

[10:07]

hverkuil

(I don't think you can call bound before registered, I think bound expects that the subdev is already registered)
The parent driver would call that (max9286 in this case).

[10:07]

***

NoGuest17 has joined #linux-media

[10:08]

jmondi

I think so, that's why I didn't go in the direction of calling registered() first :)
but that would be again the host driver that has to call post_registerd()
shouldn't the core do so ?

[10:08]

hverkuil

Adding such an op is easy, but the missing piece is how to let the subdev driver (rdacm20/21) know that there is a post_register step.

[10:08]

jmondi

I could suggest "just call it after bound()" but seems a bit lazy

[10:09]

hverkuil

I think this can be done by adding a flag to struct v4l2_async_notifier, indicating that it is a two-step initialization.
Then in v4l2_async_match_notify() it can test for that flag, and, if present, set a similar flag in sd->flags before calling v4l2_device_register_subdev.

[10:09]

jmondi

I think (for my use case) the decision of deferring initialization can be discerned from a dt property. If the channel amplitude is "low" we have to wait for the parent 'bound' to increase it, hence defer initialization

[10:10]

hverkuil

I don't think the core can call the post_register step since (as is the case of the max9286 I think) it has to be done in a specific order.

[10:11]

jmondi

I feel like the decision of deferring or not should be in the subdev driver..
the same way I don't feel like post_registered() should be called by the host/receiver, but the core should call it unconditionally
then the subdev decides if it has to be a no-op or it should actually perform initialization
can't post_registered() be a generically be defined as an operation that is called on the subdev after the parent has been notified about the bound() event ?
as, simmetrically, registered() is called -before- bound() on the parent is executed ?

[10:11]

hverkuil

Yes, but would that work? Looking at the max9286 code you perform some magic to put the serializer in a specific mode before calling init(). Isn't that something that only the max9286 would know?

[10:14]

jmondi

you're right, I need all subdevs to be bound, not a single one...

[10:15]

hverkuil

So in v4 17/17 all that would change is that v4l2_subdev_call(source->sd, core, init, 0); is replaced by v4l2_subdev_call(source->sd, core, post_register, 0); (or something along those lines).

[10:16]

jmondi

unfortunately the "magic" applies to all cameras, and has to be performed once all of them have probed

[10:16]

***

NiksDev has joined #linux-media

[10:17]

jmondi

that would work, but still feels a little too much ad-hoc ? That's why I tried to add a completed() callback to be executed on subdevs
so that the host driver doesn't have to call post_register by itself

[10:17]

hverkuil

But again, would that work? If the host driver needs to perform a specific sequence during which the post_register of the subdev has to take place, then it is only the host driver that will know this.

[10:19]

jmondi

I was wondering if there might be other use cases that might benefit from an operation that is executed on subdevs at media graph completion time...
if completed() is called on all the subdevs (respecting the topology) I think that should work..
unfortunately I can only walk the v4l2_device subdevs list and call completed() on them in the core, and the list is filled in by the probing order, which in my case matches the topology, but that's not guaranteed

[10:19]

hverkuil

I think this will be a problem if partial completion support is added at some point in the future. Basically my proposal tries to handle this between a parent and child device, while yours is doing this in a final top-level step.

[10:26]

jmondi

yes :) I was also pushed in that direction by the fact the internal subdev ops are said to be called by the 'core only'
which makes sense tbh
would you make post_registered() an internal op ?

[10:27]

hverkuil

Sorry, but 'core' in v4l2_subdev_core_ops doesn't mean that they can only be called from the core framework, just that they are generic ops that deal with basic core functionality.

[10:29]

jmondi

core != internal :)
* .. note::
* Never call this from drivers, only the v4l2 framework can call
* these ops.

[10:30]

hverkuil

v4l2_subdev_internal_ops are indeed framework only.
So post_register would have to be part of v4l2_subdev_core_ops.
(or whatever name we would give to it).

[10:31]

jmondi

yes, in the core ops if the driver has to call them (core ops where init() lives, btw :)

[10:32]

hverkuil

Trying to understand the max9286/rdacm sequence: with your patch series the max9286_completed function would be called first, then rdacm20_completed, right?

[10:34]

jmondi

that's the idea

[10:34]

hverkuil

So in this case having a post_registered being called unconditionally in v4l2_async_match_notify() after the v4l2_async_notifier_call_bound() would work.

[10:35]

jmondi

not really because it would be called after each camera has bound, for each single camera
what I need is to set some magic bit on the max9286 after -all- cameras have bound
and then perform deferred initialization on the rdacm2x
in the max9286's bound function we have a check for "have all cameras' probed?" if no we return earlier
otherwise in my 'init' series I went on, set the magic MAX9286_REV_AMP_HIGH bit, then call init() on all cameras

[10:35]

hverkuil

right, so wouldn't that work?

[10:39]

jmondi

(we had to make that check in bound() because being the max9286 notifier a subnotifier, it does not have any complete() callback, which only exists for the root notifier)
sorry I missed what "that" is :)

[10:39]

hverkuil

(12:38:27) jmondi: in the max9286's bound function we have a check for "have all cameras' probed?" if no we return earlier
(12:39:04) jmondi: otherwise in my 'init' series I went on, set the magic MAX9286_REV_AMP_HIGH bit, then call init() on all cameras

[10:40]

***

camus1 has joined #linux-media

[10:40]

hverkuil

Once all enabled sources have probed, then you call post_register for them.

[10:41]

***

camus1 has quit IRC ()

[10:41]

jmondi

I could do so, yes
as you said it's just about s/init/post_register
what I was not convinced about was that I assumed post_register had to be an internal op, and that cannot be called by drivers
if we make it a core op, well, that's what init() is already :)
(and I was trying to sketch out a more generic solution that would offer a 'completed()' callback as an internal ops, like my small series I sent you does)

[10:41]

hverkuil

So I think if a flag 'need_post_register' is added to struct v4l2_async_notifier, then v4l2_async_match_notify can check that flag. If false, then it will call post_register, if true, then it leaves it to the host driver.
(need_post_register is a poor name, BTW, it's more about doing a manual post_register step)
Perhaps: host_calls_post_register?

[10:43]

jmondi

if true the cores does not call it, if false it does so

[10:45]

***

camus has quit IRC (Ping timeout: 480 seconds)

[10:45]

jmondi

I think it can work for my use case...
are you ok with the camera subdev deciding if it has to defer init to post_register() by inspecting a dt prop ?
(or whatever else, what I mean is that the mechanism to decide if initialization has to be deferred would be driver specific)
(and both the receiver and the transmitter should get the same info, the former to set the host_calls_post_register flag, the latter to decide if it has to defer init or not)

[10:45]

hverkuil

I'm not sure about that. It is the host that will call post_register, so I feel that it is the host that has to tell the subdev(s) that it will do so. Also, do you need to? In this proposal the core will call post_register if the host will not handle it, so all a subdev driver has to do is implement post_register, and it will work regardless. There is no need to check for a dt property.

[10:50]

jmondi

you're right, the core would call post_register() anyhow, so I can simply put the bulk of the initilization there and it should just work
I'm also very happy to save adding a dt-property for that to the cameras
sure, the driver will look "strange" to users that do not need deferred initialization, but it would work

[10:51]

hverkuil

that's why they invented code comments :-)

[10:53]

jmondi

yeah but who reads them anyway
:p
thanks a lot for your time, I think I can work on this proposal
you know what would make me really happy ? if I could have the rest of the series merged in the meantime :)
it's all reviewed and the changes on top can be applied there separately
I've just tested the series without the 2 topmost patches and I got a single failure on 20 boots, so it's not perfect but it's still an improvement compared to the current mailine implementation
are we too late for a pull request if I resend it right now ?

[10:53]

hverkuil

That should be OK, there are a whole bunch of PRs pending anyway.

However, before you post a v5 check that it applies cleanly to https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=for-v5.14j

That has the subdev-wide state struct support. If it doesn't apply, then you need to rebase on top of that. Otherwise you can just base it on the normal media_tree master branch.

[10:56]

jmondi

I will check and resend a rebased version just after lunch

[10:59]

hverkuil

I think it will be OK, but it doesn't hurt to check.

[10:59]

jmondi

sure I will
thanks again for your time Hans!

[10:59]

hverkuil

no problem!

[11:01]

...... (idle for 29mn)

***

NiksDev has quit IRC (Ping timeout: 480 seconds)

[11:30]

pastly-antispam has joined #linux-media
paulk_ has joined #linux-media
paulk has quit IRC (Ping timeout: 480 seconds)

[11:40]

miqztee has joined #linux-media

[11:50]

jernej has quit IRC (Quit: Free ZNC ~ Powered by LunarBNC: https://LunarBNC.net)
jernej has joined #linux-media

[12:01]

...... (idle for 27mn)

NiksDev has joined #linux-media

[12:29]

...... (idle for 25mn)

gnurou[m] has joined #linux-media
gnurou[m] is now known as gnurou

[12:54]

gnurou has quit IRC (Quit: authenticating)
gnurou has joined #linux-media
gnurou has quit IRC ()
gnurou has joined #linux-media
miqztee has quit IRC (Remote host closed the connection)

[13:05]

........ (idle for 36mn)

jernej_ has joined #linux-media
jernej has quit IRC (Ping timeout: 480 seconds)

[13:46]

.... (idle for 15mn)

lucaceresoli has quit IRC (Ping timeout: 480 seconds)

[14:02]

.... (idle for 16mn)

lucaceresoli has joined #linux-media

[14:18]

miqztee has joined #linux-media

[14:23]

....... (idle for 33mn)

jernej_ has quit IRC ()
lucaceresoli has quit IRC (Ping timeout: 480 seconds)
jernej has joined #linux-media

[14:56]

...... (idle for 25mn)

lucaceresoli has joined #linux-media

[15:25]

........ (idle for 36mn)

miqztee has quit IRC (Remote host closed the connection)

[16:01]

..... (idle for 24mn)

milek7 has joined #linux-media

[16:25]

.... (idle for 19mn)

gouchi has joined #linux-media

[16:44]

..... (idle for 24mn)

ao2 has joined #linux-media

[17:08]

.... (idle for 15mn)

milek7

is there some way to make these cheap MS2106 capture devices work?
it just spams to dmesg with "uvcvideo 1-10:1.1: Failed to set UVC probe control : -32 (exp. 26)"

[17:23]

............................................... (idle for 3h53mn)

***

b-rad has quit IRC (Ping timeout: 480 seconds)
gouchi has quit IRC (Remote host closed the connection)
jm_h has quit IRC (Remote host closed the connection)
ao2 has quit IRC (Quit: Leaving)

[21:16]

......... (idle for 41mn)

lucaceresoli has quit IRC (Ping timeout: 480 seconds)

[22:05]

andrey-konovalov has quit IRC (Ping timeout: 480 seconds)

[22:14]

↑back Search ←Prev date Next date→ Show only urls

(Click on time to select a line by its url)