Re: [PATCH] ASoC: qcom: sc8280xp: enable primary mi2s

[Luca Weiss]

On Tue Feb 4, 2025 at 11:08 AM CET, neil.armstrong wrote:
> On 04/02/2025 11:04, Luca Weiss wrote:
>> On Tue Feb 4, 2025 at 9:19 AM CET, neil.armstrong wrote:
>>> On 04/02/2025 00:27, Luca Weiss wrote:
>>>> On Mon Feb 3, 2025 at 5:33 PM CET, Danila Tikhonov wrote:
>>>>> On 03/02/2025 17:23, neil.armstrong@xxxxxxxxxx wrote:
>>>>>> On 03/02/2025 12:38, Danila Tikhonov wrote:
>>>>>>> When using primary mi2s on sc8280xp-compatible SoCs, the correct clock
>>>>>>> needs to get enabled to be able to use the mi2s interface.
>>>>>>>
>>>>>>> Signed-off-by: Danila Tikhonov <danila@xxxxxxxxxxx>
>>>>>>> ---
>>>>>>>     sound/soc/qcom/sc8280xp.c | 13 +++++++++++++
>>>>>>>     1 file changed, 13 insertions(+)
>>>>>>>
>>>>>>> diff --git a/sound/soc/qcom/sc8280xp.c b/sound/soc/qcom/sc8280xp.c
>>>>>>> index 311377317176..03687de1ebb0 100644
>>>>>>> --- a/sound/soc/qcom/sc8280xp.c
>>>>>>> +++ b/sound/soc/qcom/sc8280xp.c
>>>>>>> @@ -14,6 +14,8 @@
>>>>>>>     #include "common.h"
>>>>>>>     #include "sdw.h"
>>>>>>>     +#define MI2S_BCLK_RATE    1536000
>>>>>>> +
>>>>>>>     struct sc8280xp_snd_data {
>>>>>>>         bool stream_prepared[AFE_PORT_MAX];
>>>>>>>         struct snd_soc_card *card;
>>>>>>> @@ -25,13 +27,24 @@ struct sc8280xp_snd_data {
>>>>>>>       static int sc8280xp_snd_init(struct snd_soc_pcm_runtime *rtd)
>>>>>>>     {
>>>>>>> +    unsigned int codec_dai_fmt = SND_SOC_DAIFMT_BC_FC;
>>>>>>> +    unsigned int fmt = SND_SOC_DAIFMT_BP_FP;
>>>>>>>         struct sc8280xp_snd_data *data =
>>>>>>> snd_soc_card_get_drvdata(rtd->card);
>>>>>>> +    struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(rtd, 0);
>>>>>>>         struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
>>>>>>>         struct snd_soc_card *card = rtd->card;
>>>>>>>         struct snd_soc_jack *dp_jack  = NULL;
>>>>>>>         int dp_pcm_id = 0;
>>>>>>>           switch (cpu_dai->id) {
>>>>>>> +    case PRIMARY_MI2S_RX:
>>>>>>> +        codec_dai_fmt |= SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_I2S;
>>>>>>> +        snd_soc_dai_set_sysclk(cpu_dai,
>>>>>>> +            Q6AFE_LPASS_CLK_ID_PRI_MI2S_IBIT,
>>>>>>> +            MI2S_BCLK_RATE, SNDRV_PCM_STREAM_PLAYBACK);
>>>>>>
>>>>>> How is this possible ? sc8280xp uses the q6prm clock driver, and
>>>>>> there's no
>>>>>> way this call sets the Q6PRM_LPASS_CLK_ID_PRI_MI2S_IBIT, or I totally
>>>>>> missed
>>>>>> something.
>>>>>>
>>>>>> And prm is neither a dai nor has the set_sysclk callback.
>>>>>>
>>>>>> Neil
>>>>>>
>>>>> Oh, thanks for the answer.
>>>>>
>>>>> This comes from qcm6490 compatibility. Actually it is needed for
>>>>> Nothing Phone (1) and Luca Weiss just suggested me to use
>>>>> qcom,qcm6490-idp-sndcard like do they do it for FP5. (SM7325 is the
>>>>> closest to QCM6490):
>>>>> https://github.com/sc7280-mainline/linux/pull/5#discussion_r1848984788
>>>>> Actually I also think it is a bit incorrect.
>>>>
>>>> For reference, this is coming from this and following:
>>>> https://lore.kernel.org/linux-arm-msm/e8a24709-de96-4d09-ba00-1e084a656c68@xxxxxxxxxx/
>>>
>>> Sure, but do you use sound/soc/qcom/sc8280xp.c ? it's designed for audioreach
>>> compatible audio architecture (post sm8250).
>> 
>> I would've also thought qcm6490-idp and qcm6490-rb3gen2 are
>> pre-audioreach, so sm8250.c would be a better match - but these two
>> devices already use sc8280xp.c driver for some reason?
>
> I think they were added to sc8280xp.c because it has the logic to change
> the card name and some other tweaks for record channels setup,
> but it can be easily added to sm8250.c.

This is probably a better idea then...

>
> Neil
>
>> 
>> Maybe someone with more insight on these devices can help clarify this?
>> I'll also need this for qcm6490-fairphone-fp5 soon.
>> 
>> Regards Luca
>> 
>>>
>>> sound/soc/qcom/sm8250.c has already support for PRIMARY_MI2S_RX via
>>> the Q6AFE_LPASS_CLK_ID_PRI_MI2S_IBIT, and it compatible with the previous
>>> audio architecture (non audioreach).
>>>
>>> Neil
>>>
>>>>
>>>> Regards
>>>> Luca
>>>>
>>>>>
>>>>> ---
>>>>> Regards,
>>>>> Danila
>>>>>>> +        snd_soc_dai_set_fmt(cpu_dai, fmt);
>>>>>>> +        snd_soc_dai_set_fmt(codec_dai, codec_dai_fmt);
>>>>>>> +        break;
>>>>>>>         case WSA_CODEC_DMA_RX_0:
>>>>>>>         case WSA_CODEC_DMA_RX_1:
>>>>>>>             /*
>>>>>>
>>>>
>>