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

[Jiebing Chen <jiebing.chen@xxxxxxxxxxx>]

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


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