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@kernel.org>
Signed-off-by: Frank Oltmanns <frank@oltmanns.dev>
Cc: stable@vger.kernel.org
---
drivers/clk/sunxi-ng/ccu_common.c | 19 +++++++++++++++++++
drivers/clk/sunxi-ng/ccu_common.h | 3 +++
2 files changed, 22 insertions(+)
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.0Frank Oltmanns <frank@oltmanns.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@kernel.org>
> Signed-off-by: Frank Oltmanns <frank@oltmanns.dev>
> Cc: stable@vger.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.
> 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
Hi Måns,
21.05.2024 15:43:10 Måns Rullgård <mans@mansr.com>:
> Frank Oltmanns <frank@oltmanns.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@kernel.org>
>> Signed-off-by: Frank Oltmanns <frank@oltmanns.dev>
>> Cc: stable@vger.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.
Best regards,
Frank
>
>> 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
Frank Oltmanns <frank@oltmanns.dev> writes:
> Hi Måns,
>
> 21.05.2024 15:43:10 Måns Rullgård <mans@mansr.com>:
>
>> Frank Oltmanns <frank@oltmanns.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@kernel.org>
>>> Signed-off-by: Frank Oltmanns <frank@oltmanns.dev>
>>> Cc: stable@vger.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
Måns Rullgård <mans@mansr.com> writes: > Frank Oltmanns <frank@oltmanns.dev> writes: > >> Hi Måns, >> >> 21.05.2024 15:43:10 Måns Rullgård <mans@mansr.com>: >> >>> Frank Oltmanns <frank@oltmanns.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@kernel.org> >>>> Signed-off-by: Frank Oltmanns <frank@oltmanns.dev> >>>> Cc: stable@vger.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: [...] >>> 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. It turns out HDMI output is broken in v6.9 for a different reason. However, this commit (b914ec33b391 clk: sunxi-ng: common: Support minimum and maximum rate) requires two others as well in order not to break things on the A20: cedb7dd193f6 drm/sun4i: hdmi: Convert encoder to atomic 9ca6bc246035 drm/sun4i: hdmi: Move mode_set into enable With those two (the second depends on the first) cherry-picked on top of v6.6.31, the HDMI output is working again. Likewise on v6.8.10. -- Måns Rullgård
On 23.05.24 20:58, Måns Rullgård wrote: > Måns Rullgård <mans@mansr.com> writes: >> Frank Oltmanns <frank@oltmanns.dev> writes: >>> 21.05.2024 15:43:10 Måns Rullgård <mans@mansr.com>: >>>> Frank Oltmanns <frank@oltmanns.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@kernel.org> >>>>> Signed-off-by: Frank Oltmanns <frank@oltmanns.dev> >>>>> Cc: stable@vger.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: > [...] > >>>> Reverting this commit makes it work again. >>> Thank you for your detailed report! > [...] > It turns out HDMI output is broken in v6.9 for a different reason. > However, this commit (b914ec33b391 clk: sunxi-ng: common: Support > minimum and maximum rate) requires two others as well in order not > to break things on the A20: > > cedb7dd193f6 drm/sun4i: hdmi: Convert encoder to atomic > 9ca6bc246035 drm/sun4i: hdmi: Move mode_set into enable > > With those two (the second depends on the first) cherry-picked on top of > v6.6.31, the HDMI output is working again. Likewise on v6.8.10. They from what I can see are not yet in 6.6.y or on their way there (6.8 is EOL now). Did anyone ask Greg to pick this up? If not: Månsm could you maybe do that? CCing him on a reply and asking is likely enough if both changes apply cleanly. Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page. #regzbot introduced: 547263745e15a0 #regzbot fix: drm/sun4i: hdmi: Move mode_set into enable #regzbot poke
On Wed, Jun 12, 2024 at 03:28:01PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote: > On 23.05.24 20:58, Måns Rullgård wrote: > > Måns Rullgård <mans@mansr.com> writes: > >> Frank Oltmanns <frank@oltmanns.dev> writes: > >>> 21.05.2024 15:43:10 Måns Rullgård <mans@mansr.com>: > >>>> Frank Oltmanns <frank@oltmanns.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@kernel.org> > >>>>> Signed-off-by: Frank Oltmanns <frank@oltmanns.dev> > >>>>> Cc: stable@vger.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: > > [...] > > > >>>> Reverting this commit makes it work again. > >>> Thank you for your detailed report! > > [...] > > It turns out HDMI output is broken in v6.9 for a different reason. > > However, this commit (b914ec33b391 clk: sunxi-ng: common: Support > > minimum and maximum rate) requires two others as well in order not > > to break things on the A20: > > > > cedb7dd193f6 drm/sun4i: hdmi: Convert encoder to atomic > > 9ca6bc246035 drm/sun4i: hdmi: Move mode_set into enable > > > > With those two (the second depends on the first) cherry-picked on top of > > v6.6.31, the HDMI output is working again. Likewise on v6.8.10. > > They from what I can see are not yet in 6.6.y or on their way there (6.8 > is EOL now). Did anyone ask Greg to pick this up? If not: Månsm could > you maybe do that? CCing him on a reply and asking is likely enough if > both changes apply cleanly. Both now queued up, thanks. greg k-h
On Sun, 10 Mar 2024 14:21:11 +0100, Frank Oltmanns wrote: > 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 > > [ ... ] Acked-by: Maxime Ripard <mripard@kernel.org> Thanks! Maxime
Dne nedelja, 10. marec 2024 ob 14:21:11 CET je Frank Oltmanns napisal(a):
> 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@kernel.org>
> Signed-off-by: Frank Oltmanns <frank@oltmanns.dev>
> Cc: stable@vger.kernel.org
This looks pretty nice now.
Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Best regards,
Jernej
> ---
> drivers/clk/sunxi-ng/ccu_common.c | 19 +++++++++++++++++++
> drivers/clk/sunxi-ng/ccu_common.h | 3 +++
> 2 files changed, 22 insertions(+)
>
> 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;
>
>
© 2016 - 2025 Red Hat, Inc.