From: Maxime Ripard <maxime@cerno.tech>
To: "Clément Péron" <peron.clem@gmail.com>
Cc: devicetree <devicetree@vger.kernel.org>,
Linux-ALSA <alsa-devel@alsa-project.org>,
Samuel Holland <samuel@sholland.org>,
linux-kernel <linux-kernel@vger.kernel.org>,
Takashi Iwai <tiwai@suse.com>,
Jernej Skrabec <jernej.skrabec@siol.net>,
Liam Girdwood <lgirdwood@gmail.com>,
Rob Herring <robh+dt@kernel.org>,
Marcus Cooper <codekipper@gmail.com>,
Chen-Yu Tsai <wens@csie.org>, Mark Brown <broonie@kernel.org>,
linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v3 3/7] ASoC: sun4i-i2s: Add support for H6 I2S
Date: Thu, 17 Sep 2020 16:06:11 +0200 [thread overview]
Message-ID: <20200917140611.5qpsz24yfii5kzcn@gilmour.lan> (raw)
In-Reply-To: <CAJiuCcdWQRVMeTLvxibZ37CF9BMiC_L2bWBDiin2Uz0CWq2FuQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4634 bytes --]
Hi Clement,
On Thu, Sep 17, 2020 at 03:55:45PM +0200, Clément Péron wrote:
> Hi Maxime and Samuel,
>
> On Thu, 17 Sep 2020 at 15:21, Maxime Ripard <maxime@cerno.tech> wrote:
> >
> > Hi,
> >
> > On Sat, Sep 12, 2020 at 03:29:55PM -0500, Samuel Holland wrote:
> > > On 9/10/20 9:33 AM, Maxime Ripard wrote:
> > > > On Thu, Sep 03, 2020 at 09:54:39PM -0500, Samuel Holland wrote:
> > > >> On 9/3/20 3:58 PM, Maxime Ripard wrote:
> > > >>> On Thu, Sep 03, 2020 at 10:02:31PM +0200, Clément Péron wrote:
> > > >>>> Hi Maxime,
> > > >>>>
> > > >>>> On Wed, 29 Jul 2020 at 17:16, Mark Brown <broonie@kernel.org> wrote:
> > > >>>>>
> > > >>>>> On Wed, Jul 29, 2020 at 04:39:27PM +0200, Maxime Ripard wrote:
> > > >>>>>
> > > >>>>>> It really looks like the polarity of LRCK is fine though. The first word
> > > >>>>>> is sent with LRCK low, and then high, so we have channel 0 and then
> > > >>>>>> channel 1 which seems to be the proper ordering?
> > > >>
> > > >> Which image file is this in reference to?
> > > >>
> > > >>>>> Yes, that's normal.
> > > >>>>
> > > >>>> Thank you very much for this test.
> > > >>>>
> > > >>>> So I will revert the following commit:
> > > >>>>
> > > >>>> ASoC: sun4i-i2s: Fix the LRCK polarity
> > > >>>>
> > > >>>> https://github.com/clementperon/linux/commit/dd657eae8164f7e4bafe8b875031a7c6c50646a9
> > > >>>
> > > >>> Like I said, the current code is working as expected with regard to the
> > > >>> LRCK polarity. The issue is that the samples are delayed and start to be
> > > >>> transmitted on the wrong phase of the signal.
> > > >>
> > > >> Since an I2S LRCK frame is radially symmetric, "wrong phase" and "inverted
> > > >> polarity" look the same. The only way to definitively distinguish them is by
> > > >> looking at the sample data.
> > > >>
> > > >> In "i2s-h6.png", the samples are all zeroes, so you're assuming that the first
> > > >> sample transmitted (that is, when the bit clock starts transitioning) was a
> > > >> "left" sample.
> > > >>
> > > >> However, in "h6-i2s-start-data.png", there are pairs of samples we can look at.
> > > >> I'm still assuming that similar samples are a left/right pair, but that's
> > > >> probably a safe assumption. Here we see the first sample in each pair is
> > > >> transmitted with LRCK *high*, and the second sample in the pair is transmitted
> > > >> with LRCK *low*. This is the opposite of your claim above.
> > > >>
> > > >> An ideal test would put left/right markers and frame numbers in the data
> > > >> channel. The Python script below can generate such a file. Then you would know
> > > >> how much startup delay there is, which channel the "first sample" came from, and
> > > >> how each channel maps to the LRCK level.
> > > >>
> > > >> It would also be helpful to test DSP_A mode, where the LRCK signal is
> > > >> asymmetric and an inversion would be obvious.
> > > >
> > > > I had no idea that there was a wave module in Python, that's a great
> > > > suggestion, thanks!
> > > >
> > > > You'll find attached the screenshots for both the I2S and DSP_A formats.
> > > > I zoomed out a bit to be able to have the first valid samples, but it
> > > > should be readable.
> > > >
> > > > The code I used is there:
> > > > https://github.com/mripard/linux/tree/sunxi/h6-i2s-test
> > > >
> > > > It's basically the v3, plus the DT bits.
> > > >
> > > > As you can see, in the i2s case, LRCK starts low and then goes up, with
> > > > the first channel (0x2*** samples) transmitted first, so everything
> > > > looks right here.
> > > >
> > > > On the DSP_A screenshot, LRCK will be low with small bursts high, and
> > > > once again with the first channel being transmitted first, so it looks
> > > > right to me too.
> > >
> > > Indeed, for H6 i2s0 with LRCK inversion in software, everything looks correct on
> > > the wire.
> > >
> > > It's still concerning to me that the BSP has no evidence of this inversion,
> > > either for i2s0 or i2s1[1]. And the inversion seems not to be required for HDMI
> > > audio on mainline either (but there could be an inversion on the HDMI side or on
> > > the interconnect).
> >
> > One can only guess here, but it's also quite easy to fix it at the card
> > level (or maybe there's a similar inversion in the codecs, or whatever).
>
> Thanks for the test and the explanation.
>
> Quite disturbing that there is no evidence of the LRCK inversion in
> kernel vendor indeed...
> Could it be an issue with the mainline code?
I'm not sure what you mean here, this was tested with mainline?
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
next prev parent reply other threads:[~2020-09-17 14:07 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-26 10:41 [PATCH v3 0/7] Add H6 I2S support Clément Péron
2020-04-26 10:41 ` [PATCH v3 1/7] ASoC: sun4i-i2s: Adjust LRCLK width Clément Péron
2020-04-28 8:02 ` Maxime Ripard
2020-04-26 10:41 ` [PATCH v3 2/7] dt-bindings: ASoC: sun4i-i2s: Add H6 compatible Clément Péron
2020-04-28 8:08 ` Maxime Ripard
2020-05-11 22:26 ` Rob Herring
2020-04-26 10:41 ` [PATCH v3 3/7] ASoC: sun4i-i2s: Add support for H6 I2S Clément Péron
2020-04-28 8:13 ` Maxime Ripard
2020-04-28 8:55 ` Clément Péron
2020-04-29 12:35 ` Maxime Ripard
2020-04-29 16:33 ` Clément Péron
2020-04-30 8:46 ` Maxime Ripard
2020-04-30 14:00 ` Clément Péron
2020-05-04 12:09 ` Maxime Ripard
2020-05-04 19:43 ` Clément Péron
2020-07-29 14:39 ` Maxime Ripard
2020-07-29 15:15 ` Mark Brown
2020-09-03 20:02 ` Clément Péron
2020-09-03 20:58 ` Maxime Ripard
2020-09-04 2:54 ` Samuel Holland
[not found] ` <20200910143314.qku7po6htiiq5lzf@gilmour.lan>
2020-09-12 20:29 ` Samuel Holland
2020-09-17 13:21 ` Maxime Ripard
2020-09-17 13:55 ` Clément Péron
2020-09-17 14:06 ` Maxime Ripard [this message]
2020-09-20 12:38 ` Clément Péron
2020-04-26 10:41 ` [PATCH v3 4/7] ASoC: sun4i-i2s: Set sign extend sample Clément Péron
2020-04-26 10:41 ` [PATCH v3 5/7] ASoc: sun4i-i2s: Add 20 and 24 bit support Clément Péron
2020-04-26 10:41 ` [PATCH v3 6/7] ASoC: sun4i-i2s: Adjust regmap settings Clément Péron
2020-04-27 11:03 ` Chen-Yu Tsai
2020-05-03 11:42 ` Clément Péron
2020-04-26 10:41 ` [PATCH v3 7/7] arm64: dts: sun50i-h6: Add HDMI audio to H6 DTSI Clément Péron
2020-04-26 11:51 ` Clément Péron
2020-04-28 8:14 ` Maxime Ripard
2020-04-28 14:36 ` Clément Péron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200917140611.5qpsz24yfii5kzcn@gilmour.lan \
--to=maxime@cerno.tech \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=codekipper@gmail.com \
--cc=devicetree@vger.kernel.org \
--cc=jernej.skrabec@siol.net \
--cc=lgirdwood@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=peron.clem@gmail.com \
--cc=robh+dt@kernel.org \
--cc=samuel@sholland.org \
--cc=tiwai@suse.com \
--cc=wens@csie.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).