Hi all,
I see the following as a definitive improvement compared to the
previous situation. It's probably possible to improve it further and I
welcome comments in that direction. In general I would like to know how
other Zoran developers and users feel about this change, if there are
major objections, etc. And of course I welcome reports from testers :)
* * * * *
The memory allocation logic for the V4L buffers in the zr36067 driver
is a bit weird at the moment. It arbitrarily limits the size of
kmalloc'd buffers to 128 kB, and for larger buffers, it uses quirks
which are either marked as dangerous or require an external patch
(which I think is no longer maintained.) This basically limits
regular users to buffer sizes of 128 kB, which means a ridiculously
small resolution (320x200).
While it is true that buffers larger than 128 kB may be difficult to
obtain from kmalloc, on recent systems with a lot of memory it isn't
impossible. On my system I have setup the zr36067 driver to allocate
4 buffers of 1 MB each, and kmalloc managed to allocate them without
problems for months. I reduced the physical memory from 1 GB to 256 MB
to see how it would behave, and while the allocations start failing
when the system is short on memory, I can still get tvtime to work by
either running it early or by closing other applications before I
start it.
So I am proposing a rework of the V4L buffer allocation strategy:
* We drop the get_high_mem() alternative allocation method. Did this
work for anyone in recent times? It never worked for me on x86-64.
And if I read the code correctly, it also can't work if more than
one Zoran adapter is installed.
* We suppress the arbitrary limit of 128 kB for kmalloc'd buffers. I
see no reason to not take our chance. If the allocation succeeds
then why not do it?
Note that this doesn't change the default behavior of the driver, as
the default buffer size is 128 kB, and for this size kmalloc was
already used so far.
Signed-off-by: Jean Delvare <[EMAIL PROTECTED]>
Cc: Trent Piepho <[EMAIL PROTECTED]>
Cc: Ronald S. Bultje <[EMAIL PROTECTED]>
Cc: Martin Samuelsson <[EMAIL PROTECTED]>
---
drivers/media/video/zoran_driver.c | 146 +++---------------------------------
1 file changed, 15 insertions(+), 131 deletions(-)
--- linux-2.6.27-rc5.orig/drivers/media/video/zoran_driver.c 2008-09-03
13:23:18.000000000 +0200
+++ linux-2.6.27-rc5/drivers/media/video/zoran_driver.c 2008-09-03
13:47:55.000000000 +0200
@@ -75,7 +75,6 @@
#include "videocodec.h"
#include <asm/byteorder.h>
-#include <asm/io.h>
#include <asm/uaccess.h>
#include <linux/proc_fs.h>
@@ -234,78 +233,9 @@ static void jpg_fbuffer_free(struct file
/*
* Allocate the V4L grab buffers
*
- * These have to be pysically contiguous.
- * If v4l_bufsize <= MAX_KMALLOC_MEM we use kmalloc
- * else we try to allocate them with bigphysarea_alloc_pages
- * if the bigphysarea patch is present in the kernel,
- * else we try to use high memory (if the user has bootet
- * Linux with the necessary memory left over).
+ * These have to be physically contiguous.
*/
-static unsigned long
-get_high_mem (unsigned long size)
-{
-/*
- * Check if there is usable memory at the end of Linux memory
- * of at least size. Return the physical address of this memory,
- * return 0 on failure.
- *
- * The idea is from Alexandro Rubini's book "Linux device drivers".
- * The driver from him which is downloadable from O'Reilly's
- * web site misses the "virt_to_phys(high_memory)" part
- * (and therefore doesn't work at all - at least with 2.2.x kernels).
- *
- * It should be unnecessary to mention that THIS IS DANGEROUS,
- * if more than one driver at a time has the idea to use this memory!!!!
- */
-
- volatile unsigned char __iomem *mem;
- unsigned char c;
- unsigned long hi_mem_ph;
- unsigned long i;
-
- /* Map the high memory to user space */
-
- hi_mem_ph = virt_to_phys(high_memory);
-
- mem = ioremap(hi_mem_ph, size);
- if (!mem) {
- dprintk(1,
- KERN_ERR "%s: get_high_mem() - ioremap failed\n",
- ZORAN_NAME);
- return 0;
- }
-
- for (i = 0; i < size; i++) {
- /* Check if it is memory */
- c = i & 0xff;
- writeb(c, mem + i);
- if (readb(mem + i) != c)
- break;
- c = 255 - c;
- writeb(c, mem + i);
- if (readb(mem + i) != c)
- break;
- writeb(0, mem + i); /* zero out memory */
-
- /* give the kernel air to breath */
- if ((i & 0x3ffff) == 0x3ffff)
- schedule();
- }
-
- iounmap(mem);
-
- if (i != size) {
- dprintk(1,
- KERN_ERR
- "%s: get_high_mem() - requested %lu, avail %lu\n",
- ZORAN_NAME, size, i);
- return 0;
- }
-
- return hi_mem_ph;
-}
-
static int
v4l_fbuffer_alloc (struct file *file)
{
@@ -313,7 +243,6 @@ v4l_fbuffer_alloc (struct file *file)
struct zoran *zr = fh->zr;
int i, off;
unsigned char *mem;
- unsigned long pmem = 0;
/* we might have old buffers lying around... */
if (fh->v4l_buffers.ready_to_be_freed) {
@@ -327,19 +256,9 @@ v4l_fbuffer_alloc (struct file *file)
"%s: v4l_fbuffer_alloc() - buffer %d allready
allocated!?\n",
ZR_DEVNAME(zr), i);
- //udelay(20);
- if (fh->v4l_buffers.buffer_size <= MAX_KMALLOC_MEM) {
- /* Use kmalloc */
-
- mem = kmalloc(fh->v4l_buffers.buffer_size, GFP_KERNEL);
- if (!mem) {
- dprintk(1,
- KERN_ERR
- "%s: v4l_fbuffer_alloc() - kmalloc for
V4L buf %d failed\n",
- ZR_DEVNAME(zr), i);
- v4l_fbuffer_free(file);
- return -ENOBUFS;
- }
+ mem = kmalloc(fh->v4l_buffers.buffer_size,
+ GFP_KERNEL | __GFP_NOWARN);
+ if (mem) {
fh->v4l_buffers.buffer[i].fbuffer = mem;
fh->v4l_buffers.buffer[i].fbuffer_phys =
virt_to_phys(mem);
@@ -354,45 +273,12 @@ v4l_fbuffer_alloc (struct file *file)
ZR_DEVNAME(zr), i, (unsigned long) mem,
virt_to_bus(mem));
} else {
-
- /* Use high memory which has been left at boot time */
-
- /* Ok., Ok. this is an evil hack - we make
- * the assumption that physical addresses are
- * the same as bus addresses (true at least
- * for Intel processors). The whole method of
- * obtaining and using this memory is not very
- * nice - but I hope it saves some poor users
- * from kernel hacking, which might have even
- * more evil results */
-
- if (i == 0) {
- int size =
- fh->v4l_buffers.num_buffers *
- fh->v4l_buffers.buffer_size;
-
- pmem = get_high_mem(size);
- if (pmem == 0) {
- dprintk(1,
- KERN_ERR
- "%s: v4l_fbuffer_alloc() -
get_high_mem (size = %d KB) for V4L bufs failed\n",
- ZR_DEVNAME(zr), size >> 10);
- return -ENOBUFS;
- }
- fh->v4l_buffers.buffer[0].fbuffer = NULL;
- fh->v4l_buffers.buffer[0].fbuffer_phys = pmem;
- fh->v4l_buffers.buffer[0].fbuffer_bus = pmem;
- dprintk(4,
- KERN_INFO
- "%s: v4l_fbuffer_alloc() - using %d KB
high memory\n",
- ZR_DEVNAME(zr), size >> 10);
- } else {
- fh->v4l_buffers.buffer[i].fbuffer = NULL;
- fh->v4l_buffers.buffer[i].fbuffer_phys =
- pmem + i * fh->v4l_buffers.buffer_size;
- fh->v4l_buffers.buffer[i].fbuffer_bus =
- pmem + i * fh->v4l_buffers.buffer_size;
- }
+ dprintk(1,
+ KERN_ERR
+ "%s: v4l_fbuffer_alloc() - kmalloc for V4L buf
%d failed\n",
+ ZR_DEVNAME(zr), i);
+ v4l_fbuffer_free(file);
+ return -ENOBUFS;
}
}
@@ -416,13 +302,11 @@ v4l_fbuffer_free (struct file *file)
if (!fh->v4l_buffers.buffer[i].fbuffer)
continue;
- if (fh->v4l_buffers.buffer_size <= MAX_KMALLOC_MEM) {
- mem = fh->v4l_buffers.buffer[i].fbuffer;
- for (off = 0; off < fh->v4l_buffers.buffer_size;
- off += PAGE_SIZE)
- ClearPageReserved(MAP_NR(mem + off));
- kfree((void *) fh->v4l_buffers.buffer[i].fbuffer);
- }
+ mem = fh->v4l_buffers.buffer[i].fbuffer;
+ for (off = 0; off < fh->v4l_buffers.buffer_size;
+ off += PAGE_SIZE)
+ ClearPageReserved(MAP_NR(mem + off));
+ kfree((void *) fh->v4l_buffers.buffer[i].fbuffer);
fh->v4l_buffers.buffer[i].fbuffer = NULL;
}
--
Jean Delvare
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Mjpeg-users mailing list
Mjpeg-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mjpeg-users
