[11:54] <hverkuil> mchehab: posted a v2 of the "improve CEC documentation" patch
[11:55] <mchehab> ok, thanks
[13:49] *** ParkerR has quit IRC (Ping timeout: 244 seconds)
[14:17] <ndufresne> mchehab: hverkuil: I'm sorry in advance if I'm being annoying on the ML about mtk codec, it's just that I realize they are trying to mainline a driver that requires vendor patch to be usable, plus a pixel format in the kernel API will remain forever, we don't even know the memory layout since the chromium describtion is contradictory
[14:18] <ndufresne> on a "generic" user space pint of view, it also a real pain to support, it's likely their video processor is bound to the display or implemented in DRM, in DRM only priviledge process can access the converters
[14:19] <ndufresne> same concern apply to Samsung stuff being removed from V4L2 and added in DRM, the DRM side is custom API (so per driver) and require the drm cookie
[14:19] <mchehab> ndufresne: mainlining a driver that requires a vendor patch and/or a pixel foramt that it is not docummented is not acceptable
[14:21] <hverkuil> ndufresne: I agree completely with you. I already asked mediatek to be more precise, but then you hijacked the thread and I am happy to let you run with it :-)
[14:22] <mchehab> being "hard" to implement a generic userspace app is something that it is not desirable, but sometimes it happens, specially on embedded drivers... not sure if there are much we can do with regards to that
[14:23] <hverkuil> In this case it also seems they don't quite understand it themselves, so they should sort this out first.
[14:23] <hverkuil> The decoder is definitely not for 4.8 anyway.
[14:24] <hverkuil> Although once this is sorted I expect it to go in early in the 4.9 cycle.
[14:24] <mchehab> ndufresne: btw, I saw the thread and I'm also happy that you complained about that :)
[14:28] <ndufresne> mchehab: for my part, I'm happy to do the extra work in gst, that's what I did with Samsung Tiled format, it's super hard to implement, but it's documented
[14:29] <ndufresne> One aspect of Samsung case, is that Kamil reversed engineered the format, cause he could not find the doc
[14:30] <ndufresne> All in all, if they at least figure-out what it is internally, they might get the aligment right, they probably over allocate right now
[15:09] *** benjiG has left 
[16:24] <ndufresne> hverkuil: where do we map the dma-buffer in vb2 ?
[16:34] <larsc> but it is impossible to decompress on the CPU!
[16:39] <ndufresne> lol ;-P
[16:43] <mchehab> pinchartl: please remind me: why did we create a separate topic branch for vsp1?
[16:47] <mchehab> As it doesn't depend on any external tree, I'll just fold it at master
[16:48] <javier__> mchehab: IIRC I think it was because the DRM people needed to pull it due some DRM changes that depended on it
[16:48] <mchehab> yes, that's what I remember too
[16:49] <mchehab> as I won't rebase, it should be OK to merge it back mainstream
[16:49] <mchehab> done
[16:51] <hverkuil> ndufresne: it's done in videobuf2-vmalloc/dma-contig/dma-sg.c
[16:54] <ndufresne> hverkuil: yep found it, and I notice the TODO in QBUF, that says that we should maybe map the dmabuf when consuming instead
[18:05] <ndufresne> javier__: interesting, the mmap offset from mfc is now non-zero
[18:05] <ndufresne> It's a bogus value, should be zero for this driver
[18:05] <ndufresne> In gst, the dmabuf offset is not implemented/ignored, but I guess the drivers might not ignore it ..
[18:06] <ndufresne> well, "I think" it supposed to be zero, since if I mmap the buffer and render it in software, it works fine
[18:07] <ndufresne> values are in the range 1073741824 - 1091682304
[18:07] <ndufresne> it's quite a big offset
[18:08] <javier__> ndufresne: by now, do you mean that this used to be zero?
[18:08] <ndufresne> it used to be zero yes
[18:09] <ndufresne> hverkuil: the offset for dmabuf plane, how is that interpreted in relation with bytesused and length
[18:09] <ndufresne> my first guess is that offset should be inside lenght
[18:09] <ndufresne> so offset + bytesused <= length
[18:10] <ndufresne> ha, should learn to read
[18:10] <ndufresne> "	Note that data_offset is included in bytesused."
[18:11] <ndufresne> "the size of the image in the plane is bytesused-data_offset", which for MFC is 1540096 - 1090142208,  ;-P
[18:12] <ndufresne> javier__: looks like a regression caused by a refactoring, 2d7007153f0c9b1dd00c01894df7d26ddc32b79f
[18:12] <javier__> ndufresne: interesting
[18:13] <ndufresne> still I should fix offset support, specially for the case I want to import a dmabuf with an offset ;-P
[18:13] <ndufresne> (that happens when mapping a single dmabuf to a mplane driver)
[18:18] <ndufresne> javier__: hmm, sorry, apparently that was mem_offset, not data offset
[18:46] <larsc> the mmap offset is basically just a transparent cookie that lets the kernel identify which buffer you are trying to map
[18:47] <ndufresne> I know
[18:48] <ndufresne> larsc: we are trying to track a crash when importing a dmabuf from s5p-mfc decoder into another driver
[18:49] <ndufresne> if I map in userspace and copy it's fine
[18:49] <ndufresne> quite puzzling issue
[18:49] <larsc> what kind of crash?
[18:49] <larsc> NULL pointer deref?
[18:49] <ndufresne> it crash while flyshing
[18:49] <ndufresne> a page fault
[18:52] <larsc> and offset has these strange large values, or data_offset?
[18:52] <ndufresne> larsc: if you have some pointers, https://paste.fedoraproject.org/390814/46843595/
[18:53] <ndufresne> larsc: I had a miss-leading trace, the mem_offsset and data_offset are fine
[18:55] <ndufresne> the address is over 3GB, got 2GB on this board, it's most likely wrong ;-P
[18:57] <larsc> 0xc0000000 is the start of the kernel map of the RAM, I think
[18:57] <ndufresne> ah ok, then the addres has nothing special, other then it's probably not allocated ;-P
[18:57] <larsc> it's quite even
[18:58] <ndufresne> I'm trying to find what paremeters are used here and provided by this driver
[18:58] <ndufresne> Same color format, same buffer size works with another driver so ...
[19:01] <larsc> but v4l is the importer and exporter in this case?
[19:01] <ndufresne> v4l2 is the exporter, and it crash regardless of the imported, I tested v4l2 importer and DRM importer
[19:03] <larsc> it is actually possibl to allocate DMA buffers without a kernel mapping
[19:03] <larsc> maybe that's the case here
[19:03] <larsc> and the importer is not aware of that
[19:04] <ndufresne> Can you provide more details ?
[19:04] <ndufresne> btw, it worked before, it's possibly a regression when the exnynos iommu started being merged
[19:04] <ndufresne> this is without iommu
[19:05] <larsc> trying to remember the details
[19:07] <larsc> DMA_ATTR_NO_KERNEL_MAPPING
[19:08] <larsc> but it does not look as if that is used by any v4l2 drivers yet
[19:09] <ndufresne>  drivers/media/v4l2-core/videobuf2-dma-contig.c: if (!dma_get_attr(DMA_ATTR_NO_KERNEL_MAPPING, &buf->attrs))
[19:09] <ndufresne> drivers/gpu/drm/exynos/exynos_drm_gem.c:        dma_set_attr(DMA_ATTR_NO_KERNEL_MAPPING, &exynos_gem->dma_attrs);
[19:09] <ndufresne> but not in the exporter driver, so likely not my issue
[19:11] <ndufresne> larsc: I'm having the impression that maybe the allocation isn't page aligned
[19:12] <ndufresne> which would work with iommu, but not without ...
[19:12] <ndufresne> e.g. we could be trying to map an extra page at the end, which when we mmap, we never touch
[19:29] <larsc> do you know when with which version the regression was introduced?
[19:44] <larsc> is your buffer size 2MB?
[19:51] <ndufresne> I don't know the version, it's too large, since on this board I was previously on 3.14
[19:51] <ndufresne> let's say I didn't touch it for a long time
[19:51] <ndufresne> the buffers are quite large, they are full HD NV12 kind of buffers (tiled)
[19:52] *** awalls has left 
[19:53] <ndufresne> larsc: to be more precise, Y plane is 2073600 bytes, and UV plane is 1036800 bytes
[19:54] <ndufresne> wait a minute, the decoder and converter don't agree on the size ...
[19:55] <ndufresne> there is no padding so ...
[19:55] <larsc> the flush operation tries to flush 2088960 bytes
[19:56] <ndufresne> yes, that was the number for the converter sorry, the size are 2088960/1048576
[19:56] <ndufresne> the means it crash mapping the first plane (the luma plane)
[19:56] <larsc> and the start of the buffer is the address where it crashes
[19:56] <ndufresne> javier__: the fact for the same tiled format the FIMC and the MFC don't agree ont he size looks pretty bad
[19:57] <javier__> ndufresne: agreed
[19:57] <larsc> so that probably means something gets messed up in the exporter
[19:57] <ndufresne> larsc: how do you find the start of the buffer in that trace ?
[19:57] <larsc> r0 is start r1 is end
[19:58] <ndufresne> those are physical or virtual addresses ?
[19:58] <larsc> they are supposed to be physical
[20:00] <ndufresne> I wonder how it works when doing mmap from userspace, it should be doing the same behind the scene
[20:01] <larsc> mmap on the dma_buf?
[20:02] <ndufresne> in userspace you have an FD, if you mmap() the same FD in userspace, and read it, it works
[20:03] <ndufresne> I'm thinking there is clearly something in common between in-driver dmabuf mapping and userspace mmap
[20:03] <larsc> not really
[20:03] <larsc> two different functions
[20:03] <larsc> get their addresses from different sources
[20:04] <ndufresne> isn't the address bound to the dmabuf ?
[20:05] <larsc> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/media/v4l2-core/videobuf2-dma-contig.c#n180
[20:05] <larsc> that's userspace mmap
[20:05] <larsc> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/media/v4l2-core/videobuf2-dma-contig.c#n284
[20:05] <larsc> that's buffer import
[20:06] <larsc> userspace mmap directly uses the address and length from the buffer
[20:06] <larsc> dma_buf import uses a sg table
[20:06] <larsc> that was created during export
[20:07] <ndufresne> larsc: vb2_dc_mmap() is for when you map usign the device FD, I mean mmap() using the dmabuf fd
[20:08] <larsc> ndufresne: same function
[20:08] <larsc> see http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/media/v4l2-core/videobuf2-dma-contig.c#n350
[20:08] <larsc> but this is how the sg table is created http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/media/v4l2-core/videobuf2-dma-contig.c#n368
[20:09] <javier__> larsc: and the sg table is supposed to contain a single entry for contiguous memory (i.e: cma) ?
[20:09] <larsc> which uses the same addresses as userspace mmap
[20:10] <larsc> javier__: yes
[20:10] <ndufresne> Question is what MFC driver do that makes it behave differently here
[20:10] <larsc> but the table is copied in between, maybe it gets corrupted
[20:11] <ndufresne> I got two driver, MFC (ko) and FIMC (ok)
[20:12] <larsc> good question
[20:12] <ndufresne> the driver is special in the sense it uses it's own cma pool
[20:13] <larsc> the MFC?
[20:14] <ndufresne> yes, without iommu, there is two memory bank that are reserved for this driver
[20:14] <ndufresne> so somehow, it means the driver need override a bit of vb_dc
[20:16] <larsc> how is that setup?
[20:16] <javier__> larsc: it uses Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
[20:16] <javier__> larsc: arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi
[20:19] <ndufresne> I like this commit comment "MFC hardware is known to trash random memory if one tries ..."
[20:22] <larsc> do you see something like 'Reserved memory: ...' in your bootlog?
[20:23] <ndufresne> cma: Reserved 448 MiB at 0xa3c00000
[20:24] <ndufresne> I don't yet have javier__ patches to use the generic thingy
[20:24] <ndufresne> I'm still on 4.6 here
[20:26] <javier__> ndufresne: are Marek's patches btw :)
[20:26] <ndufresne> ah sorry
[20:27] <javier__> anyways, the underlaying allocation method is the same so it shouldn't matter afaict
[20:35] <ndufresne> javier__: I notice in MFC that there is two place where the sizes seems calculated
[21:02] <jyelloz> Hi, I have an old ATI TV Wonder 600 USB DVB adapter. The DVB part works fine but I am interested in getting the v4l composite A/V part working.
[21:03] <jyelloz> The v4l part works fine on one of my systems but on another system (both running Linux 4.6.y), the v4l interface will fail to initialize with the following error message:
[21:03] <jyelloz> tvp5150: probe of 9-005c failed with error -38
[21:04] <jyelloz> This is an em28xx-based device. Has anyone seen something similar before?