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.tseng@xxxxxxxxx/. Can you help to review as well? -----Original Message----- From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> Sent: Friday, September 1, 2023 6:53 PM To: Tseng, William <william.tseng@xxxxxxxxx> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>; Kulkarni, Vandita <vandita.kulkarni@xxxxxxxxx>; Kandpal, Suraj <suraj.kandpal@xxxxxxxxx>; Lee, Shawn C <shawn.c.lee@xxxxxxxxx> 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.syrjala@xxxxxxxxxxxxxxx> > Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> > Cc: Vandita Kulkarni <vandita.kulkarni@xxxxxxxxx> > Cc: Suraj Kandpal <suraj.kandpal@xxxxxxxxx> > Cc: Lee Shawn C <shawn.c.lee@xxxxxxxxx> > Signed-off-by: William Tseng <william.tseng@xxxxxxxxx> > --- > 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
