| Message ID | 20250414-cdns-dsi-impro-v3-13-4e52551d4f07@ideasonboard.com |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | drm/bridge: cdns-dsi: Make it work a bit better | expand |
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); > >
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; + + /* + * 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);
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(-)