Hi Tomi,
On 14/04/25 16:41, Tomi Valkeinen wrote:
> The driver tries to calculate the value for REG_WAKEUP_TIME. However,
> the calculation itself is not correct, and to add on it, the resulting
> value is almost always larger than the field's size, so the actual
> result is more or less random.>
> According to the docs, figuring out the value for REG_WAKEUP_TIME
> requires HW characterization and there's no way to have a generic
> algorithm to come up with the value. That doesn't help at all...
>
> However, we know that the value must be smaller than the line time, and,
> at least in my understanding, the proper value for it is quite small.
> Testing shows that setting it to 1/10 of the line time seems to work
> well. All video modes from my HDMI monitor work with this algorithm.
>
> Hopefully we'll get more information on how to calculate the value, and
> we can then update this.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
> drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
> index 182845c54c3d..fb0623d3f854 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
> @@ -786,7 +786,13 @@ static void cdns_dsi_bridge_atomic_pre_enable(struct drm_bridge *bridge,
>
> tx_byte_period = DIV_ROUND_DOWN_ULL((u64)NSEC_PER_SEC * 8,
> phy_cfg->hs_clk_rate);
> - reg_wakeup = (phy_cfg->hs_prepare + phy_cfg->hs_zero) / tx_byte_period;
I think the primary point of failure in the original calculation is due
to fact that the hs_prepare and hs_zero are defined in picoseconds (ps),
and the tx_byte_period is in nanoseconds (ns) as evident by the usage of
NSEC_PER_SEC macro.
The resulting tx_by_period is 1000 times smaller, and the reg_wakeup - a
1000 times larger. =)
Further, the TRM does indeed mention that some characterization is
required to fine tune the exact reg_wakeup value, but it ends up giving
a vague-ish formula -
-> reg_wakeup_time = wakeup_time_dsi + wakeup_time_cl + wakeup_time_dl +
(hs_host_eot × 4 / lane_nb)
I think the characterization may only be required for the
wakeup_time_dsi component. The existing formula in the driver (after
corrected for time unit) is the wakeup_time_dl component. wakeup_time_cl
seems to be a range of constants, which the phy-core is auto-settling on
defaults. The document never specifically mentions "hs_host_eot" other
than the equation, but on the off-chance it is same as phy_cfg->eot,
then that's 0 and avoidable.
> +
> + /*
> + * Estimated time [in clock cycles] to perform LP->HS on D-PHY.
> + * It is not clear how to calculate this, so for now,
> + * set it to 1/10 of the total number of clocks in a line.
> + */
> + reg_wakeup = dsi_cfg.htotal / nlanes / 10;
> writel(REG_WAKEUP_TIME(reg_wakeup) | REG_LINE_DURATION(tmp),
> dsi->regs + VID_DPHY_TIME);
>
>
--
Regards
Aradhya