2023-04-27 14:35:18

by Francesco Dolcini

[permalink] [raw]
Subject: [PATCH v1 0/9] drm/bridge: tc358768: various fixes on PLL calculation and DSI timings

From: Francesco Dolcini <[email protected]>

This series includes multiple fixes on the tc358768 parallel RGB to DSI driver.

With the following changes I am able to have a stable display output using a TI
SN65DSI83 (DSI-LVDS bridge) and a 1280 x 800 LVDS display panel and the
register values are coherent with Toshiba documentation and configuration
spreadsheet, I was not able to test any other display sink.

= DSI Video Mode =

The driver uses the MIPI_DSI_MODE_LPM flag not correctly, because of that no HS
Video is sent at all when this flag is set by the DSI slave.

= DSI Timing Parameters =

Multiple DSI timing parameters are not correct and this was leading to black or
not stable images on some display output. The new formulas were verified with
the datasheet and a configuration spread sheet from Toshiba.

I did split the change in multiple commits, I can squash all of them together
if this is considered better for any reason, including bisect-ability.

= PLL computation =

Two issues on the PLL computation, one is a required fix to have the bridge
working when the parallel RGB input width is not 24, the second one is just
following a prescription from the Toshiba documentation. In my test it was not
making any difference.

Francesco Dolcini (9):
drm/bridge: tc358768: always enable HS video mode
drm/bridge: tc358768: fix PLL parameters computation
drm/bridge: tc358768: fix PLL target frequency
drm/bridge: tc358768: fix TCLK_ZEROCNT computation
drm/bridge: tc358768: fix TCLK_TRAILCNT computation
drm/bridge: tc358768: fix THS_ZEROCNT computation
drm/bridge: tc358768: fix TXTAGOCNT computation
drm/bridge: tc358768: fix THS_TRAILCNT computation
drm/bridge: tc358768: remove unused variable

drivers/gpu/drm/bridge/tc358768.c | 53 +++++++++++++++++--------------
1 file changed, 30 insertions(+), 23 deletions(-)

--
2.25.1


2023-04-27 14:35:24

by Francesco Dolcini

[permalink] [raw]
Subject: [PATCH v1 7/9] drm/bridge: tc358768: fix TXTAGOCNT computation

From: Francesco Dolcini <[email protected]>

Correct computation of TXTAGOCNT register.

This register must be set to a value that ensure that the
TTA-GO period = (4 x TLPX)

with the actual value of TTA-GO being

4 x (TXTAGOCNT + 1) x (HSByteClk cycle)

Fixes: ff1ca6397b1d ("drm/bridge: Add tc358768 driver")
Signed-off-by: Francesco Dolcini <[email protected]>
---
drivers/gpu/drm/bridge/tc358768.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
index 36e33cba59a2..854fc04f08d0 100644
--- a/drivers/gpu/drm/bridge/tc358768.c
+++ b/drivers/gpu/drm/bridge/tc358768.c
@@ -795,7 +795,7 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)

/* TXTAGOCNT[26:16] RXTASURECNT[10:0] */
val = tc358768_to_ns((lptxcnt + 1) * dsibclk_nsk * 4);
- val = tc358768_ns_to_cnt(val, dsibclk_nsk) - 1;
+ val = tc358768_ns_to_cnt(val, dsibclk_nsk) / 4 - 1;
val2 = tc358768_ns_to_cnt(tc358768_to_ns((lptxcnt + 1) * dsibclk_nsk),
dsibclk_nsk) - 2;
val = val << 16 | val2;
--
2.25.1

2023-04-27 14:35:28

by Francesco Dolcini

[permalink] [raw]
Subject: [PATCH v1 1/9] drm/bridge: tc358768: always enable HS video mode

From: Francesco Dolcini <[email protected]>

Always enable HS video mode setting the TXMD bit, without this change no
video output is present with DSI sinks that are setting
MIPI_DSI_MODE_LPM flag (tested with LT8912B DSI-HDMI bridge).

Previously the driver was enabling HS mode only when the DSI sink was
not explicitly setting the MIPI_DSI_MODE_LPM, however this is not
correct.

The MIPI_DSI_MODE_LPM is supposed to indicate that the sink is willing
to receive data in low power mode, however clearing the
TC358768_DSI_CONTROL_TXMD bit will make the TC358768 send video in
LP mode that is not the intended behavior.

Fixes: ff1ca6397b1d ("drm/bridge: Add tc358768 driver")
Signed-off-by: Francesco Dolcini <[email protected]>
---
drivers/gpu/drm/bridge/tc358768.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
index 7c0cbe84611b..8f349bf4fc32 100644
--- a/drivers/gpu/drm/bridge/tc358768.c
+++ b/drivers/gpu/drm/bridge/tc358768.c
@@ -866,8 +866,7 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
val = TC358768_DSI_CONFW_MODE_SET | TC358768_DSI_CONFW_ADDR_DSI_CONTROL;
val |= (dsi_dev->lanes - 1) << 1;

- if (!(dsi_dev->mode_flags & MIPI_DSI_MODE_LPM))
- val |= TC358768_DSI_CONTROL_TXMD;
+ val |= TC358768_DSI_CONTROL_TXMD;

if (!(mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS))
val |= TC358768_DSI_CONTROL_HSCKMD;
--
2.25.1

2023-04-27 14:35:28

by Francesco Dolcini

[permalink] [raw]
Subject: [PATCH v1 5/9] drm/bridge: tc358768: fix TCLK_TRAILCNT computation

From: Francesco Dolcini <[email protected]>

Correct computation of TCLK_TRAILCNT register.

The driver does not implement non-continuous clock mode, so the actual
value doesn't make a practical difference yet. However this change also
ensures that the value does not write to reserved registers bits in case
of under/overflow.

This register must be set to a value that ensures that

TCLK-TRAIL > 60ns
and
TEOT <= (105 ns + 12 x UI)

with the actual value of TCLK-TRAIL being

(TCLK_TRAILCNT + (1 to 2)) xHSByteClkCycle +
(2 + (1 to 2)) * HSBYTECLKCycle - (PHY output delay)

with PHY output delay being about

(2 to 3) x MIPIBitClk cycle in the BitClk conversion.

Fixes: ff1ca6397b1d ("drm/bridge: Add tc358768 driver")
Signed-off-by: Francesco Dolcini <[email protected]>
---
drivers/gpu/drm/bridge/tc358768.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
index aff400c36066..360c7c65f8c4 100644
--- a/drivers/gpu/drm/bridge/tc358768.c
+++ b/drivers/gpu/drm/bridge/tc358768.c
@@ -9,6 +9,7 @@
#include <linux/gpio/consumer.h>
#include <linux/i2c.h>
#include <linux/kernel.h>
+#include <linux/minmax.h>
#include <linux/module.h>
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
@@ -638,6 +639,7 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
struct mipi_dsi_device *dsi_dev = priv->output.dev;
unsigned long mode_flags = dsi_dev->mode_flags;
u32 val, val2, lptxcnt, hact, data_type;
+ s32 raw_val;
const struct drm_display_mode *mode;
u32 dsibclk_nsk, dsiclk_nsk, ui_nsk, phy_delay_nsk;
u32 dsiclk, dsibclk, video_start;
@@ -749,9 +751,9 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
dev_dbg(priv->dev, "TCLK_HEADERCNT: 0x%x\n", val);
tc358768_write(priv, TC358768_TCLK_HEADERCNT, val);

- /* TCLK_TRAIL > 60ns + 3*UI */
- val = 60 + tc358768_to_ns(3 * ui_nsk);
- val = tc358768_ns_to_cnt(val, dsibclk_nsk) - 5;
+ /* TCLK_TRAIL > 60ns AND TEOT <= 105 ns + 12*UI */
+ raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(2 * ui_nsk), dsibclk_nsk) - 5;
+ val = clamp(raw_val, 0, 127);
dev_dbg(priv->dev, "TCLK_TRAILCNT: 0x%x\n", val);
tc358768_write(priv, TC358768_TCLK_TRAILCNT, val);

--
2.25.1

2023-04-27 14:35:31

by Francesco Dolcini

[permalink] [raw]
Subject: [PATCH v1 6/9] drm/bridge: tc358768: fix THS_ZEROCNT computation

From: Francesco Dolcini <[email protected]>

Correct computation of THS_ZEROCNT register.

This register must be set to a value that ensure that
THS_PREPARE + THS_ZERO > 145ns + 10*UI

with the actual value of (THS_PREPARE + THS_ZERO) being

((1 to 2) + 1 + (TCLK_ZEROCNT + 1) + (3 to 4)) x ByteClk cycle +
+ HSByteClk x (2 + (1 to 2)) + (PHY delay)

with PHY delay being about

(8 + (5 to 6)) x MIPIBitClk cycle in the BitClk conversion.

Fixes: ff1ca6397b1d ("drm/bridge: Add tc358768 driver")
Signed-off-by: Francesco Dolcini <[email protected]>
---
drivers/gpu/drm/bridge/tc358768.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
index 360c7c65f8c4..36e33cba59a2 100644
--- a/drivers/gpu/drm/bridge/tc358768.c
+++ b/drivers/gpu/drm/bridge/tc358768.c
@@ -760,9 +760,10 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
/* 40ns + 4*UI < THS_PREPARE < 85ns + 6*UI */
val = 50 + tc358768_to_ns(4 * ui_nsk);
val = tc358768_ns_to_cnt(val, dsibclk_nsk) - 1;
- /* THS_ZERO > 145ns + 10*UI */
- val2 = tc358768_ns_to_cnt(145 - tc358768_to_ns(ui_nsk), dsibclk_nsk);
- val |= (val2 - tc358768_to_ns(phy_delay_nsk)) << 8;
+ /* THS_PREPARE + THS_ZERO > 145ns + 10*UI */
+ raw_val = tc358768_ns_to_cnt(145 - tc358768_to_ns(3 * ui_nsk), dsibclk_nsk) - 10;
+ val2 = clamp(raw_val, 0, 127);
+ val |= val2 << 8;
dev_dbg(priv->dev, "THS_HEADERCNT: 0x%x\n", val);
tc358768_write(priv, TC358768_THS_HEADERCNT, val);

--
2.25.1

2023-04-27 14:36:26

by Francesco Dolcini

[permalink] [raw]
Subject: [PATCH v1 8/9] drm/bridge: tc358768: fix THS_TRAILCNT computation

From: Francesco Dolcini <[email protected]>

Correct computation of THS_TRAILCNT register.

This register must be set to a value that ensure that
THS_TRAIL > 60 ns + 4 x UI
and
THS_TRAIL > 8 x UI
and
THS_TRAIL < TEOT
with
TEOT = 105 ns + (12 x UI)

with the actual value of THS_TRAIL being

(1 + THS_TRAILCNT) x ByteClk cycle + ((1 to 2) + 2) xHSBYTECLK cycle +
- (PHY output delay)

with PHY output delay being about

(8 + (5 to 6)) x MIPIBitClk cycle in the BitClk conversion.

Fixes: ff1ca6397b1d ("drm/bridge: Add tc358768 driver")
Signed-off-by: Francesco Dolcini <[email protected]>
---
drivers/gpu/drm/bridge/tc358768.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
index 854fc04f08d0..947c7dca567a 100644
--- a/drivers/gpu/drm/bridge/tc358768.c
+++ b/drivers/gpu/drm/bridge/tc358768.c
@@ -779,9 +779,10 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
dev_dbg(priv->dev, "TCLK_POSTCNT: 0x%x\n", val);
tc358768_write(priv, TC358768_TCLK_POSTCNT, val);

- /* 60ns + 4*UI < THS_PREPARE < 105ns + 12*UI */
- val = tc358768_ns_to_cnt(60 + tc358768_to_ns(15 * ui_nsk),
- dsibclk_nsk) - 5;
+ /* max(60ns + 4*UI, 8*UI) < THS_TRAILCNT < 105ns + 12*UI */
+ raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(18 * ui_nsk),
+ dsibclk_nsk) - 4;
+ val = clamp(raw_val, 0, 15);
dev_dbg(priv->dev, "THS_TRAILCNT: 0x%x\n", val);
tc358768_write(priv, TC358768_THS_TRAILCNT, val);

--
2.25.1

2023-04-27 14:37:18

by Francesco Dolcini

[permalink] [raw]
Subject: [PATCH v1 9/9] drm/bridge: tc358768: remove unused variable

From: Francesco Dolcini <[email protected]>

Remove the unused phy_delay_nsk variable, before it was wrongly used
to compute some register value, the fixed computation is no longer using
it and therefore can be removed.

Signed-off-by: Francesco Dolcini <[email protected]>
---
drivers/gpu/drm/bridge/tc358768.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
index 947c7dca567a..d3af42a16e69 100644
--- a/drivers/gpu/drm/bridge/tc358768.c
+++ b/drivers/gpu/drm/bridge/tc358768.c
@@ -641,7 +641,7 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
u32 val, val2, lptxcnt, hact, data_type;
s32 raw_val;
const struct drm_display_mode *mode;
- u32 dsibclk_nsk, dsiclk_nsk, ui_nsk, phy_delay_nsk;
+ u32 dsibclk_nsk, dsiclk_nsk, ui_nsk;
u32 dsiclk, dsibclk, video_start;
const u32 internal_delay = 40;
int ret, i;
@@ -725,11 +725,9 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
dsibclk);
dsiclk_nsk = (u32)div_u64((u64)1000000000 * TC358768_PRECISION, dsiclk);
ui_nsk = dsiclk_nsk / 2;
- phy_delay_nsk = dsibclk_nsk + 2 * dsiclk_nsk;
dev_dbg(priv->dev, "dsiclk_nsk: %u\n", dsiclk_nsk);
dev_dbg(priv->dev, "ui_nsk: %u\n", ui_nsk);
dev_dbg(priv->dev, "dsibclk_nsk: %u\n", dsibclk_nsk);
- dev_dbg(priv->dev, "phy_delay_nsk: %u\n", phy_delay_nsk);

/* LP11 > 100us for D-PHY Rx Init */
val = tc358768_ns_to_cnt(100 * 1000, dsibclk_nsk) - 1;
--
2.25.1

2023-05-05 17:39:31

by Robert Foss

[permalink] [raw]
Subject: Re: [PATCH v1 1/9] drm/bridge: tc358768: always enable HS video mode

On Thu, Apr 27, 2023 at 4:34 PM Francesco Dolcini <[email protected]> wrote:
>
> From: Francesco Dolcini <[email protected]>
>
> Always enable HS video mode setting the TXMD bit, without this change no
> video output is present with DSI sinks that are setting
> MIPI_DSI_MODE_LPM flag (tested with LT8912B DSI-HDMI bridge).
>
> Previously the driver was enabling HS mode only when the DSI sink was
> not explicitly setting the MIPI_DSI_MODE_LPM, however this is not
> correct.
>
> The MIPI_DSI_MODE_LPM is supposed to indicate that the sink is willing
> to receive data in low power mode, however clearing the
> TC358768_DSI_CONTROL_TXMD bit will make the TC358768 send video in
> LP mode that is not the intended behavior.
>
> Fixes: ff1ca6397b1d ("drm/bridge: Add tc358768 driver")
> Signed-off-by: Francesco Dolcini <[email protected]>
> ---
> drivers/gpu/drm/bridge/tc358768.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
> index 7c0cbe84611b..8f349bf4fc32 100644
> --- a/drivers/gpu/drm/bridge/tc358768.c
> +++ b/drivers/gpu/drm/bridge/tc358768.c
> @@ -866,8 +866,7 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
> val = TC358768_DSI_CONFW_MODE_SET | TC358768_DSI_CONFW_ADDR_DSI_CONTROL;
> val |= (dsi_dev->lanes - 1) << 1;
>
> - if (!(dsi_dev->mode_flags & MIPI_DSI_MODE_LPM))
> - val |= TC358768_DSI_CONTROL_TXMD;
> + val |= TC358768_DSI_CONTROL_TXMD;
>
> if (!(mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS))
> val |= TC358768_DSI_CONTROL_HSCKMD;
> --
> 2.25.1
>

Reviewed-by: Robert Foss <[email protected]>

2023-05-05 17:45:31

by Robert Foss

[permalink] [raw]
Subject: Re: [PATCH v1 5/9] drm/bridge: tc358768: fix TCLK_TRAILCNT computation

On Thu, Apr 27, 2023 at 4:35 PM Francesco Dolcini <[email protected]> wrote:
>
> From: Francesco Dolcini <[email protected]>
>
> Correct computation of TCLK_TRAILCNT register.
>
> The driver does not implement non-continuous clock mode, so the actual
> value doesn't make a practical difference yet. However this change also
> ensures that the value does not write to reserved registers bits in case
> of under/overflow.
>
> This register must be set to a value that ensures that
>
> TCLK-TRAIL > 60ns
> and
> TEOT <= (105 ns + 12 x UI)
>
> with the actual value of TCLK-TRAIL being
>
> (TCLK_TRAILCNT + (1 to 2)) xHSByteClkCycle +
> (2 + (1 to 2)) * HSBYTECLKCycle - (PHY output delay)
>
> with PHY output delay being about
>
> (2 to 3) x MIPIBitClk cycle in the BitClk conversion.
>
> Fixes: ff1ca6397b1d ("drm/bridge: Add tc358768 driver")
> Signed-off-by: Francesco Dolcini <[email protected]>
> ---
> drivers/gpu/drm/bridge/tc358768.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
> index aff400c36066..360c7c65f8c4 100644
> --- a/drivers/gpu/drm/bridge/tc358768.c
> +++ b/drivers/gpu/drm/bridge/tc358768.c
> @@ -9,6 +9,7 @@
> #include <linux/gpio/consumer.h>
> #include <linux/i2c.h>
> #include <linux/kernel.h>
> +#include <linux/minmax.h>
> #include <linux/module.h>
> #include <linux/regmap.h>
> #include <linux/regulator/consumer.h>
> @@ -638,6 +639,7 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
> struct mipi_dsi_device *dsi_dev = priv->output.dev;
> unsigned long mode_flags = dsi_dev->mode_flags;
> u32 val, val2, lptxcnt, hact, data_type;
> + s32 raw_val;
> const struct drm_display_mode *mode;
> u32 dsibclk_nsk, dsiclk_nsk, ui_nsk, phy_delay_nsk;
> u32 dsiclk, dsibclk, video_start;
> @@ -749,9 +751,9 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
> dev_dbg(priv->dev, "TCLK_HEADERCNT: 0x%x\n", val);
> tc358768_write(priv, TC358768_TCLK_HEADERCNT, val);
>
> - /* TCLK_TRAIL > 60ns + 3*UI */
> - val = 60 + tc358768_to_ns(3 * ui_nsk);
> - val = tc358768_ns_to_cnt(val, dsibclk_nsk) - 5;
> + /* TCLK_TRAIL > 60ns AND TEOT <= 105 ns + 12*UI */
> + raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(2 * ui_nsk), dsibclk_nsk) - 5;
> + val = clamp(raw_val, 0, 127);
> dev_dbg(priv->dev, "TCLK_TRAILCNT: 0x%x\n", val);
> tc358768_write(priv, TC358768_TCLK_TRAILCNT, val);
>
> --
> 2.25.1
>

Reviewed-by: Robert Foss <[email protected]>

2023-05-05 18:10:43

by Robert Foss

[permalink] [raw]
Subject: Re: [PATCH v1 7/9] drm/bridge: tc358768: fix TXTAGOCNT computation

On Thu, Apr 27, 2023 at 4:35 PM Francesco Dolcini <[email protected]> wrote:
>
> From: Francesco Dolcini <[email protected]>
>
> Correct computation of TXTAGOCNT register.
>
> This register must be set to a value that ensure that the
> TTA-GO period = (4 x TLPX)
>
> with the actual value of TTA-GO being
>
> 4 x (TXTAGOCNT + 1) x (HSByteClk cycle)
>
> Fixes: ff1ca6397b1d ("drm/bridge: Add tc358768 driver")
> Signed-off-by: Francesco Dolcini <[email protected]>
> ---
> drivers/gpu/drm/bridge/tc358768.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
> index 36e33cba59a2..854fc04f08d0 100644
> --- a/drivers/gpu/drm/bridge/tc358768.c
> +++ b/drivers/gpu/drm/bridge/tc358768.c
> @@ -795,7 +795,7 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
>
> /* TXTAGOCNT[26:16] RXTASURECNT[10:0] */
> val = tc358768_to_ns((lptxcnt + 1) * dsibclk_nsk * 4);
> - val = tc358768_ns_to_cnt(val, dsibclk_nsk) - 1;
> + val = tc358768_ns_to_cnt(val, dsibclk_nsk) / 4 - 1;
> val2 = tc358768_ns_to_cnt(tc358768_to_ns((lptxcnt + 1) * dsibclk_nsk),
> dsibclk_nsk) - 2;
> val = val << 16 | val2;
> --
> 2.25.1
>

Reviewed-by: Robert Foss <[email protected]>

2023-05-05 18:15:30

by Robert Foss

[permalink] [raw]
Subject: Re: [PATCH v1 8/9] drm/bridge: tc358768: fix THS_TRAILCNT computation

On Thu, Apr 27, 2023 at 4:34 PM Francesco Dolcini <[email protected]> wrote:
>
> From: Francesco Dolcini <[email protected]>
>
> Correct computation of THS_TRAILCNT register.
>
> This register must be set to a value that ensure that
> THS_TRAIL > 60 ns + 4 x UI
> and
> THS_TRAIL > 8 x UI
> and
> THS_TRAIL < TEOT
> with
> TEOT = 105 ns + (12 x UI)
>
> with the actual value of THS_TRAIL being
>
> (1 + THS_TRAILCNT) x ByteClk cycle + ((1 to 2) + 2) xHSBYTECLK cycle +
> - (PHY output delay)
>
> with PHY output delay being about
>
> (8 + (5 to 6)) x MIPIBitClk cycle in the BitClk conversion.
>
> Fixes: ff1ca6397b1d ("drm/bridge: Add tc358768 driver")
> Signed-off-by: Francesco Dolcini <[email protected]>
> ---
> drivers/gpu/drm/bridge/tc358768.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
> index 854fc04f08d0..947c7dca567a 100644
> --- a/drivers/gpu/drm/bridge/tc358768.c
> +++ b/drivers/gpu/drm/bridge/tc358768.c
> @@ -779,9 +779,10 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
> dev_dbg(priv->dev, "TCLK_POSTCNT: 0x%x\n", val);
> tc358768_write(priv, TC358768_TCLK_POSTCNT, val);
>
> - /* 60ns + 4*UI < THS_PREPARE < 105ns + 12*UI */
> - val = tc358768_ns_to_cnt(60 + tc358768_to_ns(15 * ui_nsk),
> - dsibclk_nsk) - 5;
> + /* max(60ns + 4*UI, 8*UI) < THS_TRAILCNT < 105ns + 12*UI */
> + raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(18 * ui_nsk),
> + dsibclk_nsk) - 4;
> + val = clamp(raw_val, 0, 15);
> dev_dbg(priv->dev, "THS_TRAILCNT: 0x%x\n", val);
> tc358768_write(priv, TC358768_THS_TRAILCNT, val);
>
> --
> 2.25.1
>

Reviewed-by: Robert Foss <[email protected]>

2023-05-05 18:21:59

by Robert Foss

[permalink] [raw]
Subject: Re: [PATCH v1 9/9] drm/bridge: tc358768: remove unused variable

On Thu, Apr 27, 2023 at 4:34 PM Francesco Dolcini <[email protected]> wrote:
>
> From: Francesco Dolcini <[email protected]>
>
> Remove the unused phy_delay_nsk variable, before it was wrongly used
> to compute some register value, the fixed computation is no longer using
> it and therefore can be removed.
>
> Signed-off-by: Francesco Dolcini <[email protected]>
> ---
> drivers/gpu/drm/bridge/tc358768.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
> index 947c7dca567a..d3af42a16e69 100644
> --- a/drivers/gpu/drm/bridge/tc358768.c
> +++ b/drivers/gpu/drm/bridge/tc358768.c
> @@ -641,7 +641,7 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
> u32 val, val2, lptxcnt, hact, data_type;
> s32 raw_val;
> const struct drm_display_mode *mode;
> - u32 dsibclk_nsk, dsiclk_nsk, ui_nsk, phy_delay_nsk;
> + u32 dsibclk_nsk, dsiclk_nsk, ui_nsk;
> u32 dsiclk, dsibclk, video_start;
> const u32 internal_delay = 40;
> int ret, i;
> @@ -725,11 +725,9 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
> dsibclk);
> dsiclk_nsk = (u32)div_u64((u64)1000000000 * TC358768_PRECISION, dsiclk);
> ui_nsk = dsiclk_nsk / 2;
> - phy_delay_nsk = dsibclk_nsk + 2 * dsiclk_nsk;
> dev_dbg(priv->dev, "dsiclk_nsk: %u\n", dsiclk_nsk);
> dev_dbg(priv->dev, "ui_nsk: %u\n", ui_nsk);
> dev_dbg(priv->dev, "dsibclk_nsk: %u\n", dsibclk_nsk);
> - dev_dbg(priv->dev, "phy_delay_nsk: %u\n", phy_delay_nsk);
>
> /* LP11 > 100us for D-PHY Rx Init */
> val = tc358768_ns_to_cnt(100 * 1000, dsibclk_nsk) - 1;
> --
> 2.25.1
>

Reviewed-by: Robert Foss <[email protected]>

2023-05-05 18:32:36

by Robert Foss

[permalink] [raw]
Subject: Re: [PATCH v1 0/9] drm/bridge: tc358768: various fixes on PLL calculation and DSI timings

From: Robert Foss <[email protected]>

On Thu, 27 Apr 2023 16:29:25 +0200, Francesco Dolcini wrote:
> From: Francesco Dolcini <[email protected]>
>
> This series includes multiple fixes on the tc358768 parallel RGB to DSI driver.
>
> With the following changes I am able to have a stable display output using a TI
> SN65DSI83 (DSI-LVDS bridge) and a 1280 x 800 LVDS display panel and the
> register values are coherent with Toshiba documentation and configuration
> spreadsheet, I was not able to test any other display sink.
>
> [...]

Applied, thanks!

[1/9] drm/bridge: tc358768: always enable HS video mode
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ee18698e212b
[2/9] drm/bridge: tc358768: fix PLL parameters computation
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ee18698e212b
[3/9] drm/bridge: tc358768: fix PLL target frequency
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ee18698e212b
[4/9] drm/bridge: tc358768: fix TCLK_ZEROCNT computation
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ee18698e212b
[5/9] drm/bridge: tc358768: fix TCLK_TRAILCNT computation
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ee18698e212b
[6/9] drm/bridge: tc358768: fix THS_ZEROCNT computation
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=77a089328da7
[7/9] drm/bridge: tc358768: fix TXTAGOCNT computation
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=3666aad8185a
[8/9] drm/bridge: tc358768: fix THS_TRAILCNT computation
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=bac7842cd179
[9/9] drm/bridge: tc358768: remove unused variable
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=e4a5e4442a80



Rob

2023-05-05 18:40:11

by Robert Foss

[permalink] [raw]
Subject: Re: [PATCH v1 6/9] drm/bridge: tc358768: fix THS_ZEROCNT computation

On Thu, Apr 27, 2023 at 4:33 PM Francesco Dolcini <[email protected]> wrote:
>
> From: Francesco Dolcini <[email protected]>
>
> Correct computation of THS_ZEROCNT register.
>
> This register must be set to a value that ensure that
> THS_PREPARE + THS_ZERO > 145ns + 10*UI
>
> with the actual value of (THS_PREPARE + THS_ZERO) being
>
> ((1 to 2) + 1 + (TCLK_ZEROCNT + 1) + (3 to 4)) x ByteClk cycle +
> + HSByteClk x (2 + (1 to 2)) + (PHY delay)
>
> with PHY delay being about
>
> (8 + (5 to 6)) x MIPIBitClk cycle in the BitClk conversion.
>
> Fixes: ff1ca6397b1d ("drm/bridge: Add tc358768 driver")
> Signed-off-by: Francesco Dolcini <[email protected]>
> ---
> drivers/gpu/drm/bridge/tc358768.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
> index 360c7c65f8c4..36e33cba59a2 100644
> --- a/drivers/gpu/drm/bridge/tc358768.c
> +++ b/drivers/gpu/drm/bridge/tc358768.c
> @@ -760,9 +760,10 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
> /* 40ns + 4*UI < THS_PREPARE < 85ns + 6*UI */
> val = 50 + tc358768_to_ns(4 * ui_nsk);
> val = tc358768_ns_to_cnt(val, dsibclk_nsk) - 1;
> - /* THS_ZERO > 145ns + 10*UI */
> - val2 = tc358768_ns_to_cnt(145 - tc358768_to_ns(ui_nsk), dsibclk_nsk);
> - val |= (val2 - tc358768_to_ns(phy_delay_nsk)) << 8;
> + /* THS_PREPARE + THS_ZERO > 145ns + 10*UI */
> + raw_val = tc358768_ns_to_cnt(145 - tc358768_to_ns(3 * ui_nsk), dsibclk_nsk) - 10;
> + val2 = clamp(raw_val, 0, 127);
> + val |= val2 << 8;
> dev_dbg(priv->dev, "THS_HEADERCNT: 0x%x\n", val);
> tc358768_write(priv, TC358768_THS_HEADERCNT, val);
>
> --
> 2.25.1
>

Reviewed-by: Robert Foss <[email protected]>