PineTab2 UCM2 profile

[ScottFreeCode]

Do not use: The device tree now works with the upstream Rockchip rk817 profile and is configured to pick it up, so this shouldn't be necessary and probably won't be picked up even if you try it.

---

This solves the problem of the speakers not turning off when the headphone jack is plugged in.

There are some oddities found and worked around, noted in the code.

Code is based on the UCM conf's rk817 profile, with modifications to work around a couple PineTab2 issues. The main outstanding issues are documented in comments next to the workarounds but I also did hardcode the playback mux to HP removing the SPK option as SPK seemed to disable sound in practice.

Packaging is based on alsa-ucm-pinetab.

[diederikdehaas]

Wouldn't it be better if this was submitted directly to the upstream repo? https://github.com/alsa-project/alsa-ucm-conf/

See here for the PinePhone's submission: https://github.com/alsa-project/alsa-ucm-conf/pull/134

[ScottFreeCode]

Wouldn't it be better if this was submitted directly to the upstream repo? https://github.com/alsa-project/alsa-ucm-conf/

See here for the PinePhone's submission: https://github.com/alsa-project/alsa-ucm-conf/pull/134

In the long run, yes, upstreaming any needed new profile would be ideal. But the PineTab (1) is still in this repo and not that one, too. And we're still working out the bugs (see e.g. the conversation about the backwards plugged/unplugged state). And after we work out all the bugs it might turn out we don't need this since there's an existing Rockchip profile upstream.

[diederikdehaas]

Ok. Next to my desire to get everything upstreamed, I also noticed a discussion about which directory structure to use and IIUC the current proposal is different from the (accepted) PinePhone submission.
Hence my thinking was: (let's) discuss that directly with upstream.

In the meantime, I'll (also) add this patch to my Debian package to test it out too.

[ScottFreeCode]

@Danct12 @TuxThePenguin0 I have built and tested a fix to the device tree to get it to work with the upstream rk817-sound profile, and it works perfectly! (I removed my PineTab2 profile before testing it, so I know it is using the upstream one.)

This is the patch if you HAVE NOT applied @Danct12's patch upthread for the gpio high/low:

diff --git a/arch/arm64/boot/dts/rockchip/rk3566-pinetab2.dtsi b/arch/arm64/boot/dts/rockchip/rk3566-pinetab2.dtsi
index a766f21bd..60d28ba24 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-pinetab2.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3566-pinetab2.dtsi
@@ -116,14 +116,14 @@ rk817-sound {
        pinctrl-names = "default";
        pinctrl-0 = <&hp_det_l>;
        simple-audio-card,format = "i2s";
-       simple-audio-card,name = "PineTab2";
+       simple-audio-card,name = "rk817_int";
        simple-audio-card,mclk-fs = <256>;

        simple-audio-card,widgets =
            "Microphone", "Mic Jack",
            "Headphone", "Headphones",
            "Microphone", "Microphone",
-           "Speaker", "Speakers";
+           "Speaker", "Internal Speakers";

        simple-audio-card,routing =
            "MICL", "Microphone",
@@ -132,12 +132,12 @@ rk817-sound {
            "Headphones", "HPOR",
            "Speaker Amplifier INL", "HPOL",
            "Speaker Amplifier INR", "HPOR",
-           "Speakers", "Speaker Amplifier OUTL",
-           "Speakers", "Speaker Amplifier OUTR";
+           "Internal Speakers", "Speaker Amplifier OUTL",
+           "Internal Speakers", "Speaker Amplifier OUTR";

-       simple-audio-card,hp-det-gpio = <&gpio4 RK_PC6 GPIO_ACTIVE_HIGH>;
+       simple-audio-card,hp-det-gpio = <&gpio4 RK_PC6 GPIO_ACTIVE_LOW>;
        simple-audio-card,aux-devs = <&speaker_amp>;
-       simple-audio-card,pin-switches = "Speakers", "Microphone";
+       simple-audio-card,pin-switches = "Internal Speakers", "Microphone";

        simple-audio-card,cpu {
            sound-dai = <&i2s1_8ch>;

This is the patch if you HAVE applied @Danct12's patch upthread for the gpio high/low:

diff --git a/arch/arm64/boot/dts/rockchip/rk3566-pinetab2.dtsi b/arch/arm64/boot/dts/rockchip/rk3566-pinetab2.dtsi
index 1ba6e6107..60d28ba24 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-pinetab2.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3566-pinetab2.dtsi
@@ -116,14 +116,14 @@ rk817-sound {
        pinctrl-names = "default";
        pinctrl-0 = <&hp_det_l>;
        simple-audio-card,format = "i2s";
-       simple-audio-card,name = "PineTab2";
+       simple-audio-card,name = "rk817_int";
        simple-audio-card,mclk-fs = <256>;

        simple-audio-card,widgets =
            "Microphone", "Mic Jack",
            "Headphone", "Headphones",
            "Microphone", "Microphone",
-           "Speaker", "Speakers";
+           "Speaker", "Internal Speakers";

        simple-audio-card,routing =
            "MICL", "Microphone",
@@ -132,12 +132,12 @@ rk817-sound {
            "Headphones", "HPOR",
            "Speaker Amplifier INL", "HPOL",
            "Speaker Amplifier INR", "HPOR",
-           "Speakers", "Speaker Amplifier OUTL",
-           "Speakers", "Speaker Amplifier OUTR";
+           "Internal Speakers", "Speaker Amplifier OUTL",
+           "Internal Speakers", "Speaker Amplifier OUTR";

        simple-audio-card,hp-det-gpio = <&gpio4 RK_PC6 GPIO_ACTIVE_LOW>;
        simple-audio-card,aux-devs = <&speaker_amp>;
-       simple-audio-card,pin-switches = "Speakers", "Microphone";
+       simple-audio-card,pin-switches = "Internal Speakers", "Microphone";

        simple-audio-card,cpu {
            sound-dai = <&i2s1_8ch>;

If there isn't any reason that the device tree needs to name that "Speakers" (as it is now) instead of "Internal Speakers" (as I'm proposing changing it to), or that we can't/shouldn't reuse the existing rk817_(int|ext) profile/link, then I would highly recommend closing (i.e. not merging and not using) this PR and skipping this package in favor of applying the device tree patch(es) to use the upstream rk817-sound profile!

[diederikdehaas]

I can confirm that just the DeviceTree changes suggested, makes audio work properly.

[ScottFreeCode]

Never before have I been so happy to have a pull request of mine not be merged! Looking forward to this making it out of the -testing package repo.

[diederikdehaas]

Any particular reason you went with rk817_ext and not rk817_int?

I have no idea what the reason is/was for both existing (upstream) and one seems to be a symlink to the other, so AFAIK it doesn't make a difference. But I'm still curious why you went with _ext and not _int as proposed in this MR.

[Danct12]

The PineTab 2 has external amplifier setup, so I went with rk817_ext because that's what they do too: https://github.com/alsa-project/alsa-ucm-conf/commit/b0497ca4b508d6f894d78b16e0e06616a2a36c16

[ScottFreeCode]

Ah, good find. I had assumed "int" because he had to specify "Internal Speakers" to get it to work.