| 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: <550e0461-b564-496c-8fa9-78b2cbe9ba71@amlogic.com>
Date: Wed, 15 Jan 2025 17:56:54 +0800
From: Jiebing Chen <jiebing.chen@...ogic.com>
To: Jerome Brunet <jbrunet@...libre.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
在 2025/1/15 16:43, Jerome Brunet 写道:
> [ EXTERNAL EMAIL ]
>
> 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.
we focus on why hifipll set 49152000 for S24_LE format, why not to set
the slot width = 24
we consider that 24-bit probably hasn't done a lot of testing, suggest
slot width =32
tdmout cut out fddr[23:0], set slot width = 24, fddr msb = 23
we can support it, but can't sure the tdmout 24bit function can work for
long time
it need to test for many time , so we just think it can send 24bit with
8 bit zero for data lane,
if must be send 24 bit data with zero data
we need support the 24 bit clock pll
24 *32* 2 * 768k = 1179.648M
4 718 592 000 Hz you suggested maybe is out of range hififpll
i think the 1179.648M is more suitable, Do you think so?
if you agree, we ask clk owner to add it
>
>>
>>>>> 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