Skip to content

Conversation

@plbossart
Copy link
Member

@plbossart plbossart commented Jun 22, 2020

remove SRC since it's just too big for the internal memories.
We keep the mixer w/ a regular PCM and a 'deep buffer' one (limited to 10ms periods due to memory sizes).

Tested on CHT w/ rt5640 and max98090 (Cyan Chromebook)

@plbossart
Copy link
Member Author

Tentatively Fixes #3063

Copy link
Collaborator

@singalsu singalsu left a comment

Choose a reason for hiding this comment

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

But please fix the missing PCM. I don' think we have other way to test mixer component.

@marc-hb
Copy link
Collaborator

marc-hb commented Jun 23, 2020

hence RFC status.

Someone showed me it's now possible to switch any PR back to "Draft" status. Github drafts:

  • cannot be merged
  • appear in a different color
  • can be filtered out in queries with draft:false or -is:draft (forgot which one sorry)

I've been using github drafts since the feature was created and there's no drawback whatsoever. Let's use drafts.

Also:

  • drafts do not spam CODEOWNERS automatically (you can still add anyone manually)

... so I always start every PR as a draft in case CI tells me I missed something, this saves a lot of email.

https://github.blog/2019-02-14-introducing-draft-pull-requests/
https://github.blog/changelog/2020-04-08-convert-pull-request-to-draft/

Off-topic sorry.

Add separate pipeline starting with a mixer (similar to Baytrail
switch matrix and SKL closed-source firmware)

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Add a new pipeline definition to avoid mucking with either
pipe-low-latency or pipe-media.

The code is 90% similar to pipeline-volume-playback but includes a
scheduling component.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
remove the low-latency pipeline and pcm-media.

Instead, we use 2 two pipe-host-volume-playback with different periods
connected to the same mixer-dai.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
@plbossart plbossart force-pushed the fix/byt-cht-remove-SRC branch from 3d523a3 to 96b0629 Compare June 27, 2020 19:53
@plbossart plbossart changed the title [RFC] remove SRC for Baytrail and CherryTrail remove SRC for Baytrail and CherryTrail Jun 27, 2020
@plbossart plbossart requested review from lgirdwood and singalsu June 27, 2020 20:49
Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

This all looks good, but it looks like CI is attempting a hw params that are no longer supported ?

@lgirdwood
Copy link
Member

This all looks good, but it looks like CI is attempting a hw params that are no longer supported ?

The same params are actually working on boards with a codec, but I dont see any immediate difference between the topologies.

@plbossart
Copy link
Member Author

This all looks good, but it looks like CI is attempting a hw params that are no longer supported ?

The same params are actually working on boards with a codec, but I dont see any immediate difference between the topologies.

kernel: [  177.961849]  PCM: ASoC: no BE found for SSP2.OUT
kernel: [  177.962840]  PCM: ASoC: no backend DAIs enabled for PCM

Likely a mismatch between topology and machine driver. Will test, maybe I went too fast on the nocodec support

Additional work needed to make the BE name and index configuration, as
well as LBM quirk.
We should be using a single topology here.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Same parameters, so let's use the same template

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
No one uses this topology so let's remove it.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Only the SSP settings need to be different.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
We really don't care about SRC on those platforms and the memory
footprint is too large anyways.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
@plbossart
Copy link
Member Author

This all looks good, but it looks like CI is attempting a hw params that are no longer supported ?

The same params are actually working on boards with a codec, but I dont see any immediate difference between the topologies.

kernel: [  177.961849]  PCM: ASoC: no BE found for SSP2.OUT
kernel: [  177.962840]  PCM: ASoC: no backend DAIs enabled for PCM

Likely a mismatch between topology and machine driver. Will test, maybe I went too fast on the nocodec support

What I thought, stupid edit mistake

 DAI_ADD(sof/pipe-mixer-dai-playback.m4,
-       1, SSP, SSP_NUM, SSP2-Codec,
+       1, SSP, SSP_NUM, NoCodec-2,

Fixed now, let's see how CI likes it.

@plbossart plbossart force-pushed the fix/byt-cht-remove-SRC branch from 96b0629 to f795271 Compare June 29, 2020 22:35
@plbossart plbossart requested a review from lgirdwood June 29, 2020 22:35
@aiChaoSONG
Copy link
Collaborator

SOFCI TEST

@xiulipan
Copy link
Contributor

Now the .TEXT size is 0x12ea7 on CHT, 0x12d27 on BYT

https://sof-ci.01.org/sofpr/PR3080/build6448/build/cht_xcc.txt

writing module 0 sof-cht

	Totals	Start		End		Size
	TEXT	0xff2c0000	0xff2d2ea7	0x12ea7
	DATA	0xff300010	0xff328000	0x27ff0
	BSS	0xff3023f0	0xff306258	0x3e68

https://sof-ci.01.org/sofpr/PR3080/build6448/build/byt_xcc.txt

writing module 0 sof-byt

	Totals	Start		End		Size
	TEXT	0xff2c0000	0xff2d2d27	0x12d27
	DATA	0xff300010	0xff328000	0x27ff0
	BSS	0xff302310	0xff306178	0x3e68

Copy link
Collaborator

@aiChaoSONG aiChaoSONG left a comment

Choose a reason for hiding this comment

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

Verified by CI

@lgirdwood
Copy link
Member

lgirdwood commented Jun 30, 2020

Jenkins BAT fails sound clean, could be startup DUT or Test equipment clock delta appearing as a higher frequency that settles quickly.
@zrombel any idea why CI would fail for ICL when this is unrelated.

@aiChaoSONG
Copy link
Collaborator

@lgirdwood Can we get this merged? The ICL error obviously has nothing to do with this PR.

@lgirdwood
Copy link
Member

@aiChaoSONG it was a CI server issue, it's been rerun and is now good..

@lgirdwood lgirdwood merged commit 7ef7313 into thesofproject:master Jul 7, 2020
@marc-hb
Copy link
Collaborator

marc-hb commented Jul 8, 2020

Note there's a new, unrelated regression on these same platforms now: #3146

@marc-hb
Copy link
Collaborator

marc-hb commented Jul 11, 2020

I think the wrong _defconfig files have been used for the last 3- months: #2711 (comment)

@marc-hb
Copy link
Collaborator

marc-hb commented Jul 14, 2020

PR #3167 just got merged and it turns off CONFIG_OPTIMIZE_FOR_SIZE again for baytrail (and maybe for others)

@plbossart
Copy link
Member Author

PR #3167 just got merged and it turns off CONFIG_OPTIMIZE_FOR_SIZE again for baytrail (and maybe for others)

Are you saying PR #3167 is good or bad? Can't follow your point

@marc-hb
Copy link
Collaborator

marc-hb commented Jul 14, 2020

Are you saying PR #3167 is good or bad?

I'm saying neither; I'm merely trying to mitigate any future .TEXT size confusion.

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.

6 participants