#media-maint 2019-01-17,Thu

↑back Search ←Prev date Next date→ Show only urls(Click on time to select a line by its url)

WhoWhatWhen
mchehabhi all [11:56]
hverkuilhi [11:56]
sailusHello! [12:00]
hverkuilpinchartl: ping
wait a bit longer or just start?
[12:03]
mchehablet's just start
yesterday I handled the pending PR
[12:06]
hverkuilmuch appreciated! [12:06]
sailusI second to that. [12:07]
mchehabI'll likely submit tomorrow the fixes PR [12:07]
hverkuilI would appreciate it if this series can be reviewed: https://www.mail-archive.com/linux-media@vger.kernel.org/msg143158.html [12:08]
mchehabI 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]
sailusmchehab: The tag is a little bit broken, yes, but understandable.
I'd like to discuss the SoC camera removal next.
[12:12]
mchehabgah, 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]
sailusmchehab: While I agree the tag is not formatted as it should have been, is that a grave problem? [12:14]
mchehabwell, 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]
pinchartlhverkuil: pong
sorry, I4m late
[12:17]
hverkuilmchehab: sailus: can we continue with soc-camera? [12:18]
sailusOn my behalf. [12:18]
hverkuilsailus: just go ahead [12:18]
sailusI 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]
pinchartlsailus: I agree with you, I think we can remove it [12:22]
hverkuilSakari'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]
mchehabwe just got a new sensor camera driver converted from soc_camera [12:23]
hverkuilfor 3 architectures [12:23]
mchehabif we remove it, I suspect that nobody will try to keep doing the conversion
that is my main concern
[12:23]
sailushverkuil: The references in the board files can be removed independently of the framework (and drivers) themselves. [12:23]
mchehabI 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]
hverkuilWe 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]
mchehabthe day it gets removed, all pieces of it should go to the trashcan [12:24]
sailusmchehab: I suppose no-one is actively working on porting *all* SoC camera drivers. [12:25]
hverkuilIt makes no sense to keep old soc_camera drivers around if there are converted sensors. [12:25]
sailusWill we keep it forever? [12:25]
mchehabhverkuil: fully agree with that: drivers that got converted should be removed [12:25]
hverkuilI'd move unconverted soc-camera drivers to staging and put them under BROKEN. [12:25]
mchehabhverkuil: +1 [12:26]
hverkuilThere is no need for that to block soc-camera removal. [12:26]
mchehabbtw, we should work with those 4 board files that depend on soc_camera
they should be fixed somehow
[12:26]
sailusmchehab: What is somehow?
I remember patches to "fix" board files were shunned upon already almost ten years ago...
[12:27]
hverkuilI have some code here: https://git.linuxtv.org/hverkuil/media_tree.git/commit/?h=rm-soc-camera&id=d7ae2fcf6e447022f0780bb86a2b63d5c7cf4d35 [12:27]
mchehabdunno. either remove the dependency or find someone to do the port [12:27]
sailusI'd just remove it indeed. [12:28]
pinchartlhverkuil: 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]
sailusBut the board file patches will not go through the media tree, or, at least, not without approval from the maintainer. [12:28]
pinchartlI'd remove the camera support from those board files [12:28]
mchehabsailus: 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]
hverkuilIt'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]
pinchartlsend 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]
mchehabgit 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]
hverkuilThis is about how we do it, the real question is *who* will do it :-) [12:32]
mchehabmchehab is assuming that sailus will do it
mchehab runs
:-)
[12:32]
hverkuilIf sailus can't, then I can do it, but that's probably going to be February at the earliest. [12:33]
sailusI can do it if we can remove SoC camera as well. :-) [12:33]
pinchartlsailus: make it part of your soc-camera removal series :-) [12:34]
sailusAs my aim is to reduce extra work that is needed to keep them compiling and sort of looking being functional. [12:34]
hverkuilmchehab: 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]
mchehabhverkuil: 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]
hverkuilOK, 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]
hverkuilThe ov9640.h header was the only culprit. [12:39]
mchehabok
yeah, let's just remove the old one (and other converted drivers - if any) and that's it
[12:39]
sailusI'm also fine with moving the Soc camera to staging and depending on BROKEN. [12:40]
hverkuilI'll prepare patches that remove old converted soc_camera sensor drivers. [12:40]
sailusDoes that work work everyone?
(Or removing it altogether, as the current patch does?)
[12:40]
hverkuilThey 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]
sailusI can do it if they can depend on BROKEN as well. [12:41]
hverkuilYes, they should depend on BROKEN. [12:42]
sailusmchehab: Does that work for you? [12:42]
hverkuilI'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]
mchehabduplicated 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]
hverkuilov9460 [12:44]
sailushverkuil: 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]
sailusAgreed then.
\o/
[12:45]
hverkuilOK, good.
Anything else soc-camera related?
[12:46]
sailushverkuil: 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]
hverkuilsailus: yes, I'll make a patch after this meeting to remove it. [12:47]
sailusOk.
I can send patches for the rest then.
I think we can merge that separately.
[12:47]
hverkuilThe 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]
mchehabthe 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]
hverkuilNote 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]
sailushverkuil: Ack. [12:50]
mchehabhmm... 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]
hverkuilI have two other topics, if we are done with soc_camera? [12:52]
sailusI guess we are. [12:52]
mchehabyep [12:52]
hverkuilThe 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]
mchehabov9640 soc_camera patch sent to ML
sounds good
[12:57]
hverkuilThe 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]
mchehabOne thing I didn't like there is: "This callback is mandatory for output queues that support requests" [13:00]
hverkuilWhat's your problem with that? [13:00]
mchehabthis basically means that, if we ever convert an output driver to support request, people should remember
or it will cause problems
[13:01]
hverkuilIf it is not set, then the core will generate a warning and return an error. [13:02]
mchehabok [13:02]
hverkuilIn other words, this is checked. [13:02]
mchehabgood
Iĺl try to review it next week
[13:02]
hverkuilIt's done in the last patch 5/5 [13:02]
mchehabok, I didn't look at the code yet [13:03]
hverkuilI don't think I have anything else. [13:03]
sailusSame here... [13:04]
hverkuilOh, I'll be at fosdem. [13:04]
mchehabneither mkrufky nor syoung seems to be here... [13:04]
hverkuilI got the confirmation that my presentation was accepted only today, very late. [13:04]
mchehabI 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]
hverkuilAnything else? [13:07]
mchehabnone from my side [13:08]
............................. (idle for 2h23mn)
***ChanServ sets mode: +v mchehab` [15:31]
................................................. (idle for 4h3mn)
syoungmchehab: sorry I missed the meeting. What is the dvb problem you found? [19:34]
mchehabI'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]
syounghmm
ok, I'll try to set up a reproduction
[19:41]
mchehabplease 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]
syoungI'm not sure I'll have time until this weekend either
but I'd like to look at it
[19:50]
.... (idle for 16mn)
mchehabok, thank you! [20:06]

↑back Search ←Prev date Next date→ Show only urls(Click on time to select a line by its url)