Message-ID: <7364fb08-fe43-167d-a083-db4a6234222c@linux.intel.com>
Date:   Tue, 23 Mar 2021 14:25:53 -0500
From:   Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
To:     Codrin Ciubotariu <codrin.ciubotariu@...rochip.com>,
        alsa-devel@...a-project.org, linux-kernel@...r.kernel.org
Cc:     gustavoars@...nel.org, tiwai@...e.com, lgirdwood@...il.com,
        mirq-linux@...e.qmqm.pl, broonie@...nel.org
Subject: Re: [RFC PATCH 0/3] Separate BE DAI HW constraints from FE ones



On 3/23/21 6:43 AM, Codrin Ciubotariu wrote:
> HW constraints are needed to set limitations for HW parameters used to
> configure the DAIs. All DAIs on the same link must agree upon the HW
> parameters, so the parameters are affected by the DAIs' features and
> their limitations. In case of DPCM, the FE DAIs can be used to perform
> different kind of conversions, such as format or rate changing, bringing
> the audio stream to a configuration supported by all the DAIs of the BE's
> link. For this reason, the limitations of the BE DAIs are not always
> important for the HW parameters between user-space and FE, only for the
> paratemers between FE and BE DAI links. This brings us to this patch-set,
> which aims to separate the FE HW constraints from the BE HW constraints.
> This way, the FE can be used to perform more efficient HW conversions, on
> the initial audio stream parameters, to parameters supported by the BE
> DAIs.
> To achieve this, the first thing needed is to detect whether a HW
> constraint rule is enforced by a FE or a BE DAI. This means that
> snd_pcm_hw_rule_add() needs to be able to differentiate between the two
> type of DAIs. For this, the runtime pointer to struct snd_pcm_runtime is
> replaced with a pointer to struct snd_pcm_substream, to be able to reach
> substream->pcm->internal to differentiate between FE and BE DAIs.
> This change affects many sound drivers (and one gpu drm driver).
> All these changes are included in the first patch, to have a better
> overview of the implications created by this change.
> The second patch adds a new struct snd_pcm_hw_constraints under struct
> snd_soc_dpcm_runtime, which is used to store the HW constraint rules
> added by the BE DAIs. This structure is initialized with a subset of the
> runtime constraint rules which does not include the rules that affect
> the buffer or period size. snd_pcm_hw_rule_add() will add the BE rules
> to the new struct snd_pcm_hw_constraints.
> The third and last patch will apply the BE rule constraints, after the
> fixup callback. If the fixup HW parameters do not respect the BE
> constraint rules, the rules will exit with an error. The FE mask and
> interval constraints are copied to the BE ones, to satisfy the
> dai_link->dpcm_merged_* flags. The dai_link->dpcm_merged_* flags are
> used to know if the FE does format or sampling rate conversion.
> 
> I tested with ad1934 and wm8731 codecs as BEs, with a not-yet-mainlined
> ASRC as FE, that can also do format conversion. I realize that the
> change to snd_pcm_hw_rule_add() has a big impact, even though all the
> drivers use snd_pcm_hw_rule_add() with substream->runtime, so passing
> substream instead of runtime is not that risky.

can you use the BE hw_params_fixup instead?

That's what we use for SOF.

The FE hw_params are propagated to the BE, and then the BE can update 
the hw_params based on its own limitations and pass the result 
downstream, e.g. to a codec.

I'll copy below my understanding of the flow, which we discussed 
recently in the SOF team:

my understanding is that we start with the front-end hw_params as the 
basis for the back-end hw_params.

static int dpcm_fe_dai_hw_params(struct snd_pcm_substream *substream,
                  struct snd_pcm_hw_params *params)
{
     struct snd_soc_pcm_runtime *fe = asoc_substream_to_rtd(substream);
     int ret, stream = substream->stream;

     mutex_lock_nested(&fe->card->mutex, SND_SOC_CARD_CLASS_RUNTIME);
     dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_FE);

     memcpy(&fe->dpcm[stream].hw_params, params,
             sizeof(struct snd_pcm_hw_params));
     ret = dpcm_be_dai_hw_params(fe, stream);
<<< the BE is handled first.
     if (ret < 0) {
         dev_err(fe->dev,"ASoC: hw_params BE failed %d\n", ret);
         goto out;
     }

     dev_dbg(fe->dev, "ASoC: hw_params FE %s rate %d chan %x fmt %d\n",
             fe->dai_link->name, params_rate(params),
             params_channels(params), params_format(params));

     /* call hw_params on the frontend */
     ret = soc_pcm_hw_params(substream, params);

then each BE will be configured

int dpcm_be_dai_hw_params(struct snd_soc_pcm_runtime *fe, int stream)
{
     struct snd_soc_dpcm *dpcm;
     int ret;

     for_each_dpcm_be(fe, stream, dpcm) {

         struct snd_soc_pcm_runtime *be = dpcm->be;
         struct snd_pcm_substream *be_substream =
             snd_soc_dpcm_get_substream(be, stream);

         /* is this op for this BE ? */
         if (!snd_soc_dpcm_be_can_update(fe, be, stream))
             continue;

         /* copy params for each dpcm */
         memcpy(&dpcm->hw_params, &fe->dpcm[stream].hw_params,
                 sizeof(struct snd_pcm_hw_params));

         /* perform any hw_params fixups */
         ret = snd_soc_link_be_hw_params_fixup(be, &dpcm->hw_params);

The fixup is the key, in SOF this is where we are going to look for 
information from the topology.

/* fixup the BE DAI link to match any values from topology */
int sof_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd, struct 
snd_pcm_hw_params *params)
{
     struct snd_interval *rate = hw_param_interval(params,
             SNDRV_PCM_HW_PARAM_RATE);
     struct snd_interval *channels = hw_param_interval(params,
                         SNDRV_PCM_HW_PARAM_CHANNELS);
     struct snd_mask *fmt = hw_param_mask(params, 
SNDRV_PCM_HW_PARAM_FORMAT);
     struct snd_soc_component *component =
         snd_soc_rtdcom_lookup(rtd, SOF_AUDIO_PCM_DRV_NAME);
     struct snd_sof_dai *dai =
         snd_sof_find_dai(component, (char *)rtd->dai_link->name);
     struct snd_soc_dpcm *dpcm;

     /* no topology exists for this BE, try a common configuration */
     if (!dai) {
         dev_warn(component->dev,
              "warning: no topology found for BE DAI %s config\n",
              rtd->dai_link->name);

         /*  set 48k, stereo, 16bits by default */
         rate->min = 48000;
         rate->max = 48000;

         channels->min = 2;
         channels->max = 2;

         snd_mask_none(fmt);
         snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S16_LE);

         return 0;
     }

     /* read format from topology */
     snd_mask_none(fmt);

     switch (dai->comp_dai.config.frame_fmt) {
     case SOF_IPC_FRAME_S16_LE:
         snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S16_LE);
         break;
     case SOF_IPC_FRAME_S24_4LE:
         snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S24_LE);
         break;
     case SOF_IPC_FRAME_S32_LE:
         snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S32_LE);
         break;
     default:
         dev_err(component->dev, "error: No available DAI format!\n");
         return -EINVAL;
     }

     /* read rate and channels from topology */
     switch (dai->dai_config->type) {
     case SOF_DAI_INTEL_SSP:
         rate->min = dai->dai_config->ssp.fsync_rate;
         rate->max = dai->dai_config->ssp.fsync_rate;
         channels->min = dai->dai_config->ssp.tdm_slots;
         channels->max = dai->dai_config->ssp.tdm_slots;

Powered by blists - more mailing lists