↑back Search ←Prev date Next date→ Show only urls | (Click on time to select a line by its url) |
Who | What | When |
---|---|---|
mchehab | hi all | [11:56] |
hverkuil | hi | [11:56] |
sailus | Hello! | [12:00] |
hverkuil | pinchartl: ping
wait a bit longer or just start? | [12:03] |
mchehab | let's just start
yesterday I handled the pending PR | [12:06] |
hverkuil | much appreciated! | [12:06] |
sailus | I second to that. | [12:07] |
mchehab | I'll likely submit tomorrow the fixes PR | [12:07] |
hverkuil | I would appreciate it if this series can be reviewed: https://www.mail-archive.com/linux-media@vger.kernel.org/msg143158.html | [12:08] |
mchehab | I received an e-mail from linux-next maintainter with this warning:
Commit 961304d17a61 ("media: dt-bindings: media: i2c: Fix i2c address for OV5645 camera sensor") has a malformed Fixes tag. didn't check yet the email came with this comment: [I am experimenting with checking the Fixes tags in commits in linux-next. Please let me know if you think I am being too strict.] so, likely it is some new bot he added | [12:09] |
sailus | mchehab: The tag is a little bit broken, yes, but understandable.
I'd like to discuss the SoC camera removal next. | [12:12] |
mchehab | gah, yes it is broken
Fixes: 09c716af36e6 [media] media: i2c/ov5645: add the device tree binding document that's not good, as it probably affects -stable workflow | [12:13] |
sailus | mchehab: While I agree the tag is not formatted as it should have been, is that a grave problem? | [12:14] |
mchehab | well, it probably breaks any parsing script that would try to get it
btw, here I have an alias for git fixes: [pretty] fixes = Fixes: %h (\"%s\") [alias] fixes = show --pretty=fixes --no-patch $ git fixes Fixes: eed2235876ef ("media: imx.rst: Update doc to reflect fixes to interlaced capture") | [12:15] |
pinchartl | hverkuil: pong
sorry, I4m late | [12:17] |
hverkuil | mchehab: sailus: can we continue with soc-camera? | [12:18] |
sailus | On my behalf. | [12:18] |
hverkuil | sailus: just go ahead | [12:18] |
sailus | I wrote a patchset to remove the SoC camera framework, but the discussion stalled without reaching a conclusion.
This one in particular: https://lore.kernel.org/linux-media/20181113224100.hs6xtblwrd74cfmg@valkosipuli.retiisi.org.uk/ I'd like to remove it, to avoid spending time on kicking dead code around. | [12:19] |
pinchartl | sailus: I agree with you, I think we can remove it | [12:22] |
hverkuil | Sakari's proposal is to remove soc-camera, but keep the headers, convert board files (all broken, soc_camera support can be ripped out), then when that's merged kill the remaining headers.
There are 4 board files that use soc_camera. | [12:22] |
mchehab | we just got a new sensor camera driver converted from soc_camera | [12:23] |
hverkuil | for 3 architectures | [12:23] |
mchehab | if we remove it, I suspect that nobody will try to keep doing the conversion
that is my main concern | [12:23] |
sailus | hverkuil: The references in the board files can be removed independently of the framework (and drivers) themselves. | [12:23] |
mchehab | I would be ok to make it depend on BROKEN and/or move to staging
(or both) anyway, I'm against removing it while keeping the headers | [12:23] |
hverkuil | We have a few duplicate sensor drivers ('normal' + soc_camera) and I propose that we kill the soc_camera versions. That's independent of the soc_camera removal. | [12:24] |
mchehab | the day it gets removed, all pieces of it should go to the trashcan | [12:24] |
sailus | mchehab: I suppose no-one is actively working on porting *all* SoC camera drivers. | [12:25] |
hverkuil | It makes no sense to keep old soc_camera drivers around if there are converted sensors. | [12:25] |
sailus | Will we keep it forever? | [12:25] |
mchehab | hverkuil: fully agree with that: drivers that got converted should be removed | [12:25] |
hverkuil | I'd move unconverted soc-camera drivers to staging and put them under BROKEN. | [12:25] |
mchehab | hverkuil: +1 | [12:26] |
hverkuil | There is no need for that to block soc-camera removal. | [12:26] |
mchehab | btw, we should work with those 4 board files that depend on soc_camera
they should be fixed somehow | [12:26] |
sailus | mchehab: What is somehow?
I remember patches to "fix" board files were shunned upon already almost ten years ago... | [12:27] |
hverkuil | I have some code here: https://git.linuxtv.org/hverkuil/media_tree.git/commit/?h=rm-soc-camera&id=d7ae2fcf6e447022f0780bb86a2b63d5c7cf4d35 | [12:27] |
mchehab | dunno. either remove the dependency or find someone to do the port | [12:27] |
sailus | I'd just remove it indeed. | [12:28] |
pinchartl | hverkuil: some old soc_camera sensor drivers could implement features not available in the non-soc_camera ones, but that will be available in the git history, so it makes sense to remove them | [12:28] |
sailus | But the board file patches will not go through the media tree, or, at least, not without approval from the maintainer. | [12:28] |
pinchartl | I'd remove the camera support from those board files | [12:28] |
mchehab | sailus: it is on arch/* so, the right procedure is to advice that people that should either fix or it will be removed... specially since we don't usually touch stuff there directly | [12:29] |
hverkuil | It's what I did in my patch, but it's untested. The hard part is to compile test these patches and figure out who should take them.
The question is who will pick this up. | [12:29] |
pinchartl | send the patches to the appropriate maintainers, and mention that you want to merge them through the media tree, asking for an ack
ack -> we're good nack -> we talk silence -> we move forward | [12:30] |
mchehab | git blame arch/arm/mach-omap1/board-ams-delta.c
could help ./scripts/get_maintainer.pl -f arch/arm/mach-omap1/board-ams-delta.c Aaro Koskinen <aaro.koskinen@iki.fi> (maintainer:OMAP1 SUPPORT) Tony Lindgren <tony@atomide.com> (maintainer:OMAP1 SUPPORT) Russell King <linux@armlinux.org.uk> (odd fixer:ARM PORT) linux-omap@vger.kernel.org (open list:OMAP1 SUPPORT) linux-arm-kernel@lists.infradead.org (moderated list:ARM SUB-ARCHITECTURES) linux-kernel@vger.kernel.org (open list) seem to work as well | [12:30] |
hverkuil | This is about how we do it, the real question is *who* will do it :-) | [12:32] |
mchehab | mchehab is assuming that sailus will do it
mchehab runs :-) | [12:32] |
hverkuil | If sailus can't, then I can do it, but that's probably going to be February at the earliest. | [12:33] |
sailus | I can do it if we can remove SoC camera as well. :-) | [12:33] |
pinchartl | sailus: make it part of your soc-camera removal series :-) | [12:34] |
sailus | As my aim is to reduce extra work that is needed to keep them compiling and sort of looking being functional. | [12:34] |
hverkuil | mchehab: regarding https://patchwork.linuxtv.org/patch/53962/ (related to this topic):
this will fix the daily build. Alternatively, I can make a patch that removes the soc_ov9640. If you prefer the removal, let me know and I'll post a new patch. | [12:35] |
mchehab | hverkuil: I guess we can just remove the old stuff
at the moment a conversion like that is finished, we can trash the old one | [12:36] |
hverkuil | OK, I'll post a new patch. I hope it can be merged soon since the daily build is now broken (there are now two ov9640.h headers) | [12:36] |
mchehab | (except, of course, if we depend on some -arch changes)
one trivial change to avoid similar work would be to prepend "soc_" on all soc_camera *.h still it is probably overkill | [12:37] |
hverkuil | The ov9640.h header was the only culprit. | [12:39] |
mchehab | ok
yeah, let's just remove the old one (and other converted drivers - if any) and that's it | [12:39] |
sailus | I'm also fine with moving the Soc camera to staging and depending on BROKEN. | [12:40] |
hverkuil | I'll prepare patches that remove old converted soc_camera sensor drivers. | [12:40] |
sailus | Does that work work everyone?
(Or removing it altogether, as the current patch does?) | [12:40] |
hverkuil | They should be removed if they are converted (I'll do that), or moved to staging if they are not (yet) converted.
If you can do that? | [12:41] |
sailus | I can do it if they can depend on BROKEN as well. | [12:41] |
hverkuil | Yes, they should depend on BROKEN. | [12:42] |
sailus | mchehab: Does that work for you? | [12:42] |
hverkuil | I'll also make patches to remove the old sh_mobile_ceu_camera.c and soc_camera_platform.c (soc_camera platform drivers).
sailus: and if you can look at the board files? | [12:42] |
mchehab | duplicated ones:
drivers/media/i2c/soc_camera/soc_mt9t112.c drivers/media/i2c/mt9t112.c drivers/media/i2c/soc_camera/soc_ov772x.c drivers/media/i2c/ov772x.c drivers/media/i2c/tw9910.c drivers/media/i2c/soc_camera/soc_tw9910.c | [12:43] |
hverkuil | ov9460 | [12:44] |
sailus | hverkuil: That was my intention, after getting approval for the original SoC camera removal series.
So yes. | [12:44] |
mchehab | (besides ov9460)
sailus: yes that works for me | [12:44] |
sailus | Agreed then.
\o/ | [12:45] |
hverkuil | OK, good.
Anything else soc-camera related? | [12:46] |
sailus | hverkuil: How about the patch that fixes the build error? I guess removing the driver altogether (as it's a duplicate) is the way to go then?
The problem is in the master branch, isn't it? | [12:46] |
hverkuil | sailus: yes, I'll make a patch after this meeting to remove it. | [12:47] |
sailus | Ok.
I can send patches for the rest then. I think we can merge that separately. | [12:47] |
hverkuil | The problem is that there are now two ov9640.h header with different contents. The media_build compat code copies all such headers to the same directory, so one of the two will be overwritten and the daily build fails for all compat builds.
Yeah, it needs to be merged quickly. | [12:48] |
mchehab | the ones that will be moved to staging are those:
drivers/media/i2c/soc_camera/soc_mt9m001.c drivers/media/i2c/soc_camera/soc_mt9v022.c drivers/media/i2c/soc_camera/soc_ov5642.c drivers/media/i2c/soc_camera/soc_ov9740.c drivers/media/i2c/soc_camera/soc_rj54n1cb0c.c (if I didn't make any mistake on this quick look) hverkuil: I'll take care of ov9640 right now just after the meeting | [12:48] |
hverkuil | Note that there is one such driver (mt9t031) in staging already, but it doesn't depend on BROKEN. I think it should be changed to depend on BROKEN.
mchehab: OK, thanks! | [12:50] |
sailus | hverkuil: Ack. | [12:50] |
mchehab | hmm... it is just the header file that it is duplicated
no the driver is still there anyway, I'll handle hverkuil: yes let's make mt9t031 depends on broken | [12:51] |
hverkuil | I have two other topics, if we are done with soc_camera? | [12:52] |
sailus | I guess we are. | [12:52] |
mchehab | yep | [12:52] |
hverkuil | The first is that epoll() doesn't work (and never did) with v4l2: see https://lkml.org/lkml/2018/12/30/24
I confirmed this, and added a test to v4l2-compliance as well (currently disabled since it fails :-) ) I'm working on patches, but it is a bit tricky. select() works fine, it's epoll that's the problem. Basically, poll_wait() should always be called in the poll function, regardless of whether there is data available or not. Anyway, it's just a heads up. The other topic is regression testing. I have a script that I plan to add to v4l-utils/contrib/test soon that does extensive vivid/vim2m/vimc regression testing. | [12:53] |
mchehab | ov9640 soc_camera patch sent to ML
sounds good | [12:57] |
hverkuil | The main blocker that causes the regression test to fail is this patch series: https://www.mail-archive.com/linux-media@vger.kernel.org/msg143158.html
(I mentioned it at the start of the meeting) I would appreciate a review. vicodec will be added to the regression tests once it is sufficiently stable and spec-compliant. | [12:58] |
mchehab | One thing I didn't like there is: "This callback is mandatory for output queues that support requests" | [13:00] |
hverkuil | What's your problem with that? | [13:00] |
mchehab | this basically means that, if we ever convert an output driver to support request, people should remember
or it will cause problems | [13:01] |
hverkuil | If it is not set, then the core will generate a warning and return an error. | [13:02] |
mchehab | ok | [13:02] |
hverkuil | In other words, this is checked. | [13:02] |
mchehab | good
Iĺl try to review it next week | [13:02] |
hverkuil | It's done in the last patch 5/5 | [13:02] |
mchehab | ok, I didn't look at the code yet | [13:03] |
hverkuil | I don't think I have anything else. | [13:03] |
sailus | Same here... | [13:04] |
hverkuil | Oh, I'll be at fosdem. | [13:04] |
mchehab | neither mkrufky nor syoung seems to be here... | [13:04] |
hverkuil | I got the confirmation that my presentation was accepted only today, very late. | [13:04] |
mchehab | I discovered a serious issue at the DVB core
tricky to fix (there is a function being called recursively there) at the error handling path | [13:04] |
hverkuil | Anything else? | [13:07] |
mchehab | none from my side | [13:08] |
............................. (idle for 2h23mn) | ||
*** | ChanServ sets mode: +v mchehab` | [15:31] |
................................................. (idle for 4h3mn) | ||
syoung | mchehab: sorry I missed the meeting. What is the dvb problem you found? | [19:34] |
mchehab | I'll email a patch
sent - it is actually part of a thread that started at the ML from a random user complaining about an issue i suspect that the issue is actually due to some bad hardware but it trigged something that looked really bad on my eyes syoung: I didn't test the patch I sent for the condition there to be trigged with non-broken hardware, we likely need to add some hack at dvb_demux to force it to return an error the first time someone tries to set up a DVB filter | [19:34] |
syoung | hmm
ok, I'll try to set up a reproduction | [19:41] |
mchehab | please also check if my approach makes sense
I wrote a big comment, as the code there is tricky but didn't have time to check myself, as I have another stuff I was needing to finish today | [19:43] |
syoung | I'm not sure I'll have time until this weekend either
but I'd like to look at it | [19:50] |
.... (idle for 16mn) | ||
mchehab | ok, thank you! | [20:06] |
↑back Search ←Prev date Next date→ Show only urls | (Click on time to select a line by its url) |