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 19986C54791 for ; Wed, 13 Mar 2024 18:11:58 +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:References:In-Reply-To: Message-ID:Date: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=TXB2UU+JB/RLXZyS/ZsT9Ld22WQmnWgz9MQsf2j5Tc0=; b=x13m0PIPdbiDIX Sy5y9trOc1TGs2GEx6AiyCnB4C7Zcpy4qVXsGdiIEX4GmRgISTRDpRxZBoJQkpcn+FzBz52rXGQon zl6d/Cnd3/A8c5tjfBeYddtlVnsN4bnXREAm2xmbLS1umMJnEAEeqwGgQk46qSQ/nk/xXPVNu/z+X MUxXRyy+um2lRu7ZQAcPyBBjWcDCKFFdPNH/zRKbZT/jZQ8HFVUNmLXkFDEGlfYBeBtwnLesa03ns ofXD3YNJ88A1MBkFlP0SYNCNTMUZTnrY513vQ186G6lrhdKA5dUNfisA+x37mE+VzxSsUL9hrT6PD Y4g3hZi6G0a6S6VXWmlA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rkT4r-0000000BFAf-2Yg7; Wed, 13 Mar 2024 18:11:45 +0000 Received: from mail-ej1-x62f.google.com ([2a00:1450:4864:20::62f]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rkT4n-0000000BF8q-2lrl for linux-arm-kernel@lists.infradead.org; Wed, 13 Mar 2024 18:11:43 +0000 Received: by mail-ej1-x62f.google.com with SMTP id a640c23a62f3a-a46682e71a9so14536066b.3 for ; Wed, 13 Mar 2024 11:11:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1710353500; x=1710958300; darn=lists.infradead.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=ZW43et20TFanng2wHbIlKUXSGaDrwCfaBPMPmqrkSI8=; b=TsJFRZcvFXgkNKoWgc5lSa+bysDANYnB40rv+5SzcLao6t56pycAjQcwtIMg/Q2Z/j 1Rd0LlQ5AbFoUOd5XpyaJtDeTxi2EFRXlB37V4XMVS2QlWGhVuOQ3rqBGV+arpECHNd7 KfPduEhzkHvLaf2v2oYsrvjgliQT+IznfAHCeZuMs0+hYF1edbKps80AzSICeueY3U1/ 46ykYHxXdxfhGf5EkKqgKiaX+wdCOWfbaqIPUjknTxu016VZAJzIO4PNLWDHK1AZ1W08 Mi8mP1uLgqhyhUR/lfr2AeaDgtTqs5HDGKYekZvPbpcNvEp2EPGiD727RgU+wkmYE4s3 yruQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710353500; x=1710958300; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ZW43et20TFanng2wHbIlKUXSGaDrwCfaBPMPmqrkSI8=; b=wcAvf+0cvZ9PNTeoq34kltCxSxyyhgQ/9Xbk9Dql28g1iDnjWMMiWGqNRotQO2TJGA Ps3DIn5oVnY0aGePSG3Kk/9jaPOEyhgALzbppQNXK9JEY/frRfGiAIe9b2Z021fB79QG G3LPZqWJTMrzxv+5RRuO1zKwpWrwgVbGZuRe9VrQr26o+u7AwCpnjzO64Lby2fEBNxL2 htmhJ0GIYTKt/h8TlWtNFXZhiL60XKp5lQMyYGNj6HENug9dma5c84t4ng1d+fL95siW UFB/2e4jstbfxbYuQKiEV0dY+mZMJ9tmvr7qwEqMpzrY2cR2w08gRq2fCV/05OV1SwHL iBLw== X-Forwarded-Encrypted: i=1; AJvYcCUg/yw+q0urypiMMzjVV9TYQMDtqIj0TwocgYmTkF2cp5Y0YM538RN/zzGVSclwl8Pk+w2//0pGG07y6zhN8XFtL9tQ672V67xFBZ5wDN9WZZEi0RE= X-Gm-Message-State: AOJu0YwkXO+HPa46OKnSle0UF9xMrW9sz3xjpVt+bsr5xoVQhDlKH8aO 96fDDWZmw4BW+lGAJ4fn1B4/Fdknhl5mVF0oDG9y9yP105PYujFD X-Google-Smtp-Source: AGHT+IFScmFk1ldBKJ8z8B585gjb18wAYdKQkZQyPD8jeKv97wE2CSFX+9yQLrXWKs0jhSFx/xnSfQ== X-Received: by 2002:a17:906:24d6:b0:a46:47cf:16bb with SMTP id f22-20020a17090624d600b00a4647cf16bbmr3623131ejb.2.1710353499286; Wed, 13 Mar 2024 11:11:39 -0700 (PDT) Received: from jernej-laptop.localnet (86-58-6-171.dynamic.telemach.net. [86.58.6.171]) by smtp.gmail.com with ESMTPSA id lr1-20020a170906fb8100b00a442e2940fdsm5116418ejb.179.2024.03.13.11.11.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Mar 2024 11:11:38 -0700 (PDT) From: Jernej =?utf-8?B?xaBrcmFiZWM=?= To: Maxime Ripard , Chen-Yu Tsai , Maarten Lankhorst , Thomas Zimmermann , David Airlie , Daniel Vetter , Samuel Holland , Frank Oltmanns Cc: Icenowy Zheng , Ondrej Jirman , dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org, Frank Oltmanns Subject: Re: [PATCH] drm/sun4i: tcon: Support keeping dclk rate upon ancestor clock changes Date: Wed, 13 Mar 2024 19:11:37 +0100 Message-ID: <3793448.kQq0lBPeGt@jernej-laptop> In-Reply-To: <20240310-tcon_keep_stable_rate-v1-1-0296b0a85c02@oltmanns.dev> References: <20240310-tcon_keep_stable_rate-v1-1-0296b0a85c02@oltmanns.dev> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240313_111141_769983_35708D73 X-CRM114-Status: GOOD ( 42.83 ) 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="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Frank! Thanks on tackling this issue. Dne nedelja, 10. marec 2024 ob 14:32:29 CET je Frank Oltmanns napisal(a): > 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 > --- > 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@oltmanns.dev/ > [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)); Do we need that delay though? Since clock is set exclusive on TV TCONs, then it shouldn't be changed. Alternative, simpler variation would be something like this: https://elixir.bootlin.com/linux/v6.8/source/drivers/tty/serial/8250/8250_dw.c#L333 > + > + 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; Is there any specific reason for global variable? Note that R40 and T507 have 2 LCD and 2 TV TCONs. If it's ever used there, it won't fly. Please move to TCON struct. You can drop a few fields for doing so. > + > +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); Can't be registration done at TCON init time? Best regards, Jernej > + } > +} > + > 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, > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel