ARM Sunxi Platform Development
 help / color / mirror / Atom feed
From: Maxime Ripard <mripard@kernel.org>
To: "Jernej Škrabec" <jernej.skrabec@gmail.com>
Cc: Frank Oltmanns <frank@oltmanns.dev>, Chen-Yu Tsai <wens@csie.org>,
	 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	 David Airlie <airlied@gmail.com>,
	Daniel Vetter <daniel@ffwll.ch>,
	 Samuel Holland <samuel@sholland.org>,
	Icenowy Zheng <uwu@icenowy.me>, Ondrej Jirman <x@xnux.eu>,
	 dri-devel@lists.freedesktop.org,
	linux-arm-kernel@lists.infradead.org,
	 linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] drm/sun4i: tcon: Support keeping dclk rate upon ancestor clock changes
Date: Thu, 21 Mar 2024 17:24:58 +0100	[thread overview]
Message-ID: <20240321-faithful-caterpillar-of-health-52d87f@houat> (raw)
In-Reply-To: <5448341.Sb9uPGUboI@jernej-laptop>

[-- Attachment #1: Type: text/plain, Size: 3069 bytes --]

On Thu, Mar 14, 2024 at 06:20:58PM +0100, Jernej Škrabec wrote:
> Dne četrtek, 14. marec 2024 ob 15:42:24 CET je Maxime Ripard napisal(a):
> > 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@oltmanns.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.

IIRC, it was because the D-PHY and DSI controller derive from the same
clock, and we needed to make sure setting one wouldn't affect the other
one.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

      reply	other threads:[~2024-03-21 16:25 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-10 13:32 [PATCH] drm/sun4i: tcon: Support keeping dclk rate upon ancestor clock changes Frank Oltmanns
2024-03-10 22:23 ` Ondřej Jirman
2024-03-11  8:53   ` Frank Oltmanns
2024-03-13 18:11 ` Jernej Škrabec
2024-03-14  5:43   ` Frank Oltmanns
2024-03-14 14:42 ` Maxime Ripard
2024-03-14 17:20   ` Jernej Škrabec
2024-03-21 16:24     ` Maxime Ripard [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240321-faithful-caterpillar-of-health-52d87f@houat \
    --to=mripard@kernel.org \
    --cc=airlied@gmail.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=frank@oltmanns.dev \
    --cc=jernej.skrabec@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sunxi@lists.linux.dev \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=samuel@sholland.org \
    --cc=tzimmermann@suse.de \
    --cc=uwu@icenowy.me \
    --cc=wens@csie.org \
    --cc=x@xnux.eu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).