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 34DE0C25B78 for ; Wed, 22 May 2024 18:07:49 +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:References :In-Reply-To:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=4yJsVdj+U/suXiEeJ3CHMVU2vyeVDx7GY98A1gnLGiQ=; b=vtDGIAg3fbVMOg MGCRp0aAyDqBFmGk0pORv/Vc91tw0CIw9h9z8dkK7zQ/hJsToFnFauKWnXps0whGlBV/uqZX/fIst kD8lcJ5hcQlQtIPPtJI8OXxLOQlygZohng7dGv24OzzP7N6OMGMAjGHd29jhJl6wZjSJJRUKGO2we yi/duOA5qzZEZNUehHK+dbYeCeUU6eQI3bFhV19FeirL+13ktnjDu9v7wL9q5glEQCMaABvu+a//l uV9DeQJ9guddsHCK+dt58YijK/dEDXPhmDh7xxsMaSSZTkAFXZc/wkXRs89l8eSw2DpS3bXbOX2Fa 0xqNZ8YzJe9v/L6N/hvA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1s9qND-00000003lca-0JyF; Wed, 22 May 2024 18:07:36 +0000 Received: from unicorn.mansr.com ([2001:8b0:ca0d:1::2]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1s9qN6-00000003lZB-1Chf for linux-arm-kernel@lists.infradead.org; Wed, 22 May 2024 18:07:30 +0000 Received: from raven.mansr.com (raven.mansr.com [IPv6:2001:8b0:ca0d:1::3]) by unicorn.mansr.com (Postfix) with ESMTPS id BDBFD15364; Wed, 22 May 2024 19:07:21 +0100 (BST) Received: by raven.mansr.com (Postfix, from userid 51770) id AF78A219FCA; Wed, 22 May 2024 19:07:21 +0100 (BST) From: =?iso-8859-1?Q?M=E5ns_Rullg=E5rd?= To: Frank Oltmanns Cc: Michael Turquette , Stephen Boyd , Chen-Yu Tsai , Jernej Skrabec , Samuel Holland , Guido =?iso-8859-1?Q?G=FCnther?= , Purism Kernel Team , Ondrej Jirman , Neil Armstrong , Jessica Zhang , Sam Ravnborg , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Daniel Vetter , Rob Herring , Krzysztof Kozlowski , Conor Dooley , linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org, stable@vger.kernel.org Subject: Re: [PATCH v4 1/5] clk: sunxi-ng: common: Support minimum and maximum rate In-Reply-To: (Frank Oltmanns's message of "Wed, 22 May 2024 08:33:01 +0200 (GMT+02:00)") References: <20240310-pinephone-pll-fixes-v4-0-46fc80c83637@oltmanns.dev> <20240310-pinephone-pll-fixes-v4-1-46fc80c83637@oltmanns.dev> Date: Wed, 22 May 2024 19:07:21 +0100 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.3 (gnu/linux) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240522_110728_616795_959091FA X-CRM114-Status: GOOD ( 25.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="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Frank Oltmanns writes: > Hi M=E5ns, > > 21.05.2024 15:43:10 M=E5ns Rullg=E5rd : > >> Frank Oltmanns writes: >> >>> The Allwinner SoC's typically have an upper and lower limit for their >>> clocks' rates. Up until now, support for that has been implemented >>> separately for each clock type. >>> >>> Implement that functionality in the sunxi-ng's common part making use of >>> the CCF rate liming capabilities, so that it is available for all clock >>> types. >>> >>> Suggested-by: Maxime Ripard >>> Signed-off-by: Frank Oltmanns >>> Cc: stable@vger.kernel.org >>> --- >>> drivers/clk/sunxi-ng/ccu_common.c | 19 +++++++++++++++++++ >>> drivers/clk/sunxi-ng/ccu_common.h |=A0 3 +++ >>> 2 files changed, 22 insertions(+) >> >> This just landed in 6.6 stable, and it broke HDMI output on an A20 based >> device, the clocks ending up all wrong as seen in this diff of >> /sys/kernel/debug/clk/clk_summary: >> >> @@ -70,16 +71,14 @@ >> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 apb1-i2c0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0= =A0=A0=A0=A0 24000000=A0=A0=A0 0 >> >> =A0=A0=A0=A0=A0=A0=A0 pll-gpu=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0=A0 0=A0=A0= =A0=A0=A0=A0=A0 1200000000=A0 0 >> -=A0=A0=A0=A0=A0=A0 pll-video1=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0 3=A0=A0=A0=A0=A0=A0 3=A0=A0=A0=A0=A0=A0=A0 1=A0=A0=A0=A0=A0= =A0=A0 159000000=A0=A0 0 >> +=A0=A0=A0=A0=A0=A0 pll-video1=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0 2=A0=A0=A0=A0=A0=A0 2=A0=A0=A0=A0=A0=A0=A0 1=A0=A0=A0=A0=A0= =A0=A0 159000000=A0=A0 0 >> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 hdmi=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 1=A0=A0=A0=A0=A0=A0 1=A0=A0=A0=A0=A0=A0=A0 0= =A0=A0=A0=A0=A0=A0=A0 39750000=A0=A0=A0 0 >> >> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 tcon0-ch1-sclk2=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0 1=A0=A0=A0=A0=A0=A0 1=A0=A0=A0=A0=A0=A0=A0 1=A0=A0=A0=A0=A0=A0=A0= 39750000=A0=A0=A0 0 >> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 tcon0-ch1-sclk1=A0=A0=A0=A0=A0= =A0=A0=A0 1=A0=A0=A0=A0=A0=A0 1=A0=A0=A0=A0=A0=A0=A0 1=A0=A0=A0=A0=A0=A0=A0= 39750000=A0=A0=A0 0 >> >> -=A0=A0=A0=A0=A0=A0=A0=A0=A0 pll-video1-2x=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0 1=A0=A0=A0=A0=A0=A0 1=A0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0=A0= 318000000=A0=A0 0 >> +=A0=A0=A0=A0=A0=A0=A0=A0=A0 pll-video1-2x=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0 0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0=A0= 318000000=A0=A0 0 >> >> -=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 hdmi-tmds=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0 2=A0=A0=A0=A0=A0=A0 2=A0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0= =A0=A0=A0 39750000=A0=A0=A0 0 >> -=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 hdmi-ddc=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0 1=A0=A0=A0=A0=A0=A0 1=A0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0= =A0=A0=A0=A0 1987500=A0=A0=A0=A0 0 >> =A0=A0=A0=A0=A0=A0=A0 pll-periph-base=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0 2=A0=A0=A0=A0=A0=A0 2=A0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0=A0= 1200000000=A0 0 >> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 mbus=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 1=A0=A0=A0=A0=A0=A0 1=A0=A0=A0=A0=A0=A0=A0 0= =A0=A0=A0=A0=A0=A0=A0 300000000=A0=A0 0 >> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 pll-periph-sata=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0 0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0=A0= 100000000=A0=A0 0 >> @@ -199,7 +198,7 @@ >> >> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 ace=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0= =A0 0=A0=A0=A0=A0=A0=A0=A0 384000000=A0=A0 0 >> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 ve=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0= =A0 0=A0=A0=A0=A0=A0=A0=A0 384000000=A0=A0 0 >> -=A0=A0=A0=A0=A0=A0 pll-video0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0 4=A0=A0=A0=A0=A0=A0 4=A0=A0=A0=A0=A0=A0=A0 2=A0=A0=A0=A0=A0= =A0=A0 297000000=A0=A0 0 >> +=A0=A0=A0=A0=A0=A0 pll-video0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0 5=A0=A0=A0=A0=A0=A0 5=A0=A0=A0=A0=A0=A0=A0 2=A0=A0=A0=A0=A0= =A0=A0 297000000=A0=A0 0 >> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 hdmi1=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0=A0 0=A0= =A0=A0=A0=A0=A0=A0 297000000=A0=A0 0 >> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 tcon1-ch1-sclk2=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0 0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0=A0= 297000000=A0=A0 0 >> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 tcon1-ch1-sclk1=A0=A0=A0=A0=A0= =A0=A0=A0 0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0=A0= 297000000=A0=A0 0 >> @@ -222,8 +221,10 @@ >> >> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 de-be0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0 1=A0=A0=A0=A0=A0=A0 1=A0=A0=A0=A0=A0=A0=A0 1=A0= =A0=A0=A0=A0=A0=A0 297000000=A0=A0 0 >> >> -=A0=A0=A0=A0=A0=A0=A0=A0=A0 pll-video0-2x=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0 0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0=A0= 594000000=A0=A0 0 >> +=A0=A0=A0=A0=A0=A0=A0=A0=A0 pll-video0-2x=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0 1=A0=A0=A0=A0=A0=A0 1=A0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0=A0= 594000000=A0=A0 0 >> >> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 hdmi-tmds=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0 2=A0=A0=A0=A0=A0=A0 2=A0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0= =A0=A0=A0 594000000=A0=A0 0 >> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 hdmi-ddc=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0 1=A0=A0=A0=A0=A0=A0 1=A0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0= =A0=A0=A0=A0 29700000=A0=A0=A0 0 >> =A0=A0=A0=A0=A0=A0=A0 pll-audio-base=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0 0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0=A0= 1500000=A0=A0=A0=A0 0 >> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 pll-audio-8x=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0= =A0=A0 3000000=A0=A0=A0=A0 0 >> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 i2s2=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0 0=A0=A0=A0=A0=A0=A0=A0 0= =A0=A0=A0=A0=A0=A0=A0 3000000=A0=A0=A0=A0 0 >> >> Reverting this commit makes it work again. > > Thank you for your detailed report! > > I've had a first look at hdmi-tmds and hdmi-ddc, and neither seems to > be calling ccu_is_better_rate() in their determine_rate() > functions. Their parents have the exact same rates in your diff, so, > my current working assumption is that they can't be the cause either. > > I'll have a more detailed look over the weekend. Until then, if anyone > has some ideas where I should have a look next, please share your > thoughts. In case it's relevant, this system doesn't use the HDMI DDC, the physical DDC pins being connected to a different I2C adapter for various reasons. >From the clk_summary diff, I see a few things: 1. hdmi-tmds has changed parent from pll-video1-2x to pll-video0-2x. 2. The ratio of hdmi-tmds to its parent has changed from 1/8 to 1. 3. The resulting rate bears no relation to the pixel clock from EDID. I tried kernel 6.9.1 as well, and that doesn't work either. I'll keep digging and try to narrow it down. >>> diff --git a/drivers/clk/sunxi-ng/ccu_common.c b/drivers/clk/sunxi-ng/c= cu_common.c >>> index 8babce55302f..ac0091b4ce24 100644 >>> --- a/drivers/clk/sunxi-ng/ccu_common.c >>> +++ b/drivers/clk/sunxi-ng/ccu_common.c >>> @@ -44,6 +44,16 @@ bool ccu_is_better_rate(struct ccu_common *common, >>> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 unsigned long current_rate, >>> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 unsigned long best_rate) >>> { >>> +=A0=A0 unsigned long min_rate, max_rate; >>> + >>> +=A0=A0 clk_hw_get_rate_range(&common->hw, &min_rate, &max_rate); >>> + >>> +=A0=A0 if (current_rate > max_rate) >>> +=A0=A0=A0=A0=A0=A0 return false; >>> + >>> +=A0=A0 if (current_rate < min_rate) >>> +=A0=A0=A0=A0=A0=A0 return false; >>> + >>> =A0=A0=A0 if (common->features & CCU_FEATURE_CLOSEST_RATE) >>> =A0=A0=A0=A0=A0=A0=A0 return abs(current_rate - target_rate) < abs(best= _rate - target_rate); >>> >>> @@ -122,6 +132,7 @@ static int sunxi_ccu_probe(struct sunxi_ccu *ccu, s= truct device *dev, >>> >>> =A0=A0=A0 for (i =3D 0; i < desc->hw_clks->num ; i++) { >>> =A0=A0=A0=A0=A0=A0=A0 struct clk_hw *hw =3D desc->hw_clks->hws[i]; >>> +=A0=A0=A0=A0=A0=A0 struct ccu_common *common =3D hw_to_ccu_common(hw); >>> =A0=A0=A0=A0=A0=A0=A0 const char *name; >>> >>> =A0=A0=A0=A0=A0=A0=A0 if (!hw) >>> @@ -136,6 +147,14 @@ static int sunxi_ccu_probe(struct sunxi_ccu *ccu, = struct device *dev, >>> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 pr_err("Couldn't register clock %d - = %s\n", i, name); >>> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 goto err_clk_unreg; >>> =A0=A0=A0=A0=A0=A0=A0 } >>> + >>> +=A0=A0=A0=A0=A0=A0 if (common->max_rate) >>> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 clk_hw_set_rate_range(hw, common->min_r= ate, >>> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0 common->max_rate); >>> +=A0=A0=A0=A0=A0=A0 else >>> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 WARN(common->min_rate, >>> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 "No max_rate, ignoring m= in_rate of clock %d - %s\n", >>> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 i, name); >>> =A0=A0=A0 } >>> >>> =A0=A0=A0 ret =3D of_clk_add_hw_provider(node, of_clk_hw_onecell_get, >>> diff --git a/drivers/clk/sunxi-ng/ccu_common.h b/drivers/clk/sunxi-ng/c= cu_common.h >>> index 942a72c09437..329734f8cf42 100644 >>> --- a/drivers/clk/sunxi-ng/ccu_common.h >>> +++ b/drivers/clk/sunxi-ng/ccu_common.h >>> @@ -31,6 +31,9 @@ struct ccu_common { >>> =A0=A0=A0 u16=A0=A0=A0=A0 lock_reg; >>> =A0=A0=A0 u32=A0=A0=A0=A0 prediv; >>> >>> +=A0=A0 unsigned long=A0=A0 min_rate; >>> +=A0=A0 unsigned long=A0=A0 max_rate; >>> + >>> =A0=A0=A0 unsigned long=A0=A0 features; >>> =A0=A0=A0 spinlock_t=A0 *lock; >>> =A0=A0=A0 struct clk_hw=A0=A0 hw; >>> >>> -- >>> >>> 2.44.0 >>> >> >> -- >> M=E5ns Rullg=E5rd > -- = M=E5ns Rullg=E5rd _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel