[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <yw1x4jap90va.fsf@mansr.com>
Date: Wed, 22 May 2024 19:07:21 +0100
From: Måns Rullgård <mans@...sr.com>
To: Frank Oltmanns <frank@...manns.dev>
Cc: Michael Turquette <mturquette@...libre.com>, Stephen Boyd
<sboyd@...nel.org>, Chen-Yu Tsai <wens@...e.org>, Jernej Skrabec
<jernej.skrabec@...il.com>, Samuel Holland <samuel@...lland.org>, Guido
Günther <agx@...xcpu.org>, Purism Kernel Team
<kernel@...i.sm>, Ondrej
Jirman <megi@....cz>, Neil Armstrong <neil.armstrong@...aro.org>, Jessica
Zhang <quic_jesszhan@...cinc.com>, Sam Ravnborg <sam@...nborg.org>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard
<mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, David
Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>, Rob Herring
<robh+dt@...nel.org>, Krzysztof Kozlowski
<krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>,
linux-clk@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-sunxi@...ts.linux.dev, linux-kernel@...r.kernel.org,
dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
stable@...r.kernel.org
Subject: Re: [PATCH v4 1/5] clk: sunxi-ng: common: Support minimum and
maximum rate
Frank Oltmanns <frank@...manns.dev> writes:
> Hi Måns,
>
> 21.05.2024 15:43:10 Måns Rullgård <mans@...sr.com>:
>
>> Frank Oltmanns <frank@...manns.dev> 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 <mripard@...nel.org>
>>> Signed-off-by: Frank Oltmanns <frank@...manns.dev>
>>> Cc: stable@...r.kernel.org
>>> ---
>>> drivers/clk/sunxi-ng/ccu_common.c | 19 +++++++++++++++++++
>>> drivers/clk/sunxi-ng/ccu_common.h | 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 @@
>> apb1-i2c0 0 0 0 24000000 0
>>
>> pll-gpu 0 0 0 1200000000 0
>> - pll-video1 3 3 1 159000000 0
>> + pll-video1 2 2 1 159000000 0
>> hdmi 1 1 0 39750000 0
>>
>> tcon0-ch1-sclk2 1 1 1 39750000 0
>> tcon0-ch1-sclk1 1 1 1 39750000 0
>>
>> - pll-video1-2x 1 1 0 318000000 0
>> + pll-video1-2x 0 0 0 318000000 0
>>
>> - hdmi-tmds 2 2 0 39750000 0
>> - hdmi-ddc 1 1 0 1987500 0
>> pll-periph-base 2 2 0 1200000000 0
>> mbus 1 1 0 300000000 0
>> pll-periph-sata 0 0 0 100000000 0
>> @@ -199,7 +198,7 @@
>>
>> ace 0 0 0 384000000 0
>> ve 0 0 0 384000000 0
>> - pll-video0 4 4 2 297000000 0
>> + pll-video0 5 5 2 297000000 0
>> hdmi1 0 0 0 297000000 0
>> tcon1-ch1-sclk2 0 0 0 297000000 0
>> tcon1-ch1-sclk1 0 0 0 297000000 0
>> @@ -222,8 +221,10 @@
>>
>> de-be0 1 1 1 297000000 0
>>
>> - pll-video0-2x 0 0 0 594000000 0
>> + pll-video0-2x 1 1 0 594000000 0
>>
>> + hdmi-tmds 2 2 0 594000000 0
>> + hdmi-ddc 1 1 0 29700000 0
>> pll-audio-base 0 0 0 1500000 0
>> pll-audio-8x 0 0 0 3000000 0
>> i2s2 0 0 0 3000000 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/ccu_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,
>>> unsigned long current_rate,
>>> unsigned long best_rate)
>>> {
>>> + unsigned long min_rate, max_rate;
>>> +
>>> + clk_hw_get_rate_range(&common->hw, &min_rate, &max_rate);
>>> +
>>> + if (current_rate > max_rate)
>>> + return false;
>>> +
>>> + if (current_rate < min_rate)
>>> + return false;
>>> +
>>> if (common->features & CCU_FEATURE_CLOSEST_RATE)
>>> return abs(current_rate - target_rate) < abs(best_rate - target_rate);
>>>
>>> @@ -122,6 +132,7 @@ static int sunxi_ccu_probe(struct sunxi_ccu *ccu, struct device *dev,
>>>
>>> for (i = 0; i < desc->hw_clks->num ; i++) {
>>> struct clk_hw *hw = desc->hw_clks->hws[i];
>>> + struct ccu_common *common = hw_to_ccu_common(hw);
>>> const char *name;
>>>
>>> if (!hw)
>>> @@ -136,6 +147,14 @@ static int sunxi_ccu_probe(struct sunxi_ccu *ccu, struct device *dev,
>>> pr_err("Couldn't register clock %d - %s\n", i, name);
>>> goto err_clk_unreg;
>>> }
>>> +
>>> + if (common->max_rate)
>>> + clk_hw_set_rate_range(hw, common->min_rate,
>>> + common->max_rate);
>>> + else
>>> + WARN(common->min_rate,
>>> + "No max_rate, ignoring min_rate of clock %d - %s\n",
>>> + i, name);
>>> }
>>>
>>> ret = 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/ccu_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 {
>>> u16 lock_reg;
>>> u32 prediv;
>>>
>>> + unsigned long min_rate;
>>> + unsigned long max_rate;
>>> +
>>> unsigned long features;
>>> spinlock_t *lock;
>>> struct clk_hw hw;
>>>
>>> --
>>>
>>> 2.44.0
>>>
>>
>> --
>> Måns Rullgård
>
--
Måns Rullgård
Powered by blists - more mailing lists