Thanks for the tip! Let me reformat the patch. wt., 8 wrz 2020 o 20:06 Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> napisał(a): > > > > On 9/8/20 12:42 PM, Radosław Biernacki wrote: > > Sorry for missing the response for so long. > > Somehow lost this thread in my mailbox. > > That happens to all of us! > > > śr., 6 maj 2020 o 00:04 Pierre-Louis Bossart > > <pierre-louis.bossart@xxxxxxxxxxxxxxx> napisał(a): > >> > >> > >>>>> This single fix address two issues on machines with nau88125: > >>>>> 1) Audio distortion, due to lack of required clock rate on MCLK line > >>>>> 2) Loud audible "pops" on headphones if there is no sysclk during nau8825 > >>>>> playback power up sequence > >>>>> > >>>>> Explanation for: > >>>>> 1) Due to Skylake HW limitation, MCLK pin can only output 24MHz clk > >>>>> rate (it can be only connected to XTAL parent clk). The BCLK pin > >>>>> can be driven by dividers and therefore FW is able to set it to rate > >>>>> required by chosen audio format. According to nau8825 datasheet, 256*FS > >>>>> sysclk gives the best audio quality and the only way to achieve this > >>>>> (taking into account the above limitations) its to regenerate the MCLK > >>>>> from BCLK on nau8825 side by FFL. Without required clk rate, audio is > >>>>> distorted by added harmonics. > >>>> > >>>> The BCLK is going to be a multiple of 50 * Fs due to clocking > >>>> restrictions. Can the codec regenerate a good-enough sysclk from this? > >>> > >>> According to Intel, silicon has a limitation, on SKL/KBL only clk_id = > >>> SKL_XTAL, .name = "xtal" is available for IO domain. > >>> As mentioned in the commit: > >>> MCLK is generated by using 24MHz Xtal directly or applying a divider > >>> (so no way of achieving the rate required by audio format). > >>> BCLK/FS is generated from 24MHz and uses dividers and additional > >>> padding bits are used to match the clock source. > >>> Next gen silicon has the possibility of using additional clock sources. > >>> > >>> Summing up, using MCLK from SKL to NAU88L25 is not an option. > >>> The only option we found is to use BCLK and regen the required clock > >>> rate by FLL on the NAU88l25 side. > >> > >> Right, this 24 MHz is a recurring problem. > >> But what I was asking was if the NAU8825 is fine working with e.g. a > >> 2.4MHz bit clock. i.e. with 25 bit slots or padding at the end of the frame? > > > > From our tests NAU8825 is working fine with these parameters. > > Also the output audio signal looks fine on the scope and FFT and there > > are no audible glitches. > > > >> > >>> > >>>>> > >>>>> 2) Currently Skylake does not output MCLK/FS when the back-end DAI op > >>>>> hw_param is called, so we cannot switch to MCLK/FS in hw_param. This > >>>>> patch reduces pop by letting nau8825 keep using its internal VCO clock > >>>>> during widget power up sequence, until SNDRV_PCM_TRIGGER_START when > >>>>> MCLK/FS is available. Once device resumes, the system will only enable > >>>>> power sequence for playback without doing hardware parameter, audio > >>>>> format, and PLL configure. In the mean time, the jack detecion sequence > >>>>> has changed PLL parameters and switched to internal clock. Thus, the > >>>>> playback signal distorted without correct PLL parameters. That is why > >>>>> we need to configure the PLL again in SNDRV_PCM_TRIGGER_RESUME case. > >>>> > >>>> IIRC the FS can be controlled with the clk_ api with the Skylake driver, > >>>> as done for some KBL platforms. Or is this not supported by the firmware > >>>> used by this machine? > >>> > >>> According to Ben, SKL had limitations in FW for managing the clk's > >>> back in the days. > >>> Can you point to the other driver you mention so we can cross check? > >> > >> There are two KBL drivers that control the SSP clocks from the machine > >> driver, but indeed I don't know if this would work on Firmware, it'd be > >> a question for Lech/Cezary. > >> > >> kbl_rt5663_max98927.c: ret = clk_prepare_enable(priv->mclk); > >> kbl_rt5663_max98927.c: ret = clk_prepare_enable(priv->sclk); > >> kbl_rt5663_rt5514_max98927.c: ret = > >> clk_prepare_enable(priv->mclk); > >> kbl_rt5663_rt5514_max98927.c: ret = > >> clk_prepare_enable(priv->sclk); > >> kbl_rt5663_rt5514_max98927.c: ret = > >> clk_prepare_enable(priv->mclk); > >> > > > > Czarek answered this and we got the same response from other Intel > > devs while consulting this change: > > FW cannot request a chosen rate (48k) for MCLK pin as it does not > > "align with what's present on SKL hw". > > > > The only way we found out for NAU8825 to cooperate at chosen rate with > > SKL HW is to regen the MCLK from BCLK by FLL as mentioned above. > > NHTL is used to set SSP0 (48k, 24/25 bit on 24MHz crystal). > > If I get all of this right, use of NHTL and HW "abilities" would > > explain why there is no call to change SSP from a machine driver. > > > > > > If all of this is ok I will send V3 with msleep() removed. > > Sounds good. > > You may want to simplify your commit message and just state what you > described, e.g. > > "Since 64xfs clocks cannot be generated, the NAU8825 is configured to > re-generate its system clock from the BCLK using the FLL. The link is > configured to use a 48kHz frame rate, and 24 bits in 25-bit slot. The > SSP configuration is extracted from NHLT settings and not dynamically > changed. Listening tests and measurements do not show any distortion or > issues". > > >
