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

board_helpers: support DAI link ID customization #4736

Merged
merged 3 commits into from
Jan 24, 2024

Conversation

brentlu
Copy link

@brentlu brentlu commented Dec 8, 2023

To support boards in driver sof_ssp_amp, we add a new field "link_id_overwrite" to sof_card_private structure so machine driver could specify the ID for each DAI link (just like the way to specify the link order in link_order_overwrite).

sof_ssp_amp_card.dai_link = dai_links;
if (ctx->ssp_mask_hdmi_in) {
/* the topology supports HDMI-IN uses fixed BE ID for DAI links */
ctx->link_id_overwrite = NO_CODEC_LINK_IDS;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, what if ctx->ssp_mask_hdmi_in is true, but ctx->amp_type != CODEC_NONE? In that case, the second be_id will still be SPK_BE_ID, right?

Copy link
Author

Choose a reason for hiding this comment

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

In this case, amp's BE ID will always be SPK_BE_ID regardless the number of HDMI-IN links.

@brentlu brentlu force-pushed the intel-board-8 branch 2 times, most recently from e89d7be to 2e97e34 Compare December 11, 2023 00:54
@bardliao
Copy link
Collaborator

SOFCI TEST

SOF_LINK_BT_OFFLOAD, \
SOF_LINK_NONE)

#define NO_CODEC_LINK_IDS SOF_LINK_ORDER(HDMI_IN_BE_ID, \
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I am not following what this 'NO_CODEC' means?

We already have a nocodec mode, but then we don't use this machine driver at all.

I guess I must be missing a lot of context on what you are trying to do @brentlu

Copy link
Author

Choose a reason for hiding this comment

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

It just means the boards without headphone codec. Sorry for the confusing with the nocodec mode. I will rename the macros.

@brentlu
Copy link
Author

brentlu commented Jan 5, 2024

It's designed for sof_ssp_amp.c where id field of each BE dai link is a fixed value. Like the id of DMIC01 link is always 3 even when AMP BE link (id=2) is absent.

To pass the id number of each BE DAI link to intel-board-helper module, a variable link_id_overwrite is added to sof_card_private structure. The link_id_overwrite will store the id number of all BE DAI links in one 64-bit variable. To simplify the implementation, we leverage the macro SOF_LINK_ORDER to encode id numbers so each link has 4 bit space for the id number.

#define SSP_AMP_LINK_IDS SOF_LINK_ORDER(HDMI_IN_BE_ID,
SPK_BE_ID,
DMIC01_BE_ID,
DMIC16K_BE_ID,
INTEL_HDMI_BE_ID,
BT_OFFLOAD_BE_ID,
0)

@plbossart
Copy link
Member

It's designed for sof_ssp_amp.c where id field of each BE dai link is a fixed value. Like the id of DMIC01 link is always 3 even when AMP BE link (id=2) is absent.

To pass the id number of each BE DAI link to intel-board-helper module, a variable link_id_overwrite is added to sof_card_private structure. The link_id_overwrite will store the id number of all BE DAI links in one 64-bit variable. To simplify the implementation, we leverage the macro SOF_LINK_ORDER to encode id numbers so each link has 4 bit space for the id number.

#define SSP_AMP_LINK_IDS SOF_LINK_ORDER(HDMI_IN_BE_ID, SPK_BE_ID, DMIC01_BE_ID, DMIC16K_BE_ID, INTEL_HDMI_BE_ID, BT_OFFLOAD_BE_ID, 0)

You need to use u64 is you assume a 64-bit variable, and add a description of the encoding in the commit message and the structure.

@brentlu
Copy link
Author

brentlu commented Jan 9, 2024

You need to use u64 is you assume a 64-bit variable, and add a description of the encoding in the commit message and the structure.

I checked the code again and realized only 28 bits are used, so I left the variable type unchanged. Extra comment is added to commit message and structure.

Add an new field link_id_overwrite to sof_card_private structure to
support machine drivers which DAI link ID is fixed number or
discontinue (i.e. no-codec boards). If this field is zero, DAI array
index will be used as link ID. Otherwise the value extracted from
link_id_overwrite will be used.

The field link_id_overwrite is supposed to be initialized by
SOF_LINK_IDS macro like following example.

ctx->link_id_overwrite = SOF_LINK_IDS(HEADPHONE_BE_ID,  \
				      DMIC01_BE_ID,     \
				      DMIC16K_BE_ID,    \
				      IDISP_HDMI_BE_ID, \
				      SPK_BE_ID,        \
				      BT_OFFLOAD_BE_ID, \
				      HDMI_IN_BE_ID)

An exception is that, if you use link_order_overwrite to overwrite
DAI link order, then you need to use the same order to build
link_id_overwrite variable as well.

Signed-off-by: Brent Lu <[email protected]>
Use intel_board module to generate DAI link array and update num_links
field in snd_soc_card structure.

Signed-off-by: Brent Lu <[email protected]>
Since there is a helper function to generate entire DAI link array, we
switch individual dai link helpers to static function. No functional
change in this commit.

Signed-off-by: Brent Lu <[email protected]>
Copy link
Member

@plbossart plbossart left a comment

Choose a reason for hiding this comment

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

sounds good but @bardliao must approve and merge

@bardliao bardliao merged commit 192b399 into thesofproject:topic/sof-dev Jan 24, 2024
13 of 14 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.

3 participants