From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0F17AC83F12 for ; Mon, 28 Aug 2023 14:18:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231908AbjH1ORr (ORCPT ); Mon, 28 Aug 2023 10:17:47 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45216 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231695AbjH1ORW (ORCPT ); Mon, 28 Aug 2023 10:17:22 -0400 Received: from mout-p-102.mailbox.org (mout-p-102.mailbox.org [80.241.56.152]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BEB71E7; Mon, 28 Aug 2023 07:17:15 -0700 (PDT) Received: from smtp2.mailbox.org (smtp2.mailbox.org [10.196.197.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-384) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-102.mailbox.org (Postfix) with ESMTPS id 4RZCLH3MqCz9scf; Mon, 28 Aug 2023 16:17:11 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oltmanns.dev; s=MBO0001; t=1693232231; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=s+8pHBapEaGXy1jvUtl21Qsxu+aNw3OPEPZ0IC0tfoU=; b=KhQU8OSWRXoTxFuCN9bgCewsVz5vGYuF5TgU4KK0f6QdpC0tD/0Zeh3CgehSpAO7HpQdNP FG62G59jyF1WWcPDprdzP9XkXI8VYyKV1zsRwG+JjDA0qRGMr+13zoffDYByQlinby7kNu kDrzCtahTq4Du7tkBCwDVbWC56w7TbSscOymVeYii8G0QF3qGbbxIyA0Ha5jc3iFajYfpb v6r+Zu/iPhopSb3cHyeWRYgC74qzjJpPJFjAmHuB06sIHe0us7z+G2RQ1hQmDbc4FXO+mh gG1N92rlyJZat0jtul2Va4h7G2or2X255lmv6qSdDl+WyYPLts7O0P9OyRk0Dg== References: <20230825-pll-mipi_keep_rate-v1-0-35bc43570730@oltmanns.dev> <87ledzqhwx.fsf@oltmanns.dev> From: Frank Oltmanns To: Maxime Ripard Cc: Michael Turquette , Stephen Boyd , Chen-Yu Tsai , Jernej Skrabec , Samuel Holland , David Airlie , Daniel Vetter , Ondrej Jirman , Icenowy Zheng , linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev, dri-devel@lists.freedesktop.org, Icenowy Zheng Subject: Re: [PATCH 0/3] Make Allwinner A64's pll-mipi keep its rate when parent rate changes In-reply-to: Date: Mon, 28 Aug 2023 16:17:08 +0200 Message-ID: <87cyz7uu8r.fsf@oltmanns.dev> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-clk@vger.kernel.org On 2023-08-28 at 10:04:51 +0200, Maxime Ripard wrote: > On Fri, Aug 25, 2023 at 05:07:58PM +0200, Frank Oltmanns wrote: >> Thank you for your feedback, Maxime! >> >> On 2023-08-25 at 10:13:53 +0200, Maxime Ripard wrot= e: >> > [[PGP Signed Part:Undecided]] >> > Hi, >> > >> > On Fri, Aug 25, 2023 at 07:36:36AM +0200, Frank Oltmanns wrote: >> >> I would like to make the Allwinner A64's pll-mipi to keep its rate wh= en >> >> its parent's (pll-video0) rate changes. Keeping pll-mipi's rate is >> >> required, to let the A64 drive both an LCD and HDMI display at the sa= me >> >> time, because both have pll-video0 as an ancestor. >> >> >> >> PATCH 1 adds this functionality as a feature into the clk framework (= new >> >> flag: CLK_KEEP_RATE). >> >> >> >> Cores that use this flag, store a rate as req_rate when it or one of = its >> >> descendants requests a new rate. >> >> >> >> That rate is then restored in the clk_change_rate recursion, which wa= lks >> >> through the tree. It will reach the flagged core (e.g. pll-mipi) after >> >> the parent's rate (e.g. pll-video0) has already been set to the new >> >> rate. It will then call determine_rate (which requests the parent's >> >> current, i.e. new, rate) to determine a rate that is close to the >> >> flagged core's previous rate. Afterward it will re-calculate the rates >> >> for the flagged core's subtree. >> > >> > I don't think it's the right way forward. It makes the core logic more >> > complicated, for something that is redundant with the notifiers >> > mechanism that has been the go-to for that kind of things so far. >> >> Yeah, that was my initial idea as well. But I couldn't get it to work. >> See details below. >> >> Do you have an example of a clock that restores its previous rate after >> the parent rate has changed? I've looked left and right, but to me it >> seems that notifiers are mainly used for setting clocks into some kind >> of "safe mode" prior to the rate change. Examples: >> >> sunxi-ng: >> https://elixir.bootlin.com/linux/v6.4.11/source/drivers/clk/sunxi-ng/ccu= _mux.c#L273 >> https://elixir.bootlin.com/linux/v6.4.11/source/drivers/clk/sunxi-ng/ccu= _common.c#L60 >> >> but also others: >> https://elixir.bootlin.com/linux/v6.4.11/source/drivers/clk/at91/clk-mas= ter.c#L248 >> https://elixir.bootlin.com/linux/v6.4.11/source/drivers/clk/meson/meson8= b.c#L3755 >> https://elixir.bootlin.com/linux/v6.4.11/source/drivers/clk/qcom/clk-cpu= -8996.c#L546 > > There's examples for phases and parents, but not for rates afaics. We > shouldn't behave any differently though. > >> > It's not really obvious to me why the notifiers don't work there. >> > >> >> This work is inspired by an out-of-tree patchset [1] [2] [3]. >> >> Unfortunately, the patchset uses clk_set_rate() in a notifier callbac= k, >> >> which the following comment on clk_notifier_register() forbids: "The >> >> callbacks associated with the notifier must not re-enter into the clk >> >> framework by calling any top-level clk APIs." [4] Furthermore, that >> >> out-of-tree patchset no longer works with the current linux-next, >> >> because setting pll-mipi is now also resetting pll-video0 [5]. >> > >> > Is it because of the "The callbacks associated with the notifier must >> > not re-enter into the clk framework by calling any top-level clk APIs." >> > comment? >> >> I don't think that's the reason. > > I'm not sure I follow you there. How can we find a solution to a problem > you don't know about or can't know for sure? I was hoping that the discussion here would give me some clues (and it does). You have already explained, that the issue is the locks. I'm still confused why Icenowy's patches work. They use clk_set_rate() in a notifier callback and despite that they work (up until kernel 6.5). The only thing that has changed here (that I'm aware of), is that pll-mipi now sets the parent rate in clk-next. >> I'm fairly certain that the problem is, that pll-mipi tries to set the >> parent rate. Maybe it should check if the parent is locked, before >> determining a rate that requires the parent rate to change. =F0=9F=A4=94 > > Why would the clock framework documentation mention an issue that only > arises with a single clock on a single SoC? No, sorry, that's not what I said or meant. I was wondering if ccu_nkm_determine_rate should check if the parent rate has no exclusive lock, before assuming it can change the parent rate, so that Icenowy's patches still work. > That comment in the clock framework you linked to clearly stated that > you can't use a top-level clock function in a notifier, and that's > because of the locking. Yes, it does. And that's why I thought that calling clk_set_rate() in the notifier callback was never the right choice. > If it's not what you're trying to fix, then I'd really like to know what > issue you're trying to fix *in the framework* (so, not on the pll-mipi > clock, or the A64). I'm not trying to "fix" anything in the framework in the sense that the framework has a bug. I propose to add a new feature to the framework, so that I can extend pll-mipi, so that the A64 can drive both an LCD and HDMI at the same time. >> Currently, it only calls clk_hw_can_set_rate_parent() which only >> checks the flag, but does not check if it is really possible to change >> the parent's rate. >> >> Regardless, please don't prematurely dismiss my proposal. It has the >> advantage that it is not specific for sunxi-ng, but could be used for >> other drivers as well. > > Just like the two solutions I provided. > >> Maybe there other instances of exclusive locks today where the >> CLK_KEEP_RATE flag might work equally well. =F0=9F=A4=B7 > > If exclusive locks work equally well, why would we need CLK_KEEP_RATE? As I wrote in my other mail. The word "keep" was apparently a bad choice. Maybe CLK_RESTORE_RATE_CLOSELY? The difference is that an exclusive lock prevents other clocks from changing the parent, whereas "CLK_KEEP_RATE" (sic) allows those changes to happen and deal with it by restoring the previous rate as closely as possible after the parent rate changed. >> > If so, I think the thing we should emphasize is that it's about *any >> > top-level clk API*, as in clk_set_rate() or clk_set_parent(). >> > >> > The issue is that any consumer-facing API is taking the clk_prepare lo= ck >> > and thus we would have reentrancy. But we're a provider there, and none >> > of the clk_hw_* functions are taking that lock. Neither do our own fun= ction. >> > >> > So we could call in that notifier our set_rate callback directly, or we >> > could create a clk_hw_set_rate() function. >> > >> > The first one will create cache issue between the actual rate that the >> > common clock framework is running and the one we actually enforced, but >> > we could create a function to flush the CCF cache. >> > >> > The second one is probably simpler. >> >> I'm probably missing something, because I don't think this would work. >> For reference, this is our tree: >> >> pll-video0 >> hdmi-phy-clk >> hdmi >> tcon1 >> pll-mipi >> tcon0 >> tcon-data-clock >> >> When pll-video0's rate is changed (e.g. because a HDMI monitor is >> plugged in), the rates of the complete subtree for pll-video0 are >> recalculated, including tcon0 and tcon-data-clock. The rate of tcon0 is >> based on the rate that was recalculated for pll-mipi, which - in turn - >> was of course recalculated based on the pll-video0's new rate. These >> values are stored by the clk framework in a private struct. They are >> calculated before actually performing any rate changes. >> >> So, if a notifier sets pll-mipi's rate to something else than was >> previously recalculated, the clk framework would still try to set tcon0 >> to the value that it previously calculated. >> >> So, we would have to recalculate pll-mipi's subtree after changing its >> rate (that's what PATCH 1 is doing). > > Then we should make that function I was telling you about deal with all > this. See my other mail. I mis-remembered. That is already available via the CLK_RECALC_NEW_RATES flag. Best regards, Frank > > Maxime From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id DB65CC83F12 for ; Mon, 28 Aug 2023 14:17:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date: In-reply-to:Subject:Cc:To:From:References:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=pkn0IUfku0ZswkrW2JZpOulEciLunkxuF2oeTrhpjOw=; b=bNsmAdp8znR6R4rdSbTMwizoUQ cBpDNw7i0I5vGkR0BnTFyXG/ITZrctVAyAVn0rueiFNFxNWYCR3m2JEIepUlJni4bLwiCz/+GuBgf eATD6W4a0iPr+xyspZGxMWv26RxSLzIZ3f8AubwLwHU/VkD0jxyrwxKdweKIFKMpFOnarP36fdvsI AsyIvQYhP0GB2nR4/tzVIkIUZNdeNXzw/IGy1wYwWvaNHC+2qKp4AtSNBcvOU9/5B7cQIwlhR2mzJ ZgNn8QswZwubMZ/t6tyW+0dV9RV1oSe2RT8Kv01Mw+ynNVtL2W1CWA0NaMVik7xw60cYNRVcu5Ab5 y8E8kUTg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qad3S-009hZ0-0M; Mon, 28 Aug 2023 14:17:22 +0000 Received: from mout-p-102.mailbox.org ([2001:67c:2050:0:465::102]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qad3O-009hYD-2d for linux-arm-kernel@lists.infradead.org; Mon, 28 Aug 2023 14:17:20 +0000 Received: from smtp2.mailbox.org (smtp2.mailbox.org [10.196.197.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-384) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-102.mailbox.org (Postfix) with ESMTPS id 4RZCLH3MqCz9scf; Mon, 28 Aug 2023 16:17:11 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oltmanns.dev; s=MBO0001; t=1693232231; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=s+8pHBapEaGXy1jvUtl21Qsxu+aNw3OPEPZ0IC0tfoU=; b=KhQU8OSWRXoTxFuCN9bgCewsVz5vGYuF5TgU4KK0f6QdpC0tD/0Zeh3CgehSpAO7HpQdNP FG62G59jyF1WWcPDprdzP9XkXI8VYyKV1zsRwG+JjDA0qRGMr+13zoffDYByQlinby7kNu kDrzCtahTq4Du7tkBCwDVbWC56w7TbSscOymVeYii8G0QF3qGbbxIyA0Ha5jc3iFajYfpb v6r+Zu/iPhopSb3cHyeWRYgC74qzjJpPJFjAmHuB06sIHe0us7z+G2RQ1hQmDbc4FXO+mh gG1N92rlyJZat0jtul2Va4h7G2or2X255lmv6qSdDl+WyYPLts7O0P9OyRk0Dg== References: <20230825-pll-mipi_keep_rate-v1-0-35bc43570730@oltmanns.dev> <87ledzqhwx.fsf@oltmanns.dev> From: Frank Oltmanns To: Maxime Ripard Cc: Michael Turquette , Stephen Boyd , Chen-Yu Tsai , Jernej Skrabec , Samuel Holland , David Airlie , Daniel Vetter , Ondrej Jirman , Icenowy Zheng , linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev, dri-devel@lists.freedesktop.org, Icenowy Zheng Subject: Re: [PATCH 0/3] Make Allwinner A64's pll-mipi keep its rate when parent rate changes In-reply-to: Date: Mon, 28 Aug 2023 16:17:08 +0200 Message-ID: <87cyz7uu8r.fsf@oltmanns.dev> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230828_071718_997768_DA84195A X-CRM114-Status: GOOD ( 54.37 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Ck9uIDIwMjMtMDgtMjggYXQgMTA6MDQ6NTEgKzAyMDAsIE1heGltZSBSaXBhcmQgPG1yaXBhcmRA a2VybmVsLm9yZz4gd3JvdGU6Cj4gT24gRnJpLCBBdWcgMjUsIDIwMjMgYXQgMDU6MDc6NThQTSAr MDIwMCwgRnJhbmsgT2x0bWFubnMgd3JvdGU6Cj4+IFRoYW5rIHlvdSBmb3IgeW91ciBmZWVkYmFj aywgTWF4aW1lIQo+Pgo+PiBPbiAyMDIzLTA4LTI1IGF0IDEwOjEzOjUzICswMjAwLCBNYXhpbWUg UmlwYXJkIDxtcmlwYXJkQGtlcm5lbC5vcmc+IHdyb3RlOgo+PiA+IFtbUEdQIFNpZ25lZCBQYXJ0 OlVuZGVjaWRlZF1dCj4+ID4gSGksCj4+ID4KPj4gPiBPbiBGcmksIEF1ZyAyNSwgMjAyMyBhdCAw NzozNjozNkFNICswMjAwLCBGcmFuayBPbHRtYW5ucyB3cm90ZToKPj4gPj4gSSB3b3VsZCBsaWtl IHRvIG1ha2UgdGhlIEFsbHdpbm5lciBBNjQncyBwbGwtbWlwaSB0byBrZWVwIGl0cyByYXRlIHdo ZW4KPj4gPj4gaXRzIHBhcmVudCdzIChwbGwtdmlkZW8wKSByYXRlIGNoYW5nZXMuIEtlZXBpbmcg cGxsLW1pcGkncyByYXRlIGlzCj4+ID4+IHJlcXVpcmVkLCB0byBsZXQgdGhlIEE2NCBkcml2ZSBi b3RoIGFuIExDRCBhbmQgSERNSSBkaXNwbGF5IGF0IHRoZSBzYW1lCj4+ID4+IHRpbWUsIGJlY2F1 c2UgYm90aCBoYXZlIHBsbC12aWRlbzAgYXMgYW4gYW5jZXN0b3IuCj4+ID4+Cj4+ID4+IFBBVENI IDEgYWRkcyB0aGlzIGZ1bmN0aW9uYWxpdHkgYXMgYSBmZWF0dXJlIGludG8gdGhlIGNsayBmcmFt ZXdvcmsgKG5ldwo+PiA+PiBmbGFnOiBDTEtfS0VFUF9SQVRFKS4KPj4gPj4KPj4gPj4gQ29yZXMg dGhhdCB1c2UgdGhpcyBmbGFnLCBzdG9yZSBhIHJhdGUgYXMgcmVxX3JhdGUgd2hlbiBpdCBvciBv bmUgb2YgaXRzCj4+ID4+IGRlc2NlbmRhbnRzIHJlcXVlc3RzIGEgbmV3IHJhdGUuCj4+ID4+Cj4+ ID4+IFRoYXQgcmF0ZSBpcyB0aGVuIHJlc3RvcmVkIGluIHRoZSBjbGtfY2hhbmdlX3JhdGUgcmVj dXJzaW9uLCB3aGljaCB3YWxrcwo+PiA+PiB0aHJvdWdoIHRoZSB0cmVlLiBJdCB3aWxsIHJlYWNo IHRoZSBmbGFnZ2VkIGNvcmUgKGUuZy4gcGxsLW1pcGkpIGFmdGVyCj4+ID4+IHRoZSBwYXJlbnQn cyByYXRlIChlLmcuIHBsbC12aWRlbzApIGhhcyBhbHJlYWR5IGJlZW4gc2V0IHRvIHRoZSBuZXcK Pj4gPj4gcmF0ZS4gSXQgd2lsbCB0aGVuIGNhbGwgZGV0ZXJtaW5lX3JhdGUgKHdoaWNoIHJlcXVl c3RzIHRoZSBwYXJlbnQncwo+PiA+PiBjdXJyZW50LCBpLmUuIG5ldywgcmF0ZSkgdG8gZGV0ZXJt aW5lIGEgcmF0ZSB0aGF0IGlzIGNsb3NlIHRvIHRoZQo+PiA+PiBmbGFnZ2VkIGNvcmUncyBwcmV2 aW91cyByYXRlLiBBZnRlcndhcmQgaXQgd2lsbCByZS1jYWxjdWxhdGUgdGhlIHJhdGVzCj4+ID4+ IGZvciB0aGUgZmxhZ2dlZCBjb3JlJ3Mgc3VidHJlZS4KPj4gPgo+PiA+IEkgZG9uJ3QgdGhpbmsg aXQncyB0aGUgcmlnaHQgd2F5IGZvcndhcmQuIEl0IG1ha2VzIHRoZSBjb3JlIGxvZ2ljIG1vcmUK Pj4gPiBjb21wbGljYXRlZCwgZm9yIHNvbWV0aGluZyB0aGF0IGlzIHJlZHVuZGFudCB3aXRoIHRo ZSBub3RpZmllcnMKPj4gPiBtZWNoYW5pc20gdGhhdCBoYXMgYmVlbiB0aGUgZ28tdG8gZm9yIHRo YXQga2luZCBvZiB0aGluZ3Mgc28gZmFyLgo+Pgo+PiBZZWFoLCB0aGF0IHdhcyBteSBpbml0aWFs IGlkZWEgYXMgd2VsbC4gQnV0IEkgY291bGRuJ3QgZ2V0IGl0IHRvIHdvcmsuCj4+IFNlZSBkZXRh aWxzIGJlbG93Lgo+Pgo+PiBEbyB5b3UgaGF2ZSBhbiBleGFtcGxlIG9mIGEgY2xvY2sgdGhhdCBy ZXN0b3JlcyBpdHMgcHJldmlvdXMgcmF0ZSBhZnRlcgo+PiB0aGUgcGFyZW50IHJhdGUgaGFzIGNo YW5nZWQ/IEkndmUgbG9va2VkIGxlZnQgYW5kIHJpZ2h0LCBidXQgdG8gbWUgaXQKPj4gc2VlbXMg dGhhdCBub3RpZmllcnMgYXJlIG1haW5seSB1c2VkIGZvciBzZXR0aW5nIGNsb2NrcyBpbnRvIHNv bWUga2luZAo+PiBvZiAic2FmZSBtb2RlIiBwcmlvciB0byB0aGUgcmF0ZSBjaGFuZ2UuIEV4YW1w bGVzOgo+Pgo+PiBzdW54aS1uZzoKPj4gaHR0cHM6Ly9lbGl4aXIuYm9vdGxpbi5jb20vbGludXgv djYuNC4xMS9zb3VyY2UvZHJpdmVycy9jbGsvc3VueGktbmcvY2N1X211eC5jI0wyNzMKPj4gaHR0 cHM6Ly9lbGl4aXIuYm9vdGxpbi5jb20vbGludXgvdjYuNC4xMS9zb3VyY2UvZHJpdmVycy9jbGsv c3VueGktbmcvY2N1X2NvbW1vbi5jI0w2MAo+Pgo+PiBidXQgYWxzbyBvdGhlcnM6Cj4+IGh0dHBz Oi8vZWxpeGlyLmJvb3RsaW4uY29tL2xpbnV4L3Y2LjQuMTEvc291cmNlL2RyaXZlcnMvY2xrL2F0 OTEvY2xrLW1hc3Rlci5jI0wyNDgKPj4gaHR0cHM6Ly9lbGl4aXIuYm9vdGxpbi5jb20vbGludXgv djYuNC4xMS9zb3VyY2UvZHJpdmVycy9jbGsvbWVzb24vbWVzb244Yi5jI0wzNzU1Cj4+IGh0dHBz Oi8vZWxpeGlyLmJvb3RsaW4uY29tL2xpbnV4L3Y2LjQuMTEvc291cmNlL2RyaXZlcnMvY2xrL3Fj b20vY2xrLWNwdS04OTk2LmMjTDU0Ngo+Cj4gVGhlcmUncyBleGFtcGxlcyBmb3IgcGhhc2VzIGFu ZCBwYXJlbnRzLCBidXQgbm90IGZvciByYXRlcyBhZmFpY3MuIFdlCj4gc2hvdWxkbid0IGJlaGF2 ZSBhbnkgZGlmZmVyZW50bHkgdGhvdWdoLgo+Cj4+ID4gSXQncyBub3QgcmVhbGx5IG9idmlvdXMg dG8gbWUgd2h5IHRoZSBub3RpZmllcnMgZG9uJ3Qgd29yayB0aGVyZS4KPj4gPgo+PiA+PiBUaGlz IHdvcmsgaXMgaW5zcGlyZWQgYnkgYW4gb3V0LW9mLXRyZWUgcGF0Y2hzZXQgWzFdIFsyXSBbM10u Cj4+ID4+IFVuZm9ydHVuYXRlbHksIHRoZSBwYXRjaHNldCB1c2VzIGNsa19zZXRfcmF0ZSgpIGlu IGEgbm90aWZpZXIgY2FsbGJhY2ssCj4+ID4+IHdoaWNoIHRoZSBmb2xsb3dpbmcgY29tbWVudCBv biBjbGtfbm90aWZpZXJfcmVnaXN0ZXIoKSBmb3JiaWRzOiAiVGhlCj4+ID4+IGNhbGxiYWNrcyBh c3NvY2lhdGVkIHdpdGggdGhlIG5vdGlmaWVyIG11c3Qgbm90IHJlLWVudGVyIGludG8gdGhlIGNs awo+PiA+PiBmcmFtZXdvcmsgYnkgY2FsbGluZyBhbnkgdG9wLWxldmVsIGNsayBBUElzLiIgWzRd IEZ1cnRoZXJtb3JlLCB0aGF0Cj4+ID4+IG91dC1vZi10cmVlIHBhdGNoc2V0IG5vIGxvbmdlciB3 b3JrcyB3aXRoIHRoZSBjdXJyZW50IGxpbnV4LW5leHQsCj4+ID4+IGJlY2F1c2Ugc2V0dGluZyBw bGwtbWlwaSBpcyBub3cgYWxzbyByZXNldHRpbmcgcGxsLXZpZGVvMCBbNV0uCj4+ID4KPj4gPiBJ cyBpdCBiZWNhdXNlIG9mIHRoZSAiVGhlIGNhbGxiYWNrcyBhc3NvY2lhdGVkIHdpdGggdGhlIG5v dGlmaWVyIG11c3QKPj4gPiBub3QgcmUtZW50ZXIgaW50byB0aGUgY2xrIGZyYW1ld29yayBieSBj YWxsaW5nIGFueSB0b3AtbGV2ZWwgY2xrIEFQSXMuIgo+PiA+IGNvbW1lbnQ/Cj4+Cj4+IEkgZG9u J3QgdGhpbmsgdGhhdCdzIHRoZSByZWFzb24uCj4KPiBJJ20gbm90IHN1cmUgSSBmb2xsb3cgeW91 IHRoZXJlLiBIb3cgY2FuIHdlIGZpbmQgYSBzb2x1dGlvbiB0byBhIHByb2JsZW0KPiB5b3UgZG9u J3Qga25vdyBhYm91dCBvciBjYW4ndCBrbm93IGZvciBzdXJlPwoKSSB3YXMgaG9waW5nIHRoYXQg dGhlIGRpc2N1c3Npb24gaGVyZSB3b3VsZCBnaXZlIG1lIHNvbWUgY2x1ZXMgKGFuZCBpdApkb2Vz KS4gWW91IGhhdmUgYWxyZWFkeSBleHBsYWluZWQsIHRoYXQgdGhlIGlzc3VlIGlzIHRoZSBsb2Nr cy4gSSdtCnN0aWxsIGNvbmZ1c2VkIHdoeSBJY2Vub3d5J3MgcGF0Y2hlcyB3b3JrLiBUaGV5IHVz ZSBjbGtfc2V0X3JhdGUoKSBpbiBhCm5vdGlmaWVyIGNhbGxiYWNrIGFuZCBkZXNwaXRlIHRoYXQg dGhleSB3b3JrICh1cCB1bnRpbCBrZXJuZWwgNi41KS4gVGhlCm9ubHkgdGhpbmcgdGhhdCBoYXMg Y2hhbmdlZCBoZXJlICh0aGF0IEknbSBhd2FyZSBvZiksIGlzIHRoYXQgcGxsLW1pcGkKbm93IHNl dHMgdGhlIHBhcmVudCByYXRlIGluIGNsay1uZXh0LgoKPj4gSSdtIGZhaXJseSBjZXJ0YWluIHRo YXQgdGhlIHByb2JsZW0gaXMsIHRoYXQgcGxsLW1pcGkgdHJpZXMgdG8gc2V0IHRoZQo+PiBwYXJl bnQgcmF0ZS4gTWF5YmUgaXQgc2hvdWxkIGNoZWNrIGlmIHRoZSBwYXJlbnQgaXMgbG9ja2VkLCBi ZWZvcmUKPj4gZGV0ZXJtaW5pbmcgYSByYXRlIHRoYXQgcmVxdWlyZXMgdGhlIHBhcmVudCByYXRl IHRvIGNoYW5nZS4g8J+klAo+Cj4gV2h5IHdvdWxkIHRoZSBjbG9jayBmcmFtZXdvcmsgZG9jdW1l bnRhdGlvbiBtZW50aW9uIGFuIGlzc3VlIHRoYXQgb25seQo+IGFyaXNlcyB3aXRoIGEgc2luZ2xl IGNsb2NrIG9uIGEgc2luZ2xlIFNvQz8KCk5vLCBzb3JyeSwgdGhhdCdzIG5vdCB3aGF0IEkgc2Fp ZCBvciBtZWFudC4gSSB3YXMgd29uZGVyaW5nIGlmCmNjdV9ua21fZGV0ZXJtaW5lX3JhdGUgc2hv dWxkIGNoZWNrIGlmIHRoZSBwYXJlbnQgcmF0ZSBoYXMgbm8gZXhjbHVzaXZlCmxvY2ssIGJlZm9y ZSBhc3N1bWluZyBpdCBjYW4gY2hhbmdlIHRoZSBwYXJlbnQgcmF0ZSwgc28gdGhhdCBJY2Vub3d5 J3MKcGF0Y2hlcyBzdGlsbCB3b3JrLgoKPiBUaGF0IGNvbW1lbnQgaW4gdGhlIGNsb2NrIGZyYW1l d29yayB5b3UgbGlua2VkIHRvIGNsZWFybHkgc3RhdGVkIHRoYXQKPiB5b3UgY2FuJ3QgdXNlIGEg dG9wLWxldmVsIGNsb2NrIGZ1bmN0aW9uIGluIGEgbm90aWZpZXIsIGFuZCB0aGF0J3MKPiBiZWNh dXNlIG9mIHRoZSBsb2NraW5nLgoKWWVzLCBpdCBkb2VzLiBBbmQgdGhhdCdzIHdoeSBJIHRob3Vn aHQgdGhhdCBjYWxsaW5nIGNsa19zZXRfcmF0ZSgpIGluCnRoZSBub3RpZmllciBjYWxsYmFjayB3 YXMgbmV2ZXIgdGhlIHJpZ2h0IGNob2ljZS4KCj4gSWYgaXQncyBub3Qgd2hhdCB5b3UncmUgdHJ5 aW5nIHRvIGZpeCwgdGhlbiBJJ2QgcmVhbGx5IGxpa2UgdG8ga25vdyB3aGF0Cj4gaXNzdWUgeW91 J3JlIHRyeWluZyB0byBmaXggKmluIHRoZSBmcmFtZXdvcmsqIChzbywgbm90IG9uIHRoZSBwbGwt bWlwaQo+IGNsb2NrLCBvciB0aGUgQTY0KS4KCkknbSBub3QgdHJ5aW5nIHRvICJmaXgiIGFueXRo aW5nIGluIHRoZSBmcmFtZXdvcmsgaW4gdGhlIHNlbnNlIHRoYXQgdGhlCmZyYW1ld29yayBoYXMg YSBidWcuIEkgcHJvcG9zZSB0byBhZGQgYSBuZXcgZmVhdHVyZSB0byB0aGUgZnJhbWV3b3JrLCBz bwp0aGF0IEkgY2FuIGV4dGVuZCBwbGwtbWlwaSwgc28gdGhhdCB0aGUgQTY0IGNhbiBkcml2ZSBi b3RoIGFuIExDRCBhbmQKSERNSSBhdCB0aGUgc2FtZSB0aW1lLgoKPj4gQ3VycmVudGx5LCBpdCBv bmx5IGNhbGxzIGNsa19od19jYW5fc2V0X3JhdGVfcGFyZW50KCkgd2hpY2ggb25seQo+PiBjaGVj a3MgdGhlIGZsYWcsIGJ1dCBkb2VzIG5vdCBjaGVjayBpZiBpdCBpcyByZWFsbHkgcG9zc2libGUg dG8gY2hhbmdlCj4+IHRoZSBwYXJlbnQncyByYXRlLgo+Pgo+PiBSZWdhcmRsZXNzLCBwbGVhc2Ug ZG9uJ3QgcHJlbWF0dXJlbHkgZGlzbWlzcyBteSBwcm9wb3NhbC4gSXQgaGFzIHRoZQo+PiBhZHZh bnRhZ2UgdGhhdCBpdCBpcyBub3Qgc3BlY2lmaWMgZm9yIHN1bnhpLW5nLCBidXQgY291bGQgYmUg dXNlZCBmb3IKPj4gb3RoZXIgZHJpdmVycyBhcyB3ZWxsLgo+Cj4gSnVzdCBsaWtlIHRoZSB0d28g c29sdXRpb25zIEkgcHJvdmlkZWQuCj4KPj4gTWF5YmUgdGhlcmUgb3RoZXIgaW5zdGFuY2VzIG9m IGV4Y2x1c2l2ZSBsb2NrcyB0b2RheSB3aGVyZSB0aGUKPj4gQ0xLX0tFRVBfUkFURSBmbGFnIG1p Z2h0IHdvcmsgZXF1YWxseSB3ZWxsLiDwn6S3Cj4KPiBJZiBleGNsdXNpdmUgbG9ja3Mgd29yayBl cXVhbGx5IHdlbGwsIHdoeSB3b3VsZCB3ZSBuZWVkIENMS19LRUVQX1JBVEU/CgpBcyBJIHdyb3Rl IGluIG15IG90aGVyIG1haWwuIFRoZSB3b3JkICJrZWVwIiB3YXMgYXBwYXJlbnRseSBhIGJhZApj aG9pY2UuIE1heWJlIENMS19SRVNUT1JFX1JBVEVfQ0xPU0VMWT8gVGhlIGRpZmZlcmVuY2UgaXMg dGhhdCBhbgpleGNsdXNpdmUgbG9jayBwcmV2ZW50cyBvdGhlciBjbG9ja3MgZnJvbSBjaGFuZ2lu ZyB0aGUgcGFyZW50LCB3aGVyZWFzCiJDTEtfS0VFUF9SQVRFIiAoc2ljKSBhbGxvd3MgdGhvc2Ug Y2hhbmdlcyB0byBoYXBwZW4gYW5kIGRlYWwgd2l0aCBpdCBieQpyZXN0b3JpbmcgdGhlIHByZXZp b3VzIHJhdGUgYXMgY2xvc2VseSBhcyBwb3NzaWJsZSBhZnRlciB0aGUgcGFyZW50IHJhdGUKY2hh bmdlZC4KCj4+ID4gSWYgc28sIEkgdGhpbmsgdGhlIHRoaW5nIHdlIHNob3VsZCBlbXBoYXNpemUg aXMgdGhhdCBpdCdzIGFib3V0ICphbnkKPj4gPiB0b3AtbGV2ZWwgY2xrIEFQSSosIGFzIGluIGNs a19zZXRfcmF0ZSgpIG9yIGNsa19zZXRfcGFyZW50KCkuCj4+ID4KPj4gPiBUaGUgaXNzdWUgaXMg dGhhdCBhbnkgY29uc3VtZXItZmFjaW5nIEFQSSBpcyB0YWtpbmcgdGhlIGNsa19wcmVwYXJlIGxv Y2sKPj4gPiBhbmQgdGh1cyB3ZSB3b3VsZCBoYXZlIHJlZW50cmFuY3kuIEJ1dCB3ZSdyZSBhIHBy b3ZpZGVyIHRoZXJlLCBhbmQgbm9uZQo+PiA+IG9mIHRoZSBjbGtfaHdfKiBmdW5jdGlvbnMgYXJl IHRha2luZyB0aGF0IGxvY2suIE5laXRoZXIgZG8gb3VyIG93biBmdW5jdGlvbi4KPj4gPgo+PiA+ IFNvIHdlIGNvdWxkIGNhbGwgaW4gdGhhdCBub3RpZmllciBvdXIgc2V0X3JhdGUgY2FsbGJhY2sg ZGlyZWN0bHksIG9yIHdlCj4+ID4gY291bGQgY3JlYXRlIGEgY2xrX2h3X3NldF9yYXRlKCkgZnVu Y3Rpb24uCj4+ID4KPj4gPiBUaGUgZmlyc3Qgb25lIHdpbGwgY3JlYXRlIGNhY2hlIGlzc3VlIGJl dHdlZW4gdGhlIGFjdHVhbCByYXRlIHRoYXQgdGhlCj4+ID4gY29tbW9uIGNsb2NrIGZyYW1ld29y ayBpcyBydW5uaW5nIGFuZCB0aGUgb25lIHdlIGFjdHVhbGx5IGVuZm9yY2VkLCBidXQKPj4gPiB3 ZSBjb3VsZCBjcmVhdGUgYSBmdW5jdGlvbiB0byBmbHVzaCB0aGUgQ0NGIGNhY2hlLgo+PiA+Cj4+ ID4gVGhlIHNlY29uZCBvbmUgaXMgcHJvYmFibHkgc2ltcGxlci4KPj4KPj4gSSdtIHByb2JhYmx5 IG1pc3Npbmcgc29tZXRoaW5nLCBiZWNhdXNlIEkgZG9uJ3QgdGhpbmsgdGhpcyB3b3VsZCB3b3Jr Lgo+PiBGb3IgcmVmZXJlbmNlLCB0aGlzIGlzIG91ciB0cmVlOgo+Pgo+PiAgICAgcGxsLXZpZGVv MAo+PiAgICAgICAgaGRtaS1waHktY2xrCj4+ICAgICAgICBoZG1pCj4+ICAgICAgICB0Y29uMQo+ PiAgICAgICAgcGxsLW1pcGkKPj4gICAgICAgICAgIHRjb24wCj4+ICAgICAgICAgICAgICB0Y29u LWRhdGEtY2xvY2sKPj4KPj4gV2hlbiBwbGwtdmlkZW8wJ3MgcmF0ZSBpcyBjaGFuZ2VkIChlLmcu IGJlY2F1c2UgYSBIRE1JIG1vbml0b3IgaXMKPj4gcGx1Z2dlZCBpbiksIHRoZSByYXRlcyBvZiB0 aGUgY29tcGxldGUgc3VidHJlZSBmb3IgcGxsLXZpZGVvMCBhcmUKPj4gcmVjYWxjdWxhdGVkLCBp bmNsdWRpbmcgdGNvbjAgYW5kIHRjb24tZGF0YS1jbG9jay4gVGhlIHJhdGUgb2YgdGNvbjAgaXMK Pj4gYmFzZWQgb24gdGhlIHJhdGUgdGhhdCB3YXMgcmVjYWxjdWxhdGVkIGZvciBwbGwtbWlwaSwg d2hpY2ggLSBpbiB0dXJuIC0KPj4gd2FzIG9mIGNvdXJzZSByZWNhbGN1bGF0ZWQgYmFzZWQgb24g dGhlIHBsbC12aWRlbzAncyBuZXcgcmF0ZS4gVGhlc2UKPj4gdmFsdWVzIGFyZSBzdG9yZWQgYnkg dGhlIGNsayBmcmFtZXdvcmsgaW4gYSBwcml2YXRlIHN0cnVjdC4gVGhleSBhcmUKPj4gY2FsY3Vs YXRlZCBiZWZvcmUgYWN0dWFsbHkgcGVyZm9ybWluZyBhbnkgcmF0ZSBjaGFuZ2VzLgo+Pgo+PiBT bywgaWYgYSBub3RpZmllciBzZXRzIHBsbC1taXBpJ3MgcmF0ZSB0byBzb21ldGhpbmcgZWxzZSB0 aGFuIHdhcwo+PiBwcmV2aW91c2x5IHJlY2FsY3VsYXRlZCwgdGhlIGNsayBmcmFtZXdvcmsgd291 bGQgc3RpbGwgdHJ5IHRvIHNldCB0Y29uMAo+PiB0byB0aGUgdmFsdWUgdGhhdCBpdCBwcmV2aW91 c2x5IGNhbGN1bGF0ZWQuCj4+Cj4+IFNvLCB3ZSB3b3VsZCBoYXZlIHRvIHJlY2FsY3VsYXRlIHBs bC1taXBpJ3Mgc3VidHJlZSBhZnRlciBjaGFuZ2luZyBpdHMKPj4gcmF0ZSAodGhhdCdzIHdoYXQg UEFUQ0ggMSBpcyBkb2luZykuCj4KPiBUaGVuIHdlIHNob3VsZCBtYWtlIHRoYXQgZnVuY3Rpb24g SSB3YXMgdGVsbGluZyB5b3UgYWJvdXQgZGVhbCB3aXRoIGFsbAo+IHRoaXMuCgpTZWUgbXkgb3Ro ZXIgbWFpbC4gSSBtaXMtcmVtZW1iZXJlZC4gVGhhdCBpcyBhbHJlYWR5IGF2YWlsYWJsZSB2aWEg dGhlCkNMS19SRUNBTENfTkVXX1JBVEVTIGZsYWcuCgpCZXN0IHJlZ2FyZHMsCiAgRnJhbmsKCj4K PiBNYXhpbWUKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f CmxpbnV4LWFybS1rZXJuZWwgbWFpbGluZyBsaXN0CmxpbnV4LWFybS1rZXJuZWxAbGlzdHMuaW5m cmFkZWFkLm9yZwpodHRwOi8vbGlzdHMuaW5mcmFkZWFkLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2xp bnV4LWFybS1rZXJuZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id DE015C83F12 for ; Mon, 28 Aug 2023 14:17:17 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2869310E2EB; Mon, 28 Aug 2023 14:17:17 +0000 (UTC) Received: from mout-p-102.mailbox.org (mout-p-102.mailbox.org [80.241.56.152]) by gabe.freedesktop.org (Postfix) with ESMTPS id A12F310E2EC for ; Mon, 28 Aug 2023 14:17:15 +0000 (UTC) Received: from smtp2.mailbox.org (smtp2.mailbox.org [10.196.197.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-384) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-102.mailbox.org (Postfix) with ESMTPS id 4RZCLH3MqCz9scf; Mon, 28 Aug 2023 16:17:11 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oltmanns.dev; s=MBO0001; t=1693232231; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=s+8pHBapEaGXy1jvUtl21Qsxu+aNw3OPEPZ0IC0tfoU=; b=KhQU8OSWRXoTxFuCN9bgCewsVz5vGYuF5TgU4KK0f6QdpC0tD/0Zeh3CgehSpAO7HpQdNP FG62G59jyF1WWcPDprdzP9XkXI8VYyKV1zsRwG+JjDA0qRGMr+13zoffDYByQlinby7kNu kDrzCtahTq4Du7tkBCwDVbWC56w7TbSscOymVeYii8G0QF3qGbbxIyA0Ha5jc3iFajYfpb v6r+Zu/iPhopSb3cHyeWRYgC74qzjJpPJFjAmHuB06sIHe0us7z+G2RQ1hQmDbc4FXO+mh gG1N92rlyJZat0jtul2Va4h7G2or2X255lmv6qSdDl+WyYPLts7O0P9OyRk0Dg== References: <20230825-pll-mipi_keep_rate-v1-0-35bc43570730@oltmanns.dev> <87ledzqhwx.fsf@oltmanns.dev> From: Frank Oltmanns To: Maxime Ripard Subject: Re: [PATCH 0/3] Make Allwinner A64's pll-mipi keep its rate when parent rate changes In-reply-to: Date: Mon, 28 Aug 2023 16:17:08 +0200 Message-ID: <87cyz7uu8r.fsf@oltmanns.dev> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: dri-devel@lists.freedesktop.org, Icenowy Zheng , Samuel Holland , Stephen Boyd , Michael Turquette , linux-kernel@vger.kernel.org, Jernej Skrabec , linux-sunxi@lists.linux.dev, Chen-Yu Tsai , Ondrej Jirman , linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Icenowy Zheng Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On 2023-08-28 at 10:04:51 +0200, Maxime Ripard wrote: > On Fri, Aug 25, 2023 at 05:07:58PM +0200, Frank Oltmanns wrote: >> Thank you for your feedback, Maxime! >> >> On 2023-08-25 at 10:13:53 +0200, Maxime Ripard wrot= e: >> > [[PGP Signed Part:Undecided]] >> > Hi, >> > >> > On Fri, Aug 25, 2023 at 07:36:36AM +0200, Frank Oltmanns wrote: >> >> I would like to make the Allwinner A64's pll-mipi to keep its rate wh= en >> >> its parent's (pll-video0) rate changes. Keeping pll-mipi's rate is >> >> required, to let the A64 drive both an LCD and HDMI display at the sa= me >> >> time, because both have pll-video0 as an ancestor. >> >> >> >> PATCH 1 adds this functionality as a feature into the clk framework (= new >> >> flag: CLK_KEEP_RATE). >> >> >> >> Cores that use this flag, store a rate as req_rate when it or one of = its >> >> descendants requests a new rate. >> >> >> >> That rate is then restored in the clk_change_rate recursion, which wa= lks >> >> through the tree. It will reach the flagged core (e.g. pll-mipi) after >> >> the parent's rate (e.g. pll-video0) has already been set to the new >> >> rate. It will then call determine_rate (which requests the parent's >> >> current, i.e. new, rate) to determine a rate that is close to the >> >> flagged core's previous rate. Afterward it will re-calculate the rates >> >> for the flagged core's subtree. >> > >> > I don't think it's the right way forward. It makes the core logic more >> > complicated, for something that is redundant with the notifiers >> > mechanism that has been the go-to for that kind of things so far. >> >> Yeah, that was my initial idea as well. But I couldn't get it to work. >> See details below. >> >> Do you have an example of a clock that restores its previous rate after >> the parent rate has changed? I've looked left and right, but to me it >> seems that notifiers are mainly used for setting clocks into some kind >> of "safe mode" prior to the rate change. Examples: >> >> sunxi-ng: >> https://elixir.bootlin.com/linux/v6.4.11/source/drivers/clk/sunxi-ng/ccu= _mux.c#L273 >> https://elixir.bootlin.com/linux/v6.4.11/source/drivers/clk/sunxi-ng/ccu= _common.c#L60 >> >> but also others: >> https://elixir.bootlin.com/linux/v6.4.11/source/drivers/clk/at91/clk-mas= ter.c#L248 >> https://elixir.bootlin.com/linux/v6.4.11/source/drivers/clk/meson/meson8= b.c#L3755 >> https://elixir.bootlin.com/linux/v6.4.11/source/drivers/clk/qcom/clk-cpu= -8996.c#L546 > > There's examples for phases and parents, but not for rates afaics. We > shouldn't behave any differently though. > >> > It's not really obvious to me why the notifiers don't work there. >> > >> >> This work is inspired by an out-of-tree patchset [1] [2] [3]. >> >> Unfortunately, the patchset uses clk_set_rate() in a notifier callbac= k, >> >> which the following comment on clk_notifier_register() forbids: "The >> >> callbacks associated with the notifier must not re-enter into the clk >> >> framework by calling any top-level clk APIs." [4] Furthermore, that >> >> out-of-tree patchset no longer works with the current linux-next, >> >> because setting pll-mipi is now also resetting pll-video0 [5]. >> > >> > Is it because of the "The callbacks associated with the notifier must >> > not re-enter into the clk framework by calling any top-level clk APIs." >> > comment? >> >> I don't think that's the reason. > > I'm not sure I follow you there. How can we find a solution to a problem > you don't know about or can't know for sure? I was hoping that the discussion here would give me some clues (and it does). You have already explained, that the issue is the locks. I'm still confused why Icenowy's patches work. They use clk_set_rate() in a notifier callback and despite that they work (up until kernel 6.5). The only thing that has changed here (that I'm aware of), is that pll-mipi now sets the parent rate in clk-next. >> I'm fairly certain that the problem is, that pll-mipi tries to set the >> parent rate. Maybe it should check if the parent is locked, before >> determining a rate that requires the parent rate to change. =F0=9F=A4=94 > > Why would the clock framework documentation mention an issue that only > arises with a single clock on a single SoC? No, sorry, that's not what I said or meant. I was wondering if ccu_nkm_determine_rate should check if the parent rate has no exclusive lock, before assuming it can change the parent rate, so that Icenowy's patches still work. > That comment in the clock framework you linked to clearly stated that > you can't use a top-level clock function in a notifier, and that's > because of the locking. Yes, it does. And that's why I thought that calling clk_set_rate() in the notifier callback was never the right choice. > If it's not what you're trying to fix, then I'd really like to know what > issue you're trying to fix *in the framework* (so, not on the pll-mipi > clock, or the A64). I'm not trying to "fix" anything in the framework in the sense that the framework has a bug. I propose to add a new feature to the framework, so that I can extend pll-mipi, so that the A64 can drive both an LCD and HDMI at the same time. >> Currently, it only calls clk_hw_can_set_rate_parent() which only >> checks the flag, but does not check if it is really possible to change >> the parent's rate. >> >> Regardless, please don't prematurely dismiss my proposal. It has the >> advantage that it is not specific for sunxi-ng, but could be used for >> other drivers as well. > > Just like the two solutions I provided. > >> Maybe there other instances of exclusive locks today where the >> CLK_KEEP_RATE flag might work equally well. =F0=9F=A4=B7 > > If exclusive locks work equally well, why would we need CLK_KEEP_RATE? As I wrote in my other mail. The word "keep" was apparently a bad choice. Maybe CLK_RESTORE_RATE_CLOSELY? The difference is that an exclusive lock prevents other clocks from changing the parent, whereas "CLK_KEEP_RATE" (sic) allows those changes to happen and deal with it by restoring the previous rate as closely as possible after the parent rate changed. >> > If so, I think the thing we should emphasize is that it's about *any >> > top-level clk API*, as in clk_set_rate() or clk_set_parent(). >> > >> > The issue is that any consumer-facing API is taking the clk_prepare lo= ck >> > and thus we would have reentrancy. But we're a provider there, and none >> > of the clk_hw_* functions are taking that lock. Neither do our own fun= ction. >> > >> > So we could call in that notifier our set_rate callback directly, or we >> > could create a clk_hw_set_rate() function. >> > >> > The first one will create cache issue between the actual rate that the >> > common clock framework is running and the one we actually enforced, but >> > we could create a function to flush the CCF cache. >> > >> > The second one is probably simpler. >> >> I'm probably missing something, because I don't think this would work. >> For reference, this is our tree: >> >> pll-video0 >> hdmi-phy-clk >> hdmi >> tcon1 >> pll-mipi >> tcon0 >> tcon-data-clock >> >> When pll-video0's rate is changed (e.g. because a HDMI monitor is >> plugged in), the rates of the complete subtree for pll-video0 are >> recalculated, including tcon0 and tcon-data-clock. The rate of tcon0 is >> based on the rate that was recalculated for pll-mipi, which - in turn - >> was of course recalculated based on the pll-video0's new rate. These >> values are stored by the clk framework in a private struct. They are >> calculated before actually performing any rate changes. >> >> So, if a notifier sets pll-mipi's rate to something else than was >> previously recalculated, the clk framework would still try to set tcon0 >> to the value that it previously calculated. >> >> So, we would have to recalculate pll-mipi's subtree after changing its >> rate (that's what PATCH 1 is doing). > > Then we should make that function I was telling you about deal with all > this. See my other mail. I mis-remembered. That is already available via the CLK_RECALC_NEW_RATES flag. Best regards, Frank > > Maxime