cafe_ccic: default to allocating DMA buffers at probe time
PATCH 1/1 cafe_ccic: default to allocating DMA buffers at probe time
By default, we allocate DMA buffers when actually reading from the video capture device. On a system with 128MB or 256MB of ram, it's very easy for that memory to quickly become fragmented. We've had users report having 30+MB of memory free, but the cafe_ccic driver is still unable to allocate DMA buffers.
Our workaround has been to make use of the 'alloc_bufs_at_load' parameter to allocate DMA buffers during device probing. This patch makes DMA buffer allocation happen during device probe by default, and changes the parameter to 'alloc_bufs_at_read'. The camera hardware is there, if the cafe_ccic driver is enabled/loaded it should do its best to ensure that the camera is actually usable; delaying DMA buffer allocation saves an insignicant amount of memory, and causes the driver to be much less useful.
---
drivers/media/video/cafe_ccic.c | 18 +++++++++--------- 1 files changed, 9 insertions(+), 9 deletions(-)
drivers/media/video/cafe_ccic.c +9 -9
@@ -63,13 +63,13 @@ MODULE_SUPPORTED_DEVICE("Video");*/ #define MAX_DMA_BUFS 3 -static int alloc_bufs_at_load =0 ; -module_param (alloc_bufs_at_load, bool,0444 ); -MODULE_PARM_DESC (alloc_bufs_at_load, -"Non-zero value causes DMA buffers to be allocated at module " -"load time. This increases the chances of successfully getting " -"those buffers, but at the cost of nailing down the memory from " -"the outset." ); +static int alloc_bufs_at_read =0 ; +module_param (alloc_bufs_at_read, bool,0444 ); +MODULE_PARM_DESC (alloc_bufs_at_read, +"Non-zero value causes DMA buffers to be allocated when the " +"video capture device is read, rather than at module load " +"time. This saves memory, but decreases the chances of " +"successfully getting those buffers." );static int n_dma_bufs =3 ;module_param (n_dma_bufs, uint,0644 );
@@ -1503,7 +1503,7 @@ static int cafe_v4l_release(struct inode *inode, struct file *filp) }if (cam->users ==0 ) {cafe_ctlr_power_down (cam); -if (! alloc_bufs_at_load) +if (alloc_bufs_at_read)cafe_free_dma_bufs (cam); }mutex_unlock (&cam->s_mutex);
@@ -2162,7 +2162,7 @@ static int cafe_pci_probe(struct pci_dev *pdev,/* * If so requested, try to get our DMA buffers now. */ -if (alloc_bufs_at_load) { +if (!alloc_bufs_at_read) {if (cafe_alloc_dma_bufs (cam,1 ))cam_warn (cam,"Unable to alloc DMA buffers at load" " will try again later." );
Quoted 6 lines · Andres Salomon
This patch makes DMA buffer allocation happen during device probe by default, and changes the parameter to '
alloc_bufs_at_read'. The camera hardware is there, if thecafe_ccicdriver is enabled/loaded it should do its best to ensure that the camera is actually usable; delaying DMA buffer allocation saves an insignicant amount of memory, and causes the driver to be much less useful.
The amount of memory isn't quite "insignificant" (three 640x480x2 buffers), but the OLPC people clearly need things to work this way. There are, as far as I know, no other systems out there using the CAFE controller. Making things work by default for the one user (with a fair number of deployed systems!) makes sense to me. So...
That said, I do prefer the original name for the parameter.
jon
Jonathan Corbet / LWN.net / [email protected]
Quoted 14 lines · Jonathan Corbet
Quoted 6 lines · Andres Salomon
This patch makes DMA buffer allocation happen during device probe by default, and changes the parameter to '
alloc_bufs_at_read'. The camera hardware is there, if thecafe_ccicdriver is enabled/loaded it should do its best to ensure that the camera is actually usable; delaying DMA buffer allocation saves an insignicant amount of memory, and causes the driver to be much less useful.The amount of memory isn't quite "insignificant" (three 640x480x2 buffers), but the OLPC people clearly need things to work this way. There are, as far as I know, no other systems out there using the CAFE controller. Making things work by default for the one user (with a fair number of deployed systems!) makes sense to me. So...
Quoted 3 lines
Acked-by Jonathan Corbet <[email protected]>That said, I do prefer the original name for the parameter.
Changing the parameter name will break the configuration of anyone who was using the existing parameter. It also means everyone who has multiple kernels installed and uses this parameter will need to have one configuration for old kernels and a different configuration for new kernels.
In the default really needs to be changed (what's so hard about setting alloc_bufs_at_load in /etc/modprobe{.d,.config}?) then just change the default of the existing parameter. That avoids the whole problem with different configration files for different module versions.
Quoted 15 lines · On Wed, 19 Sep 2007 15:34:42 -0700 (PDT) Trent Piepho <[email protected]>…
Quoted 14 lines · Jonathan Corbet
Quoted 6 lines · Andres Salomon
This patch makes DMA buffer allocation happen during device probe by default, and changes the parameter to '
alloc_bufs_at_read'. The camera hardware is there, if thecafe_ccicdriver is enabled/loaded it should do its best to ensure that the camera is actually usable; delaying DMA buffer allocation saves an insignicant amount of memory, and causes the driver to be much less useful.The amount of memory isn't quite "insignificant" (three 640x480x2 buffers), but the OLPC people clearly need things to work this way. There are, as far as I know, no other systems out there using the CAFE controller. Making things work by default for the one user (with a fair number of deployed systems!) makes sense to me. So...
Quoted 6 lines
Quoted 3 lines
Acked-by Jonathan Corbet <[email protected]>That said, I do prefer the original name for the parameter.
Changing the parameter name will break the configuration of anyone who was using the existing parameter. It also means everyone who has multiple
We (OLPC) are the only ones using this driver, as we are the only ones with this hardware. Marvell designed the cafe for us, and it is not available on the market yet. Backwards compatibility at this point is not a concern.
Quoted 8 lines
kernels installed and uses this parameter will need to have one configuration for old kernels and a different configuration for new kernels.
In the default really needs to be changed (what's so hard about setting
alloc_bufs_at_loadin/etc/modprobe{.d,.config}?) then just change the default of the existing parameter. That avoids the whole problem with different configration files for different module versions.
We're building the driver in statically; there's no point for us to make it a module. We could add a kernel argument, yes, but that quickly becomes unwieldy as we require more and more. We've actually been trying to cut down the number of kernel args we use.
That said, I'm not opposed to keeping the parameter name the same while making the default 1; I just thought that the name 'alloc_bufs_at_read' was clearer. Another option is to change it to 'no_alloc_bufs_at_load'. Jon, any preference there?
Signature · 1 line
Andres Salomon <[email protected]>
Quoted 27 lines · Andres Salomon
Quoted 11 lines · On Wed, 19 Sep 2007 15:34:42 -0700 (PDT) Trent Piepho <[email protected]>…
Quoted 8 lines · Jonathan Corbet
Quoted 6 lines · Andres Salomon
This patch makes DMA buffer allocation happen during device probe by default, and changes the parameter to '
alloc_bufs_at_read'. The camera hardware is there, if thecafe_ccicdriver is enabled/loaded it should do its best to ensure that the camera is actually usable; delaying DMA buffer allocation saves an insignicant amount of memory, and causes the driver to be much less useful.Changing the parameter name will break the configuration of anyone who was using the existing parameter. It also means everyone who has multiple
We (OLPC) are the only ones using this driver, as we are the only ones with this hardware. Marvell designed the cafe for us, and it is not available on the market yet. Backwards compatibility at this point is not a concern.
Quoted 8 lines
kernels installed and uses this parameter will need to have one configuration for old kernels and a different configuration for new kernels.
In the default really needs to be changed (what's so hard about setting
alloc_bufs_at_loadin/etc/modprobe{.d,.config}?) then just change the default of the existing parameter. That avoids the whole problem with different configration files for different module versions.
Quoted 4 lines
We're building the driver in statically; there's no point for us to make it a module. We could add a kernel argument, yes, but that quickly becomes unwieldy as we require more and more. We've actually been trying to cut down the number of kernel args we use.
Ok, that seems like a good reason to change the default.
Quoted 6 lines
That said, I'm not opposed to keeping the parameter name the same while making the default 1; I just thought that the name '
alloc_bufs_at_read' was clearer. Another option is to change it to 'no_alloc_bufs_at_load'. Jon, any preference there?
It seems like the name 'alloc_bufs_at_read' isn't quite right, since the buffers will be allocated when the format is set, from cafe_vidioc_s_fmt_cap().
Quoted 4 lines · Andres Salomon
That said, I'm not opposed to keeping the parameter name the same while making the default 1; I just thought that the name '
alloc_bufs_at_read' was clearer. Another option is to change it to 'no_alloc_bufs_at_load'. Jon, any preference there?
I don't think that negating it by adding no_ at the front helps much. In general, I prefer the name it has now, but it's not *that* big a deal. We've probably already expended more bandwidth than it's worth.
jon
Hi Andres,
Still missing on your patch is your SOB ;) From what I got from Jon, he is ok with your patch.
Cheers,
Mauro.
Em Qua, 2007-09-19 às 01:44 -0400, Andres Salomon escreveu:
Quoted 67 lines
By default, we allocate DMA buffers when actually reading from the video capture device. On a system with 128MB or 256MB of ram, it's very easy for that memory to quickly become fragmented. We've had users report having 30+MB of memory free, but the
cafe_ccicdriver is still unable to allocate DMA buffers.Our workaround has been to make use of the '
alloc_bufs_at_load' parameter to allocate DMA buffers during device probing. This patch makes DMA buffer allocation happen during device probe by default, and changes the parameter to 'alloc_bufs_at_read'. The camera hardware is there, if thecafe_ccicdriver is enabled/loaded it should do its best to ensure that the camera is actually usable; delaying DMA buffer allocation saves an insignicant amount of memory, and causes the driver to be much less useful.
---drivers/media/video/cafe_ccic.c | 18 +++++++++--------- 1 files changed, 9 insertions(+), 9 deletions(-)drivers/media/video/cafe_ccic.c +9
-9@@ -63,13 +63,13 @@ MODULE_SUPPORTED_DEVICE("Video");*/ #define MAX_DMA_BUFS 3 -static int alloc_bufs_at_load =0 ; -module_param (alloc_bufs_at_load, bool,0444 ); -MODULE_PARM_DESC (alloc_bufs_at_load, -"Non-zero value causes DMA buffers to be allocated at module " -"load time. This increases the chances of successfully getting " -"those buffers, but at the cost of nailing down the memory from " -"the outset." ); +static int alloc_bufs_at_read =0 ; +module_param (alloc_bufs_at_read, bool,0444 ); +MODULE_PARM_DESC (alloc_bufs_at_read, +"Non-zero value causes DMA buffers to be allocated when the " +"video capture device is read, rather than at module load " +"time. This saves memory, but decreases the chances of " +"successfully getting those buffers." );static int n_dma_bufs =3 ;module_param (n_dma_bufs, uint,0644 );@@ -1503,7 +1503,7 @@ static int cafe_v4l_release(struct inode *inode, struct file *filp) }if (cam->users ==0 ) {cafe_ctlr_power_down (cam); -if (! alloc_bufs_at_load) +if (alloc_bufs_at_read)cafe_free_dma_bufs (cam); }mutex_unlock (&cam->s_mutex);@@ -2162,7 +2162,7 @@ static int cafe_pci_probe(struct pci_dev *pdev,/* * If so requested, try to get our DMA buffers now. */ -if (alloc_bufs_at_load) { +if (!alloc_bufs_at_read) {if (cafe_alloc_dma_bufs (cam,1 ))cam_warn (cam,"Unable to alloc DMA buffers at load" " will try again later." );_______________________________________________
v4l-dvb-maintainer mailing list
[email protected]
http://www.linuxtv.org/cgi-bin/mailman/listinfo/v4l-dvb-maintainer
Signature · 2 lines
Cheers, Mauro
Quoted 7 lines · On Mon, 24 Sep 2007 12:04:17 -0300 Mauro Carvalho Chehab…
Hi Andres,
Still missing on your patch is your SOB ;) From what I got from Jon, he is ok with your patch.
Cheers,
Mauro.
Whoops, sorry about that! well, here it is (for the patch below):
Quoted 73 lines
Em Qua, 2007-09-19 às 01:44 -0400, Andres Salomon escreveu:
Quoted 67 lines
By default, we allocate DMA buffers when actually reading from the video capture device. On a system with 128MB or 256MB of ram, it's very easy for that memory to quickly become fragmented. We've had users report having 30+MB of memory free, but the
cafe_ccicdriver is still unable to allocate DMA buffers.Our workaround has been to make use of the '
alloc_bufs_at_load' parameter to allocate DMA buffers during device probing. This patch makes DMA buffer allocation happen during device probe by default, and changes the parameter to 'alloc_bufs_at_read'. The camera hardware is there, if thecafe_ccicdriver is enabled/loaded it should do its best to ensure that the camera is actually usable; delaying DMA buffer allocation saves an insignicant amount of memory, and causes the driver to be much less useful.
---drivers/media/video/cafe_ccic.c | 18 +++++++++--------- 1 files changed, 9 insertions(+), 9 deletions(-)drivers/media/video/cafe_ccic.c +9
-9@@ -63,13 +63,13 @@ MODULE_SUPPORTED_DEVICE("Video");*/ #define MAX_DMA_BUFS 3 -static int alloc_bufs_at_load =0 ; -module_param (alloc_bufs_at_load, bool,0444 ); -MODULE_PARM_DESC (alloc_bufs_at_load, -"Non-zero value causes DMA buffers to be allocated at module " -"load time. This increases the chances of successfully getting " -"those buffers, but at the cost of nailing down the memory from " -"the outset." ); +static int alloc_bufs_at_read =0 ; +module_param (alloc_bufs_at_read, bool,0444 ); +MODULE_PARM_DESC (alloc_bufs_at_read, +"Non-zero value causes DMA buffers to be allocated when the " +"video capture device is read, rather than at module load " +"time. This saves memory, but decreases the chances of " +"successfully getting those buffers." );static int n_dma_bufs =3 ;module_param (n_dma_bufs, uint,0644 );@@ -1503,7 +1503,7 @@ static int cafe_v4l_release(struct inode *inode, struct file *filp) }if (cam->users ==0 ) {cafe_ctlr_power_down (cam); -if (! alloc_bufs_at_load) +if (alloc_bufs_at_read)cafe_free_dma_bufs (cam); }mutex_unlock (&cam->s_mutex);@@ -2162,7 +2162,7 @@ static int cafe_pci_probe(struct pci_dev *pdev,/* * If so requested, try to get our DMA buffers now. */ -if (alloc_bufs_at_load) { +if (!alloc_bufs_at_read) {if (cafe_alloc_dma_bufs (cam,1 ))cam_warn (cam,"Unable to alloc DMA buffers at load" " will try again later." );_______________________________________________
v4l-dvb-maintainer mailing list
[email protected]
http://www.linuxtv.org/cgi-bin/mailman/listinfo/v4l-dvb-maintainerSignature · 2 lines
Cheers, Mauro
Signature · 1 line
Andres Salomon <[email protected]>