| ↑back Search ←Prev date Next date→ Show only urls | (Click on time to select a line by its url) |
| Who | What | When |
|---|---|---|
| *** | ChanServ sets mode: +v mchehab | [00:29] |
| .......................................................................................................................................... (idle for 11h28mn) | ||
| hverkuil | good morning/afternoon! | [11:57] |
| sailus | Hello! | [11:58] |
| hverkuil | mchehab: ping | [12:02] |
| pinchartl | hello | [12:03] |
| sailus | mchehab: Bom dia! | [12:06] |
| hverkuil | hmm... | [12:09] |
| sailus | Before mchehab is here, I'd like to notify I wrote a small patch to address videobuf2 build failures.
It's here: https://patchwork.linuxtv.org/patch/47350/ Feel free to review. I'll repost that for a wider audience as it involves files outside the media tree. Once at least someone else is happy with it, that is. :-) | [12:09] |
| pinchartl | sailus: sounds complicated | [12:10] |
| sailus | Does it? | [12:10] |
| pinchartl | I don't like having to select VIDEOBUF2_V4L2 explicitly | [12:11] |
| sailus | Reposting the patch, or the patch itself? | [12:11] |
| pinchartl | can't you make it so that it will be selected if VIDEO_V4L2 ? | [12:11] |
| sailus | pinchartl: Yes, I could.
But it'd involve building videobuf2-v4l2 even if it's not needed in case V4L2 is enabled and videobuf2 is built. But no V4L2 driver uses videobuf2. That's entirely possible. Phone call. | [12:11] |
| pinchartl | possible but very unlikely | [12:14] |
| sailus | Back.
Would you prefer that instead? | [12:16] |
| hverkuil | you could select it if (VIDEO_V4L2 && (VIDEOBUF2_VMALLOC || VIDEOBUF2_DMA_CONTIG || VIDEOBUF2_DMA_SG) | [12:16] |
| sailus | I don't object to it, but opted for the current implementation to avoid building videobuf2-v4l2 if it's not needed. | [12:16] |
| hverkuil | In theory though even that can fail if you have DVB-only devices using vb2 and otherwise only radio devices. | [12:17] |
| sailus | hverkuil: Yes, but it will mean videobuf2-v4l2 may be built even if it's not needed. | [12:17] |
| pinchartl | hverkuil: or just if (VIDEO_V4L2 && VIDEOBUF2_CORE) ? | [12:17] |
| sailus | pinchartl: That's shorter, yes. | [12:17] |
| hverkuil | ah, yes. | [12:17] |
| pinchartl | and yes it will build it unnecessarily if we only have radio devices on the V4L2 side, but I don't think that's a big issue | [12:17] |
| sailus | Or other drivers not using videobuf2. But all drivers dealing with buffers should use videobuf2 OTOH.
I'll post v2 with that change. The patch will be much smaller. | [12:18] |
| hverkuil | sailus: should yes, but that won't happen. | [12:19] |
| sailus | hverkuil: This gives one more motive to do that. :-) | [12:19] |
| hverkuil | I'm OK with this change. | [12:20] |
| sailus | Anything else? :-)
I'll have another phone call in 8 minutes. | [12:22] |
| pinchartl | nope | [12:22] |
| hverkuil | I'm really waiting for mchehab since I need to know when he's going to merge pull requests. | [12:23] |
| sailus | I'd be interested in that as well.
pinchartl, hverkuil: Feel free to review "[PATCH 0/5] Move finding the best match for size to V4L2 common" if you have time, too. It's making selection of discrete modes in the framework actually useful for drivers. | [12:23] |
| pinchartl | sailus: I'll check that after reviewing the request API :-) | [12:25] |
| sailus | pinchartl: Bah. These are trivial patches. A bunch of sensor drivers have been posted that could already use them. | [12:25] |
| syoung | Hello
Sorry for being late | [12:28] |
| hverkuil | syoung: we're still waiting for Mauro. I guess he forgot or is traveling.
I haven't heard from him in a while. | [12:30] |
| syoung | hverkuil: ok, thanks | [12:36] |
| ..... (idle for 23mn) | ||
| mchehab | gah, sorry, daylight saving time shifted this week
my alarm ringed at the wrong time mchehab is reading the backlogs | [12:59] |
| hverkuil | OK, I'm still here. | [13:01] |
| mchehab | done
yeah, a patch making VB2 selection independent from V4L2 seems required I partially agree with pinchartl: having to add a select on every driver doesn't sound good | [13:04] |
| hverkuil | (back in a minute, have to grab some coffee) | [13:07] |
| mchehab | but building it when no drivers need seems a bad idea | [13:07] |
| sailus | mchehab: I'm fine changing the patch. After all the situations where videobuf2-v4l2 is built when it's not really needed are rare. | [13:07] |
| mchehab | maybe some depends on magic could be used, just like we did in the past for tuners - it is complex, but doesn't give much headaches
sailus: well, for drivers that depends on VB 1, it is not needed | [13:07] |
| sailus | Also the videodev module contains a lot of stuff that might not be needed by a lot of drivers.
No-one complains about that. | [13:08] |
| mchehab | nor for radio devices | [13:08] |
| sailus | Yes. But how common is it that only drivers for radio devices are built? | [13:09] |
| hverkuil | back | [13:09] |
| mchehab | sailus: we have no idea :-) | [13:09] |
| sailus | And if it's built as a module, it won't be loaded automatically anyway. | [13:09] |
| mchehab | the thing is that it makes kernel larger for embedded environments where this is not needed | [13:10] |
| sailus | Which is a lot better than v4l2-clk which has always been part of videodev.
v4l2-clk is needed almost nowhere these days. | [13:10] |
| pinchartl | mchehab: we'll only build the module unnecessarily if the kernel is build with DVB drivers that need VB2 + V4L2 radio only. that's an unlikely configuration | [13:10] |
| sailus | Whereas videobuf2-v4l2 is very, very commonly needed. | [13:10] |
| mchehab | yes, I know that there aren't many cases
well, come up with a newer proposal. I'll think about that too | [13:11] |
| sailus | A new proposal?
There are two options here essentially. | [13:11] |
| mchehab | with regards to pull requests, I should be handling today and tomorrow
I mean, a v2 | [13:12] |
| sailus | My original patch adds a Kconfig option.
Right. I'll send v2, without requiring a lot of Kconfig changes. It'll be just few lines actually. mchehab: I'm looking forward to that. :-) | [13:12] |
| mchehab | I like the idea of a VB2_V4L2 kconfig var
maybe if you add a: depends on <something> | [13:13] |
| hverkuil | mchehab: OK, I have four pull requests (two have been posted, two are pending), but they have dependencies. | [13:13] |
| mchehab | default y
would do the righ thing something like: | [13:13] |
| sailus | That might cause a lot of make oldconfig churn. I prefer solutions that are quiet. :-) | [13:14] |
| hverkuil | These pull requests should go first: "[GIT PULL v2 FOR v4.17] media: replace g/s_parm by g/s_frame_interval" and "[GIT PULL FOR v4.17] Add tda1997x.c" | [13:15] |
| mchehab | config VIDEOBUF2_V4L2
tristate select VIDEOBUF2_CORE depends on VIDEO_V4L2 depends on VIDEOBUF2_DVB | VIDEOBUF2_DMA_SG | VIDEOBUF2_VMALLOC default y | [13:15] |
| sailus | And drivers would still need to depend on that option to be buildable.
Plus you'd have one more Kconfig option visible to the user. So in that respect I see no benefits in this solution compared to either of the two discussed previously. If I understand the above right, that is. | [13:15] |
| hverkuil | If they can be merged today, then I can rebase the other two on top (media compliance cleanups and the new sh ceu driver) and post the pull request for those. | [13:16] |
| mchehab | sailus: with the sabove, drivers don't need to depend on it
s/sabove/above/ | [13:16] |
| sailus | But if you disable that, then there will be a build failure. | [13:16] |
| mchehab | and this is not visible to the user | [13:16] |
| sailus | I don't think that's acceptable. | [13:16] |
| mchehab | disable? how? | [13:16] |
| sailus | Ah. | [13:16] |
| mchehab | it is not visible | [13:17] |
| sailus | Right. | [13:17] |
| mchehab | hverkuil: I'll handle those two pull requests today | [13:17] |
| sailus | Isn't that achieving the same thing than building videobuf2-v4l2 if both videobuf2 and V4L2 are enabled? | [13:17] |
| mchehab | sailus: I didn't actually looked at the building issue | [13:18] |
| sailus | The drivers don't depend on VIDEOBUF2_CORE now either.
:-) | [13:18] |
| mchehab | you would need to do some work, in order to adjust the exact dependency chain there :-) | [13:18] |
| sailus | I'll post v2 and let's proceed from that. | [13:19] |
| mchehab | OK | [13:19] |
| sailus | s/that/there/ | [13:19] |
| hverkuil | You need the 'depends on VIDEOBUF2_DMA_CONTIG || ...' to get the 'y' vs 'm' right for the videobuf2-v4l2 module. | [13:19] |
| mchehab | yes
but if done right, no changes elsewhere would be needed | [13:20] |
| hverkuil | (it was a nightmare getting that right for CEC :-) ) | [13:21] |
| sailus | mchehab: Please send a patch. :-) | [13:21] |
| mchehab | sailus: basically, it need to depends on every VIDEOBUF2_* that drivers select
maybe depends on VB2 core would be enough | [13:22] |
| hverkuil | mchehab: I think a depends on vb2 core is enough given the sequence in videobuf2/Kconfig | [13:24] |
| mchehab | select VIDEOBUF2_CORE
select VIDEOBUF2_DMA_CONTIG select VIDEOBUF2_DMA_SG select VIDEOBUF2_MEMOPS select VIDEOBUF2_VMALLOC btw, while here I would also change the logic at VIDEOBUF2_CORE to use the same approach and remove explicit select for it | [13:25] |
| sailus | Actually radio devices don't depend on VIDEO_V4L2, but VIDEO_DEV.
So if videobuf2-v4l2 is built when both VIDEO_V4L2 and VIDEOBUF2_CORE are enabled, there are only remote chances of building it without needing it. | [13:31] |
| hverkuil | Yeah, VIDEO_DEV vs VIDEO_V4L2 always confused me. | [13:32] |
| mchehab | sailus, something like: https://pastebin.com/2FpaVqa5
radio devices don't use VB2 ah, you just said the same thing :-) I guess the patch snippet I posted should do the trick and simplify the Kconfig logic | [13:32] |
| sailus | It seems less obvious to me than the current state. | [13:35] |
| mchehab | sailus: it can be less obvious, but using depends on works better than using select | [13:36] |
| sailus | What's wrong with using select? | [13:37] |
| mchehab | it doesn't properly evaluate dependencies
the less we use it, the better for example: config VIDEOBUF2_DMA_SG tristate depends on HAS_DMA the depends on here does nothing all drivers that use this symbol *need* to have a "depends on HAS_DMA" themselves | [13:37] |
| sailus | Hmm.
Let me see if I can get somewhere with this approach. | [13:40] |
| mchehab | ok | [13:42] |
| .................. (idle for 1h27mn) | ||
| sailus | I looked into this a bit --- I don't think there's a way to improve the videobuf2 Kconfig options work by converting select to depends on.
Unless you add more options visible to drivers, that is. It'd be like that a driver depends on something such as HAS_VIDEOBUF2_DMA_SG, and then would select VIDEOBUF2_DMA_SG. I'm certainly open to doing that, though, if people think it's really worth it. | [15:09] |
| *** | snawrocki has quit IRC (Quit: Leaving) | [15:19] |
| ↑back Search ←Prev date Next date→ Show only urls | (Click on time to select a line by its url) |