Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tools: Topology2: Use for LNL own platform configuration lnl.conf #8888

Merged
merged 1 commit into from
May 6, 2024

Conversation

singalsu
Copy link
Collaborator

The new lnl.conf is copy of mtl.conf but DMIC_DRIVER_VERSION needs to be increased by one for a small registers change.

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.

LGTM. @jsarha @ranj063 pls review.

@singalsu
Copy link
Collaborator Author

NOTE: This PR can't be merged before alsa-utils is updated.

@singalsu singalsu changed the title Tools: Topology2: Use for LNL own platform configuration lnl.conf [DNM] Tools: Topology2: Use for LNL own platform configuration lnl.conf Feb 28, 2024
@singalsu
Copy link
Collaborator Author

NOTE: #8889 contains a quick fix for the issue with HDA generic LNL.

Copy link
Contributor

@jsarha jsarha left a comment

Choose a reason for hiding this comment

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

LGTM

@plbossart
Copy link
Member

what register change @singalsu ? I thought MTL and LNL could use exactly the same blobs?

@singalsu
Copy link
Collaborator Author

what register change @singalsu ? I thought MTL and LNL could use exactly the same blobs?

I also thought so, but I realized there's one bitfield in OUTCONTROL removed and marked as reserved. We should not set those bits due to risk of them becoming reused for something else later. Though there should be no harm from doing it now.

@lgirdwood lgirdwood added this to the v2.10 milestone Mar 4, 2024
@plbossart
Copy link
Member

what register change @singalsu ? I thought MTL and LNL could use exactly the same blobs?

I also thought so, but I realized there's one bitfield in OUTCONTROL removed and marked as reserved. We should not set those bits due to risk of them becoming reused for something else later. Though there should be no harm from doing it now.

Do we need to program this OUTCONTROL bitfield, how important is it?
I mean, if we can keep the same blobs for MTL and LNL it's best to do so and not use an obscure feature of the hardware we never knew we didn't need.

@singalsu
Copy link
Collaborator Author

singalsu commented Mar 5, 2024

Do we need to program this OUTCONTROL bitfield, how important is it?
I mean, if we can keep the same blobs for MTL and LNL it's best to do so and not use an obscure feature of the hardware we never knew we didn't need.

It's the DMA burst size in MTL and it needs to be set. I haven't tried to not set it with MTL but in earlier platforms having non-matching configuration in DMIC IP and in DMA resulted in not working capture. In LNL and after we should not write to registers non-existing control bits.

We identify in topology DMIC driver versions 1-4, with 5 coming, so we already handle a number of different hardware versions in ChromeOS topology builds. We don't have release with v2 (was Sue Creek). But three versions are in use with v1 APL to TGL, v3 MTL, v4 LNL.

@plbossart
Copy link
Member

@singalsu it's pretty obvious why the change happened: the DMA burst is irrelevant for LNL+ since it uses a different DMA. That's not really a DMIC IP change, more its interface. I have no objection if we avoid setting something that is not used, but we should not expect any behavioral change with this PR. Reserved bits are ignored....

@singalsu
Copy link
Collaborator Author

singalsu commented Mar 5, 2024

@singalsu it's pretty obvious why the change happened: the DMA burst is irrelevant for LNL+ since it uses a different DMA. That's not really a DMIC IP change, more its interface. I have no objection if we avoid setting something that is not used, but we should not expect any behavioral change with this PR. Reserved bits are ignored....

Yes, that is true. There is no change in operation from not setting those bits or leaving them set. But it would be good to add to LNL driver a warning of trying to set these bits in the blob similarly as there is for other reserved fields. We used to error about those but due to delays in maintaining the blobs it was too restricting.

Also those bits have been removed in the internal specification for the ACE2.x DMIC IP.

The new lnl.conf is copy of mtl.conf but DMIC_DRIVER_VERSION needs
to be increased by one for a small registers change.

Signed-off-by: Seppo Ingalsuo <[email protected]>
@singalsu singalsu force-pushed the tplg2_add_lnl_conf branch from 566fe5e to 4172a88 Compare April 16, 2024 07:47
@singalsu singalsu marked this pull request as ready for review April 16, 2024 07:47
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.

@singalsu still DNM ?

@singalsu
Copy link
Collaborator Author

singalsu commented May 6, 2024

@singalsu still DNM ?

It's now safe to merge. Alsa-utils patch 19a75d0ebcc9602b7be0043d58740c51fed2ca2c
"topology: nhlt: Intel: Clear DMIC BFTH bits for version" is now included.

@singalsu singalsu changed the title [DNM] Tools: Topology2: Use for LNL own platform configuration lnl.conf Tools: Topology2: Use for LNL own platform configuration lnl.conf May 6, 2024
@kv2019i kv2019i merged commit 07c9043 into thesofproject:main May 6, 2024
44 of 45 checks passed
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