| ↑back Search ←Prev date Next date→ Show only urls | (Click on time to select a line by its url) |
| Who | What | When | |
|---|---|---|---|
| *** | mchehab has quit IRC (Ping timeout: 260 seconds) | [03:15] | |
| ......................................................... (idle for 4h40mn) | |||
| mmattice has quit IRC (Ping timeout: 265 seconds) | [07:55] | ||
| ....................... (idle for 1h54mn) | |||
| mchehab | hverkuil: ping | [09:49] | |
| hverkuil | mchehab: pong | [09:52] | |
| mchehab | hverkuil: I'm needing to test VBI reception on a device... is it possible to use vivid + pvr-350 for VBI? | [09:54] | |
| hverkuil | The pvr350 is sufficient.
I remember I once shared mpeg files with you that contain embedded VBI that can be played back with the PVR350. What std? NTSC or PAL? With PAL you can really only output the WSS since the PVR350 cannot output teletext data. CC captioning works will with NTSC. | [09:55] | |
| mchehab | ntsc
OK, got it another thing: if I send a progressive video to PVR350, the output will be progressive? | [09:57] | |
| hverkuil | I am never, ever tested with progressive video and I would be extremely surprised if that would work.
with ivtv. | [10:00] | |
| mchehab | ok | [10:01] | |
| hverkuil | Can you even use VBI with progressive video? The various VBI standards like CC all assume interlaced. | [10:02] | |
| mchehab | no, this is another test ;) | [10:03] | |
| hverkuil | Ah. | [10:03] | |
| mchehab | just want to make sure that no regressions will happen on tvp5150
I'm currently missing a way to test those things... analog TV signal was decomissioned last month and I don't know where I stored the device I used to test progressive video (I have another one, but it had a firmware corruption) | [10:03] | |
| pinchartl: bad news, applying your patch series (without my patch) broke S-Video/Composite input selection
tested only with progressive video at those two inputs. Didn't test RF signal | [10:14] | ||
| pinchartl | strange, I'll have a look | [10:17] | |
| mchehab | pinchartl: btw, making S-Video x Composite selection work is tricky | [10:18] | |
| pinchartl | do you agree that the approach from that patch series is better though ? instead of separating the DT and non-DT cases, it tries to unify them | [10:18] | |
| mchehab | pinchartl: provided that it works, yes | [10:18] | |
| pinchartl | yes of course :-) | [10:19] | |
| mchehab | as it should fix for all devices | [10:19] | |
| pinchartl | I dug up my Gumstix + TVP5151 and I'm trying to test the patches there too
the subdev API is used in very different ways by different devices for instance the .reset() operation used by em28xx is a big big mess we should really move away from it, unfortunately that's no easy job | [10:19] | |
| mchehab | I remember Javier tried to make Svideo work on his device for almost 2 days without success | [10:20] | |
| pinchartl | I can't test s-video here as it's not wired on my board | [10:21] | |
| mchehab | Javier made a cable to test S-Video
if your board has the two composite entries, you could wire one | [10:22] | |
| pinchartl | that's the problem, it has a single composite input | [10:22] | |
| mchehab | ah
Javier's board has two composite inputs. So, it is not hard to use them for S-Video | [10:23] | |
| pinchartl | but didn't you say in your e-mail that s-video is working and composite is broken ?
that I could test | [10:27] | |
| mchehab | please notice that I was switching the video input between TV/S-Video/Composite
(maybe the problem would be that, after switching to S-video, it doesn't return to composite anymore - just testing it ATM no. just starting tvtime in Composite doesn't work also, I'm now noticing some glitches at the top border of the image... not sure if it is related to your patch | [10:29] | |
| pinchartl | by the way, could you already merge the first three patches from the series ? they're unrelated and should really not hurt | [10:36] | |
| mchehab | yeah, I'll merge them today | [10:37] | |
| pinchartl | thanks
I'd also appreciate if you could test "v4l: tvp5150: Don't reset device in get/set format handlers" with your patch. it's a tricky one, I'd like to make sure it doesn't break anything | [10:37] | |
| mchehab | I suspect that this could be the culpit of the issues with svideo
I'm applying my patch on the top of yours, to be sure that ignoring the s_stream() won't fix the issue with s-video the top border glitches disappear if I apply my patch maybe em28xx relies on interrupts to identify the start of a frame | [10:38] | |
| pinchartl | on top of the whole series ? | [10:40] | |
| mchehab | yes | [10:40] | |
| pinchartl | could you please push that to a temporary branch somewhere ? | [10:41] | |
| mchehab | it didn't solve the composite issue
hmm.. the glitch seems unrelated... it happens after switching inputs | [10:41] | |
| pinchartl | ok
could you test your patch on top of the four first patches only ? | [10:42] | |
| mchehab | what do you mean?
the em28xx patches are applied here... | [10:43] | |
| pinchartl | patches 1/6 to 4/6 from my series + your patch
without patches 5/6 and 6/6 | [10:43] | |
| mchehab | well, my patch makes it ignore patch 6/6
I'm now reverting patch 5 | [10:44] | |
| pinchartl | please revert patch 6 too, just in case | [10:45] | |
| mchehab | ok, applying patches 1-4 only
initially, without my patch hmm... it should be with my patch, I guess otherwise, it will do the wrong thing on s_stream ok, patches 1-4 applied + my patch everything working, no glitches so, removing tvp5150_reset(sd, 0) from tvp5150_fill_fmt() didn't make any difference let me apply just patch 5 on the top of it | [10:46] | |
| pinchartl | yes, with your patch
1-4 + your patch thanks | [10:51] | |
| mchehab | yes, my series is currently 1-5 + my patch | [10:51] | |
| pinchartl | and that works ? | [10:51] | |
| mchehab | testing... | [10:54] | |
| pinchartl | ok, gumstix finally booting on v4.9, I'll test it here too | [10:54] | |
| mchehab | (there were some conflicts to solve)
what I'm afraid is that maybe tvp5150 is wired on some different way between em28xx and omap3 | [10:54] | |
| pinchartl | 5/6 shouldn't include any functional change, so I think it could already be applied too with 1/6 - 3/6
yes, that's certainly a possibility | [10:55] | |
| mchehab | it is actually somewhat common that different archs use different wirings for analog demux | [10:55] | |
| pinchartl | I suppose we don't have access to the schematics of the em28xx boards | [10:55] | |
| mchehab | we had to solve several cases like that with saa711x
and still, one driver uses a different driver for it, because it is too different I guess I don't have any schematics with em28xx+tvp5150, but I may try to double check hmm.. something really bad happened when I applied patch 5... maybe unrelated it is now losing sync | [10:55] | |
| pinchartl | let me double check that patch 5 produces the same code | [10:58] | |
| mchehab | producing funny images
+#define TVP5150_MISC_CTL_GPCL BIT(6) - val = (val & ~0x40) | 0x10; + val = (val & ~TVP5150_MISC_CTL_GPCL) | TVP5150_MISC_CTL_HVLK; no, that's right patch 5 looks correct on my eyes (except if gcc would be doing something wrong | [10:58] | |
| pinchartl | the objdump -d outputs are different :-S | [11:04] | |
| mchehab | gah | [11:04] | |
| pinchartl | ah I know why | [11:05] | |
| mchehab | missing parenthesis? | [11:05] | |
| pinchartl | in the tvp5150_selmux() function
val = (val & ~TVP5150_MISC_CTL_HVLK) | ~TVP5150_MISC_CTL_GPCL; there's an extra ~ it should be val = (val & ~TVP5150_MISC_CTL_HVLK) | TVP5150_MISC_CTL_GPCL; | [11:05] | |
| mchehab | recompiling
well, that explains why composite broke :-p it is in the else case that selects composite | [11:06] | |
| pinchartl | yes it should :-)
it's surprising how easily we can miss such a small detail | [11:07] | |
| mchehab | true
fixed! patch 5 is now OK let me apply patch 6 | [11:08] | |
| pinchartl | fingers crossed
by the way there's an issue with patch 4/6 for the DT platforms as tvp5150_reset() isn't called at all then I'm trying to fix that | [11:09] | |
| mchehab | I'm applying patch 6 with my patch applied first... shouldn't make any difference
then, I'll remove my patch and see ;) so far, so good ok, final test: just your patches with your patch 5 fixed it seems to be working | [11:10] | |
| pinchartl | \o/
let me make sure it works with the OMAP3 and I'll then resubmit with patch 5 fixed and with a change to patch 4 | [11:16] | |
| mchehab | let me rebase my tree removing the reverted patches in the middle | [11:16] | |
| pinchartl | ok, testing OMAP3 with be more difficult than expected
commit f7b4b54e6364 ("[media] tvp5150: add HW input connectors support") broke it javier___: ^ | [11:18] | |
| mchehab | in order to test if the s-video/composite input change works, you'll need to tell OMAP3 that you have S-Video too
javier___ is out today national holiday | [11:19] | |
| pinchartl | my board doesn't have s-video so I can't test that anyway | [11:20] | |
| mchehab | yes, but you need to check the logic that switches input | [11:21] | |
| pinchartl | given that OMAP3 ISP is broken already by the above commit, should we just go and merge the series, and fix OMAP3 ISP on top of it ? | [11:21] | |
| mchehab | at the selmux function
what's the problem with commit f7b4b54e6364? it seems that it is calling tvp5150_selmux() as expected | [11:21] | |
| pinchartl | it creates new pads, modifying the index of the source pad
it breaks link validation and probably pipeline walk, I haven't checked that yet but it's totally messed up | [11:23] | |
| mchehab | that is one of the problems that we've discussed in the past... hardcoding pad numbers on userspace is really a very bad idea
easy to mess up | [11:24] | |
| pinchartl | it's not just about userspace | [11:24] | |
| mchehab | at kernelspace, we can always use defines | [11:25] | |
| pinchartl | I've modified the userspace test script
it's a kernelspace issue let me show you | [11:25] | |
| mchehab | I had to add several such defines in order to standardize the PAD numbers on different devices with the same function | [11:26] | |
| pinchartl | https://paste.kde.org/pjl8ucxv9 | [11:26] | |
| mchehab | as drivers like em28xx can use different subdevices, wired the same way | [11:26] | |
| pinchartl | with that patch applied, the driver only exposes get/set format on the sink pad, not the source pad
it's not a problem of hardcoding pad numbers it's worse than that and pad 2 and 3 should really not be there it's really utterly broken I wonder which driver that has been tested against the best might be to revert that commit and implement it cleanly | [11:26] | |
| mchehab | well, all analog TV demods have at least 3 pads...
one sink, where it receives the signal to be decoded... one source, for VBI one source for video and some has a 4th pad, for audio | [11:28] | |
| pinchartl | seriously, we can't expect drivers to parse that
I mean without modifications an "Unknown" pad is plain wrong | [11:29] | |
| mchehab | yes, that's wrong | [11:29] | |
| pinchartl | but anyway, my point is that it's completely broken already | [11:30] | |
| mchehab | pad 4 there is wrong | [11:30] | |
| pinchartl | could you please test https://paste.kde.org/pevgumldh/5bq4mq/raw on top of my 6 patches ?
with em28xx if it works, I'll squash that with patch 4/6 the tvp5150 driver needs lots of work. the chip should be put in power down mode when not streaming, and the default init values should have streaming disabled. I'm quite reluctant to fix that myself though, as the whole setup is very fragile with so many ways that em28xx devices could break | [11:30] | |
| mchehab | works | [11:33] | |
| pinchartl | and the only reason I need tvp5150 is to test bt.656 and interlaced input with the omap3
thanks, I'll submit a v2 then | [11:34] | |
| mchehab | ok
yeah, tvp5150 was written at the time we didn't care about PM (there were some serious issues with PM support on the Kernel on that time) and even vendors didn't put the devices to sleep on their drivers, as far as I can tell from the USB sniffs I did on that time putting tm5150 in power down mode could be an interesting fix yet, I suspect that most em28xx solves it on a different way: they simply disable tm5150 via GPIO pin s/tm/tvp/ | [11:34] | |
| pinchartl | possibly
we should also get rid of the .reset() subdev operation but again that's used by em28xx, so it would be difficult | [11:37] | |
| mchehab | I guess reset is needed when the chip is powered up via GPIO
but don't remember anymore the dirty details the problem is that hybrid devices use GPIO to disable/enable analog or digital parts of the hardware several devices require to rewrite the registers after powered up via GPIO the I2C drivers don't know if the chip they control are powered down or up, because the GPIO control is done at the bridge driver changing such logic could be harsh | [11:37] | |
| pinchartl | that's the problem, the GPIO shouldn't be controlled by the bridge driver
it's fixable, but difficult as the risk of regression is very high | [11:41] | |
| javier___ | pinchartl: hi, on holiday here so I couldn't follow all the discussion | [11:42] | |
| pinchartl | javier___: we can talk about it next week, nothing urgen | [11:43] | |
| javier___ | about f7b4b54e6364 ("[media] tvp5150: add HW input connectors support"), that was part of a series that also had DT changes to support the input signals
https://patchwork.kernel.org/patch/8238771/ | [11:43] | |
| pinchartl | I'd like to see that reverted and implemented cleanly | [11:44] | |
| javier___ | yes, agreed. You latter asked the DT binding to be reverted until we decide how to properly define the input connectors in the DT | [11:44] | |
| pinchartl | especially given that DT parsing doesn't match the DT bindings | [11:44] | |
| javier___ | https://patchwork.kernel.org/patch/8395521/ | [11:44] | |
| pinchartl | yes
so we first need to sort out DT then implement the code properly and test it | [11:45] | |
| javier___ | pinchartl: yes, I thought the DT parsing logic was reverted as well... but it seems not | [11:45] | |
| pinchartl | by the way, which platform did you test that series with ? | [11:45] | |
| javier___ | pinchartl: IGEPv2 board + expansion board (TVP5151 with 2 composite connectors)
https://www.isee.biz/products/igep-expansion-boards/igepv2-expansion | [11:45] | |
| pinchartl | I wonder how you managed to get the OMAP3 ISP running
the driver can't validate the pipeline | [11:47] | |
| commit 55606310e77f ("[media] tvp5150: create the expected number of pads") would need to be reverted too | [11:55] | ||
| javier___ | pinchartl: it worked for me, with the DT changes to have the connectors defined in the DT
the problem with the driver without that series is that it always used the same composite input signal so if you had a board that used the other composite input pin from the TVP5150/1, it wouldn't work | [11:56] | |
| pinchartl | yes, there's a need to define connectors, and implement link setup
but it's not correct we can work together next week to fix that | [11:59] | |
| javier___ | pinchartl: yes, agreed. My latest attempt was https://lkml.org/lkml/2016/4/12/983
IIUC you agree with the approach but just not with the DT bindings? | [12:00] | |
| pinchartl | yes, and my comment about the DT bindings was minor, it was only about usings a ports subnode and no connectors subnode
I'm not sure I reviewed the code though, I focussed on the DT bindings but once we agree on the bindings, the code shouldn't be an issue | [12:02] | |
| javier___ | pinchartl: yes, I was just busy with other stuff so I let this feel through the cracks... sorry about that
pinchartl: Ok, I'll re-spin that next week and you can review the code then | [12:03] | |
| pinchartl | no worries
thanks for the support | [12:04] | |
| javier___ | for now, I think the best is to merge your tvp5150 fixes for the em28xx since the tvp5150 is broken anyways due the code and dt binding not being in sync... | [12:05] | |
| mchehab | pinchartl: try reverting javier___'s patch and test it on omap3 to be sure that it works there too | [12:05] | |
| javier___ | tvp5150 + omap3 is broken I mean
pinchartl, mchehab: I've to leave again. Have a nice day! | [12:05] | |
| mchehab | javier___: have a nice holiday!
if you're willing to do such test, please be sure to test if selmux is doing the right thing on omap3 - e. g. switching to S-Video will produce a black image and switching back to Composite will restore it | [12:07] | |
| pinchartl | javier___: agreed. enjoy your weekend | [12:08] | |
| mchehab | (that's the behavior that happens here when I keep S-video input disconnected and switch between composite and svideo) | [12:09] | |
| pinchartl | I'll try to test that later today, I have to go now I'm afraid | [12:09] | |
| mchehab | yeah, later is ok | [12:10] | |
| pinchartl | I managed to capture frames with the OMAP3 ISP after reverting a few patchees | [12:10] | |
| mchehab | I also intend to test it on WinTV USB2 (with has only S-video) | [12:10] | |
| pinchartl | but they're all black. which isn't surprising given that my composite source seems not to work | [12:10] | |
| mchehab | (actually, it is possible to use composite there with a S-Video to composite cable)
here, I tested my composite source on a separate screen before testing with real hardware :) just to be sure that the source was reliable enough as I'm using a really old hardware as my composite source | [12:10] | |
| sailus | neg: Ping? | [12:22] | |
| neg | sailus: pong | [12:23] | |
| ................. (idle for 1h20mn) | |||
| sailus | neg: Pung.
I thought of asking about the patchset you needed the few routing patches for... how is it doing? :-) | [13:43] | |
| neg | sailus: thanks for asking :-) I'm still waiting for review comments on v2 of the Gen3 VIN series, I get review from Laruent on the CSI-2 part and I really need to free some time to dig in to those but the routing patches you are interessted in are only related to the VIN series.
sailus: If you are waiting for the routing patches for something else maybe we should try and submit them outside the VIN series ? | [13:50] | |
| sailus | neg: I don't need the routing patches as such, but the other patchset I have depends on the two routing patches.
I'd just prefer to get them in. Without rebasing them below your patches, in which case you'd need to also rebase. | [13:56] | |
| *** | svarbanov has quit IRC (Ping timeout: 250 seconds) | [13:57] | |
| neg | I'm fine with rebasing since I don't know when my VIN work will move forward and I don't want to block you | [14:03] | |
| sailus | I don't think it'd be a lot of rebasing actually. | [14:07] | |
| pinchartl | mchehab: I managed to test the patch series with the OMAP3 ISP. after changing the hardcoded input default from composite1 to composite0 I can capture images
once we'll sort out the current tvp5150 DT + connectors mess, that should work in mainline with any hack the image quality is pretty bad though, but it might be my source | [14:09] | |
| ..... (idle for 21mn) | |||
| *** | awalls1 has left | [14:31] | |
| ............ (idle for 58mn) | |||
| hverkuil has left | [15:29] | ||
| ........ (idle for 39mn) | |||
| mchehab | pinchartl: good
image quality can be due to your source... here, the quality is not 100%, but it is not that bad | [16:08] | |
| sailus | neg: I sent a pull request on the rebased cleanup / fix set. Could you rebase yours on that? I think it'll be renaming a few function calls in your set.
The two routing patches you need can also be found here (rebased): http://git.retiisi.org.uk/?p=~sailus/linux.git;a=shortlog;h=refs/heads/routing | [16:23] | |
| ....... (idle for 32mn) | |||
| *** | prabhakarlad has left | [16:55] | |
| ..... (idle for 24mn) | |||
| neg | sailus: thanks, will rebase | [17:19] | |
| ................... (idle for 1h34mn) | |||
| *** | jpabq has quit IRC (Ping timeout: 258 seconds) | [18:53] | |
| ........ (idle for 35mn) | |||
| awalls1 has left | [19:28] | ||
| ................................. (idle for 2h41mn) | |||
| tfiga has quit IRC (Remote host closed the connection) | [22:09] | ||
| ................. (idle for 1h23mn) | |||
| cybrNaut has quit IRC (Ping timeout: 258 seconds) | [23:32] | ||
| ↑back Search ←Prev date Next date→ Show only urls | (Click on time to select a line by its url) |