| ↑back Search ←Prev date Next date→ Show only urls | (Click on time to select a line by its url) |
| Who | What | When |
|---|---|---|
| gnurou | sailus: please send the patches to the list as well so I can comment, even without the rest of the series | [02:17] |
| ................................................................... (idle for 5h32mn) | ||
| sailus | gnurou: I can do that.
However they just compile, there are definitely bugs in them. They still should give a somewhat good picture. | [07:49] |
| ....... (idle for 32mn) | ||
| mchehab: Bom dia! | [08:22] | |
| mchehab | moikka | [08:23] |
| sailus | I was thinking of asking you to handle a pull request, but I think I might not have sent it in the first place. :-P
Which might explain why the patches are not merged. How do you do? | [08:24] |
| mchehab | yeah, that's a good reason for me to not pick... you need to send it first :-)
fine, and you? | [08:26] |
| sailus | Fine, too!
I'll resend the pull request in a moment. | [08:29] |
| mchehab | s/resend/send :-p | [08:30] |
| sailus | What actually happened was that I sent one, but marked it as superseded as I found a bug in the patches.
And I resent the patches but forgot to resend a pull request. | [08:37] |
| mchehab | ah | [08:37] |
| sailus | Sent. | [08:38] |
| ....................... (idle for 1h52mn) | ||
| pinchartl | hverkuil: haven't had time to review it
mchehab: same answer | [10:30] |
| ........... (idle for 53mn) | ||
| hverkuil | kbingham: ping | [11:27] |
| kbingham[m] | Postponed pong. Just grabbing tea. Back in 5 | [11:28] |
| hverkuil | ok
Looking at: https://patchwork.linuxtv.org/patch/47076/ Just wondered what the status is. The other patches of that series look good to me and I would like to merge them. Should I take this patch as well? | [11:28] |
| mchehab | pinchartl: do you foresee that you'll be able to review it soon? otherwise, better to assume that lyakh did a good job and apply it. after all, the patch is there for more than a year! | [11:30] |
| kbingham | hverkuil: And back :)
hverkuil: Unfortunately I couldn't find anyone to test the Wheat board ... but I have managed to get renesas to set up a board with remote access in Japan ... I just haven't had time to test it myself yet. | [11:32] |
| hverkuil | OK. Should I wait merging this series? Or merge the other 4 and just wait with this patch? | [11:34] |
| kbingham | hverkuil: Actually - I think it's a no-op for you .? The ADV7511 is handled through drm ? | [11:36] |
| hverkuil | Ah, that's the drm adv7511 driver, not the v4l adv7511 driver (there are two). | [11:37] |
| kbingham | (There is also an ADV7511 in media/ which conflicts in the platform drivers .... I was pondering if we should make the config flags prevent them both being built-in at the same time ... though that won't stop modules being loaded...
hehe yes :) kbingham confesses he hasn't looked at the media/adv7511 driver as it's not used by the rcar-platforms... | [11:37] |
| hverkuil | OK, so I can take the adv7604 patches and drm deals with the other three.
We use the media adv7511 driver in our products. | [11:38] |
| kbingham | hverkuil: perfect :)
hverkuil: I guess I possibly need to apply a similar implementation into the media/adv7511 if the bindings are common... | [11:39] |
| hverkuil | It would be nice. I can test it (although that may take 2-3 weeks before I can get around to that).
kbingham: this is the only other remaining patch from you in my TODO list: https://patchwork.linuxtv.org/patch/46509/ I gather that needs more work? | [11:41] |
| kbingham | hverkuil: My replies to geertu were arguing against what he suggested.... but actually - I had thought about that patch at some point ... and I think the context might be wrong... I wonder if the reason I was able to register the notifier twice was actually just a 'fault between keyboard and chair/my fault' type issue..
I posted the patch because the failure if you double register a notifier - is really obtuse and doesn't help you pin down what actually happened very well because the linked-list is corrupt. Now usually drivers won't go double-registering notifiers :) so it's probably a bit extraneous - and if CONFIG_DEBUG_LIST would catch the fault - then perhaps it's not necessary However the other part of my comment was reporting on how I don't think CONFIG_DEBUG_LIST will actaully catch all double-adds! - only a consecutive double add :D <executive summary: I'll let you decide if the cost of walking the noitifer list each time you add a notifier is worth spending to ensure that people don't add a notifier twice, and won't object if you drop it> In fact - drop it. The right place to catch double list adds is in the CONFIG_DEBUG_LIST code. | [11:45] |
| hverkuil | OK, thanks! | [11:52] |
| .................... (idle for 1h37mn) | ||
| pinchartl | mchehab: it should be reviewed, not necessarily by me, but at least by someone
(and ideally someone who knows a bit about the driver) I will be on holidays next week, my next opportunity will be the week after that | [13:29] |
| mchehab | it waited for so long, I doubt that waiting for two weeks would be too bad...
it will be during -rc6, so we should take some care to avoid it to sleep yet another kernel release lyakh: ^ | [13:31] |
| lyakh | the first review *opportunity* isn't yet even an indication whether that opportunity will be used, and then if there more changes required, assuming I'd be able to address them within a day or two, how long it will take till the next review. This is v6, so, you can calculate the average review iteration duration... | [13:36] |
| pinchartl | lyakh: I stand my point that someone should review the patches. if that happens while I'm away next week I'll have a look when I'm back, otherwise I'll have to do the review myself
I have to go now though I'll be back on the 19th | [13:39] |
| lyakh | I'm all for reviews, I'm quite far from the idea of my patches being perfect, but such review cycles are absolutely unacceptable | [13:41] |
| sailus | lyakh: I'm not a proponent of ignoring patches posted to the list, but we're all pretty busy with what we're doing. People may just miss the patches. Feel free to ping the maintainer (as well as others) more often.
I think in general it'd help a lot if there were more developers reviewing the code, not just submitting it. :-I | [13:44] |
| lyakh | sailus: I have been doing exactly that more or less every couple of weeks all these 1.5 years long
(pinging the maintainer) it's definitely not the case of patches being missed | [13:44] |
| sailus | I remember pinchartl had an action point to check the tooling the DRM people were using to manage their tree.
I think these are not the only patches that would have deserved more reviews. :-I lyakh: These were the patches you resent yesterday? | [13:45] |
| lyakh | sailus: yes | [13:50] |
| sailus | I'll try to review them early next week. | [13:54] |
| lyakh | sailus: would be great, thanks! | [13:58] |
| ..... (idle for 20mn) | ||
| hverkuil | jmondi: ping | [14:18] |
| jmondi | hverkuil: pong | [14:18] |
| hverkuil | jmondi: I'm looking at your ecovec series.
I guess the arch/sh patch would have to go through us as well since there is no arch/sh maintainer, right? Regarding the mt9t112: | [14:19] |
| jmondi | hverkuil: that was my hope, yes | [14:19] |
| hverkuil | I would suggest that you add a follow-up patch that adds a TODO comment explaining what is missing and why support for this hasn't been added (no HW, no datasheet).
I don't feel it is fair to move it to staging just because of this. It has been in mainline for many years. | [14:22] |
| jmondi | hverkuil: I know, and I feel the only thing is missing is framerate control... | [14:23] |
| hverkuil | And the reasons why this hasn't been done are good reasons. It remains a useful driver, it's just that v4l2-compliance will complain about this missing functionality. | [14:23] |
| jmondi | hverkuil: I can follow up with a patch, describing at the driver beginning what is missing | [14:24] |
| hverkuil | So if you can post a follow-up patch adding some comment along this line to the sensor driver, then I will take this series.
I'd like to see soc-camera die :-) | [14:24] |
| jmondi | hverkuil: thanks! I'll try to do that today!
and we're just two platforms away to get rid of soc_camera users \o/ | [14:25] |
| ..... (idle for 22mn) | ||
| neg | hverkuil: quick question, if the driver needs to drop a frame due to lack of buffers shall the sequence number be increased? Or is that only incremented when a buffer is returned to userspace?
hverkuil: My interpretation of the doc is that it shall be incremented if a frame is dropped, but by doing so v4l2-compliance complans with warnings if a frame is dropped 'warn: v4l2-test-buffers.cpp(268): got sequence number 56, expected 55' so just wanted to make sure :-) | [14:47] |
| hverkuil | Ideally the sequence counter should be increased even if a frame is dropped. However, not all drivers/hardware can actually detect this so this is not something userspace can rely on in practice.
Another issue is that you really should not see dropped frames unless something is wrong. That's the reason why v4l2-compliance will complain since this is not something that should happen in the first place (unless applications can't keep up for some reason). | [14:51] |
| neg | hverkuil: Ok then maybe my definition of dropped frames are a bit of. The situation I have is that userspace don't feed the driver with new buffers fast enugh so it have to start throwing away frames. This happens for example if I try to write each frame to my NFS mounted rootfs, while if the application do nothing with the buffers it's able to keep up and no frames needs be dropped. Is this correct usage of the
term? | [14:55] |
| hverkuil | Yes. This is one reason why you can get dropped frames.
But this should not happen when using v4l2-compliance, does it? | [14:57] |
| neg | OK thanks is v4l2-compliance know to cause this issue? I get droped frames due to this for 'v4l2-compliance -d 26 -s' while 'yavata -n 4 --capture=100' I get no dropped frames | [14:58] |
| hverkuil | Is this a capture device node or output?
(-d 26) | [14:59] |
| neg | It's a capture device, it's the VIN on Koelsch with a patchset I'm preparing | [15:00] |
| hverkuil | Then this really shouldn't happen. v4l2-compliance doesn't do anything with the buffers, it just requeues them.
One possible option might be that the minimum number of buffers is too low. What does the driver require? | [15:00] |
| neg | Ahh I see my error I need to set min_buffers_needed to a greater value for v4l2-compliance to be happy while for yavta I set this on the command (-n 4)
hverkuil: Sorry for the noies and thanks for your help | [15:02] |
| hverkuil | Right. Usually it needs to be at least 3: one is DMAed, a second has to be queued up and the third is processed by userspace.
Some drivers can get away with 2 depending on the DMA engine. If memory serves v4l2-compliance requests 1 buffer, and thus relies on the driver to increase that to something that works for the hardware. | [15:03] |
| neg | Yes that is what I missed in my work, previousy the VIN could work with one but using a slow single capture mode. When reworking that to use the faster continues mode I missed to increase min_buffers_needed. I did not notice as I always specify numbers of buffers when testing with yavta and got confused when verifying with v4l2-compliance :-) | [15:05] |
| ........... (idle for 54mn) | ||
| *** | prabhakarlad has left | [15:59] |
| ............. (idle for 1h1mn) | ||
| Hedged-Handful has quit IRC (Quit: ZNC 1.7.x-git-876-42939c9 - https://znc.in) | [17:00] | |
| ........ (idle for 35mn) | ||
| hverkuil | neg: OK if I mark this as 'Obsoleted'? https://patchwork.linuxtv.org/patch/41742/
I gather there will be an updated version in the future. | [17:35] |
| .......... (idle for 47mn) | ||
| neg | hverkuil: yes no problem marking that os obsoleted | [18:22] |
| ....... (idle for 31mn) | ||
| headless | neg: os?
or as? :-) | [18:53] |
| neg | headless: :-) | [18:59] |
| .......................................... (idle for 3h26mn) | ||
| *** | pfallenop has quit IRC (Read error: Connection reset by peer) | [22:25] |
| ↑back Search ←Prev date Next date→ Show only urls | (Click on time to select a line by its url) |