Skip to content

Tidy support for 8/16 mics and parallel decimators#197

Merged
ed-xmos merged 16 commits intoxmos:developfrom
ed-xmos:feature/tidy_par_dec
Aug 8, 2023
Merged

Tidy support for 8/16 mics and parallel decimators#197
ed-xmos merged 16 commits intoxmos:developfrom
ed-xmos:feature/tidy_par_dec

Conversation

@ed-xmos
Copy link
Copy Markdown
Contributor

@ed-xmos ed-xmos commented Jul 27, 2023

Aaron Stewart and others added 7 commits June 15, 2023 14:23
Resolves a buffer overrun (out of bounds array access) exception and
ET_ILLEGAL_RESOURCE exception.
Works with 8 or 16 mics in DDR.

Uses decimator that processes samples on multiple threads.

Added API for enable/disable assertion (for debug purposes).

Optionally support button input for selection of which two MICs
are output to the I2S DAC.

Port/Pin configuration is done via app_config.h settings.
Set MIC_TILE in app_conf.h to match desired configuration along with
other options.

When using on Tile 0, USB may not be available and push buttons and
LEDs are not to be used (LEDs will indicate MIC Data for 4 of the
8 data lines). For button mic selection functionality is provided
via another pin (X0D12, J12 - Pin2) with an internal pull-up applied.

When using Tile 1, another pin must be used to drive CODEC_RST_N
high (using the test point on the board), in this example, X1D38
(Port 1O) is used for this purpose. A 3k3 series resistor is used on
this pin which is sufficient to override the weak pull-down on the
board while avoiding signal contention.
This change has not been tested, and there needs to be a decision
on whether this decimator should be part of the mic_array
namespace or left as an application specific one. Which option
is chosen likely dictates whether the name MyMicArray is appropriate.
There appears to be issues with Prefap.hpp being included in the
API header when defining a custom class template.
@mbanth mbanth assigned ed-xmos and unassigned mbanth Jul 27, 2023
@ed-xmos ed-xmos requested a review from mbanth July 27, 2023 16:05
@ed-xmos
Copy link
Copy Markdown
Contributor Author

ed-xmos commented Jul 27, 2023

This adds the 16ch de-interleave which will close this #193. It has a passing unit test and I have tested it in hardware too. These changes do not change other core mic_array features so I think the risk of merging without a full CI run (which doesn't exist yet! #198) is low.

There is also a sample app to show use of the parallel decimators which does not run without specific hardware and so it is not so useful. Perhaps in time, the 16 mic aggregator could replace it or perhaps the 16 mic sum/delay example. However I do not think it is urgent.

@ed-xmos
Copy link
Copy Markdown
Contributor Author

ed-xmos commented Jul 27, 2023

mbanth
mbanth previously requested changes Jul 28, 2023
Comment thread demos/common/src/aic3204/aic3204.c
Comment thread demos/common/src/dac_port.c
Comment thread demos/common/src/dac_port.c Outdated
Comment thread demos/demo_par_decimator/src/app_config.h
Comment thread demos/demo_par_decimator/src/app_i2s.c Outdated
static
i2s_restart_t app_i2s_restart(void* app_context)
{
static unsigned do_restart = 0;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does the value of do_restart ever change?

Comment thread doc/programming_guide/src/vanilla_api.rst
Comment thread lib_mic_array/src/deinterleave16.S Outdated
Comment thread tests/unit/src/test_deinterleave16.c Outdated
Comment thread tests/unit/src/test_deinterleave16.c
</Link>
</Links>
<ExternalDevices>
<Device NodeId="0" Tile="0" Class="SQIFlash" Name="bootFlash" Type="S25FL116K" PageSize="256" SectorSize="4096" NumPages="16384">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit surprised at the inclusion of the Type, PageSize, SectorSize and NumPages arguments. I thought that recent changes Tools 15.2 meant that we no longer needed these arguments and that their inclusion caused problems. However, my notoriously bad memory means that I am not sure of the facts, and I see that the other .xn files in this repository look similar to this one as far as this line goes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ed-xmos ed-xmos requested a review from xhuw August 8, 2023 11:13
Copy link
Copy Markdown

@xhuw xhuw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your changes since Michael's review appear to solve all of his comments. I didn't look at any previous changes.
You might need to get a super admin to work around Michael's "needs work" this approval might do the job though

@xhuw xhuw dismissed mbanth’s stale review August 8, 2023 11:54

He is on holiday and Ed is blocked

@xhuw
Copy link
Copy Markdown

xhuw commented Aug 8, 2023

I "dismissed" Michaels review so now it's mergeable

@ed-xmos ed-xmos merged commit 7c921c6 into xmos:develop Aug 8, 2023
@ed-xmos ed-xmos deleted the feature/tidy_par_dec branch August 8, 2023 12:21
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.

4 participants