<!-- Some styling for better description lists --><style type='text/css'>dt { font-weight: bold;float: left;display:inline;margin-right: 1em} dd { display:block; margin-left: 2em}</style>
***: camus1 has joined #linux-media
<br> camus has quit IRC (Read error: Connection reset by peer)
<br> NiksDev has joined #linux-media
<br> NiksDev2 has quit IRC (Remote host closed the connection)
<br> NiksDev has quit IRC (Ping timeout: 480 seconds)
<br> camus has joined #linux-media
<br> camus1 has quit IRC (Remote host closed the connection)
<br> camus1 has joined #linux-media
<br> camus has quit IRC (Ping timeout: 480 seconds)
<br> jm_h has joined #linux-media
<br> svarbanov has joined #linux-media
<br> camus has joined #linux-media
<br> camus1 has quit IRC (Remote host closed the connection)
<br> miqztee has joined #linux-media
<br> ao2 has joined #linux-media
<br> andrey-konovalov has joined #linux-media
<br> ao2 has quit IRC (Quit: Leaving)
hverkuil: <u>jmondi</u>: I might be delayed a bit. Just ping when you are ready.
jmondi: <u>hverkuil</u>: I'll send you a few patches to discuss on, ping me back whenever you are ready
***: andrey-konovalov has quit IRC (Ping timeout: 480 seconds)
<br> miqztee has quit IRC (Remote host closed the connection)
<br> jernej_ has joined #linux-media
<br> jernej has quit IRC (Read error: Connection reset by peer)
<br> andrey-konovalov has joined #linux-media
<br> andrey-konovalov has quit IRC (Ping timeout: 480 seconds)
<br> andrey-konovalov has joined #linux-media
<br> jernej_ has quit IRC ()
<br> lucaceresoli has joined #linux-media
hverkuil: <u>jmondi</u>: I'm ready in 15 minutes.
jmondi: whenever :)
hverkuil: <u>jmondi</u>: ping
jmondi: <u>hverkuil</u>: pong
hverkuil: sorry for the delay, something unexpected came up this morning throwing my schedule in disarray :-)
jmondi: no worries, thank you for your time
<br> I've sent you a small series to comment on
<br> I'm not happy with it, but it shows where I would like to go
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.
***: jernej has joined #linux-media
jmondi: happy to receive suggestions :)
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.
<br> 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.
<br> 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().
<br> And v4l2_device_register_subdev() will call the registered() op of the subdev if defined.
<br> The problem is that the registered() op is called before the v4l2_async_notifier_call_bound().
jmondi: executing bound() before registered() would probably be enough, yes
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.
jmondi: where do you envision post_register() to be called ?
hverkuil: (I don't think you can call bound before registered, I think bound expects that the subdev is already registered)
<br> The parent driver would call that (max9286 in this case).
***: NoGuest17 has joined #linux-media
jmondi: I think so, that's why I didn't go in the direction of calling registered() first :)
<br> but that would be again the host driver that has to call post_registerd()
<br> shouldn't the core do so ?
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.
jmondi: I could suggest "just call it after bound()" but seems a bit lazy
hverkuil: I think this can be done by adding a flag to struct v4l2_async_notifier, indicating that it is a two-step initialization.
<br> 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.
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
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.
jmondi: I feel like the decision of deferring or not should be in the subdev driver..
<br> the same way I don't feel like post_registered() should be called by the host/receiver, but the core should call it unconditionally
<br> then the subdev decides if it has to be a no-op or it should actually perform initialization
<br> 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 ?
<br> as, simmetrically, registered() is called -before- bound() on the parent is executed ?
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?
jmondi: you're right, I need all subdevs to be bound, not a single one...
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).
jmondi: unfortunately the "magic" applies to all cameras, and has to be performed once all of them have probed
***: NiksDev has joined #linux-media
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
<br> so that the host driver doesn't have to call post_register by itself
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.
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...
<br> if completed() is called on all the subdevs (respecting the topology) I think that should work..
<br> 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
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.
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'
<br> which makes sense tbh
<br> would you make post_registered() an internal op ?
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.
jmondi: core != internal :)
<br> * .. note::
<br> * Never call this from drivers, only the v4l2 framework can call
<br> * these ops.
hverkuil: v4l2_subdev_internal_ops are indeed framework only.
<br> So post_register would have to be part of v4l2_subdev_core_ops.
<br> (or whatever name we would give to it).
jmondi: yes, in the core ops if the driver has to call them (core ops where init() lives, btw :)
hverkuil: Trying to understand the max9286/rdacm sequence: with your patch series the max9286_completed function would be called first, then rdacm20_completed, right?
jmondi: that's the idea
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.
jmondi: not really because it would be called after each camera has bound, for each single camera
<br> what I need is to set some magic bit on the max9286 after -all- cameras have bound
<br> and then perform deferred initialization on the rdacm2x
<br> in the max9286's bound function we have a check for "have all cameras' probed?" if no we return earlier
<br> otherwise in my 'init' series I went on, set the magic MAX9286_REV_AMP_HIGH bit, then call init() on all cameras
hverkuil: right, so wouldn't that work?
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)
<br> sorry I missed what "that" is :)
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
<br> (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
***: camus1 has joined #linux-media
hverkuil: Once all enabled sources have probed, then you call post_register for them.
***: camus1 has quit IRC ()
jmondi: I could do so, yes
<br> as you said it's just about s/init/post_register
<br> 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
<br> if we make it a core op, well, that's what init() is already :)
<br> (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)
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.
<br> (need_post_register is a poor name, BTW, it's more about doing a manual post_register step)
<br> <u>Perhaps</u>: host_calls_post_register?
jmondi: if true the cores does not call it, if false it does so
***: camus has quit IRC (Ping timeout: 480 seconds)
jmondi: I think it can work for my use case...
<br> are you ok with the camera subdev deciding if it has to defer init to post_register() by inspecting a dt prop ?
<br> (or whatever else, what I mean is that the mechanism to decide if initialization has to be deferred would be driver specific)
<br> (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)
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.
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
<br> I'm also very happy to save adding a dt-property for that to the cameras
<br> sure, the driver will look "strange" to users that do not need deferred initialization, but it would work
hverkuil: that's why they invented code comments :-)
jmondi: yeah but who reads them anyway
<br> :p
<br> thanks a lot for your time, I think I can work on this proposal
<br> you know what would make me really happy ? if I could have the rest of the series merged in the meantime :)
<br> it's all reviewed and the changes on top can be applied there separately
<br> 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
<br> are we too late for a pull request if I resend it right now ?
hverkuil: That should be OK, there are a whole bunch of PRs pending anyway.
<br> 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
<br> 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.
jmondi: I will check and resend a rebased version just after lunch
hverkuil: I think it will be OK, but it doesn't hurt to check.
jmondi: sure I will
<br> thanks again for your time Hans!
hverkuil: no problem!
***: NiksDev has quit IRC (Ping timeout: 480 seconds)
<br> pastly-antispam has joined #linux-media
<br> paulk_ has joined #linux-media
<br> paulk has quit IRC (Ping timeout: 480 seconds)
<br> miqztee has joined #linux-media
<br> jernej has quit IRC (Quit: Free ZNC ~ Powered by LunarBNC: https://LunarBNC.net)
<br> jernej has joined #linux-media
<br> NiksDev has joined #linux-media
<br> gnurou[m] has joined #linux-media
<br> gnurou[m] is now known as gnurou
<br> gnurou has quit IRC (Quit: authenticating)
<br> gnurou has joined #linux-media
<br> gnurou has quit IRC ()
<br> gnurou has joined #linux-media
<br> miqztee has quit IRC (Remote host closed the connection)
<br> jernej_ has joined #linux-media
<br> jernej has quit IRC (Ping timeout: 480 seconds)
<br> lucaceresoli has quit IRC (Ping timeout: 480 seconds)
<br> lucaceresoli has joined #linux-media
<br> miqztee has joined #linux-media
<br> jernej_ has quit IRC ()
<br> lucaceresoli has quit IRC (Ping timeout: 480 seconds)
<br> jernej has joined #linux-media
<br> lucaceresoli has joined #linux-media
<br> miqztee has quit IRC (Remote host closed the connection)
<br> milek7 has joined #linux-media
<br> gouchi has joined #linux-media
<br> ao2 has joined #linux-media
milek7: is there some way to make these cheap MS2106 capture devices work?
<br> it just spams to dmesg with "uvcvideo 1-10:1.1: Failed to set UVC probe control : -32 (exp. 26)"
***: b-rad has quit IRC (Ping timeout: 480 seconds)
<br> gouchi has quit IRC (Remote host closed the connection)
<br> jm_h has quit IRC (Remote host closed the connection)
<br> ao2 has quit IRC (Quit: Leaving)
<br> lucaceresoli has quit IRC (Ping timeout: 480 seconds)
<br> andrey-konovalov has quit IRC (Ping timeout: 480 seconds)