[PATCH] drm/i915/dsi: Fix command mode line time calculation

[PATCH] drm/i915/dsi: Fix command mode line time calculation

[Jinman Ma]

The command mode path in gen11_dsi_set_transcoder_timings() first
truncates the AFE clock period to an integer number of nanoseconds, and
then truncates the resulting line time to microseconds.

At high link rates, 1000000 / afe_clk() can evaluate to zero. Even at
lower rates, the successive integer divisions can make line_time_us
zero. DIV_ROUND_UP(400, line_time_us) then triggers a divide error
during the atomic commit and prevents i915 KMS from initializing.

Calculate the line time directly in nanoseconds using 64-bit arithmetic.
Use bits per pixel with the per-lane AFE bit clock and retain nanosecond
precision until calculating how many lines are needed for 400
microseconds.

Tested on a Huawei MateBook E with an Alder Lake-P GPU and a 2560x1600
dual-link DSI panel. The change prevents the divide error and allows KMS
to reach the display manager.

Signed-off-by: Jinman Ma <justdreemurr@fzu.edu.cn>
---
This does not completely fix the panel. After entering Plasma, the image
still shows visible tearing or jitter, sometimes with an obvious offset
between the two halves of the dual-link panel. Switching to a text
console can also leave the panel black. I am continuing to investigate
this as a separate timing or synchronization issue.

Please let me know which additional i915 debug logs or register dumps
would be most useful for the remaining issue. I'm pleasured to collect them.
---
 drivers/gpu/drm/i915/display/icl_dsi.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
index ea0cdb782..c89e24de7 100644
--- a/drivers/gpu/drm/i915/display/icl_dsi.c
+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
@@ -917,16 +917,19 @@ gen11_dsi_set_transcoder_timings(struct intel_encoder *encoder,
 	if (is_vid_mode(intel_dsi)) {
 		vtotal = adjusted_mode->crtc_vtotal;
 	} else {
-		int bpp, line_time_us, byte_clk_period_ns;
+		u64 line_time_ns;
+		int bpp;
 
 		if (crtc_state->dsc.compression_enable)
 			bpp = fxp_q4_to_int(crtc_state->dsc.compressed_bpp_x16);
 		else
 			bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
 
-		byte_clk_period_ns = 1000000 / afe_clk(encoder, crtc_state);
-		line_time_us = (htotal * (bpp / 8) * byte_clk_period_ns) / (1000 * intel_dsi->lane_count);
-		vtotal = vactive + DIV_ROUND_UP(400, line_time_us);
+		line_time_ns = DIV_ROUND_UP_ULL((u64)htotal * bpp * 1000000,
+						afe_clk(encoder, crtc_state) *
+						intel_dsi->lane_count);
+		vtotal = vactive +
+			DIV_ROUND_UP_ULL(400 * 1000, line_time_ns);
 	}
 	vsync_start = adjusted_mode->crtc_vsync_start;
 	vsync_end = adjusted_mode->crtc_vsync_end;
-- 
2.55.0

Re: [PATCH] drm/i915/dsi: Fix command mode line time calculation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] DIV_ROUND_UP_ULL is incorrectly used with a 64-bit divisor and a 32-bit numerator, which violates the underlying macro contract.

Pre-existing issues:
- [High] The command mode line time calculation fails to account for dual-link DSI panel bandwidth scaling, resulting in a computed line time that is exactly twice the physical line time and halving the requested vertical blanking period.
--

--- Patch [1]: [PATCH] drm/i915/dsi: Fix command mode line time calculation ---
commit edcb713539337dd734175d1e81417b69dd057925
Author: Jinman Ma <justdreemurr@fzu.edu.cn>

drm/i915/dsi: Fix command mode line time calculation

This commit changes the line time calculation for command mode DSI panels
from microseconds to nanoseconds. This prevents divide-by-zero errors during
atomic commits at high link rates.

> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -917,16 +917,19 @@ gen11_dsi_set_transcoder_timings(struct intel_encoder *encoder,
>  	if (is_vid_mode(intel_dsi)) {
>  		vtotal = adjusted_mode->crtc_vtotal;
>  	} else {
> -		int bpp, line_time_us, byte_clk_period_ns;
> +		u64 line_time_ns;
> +		int bpp;
>  
>  		if (crtc_state->dsc.compression_enable)
>  			bpp = fxp_q4_to_int(crtc_state->dsc.compressed_bpp_x16);
>  		else
>  			bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
>  
> -		byte_clk_period_ns = 1000000 / afe_clk(encoder, crtc_state);
> -		line_time_us = (htotal * (bpp / 8) * byte_clk_period_ns) / (1000 * intel_dsi->lane_count);
> -		vtotal = vactive + DIV_ROUND_UP(400, line_time_us);
> +		line_time_ns = DIV_ROUND_UP_ULL((u64)htotal * bpp * 1000000,
> +						afe_clk(encoder, crtc_state) *
> +						intel_dsi->lane_count);

[Severity: High]
This is a pre-existing issue, but does this calculation fail to account for
dual-link DSI panel bandwidth scaling?

Because the calculation uses the full htotal against the single-port
bandwidth, and afe_clk is already halved for dual-link setups during VBT
parsing, this results in a computed line_time_ns that is exactly twice the
physical line time.

Further down in gen11_dsi_set_transcoder_timings(), htotal is properly
halved for dual-link configurations:

    if (intel_dsi->dual_link) {
        hactive /= 2;
        if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK)
            hactive += intel_dsi->pixel_overlap;
        htotal /= 2;
    }

Since this htotal halving happens after line_time_ns is calculated, could
this be requesting half the necessary blanking lines, providing ~200 us
of vertical blanking time instead of the required 400 us? This might explain
the persistent tearing and jitter mentioned in the commit message.

> +		vtotal = vactive +
> +			DIV_ROUND_UP_ULL(400 * 1000, line_time_ns);

[Severity: Low]
Does this usage of DIV_ROUND_UP_ULL() reverse the expected types for the
numerator and denominator?

The DIV_ROUND_UP_ULL() macro is designed for a 64-bit numerator and a 32-bit
denominator. When do_div() is used internally on 32-bit architectures, it
implicitly truncates the denominator to 32 bits.

While line_time_ns easily fits within 32 bits here, so there is no
functional impact, is there a risk of violating the underlying macro
contract by using a 32-bit numerator and a 64-bit divisor?

>  	}
>  	vsync_start = adjusted_mode->crtc_vsync_start;
>  	vsync_end = adjusted_mode->crtc_vsync_end;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803104851.267090-1-justdreemurr@fzu.edu.cn?part=1

[PATCH v2] [PATCH v2] drm/i915/dsi: Fix command mode line time calculation

[Jinman Ma]

Thank you for the review. I verified both reported issues and addressed
them in v2.

For dual-link DSI configurations, intel_dsi->pclk and afe_clk() describe
the per-link bandwidth, while the previous calculation used the full
horizontal total. This made the calculated line time twice the actual
per-link line time and provided only about half of the required 400 us
vertical blanking interval.

v2 now calculates the line time using a per-link horizontal total by
dividing htotal by two for dual-link configurations.

The second DIV_ROUND_UP_ULL() call also used line_time_ns, a u64 value,
as its divisor. This does not match the macro's u32 divisor contract.
v2 replaces it with DIV64_U64_ROUND_UP(), which supports a 64-bit
numerator and divisor.

The changes were tested on a Huawei MateBook E with a Tiger Lake GPU and
a 2560x1600 RGB888 dual-link front-back command-mode DSI panel.

Before v2, the driver programmed a vtotal below vsync_end and reported:

i915 0000:00:02.0: [drm] *ERROR* Invalid vsync_end value
i915 0000:00:02.0: [drm] *ERROR*
[CRTC:171:pipe A] mismatch in hw.pipe_mode.crtc_vtotal
(expected 1710, found 1622)

With v2, the per-link line time is approximately 8.18 us. The driver
requests 49 blanking lines, providing approximately 400.9 us of vertical
blanking and programming vtotal to 1649.

After a clean build and boot:

  - vtotal is programmed to 1649
  - Invalid vsync_end is no longer reported
  - no divide error or kernel Oops occurs
  - no FIFO underrun, GPU hang, or atomic update failure is reported
  - the DSI connector and display pipe initialize successfully

The existing modeset verification differences between the mode timings
and command-mode transcoder timings remain unchanged and are outside the
scope of this patch.


Signed-off-by: Jinman Ma <justdreemurr@fzu.edu.cn>
---

Changes in v2:
  - Use the per-link htotal when calculating dual-link DSI line time.
  - Use DIV64_U64_ROUND_UP() when dividing by the u64 line_time_ns.
  - Preserve the existing bpp selection for compressed and uncompressed
      configurations.

Testing notes:

The corrected dual-link calculation removes the Invalid vsync_end error
and provides the required 400 us vertical blanking interval. A slight
intermittent display jitter is still visible, so the dual-link line-time
error was not the sole cause of the previously reported jitter.

TTY switching and suspend/resume can also leave the internal panel
blank. These appear to involve separate fbcon and DSI command-mode
resume paths and are not addressed by this patch. They will be reported
and investigated separately.

 drivers/gpu/drm/i915/display/icl_dsi.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
index ea0cdb782..0f942cc6e 100644
--- a/drivers/gpu/drm/i915/display/icl_dsi.c
+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
@@ -917,16 +917,24 @@ gen11_dsi_set_transcoder_timings(struct intel_encoder *encoder,
 	if (is_vid_mode(intel_dsi)) {
 		vtotal = adjusted_mode->crtc_vtotal;
 	} else {
-		int bpp, line_time_us, byte_clk_period_ns;
+		int bpp;
+		u64 line_time_ns;
+		u16 link_htotal = htotal;
+
+		if (intel_dsi->dual_link)
+			link_htotal /= 2;
 
 		if (crtc_state->dsc.compression_enable)
 			bpp = fxp_q4_to_int(crtc_state->dsc.compressed_bpp_x16);
 		else
 			bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
+		line_time_ns =
+			DIV_ROUND_UP_ULL((u64)link_htotal * bpp * 1000000,
+					afe_clk(encoder, crtc_state) *
+					intel_dsi->lane_count);
 
-		byte_clk_period_ns = 1000000 / afe_clk(encoder, crtc_state);
-		line_time_us = (htotal * (bpp / 8) * byte_clk_period_ns) / (1000 * intel_dsi->lane_count);
-		vtotal = vactive + DIV_ROUND_UP(400, line_time_us);
+		vtotal = vactive +
+			DIV64_U64_ROUND_UP(400000ULL, line_time_ns);
 	}
 	vsync_start = adjusted_mode->crtc_vsync_start;
 	vsync_end = adjusted_mode->crtc_vsync_end;
-- 
2.55.0

✗ LGCI.VerificationFailed: failure for drm/i915/dsi: Fix command mode line time calculation

[Patchwork]

== Series Details ==

Series: drm/i915/dsi: Fix command mode line time calculation
URL   : https://patchwork.freedesktop.org/series/171619/
State : failure

== Summary ==

Series author address 'justdreemurr@fzu.edu.cn' is not on the allowlist, which prevents CI from being automatically triggered.
If you want CI to run for this series, ask Patchwork project owners to click 'retest' on the series in Patchwork.
Exception occurred during validation, bailing out!
Build URL: http://intel-gfx-ci-public.igk.intel.com:8080/job/xe_pw_trigger/1238132/ (on master)

✗ LGCI.VerificationFailed: failure for drm/i915/dsi: Fix command mode line time calculation

[Patchwork]

== Series Details ==

Series: drm/i915/dsi: Fix command mode line time calculation
URL   : https://patchwork.freedesktop.org/series/171620/
State : failure

== Summary ==

Series author address 'justdreemurr@fzu.edu.cn' is not on the allowlist, which prevents CI from being automatically triggered.
If you want CI to run for this series, ask Patchwork project owners to click 'retest' on the series in Patchwork.
Exception occurred during validation, bailing out!
Build URL: http://gfx-ci.igk.intel.com:8080/job/CI_PW_kernel/183420/ (on built-in)