Message ID | 20220114150129.v2.3.I3c79b1466c14b02980071221e5b99283cd26ec77@changeid (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | (Re)enable DP/HDMI audio for RK3399 Gru | expand |
Hi, On Sat, Jan 15, 2022 at 7:03 AM Brian Norris <briannorris@chromium.org> wrote: > > Now that the cdn-dp driver supports plug-change callbacks, let's wire it > up. > > Signed-off-by: Brian Norris <briannorris@chromium.org> > --- > > (no changes since v1) > > sound/soc/rockchip/rk3399_gru_sound.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/sound/soc/rockchip/rk3399_gru_sound.c b/sound/soc/rockchip/rk3399_gru_sound.c > index e2d52d8d0ff9..eeef3ed70037 100644 > --- a/sound/soc/rockchip/rk3399_gru_sound.c > +++ b/sound/soc/rockchip/rk3399_gru_sound.c > @@ -164,6 +164,25 @@ static int rockchip_sound_da7219_hw_params(struct snd_pcm_substream *substream, > return 0; > } > > +static struct snd_soc_jack cdn_dp_card_jack; > + > +static int rockchip_sound_cdndp_init(struct snd_soc_pcm_runtime *rtd) > +{ > + struct snd_soc_component *component = asoc_rtd_to_codec(rtd, 0)->component; Using snd_soc_card_get_codec_dai() might be a better choice throughout this driver. While it will work for the cdn_dp case, because it is the first DAI in |rockchip_dais[]|, all the invocations for the other codecs are likely returning the wrong DAI. For this particular patch it works either way, so Reviewed-by: Chen-Yu Tsai <wenst@chromium.org> > + struct snd_soc_card *card = rtd->card; > + int ret; > + > + /* Enable jack detection. */ > + ret = snd_soc_card_jack_new(card, "DP Jack", SND_JACK_LINEOUT, > + &cdn_dp_card_jack, NULL, 0); > + if (ret) { > + dev_err(card->dev, "Can't create DP Jack %d\n", ret); > + return ret; > + } > + > + return snd_soc_component_set_jack(component, &cdn_dp_card_jack, NULL); > +} > + > static int rockchip_sound_da7219_init(struct snd_soc_pcm_runtime *rtd) > { > struct snd_soc_component *component = asoc_rtd_to_codec(rtd, 0)->component; > @@ -315,6 +334,7 @@ static const struct snd_soc_dai_link rockchip_dais[] = { > [DAILINK_CDNDP] = { > .name = "DP", > .stream_name = "DP PCM", > + .init = rockchip_sound_cdndp_init, > .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | > SND_SOC_DAIFMT_CBS_CFS, > SND_SOC_DAILINK_REG(cdndp), > -- > 2.34.1.703.g22d0c6ccf7-goog > > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip
Hi Chen-Yu, On Mon, Jan 17, 2022 at 05:01:52PM +0800, Chen-Yu Tsai wrote: > On Sat, Jan 15, 2022 at 7:03 AM Brian Norris <briannorris@chromium.org> wrote: > > > > Now that the cdn-dp driver supports plug-change callbacks, let's wire it > > up. > > > > Signed-off-by: Brian Norris <briannorris@chromium.org> > > --- > > > > (no changes since v1) > > > > sound/soc/rockchip/rk3399_gru_sound.c | 20 ++++++++++++++++++++ > > 1 file changed, 20 insertions(+) > > > > diff --git a/sound/soc/rockchip/rk3399_gru_sound.c b/sound/soc/rockchip/rk3399_gru_sound.c > > index e2d52d8d0ff9..eeef3ed70037 100644 > > --- a/sound/soc/rockchip/rk3399_gru_sound.c > > +++ b/sound/soc/rockchip/rk3399_gru_sound.c > > @@ -164,6 +164,25 @@ static int rockchip_sound_da7219_hw_params(struct snd_pcm_substream *substream, > > return 0; > > } > > > > +static struct snd_soc_jack cdn_dp_card_jack; > > + > > +static int rockchip_sound_cdndp_init(struct snd_soc_pcm_runtime *rtd) > > +{ > > + struct snd_soc_component *component = asoc_rtd_to_codec(rtd, 0)->component; > > Using snd_soc_card_get_codec_dai() might be a better choice throughout this > driver. While it will work for the cdn_dp case, because it is the first DAI > in |rockchip_dais[]|, all the invocations for the other codecs are likely > returning the wrong DAI. I'll admit, I'm not very familiar with the ASoC object model, so you may well be correct that there's something fishy in here. But I did trace through the objects involved here, and we *are* getting the correct DAI for both this case and the DA7219 case (preexisting code). It looks like we actually have a new runtime for each of our static dai_links: devm_snd_soc_register_card() ... for_each_card_prelinks() snd_soc_add_pcm_runtime() So I think this is valid to keep as-is. > For this particular patch it works either way, so > > Reviewed-by: Chen-Yu Tsai <wenst@chromium.org> Thanks for looking! Brian
On Wed, Jan 19, 2022 at 4:18 AM Brian Norris <briannorris@chromium.org> wrote: > > Hi Chen-Yu, > > On Mon, Jan 17, 2022 at 05:01:52PM +0800, Chen-Yu Tsai wrote: > > On Sat, Jan 15, 2022 at 7:03 AM Brian Norris <briannorris@chromium.org> wrote: > > > > > > Now that the cdn-dp driver supports plug-change callbacks, let's wire it > > > up. > > > > > > Signed-off-by: Brian Norris <briannorris@chromium.org> > > > --- > > > > > > (no changes since v1) > > > > > > sound/soc/rockchip/rk3399_gru_sound.c | 20 ++++++++++++++++++++ > > > 1 file changed, 20 insertions(+) > > > > > > diff --git a/sound/soc/rockchip/rk3399_gru_sound.c b/sound/soc/rockchip/rk3399_gru_sound.c > > > index e2d52d8d0ff9..eeef3ed70037 100644 > > > --- a/sound/soc/rockchip/rk3399_gru_sound.c > > > +++ b/sound/soc/rockchip/rk3399_gru_sound.c > > > @@ -164,6 +164,25 @@ static int rockchip_sound_da7219_hw_params(struct snd_pcm_substream *substream, > > > return 0; > > > } > > > > > > +static struct snd_soc_jack cdn_dp_card_jack; > > > + > > > +static int rockchip_sound_cdndp_init(struct snd_soc_pcm_runtime *rtd) > > > +{ > > > + struct snd_soc_component *component = asoc_rtd_to_codec(rtd, 0)->component; > > > > Using snd_soc_card_get_codec_dai() might be a better choice throughout this > > driver. While it will work for the cdn_dp case, because it is the first DAI > > in |rockchip_dais[]|, all the invocations for the other codecs are likely > > returning the wrong DAI. > > I'll admit, I'm not very familiar with the ASoC object model, so you may > well be correct that there's something fishy in here. But I did trace > through the objects involved here, and we *are* getting the correct DAI > for both this case and the DA7219 case (preexisting code). Neither am I, so ... > It looks like we actually have a new runtime for each of our static > dai_links: > > devm_snd_soc_register_card() > ... > for_each_card_prelinks() > snd_soc_add_pcm_runtime() > > So I think this is valid to keep as-is. I missed this bit. As you say, things are good. > > For this particular patch it works either way, so > > > > Reviewed-by: Chen-Yu Tsai <wenst@chromium.org> > > Thanks for looking! And thanks for double checking!
diff --git a/sound/soc/rockchip/rk3399_gru_sound.c b/sound/soc/rockchip/rk3399_gru_sound.c index e2d52d8d0ff9..eeef3ed70037 100644 --- a/sound/soc/rockchip/rk3399_gru_sound.c +++ b/sound/soc/rockchip/rk3399_gru_sound.c @@ -164,6 +164,25 @@ static int rockchip_sound_da7219_hw_params(struct snd_pcm_substream *substream, return 0; } +static struct snd_soc_jack cdn_dp_card_jack; + +static int rockchip_sound_cdndp_init(struct snd_soc_pcm_runtime *rtd) +{ + struct snd_soc_component *component = asoc_rtd_to_codec(rtd, 0)->component; + struct snd_soc_card *card = rtd->card; + int ret; + + /* Enable jack detection. */ + ret = snd_soc_card_jack_new(card, "DP Jack", SND_JACK_LINEOUT, + &cdn_dp_card_jack, NULL, 0); + if (ret) { + dev_err(card->dev, "Can't create DP Jack %d\n", ret); + return ret; + } + + return snd_soc_component_set_jack(component, &cdn_dp_card_jack, NULL); +} + static int rockchip_sound_da7219_init(struct snd_soc_pcm_runtime *rtd) { struct snd_soc_component *component = asoc_rtd_to_codec(rtd, 0)->component; @@ -315,6 +334,7 @@ static const struct snd_soc_dai_link rockchip_dais[] = { [DAILINK_CDNDP] = { .name = "DP", .stream_name = "DP PCM", + .init = rockchip_sound_cdndp_init, .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS, SND_SOC_DAILINK_REG(cdndp),
Now that the cdn-dp driver supports plug-change callbacks, let's wire it up. Signed-off-by: Brian Norris <briannorris@chromium.org> --- (no changes since v1) sound/soc/rockchip/rk3399_gru_sound.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)