board_helper: support HDA link #5144

brentlu · 2024-08-14T03:28:16Z

Add HDA link support to board helper module.
Modify skl_hda_dsp_generic driver to create dai link array with board helper module.

plbossart · 2024-08-14T10:08:52Z

@bardliao i'll let you review/merge

bardliao · 2024-08-14T12:26:19Z

sound/soc/intel/boards/sof_board_helpers.c

@@ -624,6 +778,9 @@ sof_intel_board_get_ctx(struct device *dev, unsigned long board_quirk)
 				SOF_SSP_PORT_BT_OFFLOAD_SHIFT;
 	}

+	if (board_quirk & SOF_HDA_CODEC_PRESENT)
+		ctx->hda_codec_present = true;


Should we set ctx->hda_codec_present by HDA_EXT_CODEC(codec_mask)?

It's done in machine driver since we didn't pass codec_mask to board helper module.

It's done in machine driver since we didn't pass codec_mask to board helper module.

Can we set ctx->hda_codec_present in machine driver? Just like ctx->hdmi.idisp_codec = true; I am thinking is SOF_HDA_CODEC_PRESENT really needed?

sure, we could remove that flag.

bardliao · 2024-08-14T12:41:43Z

sound/soc/intel/boards/skl_hda_dsp_generic.c

+	card->name = "hda-dsp";
+	card->owner = THIS_MODULE;
+	card->fully_routed = true;
+	card->late_probe = skl_hda_card_late_probe;


Hmm, we need to add Fixes: 33e59e50ee76 ("ASoC: Intel: skl_hda_dsp_generic: Allocate snd_soc_card dynamically") in the commit message, and it is better to put the commit in the first commit. @ujfalusi FYI

omg, how did I done this??? and why it compiled in the first place...

@brentlu, can you add the Cc: [email protected] as well to make sure it is backported to stable.

Err...not sure how to do it...

You just need to add a line above the Fixes tag in the commit message:

Cc: [email protected]

done. Thanks~

bardliao · 2024-08-15T06:07:55Z

sound/soc/intel/boards/skl_hda_dsp_generic.c

+	unsigned long board_quirk = 0;
+	int ssp_bt;
+
+	if (mach_params->codec_mask & EXT_CODEC_MASK)


We can reuse HDA_EXT_CODEC(mach_params->codec_mask)

ujfalusi · 2024-08-26T06:55:30Z

sound/soc/intel/boards/skl_hda_dsp_generic.c

+	card->name = "hda-dsp";
+	card->owner = THIS_MODULE;
+	card->fully_routed = true;
+	card->late_probe = skl_hda_card_late_probe;


omg, how did I done this??? and why it compiled in the first place...

ujfalusi · 2024-08-26T06:57:36Z

sound/soc/intel/boards/skl_hda_dsp_generic.c

+	card->name = "hda-dsp";
+	card->owner = THIS_MODULE;
+	card->fully_routed = true;
+	card->late_probe = skl_hda_card_late_probe;


@brentlu, can you add the Cc: [email protected] as well to make sure it is backported to stable.

ujfalusi · 2024-08-26T07:18:05Z

sound/soc/intel/boards/skl_hda_dsp_generic.c

@@ -278,6 +162,7 @@ module_platform_driver(skl_hda_audio)
 /* Module information */
 MODULE_DESCRIPTION("SKL/KBL/BXT/APL HDA Generic Machine driver");
 MODULE_AUTHOR("Rakesh Ughreja <[email protected]>");
+MODULE_AUTHOR("Brent Lu <[email protected]>");


Probably mention this in the comment message if it is really needed?

ujfalusi · 2024-08-26T07:30:26Z

sound/soc/intel/boards/skl_hda_dsp_generic.c

+#define BT_OFFLOAD_BE_ID	8
+
+#define HDA_LINK_ORDER	SOF_LINK_ORDER(SOF_LINK_IDISP_HDMI,  \
+					SOF_LINK_HDA,        \


it is interesting that checkpatch is not complaining, but it would look better if the lines are aligned, or follow the other files and use 1 less TAB.

plbossart · 2024-08-26T08:12:37Z

sound/soc/intel/boards/skl_hda_dsp_generic.c

@@ -278,6 +162,7 @@ module_platform_driver(skl_hda_audio)
 /* Module information */
 MODULE_DESCRIPTION("SKL/KBL/BXT/APL HDA Generic Machine driver");
 MODULE_AUTHOR("Rakesh Ughreja <[email protected]>");
+MODULE_AUTHOR("Brent Lu <[email protected]>");


My take is that references to 'author' should be avoided, be it in a file header or module description. If I added this to all the files I touched I would be the author of pretty much everything in SOF. same for Ranjani, Peter, Kai, Bard. The commit tamper-proof Signed-off-by is enough to give you credit for your work.

I just thought it is kinda mechanism that you could get notification when someone submit a patch to alsa-dev about it. I'll remove it from machine drivers if I got a chance.

ujfalusi · 2024-08-27T06:24:23Z

sound/soc/intel/boards/skl_hda_dsp_generic.c

+	card->name = "hda-dsp";
+	card->owner = THIS_MODULE;
+	card->fully_routed = true;
+	card->late_probe = skl_hda_card_late_probe;


You just need to add a line above the Fixes tag in the commit message:

Cc: [email protected]

ujfalusi · 2024-08-27T06:26:24Z

sound/soc/intel/boards/sof_board_helpers.c

+		return ret;
+	}
+
+	return 0;


You can simplify the code by:

if (ret) dev_err(rtd->dev, "fail to add hda routes, ret %d\n", ret); return ret;

Use semicolon to complete an expression instead of comma for following build error. sound/soc/intel/boards/skl_hda_dsp_generic.c:108:2: error: expected expression dev_dbg(&pdev->dev, "board_quirk = %lx\n", board_quirk); Cc: [email protected] Fixes: 33e59e5 ("ASoC: Intel: skl_hda_dsp_generic: Allocate snd_soc_card dynamically") Signed-off-by: Brent Lu <[email protected]>

Add a helper function for machine drivers to initialize HDA external codec DAI link. Signed-off-by: Brent Lu <[email protected]>

Use intel_board module to create DAI link array for Intel iDisp HDMI, HDA external codec, DMIC01, DMIC16K, and BT audio offload DAI BE links. Signed-off-by: Brent Lu <[email protected]>

bardliao · 2024-08-30T02:20:55Z

SOFCI TEST

brentlu requested review from lgirdwood, plbossart, ranj063, kv2019i, dbaluta, bardliao and ujfalusi as code owners August 14, 2024 03:28

brentlu force-pushed the hda-mach-4 branch 2 times, most recently from 3dbbdb0 to ae551c1 Compare August 14, 2024 06:31

bardliao reviewed Aug 14, 2024

View reviewed changes

brentlu force-pushed the hda-mach-4 branch 2 times, most recently from 7e8dfd5 to 09f7f05 Compare August 15, 2024 02:50

bardliao reviewed Aug 15, 2024

View reviewed changes

brentlu force-pushed the hda-mach-4 branch 2 times, most recently from a4c2299 to eea26b9 Compare August 19, 2024 02:29

bardliao previously approved these changes Aug 19, 2024

View reviewed changes

brentlu dismissed bardliao’s stale review via 023ca5f August 23, 2024 16:05

brentlu force-pushed the hda-mach-4 branch from eea26b9 to 023ca5f Compare August 23, 2024 16:05

bardliao previously approved these changes Aug 26, 2024

View reviewed changes

ujfalusi reviewed Aug 26, 2024

View reviewed changes

plbossart reviewed Aug 26, 2024

View reviewed changes

brentlu dismissed bardliao’s stale review via f96e28b August 27, 2024 01:40

brentlu force-pushed the hda-mach-4 branch from 023ca5f to f96e28b Compare August 27, 2024 01:40

ujfalusi reviewed Aug 27, 2024

View reviewed changes

brentlu force-pushed the hda-mach-4 branch from f96e28b to db00972 Compare August 28, 2024 01:26

bardliao approved these changes Aug 28, 2024

View reviewed changes

brentlu added 2 commits August 28, 2024 17:03

ASoC: Intel: board_helpers: support HDA link initialization

635a9ed

Add a helper function for machine drivers to initialize HDA external codec DAI link. Signed-off-by: Brent Lu <[email protected]>

ASoC: Intel: skl_hda_dsp_generic: use common module for DAI links

db00972

Use intel_board module to create DAI link array for Intel iDisp HDMI, HDA external codec, DMIC01, DMIC16K, and BT audio offload DAI BE links. Signed-off-by: Brent Lu <[email protected]>

ujfalusi approved these changes Aug 29, 2024

View reviewed changes

bardliao merged commit 58b3797 into thesofproject:topic/sof-dev Aug 30, 2024
11 of 14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

board_helper: support HDA link #5144

board_helper: support HDA link #5144

brentlu commented Aug 14, 2024

plbossart commented Aug 14, 2024

bardliao Aug 14, 2024

brentlu Aug 15, 2024

bardliao Aug 15, 2024

brentlu Aug 19, 2024

bardliao Aug 14, 2024

brentlu Aug 19, 2024

ujfalusi Aug 26, 2024

ujfalusi Aug 26, 2024

brentlu Aug 27, 2024

ujfalusi Aug 27, 2024

brentlu Aug 28, 2024

bardliao Aug 15, 2024

brentlu Aug 19, 2024

ujfalusi Aug 26, 2024

ujfalusi Aug 26, 2024

ujfalusi Aug 26, 2024

brentlu Aug 27, 2024

ujfalusi Aug 26, 2024

brentlu Aug 27, 2024

plbossart Aug 26, 2024

brentlu Aug 27, 2024

ujfalusi Aug 27, 2024

ujfalusi Aug 27, 2024

brentlu Aug 28, 2024

bardliao commented Aug 30, 2024

board_helper: support HDA link #5144

board_helper: support HDA link #5144

Conversation

brentlu commented Aug 14, 2024

plbossart commented Aug 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bardliao commented Aug 30, 2024