Skip to content

EQ FIR: memory: Add trace error for alloc fail and add a 1024 size buffer#445

Merged
lgirdwood merged 1 commit intothesofproject:masterfrom
singalsu:eq_fir_heap_rt_1k_proposal
Oct 1, 2018
Merged

EQ FIR: memory: Add trace error for alloc fail and add a 1024 size buffer#445
lgirdwood merged 1 commit intothesofproject:masterfrom
singalsu:eq_fir_heap_rt_1k_proposal

Conversation

@singalsu
Copy link
Copy Markdown
Collaborator

@singalsu singalsu commented Oct 1, 2018

This patch adds a single 1024 bytes buffer into heap for e.g. rzalloc() usage. The FIR EQ code didn't trace the allocation failure so it's added to see better if this happens.

The 1k allocation for PCM samples delay line is quite typical since a 32 bit 100 tap stereo FIR will need 800 bytes.

Signed-off-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com

@singalsu singalsu requested review from lgirdwood and tlauda October 1, 2018 12:12
…ffer

This patch adds a single 1024 bytes buffer into heap for e.g. rzalloc()
usage. The FIR EQ code didn't trace the allocation failure so it's added
to see better if this happens. The 1k allocation for PCM samples delay
line is quite typical since a 32 bit 100 tap stereo FIR will need 800
bytes.

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
@singalsu singalsu force-pushed the eq_fir_heap_rt_1k_proposal branch from f93a990 to 0f06203 Compare October 1, 2018 12:26
@singalsu
Copy link
Copy Markdown
Collaborator Author

singalsu commented Oct 1, 2018

I just pushed update for this. The first try caused CNL and ICL build fail.

@lgirdwood
Copy link
Copy Markdown
Member

@singalsu if this is a buffer, please use the buffer allocate call, I want to try and free up the system pool for smaller allocations.

@singalsu
Copy link
Copy Markdown
Collaborator Author

singalsu commented Oct 1, 2018

@lgirdwood This component used earlier rballoc() but commit 5118c6d changed it to rzalloc(). The smallest practical allocation will be about 500 bytes. Typical maybe closer to 1kB. Should we move it back?

@lgirdwood
Copy link
Copy Markdown
Member

@singalsu ah yes there is some simplification happening for the alloc APIs.

@singalsu
Copy link
Copy Markdown
Collaborator Author

singalsu commented Oct 2, 2018

@lgirdwood Did this go wrong. It looks like the patch was first merged and then removed. E.g. the trace that I added about allocation fail is gone.

commit 0b2e212c12e7b1288b169323de8a914e9ebe7781 (upstream/revert-445-eq_fir_heap_rt_1k_proposal)
Author: Liam Girdwood <lgirdwood@gmail.com>
Date:   Mon Oct 1 20:56:32 2018 +0100

    Revert "EQ FIR: memory: Add trace error for alloc fail and add a 1024 size buffer"

diff --git a/src/audio/eq_fir.c b/src/audio/eq_fir.c
index ed6d4db1..2a0c19fa 100644
--- a/src/audio/eq_fir.c
+++ b/src/audio/eq_fir.c
@@ -204,11 +204,8 @@ static int eq_fir_setup(struct comp_data *cd, int nch)
 
        /* Allocate all FIR channels data in a big chunk and clear it */
        cd->fir_delay = rzalloc(RZONE_RUNTIME, SOF_MEM_CAPS_RAM, size_sum);
-       if (!cd->fir_delay) {
-               trace_eq_error("eda");
-               trace_value(size_sum);
+       if (!cd->fir_delay)
                return -ENOMEM;
-       }

What should I do?

@lgirdwood
Copy link
Copy Markdown
Member

@singalsu it was merged and then reverted due to build failure caused by an earlier PR. Best to reabse on top of master and then fix the build error.

@singalsu
Copy link
Copy Markdown
Collaborator Author

singalsu commented Oct 2, 2018

Thanks, I've included Sue Creek now. I'll build now all the platform compilers so I can make sure at my own computer that everything builds. I'll push the update later today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants