Re: [PATCH 2/6] ASoC: samsung: i2s: configure PSR from sound card

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3460 bytes --]

On Tue, Nov 08, 2022 at 10:53:40AM +0530, Padmanabhan Rajanbabu wrote:

> > > We can overcome this scenario to an extent if we can get a flexibility
> > > to Configure both PSR as well as RFS.

> > Why does it make sense for the machine driver to worry about this rather
> > than having the I2S controller driver configure the clock tree?

> _____           |                  __
> | 
> |         |	        |	             |   \
> |
> |CMU|	        |	             |     \
> |
> |FSD  |-  |---|-|--------->|       \        _________    _________
> |
> |___  |    |    | |op_clk0|         |      |               |     |
> |             |
> 	  |    | |	             |MUX|----|  PSR       |----|  RFS
> |--cdclk  |
> 	  |    | |              |         |      |_______|     |_______|
> |
> 	  |    | |--------->|        /
> |
> 	  |    |  op_clk1 |      /
> |
> 	  |    | 	             |_ /
> |
> 	  |    |___________________________________________|
> 	  |
> 	  |-----> To other FSD SoC Peripherals

> In FSD I2S, the clock source is not an independent source but a common clock
> source being shared by many IPs in the same domain.

> Changing the clock tree will impact other IPs in the domain as they are
> dependent on the same source for functionality.

I'm not sure I follow.  Perhaps your diagram is unclear but it looks
like PSR and RFS are both after a mux which appears to select which
clock is going to be used by the I2S controller?  The usage by other
clocks appears to be upstream of the mux and dividers.

> We can understand your point to bring the PSR changes under the I2S CPU DAI
> driver by adding a separate compatible and data for the FSD SoC. But If we
> take
> the example of existing sound cards such as sound/soc/samsung/tm2_wm5110.c,
> the op_clk is supplied via external audio pll to the controller and PLL
> configuration
> is taken care by the sound card. Since the configuration of PLL is more
> specific to
> the tm2 platform, it makes use of the flexibility of changing the RFS and
> BFS
> using the sysclk and clkdiv hooks provided by exynos7-i2s CPU DAI along with
> PLL tuning for precise sampling frequency.

The big reason for the clocking control (and indeed having a custom
machine driver) with the WM5110 is that it has multiple clocks to
control and a good deal of flexibility with placing them in clock
domains and so on which have power and performance impacts.  It's
frankly a bit unclear to me if the CPU I2S controller even needs the
bitclock configuring given that the clocks are being driven by the CODEC
there, but regardless it's not clear to me why the I2S controller would
need anything other than the input clock to the block configuring? 

> Similar to the above example, the choice of clock source under discussion is
> not a
> limitation of exynos7-i2s controller, but instead is a limitation on the FSD
> SoC.
> By using the proposed change, we can ensure that the exynos CPU DAI driver
> is
> giving additional hooks similar to existing hooks for BFS, RFS and CDCLK
> direction
> so that sound cards can use snd_soc_dai_set_sysclk and
> snd_soc_dai_set_clkdiv
> to customize the same.

I'm still not seeing anything that articulates why pushing the
configuration of the dividers within the block into the machine driver
solves a problem here.  Again, what's the upside to configuring clocks
that are purely within the block?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]