Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
References: <20211002234313.3209294-1-martin.blumenstingl@googlemail.com>
User-agent: mu4e 1.6.6; emacs 27.1
From:   Jerome Brunet <jbrunet@baylibre.com>
To:     Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
        alsa-devel@alsa-project.org, linux-amlogic@lists.infradead.org
Cc:     lgirdwood@gmail.com, broonie@kernel.org, perex@perex.cz,
        tiwai@suse.com, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH v1 0/1] ASoC: meson: aiu: HDMI codec control
 questions and issues
Date:   Sun, 03 Oct 2021 17:57:34 +0200
In-reply-to: <20211002234313.3209294-1-martin.blumenstingl@googlemail.com>
Message-ID: <1j35pivzho.fsf@starbuckisacylon.baylibre.com>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk


On Sun 03 Oct 2021 at 01:43, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> Hello Jerome et al.,
>
> on Amlogic Meson SoCs which use the AIU audio controller we have the
> so-called "machine gun noise" (MGN) issue, reproducible for example in
> Kodi. So far nobody was able to identify the cause of this issue. My
> idea was to at least narrow down the issue so we can categorize it. For
> that I wanted to get the SPDIF output from AIU to the HDMI controller
> working.

Ok, I'll clarify (again) about this issue.

This issue is on AIU only (not the AXG series), on the i2s encoder
regardless of the output (HDMI, internal and external codec)

This encoder support either 2ch or 8ch mode, nothing else.
As long as only 2ch was supported, there was no problem.
In order to switch between 2ch and 8ch mode.  AIU_RST_SOFT_I2S_FAST and
AIU_I2S_SYNC shall be "poked" (check iu-fifo-i2s.c and
aiu-encoder-i2c.c). Without this, the switch is not done.

What these do, how and when it should be done is unclear. I tried
several combination. At the time I found something stable-ish but it was
not perfect.

At the time, It was completely new driver. Even if was not rock solid,
it was still progress and I opted to upstream it with an imperfect 8ch
support so people could help debug it. This was mentioned in the
original submission.

The other solution is to restrict to 2ch mode and remove
AIU_RST_SOFT_I2S_FAST and AIU_I2S_SYNC pokes. There will be no noise
anymore.

>
> On Amlogic Meson GXBB/GXL/GXM SoCs a DesignWare HDMI TX controller is
> used. This has an SPDIF input but there's currently not driver for it.
> On Meson8/8b/8m2 SoCs I am working on a HDMI driver for the TransSwitch
> HDMI controller which - just like DesignWare HDMI TX - supports SPDIF
> and I2S inputs.
> I decided to add SPDIF support to the latter since the code from the
> vendor driver is much easier.
>
> It took me a while to figure out why I would not get any audio output
> from AIU SPDIF to the HDMI controller - or from there to the sink.
> The "fix" for this issue is the RFC patch which is part of this series.
> Any feedback would be great as I am still new to the ASoC subsystem.
>
> Another part I am still struggling with is the audio "routing" (due to
> lack of a better term - please correct me if this is not the right word
> to use for this case). I have the following description in my .dts:
> 	sound {
> 		compatible = "amlogic,gx-sound-card";
> 		model = "M8B-ODROID-C1";
>
> 		assigned-clocks = <&clkc CLKID_MPLL0>,
> 				  <&clkc CLKID_MPLL1>;
> 		assigned-clock-rates = <294912000>,
> 				       <270950400>;
> 		dai-link-0 {
> 			sound-dai = <&aiu AIU_CPU CPU_I2S_FIFO>;
> 		};
>
> 		dai-link-1 {
> 			sound-dai = <&aiu AIU_CPU CPU_SPDIF_FIFO>;
> 		};
>
> 		dai-link-2 {
> 			sound-dai = <&aiu AIU_CPU CPU_I2S_ENCODER>;
> 			dai-format = "i2s";
> 			mclk-fs = <256>;
>
> 			codec-0 {
> 				sound-dai = <&aiu AIU_HDMI CTRL_I2S>;
> 			};
> 		};
>
> 		dai-link-3 {
> 			sound-dai = <&aiu AIU_CPU CPU_SPDIF_ENCODER>;
>
> 			codec-0 {
> 				sound-dai = <&aiu AIU_HDMI CTRL_PCM>;

Ok, the obove sums up the problem around this RFC.

Here you describe a DAI link (think of it as wires) between the SPDIF
encoder (output) and AIU_HDMI input PCM. This is not what the HW is and
it is not possible.

Let's start from the HDMI controller.
The designware (on amlogic SoC) has 2 interface for audio input.
1) PCM/I2S: a classic interface 2 clocks and N data line
2) SPDIF: The Sony Philips 1 wire interface

Whatever comes up on 1) has to be some sort of i2s signal. So SPDIF
won't fly there.

AIU_HDMI output is Hardwired to 1). It is just just a digital mux,
implemented as an ASoC codec which allows to seleted one of 2 audio
sources:
A) the i2s output implemented as part of the AIU
B) the PCM output, part the AUDIN (yes, an output in AUDIN) block. This
is not implemented ATM.

So trying to handle SPDIF through AIU_HDMI is wrong.

> 			};
> 		};
>
> 		dai-link-4 {
> 			sound-dai = <&aiu AIU_HDMI CTRL_OUT>;
>
> 			codec-0 {
> 				sound-dai = <&hdmi_tx>;

your HDMI controller should have an argument to the phandle if it
supported both I2S and SPDIF inputs

> 			};
> 		};
> 	};
> The driver for &hdmi_tx defines:
> 	struct hdmi_codec_pdata pdata = {
> 		.ops			= &txc_48352_hdmi_codec_ops,
> 		.i2s			= 1,
> 		.spdif			= 1,
> 		.max_i2s_channels	= 8,
> 		.data			= priv,
> 	};
> In hdmi_codec_ops.hw_params I always get fmt->fmt HDMI_I2S unless I
> remove all I2S references from the .dts snipped above (only then
> HDMI_SPDIF is detected). Based on the selection of the "HDMI Source"
> enum in aiu-codec-ctrl I was expecting the format to update as well.
> That unfortunately doesn't happen and I don't know how that can be
> achieved.

It is not meant to. The dai_link and the endpoint are i2s.
Your HDMI controller should have 2 inputs and should have a way to
select one or the other. The format at each of the (internal) input does
not change

>
>
> Best regards,
> Martin
>
>
> Martin Blumenstingl (1):
>   ASoC: meson: aiu: Fix HDMI codec control selection
>
>  sound/soc/meson/aiu-codec-ctrl.c  | 108 ++++++++++++++++++++++--------
>  sound/soc/meson/aiu-encoder-i2s.c |   6 --
>  2 files changed, 80 insertions(+), 34 deletions(-)