On Mon, 2019-04-08 at 08:14 -0500, Pierre-Louis Bossart wrote:
>
> 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?
Hi Pierre,
I can confirm that this patch is necessary even if INFO_RESUME is not
set.
Thanks,
Ranjani
>
> 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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
