* [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup
@ 2024-05-06 13:34 Michael Walle
2024-05-06 13:34 ` [PATCH 01/20] drm/bridge: add dsi_lp11_notify mechanism Michael Walle
` (20 more replies)
0 siblings, 21 replies; 25+ messages in thread
From: Michael Walle @ 2024-05-06 13:34 UTC (permalink / raw
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren
Cc: Daniel Semkowicz, dri-devel, linux-kernel, linux-mediatek,
linux-arm-kernel, Michael Walle
This patchset fixes the bridge initialization according to the
datasheet. Not sure how that even worked before. Maybe because the
initialization was done prior to linux (?).
The bridge has some peculiarities:
(1) The reset has to be deasserted in LP-11 mode
(2) For I2C access the bridge needs the DSI clock
(3) The bridge has to be configured while the video stream is
disabled.
(4) The bridge has limitations on the display timings. In particular,
the horizontal pulse width has to be at least 8 pixels wide and
both the horizontal pulse width as well as the back porch has to
be even. According to the datasheet the horizontal front porch as
well but in line sync mode, this is ignored. Also line sync is the
only supported mode for this bridge, therefore, the front porch
is always ignored.
The most controversial patch is probably "drm/bridge: add
dsi_lp11_notify mechanism" which is needed for (1) above. Some time ago
there was a series [1] to add a manual power-up, which was abandoned and
which didn't suite the needs for this bridge anyway.
Also, this will gradually change the tc_ prefix to tc358775_ while the
functions are refactored.
The bridge was successfully tested on a Mediatek MT8195 SoC with the
following panels:
- Innolux G101ICE
- AUO G121EAN01.0
- Innolux G156HCE (dual-link LVDS)
[1] https://lore.kernel.org/r/20231016165355.1327217-1-dmitry.baryshkov@linaro.org/
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
Michael Walle (20):
drm/bridge: add dsi_lp11_notify mechanism
drm/mediatek: dsi: provide LP-11 mode during .pre_enable
drm/mediatek: dsi: add support for .dsi_lp11_notity()
drm/bridge: tc358775: fix regulator supply id
drm/bridge: tc358775: add crtc modes fixup
drm/bridge: tc358775: redefine LV_MX()
drm/bridge: tc358775: use regmap instead of open coded access functions
drm/bridge: tc358775: remove error message if regulator is missing
drm/bridge: tc358775: remove complex vsdelay calculation
drm/bridge: tc358775: simplify lvds_link property
drm/bridge: tc358775: reformat weird indentation
drm/bridge: tc358775: correctly configure LVDS clock
drm/bridge: tc358775: split the init code
drm/bridge: tc358775: configure PLL depending on the LVDS clock
drm/bridge: tc358775: dynamically configure DSI link settings
drm/bridge: tc358775: use proper defines to configure LVDS timings
drm/bridge: tc358775: move bridge power up/down into functions
drm/bridge: tc358775: fix the power-up/down delays
drm/bridge: tc358775: fix power-up sequencing
drm/bridge: tc358775: use devm_drm_bridge_add()
drivers/gpu/drm/bridge/Kconfig | 1 +
drivers/gpu/drm/bridge/tc358775.c | 601 ++++++++++++++++++++-----------------
drivers/gpu/drm/drm_bridge.c | 16 +
drivers/gpu/drm/mediatek/mtk_dsi.c | 8 +-
include/drm/drm_bridge.h | 12 +
5 files changed, 355 insertions(+), 283 deletions(-)
---
base-commit: 9221b2819b8a4196eecf5476d66201be60fbcf29
change-id: 20240506-tc358775-fix-powerup-53f490043179
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 01/20] drm/bridge: add dsi_lp11_notify mechanism
2024-05-06 13:34 [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
@ 2024-05-06 13:34 ` Michael Walle
2024-05-07 8:37 ` Alexander Stein
2024-05-06 13:34 ` [PATCH 02/20] drm/mediatek: dsi: provide LP-11 mode during .pre_enable Michael Walle
` (19 subsequent siblings)
20 siblings, 1 reply; 25+ messages in thread
From: Michael Walle @ 2024-05-06 13:34 UTC (permalink / raw
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren
Cc: Daniel Semkowicz, dri-devel, linux-kernel, linux-mediatek,
linux-arm-kernel, Michael Walle
Some bridges have very strict power-up reqirements. In this case, the
Toshiba TC358775. The reset has to be deasserted while *both* the DSI
clock and DSI data lanes are in LP-11 mode. After the reset is relased,
the bridge needs the DSI clock to actually be able to process I2C
access. This access will configure the DSI side of the bridge during
which the DSI data lanes have to be in LP-11 mode. After everything is
configured the video stream can finally be enabled.
This means:
(1) The bridge has to be configured completely in .pre_enable() op
(with the clock turned on and data lanes in LP-11 mode, thus
.pre_enable_prev_first has to be set).
(2) The bridge will enable its output in the .enable() op
(3) There must be some mechanism before (1) where the bridge can
release its reset while the clock lane is still in LP-11 mode.
Unfortunately, (3) is crucial for a correct operation of the bridge.
To satisfy this requriment, introduce a new callback .dsi_lp11_notify()
which will be called by the DSI host driver.
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
drivers/gpu/drm/drm_bridge.c | 16 ++++++++++++++++
include/drm/drm_bridge.h | 12 ++++++++++++
2 files changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 28abe9aa99ca..98cd6558aecb 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -1339,6 +1339,22 @@ void drm_bridge_hpd_notify(struct drm_bridge *bridge,
}
EXPORT_SYMBOL_GPL(drm_bridge_hpd_notify);
+/**
+ * drm_bridge_dsi_lp11_notify - notify clock/data lanes LP-11 mode
+ * @bridge: bridge control structure
+ *
+ * DSI host drivers shall call this function while the clock and data lanes
+ * are still in LP-11 mode.
+ *
+ * This function shall be called in a context that can sleep.
+ */
+void drm_bridge_dsi_lp11_notify(struct drm_bridge *bridge)
+{
+ if (bridge->funcs->dsi_lp11_notify)
+ bridge->funcs->dsi_lp11_notify(bridge);
+}
+EXPORT_SYMBOL_GPL(drm_bridge_dsi_lp11_notify);
+
#ifdef CONFIG_OF
/**
* of_drm_find_bridge - find the bridge corresponding to the device node in
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 4baca0d9107b..4ef61274e0a8 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -630,6 +630,17 @@ struct drm_bridge_funcs {
*/
void (*hpd_disable)(struct drm_bridge *bridge);
+ /**
+ * dsi_lp11_notify:
+ *
+ * Will be called by the DSI host driver while both the DSI clock
+ * lane as well as the DSI data lanes are in LP-11 mode. Some bridges
+ * need this state while releasing the reset, for example.
+ * Not all DSI host drivers will support this. Therefore, the DSI
+ * bridge driver must not rely on this op to be called.
+ */
+ void (*dsi_lp11_notify)(struct drm_bridge *bridge);
+
/**
* @debugfs_init:
*
@@ -898,6 +909,7 @@ void drm_bridge_hpd_enable(struct drm_bridge *bridge,
void drm_bridge_hpd_disable(struct drm_bridge *bridge);
void drm_bridge_hpd_notify(struct drm_bridge *bridge,
enum drm_connector_status status);
+void drm_bridge_dsi_lp11_notify(struct drm_bridge *bridge);
#ifdef CONFIG_DRM_PANEL_BRIDGE
bool drm_bridge_is_panel(const struct drm_bridge *bridge);
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 02/20] drm/mediatek: dsi: provide LP-11 mode during .pre_enable
2024-05-06 13:34 [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
2024-05-06 13:34 ` [PATCH 01/20] drm/bridge: add dsi_lp11_notify mechanism Michael Walle
@ 2024-05-06 13:34 ` Michael Walle
2024-05-06 13:34 ` [PATCH 03/20] drm/mediatek: dsi: add support for .dsi_lp11_notity() Michael Walle
` (18 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Michael Walle @ 2024-05-06 13:34 UTC (permalink / raw
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren
Cc: Daniel Semkowicz, dri-devel, linux-kernel, linux-mediatek,
linux-arm-kernel, Michael Walle
As per specification in drivers/gpu/drm/drm_bridge.c the data lanes
should be in LP-11 mode after .pre_enable() has been run. HS mode of the
data lanes are enabled with mtk_dsi_start(). Therefore, move that call
to the .enable() callback.
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index c255559cc56e..ed45c9cc3137 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -711,8 +711,6 @@ static void mtk_output_dsi_enable(struct mtk_dsi *dsi)
mtk_dsi_set_mode(dsi);
mtk_dsi_clk_hs_mode(dsi, 1);
- mtk_dsi_start(dsi);
-
dsi->enabled = true;
}
@@ -759,7 +757,7 @@ static void mtk_dsi_bridge_atomic_enable(struct drm_bridge *bridge,
if (dsi->refcount == 0)
return;
- mtk_output_dsi_enable(dsi);
+ mtk_dsi_start(dsi);
}
static void mtk_dsi_bridge_atomic_pre_enable(struct drm_bridge *bridge,
@@ -771,6 +769,9 @@ static void mtk_dsi_bridge_atomic_pre_enable(struct drm_bridge *bridge,
ret = mtk_dsi_poweron(dsi);
if (ret < 0)
DRM_ERROR("failed to power on dsi\n");
+
+ /* Enter LP-11 state */
+ mtk_output_dsi_enable(dsi);
}
static void mtk_dsi_bridge_atomic_post_disable(struct drm_bridge *bridge,
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 03/20] drm/mediatek: dsi: add support for .dsi_lp11_notity()
2024-05-06 13:34 [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
2024-05-06 13:34 ` [PATCH 01/20] drm/bridge: add dsi_lp11_notify mechanism Michael Walle
2024-05-06 13:34 ` [PATCH 02/20] drm/mediatek: dsi: provide LP-11 mode during .pre_enable Michael Walle
@ 2024-05-06 13:34 ` Michael Walle
2024-05-06 13:34 ` [PATCH 04/20] drm/bridge: tc358775: fix regulator supply id Michael Walle
` (17 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Michael Walle @ 2024-05-06 13:34 UTC (permalink / raw
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren
Cc: Daniel Semkowicz, dri-devel, linux-kernel, linux-mediatek,
linux-arm-kernel, Michael Walle
drm_bridge_dsi_lp11_notify() shall be called while both the clock and
data lanes are still in LP-11 mode. Add the callback.
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index ed45c9cc3137..d4a5a2bd591a 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -709,6 +709,7 @@ static void mtk_output_dsi_enable(struct mtk_dsi *dsi)
mtk_dsi_lane_ready(dsi);
mtk_dsi_set_mode(dsi);
+ drm_bridge_dsi_lp11_notify(dsi->next_bridge);
mtk_dsi_clk_hs_mode(dsi, 1);
dsi->enabled = true;
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 04/20] drm/bridge: tc358775: fix regulator supply id
2024-05-06 13:34 [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
` (2 preceding siblings ...)
2024-05-06 13:34 ` [PATCH 03/20] drm/mediatek: dsi: add support for .dsi_lp11_notity() Michael Walle
@ 2024-05-06 13:34 ` Michael Walle
2024-05-06 13:34 ` [PATCH 05/20] drm/bridge: tc358775: add crtc modes fixup Michael Walle
` (16 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Michael Walle @ 2024-05-06 13:34 UTC (permalink / raw
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren
Cc: Daniel Semkowicz, dri-devel, linux-kernel, linux-mediatek,
linux-arm-kernel, Michael Walle
The regulator id is given without the "-supply" postfix. With that
fixed, the driver will look up the correct regulator from the device
tree.
Fixes: b26975593b17 ("display/drm/bridge: TC358775 DSI/LVDS driver")
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
drivers/gpu/drm/bridge/tc358775.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
index 3b7cc3be2ccd..980f71ea5a6a 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -680,14 +680,14 @@ static int tc_probe(struct i2c_client *client)
if (ret)
return ret;
- tc->vddio = devm_regulator_get(dev, "vddio-supply");
+ tc->vddio = devm_regulator_get(dev, "vddio");
if (IS_ERR(tc->vddio)) {
ret = PTR_ERR(tc->vddio);
dev_err(dev, "vddio-supply not found\n");
return ret;
}
- tc->vdd = devm_regulator_get(dev, "vdd-supply");
+ tc->vdd = devm_regulator_get(dev, "vdd");
if (IS_ERR(tc->vdd)) {
ret = PTR_ERR(tc->vdd);
dev_err(dev, "vdd-supply not found\n");
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 05/20] drm/bridge: tc358775: add crtc modes fixup
2024-05-06 13:34 [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
` (3 preceding siblings ...)
2024-05-06 13:34 ` [PATCH 04/20] drm/bridge: tc358775: fix regulator supply id Michael Walle
@ 2024-05-06 13:34 ` Michael Walle
2024-05-06 13:34 ` [PATCH 06/20] drm/bridge: tc358775: redefine LV_MX() Michael Walle
` (15 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Michael Walle @ 2024-05-06 13:34 UTC (permalink / raw
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren
Cc: Daniel Semkowicz, dri-devel, linux-kernel, linux-mediatek,
linux-arm-kernel, Michael Walle
The bridge has some limitations regarding the horizontal display
timings. In particular, the pulse width has to be at least 8 pixels
and all horizontal timings have to be a multiple of two pixels, except
for the front porch which is ignored by the bridge anyway.
To accommodate that, add pixels to the pulse width and the back porch
until these requirements are satisfied. The added pixels are then
substracted from the front porch so we don't actually change the pixel
clock (or framerate).
Fixes: b26975593b17 ("display/drm/bridge: TC358775 DSI/LVDS driver")
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
drivers/gpu/drm/bridge/tc358775.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
index 980f71ea5a6a..720c0d63fd6a 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -502,6 +502,37 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
d2l_write(tc->i2c, LVCFG, val);
}
+/*
+ * According to the datasheet, the horizontal back porch, front porch and sync
+ * length must be a multiple of 2 and the minimal horizontal pulse width is 8.
+ * To workaround this, we modify the back porch and the sync pulse width by
+ * adding enough pixels. These pixels will then be substracted from the front
+ * porch which is ignored by the bridge. Hopefully, this marginal modified
+ * timing is tolerated by the panel. The alternative is either a black screen
+ * (if the sync pulse width is too short or a shifted picture if the lengths
+ * are not even).
+ */
+static bool tc_mode_fixup(struct drm_bridge *bridge,
+ const struct drm_display_mode *mode,
+ struct drm_display_mode *adj)
+{
+ u16 hsync_len, hback_porch;
+
+ hback_porch = adj->htotal - adj->hsync_end;
+ if (hback_porch & 1) {
+ adj->hsync_end -= 1;
+ adj->hsync_start -= 1;
+ }
+
+ hsync_len = adj->hsync_end - adj->hsync_start;
+ if (hsync_len < 8)
+ adj->hsync_start -= 8 - hsync_len;
+ else if (hsync_len & 1)
+ adj->hsync_start -= 1;
+
+ return adj->hsync_start >= adj->hdisplay;
+}
+
static enum drm_mode_status
tc_mode_valid(struct drm_bridge *bridge,
const struct drm_display_info *info,
@@ -603,6 +634,7 @@ static const struct drm_bridge_funcs tc_bridge_funcs = {
.attach = tc_bridge_attach,
.pre_enable = tc_bridge_pre_enable,
.enable = tc_bridge_enable,
+ .mode_fixup = tc_mode_fixup,
.mode_valid = tc_mode_valid,
.post_disable = tc_bridge_post_disable,
};
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 06/20] drm/bridge: tc358775: redefine LV_MX()
2024-05-06 13:34 [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
` (4 preceding siblings ...)
2024-05-06 13:34 ` [PATCH 05/20] drm/bridge: tc358775: add crtc modes fixup Michael Walle
@ 2024-05-06 13:34 ` Michael Walle
2024-05-06 13:34 ` [PATCH 07/20] drm/bridge: tc358775: use regmap instead of open coded access functions Michael Walle
` (14 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Michael Walle @ 2024-05-06 13:34 UTC (permalink / raw
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren
Cc: Daniel Semkowicz, dri-devel, linux-kernel, linux-mediatek,
linux-arm-kernel, Michael Walle
Drop the FLD_VAL macro, just use bit shifts. This is a preparation patch
to switch to regmap and to remove the FLD_VAL().
While at it, reformat the LV_x enum.
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
drivers/gpu/drm/bridge/tc358775.c | 36 ++++++------------------------------
1 file changed, 6 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
index 720c0d63fd6a..7ae86e8d4c72 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -124,39 +124,15 @@
#define LV_MX1619 0x0490 /* Bit 16 to 19 */
#define LV_MX2023 0x0494 /* Bit 20 to 23 */
#define LV_MX2427 0x0498 /* Bit 24 to 27 */
-#define LV_MX(b0, b1, b2, b3) (FLD_VAL(b0, 4, 0) | FLD_VAL(b1, 12, 8) | \
- FLD_VAL(b2, 20, 16) | FLD_VAL(b3, 28, 24))
+#define LV_MX(b0, b1, b2, b3) \
+ (((b3) << 24) | ((b2) << 16) | ((b1) << 8) | (b0))
/* Input bit numbers used in mux registers */
enum {
- LVI_R0,
- LVI_R1,
- LVI_R2,
- LVI_R3,
- LVI_R4,
- LVI_R5,
- LVI_R6,
- LVI_R7,
- LVI_G0,
- LVI_G1,
- LVI_G2,
- LVI_G3,
- LVI_G4,
- LVI_G5,
- LVI_G6,
- LVI_G7,
- LVI_B0,
- LVI_B1,
- LVI_B2,
- LVI_B3,
- LVI_B4,
- LVI_B5,
- LVI_B6,
- LVI_B7,
- LVI_HS,
- LVI_VS,
- LVI_DE,
- LVI_L0
+ LVI_R0, LVI_R1, LVI_R2, LVI_R3, LVI_R4, LVI_R5, LVI_R6, LVI_R7,
+ LVI_G0, LVI_G1, LVI_G2, LVI_G3, LVI_G4, LVI_G5, LVI_G6, LVI_G7,
+ LVI_B0, LVI_B1, LVI_B2, LVI_B3, LVI_B4, LVI_B5, LVI_B6, LVI_B7,
+ LVI_HS, LVI_VS, LVI_DE, LVI_L0
};
#define LVCFG 0x049C /* LVDS Configuration */
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 07/20] drm/bridge: tc358775: use regmap instead of open coded access functions
2024-05-06 13:34 [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
` (5 preceding siblings ...)
2024-05-06 13:34 ` [PATCH 06/20] drm/bridge: tc358775: redefine LV_MX() Michael Walle
@ 2024-05-06 13:34 ` Michael Walle
2024-05-06 13:34 ` [PATCH 08/20] drm/bridge: tc358775: remove error message if regulator is missing Michael Walle
` (13 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Michael Walle @ 2024-05-06 13:34 UTC (permalink / raw
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren
Cc: Daniel Semkowicz, dri-devel, linux-kernel, linux-mediatek,
linux-arm-kernel, Michael Walle
The DSI bridge also supports access via DSI in-band reads and writes.
Prepare the driver for that by converting all the access functions to
regmap. This also have the advantage that it will make tracing and
debugging easier and we can use all the bit manipulation helpers from
regmap.
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
drivers/gpu/drm/bridge/tc358775.c | 150 +++++++++++++++++---------------------
1 file changed, 68 insertions(+), 82 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
index 7ae86e8d4c72..b7f15164e655 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -16,6 +16,7 @@
#include <linux/media-bus-format.h>
#include <linux/module.h>
#include <linux/of_device.h>
+#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
#include <linux/slab.h>
@@ -238,7 +239,7 @@ enum tc3587x5_type {
};
struct tc_data {
- struct i2c_client *i2c;
+ struct regmap *regmap;
struct device *dev;
struct drm_bridge bridge;
@@ -309,42 +310,6 @@ static void tc_bridge_post_disable(struct drm_bridge *bridge)
usleep_range(10000, 11000);
}
-static void d2l_read(struct i2c_client *i2c, u16 addr, u32 *val)
-{
- int ret;
- u8 buf_addr[2];
-
- put_unaligned_be16(addr, buf_addr);
- ret = i2c_master_send(i2c, buf_addr, sizeof(buf_addr));
- if (ret < 0)
- goto fail;
-
- ret = i2c_master_recv(i2c, (u8 *)val, sizeof(*val));
- if (ret < 0)
- goto fail;
-
- pr_debug("d2l: I2C : addr:%04x value:%08x\n", addr, *val);
- return;
-
-fail:
- dev_err(&i2c->dev, "Error %d reading from subaddress 0x%x\n",
- ret, addr);
-}
-
-static void d2l_write(struct i2c_client *i2c, u16 addr, u32 val)
-{
- u8 data[6];
- int ret;
-
- put_unaligned_be16(addr, data);
- put_unaligned_le32(val, data + 2);
-
- ret = i2c_master_send(i2c, data, ARRAY_SIZE(data));
- if (ret < 0)
- dev_err(&i2c->dev, "Error %d writing to subaddress 0x%x\n",
- ret, addr);
-}
-
/* helper function to access bus_formats */
static struct drm_connector *get_connector(struct drm_encoder *encoder)
{
@@ -358,12 +323,33 @@ static struct drm_connector *get_connector(struct drm_encoder *encoder)
return NULL;
}
+static const struct reg_sequence tc_lvmux_vesa24[] = {
+ { LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2, LVI_R3) },
+ { LV_MX0407, LV_MX(LVI_R4, LVI_R7, LVI_R5, LVI_G0) },
+ { LV_MX0811, LV_MX(LVI_G1, LVI_G2, LVI_G6, LVI_G7) },
+ { LV_MX1215, LV_MX(LVI_G3, LVI_G4, LVI_G5, LVI_B0) },
+ { LV_MX1619, LV_MX(LVI_B6, LVI_B7, LVI_B1, LVI_B2) },
+ { LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0) },
+ { LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R6) },
+};
+
+/* JEIDA-24/JEIDA-18 have the same mapping */
+static const struct reg_sequence tc_lvmux_jeida18_24[] = {
+ { LV_MX0003, LV_MX(LVI_R2, LVI_R3, LVI_R4, LVI_R5) },
+ { LV_MX0407, LV_MX(LVI_R6, LVI_R1, LVI_R7, LVI_G2) },
+ { LV_MX0811, LV_MX(LVI_G3, LVI_G4, LVI_G0, LVI_G1) },
+ { LV_MX1215, LV_MX(LVI_G5, LVI_G6, LVI_G7, LVI_B2) },
+ { LV_MX1619, LV_MX(LVI_B0, LVI_B1, LVI_B3, LVI_B4) },
+ { LV_MX2023, LV_MX(LVI_B5, LVI_B6, LVI_B7, LVI_L0) },
+ { LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R0) },
+};
+
static void tc_bridge_enable(struct drm_bridge *bridge)
{
struct tc_data *tc = bridge_to_tc(bridge);
u32 hback_porch, hsync_len, hfront_porch, hactive, htime1, htime2;
u32 vback_porch, vsync_len, vfront_porch, vactive, vtime1, vtime2;
- u32 val = 0;
+ unsigned int val = 0;
u16 dsiclk, clkdiv, byteclk, t1, t2, t3, vsdelay;
struct drm_display_mode *mode;
struct drm_connector *connector = get_connector(bridge->encoder);
@@ -386,28 +372,29 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
htime2 = (hfront_porch << 16) + hactive;
vtime2 = (vfront_porch << 16) + vactive;
- d2l_read(tc->i2c, IDREG, &val);
+ regmap_read(tc->regmap, IDREG, &val);
dev_info(tc->dev, "DSI2LVDS Chip ID.%02x Revision ID. %02x **\n",
(val >> 8) & 0xFF, val & 0xFF);
- d2l_write(tc->i2c, SYSRST, SYS_RST_REG | SYS_RST_DSIRX | SYS_RST_BM |
- SYS_RST_LCD | SYS_RST_I2CM);
+ regmap_write(tc->regmap, SYSRST,
+ SYS_RST_REG | SYS_RST_DSIRX | SYS_RST_BM | SYS_RST_LCD |
+ SYS_RST_I2CM);
usleep_range(30000, 40000);
- d2l_write(tc->i2c, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
- d2l_write(tc->i2c, PPI_LPTXTIMECNT, LPX_PERIOD);
- d2l_write(tc->i2c, PPI_D0S_CLRSIPOCOUNT, 3);
- d2l_write(tc->i2c, PPI_D1S_CLRSIPOCOUNT, 3);
- d2l_write(tc->i2c, PPI_D2S_CLRSIPOCOUNT, 3);
- d2l_write(tc->i2c, PPI_D3S_CLRSIPOCOUNT, 3);
+ regmap_write(tc->regmap, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
+ regmap_write(tc->regmap, PPI_LPTXTIMECNT, LPX_PERIOD);
+ regmap_write(tc->regmap, PPI_D0S_CLRSIPOCOUNT, 3);
+ regmap_write(tc->regmap, PPI_D1S_CLRSIPOCOUNT, 3);
+ regmap_write(tc->regmap, PPI_D2S_CLRSIPOCOUNT, 3);
+ regmap_write(tc->regmap, PPI_D3S_CLRSIPOCOUNT, 3);
val = ((L0EN << tc->num_dsi_lanes) - L0EN) | DSI_CLEN_BIT;
- d2l_write(tc->i2c, PPI_LANEENABLE, val);
- d2l_write(tc->i2c, DSI_LANEENABLE, val);
+ regmap_write(tc->regmap, PPI_LANEENABLE, val);
+ regmap_write(tc->regmap, DSI_LANEENABLE, val);
- d2l_write(tc->i2c, PPI_STARTPPI, PPI_START_FUNCTION);
- d2l_write(tc->i2c, DSI_STARTDSI, DSI_RX_START);
+ regmap_write(tc->regmap, PPI_STARTPPI, PPI_START_FUNCTION);
+ regmap_write(tc->regmap, DSI_STARTDSI, DSI_RX_START);
/* Video event mode vs pulse mode bit, does not exist for tc358775 */
if (tc->type == TC358765)
@@ -431,42 +418,28 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
vsdelay = (clkdiv * (t1 + t3) / byteclk) - hback_porch - hsync_len - hactive;
val |= TC358775_VPCTRL_VSDELAY(vsdelay);
- d2l_write(tc->i2c, VPCTRL, val);
+ regmap_write(tc->regmap, VPCTRL, val);
- d2l_write(tc->i2c, HTIM1, htime1);
- d2l_write(tc->i2c, VTIM1, vtime1);
- d2l_write(tc->i2c, HTIM2, htime2);
- d2l_write(tc->i2c, VTIM2, vtime2);
+ regmap_write(tc->regmap, HTIM1, htime1);
+ regmap_write(tc->regmap, VTIM1, vtime1);
+ regmap_write(tc->regmap, HTIM2, htime2);
+ regmap_write(tc->regmap, VTIM2, vtime2);
- d2l_write(tc->i2c, VFUEN, VFUEN_EN);
- d2l_write(tc->i2c, SYSRST, SYS_RST_LCD);
- d2l_write(tc->i2c, LVPHY0, LV_PHY0_PRBS_ON(4) | LV_PHY0_ND(6));
+ regmap_write(tc->regmap, VFUEN, VFUEN_EN);
+ regmap_write(tc->regmap, SYSRST, SYS_RST_LCD);
+ regmap_write(tc->regmap, LVPHY0, LV_PHY0_PRBS_ON(4) | LV_PHY0_ND(6));
dev_dbg(tc->dev, "bus_formats %04x bpc %d\n",
connector->display_info.bus_formats[0],
tc->bpc);
- if (connector->display_info.bus_formats[0] ==
- MEDIA_BUS_FMT_RGB888_1X7X4_SPWG) {
- /* VESA-24 */
- d2l_write(tc->i2c, LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2, LVI_R3));
- d2l_write(tc->i2c, LV_MX0407, LV_MX(LVI_R4, LVI_R7, LVI_R5, LVI_G0));
- d2l_write(tc->i2c, LV_MX0811, LV_MX(LVI_G1, LVI_G2, LVI_G6, LVI_G7));
- d2l_write(tc->i2c, LV_MX1215, LV_MX(LVI_G3, LVI_G4, LVI_G5, LVI_B0));
- d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_B6, LVI_B7, LVI_B1, LVI_B2));
- d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0));
- d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R6));
- } else {
- /* JEIDA-18 and JEIDA-24 */
- d2l_write(tc->i2c, LV_MX0003, LV_MX(LVI_R2, LVI_R3, LVI_R4, LVI_R5));
- d2l_write(tc->i2c, LV_MX0407, LV_MX(LVI_R6, LVI_R1, LVI_R7, LVI_G2));
- d2l_write(tc->i2c, LV_MX0811, LV_MX(LVI_G3, LVI_G4, LVI_G0, LVI_G1));
- d2l_write(tc->i2c, LV_MX1215, LV_MX(LVI_G5, LVI_G6, LVI_G7, LVI_B2));
- d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_B0, LVI_B1, LVI_B3, LVI_B4));
- d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B5, LVI_B6, LVI_B7, LVI_L0));
- d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R0));
- }
+ if (connector->display_info.bus_formats[0] == MEDIA_BUS_FMT_RGB888_1X7X4_SPWG)
+ regmap_multi_reg_write(tc->regmap, tc_lvmux_vesa24,
+ ARRAY_SIZE(tc_lvmux_vesa24));
+ else
+ regmap_multi_reg_write(tc->regmap, tc_lvmux_jeida18_24,
+ ARRAY_SIZE(tc_lvmux_jeida18_24));
- d2l_write(tc->i2c, VFUEN, VFUEN_EN);
+ regmap_write(tc->regmap, VFUEN, VFUEN_EN);
val = LVCFG_LVEN_BIT;
if (tc->lvds_link == DUAL_LINK) {
@@ -475,7 +448,7 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
} else {
val |= TC358775_LVCFG_PCLKDIV(DIVIDE_BY_3);
}
- d2l_write(tc->i2c, LVCFG, val);
+ regmap_write(tc->regmap, LVCFG, val);
}
/*
@@ -617,7 +590,7 @@ static const struct drm_bridge_funcs tc_bridge_funcs = {
static int tc_attach_host(struct tc_data *tc)
{
- struct device *dev = &tc->i2c->dev;
+ struct device *dev = tc->dev;
struct mipi_dsi_host *host;
struct mipi_dsi_device *dsi;
int ret;
@@ -665,6 +638,14 @@ static int tc_attach_host(struct tc_data *tc)
return 0;
}
+static const struct regmap_config tc358775_regmap_config = {
+ .reg_bits = 16,
+ .val_bits = 32,
+ .max_register = 0xffff,
+ .reg_format_endian = REGMAP_ENDIAN_BIG,
+ .val_format_endian = REGMAP_ENDIAN_LITTLE,
+};
+
static int tc_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
@@ -679,6 +660,11 @@ static int tc_probe(struct i2c_client *client)
tc->i2c = client;
tc->type = (enum tc3587x5_type)(unsigned long)of_device_get_match_data(dev);
+ tc->regmap = devm_regmap_init_i2c(client, &tc358775_regmap_config);
+ if (IS_ERR(tc->regmap))
+ return dev_err_probe(dev, PTR_ERR(tc->regmap),
+ "regmap i2c init failed\n");
+
tc->panel_bridge = devm_drm_of_get_bridge(dev, dev->of_node,
TC358775_LVDS_OUT0, 0);
if (IS_ERR(tc->panel_bridge))
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 08/20] drm/bridge: tc358775: remove error message if regulator is missing
2024-05-06 13:34 [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
` (6 preceding siblings ...)
2024-05-06 13:34 ` [PATCH 07/20] drm/bridge: tc358775: use regmap instead of open coded access functions Michael Walle
@ 2024-05-06 13:34 ` Michael Walle
2024-05-06 13:34 ` [PATCH 09/20] drm/bridge: tc358775: remove complex vsdelay calculation Michael Walle
` (12 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Michael Walle @ 2024-05-06 13:34 UTC (permalink / raw
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren
Cc: Daniel Semkowicz, dri-devel, linux-kernel, linux-mediatek,
linux-arm-kernel, Michael Walle
A missing regulator node will automatically be replaced by a dummy. Thus
regulators are optional anyway. Remove the error message.
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
drivers/gpu/drm/bridge/tc358775.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
index b7f15164e655..54aea58a3406 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -675,18 +675,12 @@ static int tc_probe(struct i2c_client *client)
return ret;
tc->vddio = devm_regulator_get(dev, "vddio");
- if (IS_ERR(tc->vddio)) {
- ret = PTR_ERR(tc->vddio);
- dev_err(dev, "vddio-supply not found\n");
- return ret;
- }
+ if (IS_ERR(tc->vddio))
+ return PTR_ERR(tc->vddio);
tc->vdd = devm_regulator_get(dev, "vdd");
- if (IS_ERR(tc->vdd)) {
- ret = PTR_ERR(tc->vdd);
- dev_err(dev, "vdd-supply not found\n");
- return ret;
- }
+ if (IS_ERR(tc->vdd))
+ return PTR_ERR(tc->vdd);
tc->stby_gpio = devm_gpiod_get_optional(dev, "stby", GPIOD_OUT_HIGH);
if (IS_ERR(tc->stby_gpio))
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 09/20] drm/bridge: tc358775: remove complex vsdelay calculation
2024-05-06 13:34 [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
` (7 preceding siblings ...)
2024-05-06 13:34 ` [PATCH 08/20] drm/bridge: tc358775: remove error message if regulator is missing Michael Walle
@ 2024-05-06 13:34 ` Michael Walle
2024-05-06 13:34 ` [PATCH 10/20] drm/bridge: tc358775: simplify lvds_link property Michael Walle
` (11 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Michael Walle @ 2024-05-06 13:34 UTC (permalink / raw
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren
Cc: Daniel Semkowicz, dri-devel, linux-kernel, linux-mediatek,
linux-arm-kernel, Michael Walle
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.
While at it, replace the TC358775_VPCTRL_MSF() and
TC358775_VPCTRL_OPXLFMT() inline functions by the usual macros for a bit
flag.
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
drivers/gpu/drm/bridge/tc358775.c | 49 +++++++--------------------------------
1 file changed, 8 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
index 54aea58a3406..a9d731e87970 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -109,7 +109,9 @@
#define RDPKTLN 0x0404 /* Command Read Packet Length */
#define VPCTRL 0x0450 /* Video Path Control */
-#define EVTMODE BIT(5) /* Video event mode enable, tc35876x only */
+#define VPCTRL_MSF BIT(0)
+#define VPCTRL_OPXLFMT BIT(8)
+#define VPCTRL_EVTMODE BIT(5) /* Video event mode enable, tc35876x only */
#define HTIM1 0x0454 /* Horizontal Timing Control 1 */
#define HTIM2 0x0458 /* Horizontal Timing Control 2 */
#define VTIM1 0x045C /* Vertical Timing Control 1 */
@@ -187,30 +189,6 @@ enum {
#define L0EN BIT(1)
-#define TC358775_VPCTRL_VSDELAY__MASK 0x3FF00000
-#define TC358775_VPCTRL_VSDELAY__SHIFT 20
-static inline u32 TC358775_VPCTRL_VSDELAY(uint32_t val)
-{
- return ((val) << TC358775_VPCTRL_VSDELAY__SHIFT) &
- TC358775_VPCTRL_VSDELAY__MASK;
-}
-
-#define TC358775_VPCTRL_OPXLFMT__MASK 0x00000100
-#define TC358775_VPCTRL_OPXLFMT__SHIFT 8
-static inline u32 TC358775_VPCTRL_OPXLFMT(uint32_t val)
-{
- return ((val) << TC358775_VPCTRL_OPXLFMT__SHIFT) &
- TC358775_VPCTRL_OPXLFMT__MASK;
-}
-
-#define TC358775_VPCTRL_MSF__MASK 0x00000001
-#define TC358775_VPCTRL_MSF__SHIFT 0
-static inline u32 TC358775_VPCTRL_MSF(uint32_t val)
-{
- return ((val) << TC358775_VPCTRL_MSF__SHIFT) &
- TC358775_VPCTRL_MSF__MASK;
-}
-
#define TC358775_LVCFG_PCLKDIV__MASK 0x000000f0
#define TC358775_LVCFG_PCLKDIV__SHIFT 4
static inline u32 TC358775_LVCFG_PCLKDIV(uint32_t val)
@@ -350,7 +328,6 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
u32 hback_porch, hsync_len, hfront_porch, hactive, htime1, htime2;
u32 vback_porch, vsync_len, vfront_porch, vactive, vtime1, vtime2;
unsigned int val = 0;
- u16 dsiclk, clkdiv, byteclk, t1, t2, t3, vsdelay;
struct drm_display_mode *mode;
struct drm_connector *connector = get_connector(bridge->encoder);
@@ -398,27 +375,17 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
/* Video event mode vs pulse mode bit, does not exist for tc358775 */
if (tc->type == TC358765)
- val = EVTMODE;
+ val = VPCTRL_EVTMODE;
else
val = 0;
if (tc->bpc == 8)
- val |= TC358775_VPCTRL_OPXLFMT(1);
+ val |= VPCTRL_OPXLFMT;
else /* bpc = 6; */
- val |= TC358775_VPCTRL_MSF(1);
-
- dsiclk = mode->crtc_clock * 3 * tc->bpc / tc->num_dsi_lanes / 1000;
- clkdiv = dsiclk / (tc->lvds_link == DUAL_LINK ? DIVIDE_BY_6 : DIVIDE_BY_3);
- byteclk = dsiclk / 4;
- t1 = hactive * (tc->bpc * 3 / 8) / tc->num_dsi_lanes;
- t2 = ((100000 / clkdiv)) * (hactive + hback_porch + hsync_len + hfront_porch) / 1000;
- t3 = ((t2 * byteclk) / 100) - (hactive * (tc->bpc * 3 / 8) /
- tc->num_dsi_lanes);
-
- vsdelay = (clkdiv * (t1 + t3) / byteclk) - hback_porch - hsync_len - hactive;
+ val |= VPCTRL_MSF;
- val |= TC358775_VPCTRL_VSDELAY(vsdelay);
- regmap_write(tc->regmap, VPCTRL, val);
+ regmap_update_bits(tc->regmap, VPCTRL, val,
+ VPCTRL_OPXLFMT | VPCTRL_MSF | VPCTRL_EVTMODE);
regmap_write(tc->regmap, HTIM1, htime1);
regmap_write(tc->regmap, VTIM1, vtime1);
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 10/20] drm/bridge: tc358775: simplify lvds_link property
2024-05-06 13:34 [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
` (8 preceding siblings ...)
2024-05-06 13:34 ` [PATCH 09/20] drm/bridge: tc358775: remove complex vsdelay calculation Michael Walle
@ 2024-05-06 13:34 ` Michael Walle
2024-05-06 13:34 ` [PATCH 11/20] drm/bridge: tc358775: reformat weird indentation Michael Walle
` (10 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Michael Walle @ 2024-05-06 13:34 UTC (permalink / raw
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren
Cc: Daniel Semkowicz, dri-devel, linux-kernel, linux-mediatek,
linux-arm-kernel, Michael Walle
The LVDS link can either be a single link or a dual link. No need for a
u8. Replace it with a bool "lvds_dual_link".
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
drivers/gpu/drm/bridge/tc358775.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
index a9d731e87970..be2175571b99 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -231,7 +231,7 @@ struct tc_data {
struct regulator *vddio;
struct gpio_desc *reset_gpio;
struct gpio_desc *stby_gpio;
- u8 lvds_link; /* single-link or dual-link */
+ bool lvds_dual_link;
u8 bpc;
enum tc3587x5_type type;
@@ -409,7 +409,7 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
regmap_write(tc->regmap, VFUEN, VFUEN_EN);
val = LVCFG_LVEN_BIT;
- if (tc->lvds_link == DUAL_LINK) {
+ if (tc->lvds_dual_link) {
val |= TC358775_LVCFG_LVDLINK(1);
val |= TC358775_LVCFG_PCLKDIV(DIVIDE_BY_6);
} else {
@@ -460,8 +460,8 @@ tc_mode_valid(struct drm_bridge *bridge,
* Maximum pixel clock speed 135MHz for single-link
* 270MHz for dual-link
*/
- if ((mode->clock > 135000 && tc->lvds_link == SINGLE_LINK) ||
- (mode->clock > 270000 && tc->lvds_link == DUAL_LINK))
+ if ((mode->clock > 135000 && !tc->lvds_dual_link) ||
+ (mode->clock > 270000 && tc->lvds_dual_link))
return MODE_CLOCK_HIGH;
switch (info->bus_formats[0]) {
@@ -516,7 +516,6 @@ static int tc358775_parse_dt(struct device_node *np, struct tc_data *tc)
of_node_put(tc->host_node);
- tc->lvds_link = SINGLE_LINK;
endpoint = of_graph_get_endpoint_by_regs(tc->dev->of_node,
TC358775_LVDS_OUT1, -1);
if (endpoint) {
@@ -525,13 +524,14 @@ static int tc358775_parse_dt(struct device_node *np, struct tc_data *tc)
if (remote) {
if (of_device_is_available(remote))
- tc->lvds_link = DUAL_LINK;
+ tc->lvds_dual_link = true;
of_node_put(remote);
}
}
dev_dbg(tc->dev, "no.of dsi lanes: %d\n", tc->num_dsi_lanes);
- dev_dbg(tc->dev, "operating in %d-link mode\n", tc->lvds_link);
+ dev_dbg(tc->dev, "operating in %s-link mode\n",
+ tc->lvds_dual_link ? "dual" : "single");
return 0;
}
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 11/20] drm/bridge: tc358775: reformat weird indentation
2024-05-06 13:34 [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
` (9 preceding siblings ...)
2024-05-06 13:34 ` [PATCH 10/20] drm/bridge: tc358775: simplify lvds_link property Michael Walle
@ 2024-05-06 13:34 ` Michael Walle
2024-05-06 13:34 ` [PATCH 12/20] drm/bridge: tc358775: correctly configure LVDS clock Michael Walle
` (9 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Michael Walle @ 2024-05-06 13:34 UTC (permalink / raw
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren
Cc: Daniel Semkowicz, dri-devel, linux-kernel, linux-mediatek,
linux-arm-kernel, Michael Walle
Reformat the indentation of the mipi_dsi_device_info initialization.
While at it, move it to the top of the function.
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
drivers/gpu/drm/bridge/tc358775.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
index be2175571b99..e6d1f0c686ac 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -557,14 +557,15 @@ static const struct drm_bridge_funcs tc_bridge_funcs = {
static int tc_attach_host(struct tc_data *tc)
{
+ const struct mipi_dsi_device_info info = {
+ .type = "tc358775",
+ .channel = 0,
+ .node = NULL,
+ };
struct device *dev = tc->dev;
struct mipi_dsi_host *host;
struct mipi_dsi_device *dsi;
int ret;
- const struct mipi_dsi_device_info info = { .type = "tc358775",
- .channel = 0,
- .node = NULL,
- };
host = of_find_mipi_dsi_host_by_node(tc->host_node);
if (!host)
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 12/20] drm/bridge: tc358775: correctly configure LVDS clock
2024-05-06 13:34 [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
` (10 preceding siblings ...)
2024-05-06 13:34 ` [PATCH 11/20] drm/bridge: tc358775: reformat weird indentation Michael Walle
@ 2024-05-06 13:34 ` Michael Walle
2024-05-06 13:34 ` [PATCH 13/20] drm/bridge: tc358775: split the init code Michael Walle
` (8 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Michael Walle @ 2024-05-06 13:34 UTC (permalink / raw
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren
Cc: Daniel Semkowicz, dri-devel, linux-kernel, linux-mediatek,
linux-arm-kernel, Michael Walle
The driver assumes a DSI link with four lanes for now and has the LVDS
clock divider hardcoded to either 3 or 6. Take the number of lanes into
account, too. Also, explicitly set the clock source to the DSI clock.
While at it, replace the TC358775_LVCFG_PCLKDIV() and
TC358775_LVCFG_LVDLINK() inline functions style by the more common
linux bitfields functions.
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
drivers/gpu/drm/bridge/tc358775.c | 48 +++++++++++++++++----------------------
1 file changed, 21 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
index e6d1f0c686ac..eea41054c6fa 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -139,6 +139,12 @@ enum {
};
#define LVCFG 0x049C /* LVDS Configuration */
+#define LVCFG_LVEN BIT(0)
+#define LVCFG_LVDLINK BIT(1)
+#define LVCFG_PCLKDIV GENMASK(7, 4)
+#define LVCFG_PCLKSEL GENMASK(11, 10)
+#define PCLKSEL_HSRCK 0 /* DSI clock */
+
#define LVPHY0 0x04A0 /* LVDS PHY 0 */
#define LV_PHY0_RST(v) FLD_VAL(v, 22, 22) /* PHY reset */
#define LV_PHY0_IS(v) FLD_VAL(v, 15, 14)
@@ -183,28 +189,8 @@ enum {
#define DEBUG01 0x05A4 /* LVDS Data */
#define DSI_CLEN_BIT BIT(0)
-#define DIVIDE_BY_3 3 /* PCLK=DCLK/3 */
-#define DIVIDE_BY_6 6 /* PCLK=DCLK/6 */
-#define LVCFG_LVEN_BIT BIT(0)
-
#define L0EN BIT(1)
-#define TC358775_LVCFG_PCLKDIV__MASK 0x000000f0
-#define TC358775_LVCFG_PCLKDIV__SHIFT 4
-static inline u32 TC358775_LVCFG_PCLKDIV(uint32_t val)
-{
- return ((val) << TC358775_LVCFG_PCLKDIV__SHIFT) &
- TC358775_LVCFG_PCLKDIV__MASK;
-}
-
-#define TC358775_LVCFG_LVDLINK__MASK 0x00000002
-#define TC358775_LVCFG_LVDLINK__SHIFT 1
-static inline u32 TC358775_LVCFG_LVDLINK(uint32_t val)
-{
- return ((val) << TC358775_LVCFG_LVDLINK__SHIFT) &
- TC358775_LVCFG_LVDLINK__MASK;
-}
-
enum tc358775_ports {
TC358775_DSI_IN,
TC358775_LVDS_OUT0,
@@ -327,6 +313,8 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
struct tc_data *tc = bridge_to_tc(bridge);
u32 hback_porch, hsync_len, hfront_porch, hactive, htime1, htime2;
u32 vback_porch, vsync_len, vfront_porch, vactive, vtime1, vtime2;
+ int bpp = mipi_dsi_pixel_format_to_bpp(tc->dsi->format);
+ int clkdiv;
unsigned int val = 0;
struct drm_display_mode *mode;
struct drm_connector *connector = get_connector(bridge->encoder);
@@ -408,14 +396,20 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
regmap_write(tc->regmap, VFUEN, VFUEN_EN);
- val = LVCFG_LVEN_BIT;
- if (tc->lvds_dual_link) {
- val |= TC358775_LVCFG_LVDLINK(1);
- val |= TC358775_LVCFG_PCLKDIV(DIVIDE_BY_6);
- } else {
- val |= TC358775_LVCFG_PCLKDIV(DIVIDE_BY_3);
- }
+ /* Configure LVDS clock */
+ clkdiv = bpp / tc->num_dsi_lanes;
+ if (!tc->lvds_dual_link)
+ clkdiv /= 2;
+
+ val = u32_encode_bits(clkdiv, LVCFG_PCLKDIV);
+ val |= u32_encode_bits(PCLKSEL_HSRCK, LVCFG_PCLKSEL);
+ if (tc->lvds_dual_link)
+ val |= LVCFG_LVDLINK;
+
regmap_write(tc->regmap, LVCFG, val);
+
+ /* Finally, enable the LVDS transmitter */
+ regmap_write(tc->regmap, LVCFG, val | LVCFG_LVEN);
}
/*
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 13/20] drm/bridge: tc358775: split the init code
2024-05-06 13:34 [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
` (11 preceding siblings ...)
2024-05-06 13:34 ` [PATCH 12/20] drm/bridge: tc358775: correctly configure LVDS clock Michael Walle
@ 2024-05-06 13:34 ` Michael Walle
2024-05-06 13:34 ` [PATCH 14/20] drm/bridge: tc358775: configure PLL depending on the LVDS clock Michael Walle
` (7 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Michael Walle @ 2024-05-06 13:34 UTC (permalink / raw
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren
Cc: Daniel Semkowicz, dri-devel, linux-kernel, linux-mediatek,
linux-arm-kernel, Michael Walle
Split the initialization code in tc_bridge_enable() into specific
functions. This is a preparation for further code cleanup and fixes.
No functional change.
While at it, rename tc_bridge_enable() to the more specific
tc358775_bridge_enable().
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
drivers/gpu/drm/bridge/tc358775.c | 106 ++++++++++++++++++++++++--------------
1 file changed, 66 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
index eea41054c6fa..4ec059531c5f 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -308,18 +308,30 @@ static const struct reg_sequence tc_lvmux_jeida18_24[] = {
{ LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R0) },
};
-static void tc_bridge_enable(struct drm_bridge *bridge)
+static void tc358775_configure_dsi(struct tc_data *tc)
+{
+ unsigned int val;
+
+ regmap_write(tc->regmap, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
+ regmap_write(tc->regmap, PPI_LPTXTIMECNT, LPX_PERIOD);
+ regmap_write(tc->regmap, PPI_D0S_CLRSIPOCOUNT, 3);
+ regmap_write(tc->regmap, PPI_D1S_CLRSIPOCOUNT, 3);
+ regmap_write(tc->regmap, PPI_D2S_CLRSIPOCOUNT, 3);
+ regmap_write(tc->regmap, PPI_D3S_CLRSIPOCOUNT, 3);
+
+ val = ((L0EN << tc->num_dsi_lanes) - L0EN) | DSI_CLEN_BIT;
+ regmap_write(tc->regmap, PPI_LANEENABLE, val);
+ regmap_write(tc->regmap, DSI_LANEENABLE, val);
+
+ regmap_write(tc->regmap, PPI_STARTPPI, PPI_START_FUNCTION);
+ regmap_write(tc->regmap, DSI_STARTDSI, DSI_RX_START);
+}
+
+static void tc358775_configure_lvds_timings(struct tc_data *tc,
+ struct drm_display_mode *mode)
{
- struct tc_data *tc = bridge_to_tc(bridge);
u32 hback_porch, hsync_len, hfront_porch, hactive, htime1, htime2;
u32 vback_porch, vsync_len, vfront_porch, vactive, vtime1, vtime2;
- int bpp = mipi_dsi_pixel_format_to_bpp(tc->dsi->format);
- int clkdiv;
- unsigned int val = 0;
- struct drm_display_mode *mode;
- struct drm_connector *connector = get_connector(bridge->encoder);
-
- mode = &bridge->encoder->crtc->state->adjusted_mode;
hback_porch = mode->htotal - mode->hsync_end;
hsync_len = mode->hsync_end - mode->hsync_start;
@@ -337,30 +349,6 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
htime2 = (hfront_porch << 16) + hactive;
vtime2 = (vfront_porch << 16) + vactive;
- regmap_read(tc->regmap, IDREG, &val);
-
- dev_info(tc->dev, "DSI2LVDS Chip ID.%02x Revision ID. %02x **\n",
- (val >> 8) & 0xFF, val & 0xFF);
-
- regmap_write(tc->regmap, SYSRST,
- SYS_RST_REG | SYS_RST_DSIRX | SYS_RST_BM | SYS_RST_LCD |
- SYS_RST_I2CM);
- usleep_range(30000, 40000);
-
- regmap_write(tc->regmap, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
- regmap_write(tc->regmap, PPI_LPTXTIMECNT, LPX_PERIOD);
- regmap_write(tc->regmap, PPI_D0S_CLRSIPOCOUNT, 3);
- regmap_write(tc->regmap, PPI_D1S_CLRSIPOCOUNT, 3);
- regmap_write(tc->regmap, PPI_D2S_CLRSIPOCOUNT, 3);
- regmap_write(tc->regmap, PPI_D3S_CLRSIPOCOUNT, 3);
-
- val = ((L0EN << tc->num_dsi_lanes) - L0EN) | DSI_CLEN_BIT;
- regmap_write(tc->regmap, PPI_LANEENABLE, val);
- regmap_write(tc->regmap, DSI_LANEENABLE, val);
-
- regmap_write(tc->regmap, PPI_STARTPPI, PPI_START_FUNCTION);
- regmap_write(tc->regmap, DSI_STARTDSI, DSI_RX_START);
-
/* Video event mode vs pulse mode bit, does not exist for tc358775 */
if (tc->type == TC358765)
val = VPCTRL_EVTMODE;
@@ -381,20 +369,31 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
regmap_write(tc->regmap, VTIM2, vtime2);
regmap_write(tc->regmap, VFUEN, VFUEN_EN);
+}
+
+static void tc358775_configure_pll(struct tc_data *tc, int pixelclk)
+{
regmap_write(tc->regmap, SYSRST, SYS_RST_LCD);
regmap_write(tc->regmap, LVPHY0, LV_PHY0_PRBS_ON(4) | LV_PHY0_ND(6));
+}
- dev_dbg(tc->dev, "bus_formats %04x bpc %d\n",
- connector->display_info.bus_formats[0],
- tc->bpc);
- if (connector->display_info.bus_formats[0] == MEDIA_BUS_FMT_RGB888_1X7X4_SPWG)
+static void tc358775_configure_color_mapping(struct tc_data *tc, u32 fmt)
+{
+ dev_dbg(tc->dev, "bus_formats %04x bpc %d\n", fmt, tc->bpc);
+
+ if (fmt == MEDIA_BUS_FMT_RGB888_1X7X4_SPWG)
regmap_multi_reg_write(tc->regmap, tc_lvmux_vesa24,
ARRAY_SIZE(tc_lvmux_vesa24));
else
regmap_multi_reg_write(tc->regmap, tc_lvmux_jeida18_24,
ARRAY_SIZE(tc_lvmux_jeida18_24));
+}
- regmap_write(tc->regmap, VFUEN, VFUEN_EN);
+static void tc358775_configure_lvds_clock(struct tc_data *tc)
+{
+ int bpp = mipi_dsi_pixel_format_to_bpp(tc->dsi->format);
+ unsigned int val;
+ int clkdiv;
/* Configure LVDS clock */
clkdiv = bpp / tc->num_dsi_lanes;
@@ -407,9 +406,36 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
val |= LVCFG_LVDLINK;
regmap_write(tc->regmap, LVCFG, val);
+}
+
+static void tc358775_bridge_enable(struct drm_bridge *bridge)
+{
+ struct tc_data *tc = bridge_to_tc(bridge);
+ unsigned int val = 0;
+ struct drm_display_mode *mode;
+ struct drm_connector *connector = get_connector(bridge->encoder);
+
+ mode = &bridge->encoder->crtc->state->adjusted_mode;
+
+ regmap_read(tc->regmap, IDREG, &val);
+
+ dev_info(tc->dev, "DSI2LVDS Chip ID.%02x Revision ID. %02x **\n",
+ (val >> 8) & 0xFF, val & 0xFF);
+
+ regmap_write(tc->regmap, SYSRST,
+ SYS_RST_REG | SYS_RST_DSIRX | SYS_RST_BM | SYS_RST_LCD |
+ SYS_RST_I2CM);
+ usleep_range(30000, 40000);
+
+ tc358775_configure_dsi(tc);
+ tc358775_configure_lvds_timings(tc, mode);
+ tc358775_configure_pll(tc, mode->crtc_clock);
+ tc358775_configure_color_mapping(tc, connector->display_info.bus_formats[0]);
+ regmap_write(tc->regmap, VFUEN, VFUEN_EN);
+ tc358775_configure_lvds_clock(tc);
/* Finally, enable the LVDS transmitter */
- regmap_write(tc->regmap, LVCFG, val | LVCFG_LVEN);
+ regmap_update_bits(tc->regmap, LVCFG, LVCFG_LVEN, LVCFG_LVEN);
}
/*
@@ -543,7 +569,7 @@ static int tc_bridge_attach(struct drm_bridge *bridge,
static const struct drm_bridge_funcs tc_bridge_funcs = {
.attach = tc_bridge_attach,
.pre_enable = tc_bridge_pre_enable,
- .enable = tc_bridge_enable,
+ .enable = tc358775_bridge_enable,
.mode_fixup = tc_mode_fixup,
.mode_valid = tc_mode_valid,
.post_disable = tc_bridge_post_disable,
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 14/20] drm/bridge: tc358775: configure PLL depending on the LVDS clock
2024-05-06 13:34 [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
` (12 preceding siblings ...)
2024-05-06 13:34 ` [PATCH 13/20] drm/bridge: tc358775: split the init code Michael Walle
@ 2024-05-06 13:34 ` Michael Walle
2024-05-06 13:34 ` [PATCH 15/20] drm/bridge: tc358775: dynamically configure DSI link settings Michael Walle
` (6 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Michael Walle @ 2024-05-06 13:34 UTC (permalink / raw
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren
Cc: Daniel Semkowicz, dri-devel, linux-kernel, linux-mediatek,
linux-arm-kernel, Michael Walle
The PLL setting was hardcoded to a LVDS clock between 60MHz and 135MHz.
This adds support for slower frequencies. Also, rework the reset
sequence to match the initialization sequence provided by the vendor.
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
drivers/gpu/drm/bridge/tc358775.c | 50 ++++++++++++++++++++++++++++++++-------
1 file changed, 42 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
index 4ec059531c5f..e3fba7ac71ec 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -30,8 +30,6 @@
#include <drm/drm_panel.h>
#include <drm/drm_probe_helper.h>
-#define FLD_VAL(val, start, end) FIELD_PREP(GENMASK(start, end), val)
-
/* Registers */
/* DSI D-PHY Layer Registers */
@@ -146,10 +144,10 @@ enum {
#define PCLKSEL_HSRCK 0 /* DSI clock */
#define LVPHY0 0x04A0 /* LVDS PHY 0 */
-#define LV_PHY0_RST(v) FLD_VAL(v, 22, 22) /* PHY reset */
-#define LV_PHY0_IS(v) FLD_VAL(v, 15, 14)
-#define LV_PHY0_ND(v) FLD_VAL(v, 4, 0) /* Frequency range select */
-#define LV_PHY0_PRBS_ON(v) FLD_VAL(v, 20, 16) /* Clock/Data Flag pins */
+#define LVPHY0_LV_ND GENMASK(4, 0)
+#define LVPHY0_LV_FS GENMASK(6, 5)
+#define LVPHY0_LV_IS GENMASK(15, 14) /* charge pump current */
+#define LVPHY0_LV_RST BIT(22)
#define LVPHY1 0x04A4 /* LVDS PHY 1 */
#define SYSSTAT 0x0500 /* System Status */
@@ -223,6 +221,14 @@ struct tc_data {
enum tc3587x5_type type;
};
+struct tc358775_pll_settings {
+ unsigned int min_khz;
+ unsigned int max_khz;
+ u8 fs;
+ u8 nd;
+ u8 is;
+};
+
static inline struct tc_data *bridge_to_tc(struct drm_bridge *b)
{
return container_of(b, struct tc_data, bridge);
@@ -371,10 +377,38 @@ static void tc358775_configure_lvds_timings(struct tc_data *tc,
regmap_write(tc->regmap, VFUEN, VFUEN_EN);
}
-static void tc358775_configure_pll(struct tc_data *tc, int pixelclk)
+static const struct tc358775_pll_settings tc358775_pll_settings[] = {
+ { 25000, 30000, 2, 27, 1 },
+ { 30000, 60000, 1, 13, 1 },
+ { 60000, 135000, 0, 6, 1 },
+ {}
+};
+
+static void tc358775_configure_pll(struct tc_data *tc, unsigned int pixelclk)
{
+ const struct tc358775_pll_settings *settings;
+ unsigned int val;
+
+ if (tc->lvds_dual_link)
+ pixelclk /= 2;
+
+ for (settings = tc358775_pll_settings; settings->min_khz; settings++)
+ if (pixelclk > settings->min_khz &&
+ pixelclk < settings->max_khz)
+ break;
+
+ if (!settings->min_khz)
+ return;
+
+ val = u32_encode_bits(settings->fs, LVPHY0_LV_FS);
+ val |= u32_encode_bits(settings->nd, LVPHY0_LV_ND);
+ val |= u32_encode_bits(settings->is, LVPHY0_LV_IS);
+
+ regmap_write(tc->regmap, LVPHY0, val | LVPHY0_LV_RST);
+ usleep_range(100, 150);
+ regmap_write(tc->regmap, LVPHY0, val);
+
regmap_write(tc->regmap, SYSRST, SYS_RST_LCD);
- regmap_write(tc->regmap, LVPHY0, LV_PHY0_PRBS_ON(4) | LV_PHY0_ND(6));
}
static void tc358775_configure_color_mapping(struct tc_data *tc, u32 fmt)
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 15/20] drm/bridge: tc358775: dynamically configure DSI link settings
2024-05-06 13:34 [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
` (13 preceding siblings ...)
2024-05-06 13:34 ` [PATCH 14/20] drm/bridge: tc358775: configure PLL depending on the LVDS clock Michael Walle
@ 2024-05-06 13:34 ` Michael Walle
2024-05-06 13:34 ` [PATCH 16/20] drm/bridge: tc358775: use proper defines to configure LVDS timings Michael Walle
` (5 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Michael Walle @ 2024-05-06 13:34 UTC (permalink / raw
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren
Cc: Daniel Semkowicz, dri-devel, linux-kernel, linux-mediatek,
linux-arm-kernel, Michael Walle
Instead of hardcoding the settings for just one (unknown) particular
frequency and lane setting, compute the DSI link parameters using the
handy phy_mipi_dphy_get_default_config() helper function.
The DSI_START and DSI_BUSY registers were removed in version 0.6 of the
datasheet. It seems that it applies to a different bridge and was just a
leftover. Remove the DSI_START handling and the (unused) DSI_BUSY macro.
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
drivers/gpu/drm/bridge/Kconfig | 1 +
drivers/gpu/drm/bridge/tc358775.c | 58 +++++++++++++++++++++++----------------
2 files changed, 35 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index c621be1a99a8..ed018d6f1da3 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -349,6 +349,7 @@ config DRM_TOSHIBA_TC358775
select REGMAP_I2C
select DRM_PANEL
select DRM_MIPI_DSI
+ select GENERIC_PHY_MIPI_DPHY
help
Toshiba TC358775 DSI/LVDS bridge chip driver.
diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
index e3fba7ac71ec..33a97ddba7b5 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -19,6 +19,7 @@
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
#include <linux/slab.h>
+#include <linux/phy/phy-mipi-dphy.h>
#include <asm/unaligned.h>
@@ -49,12 +50,14 @@
/* DSI PPI Layer Registers */
#define PPI_STARTPPI 0x0104 /* START control bit of PPI-TX function. */
-#define PPI_START_FUNCTION 1
+#define PPI_STARTPPI_STARTPPI BIT(0)
#define PPI_BUSYPPI 0x0108
#define PPI_LINEINITCNT 0x0110 /* Line Initialization Wait Counter */
#define PPI_LPTXTIMECNT 0x0114
#define PPI_LANEENABLE 0x0134 /* Enables each lane at the PPI layer. */
+#define LANEENABLE_CLEN BIT(0)
+#define LANEENABLE_L0EN BIT(1)
#define PPI_TX_RX_TA 0x013C /* DSI Bus Turn Around timing parameters */
/* Analog timer function enable */
@@ -89,10 +92,7 @@
#define PPI_CLRSIPO 0x01E4 /* Clear SIPO values, Slave mode use only. */
#define HSTIMEOUT 0x01F0 /* HS Rx Time Out Counter */
#define HSTIMEOUTENABLE 0x01F4 /* Enable HS Rx Time Out Counter */
-#define DSI_STARTDSI 0x0204 /* START control bit of DSI-TX function */
-#define DSI_RX_START 1
-#define DSI_BUSYDSI 0x0208
#define DSI_LANEENABLE 0x0210 /* Enables each lane at the Protocol layer. */
#define DSI_LANESTATUS0 0x0214 /* Displays lane is in HS RX mode. */
#define DSI_LANESTATUS1 0x0218 /* Displays lane is in ULPS or STOP state */
@@ -174,21 +174,12 @@ enum {
/* Chip ID and Revision ID Register */
#define IDREG 0x0580
-#define LPX_PERIOD 4
-#define TTA_GET 0x40000
-#define TTA_SURE 6
-#define SINGLE_LINK 1
-#define DUAL_LINK 2
-
#define TC358775XBG_ID 0x00007500
/* Debug Registers */
#define DEBUG00 0x05A0 /* Debug */
#define DEBUG01 0x05A4 /* LVDS Data */
-#define DSI_CLEN_BIT BIT(0)
-#define L0EN BIT(1)
-
enum tc358775_ports {
TC358775_DSI_IN,
TC358775_LVDS_OUT0,
@@ -314,23 +305,42 @@ static const struct reg_sequence tc_lvmux_jeida18_24[] = {
{ LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R0) },
};
-static void tc358775_configure_dsi(struct tc_data *tc)
+/* All the DSI timing is counted by the HS byte clock internally */
+static uint32_t tc358775_ps_to_cnt(unsigned long long ps,
+ struct phy_configure_opts_mipi_dphy *cfg)
{
+ unsigned long hs_byte_clk = cfg->hs_clk_rate / 8;
+
+ return DIV_ROUND_UP(ps * hs_byte_clk, PSEC_PER_SEC);
+}
+
+static void tc358775_configure_dsi(struct tc_data *tc, unsigned int pixelclk)
+{
+ int bpp = mipi_dsi_pixel_format_to_bpp(tc->dsi->format);
+ struct phy_configure_opts_mipi_dphy cfg;
unsigned int val;
- regmap_write(tc->regmap, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
- regmap_write(tc->regmap, PPI_LPTXTIMECNT, LPX_PERIOD);
- regmap_write(tc->regmap, PPI_D0S_CLRSIPOCOUNT, 3);
- regmap_write(tc->regmap, PPI_D1S_CLRSIPOCOUNT, 3);
- regmap_write(tc->regmap, PPI_D2S_CLRSIPOCOUNT, 3);
- regmap_write(tc->regmap, PPI_D3S_CLRSIPOCOUNT, 3);
+ phy_mipi_dphy_get_default_config(pixelclk * 1000, bpp,
+ tc->num_dsi_lanes, &cfg);
+
+ regmap_write(tc->regmap, PPI_TX_RX_TA,
+ (tc358775_ps_to_cnt(cfg.ta_get, &cfg) << 16) |
+ tc358775_ps_to_cnt(cfg.ta_sure, &cfg));
+ regmap_write(tc->regmap, PPI_LPTXTIMECNT,
+ tc358775_ps_to_cnt(cfg.lpx, &cfg));
+
+ val = tc358775_ps_to_cnt(cfg.hs_settle, &cfg);
+ regmap_write(tc->regmap, PPI_D0S_CLRSIPOCOUNT, val);
+ regmap_write(tc->regmap, PPI_D1S_CLRSIPOCOUNT, val);
+ regmap_write(tc->regmap, PPI_D2S_CLRSIPOCOUNT, val);
+ regmap_write(tc->regmap, PPI_D3S_CLRSIPOCOUNT, val);
- val = ((L0EN << tc->num_dsi_lanes) - L0EN) | DSI_CLEN_BIT;
+ val = LANEENABLE_CLEN;
+ val |= (LANEENABLE_L0EN << tc->num_dsi_lanes) - LANEENABLE_L0EN;
regmap_write(tc->regmap, PPI_LANEENABLE, val);
regmap_write(tc->regmap, DSI_LANEENABLE, val);
- regmap_write(tc->regmap, PPI_STARTPPI, PPI_START_FUNCTION);
- regmap_write(tc->regmap, DSI_STARTDSI, DSI_RX_START);
+ regmap_write(tc->regmap, PPI_STARTPPI, PPI_STARTPPI_STARTPPI);
}
static void tc358775_configure_lvds_timings(struct tc_data *tc,
@@ -461,7 +471,7 @@ static void tc358775_bridge_enable(struct drm_bridge *bridge)
SYS_RST_I2CM);
usleep_range(30000, 40000);
- tc358775_configure_dsi(tc);
+ tc358775_configure_dsi(tc, mode->crtc_clock);
tc358775_configure_lvds_timings(tc, mode);
tc358775_configure_pll(tc, mode->crtc_clock);
tc358775_configure_color_mapping(tc, connector->display_info.bus_formats[0]);
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 16/20] drm/bridge: tc358775: use proper defines to configure LVDS timings
2024-05-06 13:34 [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
` (14 preceding siblings ...)
2024-05-06 13:34 ` [PATCH 15/20] drm/bridge: tc358775: dynamically configure DSI link settings Michael Walle
@ 2024-05-06 13:34 ` Michael Walle
2024-05-06 13:34 ` [PATCH 17/20] drm/bridge: tc358775: move bridge power up/down into functions Michael Walle
` (4 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Michael Walle @ 2024-05-06 13:34 UTC (permalink / raw
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren
Cc: Daniel Semkowicz, dri-devel, linux-kernel, linux-mediatek,
linux-arm-kernel, Michael Walle
Provide bitfield macros for the individual fields in the LVDS timing
registers and get rid of the magic values.
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
drivers/gpu/drm/bridge/tc358775.c | 52 +++++++++++++++++++++++++--------------
1 file changed, 33 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
index 33a97ddba7b5..c50554ec4b28 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -111,11 +111,19 @@
#define VPCTRL_OPXLFMT BIT(8)
#define VPCTRL_EVTMODE BIT(5) /* Video event mode enable, tc35876x only */
#define HTIM1 0x0454 /* Horizontal Timing Control 1 */
+#define HTIM1_HPW GENMASK(8, 0)
+#define HTIM1_HBPR GENMASK(24, 16)
#define HTIM2 0x0458 /* Horizontal Timing Control 2 */
+#define HTIM2_HACT GENMASK(10, 0)
+#define HTIM2_HFPR GENMASK(24, 16)
#define VTIM1 0x045C /* Vertical Timing Control 1 */
+#define VTIM1_VPW GENMASK(7, 0)
+#define VTIM1_VBPR GENMASK(23, 16)
#define VTIM2 0x0460 /* Vertical Timing Control 2 */
+#define VTIM2_VACT GENMASK(10, 0)
+#define VTIM2_VFPR GENMASK(23, 16)
#define VFUEN 0x0464 /* Video Frame Timing Update Enable */
-#define VFUEN_EN BIT(0) /* Upload Enable */
+#define VFUEN_VFUEN BIT(0) /* Upload Enable */
/* Mux Input Select for LVDS LINK Input */
#define LV_MX0003 0x0480 /* Bit 0 to 3 */
@@ -346,24 +354,19 @@ static void tc358775_configure_dsi(struct tc_data *tc, unsigned int pixelclk)
static void tc358775_configure_lvds_timings(struct tc_data *tc,
struct drm_display_mode *mode)
{
- u32 hback_porch, hsync_len, hfront_porch, hactive, htime1, htime2;
- u32 vback_porch, vsync_len, vfront_porch, vactive, vtime1, vtime2;
+ u32 hback_porch, hsync_len, hfront_porch, hactive;
+ u32 vback_porch, vsync_len, vfront_porch, vactive;
+ unsigned int val;
hback_porch = mode->htotal - mode->hsync_end;
hsync_len = mode->hsync_end - mode->hsync_start;
+ hactive = mode->hdisplay;
+ hfront_porch = mode->hsync_start - mode->hdisplay;
+
vback_porch = mode->vtotal - mode->vsync_end;
vsync_len = mode->vsync_end - mode->vsync_start;
-
- htime1 = (hback_porch << 16) + hsync_len;
- vtime1 = (vback_porch << 16) + vsync_len;
-
- hfront_porch = mode->hsync_start - mode->hdisplay;
- hactive = mode->hdisplay;
- vfront_porch = mode->vsync_start - mode->vdisplay;
vactive = mode->vdisplay;
-
- htime2 = (hfront_porch << 16) + hactive;
- vtime2 = (vfront_porch << 16) + vactive;
+ vfront_porch = mode->vsync_start - mode->vdisplay;
/* Video event mode vs pulse mode bit, does not exist for tc358775 */
if (tc->type == TC358765)
@@ -379,12 +382,23 @@ static void tc358775_configure_lvds_timings(struct tc_data *tc,
regmap_update_bits(tc->regmap, VPCTRL, val,
VPCTRL_OPXLFMT | VPCTRL_MSF | VPCTRL_EVTMODE);
- regmap_write(tc->regmap, HTIM1, htime1);
- regmap_write(tc->regmap, VTIM1, vtime1);
- regmap_write(tc->regmap, HTIM2, htime2);
- regmap_write(tc->regmap, VTIM2, vtime2);
+ val = u32_encode_bits(hsync_len, HTIM1_HPW);
+ val |= u32_encode_bits(hback_porch, HTIM1_HBPR);
+ regmap_write(tc->regmap, HTIM1, val);
+
+ val = u32_encode_bits(hactive, HTIM2_HACT);
+ val |= u32_encode_bits(hfront_porch, HTIM2_HFPR);
+ regmap_write(tc->regmap, HTIM2, val);
+
+ val = u32_encode_bits(vsync_len, VTIM1_VPW);
+ val |= u32_encode_bits(vback_porch, VTIM1_VBPR);
+ regmap_write(tc->regmap, VTIM1, val);
+
+ val = u32_encode_bits(vactive, VTIM2_VACT);
+ val |= u32_encode_bits(vfront_porch, VTIM2_VFPR);
+ regmap_write(tc->regmap, VTIM2, val);
- regmap_write(tc->regmap, VFUEN, VFUEN_EN);
+ regmap_write(tc->regmap, VFUEN, VFUEN_VFUEN);
}
static const struct tc358775_pll_settings tc358775_pll_settings[] = {
@@ -475,7 +489,7 @@ static void tc358775_bridge_enable(struct drm_bridge *bridge)
tc358775_configure_lvds_timings(tc, mode);
tc358775_configure_pll(tc, mode->crtc_clock);
tc358775_configure_color_mapping(tc, connector->display_info.bus_formats[0]);
- regmap_write(tc->regmap, VFUEN, VFUEN_EN);
+ regmap_write(tc->regmap, VFUEN, VFUEN_VFUEN);
tc358775_configure_lvds_clock(tc);
/* Finally, enable the LVDS transmitter */
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 17/20] drm/bridge: tc358775: move bridge power up/down into functions
2024-05-06 13:34 [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
` (15 preceding siblings ...)
2024-05-06 13:34 ` [PATCH 16/20] drm/bridge: tc358775: use proper defines to configure LVDS timings Michael Walle
@ 2024-05-06 13:34 ` Michael Walle
2024-05-06 13:34 ` [PATCH 18/20] drm/bridge: tc358775: fix the power-up/down delays Michael Walle
` (3 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Michael Walle @ 2024-05-06 13:34 UTC (permalink / raw
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren
Cc: Daniel Semkowicz, dri-devel, linux-kernel, linux-mediatek,
linux-arm-kernel, Michael Walle
Move the bridge power-up and power-down handling into own functions.
This is a preparation patch to fix the power-up sequencing of the
bridge. No functional change.
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
drivers/gpu/drm/bridge/tc358775.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
index c50554ec4b28..d5b3d691d2c1 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -215,6 +215,7 @@ struct tc_data {
struct gpio_desc *reset_gpio;
struct gpio_desc *stby_gpio;
bool lvds_dual_link;
+ bool powered;
u8 bpc;
enum tc3587x5_type type;
@@ -233,9 +234,8 @@ static inline struct tc_data *bridge_to_tc(struct drm_bridge *b)
return container_of(b, struct tc_data, bridge);
}
-static void tc_bridge_pre_enable(struct drm_bridge *bridge)
+static void tc358775_power_up(struct tc_data *tc)
{
- struct tc_data *tc = bridge_to_tc(bridge);
struct device *dev = &tc->dsi->dev;
int ret;
@@ -256,9 +256,8 @@ static void tc_bridge_pre_enable(struct drm_bridge *bridge)
usleep_range(10, 20);
}
-static void tc_bridge_post_disable(struct drm_bridge *bridge)
+static void tc358775_power_down(struct tc_data *tc)
{
- struct tc_data *tc = bridge_to_tc(bridge);
struct device *dev = &tc->dsi->dev;
int ret;
@@ -279,6 +278,20 @@ static void tc_bridge_post_disable(struct drm_bridge *bridge)
usleep_range(10000, 11000);
}
+static void tc_bridge_pre_enable(struct drm_bridge *bridge)
+{
+ struct tc_data *tc = bridge_to_tc(bridge);
+
+ tc358775_power_up(tc);
+}
+
+static void tc_bridge_post_disable(struct drm_bridge *bridge)
+{
+ struct tc_data *tc = bridge_to_tc(bridge);
+
+ tc358775_power_down(tc);
+}
+
/* helper function to access bus_formats */
static struct drm_connector *get_connector(struct drm_encoder *encoder)
{
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 18/20] drm/bridge: tc358775: fix the power-up/down delays
2024-05-06 13:34 [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
` (16 preceding siblings ...)
2024-05-06 13:34 ` [PATCH 17/20] drm/bridge: tc358775: move bridge power up/down into functions Michael Walle
@ 2024-05-06 13:34 ` Michael Walle
2024-05-06 13:34 ` [PATCH 19/20] drm/bridge: tc358775: fix power-up sequencing Michael Walle
` (2 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Michael Walle @ 2024-05-06 13:34 UTC (permalink / raw
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren
Cc: Daniel Semkowicz, dri-devel, linux-kernel, linux-mediatek,
linux-arm-kernel, Michael Walle
Implement the delays according to Figure 8-10 and 8-11 of the datasheet.
In particular, the datasheet states that the *maximum* time between
enabling the VDDIO and VDD is 10ms. Currently, as implemented this is
always violated. Of course, this is only a best effort because we cannot
be sure enabling of the two regulators will be that fast.
The time between releasing the stby GPIO and releasing the reset GPIO
must be at least 10us and not 10ms as it was before this patch. After
reset is released, there must be at least a delay of 200us until the
first HS clock is received.
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
drivers/gpu/drm/bridge/tc358775.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
index d5b3d691d2c1..99dbbb1fee78 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -242,18 +242,16 @@ static void tc358775_power_up(struct tc_data *tc)
ret = regulator_enable(tc->vddio);
if (ret < 0)
dev_err(dev, "regulator vddio enable failed, %d\n", ret);
- usleep_range(10000, 11000);
ret = regulator_enable(tc->vdd);
if (ret < 0)
dev_err(dev, "regulator vdd enable failed, %d\n", ret);
- usleep_range(10000, 11000);
gpiod_set_value(tc->stby_gpio, 0);
- usleep_range(10000, 11000);
+ usleep_range(10, 20);
gpiod_set_value(tc->reset_gpio, 0);
- usleep_range(10, 20);
+ usleep_range(200, 250);
}
static void tc358775_power_down(struct tc_data *tc)
@@ -265,17 +263,14 @@ static void tc358775_power_down(struct tc_data *tc)
usleep_range(10, 20);
gpiod_set_value(tc->stby_gpio, 1);
- usleep_range(10000, 11000);
ret = regulator_disable(tc->vdd);
if (ret < 0)
dev_err(dev, "regulator vdd disable failed, %d\n", ret);
- usleep_range(10000, 11000);
ret = regulator_disable(tc->vddio);
if (ret < 0)
dev_err(dev, "regulator vddio disable failed, %d\n", ret);
- usleep_range(10000, 11000);
}
static void tc_bridge_pre_enable(struct drm_bridge *bridge)
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 19/20] drm/bridge: tc358775: fix power-up sequencing
2024-05-06 13:34 [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
` (17 preceding siblings ...)
2024-05-06 13:34 ` [PATCH 18/20] drm/bridge: tc358775: fix the power-up/down delays Michael Walle
@ 2024-05-06 13:34 ` Michael Walle
2024-05-06 13:34 ` [PATCH 20/20] drm/bridge: tc358775: use devm_drm_bridge_add() Michael Walle
2024-06-03 11:47 ` [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
20 siblings, 0 replies; 25+ messages in thread
From: Michael Walle @ 2024-05-06 13:34 UTC (permalink / raw
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren
Cc: Daniel Semkowicz, dri-devel, linux-kernel, linux-mediatek,
linux-arm-kernel, Michael Walle
The reset line of this bridge must be released while the DSI data lanes
and DSI clock lane are in LP-11 mode. After that the DSI clock has to be
turned on, which is a requirement to have I2C work.
To achieve this, use the new .lp11_notify() callback where the reset
line is released. Set .pre_enable_prev_first to make sure, there is a
valid DSI clock during the .pre_enabe() op. In .pre_enable() the bridge
will be fully configured but the LVDS transmitter will remain disabled.
It will eventually be enabled in the .enable() op.
With the correct initialization sequence we don't need the additional
reset, nor the additional write to VFUEN. With that fixed, the init
sequence is exactly how the vendor is requiring it.
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
drivers/gpu/drm/bridge/tc358775.c | 62 +++++++++++++++++++++++----------------
1 file changed, 37 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
index 99dbbb1fee78..31f89b7d5a3a 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -239,6 +239,9 @@ static void tc358775_power_up(struct tc_data *tc)
struct device *dev = &tc->dsi->dev;
int ret;
+ if (tc->powered)
+ return;
+
ret = regulator_enable(tc->vddio);
if (ret < 0)
dev_err(dev, "regulator vddio enable failed, %d\n", ret);
@@ -252,6 +255,8 @@ static void tc358775_power_up(struct tc_data *tc)
gpiod_set_value(tc->reset_gpio, 0);
usleep_range(200, 250);
+
+ tc->powered = true;
}
static void tc358775_power_down(struct tc_data *tc)
@@ -271,20 +276,8 @@ static void tc358775_power_down(struct tc_data *tc)
ret = regulator_disable(tc->vddio);
if (ret < 0)
dev_err(dev, "regulator vddio disable failed, %d\n", ret);
-}
-static void tc_bridge_pre_enable(struct drm_bridge *bridge)
-{
- struct tc_data *tc = bridge_to_tc(bridge);
-
- tc358775_power_up(tc);
-}
-
-static void tc_bridge_post_disable(struct drm_bridge *bridge)
-{
- struct tc_data *tc = bridge_to_tc(bridge);
-
- tc358775_power_down(tc);
+ tc->powered = false;
}
/* helper function to access bus_formats */
@@ -474,12 +467,25 @@ static void tc358775_configure_lvds_clock(struct tc_data *tc)
regmap_write(tc->regmap, LVCFG, val);
}
-static void tc358775_bridge_enable(struct drm_bridge *bridge)
+static void tc358775_dsi_lp11_notify(struct drm_bridge *bridge)
{
struct tc_data *tc = bridge_to_tc(bridge);
- unsigned int val = 0;
- struct drm_display_mode *mode;
+
+ tc358775_power_up(tc);
+}
+
+static void tc358775_bridge_pre_enable(struct drm_bridge *bridge)
+{
struct drm_connector *connector = get_connector(bridge->encoder);
+ struct tc_data *tc = bridge_to_tc(bridge);
+ struct drm_display_mode *mode;
+ unsigned int val = 0;
+
+ /*
+ * Legacy behavior, make sure this bridge is powered even if
+ * drm_bridge_dsi_lp11_notify() isn't called by the DSI host
+ */
+ tc358775_power_up(tc);
mode = &bridge->encoder->crtc->state->adjusted_mode;
@@ -488,22 +494,27 @@ static void tc358775_bridge_enable(struct drm_bridge *bridge)
dev_info(tc->dev, "DSI2LVDS Chip ID.%02x Revision ID. %02x **\n",
(val >> 8) & 0xFF, val & 0xFF);
- regmap_write(tc->regmap, SYSRST,
- SYS_RST_REG | SYS_RST_DSIRX | SYS_RST_BM | SYS_RST_LCD |
- SYS_RST_I2CM);
- usleep_range(30000, 40000);
-
tc358775_configure_dsi(tc, mode->crtc_clock);
tc358775_configure_lvds_timings(tc, mode);
tc358775_configure_pll(tc, mode->crtc_clock);
tc358775_configure_color_mapping(tc, connector->display_info.bus_formats[0]);
- regmap_write(tc->regmap, VFUEN, VFUEN_VFUEN);
tc358775_configure_lvds_clock(tc);
+}
+
+static void tc358775_bridge_enable(struct drm_bridge *bridge)
+{
+ struct tc_data *tc = bridge_to_tc(bridge);
- /* Finally, enable the LVDS transmitter */
regmap_update_bits(tc->regmap, LVCFG, LVCFG_LVEN, LVCFG_LVEN);
}
+static void tc358775_bridge_post_disable(struct drm_bridge *bridge)
+{
+ struct tc_data *tc = bridge_to_tc(bridge);
+
+ tc358775_power_down(tc);
+}
+
/*
* According to the datasheet, the horizontal back porch, front porch and sync
* length must be a multiple of 2 and the minimal horizontal pulse width is 8.
@@ -634,11 +645,12 @@ static int tc_bridge_attach(struct drm_bridge *bridge,
static const struct drm_bridge_funcs tc_bridge_funcs = {
.attach = tc_bridge_attach,
- .pre_enable = tc_bridge_pre_enable,
+ .dsi_lp11_notify = tc358775_dsi_lp11_notify,
+ .pre_enable = tc358775_bridge_pre_enable,
.enable = tc358775_bridge_enable,
+ .post_disable = tc358775_bridge_post_disable,
.mode_fixup = tc_mode_fixup,
.mode_valid = tc_mode_valid,
- .post_disable = tc_bridge_post_disable,
};
static int tc_attach_host(struct tc_data *tc)
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 20/20] drm/bridge: tc358775: use devm_drm_bridge_add()
2024-05-06 13:34 [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
` (18 preceding siblings ...)
2024-05-06 13:34 ` [PATCH 19/20] drm/bridge: tc358775: fix power-up sequencing Michael Walle
@ 2024-05-06 13:34 ` Michael Walle
2024-06-03 11:47 ` [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
20 siblings, 0 replies; 25+ messages in thread
From: Michael Walle @ 2024-05-06 13:34 UTC (permalink / raw
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren
Cc: Daniel Semkowicz, dri-devel, linux-kernel, linux-mediatek,
linux-arm-kernel, Michael Walle
Use the device resource managed version of drm_bridge_add(). This
simplifies the error handling and we can get rid of tc_remove_bridge().
Also, add a check for the return code.
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
drivers/gpu/drm/bridge/tc358775.c | 21 ++++-----------------
1 file changed, 4 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
index 31f89b7d5a3a..1d2547e4c4e3 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -762,26 +762,14 @@ static int tc_probe(struct i2c_client *client)
tc->bridge.funcs = &tc_bridge_funcs;
tc->bridge.of_node = dev->of_node;
tc->bridge.pre_enable_prev_first = true;
- drm_bridge_add(&tc->bridge);
- i2c_set_clientdata(client, tc);
-
- ret = tc_attach_host(tc);
+ ret = devm_drm_bridge_add(tc->dev, &tc->bridge);
if (ret)
- goto err_bridge_remove;
-
- return 0;
-
-err_bridge_remove:
- drm_bridge_remove(&tc->bridge);
- return ret;
-}
+ return ret;
-static void tc_remove(struct i2c_client *client)
-{
- struct tc_data *tc = i2c_get_clientdata(client);
+ i2c_set_clientdata(client, tc);
- drm_bridge_remove(&tc->bridge);
+ return tc_attach_host(tc);
}
static const struct i2c_device_id tc358775_i2c_ids[] = {
@@ -805,7 +793,6 @@ static struct i2c_driver tc358775_driver = {
},
.id_table = tc358775_i2c_ids,
.probe = tc_probe,
- .remove = tc_remove,
};
module_i2c_driver(tc358775_driver);
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 01/20] drm/bridge: add dsi_lp11_notify mechanism
2024-05-06 13:34 ` [PATCH 01/20] drm/bridge: add dsi_lp11_notify mechanism Michael Walle
@ 2024-05-07 8:37 ` Alexander Stein
2024-05-07 13:39 ` Dmitry Baryshkov
0 siblings, 1 reply; 25+ messages in thread
From: Alexander Stein @ 2024-05-07 8:37 UTC (permalink / raw
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren, dri-devel,
Michael Walle
Cc: Daniel Semkowicz, dri-devel, linux-kernel, linux-mediatek,
linux-arm-kernel, Michael Walle
Hi Michael,
Am Montag, 6. Mai 2024, 15:34:30 CEST schrieb Michael Walle:
> Some bridges have very strict power-up reqirements. In this case, the
> Toshiba TC358775. The reset has to be deasserted while *both* the DSI
> clock and DSI data lanes are in LP-11 mode. After the reset is relased,
> the bridge needs the DSI clock to actually be able to process I2C
> access. This access will configure the DSI side of the bridge during
> which the DSI data lanes have to be in LP-11 mode.
Apparently this is an issue for a lot of DSI bridges. But enabling LP-11 for
a bridge is impossible with current documentation [1], which states "A DSI
host should keep the PHY powered down until the pre_enable operation is
called."
Additionally tc358767/tc9595 (DSI-DP bridge) needs LP-11 for AUX channel
access to even get EDID. This is a requirement before pre_enable would
even be possible.
So some changes to the current flow are needed. But I am not so sure
about LP-11 notification. IMHO a device request to the DSI host to
enable LP-11 seems more sensible.
Best regards,
Alexander
[1] https://www.kernel.org/doc/html/latest/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation
> After everything is
> configured the video stream can finally be enabled.
>
> This means:
> (1) The bridge has to be configured completely in .pre_enable() op
> (with the clock turned on and data lanes in LP-11 mode, thus
> .pre_enable_prev_first has to be set).
> (2) The bridge will enable its output in the .enable() op
> (3) There must be some mechanism before (1) where the bridge can
> release its reset while the clock lane is still in LP-11 mode.
>
> Unfortunately, (3) is crucial for a correct operation of the bridge.
> To satisfy this requriment, introduce a new callback .dsi_lp11_notify()
> which will be called by the DSI host driver.
>
> Signed-off-by: Michael Walle <mwalle@kernel.org>
> ---
> drivers/gpu/drm/drm_bridge.c | 16 ++++++++++++++++
> include/drm/drm_bridge.h | 12 ++++++++++++
> 2 files changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 28abe9aa99ca..98cd6558aecb 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -1339,6 +1339,22 @@ void drm_bridge_hpd_notify(struct drm_bridge *bridge,
> }
> EXPORT_SYMBOL_GPL(drm_bridge_hpd_notify);
>
> +/**
> + * drm_bridge_dsi_lp11_notify - notify clock/data lanes LP-11 mode
> + * @bridge: bridge control structure
> + *
> + * DSI host drivers shall call this function while the clock and data lanes
> + * are still in LP-11 mode.
> + *
> + * This function shall be called in a context that can sleep.
> + */
> +void drm_bridge_dsi_lp11_notify(struct drm_bridge *bridge)
> +{
> + if (bridge->funcs->dsi_lp11_notify)
> + bridge->funcs->dsi_lp11_notify(bridge);
> +}
> +EXPORT_SYMBOL_GPL(drm_bridge_dsi_lp11_notify);
> +
> #ifdef CONFIG_OF
> /**
> * of_drm_find_bridge - find the bridge corresponding to the device node in
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 4baca0d9107b..4ef61274e0a8 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -630,6 +630,17 @@ struct drm_bridge_funcs {
> */
> void (*hpd_disable)(struct drm_bridge *bridge);
>
> + /**
> + * dsi_lp11_notify:
> + *
> + * Will be called by the DSI host driver while both the DSI clock
> + * lane as well as the DSI data lanes are in LP-11 mode. Some bridges
> + * need this state while releasing the reset, for example.
> + * Not all DSI host drivers will support this. Therefore, the DSI
> + * bridge driver must not rely on this op to be called.
> + */
> + void (*dsi_lp11_notify)(struct drm_bridge *bridge);
> +
> /**
> * @debugfs_init:
> *
> @@ -898,6 +909,7 @@ void drm_bridge_hpd_enable(struct drm_bridge *bridge,
> void drm_bridge_hpd_disable(struct drm_bridge *bridge);
> void drm_bridge_hpd_notify(struct drm_bridge *bridge,
> enum drm_connector_status status);
> +void drm_bridge_dsi_lp11_notify(struct drm_bridge *bridge);
>
> #ifdef CONFIG_DRM_PANEL_BRIDGE
> bool drm_bridge_is_panel(const struct drm_bridge *bridge);
>
>
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 01/20] drm/bridge: add dsi_lp11_notify mechanism
2024-05-07 8:37 ` Alexander Stein
@ 2024-05-07 13:39 ` Dmitry Baryshkov
2024-06-03 13:39 ` Michael Walle
0 siblings, 1 reply; 25+ messages in thread
From: Dmitry Baryshkov @ 2024-05-07 13:39 UTC (permalink / raw
To: Alexander Stein
Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren, dri-devel,
Michael Walle, Daniel Semkowicz, linux-kernel, linux-mediatek,
linux-arm-kernel
On Tue, May 07, 2024 at 10:37:54AM +0200, Alexander Stein wrote:
> Hi Michael,
>
> Am Montag, 6. Mai 2024, 15:34:30 CEST schrieb Michael Walle:
> > Some bridges have very strict power-up reqirements. In this case, the
> > Toshiba TC358775. The reset has to be deasserted while *both* the DSI
> > clock and DSI data lanes are in LP-11 mode. After the reset is relased,
> > the bridge needs the DSI clock to actually be able to process I2C
> > access. This access will configure the DSI side of the bridge during
> > which the DSI data lanes have to be in LP-11 mode.
>
> Apparently this is an issue for a lot of DSI bridges. But enabling LP-11 for
> a bridge is impossible with current documentation [1], which states "A DSI
> host should keep the PHY powered down until the pre_enable operation is
> called."
> Additionally tc358767/tc9595 (DSI-DP bridge) needs LP-11 for AUX channel
> access to even get EDID. This is a requirement before pre_enable would
> even be possible.
>
> So some changes to the current flow are needed. But I am not so sure
> about LP-11 notification. IMHO a device request to the DSI host to
> enable LP-11 seems more sensible.
Granted that there can be several DSI devices sharing the DSI bus (aka
split-link), I was toying with the idea of making the DSI host call
attached DSI devices when the transition happens.
I don't have a fully working PoC and I probably won't have it ready til
the end of May because of the lack of time and different local
priorities.
>
> Best regards,
> Alexander
>
> [1] https://www.kernel.org/doc/html/latest/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation
>
> > After everything is
> > configured the video stream can finally be enabled.
> >
> > This means:
> > (1) The bridge has to be configured completely in .pre_enable() op
> > (with the clock turned on and data lanes in LP-11 mode, thus
> > .pre_enable_prev_first has to be set).
> > (2) The bridge will enable its output in the .enable() op
> > (3) There must be some mechanism before (1) where the bridge can
> > release its reset while the clock lane is still in LP-11 mode.
> >
> > Unfortunately, (3) is crucial for a correct operation of the bridge.
> > To satisfy this requriment, introduce a new callback .dsi_lp11_notify()
> > which will be called by the DSI host driver.
> >
> > Signed-off-by: Michael Walle <mwalle@kernel.org>
> > ---
> > drivers/gpu/drm/drm_bridge.c | 16 ++++++++++++++++
> > include/drm/drm_bridge.h | 12 ++++++++++++
> > 2 files changed, 28 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> > index 28abe9aa99ca..98cd6558aecb 100644
> > --- a/drivers/gpu/drm/drm_bridge.c
> > +++ b/drivers/gpu/drm/drm_bridge.c
> > @@ -1339,6 +1339,22 @@ void drm_bridge_hpd_notify(struct drm_bridge *bridge,
> > }
> > EXPORT_SYMBOL_GPL(drm_bridge_hpd_notify);
> >
> > +/**
> > + * drm_bridge_dsi_lp11_notify - notify clock/data lanes LP-11 mode
> > + * @bridge: bridge control structure
> > + *
> > + * DSI host drivers shall call this function while the clock and data lanes
> > + * are still in LP-11 mode.
> > + *
> > + * This function shall be called in a context that can sleep.
> > + */
> > +void drm_bridge_dsi_lp11_notify(struct drm_bridge *bridge)
> > +{
> > + if (bridge->funcs->dsi_lp11_notify)
> > + bridge->funcs->dsi_lp11_notify(bridge);
> > +}
> > +EXPORT_SYMBOL_GPL(drm_bridge_dsi_lp11_notify);
> > +
> > #ifdef CONFIG_OF
> > /**
> > * of_drm_find_bridge - find the bridge corresponding to the device node in
> > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> > index 4baca0d9107b..4ef61274e0a8 100644
> > --- a/include/drm/drm_bridge.h
> > +++ b/include/drm/drm_bridge.h
> > @@ -630,6 +630,17 @@ struct drm_bridge_funcs {
> > */
> > void (*hpd_disable)(struct drm_bridge *bridge);
> >
> > + /**
> > + * dsi_lp11_notify:
> > + *
> > + * Will be called by the DSI host driver while both the DSI clock
> > + * lane as well as the DSI data lanes are in LP-11 mode. Some bridges
> > + * need this state while releasing the reset, for example.
> > + * Not all DSI host drivers will support this. Therefore, the DSI
> > + * bridge driver must not rely on this op to be called.
> > + */
> > + void (*dsi_lp11_notify)(struct drm_bridge *bridge);
> > +
> > /**
> > * @debugfs_init:
> > *
> > @@ -898,6 +909,7 @@ void drm_bridge_hpd_enable(struct drm_bridge *bridge,
> > void drm_bridge_hpd_disable(struct drm_bridge *bridge);
> > void drm_bridge_hpd_notify(struct drm_bridge *bridge,
> > enum drm_connector_status status);
> > +void drm_bridge_dsi_lp11_notify(struct drm_bridge *bridge);
> >
> > #ifdef CONFIG_DRM_PANEL_BRIDGE
> > bool drm_bridge_is_panel(const struct drm_bridge *bridge);
> >
> >
>
>
> --
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> http://www.tq-group.com/
>
>
--
With best wishes
Dmitry
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup
2024-05-06 13:34 [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
` (19 preceding siblings ...)
2024-05-06 13:34 ` [PATCH 20/20] drm/bridge: tc358775: use devm_drm_bridge_add() Michael Walle
@ 2024-06-03 11:47 ` Michael Walle
20 siblings, 0 replies; 25+ messages in thread
From: Michael Walle @ 2024-06-03 11:47 UTC (permalink / raw
To: Michael Walle, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Daniel Vetter, Chun-Kuang Hu, Philipp Zabel, Matthias Brugger,
AngeloGioacchino Del Regno, Sam Ravnborg, Vinay Simha BN,
Tony Lindgren
Cc: Daniel Semkowicz, dri-devel, linux-kernel, linux-mediatek,
linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 1758 bytes --]
On Mon May 6, 2024 at 3:34 PM CEST, Michael Walle wrote:
> This patchset fixes the bridge initialization according to the
> datasheet. Not sure how that even worked before. Maybe because the
> initialization was done prior to linux (?).
>
> The bridge has some peculiarities:
> (1) The reset has to be deasserted in LP-11 mode
> (2) For I2C access the bridge needs the DSI clock
> (3) The bridge has to be configured while the video stream is
> disabled.
> (4) The bridge has limitations on the display timings. In particular,
> the horizontal pulse width has to be at least 8 pixels wide and
> both the horizontal pulse width as well as the back porch has to
> be even. According to the datasheet the horizontal front porch as
> well but in line sync mode, this is ignored. Also line sync is the
> only supported mode for this bridge, therefore, the front porch
> is always ignored.
>
> The most controversial patch is probably "drm/bridge: add
> dsi_lp11_notify mechanism" which is needed for (1) above. Some time ago
> there was a series [1] to add a manual power-up, which was abandoned and
> which didn't suite the needs for this bridge anyway.
>
> Also, this will gradually change the tc_ prefix to tc358775_ while the
> functions are refactored.
>
> The bridge was successfully tested on a Mediatek MT8195 SoC with the
> following panels:
> - Innolux G101ICE
> - AUO G121EAN01.0
> - Innolux G156HCE (dual-link LVDS)
>
> [1] https://lore.kernel.org/r/20231016165355.1327217-1-dmitry.baryshkov@linaro.org/
Any comments on this series, besides the discussion on how to do the
reset during LP11?
Most of the other patches should be more or less self contained.
-michael
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 01/20] drm/bridge: add dsi_lp11_notify mechanism
2024-05-07 13:39 ` Dmitry Baryshkov
@ 2024-06-03 13:39 ` Michael Walle
0 siblings, 0 replies; 25+ messages in thread
From: Michael Walle @ 2024-06-03 13:39 UTC (permalink / raw
To: Dmitry Baryshkov, Alexander Stein
Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Sam Ravnborg, Vinay Simha BN, Tony Lindgren, dri-devel,
Daniel Semkowicz, linux-kernel, linux-mediatek, linux-arm-kernel,
Marek Vasut
[-- Attachment #1.1: Type: text/plain, Size: 5334 bytes --]
[+ Marek ]
Hi Dmitry,
> > > Some bridges have very strict power-up reqirements. In this case, the
> > > Toshiba TC358775. The reset has to be deasserted while *both* the DSI
> > > clock and DSI data lanes are in LP-11 mode. After the reset is relased,
> > > the bridge needs the DSI clock to actually be able to process I2C
> > > access. This access will configure the DSI side of the bridge during
> > > which the DSI data lanes have to be in LP-11 mode.
> >
> > Apparently this is an issue for a lot of DSI bridges. But enabling LP-11 for
> > a bridge is impossible with current documentation [1], which states "A DSI
> > host should keep the PHY powered down until the pre_enable operation is
> > called."
> > Additionally tc358767/tc9595 (DSI-DP bridge) needs LP-11 for AUX channel
> > access to even get EDID. This is a requirement before pre_enable would
> > even be possible.
> >
> > So some changes to the current flow are needed. But I am not so sure
> > about LP-11 notification. IMHO a device request to the DSI host to
> > enable LP-11 seems more sensible.
>
> Granted that there can be several DSI devices sharing the DSI bus (aka
> split-link), I was toying with the idea of making the DSI host call
> attached DSI devices when the transition happens.
So almost the same, as this patch?
> I don't have a fully working PoC and I probably won't have it ready til
> the end of May because of the lack of time and different local
> priorities.
Any news regarding this?
-michael
> > Best regards,
> > Alexander
> >
> > [1] https://www.kernel.org/doc/html/latest/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation
> >
> > > After everything is
> > > configured the video stream can finally be enabled.
> > >
> > > This means:
> > > (1) The bridge has to be configured completely in .pre_enable() op
> > > (with the clock turned on and data lanes in LP-11 mode, thus
> > > .pre_enable_prev_first has to be set).
> > > (2) The bridge will enable its output in the .enable() op
> > > (3) There must be some mechanism before (1) where the bridge can
> > > release its reset while the clock lane is still in LP-11 mode.
> > >
> > > Unfortunately, (3) is crucial for a correct operation of the bridge.
> > > To satisfy this requriment, introduce a new callback .dsi_lp11_notify()
> > > which will be called by the DSI host driver.
> > >
> > > Signed-off-by: Michael Walle <mwalle@kernel.org>
> > > ---
> > > drivers/gpu/drm/drm_bridge.c | 16 ++++++++++++++++
> > > include/drm/drm_bridge.h | 12 ++++++++++++
> > > 2 files changed, 28 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> > > index 28abe9aa99ca..98cd6558aecb 100644
> > > --- a/drivers/gpu/drm/drm_bridge.c
> > > +++ b/drivers/gpu/drm/drm_bridge.c
> > > @@ -1339,6 +1339,22 @@ void drm_bridge_hpd_notify(struct drm_bridge *bridge,
> > > }
> > > EXPORT_SYMBOL_GPL(drm_bridge_hpd_notify);
> > >
> > > +/**
> > > + * drm_bridge_dsi_lp11_notify - notify clock/data lanes LP-11 mode
> > > + * @bridge: bridge control structure
> > > + *
> > > + * DSI host drivers shall call this function while the clock and data lanes
> > > + * are still in LP-11 mode.
> > > + *
> > > + * This function shall be called in a context that can sleep.
> > > + */
> > > +void drm_bridge_dsi_lp11_notify(struct drm_bridge *bridge)
> > > +{
> > > + if (bridge->funcs->dsi_lp11_notify)
> > > + bridge->funcs->dsi_lp11_notify(bridge);
> > > +}
> > > +EXPORT_SYMBOL_GPL(drm_bridge_dsi_lp11_notify);
> > > +
> > > #ifdef CONFIG_OF
> > > /**
> > > * of_drm_find_bridge - find the bridge corresponding to the device node in
> > > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> > > index 4baca0d9107b..4ef61274e0a8 100644
> > > --- a/include/drm/drm_bridge.h
> > > +++ b/include/drm/drm_bridge.h
> > > @@ -630,6 +630,17 @@ struct drm_bridge_funcs {
> > > */
> > > void (*hpd_disable)(struct drm_bridge *bridge);
> > >
> > > + /**
> > > + * dsi_lp11_notify:
> > > + *
> > > + * Will be called by the DSI host driver while both the DSI clock
> > > + * lane as well as the DSI data lanes are in LP-11 mode. Some bridges
> > > + * need this state while releasing the reset, for example.
> > > + * Not all DSI host drivers will support this. Therefore, the DSI
> > > + * bridge driver must not rely on this op to be called.
> > > + */
> > > + void (*dsi_lp11_notify)(struct drm_bridge *bridge);
> > > +
> > > /**
> > > * @debugfs_init:
> > > *
> > > @@ -898,6 +909,7 @@ void drm_bridge_hpd_enable(struct drm_bridge *bridge,
> > > void drm_bridge_hpd_disable(struct drm_bridge *bridge);
> > > void drm_bridge_hpd_notify(struct drm_bridge *bridge,
> > > enum drm_connector_status status);
> > > +void drm_bridge_dsi_lp11_notify(struct drm_bridge *bridge);
> > >
> > > #ifdef CONFIG_DRM_PANEL_BRIDGE
> > > bool drm_bridge_is_panel(const struct drm_bridge *bridge);
> > >
> > >
> >
> >
> > --
> > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> > Amtsgericht München, HRB 105018
> > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> > http://www.tq-group.com/
> >
> >
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2024-06-03 13:39 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-06 13:34 [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
2024-05-06 13:34 ` [PATCH 01/20] drm/bridge: add dsi_lp11_notify mechanism Michael Walle
2024-05-07 8:37 ` Alexander Stein
2024-05-07 13:39 ` Dmitry Baryshkov
2024-06-03 13:39 ` Michael Walle
2024-05-06 13:34 ` [PATCH 02/20] drm/mediatek: dsi: provide LP-11 mode during .pre_enable Michael Walle
2024-05-06 13:34 ` [PATCH 03/20] drm/mediatek: dsi: add support for .dsi_lp11_notity() Michael Walle
2024-05-06 13:34 ` [PATCH 04/20] drm/bridge: tc358775: fix regulator supply id Michael Walle
2024-05-06 13:34 ` [PATCH 05/20] drm/bridge: tc358775: add crtc modes fixup Michael Walle
2024-05-06 13:34 ` [PATCH 06/20] drm/bridge: tc358775: redefine LV_MX() Michael Walle
2024-05-06 13:34 ` [PATCH 07/20] drm/bridge: tc358775: use regmap instead of open coded access functions Michael Walle
2024-05-06 13:34 ` [PATCH 08/20] drm/bridge: tc358775: remove error message if regulator is missing Michael Walle
2024-05-06 13:34 ` [PATCH 09/20] drm/bridge: tc358775: remove complex vsdelay calculation Michael Walle
2024-05-06 13:34 ` [PATCH 10/20] drm/bridge: tc358775: simplify lvds_link property Michael Walle
2024-05-06 13:34 ` [PATCH 11/20] drm/bridge: tc358775: reformat weird indentation Michael Walle
2024-05-06 13:34 ` [PATCH 12/20] drm/bridge: tc358775: correctly configure LVDS clock Michael Walle
2024-05-06 13:34 ` [PATCH 13/20] drm/bridge: tc358775: split the init code Michael Walle
2024-05-06 13:34 ` [PATCH 14/20] drm/bridge: tc358775: configure PLL depending on the LVDS clock Michael Walle
2024-05-06 13:34 ` [PATCH 15/20] drm/bridge: tc358775: dynamically configure DSI link settings Michael Walle
2024-05-06 13:34 ` [PATCH 16/20] drm/bridge: tc358775: use proper defines to configure LVDS timings Michael Walle
2024-05-06 13:34 ` [PATCH 17/20] drm/bridge: tc358775: move bridge power up/down into functions Michael Walle
2024-05-06 13:34 ` [PATCH 18/20] drm/bridge: tc358775: fix the power-up/down delays Michael Walle
2024-05-06 13:34 ` [PATCH 19/20] drm/bridge: tc358775: fix power-up sequencing Michael Walle
2024-05-06 13:34 ` [PATCH 20/20] drm/bridge: tc358775: use devm_drm_bridge_add() Michael Walle
2024-06-03 11:47 ` [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup Michael Walle
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).