Message-ID: <CAC=S1nizJHcvxG2-HO7MhwWup-o6SO0XBp1_OBN35SCtUW73iA@mail.gmail.com>
Date: Tue, 29 Oct 2024 16:52:38 +0800
From: Fei Shao <fshao@...omium.org>
To: Trevor Wu (吳文良) <Trevor.Wu@...iatek.com>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Cc: "krzk@...nel.org" <krzk@...nel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, 
	"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>, 
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>, 
	"linux-sound@...r.kernel.org" <linux-sound@...r.kernel.org>, "broonie@...nel.org" <broonie@...nel.org>, 
	"conor+dt@...nel.org" <conor+dt@...nel.org>, "robh@...nel.org" <robh@...nel.org>, 
	"lgirdwood@...il.com" <lgirdwood@...il.com>, 
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>, 
	"matthias.bgg@...il.com" <matthias.bgg@...il.com>, "krzk+dt@...nel.org" <krzk+dt@...nel.org>
Subject: Re: [PATCH 1/4] ASoC: dt-bindings: mediatek,mt8188-mt6359: Add adsp
 and dai-link properties

On Tue, Oct 29, 2024 at 11:05 AM Trevor Wu (吳文良) <Trevor.Wu@...iatek.com> wrote:
>
> On Mon, 2024-10-28 at 15:25 +0100, AngeloGioacchino Del Regno wrote:
> > External email : Please do not click links or open attachments until
> > you have verified the sender or the content.
> >
> >
> > Il 28/10/24 12:10, Fei Shao ha scritto:
> > > On Mon, Oct 28, 2024 at 4:54 AM Krzysztof Kozlowski <
> > > krzk@...nel.org> wrote:
> > > >
> > > > On Fri, Oct 25, 2024 at 06:44:41PM +0800, Fei Shao wrote:
> > > > > Add "mediatek,adsp" property for the ADSP handle if ADSP is
> > > > > enabled on
> > > > > the platform.
> > > >
> > > > We see this from the diff.
> > > >
> > > > > Add "mediatek,dai-link" property for the required DAI links to
> > > > > configure
> > > > > sound card.
> > > >
> > > > We see this from the diff.
> > > >
> > > > >
> > > > > Both properties are commonly used in the MediaTek sound card
> > > > > driver.
> > > >
> > > > If they are used, why suddenly they are needed? What changed?
> > >
> > > Nothing has changed. These should have been added altogether when
> > > the
> > > binding was first introduced. This patch is to fill the gaps and
> > > fix
> > > dtbs_check warnings, like I mentioned in the cover letter.
> > > I can add a line in the commit message saying it's to fix the
> > > warning
> > > in addition to the cover letter, if that's preferred.
> > >
> > > >
> > > > >
> > > > > Signed-off-by: Fei Shao <fshao@...omium.org>
> > > > > ---
> > > > >
> > > > >   .../bindings/sound/mediatek,mt8188-mt6359.yaml         | 10
> > > > > ++++++++++
> > > > >   1 file changed, 10 insertions(+)
> > > > >
> > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/sound/mediatek,mt8188-
> > > > > mt6359.yaml
> > > > > b/Documentation/devicetree/bindings/sound/mediatek,mt8188-
> > > > > mt6359.yaml
> > > > > index f94ad0715e32..701cedfa38d2 100644
> > > > > --- a/Documentation/devicetree/bindings/sound/mediatek,mt8188-
> > > > > mt6359.yaml
> > > > > +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8188-
> > > > > mt6359.yaml
> > > > > @@ -29,6 +29,16 @@ properties:
> > > > >       $ref: /schemas/types.yaml#/definitions/phandle
> > > > >       description: The phandle of MT8188 ASoC platform.
> > > > >
> > > > > +  mediatek,adsp:
> > > > > +    $ref: /schemas/types.yaml#/definitions/phandle
> > > > > +    description: The phandle of MT8188 ADSP platform.
> > > >
> > > > And what is the difference between ASoC and ADSP platforms? What
> > > > are
> > > > they used for?
> > >
> > > I'm not a MediaTek or audio folks, and I'm afraid that I'm not the
> > > best person to explain the details accurately in front of experts
> > > on
> > > the list... I know it's an audio DSP but that explains nothing.
> > > MediaTek didn't provide a meaningful explanation in the tree or
> > > commits, and I want to avoid adding additional but likely
> > > misleading
> > > descriptions from someone who doesn't have enough knowledge,
> > > potentially causing even more confusing situations in the future.
> > > Plus, the same changes were accepted as-is in the past, so I
> > > assumed
> > > they might be self-explanatory to people who are familiar with the
> > > matter.
> > >
> >
> > The Audio DSP is a Tensilica HiFi-5 DSP, and it's a block that is -
> > hardware
> > speaking - separated from the rest of the Audio interfaces of the
> > SoC.
> >
> > The whole sound subsystem can work either with or without the DSP, in
> > the sense
> > that the DSP itself can remain unpowered and completely unconfigured
> > if its
> > functionality is not desired - hence, this is a board specific
> > configuration:
> > if the board wants to use the DSP, we use the DSP - otherwise, we
> > just don't.
> >
> > Regarding the two "platforms", in short:
> > "mediatek,platform" -> Audio Front End (AFE)
> > "mediatek,adsp" -> Audio DSP
> >
> > Now, you can either link the AFE DAIs to the I2S
> >
> > As for "mediatek,platform" - that's used to link the Analog Front End
> > (AFE) as
> > the DAI Link platform (so the path is direct to/from DL/UL DAIs to
> > AFE) or the
> > ADSP one as the DAI Link platform (so that the path is to/from DL/UL
> > DAIs to
> > DSP to AFE), but that - of course - still requires an AFE, otherwise
> > you cannot
> > get the audio out of the speakers or in from the mic anyway.
> >
> > > >
> > > > > +
> > > > > +  mediatek,dai-link:
> > > > > +    $ref: /schemas/types.yaml#/definitions/string-array
> > > > > +    description:
> > > > > +      A list of the desired dai-links in the sound card. Each
> > > > > entry is a
> > > > > +      name defined in the machine driver.
> > > >
> > > > The list is provided below. I don't understand why do you need
> > > > it. Your
> > > > msg is pretty useless - you describe what you do, instead of why.
> > >
> > > I think this is used to explicitly list the intermediate but hidden
> > > DAIs, but again, there's not much info about them unless MediaTek
> > > can
> > > explain more details and why they need a vendor property for this.
> > >
> >
> > Yes, this is used for exactly that... but I believe that we can
> > deprecate this
> > now that we have support for the "standard" `audio-routing` property
> > and for the
> > DAI Link nodes (examples that you can find in current device trees
> > are mm-dai-link,
> > hs-playback-dai-link, or any other subnode of the sound card).
> >
> > Specifically, those subnodes *do* require a "link-name" property,
> > which *does*
> > effectively contain the same DAI Link names as the ones that are
> > inside of the
> > "mediatek,dai-link" property.
> >
> > On MT8195, you can find both the subnodes and the mediatek,dai-link -
> > yes - but
> > that was done to retain compatibility of the device tree with old
> > drivers (so,
> > an unusual case of new device tree on old kernel).
> >
> > Finally, I believe that we can avoid adding that "mediatek,dai-link"
> > property
> > to the MT8188 binding, and rely on:
> >   1. Whatever is provided in struct snd_soc_card for that device; and
> >   2. Whatever is provided in device trees as dai link subnodes, which
> > would
> >      restrict N.1 as that's anyway describing card prelinks.
> >
>
> The "mediatek,dai-link" property is utilized to hide the unused dai-
> links in the sound card. By hiding the unused FE links, it can save the
> necessary memory and prevent conflicts where both the DSP and AP
> control the same AFE Memifs.
>
> This concept was first implemented in mt8195 as Mark aimed to avoid the
> need for a separate driver for different system configurations.
> With the introduction of DSP (SOF), if certain AFE Memifs are already
> in use in the DSP route, they should be excluded from the PCM nodes
> created for the AFE platform.
>
> At that time, we did not have a better way to handle these scenarios,
> so we made use of a vendor-defined property.
>
> It has been a while since I last kept up with the updates in mt8188, so
> I'm uncertain if the current mechanism for sound card description is
> sufficient for handling such scenarios. If it is, I agree that we can
> deprecate such a vendor property from mt8188.
>
> Thanks,
> Trevor
>
>
> > Cheers,
> > Angelo

Many thanks Angelo and Trevor for the information.
The audio-routing property is in place, so I'll check if removing the
vendor property makes any impact, and refresh the series accordingly.

Thanks,
Fei



> >
> > > Regards,
> > > Fei
> > >
> > > >
> > > > Best regards,
> > > > Krzysztof
> > > >
> >
> >

Powered by blists - more mailing lists