* Re: [PATCH v2] drm/msm/dsi: round 6G byte clock rate to the PLL-achievable value
2026-07-07 1:32 ` [PATCH v2] drm/msm/dsi: round 6G byte clock rate to the PLL-achievable value Kavan Smith
@ 2026-07-12 11:35 ` Dmitry Baryshkov
2026-07-12 23:46 ` Dmitry Baryshkov
1 sibling, 0 replies; 3+ messages in thread
From: Dmitry Baryshkov @ 2026-07-12 11:35 UTC (permalink / raw)
To: Kavan Smith
Cc: robdclark, quic_abhinavk, sean, marijn.suijten, airlied, simona,
linux-arm-msm, dri-devel, freedreno, linux-kernel, stable,
Daniel Mack
On Mon, Jul 06, 2026 at 06:32:40PM -0700, Kavan Smith wrote:
> 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(-)
Don't send next iterations of patches as a reply to the previous thread.
Always start a new thread for the new iteration.
>
> 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);
There is no such code upstream. You are reverting a code from v1, but it
wasn't applied. Please don't send iterative patches (fix for the fix).
I'll drop the chunk while applying, but in future please don't make
such mistakes.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> + 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
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2] drm/msm/dsi: round 6G byte clock rate to the PLL-achievable value
2026-07-07 1:32 ` [PATCH v2] drm/msm/dsi: round 6G byte clock rate to the PLL-achievable value Kavan Smith
2026-07-12 11:35 ` Dmitry Baryshkov
@ 2026-07-12 23:46 ` Dmitry Baryshkov
1 sibling, 0 replies; 3+ messages in thread
From: Dmitry Baryshkov @ 2026-07-12 23:46 UTC (permalink / raw)
To: quic_abhinavk, Rob Clark, Kavan Smith
Cc: sean, marijn.suijten, airlied, simona, linux-arm-msm, dri-devel,
freedreno, linux-kernel, stable, Daniel Mack
On Mon, 06 Jul 2026 18:32:40 -0700, Kavan Smith wrote:
> 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.
>
> [...]
Applied to msm-fixes, thanks!
[1/1] drm/msm/dsi: round 6G byte clock rate to the PLL-achievable value
https://gitlab.freedesktop.org/lumag/msm/-/commit/6cd33b6f4155
Best regards,
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 3+ messages in thread