Re: [PATCH 3/3] arm64: dts: amlogic: Add Amlogic S4 Audio

[Jiebing Chen <jiebing.chen@amlogic.com>]

在 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@amlogic.com> 
>> wrote:
>>
>>> 在 2025/1/14 19:16, Jerome Brunet 写道:
>>>> [ EXTERNAL EMAIL ]
>>>>
>>>> On Tue 14 Jan 2025 at 16:52, Jiebing Chen 
>>>> <jiebing.chen@amlogic.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@kernel.org> wrote:
>>>>>>
>>>>>>> From: jiebing chen <jiebing.chen@amlogic.com>
>>>>>>>
>>>>>>> Add basic audio driver support for the Amlogic S4 based Amlogic
>>>>>>> AQ222 board.
>>>>>>>
>>>>>>> Signed-off-by: jiebing chen <jiebing.chen@amlogic.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;
         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,


>>> SNDRV_PCM_FMTBIT_S24_3LE, 24 bit in memory
>> I think you are mixing up slot width and memory representation
>>