I may be a bit late to join today's meeting. Feel free to start already.
sailus: you're early :-)
hi all
hi all
mchehab: ping
hi all
sailus said he'll be a bit late, so we can just start.
ok
I have one imx7 smatch fix pending: https://patchwork.linuxtv.org/patch/54595/
And sailus posted ipu3 warning fixes.
hverkuil: speaking of i.MX7, do you know what the driver's status is ? is there effort to get it out of staging ?
I quickly saw ipu3 patchset...
I'm concerned about the IPU3 driver too from that regard, there's very little activity going on, it's mostly cosmetic
I have some comments to it, but didn't have any time this week to write about it
(need to do some paperwork this week - that's taking a lot of time)
pinchartl: not so much imx7 as imx in general. We need to look at that. I think non-standard control handling is really the main problem. We (or probably me) should talk to Steve about that, but haven't had time.
pinchartl: I share the same concerns... that's basically the problem with staging... some vendors tend to be happy on having stuff there and don't do much efforts on moving things out of it
Um, ipu3 is just in, give it a chance :-)
hverkuil: I may have an opportunity to work on i.MX7 but it's not sure yet
hverkuil: true, but somehow, that reminds what happened with atomisp driver
hverkuil: I've requested changes fpr the IPU3 driver for more than a month (it may be two) and nothing has moved, despite internal pressure
From what I remember, having discussed this with Sakari some time ago, the only blocking issue with imx is that the video nodes inherit controls from subdevs. While convenient, it is also against the media controller design where you access them through v4l-subdev devices.
I'm concerned that the people at Intel tasked with upstreaming of the IPU3 lack both the necessary experience, and access to developers who can spend a significant amount of time coaching them
if there is no progress by the end of the year, we'll kick it out.
it would take less time for me to fix the issues than training the Intel developers, so I won't do it
hverkuil: do you know if sailus tested the ipu3 driver after his fixes? I'm afraid that some of the changes he did actually broke it
mchehab: no idea.
(the alignment ones)
I'm still working on the test-media regression script: it's now running for every daily build. I have a bunch of patches pending to fix issues found, but I am still debugging some remaining issues.
It is really only usable if you can run it cleanly, otherwise it is hard to detect regressions.
mchehab: I don't know, but last time I discussed that with him he didn't have access to a platform for easy testing
(I could be wrong though)
Hello!
it would be good to have a tested-by on his patchsets from IPU3 developers in China
sailus: we're talking about you :-)
ah, btw, could you please update the media summit report? It would be really great if it was already published...
mchehab: good luck with that :-)
heh
mchehab: The fixes have been compile tested only, but there's no change in alignment as far as I can see.
I can ask Raj to test them.
that's the most painful period of the year...
Thanks for reminding about the summit report.
sailus: please do that... also, if I remind correctly from some discussions, there's an issue with some macros used there for alignment purposes on userspace
I got one or two gspca bug reports (fallout apparently from the vb2 conversion): I plan to test that next week and hopefully post a fix.
I'll try to check it out tomorrow.
(one of the headers is used on userspace too, and some kernel-internal macros are not visible outside kernel
I guess Alex raised this issue)
hverkuil: yes, I saw the thread
Well, the header should go through the same conversion than the rest of them, it's not just a matter of copying it.
That's a good point, if there's nothing else than __aligned vs. __attribute((aligned())), then it's worth dropping the patch for now.
then you need to patch the kernel "make headers" target
and submit to whomever maintains it
David Howells?
I don't think we'll start exporting staging headers.
no, but this needs to be solved when moving it out of staging
I would prefer to not apply a patch that would break it, as it would be one thing that we'll likely miss when moving it out of staging
bbiab
mchehab: I'm not quite sure what you're referring to. What specifically would break it?
pinchartl: how is libcamera progressing?
hverkuil: pretty well I would say
(sorry for the delay)
January was crazy
we hit the first milestone, you can capture images from uvc, ipu3 and vimc
(raw images only for ipu3)
with no controls though
I wish I had the time to look at it in more detail, but I see a lot of activity.
we're spending February cleaning up and repaying some of the technical debt
If you want me to check something specific, then ping me.
March will be focused on proper IPU3 support with multiple streams, and to add Rockship ISP support too
thanks. for now I don't think there's any specific need for review
we plan to call for a public review of the API later, around May
OK, good to know.
but of course early feedback will be considered too :-)
we'll use the request API at some point
when the IPU3 driver will implement support for it...
(which may very well mean never)
pinchartl: There is some work to be done on the framework side, it's not just about the ImgU driver.
right
sailus: I know. that's why I have little hope
I would love if Intel assigned you to that task :-)
back
sailus: I mean that, if we add something there on a header that should be included also on userspace, it should not contain kernel internal macros, as otherwise, by the time we move it to include/media, it will cause problems
and we'll very likely forget to check it
pinchartl: good news... I'll seek for some time to test it... I got one hardware with ipu3 for such tests
(a dell laptop)
mchehab: it won't work on that
as that machine has completely different ACPI tables
not compatible with the upstream IPU3 driver
the problem needs to be fixed on the kernel side
as at the moment the driver won't even probe successfully
so libcamera can't do much :-)
yeah, I'm well aware that Intel needs to help fixing the Kernel driver
(or with a firmware update, via DELL, in order to fix the ACPI tables)
mchehab: I suppose that if someone were to compile the header without converting the macros, there would be a compiler error instead of just ignoring the unknown macros.
mchehab: firmware update is likely not an option, that would break Windows
sorry about that
pinchartl: Not necessarily.
sailus: very likely yes
pinchartl: ACPI allows to test the OS and do different things depending on it
pinchartl: But I agree, a firmware update is a less likely solution and it would require making changes for each model as well as updating the BIOS.
mchehab: to some extent, in methods at least. OS-dependent tables would be a different issue
anyway, a firmware upgrade could come together with a new windows driver patch
for newer products, that sounds to be the right solution
support for existing ones would be easier if the driver could parse the current ACPI tables though
in any case, probably only Intel knows (and maybe Dell, but not likely authorized to IP about that for an OSS driver) how to properly parse the existing Dell ACPI tables
I mean:
in any case, probably only Intel knows (and maybe Dell, but not likely authorized disclose IP data about that for an OSS driver) how to properly parse the existing Dell ACPI tables
anything else for today's meeting?
There a few dvb patches of the flavour "check return value", e.g. https://patchwork.linuxtv.org/patch/53646/ This can introduce regressions, so I'm not sure what to do. So we ask for these to be tested on real hardware else reject? On the other hand it doesn't seem bad to error on write failure.
not from my side
Neither on mine.
the dvb patch should be tested, is this drxk the only one?
there were similar mxl and LG patches that i believe i nacked a few weeks / months ago
mkrufky: there a few others
in general, its good to check return values, but in specific cases, i recall there being issues on read that were safe to ignore
and i cant recall which , either mxl or lg ...  one of them would always error
maybe it wasnt actually the demod that im thinking of, it could have been an issue on the bridge.   either way, we didnt want the demod driver to error out
but, if it's tested, then its fine to merge, IMHO
ok. I can't find the other patches right now but I'll work through them
(until somebody else says it breaks something :-P )
syoung: those should be tested on real hardware
there were regressions already due to stuff like that in the past
right.
mchehab: you mentioned that Brad Love is now a "power user". There patches from him in patchwork. Will he be creating pull request for his own patches?
syoung: I *expect* so
if, instead of `goto error` it would just log a warning, that would be middle ground
but he didn't submit any PR yet
we should probably ping him
i'll ping him, whats the message?
should i invite him to these meetings?
when he would be sending us PR with hsis pending stuff?
I'm ok if he and other power users would join us here
mkrufky: a warning is not a bad idea. I'll go through the patches and suggest testing or just logging a warning
what i am saying is....
many of these guys send patches they think are correct without testing, much of it is automated.
syoung: if the code runs at probing part, or if it runs when DVB devices are in cold state, a warning doesn't seem pretty
if the only effect is to add a log, then its harmless, and definitely better coding practice rather than to simply not check the ret val
however ....  if it errors out, and wasnt tested, it could really break stuff
mkrufky: warnings are not as harmless as it seems
it could produce lots of crap at dmesg, and people may think it is an actual bug
indeed.   a single printk inside a calibration loop might turn out to be a lot more than one thinks
haha oops i woke brad up
well it's pretty early for you guys :)
i forgot its earlier for him than me
he'll get back to us later
btw, if we're adding warnings for those, it should be using pr_warn_once() & variants
in order to avoid filling it with lots of crap
that makes sense
(and, again, only after device is in warm state)
e.g. after firmware load
several drivers use some device reads in order to check if the device is in cold or warm state
anything else?
nothing from me
(nothing)
ok, let me return to my paperwork
have fun :)
:-/