Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1605577 > unrolled thread

Re: [PATCH v2 2/2] ASoC: codec: wm8960: Relax bit clock computation

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 2/2] ASoC: codec: wm8960: Relax bit clock computation Charles Keepax <ckeepax@opensource.wolfsonmicro.com> - 2017-03-21 14:00 +0100
    Re: [alsa-devel] [PATCH v2 2/2] ASoC: codec: wm8960: Relax bit clock computation Daniel Baluta <daniel.baluta@gmail.com> - 2017-03-21 15:20 +0100
      Re: [alsa-devel] [PATCH v2 2/2] ASoC: codec: wm8960: Relax bit clock  computation Charles Keepax <ckeepax@opensource.wolfsonmicro.com> - 2017-03-21 15:30 +0100
        Re: [alsa-devel] [PATCH v2 2/2] ASoC: codec: wm8960: Relax bit clock computation Daniel Baluta <daniel.baluta@gmail.com> - 2017-03-21 15:30 +0100
          Re: [alsa-devel] [PATCH v2 2/2] ASoC: codec: wm8960: Relax bit clock  computation Charles Keepax <ckeepax@opensource.wolfsonmicro.com> - 2017-03-21 15:40 +0100

#1605577 — Re: [PATCH v2 2/2] ASoC: codec: wm8960: Relax bit clock computation

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@opensource.wolfsonmicro.com>
> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> ---
> 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?

Thanks,
Charles

[toc] | [next] | [standalone]

#1605645 — Re: [alsa-devel] [PATCH v2 2/2] ASoC: codec: wm8960: Relax bit clock computation

On Tue, Mar 21, 2017 at 2:52 PM, Charles Keepax
<ckeepax@opensource.wolfsonmicro.com> 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@opensource.wolfsonmicro.com>
>> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
>> ---
>> 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.

Is this possible even possible?

Anyhow, I noticed that so far wm8960->clk_id is never set to
WM8960_SYSCLK_PLL :).

So, my proposal is to merge this patch which improves the existing code and deal
with PLL fallback in a separate patch later.

I am afraid of touching things which I don't understand how they work :D.

thanks,
Daniel.

[toc] | [prev] | [next] | [standalone]

#1605648 — Re: [alsa-devel] [PATCH v2 2/2] ASoC: codec: wm8960: Relax bit clock computation

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@opensource.wolfsonmicro.com> 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@opensource.wolfsonmicro.com>
> >> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> >> ---
> >> 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

[toc] | [prev] | [next] | [standalone]

#1605649 — Re: [alsa-devel] [PATCH v2 2/2] ASoC: codec: wm8960: Relax bit clock computation

On Tue, Mar 21, 2017 at 4:20 PM, Charles Keepax
<ckeepax@opensource.wolfsonmicro.com> wrote:
> 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@opensource.wolfsonmicro.com> 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@opensource.wolfsonmicro.com>
>> >> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
>> >> ---
>> >> 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.

Oh, I see. So we still use the PLL when no exact or relaxed match
is found.

[toc] | [prev] | [next] | [standalone]

#1605655 — Re: [alsa-devel] [PATCH v2 2/2] ASoC: codec: wm8960: Relax bit clock computation

On Tue, Mar 21, 2017 at 04:25:40PM +0200, Daniel Baluta wrote:
> On Tue, Mar 21, 2017 at 4:20 PM, Charles Keepax
> <ckeepax@opensource.wolfsonmicro.com> wrote:
> > 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@opensource.wolfsonmicro.com> wrote:
> >> > On Tue, Mar 21, 2017 at 12:09:36PM +0200, Daniel Baluta wrote:
> >> >>       * 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.
> 
> Oh, I see. So we still use the PLL when no exact or relaxed match
> is found.

Yeah exactly or in the case that it is requested directly.

Thanks,
Charles

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web