On Tue, Mar 21, 2017 at 04:05:15PM +0200, Daniel Baluta wrote: > On Tue, Mar 21, 2017 at 2:52 PM, Charles Keepax > <ckeepax@xxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote: > > On Tue, Mar 21, 2017 at 12:09:36PM +0200, Daniel Baluta wrote: > >> WM8960 derives bit clock from sysclock using BCLKDIV[3:0] of R8 > >> clocking register (See WM8960 datasheet, page 71). > >> > >> There are use cases, like this: > >> aplay -Dhw:0,0 -r 48000 -c 1 -f S20_3LE -t raw audio48k20b_3LE1c.pcm > >> > >> where no BCLKDIV applied to sysclock can give us the exact requested > >> bitclk, so driver fails to configure clocking and aplay fails to run. > >> > >> Fix this by relaxing bitclk computation, so that when no exact value > >> can be derived from sysclk pick the closest value greater than > >> expected bitclk. > >> > >> Suggested-by: Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxxxxxxxx> > >> Signed-off-by: Daniel Baluta <daniel.baluta@xxxxxxx> > >> --- > >> Changes since v1: > >> * use a marker to check if a match is found > >> * didn't removed PLL as Charles suggested because there is > >> a special PLL mode which explictly uses PLL. We could start > >> a discussion on not using PLL when deriving bitclk, but this > >> is to be done in another patch. > >> > > > > Could you elaborate on this a little more am I not sure I follow > > 100%? There is a mode which explictly requires the PLL to be used > > (WM8960_SYSCLK_PLL) but in that case your wm8960_configure_sysclk > > code will not be called so I don't see what is causing that to have > > an effect on this patch? > > My doubt is, what happens if wm8960_configure_clocking is called with > wm8960->clk_id = WM8960_SYSCLK_PLL and we remove the PLL > as suggested. I wasn't suggesting removing the PLL just that if we find a "relaxed match" we don't need to then check the PLL for a better match, as I suspect that a slightly higher than needed bit clock has less power/performance impact than firing up the PLL. Which removes the need to differenciate between a relaxed and bang on match in wm8960_configure_sysclk and means you don't have to do the caching the values across the PLL code that you do now. Thanks, Charles _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
