[08:38] <sailus> mchehab: Bom dia!
[09:14] <pinchartl> hverkuil1: back from holidays ?
[09:16] <hverkuil1> pinchartl: yes.
[09:16] <hverkuil1> I'm still in the Netherlands this week, but I'm working from there. Back to Oslo Friday evening.
[09:18] <pinchartl> are you drowning in e-mails ?
[09:20] <hverkuil1> No, but that's because almost all my colleagues were on vacation as well :-)
[09:20] <pinchartl> :-)
[09:21] <pinchartl> I've been struggling with the request API lately, or to be more accurate with part of its implementation. I'd like to discuss that with you when you'll have some time
[09:24] <hverkuil1> pinchartl: I plan to review Sakari's request API patch series this week (probably today/tomorrow), so after I've done that it would be a good time to discuss that.
[09:24] <pinchartl> it's an evolution of mine
[09:25] <pinchartl> you might want to review the integration of both series though
[09:25] <pinchartl> I've pushed it to git://linuxtv.org/pinchartl/media.git drm/du/vsp1-kms/request
[09:30] <sailus> pinchartl: We do need a single series in the end.
[09:30] <sailus> I don't think there are many differences remaining now.
[09:30] <pinchartl> to my knowledge I've incorporated all your changes in the above branch
[09:30] <pinchartl> could you check that ?
[09:31] <hverkuil1> If that's the case, would it be useful if pinchartl posts a new patch series superceding sailus' series?
[09:33] <pinchartl> hverkuil1: I will, but I'd like Sakari to check it first to make sure I haven't forgotten anything
[09:33] <pinchartl> or I can just post it and let him review it then
[09:33] <sailus> pinchartl: Except the one that shouldn't be there. :-) I'll try to check that later this week.
[09:35] <pinchartl> thanks
[10:20] <hverkuil1> pinchartl, sailus: so I can wait with reviewing request API code until a new series has been posted?
[10:24] <pinchartl> hverkuil1: that's fine with me
[10:24] <pinchartl> but there's an item I'd like to discuss with you before posting the next version
[10:25] <hverkuil1> pinchartl: can we discuss that in half an hour?
[10:25] <hverkuil1> 1pm my time would work.
[10:25] <pinchartl> sure
[10:41] <hverkuil1> pinchartl: sailus: FYI: I want to make time available to more actively pursue the request API, esp. for the initial use case of codecs. The lack of this API in mainline is starting to be a blocker. And it looks like I'll have more time free to work on this (dangerous prediction!).
[10:43] <pinchartl> hverkuil1: we need to really collaborate closely then, as there are very different use cases that come with very different requirements, and we can only have a single request API
[10:43] <pinchartl> (although I'd be open to discussing that last point, but I doubt multiple APIs would be a good idea)
[10:53] <sailus> The same time works for me as well.
[11:01] <hverkuil1> pinchartl: I'm ready
[11:04] <pinchartl> so am I
[11:05] <pinchartl> so, the problem I ran into was verification of requests
[11:05] <pinchartl> when a request is submitted (I use the word submit here to mean either queue or apply, from a verification point of view the two are similar) it needs to be verified by the driver to ensure that the requested configuration is valid
[11:05] <pinchartl> I hope you agree so far :-)
[11:06] <sailus> Me, too!
[11:06] <hverkuil1> brb
[11:06] <pinchartl> so much for the agreement... :-D
[11:07] <pinchartl> sailus: this is the issue we've previously discussed together, I'll try to explain it to Hans and then I'll let you chime in if that's fine with you
[11:07] <hverkuil1> back
[11:08] <hverkuil1> I agree
[11:09] <pinchartl> good :-)
[11:09] <pinchartl> the part of the request I was trying to validate was link configuration
[11:09] <pinchartl> (obviously in relationship with pad formats)
[11:10] <pinchartl> the request only contains a partial configuration of the device
[11:10] <hverkuil1> although I am not sure if you can do a full verification when you queue it as by the time it has to be applied things may have changed.
[11:10] <pinchartl> in the case of links, it contains the state of links that are changed in that request
[11:10] <pinchartl> ok, on that topic
[11:10] <pinchartl> I think verification at queue time needs to be doable
[11:11] <pinchartl> it's the only part that runs synchronously and can return an error to the user
[11:11] <pinchartl> applying a queued request is done asynchronously
[11:11] <pinchartl> and often from an interrupt handler
[11:11] <pinchartl> you don't want to perform time-consuming verification at that point
[11:11] <pinchartl> what are you worried about that could make request validation at queue time difficult or even impossible ?
[11:12] <hverkuil1> Not impossible, but difficult.
[11:13] <pinchartl> how so ?
[11:13] <pinchartl> (I like difficult better than impossible, it's already one step closer to making it happen :-))
[11:14] <hverkuil1> If you have multiple queued partial config changes, then validation of a new request can be tricky. You basically have to take all the previous not-yet-applied requests into account when doing the validation.
[11:14] <hverkuil1> Depends on the details, this scenario is worst-case.
[11:14] <pinchartl> yes, that's the hard part
[11:14] <pinchartl> that's exactly the problem I ran into
[11:15] <pinchartl> and I got an idea to solve it
[11:15] <pinchartl> depending on whether you'll like it, it's clever or terrible :-)
[11:15] <pinchartl> the idea is to create a state object that spans the whole device
[11:16] <pinchartl> I think I've briefly mentioned that to you previously
[11:16] <hverkuil1> Yes, I remember.
[11:16] <pinchartl> the state object would contain formats, selection rectangle, links, and in a way I haven't experimented with you, controls
[11:16] <pinchartl> there would be an active state object for the active state
[11:16] <pinchartl> and one state object instance created for every request
[11:17] <pinchartl> creating a state object for a request would "simply" duplicate the state of the previous request, and modify it based on the contents of the request
[11:17] <pinchartl> then the driver will have to validate the state as a whole
[11:18] <pinchartl> instead of having to fish for link state from the active link state and all requests previously queued, it would get a full link state from the state object and validate that
[11:18] <pinchartl> it's much easier for drivers
[11:18] <pinchartl> and not difficult to implement either in the core
[11:18] <pinchartl> the only drawback would be a slight overhead due to duplication of state structures for the whole device
[11:19] <pinchartl> so far, any comment about that approach ?
[11:19] <pinchartl> sailus: same question
[11:19] <sailus> (Reading the backlog.)
[11:20] <pinchartl> there are two issues that need to be solved, how to implement state validation across subdevs (a new subdev op ? reusing existing ops ?) and how to handle direct modifications of the device state using the non-request API when you have requests queued
[11:21] <sailus> pinchartl: What kind of requests would you apply from an interrupt handler?
[11:22] <sailus> If it touches the graph state in any way, you need the graph mutex --- at least for now.
[11:23] <pinchartl> it will need to affect the graph state
[11:23] <pinchartl> as links can be included in requests
[11:23] <sailus> I somehow feel a mutex is more suitable for serialising access to the graph.
[11:23] <pinchartl> we could push that to a work queue, but it will add latency
[11:24] <sailus> More fine grained locking would be beneficial, but if we want to have a single variant of the graph on which all the operations are based on, we need to serialise access to that.
[11:24] <hverkuil1> Wouldn't it also be possible to keep track of a 'request' state? I.e., you have the current applied state, and a state consisting of the current state + all pending requests. When a new request comes in, you add it to that 'request' state and validate it.
[11:25] <sailus> hverkuil1: Yes. That's how it should be done.
[11:25] <pinchartl> sailus: it's a valid point, but let's discuss it separately
[11:25] <hverkuil1> That way you don't need to keep a state object for each request, but just a single extra state for the whole device.
[11:25] <sailus> Then you can validate further requests based on the queued state.
[11:25] <sailus> :-)
[11:26] <pinchartl> hverkuil1: it's an option too, but drivers also need to apply requests, and for that having a full consistent state for each request is useful
[11:26] <pinchartl> also
[11:26] <pinchartl> when applying a request
[11:26] <pinchartl> updating the active state would just be a matter of replacing the active state pointer with the pointer to the state contained in the request
[11:26] <hverkuil1> Ah yes, that makes sense. I'm happy with extra memory overhead to keep the code as simple as possible. Complexity is our enemy here.
[11:27] <sailus> pinchartl: Good point as well.
[11:27] <sailus> The data structures in which the graph is stored would need to be changed.
[11:27] <sailus> I guess that'd be needed anyway.
[11:27] <pinchartl> sailus: I think so
[11:27] <sailus> Then we'd just have a bunch  graph state objects, one of which would be the current one.
[11:27] <sailus> s/bunch/bunch of/
[11:28] <sailus> And one of which would be the head of the queue.
[11:28] <pinchartl> the graph state (as in the links state) would just be one part of the global state, yes
[11:28] <pinchartl> how to validate requests is something I can draft and submit a proposal for, we don't have to discuss that now
[11:28] <hverkuil1> If you have a request touching multiple subdevs, is everything stored in one big data struct, or is the data split up per subdev?
[11:29] <sailus> In one data structure, I'd say.
[11:29] <pinchartl> it would be split per subdev
[11:29] <sailus> Hmm.
[11:29] <pinchartl> with one top-level data structure
[11:29] <pinchartl> containing a list of subdev states
[11:29] <sailus> Yeah...
[11:29] <hverkuil1> Right. So to ask a subdev to validate the request data, it would just have to validate it's own dataset.
[11:30] <pinchartl> yes
[11:30] <hverkuil1> I'd say that's a new core op.
[11:30] <sailus> Based on the work I've done with requests and video buffer queues so far, I'd say the driver needs to be able to easily know what exactly can be found in a request, without having to loop over all possibilities.
[11:30] <pinchartl> hverkuil1: I agree
[11:30] <pinchartl> the idea is to also allow subdev drivers to subclass the subdev state structure to store additional driver-specific information
[11:30] <pinchartl> for instance
[11:30] <sailus> Most drivers handle multiple sub-devices.
[11:30] <pinchartl> if you need to compute PLL parameters based on the content of the request
[11:30] <sailus> And video devices.
[11:31] <pinchartl> that calculation can be very expensive
[11:31] <pinchartl> so a subdev driver would do it at validation timee
[11:31] <pinchartl> and store it in the state structure
[11:31] <sailus> pinchartl: Do you think that's really meaningful example?
[11:31] <pinchartl> to be reused when the request is applied
[11:31] <sailus> I agree we could need to store something for a request that originally wasn't a part of it.
[11:31] <pinchartl> the point is to avoid computing parameters twice when they're needed both at validation time and apply time
[11:32] <hverkuil1> sailus: the data you're concerned about is all control values, I expect. Correct?
[11:32] <sailus> Agreed. I think just that the example was a bit artificial one. :-)
[11:32] <pinchartl> the example is a bit artificial, yes
[11:32] <sailus> hverkuil1: Video buffers, for instance.
[11:32] <pinchartl> I just made it up right now :-)
[11:32] <sailus> That's all I've used requests for so far.
[11:32] <sailus> I think it'd apply for controls as well.
[11:32] <sailus> Or other configurations.
[11:32] <pinchartl> what I haven't figured out yet is how to store control state in the state object
[11:32] <hverkuil1> video buffers +  associated new control values?
[11:33] <pinchartl> as currently the control state is stored in the control objects, part of the control handler
[11:33] <sailus> hverkuil1: Yes.
[11:33] <hverkuil1> Right. So we want to make sure the controls are easily obtained.
[11:34] <sailus> Especially the controls that have changed.
[11:34] <pinchartl> yes
[11:34] <pinchartl> controls are first-class citizens here
[11:34] <pinchartl> I want to make them easy to support
[11:35] <hverkuil1> I think I'd have to see the code. My initial implementation made the controls easy to obtain, but I don't know what has changed since then.
[11:36] <pinchartl> nothing, given that I don't handle controls yet :-)
[11:36] <pinchartl> but if we want to store control values in state objects, I'll have to rework that part
[11:36] <pinchartl> sailus: and I'll also make sure it's easy for drivers to know what has changed in a request
[11:37] <hverkuil1> Do we need to? I mean, my implementation stored them in the control handler data struct, which is basically a control-specific state object.
[11:37] <pinchartl> in the APIs that take a state object only, there should be flags telling what part of the state has changed
[11:37] <pinchartl> if we want to apply a state by just setting a pointer, we need to
[11:38] <pinchartl> I can't really comment on that yet as I haven't worked on that part of the code
[11:39] <sailus> Video buffers are meaningful as far as they're a part of a request. I presume changes in control values mostly work the same way.
[11:40] <sailus> E.g. there's no need to write device's register related to a control unless the control value changes.
[11:40] <sailus> The control framework does a pretty good job with that at the moment; that works well for devices behind a slow bus.
[11:41] <pinchartl> sailus: I agree, drivers need to know what has changed
[11:42] <pinchartl> then it will be up to the driver to decide how to apply the changes, either incrementally, or by reconfiguring everything
[11:44] <pinchartl> the last point I'm unsure about is how to handle interactions between the request and non-request APIs
[11:45] <hverkuil1> What is not really available at the moment w.r.t. the control request implementation is the cumulative state. I don't believe I implemented that. Unless Sakari did that?
[11:45] <pinchartl> Sakari's patch set doesn't touch the control framework as far as I know
[11:47] <sailus> I haven't done anything regarding that, but I think we can't generally support both at the same time.
[11:47] <hverkuil1> Ah, right. I thought it did :-)
[11:47] <sailus> If there are requests queued, other IOCTLs for the media device touching any dependent entity should return an error.
[11:48] <pinchartl> I think I agree
[11:48] <sailus> For the graph may not be fully connected.
[11:48] <pinchartl> get ioctls should probably be supported though
[11:48] <hverkuil1> So all the work on the request API that has been done has been ignoring control handling?
[11:48] <pinchartl> hverkuil1: yes
[11:48] <sailus> There could be multiple applications using independent portions of the media device while not all of them might use requests.
[11:49] <hverkuil1> OK. In that case it is best if I add that on top of the most up-to-date patch series.
[11:49] <pinchartl> sailus: a whole new can of worms. I think I'll ignore it for now
[11:49] <sailus> There's a but though: it may be that controls should still be allowed in some cases, as long as they don't affect request validation.
[11:49] <pinchartl> hverkuil1: I plan to work on controls after finishing the state object implementation
[11:50] <sailus> For instance, the automatic white balance / exposure / focus loop should have real-time access to controls without waiting for the requests.
[11:50] <sailus> pinchartl: I think we could do that, yes.
[11:50] <sailus> Let's not try to solve everything at one go...
[11:50] <hverkuil1> At least in my initial implementation you had to specify which controls can be used for requests.
[11:51] <pinchartl> hverkuil1: that wouldn't be enough. the controls that Sakari mentioned are certainly usable with the request API
[11:52] <pinchartl> but in the case of a userspace 3A control loop, some level of short-circuiting the request pipeline would be needed
[11:52] <pinchartl> sailus: that's a very interesting, and probably very tricky too, case
[11:53] <sailus> pinchartl: I'm not sure it'd be meaningful to use them with the request API.
[11:53] <sailus> There's a latency in sensors until they take effect, so they need to be applied well in advance.
[11:53] <pinchartl> sailus: I think it would. especially the exposure time, to implement exposure bracketing
[11:53] <hverkuil1> You'd either use them in the feedback loop, or in the request API, not both at the same time. That's asking for problems.
[11:53] <sailus> And there's a risk of missing the timing window you need to apply the register writes.
[11:53] <pinchartl> you need to be able to queue 3 video buffers with specific exposure times associated with them
[11:54] <sailus> I'd still leave it up to the user space to handle that.
[11:54] <pinchartl> hverkuil1: not both at the same time, I agree, but that's a runtime decision, not something a driver author can decide
[11:54] <sailus> It could be possible to implement that in the kernel, but we need a latency model in the kernel for those settings which is sensor specific, and a way to handle errors (missing the time window).
[11:55] <sailus> I wouldn't try doing that now.
[11:55] <pinchartl> sailus: that largely depends on the hardware
[11:55] <hverkuil1> Sure, but I also don't think this is something you would have to write code for to handle it.
[11:55] <sailus> The user space developers would be happy though. :-)
[11:56] <hverkuil1> pinchartl: I've lost track of what your actual questions are :-)
[11:58] <pinchartl> hverkuil1: how to handle interactions between the request and non-request APIs
[11:59] <pinchartl> maybe the answer is to ignore them for now until the state object implementation is ready
[12:01] <hverkuil1> I don't think mixing this should be allowed. Once you start to use the request API you should keep using it. For controls this could be slightly different:
[12:01] <hverkuil1> in my original implementation the driver specified which controls could be used in a request, since requests do not make sense for all controls.
[12:02] <pinchartl> I'd still have to implement the get part of the non-request API though. reading a link state or getting a format should reflect the current state of the device. if I implement the get functions to read the active state object when it exists this should be pretty easy
[12:02] <hverkuil1> So I think once a control that's allowed to participate in a request is actually used in a request, it should no longer be allowed to be set directly.
[12:02] <hverkuil1> At least for now.
[12:03] <hverkuil1> It's pulling the rug from under the request API principle.
[12:03] <hverkuil1> if you do that
[12:03] <sailus> hverkuil1: As long as requests are queued, right?
[12:04] <hverkuil1> yes. As long as a control isn't used in a request, then it is always fine to set it directly.
[12:05] <pinchartl> I expect that the devil will be in the details there. let's see about that at implementation time if that's fine with you
[12:06] <hverkuil1> pinchartl: I agree.
[12:10] <sailus> Sounds good to me.
[12:10] <pinchartl> thanks
[12:12] <pinchartl> I guess it's back to the code now :-)
[14:00] <mchehab> sailus: moikka
[14:04] *** arnd has quit IRC (Ping timeout: 264 seconds)
[15:26] *** kbingham has quit IRC (Ping timeout: 240 seconds)
[16:55] <neg> headless: yes sorry the weekend got in the way.. I have now looked at the tool you use and figured out why you get the wrong field order. I replied to the ML about my findings, hope I did not missunderstod how you use it.
[16:57] <headless> neg, I saw
[16:57] <headless> hi & thank you
[17:51] <headless> neg, have you seen my patch for the old driver?
[17:52] <headless> this time I did CC you :-)
[18:28] <larsc> can we get coroutines in the kernel? That would help with the whole probe ordering thing ;)
[18:35] <headless> let's rewrite the kernel in Modula-2 :-)
[18:35] <headless> or better, in Modula-3
[18:36] <larsc> Modula-3000
[18:39] <neg> headless: yes I did see it, planing to adress it and Hans patch removing the same driver tomorrow :-)
[18:42] <headless> neg, if it gfets removed in 3.8, no problem then
[18:42] <headless> *gets
[18:49] <headless> 4.8, of/c
[19:32] *** awalls1 has left 
[23:16] <gnac> my Haupage 1229 with SAA7164 based chipset recently bit the dust. I'm looking for a replacement capture card for my linux based pvr. It looks like the newer models are using a different chipset.
[23:17] <gnac> Can anyone recomend a good capture card with linux support?