Message-ID: <CAHgnY3=bRyAoDDm8dCuT0z7MaF_LzCa-MDYRqQgKVqgVhFcJjw@mail.gmail.com>
Date: Mon, 9 Sep 2024 07:29:50 +0200
From: Daniel Semkowicz <dse@...umatec.com>
To: Michael Walle <mwalle@...nel.org>
Cc: Andrzej Hajda <andrzej.hajda@...el.com>, Neil Armstrong <neil.armstrong@...aro.org>, 
	Robert Foss <rfoss@...nel.org>, Laurent Pinchart <Laurent.pinchart@...asonboard.com>, 
	Jonas Karlman <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>, 
	Chun-Kuang Hu <chunkuang.hu@...nel.org>, Philipp Zabel <p.zabel@...gutronix.de>, 
	Matthias Brugger <matthias.bgg@...il.com>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, Sam Ravnborg <sam@...nborg.org>, 
	Vinay Simha BN <simhavcs@...il.com>, Tony Lindgren <tony@...mide.com>, dri-devel@...ts.freedesktop.org, 
	linux-kernel@...r.kernel.org, linux-mediatek@...ts.infradead.org, 
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 09/20] drm/bridge: tc358775: remove complex vsdelay calculation

On Fri, Sep 6, 2024 at 4:34 PM Michael Walle <mwalle@...nel.org> wrote:
>
> Hi Daniel,
>
> > > To cite the datasheet on VSDELAY:
> > >   During DSI link speed is slower than that of LVDS link’s, data needs
> > >   to be buffer within 775XBG before outputting to prevent data from
> > >   underflow. Register field VPCTRL[VSDELAY] is used to for this purpose
> > >
> > > This driver assumes that the DSI link speed is the pixel clock (as does
> > > every DSI bridge driver), after all the LVDS clock is derived from the
> > > DSI clock. Thus we know for a fact, that the DSI link is not slower than
> > > the LVDS side. Just use the (sane) default value of the bridge and drop
> > > the complicated calculation here.
> >
> > I am not convinced this is a good idea to revert to a default
> > VSdelay value. I tested your patch series with RK3399 platform
> > and default value (5) was not enough there. There was small data
> > underflow visible, resulting in display offset. Removing this patch
> > and using the original calculation formula fixed the problem.
> > The calculated VSDELAY value seems to be a lot bigger than required,
> > but keeps us on the safe side.
>
> Did you use just parts of this series or did you port the "lp11
> notify" mechanism to the rk3399 platform? Please keep in mind, that
> this bridge doesn't really work if the reset isn't deasserted during
> lp-11 mode and lots of odd things happen.

I pulled in your complete series and ported "lp11 notify" to RK3399.
Both reset and standby pins are connected to the bridge on my board.

>
> Also, do you know if you have an EEPROM attached to the bridge or
> does any firmware part initialize that bridge?

The initialization is done only by Linux kernel driver with your patches.

>
> > It looks that hback-porch value for panel is used also on DSI link,
> > effectively delaying hactive data delivered to TC358775 bridge.
> > I suspect this causes the requirement for higher VSDELAY.
>
> It was ages ago since I've worked on this bridge and extensively
> tested and even measured and decoded the DSI link and the LVDS
> stream. But IIRC this delay was only to compensate the difference
> between the DSI clock and the LVDS clock, that is, if you push the
> pixel stream slower into the bridge than the bridge is pushing it
> out to the LVDS panel.
>
> So the back porch should be irrelevant here (?!).

That was also my initial understanding, but for some reason original algorithm
for VSDELAY in tc358775 driver, takes blanking period into calculation.
It does not base only on DSI and LVDS clocks.
Additionally, my experiments show that the default value is too low
in my configuration.

>
> -michael

Kind regards
Daniel

Powered by blists - more mailing lists