Re: [PATCH 1/5] ASoC: mediatek: mt8188: separate ADDA playback dai from capture dai

[Trevor Wu (吳文良)]

On Wed, 2023-05-17 at 13:40 +0200, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Il 17/05/23 13:15, Trevor Wu ha scritto:
> > MT8188 will support SOF. In SOF, be_hw_params_fixup callback are
> > used to
> > configure BE hardware parameters. However, playback and capture
> > stream
> > share the same callback function in which it can't know the stream
> > type.
> > 
> > It's possible to require different paremters for playback and
> > capture
> > stream, so separate them into two dais for SOF usage.
> > 
> > Signed-off-by: Trevor Wu <trevor.wu@xxxxxxxxxxxx>
> > ---
> >   sound/soc/mediatek/mt8188/mt8188-afe-common.h |  3 +-
> >   sound/soc/mediatek/mt8188/mt8188-dai-adda.c   | 76 ++++++++++--
> > -------
> >   sound/soc/mediatek/mt8188/mt8188-mt6359.c     | 34 +++++++--
> >   3 files changed, 68 insertions(+), 45 deletions(-)
> > 
> > diff --git a/sound/soc/mediatek/mt8188/mt8188-afe-common.h
> > b/sound/soc/mediatek/mt8188/mt8188-afe-common.h
> > index eb7e57c239bd..1304d685a306 100644
> > --- a/sound/soc/mediatek/mt8188/mt8188-afe-common.h
> > +++ b/sound/soc/mediatek/mt8188/mt8188-afe-common.h
> > @@ -39,7 +39,7 @@ enum {
> >       MT8188_AFE_MEMIF_END,
> >       MT8188_AFE_MEMIF_NUM = (MT8188_AFE_MEMIF_END -
> > MT8188_AFE_MEMIF_START),
> >       MT8188_AFE_IO_START = MT8188_AFE_MEMIF_END,
> > -     MT8188_AFE_IO_ADDA = MT8188_AFE_IO_START,
> > +     MT8188_AFE_IO_DL_SRC = MT8188_AFE_IO_START,
> >       MT8188_AFE_IO_DMIC_IN,
> >       MT8188_AFE_IO_DPTX,
> >       MT8188_AFE_IO_ETDM_START,
> > @@ -52,6 +52,7 @@ enum {
> >       MT8188_AFE_IO_ETDM_NUM =
> >               (MT8188_AFE_IO_ETDM_END - MT8188_AFE_IO_ETDM_START),
> >       MT8188_AFE_IO_PCM = MT8188_AFE_IO_ETDM_END,
> > +     MT8188_AFE_IO_UL_SRC,
> >       MT8188_AFE_IO_END,
> >       MT8188_AFE_IO_NUM = (MT8188_AFE_IO_END -
> > MT8188_AFE_IO_START),
> >       MT8188_DAI_END = MT8188_AFE_IO_END,
> > diff --git a/sound/soc/mediatek/mt8188/mt8188-dai-adda.c
> > b/sound/soc/mediatek/mt8188/mt8188-dai-adda.c
> > index fed9f927e623..9a6673a6f28a 100644
> > --- a/sound/soc/mediatek/mt8188/mt8188-dai-adda.c
> > +++ b/sound/soc/mediatek/mt8188/mt8188-dai-adda.c
> > @@ -53,8 +53,7 @@ enum {
> >   };
> > 
> >   struct mtk_dai_adda_priv {
> > -     unsigned int dl_rate;
> > -     unsigned int ul_rate;
> > +     bool hires_required;
> >   };
> > 
> >   static unsigned int afe_adda_dl_rate_transform(struct
> > mtk_base_afe *afe,
> 
> ..snip..
> 
> > @@ -503,13 +495,15 @@ static int mtk_dai_adda_hw_params(struct
> > snd_pcm_substream *substream,
> >       dev_dbg(afe->dev, "%s(), id %d, stream %d, rate %u\n",
> >               __func__, id, substream->stream, rate);
> > 
> > -     if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> > -             adda_priv->dl_rate = rate;
> > +     if (rate > ADDA_HIRES_THRES)
> > +             adda_priv->hires_required = 1;
> > +     else
> > +             adda_priv->hires_required = 0;
> > +
> 
> hires_required is a boolean, so assigning 1 or 0 should be replaced
> with
> assigning true or false; regardless of that, what about...
> 
>         adda_priv->hires_required = (rate > ADDA_HIRES_THRES);
> 

It's great!! I will adopt it in V2.

Thanks,
Trevor

> > +     if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> >               ret = mtk_dai_da_configure(afe, rate, id);
> > -     } else {
> > -             adda_priv->ul_rate = rate;
> > +     else
> >               ret = mtk_dai_ad_configure(afe, rate, id);
> > -     }
> > 
> >       return ret;
> >   }
> 
> Regards,
> Angelo