[PATCH] drm/msm/dsi: don't re-lock the PHY PLL on every DSI command

[PATCH] drm/msm/dsi: don't re-lock the PHY PLL on every DSI command

[Kavan Smith]

msm_dsi_host_xfer_prepare() runs for every DSI command, including runtime
DCS writes such as backlight (MIPI DCS 0x51), and unconditionally calls
link_clk_set_rate() before enabling the link clocks. On MSM8916 (DSI 6G
v1.3.1) the requested byte-clock rate never exactly equals the DSI PHY
PLL's achievable rate (e.g. 56250000 Hz requested vs 56246337 Hz from the
PLL), so the clk framework treats every call as a rate change and re-locks
the PLL.

During video-mode operation the byte, pixel and byte-intf clocks are
already running and feeding continuous scanout. Re-locking the PLL glitches
that live clock. On a video-mode panel with no internal timing generator
(e.g. samsung,s6d7aa0 / lsl080al03 on the Samsung Galaxy Tab A 8.0, which
regenerates its H/V timing directly from the DSI clock lane) the glitch
makes the panel lose pixel lock, producing ~1 second of displaced/wrapped
scanout on every DCS command. No FIFO underrun or dsi_err_worker error
fires; it is a silent clock-domain glitch. Stock (downstream MDSS) firmware
sends the same 0x51 with CMD_CLK_CTRL, which only refcount-enables the
clocks and never re-runs clk_set_rate, and does not glitch.

The link rate is already programmed at power-on by msm_dsi_host_power_on(),
so skip the redundant re-set once the link is up; init-time transfers
(before power_on) still set the rate.

Link: https://lists.freedesktop.org/archives/dri-devel/2018-April/172218.html
Signed-off-by: Kavan Smith <kavansmith82@gmail.com>
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -2170,7 +2170,17 @@
 	 * mdp clock need to be enabled to receive dsi interrupt
 	 */
 	pm_runtime_get_sync(&msm_host->pdev->dev);
-	cfg_hnd->ops->link_clk_set_rate(msm_host);
+	/*
+	 * Don't re-set the link clock rate when the link is already up. The
+	 * requested byte-clock rate rarely equals the DSI PHY PLL's achievable
+	 * rate, so clk_set_rate() re-locks the PLL on every command; for a
+	 * video-mode panel with no internal timing generator that clock glitch
+	 * makes the panel lose pixel lock mid-scanout (~1s of displaced image on
+	 * each DCS write, e.g. every backlight update). The rate is already set
+	 * at power-on.
+	 */
+	if (!msm_host->power_on)
+		cfg_hnd->ops->link_clk_set_rate(msm_host);
 	cfg_hnd->ops->link_clk_enable(msm_host);
 
 	/* TODO: vote for bus bandwidth */

Re: [PATCH] drm/msm/dsi: don't re-lock the PHY PLL on every DSI command

[Dmitry Baryshkov]

On Mon, Jul 06, 2026 at 11:07:53AM -0700, Kavan Smith wrote:
> msm_dsi_host_xfer_prepare() runs for every DSI command, including runtime
> DCS writes such as backlight (MIPI DCS 0x51), and unconditionally calls
> link_clk_set_rate() before enabling the link clocks. On MSM8916 (DSI 6G
> v1.3.1) the requested byte-clock rate never exactly equals the DSI PHY
> PLL's achievable rate (e.g. 56250000 Hz requested vs 56246337 Hz from the
> PLL), so the clk framework treats every call as a rate change and re-locks
> the PLL.
> 
> During video-mode operation the byte, pixel and byte-intf clocks are
> already running and feeding continuous scanout. Re-locking the PLL glitches
> that live clock. On a video-mode panel with no internal timing generator
> (e.g. samsung,s6d7aa0 / lsl080al03 on the Samsung Galaxy Tab A 8.0, which
> regenerates its H/V timing directly from the DSI clock lane) the glitch
> makes the panel lose pixel lock, producing ~1 second of displaced/wrapped
> scanout on every DCS command. No FIFO underrun or dsi_err_worker error
> fires; it is a silent clock-domain glitch. Stock (downstream MDSS) firmware
> sends the same 0x51 with CMD_CLK_CTRL, which only refcount-enables the
> clocks and never re-runs clk_set_rate, and does not glitch.
> 
> The link rate is already programmed at power-on by msm_dsi_host_power_on(),
> so skip the redundant re-set once the link is up; init-time transfers
> (before power_on) still set the rate.

THe problem is that by definition the transfer callbacks can be sent at
any time, see the comment at mipi_dsi_host_ops. I think the proper fix
would be to call clk_round_rate() beforehand, making the CCF skip all
further rate sets.


> 
> Link: https://lists.freedesktop.org/archives/dri-devel/2018-April/172218.html

This should be Reported-by / Closes. Also probably add Fixes and cc:stable.

> Signed-off-by: Kavan Smith <kavansmith82@gmail.com>
> ---
>  drivers/gpu/drm/msm/dsi/dsi_host.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 

-- 
With best wishes
Dmitry

[PATCH v2] drm/msm/dsi: round 6G byte clock rate to the PLL-achievable value

[Kavan Smith]

MSM8916 runtime DSI commands still go through
msm_dsi_host_xfer_prepare(), which re-applies the link clock rate before
enabling the link clocks. That is fine in principle, but on DSI 6G the
requested byte clock rate often does not exactly match the DSI PHY PLL's
realizable rate. For example, the driver can request 56250000 Hz while the
PLL actually runs at 56246337 Hz.

Because the requested and actual rates differ slightly, every later
link_clk_set_rate() call is treated as a real clock change and re-locks
the PLL. On a video-mode panel without an internal timing generator, such
as samsung,s6d7aa0 / lsl080al03 on MSM8916, that live-clock glitch makes
the panel lose pixel lock and visibly corrupts scanout on each runtime DCS
command, including backlight writes.

Fix this by rounding the computed 6G byte clock rate up front, before it is
stored in msm_host->byte_clk_rate and reused by later transfers. Once the
host carries the PLL-achievable rate instead of the idealized one,
repeated link_clk_set_rate() calls become no-ops in the common clock
framework and no longer re-lock the PLL.

This keeps the normal transfer callback sequencing intact, preserves the
OPP vote path in link_clk_set_rate(), and matches the fix direction
suggested in the original 2018 discussion.

Reported-by: Daniel Mack <daniel@zonque.org>
Closes: https://lore.kernel.org/all/1a682c5b-7fc9-3aaa-120b-64b239a355a3@zonque.org/
Fixes: 6b16f05aa39f ("drm/msm/dsi: Split clk rate setting and enable")
Cc: stable@vger.kernel.org
Signed-off-by: Kavan Smith <kavansmith82@gmail.com>
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index eabdaa4..5119862 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -603,12 +603,24 @@ static void dsi_calc_pclk(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
 
 int dsi_calc_clk_rate_6g(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
 {
+	long rounded_byte_clk_rate;
+
 	if (!msm_host->mode) {
 		pr_err("%s: mode not set\n", __func__);
 		return -EINVAL;
 	}
 
 	dsi_calc_pclk(msm_host, is_bonded_dsi);
+
+	rounded_byte_clk_rate = clk_round_rate(msm_host->byte_clk,
+					       msm_host->byte_clk_rate);
+	if (rounded_byte_clk_rate < 0) {
+		pr_err("%s: failed to round byte clock rate, %ld\n",
+		       __func__, rounded_byte_clk_rate);
+		return rounded_byte_clk_rate;
+	}
+
+	msm_host->byte_clk_rate = rounded_byte_clk_rate;
 	msm_host->esc_clk_rate = clk_get_rate(msm_host->esc_clk);
 	return 0;
 }
@@ -2056,18 +2068,7 @@ int msm_dsi_host_xfer_prepare(struct mipi_dsi_host *host,
 	 * mdp clock need to be enabled to receive dsi interrupt
 	 */
 	pm_runtime_get_sync(&msm_host->pdev->dev);
-	/*
-	 * Do NOT re-set the link clock rate when the link is already up and
-	 * streaming. On MSM8916 the requested byte-clock rate never exactly equals
-	 * the DSI PHY PLL's achievable rate, so clk_set_rate() re-locks the PLL on
-	 * every command. For a video-mode panel with no internal timing generator
-	 * (e.g. s6d7aa0), that clock glitch makes the panel lose pixel lock mid-
-	 * scanout -> ~1s of displaced/wrapped image on every DCS write (backlight).
-	 * The rate is already correct from power-on; downstream MDSS only refcount-
-	 * enables the clocks here (CMD_CLK_CTRL) and never re-sets the rate.
-	 */
-	if (!msm_host->power_on)
-		cfg_hnd->ops->link_clk_set_rate(msm_host);
+	cfg_hnd->ops->link_clk_set_rate(msm_host);
 	cfg_hnd->ops->link_clk_enable(msm_host);
 
 	/* TODO: vote for bus bandwidth */
-- 
2.43.0