Re: [Intel-gfx] [PATCH v3] drm/i915/dsi: let HW maintain the HS-TRAIL timing

["Kulkarni, Vandita" <vandita.kulkarni@intel.com>]

> -----Original Message-----
> From: Tseng, William <william.tseng@intel.com>
> Sent: Friday, December 17, 2021 3:39 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Tseng, William <william.tseng@intel.com>; Ville Syrjala
> <ville.syrjala@linux.intel.com>; Jani Nikula <jani.nikula@linux.intel.com>;
> Kulkarni, Vandita <vandita.kulkarni@intel.com>; Lee, Shawn C
> <shawn.c.lee@intel.com>; Chiou, Cooper <cooper.chiou@intel.com>
> Subject: [PATCH v3] drm/i915/dsi: let HW maintain the HS-TRAIL timing
> 
> 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.
> 

Looks like when we try to convert to esc clocks needed, the value gets rounded ( due to register limitation) and ends up being higher than expected.
Leading to TEOT higher than needed, though we are in limits for individual values.
Right now, in such cases I do not see any option from spec to calculate in a different way like it provided in case of clk_prepare.
This patch is needed until we have proper way to update the trail_cnt in the register.
Until then lets not override the HW defaults.

Acked-by: Vandita Kulkarni <vandita.kulkarni@intel.com>

> 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>
> ---
>  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 168c84a74d30..992e357e3f44 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -1863,14 +1863,13 @@ static void icl_dphy_param_init(struct intel_dsi
> *intel_dsi)
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct mipi_config *mipi_config = dev_priv->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;
> 
>  	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);
> 
> @@ -1897,14 +1896,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) { @@ -1948,17 +1939,13
> @@ static void icl_dphy_param_init(struct intel_dsi *intel_dsi)
>  			       CLK_PRE_OVERRIDE |
>  			       CLK_PRE(tclk_pre_cnt) |
>  			       CLK_POST_OVERRIDE |
> -			       CLK_POST(tclk_post_cnt) |
> -			       CLK_TRAIL_OVERRIDE |
> -			       CLK_TRAIL(trail_cnt));
> +			       CLK_POST(tclk_post_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.17.1