<!-- 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>
hverkuil: <u>mchehab</u>: thank you!
mchehab: anytime
<br> <u>sailus</u>: the fwnode patches broke (even more) the media_build tree:
<br> /devel/v4l/media_build/v4l/tvp5150.c: In function 'tvp5150_parse_dt':
<br> /devel/v4l/media_build/v4l/tvp5150.c:1377:35: error: implicit declaration of function 'of_fwnode_handle' [-Werror=implicit-function-declaration]
<br> ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &bus_cfg);
<br> ^~~~~~~~~~~~~~~~
hverkuil: <u>mchehab</u>: I have no idea yet how to fix the compat build.
<br> haven't had time either.
rellla: <u>hverkuil</u>: i'm following the discussion on the ml. isn't https://git.linuxtv.org/pinchartl/media.git/log/?h=drm/du/vsp1-kms/request somewhat related? though, i did not look into the code more deeply...
hverkuil: <u>rellla</u>: yes, it is related, but that doesn't have support for per-frame-controls, which is what is needed for codecs.
mchehab: <u>hverkuil</u>: yes, this one can be tricky to fix
<br> one alternative would be to:
<br> #define of_fwnode_handle(ep) ep
<br> and add some backward-compatible code for v4l2_fwnode_endpoint_parse()
<br> & friends
hverkuil: <u>mchehab</u>: another approach that I had in mind was to just disable those drivers that depend on it. They are mostly platform drivers and not really the focus of media_build.
<br> I tried this, but it didn't work.
<br> I think fwnode is more strict: several drivers use #ifdef CONFIG_OF, and if that's not defined they still compiled fine.
<br> I think now those drivers select FWNODE, so it is always compiled in.
<br> <u>sailus</u>: is that correct? If so, then I'm not sure we want that.
mchehab: tvp5150 is not a platform driver only
<br> it is used by em28xx
<br> hmm...
<br> config VIDEO_TVP5150
<br> tristate "Texas Instruments TVP5150 video decoder"
<br> depends on VIDEO_V4L2 && I2C
<br> select V4L2_FWNODE
<br> if V4L2_FWNODE depends on OF, we have a problem
<br> yes, it does
<br> or maybe not
<br> no, it seems that fwnode_* don't depend on anything
<br> yeah, from a quick look, v4l2-fwnode relies only on drivers/base/property.c
<br> with doesn't depend on OF
<br> I guess a trivial fix for non-platform drivers would be to do:
<br> #define v4l2_fwnode_endpoint_parse(a,b)
<br> actually:
<br> #define v4l2_fwnode_endpoint_parse(a,b) 0
<br> that will generate some warnings, but won't prevent building
<br> <u>hverkuil</u>: ^
hverkuil: I can try that.
sailus: <u>mchehab</u>: of_fwnode_handle() is defined in include/linux/of.h .
<br> Please don't add local definitions for it.
<br> I can send a patch for that.
<br> <u>mchehab</u>: V4L2_FWNODE does not depend on OF.
<br> Meeting. Back after ½ an hour.
<br> Back.
<br> <u>mchehab</u>: Do you have .config to reproduce the compilation error?
mchehab: sailus, no, but .config doesn't matter much on media_build
<br> I'm building on Kernel 4.11
sailus: Hmm. On v4.11?
mchehab: $ grep CONFIG_OF /boot/config-4.11.3-200.fc25.x86_64
<br> # CONFIG_OF is not set
<br> yes
<br> media_build is the backport tree
sailus: Right. I have to admit I've never used it.
<br> I think taking some of the patches merged lately will be needed to make things work there.
mchehab: basically, it has a series of logic that handles backporting
sailus: If you have drivers using V4L2 fwnode, that is.
mchehab: we need to add something there that will make it build with Kernels < 4.12 and without OF
sailus: Ok.
<br> In that case adding local macro definitions for these functions would probably be a workable approach.
hverkuil: <u>sailus</u>: so we don't need drivers that require CONFIG_OF, but some are optional (e.g. adv7604) and can be used without OF. Those should still compile.
mchehab: the main issues are on I2C drivers that are used by non-platform drivers
<br> btw, tvp5150 should not depend on CONFIG_OF
<br> as it is used on x86
<br> together with em28xx
sailus: Tvp5150 doesn't depend on OF.
<br> How would it?
<br> It uses OF if enabled though.
mchehab: if of_fwnode_handle() works without OF, that is fine
<br> otherwise, we'll need to add some logic upstream for this to not break:
<br> ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &bus_cfg);
<br> tvp5150 doesn't need OF, if one is using it with em28xx driver
<br> with passes stuff to it via platform_data
sailus: You get NULL if CONFIG_OF is disabled.
<br> There are nop implementations for much of the OF interface.
mchehab: if (IS_ENABLED(CONFIG_OF) && np) {
<br> res = tvp5150_parse_dt(core, np);
<br> if (res) {
<br> dev_err(sd->dev, "DT parsing error: %d\n", res);
<br> return res;
<br> }
<br> } else {
<br> /* Default to BT.656 embedded sync */
<br> core->mbus_type = V4L2_MBUS_BT656;
<br> }
<br> tvp5150 should handle it fine if !OF
<br> as it will never call v4l2_fwnode_endpoint_parse()
<br> all we need to ensure is that it will build without OF (at upstream)
<br> at media_build, if we define v4l2_fwnode_endpoint_parse to do nothing, it will also build
<br> #define v4l2_fwnode_endpoint_parse(a,b) 0
<br> should do the trick
<br> I guess that, on drivers that work if !OF, the above is enough for them to build
sailus: Ack. Sounds good to me.
javier__: mchehab, sailus shouldn't instead V4L2_FWNODE depends on OF || ACPI and have stub functions if !V4L2_FWNODE ?
<br> I'm not following why CONFIG_OF being enabled or not should affect a driver if fwnode is a new level of indirection (IIUC)
mchehab: <u>javier__</u>: V4L2_FWNODE is selected... so it can't have dependencies
javier__: hmm, I see..
mchehab: the header file and c code, however, could eventually need to have tests for OF || ACPI
mort: hey, I have one buffer of Y, one buffer of U, and one of V. I need to convert that to NV12 for a hardware encoder. The brightness part is easy, just memcpy the Y buffer into the NV12 buffer, but I'm having trouble with the colors
javier__: <u>mchehab</u>: and why drivers are selecting instead of being a user visible Kconfig symbol and doing depens on V4L2_FWNODE || COMPILE_TEST ?
mort: https://p.iotek.org/g58 I've tried various variations of that, where ylen is the length of the y buffer, bufsize is the length of the `to` buffer, and `to` is the destination
<br> does anyone know anything about this, or know where I could find some resources about it online?
sailus: <u>javier__</u>: It could, yes.
<br> It requires either to work.
mchehab: hmm... it is not that simple... on 4.11, I 'm getting this:
<br> ./include/linux/of.h:162:0: warning: "of_fwnode_handle" redefined
<br> #define of_fwnode_handle(node) (&(node)->fwnode)
sailus: <u>mchehab</u>: Yes, that's the older definition.
mchehab: it seems that you forgot to add #include <of.h> at tvp5150
sailus: It can be tested with #ifdef because it's not a function.
mchehab: that's probably why it is not building
<br> <u>sailus</u>: the compat.h header is include before of.h
sailus: Oh, ok.
<br> If you want to support kernel versions where of_fwnode_handle() isn't defined something else may be needed.
mchehab: ok, fixed
arnd: sailus, mchehab: I just ran into another problem with "[media] v4l: Remove V4L2 OF framework in favour of V4L2 fwnode framework" conflicting with 4a5f8ae50b66 ("[media] davinci: vpif_capture: get subdevs from DT when available")
<br> I'll also let khilman know about it, and can send him over here
javier__: <u>arnd</u>: Hans already pointed out to kevin https://www.spinics.net/lists/arm-kernel/msg586480.html
arnd: ok, thanks
javier__: yw
mchehab: <u>arnd</u>: not sure how I didn't get it
<br> perhaps it is not building it with COMPILE_TEST
<br> on x86
<br> <u>sailus</u>: please send a fix ASAP
arnd: <u>mchehab</u>: good question, I also didn't get it on an ARM allmodconfig build, only on a randconfig build for mach-davinci
mchehab: I double-checked here:
<br> $ grep VIDEO_DAVINCI_VPIF_CAPTURE .config
<br> CONFIG_VIDEO_DAVINCI_VPIF_CAPTURE=y
<br> with should be enough to get it, in thesis
arnd: obj-$(CONFIG_ARCH_DAVINCI) += davinci/
<br> I think this should be made "obj-y"
<br> so the Kconfig symbol can be enabled, but the directory is never entered on most configurations
<br> the same problem appears to exist for drivers/media/platform/blackfin, possibly others
mchehab: it seems those are the only two cases
hverkuil: <u>mchehab</u>: it would be much better if we make proper vendor directories under media/platform.
mchehab: the other vars are V4L2 specific
hverkuil: We do for some, but not all.
mchehab: we could do that, but not sure how this will help
hverkuil: it won't help with this, but it keeps the directory layout cleaner. And each per-vendor directory can have its own Kconfig instead of keeping almost everything in platform/Kconfig.
<br> Just an idea for a rainy afternoon...
sailus: <u>mchehab</u>: Kevin's patches to add OF support haven't been merged yet, have they?
<br> I've tested this with allyesconfig both on x86 and ARM.
hverkuil: <u>sailus</u>: yes, they were merged but for some reason apparently never actually compiled.
mchehab: <u>hverkuil</u>: i actually prefer to keep most stuff at platform/Kconfig :-) It is a single place to check for drivers that don't use COMPILE_TEST :-p
sailus: Ah, I see them now. They were merged after the fwnode set.
<br> I can send a fix.
mchehab: please do it ASAP
sailus: ETA ~ ½ an hour.
mchehab: ok, thanks!
<br> <u>arnd</u>: btw, in the specific case of blackfin, it doesn't matter changing to obj-y, as blackfin doesn't compile with COMPILE_TEST
arnd: right
mchehab: IMHO, long term, we should move all those crap to staging :-p
<br> anyway, I'll submit a patch changing both to obj-y
<br> at least part of Davinci will now build on x86
<br> (after getting the fix from sailus)
***: prabhakarlad has left
sailus: <u>mchehab</u>: Sent.
mchehab: thanks
<br> I'm doing a full build before pushing it, to be sure that davinci COMPILE_TEST drivers won't fail
hverkuil: After updating my arm-davinci config file so everything is built-in instead of a module it now fails to compile as well.
<br> Stupid bug.
mchehab: <u>hverkuil</u>: just pushed the minimal fixes for fwmode on media_build
<br> with that, tvp5150 and v4l2-fwnode builds
<br> I guess it shouldn't be too hard to add the missing bits for OF support, if someone need to build platform drivers with media_build
<br> the functions it uses are at drivers/base/property.c
<br> and aren't complex
<br> ok, build breakage fixed
<br> make install (on media_build) also fixed
sailus: Nice!
-: headless tries to install sphinx and is perplexed...
headless: so many programs called sphinx... :-)
<br> and all of them wrong
mchehab: on some distros, it is called python*-sphinx
<br> or something like that
<br> that's on fedora:
<br> python2-sphinx.noarch : Python documentation generator
<br> python3-sphinx.noarch : Python documentation generator
headless: <u>mchehab</u>: yeah, found it finally -- python3-sphinx
sailus: <u>headless</u>: On Ubuntu (I presume Debian as well):
<br> $ dpkg --get-selections|grep sphinx
<br> libjs-sphinxdoc install
<br> python-sphinx install
<br> python-sphinx-rtd-theme install
<br> sphinx-common install
<br> sphinx-doc install
<br> sphinx-rtd-theme-common install
mchehab: yeah, you need the rtd-theme in order to get a decent output
<br> (not mandatory, though)
<br> bbiab
headless: mchehab, sailus: thanx!
<br> Fedora 24 here, never had spo broken update before
<br> *so
<br> X hangs, kmail crashes
mchehab: hun
<br> huh
-: mchehab uses F25
mchehab: no issues
<br> it is running smoothly on my machines
<br> I didn't have troubles with F24 neither
<br> although I had to do some updates on one machine, due to some GPU bits that were present only on newer Kernels/newer X drivers
sailus: <u>mchehab</u>: There are a few sensor and lens drivers submitted by other Intel folks. Some are drivers for Intel hardware whereas others are sensor etc. drivers. Would you prefer me to pick up the drivers and send you a pull request, or handle them yourself?
mchehab: feel free to pick them and send a pull request
sailus: <u>mchehab</u>: Thanks. I'll do that.
mchehab: that makes easier for me to know that you already looked ont hem and they're fine
<br> <u>hverkuil</u>: your CEC makefile changes seem to be causing breakage with adv7604 with some randconfigs
<br> CONFIG_VIDEO_ADV7604=y
<br> # CONFIG_VIDEO_ADV7604_CEC is not set
<br> CONFIG_COMPILE_TEST=y
<br> it seems to be some already existing bug
<br> I'm fixing it
<br> hmm... it is not trival to fix
<br> due to cec-edid
<br> no idea how to solve the issues at adv76xx_set_edid() without cec-edid
<br> with is dependent of CONFIG_CEC
hverkuil: let me take a look
mchehab: a trivial fix would be:
<br> tristate "Analog Devices ADV7604 decoder"
<br> - depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
<br> + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && CEC
<br> but I guess that's not what you want
hverkuil: is this from a kbuild test robot report?
<br> If CEC_CORE is not defined, then static inlines from media/cec.h should take over.
<br> I see what is going on and I think I have a fix.
<br> <u>mchehab</u>: posted patch to fix this.
<br> Hmm, that fix needs to go to 4.12 too.
mchehab: ok, I'll add a c/c stable on it
<br> actually, apply on fixes branch