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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 14 Mar 2024 18:20:58 +0100
From: Jernej Škrabec <jernej.skrabec@...il.com>
To: Frank Oltmanns <frank@...manns.dev>, Maxime Ripard <mripard@...nel.org>
Cc: Chen-Yu Tsai <wens@...e.org>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>,
 Daniel Vetter <daniel@...ll.ch>, Samuel Holland <samuel@...lland.org>,
 Icenowy Zheng <uwu@...nowy.me>, Ondrej Jirman <x@...x.eu>,
 dri-devel@...ts.freedesktop.org, linux-arm-kernel@...ts.infradead.org,
 linux-sunxi@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject:
 Re: [PATCH] drm/sun4i: tcon: Support keeping dclk rate upon ancestor clock
 changes

Dne četrtek, 14. marec 2024 ob 15:42:24 CET je Maxime Ripard napisal(a):
> Hi,
> 
> On Sun, Mar 10, 2024 at 02:32:29PM +0100, Frank Oltmanns wrote:
> > 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@...manns.dev>
> > ---
> > 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.
> 
> Workqueues don't have an upper boundary on when they execute. As we
> discussed multiple times, this should be solved in the clock framework
> itself, not bypassing it.

I think TCON code still needs to be touched due to clk_rate_exclusive_get()
calls which effectively lock whole chain. You can't have both TCONs locking
rate on A64 for this to work correctly.

What was original reason for clk_rate_exclusive_get()? I forgot already.

Best regards,
Jernej

> 
> Maxime
>