Re: [PATCH v2] ASoC: qcom: Add four speaker support on MI2S secondary

[Srinivasa Rao Mandadapu]

Hi Srini,

Thanks for your Time!!!

On 6/9/2021 1:40 PM, Srinivas Kandagatla wrote:
> On 09/06/2021 08:13, Srinivasa Rao Mandadapu wrote:
> > Add four speaker support on MI2S secondary block
> > by using I2S SD1 line on gpio52 pin, and add channel map
> > control support in the lpass-cpu audio driver.
> >
> > Signed-off-by: Srinivasa Rao Mandadapu <srivasam@xxxxxxxxxxxxxx>
> > ---
> > Changes Since V1:
> >     -- removed set_channel_map/get_channel_map implementation as 
> > default kcontrols
> >         added in pcm_new API.
> >
> >   sound/soc/qcom/lpass-cpu.c    | 32 ++++++++++++++++++++++++++++++++
> >   sound/soc/qcom/lpass-sc7180.c |  1 +
> >   sound/soc/qcom/lpass.h        |  2 ++
> >   3 files changed, 35 insertions(+)
> >
> > diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
> > index c62d2612e8f5..15d69e311b38 100644
> > --- a/sound/soc/qcom/lpass-cpu.c
> > +++ b/sound/soc/qcom/lpass-cpu.c
> > @@ -29,6 +29,14 @@
> >   #define LPASS_CPU_I2S_SD0_1_2_MASK    GENMASK(2, 0)
> >   #define LPASS_CPU_I2S_SD0_1_2_3_MASK    GENMASK(3, 0)
> >   +/*
> > + * Channel maps for Quad channel playbacks on MI2S Secondary
> > + */
> > +static struct snd_pcm_chmap_elem lpass_quad_chmaps = {
> > +        .channels = 4,
> > +        .map = { SNDRV_CHMAP_FL, SNDRV_CHMAP_RL,
> > +                SNDRV_CHMAP_FR, SNDRV_CHMAP_RR }
>
> AFAIU, You would need an empty entry at the end of this list if not we 
> will endup in a dereferencing memory sitting right next to this array 
> which will lead to random memory corruptions...
>
> Have a look at some of the examples in existing codec drivers.
>
> Other than that patch looks good.
>
> --srini
Okay. Will change accordingly.
> > +};
> >   static int lpass_cpu_init_i2sctl_bitfields(struct device *dev,
> >               struct lpaif_i2sctl *i2sctl, struct regmap *map)
> >   {
> > @@ -324,6 +332,25 @@ const struct snd_soc_dai_ops 
> > asoc_qcom_lpass_cpu_dai_ops = {
> >   };
> >   EXPORT_SYMBOL_GPL(asoc_qcom_lpass_cpu_dai_ops);
> >   +int lpass_cpu_pcm_new(struct snd_soc_pcm_runtime *rtd,
> > +                struct snd_soc_dai *dai)
> > +{
> > +    int ret;
> > +    struct snd_soc_dai_driver *drv = dai->driver;
> > +    struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
> > +
> > +    if (drvdata->mi2s_playback_sd_mode[dai->id] == 
> > LPAIF_I2SCTL_MODE_QUAD01) {
> > +        ret =  snd_pcm_add_chmap_ctls(rtd->pcm, 
> > SNDRV_PCM_STREAM_PLAYBACK,
> > +                &lpass_quad_chmaps, drv->playback.channels_max, 0,
> > +                NULL);
> > +        if (ret < 0)
> > +            return ret;
> > +    }
> > +
> > +    return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(lpass_cpu_pcm_new);
> > +
> >   int asoc_qcom_lpass_cpu_dai_probe(struct snd_soc_dai *dai)
> >   {
> >       struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
> > @@ -856,6 +883,11 @@ int asoc_qcom_lpass_cpu_platform_probe(struct 
> > platform_device *pdev)
> >                   PTR_ERR(drvdata->mi2s_bit_clk[dai_id]));
> >               return PTR_ERR(drvdata->mi2s_bit_clk[dai_id]);
> >           }
> > +        if (drvdata->mi2s_playback_sd_mode[dai_id] ==
> > +            LPAIF_I2SCTL_MODE_QUAD01) {
> > + variant->dai_driver[dai_id].playback.channels_min = 4;
> > + variant->dai_driver[dai_id].playback.channels_max = 4;
> > +        }
> >       }
> >         /* Allocation for i2sctl regmap fields */
> > diff --git a/sound/soc/qcom/lpass-sc7180.c 
> > b/sound/soc/qcom/lpass-sc7180.c
> > index 8c168d3c589e..77a556b27cf0 100644
> > --- a/sound/soc/qcom/lpass-sc7180.c
> > +++ b/sound/soc/qcom/lpass-sc7180.c
> > @@ -58,6 +58,7 @@ static struct snd_soc_dai_driver 
> > sc7180_lpass_cpu_dai_driver[] = {
> >           },
> >           .probe    = &asoc_qcom_lpass_cpu_dai_probe,
> >           .ops    = &asoc_qcom_lpass_cpu_dai_ops,
> > +        .pcm_new = lpass_cpu_pcm_new,
> >       }, {
> >           .id = LPASS_DP_RX,
> >           .name = "Hdmi",
> > diff --git a/sound/soc/qcom/lpass.h b/sound/soc/qcom/lpass.h
> > index 83b2e08ade06..623ddccdafff 100644
> > --- a/sound/soc/qcom/lpass.h
> > +++ b/sound/soc/qcom/lpass.h
> > @@ -259,5 +259,7 @@ void asoc_qcom_lpass_cpu_platform_shutdown(struct 
> > platform_device *pdev);
> >   int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev);
> >   int asoc_qcom_lpass_cpu_dai_probe(struct snd_soc_dai *dai);
> >   extern const struct snd_soc_dai_ops asoc_qcom_lpass_cpu_dai_ops;
> > +int lpass_cpu_pcm_new(struct snd_soc_pcm_runtime *rtd,
> > +                struct snd_soc_dai *dai);
> >     #endif /* __LPASS_H__ */
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.