Laurent Pinchart wrote: > Hi Mauro, > > On Thursday 06 May 2010 14:38:36 Mauro Carvalho Chehab wrote: >> Laurent Pinchart wrote: >>> On Thursday 06 May 2010 01:29:54 Aguirre, Sergio wrote: >>>>> -----Original Message----- >>>>> From: Mauro Carvalho Chehab [mailto:mchehab@xxxxxxxxxx] >>>>> Sent: Wednesday, May 05, 2010 6:24 PM >>>>> To: Aguirre, Sergio >>>>> Cc: linux-media@xxxxxxxxxxxxxxx >>>>> Subject: Re: [videobuf] Query: Condition bytesize limit in >>>>> videobuf_reqbufs -> buf_setup() call? >>>>> >>>>> Aguirre, Sergio wrote: >>>>>> Hi all, >>>>>> >>>>>> While working on an old port of the omap3 camera-isp driver, >>>>>> I have faced some problem. >>>>>> >>>>>> Basically, when calling VIDIOC_REQBUFS with a certain buffer >>>>>> >>>>>> Count, we had a software limit for total size, calculated depending on: >>>>>> Total bytesize = bytesperline x height x count >>>>>> >>>>>> So, we had an arbitrary limit to, say 32 MB, which was generic. >>>>>> >>>>>> Now, we want to condition it ONLY when MMAP buffers will be used. >>>>>> Meaning, we don't want to keep that policy when the kernel is not >>>>>> allocating the space >>>>>> >>>>>> But the thing is that, according to videobuf documentation, buf_setup >>>>>> is the one who should put a RAM usage limit. BUT the memory type >>>>>> passed to reqbufs is not propagated to buf_setup, therefore forcing me >>>>>> to go to a non-standard memory limitation in my reqbufs callback >>>>>> function, instead of doing it properly inside buf_setup. >>>>>> >>>>>> Is this scenario a good consideration to change buf_setup API, and >>>>>> propagate buffers memory type aswell? >>>>> I don't see any problem on propagating the memory type to buffer_setup, >>>>> if this is really needed. Yet, I can't see why you would restrict the >>>>> buffer size to 32 MB on one case, and not restrict the size at all with >>>>> non-MMAP types. >>>> Ok, my reason for doing that is because I thought that there should be a >>>> memory limit in whichever place you're doing the buffer allocations. >>>> >>>> MMAP is allocating buffers in kernel, so kernel should provide a memory >>>> restriction, if applies. >>>> >>>> USERPTR is allocating buffers in userspace, so userspace should provide >>>> a memory restriction, if applies. >>> I agree with the intend here, but not with the current implementation >>> which has a hardcoded arbitrary limit. Do you think it would be possible >>> to compute a meaningful default limit in the V4L2 core, with a way for >>> userspace to modify it (with root privileges of course) ? >> On almost all drivers, the limit is not arbitrary. It is a reasonable >> number of buffers (like 16 buffers). A limit in terms of the number of >> buffers is meaningful for V4L2 API, and also, has a "physical meaning": >> considering that almost all drivers that use videobuf can do at maximum 30 >> fps, 16 buffers mean that the maximum delay that the driver will apply to >> the stream is 533 ms. >> >> Some drivers even provide a modprobe parameter to allow changing this limit >> (for example, bttv allows changing it up to 32 buffers), but only during >> module load time. I can't foresee any use case where this maximum limit >> would need to be dynamically adjusted. Root can always change it by >> removing and re-inserting the module with a new maximum size. > > I wasn't talking about the limit on the number of buffers, but on the amount > of memory. That's what Sergio was mentioning, and that's what is done in the > OMAP3 ISP driver. The memory consumption is basically dictated by the maximum size of an image (resolution x bpp / 8) and the number of buffers. An arbitrary value in terms of megabytes is meaningless: it is just a random number above some reasonable limit. The proper way to limit memory is to do something like: #define MAX_HRES 1920 #define MAX_VRES 1650 #define MAX_DEPTH 3 #define MAX_BUFS 4 #define MAX_MEMSIZE (MAX_HRES * MAX_VRES * MAX_DEPTH * MAX_BUFS) buf_setup(...) { if (size > MAX_MEMSIZE) fix_it_by_reducing_number_of_buffers(); } -- Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
