<!-- 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>
mkrufky: :-)
mchehab: hi all
pinchartl: hello
sailus: <u>hverkuil</u>: Ping?
hverkuil: sorry, another meeting ran late.
<br> I'm ready now :-)
syoung: Hello
mchehab: anything to be discussed today? :-)
hverkuil: I'm still catching up from my vacation. Anything urgent I need to look at?
<br> I plan to do code reviews tomorrow/Monday.
mkrufky: brad's patch "Â lgdt3306a: Set fe ops.release to NULL if probed" ... i havent yet had time to check, but apparantly this is what other drivers do now as well?
mchehab: I'm currently stuck trying to check the issues on RPi
mkrufky: ah, i saw that long dvb-usb rpi thread
mchehab: that patch sounds wrong
<br> probably indicates a bug somewhere else - or it is a reflex of a bug that was already fixed upstream
mkrufky: thats what i thought also, at first.
<br> seems like it's due to this transitional period of multiple attach api's being used
mchehab: ah
<br> yeah, makes sense
<br> we should work to make easier for the new way and get rid of the old i2c low-level binding via dvb_attach()
hverkuil: <u>FYI</u>: ethernet on the Rpi is broken since 4.15-rc2. They know about that and the easy fix is to revert a patch. Wasted almost a day to figure that out :-(
mchehab: the real problem here is that now there are lots of duplicated code for every driver converted to the new way
<br> <u>hverkuil</u>: on 4.15.-rc7, it seems that something got wrong also with USB
<br> or with DT
<br> (I suspect the latest)
hverkuil: <u>mchehab</u>: s/ethernet/USB/
sailus: <u>mchehab</u>: On the notes from Prague meeting --- I can finish and post them to the list, if that's fine for you.
hverkuil: (no usb means no ethernet on the rpi)
mchehab: true
<br> do you have the patch that should be reverted? people should apply it upstream :-)
<br> <u>sailus</u>: that would be great!
hverkuil: <u>mchehab</u>: reverting commit 014d6da6cb2525d7f48fb08c705cb130cc7b5f4a fixes it. There are two competing 'real' fixes, but I don't know which one will go in.
mchehab: yeah, I was suspecting that 014d6da6cb2525d7f48fb08c705cb130cc7b5f4a was responsible for the breakage
<br> that's basically one of the DT differences since 4.14
<br> (the other one adds a bluetooth setting)
<br> when I installed 4.14 again, I booted, by mistake, 4.15-rc7, and USB worked
<br> (because it took the DT files from 4.14)
hverkuil: <u>sailus</u>: thanks for doing the notes, much appreciated.
mchehab: <u>mkrufky</u>: hverkuil pointed that the DVB mmap API should have a way to detect buffer losses. He actually suggested adding timestamp/buffer counter there...
sailus: <u>mchehab</u>: I just thought that someone has to do them. There's important material there esp. for those who weren't there.
mchehab: yeah, sure
<br> after sleeping it for a while - and investigating the issues with RPi3 - I'm agreeing with hverkuil, although I think we should report it on a different way for DVB
mkrufky: there is continuity counter inside the payload
mchehab: on DVB, two things can happen:
<br> - Kernel can discard a frame due to CRC;
<br> currently, this is not reported with read() API
mkrufky: no "frames" in DVB
<br> a packet?
<br> 188 bytes?
mchehab: - userspace can be too slow and ringbuffer may be discarded
<br> s/discarded/overrided
<br> this is currently reported by read() interface
hverkuil: how does read() report that?
mchehab: basically, dmxdev fills buffer->error = -EOVERFLOW;
<br> and, on the next read(), if buffer->error, instead of filling the buffer, it returns the error code there
<br> so, it is not really a continuity counter
<br> just an error "metadata"
hverkuil: Clear, thank you!
mchehab: what I'm thinking would be to add, for mmap DQBUF, one "flag" field - currently with just one flag: DVB_BUFFER_OVERFLOW
<br> and another one "crc_errors" with the number of packets discarded due to CRC errors
mkrufky: you dont think BUFFER_OVERFLOW as a generic term wont ever be used outside of DVB?
<br> er, too many double negatives, my bad
mchehab: as those are metadata, userspace can ignore if they want - or use it - for example, to help indicate that the signal has bad quality
<br> well, this will be at the DVB-specific DQBUF struct
mkrufky: is the kernel really checking the CRC on all packets??
hverkuil: Sorry to jump in, but this is more a discussion for #v4l.
mchehab: struct dmx_buffer {
<br> __u32 index;
<br> __u32 bytesused;
<br> __u32 offset;
<br> __u32 length;
<br> };
<br> <u>hverkuil</u>: actually for #linuxtv
<br> <u>mkrufky</u>: it is optional
<br> <u>hverkuil</u>: sorry for OOT
mkrufky: he's probably right
mchehab: any other things related to patch handling?
hverkuil: <u>mchehab</u>: when will the merge window close? Linus said there will be an rc8.
mchehab: * - %DMX_CHECK_CRC - only deliver sections where the CRC check succeeded;
hverkuil: I mean the window for 4.16 pull requests.
mkrufky: yes, i recall this option... i thought (and hope) it is disabled by default
mchehab: <u>hverkuil</u>: I don't plan to apply any new driver anymore for 4.16
pinchartl: <u>mchehab</u>: so if userspace is too slow, the next read() call will return an error, and userspace will then call read() again to continue reading data ? sounds like the way DVB handles slow userspace is to make it even slower, nice :-)
mchehab: so, if you have new functionality, please postpone for 4.17
<br> let's now focus on bug fixes and trivial patches
hverkuil: OK, good to know.
mchehab: <u>pinchartl</u>: if a loop like while (1) { read(fp) } will have more than one buffer overrun for a buffer that it is usually 1MB, there's something really broken on the machine - DVB won't be the main issue for the user
mkrufky: +1
<br> DVB cant be more than ~ 40 MBps per device
mchehab: actually, ~60MBps
mkrufky: maybe in the future with different modulations, but for now thats the max
mchehab: on DVB-S2 and DVB-C
mkrufky: fine, 60 MBps :-)
pinchartl: <u>mchehab</u>: of course there should be no overrun in the first place, but I found it funny that the API to handle a buffer overrun, usually caused by a too large processing delay in the application, is to force the application to issue an extra system call. there's some irony in there
<br> especially now that syscalls have suddently got more expensive with a page table switch *and* a branch predictor invalidation :(
mkrufky: i agree. if we were re-writing that API today I'd also want it to be better than this
pinchartl: s/suddently/suddenly/
mchehab: yep
pinchartl: and don't get me wrong, I'm not just blaming DVB, V4L is particularly syscall-heavy
mchehab: that's a 1997 API
pinchartl: especially in the setup phase
mchehab: (both v4l and dvb APIs actually are previous millenium APIs)
<br> DVB is very light at setup phase
pinchartl: V4L2 dates from 2002, it was merged in 2.4.0
<br> V4L1 is from 1999
<br> so only DVB is a second millenium API I'm afraid :)
mkrufky: DVB API was previously known as Convergence API before joining the kernel
<br> but it was the same API
mchehab: it is a set of improvements over a second millenium API - whose original focus were to support more TV standards
<br> anyway, the discussion here is funny, but not relevant :-)
pinchartl: yes :-)
mkrufky: history of the DVB API is quite interesting, if you know what to search for ... the missing keyword is "convergence"
pinchartl: (I was clearly not serious about evaluating APIs based on the year they were merged)
mkrufky: :-P
mchehab: <u>pinchartl</u>: you probably don't know, but, in the past, some digital TV cards were actually supported by both V4l and DVB APIs
mkrufky: <u>mchehab</u>: the brad patches that i added my reviewed-by tag are small enough for this merge window. if you want to hold off on the one (patch 2/9) than Devin had questions about, thats fine ....
mchehab: xawtv4 still has traces of that time, AFAICT
mkrufky: or if you just want to wait for my PR i am trying to force myself to have a full dev env set up this weeekend (shame on me)
<br> ah, mauro is talking about the original pcHDTV patches using those oren demods
<br> in 2.4.x !!!
pinchartl: speaking of review, hverkuil, what's your opinion of the request API patches ?
<br> <u>sailus</u>: same question, how do we move forward ?
hverkuil: <u>pinchartl</u>: as mentioned above, I plan to do the review tomorrow/Monday, so I know more once I've done that.
mchehab: <u>mkrufky</u>: yeah, feel free to pull brad's patches
<br> patch 2/9 require more tests, though
hverkuil: It's an RFC patch series and not yet using my control framework changes (itself very much RFC), so it is an early version.
sailus: I took a look some days ago.
<br> It seems that the basis for these is an old version of the earlier request API patches.
mchehab: <u>mkrufky</u>: patch 3/9 should be kept together with 2/9
<br> (maybe even merged together)
mkrufky: agreed
hverkuil: <u>pinchartl</u>: did you already post a review? We're talking about the patch series from December 15th, right?
sailus: I'm not too fond of that as the newer patches have a number of bugs fixed that are now back in the new proposal.
pinchartl: <u>hverkuil</u>: yes, that one. I haven't yet, no. I should be able to in a few days
sailus: The work also is evidently geared to support the use case at hand, with little consideration to else.
pinchartl: <u>sailus</u>: that's my concern too
<br> it's a codec API, not a request API
sailus: So that leaves me wondering whether this is the way forward, or whether it'd be better to take the older patches and work based on them.
<br> I'll go through both sets again, to be able to judge that.
mchehab: if there are some bug fixes inside it, please send it on a separate patch series to be applied before the actual request/codec API patchset
sailus: That said, there are some things in the new set that were unaddressed in the old one, too.
hverkuil: <u>pinchartl</u>: sailus: my understanding is that his patch series is primarily to review the uAPI, and not so much the internals. Since it isn't using my control fw patches yet I suspect the next patch series will likely look quite different.
mchehab: any other things for discussions?
hverkuil: not from me.
pinchartl: not from me either
mkrufky: nor I
sailus: <u>mchehab</u>: I'd have a few pull requests for 4.16. Nothing complicated.
mchehab: <u>sailus</u>: ok, will try to apply likely at the beginning next week
<br> s/beginning/beginning of the/
sailus: Obrigado!
***: ChanServ sets mode: +v mchehab
<br> ChanServ sets mode: +v snawrocki
<br> snawrocki has left "Leaving"