* [PATCH v4] drm/i915/dsi: let HW maintain the HS-TRAIL timing [not found] <20211217100903.32599-1-william.tseng@intel.com> @ 2025-03-11 10:06 ` William Tseng 2025-03-13 9:55 ` Jani Nikula 2025-03-28 9:59 ` Jani Nikula 0 siblings, 2 replies; 6+ messages in thread From: William Tseng @ 2025-03-11 10:06 UTC (permalink / raw) To: intel-gfx, intel-xe Cc: William Tseng, Ville Syrjala, Jani Nikula, Vandita Kulkarni, Lee Shawn C, Cooper Chiou This change is to avoid over-specification of the TEOT timing parameter, which is derived from software in current design. Supposed that THS-TRAIL and THS-EXIT have the minimum values, i.e., 60 and 100 in ns. If SW is overriding the HW default, the TEOT value becomes 150 ns, approximately calculated by the following formula. DIV_ROUND_UP(60/50)*50 + DIV_ROUND_UP(100/50))*50/2, where 50 is LP Escape Clock time in ns. The TEOT value 150 ns is larger than the maximum value, around 136 ns if UI is 1.8ns, (105 ns + 12*UI, defined by MIPI DPHY specification). However, the TEOT value will meet the specification if THS-TRAIL is set to the HW default, instead of software overriding. The timing change is made for both data lane and clock lane. v1: initial version. v2: change clock lane dphy timings. v3: remove calculation of trail cnt. v4: rebase. Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13891 Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Vandita Kulkarni <vandita.kulkarni@intel.com> Cc: Lee Shawn C <shawn.c.lee@intel.com> Cc: Cooper Chiou <cooper.chiou@intel.com> Signed-off-by: William Tseng <william.tseng@intel.com> Acked-by: Vandita Kulkarni <vandita.kulkarni@intel.com> --- drivers/gpu/drm/i915/display/icl_dsi.c | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c index 82bf6c654de2..6cf9c9275031 100644 --- a/drivers/gpu/drm/i915/display/icl_dsi.c +++ b/drivers/gpu/drm/i915/display/icl_dsi.c @@ -1846,14 +1846,13 @@ 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; tlpx_ns = intel_dsi_tlpx_ns(intel_dsi); - tclk_trail_ns = max(mipi_config->tclk_trail, mipi_config->ths_trail); ths_prepare_ns = max(mipi_config->ths_prepare, mipi_config->tclk_prepare); @@ -1880,14 +1879,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(display->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) { @@ -1920,17 +1911,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_TRAIL_OVERRIDE | - CLK_TRAIL(trail_cnt)); + CLK_PRE(tclk_pre_cnt)); /* 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v4] drm/i915/dsi: let HW maintain the HS-TRAIL timing 2025-03-11 10:06 ` [PATCH v4] drm/i915/dsi: let HW maintain the HS-TRAIL timing William Tseng @ 2025-03-13 9:55 ` Jani Nikula 2025-03-14 2:23 ` Tseng, William 2025-03-28 9:59 ` Jani Nikula 1 sibling, 1 reply; 6+ messages in thread From: Jani Nikula @ 2025-03-13 9:55 UTC (permalink / raw) To: William Tseng, intel-gfx, intel-xe Cc: William Tseng, Ville Syrjala, Vandita Kulkarni, Lee Shawn C, Cooper Chiou On Tue, 11 Mar 2025, William Tseng <william.tseng@intel.com> wrote: > This change is to avoid over-specification of the TEOT timing > parameter, which is derived from software in current design. > > Supposed that THS-TRAIL and THS-EXIT have the minimum values, > i.e., 60 and 100 in ns. If SW is overriding the HW default, > the TEOT value becomes 150 ns, approximately calculated by > the following formula. > > DIV_ROUND_UP(60/50)*50 + DIV_ROUND_UP(100/50))*50/2, where 50 > is LP Escape Clock time in ns. > > The TEOT value 150 ns is larger than the maximum value, > around 136 ns if UI is 1.8ns, (105 ns + 12*UI, defined by MIPI > DPHY specification). > > However, the TEOT value will meet the specification if THS-TRAIL > is set to the HW default, instead of software overriding. > > The timing change is made for both data lane and clock lane. What does the VBT contain? Granted, the spec only describes the relevant fields as "TClkTrail: Clock Trail value" and "THSTrail: HS Trail value". But we might have panels and setups where we depend on VBT providing the values. So why do we pick and choose some VBT values for overrides and not others? I also don't get why we do this: tclk_trail_ns = max(mipi_config->tclk_trail, mipi_config->ths_trail); and then use that for both. Again, would be interesting to see what VBT has for them. BR, Jani. > > v1: initial version. > v2: change clock lane dphy timings. > v3: remove calculation of trail cnt. > v4: rebase. > > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13891 > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Vandita Kulkarni <vandita.kulkarni@intel.com> > Cc: Lee Shawn C <shawn.c.lee@intel.com> > Cc: Cooper Chiou <cooper.chiou@intel.com> > Signed-off-by: William Tseng <william.tseng@intel.com> > Acked-by: Vandita Kulkarni <vandita.kulkarni@intel.com> > --- > drivers/gpu/drm/i915/display/icl_dsi.c | 19 +++---------------- > 1 file changed, 3 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c > index 82bf6c654de2..6cf9c9275031 100644 > --- a/drivers/gpu/drm/i915/display/icl_dsi.c > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c > @@ -1846,14 +1846,13 @@ 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; > > tlpx_ns = intel_dsi_tlpx_ns(intel_dsi); > > - tclk_trail_ns = max(mipi_config->tclk_trail, mipi_config->ths_trail); > ths_prepare_ns = max(mipi_config->ths_prepare, > mipi_config->tclk_prepare); > > @@ -1880,14 +1879,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(display->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) { > @@ -1920,17 +1911,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_TRAIL_OVERRIDE | > - CLK_TRAIL(trail_cnt)); > + CLK_PRE(tclk_pre_cnt)); > > /* 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)); -- Jani Nikula, Intel ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v4] drm/i915/dsi: let HW maintain the HS-TRAIL timing 2025-03-13 9:55 ` Jani Nikula @ 2025-03-14 2:23 ` Tseng, William 0 siblings, 0 replies; 6+ messages in thread From: Tseng, William @ 2025-03-14 2:23 UTC (permalink / raw) To: Jani Nikula, intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org Cc: Ville Syrjala, Vandita Kulkarni, Lee, Shawn C, Cooper Chiou > -----Original Message----- > From: Jani Nikula <jani.nikula@linux.intel.com> > Sent: Thursday, March 13, 2025 5:55 PM > To: Tseng, William <william.tseng@intel.com>; intel- > gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org > Cc: Tseng, William <william.tseng@intel.com>; Ville Syrjala > <ville.syrjala@linux.intel.com>; Vandita Kulkarni > <vandita.kulkarni@intel.com>; Lee, Shawn C <shawn.c.lee@intel.com>; > Cooper Chiou <cooper.chiou@intel.com> > Subject: Re: [PATCH v4] drm/i915/dsi: let HW maintain the HS-TRAIL timing > > On Tue, 11 Mar 2025, William Tseng <william.tseng@intel.com> wrote: > > This change is to avoid over-specification of the TEOT timing > > parameter, which is derived from software in current design. > > > > Supposed that THS-TRAIL and THS-EXIT have the minimum values, i.e., 60 > > and 100 in ns. If SW is overriding the HW default, the TEOT value > > becomes 150 ns, approximately calculated by the following formula. > > > > DIV_ROUND_UP(60/50)*50 + DIV_ROUND_UP(100/50))*50/2, where 50 > > is LP Escape Clock time in ns. > > > > The TEOT value 150 ns is larger than the maximum value, around 136 ns > > if UI is 1.8ns, (105 ns + 12*UI, defined by MIPI DPHY specification). > > > > However, the TEOT value will meet the specification if THS-TRAIL is > > set to the HW default, instead of software overriding. > > > > The timing change is made for both data lane and clock lane. > > What does the VBT contain? > In VBT, the value of THSTRAIL is set to 60 and TCLKTRAIL is set to 70 for the device with DSI panel. > Granted, the spec only describes the relevant fields as "TClkTrail: > Clock Trail value" and "THSTrail: HS Trail value". > > But we might have panels and setups where we depend on VBT providing the > values. So why do we pick and choose some VBT values for overrides and not > others? > Maybe timing conformance is the consideration which may be subject to the change of data rate. We need to keep it adjustable. > I also don't get why we do this: > > tclk_trail_ns = max(mipi_config->tclk_trail, mipi_config->ths_trail); > The reason might be linked to the minimum values for both TClkTrail and THSTrail, defined in Table 18, MIPI D-PHY version 2.5. We need to get a value above the required minimum value for both timing parameters, via the formula. Regards William > and then use that for both. Again, would be interesting to see what VBT has > for them. > > > BR, > Jani. > > > > > > v1: initial version. > > v2: change clock lane dphy timings. > > v3: remove calculation of trail cnt. > > v4: rebase. > > > > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13891 > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > Cc: Vandita Kulkarni <vandita.kulkarni@intel.com> > > Cc: Lee Shawn C <shawn.c.lee@intel.com> > > Cc: Cooper Chiou <cooper.chiou@intel.com> > > Signed-off-by: William Tseng <william.tseng@intel.com> > > Acked-by: Vandita Kulkarni <vandita.kulkarni@intel.com> > > --- > > drivers/gpu/drm/i915/display/icl_dsi.c | 19 +++---------------- > > 1 file changed, 3 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c > > b/drivers/gpu/drm/i915/display/icl_dsi.c > > index 82bf6c654de2..6cf9c9275031 100644 > > --- a/drivers/gpu/drm/i915/display/icl_dsi.c > > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c > > @@ -1846,14 +1846,13 @@ 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; > > > > tlpx_ns = intel_dsi_tlpx_ns(intel_dsi); > > > > - tclk_trail_ns = max(mipi_config->tclk_trail, mipi_config->ths_trail); > > ths_prepare_ns = max(mipi_config->ths_prepare, > > mipi_config->tclk_prepare); > > > > @@ -1880,14 +1879,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(display->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) { @@ -1920,17 +1911,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_TRAIL_OVERRIDE | > > - CLK_TRAIL(trail_cnt)); > > + CLK_PRE(tclk_pre_cnt)); > > > > /* 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)); > > -- > Jani Nikula, Intel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] drm/i915/dsi: let HW maintain the HS-TRAIL timing 2025-03-11 10:06 ` [PATCH v4] drm/i915/dsi: let HW maintain the HS-TRAIL timing William Tseng 2025-03-13 9:55 ` Jani Nikula @ 2025-03-28 9:59 ` Jani Nikula 2025-03-31 8:07 ` Tseng, William 1 sibling, 1 reply; 6+ messages in thread From: Jani Nikula @ 2025-03-28 9:59 UTC (permalink / raw) To: William Tseng, intel-gfx, intel-xe Cc: William Tseng, Ville Syrjala, Vandita Kulkarni, Lee Shawn C, Cooper Chiou On Tue, 11 Mar 2025, William Tseng <william.tseng@intel.com> wrote: > This change is to avoid over-specification of the TEOT timing > parameter, which is derived from software in current design. > > Supposed that THS-TRAIL and THS-EXIT have the minimum values, > i.e., 60 and 100 in ns. If SW is overriding the HW default, > the TEOT value becomes 150 ns, approximately calculated by > the following formula. > > DIV_ROUND_UP(60/50)*50 + DIV_ROUND_UP(100/50))*50/2, where 50 > is LP Escape Clock time in ns. > > The TEOT value 150 ns is larger than the maximum value, > around 136 ns if UI is 1.8ns, (105 ns + 12*UI, defined by MIPI > DPHY specification). > > However, the TEOT value will meet the specification if THS-TRAIL > is set to the HW default, instead of software overriding. > > The timing change is made for both data lane and clock lane. > > v1: initial version. > v2: change clock lane dphy timings. > v3: remove calculation of trail cnt. > v4: rebase. > > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13891 > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Vandita Kulkarni <vandita.kulkarni@intel.com> > Cc: Lee Shawn C <shawn.c.lee@intel.com> > Cc: Cooper Chiou <cooper.chiou@intel.com> > Signed-off-by: William Tseng <william.tseng@intel.com> > Acked-by: Vandita Kulkarni <vandita.kulkarni@intel.com> Pushed to drm-intel-next. Thanks for the patch, and sorry for the delay. BR, Jani. > --- > drivers/gpu/drm/i915/display/icl_dsi.c | 19 +++---------------- > 1 file changed, 3 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c > index 82bf6c654de2..6cf9c9275031 100644 > --- a/drivers/gpu/drm/i915/display/icl_dsi.c > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c > @@ -1846,14 +1846,13 @@ 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; > > tlpx_ns = intel_dsi_tlpx_ns(intel_dsi); > > - tclk_trail_ns = max(mipi_config->tclk_trail, mipi_config->ths_trail); > ths_prepare_ns = max(mipi_config->ths_prepare, > mipi_config->tclk_prepare); > > @@ -1880,14 +1879,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(display->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) { > @@ -1920,17 +1911,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_TRAIL_OVERRIDE | > - CLK_TRAIL(trail_cnt)); > + CLK_PRE(tclk_pre_cnt)); > > /* 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)); -- Jani Nikula, Intel ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v4] drm/i915/dsi: let HW maintain the HS-TRAIL timing 2025-03-28 9:59 ` Jani Nikula @ 2025-03-31 8:07 ` Tseng, William 2025-04-03 12:25 ` Jani Nikula 0 siblings, 1 reply; 6+ messages in thread From: Tseng, William @ 2025-03-31 8:07 UTC (permalink / raw) To: Jani Nikula, intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org Cc: Ville Syrjala, Vandita Kulkarni, Lee, Shawn C, Cooper Chiou > -----Original Message----- > From: Jani Nikula <jani.nikula@linux.intel.com> > Sent: Friday, March 28, 2025 5:59 PM > To: Tseng, William <william.tseng@intel.com>; intel- > gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org > Cc: Tseng, William <william.tseng@intel.com>; Ville Syrjala > <ville.syrjala@linux.intel.com>; Vandita Kulkarni > <vandita.kulkarni@intel.com>; Lee, Shawn C <shawn.c.lee@intel.com>; > Cooper Chiou <cooper.chiou@intel.com> > Subject: Re: [PATCH v4] drm/i915/dsi: let HW maintain the HS-TRAIL timing > > On Tue, 11 Mar 2025, William Tseng <william.tseng@intel.com> wrote: > > This change is to avoid over-specification of the TEOT timing > > parameter, which is derived from software in current design. > > > > Supposed that THS-TRAIL and THS-EXIT have the minimum values, i.e., 60 > > and 100 in ns. If SW is overriding the HW default, the TEOT value > > becomes 150 ns, approximately calculated by the following formula. > > > > DIV_ROUND_UP(60/50)*50 + DIV_ROUND_UP(100/50))*50/2, where 50 > > is LP Escape Clock time in ns. > > > > The TEOT value 150 ns is larger than the maximum value, around 136 ns > > if UI is 1.8ns, (105 ns + 12*UI, defined by MIPI DPHY specification). > > > > However, the TEOT value will meet the specification if THS-TRAIL is > > set to the HW default, instead of software overriding. > > > > The timing change is made for both data lane and clock lane. > > > > v1: initial version. > > v2: change clock lane dphy timings. > > v3: remove calculation of trail cnt. > > v4: rebase. > > > > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13891 > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > Cc: Vandita Kulkarni <vandita.kulkarni@intel.com> > > Cc: Lee Shawn C <shawn.c.lee@intel.com> > > Cc: Cooper Chiou <cooper.chiou@intel.com> > > Signed-off-by: William Tseng <william.tseng@intel.com> > > Acked-by: Vandita Kulkarni <vandita.kulkarni@intel.com> > > Pushed to drm-intel-next. Thanks for the patch, and sorry for the delay. > > BR, > Jani. > Hi Jani No problem. Thank you so much for your time. Regards William > > --- > > drivers/gpu/drm/i915/display/icl_dsi.c | 19 +++---------------- > > 1 file changed, 3 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c > > b/drivers/gpu/drm/i915/display/icl_dsi.c > > index 82bf6c654de2..6cf9c9275031 100644 > > --- a/drivers/gpu/drm/i915/display/icl_dsi.c > > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c > > @@ -1846,14 +1846,13 @@ 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; > > > > tlpx_ns = intel_dsi_tlpx_ns(intel_dsi); > > > > - tclk_trail_ns = max(mipi_config->tclk_trail, mipi_config->ths_trail); > > ths_prepare_ns = max(mipi_config->ths_prepare, > > mipi_config->tclk_prepare); > > > > @@ -1880,14 +1879,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(display->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) { @@ -1920,17 +1911,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_TRAIL_OVERRIDE | > > - CLK_TRAIL(trail_cnt)); > > + CLK_PRE(tclk_pre_cnt)); > > > > /* 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)); > > -- > Jani Nikula, Intel ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v4] drm/i915/dsi: let HW maintain the HS-TRAIL timing 2025-03-31 8:07 ` Tseng, William @ 2025-04-03 12:25 ` Jani Nikula 0 siblings, 0 replies; 6+ messages in thread From: Jani Nikula @ 2025-04-03 12:25 UTC (permalink / raw) To: 20250311100626.533888-1-william.tseng@intel.com, intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org Cc: Ville Syrjala, Lee, Shawn C, Cooper Chiou On Mon, 31 Mar 2025, "Tseng, William" <william.tseng@intel.com> wrote: > No problem. Thank you so much for your time. I was reading the Windows driver code when reviewing your changes, and spotted some other things. Posted at [1]. I'd appreciate it if you could test or review them. I don't have the hardware... BR, Jani. [1] https://lore.kernel.org/r/cover.1743682608.git.jani.nikula@intel.com -- Jani Nikula, Intel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-04-03 12:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20211217100903.32599-1-william.tseng@intel.com>
2025-03-11 10:06 ` [PATCH v4] drm/i915/dsi: let HW maintain the HS-TRAIL timing William Tseng
2025-03-13 9:55 ` Jani Nikula
2025-03-14 2:23 ` Tseng, William
2025-03-28 9:59 ` Jani Nikula
2025-03-31 8:07 ` Tseng, William
2025-04-03 12:25 ` Jani Nikula
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox