Re: [PATCH] ASoC: cAVS: add device_link to HDMI audio

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 4/7/19 8:21 PM, libin.yang@xxxxxxxxx wrote:
From: Libin Yang <libin.yang@xxxxxxxxx>

In resume from S3, HDAC HDMI codec driver dapm event callback may be
operated before HDMI codec driver turns on the display audio power
domain because of the contest between display driver and hdmi codec driver.

This patch adds the device_link between cAVS generic machine device
(consumer) and hdmi codec device (supplier) to make sure the sequence is
always correct.

Signed-off-by: Libin Yang <libin.yang@xxxxxxxxx>
---
  sound/soc/intel/boards/skl_hda_dsp_common.c  | 15 +++++++++++++++
  sound/soc/intel/boards/skl_hda_dsp_common.h  |  1 +
  sound/soc/intel/boards/skl_hda_dsp_generic.c | 12 ++++++++++++

It's Monday morning and I have mixed feelings about this fix.

0. What does 'resume from S3' mean? from the description it looks like the application was playing already before entering S3? I am ready to bet it's a use case that was never tested in the past given that Chromebooks tend to stop all streams before entering S3...

1. As discussed in other threads, Takashi suggested that maybe the INFO_RESUME flag should be removed? What would be the impact should we indeed remove this flag, is this patch still necessary?

2. if this is really a generic issue then shouldn't it be fixed for all users of the iDISP link? Why stop at the HDAudio machine driver?

3. we already have the component model to deal with interaction between i915 and audio, now we are adding a second layer. That looks clunky.

Thanks
-Pierre

  3 files changed, 28 insertions(+)

diff --git a/sound/soc/intel/boards/skl_hda_dsp_common.c b/sound/soc/intel/boards/skl_hda_dsp_common.c
index 9b30c0e..01c8937 100644
--- a/sound/soc/intel/boards/skl_hda_dsp_common.c
+++ b/sound/soc/intel/boards/skl_hda_dsp_common.c
@@ -17,6 +17,18 @@
#define NAME_SIZE 32 +static int skl_hdmi_init(struct snd_soc_pcm_runtime *rtd)
+{
+	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(rtd->card);
+	struct snd_soc_dai *dai = rtd->codec_dai;
+
+	if (!ctx->link)
+		ctx->link = device_link_add(rtd->card->dev, dai->dev,
+					    DL_FLAG_RPM_ACTIVE);
+
+	return 0;
+}
+
  int skl_hda_hdmi_add_pcm(struct snd_soc_card *card, int device)
  {
  	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(card);
@@ -48,6 +60,7 @@ struct snd_soc_dai_link skl_hda_be_dai_links[HDA_DSP_MAX_BE_DAI_LINKS] = {
  		.cpu_dai_name = "iDisp1 Pin",
  		.codec_name = "ehdaudio0D2",
  		.codec_dai_name = "intel-hdmi-hifi1",
+		.init = skl_hdmi_init,
  		.dpcm_playback = 1,
  		.no_pcm = 1,
  		.trigger[0] = SND_SOC_DPCM_TRIGGER_POST,
@@ -58,6 +71,7 @@ struct snd_soc_dai_link skl_hda_be_dai_links[HDA_DSP_MAX_BE_DAI_LINKS] = {
  		.cpu_dai_name = "iDisp2 Pin",
  		.codec_name = "ehdaudio0D2",
  		.codec_dai_name = "intel-hdmi-hifi2",
+		.init = skl_hdmi_init,
  		.dpcm_playback = 1,
  		.no_pcm = 1,
  		.trigger[0] = SND_SOC_DPCM_TRIGGER_POST,
@@ -68,6 +82,7 @@ struct snd_soc_dai_link skl_hda_be_dai_links[HDA_DSP_MAX_BE_DAI_LINKS] = {
  		.cpu_dai_name = "iDisp3 Pin",
  		.codec_name = "ehdaudio0D2",
  		.codec_dai_name = "intel-hdmi-hifi3",
+		.init = skl_hdmi_init,
  		.dpcm_playback = 1,
  		.no_pcm = 1,
  		.trigger[0] = SND_SOC_DPCM_TRIGGER_POST,
diff --git a/sound/soc/intel/boards/skl_hda_dsp_common.h b/sound/soc/intel/boards/skl_hda_dsp_common.h
index 87c50af..df5cc6b 100644
--- a/sound/soc/intel/boards/skl_hda_dsp_common.h
+++ b/sound/soc/intel/boards/skl_hda_dsp_common.h
@@ -29,6 +29,7 @@ struct skl_hda_private {
  	int pcm_count;
  	int dai_index;
  	const char *platform_name;
+	struct device_link *link;
  };
extern struct snd_soc_dai_link skl_hda_be_dai_links[HDA_DSP_MAX_BE_DAI_LINKS];
diff --git a/sound/soc/intel/boards/skl_hda_dsp_generic.c b/sound/soc/intel/boards/skl_hda_dsp_generic.c
index b9a21e6..ceca11e 100644
--- a/sound/soc/intel/boards/skl_hda_dsp_generic.c
+++ b/sound/soc/intel/boards/skl_hda_dsp_generic.c
@@ -166,8 +166,20 @@ static int skl_hda_audio_probe(struct platform_device *pdev)
  	return devm_snd_soc_register_card(&pdev->dev, &hda_soc_card);
  }
+static int skl_hda_audio_remove(struct platform_device *pdev)
+{
+	struct snd_soc_card *card = platform_get_drvdata(pdev);
+	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(card);
+
+	if (ctx->link)
+		device_link_del(ctx->link);
+
+	return 0;
+}
+
  static struct platform_driver skl_hda_audio = {
  	.probe = skl_hda_audio_probe,
+	.remove = skl_hda_audio_remove,
  	.driver = {
  		.name = "skl_hda_dsp_generic",
  		.pm = &snd_soc_pm_ops,

_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux