Hi Harsha, I've put the reason for this change in the commit message. I had to split be_hw_params_fixup function for different codecs because current approach (made for kernel 4.4) used in kernel 5.4, leads to crash while trying to get snd_soc_dpcm with container_of() macro in kabylake_ssp_fixup(). The crash call path looks as below: soc_pcm_hw_params() snd_soc_dai_hw_params(codec_dai, substream, &codec_params); rtd->dai_link->be_hw_params_fixup(rtd, params) kabylake_ssp_fixup() In this case, codec_params is just a copy of an internal structure and is not embedded into struct snd_soc_dpcm thus we cannot use container_of() on it. Best regards, Lukasz pon., 29 cze 2020 o 18:51 N, Harshapriya <harshapriya.n@xxxxxxxxx> napisał(a): > > > -----Original Message----- > > From: Łukasz Majczak <lma@xxxxxxxxxxxx> > > Sent: Monday, June 29, 2020 4:11 AM > > To: N, Harshapriya <harshapriya.n@xxxxxxxxx> > > Cc: alsa-devel@xxxxxxxxxxxxxxxx; Pierre-Louis Bossart <pierre- > > louis.bossart@xxxxxxxxxxxxxxx>; Jie Yang <yang.jie@xxxxxxxxxxxxxxx>; Radoslaw > > Biernacki <rad@xxxxxxxxxxxx>; Ross Zwisler <zwisler@xxxxxxxxxx>; linux- > > kernel@xxxxxxxxxxxxxxx; Liam Girdwood <liam.r.girdwood@xxxxxxxxxxxxxxx>; > > Bob Brandt <brndt@xxxxxxxxxx>; Marcin Wojtas <mw@xxxxxxxxxxxx>; Alex > > Levin <levinale@xxxxxxxxxxxx> > > Subject: Re: [PATCH v3] ASoC: Intel: kbl_rt5663_rt5514_max98927: Split > > be_hw_params_fixup function > > > > Hi Harsha, > > > > We would like to continue the work on this, could you please suggest the > > correct approach. > > > > Best regards, > > Lukasz > > > > czw., 21 maj 2020 o 20:10 Pierre-Louis Bossart <pierre- > > louis.bossart@xxxxxxxxxxxxxxx> napisał(a): > > > > > > > > > > > > On 5/21/20 12:30 PM, Łukasz Majczak wrote: > > > > Hi Pierre > > > > > > > > If you will take a look at the original kabylake_ssp_fixup() you > > > > will see that it is checking whether the related FE is "Kbl Audio > > > > Port", "Kbl Audio Headset Playback", "Kbl Audio Capture Port" or > > > > "Kbl Audio DMIC cap" - then for the first 3 cases it sets min/max > > > > channels to 2 while for the "Kbl DMIC cap" it can be 2 or 4, that's > > > > is why I'm trying to split this, but maybe I'm missing here something. > > > > > > I don't understand this code either. > > > > > > I believe the intent is that for all SSP1-RT5663 usages, we should use > > > > > > rate->min = rate->max = 48000; > > > chan->min = chan->max = 2; > > > snd_mask_none(fmt); > > > snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S24_LE); > > > > > > That is pretty easy to move to a dedicated ssp1 fixup. > > > > > > for SSP0, we have RT5514 for capture and max98927 for playback, but > > > the existing code does not explicitly deal with rate/channels/format > > > for all cases, so it's not clear what should happen. > > > > > > Harsha, can you help here? > Apologies for missing the email I had to respond to. > > SSP0 - has the speakers > SSP1 - has headset and dmic > For speakers and headsets its 48Khz, 2 ch and 24 bit and this setting is done based on the front-end dai > For speakers, however support only 16 bit, so we set it back to 16 bit > If the front end dai is dmic, then the channel is set to either 2 or 4 dmic_constraints. No other formats need to be set. > > All the SSP1 usages do not have the same parameters (as dmic is on SSP1 and its different as given above) > Most parameters are same for speakers and headset which are on different SSP. This is the reason we had a single fixup function. > > Is there a reason why the fixup function needs to be split? > > > > > > > > > > > > Best regards, > > > > Lukasz > > > > > > > > czw., 21 maj 2020 o 19:17 Pierre-Louis Bossart > > > > <pierre-louis.bossart@xxxxxxxxxxxxxxx> napisał(a): > > > >> > > > >> > > > >> > > > >> On 5/21/20 12:08 PM, Łukasz Majczak wrote: > > > >>>> > > > >>>> don't add a new dailink, this is not right. > > > >>>> > > > >>> Can you advise a better solution how to assign different fixup > > > >>> functions to mic and to speakers? I was looking at "dmic01" > > > >>> dailink in skl_nau88l25_max98357a.c as an example. > > > >> > > > >> I am not sure I follow. the DMICs are handled on a shared SSP, so > > > >> how would one set a different fixup? The word length have to be the same.
