Re: [PATCH v2 2/3] ASoC: mediatek: common: revise SOF common code

[Trevor Wu (吳文良)]

On Thu, 2023-08-03 at 10:23 +0200, AngeloGioacchino Del Regno wrote:
> Il 03/08/23 07:26, Trevor Wu ha scritto:
> > Originally, normal dai link fixup callback is overwritten by sof
> > fixup
> > callback on mtk_sof_card_late_probe and it relies on the mapping
> > defined
> > on struct sof_conn_stream.
> > 
> > It's not flexible. When a new hardware connection is adopted, user
> > needs
> > to update struct sof_conn_stream defined in machine driver which is
> > used
> > to specify the mapping relationship of normal BE and SOF BE.
> > 
> > In the patch, mtk_sof_check_tplg_be_dai_link_fixup() is introduced
> > for
> > all normal BEs. In mtk_sof_late_probe, back up normal BE fixup if
> > it
> > exists and then overwrite be_hw_params_fixup by the new callback.
> > 
> > There are two cases for FE and BE connection.
> > 
> > case 1:
> > SOF FE -> normal BE
> >         -> SOF_BE
> > 
> > case 2:
> > normal FE -> normal BE
> > 
> > In the new fixup callback, it tries to find SOF_BE which connects
> > to the
> > same FE, and then reuses the fixup of SOF_BE. If no SOF_BE exists,
> > it must be case 2, so rollback to the original fixup if it exists.
> > 
> > As a result, the predefined relation is not needed anymore.
> > Hardware
> > connection can be controlled by the mixer control for AFE
> > interconn.
> > Then, DPCM finds the BE mapping at runtime.
> > 
> > Signed-off-by: Trevor Wu <trevor.wu@xxxxxxxxxxxx>
> > ---
> >   .../soc/mediatek/common/mtk-dsp-sof-common.c  | 106
> > +++++++++++++++---
> >   .../soc/mediatek/common/mtk-dsp-sof-common.h  |   8 ++
> >   2 files changed, 99 insertions(+), 15 deletions(-)
> > 
> > diff --git a/sound/soc/mediatek/common/mtk-dsp-sof-common.c
> > b/sound/soc/mediatek/common/mtk-dsp-sof-common.c
> > index 6fef16306f74..3fb193c1f0d9 100644
> > --- a/sound/soc/mediatek/common/mtk-dsp-sof-common.c
> > +++ b/sound/soc/mediatek/common/mtk-dsp-sof-common.c
> > @@ -54,6 +54,8 @@ int mtk_sof_card_probe(struct snd_soc_card *card)
> >   {
> >   	int i;
> >   	struct snd_soc_dai_link *dai_link;
> > +	struct mtk_soc_card_data *soc_card_data =
> > snd_soc_card_get_drvdata(card);
> > +	struct mtk_sof_priv *sof_priv = soc_card_data->sof_priv;
> >   
> >   	/* Set stream_name to help sof bind widgets */
> >   	for_each_card_prelinks(card, i, dai_link) {
> > @@ -61,10 +63,74 @@ int mtk_sof_card_probe(struct snd_soc_card
> > *card)
> >   			dai_link->stream_name = dai_link->name;
> >   	}
> >   
> > +	INIT_LIST_HEAD(&sof_priv->dai_link_list);
> > +
> >   	return 0;
> >   }
> >   EXPORT_SYMBOL_GPL(mtk_sof_card_probe);
> >   
> 
> static int mtk_sof_find_tplg_be_dai_link(struct snd_soc_pcm_runtime
> *rtd,
> 					 struct snd_soc_dai_link
> **sof_dai_link)
> {
> 	... variables here...
> 
> 	for_each_pcm_streams() {
> 		fe = NULL;
> 
> 		.....
> 
> 		if (!strcmp(...)) {
> 			sof_dai_link = ....
> 			return 0;
> 		}
> 	}
> 	return -ENOENT (or something else error);
> }
> 
> P.S.: otherwise just make this function return a snd_soc_dai_link
> pointer...
> 
> > +/* fixup the BE DAI link to match any values from topology */
> > +static int mtk_sof_check_tplg_be_dai_link_fixup(struct
> > snd_soc_pcm_runtime *rtd,
> > +						struct
> > snd_pcm_hw_params *params)
> > +{
> > +	struct snd_soc_card *card = rtd->card;
> > +	struct mtk_soc_card_data *soc_card_data =
> > snd_soc_card_get_drvdata(card);
> > +	struct mtk_sof_priv *sof_priv = soc_card_data->sof_priv;
> > +	struct snd_soc_pcm_runtime *fe;
> > +	struct snd_soc_pcm_runtime *be;
> > +	struct snd_soc_dai_link *sof_dai_link = NULL;
> > +	struct mtk_dai_link *dai_link;
> > +	struct snd_soc_dpcm *dpcm;
> > +	int i, stream;
> > +	int ret = 0;
> > +
> > +	for_each_pcm_streams(stream) {
> > +		fe = NULL;
> > +		for_each_dpcm_fe(rtd, stream, dpcm) {
> > +			fe = dpcm->fe;
> > +			if (fe)
> > +				break;
> > +		}
> > +
> > +		if (!fe)
> > +			continue;
> > +
> > +		for_each_dpcm_be(fe, stream, dpcm) {
> > +			be = dpcm->be;
> > +			if (be == rtd)
> > +				continue;
> > +
> > +			for (i = 0; i < sof_priv->num_streams; i++) {
> > +				const struct sof_conn_stream *conn =
> > &sof_priv->conn_streams[i];
> > +
> > +				if (!strcmp(be->dai_link->name, conn-
> > >sof_link)) {
> > +					sof_dai_link = be->dai_link;
> > +					goto FIXUP;
> > +				}
> > +			}
> > +		}
> > +	}
> > +
> > +FIXUP:
> 
> Please, lowercase labels... or you can avoid gotos entirely:
> 	ret = mtk_sof_find_tplg_be_dai_link(...)
> 	if (ret == 0 && sof_dai_link) {
> 		if (sof_priv->sof_dai_link_fixup)
> 			return sof_priv->sof_dai_link_fixup(rtd,
> params);
> 		else if (sof_dai_link->be_hw_params_fixup)
> 			return sof_dai_link->be_hw_params_fixup(be,
> params);	
> 	} else {
> 		list_for_each_entry(dai_link, &sof_priv->dai_link_list, 
> list) {
> 			if (strcmp(dai_link->name, rtd->dai_link->name) 
> == 0) {
> 				if (dai_link->be_hw_params_fixup)
> 					return dai_link-
> >be_hw_params_fixup(rtd, params);
> 				else
> 					break;
> 			}
> 		}
> 	}
> 
> 	return 0;
> }
> 
> P.S.: I'm truly sorry for not noticing that before!
> 
> Regards,
> Angelo

Hi Angelo,

Thanks for your suggestion.
I will add mtk_sof_find_tplg_be_dai_link() to improve the readability
and remove the use of goto in the next patch.

Thanks,
Trevor