Thanks for the comment.
I will revise this patch, so the change is only removing POST overriding.
In addition, the patch for removing TRAIL was submitted as
https://patchwork.kernel.org/project/intel-gfx/patch/20211217100903.32599-1-william.ts...@intel.com/.
Can you help to review as well?
-----Original Message-----
From: Ville Syrjälä <ville.syrj...@linux.intel.com>
Sent: Friday, September 1, 2023 6:53 PM
To: Tseng, William <william.ts...@intel.com>
Cc: intel-gfx@lists.freedesktop.org; Jani Nikula <jani.nik...@linux.intel.com>;
Kulkarni, Vandita <vandita.kulka...@intel.com>; Kandpal, Suraj
<suraj.kand...@intel.com>; Lee, Shawn C <shawn.c....@intel.com>
Subject: Re: [PATCH] drm/i915/dsi: let HW maintain HS-TRAIL and CLK_POST
On Fri, Sep 01, 2023 at 05:51:00PM +0800, William Tseng wrote:
> This change is to adjust TEOT timing and TCLK-POST timing so DSI
> signaling can meet CTS specification.
>
> For clock lane, the measured TEOT may be changed from 142.64 ns to
> 107.36 ns, which is less than (105 ns+12*UI) and is conformed to mipi
> D-PHY v1.2 CTS v1.0.
>
> As to TCLK-POST, it may be changed from 133.44 ns to 178.72 ns, which
> is greater than (60 ns+52*UI) and is conformed to the CTS standard.
>
> The computed UI is around 1.47 ns.
>
> Cc: Ville Syrjala <ville.syrj...@linux.intel.com>
> Cc: Jani Nikula <jani.nik...@linux.intel.com>
> Cc: Vandita Kulkarni <vandita.kulka...@intel.com>
> Cc: Suraj Kandpal <suraj.kand...@intel.com>
> Cc: Lee Shawn C <shawn.c....@intel.com>
> Signed-off-by: William Tseng <william.ts...@intel.com>
> ---
> drivers/gpu/drm/i915/display/icl_dsi.c | 31
> ++++----------------------
> 1 file changed, 4 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> b/drivers/gpu/drm/i915/display/icl_dsi.c
> index ad6488e9c2b2..4a13f467ca46 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -1819,10 +1819,10 @@ static void icl_dphy_param_init(struct intel_dsi
> *intel_dsi)
> struct intel_connector *connector = intel_dsi->attached_connector;
> struct mipi_config *mipi_config = connector->panel.vbt.dsi.config;
> u32 tlpx_ns;
> - u32 prepare_cnt, exit_zero_cnt, clk_zero_cnt, trail_cnt;
> - u32 ths_prepare_ns, tclk_trail_ns;
> + u32 prepare_cnt, exit_zero_cnt, clk_zero_cnt;
> + u32 ths_prepare_ns;
> u32 hs_zero_cnt;
> - u32 tclk_pre_cnt, tclk_post_cnt;
> + u32 tclk_pre_cnt;
>
> tlpx_ns = intel_dsi_tlpx_ns(intel_dsi);
>
> @@ -1853,14 +1853,6 @@ static void icl_dphy_param_init(struct intel_dsi
> *intel_dsi)
> clk_zero_cnt = ICL_CLK_ZERO_CNT_MAX;
> }
>
> - /* trail cnt in escape clocks*/
> - trail_cnt = DIV_ROUND_UP(tclk_trail_ns, tlpx_ns);
> - if (trail_cnt > ICL_TRAIL_CNT_MAX) {
> - drm_dbg_kms(&dev_priv->drm, "trail_cnt out of range (%d)\n",
> - trail_cnt);
> - trail_cnt = ICL_TRAIL_CNT_MAX;
> - }
> -
> /* tclk pre count in escape clocks */
> tclk_pre_cnt = DIV_ROUND_UP(mipi_config->tclk_pre, tlpx_ns);
> if (tclk_pre_cnt > ICL_TCLK_PRE_CNT_MAX) { @@ -1869,15 +1861,6 @@
> static void icl_dphy_param_init(struct intel_dsi *intel_dsi)
> tclk_pre_cnt = ICL_TCLK_PRE_CNT_MAX;
> }
>
> - /* tclk post count in escape clocks */
> - tclk_post_cnt = DIV_ROUND_UP(mipi_config->tclk_post, tlpx_ns);
> - if (tclk_post_cnt > ICL_TCLK_POST_CNT_MAX) {
> - drm_dbg_kms(&dev_priv->drm,
> - "tclk_post_cnt out of range (%d)\n",
> - tclk_post_cnt);
> - tclk_post_cnt = ICL_TCLK_POST_CNT_MAX;
> - }
> -
> /* hs zero cnt in escape clocks */
> hs_zero_cnt = DIV_ROUND_UP(mipi_config->ths_prepare_hszero -
> ths_prepare_ns, tlpx_ns);
> @@ -1902,19 +1885,13 @@ static void icl_dphy_param_init(struct intel_dsi
> *intel_dsi)
> CLK_ZERO_OVERRIDE |
> CLK_ZERO(clk_zero_cnt) |
> CLK_PRE_OVERRIDE |
> - CLK_PRE(tclk_pre_cnt) |
> - CLK_POST_OVERRIDE |
> - CLK_POST(tclk_post_cnt) |
> - CLK_TRAIL_OVERRIDE |
> - CLK_TRAIL(trail_cnt));
> + CLK_PRE(tclk_pre_cnt));
Windows seems set these overrides:
icl clk DPHY: PREPARE,ZERO
icl clk DSI: PREPARE,ZERO
icl data DPHY: PREPARE,ZERO,EXIT
icl data DSI: PREPARE,ZERO,EXIT
tgl clk DPHY: PREPARE,ZERO (?)
tgl clk DSI: PREPARE,ZERO,POST (?)
tgl data DPHY: PREPARE,ZERO,EXIT
tgl data DSI: PREPARE,ZERO,EXIT
adl clk DPHY: PREPARE,ZERO (?) (also PRE for 2.0-2.5 GHz data rate)
adl clk DSI: PREPARE,ZERO,POST (?) (also PRE for 2.0-2.5 GHz data rate)
adl data DPHY: PREPARE,ZERO,EXIT
adl data DSI : PREPARE,ZERO,EXIT
Didn't see any justification for the weird mismatch between DSI vs. DPHY POST
override on tgl+.
Anyways, looks like removing TRAIL is not particularly controversial since
Windows also never overrides it. So probably you should split that up into a
separate patch.
>
> /* data lanes dphy timings */
> intel_dsi->dphy_data_lane_reg = (HS_PREPARE_OVERRIDE |
> HS_PREPARE(prepare_cnt) |
> HS_ZERO_OVERRIDE |
> HS_ZERO(hs_zero_cnt) |
> - HS_TRAIL_OVERRIDE |
> - HS_TRAIL(trail_cnt) |
> HS_EXIT_OVERRIDE |
> HS_EXIT(exit_zero_cnt));
>
> --
> 2.34.1
--
Ville Syrjälä
Intel
