| Message ID | 20241026041057.247640-1-marex@denx.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | drm/bridge: tc358767: Fix use of unadjusted mode in the driver | expand |
On Sat, 26 Oct 2024 06:10:42 +0200, Marek Vasut wrote: > The driver configures mostly Pixel PLL from the clock cached in > local copy of the mode. Make sure the driver uses adjusted mode > which contains the updated Pixel PLL settings negotiated in > tc_dpi_atomic_check()/tc_edp_atomic_check(). > > Applied, thanks! [1/1] drm/bridge: tc358767: Fix use of unadjusted mode in the driver https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/b59d1d9d9ab2 Rob
Hi Marek, Am Samstag, 26. Oktober 2024, 06:10:42 CET schrieb Marek Vasut: > The driver configures mostly Pixel PLL from the clock cached in > local copy of the mode. Make sure the driver uses adjusted mode > which contains the updated Pixel PLL settings negotiated in > tc_dpi_atomic_check()/tc_edp_atomic_check(). > > Signed-off-by: Marek Vasut <marex@denx.de> > --- > Cc: Andrzej Hajda <andrzej.hajda@intel.com> > Cc: David Airlie <airlied@gmail.com> > Cc: Jernej Skrabec <jernej.skrabec@gmail.com> > Cc: Jonas Karlman <jonas@kwiboo.se> > Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Neil Armstrong <neil.armstrong@linaro.org> > Cc: Robert Foss <rfoss@kernel.org> > Cc: Simona Vetter <simona@ffwll.ch> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: dri-devel@lists.freedesktop.org > --- > drivers/gpu/drm/bridge/tc358767.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c > index 7968183510e63..0d523322fdd8e 100644 > --- a/drivers/gpu/drm/bridge/tc358767.c > +++ b/drivers/gpu/drm/bridge/tc358767.c > @@ -1764,7 +1764,7 @@ static void tc_bridge_mode_set(struct drm_bridge *bridge, > { > struct tc_data *tc = bridge_to_tc(bridge); > > - drm_mode_copy(&tc->mode, mode); > + drm_mode_copy(&tc->mode, adj); > } > > static const struct drm_edid *tc_edid_read(struct drm_bridge *bridge, > This unfortunately breaks my DSI->DP setup on TQMa8MPxL/MBa8MPxL. When I revert it, DP works again. I'm currently running on next-20241114 with patches regarding DSI initialization. before: tc358767 1-000f: PLL: requested 148500000 pixelclock, ref 26000000 tc358767 1-000f: PLL: got 147333333, delta -1166667 tc358767 1-000f: PLL: 26000000 / 1 / 1 * 17 / 3 tc358767 1-000f: set mode 1920x1080 tc358767 1-000f: H margin 148,88 sync 44 tc358767 1-000f: V margin 36,4 sync 5 tc358767 1-000f: total: 2200x1125 after: tc358767 1-000f: PLL: requested 147333000 pixelclock, ref 26000000 tc358767 1-000f: PLL: got 146250000, delta -1083000 tc358767 1-000f: PLL: 26000000 / 1 / 4 * 45 / 2 tc358767 1-000f: set mode 1920x1080 tc358767 1-000f: H margin 148,88 sync 44 tc358767 1-000f: V margin 36,4 sync 5 tc358767 1-000f: total: 2200x1125 The reason this breaks is that the adjusted clock frequency is slightly off to the previously calculated one: 147333333 <-> 147333000 This is because mode clock is only specyfied in KHz. With this incorrect input the new result is even lower than requested, thus failing to setup correctly. As I don't see a quick fix, I propose a revert meanwhile. Any comments? Best regards, Alexander
On 11/14/24 10:27 AM, Alexander Stein wrote: > Hi Marek, Hi, > Am Samstag, 26. Oktober 2024, 06:10:42 CET schrieb Marek Vasut: >> The driver configures mostly Pixel PLL from the clock cached in >> local copy of the mode. Make sure the driver uses adjusted mode >> which contains the updated Pixel PLL settings negotiated in >> tc_dpi_atomic_check()/tc_edp_atomic_check(). >> >> Signed-off-by: Marek Vasut <marex@denx.de> >> --- >> Cc: Andrzej Hajda <andrzej.hajda@intel.com> >> Cc: David Airlie <airlied@gmail.com> >> Cc: Jernej Skrabec <jernej.skrabec@gmail.com> >> Cc: Jonas Karlman <jonas@kwiboo.se> >> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> >> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> Cc: Maxime Ripard <mripard@kernel.org> >> Cc: Neil Armstrong <neil.armstrong@linaro.org> >> Cc: Robert Foss <rfoss@kernel.org> >> Cc: Simona Vetter <simona@ffwll.ch> >> Cc: Thomas Zimmermann <tzimmermann@suse.de> >> Cc: dri-devel@lists.freedesktop.org >> --- >> drivers/gpu/drm/bridge/tc358767.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c >> index 7968183510e63..0d523322fdd8e 100644 >> --- a/drivers/gpu/drm/bridge/tc358767.c >> +++ b/drivers/gpu/drm/bridge/tc358767.c >> @@ -1764,7 +1764,7 @@ static void tc_bridge_mode_set(struct drm_bridge *bridge, >> { >> struct tc_data *tc = bridge_to_tc(bridge); >> >> - drm_mode_copy(&tc->mode, mode); >> + drm_mode_copy(&tc->mode, adj); >> } >> >> static const struct drm_edid *tc_edid_read(struct drm_bridge *bridge, >> > > This unfortunately breaks my DSI->DP setup on TQMa8MPxL/MBa8MPxL. > > When I revert it, DP works again. I'm currently running on next-20241114 > with patches regarding DSI initialization. > > before: > tc358767 1-000f: PLL: requested 148500000 pixelclock, ref 26000000 > tc358767 1-000f: PLL: got 147333333, delta -1166667 > tc358767 1-000f: PLL: 26000000 / 1 / 1 * 17 / 3 > tc358767 1-000f: set mode 1920x1080 > tc358767 1-000f: H margin 148,88 sync 44 > tc358767 1-000f: V margin 36,4 sync 5 > tc358767 1-000f: total: 2200x1125 > > after: > tc358767 1-000f: PLL: requested 147333000 pixelclock, ref 26000000 > tc358767 1-000f: PLL: got 146250000, delta -1083000 > tc358767 1-000f: PLL: 26000000 / 1 / 4 * 45 / 2 > tc358767 1-000f: set mode 1920x1080 > tc358767 1-000f: H margin 148,88 sync 44 > tc358767 1-000f: V margin 36,4 sync 5 > tc358767 1-000f: total: 2200x1125 > > The reason this breaks is that the adjusted clock frequency is slightly off > to the previously calculated one: 147333333 <-> 147333000 > This is because mode clock is only specyfied in KHz. With this incorrect > input the new result is even lower than requested, thus failing to setup > correctly. > > As I don't see a quick fix, I propose a revert meanwhile. Any comments? Sigh, the TC358767 is really a gift that keeps on giving. Either revert it for now, or give me a week or three until I get back to digging in the TC358767. Whichever you prefer. Sorry for the slowness.
diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c index 7968183510e63..0d523322fdd8e 100644 --- a/drivers/gpu/drm/bridge/tc358767.c +++ b/drivers/gpu/drm/bridge/tc358767.c @@ -1764,7 +1764,7 @@ static void tc_bridge_mode_set(struct drm_bridge *bridge, { struct tc_data *tc = bridge_to_tc(bridge); - drm_mode_copy(&tc->mode, mode); + drm_mode_copy(&tc->mode, adj); } static const struct drm_edid *tc_edid_read(struct drm_bridge *bridge,
The driver configures mostly Pixel PLL from the clock cached in local copy of the mode. Make sure the driver uses adjusted mode which contains the updated Pixel PLL settings negotiated in tc_dpi_atomic_check()/tc_edp_atomic_check(). Signed-off-by: Marek Vasut <marex@denx.de> --- Cc: Andrzej Hajda <andrzej.hajda@intel.com> Cc: David Airlie <airlied@gmail.com> Cc: Jernej Skrabec <jernej.skrabec@gmail.com> Cc: Jonas Karlman <jonas@kwiboo.se> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Maxime Ripard <mripard@kernel.org> Cc: Neil Armstrong <neil.armstrong@linaro.org> Cc: Robert Foss <rfoss@kernel.org> Cc: Simona Vetter <simona@ffwll.ch> Cc: Thomas Zimmermann <tzimmermann@suse.de> Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/bridge/tc358767.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)