[PATCH v2 0/3] (Re)enable DP/HDMI audio for RK3399 Gru
This series fixes DP/HDMI audio for RK3399 Gru systems.
First, there was a regression with the switch to SPDIF. Patch 1 can be taken separately as a regression fix if desired. But it's not quite so useful (at least on Chrome OS systems) without the second part.
Second, jack detection was never upstreamed, because the hdmi-codec dependencies were still being worked out when this platform was first supported.
Patches cover a few subsystems. Perhaps this is something for arm-soc?
Changes in v2: - (Un)set pinctrl, because the default assumes we're routing out to external pins
Brian Norris (3): arm64: dts: rockchip: Switch RK3399-Gru DP to SPDIF output drm/rockchip: cdn-dp: Support HDMI codec plug-change callback ASoC: rk3399_gru_sound: Wire up DP jack detection
arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi | 17 ++++++++---- drivers/gpu/drm/rockchip/cdn-dp-core.c | 28 ++++++++++++++++++++ drivers/gpu/drm/rockchip/cdn-dp-core.h | 4 +++ sound/soc/rockchip/rk3399_gru_sound.c | 20 ++++++++++++++ 4 files changed, 64 insertions(+), 5 deletions(-)
Commit b18c6c3c7768 ("ASoC: rockchip: cdn-dp sound output use spdif") switched the platform to SPDIF, but we didn't fix up the device tree.
Drop the pinctrl settings, because the 'spdif_bus' pins are either: * unused (on kevin, bob), so the settings is ~harmless * used by a different function (on scarlet), which causes probe failures (!!)
Fixes: b18c6c3c7768 ("ASoC: rockchip: cdn-dp sound output use spdif") Signed-off-by: Brian Norris briannorris@chromium.org ---
Changes in v2: - (Un)set pinctrl, because the default assumes we're routing out to external pins
arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi index 45a5ae5d2027..162f08bca0d4 100644 --- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi @@ -286,7 +286,7 @@ max98357a: max98357a {
sound: sound { compatible = "rockchip,rk3399-gru-sound"; - rockchip,cpu = <&i2s0 &i2s2>; + rockchip,cpu = <&i2s0 &spdif>; }; };
@@ -437,10 +437,6 @@ &i2s0 { status = "okay"; };
-&i2s2 { - status = "okay"; -}; - &io_domains { status = "okay";
@@ -537,6 +533,17 @@ &sdmmc { vqmmc-supply = <&ppvar_sd_card_io>; };
+&spdif { + status = "okay"; + + /* + * SPDIF is routed internally to DP; we either don't use these pins, or + * mux them to something else. + */ + /delete-property/ pinctrl-0; + /delete-property/ pinctrl-names; +}; + &spi1 { status = "okay";
On Sat, Jan 15, 2022 at 7:03 AM Brian Norris briannorris@chromium.org wrote:
Commit b18c6c3c7768 ("ASoC: rockchip: cdn-dp sound output use spdif") switched the platform to SPDIF, but we didn't fix up the device tree.
Drop the pinctrl settings, because the 'spdif_bus' pins are either:
- unused (on kevin, bob), so the settings is ~harmless
- used by a different function (on scarlet), which causes probe failures (!!)
I suppose that means the default pinctrl should be dropped? Or maybe this use case is the outlier. Up to Heiko?
Fixes: b18c6c3c7768 ("ASoC: rockchip: cdn-dp sound output use spdif") Signed-off-by: Brian Norris briannorris@chromium.org
Reviewed-by: Chen-Yu Tsai wenst@chromium.org
Am Montag, 17. Januar 2022, 08:44:37 CET schrieb Chen-Yu Tsai:
On Sat, Jan 15, 2022 at 7:03 AM Brian Norris briannorris@chromium.org wrote:
Commit b18c6c3c7768 ("ASoC: rockchip: cdn-dp sound output use spdif") switched the platform to SPDIF, but we didn't fix up the device tree.
Drop the pinctrl settings, because the 'spdif_bus' pins are either:
- unused (on kevin, bob), so the settings is ~harmless
- used by a different function (on scarlet), which causes probe failures (!!)
I suppose that means the default pinctrl should be dropped? Or maybe this use case is the outlier. Up to Heiko?
Interesting question. Right now it looks like Gru is the only one using spdif in that way, so I'd think dropping the pinctrl here is the "saner" option at this time ;-)
I guess we can reevaluate if this becomes more widespread
Fixes: b18c6c3c7768 ("ASoC: rockchip: cdn-dp sound output use spdif") Signed-off-by: Brian Norris briannorris@chromium.org
Reviewed-by: Chen-Yu Tsai wenst@chromium.org
Some audio servers like to monitor a jack device (perhaps combined with EDID, for audio-presence info) to determine DP/HDMI audio presence.
Signed-off-by: Brian Norris briannorris@chromium.org ---
(no changes since v1)
drivers/gpu/drm/rockchip/cdn-dp-core.c | 28 ++++++++++++++++++++++++++ drivers/gpu/drm/rockchip/cdn-dp-core.h | 4 ++++ 2 files changed, 32 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c index 16497c31d9f9..edd6a1fc46cd 100644 --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c @@ -586,6 +586,13 @@ static bool cdn_dp_check_link_status(struct cdn_dp_device *dp) return drm_dp_channel_eq_ok(link_status, min(port->lanes, sink_lanes)); }
+static void cdn_dp_audio_handle_plugged_change(struct cdn_dp_device *dp, + bool plugged) +{ + if (dp->codec_dev) + dp->plugged_cb(dp->codec_dev, plugged); +} + static void cdn_dp_encoder_enable(struct drm_encoder *encoder) { struct cdn_dp_device *dp = encoder_to_dp(encoder); @@ -641,6 +648,9 @@ static void cdn_dp_encoder_enable(struct drm_encoder *encoder) DRM_DEV_ERROR(dp->dev, "Failed to valid video %d\n", ret); goto out; } + + cdn_dp_audio_handle_plugged_change(dp, true); + out: mutex_unlock(&dp->lock); } @@ -651,6 +661,8 @@ static void cdn_dp_encoder_disable(struct drm_encoder *encoder) int ret;
mutex_lock(&dp->lock); + cdn_dp_audio_handle_plugged_change(dp, false); + if (dp->active) { ret = cdn_dp_disable(dp); if (ret) { @@ -846,11 +858,27 @@ static int cdn_dp_audio_get_eld(struct device *dev, void *data, return 0; }
+static int cdn_dp_audio_hook_plugged_cb(struct device *dev, void *data, + hdmi_codec_plugged_cb fn, + struct device *codec_dev) +{ + struct cdn_dp_device *dp = dev_get_drvdata(dev); + + mutex_lock(&dp->lock); + dp->plugged_cb = fn; + dp->codec_dev = codec_dev; + cdn_dp_audio_handle_plugged_change(dp, dp->connected); + mutex_unlock(&dp->lock); + + return 0; +} + static const struct hdmi_codec_ops audio_codec_ops = { .hw_params = cdn_dp_audio_hw_params, .audio_shutdown = cdn_dp_audio_shutdown, .mute_stream = cdn_dp_audio_mute_stream, .get_eld = cdn_dp_audio_get_eld, + .hook_plugged_cb = cdn_dp_audio_hook_plugged_cb, .no_capture_mute = 1, };
diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.h b/drivers/gpu/drm/rockchip/cdn-dp-core.h index 81ac9b658a70..d808a9de45ed 100644 --- a/drivers/gpu/drm/rockchip/cdn-dp-core.h +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.h @@ -10,6 +10,7 @@ #include <drm/drm_dp_helper.h> #include <drm/drm_panel.h> #include <drm/drm_probe_helper.h> +#include <sound/hdmi-codec.h>
#include "rockchip_drm_drv.h"
@@ -101,5 +102,8 @@ struct cdn_dp_device {
u8 dpcd[DP_RECEIVER_CAP_SIZE]; bool sink_has_audio; + + hdmi_codec_plugged_cb plugged_cb; + struct device *codec_dev; }; #endif /* _CDN_DP_CORE_H */
On Sat, Jan 15, 2022 at 7:03 AM Brian Norris briannorris@chromium.org wrote:
Some audio servers like to monitor a jack device (perhaps combined with EDID, for audio-presence info) to determine DP/HDMI audio presence.
Signed-off-by: Brian Norris briannorris@chromium.org
Reviewed-by: Chen-Yu Tsai wenst@chromium.org
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; + 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),
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!
On Fri, 14 Jan 2022 15:02:06 -0800, Brian Norris wrote:
This series fixes DP/HDMI audio for RK3399 Gru systems.
First, there was a regression with the switch to SPDIF. Patch 1 can be taken separately as a regression fix if desired. But it's not quite so useful (at least on Chrome OS systems) without the second part.
Second, jack detection was never upstreamed, because the hdmi-codec dependencies were still being worked out when this platform was first supported.
[...]
Applied as fix for 5.17, thanks!
[1/3] arm64: dts: rockchip: Switch RK3399-Gru DP to SPDIF output commit: b5fbaf7d779f5f02b7f75b080e7707222573be2a
Best regards,
On Fri, 14 Jan 2022 15:02:06 -0800, Brian Norris wrote:
This series fixes DP/HDMI audio for RK3399 Gru systems.
First, there was a regression with the switch to SPDIF. Patch 1 can be taken separately as a regression fix if desired. But it's not quite so useful (at least on Chrome OS systems) without the second part.
Second, jack detection was never upstreamed, because the hdmi-codec dependencies were still being worked out when this platform was first supported.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[2/3] drm/rockchip: cdn-dp: Support HDMI codec plug-change callback commit: 9da1467b49ad6c02840e8f331c5da69f6a5bdb2e [3/3] ASoC: rk3399_gru_sound: Wire up DP jack detection commit: 6a8bc4b68ca0c6ef73518b692c00b7e1e010d056
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (5)
-
Brian Norris -
Chen-Yu Tsai -
Heiko Stuebner -
Heiko Stübner
-
Mark Brown