Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Padmanabhan Rajanbabu" <p.rajanbabu@samsung.com>
To:     "'Mark Brown'" <broonie@kernel.org>
Cc:     <lgirdwood@gmail.com>, <robh+dt@kernel.org>,
        <krzysztof.kozlowski+dt@linaro.org>, <s.nawrocki@samsung.com>,
        <perex@perex.cz>, <tiwai@suse.com>, <pankaj.dubey@samsung.com>,
        <alim.akhtar@samsung.com>, <rcsekar@samsung.com>,
        <aswani.reddy@samsung.com>, <alsa-devel@alsa-project.org>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-samsung-soc@vger.kernel.org>
In-Reply-To: <Y2vlqatX7dfPJ3Zi@sirena.org.uk>
Subject: RE: [PATCH 2/6] ASoC: samsung: i2s: configure PSR from sound card
Date:   Tue, 3 Jan 2023 10:25:02 +0530
Message-ID: <025401d91f2f$8bf78a00$a3e69e00$@samsung.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Thread-Index: AQKFI9qd/qMOsyXBjKz4d8Q6MS4QvwJEXTuNARMAzQMCgN8f5gMWNkuwArAClEQCgEQWDwHlvOpzrLPsmiA=
Content-Language: en-in
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 105P
DLP-Filter: Pass
References: <20221014102151.108539-1-p.rajanbabu@samsung.com>
        <CGME20221014104857epcas5p2a275a1d606ca066227228d13bcf5b120@epcas5p2.samsung.com>
        <20221014102151.108539-3-p.rajanbabu@samsung.com>
        <Y0lPz91gbovAub9D@sirena.org.uk>
        <04a101d8e523$30804b80$9180e280$@samsung.com>
        <Y1KIT4nk7C8SQ45x@sirena.org.uk>
        <01ba01d8f332$44eb3810$cec1a830$@samsung.com>
        <Y2vlqatX7dfPJ3Zi@sirena.org.uk>
Precedence: bulk


> -----Original Message-----
> From: Mark Brown [mailto:broonie@kernel.org]
> Sent: 09 November 2022 11:09 PM
> To: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>
> Cc: lgirdwood@gmail.com; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com;
> perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com;
> alim.akhtar@samsung.com; rcsekar@samsung.com;
> aswani.reddy@samsung.com; alsa-devel@alsa-project.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-samsung-
> soc@vger.kernel.org
> Subject: Re: [PATCH 2/6] ASoC: samsung: i2s: configure PSR from sound card
> 
> 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?

Okay, I can understand the reason for de-linking these changes from the
machine
Driver. I'll post the v2 patches integrating the PSR changes into cpu dai
driver.

Thanks,
Padmanabhan R.