Re: [RFC] [PATCH 1/6] SoC Camera: add driver for OMAP1 camera interface
Monday 16 August 2010 12:17:36 Marin Mitov wrote: On Saturday, August 14, 2010 08:33:09 pm Guennadi Liakhovetski wrote: On Fri, 13 Aug 2010, Janusz Krzysztofik wrote: Friday 13 August 2010 11:11:52 Marin Mitov napisał(a): On Friday, August 13, 2010 11:52:41 am Guennadi Liakhovetski wrote: On Fri, 13 Aug 2010, Janusz Krzysztofik wrote: Thursday 12 August 2010 23:38:17 Guennadi Liakhovetski napisał(a): 1. We've discussed this dynamic switching a bit on IRC today. The first reaction was - you probably should concentrate on getting the contiguous version to work reliably. I.e., to reserve the memory in the board init code similar, how other contig users currently do it. I already tried before to find out how I could allocate memory at init without reinventing a new videobuf-dma-contig implementation. Since in the Documentation/video4linux/videobuf I've read that videobuf does not currently play well with drivers that play tricks by allocating DMA space at system boot time, I've implemented the alternate sg path. If it's not quite true what the documentation says and you can give me a hint how this could be done, I might try again. For an example look at arch/arm/mach-mx3/mach-pcm037.c::pcm037_camera_alloc_dma(). Yes, this is the solution that suffers from the already discussed limitation of not being able to remap a memory with different attributes, which affects OMAP1 as well. For preallocating dma-coherent memory for device personal use during device probe() time (when the memory is less fragmented compared to open() time) see also dt3155_alloc_coherent/dt3155_free_coherent in drivers/staging/dt3155v4l/dt3155vfl.c (for x86 arch, I do not know if it works for arm arch) With this workaround applied, I get much better results, thank you Marin. However, it seems not bullet proof, since mmap still happens to fail for a reason not quite clear to me: What exactly does this mean - happens to fail - you mean starting and stopping mplayer several times? Can you verify, that you're not leaking memory? That you're freeing all allocated DMA memory again? Are you using the same parameters to mplayer, right? As for the work-around - can you not do this in your board late-initcall function? Not sure whether and how one can get this in the mainline. This is in principle the same, as in the above dma_declare_coherent_memory() example, only open-coded without the ioremap. My believe is that dma_declare_coherent_memory() could be used if your frame grabber has local RAM buffer (like video buffer if the graphic adapters) defined by BAR - that is why you need ioremap(). If this RAM turns out to be coherent you use dma_declare_coherent_memory() and any further invocation of dma_alloc_coherent() will allocate from it (till it is exosted). My use of dt3155_alloc_coherent()/dt3155_free_coherent() is to allocate a block of coherent 4MB memory during driver probe() method and use it latter (via videobuff_dma_contig framework)). Maybe we can add a suitable function to the dma-alloc API... Could be of general use, I am thinking about this. This could be done by just renaming dt3155_alloc_coherent()/dt3155_free_coherent() to something acceptable (dma_reserve_coherent_memory()/dma_release_reserved_memory(), I am open for suggestions) and export them. Should be added to drivers/base/dma-coherent.c. Hi Marin, Since I've finaly managed to make use of your method without any previously observerd limitations (see below), I'm interested in it being implemented system-wide. Are you going to submit a patch? I would suggest creating one common function that allocates and fills the dev-dma_mem structure, and two wrappers that call it: a dma_declare_coherent_memory() replacement, that passes an ioremapped device memory address to the common fuction, and your proposed dma_reserve_coherent_memory(), that passes a pointer returned by the dma_alloc_coherent() instead. [ 6067.22] omap1-camera omap1-camera.0: OMAP1 Camera driver attached to camera 0 [ 6067.65] omap1-camera omap1-camera.0: omap1_cam_try_fmt: format 32315659 not found [ 6067.68] omap1-camera omap1-camera.0: omap1_cam_try_fmt: format 32315559 not found [ 6068.48] mplayer: page allocation failure. order:6, mode:0xd0 [ 6068.50] Backtrace: [ 6068.52] [c0028950] (dump_backtrace+0x0/0x110) from [c0028ea8] (dump_stack+0x18/0x1c) [ 6068.56] r6:0006 r5:00d0 r4:c1bcf000 [ 6068.59] [c0028e90] (dump_stack+0x0/0x1c) from [c0074e24] (__alloc_pages_nodemask+0x504/0x560) [ 6068.62] [c0074920] (__alloc_pages_nodemask+0x0/0x560) from [c002ae14] (__dma_alloc+0x108/0x354) [ 6068.66] [c002ad0c] (__dma_alloc+0x0/0x354) from [c002b0ec] (dma_alloc_coherent+0x58/0x64) [
Re: [RFC] [PATCH 1/6] SoC Camera: add driver for OMAP1 camera interface
On Thu, 19 Aug 2010, Janusz Krzysztofik wrote: Hi Marin, Since I've finaly managed to make use of your method without any previously observerd limitations (see below), I'm interested in it being implemented system-wide. Are you going to submit a patch? I'm about to submit a patch, which you'll be most welcome to test. Just give me a couple more hours. I would suggest creating one common function that allocates and fills the dev-dma_mem structure, and two wrappers that call it: a dma_declare_coherent_memory() replacement, that passes an ioremapped device memory address to the common fuction, and your proposed dma_reserve_coherent_memory(), that passes a pointer returned by the dma_alloc_coherent() instead. [ 6067.22] omap1-camera omap1-camera.0: OMAP1 Camera driver attached to camera 0 [ 6067.65] omap1-camera omap1-camera.0: omap1_cam_try_fmt: format 32315659 not found [ 6067.68] omap1-camera omap1-camera.0: omap1_cam_try_fmt: format 32315559 not found [ 6068.48] mplayer: page allocation failure. order:6, mode:0xd0 [ 6068.50] Backtrace: [ 6068.52] [c0028950] (dump_backtrace+0x0/0x110) from [c0028ea8] (dump_stack+0x18/0x1c) [ 6068.56] r6:0006 r5:00d0 r4:c1bcf000 [ 6068.59] [c0028e90] (dump_stack+0x0/0x1c) from [c0074e24] (__alloc_pages_nodemask+0x504/0x560) [ 6068.62] [c0074920] (__alloc_pages_nodemask+0x0/0x560) from [c002ae14] (__dma_alloc+0x108/0x354) [ 6068.66] [c002ad0c] (__dma_alloc+0x0/0x354) from [c002b0ec] (dma_alloc_coherent+0x58/0x64) [ 6068.70] [c002b094] (dma_alloc_coherent+0x0/0x64) from [bf000a44] (__videobuf_mmap_mapper+0x10c/0x374 [videobuf_dma_contig]) [ 6068.74] r7:c16934c0 r6: r5:c171baec r4: [ 6068.77] [bf000938] (__videobuf_mmap_mapper+0x0/0x374 [videobuf_dma_contig]) from [c01f9a78] (videobuf_mmap_mapper+0xc4/0x108) [ 6068.81] [c01f99b4] (videobuf_mmap_mapper+0x0/0x108) from [c01fc1ac] (soc_camera_mmap+0x80/0x140) [ 6068.84] r5:c1a3b4e0 r4: [ 6068.87] [c01fc12c] (soc_camera_mmap+0x0/0x140) from [c01eeba8] (v4l2_mmap+0x4c/0x5c) [ 6068.90] r7:c145c000 r6:00ff r5:c16934c0 r4: [ 6068.93] [c01eeb5c] (v4l2_mmap+0x0/0x5c) from [c0085de4] (mmap_region+0x238/0x458) [ 6068.97] [c0085bac] (mmap_region+0x0/0x458) from [c00862c0] (do_mmap_pgoff+0x2bc/0x320) [ 6069.00] [c0086004] (do_mmap_pgoff+0x0/0x320) from [c00863c0] (sys_mmap_pgoff+0x9c/0xc8) [ 6069.04] [c0086324] (sys_mmap_pgoff+0x0/0xc8) from [c0024f00] (ret_fast_syscall+0x0/0x2c) [ 6069.20] Mem-info: [ 6069.22] Normal per-cpu: [ 6069.24] CPU0: hi:0, btch: 1 usd: 0 [ 6069.26] active_anon:676 inactive_anon:682 isolated_anon:0 [ 6069.26] active_file:422 inactive_file:2348 isolated_file:0 [ 6069.26] unevictable:177 dirty:0 writeback:0 unstable:0 [ 6069.26] free:1166 slab_reclaimable:0 slab_unreclaimable:0 [ 6069.26] mapped:1120 shmem:0 pagetables:121 bounce:0 [ 6069.35] Normal free:4664kB min:720kB low:900kB high:1080kB active_anon:2704kB inactive_anon:2728kB active_file:1688kB inactive_file:9392kB unevictable:708kB isolated(anon):0kB isolated(file):0kB present:32512kB mlocked:0kB dirty:0kB writeback:0kB mapped:4480kB shmem:0kB slab_reclaimable:0kB slab_unreclaimable:0kB kernel_stack:552kB pagetables:484kB unstable:0kB bounce:0kB writeback_tmp:0kB pages_scanned:0 all_unreclaimable? no [ 6069.46] lowmem_reserve[]: 0 0 [ 6069.47] Normal: 6*4kB 20*8kB 14*16kB 29*32kB 26*64kB 9*128kB 2*256kB 0*512kB 0*1024kB 0*2048kB 0*4096kB = 4664kB [ 6069.53] 2960 total pagecache pages [ 6069.55] 8192 pages of RAM [ 6069.56] 1322 free pages [ 6069.58] 1114 reserved pages [ 6069.59] 750 slab pages [ 6069.61] 2476 pages shared [ 6069.63] 0 pages swap cached [ 6069.64] omap1-camera omap1-camera.0: dma_alloc_coherent size 204800 failed [ 6069.68] omap1-camera omap1-camera.0: OMAP1 Camera driver detached from camera 0 Maybe I should preallocate a few more pages than will be actually used by the driver? That was it. I was trying to reserve exact frame size, times number of buffers. Apparently, the frame size should be rounded up to the nearest power of 2 first for it to work as expected. No, I don't think you should go to the next power of 2 - that's too crude. Try rounding your buffer size to the page size, that should suffice. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at
Re: [RFC] [PATCH 1/6] SoC Camera: add driver for OMAP1 camera interface
On Thursday, August 19, 2010 02:39:47 pm Guennadi Liakhovetski wrote: On Thu, 19 Aug 2010, Janusz Krzysztofik wrote: Hi Marin, Since I've finaly managed to make use of your method without any previously observerd limitations (see below), I'm interested in it being implemented system-wide. Are you going to submit a patch? It is ready, I just wait for the invitation. Marin Mitov I'm about to submit a patch, which you'll be most welcome to test. Just give me a couple more hours. I would suggest creating one common function that allocates and fills the dev-dma_mem structure, and two wrappers that call it: a dma_declare_coherent_memory() replacement, that passes an ioremapped device memory address to the common fuction, and your proposed dma_reserve_coherent_memory(), that passes a pointer returned by the dma_alloc_coherent() instead. No, I don't think you should go to the next power of 2 - that's too crude. Try rounding your buffer size to the page size, that should suffice. Allocated coherent memory is always a power of 2. Thanks. Marin Mitov Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] [PATCH 1/6] SoC Camera: add driver for OMAP1 camera interface
Thursday 19 August 2010 14:16:21 Marin Mitov napisał(a):
On Thursday, August 19, 2010 02:39:47 pm Guennadi Liakhovetski wrote:
No, I don't think you should go to the next power of 2 - that's too
crude. Try rounding your buffer size to the page size, that should
suffice.
Guennadi,
If you have a look at how a device reserved memory is next allocated to a
driver with drivers/base/dma-coherent.c::dma_alloc_from_coherent(), then than
you may find my conclusion on a power of 2 as true:
int dma_alloc_from_coherent(struct device *dev, ssize_t size,
dma_addr_t *dma_handle, void **ret)
{
...
int order = get_order(size);
...
pageno = bitmap_find_free_region(mem-bitmap, mem-size, order);
...
}
Allocated coherent memory is always a power of 2.
Marin,
For ARM, this seems true as long as allocated with the above from a device
assigned pool, but not true for a (pre)allocation from a generic system RAM.
See arch/arm/mm/dma-mapping.c::__dma_alloc_buffer(), where it looks like extra
pages are freed:
static struct page *__dma_alloc_buffer(struct device *dev, size_t size, gfp_t
gfp)
{
unsigned long order = get_order(size);
...
page = alloc_pages(gfp, order);
...
split_page(page, order);
for (p = page + (size PAGE_SHIFT), e = page + (1 order); p e;
p++)
__free_page(p);
...
}
Thanks,
Janusz
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] [PATCH 1/6] SoC Camera: add driver for OMAP1 camera interface
On Thursday, August 19, 2010 08:09:27 pm Janusz Krzysztofik wrote:
Thursday 19 August 2010 14:16:21 Marin Mitov napisał(a):
On Thursday, August 19, 2010 02:39:47 pm Guennadi Liakhovetski wrote:
No, I don't think you should go to the next power of 2 - that's too
crude. Try rounding your buffer size to the page size, that should
suffice.
Guennadi,
If you have a look at how a device reserved memory is next allocated to a
driver with drivers/base/dma-coherent.c::dma_alloc_from_coherent(), then than
you may find my conclusion on a power of 2 as true:
int dma_alloc_from_coherent(struct device *dev, ssize_t size,
dma_addr_t *dma_handle, void **ret)
{
...
int order = get_order(size);
...
pageno = bitmap_find_free_region(mem-bitmap, mem-size, order);
...
}
Allocated coherent memory is always a power of 2.
Marin,
For ARM, this seems true as long as allocated with the above from a device
assigned pool, but not true for a (pre)allocation from a generic system RAM.
See arch/arm/mm/dma-mapping.c::__dma_alloc_buffer(), where it looks like
extra
pages are freed:
static struct page *__dma_alloc_buffer(struct device *dev, size_t size, gfp_t
gfp)
{
unsigned long order = get_order(size);
...
page = alloc_pages(gfp, order);
...
split_page(page, order);
for (p = page + (size PAGE_SHIFT), e = page + (1 order); p e;
p++)
__free_page(p);
...
}
Thanks for the clarification.
Marin Mitov
Thanks,
Janusz
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] [PATCH 1/6] SoC Camera: add driver for OMAP1 camera interface
Saturday 14 August 2010 19:33:09 Guennadi Liakhovetski napisał(a): On Fri, 13 Aug 2010, Janusz Krzysztofik wrote: Friday 13 August 2010 11:11:52 Marin Mitov napisał(a): On Friday, August 13, 2010 11:52:41 am Guennadi Liakhovetski wrote: On Fri, 13 Aug 2010, Janusz Krzysztofik wrote: Thursday 12 August 2010 23:38:17 Guennadi Liakhovetski napisał(a): 1. We've discussed this dynamic switching a bit on IRC today. The first reaction was - you probably should concentrate on getting the contiguous version to work reliably. I.e., to reserve the memory in the board init code similar, how other contig users currently do it. I already tried before to find out how I could allocate memory at init without reinventing a new videobuf-dma-contig implementation. Since in the Documentation/video4linux/videobuf I've read that videobuf does not currently play well with drivers that play tricks by allocating DMA space at system boot time, I've implemented the alternate sg path. If it's not quite true what the documentation says and you can give me a hint how this could be done, I might try again. For an example look at arch/arm/mach-mx3/mach-pcm037.c::pcm037_camera_alloc_dma(). Yes, this is the solution that suffers from the already discussed limitation of not being able to remap a memory with different attributes, which affects OMAP1 as well. For preallocating dma-coherent memory for device personal use during device probe() time (when the memory is less fragmented compared to open() time) see also dt3155_alloc_coherent/dt3155_free_coherent in drivers/staging/dt3155v4l/dt3155vfl.c (for x86 arch, I do not know if it works for arm arch) With this workaround applied, I get much better results, thank you Marin. However, it seems not bullet proof, since mmap still happens to fail for a reason not quite clear to me: What exactly does this mean - happens to fail - you mean starting and stopping mplayer several times? What I am able to reproduce is a failure on very first camera access after boot. Then, it seems working as expected for subsequent accesses, as long as there is no other activity (VoIP or video over bluetooth PAN in my case). Then, it usualy fails once, and starts working back againg. However, it happened once that it broke permanently after a bluetooth related error. Without the work-around, it works after boot for ca. 50% tries, then, after some VoIP or video over PAN activity, refuses to allocate memory for DMA any longer. IOW, the work-around helps, but doesn't provide a 100% guarrantee. Can you verify, that you're not leaking memory? That you're freeing all allocated DMA memory again? Are you using the same parameters to mplayer, right? I've been using the same testing procedure for the last 2-3 months, since I started the driver development. It never failed to allocate DMA memory for SG mode. As for the work-around - can you not do this in your board late-initcall function? One of the arguments of this custom fuction is the camera interface device structre pointer. I don't know how I could get access to this pointer from my board code. Not sure whether and how one can get this in the mainline. This is in principle the same, as in the above dma_declare_coherent_memory() example, only open-coded without the ioremap. Maybe we can add a suitable function to the dma-alloc API... With my limited skills, I'd rather wait for a solution promissed by RMK for next merge window ;). Thanks, Janusz Thanks Guennadi [ 6067.22] omap1-camera omap1-camera.0: OMAP1 Camera driver attached to camera 0 [ 6067.65] omap1-camera omap1-camera.0: omap1_cam_try_fmt: format 32315659 not found [ 6067.68] omap1-camera omap1-camera.0: omap1_cam_try_fmt: format 32315559 not found [ 6068.48] mplayer: page allocation failure. order:6, mode:0xd0 [ 6068.50] Backtrace: [ 6068.52] [c0028950] (dump_backtrace+0x0/0x110) from [c0028ea8] (dump_stack+0x18/0x1c) [ 6068.56] r6:0006 r5:00d0 r4:c1bcf000 [ 6068.59] [c0028e90] (dump_stack+0x0/0x1c) from [c0074e24] (__alloc_pages_nodemask+0x504/0x560) [ 6068.62] [c0074920] (__alloc_pages_nodemask+0x0/0x560) from [c002ae14] (__dma_alloc+0x108/0x354) [ 6068.66] [c002ad0c] (__dma_alloc+0x0/0x354) from [c002b0ec] (dma_alloc_coherent+0x58/0x64) [ 6068.70] [c002b094] (dma_alloc_coherent+0x0/0x64) from [bf000a44] (__videobuf_mmap_mapper+0x10c/0x374 [videobuf_dma_contig]) [ 6068.74] r7:c16934c0 r6: r5:c171baec r4: [ 6068.77] [bf000938] (__videobuf_mmap_mapper+0x0/0x374 [videobuf_dma_contig]) from [c01f9a78] (videobuf_mmap_mapper+0xc4/0x108) [ 6068.81] [c01f99b4] (videobuf_mmap_mapper+0x0/0x108) from [c01fc1ac] (soc_camera_mmap+0x80/0x140) [ 6068.84] r5:c1a3b4e0 r4: [ 6068.87] [c01fc12c]
Re: [RFC] [PATCH 1/6] SoC Camera: add driver for OMAP1 camera interface
Saturday 14 August 2010 06:47:56 Marin Mitov napisał(a): On Friday, August 13, 2010 10:13:08 pm Janusz Krzysztofik wrote: Friday 13 August 2010 11:11:52 Marin Mitov napisał(a): On Friday, August 13, 2010 11:52:41 am Guennadi Liakhovetski wrote: On Fri, 13 Aug 2010, Janusz Krzysztofik wrote: Thursday 12 August 2010 23:38:17 Guennadi Liakhovetski napisał(a): 1. We've discussed this dynamic switching a bit on IRC today. The first reaction was - you probably should concentrate on getting the contiguous version to work reliably. I.e., to reserve the memory in the board init code similar, how other contig users currently do it. I already tried before to find out how I could allocate memory at init without reinventing a new videobuf-dma-contig implementation. Since in the Documentation/video4linux/videobuf I've read that videobuf does not currently play well with drivers that play tricks by allocating DMA space at system boot time, I've implemented the alternate sg path. If it's not quite true what the documentation says and you can give me a hint how this could be done, I might try again. For an example look at arch/arm/mach-mx3/mach-pcm037.c::pcm037_camera_alloc_dma(). Yes, this is the solution that suffers from the already discussed limitation of not being able to remap a memory with different attributes, which affects OMAP1 as well. For preallocating dma-coherent memory for device personal use during device probe() time (when the memory is less fragmented compared to open() time) see also dt3155_alloc_coherent/dt3155_free_coherent in drivers/staging/dt3155v4l/dt3155vfl.c (for x86 arch, I do not know if it works for arm arch) With this workaround applied, I get much better results, thank you Marin. However, it seems not bullet proof, since mmap still happens to fail for a reason not quite clear to me: This is just a preallocation of coherent memory kept for further private driver use, should not be connected to mmap problem. I'm not sure if it is related or not. This mmap problem exhibits ultimately when my driver is trying to allocate a piece of memory that has been preallocated that custom way. Maybe I should preallocate a few more pages than will be actually used by the driver? Anyways, I'm not sure if this piece of code could be accepted for inclusion into the mainline tree, perhaps only under drivers/staging. The idea for the piece of code I have proposed to you is taken from the functions dma_declare_coherent_memory()/dma_release_declared_memory() in mainline drivers/base/dma-coherent.c Then why not using the dma_declare_coherent_memory(), possibly after providing a patch that corrects a problem with it if there is one, instead of reimplementing it inside a driver? If I understood what a difference it could make if we put a dma_alloc_coherent() returned virtual address space pointer to the allocated region, instead of the ioremapped physical address base of this region, to the dev-dma_mem-virt_base, then maybe I could say something more on this. Thanks, Janusz -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] [PATCH 1/6] SoC Camera: add driver for OMAP1 camera interface
On Fri, 13 Aug 2010, Janusz Krzysztofik wrote: Friday 13 August 2010 11:11:52 Marin Mitov napisał(a): On Friday, August 13, 2010 11:52:41 am Guennadi Liakhovetski wrote: On Fri, 13 Aug 2010, Janusz Krzysztofik wrote: Thursday 12 August 2010 23:38:17 Guennadi Liakhovetski napisał(a): 1. We've discussed this dynamic switching a bit on IRC today. The first reaction was - you probably should concentrate on getting the contiguous version to work reliably. I.e., to reserve the memory in the board init code similar, how other contig users currently do it. I already tried before to find out how I could allocate memory at init without reinventing a new videobuf-dma-contig implementation. Since in the Documentation/video4linux/videobuf I've read that videobuf does not currently play well with drivers that play tricks by allocating DMA space at system boot time, I've implemented the alternate sg path. If it's not quite true what the documentation says and you can give me a hint how this could be done, I might try again. For an example look at arch/arm/mach-mx3/mach-pcm037.c::pcm037_camera_alloc_dma(). Yes, this is the solution that suffers from the already discussed limitation of not being able to remap a memory with different attributes, which affects OMAP1 as well. For preallocating dma-coherent memory for device personal use during device probe() time (when the memory is less fragmented compared to open() time) see also dt3155_alloc_coherent/dt3155_free_coherent in drivers/staging/dt3155v4l/dt3155vfl.c (for x86 arch, I do not know if it works for arm arch) With this workaround applied, I get much better results, thank you Marin. However, it seems not bullet proof, since mmap still happens to fail for a reason not quite clear to me: What exactly does this mean - happens to fail - you mean starting and stopping mplayer several times? Can you verify, that you're not leaking memory? That you're freeing all allocated DMA memory again? Are you using the same parameters to mplayer, right? As for the work-around - can you not do this in your board late-initcall function? Not sure whether and how one can get this in the mainline. This is in principle the same, as in the above dma_declare_coherent_memory() example, only open-coded without the ioremap. Maybe we can add a suitable function to the dma-alloc API... Thanks Guennadi [ 6067.22] omap1-camera omap1-camera.0: OMAP1 Camera driver attached to camera 0 [ 6067.65] omap1-camera omap1-camera.0: omap1_cam_try_fmt: format 32315659 not found [ 6067.68] omap1-camera omap1-camera.0: omap1_cam_try_fmt: format 32315559 not found [ 6068.48] mplayer: page allocation failure. order:6, mode:0xd0 [ 6068.50] Backtrace: [ 6068.52] [c0028950] (dump_backtrace+0x0/0x110) from [c0028ea8] (dump_stack+0x18/0x1c) [ 6068.56] r6:0006 r5:00d0 r4:c1bcf000 [ 6068.59] [c0028e90] (dump_stack+0x0/0x1c) from [c0074e24] (__alloc_pages_nodemask+0x504/0x560) [ 6068.62] [c0074920] (__alloc_pages_nodemask+0x0/0x560) from [c002ae14] (__dma_alloc+0x108/0x354) [ 6068.66] [c002ad0c] (__dma_alloc+0x0/0x354) from [c002b0ec] (dma_alloc_coherent+0x58/0x64) [ 6068.70] [c002b094] (dma_alloc_coherent+0x0/0x64) from [bf000a44] (__videobuf_mmap_mapper+0x10c/0x374 [videobuf_dma_contig]) [ 6068.74] r7:c16934c0 r6: r5:c171baec r4: [ 6068.77] [bf000938] (__videobuf_mmap_mapper+0x0/0x374 [videobuf_dma_contig]) from [c01f9a78] (videobuf_mmap_mapper+0xc4/0x108) [ 6068.81] [c01f99b4] (videobuf_mmap_mapper+0x0/0x108) from [c01fc1ac] (soc_camera_mmap+0x80/0x140) [ 6068.84] r5:c1a3b4e0 r4: [ 6068.87] [c01fc12c] (soc_camera_mmap+0x0/0x140) from [c01eeba8] (v4l2_mmap+0x4c/0x5c) [ 6068.90] r7:c145c000 r6:00ff r5:c16934c0 r4: [ 6068.93] [c01eeb5c] (v4l2_mmap+0x0/0x5c) from [c0085de4] (mmap_region+0x238/0x458) [ 6068.97] [c0085bac] (mmap_region+0x0/0x458) from [c00862c0] (do_mmap_pgoff+0x2bc/0x320) [ 6069.00] [c0086004] (do_mmap_pgoff+0x0/0x320) from [c00863c0] (sys_mmap_pgoff+0x9c/0xc8) [ 6069.04] [c0086324] (sys_mmap_pgoff+0x0/0xc8) from [c0024f00] (ret_fast_syscall+0x0/0x2c) [ 6069.20] Mem-info: [ 6069.22] Normal per-cpu: [ 6069.24] CPU0: hi:0, btch: 1 usd: 0 [ 6069.26] active_anon:676 inactive_anon:682 isolated_anon:0 [ 6069.26] active_file:422 inactive_file:2348 isolated_file:0 [ 6069.26] unevictable:177 dirty:0 writeback:0 unstable:0 [ 6069.26] free:1166 slab_reclaimable:0 slab_unreclaimable:0 [ 6069.26] mapped:1120 shmem:0 pagetables:121 bounce:0 [ 6069.35] Normal free:4664kB min:720kB low:900kB high:1080kB active_anon:2704kB inactive_anon:2728kB active_file:1688kB inactive_file:9392kB unevictable:708kB isolated(anon):0kB isolated(file):0kB present:32512kB mlocked:0kB
Re: [RFC] [PATCH 1/6] SoC Camera: add driver for OMAP1 camera interface
Thursday 12 August 2010 23:38:17 Guennadi Liakhovetski napisał(a): On Sun, 1 Aug 2010, Janusz Krzysztofik wrote: Friday 30 July 2010 20:49:05 Janusz Krzysztofik napisaÅ(a): I think the right way would be if implemented at the videobuf-core level. Then, drivers should be able to initialize both paths, providing queue callbacks for both sets of methods, contig and sg, for videobuf sole use. Ok, here're my thoughts about this: 1. We've discussed this dynamic switching a bit on IRC today. The first reaction was - you probably should concentrate on getting the contiguous version to work reliably. I.e., to reserve the memory in the board init code similar, how other contig users currently do it. I already tried before to find out how I could allocate memory at init without reinventing a new videobuf-dma-contig implementation. Since in the Documentation/video4linux/videobuf I've read that videobuf does not currently play well with drivers that play tricks by allocating DMA space at system boot time, I've implemented the alternate sg path. If it's not quite true what the documentation says and you can give me a hint how this could be done, I might try again. But given problems with this aproach in the current ARM tree [1], this might be a bit difficult. Still, those problems have to be and will be fixed somehow eventually, so, you might prefer to still just go that route. My board uses two drivers that allocate dma memory at boot time: drivers/video/omap/lcdc.c and sounc/soc/omap/omap-pcm.c. Both use alloc_dma_writecombine() for this and work without problems. 2. If you do want to do the switching - we also discussed, how forthcoming changes to the videobuf subsystem will affest this work. I do not think it would be possible to implement this switching in the videobuf core. OK, I should have probably said that it looked not possible for me to do it without any additional support implemented at videobuf-core (or soc_camera) level. Remember, with the videobuf API you first call the respective implementation init method, which doesn't fail. Then, in REQBUFS ioctl you call videobuf_reqbufs(), which might already fail but normally doesn't. The biggest problem is the mmap call with the contig videobuf implementation. This one is likely to fail. So, you would have to catch the failing mmap, call videobuf_mmap_free(), then init the SG videobuf, request buffers and mmap them... That's what I've already discovered, but failed to identify a place in my code where I could intercept this failing mmap without replacing parts of the videobuf code. With my 2 patches from today, there is only one process (file descriptor, to be precise), that manages the videobuf queue. So, this all can only be implemented in your driver. The only way I'm yet able to invent is replacing the videobuf_queue-int_ops-mmap_mapper() callback with my own wrapper that would intercept a failing videobuf-dma-contig version of mmap_mapper(). This could be done in my soc_camera_host-ops-init_videobuf() after the videobuf-dma-contig.c version of the videobuf_queue-int_ops-mmap_mapper() is installed with the videobuf_queue_dma_contig_init(). Is this method close to what you have on mind? Thanks, Janusz -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] [PATCH 1/6] SoC Camera: add driver for OMAP1 camera interface
On Fri, 13 Aug 2010, Janusz Krzysztofik wrote: Thursday 12 August 2010 23:38:17 Guennadi Liakhovetski napisał(a): On Sun, 1 Aug 2010, Janusz Krzysztofik wrote: Friday 30 July 2010 20:49:05 Janusz Krzysztofik napisaÅ(a): I think the right way would be if implemented at the videobuf-core level. Then, drivers should be able to initialize both paths, providing queue callbacks for both sets of methods, contig and sg, for videobuf sole use. Ok, here're my thoughts about this: 1. We've discussed this dynamic switching a bit on IRC today. The first reaction was - you probably should concentrate on getting the contiguous version to work reliably. I.e., to reserve the memory in the board init code similar, how other contig users currently do it. I already tried before to find out how I could allocate memory at init without reinventing a new videobuf-dma-contig implementation. Since in the Documentation/video4linux/videobuf I've read that videobuf does not currently play well with drivers that play tricks by allocating DMA space at system boot time, I've implemented the alternate sg path. If it's not quite true what the documentation says and you can give me a hint how this could be done, I might try again. For an example look at arch/arm/mach-mx3/mach-pcm037.c::pcm037_camera_alloc_dma(). But given problems with this aproach in the current ARM tree [1], this might be a bit difficult. Still, those problems have to be and will be fixed somehow eventually, so, you might prefer to still just go that route. My board uses two drivers that allocate dma memory at boot time: drivers/video/omap/lcdc.c and sounc/soc/omap/omap-pcm.c. Both use alloc_dma_writecombine() for this and work without problems. dma_alloc_writecombine() also allocates contiguous RAM, right? And it doesn't use device local memory. So, it's chances to fail are the same as those of dma_alloc_coherent() in the absence of device own memory. I guess, the sound driver doesn't need much RAM, but if you build your LCDC driver as a module and load it later after startup, it might get problems allocating RAM for the framebuffer. 2. If you do want to do the switching - we also discussed, how forthcoming changes to the videobuf subsystem will affest this work. I do not think it would be possible to implement this switching in the videobuf core. OK, I should have probably said that it looked not possible for me to do it without any additional support implemented at videobuf-core (or soc_camera) level. Remember, with the videobuf API you first call the respective implementation init method, which doesn't fail. Then, in REQBUFS ioctl you call videobuf_reqbufs(), which might already fail but normally doesn't. The biggest problem is the mmap call with the contig videobuf implementation. This one is likely to fail. So, you would have to catch the failing mmap, call videobuf_mmap_free(), then init the SG videobuf, request buffers and mmap them... That's what I've already discovered, but failed to identify a place in my code where I could intercept this failing mmap without replacing parts of the videobuf code. Right, ATM soc-camera just calls videobuf_mmap_mapper() directly in its mmap method. I could add a callback to struct soc_camera_host_ops like int (*mmap)(struct soc_camera_device *, struct vm_area_struct *) and modify soc_camera_mmap() to check, whether the host driver has implemented it. If so - call it, otherwise call videobuf_mmap_mapper() directly just like now. So, other drivers would not have to be modified. And you could implement that .mmap() method, call videobuf_mmap_mapper() yourself, and if it fails for contig, fall back to SG. With my 2 patches from today, there is only one process (file descriptor, to be precise), that manages the videobuf queue. So, this all can only be implemented in your driver. The only way I'm yet able to invent is replacing the videobuf_queue-int_ops-mmap_mapper() callback with my own wrapper that would intercept a failing videobuf-dma-contig version of mmap_mapper(). This could be done in my soc_camera_host-ops-init_videobuf() after the videobuf-dma-contig.c version of the videobuf_queue-int_ops-mmap_mapper() is installed with the videobuf_queue_dma_contig_init(). Is this method close to what you have on mind? See, if the above idea would suit your needs. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] [PATCH 1/6] SoC Camera: add driver for OMAP1 camera interface
On Friday, August 13, 2010 11:52:41 am Guennadi Liakhovetski wrote: On Fri, 13 Aug 2010, Janusz Krzysztofik wrote: Thursday 12 August 2010 23:38:17 Guennadi Liakhovetski napisał(a): On Sun, 1 Aug 2010, Janusz Krzysztofik wrote: Friday 30 July 2010 20:49:05 Janusz Krzysztofik napisaÅ(a): I think the right way would be if implemented at the videobuf-core level. Then, drivers should be able to initialize both paths, providing queue callbacks for both sets of methods, contig and sg, for videobuf sole use. Ok, here're my thoughts about this: 1. We've discussed this dynamic switching a bit on IRC today. The first reaction was - you probably should concentrate on getting the contiguous version to work reliably. I.e., to reserve the memory in the board init code similar, how other contig users currently do it. I already tried before to find out how I could allocate memory at init without reinventing a new videobuf-dma-contig implementation. Since in the Documentation/video4linux/videobuf I've read that videobuf does not currently play well with drivers that play tricks by allocating DMA space at system boot time, I've implemented the alternate sg path. If it's not quite true what the documentation says and you can give me a hint how this could be done, I might try again. For an example look at arch/arm/mach-mx3/mach-pcm037.c::pcm037_camera_alloc_dma(). For preallocating dma-coherent memory for device personal use during device probe() time (when the memory is less fragmented compared to open() time) see also dt3155_alloc_coherent/dt3155_free_coherent in drivers/staging/dt3155v4l/dt3155vfl.c (for x86 arch, I do not know if it works for arm arch) But given problems with this aproach in the current ARM tree [1], this might be a bit difficult. Still, those problems have to be and will be fixed somehow eventually, so, you might prefer to still just go that route. My board uses two drivers that allocate dma memory at boot time: drivers/video/omap/lcdc.c and sounc/soc/omap/omap-pcm.c. Both use alloc_dma_writecombine() for this and work without problems. dma_alloc_writecombine() also allocates contiguous RAM, right? And it doesn't use device local memory. So, it's chances to fail are the same as those of dma_alloc_coherent() in the absence of device own memory. I guess, the sound driver doesn't need much RAM, but if you build your LCDC driver as a module and load it later after startup, it might get problems allocating RAM for the framebuffer. 2. If you do want to do the switching - we also discussed, how forthcoming changes to the videobuf subsystem will affest this work. I do not think it would be possible to implement this switching in the videobuf core. OK, I should have probably said that it looked not possible for me to do it without any additional support implemented at videobuf-core (or soc_camera) level. Remember, with the videobuf API you first call the respective implementation init method, which doesn't fail. Then, in REQBUFS ioctl you call videobuf_reqbufs(), which might already fail but normally doesn't. The biggest problem is the mmap call with the contig videobuf implementation. This one is likely to fail. So, you would have to catch the failing mmap, call videobuf_mmap_free(), then init the SG videobuf, request buffers and mmap them... That's what I've already discovered, but failed to identify a place in my code where I could intercept this failing mmap without replacing parts of the videobuf code. Right, ATM soc-camera just calls videobuf_mmap_mapper() directly in its mmap method. I could add a callback to struct soc_camera_host_ops like int (*mmap)(struct soc_camera_device *, struct vm_area_struct *) and modify soc_camera_mmap() to check, whether the host driver has implemented it. If so - call it, otherwise call videobuf_mmap_mapper() directly just like now. So, other drivers would not have to be modified. And you could implement that .mmap() method, call videobuf_mmap_mapper() yourself, and if it fails for contig, fall back to SG. With my 2 patches from today, there is only one process (file descriptor, to be precise), that manages the videobuf queue. So, this all can only be implemented in your driver. The only way I'm yet able to invent is replacing the videobuf_queue-int_ops-mmap_mapper() callback with my own wrapper that would intercept a failing videobuf-dma-contig version of mmap_mapper(). This could be done in my soc_camera_host-ops-init_videobuf() after the videobuf-dma-contig.c version of the videobuf_queue-int_ops-mmap_mapper() is installed with the videobuf_queue_dma_contig_init(). Is this method close to what you have on mind? See, if the above idea would suit your needs. Thanks Guennadi --- Guennadi Liakhovetski,
Re: [RFC] [PATCH 1/6] SoC Camera: add driver for OMAP1 camera interface
Friday 13 August 2010 11:11:52 Marin Mitov napisał(a): On Friday, August 13, 2010 11:52:41 am Guennadi Liakhovetski wrote: On Fri, 13 Aug 2010, Janusz Krzysztofik wrote: Thursday 12 August 2010 23:38:17 Guennadi Liakhovetski napisał(a): 1. We've discussed this dynamic switching a bit on IRC today. The first reaction was - you probably should concentrate on getting the contiguous version to work reliably. I.e., to reserve the memory in the board init code similar, how other contig users currently do it. I already tried before to find out how I could allocate memory at init without reinventing a new videobuf-dma-contig implementation. Since in the Documentation/video4linux/videobuf I've read that videobuf does not currently play well with drivers that play tricks by allocating DMA space at system boot time, I've implemented the alternate sg path. If it's not quite true what the documentation says and you can give me a hint how this could be done, I might try again. For an example look at arch/arm/mach-mx3/mach-pcm037.c::pcm037_camera_alloc_dma(). Yes, this is the solution that suffers from the already discussed limitation of not being able to remap a memory with different attributes, which affects OMAP1 as well. For preallocating dma-coherent memory for device personal use during device probe() time (when the memory is less fragmented compared to open() time) see also dt3155_alloc_coherent/dt3155_free_coherent in drivers/staging/dt3155v4l/dt3155vfl.c (for x86 arch, I do not know if it works for arm arch) With this workaround applied, I get much better results, thank you Marin. However, it seems not bullet proof, since mmap still happens to fail for a reason not quite clear to me: [ 6067.22] omap1-camera omap1-camera.0: OMAP1 Camera driver attached to camera 0 [ 6067.65] omap1-camera omap1-camera.0: omap1_cam_try_fmt: format 32315659 not found [ 6067.68] omap1-camera omap1-camera.0: omap1_cam_try_fmt: format 32315559 not found [ 6068.48] mplayer: page allocation failure. order:6, mode:0xd0 [ 6068.50] Backtrace: [ 6068.52] [c0028950] (dump_backtrace+0x0/0x110) from [c0028ea8] (dump_stack+0x18/0x1c) [ 6068.56] r6:0006 r5:00d0 r4:c1bcf000 [ 6068.59] [c0028e90] (dump_stack+0x0/0x1c) from [c0074e24] (__alloc_pages_nodemask+0x504/0x560) [ 6068.62] [c0074920] (__alloc_pages_nodemask+0x0/0x560) from [c002ae14] (__dma_alloc+0x108/0x354) [ 6068.66] [c002ad0c] (__dma_alloc+0x0/0x354) from [c002b0ec] (dma_alloc_coherent+0x58/0x64) [ 6068.70] [c002b094] (dma_alloc_coherent+0x0/0x64) from [bf000a44] (__videobuf_mmap_mapper+0x10c/0x374 [videobuf_dma_contig]) [ 6068.74] r7:c16934c0 r6: r5:c171baec r4: [ 6068.77] [bf000938] (__videobuf_mmap_mapper+0x0/0x374 [videobuf_dma_contig]) from [c01f9a78] (videobuf_mmap_mapper+0xc4/0x108) [ 6068.81] [c01f99b4] (videobuf_mmap_mapper+0x0/0x108) from [c01fc1ac] (soc_camera_mmap+0x80/0x140) [ 6068.84] r5:c1a3b4e0 r4: [ 6068.87] [c01fc12c] (soc_camera_mmap+0x0/0x140) from [c01eeba8] (v4l2_mmap+0x4c/0x5c) [ 6068.90] r7:c145c000 r6:00ff r5:c16934c0 r4: [ 6068.93] [c01eeb5c] (v4l2_mmap+0x0/0x5c) from [c0085de4] (mmap_region+0x238/0x458) [ 6068.97] [c0085bac] (mmap_region+0x0/0x458) from [c00862c0] (do_mmap_pgoff+0x2bc/0x320) [ 6069.00] [c0086004] (do_mmap_pgoff+0x0/0x320) from [c00863c0] (sys_mmap_pgoff+0x9c/0xc8) [ 6069.04] [c0086324] (sys_mmap_pgoff+0x0/0xc8) from [c0024f00] (ret_fast_syscall+0x0/0x2c) [ 6069.20] Mem-info: [ 6069.22] Normal per-cpu: [ 6069.24] CPU0: hi:0, btch: 1 usd: 0 [ 6069.26] active_anon:676 inactive_anon:682 isolated_anon:0 [ 6069.26] active_file:422 inactive_file:2348 isolated_file:0 [ 6069.26] unevictable:177 dirty:0 writeback:0 unstable:0 [ 6069.26] free:1166 slab_reclaimable:0 slab_unreclaimable:0 [ 6069.26] mapped:1120 shmem:0 pagetables:121 bounce:0 [ 6069.35] Normal free:4664kB min:720kB low:900kB high:1080kB active_anon:2704kB inactive_anon:2728kB active_file:1688kB inactive_file:9392kB unevictable:708kB isolated(anon):0kB isolated(file):0kB present:32512kB mlocked:0kB dirty:0kB writeback:0kB mapped:4480kB shmem:0kB slab_reclaimable:0kB slab_unreclaimable:0kB kernel_stack:552kB pagetables:484kB unstable:0kB bounce:0kB writeback_tmp:0kB pages_scanned:0 all_unreclaimable? no [ 6069.46] lowmem_reserve[]: 0 0 [ 6069.47] Normal: 6*4kB 20*8kB 14*16kB 29*32kB 26*64kB 9*128kB 2*256kB 0*512kB 0*1024kB 0*2048kB 0*4096kB = 4664kB [ 6069.53] 2960 total pagecache pages [ 6069.55] 8192 pages of RAM [ 6069.56] 1322 free pages [ 6069.58] 1114 reserved pages [ 6069.59] 750 slab pages [ 6069.61] 2476 pages shared [ 6069.63] 0 pages swap cached [ 6069.64] omap1-camera omap1-camera.0: dma_alloc_coherent size 204800 failed [ 6069.68] omap1-camera omap1-camera.0: OMAP1
Re: [RFC] [PATCH 1/6] SoC Camera: add driver for OMAP1 camera interface
On Friday, August 13, 2010 10:13:08 pm Janusz Krzysztofik wrote: Friday 13 August 2010 11:11:52 Marin Mitov napisał(a): On Friday, August 13, 2010 11:52:41 am Guennadi Liakhovetski wrote: On Fri, 13 Aug 2010, Janusz Krzysztofik wrote: Thursday 12 August 2010 23:38:17 Guennadi Liakhovetski napisał(a): 1. We've discussed this dynamic switching a bit on IRC today. The first reaction was - you probably should concentrate on getting the contiguous version to work reliably. I.e., to reserve the memory in the board init code similar, how other contig users currently do it. I already tried before to find out how I could allocate memory at init without reinventing a new videobuf-dma-contig implementation. Since in the Documentation/video4linux/videobuf I've read that videobuf does not currently play well with drivers that play tricks by allocating DMA space at system boot time, I've implemented the alternate sg path. If it's not quite true what the documentation says and you can give me a hint how this could be done, I might try again. For an example look at arch/arm/mach-mx3/mach-pcm037.c::pcm037_camera_alloc_dma(). Yes, this is the solution that suffers from the already discussed limitation of not being able to remap a memory with different attributes, which affects OMAP1 as well. For preallocating dma-coherent memory for device personal use during device probe() time (when the memory is less fragmented compared to open() time) see also dt3155_alloc_coherent/dt3155_free_coherent in drivers/staging/dt3155v4l/dt3155vfl.c (for x86 arch, I do not know if it works for arm arch) With this workaround applied, I get much better results, thank you Marin. However, it seems not bullet proof, since mmap still happens to fail for a reason not quite clear to me: This is just a preallocation of coherent memory kept for further private driver use, should not be connected to mmap problem. Maybe I should preallocate a few more pages than will be actually used by the driver? Anyways, I'm not sure if this piece of code could be accepted for inclusion into the mainline tree, perhaps only under drivers/staging. The idea for the piece of code I have proposed to you is taken from the functions dma_declare_coherent_memory()/dma_release_declared_memory() in mainline drivers/base/dma-coherent.c Thanks, Janusz -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] [PATCH 1/6] SoC Camera: add driver for OMAP1 camera interface
On Sun, 1 Aug 2010, Janusz Krzysztofik wrote: Friday 30 July 2010 20:49:05 Janusz Krzysztofik napisaÅ(a): Friday 30 July 2010 13:07:42 Guennadi Liakhovetski napisaÅ(a): On Sun, 18 Jul 2010, Janusz Krzysztofik wrote: This is a V4L2 driver for TI OMAP1 SoC camera interface. Two versions of the driver are provided, using either videobuf-dma-contig or videobuf-dma-sg. The former uses less processing power, but often fails to allocate contignuous buffer memory. The latter is free of this problem, but generates tens of DMA interrupts per frame. Hm, would it be difficult to first try contig, and if it fails - fall back to sg? Hmm, let me think about it. Hi Gennadi, For me, your idea of frist trying contig and then falling back to sg if it fails, sounds great. However, I'm not sure if implementing it at a specific hardware driver level is a good solution. Nor soc_camera framework seems the right place for it either. I think the right way would be if implemented at the videobuf-core level. Then, drivers should be able to initialize both paths, providing queue callbacks for both sets of methods, contig and sg, for videobuf sole use. Ok, here're my thoughts about this: 1. We've discussed this dynamic switching a bit on IRC today. The first reaction was - you probably should concentrate on getting the contiguous version to work reliably. I.e., to reserve the memory in the board init code similar, how other contig users currently do it. But given problems with this aproach in the current ARM tree [1], this might be a bit difficult. Still, those problems have to be and will be fixed somehow eventually, so, you might prefer to still just go that route. 2. If you do want to do the switching - we also discussed, how forthcoming changes to the videobuf subsystem will affest this work. I do not think it would be possible to implement this switching in the videobuf core. Remember, with the videobuf API you first call the respective implementation init method, which doesn't fail. Then, in REQBUFS ioctl you call videobuf_reqbufs(), which might already fail but normally doesn't. The biggest problem is the mmap call with the contig videobuf implementation. This one is likely to fail. So, you would have to catch the failing mmap, call videobuf_mmap_free(), then init the SG videobuf, request buffers and mmap them... With my 2 patches from today, there is only one process (file descriptor, to be precise), that manages the videobuf queue. So, this all can only be implemented in your driver. [1] http://thread.gmane.org/gmane.linux.ports.sh.devel/8560 I'm not sure if I have enough skills to implement this idea. However, if there is a consensus on its general usfullness as a videobuf extension, I can have a look at it in my spare time. For now, I'd propose limiting my changes for v2 to splitting both methods into separate sections, not interleaved with #ifdefs like they are now, as you've already suggested. Yep, so, I think, your choice is either to drop sg completely, or to separate the two cleanly and switch between them dynamically in your driver. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] [PATCH 1/6] SoC Camera: add driver for OMAP1 camera interface
Friday 30 July 2010 20:49:05 Janusz Krzysztofik napisał(a): Friday 30 July 2010 13:07:42 Guennadi Liakhovetski napisał(a): On Sun, 18 Jul 2010, Janusz Krzysztofik wrote: This is a V4L2 driver for TI OMAP1 SoC camera interface. Two versions of the driver are provided, using either videobuf-dma-contig or videobuf-dma-sg. The former uses less processing power, but often fails to allocate contignuous buffer memory. The latter is free of this problem, but generates tens of DMA interrupts per frame. Hm, would it be difficult to first try contig, and if it fails - fall back to sg? Hmm, let me think about it. Hi Gennadi, For me, your idea of frist trying contig and then falling back to sg if it fails, sounds great. However, I'm not sure if implementing it at a specific hardware driver level is a good solution. Nor soc_camera framework seems the right place for it either. I think the right way would be if implemented at the videobuf-core level. Then, drivers should be able to initialize both paths, providing queue callbacks for both sets of methods, contig and sg, for videobuf sole use. I'm not sure if I have enough skills to implement this idea. However, if there is a consensus on its general usfullness as a videobuf extension, I can have a look at it in my spare time. For now, I'd propose limiting my changes for v2 to splitting both methods into separate sections, not interleaved with #ifdefs like they are now, as you've already suggested. Thanks, Janusz -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] [PATCH 1/6] SoC Camera: add driver for OMAP1 camera interface
Hi Janusz
Thanks once more for the patches, from your patches it is obvious, that
you've spent quite a bit of time on them and you have not just copy-pasted
various bits and pieces from other drivers, just filling your hardware
details, but you also actually understand a lot of what is actually going
on in there. So, I think, fixing remaining mostly minor issues shouldn't
be too difficult for you either.
On Sun, 18 Jul 2010, Janusz Krzysztofik wrote:
This is a V4L2 driver for TI OMAP1 SoC camera interface.
Two versions of the driver are provided, using either videobuf-dma-contig or
videobuf-dma-sg. The former uses less processing power, but often fails to
allocate contignuous buffer memory. The latter is free of this problem, but
generates tens of DMA interrupts per frame.
Hm, would it be difficult to first try contig, and if it fails - fall back
to sg?
In its present state buffer management in your driver is pretty difficult
to follow. At the VERY least you have to extensively document your buffer
manipulations. Better yet clean it up. AFAIU, your ready pointer is
pretty much unused in the contiguous case. Or you can easily get rid of
it. So, I think, a very welcome improvement to the driver would be a
cleaner separation between the two cases. Don't try that much to reuse the
code as much as possible. Would be much better to have clean separation
between the two implementations - whether dynamically switchable at
runtime or at config time - would be best to separate the two
implementations to the point, where each of them becomes understandable
and maintainable.
So, I will do a pretty formal review this time and wait for v2. And in v2
I'd like to at the very least see detailed buffer-management
documentation, or - better yet - a rework with a clean separation.
Both versions work stable for me,
even under heavy load, on my OMAP1510 based Amstrad Delta videophone, that is
the oldest, least powerfull OMAP1 implementation.
The interface generally works in pass-through mode. Since input data byte
endianess can be swapped, it provides up to two v4l2 pixel formats per each
of
several soc_mbus formats that have their swapped endian counterparts.
Boards using this driver can provide it with the followning information:
- if and what freqency clock is expected by an on-board camera sensor,
- what is the maximum pixel clock that should be accepted from the sensor,
- what is the polarity of the sensor provided pixel clock,
- if the interface GPIO line is connected to a sensor reset/powerdown input
and what is the input polarity.
Created and tested against linux-2.6.35-rc3 on Amstrad Delta.
Signed-off-by: Janusz Krzysztofik jkrzy...@tis.icnet.pl
---
drivers/media/video/Kconfig| 14
drivers/media/video/Makefile |1
drivers/media/video/omap1_camera.c | 1656
+
include/media/omap1_camera.h | 16
4 files changed, 1687 insertions(+)
--- linux-2.6.35-rc3.orig/include/media/omap1_camera.h1970-01-01
01:00:00.0 +0100
+++ linux-2.6.35-rc3/include/media/omap1_camera.h 2010-07-18
01:31:57.0 +0200
@@ -0,0 +1,16 @@
A copyright / licence header here, please
+#ifndef __MEDIA_OMAP1_CAMERA_H_
+#define __MEDIA_OMAP1_CAMERA_H_
+
+#define OMAP1_CAMERA_IOSIZE 0x1c
+
+struct omap1_cam_platform_data {
+ unsigned long camexclk_khz;
+ unsigned long lclk_khz_max;
+ unsigned long flags;
+};
+
+#define OMAP1_CAMERA_LCLK_RISING BIT(0)
+#define OMAP1_CAMERA_RST_LOW BIT(1)
+#define OMAP1_CAMERA_RST_HIGHBIT(2)
Then you need to #include linux/bitops.h
+
+#endif /* __MEDIA_OMAP1_CAMERA_H_ */
--- linux-2.6.35-rc3.orig/drivers/media/video/Kconfig 2010-06-26
15:55:29.0 +0200
+++ linux-2.6.35-rc3/drivers/media/video/Kconfig 2010-07-02
04:12:02.0 +0200
@@ -962,6 +962,20 @@ config VIDEO_SH_MOBILE_CEU
---help---
This is a v4l2 driver for the SuperH Mobile CEU Interface
+config VIDEO_OMAP1
+ tristate OMAP1 Camera Interface driver
+ depends on VIDEO_DEV ARCH_OMAP1 SOC_CAMERA
+ select VIDEOBUF_DMA_CONTIG if !VIDEO_OMAP1_SG
+ ---help---
+ This is a v4l2 driver for the TI OMAP1 camera interface
+
+if VIDEO_OMAP1
Don't think you need this if, the depends on below should suffice.
+config VIDEO_OMAP1_SG
+ bool Scatter-gather mode
+ depends on VIDEO_OMAP1 EXPERIMENTAL
+ select VIDEOBUF_DMA_SG
+endif
+
config VIDEO_OMAP2
tristate OMAP2 Camera Capture Interface driver
depends on VIDEO_DEV ARCH_OMAP2
--- linux-2.6.35-rc3.orig/drivers/media/video/Makefile2010-06-26
15:55:29.0 +0200
+++ linux-2.6.35-rc3/drivers/media/video/Makefile 2010-06-26
17:28:09.0 +0200
@@ -165,6 +165,7 @@ obj-$(CONFIG_VIDEO_MX1) += mx1_camera.
obj-$(CONFIG_VIDEO_MX3) +=
Re: [RFC] [PATCH 1/6] SoC Camera: add driver for OMAP1 camera interface
Friday 30 July 2010 13:07:42 Guennadi Liakhovetski napisał(a):
Hi Janusz
Thanks once more for the patches, from your patches it is obvious, that
you've spent quite a bit of time on them and you have not just copy-pasted
various bits and pieces from other drivers, just filling your hardware
details, but you also actually understand a lot of what is actually going
on in there. So, I think, fixing remaining mostly minor issues shouldn't
be too difficult for you either.
Hi Guennadi,
Thanks for your review time.
I only tried to get the driver really working, not only looking good ;).
On Sun, 18 Jul 2010, Janusz Krzysztofik wrote:
This is a V4L2 driver for TI OMAP1 SoC camera interface.
Two versions of the driver are provided, using either videobuf-dma-contig
or videobuf-dma-sg. The former uses less processing power, but often
fails to allocate contignuous buffer memory. The latter is free of this
problem, but generates tens of DMA interrupts per frame.
Hm, would it be difficult to first try contig, and if it fails - fall back
to sg?
Hmm, let me think about it.
In its present state buffer management in your driver is pretty difficult
to follow. At the VERY least you have to extensively document your buffer
manipulations. Better yet clean it up. AFAIU, your ready pointer is
pretty much unused in the contiguous case. Or you can easily get rid of
it.
I was sure the ready pointer was essential for both paths. However, things
tend to change as more and more optimizations / cleanups are applied, so I
have to verify it again.
So, I think, a very welcome improvement to the driver would be a
cleaner separation between the two cases. Don't try that much to reuse the
code as much as possible. Would be much better to have clean separation
between the two implementations - whether dynamically switchable at
runtime or at config time - would be best to separate the two
implementations to the point, where each of them becomes understandable
and maintainable.
You are right. My approach was probably more adequate at development time,
when I was trying to aviod error-prone code duplication. As soon as both
paths get stable, they can be split for better readability. I'll take care of
this.
So, I will do a pretty formal review this time and wait for v2. And in v2
I'd like to at the very least see detailed buffer-management
documentation, or - better yet - a rework with a clean separation.
OK.
...
--- linux-2.6.35-rc3.orig/include/media/omap1_camera.h 1970-01-01
01:00:00.0 +0100
+++ linux-2.6.35-rc3/include/media/omap1_camera.h 2010-07-18
01:31:57.0 +0200
@@ -0,0 +1,16 @@
A copyright / licence header here, please
OK.
+#ifndef __MEDIA_OMAP1_CAMERA_H_
+#define __MEDIA_OMAP1_CAMERA_H_
+
+#define OMAP1_CAMERA_IOSIZE0x1c
+
+struct omap1_cam_platform_data {
+ unsigned long camexclk_khz;
+ unsigned long lclk_khz_max;
+ unsigned long flags;
+};
+
+#define OMAP1_CAMERA_LCLK_RISING BIT(0)
+#define OMAP1_CAMERA_RST_LOW BIT(1)
+#define OMAP1_CAMERA_RST_HIGH BIT(2)
Then you need to #include linux/bitops.h
OK.
Since I always tend to optimise out redundant includes of header files that
are otherwise included indirectly, please verify if there are more of them
that you would prefere included explicitly.
+
+#endif /* __MEDIA_OMAP1_CAMERA_H_ */
--- linux-2.6.35-rc3.orig/drivers/media/video/Kconfig 2010-06-26
15:55:29.0 +0200 +++
linux-2.6.35-rc3/drivers/media/video/Kconfig2010-07-02
04:12:02.0 +0200 @@ -962,6 +962,20 @@ config VIDEO_SH_MOBILE_CEU
---help---
This is a v4l2 driver for the SuperH Mobile CEU Interface
+config VIDEO_OMAP1
+ tristate OMAP1 Camera Interface driver
+ depends on VIDEO_DEV ARCH_OMAP1 SOC_CAMERA
+ select VIDEOBUF_DMA_CONTIG if !VIDEO_OMAP1_SG
+ ---help---
+ This is a v4l2 driver for the TI OMAP1 camera interface
+
+if VIDEO_OMAP1
Don't think you need this if, the depends on below should suffice.
OK.
My intention was to get it indented, and now I see it works like this even if
not enclosed with if-endif.
Will drop the if-endif if the compile time SG option survives.
+config VIDEO_OMAP1_SG
+ bool Scatter-gather mode
+ depends on VIDEO_OMAP1 EXPERIMENTAL
+ select VIDEOBUF_DMA_SG
+endif
+
...
+#ifndef CONFIG_VIDEO_OMAP1_SG
+#include media/videobuf-dma-contig.h
+#else
+#include media/videobuf-dma-sg.h
+#endif
I think, you can just include both headers.
OK.
...
+#define CAM_EXCLK_6MHz 600
+#define CAM_EXCLK_8MHz 800
+#define CAM_EXCLK_9_6MHz960
+#define CAM_EXCLK_12MHz1200
+#define CAM_EXCLK_24MHz2400
Why do you need this? Your platform specifies the EXCLK frequency directly
in kHz, then you multiplly it by 1000 to get pcdev-camexclk in Hz, then
