Re: [PATCH v3 3/7] ASoC: sun4i-i2s: Add support for H6 I2S
Maxime,
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/dd657eae8164f7e4bafe8b875031a7c...
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).
Even so, your research is sufficient justification for me that the code is correct as-is (with the inversion). Thank you very much for collecting the data!
Cheers, Samuel
[1]: https://github.com/Allwinner-Homlet/H6-BSP4.9-linux/blob/e634a960316dddd1eb5... where 1 == SND_SOC_DAIFMT_NB_NF, and there's no inversion in sunxi_daudio_init_fmt().
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/dd657eae8164f7e4bafe8b875031a7c...
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).
Even so, your research is sufficient justification for me that the code is correct as-is (with the inversion). Thank you very much for collecting the data!
You're welcome, thanks for that script :)
maxime
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/dd657eae8164f7e4bafe8b875031a7c...
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?
But still regarding the kernel vendor, it seems logical to have a frame-inversion in the device-tree for the HDMI I2S node. I will drop the revert patch and re-add the frame inversion in the next series.
Regards, Clement
Even so, your research is sufficient justification for me that the code is correct as-is (with the inversion). Thank you very much for collecting the data!
You're welcome, thanks for that script :)
maxime
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/dd657eae8164f7e4bafe8b875031a7c...
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
Hi Maxime,
On Thu, 17 Sep 2020 at 16:06, Maxime Ripard maxime@cerno.tech wrote:
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/dd657eae8164f7e4bafe8b875031a7c... > > 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?
Sorry i was not clear, I meant either there is an issue in the vendor kernel that doesn't set properly the LRCK or maybe we did something or forgot to do it that set this inversion.
But I just checked a device-tree used with a kernel vendor and indeed codecs are inverted but not hdmi so the vendor kernel has an issue here...
E.g this is what is used for Tanix TX6
daudio@0x05091000 { compatible = "allwinner,sunxi-tdmhdmi"; reg = <0x00 0x5091000 0x00 0x74>; clocks = <0x04 0x4d>; status = "okay"; phandle = <0x63>; device_type = "audiohdmi"; };
daudio@0x05092000 { compatible = "allwinner,sunxi-daudio"; reg = <0x00 0x5092000 0x00 0x74>; clocks = <0x04 0x4e>; pinctrl-names = "default\0sleep"; pinctrl-0 = <0x4f>; pinctrl-1 = <0x50>; pcm_lrck_period = <0x40>; slot_width_select = <0x20>; daudio_master = <0x04>; audio_format = <0x04>; signal_inversion = <0x03>; tdm_config = <0x01>; frametype = <0x00>; tdm_num = <0x02>; mclk_div = <0x01>; status = "okay"; phandle = <0x65>; device_type = "daudio2"; };
daudio@0x0508f000 { compatible = "allwinner,sunxi-daudio"; reg = <0x00 0x508f000 0x00 0x74>; clocks = <0x04 0x51>; pinctrl-names = "default\0sleep"; pinctrl-0 = <0x52>; pinctrl-1 = <0x53>; pcm_lrck_period = <0x20>; slot_width_select = <0x20>; daudio_master = <0x04>; audio_format = <0x01>; signal_inversion = <0x01>; tdm_config = <0x01>; frametype = <0x00>; tdm_num = <0x03>; mclk_div = <0x01>; status = "okay"; phandle = <0x67>; device_type = "daudio3"; };
Clement
Maxime
participants (3)
-
Clément Péron -
Maxime Ripard -
Samuel Holland