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

["Luca Weiss" <luca.weiss@fairphone.com>]

On Tue Feb 4, 2025 at 11:09 AM CET, Luca Weiss wrote:
> 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@linaro.org 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@jiaxyga.com>
>>>>>>>> ---
>>>>>>>>     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@kernel.org/
>>>>
>>>> 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...

FYI I've sent these patches now, confirmed with DisplayPort audio on
QCM6490 Fairphone 5

https://lore.kernel.org/linux-arm-msm/20250418-fp5-dp-sound-v2-2-05d65f084b05@fairphone.com/
+ following

Regards
Luca

>
>>
>> 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:
>>>>>>>>             /*
>>>>>>>
>>>>>
>>>