From: "Clément Péron" <peron.clem@gmail.com>
To: Maxime Ripard <maxime@cerno.tech>
Cc: devicetree <devicetree@vger.kernel.org>,
Linux-ALSA <alsa-devel@alsa-project.org>,
linux-kernel <linux-kernel@vger.kernel.org>,
Jernej Skrabec <jernej.skrabec@siol.net>,
Takashi Iwai <tiwai@suse.com>, Rob Herring <robh+dt@kernel.org>,
Liam Girdwood <lgirdwood@gmail.com>,
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: Mon, 4 May 2020 21:43:05 +0200 [thread overview]
Message-ID: <CAJiuCceF340FiLvyeXNZtvqftQMAmk=MtFDLT_9696ix+eH1Yw@mail.gmail.com> (raw)
In-Reply-To: <20200504120942.lnrxnnmykqnvw3fb@gilmour.lan>
Hi Maxime,
On Mon, 4 May 2020 at 14:09, Maxime Ripard <maxime@cerno.tech> wrote:
>
> Hi Clement,
>
> On Thu, Apr 30, 2020 at 04:00:14PM +0200, Clément Péron wrote:
> > On Thu, 30 Apr 2020 at 10:46, Maxime Ripard <maxime@cerno.tech> wrote:
> > > On Wed, Apr 29, 2020 at 06:33:00PM +0200, Clément Péron wrote:
> > > > On Wed, 29 Apr 2020 at 14:35, Maxime Ripard <maxime@cerno.tech> wrote:
> > > > >
> > > > > On Tue, Apr 28, 2020 at 10:55:47AM +0200, Clément Péron wrote:
> > > > > > > > +static int sun50i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
> > > > > > > > + unsigned int fmt)
> > > > > > >
> > > > > > > The alignment is off here
> > > > > > >
> > > > > > > > +{
> > > > > > > > + u32 mode, val;
> > > > > > > > + u8 offset;
> > > > > > > > +
> > > > > > > > + /*
> > > > > > > > + * DAI clock polarity
> > > > > > > > + *
> > > > > > > > + * The setup for LRCK contradicts the datasheet, but under a
> > > > > > > > + * scope it's clear that the LRCK polarity is reversed
> > > > > > > > + * compared to the expected polarity on the bus.
> > > > > > > > + */
> > > > > > >
> > > > > > > Did you check this or has it been copy-pasted?
> > > > > >
> > > > > > copy-pasted, I will check this.
> > > > >
> > > > > It's not going to be easy to do this if you only have a board with HDMI. If you
> > > > > can't test that easily, just remove the comment (or make it explicit that you
> > > > > copy pasted it?), no comment is better than a wrong one.
> > > >
> > > > I have talked with Marcus Cooper it may be able to test this this week-end.
> > > > Also this can explain why we need the "
> > > > simple-audio-card,frame-inversion;" in the device-tree.
> > > >
> > > > If think this fix has been introduced by you, correct? Could you say
> > > > on which SoC did you see this issue?
> > >
> > > This was seen on an H3
> >
> > Just two more questions:
> > - Did you observe this issue on both TDM and I2S mode?
> > - On which DAI node?
>
> This is fairly fuzzy now and I don't remember if I tested it in I2S. Let's
> assume I didn't. And similarly, I'm not sure what the exact controller was, but
> it was one of the regular controllers (so not either connected to the codec or
> the HDMI, one that goes out of the SoC).
>
> We had pretty much the same issue on the A33 in I2S for the codec though:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/sound/soc/sunxi/sun8i-codec.c?id=18c1bf35c1c09bca05cf70bc984a4764e0b0372b
>
> I didn't think of it that way then, but it might very well have been the i2s
> controller suffering the same issue.
>
> > Since recent change in sun4i-i2s.c, we had to introduce the
> > "simple-audio-card,frame-inversion" in LibreElec tree.
>
> Do you have a link to that commit ? I couldn't find anything on libreelec's
> github repo.
These commits:
https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Allwinner/devices/A64/patches/linux/04-Add-frame-inversion-to-correct-multi-channel-audio.patch
https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Allwinner/devices/H3/patches/linux/17-Add-frame-inversion-to-correct-multi-channel-audio.patch
https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Allwinner/devices/H6/patches/linux/16-Add-frame-inversion-to-correct-multi-channel-audio.patch
>
> > H3 boards are quite common in LibreElec community so I think:
> > - This fix is only needed in TDM mode
> > - Or this fix is not required for the HDMI DAI node (HDMI DAI is a
> > little bit different compare to other DAI but I think the first guess
> > is more likely)
>
> Given what we know about the A33, I'd be inclined to say the latter. I'd don't
> have the tools to check anymore, but if you have even a cheap logical analyzer,
> i2s being pretty slow you can definitely see it.
Me neither but maybe Marcus will be able to check this.
Thanks for all these informations.
>
> Maxime
next prev parent reply other threads:[~2020-05-04 19:44 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 [this message]
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
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='CAJiuCceF340FiLvyeXNZtvqftQMAmk=MtFDLT_9696ix+eH1Yw@mail.gmail.com' \
--to=peron.clem@gmail.com \
--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=maxime@cerno.tech \
--cc=robh+dt@kernel.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).