topology: add macro to support multiple fir iir eq filters by fredoh9 · Pull Request #2660 · thesofproject/sof

fredoh9 · 2020-03-29T22:49:38Z

The patch shows a proposal how to propagate pipeline filter
coefficients data definition at topology top level. It is an
useful way to avoid to create different new pipeline macros for
different filter. Define filter coefficients data with macro
DEF_EQFIR_PRIV and DEF_EQFIR_COEF, which will be replaced
unique name by adding PIPELINE number. For IIR filter, equivalent macro
is DEF_EQIIR_PRIV and DEF_EQIIR_COEF.

Makefile define each pipeline processing HSEARPROC, HSMICRPOC, SPKPROC,
DMICPROC etc. Each variable should support default value to minimize
current topologies. When new filter blob is required, define
PIPELINE_FILTERx in makefile. This method provides more flexible way
to support variable number of filters in the pipeline.

The other clean up for topology macros include definions for unique
volume tokens and unique filter coefficient definitions.

Signed-off-by: Fred Oh fred.oh@linux.intel.com

Related issues: #2535, #2534

singalsu

Sorry for causing a misunderstanding. The applying of low-pass into CML RM5682 should not be up-streamed. Only the general technique to pass from topology top level or even makefile the filter blob reference is the part that's needed. The low-pass FIR was an experiment to attenuate bits corruption effect that happened in this platform due to SSP clocks issue and the capability to conceal the issue fir FIR was weak. So, please change the topic of this PR to reflect the processing define updates and drop totally the 7 kHz low-pass blob. The brick-wall FIR is not a very good demo response and it's hard to measure the stop-band spec due to to lot of attenuation.

The demo case to upstream for the technique could be maybe an alternative topology with bass boost IIR or loudness IIR applied (I can make PR for the the m4 filter blobs today -- I need them for testbench usage as well as test response) to headset output PCM with pipe-eq-iir-volume-playback.m4. Also the various 40 Hz high-pass with gain (20 dB) and without should be cleaned up and implemented this way with generic filter blob reference and pipe-eq-iir-volume-capture.m4. Then we would have test cases for both playback and capture EQ.

I can also email to you the demo blobs to let you continue immediately with them and not wait for my PR to get merged. If you'd like to include them to your PR I can close my coming PR. We can proceed like you prefer with them!

Instead of defining keyword lowpass and interpreting it in pipeline macro the macro should get the filename reference directly. There would be eventually tons of this kind keywords and the maintenance would be impossible and at top level no idea what coefficients set gets requested. Even of top level gets a bit clutter I'd prefer that. Since the processing effect is strong it should be very visible at upper level to not accidentally enable something not desired.

Also this approach does not seem to enable to define at top level filter blobs for this kind of speaker rendering pipeline:

PCM --> EQ-IIR --> EQ-FIR --> volume --> DAI

The needs to be a way to pass the separate m4 coefficients blobs for IIR and FIR the pipeline. Currently we don't have more than max. two processing components (+volume) in pipeline but in future there can be more, e.g. three or four. So this system should scale up a bit from this proposal.

Note: I've done several edits to this while trying to make my mind so please ignore the intermediate versions emails you've possibly received.

singalsu · 2020-03-30T14:25:02Z

@fredoh9 I added with PR #2667 new FIR and IIR EQ m4 blobs those can be used for testing and as examples. There's responses like loudness and bass boost for FIR and IIR those are easy to verify subjectively or by measurement if set up in topology.

fredoh9 · 2020-03-31T20:11:25Z

I updated to address @singalsu comments. I changed title and put [WIP] as I added some examples which may not need to go to master. Or I may need to add some more real life examples to demonstrate this changes. I'm pretty sure this provide very flexible way to add more topology with different piplines.

fredoh9 · 2020-04-01T15:25:42Z

@singalsu
HSMICPROC, HSEARPROC, DMICPROC are defined in sof-cml-rt5682 pipes.

Default response is flat for iir and fir filters. It can be set by makefile, PIPELINE_FILTER1 or PIPELINE_FILTER2.

CMakeLists.txt changes for demonstration for usage of this PR. By default, fir/iir flat coefficient is set. One of topology overrides fir flat with fir loudness.

	"sof-cml-rt5682\;sof-cml-eq-fir-rt5682\;-DPLATFORM=cml\;-DHSMICPROC=eq-fir-volume"
	"sof-cml-rt5682\;sof-cml-eq-fir-loud-rt5682\;-DPLATFORM=cml\;-DHSMICPROC=eq-fir-volume\;-DPIPELINE_FILTER1=eq_fir_coef_loudness.m4"
	"sof-cml-rt5682\;sof-cml-eq-iir-rt5682\;-DPLATFORM=cml\;-DHSEARPROC=eq-iir-volume"

singalsu · 2020-04-02T06:55:24Z

Since this is direct attach DMIC the default should be eq-iir-volume and filter blob 40 Hz high-pass for 48 kHz for dmic01 and 40 Hz highpass for 16 kHz for dmic16k. The gain can be the 0 dB version.

This reveals need for added DMIC16KPROC and DMIC16K_FILTER1 macros handling to get different filter blob for 16 kHz pipeline. The DMICPROC could be reused but for flexibility better to have them separated.

singalsu · 2020-04-02T07:07:14Z

Good work, thanks! I proposed some changes for processing setup for enhanced topologies. Also need to add the upper endpoint specific layer for PIPELINE_FILTERx. Also I realized that we need to be able to differentiate the filter configs for 48 kHz and 16 kHz DMIC DAI. But you've done most of the work so those are fairly small further edits.

The patch shows a proposal how to propagate pipeline filter coefficients data definition at topology top level. It is an useful way to avoid to create different new pipeline macros for different filter. Define filter coefficients data with macro DEF_EQFIR_PRIV and DEF_EQFIR_COEF, which will be replaced unique name by adding PIPELINE number. For IIR filter, equivalent macro is DEF_EQIIR_PRIV and DEF_EQIIR_COEF. Makefile define each pipeline processing HSEARPROC, HSMICRPOC, SPKPROC, DMICPROC etc. Each variable should support default value to minimize current topologies. When new filter blob is required, define PIPELINE_FILTERx in makefile. This method provides more flexible way to support variable number of filters in the pipeline. The other clean up for topology macros include definions for unique volume tokens and unique filter coefficient definitions. Signed-off-by: Fred Oh <fred.oh@linux.intel.com>

singalsu

Thanks, this is OK for now. In the next step when PIPELINE_FILTERx is definable per end-point we can make more interesting topology variants.

singalsu · 2020-04-03T14:08:21Z

The CI checks output will be interesting. Please update your PR if there are issues.

fredoh9 · 2020-04-03T14:23:22Z

Thanks, this is OK for now. In the next step when PIPELINE_FILTERx is definable per end-point we can make more interesting topology variants.

I will create new issue to follow up this.

For headset playback or capture, default pipe is volume. If The high pass filter is required to prevent HDA codec headset glitches, it can be set by HSMICPROC=eq-iir-volume. It use use 40 Hz high pass filter with +0 db boost. If there is DMIC, this set default pipe as eq-iir, which use the same 40 Hz high-pass filter too. For DMIC16K, 40 Hz high-pass for 16khz version filter is used. Default coefficient for FIR/IIR is changed fr m coef flat to coef pass. Pass configuration has the advantage of low system load. eq_fir_coef_flat.m4 to eq_fir_coef_pass.m4 eq_iir_coef_flat.m4 to eq_iir_coef_pass.m4 Below pipe files are renamed for naming convention, pipe-eq-volume-playback.m4 -> pipe-eq-iir-eq-fir-volume-playback.m4 pipe-eq-capture-16khz.m4 -> pipe-eq-iir-volume-capture-16khz.m4 pipe-eq-capture.m4 -> pipe-eq-iir-volume-capture.m4 Signed-off-by: Fred Oh <fred.oh@linux.intel.com>

fredoh9 · 2020-04-03T16:15:33Z

Thanks, this is OK for now. In the next step when PIPELINE_FILTERx is definable per end-point we can make more interesting topology variants.

I will create new issue to follow up this.

#2707 is created

fredoh9 · 2020-04-06T18:10:08Z

SOFCI TEST

lgirdwood · 2020-04-07T13:37:32Z

CI known issues.

fredoh9 requested review from dbaluta, jajanusz, lbetlej, lgirdwood, mmaka1, plbossart, ranj063, tlauda and xiulipan as code owners March 29, 2020 22:49

fredoh9 commented Mar 29, 2020

View reviewed changes

Comment thread tools/topology/sof/pipe-eq-fir-volume-capture.m4 Resolved

fredoh9 requested a review from singalsu March 30, 2020 00:37

singalsu requested changes Mar 30, 2020

View reviewed changes