| Message ID | 1278991369-17962-2-git-send-email-bryan.wu@canonical.com |
|---|---|
| State | Accepted |
| Delegated to: | Tim Gardner |
| Headers | show |
On 07/13/2010 11:22 AM, Bryan Wu wrote: > From: David Sin <davidsin@ti.com> > > Instead of allocating and freeing PAT page array > memory each time, allocate 128k upfront and reuse the > memory. This will avoid the possibilty of not being > able to obtain the memory after driver initialization. > > Signed-off-by: David Sin <davidsin@ti.com> > --- > drivers/media/video/tiler/tiler.c | 27 +++++++++++++++------------ > 1 files changed, 15 insertions(+), 12 deletions(-) > [snip] > > + /* Array of physical pages for PAT programming, which must be a 16-byte > + aligned physical address > + */ > + dmac_va = dma_alloc_coherent(NULL, TILER_WIDTH * TILER_HEIGHT * > + sizeof(*dmac_va), &dmac_pa, GFP_ATOMIC); > + if (!dmac_va) > + return -ENOMEM; > + Returning -ENOMEM is not enough here. IMHO, at least it should deinit sita_init(). Or just follow the error exit path. Thanks, -Bryan
On 07/12/2010 09:34 PM, Bryan Wu wrote: > On 07/13/2010 11:22 AM, Bryan Wu wrote: >> From: David Sin<davidsin@ti.com> >> >> Instead of allocating and freeing PAT page array >> memory each time, allocate 128k upfront and reuse the >> memory. This will avoid the possibilty of not being >> able to obtain the memory after driver initialization. >> >> Signed-off-by: David Sin<davidsin@ti.com> >> --- >> drivers/media/video/tiler/tiler.c | 27 +++++++++++++++------------ >> 1 files changed, 15 insertions(+), 12 deletions(-) >> > > [snip] > >> >> + /* Array of physical pages for PAT programming, which must be a 16-byte >> + aligned physical address >> + */ >> + dmac_va = dma_alloc_coherent(NULL, TILER_WIDTH * TILER_HEIGHT * >> + sizeof(*dmac_va),&dmac_pa, GFP_ATOMIC); >> + if (!dmac_va) >> + return -ENOMEM; >> + > > Returning -ENOMEM is not enough here. IMHO, at least it should deinit > sita_init(). Or just follow the error exit path. > > Thanks, > -Bryan > Eh? I think this looks OK. If you can't allocate DMA memory, then you're pretty much screwed and can do nothing else but exit. applied
diff --git a/drivers/media/video/tiler/tiler.c b/drivers/media/video/tiler/tiler.c index c8008b0..bd35248 100644 --- a/drivers/media/video/tiler/tiler.c +++ b/drivers/media/video/tiler/tiler.c @@ -120,6 +120,8 @@ static u32 id; static struct mutex mtx; static struct tcm *tcm[TILER_FORMATS]; static struct tmm *tmm[TILER_FORMATS]; +static u32 *dmac_va; +static dma_addr_t dmac_pa; #define TCM(fmt) tcm[(fmt) - TILFMT_8BIT] #define TCM_SS(ssptr) TCM(TILER_GET_ACC_MODE(ssptr)) @@ -829,34 +831,24 @@ static s32 tiler_mmap(struct file *filp, struct vm_area_struct *vma) static s32 refill_pat(struct tmm *tmm, struct tcm_area *area, u32 *ptr) { s32 res = 0; - s32 size = tcm_sizeof(*area) * sizeof(*ptr); - u32 *page; - dma_addr_t page_pa; struct pat_area p_area = {0}; struct tcm_area slice, area_s; - /* must be a 16-byte aligned physical address */ - page = dma_alloc_coherent(NULL, size, &page_pa, GFP_ATOMIC); - if (!page) - return -ENOMEM; - tcm_for_each_slice(slice, *area, area_s) { p_area.x0 = slice.p0.x; p_area.y0 = slice.p0.y; p_area.x1 = slice.p1.x; p_area.y1 = slice.p1.y; - memcpy(page, ptr, sizeof(*ptr) * tcm_sizeof(slice)); + memcpy(dmac_va, ptr, sizeof(*ptr) * tcm_sizeof(slice)); ptr += tcm_sizeof(slice); - if (tmm_map(tmm, p_area, page_pa)) { + if (tmm_map(tmm, p_area, dmac_pa)) { res = -EFAULT; break; } } - dma_free_coherent(NULL, size, page, page_pa); - return res; } @@ -1423,6 +1415,9 @@ static void __exit tiler_exit(void) mutex_unlock(&mtx); + dma_free_coherent(NULL, TILER_WIDTH * TILER_HEIGHT * sizeof(*dmac_va), + dmac_va, dmac_pa); + /* close containers only once */ for (i = TILFMT_8BIT; i <= TILFMT_MAX; i++) { /* remove identical containers (tmm is unique per tcm) */ @@ -1502,6 +1497,14 @@ static s32 __init tiler_init(void) TMM_SET(TILFMT_32BIT, tmm_pat); TMM_SET(TILFMT_PAGE, tmm_pat); + /* Array of physical pages for PAT programming, which must be a 16-byte + aligned physical address + */ + dmac_va = dma_alloc_coherent(NULL, TILER_WIDTH * TILER_HEIGHT * + sizeof(*dmac_va), &dmac_pa, GFP_ATOMIC); + if (!dmac_va) + return -ENOMEM; + tiler_device = kmalloc(sizeof(*tiler_device), GFP_KERNEL); if (!tiler_device || !sita || !tmm_pat) { r = -ENOMEM;