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;
Feedback-ID: i8771445c:Fastmail
Date:   Mon, 26 Jun 2023 18:50:00 +0200
From:   Maxime Ripard <maxime@cerno.tech>
To:     Frank Oltmanns <frank@oltmanns.dev>
Cc:     Andre Przywara <andre.przywara@arm.com>,
        Chen-Yu Tsai <wens@csie.org>,
        Jernej Skrabec <jernej.skrabec@gmail.com>,
        Michael Turquette <mturquette@baylibre.com>,
        Roman Beranek <me@crly.cz>,
        Samuel Holland <samuel@sholland.org>,
        Stephen Boyd <sboyd@kernel.org>,
        linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-sunxi@lists.linux.dev
Subject: Re: [PATCH v2 1/2] clk: sunxi-ng: nkm: consider alternative parent
 rates when finding rate
Message-ID: <lagot6cwcgdmdel7ce73tbmiyfalrlt56l4rx2fi3ibpso37zi@y7rkhou4tclm>
References: <20230611090143.132257-1-frank@oltmanns.dev>
 <20230611090143.132257-2-frank@oltmanns.dev>
 <87edmh12s7.fsf@oltmanns.dev>
 <sfni3vehkhotsqrrirklhzrxzkcxzkq6nbtqokeab5if3jgm53@frh7z3iowsfe>
 <878rc7stuu.fsf@oltmanns.dev>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="u5p73wr2fp7f5ue6"
Content-Disposition: inline
In-Reply-To: <878rc7stuu.fsf@oltmanns.dev>
Precedence: bulk


--u5p73wr2fp7f5ue6
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sun, Jun 25, 2023 at 12:45:45PM +0200, Frank Oltmanns wrote:
> Hi Maxime,
>=20
> On 2023-06-12 at 14:31:21 +0200, Maxime Ripard <maxime@cerno.tech> wrote:
> > [[PGP Signed Part:Undecided]]
> > On Mon, Jun 12, 2023 at 10:51:52AM +0200, Frank Oltmanns wrote:
> >> > @@ -28,12 +68,17 @@ static unsigned long ccu_nkm_find_best(unsigned =
long parent, unsigned long rate,
> >> >  			for (_m =3D nkm->min_m; _m <=3D nkm->max_m; _m++) {
> >>
> >> According to the manual M/N has to be <=3D 3. Therefore we need a
> >> different maximum value for the _m-for-loop:
> >>
> >>         unsigned long max_m =3D min(3 * _n, nkm->max_m);
> >>         for (_m =3D nkm->min_m; _m <=3D max_m; _m++) {
> >>
> >> I suggest that I add an optional member max_mn_ratio to the structs
> >> ccu_nkm and _ccu_nkm. Optional meaning: Ignore if 0.
> >
> > Which workload is affected by this restriction?
> >
>=20
> Firstly, the restriction increases the minimum rate that pll-mipi of the
> A64 SoC can use. The rate off pll-mipi is
>         pll-video0 * K * N / M
>=20
> The Allwinner's user manual ([1], p.94) states that the maximum ratio of
> M/N (note how numerator and denominator changed) is 3. So, looking back
> to the original formula, the N / M part can be at most 1/3. That
> effectively limits the minimum rate that pll-mipi can provide to
>         min(pll-video0) * 2 * 1 / 3
>=20
> The minimum rate of pll-video0 is 192 MHz, i.e., the minimum rate for
> pll-mipi becomes 128 MHz. Without the restriction, the minimum rate
> currently is 24 MHz. It is my (albeit limited) understanding, that is no
> real limitation because no panel would request such low rates. I should
> also mention that Allwinner states in the user manual ([1], p. 94) that
> the rate must be in the 500 MHz - 1.4 GHz range.
>=20
> Secondly, it decreases the number of options for M for all N <=3D 6.
> Therefore it reduces the number of meaningful NKM combinations from 275
> (without the restriction) to 238. (With meaningful combinations, I mean
> the combinations that result in a different rate for pll-mipi, e.g.,
> K=3D2, M=3D1, N=3D2 is the same as K=3D4, M=3D1, N=3D1). The consequence =
is that the
> precision of pll-mipi is slightly reduced. Note, however, that this loss
> of precision is more than offset by the option that pll-mipi can now
> "freely" choose its parent rate.
>=20
> In conclusion, I don't see any real world limitation that this
> restriction introduces.

If we want to go that way, I'd rather use a function that validates
whether or not the current set of parameter is valid.

That way, we can express pretty much any constraints without
special-casing the main structure too much.

Maxime

--u5p73wr2fp7f5ue6
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCZJnBuAAKCRDj7w1vZxhR
xb80AQCpuUfjRsZDj4oX2zh/hfNYEzytGHWy4prrDYI0zK6udgD/eFjJYhGo55Cz
pUxEKq7T3qrcxW6icqzfjmw8A+IR4gY=
=ApE4
-----END PGP SIGNATURE-----

--u5p73wr2fp7f5ue6--