[PATCH] drm/sun4i: tcon: Support keeping dclk rate upon ancestor clock changes

From: Frank Oltmanns
Date: Sun Mar 10 2024 - 09:32:58 EST


Allow the dclk to reset its rate when a rate change is initiated from an
ancestor clock. This makes it possible to no longer to get an exclusive
lock. As a consequence, it is now possible to set new rates if
necessary, e.g. when an external display is connected.

The first user of this functionality is the A64 because PLL-VIDEO0 is an
ancestor for both HDMI and TCON0. This allows to select an optimal rate
for TCON0 as long as there is no external HDMI connection. Once a change
in PLL-VIDEO0 is performed when an HDMI connection is established, TCON0
can react gracefully and select an optimal rate based on this the new
constraint.

Signed-off-by: Frank Oltmanns <frank@xxxxxxxxxxxx>
---
I would like to make the Allwinner A64's data-clock keep its rate
when its ancestor's (pll-video0) rate changes. Keeping data-clock's rate
is required, to let the A64 drive both an LCD and HDMI display at the
same time, because both have pll-video0 as an ancestor.

TCONs that use this flag store the ideal rate for their data-clock and
subscribe to be notified when data-clock changes. When rate setting has
finished (indicated by a POST_RATE_CHANGE event) the call back function
schedules delayed work to set the data-clock's rate to the initial value
after 100 ms. Using delayed work maks sure that the clock setting is
finished.

I've implemented this functionality as a quirk, so that it is possible
to use it only for the A64.

This patch supersedes [1].

This work is inspired by an out-of-tree patchset [2] [3] [4].
Unfortunately, the patchset uses clk_set_rate() directly in a notifier
callback, 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." [5]
Furthermore, that out-of-tree patchset no longer works since 6.6,
because setting pll-mipi is now also resetting pll-video0 and therefore
causes a race condition.

Thank you for considering this contribution,
Frank

[1] https://lore.kernel.org/lkml/20230825-pll-mipi_keep_rate-v1-0-35bc43570730@xxxxxxxxxxxx/
[2] https://codeberg.org/megi/linux/commit/a37cda2fff41a67a2bacf82b1594e10335d0bd8a
[3] https://codeberg.org/megi/linux/commit/24dc09128d2c8efc6ddf19249420e9798e967a46
[4] https://codeberg.org/megi/linux/commit/728a93d46f99f0eb231ed6fa8971a45f97c7182c
[5] https://elixir.bootlin.com/linux/v6.7.9/source/drivers/clk/clk.c#L4669
---
drivers/gpu/drm/sun4i/sun4i_tcon.c | 70 ++++++++++++++++++++++++++++++++++----
drivers/gpu/drm/sun4i/sun4i_tcon.h | 12 +++++++
2 files changed, 76 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index a1a2c845ade0..b880bd44049a 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -108,9 +108,11 @@ static void sun4i_tcon_channel_set_status(struct sun4i_tcon *tcon, int channel,

if (enabled) {
clk_prepare_enable(clk);
- clk_rate_exclusive_get(clk);
+ if (!tcon->quirks->restores_rate)
+ clk_rate_exclusive_get(clk);
} else {
- clk_rate_exclusive_put(clk);
+ if (!tcon->quirks->restores_rate)
+ clk_rate_exclusive_put(clk);
clk_disable_unprepare(clk);
}
}
@@ -343,6 +345,53 @@ static void sun4i_tcon0_mode_set_dithering(struct sun4i_tcon *tcon,
regmap_write(tcon->regs, SUN4I_TCON_FRM_CTL_REG, val);
}

+static void sun4i_rate_reset_notifier_delayed_update(struct work_struct *work)
+{
+ struct sun4i_rate_reset_nb *rate_reset = container_of(work, struct sun4i_rate_reset_nb,
+ reset_rate_work.work);
+
+ clk_set_rate(rate_reset->target_clk, rate_reset->saved_rate);
+}
+
+static int sun4i_rate_reset_notifier_cb(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ struct sun4i_rate_reset_nb *rate_reset = to_sun4i_rate_reset_nb(nb);
+
+ if (event == POST_RATE_CHANGE)
+ schedule_delayed_work(&rate_reset->reset_rate_work, msecs_to_jiffies(100));
+
+ return NOTIFY_DONE;
+}
+
+static void sun4i_rate_reset_notifier_register(struct sun4i_rate_reset_nb *rate_reset_nb)
+{
+ if (rate_reset_nb->is_registered)
+ return;
+
+ rate_reset_nb->clk_nb.notifier_call = sun4i_rate_reset_notifier_cb;
+
+ INIT_DELAYED_WORK(&rate_reset_nb->reset_rate_work,
+ sun4i_rate_reset_notifier_delayed_update);
+
+ if (!clk_notifier_register(rate_reset_nb->target_clk,
+ &rate_reset_nb->clk_nb))
+ rate_reset_nb->is_registered = true;
+}
+
+static struct sun4i_rate_reset_nb tcon_rate_reset_tcon0_nb;
+
+static void sun4i_tcon0_set_dclk_rate(struct sun4i_tcon *tcon, unsigned long rate)
+{
+ clk_set_rate(tcon->dclk, rate);
+
+ if (tcon->quirks->restores_rate) {
+ tcon_rate_reset_tcon0_nb.target_clk = tcon->dclk;
+ tcon_rate_reset_tcon0_nb.saved_rate = rate;
+ sun4i_rate_reset_notifier_register(&tcon_rate_reset_tcon0_nb);
+ }
+}
+
static void sun4i_tcon0_mode_set_cpu(struct sun4i_tcon *tcon,
const struct drm_encoder *encoder,
const struct drm_display_mode *mode)
@@ -360,8 +409,8 @@ static void sun4i_tcon0_mode_set_cpu(struct sun4i_tcon *tcon,
*/
tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
- clk_set_rate(tcon->dclk, mode->crtc_clock * 1000 * (bpp / lanes)
- / SUN6I_DSI_TCON_DIV);
+ sun4i_tcon0_set_dclk_rate(tcon, mode->crtc_clock * 1000 * (bpp / lanes)
+ / SUN6I_DSI_TCON_DIV);

/* Set the resolution */
regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
@@ -434,7 +483,7 @@ static void sun4i_tcon0_mode_set_lvds(struct sun4i_tcon *tcon,

tcon->dclk_min_div = 7;
tcon->dclk_max_div = 7;
- clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
+ sun4i_tcon0_set_dclk_rate(tcon, mode->crtc_clock * 1000);

/* Set the resolution */
regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
@@ -516,7 +565,7 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,

tcon->dclk_min_div = tcon->quirks->dclk_min_div;
tcon->dclk_max_div = 127;
- clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
+ sun4i_tcon0_set_dclk_rate(tcon, mode->crtc_clock * 1000);

/* Set the resolution */
regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
@@ -1503,6 +1552,14 @@ static const struct sun4i_tcon_quirks sun8i_a33_quirks = {
.supports_lvds = true,
};

+static const struct sun4i_tcon_quirks sun50i_a64_lcd_quirks = {
+ .supports_lvds = true,
+ .has_channel_0 = true,
+ .restores_rate = true,
+ .dclk_min_div = 1,
+ .setup_lvds_phy = sun6i_tcon_setup_lvds_phy,
+};
+
static const struct sun4i_tcon_quirks sun8i_a83t_lcd_quirks = {
.supports_lvds = true,
.has_channel_0 = true,
@@ -1561,6 +1618,7 @@ const struct of_device_id sun4i_tcon_of_table[] = {
{ .compatible = "allwinner,sun9i-a80-tcon-tv", .data = &sun9i_a80_tcon_tv_quirks },
{ .compatible = "allwinner,sun20i-d1-tcon-lcd", .data = &sun20i_d1_lcd_quirks },
{ .compatible = "allwinner,sun20i-d1-tcon-tv", .data = &sun8i_r40_tv_quirks },
+ { .compatible = "allwinner,sun50i-a64-tcon-lcd", .data = &sun50i_a64_lcd_quirks },
{ }
};
MODULE_DEVICE_TABLE(of, sun4i_tcon_of_table);
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index fa23aa23fe4a..bd4abc90062b 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -243,6 +243,7 @@ struct sun4i_tcon_quirks {
bool needs_edp_reset; /* a80 edp reset needed for tcon0 access */
bool supports_lvds; /* Does the TCON support an LVDS output? */
bool polarity_in_ch0; /* some tcon1 channels have polarity bits in tcon0 pol register */
+ bool restores_rate; /* restores the initial rate when rate changes */
u8 dclk_min_div; /* minimum divider for TCON0 DCLK */

/* callback to handle tcon muxing options */
@@ -300,4 +301,15 @@ void sun4i_tcon_set_status(struct sun4i_tcon *crtc,

extern const struct of_device_id sun4i_tcon_of_table[];

+struct sun4i_rate_reset_nb {
+ struct notifier_block clk_nb;
+ struct delayed_work reset_rate_work;
+
+ struct clk *target_clk;
+ unsigned long saved_rate;
+ bool is_registered;
+};
+
+#define to_sun4i_rate_reset_nb(_nb) container_of(_nb, struct sun4i_rate_reset_nb, clk_nb)
+
#endif /* __SUN4I_TCON_H__ */

---
base-commit: dcb6c8ee6acc6c347caec1e73fb900c0f4ff9806
change-id: 20240304-tcon_keep_stable_rate-5729c7706343

Best regards,
--
Frank Oltmanns <frank@xxxxxxxxxxxx>