| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1jldvc5uwh.fsf@starbuckisacylon.baylibre.com>
Date: Wed, 15 Jan 2025 09:43:58 +0100
From: Jerome Brunet <jbrunet@...libre.com>
To: Jiebing Chen <jiebing.chen@...ogic.com>
Cc: jiebing chen via B4 Relay <devnull+jiebing.chen.amlogic.com@...nel.org>,
Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Jaroslav Kysela <perex@...ex.cz>,
Takashi Iwai <tiwai@...e.com>, Neil Armstrong
<neil.armstrong@...aro.org>, Kevin Hilman <khilman@...libre.com>, Martin
Blumenstingl <martin.blumenstingl@...glemail.com>,
linux-sound@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-amlogic@...ts.infradead.org
Subject: Re: [PATCH 3/3] arm64: dts: amlogic: Add Amlogic S4 Audio
On Wed 15 Jan 2025 at 14:16, Jiebing Chen <jiebing.chen@...ogic.com> wrote:
> 在 2025/1/15 11:38, Jiebing Chen 写道:
>>
>> 在 2025/1/14 22:15, Jerome Brunet 写道:
>>> [ EXTERNAL EMAIL ]
>>>
>>> On Tue 14 Jan 2025 at 20:34, Jiebing Chen <jiebing.chen@...ogic.com>
>>> wrote:
>>>
>>>> 在 2025/1/14 19:16, Jerome Brunet 写道:
>>>>> [ EXTERNAL EMAIL ]
>>>>>
>>>>> On Tue 14 Jan 2025 at 16:52, Jiebing Chen <jiebing.chen@...ogic.com>
>>>>> wrote:
>>>>>
>>>>>> 在 2025/1/13 22:50, Jerome Brunet 写道:
>>>>>>> [ EXTERNAL EMAIL ]
>>>>>>>
>>>>>>> On Mon 13 Jan 2025 at 14:35, jiebing chen via B4 Relay
>>>>>>> <devnull+jiebing.chen.amlogic.com@...nel.org> wrote:
>>>>>>>
>>>>>>>> From: jiebing chen <jiebing.chen@...ogic.com>
>>>>>>>>
>>>>>>>> Add basic audio driver support for the Amlogic S4 based Amlogic
>>>>>>>> AQ222 board.
>>>>>>>>
>>>>>>>> Signed-off-by: jiebing chen <jiebing.chen@...ogic.com>
>>>>>>>> ---
>>>>>>>> .../boot/dts/amlogic/meson-s4-s805x2-aq222.dts | 226
>>>>>>>> ++++++++++++
>>>>>>>> arch/arm64/boot/dts/amlogic/meson-s4.dtsi | 385
>>>>>>>> ++++++++++++++++++++-
>>>>>>>> 2 files changed, 610 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm64/boot/dts/amlogic/meson-s4-s805x2-aq222.dts
>>>> b/arch/arm64/boot/dts/amlogic/meson-s4-s805x2-aq222.dts
>>>>>>>> index
>>>>>>>> 6730c44642d2910d42ec0c4adf49fefc3514dbec..32f50a5b860435d50d9c5528b43422b705b20130
>>>>>>>> 100644
>>>>>>>> --- a/arch/arm64/boot/dts/amlogic/meson-s4-s805x2-aq222.dts
>>>>>>>> +++ b/arch/arm64/boot/dts/amlogic/meson-s4-s805x2-aq222.dts
>>>>>>>> @@ -75,6 +75,19 @@ vddio_ao1v8: regulator-vddio-ao1v8 {
>>>>>>>> regulator-always-on;
>>>>>>>> };
>>>>>>>>
>>>>>>>> + vcc5v_reg: regulator-vcc-5v {
>>>>>>>> + compatible = "regulator-fixed";
>>>>>>>> + vin-supply = <&main_12v>;
>>>>>>>> + regulator-name = "VCC5V";
>>>>>>>> + regulator-min-microvolt = <5000000>;
>>>>>>>> + regulator-max-microvolt = <5000000>;
>>>>>>>> + gpio = <&gpio GPIOH_7 GPIO_ACTIVE_HIGH>;
>>>>>>>> + startup-delay-us = <7000>;
>>>>>>>> + enable-active-high;
>>>>>>>> + regulator-boot-on;
>>>>>>>> + regulator-always-on;
>>>>>>>> + };
>>>>>>>> +
>>>>>>>> /* SY8120B1ABC DC/DC Regulator. */
>>>>>>>> vddcpu: regulator-vddcpu {
>>>>>>>> compatible = "pwm-regulator";
>>>>>>>> @@ -129,6 +142,219 @@ vddcpu: regulator-vddcpu {
>>>>>>>> <699000 98>,
>>>>>>>> <689000 100>;
>>>>>>>> };
>>>>>>>> + dmics: audio-codec-1 {
>>>>>>>> + compatible = "dmic-codec";
>>>>>>>> + #sound-dai-cells = <0>;
>>>>>>>> + num-channels = <2>;
>>>>>>>> + wakeup-delay-ms = <50>;
>>>>>>>> + sound-name-prefix = "MIC";
>>>>>>>> + };
>>>>>>>> +
>>>>>>>> + dioo2133: audio-amplifier-0 {
>>>>>>>> + compatible = "simple-audio-amplifier";
>>>>>>>> + enable-gpios = <&gpio GPIOH_8 GPIO_ACTIVE_HIGH>;
>>>>>>>> + VCC-supply = <&vcc5v_reg>;
>>>>>>>> + #sound-dai-cells = <0>;
>>>>>>>> + sound-name-prefix = "10U2";
>>>>>>>> + };
>>>>>>>> +
>>>>>>>> + spdif_dir: audio-spdif-in {
>>>>>>>> + compatible = "linux,spdif-dir";
>>>>>>>> + #sound-dai-cells = <0>;
>>>>>>>> + sound-name-prefix = "DIR";
>>>>>>>> + };
>>>>>>>> +
>>>>>>>> + spdif_dit: audio-spdif-out {
>>>>>>>> + compatible = "linux,spdif-dit";
>>>>>>>> + #sound-dai-cells = <0>;
>>>>>>>> + sound-name-prefix = "DIT";
>>>>>>>> + };
>>>>>>>> +
>>>>>>>> + sound {
>>>>>>>> + compatible = "amlogic,axg-sound-card";
>>>>>>>> + model = "aq222";
>>>>>>>> + audio-widgets = "Line", "Lineout";
>>>>>>>> + audio-aux-devs = <&tdmout_a>, <&tdmout_b>,
>>>>>>>> <&tdmout_c>,
>>>>>>>> + <&tdmin_a>, <&tdmin_b>, <&tdmin_c>,
>>>>>>>> + <&tdmin_lb>, <&dioo2133>,
>>>>>>>> <&tdmout_pad>, <&toacodec>;
>>>>>>>> + audio-routing = "TDMOUT_A IN 0", "FRDDR_A OUT 0",
>>>>>>>> + "TDMOUT_A IN 1", "FRDDR_B OUT 0",
>>>>>>>> + "TDMOUT_A IN 2", "FRDDR_C OUT 0",
>>>>>>>> + "TDM_A Playback", "TDMOUT_A OUT",
>>>>>>>> + "TDMA_OUT SEL", "TDM_A Playback",
>>>>>>>> + "TDMOUT_B IN 0", "FRDDR_A OUT 1",
>>>>>>>> + "TDMOUT_B IN 1", "FRDDR_B OUT 1",
>>>>>>>> + "TDMOUT_B IN 2", "FRDDR_C OUT 1",
>>>>>>>> + "TDM_B Playback", "TDMOUT_B OUT",
>>>>>>>> + "TDMB_OUT SEL", "TDM_B Playback",
>>>>>>>> + "TDMOUT_C IN 0", "FRDDR_A OUT 2",
>>>>>>>> + "TDMOUT_C IN 1", "FRDDR_B OUT 2",
>>>>>>>> + "TDMOUT_C IN 2", "FRDDR_C OUT 2",
>>>>>>>> + "TDM_C Playback", "TDMOUT_C OUT",
>>>>>>>> + "TDMC_OUT SEL", "TDM_C Playback",
>>>>>>>> + "TOACODEC TDMA", "TDM_A Playback",
>>>>>>>> + "TOACODEC TDMB", "TDM_B Playback",
>>>>>>>> + "TOACODEC TDMC", "TDM_C Playback",
>>>>>>>> + "SPDIFOUT_A IN 0", "FRDDR_A OUT 3",
>>>>>>>> + "SPDIFOUT_A IN 1", "FRDDR_B OUT 3",
>>>>>>>> + "SPDIFOUT_A IN 2", "FRDDR_C OUT 3",
>>>>>>>> + "SPDIFOUT_B IN 0", "FRDDR_A OUT 4",
>>>>>>>> + "SPDIFOUT_B IN 1", "FRDDR_B OUT 4",
>>>>>>>> + "SPDIFOUT_B IN 2", "FRDDR_C OUT 4",
>>>>>>>> + "TDMIN_A IN 0", "TDM_A Capture",
>>>>>>>> + "TDMIN_A IN 1", "TDM_B Capture",
>>>>>>>> + "TDMIN_A IN 2", "TDM_C Capture",
>>>>>>>> + "TDMIN_A IN 3", "TDM_A Loopback",
>>>>>>>> + "TDMIN_A IN 4", "TDM_B Loopback",
>>>>>>>> + "TDMIN_A IN 5", "TDM_C Loopback",
>>>>>>>> + "TDMIN_B IN 0", "TDM_A Capture",
>>>>>>>> + "TDMIN_B IN 1", "TDM_B Capture",
>>>>>>>> + "TDMIN_B IN 2", "TDM_C Capture",
>>>>>>>> + "TDMIN_B IN 3", "TDM_A Loopback",
>>>>>>>> + "TDMIN_B IN 4", "TDM_B Loopback",
>>>>>>>> + "TDMIN_B IN 5", "TDM_C Loopback",
>>>>>>>> + "TDMIN_C IN 0", "TDM_A Capture",
>>>>>>>> + "TDMIN_C IN 1", "TDM_B Capture",
>>>>>>>> + "TDMIN_C IN 2", "TDM_C Capture",
>>>>>>>> + "TDMIN_C IN 3", "TDM_A Loopback",
>>>>>>>> + "TDMIN_C IN 4", "TDM_B Loopback",
>>>>>>>> + "TDMIN_C IN 5", "TDM_C Loopback",
>>>>>>>> + "TDMIN_LB IN 3", "TDM_A Capture",
>>>>>>>> + "TDMIN_LB IN 4", "TDM_B Capture",
>>>>>>>> + "TDMIN_LB IN 5", "TDM_C Capture",
>>>>>>>> + "TDMIN_LB IN 0", "TDM_A Loopback",
>>>>>>>> + "TDMIN_LB IN 1", "TDM_B Loopback",
>>>>>>>> + "TDMIN_LB IN 2", "TDM_C Loopback",
>>>>>>>> + "TODDR_A IN 0", "TDMIN_A OUT",
>>>>>>>> + "TODDR_B IN 0", "TDMIN_A OUT",
>>>>>>>> + "TODDR_C IN 0", "TDMIN_A OUT",
>>>>>>>> + "TODDR_A IN 1", "TDMIN_B OUT",
>>>>>>>> + "TODDR_B IN 1", "TDMIN_B OUT",
>>>>>>>> + "TODDR_C IN 1", "TDMIN_B OUT",
>>>>>>>> + "TODDR_A IN 2", "TDMIN_C OUT",
>>>>>>>> + "TODDR_B IN 2", "TDMIN_C OUT",
>>>>>>>> + "TODDR_C IN 2", "TDMIN_C OUT",
>>>>>>>> + "TODDR_A IN 3", "SPDIFIN Capture",
>>>>>>>> + "TODDR_B IN 3", "SPDIFIN Capture",
>>>>>>>> + "TODDR_C IN 3", "SPDIFIN Capture",
>>>>>>>> + "TODDR_A IN 6", "TDMIN_LB OUT",
>>>>>>>> + "TODDR_B IN 6", "TDMIN_LB OUT",
>>>>>>>> + "TODDR_C IN 6", "TDMIN_LB OUT",
>>>>>>>> + "10U2 INL", "ACODEC LOLP",
>>>>>>>> + "10U2 INR", "ACODEC LORP",
>>>>>>>> + "Lineout", "10U2 OUTL",
>>>>>>>> + "Lineout", "10U2 OUTR";
>>>>>>>> + assigned-clocks = <&clkc_pll CLKID_HIFI_PLL>,
>>>>>>>> + <&clkc_pll CLKID_MPLL2>,
>>>>>>>> + <&clkc_pll CLKID_MPLL0>,
>>>>>>>> + <&clkc_pll CLKID_MPLL1>;
>>>>>>>> + assigned-clock-rates = <491520000>,
>>>>>>>> + <294912000>,
>>>>>>>> + <270950400>,
>>>>>>>> + <393216000>;
>>>>>>> Why do you need 4 base rates ? Which rate family does each provide ?
>>>>>> hifipll 49152000, mpll2 294912000 mpll0 270950400, mpll1 393216000,
>>>>>> the
>>>>>> accuracy of hifipll
>>>>>>
>>>>>> is relatively high, for tdm/pdm/spdif 16/48/96/192k we can use it. if
>>>>>> the
>>>>>> tdm and spdif work on
>>>>> It is fine to use the HiFi. I'm glad this clock finally got fixed
>>>>>
>>>>>> the same time, for example ,tdm 48k. spdif 44.1k, we can't use the
>>>>>> same
>>>>>> pll, so spdif need use the mpll 0
>>>>>>
>>>>>> other pll , only set a default value, at the latest chip, we remove
>>>>>> all
>>>>>> mpll for hardware, only two hifipll
>>>>> I'm not sure you understand how this works.
>>>>> There is 3 families of audio rate: 48kHz, 44.1kHz and 32kHz
>>>>>
>>>>> Each family needs a PLL assigned, so you need 3, not 4, unless there
>>>>> is
>>>>> another specific rate family you want to support. If that's the case,
>>>>> document it.
>>>>>
>>>>> Setting the rate of the PLL should follow this principle:
>>>>> * Family rate
>>>>> - multiplied by (32 x 24): to accomodate different sample sizes
>>>>> - multiplied by 2 until you reach the maximum rate of selected
>>>>> PLLs
>>>>> This allows to support rates such 192k or even 768k
>>>>>
>>>>> 491520000 is not dividable by 3, it won't allow 24 bits words. It is a
>>>>> poor choice.
>>>>>
>>>>> Have a look at the s400 for an example using the HiFi PLL. The axg was
>>>>> restricted to a 68 PLL multiplier but the S4 is not so you should be
>>>>> able to use a higher base rate (4 718 592 000 Hz), providing better
>>>>> accuracy in the end
>>>> for new soc audio ip, the hardware will not support the 24bit(include
>>>> g12a,
>>>> sm1,axg)
>>> That may be what you chose to support in your BSP but that not how it
>>> works in mainline. 24bits slot width is supported and has been tested on
>>> axg, g12 and sm1. This is not going away.
>>>
>>> I would find extremely odd that 24 bits slot width is not supported on
>>> s4,
>>> but as long you document this, it is fine by me.
>>
>> i understand your meaning, you sad we configure the slot width 24bit for
>> tdmout control
>>
>> if the format the SNDRV_PCM_FMTBIT_S24, it send the 24bit data, for the
>> format, and send the 24bit clock
>>
>> if tdmout control can cut out [24:0] from the fddr, maybe your right, we
>> can send the 24 bit accoring to the slot width
>>
>> but it can't confirm by us, we are worried that there may be potential
>> risks, so we don't use it thay way
>>
>> so this why i sad can't support the 24bit slot clock, 16/32 sample bit is
>> fully validated
>>
>>
> i did some tests for the S24_LE format use the tdm base drvier
>
> aplay -f S24_LE test.pcm -r48000 -c2
>
> # cat /proc/asound/card0/pcm0p/sub0/hw_params
> access: RW_INTERLEAVED
> format: S24_LE
> subformat: STD
> channels: 2
> rate: 48000 (48000/1)
> period_size: 6000
> buffer_size: 24000
>
> we dump the mclk
>
> aud_mst_a_mclk 2 2 0 12288000 0 0
> 50000 Y audio-controller-0 mclk
>
> according to the base driver
>
> in the api axg_tdm_set_tdm_slots function
> switch (slot_width) {
> case 0:
> slot_width = 32;
32 bits is the default slot width if none is specified, yes. So ?
> fallthrough;
> ...
>
> if dts not configure "dai-tdm-slot-width"
>
> it use the 32 bit slot width
>
> the api -> axg_tdm_iface_set_sclk
>
> srate = iface->slots * iface->slot_width * params_rate(params);
>
> set mclk rate
>
> we dump tdmout control register
>
> # devmem 0xfe330500
> 0xB001003F
>
> it set 32bit slot width to send
>
> the base driver is the smae behavior that we wound expect,
The driver set a slot width of 32 bits because you did choose any and
then behave as it should.
I don't get your point here or what such test is supposed to show.
You did not test 24bits slot width at all.
>
>
>>>> SNDRV_PCM_FMTBIT_S24_3LE, 24 bit in memory
>>> I think you are mixing up slot width and memory representation
>>>
--
Jerome
Powered by blists - more mailing lists